Re: [PATCH, RFC] xfs: completely disable toggling DAX flag via ioctl on reg files

[Eric Sandeen <sandeen@sandeen.net>]

On 7/26/18 5:08 AM, Brian Foster wrote:
> On Wed, Jul 25, 2018 at 02:20:54PM -0700, Eric Sandeen wrote:
>> 742d842 xfs: disable per-inode DAX flag was, I think, intended
>> as a short-term workaround to avoid races when toggling DAX on
>> and off of active inodes until mm/ sorted that out.
>>
>> (It's also a confusing title, as it didn't really disable
>> per-inode DAX at all.)
>>
>> However, it has the surprising (to me, at least) result that while
>> the ioctl succeeds, no behavior changes until the inode is cycled
>> out of cache and re-read from disk at some unknown later time.
>> This seems to badly violate the principle of least surprise.
>>
>> Until said races are properly resolved, it seems most prudent to
>> disallow modification of the flag on regular files altogether.
>> We can still allow per-inode DAX flagging via directory inheritance.
>>
>> Since DAX is still flagged as experimental (in part due to these
>> concerns) I don't think it's a problem to (temporarily?) break
>> this interface further.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
> 
> I'm not in tune with the latest state of dax, but if the situation is
> that we don't currently have a means to correctly switch the per-inode
> state for an active inode (and thus have simply skipped changing the
> online flag while carrying on with the on-disk flag, leading to this
> inode cache cycling requirement), then I think this makes sense. The
> current interface is essentially incomplete, I don't see any reason to
> allow unless/until it actually works sanely.
> 
> BTW, what bits are actually missing to make that happen? Why is the
> flush/inval currently in this function not sufficient?

TBH I don't actually know the low-level details. :/

> Implementation wise, I'm a little curious why we're piling on hacks
> (such as the return short-circuit and the previous #if 0) as opposed to
> just removing the code. The code can always be restored directly from
> the git history, right?

Well, fair point.  If this goes in it should probably at least remove
the other #if 0 so it's not stacking hacks on hacks, but if the issue
is in mm/ it might be a big ask for anyone eventually working on that
to dig supporting code back out of xfs git history ...

-Eric