On Mon, 13 Aug 2018 19:30:50 +0000 "Michael Kelley (EOSG)" <Michael.H.Kelley@xxxxxxxxxxxxx> wrote: > From: kys@xxxxxxxxxxxxxxxxx <kys@xxxxxxxxxxxxxxxxx> Sent: Friday, August 10, 2018 4:06 PM > > > From: Stephen Hemminger <stephen@xxxxxxxxxxxxxxxxxx> > > > > Add support for overriding the default driver for a VMBus device > > in the same way that it can be done for PCI devices. This patch > > adds the /sys/bus/vmbus/devices/.../driver_override file > > and the logic for matching. > > > > This is used by driverctl tool to do driver override. > > https://gitlab.com/driverctl/driverctl > > > > Signed-off-by: Stephen Hemminger <sthemmin@xxxxxxxxxxxxx> > > Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx> > > --- > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > > index b1b548a21f91..e6d8fdac6d8b 100644 > > --- a/drivers/hv/vmbus_drv.c > > +++ b/drivers/hv/vmbus_drv.c > > @@ -498,6 +498,54 @@ static ssize_t device_show(struct device *dev, > > } > > static DEVICE_ATTR_RO(device); > > > > +static ssize_t driver_override_store(struct device *dev, > > + struct device_attribute *attr, > > + const char *buf, size_t count) > > +{ > > + struct hv_device *hv_dev = device_to_hv_device(dev); > > + char *driver_override, *old, *cp; > > + > > + /* We need to keep extra room for a newline */ > > + if (count >= (PAGE_SIZE - 1)) > > + return -EINVAL; > > Does 'count' actually have a relationship to PAGE_SIZE, or > is PAGE_SIZE just used as an arbitrary size limit? I'm > wondering what happens on ARM64 with a 64K page size, > for example. If it's just arbitrary, coding such a constant > would be better. This comes from original code how sysfs works. Sysfs uses PAGE_SIZE for string buffers on store. This code snippet was cloned from PCI version of same thing. > > +/* > > + * Return a matching hv_vmbus_device_id pointer. > > + * If there is no match, return NULL. > > + */ > > +static const struct hv_vmbus_device_id *hv_vmbus_get_id(struct hv_driver *drv, > > + struct hv_device *dev) > > +{ > > + const uuid_le *guid = &dev->dev_type; > > + const struct hv_vmbus_device_id *id; > > > > - return NULL; > > + /* When driver_override is set, only bind to the matching driver */ > > + if (dev->driver_override && strcmp(dev->driver_override, drv->name)) > > + return NULL; > > This function needs to be covered by the device lock, so that > dev->driver_override can't be set to NULL and the memory freed > during the above 'if' statement. When called from vmbus_probe(), > the device lock is held, so it's good. But when called from > vmbus_match(), the device lock may not be held: consider the path > __driver_attach() -> driver_match_device() -> vmbus_match(). The patch clones existing locking model from PCI. So either both are broken, or somehow vmbus is behaving differently. I will investigate. _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
