* Re: RFC: iomap write invalidation
[not found] ` <20200721150615.GA10330@lst.de>
@ 2020-07-21 15:14 ` Matthew Wilcox
2020-07-21 15:16 ` Christoph Hellwig
0 siblings, 1 reply; 11+ messages in thread
From: Matthew Wilcox @ 2020-07-21 15:14 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Goldwyn Rodrigues, Dave Chinner, Damien Le Moal, Naohiro Aota,
Johannes Thumshirn, linux-btrfs, linux-fsdevel, cluster-devel,
linux-ext4, linux-xfs, Michael Kerrisk, linux-man
On Tue, Jul 21, 2020 at 05:06:15PM +0200, Christoph Hellwig wrote:
> On Tue, Jul 21, 2020 at 04:04:32PM +0100, Matthew Wilcox wrote:
> > I thought you were going to respin this with EREMCHG changed to ENOTBLK?
>
> Oh, true. I'll do that ASAP.
Michael, could we add this to manpages?
--- write.2.old 2020-07-21 11:11:17.740491825 -0400
+++ write.2 2020-07-21 11:13:05.900389249 -0400
@@ -192,6 +192,12 @@
.IR count ,
or the file offset is not suitably aligned.
.TP
+.B ENOTBLK
+The file was opened with the
+.B O_DIRECT
+flag, and the I/O could not be completed due to another process using
+the file.
+.TP
.B EIO
A low-level I/O error occurred while modifying the inode.
This error may relate to the write-back of data written by an earlier
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC: iomap write invalidation
2020-07-21 15:14 ` RFC: iomap write invalidation Matthew Wilcox
@ 2020-07-21 15:16 ` Christoph Hellwig
2020-07-21 15:27 ` Darrick J. Wong
2020-07-21 15:31 ` Matthew Wilcox
0 siblings, 2 replies; 11+ messages in thread
From: Christoph Hellwig @ 2020-07-21 15:16 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Christoph Hellwig, Goldwyn Rodrigues, Dave Chinner,
Damien Le Moal, Naohiro Aota, Johannes Thumshirn, linux-btrfs,
linux-fsdevel, cluster-devel, linux-ext4, linux-xfs,
Michael Kerrisk, linux-man
On Tue, Jul 21, 2020 at 04:14:37PM +0100, Matthew Wilcox wrote:
> On Tue, Jul 21, 2020 at 05:06:15PM +0200, Christoph Hellwig wrote:
> > On Tue, Jul 21, 2020 at 04:04:32PM +0100, Matthew Wilcox wrote:
> > > I thought you were going to respin this with EREMCHG changed to ENOTBLK?
> >
> > Oh, true. I'll do that ASAP.
>
> Michael, could we add this to manpages?
Umm, no. -ENOTBLK is internal - the file systems will retry using
buffered I/O and the error shall never escape to userspace (or even the
VFS for that matter).
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC: iomap write invalidation
2020-07-21 15:16 ` Christoph Hellwig
@ 2020-07-21 15:27 ` Darrick J. Wong
2020-07-21 15:41 ` Christoph Hellwig
2020-07-21 15:31 ` Matthew Wilcox
1 sibling, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2020-07-21 15:27 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Matthew Wilcox, Goldwyn Rodrigues, Dave Chinner, Damien Le Moal,
Naohiro Aota, Johannes Thumshirn, linux-btrfs, linux-fsdevel,
cluster-devel, linux-ext4, linux-xfs, Michael Kerrisk, linux-man
On Tue, Jul 21, 2020 at 05:16:16PM +0200, Christoph Hellwig wrote:
> On Tue, Jul 21, 2020 at 04:14:37PM +0100, Matthew Wilcox wrote:
> > On Tue, Jul 21, 2020 at 05:06:15PM +0200, Christoph Hellwig wrote:
> > > On Tue, Jul 21, 2020 at 04:04:32PM +0100, Matthew Wilcox wrote:
> > > > I thought you were going to respin this with EREMCHG changed to ENOTBLK?
> > >
> > > Oh, true. I'll do that ASAP.
> >
> > Michael, could we add this to manpages?
>
> Umm, no. -ENOTBLK is internal - the file systems will retry using
> buffered I/O and the error shall never escape to userspace (or even the
> VFS for that matter).
It's worth dropping a comment somewhere that ENOTBLK is the desired
"fall back to buffered" errcode, seeing as Dave and I missed that in
XFS...
--D
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC: iomap write invalidation
2020-07-21 15:16 ` Christoph Hellwig
2020-07-21 15:27 ` Darrick J. Wong
@ 2020-07-21 15:31 ` Matthew Wilcox
2020-07-21 15:42 ` Christoph Hellwig
1 sibling, 1 reply; 11+ messages in thread
From: Matthew Wilcox @ 2020-07-21 15:31 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Goldwyn Rodrigues, Dave Chinner, Damien Le Moal, Naohiro Aota,
Johannes Thumshirn, linux-btrfs, linux-fsdevel, cluster-devel,
linux-ext4, linux-xfs, Michael Kerrisk, linux-man
On Tue, Jul 21, 2020 at 05:16:16PM +0200, Christoph Hellwig wrote:
> On Tue, Jul 21, 2020 at 04:14:37PM +0100, Matthew Wilcox wrote:
> > On Tue, Jul 21, 2020 at 05:06:15PM +0200, Christoph Hellwig wrote:
> > > On Tue, Jul 21, 2020 at 04:04:32PM +0100, Matthew Wilcox wrote:
> > > > I thought you were going to respin this with EREMCHG changed to ENOTBLK?
> > >
> > > Oh, true. I'll do that ASAP.
> >
> > Michael, could we add this to manpages?
>
> Umm, no. -ENOTBLK is internal - the file systems will retry using
> buffered I/O and the error shall never escape to userspace (or even the
> VFS for that matter).
Ah, I made the mistake of believing the comments that I could see in
your patch instead of reading the code.
Can I suggest deleting this comment:
/*
* No fallback to buffered IO on errors for XFS, direct IO will either
* complete fully or fail.
*/
and rewording this one:
/*
* Allow a directio write to fall back to a buffered
* write *only* in the case that we're doing a reflink
* CoW. In all other directio scenarios we do not
* allow an operation to fall back to buffered mode.
*/
as part of your revised patchset?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC: iomap write invalidation
2020-07-21 15:27 ` Darrick J. Wong
@ 2020-07-21 15:41 ` Christoph Hellwig
2020-07-21 15:59 ` Darrick J. Wong
0 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2020-07-21 15:41 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Christoph Hellwig, Matthew Wilcox, Goldwyn Rodrigues,
Dave Chinner, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
linux-btrfs, linux-fsdevel, cluster-devel, linux-ext4, linux-xfs,
Michael Kerrisk, linux-man
On Tue, Jul 21, 2020 at 08:27:54AM -0700, Darrick J. Wong wrote:
> On Tue, Jul 21, 2020 at 05:16:16PM +0200, Christoph Hellwig wrote:
> > On Tue, Jul 21, 2020 at 04:14:37PM +0100, Matthew Wilcox wrote:
> > > On Tue, Jul 21, 2020 at 05:06:15PM +0200, Christoph Hellwig wrote:
> > > > On Tue, Jul 21, 2020 at 04:04:32PM +0100, Matthew Wilcox wrote:
> > > > > I thought you were going to respin this with EREMCHG changed to ENOTBLK?
> > > >
> > > > Oh, true. I'll do that ASAP.
> > >
> > > Michael, could we add this to manpages?
> >
> > Umm, no. -ENOTBLK is internal - the file systems will retry using
> > buffered I/O and the error shall never escape to userspace (or even the
> > VFS for that matter).
>
> It's worth dropping a comment somewhere that ENOTBLK is the desired
> "fall back to buffered" errcode, seeing as Dave and I missed that in
> XFS...
Sounds like a good idea, but what would a good place be?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC: iomap write invalidation
2020-07-21 15:31 ` Matthew Wilcox
@ 2020-07-21 15:42 ` Christoph Hellwig
2020-07-21 15:52 ` Matthew Wilcox
0 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2020-07-21 15:42 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Christoph Hellwig, Goldwyn Rodrigues, Dave Chinner,
Damien Le Moal, Naohiro Aota, Johannes Thumshirn, linux-btrfs,
linux-fsdevel, cluster-devel, linux-ext4, linux-xfs,
Michael Kerrisk, linux-man
On Tue, Jul 21, 2020 at 04:31:36PM +0100, Matthew Wilcox wrote:
> > Umm, no. -ENOTBLK is internal - the file systems will retry using
> > buffered I/O and the error shall never escape to userspace (or even the
> > VFS for that matter).
>
> Ah, I made the mistake of believing the comments that I could see in
> your patch instead of reading the code.
>
> Can I suggest deleting this comment:
>
> /*
> * No fallback to buffered IO on errors for XFS, direct IO will either
> * complete fully or fail.
> */
>
> and rewording this one:
>
> /*
> * Allow a directio write to fall back to a buffered
> * write *only* in the case that we're doing a reflink
> * CoW. In all other directio scenarios we do not
> * allow an operation to fall back to buffered mode.
> */
>
> as part of your revised patchset?
That isn't actually true. In current mainline we only fallback on
reflink RMW cases, but with this series we also fall back for
invalidation failures.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC: iomap write invalidation
2020-07-21 15:42 ` Christoph Hellwig
@ 2020-07-21 15:52 ` Matthew Wilcox
2020-07-21 16:03 ` Darrick J. Wong
0 siblings, 1 reply; 11+ messages in thread
From: Matthew Wilcox @ 2020-07-21 15:52 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Goldwyn Rodrigues, Dave Chinner, Damien Le Moal, Naohiro Aota,
Johannes Thumshirn, linux-btrfs, linux-fsdevel, cluster-devel,
linux-ext4, linux-xfs, Michael Kerrisk, linux-man
On Tue, Jul 21, 2020 at 05:42:40PM +0200, Christoph Hellwig wrote:
> On Tue, Jul 21, 2020 at 04:31:36PM +0100, Matthew Wilcox wrote:
> > > Umm, no. -ENOTBLK is internal - the file systems will retry using
> > > buffered I/O and the error shall never escape to userspace (or even the
> > > VFS for that matter).
> >
> > Ah, I made the mistake of believing the comments that I could see in
> > your patch instead of reading the code.
> >
> > Can I suggest deleting this comment:
> >
> > /*
> > * No fallback to buffered IO on errors for XFS, direct IO will either
> > * complete fully or fail.
> > */
> >
> > and rewording this one:
> >
> > /*
> > * Allow a directio write to fall back to a buffered
> > * write *only* in the case that we're doing a reflink
> > * CoW. In all other directio scenarios we do not
> > * allow an operation to fall back to buffered mode.
> > */
> >
> > as part of your revised patchset?
>
> That isn't actually true. In current mainline we only fallback on
> reflink RMW cases, but with this series we also fall back for
> invalidation failures.
... that's why I'm suggesting that you delete the first one and rewrite
the second one. Because they aren't true.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC: iomap write invalidation
2020-07-21 15:41 ` Christoph Hellwig
@ 2020-07-21 15:59 ` Darrick J. Wong
2020-07-21 16:01 ` Christoph Hellwig
0 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2020-07-21 15:59 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Matthew Wilcox, Goldwyn Rodrigues, Dave Chinner, Damien Le Moal,
Naohiro Aota, Johannes Thumshirn, linux-btrfs, linux-fsdevel,
cluster-devel, linux-ext4, linux-xfs, Michael Kerrisk, linux-man
On Tue, Jul 21, 2020 at 05:41:32PM +0200, Christoph Hellwig wrote:
> On Tue, Jul 21, 2020 at 08:27:54AM -0700, Darrick J. Wong wrote:
> > On Tue, Jul 21, 2020 at 05:16:16PM +0200, Christoph Hellwig wrote:
> > > On Tue, Jul 21, 2020 at 04:14:37PM +0100, Matthew Wilcox wrote:
> > > > On Tue, Jul 21, 2020 at 05:06:15PM +0200, Christoph Hellwig wrote:
> > > > > On Tue, Jul 21, 2020 at 04:04:32PM +0100, Matthew Wilcox wrote:
> > > > > > I thought you were going to respin this with EREMCHG changed to ENOTBLK?
> > > > >
> > > > > Oh, true. I'll do that ASAP.
> > > >
> > > > Michael, could we add this to manpages?
> > >
> > > Umm, no. -ENOTBLK is internal - the file systems will retry using
> > > buffered I/O and the error shall never escape to userspace (or even the
> > > VFS for that matter).
> >
> > It's worth dropping a comment somewhere that ENOTBLK is the desired
> > "fall back to buffered" errcode, seeing as Dave and I missed that in
> > XFS...
>
> Sounds like a good idea, but what would a good place be?
In the comment that precedes iomap_dio_rw() for the iomap version,
and...
...ye $deity, the old direct-io.c file is a mess of wrappers. Uh... a
new comment preceding __blockdev_direct_IO? Or blockdev_direct_IO? Or
both?
Or I guess the direct_IO documentation in vfs.rst...?
``direct_IO``
called by the generic read/write routines to perform direct_IO -
that is IO requests which bypass the page cache and transfer
data directly between the storage and the application's address
space. This function can return -ENOTBLK to signal that it is
necessary to fallback to buffered IO. Note that
blockdev_direct_IO and variants can also return -ENOTBLK.
--D
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC: iomap write invalidation
2020-07-21 15:59 ` Darrick J. Wong
@ 2020-07-21 16:01 ` Christoph Hellwig
2020-07-21 16:05 ` Darrick J. Wong
0 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2020-07-21 16:01 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Christoph Hellwig, Matthew Wilcox, Goldwyn Rodrigues,
Dave Chinner, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
linux-btrfs, linux-fsdevel, cluster-devel, linux-ext4, linux-xfs,
Michael Kerrisk, linux-man
On Tue, Jul 21, 2020 at 08:59:25AM -0700, Darrick J. Wong wrote:
> In the comment that precedes iomap_dio_rw() for the iomap version,
maybe let's just do that..
> ``direct_IO``
> called by the generic read/write routines to perform direct_IO -
> that is IO requests which bypass the page cache and transfer
> data directly between the storage and the application's address
> space. This function can return -ENOTBLK to signal that it is
> necessary to fallback to buffered IO. Note that
> blockdev_direct_IO and variants can also return -ENOTBLK.
->direct_IO is not used for iomap and various other implementations.
In fact it is a horrible hack that I've been trying to get rid of
for a while.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC: iomap write invalidation
2020-07-21 15:52 ` Matthew Wilcox
@ 2020-07-21 16:03 ` Darrick J. Wong
0 siblings, 0 replies; 11+ messages in thread
From: Darrick J. Wong @ 2020-07-21 16:03 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Christoph Hellwig, Goldwyn Rodrigues, Dave Chinner,
Damien Le Moal, Naohiro Aota, Johannes Thumshirn, linux-btrfs,
linux-fsdevel, cluster-devel, linux-ext4, linux-xfs,
Michael Kerrisk, linux-man
On Tue, Jul 21, 2020 at 04:52:01PM +0100, Matthew Wilcox wrote:
> On Tue, Jul 21, 2020 at 05:42:40PM +0200, Christoph Hellwig wrote:
> > On Tue, Jul 21, 2020 at 04:31:36PM +0100, Matthew Wilcox wrote:
> > > > Umm, no. -ENOTBLK is internal - the file systems will retry using
> > > > buffered I/O and the error shall never escape to userspace (or even the
> > > > VFS for that matter).
> > >
> > > Ah, I made the mistake of believing the comments that I could see in
> > > your patch instead of reading the code.
> > >
> > > Can I suggest deleting this comment:
> > >
> > > /*
> > > * No fallback to buffered IO on errors for XFS, direct IO will either
> > > * complete fully or fail.
> > > */
> > >
> > > and rewording this one:
> > >
> > > /*
> > > * Allow a directio write to fall back to a buffered
> > > * write *only* in the case that we're doing a reflink
> > > * CoW. In all other directio scenarios we do not
> > > * allow an operation to fall back to buffered mode.
> > > */
> > >
> > > as part of your revised patchset?
> >
> > That isn't actually true. In current mainline we only fallback on
> > reflink RMW cases, but with this series we also fall back for
> > invalidation failures.
>
> ... that's why I'm suggesting that you delete the first one and rewrite
> the second one. Because they aren't true.
/*
* We allow only three outcomes of a directio: (1) it succeeds
* completely; (2) it fails with a negative error code; or (3) it
* returns -ENOTBLK to signal that we should fall back to buffered IO.
*
* The third scenario should only happen if iomap refuses to perform the
* direct IO, or the direct write request requires CoW but is not aligned
* to the filesystem block size.
*/
?
--D
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC: iomap write invalidation
2020-07-21 16:01 ` Christoph Hellwig
@ 2020-07-21 16:05 ` Darrick J. Wong
0 siblings, 0 replies; 11+ messages in thread
From: Darrick J. Wong @ 2020-07-21 16:05 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Matthew Wilcox, Goldwyn Rodrigues, Dave Chinner, Damien Le Moal,
Naohiro Aota, Johannes Thumshirn, linux-btrfs, linux-fsdevel,
cluster-devel, linux-ext4, linux-xfs, Michael Kerrisk, linux-man
On Tue, Jul 21, 2020 at 06:01:43PM +0200, Christoph Hellwig wrote:
> On Tue, Jul 21, 2020 at 08:59:25AM -0700, Darrick J. Wong wrote:
> > In the comment that precedes iomap_dio_rw() for the iomap version,
>
> maybe let's just do that..
>
> > ``direct_IO``
> > called by the generic read/write routines to perform direct_IO -
> > that is IO requests which bypass the page cache and transfer
> > data directly between the storage and the application's address
> > space. This function can return -ENOTBLK to signal that it is
> > necessary to fallback to buffered IO. Note that
> > blockdev_direct_IO and variants can also return -ENOTBLK.
>
> ->direct_IO is not used for iomap and various other implementations.
> In fact it is a horrible hack that I've been trying to get rid of
> for a while.
Agreed, but for now there are still a number of fses who are still on
the old directio code; let's try to keep the drainbamage/confusion
potential to a minimum so it doesn't spread to our iomap shinyness. :)
--D
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-07-21 16:06 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200713074633.875946-1-hch@lst.de>
[not found] ` <20200720215125.bfz7geaftocy4r5l@fiona>
[not found] ` <20200721145313.GA9217@lst.de>
[not found] ` <20200721150432.GH15516@casper.infradead.org>
[not found] ` <20200721150615.GA10330@lst.de>
2020-07-21 15:14 ` RFC: iomap write invalidation Matthew Wilcox
2020-07-21 15:16 ` Christoph Hellwig
2020-07-21 15:27 ` Darrick J. Wong
2020-07-21 15:41 ` Christoph Hellwig
2020-07-21 15:59 ` Darrick J. Wong
2020-07-21 16:01 ` Christoph Hellwig
2020-07-21 16:05 ` Darrick J. Wong
2020-07-21 15:31 ` Matthew Wilcox
2020-07-21 15:42 ` Christoph Hellwig
2020-07-21 15:52 ` Matthew Wilcox
2020-07-21 16:03 ` Darrick J. Wong
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).