linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tom Herbert <tom@quantonium.net>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: NeilBrown <neilb@suse.com>, Thomas Graf <tgraf@suug.ch>,
	Linux Kernel Network Developers <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 7/8] rhashtable: add rhashtable_walk_prev()
Date: Sat, 5 May 2018 08:40:35 -0700	[thread overview]
Message-ID: <CAPDqMeoPjypXCioHwy9CPiT_XZfWZk8_A6cErQuxVMAJqkQEqQ@mail.gmail.com> (raw)
In-Reply-To: <20180505094324.wwjtl76oofgrtpg4@gondor.apana.org.au>

On Sat, May 5, 2018 at 2:43 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Fri, May 04, 2018 at 01:54:14PM +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>
>
> I will ack this if Tom is OK with replacing peek with it.
>
I'm not following why this is any better than peek. The point of
having rhashtable_walk_peek is to to allow the caller to get then
current element not the next one. This is needed when table is read in
multiple parts and we need to pick up with what was returned from the
last call to rhashtable_walk_next (which apparently is what this patch
is also trying to do).

There is one significant difference in that peek will return the
element in the table regardless of where the iterator is at (this is
why peek can move the iterator) and only returns NULL at end of table.
As mentioned above rhashtable_walk_prev can return NULL so then caller
needs and so rhashtable_walk_next "should be used" to get the previous
element. Doing a peek is a lot cleaner and more straightforward API in
this regard.

Tom

> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

  reply	other threads:[~2018-05-05 15:40 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-04  3:54 [PATCH 0/8] Assorted rhashtable fixes and cleanups NeilBrown
2018-05-04  3:54 ` [PATCH 8/8] rhashtable: don't hold lock on first table throughout insertion NeilBrown
2018-05-05  9:41   ` Herbert Xu
2018-05-05 22:00     ` NeilBrown
2018-05-06  5:20       ` Herbert Xu
2018-05-06 22:24         ` NeilBrown
2018-05-07  9:29           ` Herbert Xu
2018-05-08  0:23             ` NeilBrown
2018-05-04  3:54 ` [PATCH 6/8] rhashtable: further improve stability of rhashtable_walk NeilBrown
2018-05-05  9:42   ` Herbert Xu
2018-05-05 21:50     ` NeilBrown
2018-05-07  9:30       ` Herbert Xu
2018-05-08  0:54         ` NeilBrown
2018-05-04  3:54 ` [PATCH 1/8] rhashtable: silence RCU warning in rhashtable_test NeilBrown
2018-05-05  9:10   ` Herbert Xu
2018-05-05 21:49     ` NeilBrown
2018-05-04  3:54 ` [PATCH 2/8] rhashtable: remove nulls_base and related code NeilBrown
2018-05-05  9:12   ` Herbert Xu
2018-05-05 21:37     ` NeilBrown
2018-05-07  9:27       ` Herbert Xu
2018-05-08  1:14         ` NeilBrown
2018-05-04  3:54 ` [PATCH 5/8] rhashtable: remove rhashtable_walk_peek() NeilBrown
2018-05-05  9:30   ` Herbert Xu
2018-05-04  3:54 ` [PATCH 7/8] rhashtable: add rhashtable_walk_prev() NeilBrown
2018-05-05  9:43   ` Herbert Xu
2018-05-05 15:40     ` Tom Herbert [this message]
2018-05-06 22:16       ` NeilBrown
2018-05-04  3:54 ` [PATCH 4/8] rhashtable: fix race in nested_table_alloc() NeilBrown
2018-05-05  9:29   ` Herbert Xu
2018-05-05 21:48     ` NeilBrown
2018-05-06  5:18       ` Herbert Xu
2018-05-06 22:02         ` NeilBrown
2018-05-04  3:54 ` [PATCH 3/8] rhashtable: use cmpxchg() to protect ->future_tbl NeilBrown
2018-05-05  9:27   ` Herbert Xu
2018-05-05 21:45     ` NeilBrown
2018-05-04 17:07 ` [PATCH 0/8] Assorted rhashtable fixes and cleanups David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPDqMeoPjypXCioHwy9CPiT_XZfWZk8_A6cErQuxVMAJqkQEqQ@mail.gmail.com \
    --to=tom@quantonium.net \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.com \
    --cc=netdev@vger.kernel.org \
    --cc=tgraf@suug.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).