| Message ID | 20180529135239.27005-11-hch@lst.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
[+ Ulf, Adrian, linux-mmc, as linux-mmc and folks are missing in the recipient list.] On 2018/5/29 21:52, Christoph Hellwig wrote: > By completing the request entirely in the driver we can remove the > BLK_EH_HANDLED return value and thus the split responsibility between the > driver and the block layer that has been causing trouble. > > [While this keeps existing behavior it seems to mismatch the comment, > maintainers please chime in!] The comment means the timeout doesn't happen for the current in-flight request which command queue is doing, and the request is finished previously(correct me if I'm wrong). So mmc defers the complete stuff to blk_timeout_work to help. Maybe the comment should gone as well. Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com> > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Hannes Reinecke <hare@suse.com> > --- > drivers/mmc/core/queue.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c > index 56e9a803db21..648eb6743ed5 100644 > --- a/drivers/mmc/core/queue.c > +++ b/drivers/mmc/core/queue.c > @@ -111,8 +111,9 @@ static enum blk_eh_timer_return mmc_cqe_timed_out(struct request *req) > __mmc_cqe_recovery_notifier(mq); > return BLK_EH_RESET_TIMER; > } > - /* No timeout */ > - return BLK_EH_HANDLED; > + /* No timeout (XXX: huh? comment doesn't make much sense) */ > + blk_mq_complete_request(req); > + return BLK_EH_DONE; > default: > /* Timeout is handled by mmc core */ > return BLK_EH_RESET_TIMER; >
On 29/05/18 17:55, Shawn Lin wrote: > [+ Ulf, Adrian, linux-mmc, as linux-mmc and folks are missing in the > recipient list.] > > On 2018/5/29 21:52, Christoph Hellwig wrote: >> By completing the request entirely in the driver we can remove the >> BLK_EH_HANDLED return value and thus the split responsibility between the >> driver and the block layer that has been causing trouble. >> >> [While this keeps existing behavior it seems to mismatch the comment, >> maintainers please chime in!] > > The comment means the timeout doesn't happen for the current in-flight > request which command queue is doing, and the request is finished > previously(correct me if I'm wrong). So mmc defers the complete stuff to > blk_timeout_work to help. Maybe the comment should gone as well. > > Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com> > >> >> Signed-off-by: Christoph Hellwig <hch@lst.de> >> Reviewed-by: Hannes Reinecke <hare@suse.com> >> --- >> drivers/mmc/core/queue.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c >> index 56e9a803db21..648eb6743ed5 100644 >> --- a/drivers/mmc/core/queue.c >> +++ b/drivers/mmc/core/queue.c >> @@ -111,8 +111,9 @@ static enum blk_eh_timer_return >> mmc_cqe_timed_out(struct request *req) >> __mmc_cqe_recovery_notifier(mq); >> return BLK_EH_RESET_TIMER; >> } >> - /* No timeout */ >> - return BLK_EH_HANDLED; >> + /* No timeout (XXX: huh? comment doesn't make much sense) */ >> + blk_mq_complete_request(req); It seems the block layer timeout handler no longer castrates the completion path, in which case blk_mq_complete_request() is not needed here. The "No timeout" comment can be removed. >> + return BLK_EH_DONE; >> default: >> /* Timeout is handled by mmc core */ >> return BLK_EH_RESET_TIMER; >> > >
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c index 56e9a803db21..648eb6743ed5 100644 --- a/drivers/mmc/core/queue.c +++ b/drivers/mmc/core/queue.c @@ -111,8 +111,9 @@ static enum blk_eh_timer_return mmc_cqe_timed_out(struct request *req) __mmc_cqe_recovery_notifier(mq); return BLK_EH_RESET_TIMER; } - /* No timeout */ - return BLK_EH_HANDLED; + /* No timeout (XXX: huh? comment doesn't make much sense) */ + blk_mq_complete_request(req); + return BLK_EH_DONE; default: /* Timeout is handled by mmc core */ return BLK_EH_RESET_TIMER;