On Mon, Jan 27, 2020 at 01:09:46PM +0530, Devesh Sharma wrote: > On Sat, Jan 25, 2020 at 11:33 PM Jason Gunthorpe <jgg@xxxxxxxxxxxx> wrote: > > > > On Fri, Jan 24, 2020 at 12:52:40AM -0500, Devesh Sharma wrote: > > > static void bnxt_re_destroy_chip_ctx(struct bnxt_re_dev *rdev) > > > { > > > + struct bnxt_qplib_chip_ctx *chip_ctx; > > > + > > > + if (!rdev->chip_ctx) > > > + return; > > > + chip_ctx = rdev->chip_ctx; > > > + rdev->chip_ctx = NULL; > > > rdev->rcfw.res = NULL; > > > rdev->qplib_res.cctx = NULL; > > > + kfree(chip_ctx); > > > } > > > > Are you sure this kfree is late enough? I couldn't deduce if it was > > really safe to NULL chip_ctx here. > With the current design its okay to free this here because > bnxt_re_destroy_chip_ctx is indeed the last deallocation performed > before ib_device_dealloc() in any exit path. Further, the call to > bnxt_re_destroy_chip_ctx is protected by rtnl. ??? Why does device destroy path need global lock to already existing protection from ib_core? Thanks