Re: Bcache upstreaming

[Kent Overstreet <koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>]

On Fri, Feb 01, 2013 at 06:55:04AM -0800, Tejun Heo wrote:
> Hey, Mike.
> 
> On Fri, Feb 01, 2013 at 09:10:03AM -0500, Mike Snitzer wrote:
> > Well judging by the header for commit 49731baa41df404c2c3f44555869ab387363af43  
> > ("block: restore multiple bd_link_disk_holder() support") it just looks
> > like Tejun hates the fact that DM and MD are using this interface.  No
> > alternative is provided; so the "DON'T USE THIS UNLESS YOU'RE ALREADY
> > USING IT." rings hollow.
> 
> The original code was gross regarding kobj handling there so I might
> have overreacted.  Ah right, the refcnt doesn't belong there.  The
> caller should already own both the master and slave devices (creator
> of the former, exclusive opener of the latter) and that really should
> be the extent of ownership that block layer tracks.
> bk_[un]link_disk_holder() implements completely isolated refcnting
> because dm somehow calls the function for the same pair multiple
> times.
> 
> ISTR the problem w/ block layer was that because this adds a separate
> layer of refcnting, it can't be tied to the usual rule of block device
> access.  ie. we really shouldn't need bd_unlink_disk_holder() but the
> linkage's lifetime should be bound to the exclusive open of the slave
> device, which can't currently be done.

Ah ok. Yeah, that refcounting is odd.

> IIRC, there was only one case where this happens in dm, would you be
> interested in tracking that down?  I'd be happy to lose the extra
> refcnting code and tie it back to bdev exclusive open.
> 
> > Kent was talking about using MD (and though he isn't opposed to DM he
> > doesn't care to integrate with DM himself).  Either DM or MD would
> > implicitly enable bcache to use this interface.  But in the near-term I
> > cannot see why Kent shouldn't be able to use bd_link_disk_holder too.
> 
> Being part of dm or dm should make this mostly irrelevant, no?

Yeah, but who knows when that'll actually happen and since this is for
userspace I'm just going to call it. The refcounting won't affect me,
and using it in bcache won't affect ripping that out.