Messages in this thread | | | Date | Fri, 25 Oct 2013 17:40:32 -0700 | From | "Ray Jui" <> | Subject | Re: [PATCH] mmc: fix host release issue after discard operation |
| |
On 10/25/2013 3:42 AM, Seungwon Jeon wrote: > Hi Ray, > > On Fri, October 25, 2013, Ray Jui wrote: >> Under function mmc_blk_issue_rq, after an MMC discard operation, >> the MMC request data structure may be freed in memory. Later in >> the same function, the check of req->cmd_flags & MMC_REQ_SPECIAL_MASK >> is dangerous and invalid. It causes the MMC host not being released >> when it should >> >> This patch fixes the issue by marking the special request down before >> the discard/flush operation >> >> Reported by: Harold (SoonYeal) Yang <haroldsy@broadcom.com> >> Signed-off-by: Ray Jui <rjui@broadcom.com> >> --- >> drivers/mmc/card/block.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c >> index 1a3163f..9e8e970 100644 >> --- a/drivers/mmc/card/block.c >> +++ b/drivers/mmc/card/block.c >> @@ -1954,7 +1954,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc) >> >> static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req) >> { >> - int ret; >> + int ret, req_is_special = 0; > It would be better to use boolean? > > Either, in a different way, I considered to save the cmd_flags like below. > unsigned int cmd_flags = req ? req->cmd_flags : 0; > > Then, we could replace several 'if statement' without dereference. > if (req && req->cmd_flags & REQ_DISCARD) -> if (cmd_flags & REQ_DISCARD) > > Just my suggestion. > > Thanks, > Seungwon Jeon >> struct mmc_blk_data *md = mq->data; >> struct mmc_card *card = md->queue.card; >> struct mmc_host *host = card->host; >> @@ -1973,6 +1973,10 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req) >> goto out; >> } >> >> + /* mark special request here since req may be freed later */ >> + if (req && (req->cmd_flags & MMC_REQ_SPECIAL_MASK)) >> + req_is_special = 1; >> + >> mq->flags &= ~MMC_QUEUE_NEW_REQUEST; >> if (req && req->cmd_flags & REQ_DISCARD) { >> /* complete ongoing async transfer before issuing discard */ >> @@ -1999,7 +2003,7 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req) >> >> out: >> if ((!req && !(mq->flags & MMC_QUEUE_NEW_REQUEST)) || >> - (req && (req->cmd_flags & MMC_REQ_SPECIAL_MASK))) >> + req_is_special) >> /* >> * Release host when there are no more requests >> * and after special request(discard, flush) is done. >> -- >> 1.7.9.5 > > Hi Seungwon,
I like your suggestion. It makes the code simpler. I'll make the change and submit patch v2.
Thanks,
Ray Jui
| |