* fix a race between del_gendisk and BLKRRPART v2
@ 2021-05-14 13:18 Christoph Hellwig
2021-05-14 13:18 ` [PATCH 1/2] block: prevent block device lookups at the beginning of del_gendisk Christoph Hellwig
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Christoph Hellwig @ 2021-05-14 13:18 UTC (permalink / raw)
To: axboe; +Cc: Gulam Mohamed, Ming Lei, linux-block
Hi all,
this is based of a patch from Gulam and suggestions from Ming and fixes a
race between del_gendisk and BLKRRPART while also removing a global lock.
Changes since v1:
- fix the GENHD_FL_UP check in __blkdev_get
- don't change where remove_inode_hash is called for now
- improve the commit message
Diffstat:
block/genhd.c | 11 +----------
fs/block_dev.c | 18 ++++++++----------
include/linux/genhd.h | 2 --
3 files changed, 9 insertions(+), 22 deletions(-)
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] block: prevent block device lookups at the beginning of del_gendisk 2021-05-14 13:18 fix a race between del_gendisk and BLKRRPART v2 Christoph Hellwig @ 2021-05-14 13:18 ` Christoph Hellwig 2021-05-15 6:34 ` Ming Lei 2021-05-14 13:18 ` [PATCH 2/2] block: fix a race between del_gendisk and BLKRRPART Christoph Hellwig 2021-05-20 8:14 ` fix a race between del_gendisk and BLKRRPART v2 Christoph Hellwig 2 siblings, 1 reply; 9+ messages in thread From: Christoph Hellwig @ 2021-05-14 13:18 UTC (permalink / raw) To: axboe; +Cc: Gulam Mohamed, Ming Lei, linux-block As an artifact of how gendisk lookup used to work in earlier kernels, GENHD_FL_UP is only cleared very late in del_gendisk, and a global lock is used to prevent opens from succeeding while del_gendisk is tearing down the gendisk. Switch to clearing the flag early and under bd_mutex so that callers can use bd_mutex to stabilize the flag, which removes the need for the global mutex. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/genhd.c | 11 +---------- fs/block_dev.c | 15 +++++---------- include/linux/genhd.h | 2 -- 3 files changed, 6 insertions(+), 22 deletions(-) diff --git a/block/genhd.c b/block/genhd.c index 39ca97b0edc6..9f8cb7beaad1 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -29,8 +29,6 @@ static struct kobject *block_depr; -DECLARE_RWSEM(bdev_lookup_sem); - /* for extended dynamic devt allocation, currently only one major is used */ #define NR_EXT_DEVT (1 << MINORBITS) static DEFINE_IDA(ext_devt_ida); @@ -609,13 +607,8 @@ void del_gendisk(struct gendisk *disk) blk_integrity_del(disk); disk_del_events(disk); - /* - * Block lookups of the disk until all bdevs are unhashed and the - * disk is marked as dead (GENHD_FL_UP cleared). - */ - down_write(&bdev_lookup_sem); - mutex_lock(&disk->part0->bd_mutex); + disk->flags &= ~GENHD_FL_UP; blk_drop_partitions(disk); mutex_unlock(&disk->part0->bd_mutex); @@ -629,8 +622,6 @@ void del_gendisk(struct gendisk *disk) remove_inode_hash(disk->part0->bd_inode); set_capacity(disk, 0); - disk->flags &= ~GENHD_FL_UP; - up_write(&bdev_lookup_sem); if (!(disk->flags & GENHD_FL_HIDDEN)) { sysfs_remove_link(&disk_to_dev(disk)->kobj, "bdi"); diff --git a/fs/block_dev.c b/fs/block_dev.c index eb265d72fce8..580bae995b87 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1298,6 +1298,9 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode) struct gendisk *disk = bdev->bd_disk; int ret = 0; + if (!(disk->flags & GENHD_FL_UP)) + return -ENXIO; + if (!bdev->bd_openers) { if (!bdev_is_partition(bdev)) { ret = 0; @@ -1332,8 +1335,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode) whole->bd_part_count++; mutex_unlock(&whole->bd_mutex); - if (!(disk->flags & GENHD_FL_UP) || - !bdev_nr_sectors(bdev)) { + if (!bdev_nr_sectors(bdev)) { __blkdev_put(whole, mode, 1); bdput(whole); return -ENXIO; @@ -1364,16 +1366,12 @@ struct block_device *blkdev_get_no_open(dev_t dev) struct block_device *bdev; struct gendisk *disk; - down_read(&bdev_lookup_sem); bdev = bdget(dev); if (!bdev) { - up_read(&bdev_lookup_sem); blk_request_module(dev); - down_read(&bdev_lookup_sem); - bdev = bdget(dev); if (!bdev) - goto unlock; + return NULL; } disk = bdev->bd_disk; @@ -1383,14 +1381,11 @@ struct block_device *blkdev_get_no_open(dev_t dev) goto put_disk; if (!try_module_get(bdev->bd_disk->fops->owner)) goto put_disk; - up_read(&bdev_lookup_sem); return bdev; put_disk: put_disk(disk); bdput: bdput(bdev); -unlock: - up_read(&bdev_lookup_sem); return NULL; } diff --git a/include/linux/genhd.h b/include/linux/genhd.h index 7e9660ea967d..6fc26f7bdf71 100644 --- a/include/linux/genhd.h +++ b/include/linux/genhd.h @@ -306,8 +306,6 @@ static inline void bd_unlink_disk_holder(struct block_device *bdev, } #endif /* CONFIG_SYSFS */ -extern struct rw_semaphore bdev_lookup_sem; - dev_t blk_lookup_devt(const char *name, int partno); void blk_request_module(dev_t devt); #ifdef CONFIG_BLOCK -- 2.30.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] block: prevent block device lookups at the beginning of del_gendisk 2021-05-14 13:18 ` [PATCH 1/2] block: prevent block device lookups at the beginning of del_gendisk Christoph Hellwig @ 2021-05-15 6:34 ` Ming Lei 2021-05-20 17:37 ` Gulam Mohamed 0 siblings, 1 reply; 9+ messages in thread From: Ming Lei @ 2021-05-15 6:34 UTC (permalink / raw) To: Christoph Hellwig; +Cc: axboe, Gulam Mohamed, linux-block On Fri, May 14, 2021 at 03:18:41PM +0200, Christoph Hellwig wrote: > As an artifact of how gendisk lookup used to work in earlier kernels, > GENHD_FL_UP is only cleared very late in del_gendisk, and a global lock > is used to prevent opens from succeeding while del_gendisk is tearing > down the gendisk. Switch to clearing the flag early and under bd_mutex > so that callers can use bd_mutex to stabilize the flag, which removes > the need for the global mutex. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Ming Lei <ming.lei@redhat.com> -- Ming ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH 1/2] block: prevent block device lookups at the beginning of del_gendisk 2021-05-15 6:34 ` Ming Lei @ 2021-05-20 17:37 ` Gulam Mohamed 0 siblings, 0 replies; 9+ messages in thread From: Gulam Mohamed @ 2021-05-20 17:37 UTC (permalink / raw) To: Ming Lei, Christoph Hellwig; +Cc: axboe, linux-block -----Original Message----- From: Ming Lei <ming.lei@redhat.com> Sent: Saturday, May 15, 2021 12:05 PM To: Christoph Hellwig <hch@lst.de> Cc: axboe@kernel.dk; Gulam Mohamed <gulam.mohamed@oracle.com>; linux-block@vger.kernel.org Subject: Re: [PATCH 1/2] block: prevent block device lookups at the beginning of del_gendisk On Fri, May 14, 2021 at 03:18:41PM +0200, Christoph Hellwig wrote: > As an artifact of how gendisk lookup used to work in earlier kernels, > GENHD_FL_UP is only cleared very late in del_gendisk, and a global > lock is used to prevent opens from succeeding while del_gendisk is > tearing down the gendisk. Switch to clearing the flag early and under > bd_mutex so that callers can use bd_mutex to stabilize the flag, which > removes the need for the global mutex. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Ming Lei <ming.lei@redhat.com> I have tested these patches on latest kernel in our environment and didn't find any issue. The fix is working fine. Tested-by: Gulam Mohamed <gulam.mohamed@oracle.com> -- Ming ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] block: fix a race between del_gendisk and BLKRRPART 2021-05-14 13:18 fix a race between del_gendisk and BLKRRPART v2 Christoph Hellwig 2021-05-14 13:18 ` [PATCH 1/2] block: prevent block device lookups at the beginning of del_gendisk Christoph Hellwig @ 2021-05-14 13:18 ` Christoph Hellwig 2021-05-15 6:35 ` Ming Lei 2021-05-20 8:14 ` fix a race between del_gendisk and BLKRRPART v2 Christoph Hellwig 2 siblings, 1 reply; 9+ messages in thread From: Christoph Hellwig @ 2021-05-14 13:18 UTC (permalink / raw) To: axboe; +Cc: Gulam Mohamed, Ming Lei, linux-block From: Gulam Mohamed <gulam.mohamed@oracle.com> When BLKRRPART is called concurrently with del_gendisk, the partitions rescan can create a stale partition that will never be be cleaned up. Fix this by checking the the disk is up before rescanning partitions while under bd_mutex. Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com> [hch: split from a larger patch] Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/block_dev.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/block_dev.c b/fs/block_dev.c index 580bae995b87..4494411fa4d3 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1244,6 +1244,9 @@ int bdev_disk_changed(struct block_device *bdev, bool invalidate) lockdep_assert_held(&bdev->bd_mutex); + if (!(disk->flags & GENHD_FL_UP)) + return -ENXIO; + rescan: if (bdev->bd_part_count) return -EBUSY; -- 2.30.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] block: fix a race between del_gendisk and BLKRRPART 2021-05-14 13:18 ` [PATCH 2/2] block: fix a race between del_gendisk and BLKRRPART Christoph Hellwig @ 2021-05-15 6:35 ` Ming Lei 2021-05-20 17:37 ` Gulam Mohamed 0 siblings, 1 reply; 9+ messages in thread From: Ming Lei @ 2021-05-15 6:35 UTC (permalink / raw) To: Christoph Hellwig; +Cc: axboe, Gulam Mohamed, linux-block On Fri, May 14, 2021 at 03:18:42PM +0200, Christoph Hellwig wrote: > From: Gulam Mohamed <gulam.mohamed@oracle.com> > > When BLKRRPART is called concurrently with del_gendisk, the partitions > rescan can create a stale partition that will never be be cleaned up. > > Fix this by checking the the disk is up before rescanning partitions > while under bd_mutex. > > Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com> > [hch: split from a larger patch] > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/block_dev.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/block_dev.c b/fs/block_dev.c > index 580bae995b87..4494411fa4d3 100644 > --- a/fs/block_dev.c > +++ b/fs/block_dev.c > @@ -1244,6 +1244,9 @@ int bdev_disk_changed(struct block_device *bdev, bool invalidate) > > lockdep_assert_held(&bdev->bd_mutex); > > + if (!(disk->flags & GENHD_FL_UP)) > + return -ENXIO; > + > rescan: > if (bdev->bd_part_count) > return -EBUSY; Reviewed-by: Ming Lei <ming.lei@redhat.com> -- Ming ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH 2/2] block: fix a race between del_gendisk and BLKRRPART 2021-05-15 6:35 ` Ming Lei @ 2021-05-20 17:37 ` Gulam Mohamed 0 siblings, 0 replies; 9+ messages in thread From: Gulam Mohamed @ 2021-05-20 17:37 UTC (permalink / raw) To: Ming Lei, Christoph Hellwig; +Cc: axboe, linux-block -----Original Message----- From: Ming Lei <ming.lei@redhat.com> Sent: Saturday, May 15, 2021 12:05 PM To: Christoph Hellwig <hch@lst.de> Cc: axboe@kernel.dk; Gulam Mohamed <gulam.mohamed@oracle.com>; linux-block@vger.kernel.org Subject: Re: [PATCH 2/2] block: fix a race between del_gendisk and BLKRRPART On Fri, May 14, 2021 at 03:18:42PM +0200, Christoph Hellwig wrote: > From: Gulam Mohamed <gulam.mohamed@oracle.com> > > When BLKRRPART is called concurrently with del_gendisk, the partitions > rescan can create a stale partition that will never be be cleaned up. > > Fix this by checking the the disk is up before rescanning partitions > while under bd_mutex. > > Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com> > [hch: split from a larger patch] > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/block_dev.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/block_dev.c b/fs/block_dev.c index > 580bae995b87..4494411fa4d3 100644 > --- a/fs/block_dev.c > +++ b/fs/block_dev.c > @@ -1244,6 +1244,9 @@ int bdev_disk_changed(struct block_device *bdev, > bool invalidate) > > lockdep_assert_held(&bdev->bd_mutex); > > + if (!(disk->flags & GENHD_FL_UP)) > + return -ENXIO; > + > rescan: > if (bdev->bd_part_count) > return -EBUSY; Reviewed-by: Ming Lei <ming.lei@redhat.com> Tested-by: Gulam Mohamed <gulam.mohamed@oracle.com> -- Ming ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: fix a race between del_gendisk and BLKRRPART v2 2021-05-14 13:18 fix a race between del_gendisk and BLKRRPART v2 Christoph Hellwig 2021-05-14 13:18 ` [PATCH 1/2] block: prevent block device lookups at the beginning of del_gendisk Christoph Hellwig 2021-05-14 13:18 ` [PATCH 2/2] block: fix a race between del_gendisk and BLKRRPART Christoph Hellwig @ 2021-05-20 8:14 ` Christoph Hellwig 2021-05-20 13:59 ` Jens Axboe 2 siblings, 1 reply; 9+ messages in thread From: Christoph Hellwig @ 2021-05-20 8:14 UTC (permalink / raw) To: axboe; +Cc: Gulam Mohamed, Ming Lei, linux-block Jens, can you take a look at these? On Fri, May 14, 2021 at 03:18:40PM +0200, Christoph Hellwig wrote: > Hi all, > > this is based of a patch from Gulam and suggestions from Ming and fixes a > race between del_gendisk and BLKRRPART while also removing a global lock. > > Changes since v1: > - fix the GENHD_FL_UP check in __blkdev_get > - don't change where remove_inode_hash is called for now > - improve the commit message > > Diffstat: > block/genhd.c | 11 +---------- > fs/block_dev.c | 18 ++++++++---------- > include/linux/genhd.h | 2 -- > 3 files changed, 9 insertions(+), 22 deletions(-) ---end quoted text--- ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: fix a race between del_gendisk and BLKRRPART v2 2021-05-20 8:14 ` fix a race between del_gendisk and BLKRRPART v2 Christoph Hellwig @ 2021-05-20 13:59 ` Jens Axboe 0 siblings, 0 replies; 9+ messages in thread From: Jens Axboe @ 2021-05-20 13:59 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Gulam Mohamed, Ming Lei, linux-block On 5/20/21 2:14 AM, Christoph Hellwig wrote: > Jens, > > can you take a look at these? Looks good to me, applied. -- Jens Axboe ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-05-20 17:37 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-14 13:18 fix a race between del_gendisk and BLKRRPART v2 Christoph Hellwig 2021-05-14 13:18 ` [PATCH 1/2] block: prevent block device lookups at the beginning of del_gendisk Christoph Hellwig 2021-05-15 6:34 ` Ming Lei 2021-05-20 17:37 ` Gulam Mohamed 2021-05-14 13:18 ` [PATCH 2/2] block: fix a race between del_gendisk and BLKRRPART Christoph Hellwig 2021-05-15 6:35 ` Ming Lei 2021-05-20 17:37 ` Gulam Mohamed 2021-05-20 8:14 ` fix a race between del_gendisk and BLKRRPART v2 Christoph Hellwig 2021-05-20 13:59 ` Jens Axboe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.