On 2022/4/15 15:14, Qu Wenruo wrote:
On 2022/4/15 15:12, Christoph Hellwig wrote:
On Fri, Apr 15, 2022 at 03:02:41PM +0800, Qu Wenruo wrote:
But this can not be said to btrfs_submit_compressed_read(), which has
the same problem and can be triggered by EIO error easily.
Do you want to give it a try? Or mind to me fix it?
I don't really know the btrfs writeback and compression code very well,
so if you can tackle it that would be great. I'll review it and will
ask lots of stupid question in exchange :)
No stupid questions at all.
Great we have some extra reviewing eyes!
Thanks for your review in advance.
Qu
OK, it turns out things are better than we thought.
For btrfs_submit_compressed_read() and btrfs_submit_compressed_write(),
we have a different (and correct) way handling the endio.
Let's look at btrfs_submit_compressed_read() as an example.
If functions like btrfs_lookup_bio_sums() failed, although we go
finish_cb: label, our @cur_disk_byte is already updated.
Then under finish_cb: label, we first finish the current @comp_bio,
which may:
1. Decrease cb::pending_bytes and it reached 0.
Call finish_compressed_bio_read() as we're the last pending bytes.
2. Just decrease cb::pending_bytes
There are still other pending ios.
For case 1, our @cur_disk_bytnr has already reached our range end, thus
we won't do anything but exit early, without manually calling
finish_compressed_bio_read().
For case 2, we continue to wait_var_event() first, to wait all bios
on-the-fly to finish.
As since the pending_sectors will never reach 0, no one is going to
finish the @cb.
Then we're safe to call finish_compressed_bio_read() then.
In fact, those are exact what I fixed in commit 6853c64a6e76 ("btrfs:
handle errors properly inside btrfs_submit_compressed_write()") and
86ccbb4d2a2a ("btrfs: handle errors properly inside
btrfs_submit_compressed_read()").
In fact, when I saw the overkilled usage of ASSERT()s, I should know
it's myself...
So in fact we're safe since v5.16.
Thanks,
Qu
