| Message ID | 152403402206.16895.14563720960374849428.stgit2@noble |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | Assorted rhashtable improvements. RESEND | expand |
On Wed, Apr 18, 2018 at 04:47:02PM +1000, NeilBrown wrote: > rhashtable_walk_prev() returns the object returned by > the previous rhashtable_walk_next(), providing it is still in the > table (or was during this grace period). > This works even if rhashtable_walk_stop() and rhashtable_talk_start() > have been called since the last rhashtable_walk_next(). > > If there have been no calls to rhashtable_walk_next(), or if the > object is gone from the table, then NULL is returned. > > This can usefully be used in a seq_file ->start() function. > If the pos is the same as was returned by the last ->next() call, > then rhashtable_walk_prev() can be used to re-establish the > current location in the table. If it returns NULL, then > rhashtable_walk_next() should be used. > > Signed-off-by: NeilBrown <neilb@suse.com> Can you explain the need for this function and its difference from the existing rhashtable_walk_peek? Thanks,
On Wed, Apr 18 2018, Herbert Xu wrote: > On Wed, Apr 18, 2018 at 04:47:02PM +1000, NeilBrown wrote: >> rhashtable_walk_prev() returns the object returned by >> the previous rhashtable_walk_next(), providing it is still in the >> table (or was during this grace period). >> This works even if rhashtable_walk_stop() and rhashtable_talk_start() >> have been called since the last rhashtable_walk_next(). >> >> If there have been no calls to rhashtable_walk_next(), or if the >> object is gone from the table, then NULL is returned. >> >> This can usefully be used in a seq_file ->start() function. >> If the pos is the same as was returned by the last ->next() call, >> then rhashtable_walk_prev() can be used to re-establish the >> current location in the table. If it returns NULL, then >> rhashtable_walk_next() should be used. >> >> Signed-off-by: NeilBrown <neilb@suse.com> > > Can you explain the need for this function and its difference > from the existing rhashtable_walk_peek? The need is essentially the same as the need for rhashtable_walk_peek(). The difference is that the actual behaviour can be documented briefly (and so understood easily) without enumerating multiple special cases. rhashtable_walk_peek() is much the same as rhashtable_walk_prev() ?: rhashtable_walk_next() The need arises when you need to "stop" a walk and then restart at the same object, not the next one. i.e. the last object returned before the "stop" wasn't acted on. This happens with seq_file if the buffer space for ->show() is not sufficient to format an object. In the case, seq_file will stop() the iteration, make more space available (either by flushing or by reallocing) and will start again at the same location. If the seq_file client stored the rhashtable_iter in the seq_file private data, it can use rhasthable_walk_prev() to recover its position if that object is still in the hash table. If it isn't still present, rhashtable_walk_next() can be used to get the next one. In some cases it can be useful for the client to know whether it got the previous one or not. Thanks, NeilBrown
On Thu, Apr 19, 2018 at 09:08:07AM +1000, NeilBrown wrote: > > The need is essentially the same as the need for > rhashtable_walk_peek(). The difference is that the actual behaviour can > be documented briefly (and so understood easily) without enumerating > multiple special cases. > rhashtable_walk_peek() is much the same as > rhashtable_walk_prev() ?: rhashtable_walk_next() > > The need arises when you need to "stop" a walk and then restart at the > same object, not the next one. i.e. the last object returned before the > "stop" wasn't acted on. > This happens with seq_file if the buffer space for ->show() is not > sufficient to format an object. In the case, seq_file will stop() the > iteration, make more space available (either by flushing or by > reallocing) and will start again at the same location. > If the seq_file client stored the rhashtable_iter in the seq_file > private data, it can use rhasthable_walk_prev() to recover its position > if that object is still in the hash table. If it isn't still present, > rhashtable_walk_next() can be used to get the next one. In some cases > it can be useful for the client to know whether it got the previous one > or not. I see. I'm OK with this provided that you will also remove/convert users of rhashtable_walk_peek. I don't think we need two functions that do almost the same thing. Thanks,
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h index 5ce6201f246e..b1ad2b6a3f3f 100644 --- a/include/linux/rhashtable.h +++ b/include/linux/rhashtable.h @@ -397,6 +397,7 @@ static inline void rhashtable_walk_start(struct rhashtable_iter *iter) void *rhashtable_walk_next(struct rhashtable_iter *iter); void *rhashtable_walk_peek(struct rhashtable_iter *iter); +void *rhashtable_walk_prev(struct rhashtable_iter *iter); void rhashtable_walk_stop(struct rhashtable_iter *iter) __releases(RCU); void rhashtable_free_and_destroy(struct rhashtable *ht, diff --git a/lib/rhashtable.c b/lib/rhashtable.c index be7eb57d9398..d2f941146ea3 100644 --- a/lib/rhashtable.c +++ b/lib/rhashtable.c @@ -910,6 +910,36 @@ void *rhashtable_walk_next(struct rhashtable_iter *iter) } EXPORT_SYMBOL_GPL(rhashtable_walk_next); +/** + * rhashtable_walk_prev - Return the previously returned object, if available + * @iter: Hash table iterator + * + * If rhashtable_walk_next() has previously been called and the object + * it returned is still in the hash table, that object is returned again, + * otherwise %NULL is returned. + * + * If the recent rhashtable_walk_next() call was since the most recent + * rhashtable_walk_start() call then the returned object may not, strictly + * speaking, still be in the table. It will be safe to dereference. + * + * Note that the iterator is not changed and in particular it does not + * step backwards. + */ +void *rhashtable_walk_prev(struct rhashtable_iter *iter) +{ + struct rhashtable *ht = iter->ht; + struct rhash_head *p = iter->p; + + if (!p) + return NULL; + if (!iter->p_is_unsafe || ht->rhlist) + return p; + rht_for_each_rcu(p, iter->walker.tbl, iter->slot) + if (p == iter->p) + return p; + return NULL; +} + /** * rhashtable_walk_peek - Return the next object but don't advance the iterator * @iter: Hash table iterator
rhashtable_walk_prev() returns the object returned by the previous rhashtable_walk_next(), providing it is still in the table (or was during this grace period). This works even if rhashtable_walk_stop() and rhashtable_talk_start() have been called since the last rhashtable_walk_next(). If there have been no calls to rhashtable_walk_next(), or if the object is gone from the table, then NULL is returned. This can usefully be used in a seq_file ->start() function. If the pos is the same as was returned by the last ->next() call, then rhashtable_walk_prev() can be used to re-establish the current location in the table. If it returns NULL, then rhashtable_walk_next() should be used. Signed-off-by: NeilBrown <neilb@suse.com> --- include/linux/rhashtable.h | 1 + lib/rhashtable.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+)