On 4/19/18 10:43 AM, Bart Van Assche wrote: > The blk-mq timeout handling code ignores completions that occur after > blk_mq_check_expired() has been called and before blk_mq_rq_timed_out() > has reset rq->aborted_gstate. If a block driver timeout handler always > returns BLK_EH_RESET_TIMER then the result will be that the request > never terminates. > > Fix this race as follows: > - Use the deadline instead of the request generation to detect whether > or not a request timer fired after reinitialization of a request. > - Store the request state in the lowest two bits of the deadline instead > of the lowest two bits of 'gstate'. > - Rename MQ_RQ_STATE_MASK into RQ_STATE_MASK and change it from an > enumeration member into a #define such that its type can be changed > into unsigned long. That allows to write & ~RQ_STATE_MASK instead of > ~(unsigned long)RQ_STATE_MASK. > - Remove all request member variables that became superfluous due to > this change: gstate, gstate_seq and aborted_gstate_sync. > - Remove the request state information that became superfluous due to this > patch, namely RQF_MQ_TIMEOUT_EXPIRED. > - Remove the code that became superfluous due to this change, namely > the RCU lock and unlock statements in blk_mq_complete_request() and > also the synchronize_rcu() call in the timeout handler.
I like this approach, it's simpler and easier to understand. Net reduction in code is nice too. I've run it through the testing and it passes. Unless people holler loudly, I'd like to include this in 4.17. -- Jens Axboe
