* [PATCH 0/5] Miscellaneous fixes/enhancements @ 2018-08-10 23:05 kys 2018-08-10 23:06 ` [PATCH 1/5] Tools: hv: Fix a bug in the key delete code kys 0 siblings, 1 reply; 12+ messages in thread From: kys @ 2018-08-10 23:05 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, jasowang, sthemmin, Michael.H.Kelley, vkuznets Cc: K. Y. Srinivasan From: "K. Y. Srinivasan" <kys@microsoft.com> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/5] Tools: hv: Fix a bug in the key delete code 2018-08-10 23:05 [PATCH 0/5] Miscellaneous fixes/enhancements kys @ 2018-08-10 23:06 ` kys 2018-08-10 23:06 ` [PATCH 2/5] vmbus: add driver_override support kys ` (4 more replies) 0 siblings, 5 replies; 12+ messages in thread From: kys @ 2018-08-10 23:06 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, jasowang, sthemmin, Michael.H.Kelley, vkuznets Cc: K. Y. Srinivasan, stable From: "K. Y. Srinivasan" <kys@microsoft.com> Fix a bug in the key delete code - the num_records range from 0 to num_records-1. Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> Reported-by: David Binderman <dcb314@hotmail.com> Cc: <stable@vger.kernel.org> --- 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/5] vmbus: add driver_override support 2018-08-10 23:06 ` [PATCH 1/5] Tools: hv: Fix a bug in the key delete code kys @ 2018-08-10 23:06 ` kys 2018-08-13 19:30 ` Michael Kelley (EOSG) 2018-08-10 23:06 ` [PATCH 3/5] uio_hv_generic: increase size of receive and send buffers kys ` (3 subsequent siblings) 4 siblings, 1 reply; 12+ messages in thread From: kys @ 2018-08-10 23:06 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, jasowang, sthemmin, Michael.H.Kelley, vkuznets Cc: Stephen Hemminger, K . Y . Srinivasan From: Stephen Hemminger <stephen@networkplumber.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://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 <sthemmin@microsoft.com> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> --- 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 <sthemmin@microsoft.com> +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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: [PATCH 2/5] vmbus: add driver_override support 2018-08-10 23:06 ` [PATCH 2/5] vmbus: add driver_override support kys @ 2018-08-13 19:30 ` Michael Kelley (EOSG) 2018-08-13 19:40 ` gregkh ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Michael Kelley (EOSG) @ 2018-08-13 19:30 UTC (permalink / raw) To: KY Srinivasan, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, Stephen Hemminger, vkuznets@redhat.com Cc: Stephen Hemminger From: kys@linuxonhyperv.com <kys@linuxonhyperv.com> Sent: Friday, August 10, 2018 4:06 PM > From: Stephen Hemminger <stephen@networkplumber.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@microsoft.com> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/5] vmbus: add driver_override support 2018-08-13 19:30 ` Michael Kelley (EOSG) @ 2018-08-13 19:40 ` gregkh 2018-08-13 19:56 ` Stephen Hemminger 2018-08-14 16:35 ` Stephen Hemminger 2 siblings, 0 replies; 12+ messages in thread From: gregkh @ 2018-08-13 19:40 UTC (permalink / raw) To: Michael Kelley (EOSG) Cc: KY Srinivasan, linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, Stephen Hemminger, vkuznets@redhat.com On Mon, Aug 13, 2018 at 07:30:50PM +0000, Michael Kelley (EOSG) wrote: > From: kys@linuxonhyperv.com <kys@linuxonhyperv.com> Sent: Friday, August 10, 2018 4:06 PM > > > From: Stephen Hemminger <stephen@networkplumber.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@microsoft.com> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.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. sysfs buffers are PAGE_SIZE big. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/5] vmbus: add driver_override support 2018-08-13 19:30 ` Michael Kelley (EOSG) 2018-08-13 19:40 ` gregkh @ 2018-08-13 19:56 ` Stephen Hemminger 2018-08-14 16:35 ` Stephen Hemminger 2 siblings, 0 replies; 12+ messages in thread From: Stephen Hemminger @ 2018-08-13 19:56 UTC (permalink / raw) To: Michael Kelley (EOSG) Cc: KY Srinivasan, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, Stephen Hemminger, vkuznets@redhat.com On Mon, 13 Aug 2018 19:30:50 +0000 "Michael Kelley (EOSG)" <Michael.H.Kelley@microsoft.com> wrote: > From: kys@linuxonhyperv.com <kys@linuxonhyperv.com> Sent: Friday, August 10, 2018 4:06 PM > > > From: Stephen Hemminger <stephen@networkplumber.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@microsoft.com> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.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. 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. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/5] vmbus: add driver_override support 2018-08-13 19:30 ` Michael Kelley (EOSG) 2018-08-13 19:40 ` gregkh 2018-08-13 19:56 ` Stephen Hemminger @ 2018-08-14 16:35 ` Stephen Hemminger 2018-08-14 19:13 ` Michael Kelley (EOSG) 2 siblings, 1 reply; 12+ messages in thread From: Stephen Hemminger @ 2018-08-14 16:35 UTC (permalink / raw) To: Michael Kelley (EOSG) Cc: KY Srinivasan, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, Stephen Hemminger, vkuznets@redhat.com On Mon, 13 Aug 2018 19:30:50 +0000 "Michael Kelley (EOSG)" <Michael.H.Kelley@microsoft.com> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 2/5] vmbus: add driver_override support 2018-08-14 16:35 ` Stephen Hemminger @ 2018-08-14 19:13 ` Michael Kelley (EOSG) 0 siblings, 0 replies; 12+ messages in thread From: Michael Kelley (EOSG) @ 2018-08-14 19:13 UTC (permalink / raw) To: Stephen Hemminger Cc: KY Srinivasan, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, Stephen Hemminger, vkuznets From: Stephen Hemminger <stephen@networkplumber.org> Sent: Tuesday, August 14, 2018 9:35 AM > On Mon, 13 Aug 2018 19:30:50 +0000 > "Michael Kelley (EOSG)" <Michael.H.Kelley@microsoft.com> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/5] uio_hv_generic: increase size of receive and send buffers 2018-08-10 23:06 ` [PATCH 1/5] Tools: hv: Fix a bug in the key delete code kys 2018-08-10 23:06 ` [PATCH 2/5] vmbus: add driver_override support kys @ 2018-08-10 23:06 ` kys 2018-08-10 23:06 ` [PATCH 4/5] uio_hv_generic: drop #ifdef DEBUG kys ` (2 subsequent siblings) 4 siblings, 0 replies; 12+ messages in thread From: kys @ 2018-08-10 23:06 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, jasowang, sthemmin, Michael.H.Kelley, vkuznets Cc: Stephen Hemminger, K . Y . Srinivasan From: Stephen Hemminger <stephen@networkplumber.org> 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 <sthemmin@microsoft.com> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> --- 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/5] uio_hv_generic: drop #ifdef DEBUG 2018-08-10 23:06 ` [PATCH 1/5] Tools: hv: Fix a bug in the key delete code kys 2018-08-10 23:06 ` [PATCH 2/5] vmbus: add driver_override support kys 2018-08-10 23:06 ` [PATCH 3/5] uio_hv_generic: increase size of receive and send buffers kys @ 2018-08-10 23:06 ` kys 2018-08-10 23:06 ` [PATCH 5/5] Drivers: hv: vmbus: Fix synic per-cpu context initialization kys 2018-08-13 16:41 ` [PATCH 1/5] Tools: hv: Fix a bug in the key delete code Michael Kelley (EOSG) 4 siblings, 0 replies; 12+ messages in thread From: kys @ 2018-08-10 23:06 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, jasowang, sthemmin, Michael.H.Kelley, vkuznets Cc: Stephen Hemminger, K . Y . Srinivasan From: Stephen Hemminger <stephen@networkplumber.org> DEBUG is leftover from the development phase, remove it. Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> --- 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/5] Drivers: hv: vmbus: Fix synic per-cpu context initialization 2018-08-10 23:06 ` [PATCH 1/5] Tools: hv: Fix a bug in the key delete code kys ` (2 preceding siblings ...) 2018-08-10 23:06 ` [PATCH 4/5] uio_hv_generic: drop #ifdef DEBUG kys @ 2018-08-10 23:06 ` kys 2018-08-13 16:41 ` [PATCH 1/5] Tools: hv: Fix a bug in the key delete code Michael Kelley (EOSG) 4 siblings, 0 replies; 12+ messages in thread From: kys @ 2018-08-10 23:06 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, jasowang, sthemmin, Michael.H.Kelley, vkuznets Cc: Michael Kelley, K . Y . Srinivasan From: Michael Kelley <mikelley@microsoft.com> 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 <mikelley@microsoft.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> --- 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: [PATCH 1/5] Tools: hv: Fix a bug in the key delete code 2018-08-10 23:06 ` [PATCH 1/5] Tools: hv: Fix a bug in the key delete code kys ` (3 preceding siblings ...) 2018-08-10 23:06 ` [PATCH 5/5] Drivers: hv: vmbus: Fix synic per-cpu context initialization kys @ 2018-08-13 16:41 ` Michael Kelley (EOSG) 4 siblings, 0 replies; 12+ messages in thread From: Michael Kelley (EOSG) @ 2018-08-13 16:41 UTC (permalink / raw) To: KY Srinivasan, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, Stephen Hemminger, vkuznets@redhat.com Cc: stable@vger.kernel.org From: kys@linuxonhyperv.com <kys@linuxonhyperv.com> 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 <kys@microsoft.com> > Reported-by: David Binderman <dcb314@hotmail.com> > Cc: <stable@vger.kernel.org> > --- Reviewed-by: Michael Kelley <mikelley@microsoft.com> ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-08-14 19:13 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-08-10 23:05 [PATCH 0/5] Miscellaneous fixes/enhancements kys 2018-08-10 23:06 ` [PATCH 1/5] Tools: hv: Fix a bug in the key delete code kys 2018-08-10 23:06 ` [PATCH 2/5] vmbus: add driver_override support kys 2018-08-13 19:30 ` Michael Kelley (EOSG) 2018-08-13 19:40 ` gregkh 2018-08-13 19:56 ` Stephen Hemminger 2018-08-14 16:35 ` Stephen Hemminger 2018-08-14 19:13 ` Michael Kelley (EOSG) 2018-08-10 23:06 ` [PATCH 3/5] uio_hv_generic: increase size of receive and send buffers kys 2018-08-10 23:06 ` [PATCH 4/5] uio_hv_generic: drop #ifdef DEBUG kys 2018-08-10 23:06 ` [PATCH 5/5] Drivers: hv: vmbus: Fix synic per-cpu context initialization kys 2018-08-13 16:41 ` [PATCH 1/5] Tools: hv: Fix a bug in the key delete code Michael Kelley (EOSG)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox