Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1231804 > unrolled thread
| Started by | Grant Grundler <grundler@chromium.org> |
|---|---|
| First post | 2015-09-24 03:40 +0200 |
| Last post | 2015-10-05 13:00 +0200 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] mmc: block: copy resp[] data on err for MMC_IOC_MULTI_CMD Grant Grundler <grundler@chromium.org> - 2015-09-24 03:40 +0200
Re: [PATCH] mmc: block: copy resp[] data on err for MMC_IOC_MULTI_CMD Ulf Hansson <ulf.hansson@linaro.org> - 2015-10-05 13:00 +0200
| From | Grant Grundler <grundler@chromium.org> |
|---|---|
| Date | 2015-09-24 03:40 +0200 |
| Subject | [PATCH] mmc: block: copy resp[] data on err for MMC_IOC_MULTI_CMD |
| Message-ID | <qc3FT-57w-3@gated-at.bofh.it> |
MMC_IOC_CMD and MMC_IOC_MULTI_CMD ioctl() code currently bails on
any eMMC errors. However, in case there is any resp[] data, we
should attempt to copy resp[] back to user space. The user app
can then determine which command(s) failed in the MMC_IOC_MULTI_CMD
case AND/OR report better diagnostics in both cases.
Gwendal Grignou provided the idea and it was previously implemented
and tested on v3.18 ChromeOS kernel:
https://chromium-review.googlesource.com/#/c/299956
Signed-off-by: Grant Grundler <grundler@chromium.org>
Reviewed-by: Hyung Taek Ryoo <hryoo@nvidia.com>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
Tested-by: David Riley <davidriley@chromium.org>
---
I believe the patch applied correctly to Ulf's mmc.git on linaro.org.
I would have warm fuzzies if Jon or Hyung (Nvidia) could review/test this.
I made one change from the chromium.org version:
initialize err and ioc_err to zero in mmc_blk_ioctl_cmd()
drivers/mmc/card/block.c | 27 +++++++++------------------
1 file changed, 9 insertions(+), 18 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index f6acf0f..23b6c8e 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -595,7 +595,7 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
struct mmc_blk_ioc_data *idata;
struct mmc_blk_data *md;
struct mmc_card *card;
- int err;
+ int err = 0, ioc_err = 0;
idata = mmc_blk_ioctl_copy_from_user(ic_ptr);
if (IS_ERR(idata))
@@ -615,19 +615,18 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
mmc_get_card(card);
- err = __mmc_blk_ioctl_cmd(card, md, idata);
+ ioc_err = __mmc_blk_ioctl_cmd(card, md, idata);
mmc_put_card(card);
- if (!err)
- err = mmc_blk_ioctl_copy_to_user(ic_ptr, idata);
+ err = mmc_blk_ioctl_copy_to_user(ic_ptr, idata);
cmd_done:
mmc_blk_put(md);
cmd_err:
kfree(idata->buf);
kfree(idata);
- return err;
+ return ioc_err ? ioc_err : err;
}
static int mmc_blk_ioctl_multi_cmd(struct block_device *bdev,
@@ -637,7 +636,7 @@ static int mmc_blk_ioctl_multi_cmd(struct block_device *bdev,
struct mmc_ioc_cmd __user *cmds = user->cmds;
struct mmc_card *card;
struct mmc_blk_data *md;
- int i, err = -EFAULT;
+ int i, err = 0, ioc_err = 0;
__u64 num_of_cmds;
if (copy_from_user(&num_of_cmds, &user->num_of_cmds,
@@ -672,22 +671,14 @@ static int mmc_blk_ioctl_multi_cmd(struct block_device *bdev,
mmc_get_card(card);
- for (i = 0; i < num_of_cmds; i++) {
- err = __mmc_blk_ioctl_cmd(card, md, idata[i]);
- if (err) {
- mmc_put_card(card);
- goto cmd_done;
- }
- }
+ for (i = 0; i < num_of_cmds && !ioc_err; i++)
+ ioc_err = __mmc_blk_ioctl_cmd(card, md, idata[i]);
mmc_put_card(card);
/* copy to user if data and response */
- for (i = 0; i < num_of_cmds; i++) {
+ for (i = 0; i < num_of_cmds && !err; i++)
err = mmc_blk_ioctl_copy_to_user(&cmds[i], idata[i]);
- if (err)
- break;
- }
cmd_done:
mmc_blk_put(md);
@@ -697,7 +688,7 @@ cmd_err:
kfree(idata[i]);
}
kfree(idata);
- return err;
+ return ioc_err ? ioc_err : err;
}
static int mmc_blk_ioctl(struct block_device *bdev, fmode_t mode,
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Ulf Hansson <ulf.hansson@linaro.org> |
|---|---|
| Date | 2015-10-05 13:00 +0200 |
| Message-ID | <qgbES-56q-29@gated-at.bofh.it> |
| In reply to | #1231804 |
On 24 September 2015 at 03:30, Grant Grundler <grundler@chromium.org> wrote:
> MMC_IOC_CMD and MMC_IOC_MULTI_CMD ioctl() code currently bails on
> any eMMC errors. However, in case there is any resp[] data, we
> should attempt to copy resp[] back to user space. The user app
> can then determine which command(s) failed in the MMC_IOC_MULTI_CMD
> case AND/OR report better diagnostics in both cases.
>
> Gwendal Grignou provided the idea and it was previously implemented
> and tested on v3.18 ChromeOS kernel:
> https://chromium-review.googlesource.com/#/c/299956
>
> Signed-off-by: Grant Grundler <grundler@chromium.org>
> Reviewed-by: Hyung Taek Ryoo <hryoo@nvidia.com>
> Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
> Tested-by: David Riley <davidriley@chromium.org>
Thanks, applied for next!
Kind regards
Uffe
> ---
> I believe the patch applied correctly to Ulf's mmc.git on linaro.org.
> I would have warm fuzzies if Jon or Hyung (Nvidia) could review/test this.
>
> I made one change from the chromium.org version:
> initialize err and ioc_err to zero in mmc_blk_ioctl_cmd()
>
> drivers/mmc/card/block.c | 27 +++++++++------------------
> 1 file changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index f6acf0f..23b6c8e 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -595,7 +595,7 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
> struct mmc_blk_ioc_data *idata;
> struct mmc_blk_data *md;
> struct mmc_card *card;
> - int err;
> + int err = 0, ioc_err = 0;
>
> idata = mmc_blk_ioctl_copy_from_user(ic_ptr);
> if (IS_ERR(idata))
> @@ -615,19 +615,18 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
>
> mmc_get_card(card);
>
> - err = __mmc_blk_ioctl_cmd(card, md, idata);
> + ioc_err = __mmc_blk_ioctl_cmd(card, md, idata);
>
> mmc_put_card(card);
>
> - if (!err)
> - err = mmc_blk_ioctl_copy_to_user(ic_ptr, idata);
> + err = mmc_blk_ioctl_copy_to_user(ic_ptr, idata);
>
> cmd_done:
> mmc_blk_put(md);
> cmd_err:
> kfree(idata->buf);
> kfree(idata);
> - return err;
> + return ioc_err ? ioc_err : err;
> }
>
> static int mmc_blk_ioctl_multi_cmd(struct block_device *bdev,
> @@ -637,7 +636,7 @@ static int mmc_blk_ioctl_multi_cmd(struct block_device *bdev,
> struct mmc_ioc_cmd __user *cmds = user->cmds;
> struct mmc_card *card;
> struct mmc_blk_data *md;
> - int i, err = -EFAULT;
> + int i, err = 0, ioc_err = 0;
> __u64 num_of_cmds;
>
> if (copy_from_user(&num_of_cmds, &user->num_of_cmds,
> @@ -672,22 +671,14 @@ static int mmc_blk_ioctl_multi_cmd(struct block_device *bdev,
>
> mmc_get_card(card);
>
> - for (i = 0; i < num_of_cmds; i++) {
> - err = __mmc_blk_ioctl_cmd(card, md, idata[i]);
> - if (err) {
> - mmc_put_card(card);
> - goto cmd_done;
> - }
> - }
> + for (i = 0; i < num_of_cmds && !ioc_err; i++)
> + ioc_err = __mmc_blk_ioctl_cmd(card, md, idata[i]);
>
> mmc_put_card(card);
>
> /* copy to user if data and response */
> - for (i = 0; i < num_of_cmds; i++) {
> + for (i = 0; i < num_of_cmds && !err; i++)
> err = mmc_blk_ioctl_copy_to_user(&cmds[i], idata[i]);
> - if (err)
> - break;
> - }
>
> cmd_done:
> mmc_blk_put(md);
> @@ -697,7 +688,7 @@ cmd_err:
> kfree(idata[i]);
> }
> kfree(idata);
> - return err;
> + return ioc_err ? ioc_err : err;
> }
>
> static int mmc_blk_ioctl(struct block_device *bdev, fmode_t mode,
> --
> 2.6.0.rc2.230.g3dd15c0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web