Re: [PATCH] fs: gfs2: fix sleeping function called from invalid context

[Alessandro Zanni]

On Mon, May 04, 2026 at 01:27:12PM +0200, Andreas Gruenbacher wrote:
> Alessandro,
> 
> On Wed, Apr 29, 2026 at 10:15 AM Alessandro Zanni
> <alessandro.zanni87@xxxxxxxxx> wrote:
> > The issue arises on a PREEMPT kernel because gfs2_quota_init
> > calls gfs2_qd_search_bucket while holding a bit spinlock and
> > triggering a "sleeping function called from invalid context"
> > bug.
> >
> > This patch refactors the quota initialization by splitting the
> > lock into separate locks, moving the search outside the atomic
> > section and using RCU lock for a safe access without holding the
> > bit spinlock.
> >
> > Modifications of this patch:
> > 1. Use rcu_read_lock() around the invocation of the function
> >    gfs2_qd_search_bucket() for the search.
> > 2. Add the spin_lock() around the insertion into the hash table
> >    and lists.
> > 3. Remove the usage of unused spin_unlock().
> 
> How is this better than Jie Wang's fix from April 23?
> 
> https://lore.kernel.org/gfs2/CAHc6FU7DLZrunqxQkY35voRraBxUG=wNucn_3s7LqF1BdrxzCg@xxxxxxxxxxxxxx/T/#m37764558ab05e4166ad27abf85576cd67957cbfb
> 
> I've had Jie Wang's fix applied already, and I've now also updated for-next.
> 
> > Fixes: de0d95c26c41c ("gfs2: Check quota consistency on mount")
> > Reported-by: syzbot+642d0561f78362d67d3f@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Closes: https://syzkaller.appspot.com/bug?extid=642d0561f78362d67d3f
> > Signed-off-by: Alessandro Zanni <alessandro.zanni87@xxxxxxxxx>
> > ---
> >  fs/gfs2/quota.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> > index 5290865f27f1..48516cbc8b49 100644
> > --- a/fs/gfs2/quota.c
> > +++ b/fs/gfs2/quota.c
> > @@ -1456,17 +1456,15 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
> >                         qd->qd_slot = slot;
> >                         qd->qd_slot_ref = 1;
> >
> > -                       spin_lock(&qd_lock);
> > -                       spin_lock_bucket(hash);
> > +                       rcu_read_lock();
> >                         old_qd = gfs2_qd_search_bucket(hash, sdp, qc_id);
> > +                       rcu_read_unlock();
> >                         if (old_qd) {
> >                                 fs_err(sdp, "Corruption found in quota_change%u"
> >                                             "file: duplicate identifier in "
> >                                             "slot %u\n",
> >                                             sdp->sd_jdesc->jd_jid, slot);
> >
> > -                               spin_unlock_bucket(hash);
> > -                               spin_unlock(&qd_lock);
> >                                 qd_put(old_qd);
> >
> >                                 gfs2_glock_put(qd->qd_gl);
> > @@ -1480,6 +1478,8 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
> >
> >                                 continue;
> >                         }
> > +                       spin_lock(&qd_lock);
> > +                       spin_lock_bucket(hash);
> >                         BUG_ON(test_and_set_bit(slot, sdp->sd_quota_bitmap));
> >                         list_add(&qd->qd_list, &sdp->sd_quota_list);
> >                         atomic_inc(&sdp->sd_quota_count);
> > --
> > 2.47.3
> >
> 
> Thanks,
> Andreas
>

Sorry for the delay.

This patch seems already ok to me because it maintains atomicity with search and
insertion within the same critical section, bypassing the PREEMPT_RT sleep bug by
avoiding the lockref call while holding the bit-spinlock.

So, ignore mine.

Regards,
Alessandro