On Tue, May 22, 2018 at 10:49:11AM +0800, Ming Lei wrote: > On Mon, May 21, 2018 at 05:11:31PM -0600, Keith Busch wrote: > > -static void blk_mq_terminate_expired(struct blk_mq_hw_ctx *hctx, > > +static void blk_mq_check_expired(struct blk_mq_hw_ctx *hctx, > > struct request *rq, void *priv, bool reserved) > > { > > + unsigned long *next = priv; > > + > > /* > > - * We marked @rq->aborted_gstate and waited for RCU. If there were > > - * completions that we lost to, they would have finished and > > - * updated @rq->gstate by now; otherwise, the completion path is > > - * now guaranteed to see @rq->aborted_gstate and yield. If > > - * @rq->aborted_gstate still matches @rq->gstate, @rq is ours. > > + * Just do a quick check if it is expired before locking the request in > > + * so we're not unnecessarilly synchronizing across CPUs. > > */ > > - if (!(rq->rq_flags & RQF_MQ_TIMEOUT_EXPIRED) && > > - READ_ONCE(rq->gstate) == rq->aborted_gstate) > > + if (!blk_mq_req_expired(rq, next)) > > + return; > > + > > + /* > > + * We have reason to believe the request may be expired. Take a > > + * reference on the request to lock this request lifetime into its > > + * currently allocated context to prevent it from being reallocated in > > + * the event the completion by-passes this timeout handler. > > + * > > + * If the reference was already released, then the driver beat the > > + * timeout handler to posting a natural completion. > > + */ > > + if (!kref_get_unless_zero(&rq->ref)) > > + return; > > If this request is just completed in normal path and its state isn't > updated yet, timeout will hold the request, and may complete this > request again, then this req can be completed two times. Hi Ming, In the event the driver requests a normal completion, the timeout work releasing the last reference doesn't do a second completion: it only releases the request's tag back for re-allocation. Thanks, Keith