Re: [RFC] what to do with IOCB_DSYNC?

[Al Viro <viro@zeniv.linux.org.uk>]

On Sun, May 22, 2022 at 09:43:27AM +0200, Christoph Hellwig wrote:
> On Sat, May 21, 2022 at 05:48:42PM +0000, Al Viro wrote:
> > There's a problem with that variant.  Take a look at btrfs_direct_write():
> >         const bool is_sync_write = (iocb->ki_flags & IOCB_DSYNC);
> > 	...
> >         /*
> > 	 * We remove IOCB_DSYNC so that we don't deadlock when iomap_dio_rw()
> > 	 * calls generic_write_sync() (through iomap_dio_complete()), because
> > 	 * that results in calling fsync (btrfs_sync_file()) which will try to
> > 	 * lock the inode in exclusive/write mode.
> > 	 */
> > 	if (is_sync_write)
> > 		iocb->ki_flags &= ~IOCB_DSYNC;
> > 	...
> > 
> > 	/*
> > 	 * Add back IOCB_DSYNC. Our caller, btrfs_file_write_iter(), will do  
> > 	 * the fsync (call generic_write_sync()).
> > 	 */
> > 	if (is_sync_write)
> > 		iocb->ki_flags |= IOCB_DSYNC;
> > 
> > will run into trouble.  How about this (completely untested):
> 
> Which is pretty gross.  We can just add a IOMAP_DIO_NOSYNC flag
> to do what btrfs wants is a much less gross way.

Add
#define IOMAP_DIO_NOSYNC (1 << 3)
in iomap.h, pass IOMAP_DIO_PARTIAL | IOMAP_DIO_NOSYNC in btrfs and do
                if (iocb_is_dsync(iocb) && !(dio->flags & IOMAP_DIO_NOSYNC)) {
in __iomap_dio_rw(), you mean?

I wonder if we want something of that sort in another user of IOMAP_DIO_PARTIAL
(gfs2, that is)...

> Eww.  I don't think we should grow struct file for tht.

Not a problem - this one is only used before the refcount hits zero and
neither fu_llist nor fu_rcuhead are used until that happens.  So it can
go into the same union (and it's past time to make it that union anonymous,
IMO)