Re: [PATCH v5] blk-mq: Avoid that a completion can be ignored for BLK_EH_RESET_TIMER

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2018-04-11 at 17:32 +0300, Sagi Grimberg wrote:
> > > >    static void __blk_mq_requeue_request(struct request *rq)
> > > >    {
> > > >    	struct request_queue *q = rq->q;
> > > > +	enum mq_rq_state old_state = blk_mq_rq_state(rq);
> > > >    
> > > >    	blk_mq_put_driver_tag(rq);
> > > >    
> > > >    	trace_block_rq_requeue(q, rq);
> > > >    	wbt_requeue(q->rq_wb, &rq->issue_stat);
> > > >    
> > > > -	if (blk_mq_rq_state(rq) != MQ_RQ_IDLE) {
> > > > -		blk_mq_rq_update_state(rq, MQ_RQ_IDLE);
> > > > +	if (old_state != MQ_RQ_IDLE) {
> > > > +		if (!blk_mq_change_rq_state(rq, old_state, MQ_RQ_IDLE))
> > > > +			WARN_ON_ONCE(true);
> > > >    		if (q->dma_drain_size && blk_rq_bytes(rq))
> > > >    			rq->nr_phys_segments--;
> > > >    	}
> > > 
> > > Can you explain why was old_state kept as a local variable?
> > 
> > Hello Sagi,
> > 
> > Since blk_mq_requeue_request() must be called after a request has completed
> > the timeout handler will ignore requests that are being requeued. Hence it
> > is safe in this function to cache the request state in a local variable.
> 
> I understand why it is safe, I just didn't understand why it is needed.

The only reason is that it keeps the line with the blk_mq_change_rq_state() call
below the 80 character limit :-)

Bart.







[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]