On Thu, Aug 29, 2024 at 09:00:05PM +0000, Haiyang Zhang wrote: > > > > -----Original Message----- > > From: Gerhard Engleder <gerhard@xxxxxxxxxxxxxxxxxxxxx> > > Sent: Thursday, August 29, 2024 3:54 PM > > To: Shradha Gupta <shradhagupta@xxxxxxxxxxxxxxxxxxx>; linux- > > hyperv@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux- > > kernel@xxxxxxxxxxxxxxx; linux-rdma@xxxxxxxxxxxxxxx > > Cc: KY Srinivasan <kys@xxxxxxxxxxxxx>; Haiyang Zhang > > <haiyangz@xxxxxxxxxxxxx>; Wei Liu <wei.liu@xxxxxxxxxx>; Dexuan Cui > > <decui@xxxxxxxxxxxxx>; David S. Miller <davem@xxxxxxxxxxxxx>; Eric > > Dumazet <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo > > Abeni <pabeni@xxxxxxxxxx>; Long Li <longli@xxxxxxxxxxxxx>; Simon Horman > > <horms@xxxxxxxxxx>; Konstantin Taranov <kotaranov@xxxxxxxxxxxxx>; > > Souradeep Chakrabarti <schakrabarti@xxxxxxxxxxxxxxxxxxx>; Erick Archer > > <erick.archer@xxxxxxxxxxx>; Pavan Chebbi <pavan.chebbi@xxxxxxxxxxxx>; > > Ahmed Zaki <ahmed.zaki@xxxxxxxxx>; Colin Ian King > > <colin.i.king@xxxxxxxxx>; Shradha Gupta <shradhagupta@xxxxxxxxxxxxx> > > 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 > > gerhard@xxxxxxxxxxxxxxxxxxxxx. 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 <shradhagupta@xxxxxxxxxxxxxxxxxxx> > > > --- > > > .../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.
