* [PATCH 0/1] mmc: core: Further fix thread wake-up
@ 2016-12-19 13:57 Adrian Hunter
2016-12-19 13:57 ` [PATCH 1/1] " Adrian Hunter
0 siblings, 1 reply; 5+ messages in thread
From: Adrian Hunter @ 2016-12-19 13:57 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Linus Walleij, Ritesh Harjani
Hi
Here is the fix for the hang that Ritesh was seeing while testing
the Software Command Queue patches.
Adrian Hunter (1):
mmc: core: Further fix thread wake-up
drivers/mmc/core/core.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
Regards
Adrian
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/1] mmc: core: Further fix thread wake-up 2016-12-19 13:57 [PATCH 0/1] mmc: core: Further fix thread wake-up Adrian Hunter @ 2016-12-19 13:57 ` Adrian Hunter 2016-12-19 15:52 ` Ritesh Harjani 2016-12-20 10:56 ` Ulf Hansson 0 siblings, 2 replies; 5+ messages in thread From: Adrian Hunter @ 2016-12-19 13:57 UTC (permalink / raw) To: Ulf Hansson; +Cc: linux-mmc, Linus Walleij, Ritesh Harjani Commit e0097cf5f2f1 ("mmc: queue: Fix queue thread wake-up") did not go far enough. mmc_wait_for_data_req_done() still contains some problems and can be further simplified. First it should not touch context_info->is_waiting_last_req because that is a wake-up control used by the owner of the context. Secondly, it should always return when one of its wake-up conditions is met because, again, that is contolled by the owner of the context. While the current block driver does not have an issue, these problems were exposed during testing of the Software Command Queue patches. Fixes: e0097cf5f2f1 ("mmc: queue: Fix queue thread wake-up") Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- drivers/mmc/core/core.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 543eadd230e5..1076b9d89df3 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -496,8 +496,7 @@ static int __mmc_start_req(struct mmc_host *host, struct mmc_request *mrq) * Returns enum mmc_blk_status after checking errors. */ static enum mmc_blk_status mmc_wait_for_data_req_done(struct mmc_host *host, - struct mmc_request *mrq, - struct mmc_async_req *next_req) + struct mmc_request *mrq) { struct mmc_command *cmd; struct mmc_context_info *context_info = &host->context_info; @@ -507,7 +506,7 @@ static enum mmc_blk_status mmc_wait_for_data_req_done(struct mmc_host *host, wait_event_interruptible(context_info->wait, (context_info->is_done_rcv || context_info->is_new_req)); - context_info->is_waiting_last_req = false; + if (context_info->is_done_rcv) { context_info->is_done_rcv = false; cmd = mrq->cmd; @@ -527,10 +526,9 @@ static enum mmc_blk_status mmc_wait_for_data_req_done(struct mmc_host *host, __mmc_start_request(host, mrq); continue; /* wait for done/new event again */ } - } else if (context_info->is_new_req) { - if (!next_req) - return MMC_BLK_NEW_REQUEST; } + + return MMC_BLK_NEW_REQUEST; } mmc_retune_release(host); return status; @@ -660,7 +658,7 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host, mmc_pre_req(host, areq->mrq); if (host->areq) { - status = mmc_wait_for_data_req_done(host, host->areq->mrq, areq); + status = mmc_wait_for_data_req_done(host, host->areq->mrq); if (status == MMC_BLK_NEW_REQUEST) { if (ret_stat) *ret_stat = status; -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] mmc: core: Further fix thread wake-up 2016-12-19 13:57 ` [PATCH 1/1] " Adrian Hunter @ 2016-12-19 15:52 ` Ritesh Harjani 2016-12-20 10:56 ` Ulf Hansson 1 sibling, 0 replies; 5+ messages in thread From: Ritesh Harjani @ 2016-12-19 15:52 UTC (permalink / raw) To: Adrian Hunter, Ulf Hansson; +Cc: linux-mmc, Linus Walleij On 12/19/2016 7:27 PM, Adrian Hunter wrote: > Commit e0097cf5f2f1 ("mmc: queue: Fix queue thread wake-up") did not go far > enough. mmc_wait_for_data_req_done() still contains some problems and can > be further simplified. First it should not touch > context_info->is_waiting_last_req because that is a wake-up control used by > the owner of the context. Secondly, it should always return when one of its > wake-up conditions is met because, again, that is contolled by the owner of > the context. > > While the current block driver does not have an issue, these problems were > exposed during testing of the Software Command Queue patches. > > Fixes: e0097cf5f2f1 ("mmc: queue: Fix queue thread wake-up") > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Patch looks good to me. Please feel free to add. Tested-by: Harjani Ritesh <riteshh@codeaurora.org> Regards Ritesh -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] mmc: core: Further fix thread wake-up 2016-12-19 13:57 ` [PATCH 1/1] " Adrian Hunter 2016-12-19 15:52 ` Ritesh Harjani @ 2016-12-20 10:56 ` Ulf Hansson 2016-12-20 17:41 ` Bartlomiej Zolnierkiewicz 1 sibling, 1 reply; 5+ messages in thread From: Ulf Hansson @ 2016-12-20 10:56 UTC (permalink / raw) To: Adrian Hunter; +Cc: linux-mmc, Linus Walleij, Ritesh Harjani On 19 December 2016 at 14:57, Adrian Hunter <adrian.hunter@intel.com> wrote: > Commit e0097cf5f2f1 ("mmc: queue: Fix queue thread wake-up") did not go far > enough. mmc_wait_for_data_req_done() still contains some problems and can > be further simplified. First it should not touch > context_info->is_waiting_last_req because that is a wake-up control used by > the owner of the context. Secondly, it should always return when one of its > wake-up conditions is met because, again, that is contolled by the owner of > the context. > > While the current block driver does not have an issue, these problems were > exposed during testing of the Software Command Queue patches. > > Fixes: e0097cf5f2f1 ("mmc: queue: Fix queue thread wake-up") > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Thanks, applied for fixes! Kind regards Uffe > --- > drivers/mmc/core/core.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 543eadd230e5..1076b9d89df3 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -496,8 +496,7 @@ static int __mmc_start_req(struct mmc_host *host, struct mmc_request *mrq) > * Returns enum mmc_blk_status after checking errors. > */ > static enum mmc_blk_status mmc_wait_for_data_req_done(struct mmc_host *host, > - struct mmc_request *mrq, > - struct mmc_async_req *next_req) > + struct mmc_request *mrq) > { > struct mmc_command *cmd; > struct mmc_context_info *context_info = &host->context_info; > @@ -507,7 +506,7 @@ static enum mmc_blk_status mmc_wait_for_data_req_done(struct mmc_host *host, > wait_event_interruptible(context_info->wait, > (context_info->is_done_rcv || > context_info->is_new_req)); > - context_info->is_waiting_last_req = false; > + > if (context_info->is_done_rcv) { > context_info->is_done_rcv = false; > cmd = mrq->cmd; > @@ -527,10 +526,9 @@ static enum mmc_blk_status mmc_wait_for_data_req_done(struct mmc_host *host, > __mmc_start_request(host, mrq); > continue; /* wait for done/new event again */ > } > - } else if (context_info->is_new_req) { > - if (!next_req) > - return MMC_BLK_NEW_REQUEST; > } > + > + return MMC_BLK_NEW_REQUEST; > } > mmc_retune_release(host); > return status; > @@ -660,7 +658,7 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host, > mmc_pre_req(host, areq->mrq); > > if (host->areq) { > - status = mmc_wait_for_data_req_done(host, host->areq->mrq, areq); > + status = mmc_wait_for_data_req_done(host, host->areq->mrq); > if (status == MMC_BLK_NEW_REQUEST) { > if (ret_stat) > *ret_stat = status; > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] mmc: core: Further fix thread wake-up 2016-12-20 10:56 ` Ulf Hansson @ 2016-12-20 17:41 ` Bartlomiej Zolnierkiewicz 0 siblings, 0 replies; 5+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2016-12-20 17:41 UTC (permalink / raw) To: Ulf Hansson; +Cc: Adrian Hunter, linux-mmc, Linus Walleij, Ritesh Harjani Hi, On Tuesday, December 20, 2016 11:56:23 AM Ulf Hansson wrote: > On 19 December 2016 at 14:57, Adrian Hunter <adrian.hunter@intel.com> wrote: > > Commit e0097cf5f2f1 ("mmc: queue: Fix queue thread wake-up") did not go far > > enough. mmc_wait_for_data_req_done() still contains some problems and can > > be further simplified. First it should not touch > > context_info->is_waiting_last_req because that is a wake-up control used by > > the owner of the context. Secondly, it should always return when one of its > > wake-up conditions is met because, again, that is contolled by the owner of > > the context. > > > > While the current block driver does not have an issue, these problems were > > exposed during testing of the Software Command Queue patches. This sentence no longer seems to be true.. > > Fixes: e0097cf5f2f1 ("mmc: queue: Fix queue thread wake-up") > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > > Thanks, applied for fixes! Seems that I'm late but FWIW for the patch content: Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-20 17:41 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-19 13:57 [PATCH 0/1] mmc: core: Further fix thread wake-up Adrian Hunter 2016-12-19 13:57 ` [PATCH 1/1] " Adrian Hunter 2016-12-19 15:52 ` Ritesh Harjani 2016-12-20 10:56 ` Ulf Hansson 2016-12-20 17:41 ` Bartlomiej Zolnierkiewicz
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.