| Message ID | 9e542bceca1c467c99f114be7ab958066b8c7bf5.1586199103.git.zhangweiping@didiglobal.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix potential kernel panic when increase hardware queue | expand |
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. Thanks, Bart.
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. Thanks, Ming Lei
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
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;
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(-)