Re: [PATCH V2 1/2] block: fix race between adding/removing rq qos and normal IO

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

On Tue, Jun 08, 2021 at 08:04:00AM -0700, Bart Van Assche wrote:
> On 6/8/21 12:19 AM, Ming Lei wrote:
> >  static inline void rq_qos_add(struct request_queue *q, struct rq_qos *rqos)
> >  {
> > +	/*
> > +	 * No IO can be in-flight when adding rqos, so freeze queue, which
> > +	 * is fine since we only support rq_qos for blk-mq queue
> > +	 */
> > +	blk_mq_freeze_queue(q);
> >  	rqos->next = q->rq_qos;
> >  	q->rq_qos = rqos;
> > +	blk_mq_unfreeze_queue(q);
> >  
> >  	if (rqos->ops->debugfs_attrs)
> >  		blk_mq_debugfs_register_rqos(rqos);
> > @@ -110,12 +117,18 @@ static inline void rq_qos_del(struct request_queue *q, struct rq_qos *rqos)
> >  {
> >  	struct rq_qos **cur;
> >  
> > +	/*
> > +	 * No IO can be in-flight when removing rqos, so freeze queue,
> > +	 * which is fine since we only support rq_qos for blk-mq queue
> > +	 */
> > +	blk_mq_freeze_queue(q);
> >  	for (cur = &q->rq_qos; *cur; cur = &(*cur)->next) {
> >  		if (*cur == rqos) {
> >  			*cur = rqos->next;
> >  			break;
> >  		}
> >  	}
> > +	blk_mq_unfreeze_queue(q);
> >  
> >  	blk_mq_debugfs_unregister_rqos(rqos);
> >  }
> 
> Although this patch looks like an improvement to me, I think we also
> need protection against concurrent rq_qos_add() and rq_qos_del() calls,
> e.g. via a mutex.

Fine, one spinlock should be enough, will do it in V3.


Thanks,
Ming