Re: [PATCH 1/5] blk-mq: re-organize blk_mq_exit_hctx() into two parts

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

On Mon, Apr 01, 2019 at 09:40:32AM +0800, Dongli Zhang wrote:
> 
> 
> On 3/31/19 11:09 AM, Ming Lei wrote:
> > This patch re-organizes blk_mq_exit_hctx() into two parts, and
> > one part is for exit hctx, and another part is for free hw queue.
> > 
> > No function change, just prepare for fixing hctx lifetime issue.
> > 
> > Cc: James Smart <james.smart@broadcom.com>
> > Cc: Bart Van Assche <bart.vanassche@wdc.com>
> > Cc: linux-scsi@vger.kernel.org,
> > Cc: Martin K . Petersen <martin.petersen@oracle.com>,
> > Cc: Christoph Hellwig <hch@lst.de>,
> > Cc: James E . J . Bottomley <jejb@linux.vnet.ibm.com>,
> > Cc: jianchao wang <jianchao.w.wang@oracle.com>
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >  block/blk-mq.c | 16 +++++++++++-----
> >  1 file changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/block/blk-mq.c b/block/blk-mq.c
> > index 70b210a308c4..53265ce45238 100644
> > --- a/block/blk-mq.c
> > +++ b/block/blk-mq.c
> > @@ -2229,6 +2229,16 @@ static void blk_mq_remove_cpuhp(struct blk_mq_hw_ctx *hctx)
> >  					    &hctx->cpuhp_dead);
> >  }
> >  
> > +static void blk_mq_free_hctx(struct request_queue *q,
> > +		struct blk_mq_hw_ctx *hctx)
> > +{
> > +	if (hctx->flags & BLK_MQ_F_BLOCKING)
> > +		cleanup_srcu_struct(hctx->srcu);
> > +
> > +	blk_free_flush_queue(hctx->fq);
> > +	sbitmap_free(&hctx->ctx_map);
> > +}
> > +
> >  /* hctx->ctxs will be freed in queue's release handler */
> >  static void blk_mq_exit_hctx(struct request_queue *q,
> >  		struct blk_mq_tag_set *set,
> > @@ -2243,12 +2253,7 @@ static void blk_mq_exit_hctx(struct request_queue *q,
> >  	if (set->ops->exit_hctx)
> >  		set->ops->exit_hctx(hctx, hctx_idx);
> >  
> > -	if (hctx->flags & BLK_MQ_F_BLOCKING)
> > -		cleanup_srcu_struct(hctx->srcu);
> > -
> >  	blk_mq_remove_cpuhp(hctx);
> > -	blk_free_flush_queue(hctx->fq);
> > -	sbitmap_free(&hctx->ctx_map);
> >  }
> >  
> >  static void blk_mq_exit_hw_queues(struct request_queue *q,
> > @@ -2262,6 +2267,7 @@ static void blk_mq_exit_hw_queues(struct request_queue *q,
> >  			break;
> >  		blk_mq_debugfs_unregister_hctx(hctx);
> >  		blk_mq_exit_hctx(q, set, hctx, i);
> > +		blk_mq_free_hctx(q, hctx);
> 
> Assuming if this direction is still followed, how about blk_mq_realloc_hw_ctxs()
> which is another caller of blk_mq_exit_hctx(), while part of blk_mq_exit_hctx()
> is extracted and encapsulated into blk_mq_free_hctx()?
> 
> Additional blk_mq_free_hctx() is missing?

Good catch!

Another two blk_mq_exit_hctx() is called just before kobject_put(&hctx->kobj), then
we can simply move blk_mq_free_hctx() into hctx's release handler(blk_mq_hw_sysfs_release),
then thing is simplified too.

Thanks,
Ming