On 4/10/18 3:55 AM, Christoph Hellwig wrote:
On Mon, Apr 09, 2018 at 06:34:55PM -0700, Bart Van Assche wrote:
If a completion occurs after blk_mq_rq_timed_out() has reset rq->aborted_gstate and the request is again in flight when the timeout expires then a request will be completed twice: a first time by the timeout handler and a second time when the regular completion occurs.
Additionally, 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.
Since the request state can be updated from two different contexts, namely regular completion and request timeout, this race cannot be fixed with RCU synchronization only. Fix this race as follows:
- Split __deadline in two variables, namely lq_deadline for legacy request queues and mq_deadline for blk-mq request queues. Use atomic operations to update mq_deadline.
I don't think we need the atomic_long_cmpxchg, and can do with a plain cmpxhg. Also unterminated cmpxchg loops are a bad idea, but I think both callers are protected from other changes so we can turn that into a WARN_ON(). That plus some minor cleanups in the version below, only boot tested:
This version looks reasonable, let me throw it through the testing machinery.