Re: [PATCH 2/3] block: let blkcg_gq grab request queue's refcnt

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

On Tue, Mar 22, 2022 at 10:33:22AM +0100, Christoph Hellwig wrote:
> On Fri, Mar 18, 2022 at 09:01:43PM +0800, Ming Lei wrote:
> > In the whole lifetime of blkcg_gq instance, ->q will be referred, such
> > as, ->pd_free_fn() is called in blkg_free, and throtl_pd_free() still
> > may touch the request queue via &tg->service_queue.pending_timer which
> > is handled by throtl_pending_timer_fn(), so it is reasonable to grab
> > request queue's refcnt by blkcg_gq instance.
> > 
> > Previously blkcg_exit_queue() is called from blk_release_queue, and it
> > is hard to avoid the use-after-free. But recently commit 1059699f87eb ("block:
> > move blkcg initialization/destroy into disk allocation/release handler")
> > is merged to for-5.18/block, it becomes simple to fix the issue by simply
> > grabbing request queue's refcnt.
> > 
> > Reported-by: Christoph Hellwig <hch@lst.de>
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >  block/blk-cgroup.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
> > index fa063c6c0338..d53b0d69dd73 100644
> > --- a/block/blk-cgroup.c
> > +++ b/block/blk-cgroup.c
> > @@ -82,6 +82,8 @@ static void blkg_free(struct blkcg_gq *blkg)
> >  		if (blkg->pd[i])
> >  			blkcg_policy[i]->pd_free_fn(blkg->pd[i]);
> >  
> > +	if (blkg->q)
> > +		blk_put_queue(blkg->q);
> 
> blkg_free can be called from RCU context, while blk_put_queue must
> not be called from RCU context.  This causes regular splats when running
> xfstests like:

Thanks for the report.

One solution is to delay 'blk_put_queue(blkg->q)' and 'kfree(blkg)'
into one work function by reusing blkg->async_bio_work as release_work.

I will prepare one patch for addressing the issue.

Thanks,
Ming