From 799ae31c58ae766a4ce9b198658b286ecaf2c575 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 23 Aug 2021 09:33:52 +0200 Subject: mtd_blkdevs: don't hold del_mtd_blktrans_dev in blktrans_{open, release} There is nothing that this protects against except for slightly reducing the window when new opens can appear just before calling del_gendisk. Reported-by: Guenter Roeck Signed-off-by: Christoph Hellwig Tested-by: Guenter Roeck Signed-off-by: Miquel Raynal Link: https://lore.kernel.org/linux-mtd/20210823073359.705281-2-hch@lst.de --- drivers/mtd/mtd_blkdevs.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'drivers/mtd/mtd_blkdevs.c') diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c index 6ce4bc57f919..94d42ba01b2f 100644 --- a/drivers/mtd/mtd_blkdevs.c +++ b/drivers/mtd/mtd_blkdevs.c @@ -207,7 +207,6 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode) if (!dev) return -ERESTARTSYS; /* FIXME: busy loop! -arnd*/ - mutex_lock(&mtd_table_mutex); mutex_lock(&dev->lock); if (dev->open) @@ -233,7 +232,6 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode) unlock: dev->open++; mutex_unlock(&dev->lock); - mutex_unlock(&mtd_table_mutex); blktrans_dev_put(dev); return ret; @@ -244,7 +242,6 @@ error_put: module_put(dev->tr->owner); kref_put(&dev->ref, blktrans_dev_release); mutex_unlock(&dev->lock); - mutex_unlock(&mtd_table_mutex); blktrans_dev_put(dev); return ret; } @@ -256,7 +253,6 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode) if (!dev) return; - mutex_lock(&mtd_table_mutex); mutex_lock(&dev->lock); if (--dev->open) @@ -272,7 +268,6 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode) } unlock: mutex_unlock(&dev->lock); - mutex_unlock(&mtd_table_mutex); blktrans_dev_put(dev); } -- cgit v1.2.3 From f214eebf8de4d2e412a35ff140687d88f056143a Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 23 Aug 2021 09:33:53 +0200 Subject: mtd_blkdevs: use lockdep_assert_held Use lockdep_assert_held to ensure mtd_table_mutex is held instead of mutex_trylock games. Signed-off-by: Christoph Hellwig Signed-off-by: Miquel Raynal Link: https://lore.kernel.org/linux-mtd/20210823073359.705281-3-hch@lst.de --- drivers/mtd/mtd_blkdevs.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) (limited to 'drivers/mtd/mtd_blkdevs.c') diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c index 94d42ba01b2f..5981fd026c87 100644 --- a/drivers/mtd/mtd_blkdevs.c +++ b/drivers/mtd/mtd_blkdevs.c @@ -310,10 +310,7 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new) struct gendisk *gd; int ret; - if (mutex_trylock(&mtd_table_mutex)) { - mutex_unlock(&mtd_table_mutex); - BUG(); - } + lockdep_assert_held(&mtd_table_mutex); mutex_lock(&blktrans_ref_mutex); list_for_each_entry(d, &tr->devs, list) { @@ -443,10 +440,7 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old) { unsigned long flags; - if (mutex_trylock(&mtd_table_mutex)) { - mutex_unlock(&mtd_table_mutex); - BUG(); - } + lockdep_assert_held(&mtd_table_mutex); if (old->disk_attributes) sysfs_remove_group(&disk_to_dev(old->disk)->kobj, -- cgit v1.2.3 From 89843828399ec825f8ec3e614634a428a951a2b3 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 23 Aug 2021 09:33:56 +0200 Subject: mtd_blkdevs: simplify blktrans_dev_get ->private_data is set before the disk is added and never cleared, so don't bother trying to handle a NULL pointer there. Signed-off-by: Christoph Hellwig Signed-off-by: Miquel Raynal Link: https://lore.kernel.org/linux-mtd/20210823073359.705281-6-hch@lst.de --- drivers/mtd/mtd_blkdevs.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) (limited to 'drivers/mtd/mtd_blkdevs.c') diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c index 5981fd026c87..eb15a84cb650 100644 --- a/drivers/mtd/mtd_blkdevs.c +++ b/drivers/mtd/mtd_blkdevs.c @@ -39,16 +39,12 @@ static void blktrans_dev_release(struct kref *kref) static struct mtd_blktrans_dev *blktrans_dev_get(struct gendisk *disk) { - struct mtd_blktrans_dev *dev; + struct mtd_blktrans_dev *dev = disk->private_data; mutex_lock(&blktrans_ref_mutex); - dev = disk->private_data; - - if (!dev) - goto unlock; kref_get(&dev->ref); -unlock: mutex_unlock(&blktrans_ref_mutex); + return dev; } @@ -204,9 +200,6 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode) struct mtd_blktrans_dev *dev = blktrans_dev_get(bdev->bd_disk); int ret = 0; - if (!dev) - return -ERESTARTSYS; /* FIXME: busy loop! -arnd*/ - mutex_lock(&dev->lock); if (dev->open) @@ -250,9 +243,6 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode) { struct mtd_blktrans_dev *dev = blktrans_dev_get(disk); - if (!dev) - return; - mutex_lock(&dev->lock); if (--dev->open) @@ -276,9 +266,6 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) struct mtd_blktrans_dev *dev = blktrans_dev_get(bdev->bd_disk); int ret = -ENXIO; - if (!dev) - return ret; - mutex_lock(&dev->lock); if (!dev->mtd) -- cgit v1.2.3 From 560a3915e3df09125d7b20088a177b2872d2d680 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 23 Aug 2021 09:33:57 +0200 Subject: mtd_blkdevs: remove blktrans_ref_mutex blktrans_ref_mutex is not actually needed. The kref is serialized internally, and devnum assignment in add_mtd_blktrans_dev happens before the disk is added and thus any of the block_device_operations methods otherwise using it are called. It is also already serialized by the global mtd_table_mutex. Signed-off-by: Christoph Hellwig Signed-off-by: Miquel Raynal Link: https://lore.kernel.org/linux-mtd/20210823073359.705281-7-hch@lst.de --- drivers/mtd/mtd_blkdevs.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) (limited to 'drivers/mtd/mtd_blkdevs.c') diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c index eb15a84cb650..84bcad1b7408 100644 --- a/drivers/mtd/mtd_blkdevs.c +++ b/drivers/mtd/mtd_blkdevs.c @@ -23,7 +23,6 @@ #include "mtdcore.h" static LIST_HEAD(blktrans_majors); -static DEFINE_MUTEX(blktrans_ref_mutex); static void blktrans_dev_release(struct kref *kref) { @@ -41,18 +40,13 @@ static struct mtd_blktrans_dev *blktrans_dev_get(struct gendisk *disk) { struct mtd_blktrans_dev *dev = disk->private_data; - mutex_lock(&blktrans_ref_mutex); kref_get(&dev->ref); - mutex_unlock(&blktrans_ref_mutex); - return dev; } static void blktrans_dev_put(struct mtd_blktrans_dev *dev) { - mutex_lock(&blktrans_ref_mutex); kref_put(&dev->ref, blktrans_dev_release); - mutex_unlock(&blktrans_ref_mutex); } @@ -299,7 +293,6 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new) lockdep_assert_held(&mtd_table_mutex); - mutex_lock(&blktrans_ref_mutex); list_for_each_entry(d, &tr->devs, list) { if (new->devnum == -1) { /* Use first free number */ @@ -311,7 +304,6 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new) } } else if (d->devnum == new->devnum) { /* Required number taken */ - mutex_unlock(&blktrans_ref_mutex); return -EBUSY; } else if (d->devnum > new->devnum) { /* Required number was free */ @@ -329,14 +321,11 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new) * minor numbers and that the disk naming code below can cope * with this number. */ if (new->devnum > (MINORMASK >> tr->part_bits) || - (tr->part_bits && new->devnum >= 27 * 26)) { - mutex_unlock(&blktrans_ref_mutex); + (tr->part_bits && new->devnum >= 27 * 26)) return ret; - } list_add_tail(&new->list, &tr->devs); added: - mutex_unlock(&blktrans_ref_mutex); mutex_init(&new->lock); kref_init(&new->ref); -- cgit v1.2.3 From 37b143d12b5f099bb4375162303ff1df1692cc5e Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 23 Aug 2021 09:33:58 +0200 Subject: mtd_blkdevs: simplify blktrans_getgeo No need to grab a mtd_blktrans_dev given that ->open already holds one and ->getgeo can only be called on an open disk. Signed-off-by: Christoph Hellwig Signed-off-by: Miquel Raynal Link: https://lore.kernel.org/linux-mtd/20210823073359.705281-8-hch@lst.de --- drivers/mtd/mtd_blkdevs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/mtd/mtd_blkdevs.c') diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c index 84bcad1b7408..e8eb4d801b15 100644 --- a/drivers/mtd/mtd_blkdevs.c +++ b/drivers/mtd/mtd_blkdevs.c @@ -257,7 +257,7 @@ unlock: static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) { - struct mtd_blktrans_dev *dev = blktrans_dev_get(bdev->bd_disk); + struct mtd_blktrans_dev *dev = bdev->bd_disk->private_data; int ret = -ENXIO; mutex_lock(&dev->lock); @@ -268,7 +268,6 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENOTTY; unlock: mutex_unlock(&dev->lock); - blktrans_dev_put(dev); return ret; } -- cgit v1.2.3 From ee28b42006c37aaeb68c83300fe5608db662082f Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 23 Aug 2021 09:33:59 +0200 Subject: mtd_blkdevs: simplify the refcounting in blktrans_{open, release} Always grab a reference to the mtd_blktrans_dev in ->open instead of just on the first open, and do away with the additional temporary references in ->open and ->release. Signed-off-by: Christoph Hellwig Signed-off-by: Miquel Raynal Link: https://lore.kernel.org/linux-mtd/20210823073359.705281-9-hch@lst.de --- drivers/mtd/mtd_blkdevs.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) (limited to 'drivers/mtd/mtd_blkdevs.c') diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c index e8eb4d801b15..12c1803284c7 100644 --- a/drivers/mtd/mtd_blkdevs.c +++ b/drivers/mtd/mtd_blkdevs.c @@ -36,14 +36,6 @@ static void blktrans_dev_release(struct kref *kref) kfree(dev); } -static struct mtd_blktrans_dev *blktrans_dev_get(struct gendisk *disk) -{ - struct mtd_blktrans_dev *dev = disk->private_data; - - kref_get(&dev->ref); - return dev; -} - static void blktrans_dev_put(struct mtd_blktrans_dev *dev) { kref_put(&dev->ref, blktrans_dev_release); @@ -191,15 +183,16 @@ static blk_status_t mtd_queue_rq(struct blk_mq_hw_ctx *hctx, static int blktrans_open(struct block_device *bdev, fmode_t mode) { - struct mtd_blktrans_dev *dev = blktrans_dev_get(bdev->bd_disk); + struct mtd_blktrans_dev *dev = bdev->bd_disk->private_data; int ret = 0; + kref_get(&dev->ref); + mutex_lock(&dev->lock); if (dev->open) goto unlock; - kref_get(&dev->ref); __module_get(dev->tr->owner); if (!dev->mtd) @@ -219,7 +212,6 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode) unlock: dev->open++; mutex_unlock(&dev->lock); - blktrans_dev_put(dev); return ret; error_release: @@ -227,7 +219,6 @@ error_release: dev->tr->release(dev); error_put: module_put(dev->tr->owner); - kref_put(&dev->ref, blktrans_dev_release); mutex_unlock(&dev->lock); blktrans_dev_put(dev); return ret; @@ -235,14 +226,13 @@ error_put: static void blktrans_release(struct gendisk *disk, fmode_t mode) { - struct mtd_blktrans_dev *dev = blktrans_dev_get(disk); + struct mtd_blktrans_dev *dev = disk->private_data; mutex_lock(&dev->lock); if (--dev->open) goto unlock; - kref_put(&dev->ref, blktrans_dev_release); module_put(dev->tr->owner); if (dev->mtd) { -- cgit v1.2.3