All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ira Weiny <ira.weiny@intel.com>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: JeffleXu <jefflexu@linux.alibaba.com>,
	Dave Chinner <dchinner@redhat.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 v6 2/7] fuse: make DAX mount option a tri-state
Date: Mon, 25 Oct 2021 12:02:01 -0700	[thread overview]
Message-ID: <20211025190201.GG3465596@iweiny-DESK2.sc.intel.com> (raw)
In-Reply-To: <YXbzeomdC5cD1xfF@redhat.com>

On Mon, Oct 25, 2021 at 02:12:10PM -0400, Vivek Goyal wrote:
> On Mon, Oct 25, 2021 at 10:52:51AM -0700, Ira Weiny wrote:
> > On Fri, Oct 22, 2021 at 02:54:03PM +0800, JeffleXu wrote:
> > > cc [Ira Weiny], author of per inode DAX on xfs/ext4
> > > 
> > > On 10/20/21 11:17 PM, Vivek Goyal wrote:
> > > > On Wed, Oct 20, 2021 at 10:52:38AM +0800, JeffleXu wrote:
> > > >>
> > > >>
> > > >> On 10/18/21 10:10 PM, Vivek Goyal wrote:
> > > >>> On Mon, Oct 11, 2021 at 11:00:47AM +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'.
> > > >>>
> > > >>> Hi Jeffle,
> > > >>>
> > > >>> I am not sure when  -o "dax=inode"  is used as a default? If user
> > > >>> specifies, "-o dax" then it is equal to "-o dax=always", otherwise
> > > >>> user will explicitly specify "-o dax=always/never/inode". So when
> > > >>> is dax=inode is used as default?
> > > >>
> > > >> That means when neither '-o dax' nor '-o dax=always/never/inode' is
> > > >> specified, it is actually equal to '-o dax=inode', which is also how
> > > >> per-file DAX on ext4/xfs works.
> > > > 
> > 
> > It's been a while so I'm fuzzy on the details of the discussions but yes that
> > is the way things are now in the code.
> > 
> > > > [ CC dave chinner] 
> > > > 
> > > > Is it not change of default behavior for ext4/xfs as well. My
> > > > understanding is that prior to this new dax options, "-o dax" enabled
> > > > dax on filesystem and if user did not specify it, DAX is disbaled
> > > > by default.
> > 
> > Technically it does change default behavior...  However, NOT in a way which
> > breaks anything.  See below.
> > 
> > > > 
> > > > Now after introduction of "-o dax=always/never/inode", if suddenly
> > > > "-o dax=inode" became the default if user did not specify anything,
> > > > that's change of behavior.
> > 
> > Technically yes but not in a broken way.
> > 
> > > >
> > > > Is that intentional. If given a choice,
> > > > I would rather not change default and ask user to opt-in for
> > > > appropriate dax functionality.
> > 
> > There is no need for this.
> > 
> > > > 
> > > > Dave, you might have thoughts on this. It makes me uncomfortable to
> > > > change virtiofs dax default now just because other filesytems did it.
> > > > 
> > > 
> > > I can only find the following discussions about the earliest record on
> > > this tri-state mount option:
> > > 
> > > https://lore.kernel.org/lkml/20200316095509.GA13788@lst.de/
> > > https://lore.kernel.org/lkml/20200401040021.GC56958@magnolia/
> > > 
> > > 
> > > Hi, Ira Weiny,
> > > 
> > > Do you have any thought on this, i.e. why the default behavior has
> > > changed after introduction of per inode dax?
> > 
> > While this is 'technically' different behavior the end user does not see any
> > difference in behavior if they continue without software changes.  Specifically
> > specifying nothing continues to operate with all the files on the FS to be
> > '_not_ DAX'.  While specifying '-o dax' forces DAX on all files.
> > 
> > This expands the default behavior in a backwards compatible manner.
> 
> This is backward compatible in a sense that if somebody upgrades to new
> kernel, things will still be same. 
> 
> I think little problematic change is that say I bring in persistent
> memory from another system (which has FS_XFLAGS_DAX set on some inodes)
> and then mount it without andy of the dax mount options, then per
> inode dax will be enabled unexpectedly if I boot with newer kernels
> but it will be disable if I mount with older kernels. Do I understand it
> right.

Indeed that will happen.  However, wouldn't the users (software) of those files
have knowledge that those files were DAX and want to continue with them in that
mode?

