[PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[Florian Westphal]

Will be used by upcoming ipset rhashtable conversion.

"walk rht with unlink+free" triggers LLM reject pattern:
"possible softirq CPU stall".

"walk rht with unlink+free + cond_resched" triggers
"possibly skipped elements".

Add a helper to detach current hash backend storage from the
rhashtable, then iterate and flush all contained elements.

Cc: herbert@gondor.apana.org.au
Cc: linux-crypto@vger.kernel.org
Link: https://sashiko.dev/#/patchset/20260828152256.8759-1-fw%40strlen.de
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 Herbert: If you prefer to take this via the crypto tree, please
 let me know.
 Otherwise, an explicit Ack would be appreciated, so this can
 be handled via nf-next.  Thanks.

 net/ipv6/ila/ could be converted to use this helper too.

 include/linux/rhashtable.h |  19 ++++++
 lib/rhashtable.c           | 127 +++++++++++++++++++++++++++++++++++++
 2 files changed, 146 insertions(+)

diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 57a2a29bef0e..213e1cb77d45 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -255,6 +255,10 @@ void rhashtable_free_and_destroy(struct rhashtable *ht,
 				 void *arg);
 void rhashtable_destroy(struct rhashtable *ht);
 
+void rhashtable_flush_and_free(struct rhashtable *ht,
+			       void (*free_fn)(void *ptr, void *arg),
+			       void *arg);
+
 struct rhash_lock_head __rcu **rht_bucket_nested(
 	const struct bucket_table *tbl, unsigned int hash);
 struct rhash_lock_head __rcu **__rht_bucket_nested(
@@ -1335,4 +1339,19 @@ static inline void rhltable_destroy(struct rhltable *hlt)
 	rhltable_free_and_destroy(hlt, NULL, NULL);
 }
 
+/**
+ * rhltable_flush_and_free - unlink and free all elements in the hash list table
+ * @hlt:	the hash list table to destroy
+ * @free_fn:	callback to release resources of element
+ * @arg:	pointer passed to free_fn
+ *
+ * See documentation for rhashtable_flush_and_free.
+ */
+static inline void rhltable_flush_and_free(struct rhltable *hlt,
+					   void (*free_fn)(void *ptr,
+							   void *arg),
+					   void *arg)
+{
+	rhashtable_flush_and_free(&hlt->ht, free_fn, arg);
+}
 #endif /* _LINUX_RHASHTABLE_H */
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 6362896e4f09..656c5021d8b2 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -1339,6 +1339,133 @@ void rhashtable_destroy(struct rhashtable *ht)
 }
 EXPORT_SYMBOL_GPL(rhashtable_destroy);
 
