[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CY4PR21MB07739595C31B9B9349604C0ADC390@CY4PR21MB0773.namprd21.prod.outlook.com>
Date: Mon, 13 Aug 2018 19:30:50 +0000
From: "Michael Kelley (EOSG)" <Michael.H.Kelley@...rosoft.com>
To: KY Srinivasan <kys@...rosoft.com>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
"olaf@...fle.de" <olaf@...fle.de>,
"apw@...onical.com" <apw@...onical.com>,
"jasowang@...hat.com" <jasowang@...hat.com>,
Stephen Hemminger <sthemmin@...rosoft.com>,
"vkuznets@...hat.com" <vkuznets@...hat.com>
CC: Stephen Hemminger <stephen@...workplumber.org>
Subject: RE: [PATCH 2/5] vmbus: add driver_override support
From: kys@...uxonhyperv.com <kys@...uxonhyperv.com> Sent: Friday, August 10, 2018 4:06 PM
> From: Stephen Hemminger <stephen@...workplumber.org>
>
> 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@...rosoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@...rosoft.com>
> ---
> 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.
> +/*
> + * 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().
Michael
Powered by blists - more mailing lists