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)
