Re: [PATCH v3 1/7] block: rename __blk_mq_alloc_rq_map

[weiping zhang <zhangweiping@didichuxing.com>]

On Tue, Apr 21, 2020 at 09:25:49AM +0800, Ming Lei wrote:
> On Tue, Apr 21, 2020 at 4:42 AM Bart Van Assche <bvanassche@acm.org> wrote:
> >
> > On 4/6/20 12:36 PM, Weiping Zhang wrote:
> > > rename __blk_mq_alloc_rq_map to __blk_mq_alloc_rq_map_and_request,
> > > actually it alloc both map and request, make function name
> > > align with function.
> > >
> > > Signed-off-by: Weiping Zhang <zhangweiping@didiglobal.com>
> > > ---
> > >   block/blk-mq.c | 6 +++---
> > >   1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/block/blk-mq.c b/block/blk-mq.c
> > > index f6291ceedee4..3a482ce7ed28 100644
> > > --- a/block/blk-mq.c
> > > +++ b/block/blk-mq.c
> > > @@ -2468,7 +2468,7 @@ static void blk_mq_init_cpu_queues(struct request_queue *q,
> > >       }
> > >   }
> > >
> > > -static bool __blk_mq_alloc_rq_map(struct blk_mq_tag_set *set, int hctx_idx)
> > > +static bool __blk_mq_alloc_rq_map_and_request(struct blk_mq_tag_set *set, int hctx_idx)
> > >   {
> > >       int ret = 0;
> > >
> > > @@ -2519,7 +2519,7 @@ static void blk_mq_map_swqueue(struct request_queue *q)
> > >               hctx_idx = set->map[HCTX_TYPE_DEFAULT].mq_map[i];
> > >               /* unmapped hw queue can be remapped after CPU topo changed */
> > >               if (!set->tags[hctx_idx] &&
> > > -                 !__blk_mq_alloc_rq_map(set, hctx_idx)) {
> > > +                 !__blk_mq_alloc_rq_map_and_request(set, hctx_idx)) {
> > >                       /*
> > >                        * If tags initialization fail for some hctx,
> > >                        * that hctx won't be brought online.  In this
> > > @@ -2983,7 +2983,7 @@ static int __blk_mq_alloc_rq_maps(struct blk_mq_tag_set *set)
> > >       int i;
> > >
> > >       for (i = 0; i < set->nr_hw_queues; i++)
> > > -             if (!__blk_mq_alloc_rq_map(set, i))
> > > +             if (!__blk_mq_alloc_rq_map_and_request(set, i))
> > >                       goto out_unwind;
> > >
> > >       return 0;
> >
> > What the __blk_mq_alloc_rq_map() function allocates is a request map and
> > requests. The new name is misleading because it suggests that only a
> > single request is allocated instead of multiple. The name
> > __blk_mq_alloc_rq_map_and_requests() is probably a better choice than
> > __blk_mq_alloc_rq_map_and_request().
> >
> > My opinion is that the old name is clear enough. I prefer the current name.
> 
> Also putting renaming patches before actual fix patches does make more trouble
> for backporting the fix to stable tree.
> 
> So please re-organize patches by fixing issues first, then following rename
> stuff.
OK, I reorder them.

Thanks
> 
> Thanks,
> Ming Lei