* [PATCH] xfs: compile out v4 support if disabled
@ 2024-03-19 7:19 Christoph Hellwig
2024-03-19 17:59 ` Darrick J. Wong
2024-03-19 21:05 ` Dave Chinner
0 siblings, 2 replies; 7+ messages in thread
From: Christoph Hellwig @ 2024-03-19 7:19 UTC (permalink / raw
To: chandan.babu; +Cc: djwong, linux-xfs
Add a strategic IS_ENABLED to let the compiler eliminate the unused
non-crc code is CONFIG_XFS_SUPPORT_V4 is disabled.
This saves almost 20k worth of .text for my .config:
$ size xfs.o.*
text data bss dec hex filename
1351126 294836 592 1646554 191fda xfs.o.new
1371453 294868 592 1666913 196f61 xfs.o.old
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_mount.h | 7 ++++++-
fs/xfs/xfs_super.c | 22 +++++++++++++---------
2 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index e880aa48de68bb..24fe6e7913c49f 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -327,6 +327,12 @@ static inline void xfs_add_ ## name (struct xfs_mount *mp) \
xfs_sb_version_add ## name(&mp->m_sb); \
}
+static inline bool xfs_has_crc(struct xfs_mount *mp)
+{
+ return IS_ENABLED(CONFIG_XFS_SUPPORT_V4) &&
+ (mp->m_features & XFS_FEAT_CRC);
+}
+
/* Superblock features */
__XFS_ADD_FEAT(attr, ATTR)
__XFS_HAS_FEAT(nlink, NLINK)
@@ -341,7 +347,6 @@ __XFS_HAS_FEAT(lazysbcount, LAZYSBCOUNT)
__XFS_ADD_FEAT(attr2, ATTR2)
__XFS_HAS_FEAT(parent, PARENT)
__XFS_ADD_FEAT(projid32, PROJID32)
-__XFS_HAS_FEAT(crc, CRC)
__XFS_HAS_FEAT(v3inodes, V3INODES)
__XFS_HAS_FEAT(pquotino, PQUOTINO)
__XFS_HAS_FEAT(ftype, FTYPE)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 6828c48b15e9bd..7d972e1179255b 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1580,17 +1580,21 @@ xfs_fs_fill_super(
if (error)
goto out_free_sb;
- /* V4 support is undergoing deprecation. */
- if (!xfs_has_crc(mp)) {
-#ifdef CONFIG_XFS_SUPPORT_V4
+ /*
+ * V4 support is undergoing deprecation.
+ *
+ * Note: this has to use an open coded m_features check as xfs_has_crc
+ * always returns false for !CONFIG_XFS_SUPPORT_V4.
+ */
+ if (!(mp->m_features & XFS_FEAT_CRC)) {
+ if (!IS_ENABLED(CONFIG_XFS_SUPPORT_V4)) {
+ xfs_warn(mp,
+ "Deprecated V4 format (crc=0) not supported by kernel.");
+ error = -EINVAL;
+ goto out_free_sb;
+ }
xfs_warn_once(mp,
"Deprecated V4 format (crc=0) will not be supported after September 2030.");
-#else
- xfs_warn(mp,
- "Deprecated V4 format (crc=0) not supported by kernel.");
- error = -EINVAL;
- goto out_free_sb;
-#endif
}
/* ASCII case insensitivity is undergoing deprecation. */
--
2.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] xfs: compile out v4 support if disabled
2024-03-19 7:19 [PATCH] xfs: compile out v4 support if disabled Christoph Hellwig
@ 2024-03-19 17:59 ` Darrick J. Wong
2024-03-19 20:09 ` Christoph Hellwig
2024-03-19 21:13 ` Dave Chinner
2024-03-19 21:05 ` Dave Chinner
1 sibling, 2 replies; 7+ messages in thread
From: Darrick J. Wong @ 2024-03-19 17:59 UTC (permalink / raw
To: Christoph Hellwig; +Cc: chandan.babu, linux-xfs
On Tue, Mar 19, 2024 at 05:19:51PM +1000, Christoph Hellwig wrote:
> Add a strategic IS_ENABLED to let the compiler eliminate the unused
> non-crc code is CONFIG_XFS_SUPPORT_V4 is disabled.
>
> This saves almost 20k worth of .text for my .config:
>
> $ size xfs.o.*
> text data bss dec hex filename
> 1351126 294836 592 1646554 191fda xfs.o.new
> 1371453 294868 592 1666913 196f61 xfs.o.old
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_mount.h | 7 ++++++-
> fs/xfs/xfs_super.c | 22 +++++++++++++---------
> 2 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index e880aa48de68bb..24fe6e7913c49f 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -327,6 +327,12 @@ static inline void xfs_add_ ## name (struct xfs_mount *mp) \
> xfs_sb_version_add ## name(&mp->m_sb); \
> }
>
> +static inline bool xfs_has_crc(struct xfs_mount *mp)
> +{
> + return IS_ENABLED(CONFIG_XFS_SUPPORT_V4) &&
> + (mp->m_features & XFS_FEAT_CRC);
Can you save even more text bytes by defining
xfs_has_{nlink,v3inodes,projid32,lazysbcount,pquotino,attr2} to 1?
And I guess defining noattr2 to 0?
--D
> +}
> +
> /* Superblock features */
> __XFS_ADD_FEAT(attr, ATTR)
> __XFS_HAS_FEAT(nlink, NLINK)
> @@ -341,7 +347,6 @@ __XFS_HAS_FEAT(lazysbcount, LAZYSBCOUNT)
> __XFS_ADD_FEAT(attr2, ATTR2)
> __XFS_HAS_FEAT(parent, PARENT)
> __XFS_ADD_FEAT(projid32, PROJID32)
> -__XFS_HAS_FEAT(crc, CRC)
> __XFS_HAS_FEAT(v3inodes, V3INODES)
> __XFS_HAS_FEAT(pquotino, PQUOTINO)
> __XFS_HAS_FEAT(ftype, FTYPE)
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 6828c48b15e9bd..7d972e1179255b 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1580,17 +1580,21 @@ xfs_fs_fill_super(
> if (error)
> goto out_free_sb;
>
> - /* V4 support is undergoing deprecation. */
> - if (!xfs_has_crc(mp)) {
> -#ifdef CONFIG_XFS_SUPPORT_V4
> + /*
> + * V4 support is undergoing deprecation.
> + *
> + * Note: this has to use an open coded m_features check as xfs_has_crc
> + * always returns false for !CONFIG_XFS_SUPPORT_V4.
> + */
> + if (!(mp->m_features & XFS_FEAT_CRC)) {
> + if (!IS_ENABLED(CONFIG_XFS_SUPPORT_V4)) {
> + xfs_warn(mp,
> + "Deprecated V4 format (crc=0) not supported by kernel.");
> + error = -EINVAL;
> + goto out_free_sb;
> + }
> xfs_warn_once(mp,
> "Deprecated V4 format (crc=0) will not be supported after September 2030.");
> -#else
> - xfs_warn(mp,
> - "Deprecated V4 format (crc=0) not supported by kernel.");
> - error = -EINVAL;
> - goto out_free_sb;
> -#endif
> }
>
> /* ASCII case insensitivity is undergoing deprecation. */
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xfs: compile out v4 support if disabled
2024-03-19 17:59 ` Darrick J. Wong
@ 2024-03-19 20:09 ` Christoph Hellwig
2024-03-19 21:13 ` Dave Chinner
1 sibling, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2024-03-19 20:09 UTC (permalink / raw
To: Darrick J. Wong; +Cc: Christoph Hellwig, chandan.babu, linux-xfs
On Tue, Mar 19, 2024 at 10:59:09AM -0700, Darrick J. Wong wrote:
> > +static inline bool xfs_has_crc(struct xfs_mount *mp)
> > +{
> > + return IS_ENABLED(CONFIG_XFS_SUPPORT_V4) &&
> > + (mp->m_features & XFS_FEAT_CRC);
>
> Can you save even more text bytes by defining
> xfs_has_{nlink,v3inodes,projid32,lazysbcount,pquotino,attr2} to 1?
> And I guess defining noattr2 to 0?
I guess I can give it a try.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xfs: compile out v4 support if disabled
2024-03-19 7:19 [PATCH] xfs: compile out v4 support if disabled Christoph Hellwig
2024-03-19 17:59 ` Darrick J. Wong
@ 2024-03-19 21:05 ` Dave Chinner
2024-03-22 3:25 ` Christoph Hellwig
1 sibling, 1 reply; 7+ messages in thread
From: Dave Chinner @ 2024-03-19 21:05 UTC (permalink / raw
To: Christoph Hellwig; +Cc: chandan.babu, djwong, linux-xfs
On Tue, Mar 19, 2024 at 05:19:51PM +1000, Christoph Hellwig wrote:
> Add a strategic IS_ENABLED to let the compiler eliminate the unused
> non-crc code is CONFIG_XFS_SUPPORT_V4 is disabled.
>
> This saves almost 20k worth of .text for my .config:
>
> $ size xfs.o.*
> text data bss dec hex filename
> 1351126 294836 592 1646554 191fda xfs.o.new
> 1371453 294868 592 1666913 196f61 xfs.o.old
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_mount.h | 7 ++++++-
> fs/xfs/xfs_super.c | 22 +++++++++++++---------
> 2 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index e880aa48de68bb..24fe6e7913c49f 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -327,6 +327,12 @@ static inline void xfs_add_ ## name (struct xfs_mount *mp) \
> xfs_sb_version_add ## name(&mp->m_sb); \
> }
>
> +static inline bool xfs_has_crc(struct xfs_mount *mp)
> +{
> + return IS_ENABLED(CONFIG_XFS_SUPPORT_V4) &&
> + (mp->m_features & XFS_FEAT_CRC);
> +}
Is that right? This can only return true if V4 support is compiled
in, but it should be the other way around - always return true if
V4 support is not compiled in. i.e:
if (!IS_ENABLED(CONFIG_XFS_SUPPORT_V4))
return true;
return mp->m_features & XFS_FEAT_CRC;
-Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xfs: compile out v4 support if disabled
2024-03-19 17:59 ` Darrick J. Wong
2024-03-19 20:09 ` Christoph Hellwig
@ 2024-03-19 21:13 ` Dave Chinner
2024-03-22 3:24 ` Christoph Hellwig
1 sibling, 1 reply; 7+ messages in thread
From: Dave Chinner @ 2024-03-19 21:13 UTC (permalink / raw
To: Darrick J. Wong; +Cc: Christoph Hellwig, chandan.babu, linux-xfs
On Tue, Mar 19, 2024 at 10:59:09AM -0700, Darrick J. Wong wrote:
> On Tue, Mar 19, 2024 at 05:19:51PM +1000, Christoph Hellwig wrote:
> > Add a strategic IS_ENABLED to let the compiler eliminate the unused
> > non-crc code is CONFIG_XFS_SUPPORT_V4 is disabled.
> >
> > This saves almost 20k worth of .text for my .config:
> >
> > $ size xfs.o.*
> > text data bss dec hex filename
> > 1351126 294836 592 1646554 191fda xfs.o.new
> > 1371453 294868 592 1666913 196f61 xfs.o.old
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> > fs/xfs/xfs_mount.h | 7 ++++++-
> > fs/xfs/xfs_super.c | 22 +++++++++++++---------
> > 2 files changed, 19 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> > index e880aa48de68bb..24fe6e7913c49f 100644
> > --- a/fs/xfs/xfs_mount.h
> > +++ b/fs/xfs/xfs_mount.h
> > @@ -327,6 +327,12 @@ static inline void xfs_add_ ## name (struct xfs_mount *mp) \
> > xfs_sb_version_add ## name(&mp->m_sb); \
> > }
> >
> > +static inline bool xfs_has_crc(struct xfs_mount *mp)
> > +{
> > + return IS_ENABLED(CONFIG_XFS_SUPPORT_V4) &&
> > + (mp->m_features & XFS_FEAT_CRC);
>
> Can you save even more text bytes by defining
> xfs_has_{nlink,v3inodes,projid32,lazysbcount,pquotino,attr2} to 1?
> And I guess defining noattr2 to 0?
That sounds like a new __XFS_HAS_V4_FEAT() that has a thrid
parameter to define the value when V4 support is not compiled in.
e.g.
#define __XFS_HAS_V4_FEAT(name, NAME, v5_support) \
static inline bool xfs_has_ ## name (struct xfs_mount *mp) \
{ \
if (!IS_ENABLED(CONFIG_XFS_SUPPORT_V4)) \
return v5_support; \
return mp->m_features & XFS_FEAT_ ## NAME; \
}
That way we have a single macro that tells us that it is a V4
defined feature, and the documents the support of that feature in V5
filesystems. And when it comes to removing v4 support, we have clear
code documentation as to which features we need to remove or make
unconditional across the code base.
-Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xfs: compile out v4 support if disabled
2024-03-19 21:13 ` Dave Chinner
@ 2024-03-22 3:24 ` Christoph Hellwig
0 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2024-03-22 3:24 UTC (permalink / raw
To: Dave Chinner; +Cc: Darrick J. Wong, Christoph Hellwig, chandan.babu, linux-xfs
On Wed, Mar 20, 2024 at 08:13:21AM +1100, Dave Chinner wrote:
> That sounds like a new __XFS_HAS_V4_FEAT() that has a thrid
> parameter to define the value when V4 support is not compiled in.
That's exactly what I did yesterday. I'll post the updated version
when I find time.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xfs: compile out v4 support if disabled
2024-03-19 21:05 ` Dave Chinner
@ 2024-03-22 3:25 ` Christoph Hellwig
0 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2024-03-22 3:25 UTC (permalink / raw
To: Dave Chinner; +Cc: Christoph Hellwig, chandan.babu, djwong, linux-xfs
On Wed, Mar 20, 2024 at 08:05:13AM +1100, Dave Chinner wrote:
> Is that right? This can only return true if V4 support is compiled
> in, but it should be the other way around - always return true if
> V4 support is not compiled in. i.e:
Yes. I actually also fixed that when I redid the patch. The downside
is that despite the additional stubs we actually remove less code now.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-03-22 3:25 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-19 7:19 [PATCH] xfs: compile out v4 support if disabled Christoph Hellwig
2024-03-19 17:59 ` Darrick J. Wong
2024-03-19 20:09 ` Christoph Hellwig
2024-03-19 21:13 ` Dave Chinner
2024-03-22 3:24 ` Christoph Hellwig
2024-03-19 21:05 ` Dave Chinner
2024-03-22 3:25 ` Christoph Hellwig
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 read-only IMAP folder(s) and NNTP newsgroup(s).