From: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> Sent: Friday, January 10, 2025 9:24 AM
>
> On Fri, Jan 10, 2025 at 04:59:28PM +0000, Michael Kelley wrote:
> >
> > This patch fixes the problem I saw with VMs in the Azure cloud. Thanks!
>
> Sorry, but the test on data is needed after all (it was just
> buggy). Otherwise we will break rhlist. So please test this
> patch instead.
>
> ---8<---
> The test on whether rhashtable_insert_one did an insertion relies
> on the value returned by rhashtable_lookup_one. Unfortunately that
> value is overwritten after rhashtable_insert_one returns. Fix this
> by saving the old value.
>
> Also simplify the test as only data == NULL matters.
>
> Reported-by: Michael Kelley <mhklinux@xxxxxxxxxxx>
> Fixes: e1d3422c95f0 ("rhashtable: Fix potential deadlock by moving schedule_work
> outside lock")
> Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
>
> diff --git a/lib/rhashtable.c b/lib/rhashtable.c
> index bf956b85455a..e36b36f3146d 100644
> --- a/lib/rhashtable.c
> +++ b/lib/rhashtable.c
> @@ -611,17 +611,20 @@ static void *rhashtable_try_insert(struct rhashtable *ht,
> const void *key,
> new_tbl = rht_dereference_rcu(tbl->future_tbl, ht);
> data = ERR_PTR(-EAGAIN);
> } else {
> + void *odata;
> +
> flags = rht_lock(tbl, bkt);
> data = rhashtable_lookup_one(ht, bkt, tbl,
> hash, key, obj);
> new_tbl = rhashtable_insert_one(ht, bkt, tbl,
> hash, obj, data);
> + odata = data;
> if (PTR_ERR(new_tbl) != -EEXIST)
> data = ERR_CAST(new_tbl);
>
> rht_unlock(tbl, bkt, flags);
>
> - if (PTR_ERR(data) == -ENOENT && !new_tbl) {
> + if (odata && !new_tbl) {
> atomic_inc(&ht->nelems);
> if (rht_grow_above_75(ht, tbl))
> schedule_work(&ht->run_work);
This patch passes my tests. I'm doing a narrow test to verify that
the boot failure when opening the Mellanox NIC is no longer occurring.
I also unloaded/reloaded the mlx5 driver a couple of times. For good
measure, I then did a full Linux kernel build, and all is good. My testing
does not broadly verify correct operation of rhashtable except as it
gets exercised implicitly by these basic tests.
Michael
