public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs: start inodegc after quota initialization
@ 2026-04-13  0:49 Pedro Demarchi Gomes
  2026-04-13 18:09 ` Darrick J. Wong
  0 siblings, 1 reply; 4+ messages in thread
From: Pedro Demarchi Gomes @ 2026-04-13  0:49 UTC (permalink / raw)
  To: Carlos Maiolino
  Cc: linux-xfs, linux-kernel, Pedro Demarchi Gomes, Shuangpeng Bai

When mounting a xfs filesystem with quotas, if an error occurs in xfs_mount
after calling xfs_inodegc_start and before m_quotainfo initialization, the
error path will call xfs_inodegc_flush assuming that m_quotainfo was already
initialized, causing the following error:

[   82.411853][   T23] ==================================================================
[   82.412678][   T23] BUG: KASAN: null-ptr-deref in mutex_lock (./include/linux/instrumented.h:55 ./include/linux/atomic/atomic-instrumented.h:4457 kernel/locking/mutex.c:159 kernel/locking/mutex.c:289)
[   82.413383][   T23] Write of size 8 at addr 0000000000000030 by task kworker/1:0/23
[   82.414187][   T23]
[   82.414450][   T23] CPU: 1 UID: 0 PID: 23 Comm: kworker/1:0 Not tainted 7.0.0-rc5-00051-gbbeb83d3182a #35 PREEMPT(
[   82.414458][   T23] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
[   82.414463][   T23] Workqueue: xfs-inodegc/loop0 xfs_inodegc_worker
[   82.414478][   T23] Call Trace:
[   82.414481][   T23]  <TASK>
[   82.414484][   T23]  dump_stack_lvl (lib/dump_stack.c:122)
[   82.414494][   T23]  print_report (mm/kasan/report.c:487)
[   82.414518][   T23]  kasan_report (mm/kasan/report.c:597)
[   82.414541][   T23]  kasan_check_range (mm/kasan/generic.c:? mm/kasan/generic.c:200)
[   82.414549][   T23]  mutex_lock (./include/linux/instrumented.h:55 ./include/linux/atomic/atomic-instrumented.h:4457 kernel/locking/mutex.c:159 kernel/locking/mutex.c:289)
[   82.414601][   T23]  xfs_qm_dqget_cache_lookup (fs/xfs/xfs_dquot.c:813)
[   82.414609][   T23]  xfs_qm_dqget_inode (fs/xfs/xfs_dquot.c:1017)
[   82.414678][   T23]  xfs_qm_dqattach_locked (fs/xfs/xfs_qm.c:331)
[   82.414688][   T23]  xfs_qm_dqattach (fs/xfs/xfs_qm.c:371)
[   82.414696][   T23]  xfs_inactive (fs/xfs/xfs_inode.c:1453)
[   82.414733][   T23]  xfs_inodegc_worker (fs/xfs/xfs_icache.c:1946 fs/xfs/xfs_icache.c:1992)
[   82.414743][   T23]  process_scheduled_works (kernel/workqueue.c:3281 kernel/workqueue.c:3359)
[   82.414755][   T23]  worker_thread (kernel/workqueue.c:?)
[   82.414775][   T23]  kthread (kernel/kthread.c:437)
[   82.414787][   T23]  ret_from_fork (arch/x86/kernel/process.c:164)
[   82.414822][   T23]  ret_from_fork_asm (arch/x86/entry/entry_64.S:255)
[   82.414834][   T23]  </TASK>

To avoid this, call xfs_inodegc_start after quota initialization.

Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
Closes: https://lore.kernel.org/all/DDC07F0C-05D2-46B4-9815-E010749FB3DC@gmail.com/
Signed-off-by: Pedro Demarchi Gomes <pedrodemargomes@gmail.com>
---
 fs/xfs/xfs_mount.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index ef1ea8a1238c..d6f75ab41337 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -1057,7 +1057,6 @@ xfs_mountfs(
 		xfs_clear_using_logged_xattrs(mp);
 
 	/* Enable background inode inactivation workers. */
-	xfs_inodegc_start(mp);
 	xfs_blockgc_start(mp);
 
 	if (xfs_has_metadir(mp)) {
@@ -1143,6 +1142,8 @@ xfs_mountfs(
 		}
 	}
 
+	xfs_inodegc_start(mp);
+
 	/*
 	 * Finish recovering the file system.  This part needed to be delayed
 	 * until after the root and real-time bitmap inodes were consistently
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] xfs: start inodegc after quota initialization
  2026-04-13  0:49 [PATCH] xfs: start inodegc after quota initialization Pedro Demarchi Gomes
@ 2026-04-13 18:09 ` Darrick J. Wong
  2026-04-15  7:26   ` Christoph Hellwig
  0 siblings, 1 reply; 4+ messages in thread
From: Darrick J. Wong @ 2026-04-13 18:09 UTC (permalink / raw)
  To: Pedro Demarchi Gomes
  Cc: Carlos Maiolino, linux-xfs, linux-kernel, Shuangpeng Bai

On Sun, Apr 12, 2026 at 09:49:25PM -0300, Pedro Demarchi Gomes wrote:
> When mounting a xfs filesystem with quotas, if an error occurs in xfs_mount
> after calling xfs_inodegc_start and before m_quotainfo initialization, the
> error path will call xfs_inodegc_flush assuming that m_quotainfo was already
> initialized, causing the following error:
> 
> [   82.411853][   T23] ==================================================================
> [   82.412678][   T23] BUG: KASAN: null-ptr-deref in mutex_lock (./include/linux/instrumented.h:55 ./include/linux/atomic/atomic-instrumented.h:4457 kernel/locking/mutex.c:159 kernel/locking/mutex.c:289)
> [   82.413383][   T23] Write of size 8 at addr 0000000000000030 by task kworker/1:0/23
> [   82.414187][   T23]
> [   82.414450][   T23] CPU: 1 UID: 0 PID: 23 Comm: kworker/1:0 Not tainted 7.0.0-rc5-00051-gbbeb83d3182a #35 PREEMPT(
> [   82.414458][   T23] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> [   82.414463][   T23] Workqueue: xfs-inodegc/loop0 xfs_inodegc_worker
> [   82.414478][   T23] Call Trace:
> [   82.414481][   T23]  <TASK>
> [   82.414484][   T23]  dump_stack_lvl (lib/dump_stack.c:122)
> [   82.414494][   T23]  print_report (mm/kasan/report.c:487)
> [   82.414518][   T23]  kasan_report (mm/kasan/report.c:597)
> [   82.414541][   T23]  kasan_check_range (mm/kasan/generic.c:? mm/kasan/generic.c:200)
> [   82.414549][   T23]  mutex_lock (./include/linux/instrumented.h:55 ./include/linux/atomic/atomic-instrumented.h:4457 kernel/locking/mutex.c:159 kernel/locking/mutex.c:289)
> [   82.414601][   T23]  xfs_qm_dqget_cache_lookup (fs/xfs/xfs_dquot.c:813)
> [   82.414609][   T23]  xfs_qm_dqget_inode (fs/xfs/xfs_dquot.c:1017)
> [   82.414678][   T23]  xfs_qm_dqattach_locked (fs/xfs/xfs_qm.c:331)
> [   82.414688][   T23]  xfs_qm_dqattach (fs/xfs/xfs_qm.c:371)
> [   82.414696][   T23]  xfs_inactive (fs/xfs/xfs_inode.c:1453)
> [   82.414733][   T23]  xfs_inodegc_worker (fs/xfs/xfs_icache.c:1946 fs/xfs/xfs_icache.c:1992)
> [   82.414743][   T23]  process_scheduled_works (kernel/workqueue.c:3281 kernel/workqueue.c:3359)
> [   82.414755][   T23]  worker_thread (kernel/workqueue.c:?)
> [   82.414775][   T23]  kthread (kernel/kthread.c:437)
> [   82.414787][   T23]  ret_from_fork (arch/x86/kernel/process.c:164)
> [   82.414822][   T23]  ret_from_fork_asm (arch/x86/entry/entry_64.S:255)
> [   82.414834][   T23]  </TASK>
> 
> To avoid this, call xfs_inodegc_start after quota initialization.
> 
> Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
> Closes: https://lore.kernel.org/all/DDC07F0C-05D2-46B4-9815-E010749FB3DC@gmail.com/
> Signed-off-by: Pedro Demarchi Gomes <pedrodemargomes@gmail.com>
> ---
>  fs/xfs/xfs_mount.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index ef1ea8a1238c..d6f75ab41337 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -1057,7 +1057,6 @@ xfs_mountfs(
>  		xfs_clear_using_logged_xattrs(mp);
>  
>  	/* Enable background inode inactivation workers. */
> -	xfs_inodegc_start(mp);
>  	xfs_blockgc_start(mp);
>  
>  	if (xfs_has_metadir(mp)) {
> @@ -1143,6 +1142,8 @@ xfs_mountfs(
>  		}
>  	}
>  
> +	xfs_inodegc_start(mp);

Why is it correct to defer inodegc until this point?  Can any code
between line 1060 and 1145 push an inode to the inodegc mechanism?
blockgc can unmap blocks from files, so why is it ok to leave the
blockgc_start call where it is?

--D

> +
>  	/*
>  	 * Finish recovering the file system.  This part needed to be delayed
>  	 * until after the root and real-time bitmap inodes were consistently
> -- 
> 2.47.3
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] xfs: start inodegc after quota initialization
  2026-04-13 18:09 ` Darrick J. Wong
