linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: hns: avoid uninitialized variable warning:
@ 2016-01-01 22:27 Arnd Bergmann
  2016-01-05 21:43 ` David Miller
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Arnd Bergmann @ 2016-01-01 22:27 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, linux-kernel, Yisen Zhuang, yankejian, lisheng011,
	huangdaode, salil.mehta, linux-arm-kernel

gcc fails to see that the use of the 'last_offset' variable
in hns_nic_reuse_page() is used correctly and issues a bogus
warning:

drivers/net/ethernet/hisilicon/hns/hns_enet.c: In function 'hns_nic_reuse_page':
drivers/net/ethernet/hisilicon/hns/hns_enet.c:541:6: warning: 'last_offset' may be used uninitialized in this function [-Wmaybe-uninitialized]

This simplifies the function to make it more obvious what is
going on to both readers and compilers, which makes the warning
go away.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Compile-tested only, and complex enough that this requires a proper
review and testing before it gets apply. Please have a look at this.

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index 5a81dafd725e..0e30846a24f8 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -499,50 +499,47 @@ static void hns_nic_reuse_page(struct sk_buff *skb, int i,
 	struct hnae_desc *desc;
 	int truesize, size;
 	int last_offset;
+	bool twobufs;
+
+	twobufs = ((PAGE_SIZE < 8192) && hnae_buf_size(ring) == HNS_BUFFER_SIZE_2048);
 
 	desc = &ring->desc[ring->next_to_clean];
 	size = le16_to_cpu(desc->rx.size);
 
-#if (PAGE_SIZE < 8192)
-	if (hnae_buf_size(ring) == HNS_BUFFER_SIZE_2048) {
+	if (twobufs) {
 		truesize = hnae_buf_size(ring);
 	} else {
 		truesize = ALIGN(size, L1_CACHE_BYTES);
 		last_offset = hnae_page_size(ring) - hnae_buf_size(ring);
 	}
 
-#else
-	truesize = ALIGN(size, L1_CACHE_BYTES);
-	last_offset = hnae_page_size(ring) - hnae_buf_size(ring);
-#endif
-
 	skb_add_rx_frag(skb, i, desc_cb->priv, desc_cb->page_offset + pull_len,
 			size - pull_len, truesize - pull_len);
 
 	 /* avoid re-using remote pages,flag default unreuse */
-	if (likely(page_to_nid(desc_cb->priv) == numa_node_id())) {
-#if (PAGE_SIZE < 8192)
-		if (hnae_buf_size(ring) == HNS_BUFFER_SIZE_2048) {
-			/* if we are only owner of page we can reuse it */
-			if (likely(page_count(desc_cb->priv) == 1)) {
-				/* flip page offset to other buffer */
-				desc_cb->page_offset ^= truesize;
-
-				desc_cb->reuse_flag = 1;
-				/* bump ref count on page before it is given*/
-				get_page(desc_cb->priv);
-			}
-			return;
-		}
-#endif
-		/* move offset up to the next cache line */
-		desc_cb->page_offset += truesize;
+	if (unlikely(page_to_nid(desc_cb->priv) != numa_node_id()))
+		return;
+
+	if (twobufs) {
+		/* if we are only owner of page we can reuse it */
+		if (likely(page_count(desc_cb->priv) == 1)) {
+			/* flip page offset to other buffer */
+			desc_cb->page_offset ^= truesize;
 
-		if (desc_cb->page_offset <= last_offset) {
 			desc_cb->reuse_flag = 1;
 			/* bump ref count on page before it is given*/
 			get_page(desc_cb->priv);
 		}
+		return;
+	}
+
+	/* move offset up to the next cache line */
+	desc_cb->page_offset += truesize;
+
+	if (desc_cb->page_offset <= last_offset) {
+		desc_cb->reuse_flag = 1;
+		/* bump ref count on page before it is given*/
+		get_page(desc_cb->priv);
 	}
 }
 


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: hns: avoid uninitialized variable warning:
  2016-01-01 22:27 [PATCH] net: hns: avoid uninitialized variable warning: Arnd Bergmann
@ 2016-01-05 21:43 ` David Miller
  2016-01-06 15:35   ` Salil Mehta
  2016-01-06  0:56 ` Yisen Zhuang
  2016-01-06  5:01 ` David Miller
  2 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2016-01-05 21:43 UTC (permalink / raw)
  To: arnd
  Cc: netdev, linux-kernel, yisen.zhuang, yankejian, lisheng011,
	huangdaode, salil.mehta, linux-arm-kernel

From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 01 Jan 2016 23:27:57 +0100

> gcc fails to see that the use of the 'last_offset' variable
> in hns_nic_reuse_page() is used correctly and issues a bogus
> warning:
> 
> drivers/net/ethernet/hisilicon/hns/hns_enet.c: In function 'hns_nic_reuse_page':
> drivers/net/ethernet/hisilicon/hns/hns_enet.c:541:6: warning: 'last_offset' may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> This simplifies the function to make it more obvious what is
> going on to both readers and compilers, which makes the warning
> go away.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Compile-tested only, and complex enough that this requires a proper
> review and testing before it gets apply. Please have a look at this.

If this goes yet another day without being reviewed, I'm just applying
it.

You hisilicon folks can't just let patches rot, you must review them
in a timely manner or else I'm applying them without waiting for you
to look at them.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: hns: avoid uninitialized variable warning:
  2016-01-01 22:27 [PATCH] net: hns: avoid uninitialized variable warning: Arnd Bergmann
  2016-01-05 21:43 ` David Miller
