Re: [PATCH rdma-next v2 1/1] RDMA/core: Fix WARNING in gid_table_release_one

[Jason Gunthorpe <jgg@ziepe.ca>]

On Wed, Nov 05, 2025 at 03:09:58PM +0200, Leon Romanovsky wrote:
> On Tue, Nov 04, 2025 at 03:36:01PM -0800, Zhu Yanjun wrote:
> > GID entry ref leak for dev syz1 index 2 ref=615
> > ...
> > Call Trace:
> >  <TASK>
> >  ib_device_release+0xd2/0x1c0 drivers/infiniband/core/device.c:509
> >  device_release+0x99/0x1c0 drivers/base/core.c:-1
> >  kobject_cleanup lib/kobject.c:689 [inline]
> >  kobject_release lib/kobject.c:720 [inline]
> >  kref_put include/linux/kref.h:65 [inline]
> >  kobject_put+0x228/0x480 lib/kobject.c:737
> >  process_one_work kernel/workqueue.c:3263 [inline]
> >  process_scheduled_works+0xae1/0x17b0 kernel/workqueue.c:3346
> >  worker_thread+0x8a0/0xda0 kernel/workqueue.c:3427
> >  kthread+0x711/0x8a0 kernel/kthread.c:463
> >  ret_from_fork+0x47c/0x820 arch/x86/kernel/process.c:158
> >  ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
> >  </TASK>
> > 
> > When the state of a GID is GID_TABLE_ENTRY_PENDING_DEL, it indicates
> > that the GID is about to be released soon. Therefore, it does not
> > appear to be a leak.
> > 
> > Fixes: b150c3862d21 ("IB/core: Introduce GID entry reference counts")
> > Reported-by: syzbot+b0da83a6c0e2e2bddbd4@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=b0da83a6c0e2e2bddbd4
> > Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
> > ---
> > V1->V2: Use flush_workqueue instead of while loop
> > ---
> >  drivers/infiniband/core/cache.c | 16 +++++++++++++---
> >  1 file changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> > index 81cf3c902e81..74211fb37020 100644
> > --- a/drivers/infiniband/core/cache.c
> > +++ b/drivers/infiniband/core/cache.c
> > @@ -799,16 +799,26 @@ static void release_gid_table(struct ib_device *device,
> >  	if (!table)
> >  		return;
> >  
> > +	mutex_lock(&table->lock);
> >  	for (i = 0; i < table->sz; i++) {
> >  		if (is_gid_entry_free(table->data_vec[i]))
> >  			continue;
> >  
> > -		WARN_ONCE(true,
> > -			  "GID entry ref leak for dev %s index %d ref=%u\n",
> > +		WARN_ONCE(table->data_vec[i]->state != GID_TABLE_ENTRY_PENDING_DEL,
> > +			  "GID entry ref leak for dev %s index %d ref=%u, state: %d\n",
> >  			  dev_name(&device->dev), i,
> > -			  kref_read(&table->data_vec[i]->kref));
> > +			  kref_read(&table->data_vec[i]->kref), table->data_vec[i]->state);
> > +		/*
> > +		 * The entry may be sitting in the WQ waiting for
> > +		 * free_gid_work(), flush it to try to clean it.
> > +		 */
> > +		mutex_unlock(&table->lock);
> > +		flush_workqueue(ib_wq);
> > +		mutex_lock(&table->lock);
> 
> I can't agree with idea that flush_workqueue() is called in the loop.

Since we almost never see these WARN_ON's it isn't really called in a
loop, but sure you could put a conditional around it to do it only
once.

The WARN on is in the wrong order, it is not a kernel bug if the
workqueue is still pending. flush the queue and then check again, and
then do the warn.

@@ -791,22 +791,31 @@ static struct ib_gid_table *alloc_gid_table(int sz)
        return NULL;
 }
 
-static void release_gid_table(struct ib_device *device,
-                             struct ib_gid_table *table)
+static bool is_gid_table_clean(struct ib_gid_table *table)
 {
        int i;
 
+       guard(mutex)(&table->lock);
+       for (i = 0; i < table->sz; i++)
+               if (!is_gid_entry_free(table->data_vec[i]))
+                       return false;
+       return true;
+}
+
+static void release_gid_table(struct ib_device *device,
+                             struct ib_gid_table *table)
+{
        if (!table)
                return;
 
-       for (i = 0; i < table->sz; i++) {
-               if (is_gid_entry_free(table->data_vec[i]))
-                       continue;
-
-               WARN_ONCE(true,
-                         "GID entry ref leak for dev %s index %d ref=%u\n",
-                         dev_name(&device->dev), i,
-                         kref_read(&table->data_vec[i]->kref));
+       if (!is_gid_table_clean(table)) {
+               /*
+                * The entry may be sitting in the WQ waiting for
+                * free_gid_work(), flush it to try to clean it.
+                */
+               flush_workqueue(ib_wq);
+               if (!is_gid_table_clean(table))
+                       WARN_ONCE(true, "GID entry has leaked");
        }
 
        mutex_destroy(&table->lock);

Jason