* refactor mddev_find_or_alloc
@ 2021-04-12 8:05 Christoph Hellwig
2021-04-12 8:05 ` [PATCH 1/3] md: factor out a mddev_alloc_unit helper from mddev_find Christoph Hellwig
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Christoph Hellwig @ 2021-04-12 8:05 UTC (permalink / raw)
To: song; +Cc: linux-raid
Hi Song,
this series refactor mddev_find_or_alloc so that is more easier to
read and groups related funtionality into separate helpers.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/3] md: factor out a mddev_alloc_unit helper from mddev_find
2021-04-12 8:05 refactor mddev_find_or_alloc Christoph Hellwig
@ 2021-04-12 8:05 ` Christoph Hellwig
2021-04-12 8:05 ` [PATCH 2/3] md: refactor mddev_find_or_alloc Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2021-04-12 8:05 UTC (permalink / raw)
To: song; +Cc: linux-raid
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 3ce5f4e0f43180..8ef06330fc66e4 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;
@@ -787,27 +808,13 @@ static struct mddev *mddev_find_or_alloc(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] 5+ messages in thread
* [PATCH 2/3] md: refactor mddev_find_or_alloc
2021-04-12 8:05 refactor mddev_find_or_alloc Christoph Hellwig
2021-04-12 8:05 ` [PATCH 1/3] md: factor out a mddev_alloc_unit helper from mddev_find Christoph Hellwig
@ 2021-04-12 8:05 ` Christoph Hellwig
2021-04-12 8:05 ` [PATCH 3/3] md: do not return existing mddevs from mddev_find_or_alloc Christoph Hellwig
2021-04-13 0:27 ` refactor mddev_find_or_alloc Song Liu
3 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2021-04-12 8:05 UTC (permalink / raw)
To: song; +Cc: linux-raid
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 8ef06330fc66e4..de6f8e511c14e7 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] 5+ messages in thread
* [PATCH 3/3] md: do not return existing mddevs from mddev_find_or_alloc
2021-04-12 8:05 refactor mddev_find_or_alloc Christoph Hellwig
2021-04-12 8:05 ` [PATCH 1/3] md: factor out a mddev_alloc_unit helper from mddev_find Christoph Hellwig
2021-04-12 8:05 ` [PATCH 2/3] md: refactor mddev_find_or_alloc Christoph Hellwig
@ 2021-04-12 8:05 ` Christoph Hellwig
2021-04-13 0:27 ` refactor mddev_find_or_alloc Song Liu
3 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2021-04-12 8:05 UTC (permalink / raw)
To: song; +Cc: linux-raid
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 de6f8e511c14e7..af9bdb907b2b47 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;
-
- if (!mddev)
- return -ENODEV;
+ int error ;
- 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] 5+ messages in thread
* Re: refactor mddev_find_or_alloc
2021-04-12 8:05 refactor mddev_find_or_alloc Christoph Hellwig
` (2 preceding siblings ...)
2021-04-12 8:05 ` [PATCH 3/3] md: do not return existing mddevs from mddev_find_or_alloc Christoph Hellwig
@ 2021-04-13 0:27 ` Song Liu
3 siblings, 0 replies; 5+ messages in thread
From: Song Liu @ 2021-04-13 0:27 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-raid
On Mon, Apr 12, 2021 at 1:05 AM Christoph Hellwig <hch@lst.de> wrote:
>
>
> Hi Song,
>
> this series refactor mddev_find_or_alloc so that is more easier to
> read and groups related funtionality into separate helpers.
Applied to md-next. Thanks!
Song
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-04-13 0:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-12 8:05 refactor mddev_find_or_alloc Christoph Hellwig
2021-04-12 8:05 ` [PATCH 1/3] md: factor out a mddev_alloc_unit helper from mddev_find Christoph Hellwig
2021-04-12 8:05 ` [PATCH 2/3] md: refactor mddev_find_or_alloc Christoph Hellwig
2021-04-12 8:05 ` [PATCH 3/3] md: do not return existing mddevs from mddev_find_or_alloc Christoph Hellwig
2021-04-13 0:27 ` refactor mddev_find_or_alloc Song Liu
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.