* [PATCH 01/15] md: remove the code to flush an old instance in md_open
2021-03-30 16:17 move bd_mutex to the gendisk Christoph Hellwig
@ 2021-03-30 16:17 ` Christoph Hellwig
2021-03-31 3:29 ` heming.zhao
2021-03-30 16:17 ` [PATCH 02/15] md: factor out a mddev_find_locked helper from mddev_find Christoph Hellwig
` (13 subsequent siblings)
14 siblings, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-30 16:17 UTC (permalink / raw)
To: Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
Due to the flush_workqueue() call in md_alloc no previous instance of
mddev can still be around at this point.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/md/md.c | 35 +++++++----------------------------
1 file changed, 7 insertions(+), 28 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 368cad6cd53a6e..cd2d825dd4f881 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -7807,43 +7807,22 @@ static int md_open(struct block_device *bdev, fmode_t mode)
* Succeed if we can lock the mddev, which confirms that
* it isn't being stopped right now.
*/
- struct mddev *mddev = mddev_find(bdev->bd_dev);
+ struct mddev *mddev = bdev->bd_disk->private_data;
int err;
- if (!mddev)
- return -ENODEV;
-
- if (mddev->gendisk != bdev->bd_disk) {
- /* we are racing with mddev_put which is discarding this
- * bd_disk.
- */
- mddev_put(mddev);
- /* Wait until bdev->bd_disk is definitely gone */
- if (work_pending(&mddev->del_work))
- flush_workqueue(md_misc_wq);
- /* Then retry the open from the top */
- return -ERESTARTSYS;
- }
- BUG_ON(mddev != bdev->bd_disk->private_data);
-
- if ((err = mutex_lock_interruptible(&mddev->open_mutex)))
- goto out;
-
+ err = mutex_lock_interruptible(&mddev->open_mutex);
+ if (err)
+ return err;
if (test_bit(MD_CLOSING, &mddev->flags)) {
mutex_unlock(&mddev->open_mutex);
- err = -ENODEV;
- goto out;
+ return -ENODEV;
}
-
- err = 0;
+ mddev_get(mddev);
atomic_inc(&mddev->openers);
mutex_unlock(&mddev->open_mutex);
bdev_check_media_change(bdev);
- out:
- if (err)
- mddev_put(mddev);
- return err;
+ return 0;
}
static void md_release(struct gendisk *disk, fmode_t mode)
--
2.30.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 01/15] md: remove the code to flush an old instance in md_open
2021-03-30 16:17 ` [PATCH 01/15] md: remove the code to flush an old instance in md_open Christoph Hellwig
@ 2021-03-31 3:29 ` heming.zhao
2021-03-31 6:53 ` Christoph Hellwig
0 siblings, 1 reply; 21+ messages in thread
From: heming.zhao @ 2021-03-31 3:29 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
On 3/31/21 12:17 AM, Christoph Hellwig wrote:
> Due to the flush_workqueue() call in md_alloc no previous instance of
> mddev can still be around at this point.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/md/md.c | 35 +++++++----------------------------
> 1 file changed, 7 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 368cad6cd53a6e..cd2d825dd4f881 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -7807,43 +7807,22 @@ static int md_open(struct block_device *bdev, fmode_t mode)
> * Succeed if we can lock the mddev, which confirms that
> * it isn't being stopped right now.
> */
> - struct mddev *mddev = mddev_find(bdev->bd_dev);
> + struct mddev *mddev = bdev->bd_disk->private_data;
> int err;
>
> - if (!mddev)
> - return -ENODEV;
> -
> - if (mddev->gendisk != bdev->bd_disk) {
> - /* we are racing with mddev_put which is discarding this
> - * bd_disk.
> - */
> - mddev_put(mddev);
> - /* Wait until bdev->bd_disk is definitely gone */
> - if (work_pending(&mddev->del_work))
> - flush_workqueue(md_misc_wq);
> - /* Then retry the open from the top */
> - return -ERESTARTSYS;
> - }
> - BUG_ON(mddev != bdev->bd_disk->private_data);
> -
> - if ((err = mutex_lock_interruptible(&mddev->open_mutex)))
> - goto out;
> -
> + err = mutex_lock_interruptible(&mddev->open_mutex);
> + if (err)
> + return err;
> if (test_bit(MD_CLOSING, &mddev->flags)) {
> mutex_unlock(&mddev->open_mutex);
> - err = -ENODEV;
> - goto out;
> + return -ENODEV;
> }
> -
> - err = 0;
> + mddev_get(mddev);
> atomic_inc(&mddev->openers);
> mutex_unlock(&mddev->open_mutex);
>
> bdev_check_media_change(bdev);
> - out:
> - if (err)
> - mddev_put(mddev);
> - return err;
> + return 0;
> }
>
> static void md_release(struct gendisk *disk, fmode_t mode)
>
Hello Christoph,
After applying your patch, the md_open() will be:
```
static int md_open(struct block_device *bdev, fmode_t mode)
{
/* ... */
struct mddev *mddev = bdev->bd_disk->private_data;
int err;
err = mutex_lock_interruptible(&mddev->open_mutex);
if (err)
return err;
if (test_bit(MD_CLOSING, &mddev->flags)) {
mutex_unlock(&mddev->open_mutex);
return -ENODEV;
}
mddev_get(mddev);
atomic_inc(&mddev->openers);
mutex_unlock(&mddev->open_mutex);
bdev_check_media_change(bdev);
return 0;
}
```
in clean path, MD_CLOSING only lives a very short time, then be cleaned in md_clean:
```
ioctl
+ test_and_set_bit(MD_CLOSING, &mddev->flags)
+ do_md_stop //case STOP_ARRAY
md_clean
mddev->flags = 0;
```
when userspace "mdadm -Ss" finish (the ioctl STOP_ARRAY returns),
mddev->flags will be zero. and you can see my patch email (date: 2021-3-30).
At this time, userspace will execute "mdadm --monitor" to scan the
closing md device. the md_open will trigger very soon. at this time,
bdev->bd_disk->private_data is only a skeleton, your shouldn't trust & use it.
So mddev with MD_CLOSING protection, the md_open is not safety.
PS:
Neil Brown legacy commit d3374825ce57ba2214d37502397 also describes this condition.
Thanks,
heming
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 01/15] md: remove the code to flush an old instance in md_open
2021-03-31 3:29 ` heming.zhao
@ 2021-03-31 6:53 ` Christoph Hellwig
0 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-31 6:53 UTC (permalink / raw)
To: heming.zhao
Cc: Christoph Hellwig, Jens Axboe, Song Liu, Konrad Rzeszutek Wilk,
Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
On Wed, Mar 31, 2021 at 11:29:39AM +0800, heming.zhao@suse.com wrote:
> when userspace "mdadm -Ss" finish (the ioctl STOP_ARRAY returns),
> mddev->flags will be zero. and you can see my patch email (date: 2021-3-30).
> At this time, userspace will execute "mdadm --monitor" to scan the
> closing md device. the md_open will trigger very soon. at this time,
> bdev->bd_disk->private_data is only a skeleton, your shouldn't trust & use it.
Ermm, the block layer rules require the device to be fully set up
when add_disk is called. So if that is not the case (and I'd like
to see hints how) we need to fix this properly instead of using a hack
in ->open.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 02/15] md: factor out a mddev_find_locked helper from mddev_find
2021-03-30 16:17 move bd_mutex to the gendisk Christoph Hellwig
2021-03-30 16:17 ` [PATCH 01/15] md: remove the code to flush an old instance in md_open Christoph Hellwig
@ 2021-03-30 16:17 ` Christoph Hellwig
2021-03-30 16:17 ` [PATCH 03/15] md: factor out a mddev_alloc_unit " Christoph Hellwig
` (12 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-30 16:17 UTC (permalink / raw)
To: Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
Factor out a self-contained helper to just lookup a mddev by the dev_t
"unit".
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/md/md.c | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index cd2d825dd4f881..9556724fdb0848 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -734,6 +734,17 @@ void mddev_init(struct mddev *mddev)
}
EXPORT_SYMBOL_GPL(mddev_init);
+static struct mddev *mddev_find_locked(dev_t unit)
+{
+ struct mddev *mddev;
+
+ list_for_each_entry(mddev, &all_mddevs, all_mddevs)
+ if (mddev->unit == unit)
+ return mddev;
+
+ return NULL;
+}
+
static struct mddev *mddev_find(dev_t unit)
{
struct mddev *mddev, *new = NULL;
@@ -745,13 +756,13 @@ static struct mddev *mddev_find(dev_t unit)
spin_lock(&all_mddevs_lock);
if (unit) {
- list_for_each_entry(mddev, &all_mddevs, all_mddevs)
- if (mddev->unit == unit) {
- mddev_get(mddev);
- spin_unlock(&all_mddevs_lock);
- kfree(new);
- return mddev;
- }
+ mddev = mddev_find_locked(unit);
+ if (mddev) {
+ mddev_get(mddev);
+ spin_unlock(&all_mddevs_lock);
+ kfree(new);
+ return mddev;
+ }
if (new) {
list_add(&new->all_mddevs, &all_mddevs);
@@ -777,12 +788,7 @@ static struct mddev *mddev_find(dev_t unit)
return NULL;
}
- is_free = 1;
- list_for_each_entry(mddev, &all_mddevs, all_mddevs)
- if (mddev->unit == dev) {
- is_free = 0;
- break;
- }
+ is_free = !mddev_find_locked(dev);
}
new->unit = dev;
new->md_minor = MINOR(dev);
--
2.30.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 03/15] md: factor out a mddev_alloc_unit helper from mddev_find
2021-03-30 16:17 move bd_mutex to the gendisk Christoph Hellwig
2021-03-30 16:17 ` [PATCH 01/15] md: remove the code to flush an old instance in md_open Christoph Hellwig
2021-03-30 16:17 ` [PATCH 02/15] md: factor out a mddev_find_locked helper from mddev_find Christoph Hellwig
@ 2021-03-30 16:17 ` Christoph Hellwig
2021-03-30 16:17 ` [PATCH 04/15] md: split mddev_find Christoph Hellwig
` (11 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-30 16:17 UTC (permalink / raw)
To: Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
Split out a self contained helper to find a free minor for the md
"unit" number.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/md/md.c | 47 +++++++++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 20 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 9556724fdb0848..f329d5b3c931d4 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -745,6 +745,27 @@ static struct mddev *mddev_find_locked(dev_t unit)
return NULL;
}
+/* find an unused unit number */
+static dev_t mddev_alloc_unit(void)
+{
+ static int next_minor = 512;
+ int start = next_minor;
+ bool is_free = 0;
+ dev_t dev = 0;
+
+ while (!is_free) {
+ dev = MKDEV(MD_MAJOR, next_minor);
+ next_minor++;
+ if (next_minor > MINORMASK)
+ next_minor = 0;
+ if (next_minor == start)
+ return 0; /* Oh dear, all in use. */
+ is_free = !mddev_find_locked(dev);
+ }
+
+ return dev;
+}
+
static struct mddev *mddev_find(dev_t unit)
{
struct mddev *mddev, *new = NULL;
@@ -771,27 +792,13 @@ static struct mddev *mddev_find(dev_t unit)
return new;
}
} else if (new) {
- /* find an unused unit number */
- static int next_minor = 512;
- int start = next_minor;
- int is_free = 0;
- int dev = 0;
- while (!is_free) {
- dev = MKDEV(MD_MAJOR, next_minor);
- next_minor++;
- if (next_minor > MINORMASK)
- next_minor = 0;
- if (next_minor == start) {
- /* Oh dear, all in use. */
- spin_unlock(&all_mddevs_lock);
- kfree(new);
- return NULL;
- }
-
- is_free = !mddev_find_locked(dev);
+ new->unit = mddev_alloc_unit();
+ if (!new->unit) {
+ spin_unlock(&all_mddevs_lock);
+ kfree(new);
+ return NULL;
}
- new->unit = dev;
- new->md_minor = MINOR(dev);
+ new->md_minor = MINOR(new->unit);
new->hold_active = UNTIL_STOP;
list_add(&new->all_mddevs, &all_mddevs);
spin_unlock(&all_mddevs_lock);
--
2.30.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 04/15] md: split mddev_find
2021-03-30 16:17 move bd_mutex to the gendisk Christoph Hellwig
` (2 preceding siblings ...)
2021-03-30 16:17 ` [PATCH 03/15] md: factor out a mddev_alloc_unit " Christoph Hellwig
@ 2021-03-30 16:17 ` Christoph Hellwig
2021-03-30 16:17 ` [PATCH 05/15] md: refactor mddev_find_or_alloc Christoph Hellwig
` (10 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-30 16:17 UTC (permalink / raw)
To: Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
Split mddev_find into a simple mddev_find that just finds an existing
mddev by the unit number, and a more complicated mddev_find that deals
with find or allocating a mddev.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/md/md.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index f329d5b3c931d4..8e60bcc9c1d10c 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -767,6 +767,22 @@ static dev_t mddev_alloc_unit(void)
}
static struct mddev *mddev_find(dev_t unit)
+{
+ struct mddev *mddev;
+
+ if (MAJOR(unit) != MD_MAJOR)
+ unit &= ~((1 << MdpMinorShift) - 1);
+
+ spin_lock(&all_mddevs_lock);
+ mddev = mddev_find_locked(unit);
+ if (mddev)
+ mddev_get(mddev);
+ spin_unlock(&all_mddevs_lock);
+
+ return mddev;
+}
+
+static struct mddev *mddev_find_or_alloc(dev_t unit)
{
struct mddev *mddev, *new = NULL;
@@ -5657,7 +5673,7 @@ static int md_alloc(dev_t dev, char *name)
* writing to /sys/module/md_mod/parameters/new_array.
*/
static DEFINE_MUTEX(disks_mutex);
- struct mddev *mddev = mddev_find(dev);
+ struct mddev *mddev = mddev_find_or_alloc(dev);
struct gendisk *disk;
int partitioned;
int shift;
@@ -6537,11 +6553,9 @@ static void autorun_devices(int part)
md_probe(dev);
mddev = mddev_find(dev);
- if (!mddev || !mddev->gendisk) {
- if (mddev)
- mddev_put(mddev);
+ if (!mddev)
break;
- }
+
if (mddev_lock(mddev))
pr_warn("md: %s locked, cannot run\n", mdname(mddev));
else if (mddev->raid_disks || mddev->major_version
--
2.30.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 05/15] md: refactor mddev_find_or_alloc
2021-03-30 16:17 move bd_mutex to the gendisk Christoph Hellwig
` (3 preceding siblings ...)
2021-03-30 16:17 ` [PATCH 04/15] md: split mddev_find Christoph Hellwig
@ 2021-03-30 16:17 ` Christoph Hellwig
2021-03-30 16:17 ` [PATCH 06/15] md: do not return existing mddevs from mddev_find_or_alloc Christoph Hellwig
` (9 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-30 16:17 UTC (permalink / raw)
To: Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
Allocate the new mddev first speculatively, which greatly simplifies
the code flow.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/md/md.c | 60 ++++++++++++++++++++-----------------------------
1 file changed, 24 insertions(+), 36 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 8e60bcc9c1d10c..ea9c92c113619b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -784,57 +784,45 @@ static struct mddev *mddev_find(dev_t unit)
static struct mddev *mddev_find_or_alloc(dev_t unit)
{
- struct mddev *mddev, *new = NULL;
+ struct mddev *mddev = NULL, *new;
if (unit && MAJOR(unit) != MD_MAJOR)
- unit &= ~((1<<MdpMinorShift)-1);
+ unit &= ~((1 << MdpMinorShift) - 1);
- retry:
- spin_lock(&all_mddevs_lock);
+ new = kzalloc(sizeof(*new), GFP_KERNEL);
+ if (!new)
+ return NULL;
+ mddev_init(new);
+ spin_lock(&all_mddevs_lock);
if (unit) {
mddev = mddev_find_locked(unit);
if (mddev) {
mddev_get(mddev);
- spin_unlock(&all_mddevs_lock);
- kfree(new);
- return mddev;
+ goto out_free_new;
}
- if (new) {
- list_add(&new->all_mddevs, &all_mddevs);
- spin_unlock(&all_mddevs_lock);
- new->hold_active = UNTIL_IOCTL;
- return new;
- }
- } else if (new) {
+ new->unit = unit;
+ if (MAJOR(unit) == MD_MAJOR)
+ new->md_minor = MINOR(unit);
+ else
+ new->md_minor = MINOR(unit) >> MdpMinorShift;
+ new->hold_active = UNTIL_IOCTL;
+ } else {
new->unit = mddev_alloc_unit();
- if (!new->unit) {
- spin_unlock(&all_mddevs_lock);
- kfree(new);
- return NULL;
- }
+ if (!new->unit)
+ goto out_free_new;
new->md_minor = MINOR(new->unit);
new->hold_active = UNTIL_STOP;
- list_add(&new->all_mddevs, &all_mddevs);
- spin_unlock(&all_mddevs_lock);
- return new;
}
- spin_unlock(&all_mddevs_lock);
-
- new = kzalloc(sizeof(*new), GFP_KERNEL);
- if (!new)
- return NULL;
- new->unit = unit;
- if (MAJOR(unit) == MD_MAJOR)
- new->md_minor = MINOR(unit);
- else
- new->md_minor = MINOR(unit) >> MdpMinorShift;
-
- mddev_init(new);
-
- goto retry;
+ list_add(&new->all_mddevs, &all_mddevs);
+ spin_unlock(&all_mddevs_lock);
+ return new;
+out_free_new:
+ spin_unlock(&all_mddevs_lock);
+ kfree(new);
+ return mddev;
}
static struct attribute_group md_redundancy_group;
--
2.30.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 06/15] md: do not return existing mddevs from mddev_find_or_alloc
2021-03-30 16:17 move bd_mutex to the gendisk Christoph Hellwig
` (4 preceding siblings ...)
2021-03-30 16:17 ` [PATCH 05/15] md: refactor mddev_find_or_alloc Christoph Hellwig
@ 2021-03-30 16:17 ` Christoph Hellwig
2021-03-30 16:17 ` [PATCH 07/15] block: remove the -ERESTARTSYS handling in blkdev_get_by_dev Christoph Hellwig
` (8 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-30 16:17 UTC (permalink / raw)
To: Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
Instead of returning an existing mddev, just for it to be discarded
later directly return -EEXIST. Rename the function to mddev_alloc now
that it doesn't find an existing mddev.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/md/md.c | 46 +++++++++++++++++++++++-----------------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index ea9c92c113619b..e614c40ca58974 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -782,26 +782,24 @@ static struct mddev *mddev_find(dev_t unit)
return mddev;
}
-static struct mddev *mddev_find_or_alloc(dev_t unit)
+static struct mddev *mddev_alloc(dev_t unit)
{
- struct mddev *mddev = NULL, *new;
+ struct mddev *new;
+ int error;
if (unit && MAJOR(unit) != MD_MAJOR)
unit &= ~((1 << MdpMinorShift) - 1);
new = kzalloc(sizeof(*new), GFP_KERNEL);
if (!new)
- return NULL;
+ return ERR_PTR(-ENOMEM);
mddev_init(new);
spin_lock(&all_mddevs_lock);
if (unit) {
- mddev = mddev_find_locked(unit);
- if (mddev) {
- mddev_get(mddev);
+ error = -EEXIST;
+ if (mddev_find_locked(unit))
goto out_free_new;
- }
-
new->unit = unit;
if (MAJOR(unit) == MD_MAJOR)
new->md_minor = MINOR(unit);
@@ -809,6 +807,7 @@ static struct mddev *mddev_find_or_alloc(dev_t unit)
new->md_minor = MINOR(unit) >> MdpMinorShift;
new->hold_active = UNTIL_IOCTL;
} else {
+ error = -ENODEV;
new->unit = mddev_alloc_unit();
if (!new->unit)
goto out_free_new;
@@ -822,7 +821,7 @@ static struct mddev *mddev_find_or_alloc(dev_t unit)
out_free_new:
spin_unlock(&all_mddevs_lock);
kfree(new);
- return mddev;
+ return ERR_PTR(error);
}
static struct attribute_group md_redundancy_group;
@@ -5661,29 +5660,29 @@ static int md_alloc(dev_t dev, char *name)
* writing to /sys/module/md_mod/parameters/new_array.
*/
static DEFINE_MUTEX(disks_mutex);
- struct mddev *mddev = mddev_find_or_alloc(dev);
+ struct mddev *mddev;
struct gendisk *disk;
int partitioned;
int shift;
int unit;
- int error;
+ int error ;
- if (!mddev)
- return -ENODEV;
-
- partitioned = (MAJOR(mddev->unit) != MD_MAJOR);
- shift = partitioned ? MdpMinorShift : 0;
- unit = MINOR(mddev->unit) >> shift;
-
- /* wait for any previous instance of this device to be
- * completely removed (mddev_delayed_delete).
+ /*
+ * Wait for any previous instance of this device to be completely
+ * removed (mddev_delayed_delete).
*/
flush_workqueue(md_misc_wq);
mutex_lock(&disks_mutex);
- error = -EEXIST;
- if (mddev->gendisk)
- goto abort;
+ mddev = mddev_alloc(dev);
+ if (IS_ERR(mddev)) {
+ mutex_unlock(&disks_mutex);
+ return PTR_ERR(mddev);
+ }
+
+ partitioned = (MAJOR(mddev->unit) != MD_MAJOR);
+ shift = partitioned ? MdpMinorShift : 0;
+ unit = MINOR(mddev->unit) >> shift;
if (name && !dev) {
/* Need to ensure that 'name' is not a duplicate.
@@ -5695,6 +5694,7 @@ static int md_alloc(dev_t dev, char *name)
if (mddev2->gendisk &&
strcmp(mddev2->gendisk->disk_name, name) == 0) {
spin_unlock(&all_mddevs_lock);
+ error = -EEXIST;
goto abort;
}
spin_unlock(&all_mddevs_lock);
--
2.30.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 07/15] block: remove the -ERESTARTSYS handling in blkdev_get_by_dev
2021-03-30 16:17 move bd_mutex to the gendisk Christoph Hellwig
` (5 preceding siblings ...)
2021-03-30 16:17 ` [PATCH 06/15] md: do not return existing mddevs from mddev_find_or_alloc Christoph Hellwig
@ 2021-03-30 16:17 ` Christoph Hellwig
2021-03-30 16:17 ` [PATCH 08/15] block: split __blkdev_get Christoph Hellwig
` (7 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-30 16:17 UTC (permalink / raw)
To: Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
Now that md has been cleaned up we can get rid of this hack.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/block_dev.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 535d29fa06fa47..0c09b6517b20b7 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1430,10 +1430,6 @@ struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode, void *holder)
if (ret)
return ERR_PTR(ret);
- /*
- * If we lost a race with 'disk' being deleted, try again. See md.c.
- */
-retry:
bdev = blkdev_get_no_open(dev);
if (!bdev)
return ERR_PTR(-ENXIO);
@@ -1480,8 +1476,6 @@ struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode, void *holder)
disk_unblock_events(disk);
put_blkdev:
blkdev_put_no_open(bdev);
- if (ret == -ERESTARTSYS)
- goto retry;
return ERR_PTR(ret);
}
EXPORT_SYMBOL(blkdev_get_by_dev);
--
2.30.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 08/15] block: split __blkdev_get
2021-03-30 16:17 move bd_mutex to the gendisk Christoph Hellwig
` (6 preceding siblings ...)
2021-03-30 16:17 ` [PATCH 07/15] block: remove the -ERESTARTSYS handling in blkdev_get_by_dev Christoph Hellwig
@ 2021-03-30 16:17 ` Christoph Hellwig
2021-03-30 16:17 ` [PATCH 09/15] block: move sync_blockdev from __blkdev_put to blkdev_put Christoph Hellwig
` (6 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-30 16:17 UTC (permalink / raw)
To: Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
Split __blkdev_get into one helper for the whole device, and one for
opening partitions. This removes the (bounded) recursion when opening
a partition.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/block_dev.c | 115 +++++++++++++++++++++++--------------------------
1 file changed, 55 insertions(+), 60 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 0c09b6517b20b7..d732001285201a 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1281,76 +1281,68 @@ int bdev_disk_changed(struct block_device *bdev, bool invalidate)
*/
EXPORT_SYMBOL_GPL(bdev_disk_changed);
-/*
- * bd_mutex locking:
- *
- * mutex_lock(part->bd_mutex)
- * mutex_lock_nested(whole->bd_mutex, 1)
- */
-static int __blkdev_get(struct block_device *bdev, fmode_t mode)
+static int blkdev_get_whole(struct block_device *bdev, fmode_t mode)
{
struct gendisk *disk = bdev->bd_disk;
int ret = 0;
- if (!bdev->bd_openers) {
- if (!bdev_is_partition(bdev)) {
- ret = 0;
- if (disk->fops->open)
- ret = disk->fops->open(bdev, mode);
-
- if (!ret)
- set_init_blocksize(bdev);
-
- /*
- * If the device is invalidated, rescan partition
- * if open succeeded or failed with -ENOMEDIUM.
- * The latter is necessary to prevent ghost
- * partitions on a removed medium.
- */
- if (test_bit(GD_NEED_PART_SCAN, &disk->state) &&
- (!ret || ret == -ENOMEDIUM))
- bdev_disk_changed(bdev, ret == -ENOMEDIUM);
-
- if (ret)
- return ret;
- } else {
- struct block_device *whole = bdgrab(disk->part0);
-
- mutex_lock_nested(&whole->bd_mutex, 1);
- ret = __blkdev_get(whole, mode);
- if (ret) {
- mutex_unlock(&whole->bd_mutex);
- bdput(whole);
- return ret;
- }
- whole->bd_part_count++;
- mutex_unlock(&whole->bd_mutex);
-
- if (!(disk->flags & GENHD_FL_UP) ||
- !bdev_nr_sectors(bdev)) {
- __blkdev_put(whole, mode, 1);
- bdput(whole);
- return -ENXIO;
- }
- set_init_blocksize(bdev);
+ if (disk->fops->open) {
+ ret = disk->fops->open(bdev, mode);
+ if (ret) {
+ /* avoid ghost partitions on a removed medium */
+ if (ret == -ENOMEDIUM &&
+ test_bit(GD_NEED_PART_SCAN, &disk->state))
+ bdev_disk_changed(bdev, true);
+ return ret;
}
+ }
+ if (!bdev->bd_openers) {
+ set_init_blocksize(bdev);
if (bdev->bd_bdi == &noop_backing_dev_info)
bdev->bd_bdi = bdi_get(disk->queue->backing_dev_info);
- } else {
- if (!bdev_is_partition(bdev)) {
- if (bdev->bd_disk->fops->open)
- ret = bdev->bd_disk->fops->open(bdev, mode);
- /* the same as first opener case, read comment there */
- if (test_bit(GD_NEED_PART_SCAN, &disk->state) &&
- (!ret || ret == -ENOMEDIUM))
- bdev_disk_changed(bdev, ret == -ENOMEDIUM);
- if (ret)
- return ret;
- }
}
+ if (test_bit(GD_NEED_PART_SCAN, &disk->state))
+ bdev_disk_changed(bdev, false);
bdev->bd_openers++;
+ return 0;;
+}
+
+static int blkdev_get_part(struct block_device *part, fmode_t mode)
+{
+ struct gendisk *disk = part->bd_disk;
+ struct block_device *whole;
+ int ret;
+
+ if (part->bd_openers)
+ goto done;
+
+ whole = bdgrab(disk->part0);
+ mutex_lock_nested(&whole->bd_mutex, 1);
+ ret = blkdev_get_whole(whole, mode);
+ if (ret) {
+ mutex_unlock(&whole->bd_mutex);
+ goto out_put_whole;
+ }
+ whole->bd_part_count++;
+ mutex_unlock(&whole->bd_mutex);
+
+ ret = -ENXIO;
+ if (!(disk->flags & GENHD_FL_UP) || !bdev_nr_sectors(part))
+ goto out_blkdev_put;
+
+ set_init_blocksize(part);
+ if (part->bd_bdi == &noop_backing_dev_info)
+ part->bd_bdi = bdi_get(disk->queue->backing_dev_info);
+done:
+ part->bd_openers++;
return 0;
+
+out_blkdev_put:
+ __blkdev_put(whole, mode, 1);
+out_put_whole:
+ bdput(whole);
+ return ret;
}
struct block_device *blkdev_get_no_open(dev_t dev)
@@ -1444,7 +1436,10 @@ struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode, void *holder)
disk_block_events(disk);
mutex_lock(&bdev->bd_mutex);
- ret =__blkdev_get(bdev, mode);
+ if (bdev_is_partition(bdev))
+ ret = blkdev_get_part(bdev, mode);
+ else
+ ret = blkdev_get_whole(bdev, mode);
if (ret)
goto abort_claiming;
if (mode & FMODE_EXCL) {
--
2.30.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 09/15] block: move sync_blockdev from __blkdev_put to blkdev_put
2021-03-30 16:17 move bd_mutex to the gendisk Christoph Hellwig
` (7 preceding siblings ...)
2021-03-30 16:17 ` [PATCH 08/15] block: split __blkdev_get Christoph Hellwig
@ 2021-03-30 16:17 ` Christoph Hellwig
2021-03-30 16:17 ` [PATCH 10/15] block: move bd_mutex to struct gendisk Christoph Hellwig
` (5 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-30 16:17 UTC (permalink / raw)
To: Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
Do the early unlocked syncing even earlier to move more code out of
the recursive path.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/block_dev.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index d732001285201a..691eef20a7ced7 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1547,16 +1547,6 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
struct gendisk *disk = bdev->bd_disk;
struct block_device *victim = NULL;
- /*
- * Sync early if it looks like we're the last one. If someone else
- * opens the block device between now and the decrement of bd_openers
- * then we did a sync that we didn't need to, but that's not the end
- * of the world and we want to avoid long (could be several minute)
- * syncs while holding the mutex.
- */
- if (bdev->bd_openers == 1)
- sync_blockdev(bdev);
-
mutex_lock_nested(&bdev->bd_mutex, for_part);
if (for_part)
bdev->bd_part_count--;
@@ -1583,6 +1573,16 @@ void blkdev_put(struct block_device *bdev, fmode_t mode)
{
struct gendisk *disk = bdev->bd_disk;
+ /*
+ * Sync early if it looks like we're the last one. If someone else
+ * opens the block device between now and the decrement of bd_openers
+ * then we did a sync that we didn't need to, but that's not the end
+ * of the world and we want to avoid long (could be several minute)
+ * syncs while holding the mutex.
+ */
+ if (bdev->bd_openers == 1)
+ sync_blockdev(bdev);
+
mutex_lock(&bdev->bd_mutex);
if (mode & FMODE_EXCL) {
--
2.30.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 10/15] block: move bd_mutex to struct gendisk
2021-03-30 16:17 move bd_mutex to the gendisk Christoph Hellwig
` (8 preceding siblings ...)
2021-03-30 16:17 ` [PATCH 09/15] block: move sync_blockdev from __blkdev_put to blkdev_put Christoph Hellwig
@ 2021-03-30 16:17 ` Christoph Hellwig
2021-04-01 8:25 ` Roger Pau Monné
2021-04-08 14:29 ` Stefan Haberland
2021-03-30 16:17 ` [PATCH 11/15] block: move adjusting bd_part_count out of __blkdev_get Christoph Hellwig
` (4 subsequent siblings)
14 siblings, 2 replies; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-30 16:17 UTC (permalink / raw)
To: Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
Replace the per-block device bd_mutex with a per-gendisk open_mutex,
thus simplifying locking wherever we deal with partitions.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
Documentation/filesystems/locking.rst | 2 +-
block/genhd.c | 3 ++-
block/partitions/core.c | 22 +++++++---------
drivers/block/loop.c | 14 +++++-----
drivers/block/xen-blkfront.c | 8 +++---
drivers/block/zram/zram_drv.c | 18 ++++++-------
drivers/block/zram/zram_drv.h | 2 +-
drivers/md/md.h | 6 ++---
drivers/s390/block/dasd_genhd.c | 8 +++---
drivers/scsi/sd.c | 4 +--
fs/block_dev.c | 37 +++++++++++----------------
fs/btrfs/volumes.c | 2 +-
fs/super.c | 8 +++---
include/linux/blk_types.h | 1 -
include/linux/genhd.h | 3 +++
15 files changed, 65 insertions(+), 73 deletions(-)
diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst
index 9774e92e449fbd..1cd8090fd4b262 100644
--- a/Documentation/filesystems/locking.rst
+++ b/Documentation/filesystems/locking.rst
@@ -475,7 +475,7 @@ prototypes::
locking rules:
======================= ===================
-ops bd_mutex
+ops open_mutex
======================= ===================
open: yes
release: yes
diff --git a/block/genhd.c b/block/genhd.c
index 8c8f543572e64e..7acedbd2474381 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1379,6 +1379,7 @@ struct gendisk *__alloc_disk_node(int minors, int node_id)
goto out_free_disk;
disk->node_id = node_id;
+ mutex_init(&disk->open_mutex);
xa_init(&disk->part_tbl);
if (xa_insert(&disk->part_tbl, 0, disk->part0, GFP_KERNEL))
goto out_destroy_part_tbl;
@@ -1596,7 +1597,7 @@ void disk_unblock_events(struct gendisk *disk)
* doesn't clear the events from @disk->ev.
*
* CONTEXT:
- * If @mask is non-zero must be called with bdev->bd_mutex held.
+ * If @mask is non-zero must be called with disk->open_mutex held.
*/
void disk_flush_events(struct gendisk *disk, unsigned int mask)
{
diff --git a/block/partitions/core.c b/block/partitions/core.c
index 1a7558917c47d6..60388a41ff92a3 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -282,7 +282,7 @@ struct device_type part_type = {
};
/*
- * Must be called either with bd_mutex held, before a disk can be opened or
+ * Must be called either with open_mutex held, before a disk can be opened or
* after all disk users are gone.
*/
void delete_partition(struct block_device *part)
@@ -308,7 +308,7 @@ static ssize_t whole_disk_show(struct device *dev,
static DEVICE_ATTR(whole_disk, 0444, whole_disk_show, NULL);
/*
- * Must be called either with bd_mutex held, before a disk can be opened or
+ * Must be called either with open_mutex held, before a disk can be opened or
* after all disk users are gone.
*/
static struct block_device *add_partition(struct gendisk *disk, int partno,
@@ -440,15 +440,15 @@ int bdev_add_partition(struct block_device *bdev, int partno,
{
struct block_device *part;
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&bdev->bd_disk->open_mutex);
if (partition_overlaps(bdev->bd_disk, start, length, -1)) {
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
return -EBUSY;
}
part = add_partition(bdev->bd_disk, partno, start, length,
ADDPART_FLAG_NONE, NULL);
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
return PTR_ERR_OR_ZERO(part);
}
@@ -461,8 +461,7 @@ int bdev_del_partition(struct block_device *bdev, int partno)
if (!part)
return -ENXIO;
- mutex_lock(&part->bd_mutex);
- mutex_lock_nested(&bdev->bd_mutex, 1);
+ mutex_lock(&bdev->bd_disk->open_mutex);
ret = -EBUSY;
if (part->bd_openers)
@@ -474,8 +473,7 @@ int bdev_del_partition(struct block_device *bdev, int partno)
delete_partition(part);
ret = 0;
out_unlock:
- mutex_unlock(&bdev->bd_mutex);
- mutex_unlock(&part->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
bdput(part);
return ret;
}
@@ -490,8 +488,7 @@ int bdev_resize_partition(struct block_device *bdev, int partno,
if (!part)
return -ENXIO;
- mutex_lock(&part->bd_mutex);
- mutex_lock_nested(&bdev->bd_mutex, 1);
+ mutex_lock(&bdev->bd_disk->open_mutex);
ret = -EINVAL;
if (start != part->bd_start_sect)
goto out_unlock;
@@ -504,8 +501,7 @@ int bdev_resize_partition(struct block_device *bdev, int partno,
ret = 0;
out_unlock:
- mutex_unlock(&part->bd_mutex);
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
bdput(part);
return ret;
}
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index a370cde3ddd49a..f5eb70a90ed067 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -651,9 +651,9 @@ static void loop_reread_partitions(struct loop_device *lo,
{
int rc;
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&bdev->bd_disk->open_mutex);
rc = bdev_disk_changed(bdev, false);
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
if (rc)
pr_warn("%s: partition scan of loop%d (%s) failed (rc=%d)\n",
__func__, lo->lo_number, lo->lo_file_name, rc);
@@ -746,7 +746,7 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
mutex_unlock(&lo->lo_mutex);
/*
* We must drop file reference outside of lo_mutex as dropping
- * the file ref can take bd_mutex which creates circular locking
+ * the file ref can take open_mutex which creates circular locking
* dependency.
*/
fput(old_file);
@@ -1259,7 +1259,7 @@ static int __loop_clr_fd(struct loop_device *lo, bool release)
mutex_unlock(&lo->lo_mutex);
if (partscan) {
/*
- * bd_mutex has been held already in release path, so don't
+ * open_mutex has been held already in release path, so don't
* acquire it if this function is called in such case.
*
* If the reread partition isn't from release path, lo_refcnt
@@ -1267,10 +1267,10 @@ static int __loop_clr_fd(struct loop_device *lo, bool release)
* current holder is released.
*/
if (!release)
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&bdev->bd_disk->open_mutex);
err = bdev_disk_changed(bdev, false);
if (!release)
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
if (err)
pr_warn("%s: partition scan of loop%d failed (rc=%d)\n",
__func__, lo_number, err);
@@ -1297,7 +1297,7 @@ static int __loop_clr_fd(struct loop_device *lo, bool release)
/*
* Need not hold lo_mutex to fput backing file. Calling fput holding
* lo_mutex triggers a circular lock dependency possibility warning as
- * fput can take bd_mutex which is usually taken before lo_mutex.
+ * fput can take open_mutex which is usually taken before lo_mutex.
*/
if (filp)
fput(filp);
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index e1c6798889f48a..fbab2ce7dfa1a5 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -2163,7 +2163,7 @@ static void blkfront_closing(struct blkfront_info *info)
return;
}
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&bdev->bd_disk->open_mutex);
if (bdev->bd_openers) {
xenbus_dev_error(xbdev, -EBUSY,
@@ -2174,7 +2174,7 @@ static void blkfront_closing(struct blkfront_info *info)
xenbus_frontend_closed(xbdev);
}
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
bdput(bdev);
}
@@ -2531,7 +2531,7 @@ static int blkfront_remove(struct xenbus_device *xbdev)
* isn't closed yet, we let release take care of it.
*/
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&disk->open_mutex);
info = disk->private_data;
dev_warn(disk_to_dev(disk),
@@ -2546,7 +2546,7 @@ static int blkfront_remove(struct xenbus_device *xbdev)
mutex_unlock(&blkfront_mutex);
}
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&disk->open_mutex);
bdput(bdev);
return 0;
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index cf8deecc39efbc..b3bf544493d340 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1781,24 +1781,24 @@ static ssize_t reset_store(struct device *dev,
zram = dev_to_zram(dev);
bdev = zram->disk->part0;
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&bdev->bd_disk->open_mutex);
/* Do not reset an active device or claimed device */
if (bdev->bd_openers || zram->claim) {
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
return -EBUSY;
}
/* From now on, anyone can't open /dev/zram[0-9] */
zram->claim = true;
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
/* Make sure all the pending I/O are finished */
fsync_bdev(bdev);
zram_reset_device(zram);
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&bdev->bd_disk->open_mutex);
zram->claim = false;
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
return len;
}
@@ -1808,7 +1808,7 @@ static int zram_open(struct block_device *bdev, fmode_t mode)
int ret = 0;
struct zram *zram;
- WARN_ON(!mutex_is_locked(&bdev->bd_mutex));
+ WARN_ON(!mutex_is_locked(&bdev->bd_disk->open_mutex));
zram = bdev->bd_disk->private_data;
/* zram was claimed to reset so open request fails */
@@ -1982,14 +1982,14 @@ static int zram_remove(struct zram *zram)
{
struct block_device *bdev = zram->disk->part0;
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&bdev->bd_disk->open_mutex);
if (bdev->bd_openers || zram->claim) {
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
return -EBUSY;
}
zram->claim = true;
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
zram_debugfs_unregister(zram);
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index 419a7e8281ee3a..74c411911b6eac 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -112,7 +112,7 @@ struct zram {
/*
* zram is claimed so open request will be failed
*/
- bool claim; /* Protected by bdev->bd_mutex */
+ bool claim; /* Protected by disk->open_mutex */
struct file *backing_dev;
#ifdef CONFIG_ZRAM_WRITEBACK
spinlock_t wb_limit_lock;
diff --git a/drivers/md/md.h b/drivers/md/md.h
index fb7eab58cfd517..a88086d4110c24 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -395,10 +395,10 @@ struct mddev {
* that we are never stopping an array while it is open.
* 'reconfig_mutex' protects all other reconfiguration.
* These locks are separate due to conflicting interactions
- * with bdev->bd_mutex.
+ * with disk->open_mutex.
* Lock ordering is:
- * reconfig_mutex -> bd_mutex
- * bd_mutex -> open_mutex: e.g. __blkdev_get -> md_open
+ * reconfig_mutex -> disk->open_mutex
+ * disk->open_mutex -> open_mutex: e.g. __blkdev_get -> md_open
*/
struct mutex open_mutex;
struct mutex reconfig_mutex;
diff --git a/drivers/s390/block/dasd_genhd.c b/drivers/s390/block/dasd_genhd.c
index a9698fba9b76ce..c93a46a38085bb 100644
--- a/drivers/s390/block/dasd_genhd.c
+++ b/drivers/s390/block/dasd_genhd.c
@@ -109,9 +109,9 @@ int dasd_scan_partitions(struct dasd_block *block)
return -ENODEV;
}
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&block->gdp->open_mutex);
rc = bdev_disk_changed(bdev, false);
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&block->gdp->open_mutex);
if (rc)
DBF_DEV_EVENT(DBF_ERR, block->base,
"scan partitions error, rc %d", rc);
@@ -145,9 +145,9 @@ void dasd_destroy_partitions(struct dasd_block *block)
bdev = block->bdev;
block->bdev = NULL;
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&bdev->bd_disk->open_mutex);
blk_drop_partitions(bdev);
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
/* Matching blkdev_put to the blkdev_get in dasd_scan_partitions. */
blkdev_put(bdev, FMODE_READ);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index ed0b1bb99f0832..dc79d2e0e9dba9 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1400,7 +1400,7 @@ static void sd_uninit_command(struct scsi_cmnd *SCpnt)
* In the latter case @inode and @filp carry an abridged amount
* of information as noted above.
*
- * Locking: called with bdev->bd_mutex held.
+ * Locking: called with bdev->bd_disk->open_mutex held.
**/
static int sd_open(struct block_device *bdev, fmode_t mode)
{
@@ -1476,7 +1476,7 @@ static int sd_open(struct block_device *bdev, fmode_t mode)
* Note: may block (uninterruptible) if error recovery is underway
* on this disk.
*
- * Locking: called with bdev->bd_mutex held.
+ * Locking: called with bdev->bd_disk->open_mutex held.
**/
static void sd_release(struct gendisk *disk, fmode_t mode)
{
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 691eef20a7ced7..4113a27ad9e72d 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -891,7 +891,6 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno)
mapping_set_gfp_mask(&inode->i_data, GFP_USER);
bdev = I_BDEV(inode);
- mutex_init(&bdev->bd_mutex);
mutex_init(&bdev->bd_fsfreeze_mutex);
spin_lock_init(&bdev->bd_size_lock);
bdev->bd_disk = disk;
@@ -1150,7 +1149,7 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
struct bd_holder_disk *holder;
int ret = 0;
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&bdev->bd_disk->open_mutex);
WARN_ON_ONCE(!bdev->bd_holder);
@@ -1195,7 +1194,7 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
out_free:
kfree(holder);
out_unlock:
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
return ret;
}
EXPORT_SYMBOL_GPL(bd_link_disk_holder);
@@ -1214,7 +1213,7 @@ void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
{
struct bd_holder_disk *holder;
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&bdev->bd_disk->open_mutex);
holder = bd_find_holder_disk(bdev, disk);
@@ -1226,7 +1225,7 @@ void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
kfree(holder);
}
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
}
EXPORT_SYMBOL_GPL(bd_unlink_disk_holder);
#endif
@@ -1238,7 +1237,7 @@ int bdev_disk_changed(struct block_device *bdev, bool invalidate)
struct gendisk *disk = bdev->bd_disk;
int ret;
- lockdep_assert_held(&bdev->bd_mutex);
+ lockdep_assert_held(&disk->open_mutex);
clear_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
@@ -1318,14 +1317,10 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode)
goto done;
whole = bdgrab(disk->part0);
- mutex_lock_nested(&whole->bd_mutex, 1);
ret = blkdev_get_whole(whole, mode);
- if (ret) {
- mutex_unlock(&whole->bd_mutex);
+ if (ret)
goto out_put_whole;
- }
whole->bd_part_count++;
- mutex_unlock(&whole->bd_mutex);
ret = -ENXIO;
if (!(disk->flags & GENHD_FL_UP) || !bdev_nr_sectors(part))
@@ -1435,7 +1430,7 @@ struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode, void *holder)
disk_block_events(disk);
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&disk->open_mutex);
if (bdev_is_partition(bdev))
ret = blkdev_get_part(bdev, mode);
else
@@ -1458,7 +1453,7 @@ struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode, void *holder)
unblock_events = false;
}
}
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&disk->open_mutex);
if (unblock_events)
disk_unblock_events(disk);
@@ -1467,7 +1462,7 @@ struct block_device *blkdev_get_by_dev(dev_t dev, fmode_t mode, void *holder)
abort_claiming:
if (mode & FMODE_EXCL)
bd_abort_claiming(bdev, holder);
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&disk->open_mutex);
disk_unblock_events(disk);
put_blkdev:
blkdev_put_no_open(bdev);
@@ -1547,7 +1542,6 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
struct gendisk *disk = bdev->bd_disk;
struct block_device *victim = NULL;
- mutex_lock_nested(&bdev->bd_mutex, for_part);
if (for_part)
bdev->bd_part_count--;
@@ -1562,7 +1556,6 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
if (!bdev_is_partition(bdev) && disk->fops->release)
disk->fops->release(disk, mode);
- mutex_unlock(&bdev->bd_mutex);
if (victim) {
__blkdev_put(victim, mode, 1);
bdput(victim);
@@ -1583,15 +1576,14 @@ void blkdev_put(struct block_device *bdev, fmode_t mode)
if (bdev->bd_openers == 1)
sync_blockdev(bdev);
- mutex_lock(&bdev->bd_mutex);
-
+ mutex_lock(&disk->open_mutex);
if (mode & FMODE_EXCL) {
struct block_device *whole = bdev_whole(bdev);
bool bdev_free;
/*
* Release a claim on the device. The holder fields
- * are protected with bdev_lock. bd_mutex is to
+ * are protected with bdev_lock. open_mutex is to
* synchronize disk_holder unlinking.
*/
spin_lock(&bdev_lock);
@@ -1622,9 +1614,10 @@ void blkdev_put(struct block_device *bdev, fmode_t mode)
* from userland - e.g. eject(1).
*/
disk_flush_events(disk, DISK_EVENT_MEDIA_CHANGE);
- mutex_unlock(&bdev->bd_mutex);
__blkdev_put(bdev, mode, 0);
+ mutex_unlock(&disk->open_mutex);
+
blkdev_put_no_open(bdev);
}
EXPORT_SYMBOL(blkdev_put);
@@ -1917,10 +1910,10 @@ void iterate_bdevs(void (*func)(struct block_device *, void *), void *arg)
old_inode = inode;
bdev = I_BDEV(inode);
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&bdev->bd_disk->open_mutex);
if (bdev->bd_openers)
func(bdev, arg);
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
spin_lock(&blockdev_superblock->s_inode_list_lock);
}
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index bc3b33efddc569..30fb9084ac145b 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1246,7 +1246,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
lockdep_assert_held(&uuid_mutex);
/*
* The device_list_mutex cannot be taken here in case opening the
- * underlying device takes further locks like bd_mutex.
+ * underlying device takes further locks like open_mutex.
*
* We also don't need the lock here as this is called during mount and
* exclusion is provided by uuid_mutex
diff --git a/fs/super.c b/fs/super.c
index 8c1baca35c160b..90b68352a26e14 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1276,9 +1276,9 @@ int get_tree_bdev(struct fs_context *fc,
}
/*
- * s_umount nests inside bd_mutex during
+ * s_umount nests inside open_mutex during
* __invalidate_device(). blkdev_put() acquires
- * bd_mutex and can't be called under s_umount. Drop
+ * open_mutex and can't be called under s_umount. Drop
* s_umount temporarily. This is safe as we're
* holding an active reference.
*/
@@ -1351,9 +1351,9 @@ struct dentry *mount_bdev(struct file_system_type *fs_type,
}
/*
- * s_umount nests inside bd_mutex during
+ * s_umount nests inside open_mutex during
* __invalidate_device(). blkdev_put() acquires
- * bd_mutex and can't be called under s_umount. Drop
+ * open_mutex and can't be called under s_umount. Drop
* s_umount temporarily. This is safe as we're
* holding an active reference.
*/
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index db026b6ec15ab7..a09660671fa47e 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -29,7 +29,6 @@ struct block_device {
int bd_openers;
struct inode * bd_inode; /* will die */
struct super_block * bd_super;
- struct mutex bd_mutex; /* open/close mutex */
void * bd_claiming;
struct device bd_device;
void * bd_holder;
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index f364619092cca0..02ea04144ece7b 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -153,6 +153,9 @@ struct gendisk {
unsigned long state;
#define GD_NEED_PART_SCAN 0
#define GD_READ_ONLY 1
+
+ struct mutex open_mutex; /* open/close mutex */
+
struct kobject *slave_dir;
struct timer_rand_state *random;
--
2.30.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 10/15] block: move bd_mutex to struct gendisk
2021-03-30 16:17 ` [PATCH 10/15] block: move bd_mutex to struct gendisk Christoph Hellwig
@ 2021-04-01 8:25 ` Roger Pau Monné
2021-04-08 14:29 ` Stefan Haberland
1 sibling, 0 replies; 21+ messages in thread
From: Roger Pau Monné @ 2021-04-01 8:25 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, Song Liu, Konrad Rzeszutek Wilk, Minchan Kim,
Nitin Gupta, Stefan Haberland, Jan Hoeppner, linux-block,
linux-raid, linux-s390, linux-scsi
On Tue, Mar 30, 2021 at 06:17:22PM +0200, Christoph Hellwig wrote:
> Replace the per-block device bd_mutex with a per-gendisk open_mutex,
> thus simplifying locking wherever we deal with partitions.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> Documentation/filesystems/locking.rst | 2 +-
> block/genhd.c | 3 ++-
> block/partitions/core.c | 22 +++++++---------
> drivers/block/loop.c | 14 +++++-----
> drivers/block/xen-blkfront.c | 8 +++---
For xenblkfront:
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Thanks, Roger.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 10/15] block: move bd_mutex to struct gendisk
2021-03-30 16:17 ` [PATCH 10/15] block: move bd_mutex to struct gendisk Christoph Hellwig
2021-04-01 8:25 ` Roger Pau Monné
@ 2021-04-08 14:29 ` Stefan Haberland
1 sibling, 0 replies; 21+ messages in thread
From: Stefan Haberland @ 2021-04-08 14:29 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Jan Hoeppner, linux-block, linux-raid,
linux-s390, linux-scsi
Am 30.03.21 um 18:17 schrieb Christoph Hellwig:
> Replace the per-block device bd_mutex with a per-gendisk open_mutex,
> thus simplifying locking wherever we deal with partitions.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> Documentation/filesystems/locking.rst | 2 +-
> block/genhd.c | 3 ++-
> block/partitions/core.c | 22 +++++++---------
> drivers/block/loop.c | 14 +++++-----
> drivers/block/xen-blkfront.c | 8 +++---
> drivers/block/zram/zram_drv.c | 18 ++++++-------
> drivers/block/zram/zram_drv.h | 2 +-
> drivers/md/md.h | 6 ++---
> drivers/s390/block/dasd_genhd.c | 8 +++---
For dasd:
Acked-by: Stefan Haberland <sth@linux.ibm.com>
Thanks,
Stefan
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 11/15] block: move adjusting bd_part_count out of __blkdev_get
2021-03-30 16:17 move bd_mutex to the gendisk Christoph Hellwig
` (9 preceding siblings ...)
2021-03-30 16:17 ` [PATCH 10/15] block: move bd_mutex to struct gendisk Christoph Hellwig
@ 2021-03-30 16:17 ` Christoph Hellwig
2021-03-30 16:17 ` [PATCH 12/15] block: split __blkdev_put Christoph Hellwig
` (3 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-30 16:17 UTC (permalink / raw)
To: Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
Keep in the callers and thus remove the for_part argument. This mirrors
what is done on the blkdev_get side and slightly simplifies
blkdev_get_part as well.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/block_dev.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 4113a27ad9e72d..044e9200783e2d 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1230,7 +1230,7 @@ void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
EXPORT_SYMBOL_GPL(bd_unlink_disk_holder);
#endif
-static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part);
+static void __blkdev_put(struct block_device *bdev, fmode_t mode);
int bdev_disk_changed(struct block_device *bdev, bool invalidate)
{
@@ -1320,12 +1320,12 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode)
ret = blkdev_get_whole(whole, mode);
if (ret)
goto out_put_whole;
- whole->bd_part_count++;
ret = -ENXIO;
if (!(disk->flags & GENHD_FL_UP) || !bdev_nr_sectors(part))
goto out_blkdev_put;
+ whole->bd_part_count++;
set_init_blocksize(part);
if (part->bd_bdi == &noop_backing_dev_info)
part->bd_bdi = bdi_get(disk->queue->backing_dev_info);
@@ -1334,7 +1334,7 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode)
return 0;
out_blkdev_put:
- __blkdev_put(whole, mode, 1);
+ __blkdev_put(whole, mode);
out_put_whole:
bdput(whole);
return ret;
@@ -1537,14 +1537,11 @@ static int blkdev_open(struct inode * inode, struct file * filp)
return 0;
}
-static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
+static void __blkdev_put(struct block_device *bdev, fmode_t mode)
{
struct gendisk *disk = bdev->bd_disk;
struct block_device *victim = NULL;
- if (for_part)
- bdev->bd_part_count--;
-
if (!--bdev->bd_openers) {
WARN_ON_ONCE(bdev->bd_holders);
sync_blockdev(bdev);
@@ -1557,7 +1554,8 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
if (!bdev_is_partition(bdev) && disk->fops->release)
disk->fops->release(disk, mode);
if (victim) {
- __blkdev_put(victim, mode, 1);
+ victim->bd_part_count--;
+ __blkdev_put(victim, mode);
bdput(victim);
}
}
@@ -1615,7 +1613,7 @@ void blkdev_put(struct block_device *bdev, fmode_t mode)
*/
disk_flush_events(disk, DISK_EVENT_MEDIA_CHANGE);
- __blkdev_put(bdev, mode, 0);
+ __blkdev_put(bdev, mode);
mutex_unlock(&disk->open_mutex);
blkdev_put_no_open(bdev);
--
2.30.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 12/15] block: split __blkdev_put
2021-03-30 16:17 move bd_mutex to the gendisk Christoph Hellwig
` (10 preceding siblings ...)
2021-03-30 16:17 ` [PATCH 11/15] block: move adjusting bd_part_count out of __blkdev_get Christoph Hellwig
@ 2021-03-30 16:17 ` Christoph Hellwig
2021-03-30 16:17 ` [PATCH 13/15] block: move bd_part_count to struct gendisk Christoph Hellwig
` (2 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-30 16:17 UTC (permalink / raw)
To: Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
Split __blkdev_put into one helper for the whole device, and one for
partitions as well as another shared helper for flushing the block
device inode mapping.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/block_dev.c | 58 ++++++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 26 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 044e9200783e2d..ade5a180ff62d3 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1230,7 +1230,13 @@ void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
EXPORT_SYMBOL_GPL(bd_unlink_disk_holder);
#endif
-static void __blkdev_put(struct block_device *bdev, fmode_t mode);
+static void blkdev_flush_mapping(struct block_device *bdev)
+{
+ WARN_ON_ONCE(bdev->bd_holders);
+ sync_blockdev(bdev);
+ kill_bdev(bdev);
+ bdev_write_inode(bdev);
+}
int bdev_disk_changed(struct block_device *bdev, bool invalidate)
{
@@ -1307,6 +1313,14 @@ static int blkdev_get_whole(struct block_device *bdev, fmode_t mode)
return 0;;
}
+static void blkdev_put_whole(struct block_device *bdev, fmode_t mode)
+{
+ if (!--bdev->bd_openers)
+ blkdev_flush_mapping(bdev);
+ if (bdev->bd_disk->fops->release)
+ bdev->bd_disk->fops->release(bdev->bd_disk, mode);
+}
+
static int blkdev_get_part(struct block_device *part, fmode_t mode)
{
struct gendisk *disk = part->bd_disk;
@@ -1334,12 +1348,24 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode)
return 0;
out_blkdev_put:
- __blkdev_put(whole, mode);
+ blkdev_put_whole(whole, mode);
out_put_whole:
bdput(whole);
return ret;
}
+static void blkdev_put_part(struct block_device *part, fmode_t mode)
+{
+ struct block_device *whole = bdev_whole(part);
+
+ if (--part->bd_openers)
+ return;
+ blkdev_flush_mapping(part);
+ whole->bd_part_count--;
+ blkdev_put_whole(whole, mode);
+ bdput(whole);
+}
+
struct block_device *blkdev_get_no_open(dev_t dev)
{
struct block_device *bdev;
@@ -1537,29 +1563,6 @@ static int blkdev_open(struct inode * inode, struct file * filp)
return 0;
}
-static void __blkdev_put(struct block_device *bdev, fmode_t mode)
-{
- struct gendisk *disk = bdev->bd_disk;
- struct block_device *victim = NULL;
-
- if (!--bdev->bd_openers) {
- WARN_ON_ONCE(bdev->bd_holders);
- sync_blockdev(bdev);
- kill_bdev(bdev);
- bdev_write_inode(bdev);
- if (bdev_is_partition(bdev))
- victim = bdev_whole(bdev);
- }
-
- if (!bdev_is_partition(bdev) && disk->fops->release)
- disk->fops->release(disk, mode);
- if (victim) {
- victim->bd_part_count--;
- __blkdev_put(victim, mode);
- bdput(victim);
- }
-}
-
void blkdev_put(struct block_device *bdev, fmode_t mode)
{
struct gendisk *disk = bdev->bd_disk;
@@ -1613,7 +1616,10 @@ void blkdev_put(struct block_device *bdev, fmode_t mode)
*/
disk_flush_events(disk, DISK_EVENT_MEDIA_CHANGE);
- __blkdev_put(bdev, mode);
+ if (bdev_is_partition(bdev))
+ blkdev_put_part(bdev, mode);
+ else
+ blkdev_put_whole(bdev, mode);
mutex_unlock(&disk->open_mutex);
blkdev_put_no_open(bdev);
--
2.30.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 13/15] block: move bd_part_count to struct gendisk
2021-03-30 16:17 move bd_mutex to the gendisk Christoph Hellwig
` (11 preceding siblings ...)
2021-03-30 16:17 ` [PATCH 12/15] block: split __blkdev_put Christoph Hellwig
@ 2021-03-30 16:17 ` Christoph Hellwig
2021-03-30 16:17 ` [PATCH 14/15] block: factor out a part_devt helper Christoph Hellwig
2021-03-30 16:17 ` [PATCH 15/15] block: remove bdget_disk Christoph Hellwig
14 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-30 16:17 UTC (permalink / raw)
To: Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
The bd_part_count value only makes sense for whole devices, so move it
to struct gendisk and give it a more descriptive name.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/partitions/core.c | 2 +-
fs/block_dev.c | 4 ++--
include/linux/blk_types.h | 3 ---
include/linux/genhd.h | 1 +
4 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/block/partitions/core.c b/block/partitions/core.c
index 60388a41ff92a3..162b9b35c53171 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -527,7 +527,7 @@ int blk_drop_partitions(struct block_device *bdev)
struct disk_part_iter piter;
struct block_device *part;
- if (bdev->bd_part_count)
+ if (bdev->bd_disk->open_partitions)
return -EBUSY;
sync_blockdev(bdev);
diff --git a/fs/block_dev.c b/fs/block_dev.c
index ade5a180ff62d3..6d7e3bd7cb7ce3 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1339,7 +1339,7 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode)
if (!(disk->flags & GENHD_FL_UP) || !bdev_nr_sectors(part))
goto out_blkdev_put;
- whole->bd_part_count++;
+ disk->open_partitions++;
set_init_blocksize(part);
if (part->bd_bdi == &noop_backing_dev_info)
part->bd_bdi = bdi_get(disk->queue->backing_dev_info);
@@ -1361,7 +1361,7 @@ static void blkdev_put_part(struct block_device *part, fmode_t mode)
if (--part->bd_openers)
return;
blkdev_flush_mapping(part);
- whole->bd_part_count--;
+ whole->bd_disk->open_partitions--;
blkdev_put_whole(whole, mode);
bdput(whole);
}
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index a09660671fa47e..fd3860d18d7ed7 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -39,9 +39,6 @@ struct block_device {
#endif
struct kobject *bd_holder_dir;
u8 bd_partno;
- /* number of times partitions within this device have been opened. */
- unsigned bd_part_count;
-
spinlock_t bd_size_lock; /* for bd_inode->i_size updates */
struct gendisk * bd_disk;
struct backing_dev_info *bd_bdi;
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 02ea04144ece7b..146d2dafdccd58 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -155,6 +155,7 @@ struct gendisk {
#define GD_READ_ONLY 1
struct mutex open_mutex; /* open/close mutex */
+ unsigned open_partitions; /* number of open partitions */
struct kobject *slave_dir;
--
2.30.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 14/15] block: factor out a part_devt helper
2021-03-30 16:17 move bd_mutex to the gendisk Christoph Hellwig
` (12 preceding siblings ...)
2021-03-30 16:17 ` [PATCH 13/15] block: move bd_part_count to struct gendisk Christoph Hellwig
@ 2021-03-30 16:17 ` Christoph Hellwig
2021-03-30 16:17 ` [PATCH 15/15] block: remove bdget_disk Christoph Hellwig
14 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-30 16:17 UTC (permalink / raw)
To: Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
Add a helper to find the dev_t for a disk + partno tuple.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/genhd.c | 25 +++++++++++++++++--------
include/linux/genhd.h | 1 +
init/do_mounts.c | 10 ++--------
3 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 7acedbd2474381..232c8420b04973 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1326,6 +1326,19 @@ static int __init proc_genhd_init(void)
module_init(proc_genhd_init);
#endif /* CONFIG_PROC_FS */
+dev_t part_devt(struct gendisk *disk, u8 partno)
+{
+ struct block_device *part = bdget_disk(disk, partno);
+ dev_t devt = 0;
+
+ if (part) {
+ devt = part->bd_dev;
+ bdput(part);
+ }
+
+ return devt;
+}
+
dev_t blk_lookup_devt(const char *name, int partno)
{
dev_t devt = MKDEV(0, 0);
@@ -1335,7 +1348,6 @@ dev_t blk_lookup_devt(const char *name, int partno)
class_dev_iter_init(&iter, &block_class, NULL, &disk_type);
while ((dev = class_dev_iter_next(&iter))) {
struct gendisk *disk = dev_to_disk(dev);
- struct block_device *part;
if (strcmp(dev_name(dev), name))
continue;
@@ -1346,13 +1358,10 @@ dev_t blk_lookup_devt(const char *name, int partno)
*/
devt = MKDEV(MAJOR(dev->devt),
MINOR(dev->devt) + partno);
- break;
- }
- part = bdget_disk(disk, partno);
- if (part) {
- devt = part->bd_dev;
- bdput(part);
- break;
+ } else {
+ devt = part_devt(disk, partno);
+ if (devt)
+ break;
}
}
class_dev_iter_exit(&iter);
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 146d2dafdccd58..819366811349eb 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -331,6 +331,7 @@ static inline void bd_unlink_disk_holder(struct block_device *bdev,
extern struct rw_semaphore bdev_lookup_sem;
+dev_t part_devt(struct gendisk *disk, u8 partno);
dev_t blk_lookup_devt(const char *name, int partno);
void blk_request_module(dev_t devt);
#ifdef CONFIG_BLOCK
diff --git a/init/do_mounts.c b/init/do_mounts.c
index a78e44ee6adb8d..74aede860de739 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -133,14 +133,8 @@ static dev_t devt_from_partuuid(const char *uuid_str)
* Attempt to find the requested partition by adding an offset
* to the partition number found by UUID.
*/
- struct block_device *part;
-
- part = bdget_disk(dev_to_disk(dev),
- dev_to_bdev(dev)->bd_partno + offset);
- if (part) {
- devt = part->bd_dev;
- bdput(part);
- }
+ devt = part_devt(dev_to_disk(dev),
+ dev_to_bdev(dev)->bd_partno + offset);
} else {
devt = dev->devt;
}
--
2.30.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 15/15] block: remove bdget_disk
2021-03-30 16:17 move bd_mutex to the gendisk Christoph Hellwig
` (13 preceding siblings ...)
2021-03-30 16:17 ` [PATCH 14/15] block: factor out a part_devt helper Christoph Hellwig
@ 2021-03-30 16:17 ` Christoph Hellwig
2021-03-30 23:37 ` Chaitanya Kulkarni
14 siblings, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2021-03-30 16:17 UTC (permalink / raw)
To: Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
Just opencode the xa_load in the callers, as none of them actually
needs a reference to the bdev.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/genhd.c | 35 +++++------------------------------
block/partitions/core.c | 25 ++++++++++++-------------
include/linux/genhd.h | 1 -
3 files changed, 17 insertions(+), 44 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 232c8420b04973..f0d4cf80dac1ea 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -778,32 +778,6 @@ void blk_request_module(dev_t devt)
request_module("block-major-%d", MAJOR(devt));
}
-/**
- * bdget_disk - do bdget() by gendisk and partition number
- * @disk: gendisk of interest
- * @partno: partition number
- *
- * Find partition @partno from @disk, do bdget() on it.
- *
- * CONTEXT:
- * Don't care.
- *
- * RETURNS:
- * Resulting block_device on success, NULL on failure.
- */
-struct block_device *bdget_disk(struct gendisk *disk, int partno)
-{
- struct block_device *bdev = NULL;
-
- rcu_read_lock();
- bdev = xa_load(&disk->part_tbl, partno);
- if (bdev && !bdgrab(bdev))
- bdev = NULL;
- rcu_read_unlock();
-
- return bdev;
-}
-
/*
* print a full list of all partitions - intended for places where the root
* filesystem can't be mounted and thus to give the victim some idea of what
@@ -1328,13 +1302,14 @@ module_init(proc_genhd_init);
dev_t part_devt(struct gendisk *disk, u8 partno)
{
- struct block_device *part = bdget_disk(disk, partno);
+ struct block_device *part;
dev_t devt = 0;
- if (part) {
+ rcu_read_lock();
+ part = xa_load(&disk->part_tbl, partno);
+ if (part)
devt = part->bd_dev;
- bdput(part);
- }
+ rcu_read_unlock();
return devt;
}
diff --git a/block/partitions/core.c b/block/partitions/core.c
index 162b9b35c53171..00e425c668a5f8 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -322,6 +322,8 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,
const char *dname;
int err;
+ lockdep_assert_held(&disk->open_mutex);
+
/*
* Partitions are not supported on zoned block devices that are used as
* such.
@@ -454,14 +456,13 @@ int bdev_add_partition(struct block_device *bdev, int partno,
int bdev_del_partition(struct block_device *bdev, int partno)
{
- struct block_device *part;
- int ret;
-
- part = bdget_disk(bdev->bd_disk, partno);
- if (!part)
- return -ENXIO;
+ struct block_device *part = NULL;
+ int ret = -ENXIO;
mutex_lock(&bdev->bd_disk->open_mutex);
+ part = xa_load(&bdev->bd_disk->part_tbl, partno);
+ if (!part)
+ goto out_unlock;
ret = -EBUSY;
if (part->bd_openers)
@@ -474,21 +475,20 @@ int bdev_del_partition(struct block_device *bdev, int partno)
ret = 0;
out_unlock:
mutex_unlock(&bdev->bd_disk->open_mutex);
- bdput(part);
return ret;
}
int bdev_resize_partition(struct block_device *bdev, int partno,
sector_t start, sector_t length)
{
- struct block_device *part;
- int ret = 0;
+ struct block_device *part = NULL;
+ int ret = -ENXIO;
- part = bdget_disk(bdev->bd_disk, partno);
+ mutex_lock(&bdev->bd_disk->open_mutex);
+ part = xa_load(&bdev->bd_disk->part_tbl, partno);
if (!part)
- return -ENXIO;
+ goto out_unlock;
- mutex_lock(&bdev->bd_disk->open_mutex);
ret = -EINVAL;
if (start != part->bd_start_sect)
goto out_unlock;
@@ -502,7 +502,6 @@ int bdev_resize_partition(struct block_device *bdev, int partno,
ret = 0;
out_unlock:
mutex_unlock(&bdev->bd_disk->open_mutex);
- bdput(part);
return ret;
}
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 819366811349eb..674fc0761105bc 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -241,7 +241,6 @@ static inline void add_disk_no_queue_reg(struct gendisk *disk)
}
extern void del_gendisk(struct gendisk *gp);
-extern struct block_device *bdget_disk(struct gendisk *disk, int partno);
void set_disk_ro(struct gendisk *disk, bool read_only);
--
2.30.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 15/15] block: remove bdget_disk
2021-03-30 16:17 ` [PATCH 15/15] block: remove bdget_disk Christoph Hellwig
@ 2021-03-30 23:37 ` Chaitanya Kulkarni
0 siblings, 0 replies; 21+ messages in thread
From: Chaitanya Kulkarni @ 2021-03-30 23:37 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe, Song Liu
Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
Minchan Kim, Nitin Gupta, Stefan Haberland, Jan Hoeppner,
linux-block, linux-raid, linux-s390, linux-scsi
On 3/30/21 09:19, Christoph Hellwig wrote:
> Just opencode the xa_load in the callers, as none of them actually
> needs a reference to the bdev.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks good.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
^ permalink raw reply [flat|nested] 21+ messages in thread