Re: [PATCH net-next 1/5] rhashtable: Don't reset walker table in rhashtable_walk_start
From: Tom Herbert <hidden>
Date: 2017-12-02 01:07:45
On Fri, Dec 1, 2017 at 3:29 PM, Tom Herbert [off-list ref] wrote:
On Fri, Dec 1, 2017 at 2:18 PM, Herbert Xu [off-list ref] wrote:quoted
On Thu, Nov 30, 2017 at 04:03:01PM -0800, Tom Herbert wrote:quoted
Remove the code that resets the walker table. The walker table should only be initialized in the walk init function or when a future table is encountered. If the walker table is NULL this is the indication that the walk has completed and this information can be used to break a multi-call walk in the table (e.g. successive calls to nelink_dump that are dumping elements of an rhashtable). This also allows us to change rhashtable_walk_start to return void since the only error it was returning was -EAGAIN for a table change. This patch changes all the callers of rhashtable_walk_start to expect void which eliminates logic needed to check the return value for a rare condition. Note that -EAGAIN will be returned in a call to rhashtable_walk_next which seems to always follow the start of the walk so there should be no behavioral change in doing this. Signed-off-by: Tom Herbert <redacted>Doesn't this mean that if a walk encounters a rehash you may end up missing half or more of the hash table?Because of tbl->rehash < tbl->size conditions in walk stop? How about we add a flag to iter that indicates table needs a reset and set it along with setting walker.tbl to NULL? On the next walk start do the reload when walker.tbl is NULL and flag is set. In this case walk start would automatically set walker.tbl which is already done by nearly all callers already in that they ignore -EAGAIN returned from start walk.
Herbert, Looking at this some more, I am wondering if the walkers list is necessary. When a rehash table is done, the new table is assigned to ht->tbl and walker->tbl is cleared for all walkers. In walk start the walker tbl is checked and if it's NULL ht->tbl is loaded. Assuming that -EAGAIN isn't interesting to callers here, it seems like we could just get iter->walker.tbl in each call to walk start and not need to maintain the walkers list at all. Am I missing something? Tom
Thanks, Tomquoted
Cheers, -- Email: Herbert Xu [off-list ref] Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt