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