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:   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


Powered by blists - more mailing lists