On Tue, Oct 25, 2022 at 09:11:37PM -0600, Keith Busch wrote:
> On Wed, Oct 26, 2022 at 09:55:21AM +0800, Ming Lei wrote:
> > @@ -1564,8 +1571,13 @@ static bool blk_mq_check_expired(struct request *rq, 
> > void *priv)
> >      * it was completed and reallocated as a new request after returning
> >      * from blk_mq_check_expired().
> >      */
> > -   if (blk_mq_req_expired(rq, next))
> > +   if (blk_mq_req_expired(rq, expired)) {
> > +           if (expired->check_only) {
> > +                   expired->has_timedout_rq = true;
> > +                   return false;
> > +           }
> >             blk_mq_rq_timed_out(rq);
> > +   }
> >     return true;
> >  }
> >  
> > @@ -1573,7 +1585,10 @@ static void blk_mq_timeout_work(struct work_struct 
> > *work)
> >  {
> >     struct request_queue *q =
> >             container_of(work, struct request_queue, timeout_work);
> > -   unsigned long next = 0;
> > +   struct blk_expired_data expired = {
> > +           .check_only = true,
> > +           .timeout_start = jiffies,
> > +   };
> >     struct blk_mq_hw_ctx *hctx;
> >     unsigned long i;
> >  
> > @@ -1593,10 +1608,24 @@ static void blk_mq_timeout_work(struct work_struct 
> > *work)
> >     if (!percpu_ref_tryget(&q->q_usage_counter))
> >             return;
> >  
> > -   blk_mq_queue_tag_busy_iter(q, blk_mq_check_expired, &next);
> > +   /* check if there is any timed-out request */
> > +   blk_mq_queue_tag_busy_iter(q, blk_mq_check_expired, &expired);
> > +   if (expired.has_timedout_rq) {
> > +           /*
> > +            * Before walking tags, we must ensure any submit started
> > +            * before the current time has finished. Since the submit
> > +            * uses srcu or rcu, wait for a synchronization point to
> > +            * ensure all running submits have finished
> > +            */
> > +           blk_mq_wait_quiesce_done(q);
> > +
> > +           expired.check_only = false;
> > +           expired.next = 0;
> > +           blk_mq_queue_tag_busy_iter(q, blk_mq_check_expired, &expired);
> 
> I think it would be easier to follow with separate callbacks instead of
> special casing for 'check_only'. One callback for checking timeouts, and
> a different one for handling them?

Both two are basically same, with two callbacks, just .check_only is saved,
nothing else, meantime with one extra similar callback added.

If you or anyone think it is one big deal, I can switch to two callback version.


Thanks,
Ming

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to