On 3/30/2021 10:45 AM, Jason Gunthorpe wrote:
On Thu, Mar 25, 2021 at 08:54:31AM -0700, Dave Jiang wrote:
Vinod,
The series fixes the various 'struct device' lifetime handling in the
idxd driver. The devm managed lifetime is incompatible with 'struct device'
objects that resides in the idxd context. Tested with
CONFIG_DEBUG_KOBJECT_RELEASE and address all issues from that.
Please consider for damengine/fixes for the 5.12-rc.
It seems like an improvement..
The flow around probe is still weird:
static int idxd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
idxd = idxd_alloc(pdev);
^^ but we don't device_initialize() this
rc = idxd_probe(idxd);
if (rc)
goto err;
err:
err_iomap:
idxd_free(idxd);
static int idxd_probe(struct idxd_device *idxd)
{
err_int:
idxd_free(idxd);
return rc;
So we call idxd_free twice on error unwinds, and that will
crash. Unify idxd_free() and idxd_conf_device_release() as
appropriate.
Confused why they are different, why are some of the kfrees missing
from the release function?
Call device_initialize in idxd_alloc() and make it so that the release
function works properly. Move all the error unwind put_device's to
idxd_pci_probe()
I think understand where you are pulling this towards. I think we'll
need to rework conf_dev initialization into the main initialization
rather than doing it at the end. Let me go rework that.
..
idxd->id = idr_alloc(&idxd_idrs[idxd->type], idxd, 0, 0, GFP_KERNEL);
Nothing ever reads the idxd_idr, so this should be an ida
Yep Dan pointed that out the other day. I have a patch that fixes it but
it's in the next patch series. I'll pull it out and into this one.
Jason