s390 common I/O layer locking (was: [PATCH v2 07/13] vfio/ccw: Convert to use vfio_register_group_dev())

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

 



On Fri, 30 Apr 2021 14:19:08 -0300
Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:

> On Fri, Apr 30, 2021 at 02:31:40PM +0200, Cornelia Huck wrote:
> > On Thu, 29 Apr 2021 15:13:47 -0300
> > Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:

> > > All the checks for !private need some kind of locking. The driver core
> > > model is that the 'struct device_driver' callbacks are all called
> > > under the device_lock (this prevents the driver unbinding during the
> > > callback). I didn't check if ccs does this or not..  
> > 
> > probe/remove/shutdown are basically a forward of the callbacks at the
> > bus level.  
> 
> These are all covered by device_lock
> 
> > The css bus should make sure that we serialize
> > irq/sch_event/chp_event with probe/remove.  
> 
> Hum it doesn't look OK, like here:
> 
> css_process_crw()
>   css_evaluate_subchannel()
>    sch = bus_find_device()
>       -- So we have a refcount on the struct device
>    css_evaluate_known_subchannel() {
> 	if (sch->driver) {
> 		if (sch->driver->sch_event)
> 			ret = sch->driver->sch_event(sch, slow);
>    }
> 
> But the above call and touches to sch->driver (which is really just
> sch->dev.driver) are unlocked and racy.
> 
> I would hold the device_lock() over all touches to sch->driver outside
> of a driver core callback.

I think this issue did not come up much before, as most drivers on the
css bus tend to stay put during the lifetime of the device; but yes, it
seems we're missing some locking.

For the css bus, we need locking for the event callbacks; for irq, this
may interact with the subchannel lock and likely needs some care.

I also looked at the other busses in the common I/O layer: scm looks
good at a glance, ccwgroup and ccw have locking for online/offline; the
other callbacks for the ccw drivers probably need to take the device
lock as well.

Common I/O layer maintainers, does that look right?




[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