On Wed 21-06-23 13:56:59, Kent Overstreet wrote: > On Wed, Jun 21, 2023 at 06:23:26PM +0200, Jan Kara wrote: > > Allocate holder object (cache or cached_dev) before offloading the > > rest of the startup to async work. This will allow us to open the block > > block device with proper holder. > > This is a pretty big change for this fix, and we'd want to retest the > error paths - that's hard to do, because the fault injection framework I > was using for that never made it upstream. I agree those are somewhat difficult to test. Although with memory allocation error injection, we can easily simulate failures in alloc_holder_object() or say later in bcache_device_init() if that's what you're after to give at least some testing to the error paths. Admittedly, I've just tested that registering and unregistering bcache devices works without giving warnings. Or are you more worried about the "reopen the block device" logic (and error handling) in the second patch? > What about just exposing a proper API for changing the holder? Wouldn't > that be simpler? It would be doable but frankly I'd prefer to avoid implementing the API for changing the holder just for bcache. For all I care bcache can also just generate random cookie (or an id from IDA) and pass it as a holder to blkdev_get_by_dev(). It would do the job as well and we then don't have to play games with changing the holder. It would just need to be propagated to the places doing blkdev_put(). Do you find that better? I'm now working on a changes that will make blkdev_get_by_dev() return bdev_handle (which contains bdev pointer but also other stuff we need to propagate to blkdev_put()) so when that is done, the cookie propagation will happen automatically. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
