diff mbox

[1/4] xfs: add scrub to XFS_BUILD_OPTIONS

Message ID f5769e20-970a-53f5-4fc7-2afd11304ce2@sandeen.net (mailing list archive)
State Accepted
Headers show

Commit Message

Eric Sandeen Dec. 1, 2017, 10:12 p.m. UTC
Advertise this config option along with the others.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---


--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Darrick J. Wong Dec. 1, 2017, 10:29 p.m. UTC | #1
On Fri, Dec 01, 2017 at 04:12:34PM -0600, Eric Sandeen wrote:
> Advertise this config option along with the others.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/fs/xfs/xfs_super.h b/fs/xfs/xfs_super.h
> index fcc5dfc..8cee8e8 100644
> --- a/fs/xfs/xfs_super.h
> +++ b/fs/xfs/xfs_super.h
> @@ -44,6 +44,12 @@
>  # define XFS_REALTIME_STRING
>  #endif
>  
> +#ifdef CONFIG_XFS_ONLINE_SCRUB
> +# define XFS_SCRUB_STRING	"scrub, "
> +#else
> +# define XFS_SCRUB_STRING
> +#endif

I don't mind this addition to the build options, though I had pictured
scrub becoming an integral part of xfs some day and not remaining a
configurable option.

That said I haven't evaluated how much scrub bloats up xfs nor do I know
if the kernel tinyfication people would actually want to turn it off??

(For my part I prefer it some day not be Kconfig option so that it won't
bitrot ala CONFIG_XFS_RT=y.)

--D

> +
>  #ifdef DEBUG
>  # define XFS_DBG_STRING		"debug"
>  #else
> @@ -54,6 +60,7 @@
>  #define XFS_BUILD_OPTIONS	XFS_ACL_STRING \
>  				XFS_SECURITY_STRING \
>  				XFS_REALTIME_STRING \
> +				XFS_SCRUB_STRING \
>  				XFS_DBG_STRING /* DBG must be last */
>  
>  struct xfs_inode;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Sandeen Dec. 1, 2017, 10:31 p.m. UTC | #2
On 12/1/17 4:29 PM, Darrick J. Wong wrote:
> On Fri, Dec 01, 2017 at 04:12:34PM -0600, Eric Sandeen wrote:
>> Advertise this config option along with the others.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/fs/xfs/xfs_super.h b/fs/xfs/xfs_super.h
>> index fcc5dfc..8cee8e8 100644
>> --- a/fs/xfs/xfs_super.h
>> +++ b/fs/xfs/xfs_super.h
>> @@ -44,6 +44,12 @@
>>  # define XFS_REALTIME_STRING
>>  #endif
>>  
>> +#ifdef CONFIG_XFS_ONLINE_SCRUB
>> +# define XFS_SCRUB_STRING	"scrub, "
>> +#else
>> +# define XFS_SCRUB_STRING
>> +#endif
> 
> I don't mind this addition to the build options, though I had pictured
> scrub becoming an integral part of xfs some day and not remaining a
> configurable option.

It could be removed at that time, unless it's considered an API?

I'm not hung up on this, if there's no reason for it that's OK.

> That said I haven't evaluated how much scrub bloats up xfs nor do I know
> if the kernel tinyfication people would actually want to turn it off??
> 
> (For my part I prefer it some day not be Kconfig option so that it won't
> bitrot ala CONFIG_XFS_RT=y.)

I'd imagine that it'd be turned on more often than RT but that's just a guess.

-Eric
 
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darrick J. Wong Feb. 1, 2018, 12:03 a.m. UTC | #3
On Fri, Dec 01, 2017 at 04:31:04PM -0600, Eric Sandeen wrote:
> On 12/1/17 4:29 PM, Darrick J. Wong wrote:
> > On Fri, Dec 01, 2017 at 04:12:34PM -0600, Eric Sandeen wrote:
> >> Advertise this config option along with the others.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >> ---
> >>
> >> diff --git a/fs/xfs/xfs_super.h b/fs/xfs/xfs_super.h
> >> index fcc5dfc..8cee8e8 100644
> >> --- a/fs/xfs/xfs_super.h
> >> +++ b/fs/xfs/xfs_super.h
> >> @@ -44,6 +44,12 @@
> >>  # define XFS_REALTIME_STRING
> >>  #endif
> >>  
> >> +#ifdef CONFIG_XFS_ONLINE_SCRUB
> >> +# define XFS_SCRUB_STRING	"scrub, "
> >> +#else
> >> +# define XFS_SCRUB_STRING
> >> +#endif
> > 
> > I don't mind this addition to the build options, though I had pictured
> > scrub becoming an integral part of xfs some day and not remaining a
> > configurable option.
> 
> It could be removed at that time, unless it's considered an API?
> 
> I'm not hung up on this, if there's no reason for it that's OK.
> 
> > That said I haven't evaluated how much scrub bloats up xfs nor do I know
> > if the kernel tinyfication people would actually want to turn it off??
> > 
> > (For my part I prefer it some day not be Kconfig option so that it won't
> > bitrot ala CONFIG_XFS_RT=y.)
> 
> I'd imagine that it'd be turned on more often than RT but that's just a guess.

Ok, now that we've merged all this stuff, it looks like disabling the
scrub code alone can reduce the module size by ~8%.  I can imagine the
occasional system builder who wants xfs but doesn't care about online
fsck, so I think I'll keep the kconfig option.  Therefore, it ought to
go in the build string.

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> 
> -Eric
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/xfs/xfs_super.h b/fs/xfs/xfs_super.h
index fcc5dfc..8cee8e8 100644
--- a/fs/xfs/xfs_super.h
+++ b/fs/xfs/xfs_super.h
@@ -44,6 +44,12 @@ 
 # define XFS_REALTIME_STRING
 #endif
 
+#ifdef CONFIG_XFS_ONLINE_SCRUB
+# define XFS_SCRUB_STRING	"scrub, "
+#else
+# define XFS_SCRUB_STRING
+#endif
+
 #ifdef DEBUG
 # define XFS_DBG_STRING		"debug"
 #else
@@ -54,6 +60,7 @@ 
 #define XFS_BUILD_OPTIONS	XFS_ACL_STRING \
 				XFS_SECURITY_STRING \
 				XFS_REALTIME_STRING \
+				XFS_SCRUB_STRING \
 				XFS_DBG_STRING /* DBG must be last */
 
 struct xfs_inode;