Re: [PATCH v3 3/9] btrfs: hunt down the BUG_ON()s inside btrfs_submit_compressed_read()
From: Qu Wenruo <hidden>
Date: 2021-06-16 22:24:09
On 2021/6/16 下午10:03, David Sterba wrote:
On Tue, Jun 15, 2021 at 08:18:30PM +0800, Qu Wenruo wrote:quoted
There are quite some BUG_ON()s inside btrfs_submit_compressed_read(), namingly all errors inside the for() loop relies on BUG_ON() to handle -ENOMEM. Hunt down these BUG_ON()s properly by: - Introduce compressed_bio::pending_bios_wait This allows us to wait for any submitted bio to finish, while still keeps the compressed_bio from being freed, as we should have compressed_bio::io_sectors not zero. - Introduce finish_compressed_bio_read() to finish the compressed_bio - Properly end the bio and finish compressed_bio when error happens Now in btrfs_submit_compressed_read() even when the bio submission failed, we can properly handle the error without triggering BUG_ON(). Signed-off-by: Qu Wenruo <redacted>Please change the subject to something like "btrfs: do proper error handling in btrfs_submit_compressed_read", same for the other patch.
Got it, will change the naming scheme for future patches too.
[...]
quoted
+ /* To wait for any submitted bio, used in error handling */ + wait_queue_head_t pending_bio_wait;This adds 24 bytes to the structure and it's only used for error handling, so that does not seem justified enough.
And after more consideration, in fact we can just remove compressed_bio::pending_bios completely, using io_sectors to replace it. For error handling case, finally we will have things like @cur_disk_bytenr to trace our submitted compressed_bio progress, and we can use io_sectors to wait. I'll go this direction in next update so that no size bump at all for compressed_bio structure. Thanks, Qu
There are system-wide wait queues, shared with other subsystems but it looks like a better fit for the exceptional case of errors. See commit 6b2bb7265f0b62605 for more details, the change is otherwise trivial and the api functions are wait_var_event(&variable, condition) and wake_up_var(&variable), where the variable is a unique key which would be the compressed_bio.quoted
+ /* Number of compressed pages in the array */ unsigned int nr_pages; -- 2.32.0