[PATCH RFC 20/28] gfs2: wait for a pending withdraw before remounting read-only

[Christian Brauner]

On Fri, Oct 16, 2026 at 02:32:01AM +0200, Andreas Gruenbacher wrote:
> Christian,
> 
> On Mon, Oct 12, 2026 at 1:27 PM Christian Brauner <brauner at kernel.org> wrote:
> > Since commit 9c4a3de6cd5b ("gfs2: Asynchronous withdraw") most of a
> > withdraw happens in do_withdraw() from a work item and SDF_JOURNAL_LIVE
> > stays set until it ran. A remount to read-only in that window goes
> > through gfs2_make_fs_ro(). Its log flushes bail out early on the
> > withdrawn filesystem before FLUSH_SHUTDOWN can clear SDF_JOURNAL_LIVE
> > and then gfs2_quota_cleanup() hits its BUG_ON():
> >
> >   kernel BUG at fs/gfs2/quota.c:1532!
> >   RIP: 0010:gfs2_quota_cleanup+0x562/0x6b0
> >    gfs2_make_fs_ro+0xf3/0x2e0
> >    gfs2_reconfigure+0x42e/0xfd0
> >    reconfigure_super+0x437/0xb20
> >
> > Just flushing the withdraw work before the remount isn't enough.
> > gfs2_withdraw() sets SDF_WITHDRAWN, calls dump_stack() and only then
> > queues the work. So a remount can see a withdrawn filesystem while
> > there's nothing to flush yet. And a withdraw can also start in the
> > middle of gfs2_make_fs_ro().
> >
> > So let do_withdraw() wake up waiters when it clears SDF_JOURNAL_LIVE and
> > let gfs2_make_fs_ro() wait for the bit after its log flushes, right
> > before gfs2_quota_cleanup(). On a healthy filesystem the shutdown flush
> > already cleared it. umount doesn't need to wait. gfs2_put_super() sets
> > SDF_NORECOVERY first and gfs2_quota_cleanup() only BUGs without it. Once
> > the bit is clear the journal isn't live anymore and gfs2_make_fs_ro()
> > doesn't write anything.
> >
> > gfs2_make_fs_rw() can leave the bit set on a withdrawn filesystem as
> > well. It checks for a withdraw and then sets SDF_JOURNAL_LIVE. If
> > do_withdraw() runs in between it finds the journal not live yet and
> > returns. Nothing clears the bit then and the next remount to read-only
> > waits for it forever. So check for the withdraw and set the bit under
> > sd_log_flush_lock, which do_withdraw() holds while it tests the bit.
> > Then either the remount sees the withdraw or do_withdraw() sees the live
> > journal.
> >
> > Reproducer, lock_nolock: a SCHED_FIFO task writes 1 to the withdraw
> > sysfs knob from cpu 1 and keeps cpu 1 busy (RT throttling and cpu 1's
> > fair server disabled) so do_withdraw() can't run, then from cpu 2:
> >
> >   mount -o remount,ro /mnt
> >
> > On lock_dlm the window lasts as long as the withdraw helper runs. The
> > window before the work is queued can be widened with a kprobe on
> > dump_stack_lvl() that delays the task writing to the withdraw knob by a
> > few seconds. A remount to read-only or sysrq u in that window hits the
> > same BUG_ON().
> >
> > Fixes: 9c4a3de6cd5b ("gfs2: Asynchronous withdraw")
> > Cc: stable at vger.kernel.org
> > Signed-off-by: Christian Brauner (Amutable) <brauner at kernel.org>
> > ---
> >  fs/gfs2/super.c | 22 ++++++++++++++++++----
> >  fs/gfs2/util.c  |  2 +-
> >  2 files changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> > index 06302c29340f..1d7b39354d65 100644
> > --- a/fs/gfs2/super.c
> > +++ b/fs/gfs2/super.c
> > @@ -147,12 +147,23 @@ int gfs2_make_fs_rw(struct gfs2_sbd *sdp)
> >         }
> >
> >         error = gfs2_quota_init(sdp);
> > -       if (!error && gfs2_withdrawn(sdp)) {
> > -               gfs2_quota_cleanup(sdp);
> > +       if (error)
> > +               return error;
> > +
> > +       /*
> > +        * do_withdraw() only takes a live journal down. Check for a withdraw
> > +        * under the lock it holds for that so that either we see the withdraw
> > +        * or it sees the journal live. Otherwise gfs2_make_fs_ro() waits for
> > +        * the bit forever.
> > +        */
> > +       down_write(&sdp->sd_log_flush_lock);
> > +       if (gfs2_withdrawn(sdp))
> >                 error = -EIO;
> > -       }
> > -       if (!error)
> > +       else
> >                 set_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
> > +       up_write(&sdp->sd_log_flush_lock);
> > +       if (error)
> > +               gfs2_quota_cleanup(sdp);
> >         return error;
> >  }
> >
> > @@ -565,6 +576,9 @@ void gfs2_make_fs_ro(struct gfs2_sbd *sdp)
> >                                    HZ * 5);
> >                 gfs2_assert_warn(sdp, gfs2_log_is_empty(sdp));
> >         }
> > +       /* A withdraw that do_withdraw() didn't finish yet leaves it live. */
> > +       if (!test_bit(SDF_NORECOVERY, &sdp->sd_flags))
> > +               wait_on_bit(&sdp->sd_flags, SDF_JOURNAL_LIVE, TASK_UNINTERRUPTIBLE);
> >         gfs2_quota_cleanup(sdp);
> >  }
> >
> > diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
> > index 83b8bb6446e5..1535ce2b7aa8 100644
> > --- a/fs/gfs2/util.c
> > +++ b/fs/gfs2/util.c
> > @@ -122,7 +122,7 @@ static void do_withdraw(struct gfs2_sbd *sdp)
> >                 up_write(&sdp->sd_log_flush_lock);
> >                 return;
> >         }
> > -       clear_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
> > +       clear_and_wake_up_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
> >         gfs2_ail_drain(sdp); /* frees all transactions */
> >         up_write(&sdp->sd_log_flush_lock);
> >
> >
> > --
> > 2.53.0
> >
> 
> this looks good all around, thank you very much. Can I add it to my gfs2 queue?

I was planning to just provide a stable branch for everyone to pull post
-rc1. Then you can just pull that in?