Re: [PATCH 1/2] perf: drop unneeded bitmap_zero() in util/header.c

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Sat, Jun 23, 2018 at 10:35:01AM +0300, Yury Norov escreveu:
> On top of next-20180622.
> 
> bitmap_zero() is called after bitmap_alloc() in perf code. But
> bitmap_alloc() internally uses calloc() which guarantees that allocated
> area is zeroed. So following bitmap_zero is unneeded. Drop it.
> 
> This happened because of confusing name for bitmap allocator. It
> should has name bitmap_zalloc instead of bitmap_alloc. This series:
> https://lkml.org/lkml/2018/6/18/841
> introduces new API for bitmap allocations in kernel, and functions
> there are named correctly. Following patch propogates the API to tools,
> and fixes naming issue.
> 
> Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> ---
>  tools/perf/tests/bitmap.c   | 2 --
>  tools/perf/tests/mem2node.c | 5 +----
>  tools/perf/util/header.c    | 3 ---
>  3 files changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/tools/perf/tests/bitmap.c b/tools/perf/tests/bitmap.c
> index 47bedf25ba69..96e7fc1ad3f9 100644
> --- a/tools/perf/tests/bitmap.c
> +++ b/tools/perf/tests/bitmap.c
> @@ -16,8 +16,6 @@ static unsigned long *get_bitmap(const char *str, int nbits)
>  	bm = bitmap_alloc(nbits);
>  
>  	if (map && bm) {
> -		bitmap_zero(bm, nbits);
> -
>  		for (i = 0; i < map->nr; i++)
>  			set_bit(map->map[i], bm);
>  	}
> diff --git a/tools/perf/tests/mem2node.c b/tools/perf/tests/mem2node.c
> index 0c3c87f86e03..d8e3d49d3638 100644
> --- a/tools/perf/tests/mem2node.c
> +++ b/tools/perf/tests/mem2node.c
> @@ -24,11 +24,8 @@ static unsigned long *get_bitmap(const char *str, int nbits)
>  	bm = bitmap_alloc(nbits);
>  
>  	if (map && bm) {
> -		bitmap_zero(bm, nbits);
> -
> -		for (i = 0; i < map->nr; i++) {
> +		for (i = 0; i < map->nr; i++)
>  			set_bit(map->map[i], bm);
> -		}
>  	}

Please refrain from doing unrelated changes to the purpose of the patch,
that just gets in the way of reviewing, i.e. what for removing those
braces? The patch should be just:

@@ -24,6 +24,7 @@ static unsigned long *get_bitmap(const char *str, int nbits)
 	bm = bitmap_alloc(nbits);
 
 	if (map && bm) {
-		bitmap_zero(bm, nbits);
 
 		for (i = 0; i < map->nr; i++) {

Because the point of this patch is just to remove this unneeded
bitmap_zero().

>  
>  	if (map)
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 540cd2dcd3e7..3a6bec22baa3 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -279,8 +279,6 @@ static int do_read_bitmap(struct feat_fd *ff, unsigned long **pset, u64 *psize)
>  	if (!set)
>  		return -ENOMEM;
>  
> -	bitmap_zero(set, size);
> -
>  	p = (u64 *) set;
>  
>  	for (i = 0; (u64) i < BITS_TO_U64(size); i++) {
> @@ -1285,7 +1283,6 @@ static int memory_node__read(struct memory_node *n, unsigned long idx)
>  		return -ENOMEM;
>  	}
>  
> -	bitmap_zero(n->set, size);
>  	n->node = idx;
>  	n->size = size;
>  
> -- 
> 2.17.1