Thread (18 messages) 18 messages, 4 authors, 2021-06-16

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
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help