@ 2016-01-06  0:56 ` Yisen Zhuang
  2016-01-06  5:01 ` David Miller
  2 siblings, 0 replies; 5+ messages in thread
From: Yisen Zhuang @ 2016-01-06  0:56 UTC (permalink / raw)
  To: Arnd Bergmann, David S. Miller
  Cc: netdev, linux-kernel, yankejian, lisheng011, huangdaode,
	salil.mehta, linux-arm-kernel



在 2016/1/2 6:27, Arnd Bergmann 写道:
> gcc fails to see that the use of the 'last_offset' variable
> in hns_nic_reuse_page() is used correctly and issues a bogus
> warning:
> 
> drivers/net/ethernet/hisilicon/hns/hns_enet.c: In function 'hns_nic_reuse_page':
> drivers/net/ethernet/hisilicon/hns/hns_enet.c:541:6: warning: 'last_offset' may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> This simplifies the function to make it more obvious what is
> going on to both readers and compilers, which makes the warning
> go away.
> 


Hi Arnd,

This patch is fine to me.

Many thanks,
Yisen

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Compile-tested only, and complex enough that this requires a proper
> review and testing before it gets apply. Please have a look at this.
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
> index 5a81dafd725e..0e30846a24f8 100644
> --- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
> +++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
> @@ -499,50 +499,47 @@ static void hns_nic_reuse_page(struct sk_buff *skb, int i,
>  	struct hnae_desc *desc;
>  	int truesize, size;
>  	int last_offset;
> +	bool twobufs;
> +
> +	twobufs = ((PAGE_SIZE < 8192) && hnae_buf_size(ring) == HNS_BUFFER_SIZE_2048);
>  
>  	desc = &ring->desc[ring->next_to_clean];
>  	size = le16_to_cpu(desc->rx.size);
>  
> -#if (PAGE_SIZE < 8192)
> -	if (hnae_buf_size(ring) == HNS_BUFFER_SIZE_2048) {
> +	if (twobufs) {
>  		truesize = hnae_buf_size(ring);
>  	} else {
>  		truesize = ALIGN(size, L1_CACHE_BYTES);
>  		last_offset = hnae_page_size(ring) - hnae_buf_size(ring);
>  	}
>  
> -#else
> -	truesize = ALIGN(size, L1_CACHE_BYTES);
> -	last_offset = hnae_page_size(ring) - hnae_buf_size(ring);
> -#endif
> -
>  	skb_add_rx_frag(skb, i, desc_cb->priv, desc_cb->page_offset + pull_len,
>  			size - pull_len, truesize - pull_len);
>  
>  	 /* avoid re-using remote pages,flag default unreuse */
> -	if (likely(page_to_nid(desc_cb->priv) == numa_node_id())) {
> -#if (PAGE_SIZE < 8192)
> -		if (hnae_buf_size(ring) == HNS_BUFFER_SIZE_2048) {
> -			/* if we are only owner of page we can reuse it */
> -			if (likely(page_count(desc_cb->priv) == 1)) {
> -				/* flip page offset to other buffer */
> -				desc_cb->page_offset ^= truesize;
> -
> -				desc_cb->reuse_flag = 1;
> -				/* bump ref count on page before it is given*/
> -				get_page(desc_cb->priv);
> -			}
> -			return;
> -		}
> -#endif
> -		/* move offset up to the next cache line */
> -		desc_cb->page_offset += truesize;
> +	if (unlikely(page_to_nid(desc_cb->priv) != numa_node_id()))
> +		return;
> +
> +	if (twobufs) {
> +		/* if we are only owner of page we can reuse it */
> +		if (likely(page_count(desc_cb->priv) == 1)) {
> +			/* flip page offset to other buffer */
> +			desc_cb->page_offset ^= truesize;
>  
> -		if (desc_cb->page_offset <= last_offset) {
>  			desc_cb->reuse_flag = 1;
>  			/* bump ref count on page before it is given*/
>  			get_page(desc_cb->priv);
>  		}
> +		return;
> +	}
> +
> +	/* move offset up to the next cache line */
> +	desc_cb->page_offset += truesize;
> +
> +	if (desc_cb->page_offset <= last_offset) {
> +		desc_cb->reuse_flag = 1;
> +		/* bump ref count on page before it is given*/
> +		get_page(desc_cb->priv);
>  	}
>  }
>  
> 
> 
> .
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: hns: avoid uninitialized variable warning:
  2016-01-01 22:27 [PATCH] net: hns: avoid uninitialized variable warning: Arnd Bergmann
  2016-01-05 21:43 ` David Miller
  2016-01-06  0:56 ` Yisen Zhuang
@ 2016-01-06  5:01 ` David Miller
  2 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2016-01-06  5:01 UTC (permalink / raw)
  To: arnd
  Cc: netdev, linux-kernel, yisen.zhuang, yankejian, lisheng011,
	huangdaode, salil.mehta, linux-arm-kernel

From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 01 Jan 2016 23:27:57 +0100

> gcc fails to see that the use of the 'last_offset' variable
> in hns_nic_reuse_page() is used correctly and issues a bogus
> warning:
> 
> drivers/net/ethernet/hisilicon/hns/hns_enet.c: In function 'hns_nic_reuse_page':
> drivers/net/ethernet/hisilicon/hns/hns_enet.c:541:6: warning: 'last_offset' may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> This simplifies the function to make it more obvious what is
> going on to both readers and compilers, which makes the warning
> go away.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied, thanks.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: hns: avoid uninitialized variable warning:
  2016-01-05 21:43 ` David Miller
@ 2016-01-06 15:35   ` Salil Mehta
  0 siblings, 0 replies; 5+ messages in thread
From: Salil Mehta @ 2016-01-06 15:35 UTC (permalink / raw)
  To: David Miller, arnd
  Cc: netdev, linux-kernel, yisen.zhuang, yankejian, lisheng011,
	huangdaode, linux-arm-kernel


On 1/5/2016 9:43 PM, David Miller wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Fri, 01 Jan 2016 23:27:57 +0100
>
>> gcc fails to see that the use of the 'last_offset' variable
>> in hns_nic_reuse_page() is used correctly and issues a bogus
>> warning:
>>
>> drivers/net/ethernet/hisilicon/hns/hns_enet.c: In function 'hns_nic_reuse_page':
>> drivers/net/ethernet/hisilicon/hns/hns_enet.c:541:6: warning: 'last_offset' may be used uninitialized in this function [-Wmaybe-uninitialized]
>>
>> This simplifies the function to make it more obvious what is
>> going on to both readers and compilers, which makes the warning
>> go away.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> Compile-tested only, and complex enough that this requires a proper
>> review and testing before it gets apply. Please have a look at this.
> If this goes yet another day without being reviewed, I'm just applying
> it.
>
> You hisilicon folks can't just let patches rot, you must review them
> in a timely manner or else I'm applying them without waiting for you
> to look at them.
Hi David and Arnd,
Apologies for the delay in response and the review. Most of us were on 
the Annual Holidays and have just returned back.

Change looks good to me!

Best Regards
Salil



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-01-06 15:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-01 22:27 [PATCH] net: hns: avoid uninitialized variable warning: Arnd Bergmann
2016-01-05 21:43 ` David Miller
2016-01-06 15:35   ` Salil Mehta
2016-01-06  0:56 ` Yisen Zhuang
2016-01-06  5:01 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).