* [PATCH 0/2] xfsprogs: various small enhancements @ 2020-08-11 16:42 Darrick J. Wong 2020-08-11 16:42 ` [PATCH 1/2] xfs_db: fix nlink usage in check Darrick J. Wong 2020-08-11 16:42 ` [PATCH 2/2] mkfs: allow setting dax flag on root directory Darrick J. Wong 0 siblings, 2 replies; 8+ messages in thread From: Darrick J. Wong @ 2020-08-11 16:42 UTC (permalink / raw) To: sandeen, darrick.wong; +Cc: linux-xfs Hi all, This short series contains a couple of enhancements. The first patch fixes a bug in xfs_check that we introduced during the 5.8 resync. The second patch allows administrators to set the DAX inode flag on the entire filesystem at format time. If you're going to start using this mess, you probably ought to just pull from my git trees, which are linked below. This is an extraordinary way to destroy everything. Enjoy! Comments and questions are, as always, welcome. --D xfsprogs git tree: https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=xfsprogs-5.8-fixes --- db/check.c | 4 ++-- man/man8/mkfs.xfs.8 | 11 +++++++++++ mkfs/xfs_mkfs.c | 14 ++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] xfs_db: fix nlink usage in check 2020-08-11 16:42 [PATCH 0/2] xfsprogs: various small enhancements Darrick J. Wong @ 2020-08-11 16:42 ` Darrick J. Wong 2020-08-11 19:25 ` Eric Sandeen 2020-08-11 16:42 ` [PATCH 2/2] mkfs: allow setting dax flag on root directory Darrick J. Wong 1 sibling, 1 reply; 8+ messages in thread From: Darrick J. Wong @ 2020-08-11 16:42 UTC (permalink / raw) To: sandeen, darrick.wong; +Cc: linux-xfs From: Darrick J. Wong <darrick.wong@oracle.com> process_inode uses a local convenience variable to abstract the differences between the ondisk nlink fields in a v1 inode and a v2 inode. Use this variable for checking and reporting errors. Fixes: 6526f30e4801 ("xfs_db: stop misusing an onstack inode") Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- db/check.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/check.c b/db/check.c index c2233a0d1ba7..ef0e82d4efa1 100644 --- a/db/check.c +++ b/db/check.c @@ -2797,10 +2797,10 @@ process_inode( be64_to_cpu(dip->di_nblocks), ino); error++; } - if (dip->di_nlink != 0) { + if (nlink != 0) { if (v) dbprintf(_("bad nlink %d for free inode %lld\n"), - be32_to_cpu(dip->di_nlink), ino); + nlink, ino); error++; } if (dip->di_mode != 0) { ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] xfs_db: fix nlink usage in check 2020-08-11 16:42 ` [PATCH 1/2] xfs_db: fix nlink usage in check Darrick J. Wong @ 2020-08-11 19:25 ` Eric Sandeen 0 siblings, 0 replies; 8+ messages in thread From: Eric Sandeen @ 2020-08-11 19:25 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs On 8/11/20 9:42 AM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > process_inode uses a local convenience variable to abstract the > differences between the ondisk nlink fields in a v1 inode and a v2 > inode. Use this variable for checking and reporting errors. Ah, otherwise we risk looking at the V2 field for a V1 inode, right? Reviewed-by: Eric Sandeen <sandeen@redhat.com> > Fixes: 6526f30e4801 ("xfs_db: stop misusing an onstack inode") > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > db/check.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > > diff --git a/db/check.c b/db/check.c > index c2233a0d1ba7..ef0e82d4efa1 100644 > --- a/db/check.c > +++ b/db/check.c > @@ -2797,10 +2797,10 @@ process_inode( > be64_to_cpu(dip->di_nblocks), ino); > error++; > } > - if (dip->di_nlink != 0) { > + if (nlink != 0) { > if (v) > dbprintf(_("bad nlink %d for free inode %lld\n"), > - be32_to_cpu(dip->di_nlink), ino); > + nlink, ino); > error++; > } > if (dip->di_mode != 0) { > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] mkfs: allow setting dax flag on root directory 2020-08-11 16:42 [PATCH 0/2] xfsprogs: various small enhancements Darrick J. Wong 2020-08-11 16:42 ` [PATCH 1/2] xfs_db: fix nlink usage in check Darrick J. Wong @ 2020-08-11 16:42 ` Darrick J. Wong 2020-08-11 19:39 ` Eric Sandeen 1 sibling, 1 reply; 8+ messages in thread From: Darrick J. Wong @ 2020-08-11 16:42 UTC (permalink / raw) To: sandeen, darrick.wong; +Cc: linux-xfs From: Darrick J. Wong <darrick.wong@oracle.com> Teach mkfs to set the DAX flag on the root directory so that all new files can be created in dax mode. This is a complement to removing the mount option. Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- man/man8/mkfs.xfs.8 | 11 +++++++++++ mkfs/xfs_mkfs.c | 14 ++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8 index 9d762a43011a..4b4fdd86b2f4 100644 --- a/man/man8/mkfs.xfs.8 +++ b/man/man8/mkfs.xfs.8 @@ -394,6 +394,17 @@ All inodes created by will have this extent size hint applied. The value must be provided in units of filesystem blocks. Directories will pass on this hint to newly created children. +.TP +.BI dax= value +All inodes created by +.B mkfs.xfs +will have the DAX flag set. +This means that directories will pass the flag on to newly created files +and files will use the DAX IO paths when possible. +This value is either 1 to enable the use or 0 to disable. +By default, +.B mkfs.xfs +will not enable DAX mode. .RE .TP .B \-f diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c index 2e6cd280e388..33507f6ea21c 100644 --- a/mkfs/xfs_mkfs.c +++ b/mkfs/xfs_mkfs.c @@ -60,6 +60,7 @@ enum { D_PROJINHERIT, D_EXTSZINHERIT, D_COWEXTSIZE, + D_DAX, D_MAX_OPTS, }; @@ -254,6 +255,7 @@ static struct opt_params dopts = { [D_PROJINHERIT] = "projinherit", [D_EXTSZINHERIT] = "extszinherit", [D_COWEXTSIZE] = "cowextsize", + [D_DAX] = "dax", }, .subopt_params = { { .index = D_AGCOUNT, @@ -369,6 +371,12 @@ static struct opt_params dopts = { .maxval = UINT_MAX, .defaultval = SUBOPT_NEEDS_VAL, }, + { .index = D_DAX, + .conflicts = { { NULL, LAST_CONFLICT } }, + .minval = 0, + .maxval = 1, + .defaultval = 1, + }, }, }; @@ -1434,6 +1442,12 @@ data_opts_parser( cli->fsx.fsx_cowextsize = getnum(value, opts, subopt); cli->fsx.fsx_xflags |= FS_XFLAG_COWEXTSIZE; break; + case D_DAX: + if (getnum(value, opts, subopt)) + cli->fsx.fsx_xflags |= FS_XFLAG_DAX; + else + cli->fsx.fsx_xflags &= ~FS_XFLAG_DAX; + break; default: return -EINVAL; } ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mkfs: allow setting dax flag on root directory 2020-08-11 16:42 ` [PATCH 2/2] mkfs: allow setting dax flag on root directory Darrick J. Wong @ 2020-08-11 19:39 ` Eric Sandeen 2020-08-11 19:54 ` Darrick J. Wong 0 siblings, 1 reply; 8+ messages in thread From: Eric Sandeen @ 2020-08-11 19:39 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs On 8/11/20 9:42 AM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Teach mkfs to set the DAX flag on the root directory so that all new > files can be created in dax mode. This is a complement to removing the > mount option. So, a new -d option, "-d dax" This is ~analogous to cowextsize, rtinherit, projinherit, and extszinherit so there is certainly precedence for this. (where only rtinherit is a boolean like this, but they are all inheritable behaviors) (I wonder if "daxinherit" would be more consistent, but won't bikeshed that (much)) > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > man/man8/mkfs.xfs.8 | 11 +++++++++++ > mkfs/xfs_mkfs.c | 14 ++++++++++++++ > 2 files changed, 25 insertions(+) > > > diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8 > index 9d762a43011a..4b4fdd86b2f4 100644 > --- a/man/man8/mkfs.xfs.8 > +++ b/man/man8/mkfs.xfs.8 > @@ -394,6 +394,17 @@ All inodes created by > will have this extent size hint applied. > The value must be provided in units of filesystem blocks. > Directories will pass on this hint to newly created children. > +.TP > +.BI dax= value > +All inodes created by > +.B mkfs.xfs > +will have the DAX flag set. > +This means that directories will pass the flag on to newly created files let's call this "children" to match the other similar options? (because technically it is passed on not only to regular files, right?) > +and files will use the DAX IO paths when possible. > +This value is either 1 to enable the use or 0 to disable. > +By default, > +.B mkfs.xfs > +will not enable DAX mode. > .RE > .TP > .B \-f > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c > index 2e6cd280e388..33507f6ea21c 100644 > --- a/mkfs/xfs_mkfs.c > +++ b/mkfs/xfs_mkfs.c > @@ -60,6 +60,7 @@ enum { > D_PROJINHERIT, > D_EXTSZINHERIT, > D_COWEXTSIZE, > + D_DAX, > D_MAX_OPTS, > }; > > @@ -254,6 +255,7 @@ static struct opt_params dopts = { > [D_PROJINHERIT] = "projinherit", > [D_EXTSZINHERIT] = "extszinherit", > [D_COWEXTSIZE] = "cowextsize", > + [D_DAX] = "dax", > }, > .subopt_params = { > { .index = D_AGCOUNT, > @@ -369,6 +371,12 @@ static struct opt_params dopts = { > .maxval = UINT_MAX, > .defaultval = SUBOPT_NEEDS_VAL, > }, > + { .index = D_DAX, > + .conflicts = { { NULL, LAST_CONFLICT } }, er.... should we conflict with reflink .... ? > + .minval = 0, > + .maxval = 1, > + .defaultval = 1, Hm, interesting that this is a little different from rtinherit: { .index = D_RTINHERIT, .conflicts = { { NULL, LAST_CONFLICT } }, .minval = 1, .maxval = 1, .defaultval = 1, }, I think this means that: -d rtinherit -d rtinherit=1 are valid, but -d rtinherit=0 is not, but -d dax -d dax=1 -d dax=0 are all valid? While the latter makes a bit more sense, I wonder if we should stay consistent w/ the rtinherit semantics. Or do you envision some sort of automatic enabling of this based on device typethat we'd need to override in the future? > + }, > }, > }; > > @@ -1434,6 +1442,12 @@ data_opts_parser( > cli->fsx.fsx_cowextsize = getnum(value, opts, subopt); > cli->fsx.fsx_xflags |= FS_XFLAG_COWEXTSIZE; > break; > + case D_DAX: > + if (getnum(value, opts, subopt)) > + cli->fsx.fsx_xflags |= FS_XFLAG_DAX; > + else > + cli->fsx.fsx_xflags &= ~FS_XFLAG_DAX; > + break; > default: > return -EINVAL; > } > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mkfs: allow setting dax flag on root directory 2020-08-11 19:39 ` Eric Sandeen @ 2020-08-11 19:54 ` Darrick J. Wong 2020-08-11 20:02 ` Eric Sandeen 0 siblings, 1 reply; 8+ messages in thread From: Darrick J. Wong @ 2020-08-11 19:54 UTC (permalink / raw) To: Eric Sandeen; +Cc: linux-xfs On Tue, Aug 11, 2020 at 02:39:01PM -0500, Eric Sandeen wrote: > On 8/11/20 9:42 AM, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > Teach mkfs to set the DAX flag on the root directory so that all new > > files can be created in dax mode. This is a complement to removing the > > mount option. > > So, a new -d option, "-d dax" > > This is ~analogous to cowextsize, rtinherit, projinherit, and extszinherit > so there is certainly precedence for this. (where only rtinherit is a boolean > like this, but they are all inheritable behaviors) > > (I wonder if "daxinherit" would be more consistent, but won't bikeshed > that (much)) /me is indifferent either way. But I guess some day we might want to have a dax= flag to indicate something like "set the data device geometry to optimize for DAX? Nah, I think if we were ever going to do that, we'd have something more like: -d usage=dax -d usage=ssd -d usage=floopy Meh. I'll change it to daxinherit, since that /is/ what it does. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > man/man8/mkfs.xfs.8 | 11 +++++++++++ > > mkfs/xfs_mkfs.c | 14 ++++++++++++++ > > 2 files changed, 25 insertions(+) > > > > > > diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8 > > index 9d762a43011a..4b4fdd86b2f4 100644 > > --- a/man/man8/mkfs.xfs.8 > > +++ b/man/man8/mkfs.xfs.8 > > @@ -394,6 +394,17 @@ All inodes created by > > will have this extent size hint applied. > > The value must be provided in units of filesystem blocks. > > Directories will pass on this hint to newly created children. > > +.TP > > +.BI dax= value > > +All inodes created by > > +.B mkfs.xfs > > +will have the DAX flag set. > > +This means that directories will pass the flag on to newly created files > > let's call this "children" to match the other similar options? > > (because technically it is passed on not only to regular files, right?) Directories and regular files, though not to other special files. Maybe we should fix that. > > +and files will use the DAX IO paths when possible. > > +This value is either 1 to enable the use or 0 to disable. > > +By default, > > +.B mkfs.xfs > > +will not enable DAX mode. > > .RE > > .TP > > .B \-f > > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c > > index 2e6cd280e388..33507f6ea21c 100644 > > --- a/mkfs/xfs_mkfs.c > > +++ b/mkfs/xfs_mkfs.c > > @@ -60,6 +60,7 @@ enum { > > D_PROJINHERIT, > > D_EXTSZINHERIT, > > D_COWEXTSIZE, > > + D_DAX, > > D_MAX_OPTS, > > }; > > > > @@ -254,6 +255,7 @@ static struct opt_params dopts = { > > [D_PROJINHERIT] = "projinherit", > > [D_EXTSZINHERIT] = "extszinherit", > > [D_COWEXTSIZE] = "cowextsize", > > + [D_DAX] = "dax", > > }, > > .subopt_params = { > > { .index = D_AGCOUNT, > > @@ -369,6 +371,12 @@ static struct opt_params dopts = { > > .maxval = UINT_MAX, > > .defaultval = SUBOPT_NEEDS_VAL, > > }, > > + { .index = D_DAX, > > + .conflicts = { { NULL, LAST_CONFLICT } }, > > er.... should we conflict with reflink .... ? > > > + .minval = 0, > > + .maxval = 1, > > + .defaultval = 1, > > Hm, interesting that this is a little different from rtinherit: > > { .index = D_RTINHERIT, > .conflicts = { { NULL, LAST_CONFLICT } }, > .minval = 1, > .maxval = 1, > .defaultval = 1, > }, > > I think this means that: > > -d rtinherit > -d rtinherit=1 > > are valid, but > > -d rtinherit=0 is not, but > > -d dax > -d dax=1 > -d dax=0 > > are all valid? TBH, I find it a little odd that you *can't* say "-d rtinherit=0" from a completeness perspective, but... > While the latter makes a bit more sense, I wonder if we should stay > consistent w/ the rtinherit semantics. Or do you envision some sort > of automatic enabling of this based on device typethat we'd need to > override in the future? ...the goal is to set this automatically once distros start shipping a libblkid that has blkid_topology_get_dax(). At that point we'll probably want a way to force it off. Unless we want the ability to specify -ddax=0 the magic seekrit hook to discover if (future) mkfs actually supports dax autodetection? Hmm, that alone sounds like sufficient justification. Ok. --D > > > + }, > > }, > > }; > > > > @@ -1434,6 +1442,12 @@ data_opts_parser( > > cli->fsx.fsx_cowextsize = getnum(value, opts, subopt); > > cli->fsx.fsx_xflags |= FS_XFLAG_COWEXTSIZE; > > break; > > + case D_DAX: > > + if (getnum(value, opts, subopt)) > > + cli->fsx.fsx_xflags |= FS_XFLAG_DAX; > > + else > > + cli->fsx.fsx_xflags &= ~FS_XFLAG_DAX; > > + break; > > default: > > return -EINVAL; > > } > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mkfs: allow setting dax flag on root directory 2020-08-11 19:54 ` Darrick J. Wong @ 2020-08-11 20:02 ` Eric Sandeen 2020-08-11 20:18 ` Darrick J. Wong 0 siblings, 1 reply; 8+ messages in thread From: Eric Sandeen @ 2020-08-11 20:02 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs On 8/11/20 12:54 PM, Darrick J. Wong wrote: > On Tue, Aug 11, 2020 at 02:39:01PM -0500, Eric Sandeen wrote: >> On 8/11/20 9:42 AM, Darrick J. Wong wrote: >>> From: Darrick J. Wong <darrick.wong@oracle.com> >>> >>> Teach mkfs to set the DAX flag on the root directory so that all new >>> files can be created in dax mode. This is a complement to removing the >>> mount option. >> >> So, a new -d option, "-d dax" >> >> This is ~analogous to cowextsize, rtinherit, projinherit, and extszinherit >> so there is certainly precedence for this. (where only rtinherit is a boolean >> like this, but they are all inheritable behaviors) >> >> (I wonder if "daxinherit" would be more consistent, but won't bikeshed >> that (much)) > > /me is indifferent either way. But I guess some day we might want to > have a dax= flag to indicate something like "set the data device > geometry to optimize for DAX? > > Nah, I think if we were ever going to do that, we'd have something more > like: > > -d usage=dax > -d usage=ssd > -d usage=floopy > > Meh. I'll change it to daxinherit, since that /is/ what it does. Ok. I'm really pretty indifferent as well. >>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> >>> --- >>> man/man8/mkfs.xfs.8 | 11 +++++++++++ >>> mkfs/xfs_mkfs.c | 14 ++++++++++++++ >>> 2 files changed, 25 insertions(+) >>> >>> >>> diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8 >>> index 9d762a43011a..4b4fdd86b2f4 100644 >>> --- a/man/man8/mkfs.xfs.8 >>> +++ b/man/man8/mkfs.xfs.8 >>> @@ -394,6 +394,17 @@ All inodes created by >>> will have this extent size hint applied. >>> The value must be provided in units of filesystem blocks. >>> Directories will pass on this hint to newly created children. >>> +.TP >>> +.BI dax= value >>> +All inodes created by >>> +.B mkfs.xfs >>> +will have the DAX flag set. >>> +This means that directories will pass the flag on to newly created files >> >> let's call this "children" to match the other similar options? >> >> (because technically it is passed on not only to regular files, right?) > > Directories and regular files, though not to other special files. > Maybe we should fix that. Ok so not all children. But also not just files. :P "... pass the flag on to newly created files and directories, so that new files will use the DAX IO paths when possible." ? >>> +and files will use the DAX IO paths when possible. >>> +This value is either 1 to enable the use or 0 to disable. ... >>> @@ -369,6 +371,12 @@ static struct opt_params dopts = { >>> .maxval = UINT_MAX, >>> .defaultval = SUBOPT_NEEDS_VAL, >>> }, >>> + { .index = D_DAX, >>> + .conflicts = { { NULL, LAST_CONFLICT } }, >> >> er.... should we conflict with reflink .... ? Thoughts? :) >>> + .minval = 0, >>> + .maxval = 1, >>> + .defaultval = 1, >> >> Hm, interesting that this is a little different from rtinherit: >> >> { .index = D_RTINHERIT, >> .conflicts = { { NULL, LAST_CONFLICT } }, >> .minval = 1, >> .maxval = 1, >> .defaultval = 1, >> }, >> >> I think this means that: >> >> -d rtinherit >> -d rtinherit=1 >> >> are valid, but >> >> -d rtinherit=0 is not, but >> >> -d dax >> -d dax=1 >> -d dax=0 >> >> are all valid? > > TBH, I find it a little odd that you *can't* say "-d rtinherit=0" from a > completeness perspective, but... We could probably loosen it up and start allowing zero here too. It wouldn't break any old scripts, right. >> While the latter makes a bit more sense, I wonder if we should stay >> consistent w/ the rtinherit semantics. Or do you envision some sort >> of automatic enabling of this based on device typethat we'd need to >> override in the future? > > ...the goal is to set this automatically once distros start shipping a > libblkid that has blkid_topology_get_dax(). At that point we'll > probably want a way to force it off. *nod* > Unless we want the ability to specify -ddax=0 the magic seekrit hook to > discover if (future) mkfs actually supports dax autodetection? Hmm, > that alone sounds like sufficient justification. Ok. Not sure I followed that... :) -Eric ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mkfs: allow setting dax flag on root directory 2020-08-11 20:02 ` Eric Sandeen @ 2020-08-11 20:18 ` Darrick J. Wong 0 siblings, 0 replies; 8+ messages in thread From: Darrick J. Wong @ 2020-08-11 20:18 UTC (permalink / raw) To: Eric Sandeen; +Cc: linux-xfs On Tue, Aug 11, 2020 at 03:02:07PM -0500, Eric Sandeen wrote: > On 8/11/20 12:54 PM, Darrick J. Wong wrote: > > On Tue, Aug 11, 2020 at 02:39:01PM -0500, Eric Sandeen wrote: > >> On 8/11/20 9:42 AM, Darrick J. Wong wrote: > >>> From: Darrick J. Wong <darrick.wong@oracle.com> > >>> > >>> Teach mkfs to set the DAX flag on the root directory so that all new > >>> files can be created in dax mode. This is a complement to removing the > >>> mount option. > >> > >> So, a new -d option, "-d dax" > >> > >> This is ~analogous to cowextsize, rtinherit, projinherit, and extszinherit > >> so there is certainly precedence for this. (where only rtinherit is a boolean > >> like this, but they are all inheritable behaviors) > >> > >> (I wonder if "daxinherit" would be more consistent, but won't bikeshed > >> that (much)) > > > > /me is indifferent either way. But I guess some day we might want to > > have a dax= flag to indicate something like "set the data device > > geometry to optimize for DAX? > > > > Nah, I think if we were ever going to do that, we'd have something more > > like: > > > > -d usage=dax > > -d usage=ssd > > -d usage=floopy > > > > Meh. I'll change it to daxinherit, since that /is/ what it does. > > Ok. I'm really pretty indifferent as well. > > >>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > >>> --- > >>> man/man8/mkfs.xfs.8 | 11 +++++++++++ > >>> mkfs/xfs_mkfs.c | 14 ++++++++++++++ > >>> 2 files changed, 25 insertions(+) > >>> > >>> > >>> diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8 > >>> index 9d762a43011a..4b4fdd86b2f4 100644 > >>> --- a/man/man8/mkfs.xfs.8 > >>> +++ b/man/man8/mkfs.xfs.8 > >>> @@ -394,6 +394,17 @@ All inodes created by > >>> will have this extent size hint applied. > >>> The value must be provided in units of filesystem blocks. > >>> Directories will pass on this hint to newly created children. > >>> +.TP > >>> +.BI dax= value > >>> +All inodes created by > >>> +.B mkfs.xfs > >>> +will have the DAX flag set. > >>> +This means that directories will pass the flag on to newly created files > >> > >> let's call this "children" to match the other similar options? > >> > >> (because technically it is passed on not only to regular files, right?) > > > > Directories and regular files, though not to other special files. > > Maybe we should fix that. > > Ok so not all children. But also not just files. :P > > "... pass the flag on to newly created files and directories, so that new > files will use the DAX IO paths when possible." ? > > >>> +and files will use the DAX IO paths when possible. > >>> +This value is either 1 to enable the use or 0 to disable. > > ... > > >>> @@ -369,6 +371,12 @@ static struct opt_params dopts = { > >>> .maxval = UINT_MAX, > >>> .defaultval = SUBOPT_NEEDS_VAL, > >>> }, > >>> + { .index = D_DAX, > >>> + .conflicts = { { NULL, LAST_CONFLICT } }, > >> > >> er.... should we conflict with reflink .... ? > > Thoughts? :) That only prevents the user from specifying a reflink cli option; it doesn't prevent them from turning on daxinherit when reflink already defaults to enabled. If you want to prevent people from formatting with the two options, you have to do it in validate_sb_features. Hm, maybe we should at least warn about that. > >>> + .minval = 0, > >>> + .maxval = 1, > >>> + .defaultval = 1, > >> > >> Hm, interesting that this is a little different from rtinherit: > >> > >> { .index = D_RTINHERIT, > >> .conflicts = { { NULL, LAST_CONFLICT } }, > >> .minval = 1, > >> .maxval = 1, > >> .defaultval = 1, > >> }, > >> > >> I think this means that: > >> > >> -d rtinherit > >> -d rtinherit=1 > >> > >> are valid, but > >> > >> -d rtinherit=0 is not, but > >> > >> -d dax > >> -d dax=1 > >> -d dax=0 > >> > >> are all valid? > > > > TBH, I find it a little odd that you *can't* say "-d rtinherit=0" from a > > completeness perspective, but... > > We could probably loosen it up and start allowing zero here too. > It wouldn't break any old scripts, right. Right. > >> While the latter makes a bit more sense, I wonder if we should stay > >> consistent w/ the rtinherit semantics. Or do you envision some sort > >> of automatic enabling of this based on device typethat we'd need to > >> override in the future? > > > > ...the goal is to set this automatically once distros start shipping a > > libblkid that has blkid_topology_get_dax(). At that point we'll > > probably want a way to force it off. > > *nod* > > > Unless we want the ability to specify -ddax=0 the magic seekrit hook to > > discover if (future) mkfs actually supports dax autodetection? Hmm, > > that alone sounds like sufficient justification. Ok. > > Not sure I followed that... :) Never mind, I talked myself out of it since script authors aren't going to be happy with an option that only sometimes works. --D > -Eric > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-08-11 20:18 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-11 16:42 [PATCH 0/2] xfsprogs: various small enhancements Darrick J. Wong 2020-08-11 16:42 ` [PATCH 1/2] xfs_db: fix nlink usage in check Darrick J. Wong 2020-08-11 19:25 ` Eric Sandeen 2020-08-11 16:42 ` [PATCH 2/2] mkfs: allow setting dax flag on root directory Darrick J. Wong 2020-08-11 19:39 ` Eric Sandeen 2020-08-11 19:54 ` Darrick J. Wong 2020-08-11 20:02 ` Eric Sandeen 2020-08-11 20:18 ` Darrick J. Wong
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.