Re: [PATCH v5 2/5] fuse: make DAX mount option a tri-state

From: Vivek Goyal <vgoyal@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: Jeffle Xu <jefflexu@linux.alibaba.com>,
	stefanha@redhat.com, miklos@szeredi.hu,
	linux-fsdevel@vger.kernel.org, virtio-fs@redhat.com,
	bo.liu@linux.alibaba.com, joseph.qi@linux.alibaba.com
Subject: Re: [PATCH v5 2/5] fuse: make DAX mount option a tri-state
Date: Thu, 23 Sep 2021 21:02:26 -0400	[thread overview]
Message-ID: <YU0jovIYv+xeinQd@redhat.com> (raw)
In-Reply-To: <20210923222618.GB2361455@dread.disaster.area>

On Fri, Sep 24, 2021 at 08:26:18AM +1000, Dave Chinner wrote:
> On Thu, Sep 23, 2021 at 03:02:41PM -0400, Vivek Goyal wrote:
> > On Thu, Sep 23, 2021 at 05:25:23PM +0800, Jeffle Xu wrote:
> > > We add 'always', 'never', and 'inode' (default). '-o dax' continues to
> > > operate the same which is equivalent to 'always'. To be consistemt with
> > > ext4/xfs's tri-state mount option, when neither '-o dax' nor '-o dax='
> > > option is specified, the default behaviour is equal to 'inode'.
> > 
> > So will "-o dax=inode" be used for per file DAX where dax mode comes
> > from server?
> > 
> > I think we discussed this. It will be better to leave "-o dax=inode"
> > alone. It should be used when we are reading dax status from file
> > attr (like ext4 and xfs). 
> > 
> > And probably create separate option say "-o dax=server" where server
> > specifies which inode should use dax.
> 
> That seems like a poor idea to me.
> 
> The server side already controls what the client side does by
> controlling the inode attributes that the client side sees.  That
> is, if the server is going to specify whether the client side data
> access is going to use dax, then the server presents the client with
> an inode that has the DAX attribute flag set on it.

Hi Dave,

Currently in fuse/virtiofs, DAX is compltely controlled by client. Server
has no say in it. If client is mounted with "-o dax", dax is enabled on
all files otherwise dax is disabled on all files. One could think of
implementing an option on server so that server could deny mmap()
requests that come from client, but so far nobody asked for such
an option on server side.

When you say "inode that has DAX attribute flag set on it", are you
referring to "S_DAX (in inode->i_flags)" or persistent attr
"FS_XFLAG_DAX"?

As of now S_DAX on client side inode is set by fuse client whenever
client mounted filesystem with "-o dax". And I think you are assuming
that DAX attribute of inode is already coming from server. That's not
the case. In fact that seems to be the proposal. Provide capability
so that server can specify which inode should be using DAX and which
inode should not be.

> 
> In that case, turning off dax on the guest side should be
> communicated to the fuse server so the server turns off the DAX flag
> on the server side iff server side policy allows it.

Not sure what do you mean by server turns of DAX flag based on client
turning off DAX. Server does not have to do anything. If client decides
to not use DAX (in guest), it will not send FUSE_SETUPMAPPING requests
to server and that's it.

> When the guest
> then purges it's local inode and reloads it from the server then it
> will get an inode with the DAX flag set according to server side
> policy.

So right now we don't have a mechanism for server to specify DAX flag.
And that's what this patch series is trying to do.

> 
> Hence if the server side is configured with "dax=always" or
> dax="never" semantics it means the client side inode flag state
> cannot control DAX mode. That means, regardless of the client side
> mount options, DAX is operating under always or never policy,

Hmm..., When you say "server side is configured with "dax=always", 
do you mean shared directory on host is mounted with "-o dax=always",
or you mean some virtiofs server specific option which enables
dax on all inodes from server side.

In general, DAX on host and DAX inside guest are completely independent.
Host filesystem could very well be mounted with dax or without dax and
that has no affect on guests's capability to be able to enable DAX or
not. 

> enforced by the server side by direct control of the client inode
> DAX attribute flag. If dax=inode is in use on both sides, the the
> server honours the requests of the client to set/clear the inode
> flags and presents the inode flag according to the state the client
> side has requested.
> 
> This policy state probably should be communicated to
> the fuse client from the server at mount time so policy conflicts
> can be be resolved at mount time (e.g. reject mount if policy
> conflict occurs, default to guest overrides server or vice versa,
> etc). This then means that that the client side mount policies will
> default to server side policy when they set "dax=inode" but also
> provide a local override for always or never local behaviour.
> 
> Hence, AFAICT, there is no need for a 'dax=server' option - this
> seems to be exactly what 'dax=inode' behaviour means on the client
> side - it behaves according to how the server side propagates the
> DAX attribute to the client for each inode.

Ok. So "-o dax=inode" in fuse will have a different meaning as opposed
to ext4/xfs. This will mean that server will pass DAX state of inode
when inode is instantiated and client should honor that. 

But what about FS_XFLAG_DAX flag then. Say host file system
does support this att and fuse/virtiofs allows querying and
setting this attribute (I don't think it is allowed now). So
will we not create a future conflict where in case of fuse/virtiofs
"-o dax=inode" means something different and it does look at
FS_XFLAG_DAX file attr.

> 
> > Otherwise it will be very confusing. People familiar with "-o dax=inode"
> > on ext4/xfs will expect file attr to work and that's not what we
> > are implementing, IIUC.
> 
> The dax mount option behaviour is already confusing enough without
> adding yet another weird, poorly documented, easily misunderstood
> mode that behaves subtly different to existing modes.
> 
> Please try to make the virtiofs behaviour compatible with existing
> modes - it's not that hard to make the client dax=inode behaviour be
> controlled by the server side without any special client side mount
> modes.

Given I want to keep the option of similar behavior for "dax=inode"
across ext4/xfs and virtiofs, I suggested "dax=server". Because I 
assumed that "dax=inode" means that dax is per inode property AND
this per inode property is specified by persistent file attr 
FS_XFLAG_DAX.

But fuse/virtiofs will not be specifying dax property of inode using
FS_XFLAG_DAX (atleast as of now). And server will set DAX property
using some bit in protocol. 

So these seem little different. If we use "dax=inode" for server
specifying DAX property of inode, then in future if client can
query/set FS_XFLAG_DAX on inode, it will be a problem. There will
be a conflict.

Use case I was imagining was, say on host, user might set FS_XFLAG_DAX
attr on relevant files and then mount virtiofs in guest with 
"-o dax=inode". Guest will query state of FS_XFLAG_DAX on inode
and enable DAX accordingly. (And server is not necessarily playing
an active role in determining which files should use DAX).

In summary, there seem to be two use cases.

A. virtiofsd/fuse-server wants do be able to come up with its own policy
   to decide which inodes should use guest.

B. guest client decides which inode use DAX based on FS_XFLAG_DAX attr
   on inode (and server does not play a role).

To be able to different between these two cases, I was suggesting using
"-o dax=inode" for B and "-o dax=server" for A.

Thanks
Vivek