Re: [PATCH 1/3 net] ibmvnic: Modify buffer size on failover

[kbuild test robot]

Hi John,

I love your patch! Yet something to improve:

[auto build test ERROR on net/master]

url:
https://github.com/0day-ci/linux/commits/John-Allen/ibmvnic-Modify-buffer-size-on-failover/20180118-190528
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   drivers/net//ethernet/ibm/ibmvnic.c: In function 'reset_rx_pools':
>> drivers/net//ethernet/ibm/ibmvnic.c:414:2: error: 'size_array' undeclared 
>> (first use in this function); did you mean 'sem_array'?
 size_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
 ^~
 sem_array
   drivers/net//ethernet/ibm/ibmvnic.c:414:2: note: each undeclared identifier 
is reported only once for each function it appears in

vim +414 drivers/net//ethernet/ibm/ibmvnic.c

   407  
   408  static int reset_rx_pools(struct ibmvnic_adapter *adapter)
   409  {
   410  struct ibmvnic_rx_pool *rx_pool;
   411  int rx_scrqs;
   412  int i, j, rc;
   413  
 > 414  size_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
   415  
be32_to_cpu(adapter->login_rsp_buf->off_rxadd_buff_size));
   416  
   417  rx_scrqs = 
be32_to_cpu(adapter->login_rsp_buf->num_rxadd_subcrqs);
   418  for (i = 0; i < rx_scrqs; i++) {
   419  rx_pool = >rx_pool[i];
   420  
   421  netdev_dbg(adapter->netdev, "Re-setting rx_pool[%d]\n", 
i);
   422  
   423  if (rx_pool->buff_size != be64_to_cpu(size_array[i])) {
   424  rx_pool->buff_size = be64_to_cpu(size_array[i]);
   425  rc = alloc_long_term_buff(adapter,
   426
_pool->long_term_buff,
   427rx_pool->size *
   428rx_pool->buff_size);
   429  } else {
   430  rc = reset_long_term_buff(adapter,
   431
_pool->long_term_buff);
   432  }
   433  if (rc)
   434  return rc;
   435  
   436  for (j = 0; j < rx_pool->size; j++)
   437  rx_pool->free_map[j] = j;
   438  
   439  memset(rx_pool->rx_buff, 0,
   440 rx_pool->size * sizeof(struct ibmvnic_rx_buff));
   441  
   442  atomic_set(_pool->available, 0);
   443  rx_pool->next_alloc = 0;
   444  rx_pool->next_free = 0;
   445  rx_pool->active = 1;
   446  }
   447  
   448  return 0;
   449  }
   450  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH 1/3 net] ibmvnic: Modify buffer size on failover

[John Allen]

On 01/15/2018 03:11 PM, John Allen wrote:
> Using newer backing devices can cause the required padding at the end of
> rx buffers to change. Currently we assume that the size of buffers will
> never change, but in the case that we failover from a backing device with
> smaller padding requirement to a backing device with a larger padding
> requirement, the vnic server will fail to post rx buffers due to
> inadequate space in our rx pool. This patch fixes the issue by checking
> whether or not the buffer size has changed on a reset and if it has,
> reallocate the buffer.
> 
> Signed-off-by: John Allen 
> ---
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index b676fa9..5b68a28 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -412,13 +412,25 @@ static int reset_rx_pools(struct ibmvnic_adapter 
> *adapter)
>   int rx_scrqs;
>   int i, j, rc;
> 
> + size_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
> + be32_to_cpu(adapter->login_rsp_buf->off_rxadd_buff_size));
> +
>   rx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_rxadd_subcrqs);
>   for (i = 0; i < rx_scrqs; i++) {
>   rx_pool = >rx_pool[i];
> 
>   netdev_dbg(adapter->netdev, "Re-setting rx_pool[%d]\n", i);
> 
> - rc = reset_long_term_buff(adapter, _pool->long_term_buff);
> + if (rx_pool->buff_size != be64_to_cpu(size_array[i])) {
> + rx_pool->buff_size = be64_to_cpu(size_array[i]);
> + rc = alloc_long_term_buff(adapter,
> +   _pool->long_term_buff,
> +   rx_pool->size *
> +   rx_pool->buff_size);

We should be freeing the long_term_buff here before allocating a new one.
I will send a new version with the changes. Please ignore this version.

-John

> + } else {
> + rc = reset_long_term_buff(adapter,
> +   _pool->long_term_buff);
> + }
>   if (rc)
>   return rc;
> 

[PATCH 1/3 net] ibmvnic: Modify buffer size on failover

[John Allen]

Using newer backing devices can cause the required padding at the end of
rx buffers to change. Currently we assume that the size of buffers will
never change, but in the case that we failover from a backing device with
smaller padding requirement to a backing device with a larger padding
requirement, the vnic server will fail to post rx buffers due to
inadequate space in our rx pool. This patch fixes the issue by checking
whether or not the buffer size has changed on a reset and if it has,
reallocate the buffer.

Signed-off-by: John Allen 
---
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index b676fa9..5b68a28 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -412,13 +412,25 @@ static int reset_rx_pools(struct ibmvnic_adapter *adapter)
int rx_scrqs;
int i, j, rc;

+   size_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
+   be32_to_cpu(adapter->login_rsp_buf->off_rxadd_buff_size));
+
rx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_rxadd_subcrqs);
for (i = 0; i < rx_scrqs; i++) {
rx_pool = >rx_pool[i];

netdev_dbg(adapter->netdev, "Re-setting rx_pool[%d]\n", i);

-   rc = reset_long_term_buff(adapter, _pool->long_term_buff);
+   if (rx_pool->buff_size != be64_to_cpu(size_array[i])) {
+   rx_pool->buff_size = be64_to_cpu(size_array[i]);
+   rc = alloc_long_term_buff(adapter,
+ _pool->long_term_buff,
+ rx_pool->size *
+ rx_pool->buff_size);
+   } else {
+   rc = reset_long_term_buff(adapter,
+ _pool->long_term_buff);
+   }
if (rc)
return rc;