| Message ID | 20210722075402.983367-4-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/9] block: delay freeing the gendisk | expand |
On Thu, Jul 22, 2021 at 09:53:56AM +0200, Christoph Hellwig wrote: > Unhash the whole device inode early in del_gendisk. This allows to > remove the first GENHD_FL_UP check in the open path as we simply > won't find a just removed inode. The second non-racy check after > taking open_mutex is still kept. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > block/genhd.c | 7 +------ > fs/block_dev.c | 2 +- > 2 files changed, 2 insertions(+), 7 deletions(-) > > diff --git a/block/genhd.c b/block/genhd.c > index 298ee78c1bda..716f5ca479ad 100644 > --- a/block/genhd.c > +++ b/block/genhd.c > @@ -585,6 +585,7 @@ void del_gendisk(struct gendisk *disk) > disk_del_events(disk); > > mutex_lock(&disk->open_mutex); > + remove_inode_hash(disk->part0->bd_inode); > disk->flags &= ~GENHD_FL_UP; > blk_drop_partitions(disk); > mutex_unlock(&disk->open_mutex); > @@ -592,12 +593,6 @@ void del_gendisk(struct gendisk *disk) > fsync_bdev(disk->part0); > __invalidate_device(disk->part0, true); > > - /* > - * Unhash the bdev inode for this device so that it can't be looked > - * up any more even if openers still hold references to it. > - */ > - remove_inode_hash(disk->part0->bd_inode); > - > set_capacity(disk, 0); > > if (!(disk->flags & GENHD_FL_HIDDEN)) { > diff --git a/fs/block_dev.c b/fs/block_dev.c > index 9ef4f1fc2cb0..932f4034ad66 100644 > --- a/fs/block_dev.c > +++ b/fs/block_dev.c > @@ -1340,7 +1340,7 @@ struct block_device *blkdev_get_no_open(dev_t dev) > disk = bdev->bd_disk; > if (!kobject_get_unless_zero(&disk_to_dev(disk)->kobj)) > goto bdput; > - if ((disk->flags & (GENHD_FL_UP | GENHD_FL_HIDDEN)) != GENHD_FL_UP) > + if (disk->flags & GENHD_FL_HIDDEN) But del_gendisk() can be called just between bdget() and checking GENHD_FL_UP. And not see difference by moving remove_inode_hash() with disk open_mutex held. Thanks, Ming
On Thu, Jul 22, 2021 at 04:19:42PM +0800, Ming Lei wrote: > > goto bdput; > > - if ((disk->flags & (GENHD_FL_UP | GENHD_FL_HIDDEN)) != GENHD_FL_UP) > > + if (disk->flags & GENHD_FL_HIDDEN) > > But del_gendisk() can be called just between bdget() and checking GENHD_FL_UP. > > And not see difference by moving remove_inode_hash() with disk open_mutex held. The difference is not about having the open_mutex held, but about doing it earlier. The only check that matters is the GENHD_FL_UP check in blkdev_get_by_dev. The earlier check just reduces the amount of work we're doing for a disk already being delete. With the early unhash there is no need for that check as we won't even find the inode for a disk in del_gendisk. We still need the non-racy check under the lock, but the patch doesn't touch that one. Maybe I need to split this into two patches and improve the commit log.
diff --git a/block/genhd.c b/block/genhd.c index 298ee78c1bda..716f5ca479ad 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -585,6 +585,7 @@ void del_gendisk(struct gendisk *disk) disk_del_events(disk); mutex_lock(&disk->open_mutex); + remove_inode_hash(disk->part0->bd_inode); disk->flags &= ~GENHD_FL_UP; blk_drop_partitions(disk); mutex_unlock(&disk->open_mutex); @@ -592,12 +593,6 @@ void del_gendisk(struct gendisk *disk) fsync_bdev(disk->part0); __invalidate_device(disk->part0, true); - /* - * Unhash the bdev inode for this device so that it can't be looked - * up any more even if openers still hold references to it. - */ - remove_inode_hash(disk->part0->bd_inode); - set_capacity(disk, 0); if (!(disk->flags & GENHD_FL_HIDDEN)) { diff --git a/fs/block_dev.c b/fs/block_dev.c index 9ef4f1fc2cb0..932f4034ad66 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1340,7 +1340,7 @@ struct block_device *blkdev_get_no_open(dev_t dev) disk = bdev->bd_disk; if (!kobject_get_unless_zero(&disk_to_dev(disk)->kobj)) goto bdput; - if ((disk->flags & (GENHD_FL_UP | GENHD_FL_HIDDEN)) != GENHD_FL_UP) + if (disk->flags & GENHD_FL_HIDDEN) goto put_disk; if (!try_module_get(bdev->bd_disk->fops->owner)) goto put_disk;
Unhash the whole device inode early in del_gendisk. This allows to remove the first GENHD_FL_UP check in the open path as we simply won't find a just removed inode. The second non-racy check after taking open_mutex is still kept. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/genhd.c | 7 +------ fs/block_dev.c | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-)