Re: [PATCH 1/2] bcache: Alloc holder object before async registration

[Jan Kara <jack@suse.cz>]

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@suse.com>
SUSE Labs, CR