On Tue, Apr 10, 2018 at 02:09:33PM +0000, Bart Van Assche wrote:
On Tue, 2018-04-10 at 21:55 +0800, Ming Lei wrote:
Then I have same question with Jianchao, what is the actual double complete in linus tree between BLK_EH_RESET_TIMER and normal completion?
Follows my understanding:
- when timeout is detected on one request, its aborted_gstate is
updated in blk_mq_check_expired().
run synchronize_rcu(), and make sure all pending completion is done
run blk_mq_rq_timed_out()
- ret = ops->timeout
- blk_mq_rq_update_aborted_gstate(req, 0)
- blk_add_timer(req);
If normal completion is done between 1) and reset aborted_gstate in 3), blk_mq_complete_request() will be called, and found that aborted_gstate is set, then the rq won't be completed really.
If normal completion is done after reset aborted_gstate in 3), it should be same with applying this patch.
Hello Ming,
Please keep in mind that all synchronize_rcu() does is to wait for pre- existing RCU readers to finish. synchronize_rcu() does not prevent that new rcu_read_lock() calls happen. It is e.g. possible that after
That is right, and I also mentioned normal completion can be done between 1) and reset aborted_gstate in 3).
blk_mq_rq_update_aborted_gstate(req, 0) has been executed that a regular completion occurs. If that request is not reused before the timer that was restarted by the timeout code expires, that request will be completed twice.
In this patch, blk_mq_add_timer(req, MQ_RQ_COMPLETE, MQ_RQ_IN_FLIGHT) is called for handling BLK_EH_RESET_TIMER. And after rq's state is changed to MQ_RQ_IN_FLIGHT, normal completion still can come and complete this rq, just like the above you described, right?
Thanks, Ming