From: Josef Bacik <josef@toxicpanda.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Johannes Thumshirn <Johannes.Thumshirn@wdc.com>,
Dave Chinner <david@fromorbit.com>,
David Sterba <dsterba@suse.com>,
"linux-btrfs @ vger . kernel . org" <linux-btrfs@vger.kernel.org>,
Filipe Manana <fdmanana@gmail.com>,
Christoph Hellwig <hch@lst.de>,
Linux FS Devel <linux-fsdevel@vger.kernel.org>
Subject: Re: [RFC PATCH] btrfs: don't call btrfs_sync_file from iomap context
Date: Wed, 2 Sep 2020 12:47:28 -0400 [thread overview]
Message-ID: <d001c8df-1c35-6426-4000-ad1222af7196@toxicpanda.com> (raw)
In-Reply-To: <20200902162944.GH6090@magnolia>
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
next prev parent reply other threads:[~2020-09-02 16:47 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-01 13:06 [RFC PATCH] btrfs: don't call btrfs_sync_file from iomap context Johannes Thumshirn
2020-09-01 13:11 ` Johannes Thumshirn
2020-09-01 14:17 ` Goldwyn Rodrigues
2020-09-01 14:20 ` Johannes Thumshirn
2020-09-01 14:37 ` Filipe Manana
2020-09-01 14:44 ` Johannes Thumshirn
2020-09-01 18:40 ` Goldwyn Rodrigues
2020-09-01 15:11 ` Josef Bacik
2020-09-01 17:45 ` Darrick J. Wong
2020-09-01 17:55 ` Josef Bacik
2020-09-01 21:46 ` Dave Chinner
2020-09-01 22:19 ` Josef Bacik
2020-09-01 23:58 ` Dave Chinner
2020-09-02 0:22 ` Josef Bacik
2020-09-02 7:12 ` Johannes Thumshirn
2020-09-02 11:10 ` Josef Bacik
2020-09-02 16:29 ` Darrick J. Wong
2020-09-02 16:47 ` Josef Bacik [this message]
2020-09-02 11:44 ` Matthew Wilcox
2020-09-02 12:20 ` Dave Chinner
2020-09-02 12:42 ` Josef Bacik
2020-09-03 2:28 ` Dave Chinner
2020-09-03 9:49 ` Filipe Manana
2020-09-03 16:32 ` Christoph Hellwig
2020-09-03 16:46 ` Josef Bacik
2020-09-07 0:04 ` Dave Chinner
2020-09-15 21:48 ` Goldwyn Rodrigues
2020-09-17 3:09 ` Dave Chinner
2020-09-17 5:52 ` Christoph Hellwig
2020-09-17 6:29 ` Dave Chinner
2020-09-17 6:42 ` Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d001c8df-1c35-6426-4000-ad1222af7196@toxicpanda.com \
--to=josef@toxicpanda.com \
--cc=Johannes.Thumshirn@wdc.com \
--cc=darrick.wong@oracle.com \
--cc=david@fromorbit.com \
--cc=dsterba@suse.com \
--cc=fdmanana@gmail.com \
--cc=hch@lst.de \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.