> 
> > The user
> > can now enable DAX on some files.  But this is an opt-in on the part of the
> > user of the FS and again does not change with existing software/scripts/etc.
> 
> Don't understand this "opt-in" bit. If user mounts an fs without
> specifying any of the dax options, then per inode dax will still be
> enabled if inode has the correct flag set.

But only users who actually set that flag 'opt-in'.

> So is setting of flag being
> considered as opt-in (insted of mount option).

Yes.

> 
> If setting of flag is being considered as opt-in, that probably will not
> work very well with virtiofs. Because server can enforce a different
> policy for enabling per file dax (instead of FS_XFLAG_DAX).

I'm not sure I understand how this happens?  I think the server probably has to
enable per INODE by default to allow the client to do what the end users wants.

I agree that if the end user is expecting DAX and the server disables it then
that is a problem but couldn't that happen before?  Maybe I'm getting confused
because I'm not familiar enough with virtiofs.

> 
> And given there are two entities here (client and server), I think it
> will be good if if we give client a chance as well to decide whether
> it wants to enable per file dax or not. I know it can alwasy do 
> "dax=never" but it can still be broken if client software remains
> same but host/server software is upgraded or commnad line changed.

But the files are 'owned' by a single user or group of users who must have
placed the file in DAX mode at some point right?

> 
> So for virtiofs, I think better behavior is to continue to not enable
> any dax until and unless user opts-in using "-o dax=foo" options.

I'm not sure, maybe.

Ira

WARNING: multiple messages have this Message-ID (diff)
From: Ira Weiny <ira.weiny@intel.com>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: miklos@szeredi.hu, virtio-fs@redhat.com,
	linux-fsdevel@vger.kernel.org, Dave Chinner <dchinner@redhat.com>,
	joseph.qi@linux.alibaba.com
Subject: Re: [Virtio-fs] [PATCH v6 2/7] fuse: make DAX mount option a tri-state
Date: Mon, 25 Oct 2021 12:02:01 -0700	[thread overview]
Message-ID: <20211025190201.GG3465596@iweiny-DESK2.sc.intel.com> (raw)
In-Reply-To: <YXbzeomdC5cD1xfF@redhat.com>

On Mon, Oct 25, 2021 at 02:12:10PM -0400, Vivek Goyal wrote:
> On Mon, Oct 25, 2021 at 10:52:51AM -0700, Ira Weiny wrote:
> > On Fri, Oct 22, 2021 at 02:54:03PM +0800, JeffleXu wrote:
> > > cc [Ira Weiny], author of per inode DAX on xfs/ext4
> > > 
> > > On 10/20/21 11:17 PM, Vivek Goyal wrote:
> > > > On Wed, Oct 20, 2021 at 10:52:38AM +0800, JeffleXu wrote:
> > > >>
> > > >>
> > > >> On 10/18/21 10:10 PM, Vivek Goyal wrote:
> > > >>> On Mon, Oct 11, 2021 at 11:00:47AM +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'.
> > > >>>
> > > >>> Hi Jeffle,
> > > >>>
> > > >>> I am not sure when  -o "dax=inode"  is used as a default? If user
> > > >>> specifies, "-o dax" then it is equal to "-o dax=always", otherwise
> > > >>> user will explicitly specify "-o dax=always/never/inode". So when
> > > >>> is dax=inode is used as default?
> > > >>
> > > >> That means when neither '-o dax' nor '-o dax=always/never/inode' is
> > > >> specified, it is actually equal to '-o dax=inode', which is also how
> > > >> per-file DAX on ext4/xfs works.
> > > > 
> > 
> > It's been a while so I'm fuzzy on the details of the discussions but yes that
> > is the way things are now in the code.
> > 
> > > > [ CC dave chinner] 
> > > > 
> > > > Is it not change of default behavior for ext4/xfs as well. My
> > > > understanding is that prior to this new dax options, "-o dax" enabled
> > > > dax on filesystem and if user did not specify it, DAX is disbaled
> > > > by default.
> > 
> > Technically it does change default behavior...  However, NOT in a way which
> > breaks anything.  See below.
> > 
> > > > 
> > > > Now after introduction of "-o dax=always/never/inode", if suddenly
> > > > "-o dax=inode" became the default if user did not specify anything,
> > > > that's change of behavior.
> > 
> > Technically yes but not in a broken way.
> > 
> > > >
> > > > Is that intentional. If given a choice,
> > > > I would rather not change default and ask user to opt-in for
> > > > appropriate dax functionality.
> > 
> > There is no need for this.
> > 
> > > > 
> > > > Dave, you might have thoughts on this. It makes me uncomfortable to
> > > > change virtiofs dax default now just because other filesytems did it.
> > > > 
> > > 
> > > I can only find the following discussions about the earliest record on
> > > this tri-state mount option:
> > > 
> > > https://lore.kernel.org/lkml/20200316095509.GA13788@lst.de/
> > > https://lore.kernel.org/lkml/20200401040021.GC56958@magnolia/
> > > 
> > > 
> > > Hi, Ira Weiny,
> > > 
> > > Do you have any thought on this, i.e. why the default behavior has
> > > changed after introduction of per inode dax?
> > 
> > While this is 'technically' different behavior the end user does not see any
> > difference in behavior if they continue without software changes.  Specifically
> > specifying nothing continues to operate with all the files on the FS to be
> > '_not_ DAX'.  While specifying '-o dax' forces DAX on all files.
> > 
> > This expands the default behavior in a backwards compatible manner.
> 
> This is backward compatible in a sense that if somebody upgrades to new
> kernel, things will still be same. 
> 
> I think little problematic change is that say I bring in persistent
> memory from another system (which has FS_XFLAGS_DAX set on some inodes)
> and then mount it without andy of the dax mount options, then per
> inode dax will be enabled unexpectedly if I boot with newer kernels
> but it will be disable if I mount with older kernels. Do I understand it
> right.

