* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.