Re: [PATCH v3 1/4] block: Add concurrent positioning ranges support

["hch@infradead.org" <hch@infradead.org>]

On Tue, Aug 10, 2021 at 11:03:41AM +0000, Damien Le Moal wrote:
> >> + * Dummy release function to make kobj happy.
> >> + */
> >> +static void blk_cranges_sysfs_nop_release(struct kobject *kobj)
> >> +{
> >> +}
> > 
> > How do we ensure the kobj is still alive while it is accessed?
> 
> q->sysfs_lock ensures that. This mutex is taken whenever revalidate registers
> new ranges (see blk_queue_set_cranges below), and is taken also when the ranges
> are unregistered (on revalidate if the ranges changed and when the request queue
> is unregistered). And blk_crange_sysfs_show() takes that lock too. So the kobj
> cannot be freed while it is being accessed (the sysfs inode lock also prevents
> it since kobj_del() will take the inode lock).

Does it?  It only protects the access inside of it, but not the object
lifetime.

> 
> > 
> >> +void blk_queue_set_cranges(struct gendisk *disk, struct blk_cranges *cr)
> > 
> > s/blk_queue/disk/
> 
> Hmmm... The argument is a gendisk, but it is the request queue that is modified.
> So following other functions like this, isn't blk_queue_ prefix better ?

Do we have blk_queue_ functions that take a gendisk anywhere?  The
ones I noticed (and the ones I've recently added) all use disk_.