* [PATCH 0/3] mmc: block: Cleanup mmc_blk_probe() path
@ 2021-03-03 12:20 Ulf Hansson
2021-03-03 12:20 ` [PATCH 1/3] mmc: block: Drop use of unlikely() in mmc_blk_probe() Ulf Hansson
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Ulf Hansson @ 2021-03-03 12:20 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson; +Cc: Adrian Hunter, linux-kernel
While working on fixing KASAN splats for the mmc block device driver, I stumbled
over a couple of annoying things in the mmc_blk_probe() path. This series takes
care of them.
Ulf Hansson (3):
mmc: block: Drop use of unlikely() in mmc_blk_probe()
mmc: block: Simplify logging during probe about added partitions
mmc: block: Fix error path in mmc_blk_probe()
drivers/mmc/core/block.c | 49 ++++++++++++++++++++--------------------
1 file changed, 25 insertions(+), 24 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/3] mmc: block: Drop use of unlikely() in mmc_blk_probe()
2021-03-03 12:20 [PATCH 0/3] mmc: block: Cleanup mmc_blk_probe() path Ulf Hansson
@ 2021-03-03 12:20 ` Ulf Hansson
2021-03-03 12:20 ` [PATCH 2/3] mmc: block: Simplify logging during probe about added partitions Ulf Hansson
2021-03-03 12:20 ` [PATCH 3/3] mmc: block: Fix error path in mmc_blk_probe() Ulf Hansson
2 siblings, 0 replies; 4+ messages in thread
From: Ulf Hansson @ 2021-03-03 12:20 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson; +Cc: Adrian Hunter, linux-kernel
mmc_blk_probe() isn't a hotpath, which makes it's questionable to use
unlikely(). Therefore let's simply drop it.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.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 8e6a623b35de..dc6b2e8f4f95 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2888,7 +2888,7 @@ static int mmc_blk_probe(struct mmc_card *card)
card->complete_wq = alloc_workqueue("mmc_complete",
WQ_MEM_RECLAIM | WQ_HIGHPRI, 0);
- if (unlikely(!card->complete_wq)) {
+ if (!card->complete_wq) {
pr_err("Failed to create mmc completion workqueue");
return -ENOMEM;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/3] mmc: block: Simplify logging during probe about added partitions
2021-03-03 12:20 [PATCH 0/3] mmc: block: Cleanup mmc_blk_probe() path Ulf Hansson
2021-03-03 12:20 ` [PATCH 1/3] mmc: block: Drop use of unlikely() in mmc_blk_probe() Ulf Hansson
@ 2021-03-03 12:20 ` Ulf Hansson
2021-03-03 12:20 ` [PATCH 3/3] mmc: block: Fix error path in mmc_blk_probe() Ulf Hansson
2 siblings, 0 replies; 4+ messages in thread
From: Ulf Hansson @ 2021-03-03 12:20 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson; +Cc: Adrian Hunter, linux-kernel
To simplify the code, move the logging into the common mmc_blk_alloc_req()
and drop the rather useless information about the partition type/id.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/block.c | 25 +++++++++----------------
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index dc6b2e8f4f95..39308b35a1fb 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2261,6 +2261,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
{
struct mmc_blk_data *md;
int devidx, ret;
+ char cap_str[10];
devidx = ida_simple_get(&mmc_blk_ida, 0, max_devices, GFP_KERNEL);
if (devidx < 0) {
@@ -2365,6 +2366,12 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
blk_queue_write_cache(md->queue.queue, true, true);
}
+ string_get_size((u64)size, 512, STRING_UNITS_2,
+ cap_str, sizeof(cap_str));
+ pr_info("%s: %s %s %s %s\n",
+ md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
+ cap_str, md->read_only ? "(ro)" : "");
+
return md;
err_putdisk:
@@ -2407,7 +2414,6 @@ static int mmc_blk_alloc_part(struct mmc_card *card,
const char *subname,
int area_type)
{
- char cap_str[10];
struct mmc_blk_data *part_md;
part_md = mmc_blk_alloc_req(card, disk_to_dev(md->disk), size, default_ro,
@@ -2417,11 +2423,6 @@ static int mmc_blk_alloc_part(struct mmc_card *card,
part_md->part_type = part_type;
list_add(&part_md->part, &md->part);
- string_get_size((u64)get_capacity(part_md->disk), 512, STRING_UNITS_2,
- cap_str, sizeof(cap_str));
- pr_info("%s: %s %s partition %u %s\n",
- part_md->disk->disk_name, mmc_card_id(card),
- mmc_card_name(card), part_md->part_type, cap_str);
return 0;
}
@@ -2558,9 +2559,8 @@ static int mmc_blk_alloc_rpmb_part(struct mmc_card *card,
string_get_size((u64)size, 512, STRING_UNITS_2,
cap_str, sizeof(cap_str));
- pr_info("%s: %s %s partition %u %s, chardev (%d:%d)\n",
- rpmb_name, mmc_card_id(card),
- mmc_card_name(card), EXT_CSD_PART_CONFIG_ACC_RPMB, cap_str,
+ pr_info("%s: %s %s %s, chardev (%d:%d)\n",
+ rpmb_name, mmc_card_id(card), mmc_card_name(card), cap_str,
MAJOR(mmc_rpmb_devt), rpmb->id);
return 0;
@@ -2876,7 +2876,6 @@ 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;
- char cap_str[10];
/*
* Check that the card supports the command class(es) we need.
@@ -2897,12 +2896,6 @@ static int mmc_blk_probe(struct mmc_card *card)
if (IS_ERR(md))
return PTR_ERR(md);
- string_get_size((u64)get_capacity(md->disk), 512, STRING_UNITS_2,
- cap_str, sizeof(cap_str));
- pr_info("%s: %s %s %s %s\n",
- md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
- cap_str, md->read_only ? "(ro)" : "");
-
if (mmc_blk_alloc_parts(card, md))
goto out;
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] mmc: block: Fix error path in mmc_blk_probe()
2021-03-03 12:20 [PATCH 0/3] mmc: block: Cleanup mmc_blk_probe() path Ulf Hansson
2021-03-03 12:20 ` [PATCH 1/3] mmc: block: Drop use of unlikely() in mmc_blk_probe() Ulf Hansson
2021-03-03 12:20 ` [PATCH 2/3] mmc: block: Simplify logging during probe about added partitions Ulf Hansson
@ 2021-03-03 12:20 ` Ulf Hansson
2 siblings, 0 replies; 4+ messages in thread
From: Ulf Hansson @ 2021-03-03 12:20 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson; +Cc: Adrian Hunter, linux-kernel
Returning zero to indicate success, when we actually have failed to probe
is wrong. As a matter of fact, it leads to that mmc_blk_remove() gets
called at a card removal and then triggers "NULL pointer dereference"
splats. This is because mmc_blk_remove() relies on data structures and
pointers to be setup from mmc_blk_probe(), of course.
There have been no errors reported about this, which is most likely because
mmc_blk_probe() never fails like this. Nevertheless, let's fix the code by
propagating the error codes correctly and prevent us from leaking memory by
calling also destroy_workqueue() in the error path.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/block.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 39308b35a1fb..02b656305042 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2876,6 +2876,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;
+ int ret = 0;
/*
* Check that the card supports the command class(es) we need.
@@ -2893,19 +2894,24 @@ static int mmc_blk_probe(struct mmc_card *card)
}
md = mmc_blk_alloc(card);
- if (IS_ERR(md))
- return PTR_ERR(md);
+ if (IS_ERR(md)) {
+ ret = PTR_ERR(md);
+ goto out_free;
+ }
- if (mmc_blk_alloc_parts(card, md))
+ ret = mmc_blk_alloc_parts(card, md);
+ if (ret)
goto out;
dev_set_drvdata(&card->dev, md);
- if (mmc_add_disk(md))
+ ret = mmc_add_disk(md);
+ if (ret)
goto out;
list_for_each_entry(part_md, &md->part, part) {
- if (mmc_add_disk(part_md))
+ ret = mmc_add_disk(part_md);
+ if (ret)
goto out;
}
@@ -2926,10 +2932,12 @@ static int mmc_blk_probe(struct mmc_card *card)
return 0;
- out:
+out:
mmc_blk_remove_parts(card, md);
mmc_blk_remove_req(md);
- return 0;
+out_free:
+ destroy_workqueue(card->complete_wq);
+ return ret;
}
static void mmc_blk_remove(struct mmc_card *card)
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-03 17:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-03 12:20 [PATCH 0/3] mmc: block: Cleanup mmc_blk_probe() path Ulf Hansson
2021-03-03 12:20 ` [PATCH 1/3] mmc: block: Drop use of unlikely() in mmc_blk_probe() Ulf Hansson
2021-03-03 12:20 ` [PATCH 2/3] mmc: block: Simplify logging during probe about added partitions Ulf Hansson
2021-03-03 12:20 ` [PATCH 3/3] mmc: block: Fix error path in mmc_blk_probe() Ulf Hansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).