From: "K. Y. Srinivasan" <[email protected]>
Miscellaneous fixes/enhancements.
K. Y. Srinivasan (1):
Tools: hv: Fix a bug in the key delete code
Michael Kelley (1):
Drivers: hv: vmbus: Fix synic per-cpu context initialization
Stephen Hemminger (3):
vmbus: add driver_override support
uio_hv_generic: increase size of receive and send buffers
uio_hv_generic: drop #ifdef DEBUG
Documentation/ABI/testing/sysfs-bus-vmbus | 21 ++++
drivers/hv/hv.c | 15 ++-
drivers/hv/vmbus_drv.c | 115 ++++++++++++++++++----
drivers/uio/uio_hv_generic.c | 7 +-
include/linux/hyperv.h | 1 +
tools/hv/hv_kvp_daemon.c | 2 +-
6 files changed, 134 insertions(+), 27 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-bus-vmbus
--
2.18.0
From: Stephen Hemminger <[email protected]>
When using DPDK there is significant performance boost by using
the largest possible send and receive buffer area.
Unfortunately, with UIO model there is not a good way to configure
this at run time. But it is okay to have a bigger buffer available
even if application only decides to use a smaller piece of it.
Signed-off-by: Stephen Hemminger <[email protected]>
Signed-off-by: K. Y. Srinivasan <[email protected]>
---
drivers/uio/uio_hv_generic.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
index c690d100adcd..35678d08d9d8 100644
--- a/drivers/uio/uio_hv_generic.c
+++ b/drivers/uio/uio_hv_generic.c
@@ -35,13 +35,13 @@
#include "../hv/hyperv_vmbus.h"
-#define DRIVER_VERSION "0.02.0"
+#define DRIVER_VERSION "0.02.1"
#define DRIVER_AUTHOR "Stephen Hemminger <sthemmin at microsoft.com>"
#define DRIVER_DESC "Generic UIO driver for VMBus devices"
#define HV_RING_SIZE 512 /* pages */
-#define SEND_BUFFER_SIZE (15 * 1024 * 1024)
-#define RECV_BUFFER_SIZE (15 * 1024 * 1024)
+#define SEND_BUFFER_SIZE (16 * 1024 * 1024)
+#define RECV_BUFFER_SIZE (31 * 1024 * 1024)
/*
* List of resources to be mapped to user space
--
2.18.0
From: "K. Y. Srinivasan" <[email protected]>
Fix a bug in the key delete code - the num_records range
from 0 to num_records-1.
Signed-off-by: K. Y. Srinivasan <[email protected]>
Reported-by: David Binderman <[email protected]>
Cc: <[email protected]>
---
tools/hv/hv_kvp_daemon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index dbf6e8bd98ba..bbb2a8ef367c 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -286,7 +286,7 @@ static int kvp_key_delete(int pool, const __u8 *key, int key_size)
* Found a match; just move the remaining
* entries up.
*/
- if (i == num_records) {
+ if (i == (num_records - 1)) {
kvp_file_info[pool].num_records--;
kvp_update_file(pool);
return 0;
--
2.18.0
From: Michael Kelley <[email protected]>
If hv_synic_alloc() errors out, the state of the per-cpu context
for some CPUs is unknown since the zero'ing is done as each
CPU is iterated over. In such case, hv_synic_cleanup() may try to
free memory based on uninitialized values. Fix this by zero'ing
the per-cpu context for all CPUs before doing any memory
allocations that might fail.
Signed-off-by: Michael Kelley <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: K. Y. Srinivasan <[email protected]>
---
drivers/hv/hv.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 748a1c4172a6..332d7c34be5c 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -189,6 +189,17 @@ static void hv_init_clockevent_device(struct clock_event_device *dev, int cpu)
int hv_synic_alloc(void)
{
int cpu;
+ struct hv_per_cpu_context *hv_cpu;
+
+ /*
+ * First, zero all per-cpu memory areas so hv_synic_free() can
+ * detect what memory has been allocated and cleanup properly
+ * after any failures.
+ */
+ for_each_present_cpu(cpu) {
+ hv_cpu = per_cpu_ptr(hv_context.cpu_context, cpu);
+ memset(hv_cpu, 0, sizeof(*hv_cpu));
+ }
hv_context.hv_numa_map = kcalloc(nr_node_ids, sizeof(struct cpumask),
GFP_KERNEL);
@@ -198,10 +209,8 @@ int hv_synic_alloc(void)
}
for_each_present_cpu(cpu) {
- struct hv_per_cpu_context *hv_cpu
- = per_cpu_ptr(hv_context.cpu_context, cpu);
+ hv_cpu = per_cpu_ptr(hv_context.cpu_context, cpu);
- memset(hv_cpu, 0, sizeof(*hv_cpu));
tasklet_init(&hv_cpu->msg_dpc,
vmbus_on_msg_dpc, (unsigned long) hv_cpu);
--
2.18.0
From: Stephen Hemminger <[email protected]>
DEBUG is leftover from the development phase, remove it.
Signed-off-by: Stephen Hemminger <[email protected]>
Signed-off-by: K. Y. Srinivasan <[email protected]>
---
drivers/uio/uio_hv_generic.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
index 35678d08d9d8..ab0c0e7e8a44 100644
--- a/drivers/uio/uio_hv_generic.c
+++ b/drivers/uio/uio_hv_generic.c
@@ -19,7 +19,6 @@
* # echo -n "ed963694-e847-4b2a-85af-bc9cfc11d6f3" \
* > /sys/bus/vmbus/drivers/uio_hv_generic/bind
*/
-#define DEBUG 1
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/device.h>
--
2.18.0
From: Stephen Hemminger <[email protected]>
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://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com%2Fdriverctl%2Fdriverctl&data=02%7C01%7Ckys%40microsoft.com%7C42e803feb2c544ef6ea908d5fd538878%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636693457619960040&sdata=kEyYHRIjNZCk%2B37moCSqbrZL426YccNQrsWpENcrZdw%3D&reserved=0
Signed-off-by: Stephen Hemminger <[email protected]>
Signed-off-by: K. Y. Srinivasan <[email protected]>
---
Documentation/ABI/testing/sysfs-bus-vmbus | 21 ++++
drivers/hv/vmbus_drv.c | 115 ++++++++++++++++++----
include/linux/hyperv.h | 1 +
3 files changed, 118 insertions(+), 19 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-bus-vmbus
diff --git a/Documentation/ABI/testing/sysfs-bus-vmbus b/Documentation/ABI/testing/sysfs-bus-vmbus
new file mode 100644
index 000000000000..91e6c065973c
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-vmbus
@@ -0,0 +1,21 @@
+What: /sys/bus/vmbus/devices/.../driver_override
+Date: August 2019
+Contact: Stephen Hemminger <[email protected]>
+Description:
+ This file allows the driver for a device to be specified which
+ will override standard static and dynamic ID matching. When
+ specified, only a driver with a name matching the value written
+ to driver_override will have an opportunity to bind to the
+ device. The override is specified by writing a string to the
+ driver_override file (echo uio_hv_generic > driver_override) and
+ may be cleared with an empty string (echo > driver_override).
+ This returns the device to standard matching rules binding.
+ Writing to driver_override does not automatically unbind the
+ device from its current driver or make any attempt to
+ automatically load the specified driver. If no driver with a
+ matching name is currently loaded in the kernel, the device
+ will not bind to any driver. This also allows devices to
+ opt-out of driver binding using a driver_override name such as
+ "none". Only a single driver may be specified in the override,
+ there is no support for parsing delimiters.
+
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;
+
+ driver_override = kstrndup(buf, count, GFP_KERNEL);
+ if (!driver_override)
+ return -ENOMEM;
+
+ cp = strchr(driver_override, '\n');
+ if (cp)
+ *cp = '\0';
+
+ device_lock(dev);
+ old = hv_dev->driver_override;
+ if (strlen(driver_override)) {
+ hv_dev->driver_override = driver_override;
+ } else {
+ kfree(driver_override);
+ hv_dev->driver_override = NULL;
+ }
+ device_unlock(dev);
+
+ kfree(old);
+
+ return count;
+}
+
+static ssize_t driver_override_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hv_device *hv_dev = device_to_hv_device(dev);
+ ssize_t len;
+
+ device_lock(dev);
+ len = snprintf(buf, PAGE_SIZE, "%s\n", hv_dev->driver_override);
+ device_unlock(dev);
+
+ return len;
+}
+static DEVICE_ATTR_RW(driver_override);
+
/* Set up per device attributes in /sys/bus/vmbus/devices/<bus device> */
static struct attribute *vmbus_dev_attrs[] = {
&dev_attr_id.attr,
@@ -528,6 +576,7 @@ static struct attribute *vmbus_dev_attrs[] = {
&dev_attr_channel_vp_mapping.attr,
&dev_attr_vendor.attr,
&dev_attr_device.attr,
+ &dev_attr_driver_override.attr,
NULL,
};
ATTRIBUTE_GROUPS(vmbus_dev);
@@ -563,17 +612,26 @@ static inline bool is_null_guid(const uuid_le *guid)
return true;
}
-/*
- * 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,
- const uuid_le *guid)
+static const struct hv_vmbus_device_id *
+hv_vmbus_dev_match(const struct hv_vmbus_device_id *id, const uuid_le *guid)
+
+{
+ if (id == NULL)
+ return NULL; /* empty device table */
+
+ for (; !is_null_guid(&id->guid); id++)
+ if (!uuid_le_cmp(id->guid, *guid))
+ return id;
+
+ return NULL;
+}
+
+static const struct hv_vmbus_device_id *
+hv_vmbus_dynid_match(struct hv_driver *drv, const uuid_le *guid)
{
const struct hv_vmbus_device_id *id = NULL;
struct vmbus_dynid *dynid;
- /* Look at the dynamic ids first, before the static ones */
spin_lock(&drv->dynids.lock);
list_for_each_entry(dynid, &drv->dynids.list, node) {
if (!uuid_le_cmp(dynid->id.guid, *guid)) {
@@ -583,18 +641,37 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(struct hv_driver *drv,
}
spin_unlock(&drv->dynids.lock);
- if (id)
- return id;
+ return id;
+}
- id = drv->id_table;
- if (id == NULL)
- return NULL; /* empty device table */
+static const struct hv_vmbus_device_id vmbus_device_null = {
+ .guid = NULL_UUID_LE,
+};
- for (; !is_null_guid(&id->guid); id++)
- if (!uuid_le_cmp(id->guid, *guid))
- return id;
+/*
+ * 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;
+
+ /* Look at the dynamic ids first, before the static ones */
+ id = hv_vmbus_dynid_match(drv, guid);
+ if (!id)
+ id = hv_vmbus_dev_match(drv->id_table, guid);
+
+ /* driver_override will always match, send a dummy id */
+ if (!id && dev->driver_override)
+ id = &vmbus_device_null;
+
+ return id;
}
/* vmbus_add_dynid - add a new device ID to this driver and re-probe devices */
@@ -643,7 +720,7 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,
if (retval)
return retval;
- if (hv_vmbus_get_id(drv, &guid))
+ if (hv_vmbus_dynid_match(drv, &guid))
return -EEXIST;
retval = vmbus_add_dynid(drv, &guid);
@@ -708,7 +785,7 @@ static int vmbus_match(struct device *device, struct device_driver *driver)
if (is_hvsock_channel(hv_dev->channel))
return drv->hvsock;
- if (hv_vmbus_get_id(drv, &hv_dev->dev_type))
+ if (hv_vmbus_get_id(drv, hv_dev))
return 1;
return 0;
@@ -725,7 +802,7 @@ static int vmbus_probe(struct device *child_device)
struct hv_device *dev = device_to_hv_device(child_device);
const struct hv_vmbus_device_id *dev_id;
- dev_id = hv_vmbus_get_id(drv, &dev->dev_type);
+ dev_id = hv_vmbus_get_id(drv, dev);
if (drv->probe) {
ret = drv->probe(dev, dev_id);
if (ret != 0)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index efda23cf32c7..2c3798bcb01c 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1125,6 +1125,7 @@ struct hv_device {
u16 device_id;
struct device device;
+ char *driver_override; /* Driver name to force a match */
struct vmbus_channel *channel;
struct kset *channels_kset;
--
2.18.0
From: [email protected] <[email protected]> Sent: Friday, August 10, 2018 4:06 PM
>
> Fix a bug in the key delete code - the num_records range
> from 0 to num_records-1.
>
> Signed-off-by: K. Y. Srinivasan <[email protected]>
> Reported-by: David Binderman <[email protected]>
> Cc: <[email protected]>
> ---
Reviewed-by: Michael Kelley <[email protected]>
From: [email protected] <[email protected]> Sent: Friday, August 10, 2018 4:06 PM
> From: Stephen Hemminger <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: K. Y. Srinivasan <[email protected]>
> ---
> 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
On Mon, Aug 13, 2018 at 07:30:50PM +0000, Michael Kelley (EOSG) wrote:
> From: [email protected] <[email protected]> Sent: Friday, August 10, 2018 4:06 PM
>
> > From: Stephen Hemminger <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: K. Y. Srinivasan <[email protected]>
> > ---
> > 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.
sysfs buffers are PAGE_SIZE big.
On Mon, 13 Aug 2018 19:30:50 +0000
"Michael Kelley (EOSG)" <[email protected]> wrote:
> From: [email protected] <[email protected]> Sent: Friday, August 10, 2018 4:06 PM
>
> > From: Stephen Hemminger <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: K. Y. Srinivasan <[email protected]>
> > ---
> > 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.
On Mon, 13 Aug 2018 19:30:50 +0000
"Michael Kelley (EOSG)" <[email protected]> wrote:
> > +/*
> > + * 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 function hv_vmbus_get_id is called from that path.
i.e. __device_attach -> driver-match_device -> vmbus_match.
and __device_attach always does:
device_lock(dev);
The code in driver _override_store uses the same device_lock
when storing the new value.
This is same locking as is done in pci-sysfs.c
From: Stephen Hemminger <[email protected]> Sent: Tuesday, August 14, 2018 9:35 AM
> On Mon, 13 Aug 2018 19:30:50 +0000
> "Michael Kelley (EOSG)" <[email protected]> wrote:
>
> > > +/*
> > > + * 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 function hv_vmbus_get_id is called from that path.
> i.e. __device_attach -> driver-match_device -> vmbus_match.
> and __device_attach always does:
> device_lock(dev);
Agreed. The __device_attach() path holds the device lock and all is good.
But the __driver_attach() path does not hold the device lock when the
match function is called, leaving the code open to a potential race. Same
problem could happen in the pci subsystem, so the issue is more generic
and probably should be evaluated and dealt with separately.
Michael
>
> The code in driver _override_store uses the same device_lock
> when storing the new value.
>
> This is same locking as is done in pci-sysfs.c