@ 2026-04-15  7:26   ` Christoph Hellwig
  2026-04-15 15:12     ` Darrick J. Wong
  0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2026-04-15  7:26 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Pedro Demarchi Gomes, Carlos Maiolino, linux-xfs, linux-kernel,
	Shuangpeng Bai

On Mon, Apr 13, 2026 at 11:09:40AM -0700, Darrick J. Wong wrote:
> > index ef1ea8a1238c..d6f75ab41337 100644
> > --- a/fs/xfs/xfs_mount.c
> > +++ b/fs/xfs/xfs_mount.c
> > @@ -1057,7 +1057,6 @@ xfs_mountfs(
> >  		xfs_clear_using_logged_xattrs(mp);
> >  
> >  	/* Enable background inode inactivation workers. */
> > -	xfs_inodegc_start(mp);
> >  	xfs_blockgc_start(mp);
> >  
> >  	if (xfs_has_metadir(mp)) {
> > @@ -1143,6 +1142,8 @@ xfs_mountfs(
> >  		}
> >  	}
> >  
> > +	xfs_inodegc_start(mp);
> 
> Why is it correct to defer inodegc until this point?  Can any code
> between line 1060 and 1145 push an inode to the inodegc mechanism?
> blockgc can unmap blocks from files, so why is it ok to leave the
> blockgc_start call where it is?

Looking at this - I think actually performing inodegc before quotas
are up sounds dangerous, as we'd miss th quota accounting.  So I
think we need to defer starting the runners after that.  But we
should support queuing up inodes before that.  blockgc has
the same issues.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] xfs: start inodegc after quota initialization
  2026-04-15  7:26   ` Christoph Hellwig
@ 2026-04-15 15:12     ` Darrick J. Wong
  0 siblings, 0 replies; 4+ messages in thread
