| Message ID | 20181101232522.702-1-honnappa.nagarahalli@arm.com |
|---|---|
| Headers | show |
| Series | hash: deprecate lock ellision and read/write concurreny flags | expand |
On Thu, Nov 01, 2018 at 06:25:18PM -0500, Honnappa Nagarahalli wrote: > Various configuration flags in rte_hash library result in increase of > number of test cases. Configuration flags for enabling transactional > memory use and read/write concurrency are not required. These features > should be supported by default. Please refer to [1] for more context. > > This patch marks these flags for deprecation in 19.02 release and cleans > up the test cases. > > [1] http://mails.dpdk.org/archives/dev/2018-October/117268.html > > Honnappa Nagarahalli (4): hash: prepare for deprecation of flags hash: > deprecate lock ellision and read/write concurreny flags test/hash: stop > using lock ellision and read/write concurreny flags doc/hash: deprecate > lock ellision and read/write concurreny flags > While I'd like to reduce the flags and do cleanup, I'm a little concerned about putting this scope of changes in so late in the release. I wonder if less drastic changes could work as well for this release, and do the cleanup later. For example, rather than deprecating the flags now, how about just change the default for when no flags are set? If user has set flags, follow the existing path - if flags is set to zero, then have the defaults be to use RW concurrency or TSX. /Bruce
> > On Thu, Nov 01, 2018 at 06:25:18PM -0500, Honnappa Nagarahalli wrote: > > Various configuration flags in rte_hash library result in increase of > > number of test cases. Configuration flags for enabling transactional > > memory use and read/write concurrency are not required. These features > > should be supported by default. Please refer to [1] for more context. > > > > This patch marks these flags for deprecation in 19.02 release and > > cleans up the test cases. > > > > [1] http://mails.dpdk.org/archives/dev/2018-October/117268.html > > > > Honnappa Nagarahalli (4): hash: prepare for deprecation of flags hash: > > deprecate lock ellision and read/write concurreny flags test/hash: > > stop using lock ellision and read/write concurreny flags doc/hash: > > deprecate lock ellision and read/write concurreny flags > > > While I'd like to reduce the flags and do cleanup, I'm a little concerned about > putting this scope of changes in so late in the release. I wonder if less drastic > changes could work as well for this release, and do the cleanup later. Thank you Bruce for the review. This patch series is not fixing any user related problems, let us skip this for 18.11. It will give us time as well to think through and get this right. > For example, rather than deprecating the flags now, how about just change > the default for when no flags are set? If user has set flags, follow the existing > path - if flags is set to zero, then have the defaults be to use RW concurrency > or TSX. This changes the behavior of the library and what the flags mean, still requires ABI change, but does not need deprecation of flags (I guess this is what you meant). However, it will not solve the problem of losing the capability to disable TSX. > > /Bruce
On 11/2/2018 5:38 PM, Honnappa Nagarahalli wrote: >> >> On Thu, Nov 01, 2018 at 06:25:18PM -0500, Honnappa Nagarahalli wrote: >>> Various configuration flags in rte_hash library result in increase of >>> number of test cases. Configuration flags for enabling transactional >>> memory use and read/write concurrency are not required. These features >>> should be supported by default. Please refer to [1] for more context. >>> >>> This patch marks these flags for deprecation in 19.02 release and >>> cleans up the test cases. >>> >>> [1] http://mails.dpdk.org/archives/dev/2018-October/117268.html >>> >>> Honnappa Nagarahalli (4): hash: prepare for deprecation of flags hash: >>> deprecate lock ellision and read/write concurreny flags test/hash: >>> stop using lock ellision and read/write concurreny flags doc/hash: >>> deprecate lock ellision and read/write concurreny flags >>> >> While I'd like to reduce the flags and do cleanup, I'm a little concerned about >> putting this scope of changes in so late in the release. I wonder if less drastic >> changes could work as well for this release, and do the cleanup later. > Thank you Bruce for the review. This patch series is not fixing any user related problems, let us skip this for 18.11. It will give us time as well to think through and get this right. There was no update in the scope of 19.02, I guess it will skip for 19.02 too. Patchset updated as "Change requested" in patchwork. > >> For example, rather than deprecating the flags now, how about just change >> the default for when no flags are set? If user has set flags, follow the existing >> path - if flags is set to zero, then have the defaults be to use RW concurrency >> or TSX. > This changes the behavior of the library and what the flags mean, still requires ABI change, but does not need deprecation of flags (I guess this is what you meant). However, it will not solve the problem of losing the capability to disable TSX. > >> >> /Bruce