On Thu, Aug 29, 2024 at 09:00:05PM +0000, Haiyang Zhang wrote: > > > > -----Original Message----- > > From: Gerhard Engleder <[email protected]> > > Sent: Thursday, August 29, 2024 3:54 PM > > To: Shradha Gupta <[email protected]>; linux- > > [email protected]; [email protected]; linux- > > [email protected]; [email protected] > > Cc: KY Srinivasan <[email protected]>; Haiyang Zhang > > <[email protected]>; Wei Liu <[email protected]>; Dexuan Cui > > <[email protected]>; David S. Miller <[email protected]>; Eric > > Dumazet <[email protected]>; Jakub Kicinski <[email protected]>; Paolo > > Abeni <[email protected]>; Long Li <[email protected]>; Simon Horman > > <[email protected]>; Konstantin Taranov <[email protected]>; > > Souradeep Chakrabarti <[email protected]>; Erick Archer > > <[email protected]>; Pavan Chebbi <[email protected]>; > > Ahmed Zaki <[email protected]>; Colin Ian King > > <[email protected]>; Shradha Gupta <[email protected]> > > 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 > > [email protected]. 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 <[email protected]> > > > --- > > > .../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
Thanks Haiyang and Gerhard. Instead of changing the apc structure value, I will pass it to the mana_pre_alloc_rxbufs() function in the next version. That should make sure other calls are unaffected. Thanks, Shradha.
