Re: [PATCH 1/2] block: get rid of BLK_MAX_TIMEOUT

[Bart Van Assche <bvanassche@acm.org>]

On Thu, 2018-12-06 at 22:18 +0800, Weiping Zhang wrote:
> Before this patch, even we set io_timeout to 30*HZ(default), but
> blk_rq_timeout always return jiffies +5*HZ,
>   [1]. if there no pending request in timeout list, the timer callback
> blk_rq_timed_out_timer will be called after 5*HZ, and then
> blk_mq_check_expired will check is there exist some request
> was delayed by compare jiffies and request->deadline, obvious
> request is not timeout because we set request's timeouts is 30*HZ.
> So for this case timer callback should be called at jiffies + 30 instead
> of jiffies + 5*HZ.
> 
>   [2]. if there are pending request in timeout list, we compare request's
> expiry and queue's expiry. If time_after(request->expire, queue->expire) modify
> queue->timeout.expire to request->expire, otherwise do nothing.
> 
> So I think this patch just solve problem in [1], no other regression, or what's
> I missing here ?

The blk_rq_timeout() function was introduced by commit 0d2602ca30e4 ("blk-mq:
improve support for shared tags maps"). I think the purpose of that function
is to make sure that the nr_active counter in struct blk_mq_hw_ctx gets updated
at least once every five seconds. So there are two problems with this patch:
- It reduces the frequency of 'nr_active' updates. I think that is wrong and
  also that it will negatively affect drivers that rely on this functionality,
  e.g. the SCSI core.
- The patch description does not match the code changes in this patch.

Bart.