+struct rht_flush_arg {
+	struct rhashtable *ht;
+	void (*free_fn)(void *ptr, void *arg);
+	void *arg;
+};
+
+static void flush_cb(void *ptr, void *arg)
+{
+	struct rht_flush_arg *fa = arg;
+
+	atomic_dec(&fa->ht->nelems);
+	if (fa->free_fn)
+		fa->free_fn(ptr, fa->arg);
+}
+
+static void rhashtable_flush_one(struct rhashtable *ht, struct rhash_head *obj,
+				 void (*free_fn)(void *ptr, void *arg),
+				 void *arg)
+{
+	struct rht_flush_arg fa = {
+		.ht = ht,
+		.free_fn = free_fn,
+		.arg = arg,
+	};
+
+	rhashtable_free_one(ht, obj, flush_cb, &fa);
+}
+
+static void rhashtable_flush_chain(struct rhashtable *ht,
+				   struct bucket_table *tbl,
+				   unsigned int hash,
+				   void (*free_fn)(void *ptr, void *arg),
+				   void *arg)
+{
+	struct rhash_lock_head __rcu **bkt = rht_bucket_var(tbl, hash);
+	struct rhash_head *pos, *next;
+	unsigned long flags;
+
+	if (!bkt)
+		return;
+
+	flags = rht_lock(tbl, bkt);
+	pos = rht_ptr(bkt, tbl, hash);
+	rht_assign_unlock(tbl, bkt, NULL, flags);
+
+	/* Nothing can reach @pos through @tbl any more: the bucket has
+	 * been emptied above, and @tbl itself is unreachable from ht->tbl
+	 * (see rhashtable_flush_and_free()). Walk it the same way
+	 * rhashtable_free_and_destroy() walks a table it exclusively
+	 * owns.
+	 */
+	while (!rht_is_a_nulls(pos)) {
+		next = rcu_dereference_raw(pos->next);
+		rhashtable_flush_one(ht, pos, free_fn, arg);
+		pos = next;
+	}
+}
+
+/**
+ * rhashtable_flush_and_free - detach and discard all current elements
+ * @ht:		the hash table to flush
+ * @free_fn:	callback to release resources of an element, may be %NULL
+ * @arg:	pointer passed to free_fn
+ *
+ * Swaps the bucket table backing @ht for a new, empty table.
+ *
+ * The detached table is then walked and every element found is
+ * unlinked, and, if @free_fn is given, handed to it for release.
+ * Note that RCU protected readers may still be accessing the elements.
+ * Releasing of resources must occur in a compatible manner.
+ *
+ * Unlike rhashtable_destroy(), @ht is left fully initialized and may
+ * continue to be used for lookups, insertions, and removals.
+ *
+ * This function may sleep, it cannot be called from atomic context or
+ * RCU read-side critical sections.
+ */
+void rhashtable_flush_and_free(struct rhashtable *ht,
+			       void (*free_fn)(void *ptr, void *arg),
+			       void *arg)
+{
+	struct bucket_table *tbl, *old_tbl, *last_tbl, *new_tbl;
+	struct rhashtable_walker *walker;
+	unsigned int i;
+
+	new_tbl = bucket_table_alloc(ht, rounded_hashtable_size(&ht->p),
+				     GFP_KERNEL);
+	if (!new_tbl)
+		new_tbl = bucket_table_alloc(ht, ht->p.min_size,
+					     GFP_KERNEL | __GFP_NOFAIL);
+
+	mutex_lock(&ht->mutex);
+
+	/* Splice the new, empty table onto the tail of the live table ... */
+	old_tbl = rht_dereference(ht->tbl, ht);
+	do {
+		last_tbl = rhashtable_last_table(ht, old_tbl);
+	} while (rhashtable_rehash_attach(ht, last_tbl, new_tbl));
+
+	/* ...then publish it as ht->tbl. */
+	rcu_assign_pointer(ht->tbl, new_tbl);
+	mutex_unlock(&ht->mutex);
+
+	tbl = old_tbl;
+	do {
+		struct bucket_table *next_tbl = rcu_dereference_raw(tbl->future_tbl);
+
+		for (i = 0; i < tbl->size; i++) {
+			cond_resched();
+			rhashtable_flush_chain(ht, tbl, i, free_fn, arg);
+		}
+
+		spin_lock(&ht->lock);
+		list_for_each_entry(walker, &tbl->walkers, list)
+			walker->tbl = NULL;
+		/* See rhashtable_rehash_table(): done under ->lock so
+		 * rhashtable_walk_stop() can use rcu_head_after_call_rcu()
+		 * to decide whether to re-link the walker onto this table.
+		 */
+		call_rcu(&tbl->rcu, bucket_table_free_rcu);
+		spin_unlock(&ht->lock);
+
+		tbl = next_tbl;
+	} while (tbl && tbl != new_tbl);
+}
+EXPORT_SYMBOL_GPL(rhashtable_flush_and_free);
+
 struct rhash_lock_head __rcu **__rht_bucket_nested(
 	const struct bucket_table *tbl, unsigned int hash)
 {
-- 
2.55.0

Re: [PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[Florian Westphal]

Florian Westphal <fw@strlen.de> wrote:
> Will be used by upcoming ipset rhashtable conversion.
> 
> "walk rht with unlink+free" triggers LLM reject pattern:
> "possible softirq CPU stall".
> 
> "walk rht with unlink+free + cond_resched" triggers
> "possibly skipped elements".
> 
> Add a helper to detach current hash backend storage from the
> rhashtable, then iterate and flush all contained elements.

As usual, I'm very incompetent and the patch sucks.

So question is merely if this function/idea is desirable/wanted
in the first place.

Re: [PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[Herbert Xu]

Florian Westphal <fw@strlen.de> wrote:
> Will be used by upcoming ipset rhashtable conversion.
> 
> "walk rht with unlink+free" triggers LLM reject pattern:
> "possible softirq CPU stall".
> 
> "walk rht with unlink+free + cond_resched" triggers
> "possibly skipped elements".
> 
> Add a helper to detach current hash backend storage from the
> rhashtable, then iterate and flush all contained elements.
> 
> Cc: herbert@gondor.apana.org.au
> Cc: linux-crypto@vger.kernel.org
> Link: https://sashiko.dev/#/patchset/20260828152256.8759-1-fw%40strlen.de
> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> Herbert: If you prefer to take this via the crypto tree, please
> let me know.
> Otherwise, an explicit Ack would be appreciated, so this can
> be handled via nf-next.  Thanks.
> 
> net/ipv6/ila/ could be converted to use this helper too.
> 
> include/linux/rhashtable.h |  19 ++++++
> lib/rhashtable.c           | 127 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 146 insertions(+)

Sorry I wasn't paying attention.

So is the problem that there is no way to remove all elements for
a given key in an rhltable?

If that is what's needed then we should just add it for rhltable
since normal rhashtable's do not contain duplicate objects for a
given key.

Thanks,
-- 
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

Re: [PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[Florian Westphal]

Herbert Xu <herbert@gondor.apana.org.au> wrote:
> Florian Westphal <fw@strlen.de> wrote:
> > Will be used by upcoming ipset rhashtable conversion.
> > 
> > "walk rht with unlink+free" triggers LLM reject pattern:
> > "possible softirq CPU stall".
> > 
> > "walk rht with unlink+free + cond_resched" triggers
> > "possibly skipped elements".
> > 
> > Add a helper to detach current hash backend storage from the
> > rhashtable, then iterate and flush all contained elements.
> > 
> > Cc: herbert@gondor.apana.org.au
> > Cc: linux-crypto@vger.kernel.org
> > Link: https://sashiko.dev/#/patchset/20260828152256.8759-1-fw%40strlen.de
> > Assisted-by: Claude:claude-sonnet-5
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > ---
> > Herbert: If you prefer to take this via the crypto tree, please
> > let me know.
> > Otherwise, an explicit Ack would be appreciated, so this can
> > be handled via nf-next.  Thanks.
> > 
> > net/ipv6/ila/ could be converted to use this helper too.
> > 
> > include/linux/rhashtable.h |  19 ++++++
> > lib/rhashtable.c           | 127 +++++++++++++++++++++++++++++++++++++
> > 2 files changed, 146 insertions(+)
> 
> Sorry I wasn't paying attention.
> 
> So is the problem that there is no way to remove all elements for
> a given key in an rhltable?

No.  The problem is that I am too dumb to remove them without having
an LLM tell me to go fuck myself.

> If that is what's needed then we should just add it for rhltable
> since normal rhashtable's do not contain duplicate objects for a
> given key.

I don't understand this response.  This isn't about rhashtable vs.
rhltable.  This is about my incompetence to flush an rhashtable or
rhashtable.  Simple version:

        rhashtable_walk_enter();
        rhashtable_walk_start();

        while ((he = rhashtable_walk_next())) {
                if (IS_ERR(he)) {
                        if (PTR_ERR(he) != -EAGAIN) { ..  break; }
                        continue;
                }

		rhashtable_remove_fast()
		/* free */
        }

        rhashtable_walk_stop();
        rhashtable_walk_exit();

... tells that this causes softirq lockup for huge tables.

Adding a lock-break after N elements via
if (flushed > 64) {
	rhashtable_walk_stop();
	cond_resched();
	rhashtable_walk_start();
}

... tells that this will skip some elements.


... Full restart on atomic_read(->nelems) > 0 post loop
seems wrong to me too.

So, to get out of this I tried to add a 'flush all elements' helper to
the core that just replaces backend storage.

Does adding such a helper make sense or not?  Thats the only question
here.  If yes, I'll make a v2. If no, I will go back to V1.  Unless you
have a better idea.

Re: [PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[Herbert Xu]

On Tue, Sep 08, 2026 at 07:30:10AM +0200, Florian Westphal wrote:
> 
> I don't understand this response.  This isn't about rhashtable vs.
> rhltable.  This is about my incompetence to flush an rhashtable or
> rhashtable.  Simple version:
> 
>         rhashtable_walk_enter();
>         rhashtable_walk_start();

You should never use rhashtable_walk for real work.  It was only ever
intended for the very limited case of netlink dumping where stability
or accuracy was not a requirement.

>         while ((he = rhashtable_walk_next())) {
>                 if (IS_ERR(he)) {
>                         if (PTR_ERR(he) != -EAGAIN) { ..  break; }
>                         continue;
>                 }
> 
> 		rhashtable_remove_fast()

So you want to free the entire table, right?

We already have rhashtable_free_and_destroy, any reason why it
doesn't work?

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

Re: [PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[Florian Westphal]

Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > I don't understand this response.  This isn't about rhashtable vs.
> > rhltable.  This is about my incompetence to flush an rhashtable or
> > rhashtable.  Simple version:
> > 
> >         rhashtable_walk_enter();
> >         rhashtable_walk_start();
> 
> You should never use rhashtable_walk for real work.  It was only ever
> intended for the very limited case of netlink dumping where stability
> or accuracy was not a requirement.
> 
> >         while ((he = rhashtable_walk_next())) {
> >                 if (IS_ERR(he)) {
> >                         if (PTR_ERR(he) != -EAGAIN) { ..  break; }
> >                         continue;
> >                 }
> > 
> > 		rhashtable_remove_fast()
> 
> So you want to free the entire table, right?

No, remove all elements. Concurrent insertion is not disabled.

> We already have rhashtable_free_and_destroy, any reason why it
> doesn't work?

It requires userspace or kernel don't add new elements.

AFAICS I can't "destroy, then re-init" without some external
mutex.  What I could do is add another indirection, i.e.

 struct htype {
-       struct rhashtable ht;   /* the hash table */
+       struct rhashtable *ht;   /* the hash table */
        struct net_prefixes __rcu *rnets[IPSET_NET_COUNT]; /* cidr prefixes */
 };

... and then alloc+init an new ht + free old one.
But its not nice either.

Re: [PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[Herbert Xu]

On Tue, Sep 08, 2026 at 11:56:09AM +0200, Florian Westphal wrote:
>
> > So you want to free the entire table, right?
> 
> No, remove all elements. Concurrent insertion is not disabled.

So what's the desired semantics for insertions that occur while
you're flushing the table?

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

Re: [PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[Florian Westphal]

Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > No, remove all elements. Concurrent insertion is not disabled.
> 
> So what's the desired semantics for insertions that occur while
> you're flushing the table?

In current ipset home-grown hashes:

If the insertion is into a region not yet scanned -> will be dropped
If the insertion is into a region already scanned -> will exist after flush.

i.e. no guarantee that table is empty after flush, but all
elements that existed before the flush are gone.

For the rhashtable variant I had proposed:

If the insertion is before the moment future_tbl is updated: will be dropped
If the insertion is after future_tbl update: will exist after flush

which I think is matches existing semantics of ipsets homegrown hash
tables (that I'd like to replace with rhashtable/rhltable).

Thanks,
Florian

Re: [PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[Herbert Xu]

On Tue, Sep 08, 2026 at 03:25:13PM +0200, Florian Westphal wrote:
>
> In current ipset home-grown hashes:
> 
> If the insertion is into a region not yet scanned -> will be dropped
> If the insertion is into a region already scanned -> will exist after flush.

OK now I understand what your patch is trying to do.  We can
continue the discussion there.

Thanks,
-- 
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

Re: [PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[Herbert Xu]

Florian Westphal <fw@strlen.de> wrote:
>
> +void rhashtable_flush_and_free(struct rhashtable *ht,
> +                              void (*free_fn)(void *ptr, void *arg),
> +                              void *arg)
> +{
> +       struct bucket_table *tbl, *old_tbl, *last_tbl, *new_tbl;
> +       struct rhashtable_walker *walker;
> +       unsigned int i;
> +
> +       new_tbl = bucket_table_alloc(ht, rounded_hashtable_size(&ht->p),
> +                                    GFP_KERNEL);
> +       if (!new_tbl)
> +               new_tbl = bucket_table_alloc(ht, ht->p.min_size,
> +                                            GFP_KERNEL | __GFP_NOFAIL);
> +
> +       mutex_lock(&ht->mutex);

Before starting the work you need to stop all existing rehashes.

This is where it helps if you impose restrictions on the caller.
For example, if you could guarantee that no insertions or removals
occur during the call to rhashtable_flush_and_free (the duration of
the call does not include the actual freeing, which can occur later),
then it's much easier since you could just call cancel_work_sync on
the rehashes.

If you can't guarantee that, then we'll need some sort of a flag to
stop the rehashes manually.

> +       /* Splice the new, empty table onto the tail of the live table ... */
> +       old_tbl = rht_dereference(ht->tbl, ht);
> +       do {
> +               last_tbl = rhashtable_last_table(ht, old_tbl);
> +       } while (rhashtable_rehash_attach(ht, last_tbl, new_tbl));

This loop is only needed if you impose no restrictions on the caller.
And if we're going to do this, then you'd need to fix the logic in
rhashtable_insert_rehash as otherwise it may interpret this as a
rehash (as opposed to a resize) which could fail with EBUSY.

In fact I think this could become a lot simpler since the two
tables don't need to mix at all.

Just add an old_tbl field to struct rhashtable, then move the
old table into it and directly write the new table to tbl.  That
way none of the complications from overlapping insertions matter
anymore.

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

Re: [PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[Florian Westphal]

Herbert Xu <herbert@gondor.apana.org.au> wrote:
> Florian Westphal <fw@strlen.de> wrote:
> >
> > +void rhashtable_flush_and_free(struct rhashtable *ht,
> > +                              void (*free_fn)(void *ptr, void *arg),
> > +                              void *arg)
> > +{
> > +       struct bucket_table *tbl, *old_tbl, *last_tbl, *new_tbl;
> > +       struct rhashtable_walker *walker;
> > +       unsigned int i;
> > +
> > +       new_tbl = bucket_table_alloc(ht, rounded_hashtable_size(&ht->p),
> > +                                    GFP_KERNEL);
> > +       if (!new_tbl)
> > +               new_tbl = bucket_table_alloc(ht, ht->p.min_size,
> > +                                            GFP_KERNEL | __GFP_NOFAIL);
> > +
> > +       mutex_lock(&ht->mutex);
> 
> Before starting the work you need to stop all existing rehashes.
> 
> This is where it helps if you impose restrictions on the caller.
> For example, if you could guarantee that no insertions or removals
> occur during the call to rhashtable_flush_and_free (the duration of
> the call does not include the actual freeing, which can occur later),
> then it's much easier since you could just call cancel_work_sync on
> the rehashes.
> 
> If you can't guarantee that, then we'll need some sort of a flag to
> stop the rehashes manually.

Hmm. why?  AFAICS the rehash worker holds ht->mutex, i.e. a rehash
might be pending, but it cannot run in parallel.

We called 'rcu_assign_pointer(ht->tbl, new_tbl);' before unlocking the
mutex, so a pending work (resize or rehash) will only see elements
that have been added right after that point.

Am I misreading anything here?

> > +       /* Splice the new, empty table onto the tail of the live table ... */
> > +       old_tbl = rht_dereference(ht->tbl, ht);
> > +       do {
> > +               last_tbl = rhashtable_last_table(ht, old_tbl);
> > +       } while (rhashtable_rehash_attach(ht, last_tbl, new_tbl));
> 
> This loop is only needed if you impose no restrictions on the caller.

Yes, no restrictions are imposed on the caller, at least thats the idea.

> In fact I think this could become a lot simpler since the two
> tables don't need to mix at all.
> 
> Just add an old_tbl field to struct rhashtable, then move the
> old table into it and directly write the new table to tbl.  That
> way none of the complications from overlapping insertions matter
> anymore.

Hmm. What do you think of this version?

+void rhashtable_flush_and_free(struct rhashtable *ht,
+                              void (*free_fn)(void *ptr, void *arg),
+                              void *arg)
+{
+       struct bucket_table *tbl, *old_tbl, *new_tbl;
+
+       new_tbl = bucket_table_alloc(ht, rounded_hashtable_size(&ht->p),
+                                    GFP_KERNEL);
+       if (!new_tbl)
+               new_tbl = bucket_table_alloc(ht, ht->p.min_size,
+                                            GFP_KERNEL | __GFP_NOFAIL);
+
+       /* Make sure we won't race with rhashtable_rehash_table() */
+       mutex_lock(&ht->mutex);
+       old_tbl = rcu_replace_pointer(ht->tbl, new_tbl, lockdep_rht_mutex_is_held(ht));
+       mutex_unlock(&ht->mutex);
+
+       /* Make sure all other CPUs no longer access the old table */
+       synchronize_rcu();

Rest as before.  This way, we don't need to worry about concurrency,
no readers or writers can reach old_tbl anymore.

I'll give this a try.

Re: [PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[Herbert Xu]

On Wed, Sep 09, 2026 at 04:45:15PM +0200, Florian Westphal wrote:
>
> Hmm. why?  AFAICS the rehash worker holds ht->mutex, i.e. a rehash
> might be pending, but it cannot run in parallel.

Because you're chaining the new table onto the end of the old table
in the original patch.  So a rehash could start moving entries
from the old table into the new table.

In fact it's still a problem if you set ht->tbl atomically, because
we need to differentiate between rehashes triggered by an insert
or remove that occurred on the old ht->tbl versus one that occured
on the new ht->tbl.

Since there is just one work struct it's impossible to tell the
difference.
 
> Hmm. What do you think of this version?
> 
> +void rhashtable_flush_and_free(struct rhashtable *ht,
> +                              void (*free_fn)(void *ptr, void *arg),
> +                              void *arg)
> +{
> +       struct bucket_table *tbl, *old_tbl, *new_tbl;
> +
> +       new_tbl = bucket_table_alloc(ht, rounded_hashtable_size(&ht->p),
> +                                    GFP_KERNEL);
> +       if (!new_tbl)
> +               new_tbl = bucket_table_alloc(ht, ht->p.min_size,
> +                                            GFP_KERNEL | __GFP_NOFAIL);
> +
> +       /* Make sure we won't race with rhashtable_rehash_table() */
> +       mutex_lock(&ht->mutex);
> +       old_tbl = rcu_replace_pointer(ht->tbl, new_tbl, lockdep_rht_mutex_is_held(ht));
> +       mutex_unlock(&ht->mutex);
> +
> +       /* Make sure all other CPUs no longer access the old table */
> +       synchronize_rcu();
> 
> Rest as before.  This way, we don't need to worry about concurrency,
> no readers or writers can reach old_tbl anymore.

Yes I think this should work.  But rehashes are still tricky if we allow
the caller to do insertions/removals during the flush operation.

It would be a lot simpler if you added the restriction that the
caller must not call insert/remove before the flush call returned.
Otherwise we would need to add a lot of complexity to rhashtable
in order to distinguish between insertions/removals on the table
before or after the flush.

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

Re: [PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[Florian Westphal]

Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > +void rhashtable_flush_and_free(struct rhashtable *ht,
> > +                              void (*free_fn)(void *ptr, void *arg),
> > +                              void *arg)
> > +{
> > +       struct bucket_table *tbl, *old_tbl, *new_tbl;
> > +
> > +       new_tbl = bucket_table_alloc(ht, rounded_hashtable_size(&ht->p),
> > +                                    GFP_KERNEL);
> > +       if (!new_tbl)
> > +               new_tbl = bucket_table_alloc(ht, ht->p.min_size,
> > +                                            GFP_KERNEL | __GFP_NOFAIL);
> > +
> > +       /* Make sure we won't race with rhashtable_rehash_table() */
> > +       mutex_lock(&ht->mutex);
> > +       old_tbl = rcu_replace_pointer(ht->tbl, new_tbl, lockdep_rht_mutex_is_held(ht));
> > +       mutex_unlock(&ht->mutex);
> > +
> > +       /* Make sure all other CPUs no longer access the old table */
> > +       synchronize_rcu();
> > 
> > Rest as before.  This way, we don't need to worry about concurrency,
> > no readers or writers can reach old_tbl anymore.
> 
> Yes I think this should work.  But rehashes are still tricky if we allow
> the caller to do insertions/removals during the flush operation.

What problems do you see?  Shouldn't a rehash be a no-op, since it
can't see anything in the hashtable once the flusher releases the
mutex post replace_pointer() call?

If a rehash is in progress, the flusher will block on ht->mutex.

Same for removals, no entries will be found, insertions are ok too as they
add to the new backing store.

> It would be a lot simpler if you added the restriction that the
> caller must not call insert/remove before the flush call returned.

How to assert that?  Or should that be an 'external' requirement?

That will, at least afaics, defeat rhashtables purpose, I'd have to
either serialize by external single lock, or add a flag that would
'eat' new insertions while the flush is running.

I could do the latter, if that helps, but I don't see why its
needed with the proposed 'replace, then synchronize_rcu()'.

> Otherwise we would need to add a lot of complexity to rhashtable
> in order to distinguish between insertions/removals on the table
> before or after the flush.

I would like to avoid extra complexity or even new tests in insert/removal
fast paths.

Thanks for reviewing!

Re: [PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[Herbert Xu]

On Thu, Sep 10, 2026 at 12:41:27PM +0200, Florian Westphal wrote:
>
> What problems do you see?  Shouldn't a rehash be a no-op, since it
> can't see anything in the hashtable once the flusher releases the
> mutex post replace_pointer() call?

The problem is that the rehash is just a deferred work which carries
no state.  So it doesn't know why it was triggered.

If we go with the atomic replacement, the issue then comes down
to the fact that if a rehash was triggered before replacement
then it should do nothing, while if a rehash was triggered after
replacement then it should do something.  However, because the
rehash doesn't have state it doesn't know.

If you do a rehash when it shouldn't be done, you may end up with
spurious EBUSY errors because it thinks the hashtable is under
attack.  While if you skip a rehash when it should've be done,
then it could leave the hashtable in a suboptimal state.  But
this should correct itself eventually, so perhaps this would be
the easiest solution.
 
> > It would be a lot simpler if you added the restriction that the
> > caller must not call insert/remove before the flush call returned.
> 
> How to assert that?  Or should that be an 'external' requirement?

It's certainly not easy to detect this from within rhashtable.

> That will, at least afaics, defeat rhashtables purpose, I'd have to
> either serialize by external single lock, or add a flag that would
> 'eat' new insertions while the flush is running.

If there is no natural way of expressing it in the caller, then
we might as well add the complexity to rhashtable.  Let me look
into this.

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

Re: [PATCH nf-next v4 01/13] rhashtable: add rhashtable_flush_and_free helper

[Florian Westphal]

Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > That will, at least afaics, defeat rhashtables purpose, I'd have to
> > either serialize by external single lock, or add a flag that would
> > 'eat' new insertions while the flush is running.
> 
> If there is no natural way of expressing it in the caller, then
> we might as well add the complexity to rhashtable.  Let me look
> into this.

Thanks Herbert.