On Fri 20-11-20 10:15:46, Christoph Hellwig wrote: > On Thu, Nov 19, 2020 at 03:39:21PM +0100, Jan Kara wrote: > > This patch is kind of difficult to review due to the size of mostly > > mechanical changes mixed with not completely mechanical changes. Can we > > perhaps split out the mechanical bits? E.g. the rq->part => rq->bdev > > renaming is mechanical and notable part of the patch. Similarly the > > part->foo => part->bd_foo bits... > > We'd end with really weird patches that way. Never mind that I'm not > even sure how we could mechnically do the renaming. Well, I believe coccinelle should be able to do the renaming automatically. > > Also I'm kind of wondering: AFAIU the new lifetime rules, gendisk holds > > bdev reference and bdev is created on gendisk allocation so bdev lifetime is > > strictly larger than gendisk lifetime. But what now keeps bdev->bd_disk > > reference safe in presence device hot unplug? In most cases we are still > > protected by gendisk reference taken in __blkdev_get() but how about > > disk->lookup_sem and disk->flags dereferences before we actually grab the > > reference? > > Good question. I'll need to think about this a bit more. My thinking was that you could use kobject_get_unless_zero(bdev->bd_device->kobj) and after you hold this reference, you can do everything else safely. In this case it is really useful that device is embedded in block_dev and not in gendisk itself... Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR