| Message ID | 20240905094935.26271-3-gakula@marvell.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Refactoring RVU NIC driver | expand |
On Thu, Sep 5, 2024 at 3:21 PM Geetha sowjanya <gakula@marvell.com> wrote: > > Group the queue(RX/TX/CQ) memory allocation and free code to single APIs. > > Signed-off-by: Geetha sowjanya <gakula@marvell.com> > --- > .../marvell/octeontx2/nic/otx2_common.h | 2 + > .../ethernet/marvell/octeontx2/nic/otx2_pf.c | 54 +++++++++++++------ > 2 files changed, 40 insertions(+), 16 deletions(-) > Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
On 9/5/24 11:49, Geetha sowjanya wrote: > Group the queue(RX/TX/CQ) memory allocation and free code to single APIs. > > Signed-off-by: Geetha sowjanya <gakula@marvell.com> > --- > .../marvell/octeontx2/nic/otx2_common.h | 2 + > .../ethernet/marvell/octeontx2/nic/otx2_pf.c | 54 +++++++++++++------ > 2 files changed, 40 insertions(+), 16 deletions(-) > > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h > index a47001a2b93f..df548aeffecf 100644 > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h > @@ -997,6 +997,8 @@ int otx2_pool_init(struct otx2_nic *pfvf, u16 pool_id, > int otx2_aura_init(struct otx2_nic *pfvf, int aura_id, > int pool_id, int numptrs); > int otx2_init_rsrc(struct pci_dev *pdev, struct otx2_nic *pf); > +void otx2_free_queue_mem(struct otx2_qset *qset); > +int otx2_alloc_queue_mem(struct otx2_nic *pf); > > /* RSS configuration APIs*/ > int otx2_rss_init(struct otx2_nic *pfvf); > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c > index 4cfeca5ca626..6dfd6d1064ad 100644 > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c > @@ -1770,15 +1770,23 @@ static void otx2_dim_work(struct work_struct *w) > dim->state = DIM_START_MEASURE; > } > > -int otx2_open(struct net_device *netdev) > +void otx2_free_queue_mem(struct otx2_qset *qset) > +{ > + kfree(qset->sq); > + qset->sq = NULL; > + kfree(qset->cq); > + qset->cq = NULL; > + kfree(qset->rq); > + qset->rq = NULL; > + kfree(qset->napi); It's strange that the napi ptr is not reset here. You should add a comment describing the reason or zero such field, too. Thanks, Paolo
>-----Original Message----- >From: Paolo Abeni <pabeni@redhat.com> >Sent: Tuesday, September 10, 2024 3:11 PM >To: Geethasowjanya Akula <gakula@marvell.com>; netdev@vger.kernel.org; >linux-kernel@vger.kernel.org >Cc: kuba@kernel.org; davem@davemloft.net; jiri@resnulli.us; >edumazet@google.com; Sunil Kovvuri Goutham <sgoutham@marvell.com>; >Subbaraya Sundeep Bhatta <sbhatta@marvell.com>; Hariprasad Kelam ><hkelam@marvell.com> >Subject: [EXTERNAL] Re: [net-next PATCH v2 2/4] octeontx2-pf: Add new APIs >for queue memory alloc/free. >On 9/5/24 11:49, Geetha sowjanya wrote: >> Group the queue(RX/TX/CQ) memory allocation and free code to single APIs. >> >> Signed-off-by: Geetha sowjanya <gakula@marvell.com> >> --- >> .../marvell/octeontx2/nic/otx2_common.h | 2 + >> .../ethernet/marvell/octeontx2/nic/otx2_pf.c | 54 +++++++++++++------ >> 2 files changed, 40 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h >> b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h >> index a47001a2b93f..df548aeffecf 100644 >> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h >> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h >> @@ -997,6 +997,8 @@ int otx2_pool_init(struct otx2_nic *pfvf, u16 pool_id, >> int otx2_aura_init(struct otx2_nic *pfvf, int aura_id, >> int pool_id, int numptrs); >> int otx2_init_rsrc(struct pci_dev *pdev, struct otx2_nic *pf); >> +void otx2_free_queue_mem(struct otx2_qset *qset); int >> +otx2_alloc_queue_mem(struct otx2_nic *pf); >> >> /* RSS configuration APIs*/ >> int otx2_rss_init(struct otx2_nic *pfvf); diff --git >> a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c >> b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c >> index 4cfeca5ca626..6dfd6d1064ad 100644 >> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c >> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c >> @@ -1770,15 +1770,23 @@ static void otx2_dim_work(struct work_struct >*w) >> dim->state = DIM_START_MEASURE; >> } >> >> -int otx2_open(struct net_device *netdev) >> +void otx2_free_queue_mem(struct otx2_qset *qset) { >> + kfree(qset->sq); >> + qset->sq = NULL; >> + kfree(qset->cq); >> + qset->cq = NULL; >> + kfree(qset->rq); >> + qset->rq = NULL; >> + kfree(qset->napi); > >It's strange that the napi ptr is not reset here. You should add a comment >describing the reason or zero such field, too. "qset->napi " ptr get initialized and free on otx2_open/otx2_stop. This ptr is not referred if the interface is in down state. Hence, ptr reset is not needed. Will add proper comment in next version. > >Thanks, > >Paolo
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h index a47001a2b93f..df548aeffecf 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h @@ -997,6 +997,8 @@ int otx2_pool_init(struct otx2_nic *pfvf, u16 pool_id, int otx2_aura_init(struct otx2_nic *pfvf, int aura_id, int pool_id, int numptrs); int otx2_init_rsrc(struct pci_dev *pdev, struct otx2_nic *pf); +void otx2_free_queue_mem(struct otx2_qset *qset); +int otx2_alloc_queue_mem(struct otx2_nic *pf); /* RSS configuration APIs*/ int otx2_rss_init(struct otx2_nic *pfvf); diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c index 4cfeca5ca626..6dfd6d1064ad 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c @@ -1770,15 +1770,23 @@ static void otx2_dim_work(struct work_struct *w) dim->state = DIM_START_MEASURE; } -int otx2_open(struct net_device *netdev) +void otx2_free_queue_mem(struct otx2_qset *qset) +{ + kfree(qset->sq); + qset->sq = NULL; + kfree(qset->cq); + qset->cq = NULL; + kfree(qset->rq); + qset->rq = NULL; + kfree(qset->napi); +} +EXPORT_SYMBOL(otx2_free_queue_mem); + +int otx2_alloc_queue_mem(struct otx2_nic *pf) { - struct otx2_nic *pf = netdev_priv(netdev); - struct otx2_cq_poll *cq_poll = NULL; struct otx2_qset *qset = &pf->qset; - int err = 0, qidx, vec; - char *irq_name; + struct otx2_cq_poll *cq_poll; - netif_carrier_off(netdev); /* RQ and SQs are mapped to different CQs, * so find out max CQ IRQs (i.e CINTs) needed. @@ -1798,7 +1806,6 @@ int otx2_open(struct net_device *netdev) /* CQ size of SQ */ qset->sqe_cnt = qset->sqe_cnt ? qset->sqe_cnt : Q_COUNT(Q_SIZE_4K); - err = -ENOMEM; qset->cq = kcalloc(pf->qset.cq_cnt, sizeof(struct otx2_cq_queue), GFP_KERNEL); if (!qset->cq) @@ -1814,6 +1821,28 @@ int otx2_open(struct net_device *netdev) if (!qset->rq) goto err_free_mem; + return 0; + +err_free_mem: + otx2_free_queue_mem(qset); + return -ENOMEM; +} +EXPORT_SYMBOL(otx2_alloc_queue_mem); + +int otx2_open(struct net_device *netdev) +{ + struct otx2_nic *pf = netdev_priv(netdev); + struct otx2_cq_poll *cq_poll = NULL; + struct otx2_qset *qset = &pf->qset; + int err = 0, qidx, vec; + char *irq_name; + + netif_carrier_off(netdev); + + err = otx2_alloc_queue_mem(pf); + if (err) + return err; + err = otx2_init_hw_resources(pf); if (err) goto err_free_mem; @@ -1979,10 +2008,7 @@ int otx2_open(struct net_device *netdev) otx2_disable_napi(pf); otx2_free_hw_resources(pf); err_free_mem: - kfree(qset->sq); - kfree(qset->cq); - kfree(qset->rq); - kfree(qset->napi); + otx2_free_queue_mem(qset); return err; } EXPORT_SYMBOL(otx2_open); @@ -2047,11 +2073,7 @@ int otx2_stop(struct net_device *netdev) for (qidx = 0; qidx < netdev->num_tx_queues; qidx++) netdev_tx_reset_queue(netdev_get_tx_queue(netdev, qidx)); - - kfree(qset->sq); - kfree(qset->cq); - kfree(qset->rq); - kfree(qset->napi); + otx2_free_queue_mem(qset); /* Do not clear RQ/SQ ringsize settings */ memset_startat(qset, 0, sqe_cnt); return 0;
Group the queue(RX/TX/CQ) memory allocation and free code to single APIs. Signed-off-by: Geetha sowjanya <gakula@marvell.com> --- .../marvell/octeontx2/nic/otx2_common.h | 2 + .../ethernet/marvell/octeontx2/nic/otx2_pf.c | 54 +++++++++++++------ 2 files changed, 40 insertions(+), 16 deletions(-)