diff mbox series

[net,1/2] ibmvnic: drop bad optimization in reuse_rx_pools()

Message ID 20211201054836.3488211-1-sukadev@linux.ibm.com (mailing list archive)
State Accepted
Commit 0584f4949609c0391ca98edf180c8ab7386c483a
Delegated to: Netdev Maintainers
Headers show
Series [net,1/2] ibmvnic: drop bad optimization in reuse_rx_pools() | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers fail 2 blamed authors not CCed: ricklind@linux.vnet.ibm.com davem@davemloft.net; 8 maintainers not CCed: benh@kernel.crashing.org ricklind@linux.vnet.ibm.com tlfalcon@linux.ibm.com paulus@samba.org linuxppc-dev@lists.ozlabs.org mpe@ellerman.id.au kuba@kernel.org davem@davemloft.net
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 20 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Sukadev Bhattiprolu Dec. 1, 2021, 5:48 a.m. UTC
When trying to decide whether or not reuse existing rx/tx pools
we tried to allow a range of values for the pool parameters rather
than exact matches. This was intended to reuse the resources for
instance when switching between two VIO servers with different
default parameters.

But this optimization is incomplete and breaks when we try to
change the number of queues for instance. The optimization needs
to be updated, so drop it for now and simplify the code.

Fixes: 489de956e7a2 ("ibmvnic: Reuse rx pools when possible")
Reported-by: Dany Madden <drt@linux.ibm.com>
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

Comments

Dany Madden Dec. 1, 2021, 6:12 p.m. UTC | #1
On 2021-11-30 21:48, Sukadev Bhattiprolu wrote:
> When trying to decide whether or not reuse existing rx/tx pools
> we tried to allow a range of values for the pool parameters rather
> than exact matches. This was intended to reuse the resources for
> instance when switching between two VIO servers with different
> default parameters.
> 
> But this optimization is incomplete and breaks when we try to
> change the number of queues for instance. The optimization needs
> to be updated, so drop it for now and simplify the code.
> 
> Fixes: 489de956e7a2 ("ibmvnic: Reuse rx pools when possible")
> Reported-by: Dany Madden <drt@linux.ibm.com>
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
Reviewed-by: Dany Madden <drt@linux.ibm.com>

> ---
>  drivers/net/ethernet/ibm/ibmvnic.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index 3cca51735421..6df92a872f0f 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -628,17 +628,9 @@ static bool reuse_rx_pools(struct ibmvnic_adapter 
> *adapter)
>  	old_buff_size = adapter->prev_rx_buf_sz;
>  	new_buff_size = adapter->cur_rx_buf_sz;
> 
> -	/* Require buff size to be exactly same for now */
> -	if (old_buff_size != new_buff_size)
> -		return false;
> -
> -	if (old_num_pools == new_num_pools && old_pool_size == new_pool_size)
> -		return true;
> -
> -	if (old_num_pools < adapter->min_rx_queues ||
> -	    old_num_pools > adapter->max_rx_queues ||
> -	    old_pool_size < adapter->min_rx_add_entries_per_subcrq ||
> -	    old_pool_size > adapter->max_rx_add_entries_per_subcrq)
> +	if (old_buff_size != new_buff_size ||
> +	    old_num_pools != new_num_pools ||
> +	    old_pool_size != new_pool_size)
>  		return false;
> 
>  	return true;
Rick Lindsley Dec. 1, 2021, 6:18 p.m. UTC | #2
On 11/30/21 21:48, Sukadev Bhattiprolu wrote:
> When trying to decide whether or not reuse existing rx/tx pools
> we tried to allow a range of values for the pool parameters rather
> than exact matches. This was intended to reuse the resources for
> instance when switching between two VIO servers with different
> default parameters.
> 
> But this optimization is incomplete and breaks when we try to
> change the number of queues for instance. The optimization needs
> to be updated, so drop it for now and simplify the code.
> 
> Fixes: 489de956e7a2 ("ibmvnic: Reuse rx pools when possible")
> Reported-by: Dany Madden <drt@linux.ibm.com>
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>

Reviewed-by: Rick Lindsley <ricklind@linux.ibm.com>

> ---
>   drivers/net/ethernet/ibm/ibmvnic.c | 14 +++-----------
>   1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
> index 3cca51735421..6df92a872f0f 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -628,17 +628,9 @@ static bool reuse_rx_pools(struct ibmvnic_adapter *adapter)
>   	old_buff_size = adapter->prev_rx_buf_sz;
>   	new_buff_size = adapter->cur_rx_buf_sz;
>   
> -	/* Require buff size to be exactly same for now */
> -	if (old_buff_size != new_buff_size)
> -		return false;
> -
> -	if (old_num_pools == new_num_pools && old_pool_size == new_pool_size)
> -		return true;
> -
> -	if (old_num_pools < adapter->min_rx_queues ||
> -	    old_num_pools > adapter->max_rx_queues ||
> -	    old_pool_size < adapter->min_rx_add_entries_per_subcrq ||
> -	    old_pool_size > adapter->max_rx_add_entries_per_subcrq)
> +	if (old_buff_size != new_buff_size ||
> +	    old_num_pools != new_num_pools ||
> +	    old_pool_size != new_pool_size)
>   		return false;
>   
>   	return true;
>
patchwork-bot+netdevbpf@kernel.org Dec. 2, 2021, 12:20 p.m. UTC | #3
Hello:

This series was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Tue, 30 Nov 2021 21:48:35 -0800 you wrote:
> When trying to decide whether or not reuse existing rx/tx pools
> we tried to allow a range of values for the pool parameters rather
> than exact matches. This was intended to reuse the resources for
> instance when switching between two VIO servers with different
> default parameters.
> 
> But this optimization is incomplete and breaks when we try to
> change the number of queues for instance. The optimization needs
> to be updated, so drop it for now and simplify the code.
> 
> [...]

Here is the summary with links:
  - [net,1/2] ibmvnic: drop bad optimization in reuse_rx_pools()
    https://git.kernel.org/netdev/net/c/0584f4949609
  - [net,2/2] ibmvnic: drop bad optimization in reuse_tx_pools()
    https://git.kernel.org/netdev/net/c/5b08560181b5

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 3cca51735421..6df92a872f0f 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -628,17 +628,9 @@  static bool reuse_rx_pools(struct ibmvnic_adapter *adapter)
 	old_buff_size = adapter->prev_rx_buf_sz;
 	new_buff_size = adapter->cur_rx_buf_sz;
 
-	/* Require buff size to be exactly same for now */
-	if (old_buff_size != new_buff_size)
-		return false;
-
-	if (old_num_pools == new_num_pools && old_pool_size == new_pool_size)
-		return true;
-
-	if (old_num_pools < adapter->min_rx_queues ||
-	    old_num_pools > adapter->max_rx_queues ||
-	    old_pool_size < adapter->min_rx_add_entries_per_subcrq ||
-	    old_pool_size > adapter->max_rx_add_entries_per_subcrq)
+	if (old_buff_size != new_buff_size ||
+	    old_num_pools != new_num_pools ||
+	    old_pool_size != new_pool_size)
 		return false;
 
 	return true;