Message ID | 20170904174535.GI4671@magnolia (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
On 9/4/17 12:45 PM, Darrick J. Wong wrote: > Create a -d option to mkfs.xfs that enables administrators to set > the CoW extent size hint on the created files. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > v2: port flags2diflags from the kernel hm, I wonder if we need a libxfs/util.c in the kernel for stuff like this so libxfs diff will keep such things in sync in the future. a project for another day I guess. One question below. > --- > libxfs/util.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++-- > man/man8/mkfs.xfs.8 | 7 ++++++ > mkfs/xfs_mkfs.c | 20 ++++++++++++++++ > 3 files changed, 89 insertions(+), 2 deletions(-) > > diff --git a/libxfs/util.c b/libxfs/util.c > index 0e2f29e..b600594 100644 > --- a/libxfs/util.c > +++ b/libxfs/util.c > @@ -175,6 +175,64 @@ libxfs_trans_ichgtime( > } > } > > +STATIC uint16_t > +xfs_flags2diflags( > + struct xfs_inode *ip, > + unsigned int xflags) > +{ > + /* can't set PREALLOC this way, just preserve it */ > + uint16_t di_flags = > + (ip->i_d.di_flags & XFS_DIFLAG_PREALLOC); > + > + if (xflags & FS_XFLAG_IMMUTABLE) > + di_flags |= XFS_DIFLAG_IMMUTABLE; > + if (xflags & FS_XFLAG_APPEND) > + di_flags |= XFS_DIFLAG_APPEND; > + if (xflags & FS_XFLAG_SYNC) > + di_flags |= XFS_DIFLAG_SYNC; > + if (xflags & FS_XFLAG_NOATIME) > + di_flags |= XFS_DIFLAG_NOATIME; > + if (xflags & FS_XFLAG_NODUMP) > + di_flags |= XFS_DIFLAG_NODUMP; > + if (xflags & FS_XFLAG_NODEFRAG) > + di_flags |= XFS_DIFLAG_NODEFRAG; > + if (xflags & FS_XFLAG_FILESTREAM) > + di_flags |= XFS_DIFLAG_FILESTREAM; > + if (S_ISDIR(VFS_I(ip)->i_mode)) { > + if (xflags & FS_XFLAG_RTINHERIT) > + di_flags |= XFS_DIFLAG_RTINHERIT; > + if (xflags & FS_XFLAG_NOSYMLINKS) > + di_flags |= XFS_DIFLAG_NOSYMLINKS; > + if (xflags & FS_XFLAG_EXTSZINHERIT) > + di_flags |= XFS_DIFLAG_EXTSZINHERIT; > + if (xflags & FS_XFLAG_PROJINHERIT) > + di_flags |= XFS_DIFLAG_PROJINHERIT; > + } else if (S_ISREG(VFS_I(ip)->i_mode)) { > + if (xflags & FS_XFLAG_REALTIME) > + di_flags |= XFS_DIFLAG_REALTIME; > + if (xflags & FS_XFLAG_EXTSIZE) > + di_flags |= XFS_DIFLAG_EXTSIZE; > + } > + > + return di_flags; > +} > + > +STATIC uint64_t > +xfs_flags2diflags2( > + struct xfs_inode *ip, > + unsigned int xflags) > +{ > + uint64_t di_flags2 = > + (ip->i_d.di_flags2 & XFS_DIFLAG2_REFLINK); > + > + if (xflags & FS_XFLAG_DAX) > + di_flags2 |= XFS_DIFLAG2_DAX; > + if (xflags & FS_XFLAG_COWEXTSIZE) > + di_flags2 |= XFS_DIFLAG2_COWEXTSIZE; > + > + return di_flags2; > +} > + > /* > * Allocate an inode on disk and return a copy of its in-core version. > * Set mode, nlink, and rdev appropriately within the inode. > @@ -254,15 +312,17 @@ libxfs_ialloc( > ip->i_d.di_extsize = pip ? 0 : fsx->fsx_extsize; > ip->i_d.di_dmevmask = 0; > ip->i_d.di_dmstate = 0; > - ip->i_d.di_flags = pip ? 0 : fsx->fsx_xflags; > + ip->i_d.di_flags = pip ? 0 : xfs_flags2diflags(ip, fsx->fsx_xflags); is this a bugfix? The rest seems ok, just wondering if the above should be hidden in here - at least a mention of the change in the changelog seems needed, but most likely thjis is a separate change, no? Or am I missing something? -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
On Fri, Sep 08, 2017 at 01:02:15PM -0500, Eric Sandeen wrote: > On 9/4/17 12:45 PM, Darrick J. Wong wrote: > > Create a -d option to mkfs.xfs that enables administrators to set > > the CoW extent size hint on the created files. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > v2: port flags2diflags from the kernel > > hm, I wonder if we need a libxfs/util.c in the kernel for stuff like > this so libxfs diff will keep such things in sync in the future. > > a project for another day I guess. One question below. > > > --- > > libxfs/util.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++-- > > man/man8/mkfs.xfs.8 | 7 ++++++ > > mkfs/xfs_mkfs.c | 20 ++++++++++++++++ > > 3 files changed, 89 insertions(+), 2 deletions(-) > > > > diff --git a/libxfs/util.c b/libxfs/util.c > > index 0e2f29e..b600594 100644 > > --- a/libxfs/util.c > > +++ b/libxfs/util.c > > @@ -175,6 +175,64 @@ libxfs_trans_ichgtime( > > } > > } > > > > +STATIC uint16_t > > +xfs_flags2diflags( > > + struct xfs_inode *ip, > > + unsigned int xflags) > > +{ > > + /* can't set PREALLOC this way, just preserve it */ > > + uint16_t di_flags = > > + (ip->i_d.di_flags & XFS_DIFLAG_PREALLOC); > > + > > + if (xflags & FS_XFLAG_IMMUTABLE) > > + di_flags |= XFS_DIFLAG_IMMUTABLE; > > + if (xflags & FS_XFLAG_APPEND) > > + di_flags |= XFS_DIFLAG_APPEND; > > + if (xflags & FS_XFLAG_SYNC) > > + di_flags |= XFS_DIFLAG_SYNC; > > + if (xflags & FS_XFLAG_NOATIME) > > + di_flags |= XFS_DIFLAG_NOATIME; > > + if (xflags & FS_XFLAG_NODUMP) > > + di_flags |= XFS_DIFLAG_NODUMP; > > + if (xflags & FS_XFLAG_NODEFRAG) > > + di_flags |= XFS_DIFLAG_NODEFRAG; > > + if (xflags & FS_XFLAG_FILESTREAM) > > + di_flags |= XFS_DIFLAG_FILESTREAM; > > + if (S_ISDIR(VFS_I(ip)->i_mode)) { > > + if (xflags & FS_XFLAG_RTINHERIT) > > + di_flags |= XFS_DIFLAG_RTINHERIT; > > + if (xflags & FS_XFLAG_NOSYMLINKS) > > + di_flags |= XFS_DIFLAG_NOSYMLINKS; > > + if (xflags & FS_XFLAG_EXTSZINHERIT) > > + di_flags |= XFS_DIFLAG_EXTSZINHERIT; > > + if (xflags & FS_XFLAG_PROJINHERIT) > > + di_flags |= XFS_DIFLAG_PROJINHERIT; > > + } else if (S_ISREG(VFS_I(ip)->i_mode)) { > > + if (xflags & FS_XFLAG_REALTIME) > > + di_flags |= XFS_DIFLAG_REALTIME; > > + if (xflags & FS_XFLAG_EXTSIZE) > > + di_flags |= XFS_DIFLAG_EXTSIZE; > > + } > > + > > + return di_flags; > > +} > > + > > +STATIC uint64_t > > +xfs_flags2diflags2( > > + struct xfs_inode *ip, > > + unsigned int xflags) > > +{ > > + uint64_t di_flags2 = > > + (ip->i_d.di_flags2 & XFS_DIFLAG2_REFLINK); > > + > > + if (xflags & FS_XFLAG_DAX) > > + di_flags2 |= XFS_DIFLAG2_DAX; > > + if (xflags & FS_XFLAG_COWEXTSIZE) > > + di_flags2 |= XFS_DIFLAG2_COWEXTSIZE; > > + > > + return di_flags2; > > +} > > + > > /* > > * Allocate an inode on disk and return a copy of its in-core version. > > * Set mode, nlink, and rdev appropriately within the inode. > > @@ -254,15 +312,17 @@ libxfs_ialloc( > > ip->i_d.di_extsize = pip ? 0 : fsx->fsx_extsize; > > ip->i_d.di_dmevmask = 0; > > ip->i_d.di_dmstate = 0; > > - ip->i_d.di_flags = pip ? 0 : fsx->fsx_xflags; > > + ip->i_d.di_flags = pip ? 0 : xfs_flags2diflags(ip, fsx->fsx_xflags); > > is this a bugfix? No. Prior to this patch, the only fsx_xflags bits that mkfs could set are the ones that correspond exactly to di_flags bits, so it was fine to set them directly. Subtle and annoying, but it worked. However, the xfs_mkfs.c changes enable us to set FS_XFLAG_COWEXTSIZE, which doesn't correspond to a di_flags bit, so now we need translation functions to return the correct di_flags/di_flags2 values for the given fsx_xflags. (Granted you could argue that COWEXTSIZE is bit 17 so it'll just get truncated when we set di_flags and therefore it's not absolutely necessary to have a translation function for di_flags but that's sloppy.) --D > The rest seems ok, just wondering if the above should be hidden in here - > at least a mention of the change in the changelog seems needed, but most > likely thjis is a separate change, no? Or am I missing something? > > -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
On 9/18/17 12:22 PM, Darrick J. Wong wrote: >>> @@ -254,15 +312,17 @@ libxfs_ialloc( >>> ip->i_d.di_extsize = pip ? 0 : fsx->fsx_extsize; >>> ip->i_d.di_dmevmask = 0; >>> ip->i_d.di_dmstate = 0; >>> - ip->i_d.di_flags = pip ? 0 : fsx->fsx_xflags; >>> + ip->i_d.di_flags = pip ? 0 : xfs_flags2diflags(ip, fsx->fsx_xflags); >> is this a bugfix? > No. > > Prior to this patch, the only fsx_xflags bits that mkfs could set are > the ones that correspond exactly to di_flags bits, so it was fine to set > them directly. Subtle and annoying, but it worked. > > However, the xfs_mkfs.c changes enable us to set FS_XFLAG_COWEXTSIZE, > which doesn't correspond to a di_flags bit, so now we need translation > functions to return the correct di_flags/di_flags2 values for the given > fsx_xflags. Oh, ok. Before I guess it was only XFS_DIFLAG_RTINHERIT, XFS_DIFLAG_PROJINHERIT and XFS_DIFLAG_EXTSZINHERIT. Fair enough, maybe I should have seen that. Thanks. (I might note it in the changelog on the way in, though, since it's subtle and annoying) ;) -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
diff --git a/libxfs/util.c b/libxfs/util.c index 0e2f29e..b600594 100644 --- a/libxfs/util.c +++ b/libxfs/util.c @@ -175,6 +175,64 @@ libxfs_trans_ichgtime( } } +STATIC uint16_t +xfs_flags2diflags( + struct xfs_inode *ip, + unsigned int xflags) +{ + /* can't set PREALLOC this way, just preserve it */ + uint16_t di_flags = + (ip->i_d.di_flags & XFS_DIFLAG_PREALLOC); + + if (xflags & FS_XFLAG_IMMUTABLE) + di_flags |= XFS_DIFLAG_IMMUTABLE; + if (xflags & FS_XFLAG_APPEND) + di_flags |= XFS_DIFLAG_APPEND; + if (xflags & FS_XFLAG_SYNC) + di_flags |= XFS_DIFLAG_SYNC; + if (xflags & FS_XFLAG_NOATIME) + di_flags |= XFS_DIFLAG_NOATIME; + if (xflags & FS_XFLAG_NODUMP) + di_flags |= XFS_DIFLAG_NODUMP; + if (xflags & FS_XFLAG_NODEFRAG) + di_flags |= XFS_DIFLAG_NODEFRAG; + if (xflags & FS_XFLAG_FILESTREAM) + di_flags |= XFS_DIFLAG_FILESTREAM; + if (S_ISDIR(VFS_I(ip)->i_mode)) { + if (xflags & FS_XFLAG_RTINHERIT) + di_flags |= XFS_DIFLAG_RTINHERIT; + if (xflags & FS_XFLAG_NOSYMLINKS) + di_flags |= XFS_DIFLAG_NOSYMLINKS; + if (xflags & FS_XFLAG_EXTSZINHERIT) + di_flags |= XFS_DIFLAG_EXTSZINHERIT; + if (xflags & FS_XFLAG_PROJINHERIT) + di_flags |= XFS_DIFLAG_PROJINHERIT; + } else if (S_ISREG(VFS_I(ip)->i_mode)) { + if (xflags & FS_XFLAG_REALTIME) + di_flags |= XFS_DIFLAG_REALTIME; + if (xflags & FS_XFLAG_EXTSIZE) + di_flags |= XFS_DIFLAG_EXTSIZE; + } + + return di_flags; +} + +STATIC uint64_t +xfs_flags2diflags2( + struct xfs_inode *ip, + unsigned int xflags) +{ + uint64_t di_flags2 = + (ip->i_d.di_flags2 & XFS_DIFLAG2_REFLINK); + + if (xflags & FS_XFLAG_DAX) + di_flags2 |= XFS_DIFLAG2_DAX; + if (xflags & FS_XFLAG_COWEXTSIZE) + di_flags2 |= XFS_DIFLAG2_COWEXTSIZE; + + return di_flags2; +} + /* * Allocate an inode on disk and return a copy of its in-core version. * Set mode, nlink, and rdev appropriately within the inode. @@ -254,15 +312,17 @@ libxfs_ialloc( ip->i_d.di_extsize = pip ? 0 : fsx->fsx_extsize; ip->i_d.di_dmevmask = 0; ip->i_d.di_dmstate = 0; - ip->i_d.di_flags = pip ? 0 : fsx->fsx_xflags; + ip->i_d.di_flags = pip ? 0 : xfs_flags2diflags(ip, fsx->fsx_xflags); if (ip->i_d.di_version == 3) { ASSERT(ip->i_d.di_ino == ino); ASSERT(uuid_equal(&ip->i_d.di_uuid, &mp->m_sb.sb_meta_uuid)); VFS_I(ip)->i_version = 1; - ip->i_d.di_flags2 = 0; + ip->i_d.di_flags2 = pip ? 0 : xfs_flags2diflags2(ip, + fsx->fsx_xflags); ip->i_d.di_crtime.t_sec = (int32_t)VFS_I(ip)->i_mtime.tv_sec; ip->i_d.di_crtime.t_nsec = (int32_t)VFS_I(ip)->i_mtime.tv_nsec; + ip->i_d.di_cowextsize = pip ? 0 : fsx->fsx_cowextsize; } flags = XFS_ILOG_CORE; diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8 index 33831c9..bbbe1c5 100644 --- a/man/man8/mkfs.xfs.8 +++ b/man/man8/mkfs.xfs.8 @@ -279,6 +279,13 @@ and .B agsize suboptions are mutually exclusive. .TP +.BI cowextsize= value +Set the copy-on-write extent size hint on all inodes created by +.BR mkfs.xfs "." +The value must be provided in units of filesystem blocks. +If the value is zero, the default value (currently 32 blocks) will be used. +Directories will pass on this hint to newly created children. +.TP .BI name= value This can be used to specify the name of the special file containing the filesystem. In this case, the log section must be specified as diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c index 024cb02..ea0a317 100644 --- a/mkfs/xfs_mkfs.c +++ b/mkfs/xfs_mkfs.c @@ -187,6 +187,8 @@ struct opt_params dopts = { "projinherit", #define D_EXTSZINHERIT 14 "extszinherit", +#define D_COWEXTSIZE 15 + "cowextsize", NULL }, .subopt_params = { @@ -303,6 +305,12 @@ struct opt_params dopts = { .maxval = UINT_MAX, .defaultval = SUBOPT_NEEDS_VAL, }, + { .index = D_COWEXTSIZE, + .conflicts = { LAST_CONFLICT }, + .minval = 0, + .maxval = UINT_MAX, + .defaultval = SUBOPT_NEEDS_VAL, + }, }, }; @@ -1638,6 +1646,13 @@ main( fsx.fsx_xflags |= XFS_DIFLAG_EXTSZINHERIT; break; + case D_COWEXTSIZE: + fsx.fsx_cowextsize = getnum(value, + &dopts, + D_COWEXTSIZE); + fsx.fsx_xflags |= + FS_XFLAG_COWEXTSIZE; + break; default: unknown('d', value); } @@ -2142,6 +2157,11 @@ _("reflink not supported without CRC support\n")); sb_feat.reflink = false; } + if ((fsx.fsx_xflags & FS_XFLAG_COWEXTSIZE) && !sb_feat.reflink) { + fprintf(stderr, +_("cowextsize not supported without reflink support\n")); + usage(); + } if (nsflag || nlflag) { if (dirblocksize < blocksize ||
Create a -d option to mkfs.xfs that enables administrators to set the CoW extent size hint on the created files. Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- v2: port flags2diflags from the kernel --- libxfs/util.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++-- man/man8/mkfs.xfs.8 | 7 ++++++ mkfs/xfs_mkfs.c | 20 ++++++++++++++++ 3 files changed, 89 insertions(+), 2 deletions(-) -- 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