linux-kernel - Re: [RFC PATCH 05/49] qed: rework qed_rdma_bmap

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]

Message-ID: <YgYi9p7oeR1NAKzv@smile.fi.intel.com>
Date:   Fri, 11 Feb 2022 10:48:54 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Yury Norov <yury.norov@...il.com>
Cc:     Rasmus Villemoes <linux@...musvillemoes.dk>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Michał Mirosław <mirq-linux@...e.qmqm.pl>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        David Laight <David.Laight@...lab.com>,
        Joe Perches <joe@...ches.com>, Dennis Zhou <dennis@...nel.org>,
        Emil Renner Berthing <kernel@...il.dk>,
        Nicholas Piggin <npiggin@...il.com>,
        Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
        Alexey Klimov <aklimov@...hat.com>,
        linux-kernel@...r.kernel.org, Ariel Elior <aelior@...vell.com>,
        Manish Chopra <manishc@...vell.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org
Subject: Re: [RFC PATCH 05/49] qed: rework qed_rdma_bmap_free()

On Thu, Feb 10, 2022 at 02:48:49PM -0800, Yury Norov wrote:
> qed_rdma_bmap_free() is mostly an opencoded version of printk("%*pb").
> Using %*pb format simplifies the code, and helps to avoid inefficient
> usage of bitmap_weight().
> 
> While here, reorganize logic to avoid calculating bmap weight if check
> is false.

I like this kind of patches,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>

> Signed-off-by: Yury Norov <yury.norov@...il.com>
> ---
> 
> This is RFC because it changes lines printing format to bitmap %*pb. If
> it hurts userspace, it's better to drop the patch.

How? The only way is some strange script that parses dmesg, but dmesg almost
never was an ABI, moreover, with printk() indexing feature (recently
introduced) the one who parses such messages can actually find the (new)
format as well.

>  drivers/net/ethernet/qlogic/qed/qed_rdma.c | 45 +++++++---------------
>  1 file changed, 14 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_rdma.c b/drivers/net/ethernet/qlogic/qed/qed_rdma.c
> index 23b668de4640..f4c04af9d4dd 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_rdma.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_rdma.c
> @@ -319,44 +319,27 @@ static int qed_rdma_alloc(struct qed_hwfn *p_hwfn)
>  void qed_rdma_bmap_free(struct qed_hwfn *p_hwfn,
>  			struct qed_bmap *bmap, bool check)
>  {
> -	int weight = bitmap_weight(bmap->bitmap, bmap->max_count);
> -	int last_line = bmap->max_count / (64 * 8);
> -	int last_item = last_line * 8 +
> -	    DIV_ROUND_UP(bmap->max_count % (64 * 8), 64);
> -	u64 *pmap = (u64 *)bmap->bitmap;
> -	int line, item, offset;
> -	u8 str_last_line[200] = { 0 };
> -
> -	if (!weight || !check)
> +	unsigned int bit, weight, nbits;
> +	unsigned long *b;
> +
> +	if (!check)
> +		goto end;
> +
> +	weight = bitmap_weight(bmap->bitmap, bmap->max_count);
> +	if (!weight)
>  		goto end;
>  
>  	DP_NOTICE(p_hwfn,
>  		  "%s bitmap not free - size=%d, weight=%d, 512 bits per line\n",
>  		  bmap->name, bmap->max_count, weight);
>  
> -	/* print aligned non-zero lines, if any */
> -	for (item = 0, line = 0; line < last_line; line++, item += 8)
> -		if (bitmap_weight((unsigned long *)&pmap[item], 64 * 8))
> +	for (bit = 0; bit < bmap->max_count; bit += 512) {
> +		b =  bmap->bitmap + BITS_TO_LONGS(bit);
> +		nbits = min(bmap->max_count - bit, 512);
> +
> +		if (!bitmap_empty(b, nbits))
>  			DP_NOTICE(p_hwfn,
> -				  "line 0x%04x: 0x%016llx 0x%016llx 0x%016llx 0x%016llx 0x%016llx 0x%016llx 0x%016llx 0x%016llx\n",
> -				  line,
> -				  pmap[item],
> -				  pmap[item + 1],
> -				  pmap[item + 2],
> -				  pmap[item + 3],
> -				  pmap[item + 4],
> -				  pmap[item + 5],
> -				  pmap[item + 6], pmap[item + 7]);
> -
> -	/* print last unaligned non-zero line, if any */
> -	if ((bmap->max_count % (64 * 8)) &&
> -	    (bitmap_weight((unsigned long *)&pmap[item],
> -			   bmap->max_count - item * 64))) {
> -		offset = sprintf(str_last_line, "line 0x%04x: ", line);
> -		for (; item < last_item; item++)
> -			offset += sprintf(str_last_line + offset,
> -					  "0x%016llx ", pmap[item]);
> -		DP_NOTICE(p_hwfn, "%s\n", str_last_line);
> +				  "line 0x%04x: %*pb\n", bit / 512, nbits, b);
>  	}
>  
>  end:
> -- 
> 2.32.0
> 

-- 
With Best Regards,
Andy Shevchenko