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

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Fri, Jul 27, 2018 at 02:51:25PM -0400, Brian Foster wrote:
> On Thu, Jul 26, 2018 at 06:20:01PM -0700, Eric Sandeen wrote:
> > On 7/26/18 4:17 PM, Dave Chinner wrote:
> > 
> > ...
> > 
> > > As it is, I don't think we can remove this now - people are using
> > > the on-disk flags already, and the inherit flag from the directory
> > > has none of the problems of changing S_DAX dynamically.
> > 
> > Which is why I left the inheritance part ...
> > 
> > > Hence just
> > > disabling it is the wrong thing to do because it removes the ability
> > > for people to manage the flags that are already on disk....
> > 
> > Well, it's EXPERIMENTAL... o_O
> > 
> > But yeah, this does remove the ability to turn off the flag, at least
> > w/o making a file copy or some other gross thing.
> > 
> 
> You could retain the ability to change the flag on existing files when
> -odax is used, right? IIUC, that means the runtime change is essentially
> a no-op and so the state change problem doesn't exist.
> 
> That's obviously tedious, but is there any other truly predictable way
> to toggle the behavior change other than a mount cycle anyways?
> 
> > > I'd much prefer we fix the page fault synchronisation problem than
> > > break stuff that /isn't actually broken/. Yes, it's current
> > > behaviour is suboptimal, but that is only supposed to be /temporary/
> > > until the aops callout problem is fixed.

I thought Jan Kara proposed checking the S_DAX state in the DAX/non-DAX
code paths and bailing out with some sort of -EAGAIN type status (or
whatever the vm_fault_t equivalent of that is) if it's inconsistent,
though the same discussion also proposed restricting DAX flag changes to
directories and files with zero size, zero i_blocks, and zero
i_delayed_blks.

> > 
> 
> I agree, but it sounds like nobody has a solution for that (let alone a
> patch). :/
> 
> > Well, it's super confusing and from the user POV more or less
> > nondeterministic, at this point.  That borders on broken/unusable IMHO.
> > "Change the flag and some time later, not really within your control, the
> > behavior will change."
> > 
> 
> Perhaps broken is strong if whatever crash problem existed was worked
> around by Christoph's change. I agree that this is certainly not usable.

He didn't work around it so much as disabled the in-core state changes
that were supposed to accompany the on-disk state changes.

> You can literally create a new/empty file, set the DAX flag, start
> writing data and see the exact opposite behavior from what is expected.
> I'm not sure suboptimal really describes that either. ;P
> 
> I guess the more important question is does anything beyond this inode
> flag switching problem hold DAX+XFS in experimental status?

In order of increasing complexity (as perceived by me):

Lack of reflink support, maintainer's continuing unease about how all
this interacts with the memory manager / "we can't quite fix this right
now", unresolved debate over what role inode flags have vs. kernel
autoconfiguration...

> The purpose of this rfc is to essentially probe whether we can lift
> some portion of dax out of experimental status, right? If there are
> other concerns that would prevent that, then perhaps this is moot
> until that changes.

I thought the point of the RFC was to turn off the parts of the
interface that have imprecisely defined outcomes until a later time when
we can deliver a precisely defined outcome.

--D

> Brian
> 
> > It's been broken for a year.  TBH I'm not sure I personally have
> > the knowledge/skill (and I almost certainly don't have the time)
> > to fix it.  Just trying to make the best of a sticky situation.
> > This seemed better to me, modulo the inability to remove the
> > flag.  :/  And I figured more permissive flag manipulation could
> > be added back later if it ever does get fixed.
> > 
> > Thanks,
> > -Eric
> >  
> > > Cheers,
> > > 
> > > Dave.
> > > 
> > --
> > 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