+Michael Chan On Tue, Nov 12, 2024 at 1:47 PM Leon Romanovsky <leon@xxxxxxxxxx> wrote: > > On Fri, Nov 08, 2024 at 12:42:39AM -0800, Selvin Xavier wrote: > > From: Kalesh AP <kalesh-anakkur.purayil@xxxxxxxxxxxx> > > > > Move the logic to setup and enable NQs to a new function. > > Similarly moved the NQ cleanup logic to a common function. > > Introdued a flag to keep track of NQ allocation status > > and added sanity checks inside bnxt_re_stop_irq() and > > bnxt_re_start_irq() to avoid possible race conditions. > > > > Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@xxxxxxxxxxxx> > > Signed-off-by: Selvin Xavier <selvin.xavier@xxxxxxxxxxxx> > > --- > > drivers/infiniband/hw/bnxt_re/bnxt_re.h | 2 + > > drivers/infiniband/hw/bnxt_re/main.c | 204 +++++++++++++++++++------------- > > 2 files changed, 123 insertions(+), 83 deletions(-) > > <...> > > > > > + rtnl_lock(); > > + if (test_and_clear_bit(BNXT_RE_FLAG_SETUP_NQ, &rdev->flags)) > > + bnxt_re_clean_nqs(rdev); > > + rtnl_unlock(); > > <...> > > > + rtnl_lock(); > > bnxt_qplib_free_ctx(&rdev->qplib_res, &rdev->qplib_ctx); > > bnxt_qplib_disable_rcfw_channel(&rdev->rcfw); > > type = bnxt_qplib_get_ring_type(rdev->chip_ctx); > > bnxt_re_net_ring_free(rdev, rdev->rcfw.creq.ring_id, type); > > + rtnl_unlock(); > > Please don't add rtnl_lock() to drivers in RDMA subsystem. BNXT driver > is managed through netdev and it is there all proper locking should be > done. The main reason for bnxt_re to take the rtnl is because of the MSIx resource configuration. This is because the NIC driver is dynamically modifying the MSIx table when the number of ring change or ndo->open/close is invoked. So we stop and restart the interrupts of RoCE also with rtnl held. > > Please work to remove existing rtnl_lock() from bnxt_re_update_en_info_rdev() too. > IMHO that lock is not needed after your driver conversion to auxbus. This check is also to synchronize between the irq_stop and restart implementation between bnxt_en and bnxt_re driver and roce driver unload. We will review this locking and see if we can handle it. But it is a major design change in both L2 and roce drivers. > > Thanks
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
