Thread (13 messages) 13 messages, 3 authors, 2017-12-02

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,
Tom
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help