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

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.