RE: [PATCH 1/1] Drivers: hv: vmbus: Fix rescind handling issues
> -Original Message-
> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> Sent: Monday, September 18, 2017 5:55 AM
> To: KY Srinivasan
> Cc: Stephen Hemminger ;
> leann.ogasaw...@canonical.com; Stephen Hemminger
> ; a...@canonical.com; o...@aepfle.de;
> marcelo.ce...@canonical.com; gre...@linuxfoundation.org; Haiyang Zhang
> ; linux-ker...@vger.kernel.org;
> jasow...@redhat.com; de...@linuxdriverproject.org
> Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Fix rescind handling issues
>
> Vitaly Kuznetsov writes:
>
> >
> > Reverting 6f3d791f300618caf82a2be0c27456edd76d5164 still helps.
>
> In addition to the above I got the following crash while playing with
> 4.14-rc1 (unmodified):
>
> [ 55.810080] kernel tried to execute NX-protected page - exploit attempt?
> (uid: 0)
> [ 55.814293] BUG: unable to handle kernel paging request at
> 8800059985f0
> [ 55.818065] IP: 0x8800059985f0
> [ 55.819925] PGD 22eb067 P4D 22eb067 PUD 22ec067 PMD 5f37063 PTE
> 85998163
> [ 55.820018] Oops: 0011 [#1] SMP
> [ 55.820018] Modules linked in: vfat fat bnx2x mdio efi_pstore hv_utils
> efivars pci_hyperv ptp pps_core pcspkr hv_balloon xfs libcrc32c hv_storvsc
> hyperv_fb hv_netvsc scsi_transport_fc hid_hyperv hyperv_keyboard
> hv_vmbus
> [ 55.834837] CPU: 0 PID: 498 Comm: kworker/0:2 Not tainted 4.14.0-rc1 #63
> [ 55.834837] Hardware name: Microsoft Corporation Virtual Machine/Virtual
> Machine, BIOS Hyper-V UEFI Release v1.0 11/26/2012
> [ 55.834837] Workqueue: events vmbus_onmessage_work [hv_vmbus]
> [ 55.834837] task: 88003f448000 task.stack: c90005398000
> [ 55.834837] RIP: 0010:0x8800059985f0
> [ 55.834837] RSP: 0018:c9000539be00 EFLAGS: 00010286
> [ 55.834837] RAX: 880005998010 RBX: 880005998000 RCX:
>
> [ 55.834837] RDX: 8800059985f0 RSI: 0246 RDI:
> 880005998000
> [ 55.860040] RBP: c9000539be18 R08: 02e6 R09:
>
> [ 55.865057] R10: c9000539bdf0 R11: a000 R12:
> 0286
> [ 55.865057] R13: 88007ae1ed00 R14: R15:
> 8800065c3200
> [ 55.865057] FS: () GS:88007ae0()
> knlGS:
> [ 55.865057] CS: 0010 DS: ES: CR0: 80050033
> [ 55.865057] CR2: 8800059985f0 CR3: 075a5000 CR4:
> 001406f0
> [ 55.886745] Call Trace:
> [ 55.886745] ? vmbus_onoffer_rescind+0xfa/0x160 [hv_vmbus]
> [ 55.890968] vmbus_onmessage+0x2a/0x90 [hv_vmbus]
> [ 55.891934] vmbus_onmessage_work+0x1d/0x30 [hv_vmbus]
> [ 55.891934] process_one_work+0x193/0x390
> [ 55.891934] worker_thread+0x48/0x3c0
> [ 55.891934] kthread+0x120/0x140
> [ 55.891934] ? process_one_work+0x390/0x390
> [ 55.891934] ? kthread_create_on_node+0x60/0x60
> [ 55.891934] ret_from_fork+0x25/0x30
> [ 55.891934] Code: 88 ff ff c0 85 99 05 00 88 ff ff d0 85 99 05 00 88 ff ff
> d0 85 99
> 05 00 88 ff ff e0 85 99 05 00 88 ff ff e0 85 99 05 00 88 ff ff 85 99 05
> 00 88 ff
> ff f0 85 99 05 00 88 ff ff 00 86 99 05 00
> [ 55.922505] RIP: 0x8800059985f0 RSP: c9000539be00
> [ 55.922505] CR2: 8800059985f0
> [ 55.922505] ---[ end trace 25226e00af3f94fb ]---
> [ 55.933590] Kernel panic - not syncing: Fatal exception
> [ 55.933590] Kernel Offset: disabled
> [ 55.933590] ---[ end Kernel panic - not syncing: Fatal exception
>
> So it seems that during
>
> while (READ_ONCE(channel->probe_done) == false) {
> /*
> * We wait here until any channel offer is currently
> * being processed.
> */
> msleep(1);
> }
>
> loop the channel disappeared. The issue may not be related to the netvsc
> hang I mentioned before. It may make sense to do refcounting for
> channels/subchannels (or employ RCU).
I will work on this issue and get you a patch to try.
K. Y
>
> --
> Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: fix type mismatch warning
On Mon, Sep 18, 2017 at 6:23 PM, Christoph Hellwig wrote: > On Mon, Sep 18, 2017 at 05:53:47PM +0200, Greg Kroah-Hartman wrote: >> > It seems like a legitimate use case of the binder modules, but >> > now there is a kernel Kconfig option that has to match a user >> > space binary. >> >> So, should we revert that? >> >> I don't really know what to suggest here, sorry... > > Keep it as is for now, and encourage people to fix it. But we should > reject any new patch that makes this worse or adds additional ioctls > that don't follow our normal model. I would argue we should leave it as non-configurable, but make a conscious decision which ABI to use for 4.14, as that is going to be in a the next generation of Android devices: a) leave the BINDER_CURRENT_PROTOCOL_VERSION==7 interface supported in all 32-bit builds forever, remaining compatible the ABI that was supported in mainline Linux in all versions until v4.13. The version 7 interface is incompatible between 32-bit and 64-bit user space, and there is no compat_ioctl implementation for it, so someone should fix it by implementing a proper compat_ioctl function. The current Kconfig comment says that v7 of the ABI is also incompatible with Android 4.5 and later user space. Can someone confirm that? The code looks like it's written under the assumption that user space would dynamically pick between v7 and v8 based on the return value of ioctl(..., BINDER_VERSION). We could also add support for switching the ABI dynamically in the kernel module based on either a module parameter or an ioctl that a future version of the binder user space can call, to add optional support for the v8 ABI on 64-bit. b) Treat the fact that we implemented the v7 binder ABI as a bug, since real Android uses v8, removing all traces of the v7 code from the kernel, and requiring users of Android 4.4 and earlier to upgrade their binder to a version that runs on modern kernels. We can probably get away with that because - Neither arm64 nor x86_64 are affected by this change. - "anbox" would normally only be used on x86_64, but not i386 or arm32 - we probably already lost support for Android 4.4 and earlier due to other kernel changes - any arm32 user space that people actually try to run with a mainline kernel should already support the v8 ABI, and might not support the v7 ABI at all. - we don't support v1 through v6 of the interface either. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: fix type mismatch warning
On Mon, Sep 18, 2017 at 09:23:03AM -0700, Christoph Hellwig wrote: > On Mon, Sep 18, 2017 at 05:53:47PM +0200, Greg Kroah-Hartman wrote: > > > It seems like a legitimate use case of the binder modules, but > > > now there is a kernel Kconfig option that has to match a user > > > space binary. > > > > So, should we revert that? > > > > I don't really know what to suggest here, sorry... > > Keep it as is for now, and encourage people to fix it. But we should > reject any new patch that makes this worse or adds additional ioctls > that don't follow our normal model. That sounds reasonable to me, thanks. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: fix type mismatch warning
On Mon, Sep 18, 2017 at 05:53:47PM +0200, Greg Kroah-Hartman wrote: > > It seems like a legitimate use case of the binder modules, but > > now there is a kernel Kconfig option that has to match a user > > space binary. > > So, should we revert that? > > I don't really know what to suggest here, sorry... Keep it as is for now, and encourage people to fix it. But we should reject any new patch that makes this worse or adds additional ioctls that don't follow our normal model. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: fix type mismatch warning
On Mon, Sep 18, 2017 at 05:35:03PM +0200, Arnd Bergmann wrote: > On Mon, Sep 18, 2017 at 4:02 PM, Greg Kroah-Hartman > wrote: > >> However, there is another problem with the Kconfig option: turning > >> it on or off creates two incompatible ABI versions, a kernel that > >> has this enabled cannot run user space that was built without it > >> or vice versa. A better solution might be to leave the option hidden > >> until the binder code is fixed to deal with both ABI versions. > > > > I don't know if that is ever going to be fixed, as it's not an issue > > that you will ever see "in the wild" from what I can tell... > > Probably not on a native Android device or even a Chromebook that > ships a binder user space together with a kernel, but what about > people using "anbox" or similar projects that allow you to run > Android apps in a container? Ugh, that's crazy, binder should not be run that way due to a variety of issues... Well, as of 4.14 those issues will be gone, so no one is doing this yet :) > It seems like a legitimate use case of the binder modules, but > now there is a kernel Kconfig option that has to match a user > space binary. So, should we revert that? I don't really know what to suggest here, sorry... greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: fix type mismatch warning
On Mon, Sep 18, 2017 at 05:35:03PM +0200, Arnd Bergmann wrote: > Probably not on a native Android device or even a Chromebook that > ships a binder user space together with a kernel, but what about > people using "anbox" or similar projects that allow you to run > Android apps in a container? > > It seems like a legitimate use case of the binder modules, but > now there is a kernel Kconfig option that has to match a user > space binary. Agreed. It's also against our general policy for userspace interfaces. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: fix type mismatch warning
On Mon, Sep 18, 2017 at 4:02 PM, Greg Kroah-Hartman wrote: >> However, there is another problem with the Kconfig option: turning >> it on or off creates two incompatible ABI versions, a kernel that >> has this enabled cannot run user space that was built without it >> or vice versa. A better solution might be to leave the option hidden >> until the binder code is fixed to deal with both ABI versions. > > I don't know if that is ever going to be fixed, as it's not an issue > that you will ever see "in the wild" from what I can tell... Probably not on a native Android device or even a Chromebook that ships a binder user space together with a kernel, but what about people using "anbox" or similar projects that allow you to run Android apps in a container? It seems like a legitimate use case of the binder modules, but now there is a kernel Kconfig option that has to match a user space binary. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: ion: create one device entry per heap
Instead a getting one common device "/dev/ion" for
all the heaps this patch allow to create one device
entry ("/dev/ionX") per heap.
Getting an entry per heap could allow to set security rules
per heap and global ones for all heaps.
Allocation requests will be only allowed if the mask_id
match with device minor.
Query request could be done on any of the devices.
Signed-off-by: Benjamin Gaignard
---
drivers/staging/android/ion/ion-ioctl.c | 9 +++--
drivers/staging/android/ion/ion.c | 20 ++--
drivers/staging/android/ion/ion.h | 10 +++---
3 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index d9f8b14..ff06ce3 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -25,9 +25,11 @@ union ion_ioctl_arg {
struct ion_heap_query query;
};
-static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
+static int validate_ioctl_arg(struct file *filp,
+ unsigned int cmd, union ion_ioctl_arg *arg)
{
int ret = 0;
+ int mask = 1 << iminor(filp->f_inode);
switch (cmd) {
case ION_IOC_HEAP_QUERY:
@@ -35,6 +37,9 @@ static int validate_ioctl_arg(unsigned int cmd, union
ion_ioctl_arg *arg)
ret |= arg->query.reserved1 != 0;
ret |= arg->query.reserved2 != 0;
break;
+ case ION_IOC_ALLOC:
+ ret = !(arg->allocation.heap_id_mask & mask);
+ break;
default:
break;
}
@@ -70,7 +75,7 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned
long arg)
if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
return -EFAULT;
- ret = validate_ioctl_arg(cmd, &data);
+ ret = validate_ioctl_arg(filp, cmd, &data);
if (WARN_ON_ONCE(ret))
return ret;
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 93e2c90..5144f1a 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -40,6 +40,8 @@
#include "ion.h"
+#define ION_DEV_MAX 32
+
static struct ion_device *internal_dev;
static int heap_id;
@@ -541,11 +543,21 @@ void ion_device_add_heap(struct ion_heap *heap)
{
struct dentry *debug_file;
struct ion_device *dev = internal_dev;
+ int ret;
if (!heap->ops->allocate || !heap->ops->free)
pr_err("%s: can not add heap with invalid ops struct.\n",
__func__);
+ heap->ddev.devt = MKDEV(MAJOR(dev->devt), heap_id);
+ dev_set_name(&heap->ddev, "ion%d", heap_id);
+ device_initialize(&heap->ddev);
+ cdev_init(&heap->chrdev, &ion_fops);
+ heap->chrdev.owner = THIS_MODULE;
+ ret = cdev_device_add(&heap->chrdev, &heap->ddev);
+ if (ret < 0)
+ return;
+
spin_lock_init(&heap->free_lock);
heap->free_list_size = 0;
@@ -595,13 +607,9 @@ static int ion_device_create(void)
if (!idev)
return -ENOMEM;
- idev->dev.minor = MISC_DYNAMIC_MINOR;
- idev->dev.name = "ion";
- idev->dev.fops = &ion_fops;
- idev->dev.parent = NULL;
- ret = misc_register(&idev->dev);
+ ret = alloc_chrdev_region(&idev->devt, 0, ION_DEV_MAX, "ion");
if (ret) {
- pr_err("ion: failed to register misc device.\n");
+ pr_err("ion: unable to allocate major\n");
kfree(idev);
return ret;
}
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 621e5f7..ef51ff5 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -17,6 +17,7 @@
#ifndef _ION_H
#define _ION_H
+#include
#include
#include
#include
@@ -26,7 +27,6 @@
#include
#include
#include
-#include
#include "../uapi/ion.h"
@@ -90,13 +90,13 @@ void ion_buffer_destroy(struct ion_buffer *buffer);
/**
* struct ion_device - the metadata of the ion device node
- * @dev: the actual misc device
+ * @dev: the actual device
* @buffers: an rb tree of all the existing buffers
* @buffer_lock: lock protecting the tree of buffers
* @lock: rwsem protecting the tree of heaps and clients
*/
struct ion_device {
- struct miscdevice dev;
+ dev_t devt;
struct rb_root buffers;
struct mutex buffer_lock;
struct rw_semaphore lock;
@@ -152,6 +152,8 @@ struct ion_heap_ops {
* struct ion_heap - represents a heap in the system
* @node: rb node to put the heap on the device's tree of heaps
* @dev: back pointer to the ion_device
+ * @ddev: device structure
+ * @chrdev:associated character device
* @type: type o
RE: [PATCH V2 1/4] vmbus: don't acquire the mutex in vmbus_hvsock_device_unregister()
> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Monday, September 18, 2017 7:00 AM
> To: KY Srinivasan
> Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org;
> o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> jasow...@redhat.com; leann.ogasaw...@canonical.com;
> marcelo.ce...@canonical.com; Stephen Hemminger
> ; Haiyang Zhang
> Subject: Re: [PATCH V2 1/4] vmbus: don't acquire the mutex in
> vmbus_hvsock_device_unregister()
>
> On Sun, Sep 17, 2017 at 08:54:16PM -0700, k...@exchange.microsoft.com
> wrote:
> > From: Dexuan Cui
> >
> > Due to commit 54a66265d675 ("Drivers: hv: vmbus: Fix rescind handling"),
> > we need this patch to resolve the below deadlock:
> >
> > after we get the mutex in vmbus_hvsock_device_unregister() and call
> > vmbus_device_unregister() -> device_unregister() -> ... ->
> device_release()
> > -> vmbus_device_release(), we'll get a deadlock, because
> > vmbus_device_release() tries to get the same mutex.
> >
> > Signed-off-by: Dexuan Cui
> > Cc: K. Y. Srinivasan
> > Cc: Haiyang Zhang
> > Cc: Stephen Hemminger
> > Signed-off-by: K. Y. Srinivasan
>
> As every one of these patches had questions from me, please break them
> up into different series, one for 4.14-final, and one for 4.15-rc1.
Will do.
Thanks,
K. Y
>
> thanks,
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: fix type mismatch warning
On Tue, Sep 05, 2017 at 10:56:13AM +0200, Arnd Bergmann wrote: > Allowing binder to expose the 64-bit API on 32-bit kernels caused a > build warning: > > drivers/android/binder.c: In function 'binder_transaction_buffer_release': > drivers/android/binder.c:2220:15: error: cast to pointer from integer of > different size [-Werror=int-to-pointer-cast] > fd_array = (u32 *)(parent_buffer + fda->parent_offset); >^ > drivers/android/binder.c: In function 'binder_translate_fd_array': > drivers/android/binder.c:2445:13: error: cast to pointer from integer of > different size [-Werror=int-to-pointer-cast] > fd_array = (u32 *)(parent_buffer + fda->parent_offset); > ^ > drivers/android/binder.c: In function 'binder_fixup_parent': > drivers/android/binder.c:2511:18: error: cast to pointer from integer of > different size [-Werror=int-to-pointer-cast] > > This adds extra type casts to avoid the warning. > > However, there is another problem with the Kconfig option: turning > it on or off creates two incompatible ABI versions, a kernel that > has this enabled cannot run user space that was built without it > or vice versa. A better solution might be to leave the option hidden > until the binder code is fixed to deal with both ABI versions. I don't know if that is ever going to be fixed, as it's not an issue that you will ever see "in the wild" from what I can tell... thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] binder: fix "cast to pointer from integer of different size" warning
On Mon, Sep 04, 2017 at 01:17:40PM +0800, Jisheng Zhang wrote: > The binder driver now could cause warnings as below on 32bit platforms > if ANDROID_BINDER_IPC_32BIT is unselected: > > drivers/android/binder.c:1550:15: warning: cast to pointer from integer > of different size [-Wint-to-pointer-cast] > > This patch fix all of them. > > Signed-off-by: Jisheng Zhang > --- > drivers/android/binder.c | 13 +++-- > 1 file changed, 7 insertions(+), 6 deletions(-) Arnd sent in a patch for this issue right after you did, that was smaller, so I'll take his version. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 1/4] vmbus: don't acquire the mutex in vmbus_hvsock_device_unregister()
On Sun, Sep 17, 2017 at 08:54:16PM -0700, k...@exchange.microsoft.com wrote:
> From: Dexuan Cui
>
> Due to commit 54a66265d675 ("Drivers: hv: vmbus: Fix rescind handling"),
> we need this patch to resolve the below deadlock:
>
> after we get the mutex in vmbus_hvsock_device_unregister() and call
> vmbus_device_unregister() -> device_unregister() -> ... -> device_release()
> -> vmbus_device_release(), we'll get a deadlock, because
> vmbus_device_release() tries to get the same mutex.
>
> Signed-off-by: Dexuan Cui
> Cc: K. Y. Srinivasan
> Cc: Haiyang Zhang
> Cc: Stephen Hemminger
> Signed-off-by: K. Y. Srinivasan
As every one of these patches had questions from me, please break them
up into different series, one for 4.14-final, and one for 4.15-rc1.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 3/4] vmbus: add per-channel sysfs info
On Sun, Sep 17, 2017 at 08:54:18PM -0700, k...@exchange.microsoft.com wrote: > From: Stephen Hemminger > > This extends existing vmbus related sysfs structure to provide per-channel > state information. This is useful when diagnosing issues with multiple > queues in networking and storage. > > The existing sysfs only displayed information about the primary > channel. The one place it reported multiple channels was the > channel_vp_mapping file which violated the sysfs convention > of one value per file. > > Signed-off-by: Stephen Hemminger > Signed-off-by: K. Y. Srinivasan > --- > Documentation/ABI/stable/sysfs-bus-vmbus | 56 ++ > drivers/hv/channel_mgmt.c| 10 +- > drivers/hv/hyperv_vmbus.h| 2 + > drivers/hv/vmbus_drv.c | 185 > +-- > include/linux/hyperv.h | 6 + > 5 files changed, 246 insertions(+), 13 deletions(-) Feels like a new feature to me, and not needed in 4.14-final. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 4/4] Drivers: hv: vmbus: Expose per-channel event counters events counters
On Sun, Sep 17, 2017 at 08:54:19PM -0700, k...@exchange.microsoft.com wrote: > From: Stephen Hemminger > > When investigating performance, it is useful to be able to look at > the number of host and guest events per-channel. This is equivalent > to per-device interrupt statistics. > > Signed-off-by: Stephen Hemminger > Signed-off-by: K. Y. Srinivasan > --- > Documentation/ABI/stable/sysfs-bus-vmbus | 14 ++ > drivers/hv/connection.c | 2 ++ > drivers/hv/vmbus_drv.c | 18 ++ > include/linux/hyperv.h | 4 > 4 files changed, 38 insertions(+) How is this a fix and not just a new feature? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 2/4] Drivers: hv: fcopy: restore correct transfer length
On Sun, Sep 17, 2017 at 08:54:17PM -0700, k...@exchange.microsoft.com wrote:
> From: Olaf Hering
>
> Till recently the expected length of bytes read by the
> daemon did depend on the context. It was either hv_start_fcopy or
> hv_do_fcopy. The daemon had a buffer size of two pages, which was much
> larger than needed.
>
> Now the expected length of bytes read by the
> daemon changed slightly. For START_FILE_COPY it is still the size of
> hv_start_fcopy. But for WRITE_TO_FILE and the other operations it is as
> large as the buffer that arrived via vmbus. In case of WRITE_TO_FILE
> that is slightly larger than a struct hv_do_fcopy. Since the buffer in
> the daemon was still larger everything was fine.
>
> Currently, the daemon reads only what is actually needed.
> The new buffer layout is as large as a struct hv_do_fcopy, for the
> WRITE_TO_FILE operation. Since the kernel expects a slightly larger
> size, hvt_op_read will return -EINVAL because the daemon will read
> slightly less than expected. Address this by restoring the expected
> buffer size in case of WRITE_TO_FILE.
>
> Fixes: 'commit c7e490fc23eb ("Drivers: hv: fcopy: convert to
> hv_utils_transport")'
> Fixes: 'commit 3f2baa8a7d2e ("Tools: hv: update buffer handling in
> hv_fcopy_daemon")'
What's with the 'commit' here?
It should look like:
Fixes: c7e490fc23eb ("Drivers: hv: fcopy: convert to hv_utils_transport")
Please fix...
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 1/4] vmbus: don't acquire the mutex in vmbus_hvsock_device_unregister()
On Sun, Sep 17, 2017 at 08:54:16PM -0700, k...@exchange.microsoft.com wrote:
> From: Dexuan Cui
>
> Due to commit 54a66265d675 ("Drivers: hv: vmbus: Fix rescind handling"),
> we need this patch to resolve the below deadlock:
So does this patch need a "Fixes:" tag, and a "stable@" tag as well, so
it gets into 4.13?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Drivers: hv: vmbus: Fix rescind handling issues
Vitaly Kuznetsov writes:
>
> Reverting 6f3d791f300618caf82a2be0c27456edd76d5164 still helps.
In addition to the above I got the following crash while playing with
4.14-rc1 (unmodified):
[ 55.810080] kernel tried to execute NX-protected page - exploit attempt?
(uid: 0)
[ 55.814293] BUG: unable to handle kernel paging request at 8800059985f0
[ 55.818065] IP: 0x8800059985f0
[ 55.819925] PGD 22eb067 P4D 22eb067 PUD 22ec067 PMD 5f37063 PTE
85998163
[ 55.820018] Oops: 0011 [#1] SMP
[ 55.820018] Modules linked in: vfat fat bnx2x mdio efi_pstore hv_utils
efivars pci_hyperv ptp pps_core pcspkr hv_balloon xfs libcrc32c hv_storvsc
hyperv_fb hv_netvsc scsi_transport_fc hid_hyperv hyperv_keyboard hv_vmbus
[ 55.834837] CPU: 0 PID: 498 Comm: kworker/0:2 Not tainted 4.14.0-rc1 #63
[ 55.834837] Hardware name: Microsoft Corporation Virtual Machine/Virtual
Machine, BIOS Hyper-V UEFI Release v1.0 11/26/2012
[ 55.834837] Workqueue: events vmbus_onmessage_work [hv_vmbus]
[ 55.834837] task: 88003f448000 task.stack: c90005398000
[ 55.834837] RIP: 0010:0x8800059985f0
[ 55.834837] RSP: 0018:c9000539be00 EFLAGS: 00010286
[ 55.834837] RAX: 880005998010 RBX: 880005998000 RCX:
[ 55.834837] RDX: 8800059985f0 RSI: 0246 RDI: 880005998000
[ 55.860040] RBP: c9000539be18 R08: 02e6 R09:
[ 55.865057] R10: c9000539bdf0 R11: a000 R12: 0286
[ 55.865057] R13: 88007ae1ed00 R14: R15: 8800065c3200
[ 55.865057] FS: () GS:88007ae0()
knlGS:
[ 55.865057] CS: 0010 DS: ES: CR0: 80050033
[ 55.865057] CR2: 8800059985f0 CR3: 075a5000 CR4: 001406f0
[ 55.886745] Call Trace:
[ 55.886745] ? vmbus_onoffer_rescind+0xfa/0x160 [hv_vmbus]
[ 55.890968] vmbus_onmessage+0x2a/0x90 [hv_vmbus]
[ 55.891934] vmbus_onmessage_work+0x1d/0x30 [hv_vmbus]
[ 55.891934] process_one_work+0x193/0x390
[ 55.891934] worker_thread+0x48/0x3c0
[ 55.891934] kthread+0x120/0x140
[ 55.891934] ? process_one_work+0x390/0x390
[ 55.891934] ? kthread_create_on_node+0x60/0x60
[ 55.891934] ret_from_fork+0x25/0x30
[ 55.891934] Code: 88 ff ff c0 85 99 05 00 88 ff ff d0 85 99 05 00 88 ff ff
d0 85 99 05 00 88 ff ff e0 85 99 05 00 88 ff ff e0 85 99 05 00 88 ff ff 85
99 05 00 88 ff ff f0 85 99 05 00 88 ff ff 00 86 99 05 00
[ 55.922505] RIP: 0x8800059985f0 RSP: c9000539be00
[ 55.922505] CR2: 8800059985f0
[ 55.922505] ---[ end trace 25226e00af3f94fb ]---
[ 55.933590] Kernel panic - not syncing: Fatal exception
[ 55.933590] Kernel Offset: disabled
[ 55.933590] ---[ end Kernel panic - not syncing: Fatal exception
So it seems that during
while (READ_ONCE(channel->probe_done) == false) {
/*
* We wait here until any channel offer is currently
* being processed.
*/
msleep(1);
}
loop the channel disappeared. The issue may not be related to the netvsc
hang I mentioned before. It may make sense to do refcounting for
channels/subchannels (or employ RCU).
--
Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: media: atomisp: Merge assignment with return
On Tue, Sep 12, 2017 at 07:55:07PM +0530, Srishti Sharma wrote: > Merge the assignment and the return statements to return the value > directly. Done using the following semantic patch by coccinelle. > > @@ > local idexpression ret; > expression e; > @@ > > -ret = > +return > e; > -return ret; > > Signed-off-by: Srishti Sharma Hi Srishti, I've merged the two patches as they're trivial and the commit message is the same. It's entirely reasonable to have a patch per driver but you should mention that in the commit message (subject line) at least. This case is a bit special because the other driver is also specific to the atomisp staging driver. Thanks. -- Sakari Ailus e-mail: sakari.ai...@iki.fi ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/2] include: linux: sysfs: Add __ATTR_NAMED macro
On Thu, Sep 14, 2017 at 2:20 AM, Jonathan Cameron
wrote:
>
>
> On 13 September 2017 12:23:31 GMT-07:00, Lars-Peter Clausen
> wrote:
>>On 09/13/2017 08:58 PM, Greg KH wrote:
>>> On Wed, Sep 13, 2017 at 06:03:10PM +0100, Jonathan Cameron wrote:
On Wed, 13 Sep 2017 14:14:07 +0530
Himanshi Jain wrote:
> Add __ATTR_NAMED macro similar to __ATTR but taking name as a
> string instead of implicit conversion of argument to string using
> the macro _stringify(_name).
>
> Signed-off-by: Himanshi Jain
> ---
> include/linux/sysfs.h | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index aa02c32..20321cf 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -104,6 +104,13 @@ struct attribute_group {
>.store = _store, \
> }
>
> +#define __ATTR_NAMED(_name, _mode, _show, _store) {
> \
I'm not sure about the naming here. The normal __ATTR macro is also
'named'. Maybe something as awful as
__ATTR_STRING_NAME ?
Greg what do you think?
>>>
>>> ick ick ick.
>>>
This is all to allow us to have names with operators in them without
checkpatch complaining about them... A worthwhile aim just to stop
more people wasting time trying to 'fix' those cases by adding
>>spaces.
>>>
>>> Yeah, but this really seems "heavy" for just a crazy sysfs name in a
>>> macro. Adding a whole new "core" define for that is a hard sell...
>>>
>>> I also want to get rid of the "generic" __ATTR type macros, and force
>>> people to use the proper _RW and friends instead. I don't want to
>>add
>>> another new one that people will start to use that I later have to
>>> change...
>>>
>>> So no, I don't like this, how about just changing your macros
>>instead?
>>> No one else has this problem :)
>>
>>Nobody else realized they have this problem yet. E.g. there are a few
>>users
>>of __ATTR in block/genhd.c that have the same issue and are likely to
>>generate the same false positives from static checkers.
>
> For IIO there is the option of moving these over to the core generated
> available callbacks, but
> that won't work in every case and is a more major change. I need to shift a
> few more drivers
> over to the available callbacks and see how well it works out. Might find
> time to do one in a
> gap between interesting talks this afternoon...
Can I help you in this? It is about exploring options as far as I can
make out, although can't really understand what options are those for
now.
Or do you want me to put comments to not to fix this checkpatch
warning as you suggested earlier?
>
> If I am feeling really keen I might write this missing docs I promised a
> while back on that stuff. Jet lag dependant...
>
> Jonathan
>>
>>--
>>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>the body of a message to majord...@vger.kernel.org
>>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: typec: tcpm: Validate source and sink caps
On Thu, Sep 07, 2017 at 06:22:13PM -0700, Badhri Jagan Sridharan wrote: > The source and sink caps should follow the following rules. > This patch validates whether the src_caps/snk_caps adheres > to it. > > 6.4.1 Capabilities Message > A Capabilities message (Source Capabilities message or Sink > Capabilities message) shall have at least one Power > Data Object for vSafe5V. The Capabilities message shall also > contain the sending Port’s information followed by up to > 6 additional Power Data Objects. Power Data Objects in a > Capabilities message shall be sent in the following order: > > 1. The vSafe5V Fixed Supply Object shall always be the first object. > 2. The remaining Fixed Supply Objects, if present, shall be sent >in voltage order; lowest to highest. > 3. The Battery Supply Objects, if present shall be sent in Minimum >Voltage order; lowest to highest. > 4. The Variable Supply (non-battery) Objects, if present, shall be >sent in Minimum Voltage order; lowest to highest. > > Errors in source/sink_caps of the local port will prevent > the port registration. Whereas, errors in source caps of partner > device would only log them. > > Signed-off-by: Badhri Jagan Sridharan > --- > drivers/staging/typec/pd.h | 2 + > drivers/staging/typec/tcpm.c | 107 > +++ > drivers/staging/typec/tcpm.h | 16 +++ > 3 files changed, 108 insertions(+), 17 deletions(-) Now that these files are moved in my usb tree, can you rebase and resend them? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: iio: tsl2x7x: clean up limit checks
On Sat, Sep 16, 2017 at 02:18:52PM +0200, Paolo Cretaro wrote: > On 16/09/2017 13:37, Dan Carpenter wrote: > > On Sat, Sep 16, 2017 at 01:11:29PM +0200, Paolo Cretaro wrote: > >> Hi Dan, > >> just minor nitpicking on the commit message: > >> > >> On 08/09/2017 12:53, Dan Carpenter wrote: > >>> The background of this code is that we can either use the default > >>> tables or load our own table with sysfs. The default tables are three > >>> element arrays of struct tsl2x7x_lux. If we load the table with sysfs > >>> then we can have as many as nine elements. Which ever way we do it, the > >>> last element is always zeroed out. > >>> > >>> The most interesting part of this patch is in the > >>> in_illuminance0_lux_table_show() function. We were using the wrong > >>> limit, "TSL2X7X_MAX_LUX_TABLE_SIZE * 3", when it should have been just > >>> "TSL2X7X_MAX_LUX_TABLE_SIZE". This creates a static checker warning > >>> that we are going of of bounds. However, since the last element is > >> out of bounds > >> > >> Regards, > >> P. > >> > >>> always zeroed out, that means we hit the break statement and the code > >>> works correctly despite the wrong limit check. > > > > What? No no. I meant it how I wrote it. The last element is > > always zeroed out meaning it's just a series of zeroes. > > Sorry, I meant the previous sentence "This creates a static checker warning > that we are going of of bounds". > Ah. Of course. Thanks Jonathan for fixing this. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] [PATCH] staging: rtl8723bs: Remove unused variable ret
On Thu, Sep 14, 2017 at 10:21:29PM +0530, Harsha Sharma wrote: > Hi, > Yes, you are right but the function returns 0 in the end and the changes > compiles well . Then the function is incorrect, it should be reporting back that error. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 1/5] staging: typec: tcpm: Drop commented out code
On Tue, Sep 12, 2017 at 09:41:20AM -0700, Guenter Roeck wrote: > On Tue, Sep 12, 2017 at 10:38:39AM +0300, Heikki Krogerus wrote: > > On Mon, Sep 11, 2017 at 08:32:04PM -0700, Guenter Roeck wrote: > > > Commented out code can be added as needed. Drop it. > > > Also drop TODO and an obsolete XXX comment. > > > > > > Signed-off-by: Guenter Roeck > > > --- > > > v2, v3: No change > > > > > > drivers/staging/typec/tcpm.c | 37 + > > > 1 file changed, 1 insertion(+), 36 deletions(-) > > > > Nice! Just to be sure: The idea is to leave tcpci.c in staging, right? > > > Yes, until we get test coverage or Greg gets tired of keeping it there > (in that case we would probably have to drop it). Well, if it's not ever going to get fixed up, why can't we just drop it? Anyway, I've pulled this series into the staging and USB trees, so all should be good for it now. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Drivers: hv: vmbus: Fix rescind handling issues
Stephen Hemminger writes: > On Sep 15, 2017 11:01 AM, "KY Srinivasan" wrote: > > > Vitaly Kuznetsov writes: > > > > > > > > I'm seeing the same issue, reverting the offending > > > > > > commit 6f3d791f300618caf82a2be0c27456edd76d5164 > > > Author: K. Y. Srinivasan > > > Date: Fri Aug 11 10:03:59 2017 -0700 > > > > > > Drivers: hv: vmbus: Fix rescind handling issues > > > > > > which made it upstream helps. Did you guys do some investigation here? > > > In case not I can take a look next week. > > > > Sorry, I have to resend - K.Y.'s k...@exchange.microsoft.com doesn't > > accept mail from me :-( > > I think this turned out to be a bug in netvsc - Stephen can elaborate. I > think the fix > has been submitted upstream as well. > > K. Y > > > > -- > > Vitaly > > It turned out that subchannel call back is run from primary channel call > back, and was deadlocking waiting for vmbus open. > > Original code had broken wait for sub channels. > The first for that induced this bug. > > The resolution is in latest network driver was to bring up sub channels from > work queue This is a bit confusing, in case you mean commit 8195b1396ec86dddbba443c74b2188b423556c74 Author: Stephen Hemminger Date: Wed Sep 6 13:53:05 2017 -0700 hv_netvsc: fix deadlock on hotplug is supposed to fix the issue. I'm testing the latest net-next which has it: $ git log --oneline drivers/net/hyperv/ 5023a6db7319 netvsc: increase default receive buffer size 8f2bb1de7334 hv_netvsc: avoid unnecessary wakeups on subchannel creation 8195b1396ec8 hv_netvsc: fix deadlock on hotplug db3cd7af9d0f hv_netvsc: Fix the channel limit in netvsc_set_rxfh() 06be580ac7b6 hv_netvsc: Simplify the limit check in netvsc_set_channels() ... and when I do # ip link set dev eth0 mtu 1400 # ip link set dev eth0 mtu 1500 (actually, you can do mtu change just once the deadlock has already happened, doing it twice just reveals the issue faster - it will hang permanently trying to get rtnl lock which is already taken) The log is: [ 248.800089] INFO: task kworker/u480:0:5 blocked for more than 120 seconds. [ 248.804065] Not tainted 4.14.0-rc1 #63 [ 248.806486] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 248.810879] kworker/u480:0 D0 5 2 0x8000 [ 248.814225] Workqueue: netns cleanup_net [ 248.816745] Call Trace: [ 248.818286] __schedule+0x1d5/0x790 [ 248.820398] ? sched_clock+0x9/0x10 [ 248.822297] ? select_idle_sibling+0x24/0x420 [ 248.824883] schedule+0x31/0x80 [ 248.826835] schedule_preempt_disabled+0x9/0x10 [ 248.829444] __mutex_lock.isra.1+0x1a3/0x4f0 [ 248.831973] ? sched_clock_cpu+0x5d/0xb0 [ 248.834300] __mutex_lock_slowpath+0xe/0x10 [ 248.836869] ? __mutex_lock_slowpath+0xe/0x10 [ 248.839488] mutex_lock+0x2a/0x30 [ 248.841378] rtnl_lock+0x10/0x20 [ 248.843431] cleanup_net+0x94/0x2e0 [ 248.845479] process_one_work+0x193/0x390 [ 248.847838] worker_thread+0x48/0x3c0 [ 248.850121] kthread+0x120/0x140 [ 248.851927] ? process_one_work+0x390/0x390 [ 248.854425] ? kthread_create_on_node+0x60/0x60 [ 248.856884] ? kthread_create_on_node+0x60/0x60 [ 248.859585] ret_from_fork+0x25/0x30 [ 248.861581] INFO: task kworker/9:2:374 blocked for more than 120 seconds. [ 248.865286] Not tainted 4.14.0-rc1 #63 [ 248.867655] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 248.872135] kworker/9:2 D0 374 2 0x8000 [ 248.875175] Workqueue: events rndis_set_subchannel [hv_netvsc] [ 248.878473] Call Trace: [ 248.879825] __schedule+0x1d5/0x790 [ 248.882122] schedule+0x31/0x80 [ 248.884160] rndis_set_subchannel+0x186/0x1e0 [hv_netvsc] [ 248.887741] ? finish_wait+0x80/0x80 [ 248.889960] process_one_work+0x193/0x390 [ 248.892349] worker_thread+0x48/0x3c0 [ 248.894484] kthread+0x120/0x140 [ 248.896364] ? process_one_work+0x390/0x390 [ 248.898756] ? kthread_create_on_node+0x60/0x60 [ 248.901229] ret_from_fork+0x25/0x30 [ 248.903180] INFO: task hypervkvpd:656 blocked for more than 120 seconds. [ 248.907120] Not tainted 4.14.0-rc1 #63 [ 248.909966] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 248.914387] hypervkvpd D0 656 1 0x [ 248.917618] Call Trace: [ 248.919119] __schedule+0x1d5/0x790 [ 248.921343] schedule+0x31/0x80 [ 248.924454] schedule_preempt_disabled+0x9/0x10 [ 248.927792] __mutex_lock.isra.1+0x1a3/0x4f0 [ 248.930951] __mutex_lock_slowpath+0xe/0x10 [ 248.934078] ? __mutex_lock_slowpath+0xe/0x10 [ 248.937466] mutex_lock+0x2a/0x30 [ 248.940228] __netlink_dump_start+0x44/0x190 [ 248.943286] rtnetlink_rcv_msg+0x1a2/0x280 [ 248.946237] ? vsnprintf+0xea/0x4d0 [ 248.94] ? rtnl_getlink+0x1c0/0x1c0 [ 248.951713] ? rtnl_getlink+0x1c0/0x1c0 [ 248.954644] ? rtnl_calcit.isra.26+0x110/0x110 [ 248.958010] netlink_rcv_skb+0x89/0x130 [ 248.960945] rtnetlink_rcv+0x10/0
Re: [PATCH] staging: atomisp: add a driver for ov5648 camera sensor
On Mon, 2017-09-11 at 20:03 +0200, Devid Antonio Filoni wrote: > On Mon, Sep 11, 2017 at 05:55:29PM +0300, Sakari Ailus wrote: > > Hi Devid, > > > > Please see my comments below. > > > > Andy: please look for "INT5648". > > Hi Sakari, > I'm replying below to your comments. I'll work on a v2 patch as soon > as we get > more comments. > > About "INT5648", I extracted it from the DSDT of my Lenovo Miix 310, > take a look > at https://pastebin.com/ExHWYr8g . First of all, thank you, Sakari, to raise a flag here. Second, Devid, please answer to the following: is it an official BIOS which is available in the wild? If it's so, please, add a paragraph to the commit message explaining how do you get this and point to the DSDT excerpt. Put an answer to above question to the commit message as well. -- Andy Shevchenko Intel Finland Oy ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