Indeed that will happen.  However, wouldn't the users (software) of those files
have knowledge that those files were DAX and want to continue with them in that
mode?

> 
> > The user
> > can now enable DAX on some files.  But this is an opt-in on the part of the
> > user of the FS and again does not change with existing software/scripts/etc.
> 
> Don't understand this "opt-in" bit. If user mounts an fs without
> specifying any of the dax options, then per inode dax will still be
> enabled if inode has the correct flag set.

But only users who actually set that flag 'opt-in'.

> So is setting of flag being
> considered as opt-in (insted of mount option).

Yes.

> 
> If setting of flag is being considered as opt-in, that probably will not
> work very well with virtiofs. Because server can enforce a different
> policy for enabling per file dax (instead of FS_XFLAG_DAX).

I'm not sure I understand how this happens?  I think the server probably has to
enable per INODE by default to allow the client to do what the end users wants.

I agree that if the end user is expecting DAX and the server disables it then
that is a problem but couldn't that happen before?  Maybe I'm getting confused
because I'm not familiar enough with virtiofs.

> 
> And given there are two entities here (client and server), I think it
> will be good if if we give client a chance as well to decide whether
> it wants to enable per file dax or not. I know it can alwasy do 
> "dax=never" but it can still be broken if client software remains
> same but host/server software is upgraded or commnad line changed.

But the files are 'owned' by a single user or group of users who must have
placed the file in DAX mode at some point right?

> 
> So for virtiofs, I think better behavior is to continue to not enable
> any dax until and unless user opts-in using "-o dax=foo" options.

I'm not sure, maybe.

