Linux-man Archive on lore.kernel.org
 help / color / Atom feed
* 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, back to index

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

Linux-man Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-man/0 linux-man/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-man linux-man/ https://lore.kernel.org/linux-man \
		linux-man@vger.kernel.org
	public-inbox-index linux-man

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-man


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git