linux-kernel - Re: [PATCH 1/2] perf: drop unneeded bitmap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 25 Jul 2018 08:20:00 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Yury Norov <ynorov@...iumnetworks.com>
Cc:     Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Kate Stewart <kstewart@...uxfoundation.org>,
        Matthew Wilcox <willy@...radead.org>,
        Philippe Ombredanne <pombredanne@...b.com>,
        David Ahern <dsahern@...il.com>,
        David Carrillo-Cisneros <davidcc@...gle.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Jin Yao <yao.jin@...ux.intel.com>,
        linux-kernel@...r.kernel.org,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Mike Snitzer <snitzer@...hat.com>
Subject: Re: [PATCH 1/2] perf: drop unneeded bitmap_zero() in util/header.c

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@...iumnetworks.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