> -----Original Message-----
> From: Gerhard Engleder <gerh...@engleder-embedded.com>
> Sent: Thursday, August 29, 2024 3:54 PM
> To: Shradha Gupta <shradhagu...@linux.microsoft.com>; linux-
> hyp...@vger.kernel.org; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; linux-r...@vger.kernel.org
> Cc: KY Srinivasan <k...@microsoft.com>; Haiyang Zhang
> <haiya...@microsoft.com>; Wei Liu <wei....@kernel.org>; Dexuan Cui
> <de...@microsoft.com>; David S. Miller <da...@davemloft.net>; Eric
> Dumazet <eduma...@google.com>; Jakub Kicinski <k...@kernel.org>; Paolo
> Abeni <pab...@redhat.com>; Long Li <lon...@microsoft.com>; Simon Horman
> <ho...@kernel.org>; Konstantin Taranov <kotara...@microsoft.com>;
> Souradeep Chakrabarti <schakraba...@linux.microsoft.com>; Erick Archer
> <erick.arc...@outlook.com>; Pavan Chebbi <pavan.che...@broadcom.com>;
> Ahmed Zaki <ahmed.z...@intel.com>; Colin Ian King
> <colin.i.k...@gmail.com>; Shradha Gupta <shradhagu...@microsoft.com>
> Subject: Re: [PATCH net-next] net: mana: Improve mana_set_channels() for
> low mem conditions
>
> [Some people who received this message don't often get email from
> gerh...@engleder-embedded.com. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
>
> On 29.08.24 16:16, Shradha Gupta wrote:
> > The mana_set_channels() function requires detaching the mana
> > driver and reattaching it with changed channel values.
> > During this operation if the system is low on memory, the reattach
> > might fail, causing the network device being down.
> > To avoid this we pre-allocate buffers at the beginning of set
> operation,
> > to prevent complete network loss
> >
> > Signed-off-by: Shradha Gupta <shradhagu...@linux.microsoft.com>
> > ---
> > .../ethernet/microsoft/mana/mana_ethtool.c | 28 +++++++++++-------
> -
> > 1 file changed, 16 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> > index d6a35fbda447..5077493fdfde 100644
> > --- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> > +++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> > @@ -345,27 +345,31 @@ static int mana_set_channels(struct net_device
> *ndev,
> > struct mana_port_context *apc = netdev_priv(ndev);
> > unsigned int new_count = channels->combined_count;
> > unsigned int old_count = apc->num_queues;
> > - int err, err2;
> > + int err;
> > +
> > + apc->num_queues = new_count;
> > + err = mana_pre_alloc_rxbufs(apc, ndev->mtu);
> > + apc->num_queues = old_count;
>
> Are you sure that temporary changing num_queues has no side effects on
> other num_queues users like mana_chn_setxdp()?
>
mana_chn_setxdp() is protected by rtnl_lock, which is OK. But I'm not sure
if all other users are protected. mana_get_stats64() seems not.
@Shradha Gupta You can add num_queues as an argument of mana_pre_alloc_rxbufs()
to avoid changing apc->num_queues.
Thanks,
- Haiyang
