linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xfs: XFS_ONLINE_SCRUB_STATS should depend on DEBUG_FS
@ 2025-10-13  8:48 Geert Uytterhoeven
  2025-10-13 16:32 ` Darrick J. Wong
  2025-10-21  9:37 ` Carlos Maiolino
  0 siblings, 2 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2025-10-13  8:48 UTC (permalink / raw)
  To: Darrick J . Wong, Carlos Maiolino
  Cc: linux-xfs, linux-kernel, Geert Uytterhoeven

Currently, XFS_ONLINE_SCRUB_STATS selects DEBUG_FS.  However, DEBUG_FS
is meant for debugging, and people may want to disable it on production
systems.  Since commit 0ff51a1fd786f47b ("xfs: enable online fsck by
default in Kconfig")), XFS_ONLINE_SCRUB_STATS is enabled by default,
forcing DEBUG_FS enabled too.

Fix this by replacing the selection of DEBUG_FS by a dependency on
DEBUG_FS, which is what most other options controlling the gathering and
exposing of statistics do.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 fs/xfs/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/Kconfig b/fs/xfs/Kconfig
index 8930d5254e1da61d..402cf7aad5ca93ab 100644
--- a/fs/xfs/Kconfig
+++ b/fs/xfs/Kconfig
@@ -156,7 +156,7 @@ config XFS_ONLINE_SCRUB_STATS
 	bool "XFS online metadata check usage data collection"
 	default y
 	depends on XFS_ONLINE_SCRUB
-	select DEBUG_FS
+	depends on DEBUG_FS
 	help
 	  If you say Y here, the kernel will gather usage data about
 	  the online metadata check subsystem.  This includes the number
-- 
2.43.0


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

* Re: [PATCH] xfs: XFS_ONLINE_SCRUB_STATS should depend on DEBUG_FS
  2025-10-13  8:48 [PATCH] xfs: XFS_ONLINE_SCRUB_STATS should depend on DEBUG_FS Geert Uytterhoeven
@ 2025-10-13 16:32 ` Darrick J. Wong
  2025-10-14  7:10   ` Geert Uytterhoeven
  2025-10-21  9:37 ` Carlos Maiolino
  1 sibling, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2025-10-13 16:32 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Carlos Maiolino, linux-xfs, linux-kernel

On Mon, Oct 13, 2025 at 10:48:46AM +0200, Geert Uytterhoeven wrote:
> Currently, XFS_ONLINE_SCRUB_STATS selects DEBUG_FS.  However, DEBUG_FS
> is meant for debugging, and people may want to disable it on production
> systems.  Since commit 0ff51a1fd786f47b ("xfs: enable online fsck by
> default in Kconfig")), XFS_ONLINE_SCRUB_STATS is enabled by default,
> forcing DEBUG_FS enabled too.
> 
> Fix this by replacing the selection of DEBUG_FS by a dependency on
> DEBUG_FS, which is what most other options controlling the gathering and
> exposing of statistics do.
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>  fs/xfs/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/Kconfig b/fs/xfs/Kconfig
> index 8930d5254e1da61d..402cf7aad5ca93ab 100644
> --- a/fs/xfs/Kconfig
> +++ b/fs/xfs/Kconfig
> @@ -156,7 +156,7 @@ config XFS_ONLINE_SCRUB_STATS
>  	bool "XFS online metadata check usage data collection"
>  	default y
>  	depends on XFS_ONLINE_SCRUB
> -	select DEBUG_FS
> +	depends on DEBUG_FS

Looks ok to me, though I wonder why there are so many "select DEBUG_FS"
in the kernel?

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

>  	help
>  	  If you say Y here, the kernel will gather usage data about
>  	  the online metadata check subsystem.  This includes the number
> -- 
> 2.43.0
> 
> 

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

* Re: [PATCH] xfs: XFS_ONLINE_SCRUB_STATS should depend on DEBUG_FS
  2025-10-13 16:32 ` Darrick J. Wong
@ 2025-10-14  7:10   ` Geert Uytterhoeven
  2025-10-14 21:05     ` Darrick J. Wong
  0 siblings, 1 reply; 5+ messages in thread
From: Geert Uytterhoeven @ 2025-10-14  7:10 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Carlos Maiolino, linux-xfs, linux-kernel

Hi Darrick,

On Mon, 13 Oct 2025 at 18:32, Darrick J. Wong <djwong@kernel.org> wrote:
> On Mon, Oct 13, 2025 at 10:48:46AM +0200, Geert Uytterhoeven wrote:
> > Currently, XFS_ONLINE_SCRUB_STATS selects DEBUG_FS.  However, DEBUG_FS
> > is meant for debugging, and people may want to disable it on production
> > systems.  Since commit 0ff51a1fd786f47b ("xfs: enable online fsck by
> > default in Kconfig")), XFS_ONLINE_SCRUB_STATS is enabled by default,
> > forcing DEBUG_FS enabled too.
> >
> > Fix this by replacing the selection of DEBUG_FS by a dependency on
> > DEBUG_FS, which is what most other options controlling the gathering and
> > exposing of statistics do.
> >
> > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > ---
> >  fs/xfs/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/xfs/Kconfig b/fs/xfs/Kconfig
> > index 8930d5254e1da61d..402cf7aad5ca93ab 100644
> > --- a/fs/xfs/Kconfig
> > +++ b/fs/xfs/Kconfig
> > @@ -156,7 +156,7 @@ config XFS_ONLINE_SCRUB_STATS
> >       bool "XFS online metadata check usage data collection"
> >       default y
> >       depends on XFS_ONLINE_SCRUB
> > -     select DEBUG_FS
> > +     depends on DEBUG_FS
>
> Looks ok to me, though I wonder why there are so many "select DEBUG_FS"
> in the kernel?

I think select is OK for pure debug functionality, which is not enabled
unless really wanted; depends on is better for optional features like
statistics, especially if they default to y.

Alternatively, the "default y" could be dropped from
XFS_ONLINE_SCRUB_STATS?

> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH] xfs: XFS_ONLINE_SCRUB_STATS should depend on DEBUG_FS
  2025-10-14  7:10   ` Geert Uytterhoeven
@ 2025-10-14 21:05     ` Darrick J. Wong
  0 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2025-10-14 21:05 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Carlos Maiolino, linux-xfs, linux-kernel

On Tue, Oct 14, 2025 at 09:10:03AM +0200, Geert Uytterhoeven wrote:
> Hi Darrick,
> 
> On Mon, 13 Oct 2025 at 18:32, Darrick J. Wong <djwong@kernel.org> wrote:
> > On Mon, Oct 13, 2025 at 10:48:46AM +0200, Geert Uytterhoeven wrote:
> > > Currently, XFS_ONLINE_SCRUB_STATS selects DEBUG_FS.  However, DEBUG_FS
> > > is meant for debugging, and people may want to disable it on production
> > > systems.  Since commit 0ff51a1fd786f47b ("xfs: enable online fsck by
> > > default in Kconfig")), XFS_ONLINE_SCRUB_STATS is enabled by default,
> > > forcing DEBUG_FS enabled too.
> > >
> > > Fix this by replacing the selection of DEBUG_FS by a dependency on
> > > DEBUG_FS, which is what most other options controlling the gathering and
> > > exposing of statistics do.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > > ---
> > >  fs/xfs/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/fs/xfs/Kconfig b/fs/xfs/Kconfig
> > > index 8930d5254e1da61d..402cf7aad5ca93ab 100644
> > > --- a/fs/xfs/Kconfig
> > > +++ b/fs/xfs/Kconfig
> > > @@ -156,7 +156,7 @@ config XFS_ONLINE_SCRUB_STATS
> > >       bool "XFS online metadata check usage data collection"
> > >       default y
> > >       depends on XFS_ONLINE_SCRUB
> > > -     select DEBUG_FS
> > > +     depends on DEBUG_FS
> >
> > Looks ok to me, though I wonder why there are so many "select DEBUG_FS"
> > in the kernel?
> 
> I think select is OK for pure debug functionality, which is not enabled
> unless really wanted; depends on is better for optional features like
> statistics, especially if they default to y.
> 
> Alternatively, the "default y" could be dropped from
> XFS_ONLINE_SCRUB_STATS?

Well it /would/ be helpful for support to be able to gather usage stats
on this still rather n{ew,ovel} feature. :)

--D

> > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> 
> Thanks!
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 

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

* Re: [PATCH] xfs: XFS_ONLINE_SCRUB_STATS should depend on DEBUG_FS
  2025-10-13  8:48 [PATCH] xfs: XFS_ONLINE_SCRUB_STATS should depend on DEBUG_FS Geert Uytterhoeven
  2025-10-13 16:32 ` Darrick J. Wong
@ 2025-10-21  9:37 ` Carlos Maiolino
  1 sibling, 0 replies; 5+ messages in thread
From: Carlos Maiolino @ 2025-10-21  9:37 UTC (permalink / raw)
  To: Darrick J . Wong, Geert Uytterhoeven; +Cc: linux-xfs, linux-kernel

On Mon, 13 Oct 2025 10:48:46 +0200, Geert Uytterhoeven wrote:
> Currently, XFS_ONLINE_SCRUB_STATS selects DEBUG_FS.  However, DEBUG_FS
> is meant for debugging, and people may want to disable it on production
> systems.  Since commit 0ff51a1fd786f47b ("xfs: enable online fsck by
> default in Kconfig")), XFS_ONLINE_SCRUB_STATS is enabled by default,
> forcing DEBUG_FS enabled too.
> 
> Fix this by replacing the selection of DEBUG_FS by a dependency on
> DEBUG_FS, which is what most other options controlling the gathering and
> exposing of statistics do.
> 
> [...]

Applied to for-next, thanks!

[1/1] xfs: XFS_ONLINE_SCRUB_STATS should depend on DEBUG_FS
      commit: f5caeb3689ea2d8a8c0790d9eea68b63e8f15496

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>


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

end of thread, other threads:[~2025-10-21  9:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-13  8:48 [PATCH] xfs: XFS_ONLINE_SCRUB_STATS should depend on DEBUG_FS Geert Uytterhoeven
2025-10-13 16:32 ` Darrick J. Wong
2025-10-14  7:10   ` Geert Uytterhoeven
2025-10-14 21:05     ` Darrick J. Wong
2025-10-21  9:37 ` Carlos Maiolino

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).