Message ID | 1480068442-5169-17-git-send-email-adrian.hunter@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Fri, Nov 25, 2016 at 11:07 AM, Adrian Hunter <adrian.hunter@intel.com> wrote: > The 4K native sector check does not allow for the 'do' loop nor the > variables used after the 'cmd_abort' label. > > 'brq' and 'req' get reassigned in the 'do' loop, so the check must not > assume what their values are. After the 'cmd_abort' label, 'mq_rq' and > 'req' are used, but 'rqc' must be NULL otherwise it can be started again. > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Looks correct, and is clearly a sign of what we all know: mmc_blk_issue_rw_rq() is hopelessly convoluted and needs to be refactored into something we can read. Shouldn't this patch just be moved to the front of the patch queue and merged as a fix? AFAICT it's a plain bug. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c index 35e8d01b3013..f9d2bcf998fb 100644 --- a/drivers/mmc/card/block.c +++ b/drivers/mmc/card/block.c @@ -2035,11 +2035,11 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc) { struct mmc_blk_data *md = mq->blkdata; struct mmc_card *card = md->queue.card; - struct mmc_blk_request *brq = &mq->mqrq_cur->brq; + struct mmc_blk_request *brq; int ret = 1, disable_multi = 0, retry = 0, type, retune_retry_done = 0; enum mmc_blk_status status; struct mmc_queue_req *mq_rq; - struct request *req = rqc; + struct request *req; struct mmc_async_req *areq; const u8 packed_nr = 2; u8 reqs = 0; @@ -2059,8 +2059,10 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc) if (mmc_large_sector(card) && !IS_ALIGNED(blk_rq_sectors(rqc), 8)) { pr_err("%s: Transfer size is not 4KB sector size aligned\n", - req->rq_disk->disk_name); + rqc->rq_disk->disk_name); mq_rq = mq->mqrq_cur; + req = rqc; + rqc = NULL; goto cmd_abort; }
The 4K native sector check does not allow for the 'do' loop nor the variables used after the 'cmd_abort' label. 'brq' and 'req' get reassigned in the 'do' loop, so the check must not assume what their values are. After the 'cmd_abort' label, 'mq_rq' and 'req' are used, but 'rqc' must be NULL otherwise it can be started again. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- drivers/mmc/card/block.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)