From: Ira Weiny <firstname.lastname@example.org> To: Christoph Hellwig <email@example.com> Cc: Jan Kara <firstname.lastname@example.org>, "Darrick J. Wong" <email@example.com>, Dave Chinner <firstname.lastname@example.org>, "Theodore Y. Ts'o" <email@example.com>, Dan Williams <firstname.lastname@example.org>, Linux Kernel Mailing List <email@example.com>, Alexander Viro <firstname.lastname@example.org>, linux-ext4 <email@example.com>, linux-xfs <firstname.lastname@example.org>, linux-fsdevel <email@example.com>, Andrew Morton <firstname.lastname@example.org>, Linus Torvalds <email@example.com> Subject: Re: [PATCH V5 00/12] Enable per-file/per-directory DAX operations V5 Date: Fri, 3 Apr 2020 08:48:29 -0700 [thread overview] Message-ID: <20200403154828.GJ3952565@iweiny-DESK2.sc.intel.com> (raw) In-Reply-To: <20200403072731.GA24176@lst.de> On Fri, Apr 03, 2020 at 09:27:31AM +0200, Christoph Hellwig wrote: > On Thu, Apr 02, 2020 at 01:55:19PM -0700, Ira Weiny wrote: > > > I'd just return an error for that case, don't play silly games like > > > evicting the inode. > > > > I think I agree with Christoph here. But I want to clarify. I was heading in > > a direction of failing the ioctl completely. But we could have the flag change > > with an appropriate error which could let the user know the change has been > > delayed. > > > > But I don't immediately see what error code is appropriate for such an > > indication. Candidates I can envision: > > > > EAGAIN > > ERESTART > > EUSERS > > EINPROGRESS > > > > None are perfect but I'm leaning toward EINPROGRESS. > > I really, really dislike that idea. The whole point of not forcing > evictions is to make it clear - no this inode is "busy" you can't > do that. A reasonably smart application can try to evict itself. I don't understand. What Darrick proposed would never need any evictions. If the file has blocks allocated the FS_XFLAG_DAX flag can not be changed. So I don't see what good eviction would do at all. > > But returning an error and doing a lazy change anyway is straight from > the playbook for arcane and confusing API designs. Jan countered with a proposal that the FS_XFLAG_DAX does change with blocks allocated. But that S_DAX would change on eviction. Adding that some eviction ioctl could be added. You then proposed just returning an error for that case. (This lead me to believe that you were ok with an eviction based change of S_DAX.) So I agreed that changing S_DAX could be delayed until an explicit eviction. But, to aid the 'smart application', a different error code could be used to indicate that the FS_XFLAG_DAX had been changed but that until that explicit eviction occurs S_DAX would remain. So I don't fully follow what you mean by 'lazy change'? Do you still really, really dislike an explicit eviction method for changing the S_DAX flag? If FS_XFLAG_DAX can never be changed on a file with blocks allocated and the user wants to change the mode of operations on their 'data'; they would have to create a new file with the proper setting and move the data there. For example copy the file into a directory marked FS_XFLAG_DAX==true? I'm ok with either interface as I think both could be clear if documented. Jan? Ira
next prev parent reply other threads:[~2020-04-03 15:48 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 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 [this message] 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=20200403154828.GJ3952565@iweiny-DESK2.sc.intel.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 \ --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).