From: Darrick J. Wong @ 2026-04-15 15:12 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Pedro Demarchi Gomes, Carlos Maiolino, linux-xfs, linux-kernel,
	Shuangpeng Bai

On Wed, Apr 15, 2026 at 12:26:11AM -0700, Christoph Hellwig wrote:
> On Mon, Apr 13, 2026 at 11:09:40AM -0700, Darrick J. Wong wrote:
> > > index ef1ea8a1238c..d6f75ab41337 100644
> > > --- a/fs/xfs/xfs_mount.c
> > > +++ b/fs/xfs/xfs_mount.c
> > > @@ -1057,7 +1057,6 @@ xfs_mountfs(
> > >  		xfs_clear_using_logged_xattrs(mp);
> > >  
> > >  	/* Enable background inode inactivation workers. */
> > > -	xfs_inodegc_start(mp);
> > >  	xfs_blockgc_start(mp);
> > >  
> > >  	if (xfs_has_metadir(mp)) {
> > > @@ -1143,6 +1142,8 @@ xfs_mountfs(
> > >  		}
> > >  	}
> > >  
> > > +	xfs_inodegc_start(mp);
> > 
> > Why is it correct to defer inodegc until this point?  Can any code
> > between line 1060 and 1145 push an inode to the inodegc mechanism?
> > blockgc can unmap blocks from files, so why is it ok to leave the
> > blockgc_start call where it is?
> 
> Looking at this - I think actually performing inodegc before quotas
> are up sounds dangerous, as we'd miss th quota accounting.  So I
> think we need to defer starting the runners after that.  But we
> should support queuing up inodes before that.  blockgc has
> the same issues.

Hmm.  Very very occasionally I'll see one generic/388 fail in xfs_repair
with slightly wrong quota icount.  /me wonders if this might be it?  In
theory quotacheck could be run to fix the accounting inaccuracies (ugh)
but it's probably better just to defer gc startup.

My original question ("can any code push an inode to gc...?") remains.

--D

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-04-15 15:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-13  0:49 [PATCH] xfs: start inodegc after quota initialization Pedro Demarchi Gomes
2026-04-13 18:09 ` Darrick J. Wong
2026-04-15  7:26   ` Christoph Hellwig
2026-04-15 15:12     ` Darrick J. Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox