From: Dave Chinner <david@fromorbit.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: Christoph Hellwig <hch@lst.de>,
chandan.babu@oracle.com, linux-xfs@vger.kernel.org
Subject: Re: [PATCH] xfs: compile out v4 support if disabled
Date: Wed, 20 Mar 2024 08:13:21 +1100 [thread overview]
Message-ID: <Zfn/8e7MhbFcvHL0@dread.disaster.area> (raw)
In-Reply-To: <20240319175909.GY1927156@frogsfrogsfrogs>
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
next prev parent reply other threads:[~2024-03-22 0:55 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2024-03-22 3:24 ` Christoph Hellwig
2024-03-19 21:05 ` Dave Chinner
2024-03-22 3:25 ` Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zfn/8e7MhbFcvHL0@dread.disaster.area \
--to=david@fromorbit.com \
--cc=chandan.babu@oracle.com \
--cc=djwong@kernel.org \
--cc=hch@lst.de \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox