Re: [PATCH 6/6] blk-mq: manage hctx map via xarray

[Ming Lei <ming.lei@redhat.com>]

On Tue, Mar 01, 2022 at 05:37:17AM -0800, Christoph Hellwig wrote:
> > -			hctxs[i] = blk_mq_alloc_and_init_hctx(set, q, i,
> > -					old_node);
> > -			WARN_ON_ONCE(!hctxs[i]);
> > +			WARN_ON_ONCE(!blk_mq_alloc_and_init_hctx(set, q, i,
> > +						old_node));
> 
> 
> Please avoid doing the actual work inside a WARN_ON statement.

OK.

> 
> >  
> >  	for (; j < end; j++) {
> > -		struct blk_mq_hw_ctx *hctx = hctxs[j];
> > +		struct blk_mq_hw_ctx *hctx = blk_mq_get_hctx(q, j);
> >  
> > -		if (hctx) {
> > +		if (hctx)
> >  			blk_mq_exit_hctx(q, set, hctx, j);
> > -			hctxs[j] = NULL;
> > -		}
> >  	}
> 
> Instead of a for loop that does xa_loads repeatedly this can just
> use xa_for_each_range.  Same for a bunch of other loops like that,
> e.g. in blk_mq_unregister_dev or the __blk_mq_register_dev failure
> path.
> 
> > @@ -919,12 +919,12 @@ static inline void *blk_mq_rq_to_pdu(struct request *rq)
> >  static inline struct blk_mq_hw_ctx *blk_mq_get_hctx(struct request_queue *q,
> >  		unsigned int hctx_idx)
> >  {
> > -	return q->queue_hw_ctx[hctx_idx];
> > +	return xa_load(&q->hctx_table, hctx_idx);
> >  }
> >  
> >  #define queue_for_each_hw_ctx(q, hctx, i)				\
> >  	for ((i) = 0; (i) < (q)->nr_hw_queues &&			\
> > -	     ({ hctx = blk_mq_get_hctx((q), (i)); 1; }); (i)++)
> > +	     (hctx = blk_mq_get_hctx((q), (i))); (i)++)
> 
> This should be using a xa_for_each loop.

I did considered xa_for_each(), but it requires rcu read lock.

Also queue_for_each_hw_ctx() is supposed to not run in fast path,
meantime xa_load() is lightweight enough too, so repeated xa_load()
is fine here.


Thanks,
Ming