On Mon, Jan 23, 2023 at 10:42 PM Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > On Mon, Jan 23, 2023 at 10:33:05PM -0800, Jakub Kicinski wrote: > > On Thu, 19 Jan 2023 22:05:28 -0800 Ajit Khaparde wrote: > > > @@ -13212,6 +13214,7 @@ static void bnxt_remove_one(struct pci_dev *pdev) > > > kfree(bp->rss_indir_tbl); > > > bp->rss_indir_tbl = NULL; > > > bnxt_free_port_stats(bp); > > > + bnxt_aux_priv_free(bp); > > > free_netdev(dev); > > > > You're still freeing the memory in which struct device sits regardless > > of its reference count. > > > > Greg, is it legal to call: > > > > auxiliary_device_delete(adev); // AKA device_del(&auxdev->dev); > > auxiliary_device_uninit(adev); // AKA put_device(&auxdev->dev); > > free(adev); // frees struct device > > Ick, the aux device release callback should be doing the freeing of the > memory, you shouldn't ever have to free it "manually" like this. To do > so would be a problem (i.e. the release callback would then free it > again, right?) Yikes! Thanks for the refresher. > > > ? I tried to explain this three times, maybe there's some wait during > > device_del() I'm not seeing which makes this safe :S Apologies. I thought since the driver was allocating the memory, it had to free it. I stand corrected. Thanks Ajit > > Nope, no intentional wait normally. You can add one by enabling a > debugging option to find all of the places where people are doing bad > things like this by delaying the freeing of the device memory until a > few seconds later, but that's generally not something you should run in > a production kernel as it finds all sorts of nasty bugs... > > thanks, > > greg k-h
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
