On 9/2/20 12:29 PM, Darrick J. Wong wrote:
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...
I'd rather not mess around with the generic iomap stuff for this, coordinating
changes between generic and fs stuff is annoying enough as it is. We've got a
strategy to work around this in btrfs so we don't have to rip out the iomap work
right now. And then we'll rip out the workaround once we've reworked the
locking, since the locking stuff will require a fair bit of testing and soak
time to be sure it's safe. Thanks,
Josef
