Re: [PATCH v3 08/21] xfs: Introduce FORCEALIGN inode flag

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Jun 12, 2024 at 07:55:31AM +0100, John Garry wrote:
> On 12/06/2024 03:10, Long Li wrote:
> > On Mon, Apr 29, 2024 at 05:47:33PM +0000, John Garry wrote:
> > > From: "Darrick J. Wong"<djwong@kernel.org>
> > > 
> > > Add a new inode flag to require that all file data extent mappings must
> > > be aligned (both the file offset range and the allocated space itself)
> > > to the extent size hint.  Having a separate COW extent size hint is no
> > > longer allowed.
> > > 
> > > The goal here is to enable sysadmins and users to mandate that all space
> > > mappings in a file must have a startoff/blockcount that are aligned to
> > > (say) a 2MB alignment and that the startblock/blockcount will follow the
> > > same alignment.
> > > 
> > > jpg: Enforce extsize is a power-of-2 and aligned with afgsize + stripe
> > >       alignment for forcealign
> > > Signed-off-by: "Darrick J. Wong"<djwong@kernel.org>
> > > Co-developed-by: John Garry<john.g.garry@oracle.com>
> > > Signed-off-by: John Garry<john.g.garry@oracle.com>
> > > ---
> > >   fs/xfs/libxfs/xfs_format.h    |  6 ++++-
> > >   fs/xfs/libxfs/xfs_inode_buf.c | 50 +++++++++++++++++++++++++++++++++++
> > >   fs/xfs/libxfs/xfs_inode_buf.h |  3 +++
> > >   fs/xfs/libxfs/xfs_sb.c        |  2 ++
> > >   fs/xfs/xfs_inode.c            | 12 +++++++++
> > >   fs/xfs/xfs_inode.h            |  2 +-
> > >   fs/xfs/xfs_ioctl.c            | 34 +++++++++++++++++++++++-
> > >   fs/xfs/xfs_mount.h            |  2 ++
> > >   fs/xfs/xfs_super.c            |  4 +++
> > >   include/uapi/linux/fs.h       |  2 ++
> > >   10 files changed, 114 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> > > index 2b2f9050fbfb..4dd295b047f8 100644
> > > --- a/fs/xfs/libxfs/xfs_format.h
> > > +++ b/fs/xfs/libxfs/xfs_format.h
> > > @@ -353,6 +353,7 @@ xfs_sb_has_compat_feature(
> > >   #define XFS_SB_FEAT_RO_COMPAT_RMAPBT   (1 << 1)		/* reverse map btree */
> > >   #define XFS_SB_FEAT_RO_COMPAT_REFLINK  (1 << 2)		/* reflinked files */
> > >   #define XFS_SB_FEAT_RO_COMPAT_INOBTCNT (1 << 3)		/* inobt block counts */
> > > +#define XFS_SB_FEAT_RO_COMPAT_FORCEALIGN (1 << 30)	/* aligned file data extents */
> > Hi, John
> > 
> > You know I've been using and testing your atomic writes patch series recently,
> > and I'm particularly interested in the changes to the on-disk format. I noticed
> > that XFS_SB_FEAT_RO_COMPAT_FORCEALIGN uses bit 30 instead of bit 4, which would
> > be the next available bit in sequence.
> > 
> > I'm wondering if using bit 30 is just a temporary solution to avoid conflicts,
> > and if the plan is to eventually use bits sequentially, for example, using bit 4?
> > I'm looking forward to your explanation.
> 
> I really don't know. I'm looking through the history and it has been like
> that this the start of my source control records.
> 
> Maybe it was a copy-and-paste error from XFS_FEAT_FORCEALIGN, whose value
> has changed since.
> 
> Anyway, I'll ask a bit more internally, and I'll look to change to (1 << 4)
> if ok.

I tend to use upper bits for ondisk features that are still under
development so that (a) there won't be collisions with other features
getting merged and (b) after the feature I'm working on gets merged, any
old fs images in my zoo will no longer mount.

--D

> Thanks,
> John
>