On Thu, Aug 29, 2024 at 09:00:05PM +0000, Haiyang Zhang wrote: > > > > -----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
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.
