Re: [PATCH net-next v2 3/7] ibmvnic: avoid allocating rwi entries

[Saeed Mahameed <saeed@kernel.org>]

On Tue, 2021-01-12 at 10:14 -0800, Sukadev Bhattiprolu wrote:
> Whenever we need to schedule a reset, we allocate an rwi (reset work
> item?) entry and add to the list of pending resets.
> 
> Since we only add one rwi for a given reason type to the list (no
> duplicates).
> we will only have a handful of reset types in the list - even in the
> worst case. In the common case we should just have a couple of
> entries
> at most.
> 
> Rather than allocating/freeing every time (and dealing with the
> corner
> case of the allocation failing), use a fixed number of rwi entries.
> The extra memory needed is tiny and most of it will be used over the
> active life of the adapter.
> 
> This also fixes a couple of tiny memory leaks. One is in
> ibmvnic_reset()
> where we don't free the rwi entries after deleting them from the list
> due
> to a transport event.  The second is in __ibmvnic_reset() where if we
> find that the adapter is being removed, we simply break out of the
> loop
> (with rc = EBUSY) but ignore any rwi entries that remain on the list.
> 
> Fixes: 2770a7984db58 ("Introduce hard reset recovery")
> Fixes: 36f1031c51a2 ("ibmvnic: Do not process reset during or after
> device removal")
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
> ---
>  drivers/net/ethernet/ibm/ibmvnic.c | 123 +++++++++++++++++--------
> ----
>  drivers/net/ethernet/ibm/ibmvnic.h |  14 ++--
>  2 files changed, 78 insertions(+), 59 deletions(-)
> 

[...]

> -enum ibmvnic_reset_reason {VNIC_RESET_FAILOVER = 1,
> +enum ibmvnic_reset_reason {VNIC_RESET_UNUSED = 0,
> +			   VNIC_RESET_FAILOVER = 1,
>  			   VNIC_RESET_MOBILITY,
>  			   VNIC_RESET_FATAL,
>  			   VNIC_RESET_NON_FATAL,
>  			   VNIC_RESET_TIMEOUT,
> -			   VNIC_RESET_CHANGE_PARAM};
> -
> -struct ibmvnic_rwi {
> -	enum ibmvnic_reset_reason reset_reason;
> -	struct list_head list;
> -};
> +			   VNIC_RESET_CHANGE_PARAM,
> +			   VNIC_RESET_MAX};	// must be last
       this is not the preferred comment style: ^^

I would just drop the comment here, it is clear from the name of the
enum.