Re: [PATCH V2 11/13] block: move blk_exit_queue into disk_release

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

On Mon, Jan 24, 2022 at 02:22:21PM +0100, Christoph Hellwig wrote:
> On Sat, Jan 22, 2022 at 07:10:52PM +0800, Ming Lei wrote:
> >  3 files changed, 41 insertions(+), 17 deletions(-)
> > 
> > diff --git a/block/blk-mq.c b/block/blk-mq.c
> > index d51b0aa2e4e4..72ae9955cf27 100644
> > --- a/block/blk-mq.c
> > +++ b/block/blk-mq.c
> > @@ -3101,6 +3101,9 @@ void blk_mq_free_rqs(struct blk_mq_tag_set *set, struct blk_mq_tags *tags,
> >  	struct blk_mq_tags *drv_tags;
> >  	struct page *page;
> >  
> > +	if (list_empty(&tags->page_list))
> > +		return;
> 
> Please split this out and document why it is needed.

Now blk_mq_sched_free_rqs() can be called from either blk_cleanup_queue
and disk_release(). If sched rq pool is freed in one of them, it can't
to be freed from another one.

> 
> > +/* Unconfigure the I/O scheduler and dissociate from the cgroup controller. */
> > +static void blk_exit_queue(struct request_queue *q)
> > +{
> > +	/*
> > +	 * Since the I/O scheduler exit code may access cgroup information,
> > +	 * perform I/O scheduler exit before disassociating from the block
> > +	 * cgroup controller.
> > +	 */
> > +	if (q->elevator) {
> > +		ioc_clear_queue(q);
> > +
> > +		mutex_lock(&q->sysfs_lock);
> > +		blk_mq_sched_free_rqs(q);
> > +		elevator_exit(q);
> > +		mutex_unlock(&q->sysfs_lock);
> > +	}
> > +}
> 
> Given that this all deals with the I/O scheduler the naming seems
> wrong.  It almost seems like we'd want to move ioc_clear_queue and
> blk_mq_sched_free_rqs into elevator_exit to have somewhat sensible
> helpers.

Looks good idea.



thanks, 
Ming