From: Dave Chinner <david@fromorbit.com> To: Vivek Goyal <vgoyal@redhat.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: Fri, 24 Sep 2021 08:26:18 +1000 [thread overview] Message-ID: <20210923222618.GB2361455@dread.disaster.area> (raw) In-Reply-To: <YUzPUYU8R5LL4mzU@redhat.com> 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. 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. 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. 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, 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. > 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. Cheers, Dave. -- Dave Chinner david@fromorbit.com
WARNING: multiple messages have this Message-ID (diff)
From: Dave Chinner <david@fromorbit.com> To: Vivek Goyal <vgoyal@redhat.com> Cc: miklos@szeredi.hu, virtio-fs@redhat.com, linux-fsdevel@vger.kernel.org, joseph.qi@linux.alibaba.com Subject: Re: [Virtio-fs] [PATCH v5 2/5] fuse: make DAX mount option a tri-state Date: Fri, 24 Sep 2021 08:26:18 +1000 [thread overview] Message-ID: <20210923222618.GB2361455@dread.disaster.area> (raw) In-Reply-To: <YUzPUYU8R5LL4mzU@redhat.com> 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. 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. 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. 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, 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. > 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. Cheers, Dave. -- Dave Chinner david@fromorbit.com
next prev parent reply other threads:[~2021-09-23 22:26 UTC|newest] Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-09-23 9:25 [PATCH v5 0/5] fuse,virtiofs: support per-file DAX Jeffle Xu 2021-09-23 9:25 ` [Virtio-fs] " Jeffle Xu 2021-09-23 9:25 ` [PATCH v5 1/5] fuse: add fuse_should_enable_dax() helper Jeffle Xu 2021-09-23 9:25 ` [Virtio-fs] " Jeffle Xu 2021-09-23 18:58 ` Vivek Goyal 2021-09-23 18:58 ` [Virtio-fs] " Vivek Goyal 2021-09-23 9:25 ` [PATCH v5 2/5] fuse: make DAX mount option a tri-state Jeffle Xu 2021-09-23 9:25 ` [Virtio-fs] " Jeffle Xu 2021-09-23 19:02 ` Vivek Goyal 2021-09-23 19:02 ` [Virtio-fs] " Vivek Goyal 2021-09-23 22:26 ` Dave Chinner [this message] 2021-09-23 22:26 ` Dave Chinner 2021-09-24 1:02 ` Vivek Goyal 2021-09-24 1:02 ` [Virtio-fs] " Vivek Goyal 2021-09-24 3:06 ` JeffleXu 2021-09-24 3:06 ` [Virtio-fs] " JeffleXu 2021-09-24 12:43 ` Vivek Goyal 2021-09-24 12:43 ` [Virtio-fs] " Vivek Goyal 2021-09-26 2:06 ` JeffleXu 2021-09-26 2:06 ` [Virtio-fs] " JeffleXu 2021-09-27 0:21 ` Dave Chinner 2021-09-27 0:21 ` [Virtio-fs] " Dave Chinner 2021-09-27 2:28 ` JeffleXu 2021-09-27 2:28 ` [Virtio-fs] " JeffleXu 2021-09-28 3:44 ` Dave Chinner 2021-09-28 3:44 ` [Virtio-fs] " Dave Chinner 2021-09-28 5:17 ` JeffleXu 2021-09-28 5:17 ` [Virtio-fs] " JeffleXu 2021-09-28 14:34 ` Vivek Goyal 2021-09-28 14:34 ` [Virtio-fs] " Vivek Goyal 2021-09-27 15:32 ` Vivek Goyal 2021-09-27 15:32 ` [Virtio-fs] " Vivek Goyal 2021-09-28 3:23 ` Dave Chinner 2021-09-28 3:23 ` [Virtio-fs] " Dave Chinner 2021-09-23 9:25 ` [PATCH v5 3/5] fuse: support per-file DAX Jeffle Xu 2021-09-23 9:25 ` [Virtio-fs] " Jeffle Xu 2021-09-23 9:25 ` [PATCH v5 4/5] fuse: enable " Jeffle Xu 2021-09-23 9:25 ` [Virtio-fs] " Jeffle Xu 2021-09-23 9:25 ` [PATCH v5 5/5] fuse: mark inode DONT_CACHE when per-file DAX hint changes Jeffle Xu 2021-09-23 9:25 ` [Virtio-fs] " Jeffle Xu 2021-09-23 9:35 ` [virtiofsd PATCH v5 0/2] virtiofsd: support per-file DAX Jeffle Xu 2021-09-23 9:35 ` [Virtio-fs] " Jeffle Xu 2021-09-23 9:35 ` [virtiofsd PATCH v5 1/2] virtiofsd: add FUSE_ATTR_DAX to fuse protocol Jeffle Xu 2021-09-23 9:35 ` [Virtio-fs] " Jeffle Xu 2021-09-23 9:35 ` [virtiofsd PATCH v5 2/2] virtiofsd: support per-file DAX Jeffle Xu 2021-09-23 9:35 ` [Virtio-fs] " Jeffle Xu 2021-09-23 18:57 ` [PATCH v5 0/5] fuse,virtiofs: " Vivek Goyal 2021-09-23 18:57 ` [Virtio-fs] " Vivek Goyal 2021-10-27 3:42 ` JeffleXu 2021-10-27 3:42 ` [Virtio-fs] " JeffleXu 2021-10-27 14:45 ` Vivek Goyal 2021-10-27 14:45 ` [Virtio-fs] " Vivek Goyal
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210923222618.GB2361455@dread.disaster.area \ --to=david@fromorbit.com \ --cc=bo.liu@linux.alibaba.com \ --cc=jefflexu@linux.alibaba.com \ --cc=joseph.qi@linux.alibaba.com \ --cc=linux-fsdevel@vger.kernel.org \ --cc=miklos@szeredi.hu \ --cc=stefanha@redhat.com \ --cc=vgoyal@redhat.com \ --cc=virtio-fs@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.