Re: [PATCH 15/15] vfio/mdev: remove an extra parent kobject reference

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 7 Jul 2022 15:40:52 +0200
Christoph Hellwig <hch@xxxxxx> wrote:

> On Wed, Jul 06, 2022 at 11:38:33AM -0300, Jason Gunthorpe wrote:
> > On Wed, Jul 06, 2022 at 09:42:19AM +0200, Christoph Hellwig wrote:  
> > > The mdev_type already holds a reference to the parent through
> > > mdev_types_kset, so drop the extra reference.  
> > 
> > I would drop this patch, but at least the explanation needs tweaking..  
> 
> I'm fine with that.  Alex, any preferences?

Modulo the bogus kobject_put()s, this essentially reverts:

commit 9a302449a58d45d0ef2aab686f64b35919bc604c
Author: Jason Gunthorpe <jgg@xxxxxxxx>
Date:   Tue Apr 6 16:40:30 2021 -0300

    vfio/mdev: Add missing reference counting to mdev_type
    
    struct mdev_type holds a pointer to the kref'd object struct mdev_parent,
    but doesn't hold the kref. The lifetime of the parent becomes implicit
    because parent_remove_sysfs_files() is supposed to remove all the access
    before the parent can be freed, but this is very hard to reason about.
    
    Make it obviously correct by adding the missing get.
    
    Reviewed-by: Christoph Hellwig <hch@xxxxxx>
    Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
    Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>
    Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
    Message-Id: <7-v2-d36939638fc6+d54-vfio2_jgg@xxxxxxxxxx>
    Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>

Which we all seemed to think was a good thing 15 months ago.  It is
still difficult to reason when the mdev_type_ktype.release function
occurs relative to the parent reference held by the kset, but without
an explanation how we're safe, I'm ok with a little paranoia and
explicit references.  Thanks,

Alex




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux