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@xxxxxxxxxx>
>>>
>>> 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@xxxxxxxxxx>
>>> ---
>>> 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