Ira


  reply	other threads:[~2021-10-25 19:27 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-11  3:00 [PATCH v6 0/7] fuse,virtiofs: support per-file DAX Jeffle Xu
2021-10-11  3:00 ` [Virtio-fs] " Jeffle Xu
2021-10-11  3:00 ` [PATCH v6 1/7] fuse: add fuse_should_enable_dax() helper Jeffle Xu
2021-10-11  3:00   ` [Virtio-fs] " Jeffle Xu
2021-10-11  3:00 ` [PATCH v6 2/7] fuse: make DAX mount option a tri-state Jeffle Xu
2021-10-11  3:00   ` [Virtio-fs] " Jeffle Xu
2021-10-18 14:10   ` Vivek Goyal
2021-10-18 14:10     ` [Virtio-fs] " Vivek Goyal
2021-10-20  2:52     ` JeffleXu
2021-10-20  2:52       ` [Virtio-fs] " JeffleXu
2021-10-20 14:48       ` Vivek Goyal
2021-10-20 14:48         ` [Virtio-fs] " Vivek Goyal
2021-10-29  8:33         ` JeffleXu
2021-10-29  8:33           ` [Virtio-fs] " JeffleXu
2021-10-29 13:03           ` Vivek Goyal
2021-10-29 13:03             ` [Virtio-fs] " Vivek Goyal
2021-11-01  8:21             ` JeffleXu
2021-11-01  8:21               ` [Virtio-fs] " JeffleXu
2021-10-20 15:17       ` Vivek Goyal
2021-10-20 15:17         ` [Virtio-fs] " Vivek Goyal
2021-10-22  6:54         ` JeffleXu
2021-10-22  6:54           ` [Virtio-fs] " JeffleXu
2021-10-25 17:52           ` Ira Weiny
2021-10-25 17:52             ` [Virtio-fs] " Ira Weiny
2021-10-25 18:12             ` Vivek Goyal
2021-10-25 18:12               ` [Virtio-fs] " Vivek Goyal
2021-10-25 19:02               ` Ira Weiny [this message]
2021-10-25 19:02                 ` Ira Weiny
2021-10-25 19:33                 ` Vivek Goyal
2021-10-25 19:33                   ` [Virtio-fs] " Vivek Goyal
2021-10-25 20:41                   ` Ira Weiny
2021-10-25 20:41                     ` [Virtio-fs] " Ira Weiny
2021-10-26 13:45                     ` Vivek Goyal
2021-10-26 13:45                       ` [Virtio-fs] " Vivek Goyal
2021-10-27  6:00                 ` JeffleXu
2021-10-27  6:00                   ` [Virtio-fs] " JeffleXu
2021-10-11  3:00 ` [PATCH v6 3/7] fuse: support per-file DAX in fuse protocol Jeffle Xu
2021-10-11  3:00   ` [Virtio-fs] " Jeffle Xu
2021-10-18 14:14   ` Vivek Goyal
2021-10-18 14:14     ` [Virtio-fs] " Vivek Goyal
2021-10-18 14:20     ` Vivek Goyal
2021-10-18 14:20       ` [Virtio-fs] " Vivek Goyal
2021-10-20  3:04       ` JeffleXu
2021-10-20  3:04         ` [Virtio-fs] " JeffleXu
2021-10-20 14:54         ` Vivek Goyal
2021-10-20 14:54           ` [Virtio-fs] " Vivek Goyal
2021-10-11  3:00 ` [PATCH v6 4/7] fuse: negotiate per-file DAX in FUSE_INIT Jeffle Xu
2021-10-11  3:00   ` [Virtio-fs] " Jeffle Xu
2021-10-18 14:30   ` Vivek Goyal
2021-10-18 14:30     ` [Virtio-fs] " Vivek Goyal
2021-10-20  3:10     ` JeffleXu
2021-10-20  3:10       ` [Virtio-fs] " JeffleXu
2021-10-20 15:44       ` Vivek Goyal
2021-10-20 15:44         ` [Virtio-fs] " Vivek Goyal
2021-10-11  3:00 ` [PATCH v6 5/7] fuse: enable per-file DAX Jeffle Xu
2021-10-11  3:00   ` [Virtio-fs] " Jeffle Xu
2021-10-18 15:11   ` Vivek Goyal
2021-10-18 15:11     ` [Virtio-fs] " Vivek Goyal
2021-10-11  3:00 ` [PATCH v6 6/7] fuse: mark inode DONT_CACHE when per-file DAX hint changes Jeffle Xu
2021-10-11  3:00   ` [Virtio-fs] " Jeffle Xu
2021-10-18 15:19   ` Vivek Goyal
2021-10-18 15:19     ` [Virtio-fs] " Vivek Goyal
2021-10-27  5:05     ` JeffleXu
2021-10-27  5:05       ` [Virtio-fs] " JeffleXu
2021-10-11  3:00 ` [PATCH v6 7/7] Documentation/filesystem/dax: record DAX on virtiofs Jeffle Xu
2021-10-11  3:00   ` [Virtio-fs] " Jeffle Xu
2021-10-15  3:33 ` [PATCH v6 0/7] fuse,virtiofs: support per-file DAX JeffleXu
2021-10-15  3:33   ` [Virtio-fs] " JeffleXu
2021-10-18 15:21 ` Vivek Goyal
2021-10-18 15:21   ` [Virtio-fs] " Vivek Goyal
2021-10-20  5:22   ` JeffleXu
2021-10-20  5:22     ` [Virtio-fs] " JeffleXu
2021-10-20 16:06     ` Vivek Goyal
2021-10-20 16:06       ` [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=20211025190201.GG3465596@iweiny-DESK2.sc.intel.com \
    --to=ira.weiny@intel.com \
    --cc=bo.liu@linux.alibaba.com \
    --cc=dchinner@redhat.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: link
Be 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.