Re: [RFC PATCH] btrfs: don't call btrfs_sync_file from iomap context

Darrick J. Wong
2020-09-02T16:30:02Z

On Wed, Sep 02, 2020 at 07:10:08AM -0400, Josef Bacik wrote:
> On 9/2/20 3:12 AM, Johannes Thumshirn wrote:
> > On 02/09/2020 02:22, Josef Bacik wrote:
> > > Instead now we have to rip
> > > it out until we figure out what to do about it.
> >
> > I don't think we need to rip out the iomap conversion. We can
> > take my fix albeit not pretty, until we have reworked the locking
> > around ->fsync(). Probably with a big fat comment attached to it.
> >
>
> We do, because your fix breaks DSYNC for AIO. You didn't hit this with
> direct io, you hit it with AIO, and the reason you hit it is because you are
> on zram, so your bio's completed before we exited iomap_dio_rw. So that was
> the last put on the iomap_dio, and thus we ran
> iomap_dio_complete() and deadlocked. We can't just drop the DSYNC thing for
> AIO because in the normal case where this doesn't happen we need to know
> when the last thing is finished in order to run ->fsync(), we can't just run
> it after submission. Thanks,

Bleh, Oracle mail (or vger or something) is being slow again...

It occurred to me that we added iomap_dio_ops.submit_io for the benefit
of btrfs. Could we solve all this for now by adding a ->write_sync
function pointer to iomap_dio_ops that could lead back into a btrfs
function that would flush the necessary bits without itself taking the
inode lock? And if a ->write_sync is not supplied, then the caller gets
generic_write_sync?

It's kind of a bandaid, but maybe less bad of one than restructuring the
btrfs locking model under time pressure...

--D

> Josef