On Thu, Apr 12, 2018 at 10:38:56AM +0800, jianchao.wang wrote:
> Hi Ming
> 
> On 04/12/2018 07:38 AM, Ming Lei wrote:
> > +    *
> > +    * Cover complete vs BLK_EH_RESET_TIMER race in slow path with
> > +    * helding queue lock.
> >      */
> >     hctx_lock(hctx, &srcu_idx);
> >     if (blk_mq_rq_aborted_gstate(rq) != rq->gstate)
> >             __blk_mq_complete_request(rq);
> > +   else {
> > +           unsigned long flags;
> > +           bool need_complete = false;
> > +
> > +           spin_lock_irqsave(q->queue_lock, flags);
> > +           if (!blk_mq_rq_aborted_gstate(rq))
> > +                   need_complete = true;
> > +           else
> > +                   blk_mq_rq_update_state(rq, MQ_RQ_COMPLETE_IN_TIMEOUT);
> > +           spin_unlock_irqrestore(q->queue_lock, flags);
> 
> What if the .timeout return BLK_EH_HANDLED during this ?
> timeout context                     irq context
>   .timeout()
>                                      blk_mq_complete_request
>                                        set state to MQ_RQ_COMPLETE_IN_TIMEOUT
>   __blk_mq_complete_request
>     WARN_ON_ONCE(blk_mq_rq_state(rq) 
>      != MQ_RQ_IN_FLIGHT);

Thinking of it further, if the normal completion from irq context can
happen after BLK_EH_HANDLED is returned from .timeout, this request may 
be recycled immediately after __blk_mq_complete_request(), then
blk_mq_complete_request() can complete the new instance early, so that
can be another race between old normal completion and new dispatch.

I guess .timeout should make sure this thing won't happen.

--
Ming

Reply via email to