From: Christoph Hellwig <hch@infradead.org>
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: axboe@kernel.dk, hare@suse.de, bvanassche@acm.org,
ming.lei@redhat.com, hch@infradead.org, jack@suse.cz,
osandov@fb.com, linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org,
Ulf Hansson <ulf.hansson@linaro.org>,
linux-mmc@vger.kernel.org
Subject: Re: [PATCH 3/5] mmc/core/block: replace GENHD_FL_UP with GENHD_FL_DISK_ADDED
Date: Wed, 21 Jul 2021 06:23:59 +0100 [thread overview]
Message-ID: <YPevb5SpgBX6eWEZ@infradead.org> (raw)
In-Reply-To: <20210720182048.1906526-4-mcgrof@kernel.org>
On Tue, Jul 20, 2021 at 11:20:46AM -0700, Luis Chamberlain wrote:
> The GENHD_FL_DISK_ADDED flag is what we really want, as the
> flag GENHD_FL_UP could be set on a semi-initialized device.
>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
> drivers/mmc/core/block.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index ce8aed562929..e9818c79fa59 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2644,7 +2644,7 @@ static void mmc_blk_remove_req(struct mmc_blk_data *md)
> * from being accepted.
> */
> card = md->queue.card;
> - if (md->disk->flags & GENHD_FL_UP) {
> + if (blk_disk_added(md->disk)) {
> device_remove_file(disk_to_dev(md->disk), &md->force_ro);
> if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
> card->ext_csd.boot_ro_lockable)
> --
I think the proper fix here is to just unwind the mmc initialization,
something like this untested patch:
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 9890a1532cb0..982f0198d8ff 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2283,7 +2283,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
sector_t size,
bool default_ro,
const char *subname,
- int area_type)
+ int area_type,
+ unsigned int part_type)
{
struct mmc_blk_data *md;
int devidx, ret;
@@ -2329,6 +2330,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
INIT_LIST_HEAD(&md->rpmbs);
md->usage = 1;
md->queue.blkdata = md;
+ md->part_type = part_type;
md->disk->major = MMC_BLOCK_MAJOR;
md->disk->minors = perdev_minors;
@@ -2381,8 +2383,43 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
cap_str, md->read_only ? "(ro)" : "");
+ device_add_disk(md->parent, md->disk, NULL);
+ md->force_ro.show = force_ro_show;
+ md->force_ro.store = force_ro_store;
+ sysfs_attr_init(&md->force_ro.attr);
+ md->force_ro.attr.name = "force_ro";
+ md->force_ro.attr.mode = S_IRUGO | S_IWUSR;
+ ret = device_create_file(disk_to_dev(md->disk), &md->force_ro);
+ if (ret)
+ goto force_ro_fail;
+
+ if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
+ card->ext_csd.boot_ro_lockable) {
+ umode_t mode;
+
+ if (card->ext_csd.boot_ro_lock & EXT_CSD_BOOT_WP_B_PWR_WP_DIS)
+ mode = S_IRUGO;
+ else
+ mode = S_IRUGO | S_IWUSR;
+
+ md->power_ro_lock.show = power_ro_lock_show;
+ md->power_ro_lock.store = power_ro_lock_store;
+ sysfs_attr_init(&md->power_ro_lock.attr);
+ md->power_ro_lock.attr.mode = mode;
+ md->power_ro_lock.attr.name =
+ "ro_lock_until_next_power_on";
+ ret = device_create_file(disk_to_dev(md->disk),
+ &md->power_ro_lock);
+ if (ret)
+ goto power_ro_lock_fail;
+ }
return md;
+ power_ro_lock_fail:
+ device_remove_file(disk_to_dev(md->disk), &md->force_ro);
+ force_ro_fail:
+ del_gendisk(md->disk);
+ // XXX: undo mmc_init_queue
err_kfree:
kfree(md);
out:
@@ -2410,7 +2447,7 @@ static struct mmc_blk_data *mmc_blk_alloc(struct mmc_card *card)
}
return mmc_blk_alloc_req(card, &card->dev, size, false, NULL,
- MMC_BLK_DATA_AREA_MAIN);
+ MMC_BLK_DATA_AREA_MAIN, 0);
}
static int mmc_blk_alloc_part(struct mmc_card *card,
@@ -2424,10 +2461,9 @@ static int mmc_blk_alloc_part(struct mmc_card *card,
struct mmc_blk_data *part_md;
part_md = mmc_blk_alloc_req(card, disk_to_dev(md->disk), size, default_ro,
- subname, area_type);
+ subname, area_type, part_type);
if (IS_ERR(part_md))
return PTR_ERR(part_md);
- part_md->part_type = part_type;
list_add(&part_md->part, &md->part);
return 0;
@@ -2628,27 +2664,23 @@ static int mmc_blk_alloc_parts(struct mmc_card *card, struct mmc_blk_data *md)
static void mmc_blk_remove_req(struct mmc_blk_data *md)
{
- struct mmc_card *card;
+ struct mmc_card *card = md->queue.card;
- if (md) {
- /*
- * Flush remaining requests and free queues. It
- * is freeing the queue that stops new requests
- * from being accepted.
- */
- card = md->queue.card;
- if (md->disk->flags & GENHD_FL_UP) {
- device_remove_file(disk_to_dev(md->disk), &md->force_ro);
- if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
- card->ext_csd.boot_ro_lockable)
- device_remove_file(disk_to_dev(md->disk),
- &md->power_ro_lock);
-
- del_gendisk(md->disk);
- }
- mmc_cleanup_queue(&md->queue);
- mmc_blk_put(md);
+ /*
+ * Flush remaining requests and free queues. It is freeing the queue
+ * that stops new requests from being accepted.
+ */
+ if (md->disk->flags & GENHD_FL_UP) {
+ device_remove_file(disk_to_dev(md->disk), &md->force_ro);
+ if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
+ card->ext_csd.boot_ro_lockable)
+ device_remove_file(disk_to_dev(md->disk),
+ &md->power_ro_lock);
+
+ del_gendisk(md->disk);
}
+ mmc_cleanup_queue(&md->queue);
+ mmc_blk_put(md);
}
static void mmc_blk_remove_parts(struct mmc_card *card,
@@ -2672,51 +2704,6 @@ static void mmc_blk_remove_parts(struct mmc_card *card,
}
}
-static int mmc_add_disk(struct mmc_blk_data *md)
-{
- int ret;
- struct mmc_card *card = md->queue.card;
-
- device_add_disk(md->parent, md->disk, NULL);
- md->force_ro.show = force_ro_show;
- md->force_ro.store = force_ro_store;
- sysfs_attr_init(&md->force_ro.attr);
- md->force_ro.attr.name = "force_ro";
- md->force_ro.attr.mode = S_IRUGO | S_IWUSR;
- ret = device_create_file(disk_to_dev(md->disk), &md->force_ro);
- if (ret)
- goto force_ro_fail;
-
- if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
- card->ext_csd.boot_ro_lockable) {
- umode_t mode;
-
- if (card->ext_csd.boot_ro_lock & EXT_CSD_BOOT_WP_B_PWR_WP_DIS)
- mode = S_IRUGO;
- else
- mode = S_IRUGO | S_IWUSR;
-
- md->power_ro_lock.show = power_ro_lock_show;
- md->power_ro_lock.store = power_ro_lock_store;
- sysfs_attr_init(&md->power_ro_lock.attr);
- md->power_ro_lock.attr.mode = mode;
- md->power_ro_lock.attr.name =
- "ro_lock_until_next_power_on";
- ret = device_create_file(disk_to_dev(md->disk),
- &md->power_ro_lock);
- if (ret)
- goto power_ro_lock_fail;
- }
- return ret;
-
-power_ro_lock_fail:
- device_remove_file(disk_to_dev(md->disk), &md->force_ro);
-force_ro_fail:
- del_gendisk(md->disk);
-
- return ret;
-}
-
#ifdef CONFIG_DEBUG_FS
static int mmc_dbg_card_status_get(void *data, u64 *val)
@@ -2882,7 +2869,7 @@ static void mmc_blk_remove_debugfs(struct mmc_card *card,
static int mmc_blk_probe(struct mmc_card *card)
{
- struct mmc_blk_data *md, *part_md;
+ struct mmc_blk_data *md;
int ret = 0;
/*
@@ -2900,6 +2887,8 @@ static int mmc_blk_probe(struct mmc_card *card)
return -ENOMEM;
}
+ dev_set_drvdata(&card->dev, md);
+
md = mmc_blk_alloc(card);
if (IS_ERR(md)) {
ret = PTR_ERR(md);
@@ -2910,18 +2899,6 @@ static int mmc_blk_probe(struct mmc_card *card)
if (ret)
goto out;
- dev_set_drvdata(&card->dev, md);
-
- ret = mmc_add_disk(md);
- if (ret)
- goto out;
-
- list_for_each_entry(part_md, &md->part, part) {
- ret = mmc_add_disk(part_md);
- if (ret)
- goto out;
- }
-
/* Add two debugfs entries */
mmc_blk_add_debugfs(card, md);
next prev parent reply other threads:[~2021-07-21 5:24 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-20 18:20 [PATCH 0/5] block: replace incorrect uses of GENHD_FL_UP Luis Chamberlain
2021-07-20 18:20 ` [PATCH 1/5] block: add flag for add_disk() completion notation Luis Chamberlain
2021-07-21 4:59 ` Christoph Hellwig
2021-07-20 18:20 ` [PATCH 2/5] md: replace GENHD_FL_UP with GENHD_FL_DISK_ADDED on is_mddev_broken() Luis Chamberlain
2021-07-21 5:03 ` Christoph Hellwig
2021-07-20 18:20 ` [PATCH 3/5] mmc/core/block: replace GENHD_FL_UP with GENHD_FL_DISK_ADDED Luis Chamberlain
2021-07-21 5:23 ` Christoph Hellwig [this message]
2021-07-20 18:20 ` [PATCH 4/5] nvme: " Luis Chamberlain
2021-07-21 5:31 ` Christoph Hellwig
2021-07-21 5:31 ` Christoph Hellwig
2021-07-20 18:20 ` [PATCH 5/5] fs/block_dev: " Luis Chamberlain
2021-07-21 5:25 ` Christoph Hellwig
2021-08-11 2:42 ` [PATCH 0/5] block: replace incorrect uses of GENHD_FL_UP luomeng
2021-08-11 5:19 ` Christoph Hellwig
2021-08-12 2:07 ` luomeng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YPevb5SpgBX6eWEZ@infradead.org \
--to=hch@infradead.org \
--cc=axboe@kernel.dk \
--cc=bvanassche@acm.org \
--cc=hare@suse.de \
--cc=jack@suse.cz \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=ming.lei@redhat.com \
--cc=osandov@fb.com \
--cc=ulf.hansson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.