[PATCH] be2net: Fix some u16 fields appropriately

[PATCH] be2net: Fix some u16 fields appropriately

[Haishuang Yan]

In be_tx_compl_process, frag_index declared as u32, so it's better to
declare last_index as u32 also.

CC: Ajit Khaparde <ajit.khaparde@broadcom.com>
Fixes: b0fd2eb28bd4 ("be2net: Declare some u16 fields as u32 to improve
performance")
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
 drivers/net/ethernet/emulex/benet/be.h      | 2 +-
 drivers/net/ethernet/emulex/benet/be_main.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index 674cf9d..2ba4d61 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -255,7 +255,7 @@ struct be_tx_stats {
 /* Structure to hold some data of interest obtained from a TX CQE */
 struct be_tx_compl_info {
 	u8 status;		/* Completion status */
-	u16 end_index;		/* Completed TXQ Index */
+	u32 end_index;		/* Completed TXQ Index */
 };
 
 struct be_tx_obj {
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 319eee3..3645344 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -2606,7 +2606,7 @@ static struct be_tx_compl_info *be_tx_compl_get(struct be_tx_obj *txo)
 }
 
 static u16 be_tx_compl_process(struct be_adapter *adapter,
-			       struct be_tx_obj *txo, u16 last_index)
+			       struct be_tx_obj *txo, u32 last_index)
 {
 	struct sk_buff **sent_skbs = txo->sent_skb_list;
 	struct be_queue_info *txq = &txo->q;
-- 
1.8.3.1

Re: [PATCH] be2net: Fix some u16 fields appropriately

[David Miller]

From: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
Date: Sun, 27 Aug 2017 15:24:45 +0800

> In be_tx_compl_process, frag_index declared as u32, so it's better to
> declare last_index as u32 also.
> 
> CC: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Fixes: b0fd2eb28bd4 ("be2net: Declare some u16 fields as u32 to improve
> performance")
> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>

That is not a legitimate reason for making this change.

> @@ -255,7 +255,7 @@ struct be_tx_stats {
>  /* Structure to hold some data of interest obtained from a TX CQE */
>  struct be_tx_compl_info {
>  	u8 status;		/* Completion status */
> -	u16 end_index;		/* Completed TXQ Index */
> +	u32 end_index;		/* Completed TXQ Index */
>  };
>  
>  struct be_tx_obj {

The ->end_index comes solely from:

	txcp->end_index = GET_TX_COMPL_BITS(wrb_index, compl);

Which is precisely a 16-bit value.

I'm not applying this, sorry.

Re: [PATCH] be2net: Fix some u16 fields appropriately

[严海双]

> On 2017年8月29日, at 上午7:19, David Miller <davem@davemloft.net> wrote:
> 
> From: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
> Date: Sun, 27 Aug 2017 15:24:45 +0800
> 
>> In be_tx_compl_process, frag_index declared as u32, so it's better to
>> declare last_index as u32 also.
>> 
>> CC: Ajit Khaparde <ajit.khaparde@broadcom.com>
>> Fixes: b0fd2eb28bd4 ("be2net: Declare some u16 fields as u32 to improve
>> performance")
>> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
> 
> That is not a legitimate reason for making this change.
> 
>> @@ -255,7 +255,7 @@ struct be_tx_stats {
>> /* Structure to hold some data of interest obtained from a TX CQE */
>> struct be_tx_compl_info {
>> 	u8 status;		/* Completion status */
>> -	u16 end_index;		/* Completed TXQ Index */
>> +	u32 end_index;		/* Completed TXQ Index */
>> };
>> 
>> struct be_tx_obj {
> 
> The ->end_index comes solely from:
> 
> 	txcp->end_index = GET_TX_COMPL_BITS(wrb_index, compl);
> 
> Which is precisely a 16-bit value.
> 
> I'm not applying this, sorry.
> 

Hi David,

The GET_TX_COMPL_BITS comes from amap_get which also returns a 32-bit value:

  6 static inline u32 amap_get(void *ptr, u32 dw_offset, u32 mask, u32 offset)
  5 {
  4     u32 *dw = (u32 *) ptr;
  3     return mask & (*(dw + dw_offset) >> offset);
  2 }
  1
869 #define AMAP_GET_BITS(_struct, field, ptr)              \
  1         amap_get(ptr,                       \
  2             offsetof(_struct, field)/32,            \
  3             amap_mask(sizeof(((_struct *)0)->field)),   \
  4             AMAP_BIT_OFFSET(_struct, field))

Re: [PATCH] be2net: Fix some u16 fields appropriately

[David Miller]

From: 严海双 <yanhaishuang@cmss.chinamobile.com>
Date: Tue, 29 Aug 2017 09:04:57 +0800

> The GET_TX_COMPL_BITS comes from amap_get which also returns a 32-bit value:

It never returns a value with more than 16-bits of significance for
this specific call.

Please stop trying to be semantically clever when arguing about this
change.

It's not about types, it's about what range of values the struct
member can actually hold.