From: Jan Kara <firstname.lastname@example.org> To: "Darrick J. Wong" <email@example.com> Cc: Christoph Hellwig <firstname.lastname@example.org>, Dave Chinner <email@example.com>, "Theodore Y. Ts'o" <firstname.lastname@example.org>, Dan Williams <email@example.com>, Jan Kara <firstname.lastname@example.org>, Ira Weiny <email@example.com>, Linux Kernel Mailing List <firstname.lastname@example.org>, Alexander Viro <email@example.com>, linux-ext4 <firstname.lastname@example.org>, linux-xfs <email@example.com>, linux-fsdevel <firstname.lastname@example.org>, Andrew Morton <email@example.com>, Linus Torvalds <firstname.lastname@example.org> Subject: Re: [PATCH V5 00/12] Enable per-file/per-directory DAX operations V5 Date: Wed, 1 Apr 2020 12:25:11 +0200 [thread overview] Message-ID: <20200401102511.GC19466@quack2.suse.cz> (raw) In-Reply-To: <20200401040021.GC56958@magnolia> On Wed 01-04-20 04:00:21, Darrick J. Wong wrote: > On Mon, Mar 16, 2020 at 10:55:09AM +0100, Christoph Hellwig wrote: > > On Mon, Mar 16, 2020 at 10:52:24AM +0100, Jan Kara wrote: > > > > This sounds reasonable to me. > > > > > > > > As for deprecating the mount option, I think at a minimum it needs to > > > > continue be accepted as an option even if it is ignored to not break > > > > existing setups. > > > > > > Agreed. But that's how we usually deprecate mount options. Also I'd say > > > that statx() support for reporting DAX state and some education of > > > programmers using DAX is required before we deprecate the mount option > > > since currently applications check 'dax' mount option to determine how much > > > memory they need to set aside for page cache before they consume everything > > > else on the machine... > > > > I don't even think we should deprecate it. It isn't painful to maintain > > and actually useful for testing. Instead we should expand it into a > > tristate: > > > > dax=off > > dax=flag > > dax=always > > > > where the existing "dax" option maps to "dax=always" and nodax maps > > to "dax=off". and dax=flag becomes the default for DAX capable devices. > > That works for me. In summary: > > - Applications must call statx to discover the current S_DAX state. > > - There exists an advisory file inode flag FS_XFLAG_DAX that can be > changed on files that have no blocks allocated to them. Changing > this flag does not necessarily change the S_DAX state immediately > but programs can query the S_DAX state via statx. I generally like the proposal but I think the fact that toggling FS_XFLAG_DAX will not have immediate effect on S_DAX will cause quite some confusion and ultimately bug reports. I'm thinking whether we could somehow improve this... For example an ioctl that would try to make set inode flags effective by evicting the inode (and returning EBUSY if the eviction is impossible for some reason)? Honza > > If FS_XFLAG_DAX is set and the fs is on pmem then it will always > enable S_DAX at inode load time; if FS_XFLAG_DAX is not set, it will > never enable S_DAX. Unless overridden... > > - There exists a dax= mount option. dax=off means "never set S_DAX, > ignore FS_XFLAG_DAX"; dax=always means "always set S_DAX (at least on > pmem), ignore FS_XFLAG_DAX"; and dax=iflag means "follow FS_XFLAG_DAX" > and is the default. "dax" by itself means "dax=always". "nodax" > means "dax=off". > > - There exists an advisory directory inode flag FS_XFLAG_DAX that can > be changed at any time. The flag state is copied into any files or > subdirectories created within that directory. If programs require > that file access runs in S_DAX mode, they'll have to create those > files themselves inside a directory with FS_XFLAG_DAX set, or mount > the fs with dax=always. > > Ok? Let's please get this part finished for 5.8, then we can get back > to arguing about fs-rmap and reflink and dax and whatnot. > > --D -- Jan Kara <email@example.com> SUSE Labs, CR
next prev parent reply other threads:[~2020-04-01 10:25 UTC|newest] Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-02-27 5:24 ira.weiny 2020-02-27 5:24 ` [PATCH V5 01/12] fs/xfs: Remove unnecessary initialization of i_rwsem ira.weiny 2020-02-27 17:25 ` Ira Weiny 2020-02-27 5:24 ` [PATCH V5 02/12] fs: Remove unneeded IS_DAX() check ira.weiny 2020-02-27 5:24 ` [PATCH V5 03/12] fs/stat: Define DAX statx attribute ira.weiny 2020-02-27 5:24 ` [PATCH V5 04/12] fs/xfs: Isolate the physical DAX flag from enabled ira.weiny 2020-02-27 5:24 ` [PATCH V5 05/12] fs/xfs: Create function xfs_inode_enable_dax() ira.weiny 2020-03-01 22:37 ` Dave Chinner 2020-02-27 5:24 ` [PATCH V5 06/12] fs: Add locking for a dynamic address space operations state ira.weiny 2020-03-02 1:26 ` Dave Chinner 2020-03-02 1:36 ` Dave Chinner 2020-02-27 5:24 ` [PATCH V5 07/12] fs: Prevent DAX state change if file is mmap'ed ira.weiny 2020-02-27 5:24 ` [PATCH V5 08/12] fs/xfs: Hold off aops users while changing DAX state ira.weiny 2020-02-27 5:24 ` [PATCH V5 09/12] fs/xfs: Clean up locking in dax invalidate ira.weiny 2020-02-27 5:24 ` [PATCH V5 10/12] fs/xfs: Allow toggle of effective DAX flag ira.weiny 2020-02-27 5:24 ` [PATCH V5 11/12] fs/xfs: Remove xfs_diflags_to_linux() ira.weiny 2020-02-27 5:24 ` [PATCH V5 12/12] Documentation/dax: Update Usage section ira.weiny 2020-03-05 15:51 ` [PATCH V5 00/12] Enable per-file/per-directory DAX operations V5 Christoph Hellwig 2020-03-09 17:04 ` Ira Weiny 2020-03-11 3:36 ` Darrick J. Wong 2020-03-11 6:29 ` Christoph Hellwig 2020-03-11 17:07 ` Dan Williams 2020-03-16 9:52 ` Jan Kara 2020-03-16 9:55 ` Christoph Hellwig 2020-04-01 4:00 ` Darrick J. Wong 2020-04-01 10:25 ` Jan Kara [this message] 2020-04-02 8:53 ` Christoph Hellwig 2020-04-02 20:55 ` Ira Weiny 2020-04-03 7:27 ` Christoph Hellwig 2020-04-03 15:48 ` Ira Weiny 2020-04-03 17:03 ` Jan Kara 2020-04-03 18:18 ` Ira Weiny 2020-04-03 18:21 ` Ira Weiny 2020-04-03 18:37 ` Darrick J. Wong 2020-04-05 6:19 ` Ira Weiny 2020-04-06 10:00 ` Jan Kara 2020-04-03 18:29 ` Darrick J. Wong 2020-04-03 16:05 ` Darrick J. Wong 2020-04-03 4:39 ` Ira Weiny 2020-03-11 6:39 ` Dave Chinner 2020-03-11 6:44 ` Christoph Hellwig 2020-03-11 17:07 ` Dan Williams 2020-03-12 0:49 ` Dave Chinner 2020-03-12 3:00 ` Darrick J. Wong 2020-03-12 7:27 ` Christoph Hellwig
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=20200401102511.GC19466@quack2.suse.cz \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH V5 00/12] Enable per-file/per-directory DAX operations V5' \ /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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).