* [PATCH v2] Remove SPDX "WITH Linux-syscall-note" from kernel-space headers
From: Masahiro Yamada @ 2019-04-02 9:08 UTC (permalink / raw)
To: Andrew Morton
Cc: Karthikeyan Ramasubramanian, Radim Krčmář,
Thomas Gleixner, Andy Gross, Vitaly Kuznetsov, Girish Mahadevan,
Sagar Dharia, Masahiro Yamada, linux-arch, H. Peter Anvin,
Arnd Bergmann, Haiyang Zhang, K. Y. Srinivasan, Sasha Levin,
Borislav Petkov, x86, linux-hyperv, linux-kernel,
Stephen Hemminger, Ingo Molnar
The "WITH Linux-syscall-note" should be added to headers exported to
the user-space.
Some kernel-space headers have "WITH Linux-syscall-note", which seems
a mistake.
[1] arch/x86/include/asm/hyperv-tlfs.h
5a4858032217 ("x86/hyper-v: move hyperv.h out of uapi") moved this file
out of uapi, but missed to update the SPDX License tag.
[2] include/asm-generic/shmparam.h
76ce2a80a28e ("Rename include/{uapi => }/asm-generic/shmparam.h really")
moved this file out of uapi, but missed to update the SPDX License tag.
[3] include/linux/qcom-geni-se.h
eddac5af0654 ("soc: qcom: Add GENI based QUP Wrapper driver") added this
file, but I do not see a good reason why its license tag must include
"WITH Linux-syscall-note".
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v2:
- Fix a typo in commit log
arch/x86/include/asm/hyperv-tlfs.h | 2 +-
include/asm-generic/shmparam.h | 2 +-
include/linux/qcom-geni-se.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 2bdbbbc..cdf44aa 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/* SPDX-License-Identifier: GPL-2.0 */
/*
* This file contains definitions from Hyper-V Hypervisor Top-Level Functional
diff --git a/include/asm-generic/shmparam.h b/include/asm-generic/shmparam.h
index 8b78c0b..b8f9035 100644
--- a/include/asm-generic/shmparam.h
+++ b/include/asm-generic/shmparam.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/* SPDX-License-Identifier: GPL-2.0 */
#ifndef __ASM_GENERIC_SHMPARAM_H
#define __ASM_GENERIC_SHMPARAM_H
diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
index 3bcd67f..dd46494 100644
--- a/include/linux/qcom-geni-se.h
+++ b/include/linux/qcom-geni-se.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/* SPDX-License-Identifier: GPL-2.0 */
/*
* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
*/
--
2.7.4
^ permalink raw reply related
* [PATCH 1/1] scsi: storvsc: Reduce default ring buffer size to 128 Kbytes
From: Michael Kelley @ 2019-04-01 21:42 UTC (permalink / raw)
To: KY Srinivasan, martin.petersen@oracle.com, Long Li,
James.Bottomley@hansenpartnership.com, emilne@redhat.com,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-scsi@vger.kernel.org
Cc: Michael Kelley
Reduce the default VMbus channel ring buffer size for storvsc
SCSI devices from 1 Mbyte to 128 Kbytes. Measurements show
that ring buffer sizes above 128 Kbytes do not increase
performance even at very high IOPS rates, so don't waste
the memory. Also remove the dependence on PAGE_SIZE, since
the ring buffer size should not change on architectures
where PAGE_SIZE is not 4 Kbytes.
Signed-off-by: Michael Kelley <mikelley@microsoft.com>
---
drivers/scsi/storvsc_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index fbaa11a..f85517d 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -385,7 +385,7 @@ enum storvsc_request_type {
* This is the end of Protocol specific defines.
*/
-static int storvsc_ringbuffer_size = (256 * PAGE_SIZE);
+static int storvsc_ringbuffer_size = (128 * 1024);
static u32 max_outstanding_req_per_channel;
static int storvsc_vcpus_per_sub_channel = 4;
--
1.8.3.1
^ permalink raw reply related
* RE: [PATCH v2 1/1] scsi: storvsc: Fix calculation of sub-channel count
From: Long Li @ 2019-04-01 21:38 UTC (permalink / raw)
To: Michael Kelley, KY Srinivasan, martin.petersen@oracle.com,
James.Bottomley@hansenpartnership.com, emilne@redhat.com,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-scsi@vger.kernel.org
In-Reply-To: <1554134985-8671-1-git-send-email-mikelley@microsoft.com>
>>>-----Original Message-----
>>>From: Michael Kelley <mikelley@microsoft.com>
>>>Sent: Monday, April 1, 2019 9:11 AM
>>>To: KY Srinivasan <kys@microsoft.com>; martin.petersen@oracle.com; Long
>>>Li <longli@microsoft.com>; James.Bottomley@hansenpartnership.com;
>>>emilne@redhat.com; linux-hyperv@vger.kernel.org; linux-
>>>kernel@vger.kernel.org; linux-scsi@vger.kernel.org
>>>Cc: Michael Kelley <mikelley@microsoft.com>
>>>Subject: [PATCH v2 1/1] scsi: storvsc: Fix calculation of sub-channel count
>>>
>>>When the number of sub-channels offered by Hyper-V is >= the number of
>>>CPUs in the VM, calculate the correct number of sub-channels.
>>>The current code produces one too many.
>>>
>>>This scenario arises only when the number of CPUs is artificially restricted
>>>(for example, with maxcpus=<n> on the kernel boot line), because Hyper-V
>>>normally offers a sub-channel count < number of CPUs.
>>>While the current code doesn't break, the extra sub-channel is unbalanced
>>>across the CPUs (for example, a total of 5 channels on a VM with 4 CPUs).
>>>
>>>Signed-off-by: Michael Kelley <mikelley@microsoft.com>
>>>Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>>>---
>>>Changes in v2:
>>>* Put num_online_cpus() inline and eliminate num_cpus local
>>> variable [Vitaly Kuznetsov]
>>>
Reviewed-by: Long Li <longli@microsoft.com>
>>>---
>>> drivers/scsi/storvsc_drv.c | 13 +++++++++++--
>>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>>diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
>>>84380ba..e186743 100644
>>>--- a/drivers/scsi/storvsc_drv.c
>>>+++ b/drivers/scsi/storvsc_drv.c
>>>@@ -668,13 +668,22 @@ static void handle_multichannel_storage(struct
>>>hv_device *device, int max_chns) {
>>> struct device *dev = &device->device;
>>> struct storvsc_device *stor_device;
>>>- int num_cpus = num_online_cpus();
>>> int num_sc;
>>> struct storvsc_cmd_request *request;
>>> struct vstor_packet *vstor_packet;
>>> int ret, t;
>>>
>>>- num_sc = ((max_chns > num_cpus) ? num_cpus : max_chns);
>>>+ /*
>>>+ * If the number of CPUs is artificially restricted, such as
>>>+ * with maxcpus=1 on the kernel boot line, Hyper-V could offer
>>>+ * sub-channels >= the number of CPUs. These sub-channels
>>>+ * should not be created. The primary channel is already created
>>>+ * and assigned to one CPU, so check against # CPUs - 1.
>>>+ */
>>>+ num_sc = min((int)(num_online_cpus() - 1), max_chns);
>>>+ if (!num_sc)
>>>+ return;
>>>+
>>> stor_device = get_out_stor_device(device);
>>> if (!stor_device)
>>> return;
>>>--
>>>1.8.3.1
^ permalink raw reply
* Re: [PATCH hyperv-fixes,v3] hv_netvsc: Fix unwanted wakeup after tx_disable
From: David Miller @ 2019-03-29 20:34 UTC (permalink / raw)
To: haiyangz
Cc: sashal, linux-hyperv, haiyangz, kys, sthemmin, olaf, vkuznets,
netdev, linux-kernel
In-Reply-To: <20190328194036.7279-1-haiyangz@linuxonhyperv.com>
From: Haiyang Zhang <haiyangz@linuxonhyperv.com>
Date: Thu, 28 Mar 2019 19:40:36 +0000
> From: Haiyang Zhang <haiyangz@microsoft.com>
>
> After queue stopped, the wakeup mechanism may wake it up again
> when ring buffer usage is lower than a threshold. This may cause
> send path panic on NULL pointer when we stopped all tx queues in
> netvsc_detach and start removing the netvsc device.
>
> This patch fix it by adding a tx_disable flag to prevent unwanted
> queue wakeup.
>
> Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
> Reported-by: Mohammed Gamal <mgamal@redhat.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* [PATCH v2 1/1] scsi: storvsc: Fix calculation of sub-channel count
From: Michael Kelley @ 2019-04-01 16:10 UTC (permalink / raw)
To: KY Srinivasan, martin.petersen@oracle.com, Long Li,
James.Bottomley@hansenpartnership.com, emilne@redhat.com,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-scsi@vger.kernel.org
Cc: Michael Kelley
When the number of sub-channels offered by Hyper-V is >= the number
of CPUs in the VM, calculate the correct number of sub-channels.
The current code produces one too many.
This scenario arises only when the number of CPUs is artificially
restricted (for example, with maxcpus=<n> on the kernel boot line),
because Hyper-V normally offers a sub-channel count < number of CPUs.
While the current code doesn't break, the extra sub-channel is
unbalanced across the CPUs (for example, a total of 5 channels on
a VM with 4 CPUs).
Signed-off-by: Michael Kelley <mikelley@microsoft.com>
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes in v2:
* Put num_online_cpus() inline and eliminate num_cpus local
variable [Vitaly Kuznetsov]
---
drivers/scsi/storvsc_drv.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 84380ba..e186743 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -668,13 +668,22 @@ static void handle_multichannel_storage(struct hv_device *device, int max_chns)
{
struct device *dev = &device->device;
struct storvsc_device *stor_device;
- int num_cpus = num_online_cpus();
int num_sc;
struct storvsc_cmd_request *request;
struct vstor_packet *vstor_packet;
int ret, t;
- num_sc = ((max_chns > num_cpus) ? num_cpus : max_chns);
+ /*
+ * If the number of CPUs is artificially restricted, such as
+ * with maxcpus=1 on the kernel boot line, Hyper-V could offer
+ * sub-channels >= the number of CPUs. These sub-channels
+ * should not be created. The primary channel is already created
+ * and assigned to one CPU, so check against # CPUs - 1.
+ */
+ num_sc = min((int)(num_online_cpus() - 1), max_chns);
+ if (!num_sc)
+ return;
+
stor_device = get_out_stor_device(device);
if (!stor_device)
return;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] MAINTAINERS: Fix Hyperv vIOMMU driver file name
From: Mukesh Ojha @ 2019-04-01 15:01 UTC (permalink / raw)
To: Sasha Levin
Cc: lantianyu1986, davem, mchehab+samsung, gregkh, nicolas.ferre,
tglx, mingo, konrad.wilk, jpoimboe, peterz, jkosina, riel, peterz,
Tianyu.Lan, luto, michael.h.kelley, kys, joe, linux-kernel,
linux-hyperv
In-Reply-To: <20190401142026.GA2792@sasha-vm>
On 4/1/2019 7:50 PM, Sasha Levin wrote:
> On Tue, Mar 26, 2019 at 02:57:09PM +0530, Mukesh Ojha wrote:
>>
>> On 3/26/2019 11:58 AM, lantianyu1986@gmail.com wrote:
>>> From: Lan Tianyu <Tianyu.Lan@microsoft.com>
>>>
>>> The Hyperv vIOMMU file name should be "hyperv-iommu.c" rather
>>
>> s/vIOMMU/IOMMU
>
> What's wrong with vIOMMU? There's no hardware involved (afaik).
What is that extra `v`, v=>virtual
To be precise , it should be s/Hyperv vIOMMU file name/Hyper-V stub
IOMMU driver file name.
Thanks,
Mukesh
>
> --
> Thanks,
> Sasha
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: Fix Hyperv vIOMMU driver file name
From: Sasha Levin @ 2019-04-01 14:20 UTC (permalink / raw)
To: Mukesh Ojha
Cc: lantianyu1986, davem, mchehab+samsung, gregkh, nicolas.ferre,
tglx, mingo, konrad.wilk, jpoimboe, peterz, jkosina, riel, peterz,
Tianyu.Lan, luto, michael.h.kelley, kys, joe, linux-kernel,
linux-hyperv
In-Reply-To: <58c16b52-5833-ede0-c4b0-8b8839ef06b2@codeaurora.org>
On Tue, Mar 26, 2019 at 02:57:09PM +0530, Mukesh Ojha wrote:
>
>On 3/26/2019 11:58 AM, lantianyu1986@gmail.com wrote:
>>From: Lan Tianyu <Tianyu.Lan@microsoft.com>
>>
>>The Hyperv vIOMMU file name should be "hyperv-iommu.c" rather
>
>s/vIOMMU/IOMMU
What's wrong with vIOMMU? There's no hardware involved (afaik).
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH 1/1] scsi: storvsc: Fix calculation of sub-channel count
From: Vitaly Kuznetsov @ 2019-04-01 13:45 UTC (permalink / raw)
To: Michael Kelley
Cc: KY Srinivasan, martin.petersen@oracle.com, Long Li,
James.Bottomley@hansenpartnership.com, emilne@redhat.com,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-scsi@vger.kernel.org
In-Reply-To: <1554076559-14873-1-git-send-email-mikelley@microsoft.com>
Michael Kelley <mikelley@microsoft.com> writes:
> When the number of sub-channels offered by Hyper-V is >= the number
> of CPUs in the VM, calculate the correct number of sub-channels.
> The current code produces one too many.
>
> This scenario arises only when the number of CPUs is artificially
> restricted (for example, with maxcpus=<n> on the kernel boot line),
> because Hyper-V normally offers a sub-channel count < number of CPUs.
> While the current code doesn't break, the extra sub-channel is
> unbalanced across the CPUs (for example, a total of 5 channels on
> a VM with 4 CPUs).
>
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> ---
> drivers/scsi/storvsc_drv.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 84380ba..fbaa11a 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -674,7 +674,17 @@ static void handle_multichannel_storage(struct hv_device *device, int max_chns)
> struct vstor_packet *vstor_packet;
> int ret, t;
>
> - num_sc = ((max_chns > num_cpus) ? num_cpus : max_chns);
> + /*
> + * If the number of CPUs is artificially restricted, such as
> + * with maxcpus=1 on the kernel boot line, Hyper-V could offer
> + * sub-channels >= the number of CPUs. These sub-channels
> + * should not be created. The primary channel is already created
> + * and assigned to one CPU, so check against # CPUs - 1.
> + */
Indeed.
> + num_sc = min((num_cpus - 1), max_chns);
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
(nit: you could've written
num_sc = min((num_online_cpus() - 1), max_chns);
and got rid of the now-unneeded num_cpus variable).
> + if (!num_sc)
> + return;
> +
> stor_device = get_out_stor_device(device);
> if (!stor_device)
> return;
--
Vitaly
^ permalink raw reply
* [PATCH 1/1] scsi: storvsc: Fix calculation of sub-channel count
From: Michael Kelley @ 2019-03-31 23:57 UTC (permalink / raw)
To: KY Srinivasan, martin.petersen@oracle.com, Long Li,
James.Bottomley@hansenpartnership.com, emilne@redhat.com,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-scsi@vger.kernel.org
Cc: Michael Kelley
When the number of sub-channels offered by Hyper-V is >= the number
of CPUs in the VM, calculate the correct number of sub-channels.
The current code produces one too many.
This scenario arises only when the number of CPUs is artificially
restricted (for example, with maxcpus=<n> on the kernel boot line),
because Hyper-V normally offers a sub-channel count < number of CPUs.
While the current code doesn't break, the extra sub-channel is
unbalanced across the CPUs (for example, a total of 5 channels on
a VM with 4 CPUs).
Signed-off-by: Michael Kelley <mikelley@microsoft.com>
---
drivers/scsi/storvsc_drv.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 84380ba..fbaa11a 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -674,7 +674,17 @@ static void handle_multichannel_storage(struct hv_device *device, int max_chns)
struct vstor_packet *vstor_packet;
int ret, t;
- num_sc = ((max_chns > num_cpus) ? num_cpus : max_chns);
+ /*
+ * If the number of CPUs is artificially restricted, such as
+ * with maxcpus=1 on the kernel boot line, Hyper-V could offer
+ * sub-channels >= the number of CPUs. These sub-channels
+ * should not be created. The primary channel is already created
+ * and assigned to one CPU, so check against # CPUs - 1.
+ */
+ num_sc = min((num_cpus - 1), max_chns);
+ if (!num_sc)
+ return;
+
stor_device = get_out_stor_device(device);
if (!stor_device)
return;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] Remove SPDX "WITH Linux-syscall-note" from kernel-space headers
From: Masahiro Yamada @ 2019-03-30 5:06 UTC (permalink / raw)
To: Andrew Morton
Cc: Karthikeyan Ramasubramanian, Radim Krčmář,
Thomas Gleixner, Andy Gross, Vitaly Kuznetsov, Girish Mahadevan,
Sagar Dharia, linux-arch, H. Peter Anvin, Arnd Bergmann,
Haiyang Zhang, K. Y. Srinivasan, Sasha Levin, Borislav Petkov,
X86 ML, linux-hyperv, Linux Kernel Mailing List,
Stephen Hemminger, Ingo Molnar
In-Reply-To: <1553921453-23777-1-git-send-email-yamada.masahiro@socionext.com>
On Sat, Mar 30, 2019 at 1:54 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> The "WITH Linux-syscall-note" should be added to headers exported to
> the user-space.
>
> Some kernel-space headers have "WITH Linux-syscall-note", which seems
> a mistake.
>
> [1] arch/x86/include/asm/hyperv-tlfs.h
>
> 5a4858032217 ("x86/hyper-v: move hyperv.h out of uapi") moved this file
> out of uapi, but missed to update the SPDX License tag.
>
> [2] include/asm-generic/shmparam.h
>
> 76ce2a80a28e ("Rename include/{uapi => }/asm-generic/shmparam.h really")
> moved this file out of uapi, but missed to update the SPDX License tag.
>
> [3] include/linux/qcom-geni-se.h
>
> eddac5af0654 ("soc: qcom: Add GENI based QUP Wrapper driver") added this
> file, but I do now see a good reason why its license tag must include
This is a fatal typo, which makes the meaning opposite.
"I do now see ..." -> "I do not see ..."
> "WITH Linux-syscall-note".
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> arch/x86/include/asm/hyperv-tlfs.h | 2 +-
> include/asm-generic/shmparam.h | 2 +-
> include/linux/qcom-geni-se.h | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index 2bdbbbc..cdf44aa 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -1,4 +1,4 @@
> -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/* SPDX-License-Identifier: GPL-2.0 */
>
> /*
> * This file contains definitions from Hyper-V Hypervisor Top-Level Functional
> diff --git a/include/asm-generic/shmparam.h b/include/asm-generic/shmparam.h
> index 8b78c0b..b8f9035 100644
> --- a/include/asm-generic/shmparam.h
> +++ b/include/asm-generic/shmparam.h
> @@ -1,4 +1,4 @@
> -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/* SPDX-License-Identifier: GPL-2.0 */
> #ifndef __ASM_GENERIC_SHMPARAM_H
> #define __ASM_GENERIC_SHMPARAM_H
>
> diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
> index 3bcd67f..dd46494 100644
> --- a/include/linux/qcom-geni-se.h
> +++ b/include/linux/qcom-geni-se.h
> @@ -1,4 +1,4 @@
> -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/* SPDX-License-Identifier: GPL-2.0 */
> /*
> * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
> */
> --
> 2.7.4
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* [PATCH] Remove SPDX "WITH Linux-syscall-note" from kernel-space headers
From: Masahiro Yamada @ 2019-03-30 4:50 UTC (permalink / raw)
To: Andrew Morton
Cc: Karthikeyan Ramasubramanian, Radim Krčmář,
Thomas Gleixner, Andy Gross, Vitaly Kuznetsov, Girish Mahadevan,
Sagar Dharia, Masahiro Yamada, linux-arch, H. Peter Anvin,
Arnd Bergmann, Haiyang Zhang, K. Y. Srinivasan, Sasha Levin,
Borislav Petkov, x86, linux-hyperv, linux-kernel,
Stephen Hemminger, Ingo Molnar
The "WITH Linux-syscall-note" should be added to headers exported to
the user-space.
Some kernel-space headers have "WITH Linux-syscall-note", which seems
a mistake.
[1] arch/x86/include/asm/hyperv-tlfs.h
5a4858032217 ("x86/hyper-v: move hyperv.h out of uapi") moved this file
out of uapi, but missed to update the SPDX License tag.
[2] include/asm-generic/shmparam.h
76ce2a80a28e ("Rename include/{uapi => }/asm-generic/shmparam.h really")
moved this file out of uapi, but missed to update the SPDX License tag.
[3] include/linux/qcom-geni-se.h
eddac5af0654 ("soc: qcom: Add GENI based QUP Wrapper driver") added this
file, but I do now see a good reason why its license tag must include
"WITH Linux-syscall-note".
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
arch/x86/include/asm/hyperv-tlfs.h | 2 +-
include/asm-generic/shmparam.h | 2 +-
include/linux/qcom-geni-se.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 2bdbbbc..cdf44aa 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/* SPDX-License-Identifier: GPL-2.0 */
/*
* This file contains definitions from Hyper-V Hypervisor Top-Level Functional
diff --git a/include/asm-generic/shmparam.h b/include/asm-generic/shmparam.h
index 8b78c0b..b8f9035 100644
--- a/include/asm-generic/shmparam.h
+++ b/include/asm-generic/shmparam.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/* SPDX-License-Identifier: GPL-2.0 */
#ifndef __ASM_GENERIC_SHMPARAM_H
#define __ASM_GENERIC_SHMPARAM_H
diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
index 3bcd67f..dd46494 100644
--- a/include/linux/qcom-geni-se.h
+++ b/include/linux/qcom-geni-se.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/* SPDX-License-Identifier: GPL-2.0 */
/*
* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
*/
--
2.7.4
^ permalink raw reply related
* [PATCH AUTOSEL 5.0 57/67] x86/hyperv: Prevent potential NULL pointer dereference
From: Sasha Levin @ 2019-03-30 0:50 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Kangjie Lu, Thomas Gleixner, pakki001, Haiyang Zhang,
Stephen Hemminger, Sasha Levin, Borislav Petkov, H. Peter Anvin,
linux-hyperv
In-Reply-To: <20190330005047.25998-1-sashal@kernel.org>
From: Kangjie Lu <kjlu@umn.edu>
[ Upstream commit 534c89c22e26b183d838294f0937ee092c82ad3a ]
The page allocation in hv_cpu_init() can fail, but the code does not
have a check for that.
Add a check and return -ENOMEM when the allocation fails.
[ tglx: Massaged changelog ]
Signed-off-by: Kangjie Lu <kjlu@umn.edu>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>
Acked-by: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: pakki001@umn.edu
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Sasha Levin <sashal@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: linux-hyperv@vger.kernel.org
Link: https://lkml.kernel.org/r/20190314054651.1315-1-kjlu@umn.edu
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/x86/hyperv/hv_init.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 7abb09e2eeb8..dfdb4ce1ae9c 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -102,9 +102,13 @@ static int hv_cpu_init(unsigned int cpu)
u64 msr_vp_index;
struct hv_vp_assist_page **hvp = &hv_vp_assist_page[smp_processor_id()];
void **input_arg;
+ struct page *pg;
input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
- *input_arg = page_address(alloc_page(GFP_KERNEL));
+ pg = alloc_page(GFP_KERNEL);
+ if (unlikely(!pg))
+ return -ENOMEM;
+ *input_arg = page_address(pg);
hv_get_vp_index(msr_vp_index);
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 45/52] x86/hyperv: Prevent potential NULL pointer dereference
From: Sasha Levin @ 2019-03-30 0:52 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Kangjie Lu, Thomas Gleixner, pakki001, Haiyang Zhang,
Stephen Hemminger, Sasha Levin, Borislav Petkov, H. Peter Anvin,
linux-hyperv
In-Reply-To: <20190330005301.26868-1-sashal@kernel.org>
From: Kangjie Lu <kjlu@umn.edu>
[ Upstream commit 534c89c22e26b183d838294f0937ee092c82ad3a ]
The page allocation in hv_cpu_init() can fail, but the code does not
have a check for that.
Add a check and return -ENOMEM when the allocation fails.
[ tglx: Massaged changelog ]
Signed-off-by: Kangjie Lu <kjlu@umn.edu>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>
Acked-by: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: pakki001@umn.edu
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Sasha Levin <sashal@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: linux-hyperv@vger.kernel.org
Link: https://lkml.kernel.org/r/20190314054651.1315-1-kjlu@umn.edu
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/x86/hyperv/hv_init.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 20c876c7c5bf..5a81a8edff12 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -101,9 +101,13 @@ static int hv_cpu_init(unsigned int cpu)
u64 msr_vp_index;
struct hv_vp_assist_page **hvp = &hv_vp_assist_page[smp_processor_id()];
void **input_arg;
+ struct page *pg;
input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
- *input_arg = page_address(alloc_page(GFP_KERNEL));
+ pg = alloc_page(GFP_KERNEL);
+ if (unlikely(!pg))
+ return -ENOMEM;
+ *input_arg = page_address(pg);
hv_get_vp_index(msr_vp_index);
--
2.19.1
^ permalink raw reply related
* RE: [PATCH v3 3/3] Drivers: hv: vmbus: Fix race condition with new ring_buffer_info mutex
From: Michael Kelley @ 2019-03-29 16:04 UTC (permalink / raw)
To: kimbrownkd, Long Li, Sasha Levin, Stephen Hemminger, Dexuan Cui
Cc: KY Srinivasan, Haiyang Zhang, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <262046fa9e89d5f483ecd5972d86f4f9608dbcc3.1552592620.git.kimbrownkd@gmail.com>
From: Kimberly Brown <kimbrownkd@gmail.com> Sent: Thursday, March 14, 2019 1:05 PM
>
> Fix a race condition that can result in a ring buffer pointer being set
> to null while a "_show" function is reading the ring buffer's data. This
> problem was discussed here: https://lkml.org/lkml/2018/10/18/779
>
> To fix the race condition, add a new mutex lock to the
> "hv_ring_buffer_info" struct. Add a new function,
> "hv_ringbuffer_pre_init()", where a channel's inbound and outbound
> ring_buffer_info mutex locks are initialized.
>
> Acquire/release the locks in the "hv_ringbuffer_cleanup()" function,
> which is where the ring buffer pointers are set to null.
>
> Acquire/release the locks in the four channel-level "_show" functions
> that access ring buffer data. Remove the "const" qualifier from the
> "vmbus_channel" parameter and the "rbi" variable of the channel-level
> "_show" functions so that the locks can be acquired/released in these
> functions.
>
> Acquire/release the locks in hv_ringbuffer_get_debuginfo(). Remove the
> "const" qualifier from the "hv_ring_buffer_info" parameter so that the
> locks can be acquired/released in this function.
>
> Signed-off-by: Kimberly Brown <kimbrownkd@gmail.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH v3 2/3] Drivers: hv: vmbus: Set ring_info field to 0 and remove memset
From: Michael Kelley @ 2019-03-29 16:01 UTC (permalink / raw)
To: kimbrownkd, Long Li, Sasha Levin, Stephen Hemminger, Dexuan Cui
Cc: KY Srinivasan, Haiyang Zhang, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <b9b604327b885d82ddad2991f55d1b54be6eb1c8.1552592620.git.kimbrownkd@gmail.com>
From: Kimberly Brown <kimbrownkd@gmail.com> Sent: Thursday, March 14, 2019 1:05 PM
>
> Set "ring_info->priv_read_index" to 0. Now, all of ring_info's fields
> are explicitly set in this function. The memset() call is no longer
> necessary, so remove it.
>
> Signed-off-by: Kimberly Brown <kimbrownkd@gmail.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH hyperv-fixes] hv_netvsc: Fix unwanted wakeup after tx_disable
From: Haiyang Zhang @ 2019-03-28 19:59 UTC (permalink / raw)
To: Stephen Hemminger
Cc: sashal@kernel.org, linux-hyperv@vger.kernel.org, KY Srinivasan,
Stephen Hemminger, olaf@aepfle.de, vkuznets, davem@davemloft.net,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190328124205.42f5f337@shemminger-XPS-13-9360>
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Thursday, March 28, 2019 3:42 PM
> To: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: Haiyang Zhang <haiyangz@linuxonhyperv.com>; sashal@kernel.org;
> linux-hyperv@vger.kernel.org; KY Srinivasan <kys@microsoft.com>; Stephen
> Hemminger <sthemmin@microsoft.com>; olaf@aepfle.de; vkuznets
> <vkuznets@redhat.com>; davem@davemloft.net; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH hyperv-fixes] hv_netvsc: Fix unwanted wakeup after
> tx_disable
>
> On Thu, 28 Mar 2019 19:00:18 +0000
> Haiyang Zhang <haiyangz@microsoft.com> wrote:
>
> > > -----Original Message-----
> > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > Sent: Thursday, March 28, 2019 2:38 PM
> > > To: Haiyang Zhang <haiyangz@linuxonhyperv.com>
> > > Cc: sashal@kernel.org; linux-hyperv@vger.kernel.org; Haiyang Zhang
> > > <haiyangz@microsoft.com>; KY Srinivasan <kys@microsoft.com>;
> Stephen
> > > Hemminger <sthemmin@microsoft.com>; olaf@aepfle.de; vkuznets
> > > <vkuznets@redhat.com>; davem@davemloft.net;
> netdev@vger.kernel.org;
> > > linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH hyperv-fixes] hv_netvsc: Fix unwanted wakeup
> > > after tx_disable
> > >
> > > On Thu, 28 Mar 2019 17:48:45 +0000
> > > Haiyang Zhang <haiyangz@linuxonhyperv.com> wrote:
> > >
> > > > +static inline void netvsc_tx_enable(struct netvsc_device *nvscdev,
> > > > + struct net_device *ndev)
> > > > +{
> > > > + nvscdev->tx_disable = false;
> > > > + mb(); /* ensure queue wake up mechanism is on */
> > > > +
> > > > + netif_tx_wake_all_queues(ndev);
> > > > +}
> > >
> > > You don't need a full mb(). virt_wmb() should be sufficient.
> >
> > I will make this change.
> >
> > > Could I suggest an alternative approach.
> > > You don't need to introduce a local tx_disable flag, the only place
> > > where a wakeup could cause problems is after a send_completion was
> > > processed during detach state.
> > >
> > > Instead, just avoid wakeup in that place.
> >
> > In netvsc_detach(), after netif_tx_disable(), we call
> > netvsc_wait_until_empty(nvdev); TX patch should not be waken up again
> while waiting for in/out ring to becomes empty.
> >
> > In my tests before this patch, there are wakeup happens before
> > netif_device_detach(), so netif_device_present(ndev) is still true at that
> time.
> >
> > In other places, like netvsc_close(), link_change(), we also don't want
> wakeup after tx_disable.
> >
> > Thanks.
> > - Haiyang
> >
> > >
> > > --- a/drivers/net/hyperv/netvsc.c
> > > +++ b/drivers/net/hyperv/netvsc.c
> > > @@ -720,6 +720,7 @@ static void netvsc_send_tx_complete(struct
> > > net_device *ndev,
> > > struct netdev_queue *txq = netdev_get_tx_queue(ndev,
> > > q_idx);
> > >
> > > if (netif_tx_queue_stopped(txq) &&
> > > + netif_device_present(ndev) &&
> > > (hv_get_avail_to_write_percent(&channel->outbound) >
> > > RING_AVAIL_PERCENT_HIWATER || queue_sends < 1)) {
> > > netif_tx_wake_queue(txq);
>
>
> Then what about doing netif_detach earlier in netvsc_detach.
>
> The state management is already (too) complex in netvsc and adding another
> boolean flag just makes it harder to understand.
If we move netif_device_detach() before the netvsc_wait_until_empty(), the remaining
Packets in the receive buffer will be passed to a detached device.
Also, in case of netvsc_close() and link_change(), we don't call netif_device_detach().
Thanks,
- Haiyang
^ permalink raw reply
* Re: [PATCH hyperv-fixes] hv_netvsc: Fix unwanted wakeup after tx_disable
From: Stephen Hemminger @ 2019-03-28 19:42 UTC (permalink / raw)
To: Haiyang Zhang
Cc: Haiyang Zhang, sashal@kernel.org, linux-hyperv@vger.kernel.org,
KY Srinivasan, Stephen Hemminger, olaf@aepfle.de, vkuznets,
davem@davemloft.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <DM6PR21MB1337250A47CEE73D1EFE0AB7CA590@DM6PR21MB1337.namprd21.prod.outlook.com>
On Thu, 28 Mar 2019 19:00:18 +0000
Haiyang Zhang <haiyangz@microsoft.com> wrote:
> > -----Original Message-----
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Sent: Thursday, March 28, 2019 2:38 PM
> > To: Haiyang Zhang <haiyangz@linuxonhyperv.com>
> > Cc: sashal@kernel.org; linux-hyperv@vger.kernel.org; Haiyang Zhang
> > <haiyangz@microsoft.com>; KY Srinivasan <kys@microsoft.com>; Stephen
> > Hemminger <sthemmin@microsoft.com>; olaf@aepfle.de; vkuznets
> > <vkuznets@redhat.com>; davem@davemloft.net; netdev@vger.kernel.org;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH hyperv-fixes] hv_netvsc: Fix unwanted wakeup after
> > tx_disable
> >
> > On Thu, 28 Mar 2019 17:48:45 +0000
> > Haiyang Zhang <haiyangz@linuxonhyperv.com> wrote:
> >
> > > +static inline void netvsc_tx_enable(struct netvsc_device *nvscdev,
> > > + struct net_device *ndev)
> > > +{
> > > + nvscdev->tx_disable = false;
> > > + mb(); /* ensure queue wake up mechanism is on */
> > > +
> > > + netif_tx_wake_all_queues(ndev);
> > > +}
> >
> > You don't need a full mb(). virt_wmb() should be sufficient.
>
> I will make this change.
>
> > Could I suggest an alternative approach.
> > You don't need to introduce a local tx_disable flag, the only place where a
> > wakeup could cause problems is after a send_completion was processed
> > during detach state.
> >
> > Instead, just avoid wakeup in that place.
>
> In netvsc_detach(), after netif_tx_disable(), we call netvsc_wait_until_empty(nvdev);
> TX patch should not be waken up again while waiting for in/out ring to becomes empty.
>
> In my tests before this patch, there are wakeup happens before netif_device_detach(),
> so netif_device_present(ndev) is still true at that time.
>
> In other places, like netvsc_close(), link_change(), we also don't want wakeup after tx_disable.
>
> Thanks.
> - Haiyang
>
> >
> > --- a/drivers/net/hyperv/netvsc.c
> > +++ b/drivers/net/hyperv/netvsc.c
> > @@ -720,6 +720,7 @@ static void netvsc_send_tx_complete(struct
> > net_device *ndev,
> > struct netdev_queue *txq = netdev_get_tx_queue(ndev, q_idx);
> >
> > if (netif_tx_queue_stopped(txq) &&
> > + netif_device_present(ndev) &&
> > (hv_get_avail_to_write_percent(&channel->outbound) >
> > RING_AVAIL_PERCENT_HIWATER || queue_sends < 1)) {
> > netif_tx_wake_queue(txq);
Then what about doing netif_detach earlier in netvsc_detach.
The state management is already (too) complex in netvsc and adding another
boolean flag just makes it harder to understand.
^ permalink raw reply
* [PATCH hyperv-fixes,v3] hv_netvsc: Fix unwanted wakeup after tx_disable
From: Haiyang Zhang @ 2019-03-28 19:40 UTC (permalink / raw)
To: sashal, linux-hyperv
Cc: haiyangz, kys, sthemmin, olaf, vkuznets, davem, netdev,
linux-kernel
From: Haiyang Zhang <haiyangz@microsoft.com>
After queue stopped, the wakeup mechanism may wake it up again
when ring buffer usage is lower than a threshold. This may cause
send path panic on NULL pointer when we stopped all tx queues in
netvsc_detach and start removing the netvsc device.
This patch fix it by adding a tx_disable flag to prevent unwanted
queue wakeup.
Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
Reported-by: Mohammed Gamal <mgamal@redhat.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/hyperv/hyperv_net.h | 1 +
drivers/net/hyperv/netvsc.c | 6 ++++--
drivers/net/hyperv/netvsc_drv.c | 32 ++++++++++++++++++++++++++------
3 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index e859ae2..49f41b6 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -987,6 +987,7 @@ struct netvsc_device {
wait_queue_head_t wait_drain;
bool destroy;
+ bool tx_disable; /* if true, do not wake up queue again */
/* Receive buffer allocated by us but manages by NetVSP */
void *recv_buf;
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 813d195..e0dce37 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -110,6 +110,7 @@ static struct netvsc_device *alloc_net_device(void)
init_waitqueue_head(&net_device->wait_drain);
net_device->destroy = false;
+ net_device->tx_disable = false;
net_device->max_pkt = RNDIS_MAX_PKT_DEFAULT;
net_device->pkt_align = RNDIS_PKT_ALIGN_DEFAULT;
@@ -719,7 +720,7 @@ static void netvsc_send_tx_complete(struct net_device *ndev,
} else {
struct netdev_queue *txq = netdev_get_tx_queue(ndev, q_idx);
- if (netif_tx_queue_stopped(txq) &&
+ if (netif_tx_queue_stopped(txq) && !net_device->tx_disable &&
(hv_get_avail_to_write_percent(&channel->outbound) >
RING_AVAIL_PERCENT_HIWATER || queue_sends < 1)) {
netif_tx_wake_queue(txq);
@@ -874,7 +875,8 @@ static inline int netvsc_send_pkt(
} else if (ret == -EAGAIN) {
netif_tx_stop_queue(txq);
ndev_ctx->eth_stats.stop_queue++;
- if (atomic_read(&nvchan->queue_sends) < 1) {
+ if (atomic_read(&nvchan->queue_sends) < 1 &&
+ !net_device->tx_disable) {
netif_tx_wake_queue(txq);
ndev_ctx->eth_stats.wake_queue++;
ret = -ENOSPC;
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 1a08679..06393b2 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -109,6 +109,15 @@ static void netvsc_set_rx_mode(struct net_device *net)
rcu_read_unlock();
}
+static void netvsc_tx_enable(struct netvsc_device *nvscdev,
+ struct net_device *ndev)
+{
+ nvscdev->tx_disable = false;
+ virt_wmb(); /* ensure queue wake up mechanism is on */
+
+ netif_tx_wake_all_queues(ndev);
+}
+
static int netvsc_open(struct net_device *net)
{
struct net_device_context *ndev_ctx = netdev_priv(net);
@@ -129,7 +138,7 @@ static int netvsc_open(struct net_device *net)
rdev = nvdev->extension;
if (!rdev->link_state) {
netif_carrier_on(net);
- netif_tx_wake_all_queues(net);
+ netvsc_tx_enable(nvdev, net);
}
if (vf_netdev) {
@@ -184,6 +193,17 @@ static int netvsc_wait_until_empty(struct netvsc_device *nvdev)
}
}
+static void netvsc_tx_disable(struct netvsc_device *nvscdev,
+ struct net_device *ndev)
+{
+ if (nvscdev) {
+ nvscdev->tx_disable = true;
+ virt_wmb(); /* ensure txq will not wake up after stop */
+ }
+
+ netif_tx_disable(ndev);
+}
+
static int netvsc_close(struct net_device *net)
{
struct net_device_context *net_device_ctx = netdev_priv(net);
@@ -192,7 +212,7 @@ static int netvsc_close(struct net_device *net)
struct netvsc_device *nvdev = rtnl_dereference(net_device_ctx->nvdev);
int ret;
- netif_tx_disable(net);
+ netvsc_tx_disable(nvdev, net);
/* No need to close rndis filter if it is removed already */
if (!nvdev)
@@ -918,7 +938,7 @@ static int netvsc_detach(struct net_device *ndev,
/* If device was up (receiving) then shutdown */
if (netif_running(ndev)) {
- netif_tx_disable(ndev);
+ netvsc_tx_disable(nvdev, ndev);
ret = rndis_filter_close(nvdev);
if (ret) {
@@ -1906,7 +1926,7 @@ static void netvsc_link_change(struct work_struct *w)
if (rdev->link_state) {
rdev->link_state = false;
netif_carrier_on(net);
- netif_tx_wake_all_queues(net);
+ netvsc_tx_enable(net_device, net);
} else {
notify = true;
}
@@ -1916,7 +1936,7 @@ static void netvsc_link_change(struct work_struct *w)
if (!rdev->link_state) {
rdev->link_state = true;
netif_carrier_off(net);
- netif_tx_stop_all_queues(net);
+ netvsc_tx_disable(net_device, net);
}
kfree(event);
break;
@@ -1925,7 +1945,7 @@ static void netvsc_link_change(struct work_struct *w)
if (!rdev->link_state) {
rdev->link_state = true;
netif_carrier_off(net);
- netif_tx_stop_all_queues(net);
+ netvsc_tx_disable(net_device, net);
event->event = RNDIS_STATUS_MEDIA_CONNECT;
spin_lock_irqsave(&ndev_ctx->lock, flags);
list_add(&event->list, &ndev_ctx->reconfig_events);
--
1.8.3.1
^ permalink raw reply related
* [PATCH hyperv-fixes,v2] hv_netvsc: Fix unwanted wakeup after tx_disable
From: Haiyang Zhang @ 2019-03-28 19:16 UTC (permalink / raw)
To: sashal, linux-hyperv
Cc: haiyangz, kys, sthemmin, olaf, vkuznets, davem, netdev,
linux-kernel
From: Haiyang Zhang <haiyangz@microsoft.com>
After queue stopped, the wakeup mechanism may wake it up again
when ring buffer usage is lower than a threshold. This may cause
send path panic on NULL pointer when we stopped all tx queues in
netvsc_detach and start removing the netvsc device.
This patch fix it by adding a tx_disable flag to prevent unwanted
queue wakeup.
Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
Reported-by: Mohammed Gamal <mgamal@redhat.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/hyperv/hyperv_net.h | 1 +
drivers/net/hyperv/netvsc.c | 6 ++++--
drivers/net/hyperv/netvsc_drv.c | 32 ++++++++++++++++++++++++++------
3 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index e859ae2..49f41b6 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -987,6 +987,7 @@ struct netvsc_device {
wait_queue_head_t wait_drain;
bool destroy;
+ bool tx_disable; /* if true, do not wake up queue again */
/* Receive buffer allocated by us but manages by NetVSP */
void *recv_buf;
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 813d195..e0dce37 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -110,6 +110,7 @@ static struct netvsc_device *alloc_net_device(void)
init_waitqueue_head(&net_device->wait_drain);
net_device->destroy = false;
+ net_device->tx_disable = false;
net_device->max_pkt = RNDIS_MAX_PKT_DEFAULT;
net_device->pkt_align = RNDIS_PKT_ALIGN_DEFAULT;
@@ -719,7 +720,7 @@ static void netvsc_send_tx_complete(struct net_device *ndev,
} else {
struct netdev_queue *txq = netdev_get_tx_queue(ndev, q_idx);
- if (netif_tx_queue_stopped(txq) &&
+ if (netif_tx_queue_stopped(txq) && !net_device->tx_disable &&
(hv_get_avail_to_write_percent(&channel->outbound) >
RING_AVAIL_PERCENT_HIWATER || queue_sends < 1)) {
netif_tx_wake_queue(txq);
@@ -874,7 +875,8 @@ static inline int netvsc_send_pkt(
} else if (ret == -EAGAIN) {
netif_tx_stop_queue(txq);
ndev_ctx->eth_stats.stop_queue++;
- if (atomic_read(&nvchan->queue_sends) < 1) {
+ if (atomic_read(&nvchan->queue_sends) < 1 &&
+ !net_device->tx_disable) {
netif_tx_wake_queue(txq);
ndev_ctx->eth_stats.wake_queue++;
ret = -ENOSPC;
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 1a08679..ffb7922 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -109,6 +109,15 @@ static void netvsc_set_rx_mode(struct net_device *net)
rcu_read_unlock();
}
+static inline void netvsc_tx_enable(struct netvsc_device *nvscdev,
+ struct net_device *ndev)
+{
+ nvscdev->tx_disable = false;
+ virt_wmb(); /* ensure queue wake up mechanism is on */
+
+ netif_tx_wake_all_queues(ndev);
+}
+
static int netvsc_open(struct net_device *net)
{
struct net_device_context *ndev_ctx = netdev_priv(net);
@@ -129,7 +138,7 @@ static int netvsc_open(struct net_device *net)
rdev = nvdev->extension;
if (!rdev->link_state) {
netif_carrier_on(net);
- netif_tx_wake_all_queues(net);
+ netvsc_tx_enable(nvdev, net);
}
if (vf_netdev) {
@@ -184,6 +193,17 @@ static int netvsc_wait_until_empty(struct netvsc_device *nvdev)
}
}
+static inline void netvsc_tx_disable(struct netvsc_device *nvscdev,
+ struct net_device *ndev)
+{
+ if (nvscdev) {
+ nvscdev->tx_disable = true;
+ virt_wmb(); /* ensure txq will not wake up after stop */
+ }
+
+ netif_tx_disable(ndev);
+}
+
static int netvsc_close(struct net_device *net)
{
struct net_device_context *net_device_ctx = netdev_priv(net);
@@ -192,7 +212,7 @@ static int netvsc_close(struct net_device *net)
struct netvsc_device *nvdev = rtnl_dereference(net_device_ctx->nvdev);
int ret;
- netif_tx_disable(net);
+ netvsc_tx_disable(nvdev, net);
/* No need to close rndis filter if it is removed already */
if (!nvdev)
@@ -918,7 +938,7 @@ static int netvsc_detach(struct net_device *ndev,
/* If device was up (receiving) then shutdown */
if (netif_running(ndev)) {
- netif_tx_disable(ndev);
+ netvsc_tx_disable(nvdev, ndev);
ret = rndis_filter_close(nvdev);
if (ret) {
@@ -1906,7 +1926,7 @@ static void netvsc_link_change(struct work_struct *w)
if (rdev->link_state) {
rdev->link_state = false;
netif_carrier_on(net);
- netif_tx_wake_all_queues(net);
+ netvsc_tx_enable(net_device, net);
} else {
notify = true;
}
@@ -1916,7 +1936,7 @@ static void netvsc_link_change(struct work_struct *w)
if (!rdev->link_state) {
rdev->link_state = true;
netif_carrier_off(net);
- netif_tx_stop_all_queues(net);
+ netvsc_tx_disable(net_device, net);
}
kfree(event);
break;
@@ -1925,7 +1945,7 @@ static void netvsc_link_change(struct work_struct *w)
if (!rdev->link_state) {
rdev->link_state = true;
netif_carrier_off(net);
- netif_tx_stop_all_queues(net);
+ netvsc_tx_disable(net_device, net);
event->event = RNDIS_STATUS_MEDIA_CONNECT;
spin_lock_irqsave(&ndev_ctx->lock, flags);
list_add(&event->list, &ndev_ctx->reconfig_events);
--
1.8.3.1
^ permalink raw reply related
* RE: [PATCH hyperv-fixes,v2] hv_netvsc: Fix unwanted wakeup after tx_disable
From: Haiyang Zhang @ 2019-03-28 19:27 UTC (permalink / raw)
To: David Miller
Cc: sashal@kernel.org, linux-hyperv@vger.kernel.org, KY Srinivasan,
Stephen Hemminger, olaf@aepfle.de, vkuznets,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190328.122214.1269115693598269959.davem@davemloft.net>
> -----Original Message-----
> From: linux-hyperv-owner@vger.kernel.org <linux-hyperv-
> owner@vger.kernel.org> On Behalf Of David Miller
> Sent: Thursday, March 28, 2019 3:22 PM
> To: haiyangz@linuxonhyperv.com
> Cc: sashal@kernel.org; linux-hyperv@vger.kernel.org; Haiyang Zhang
> <haiyangz@microsoft.com>; KY Srinivasan <kys@microsoft.com>; Stephen
> Hemminger <sthemmin@microsoft.com>; olaf@aepfle.de; vkuznets
> <vkuznets@redhat.com>; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH hyperv-fixes,v2] hv_netvsc: Fix unwanted wakeup after
> tx_disable
>
> From: Haiyang Zhang <haiyangz@linuxonhyperv.com>
> Date: Thu, 28 Mar 2019 19:16:37 +0000
>
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> >
> > After queue stopped, the wakeup mechanism may wake it up again when
> > ring buffer usage is lower than a threshold. This may cause send path
> > panic on NULL pointer when we stopped all tx queues in netvsc_detach
> > and start removing the netvsc device.
> >
> > This patch fix it by adding a tx_disable flag to prevent unwanted
> > queue wakeup.
> >
> > Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
> > Reported-by: Mohammed Gamal <mgamal@redhat.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
>
> Still need to fix the inline issues I mentioned in v1.
I will fix the inline issues.
The haiyangz@linuxonhyperv.com cannot receive emails.
My regular email is in the reply-to: field.
Also, linuxonhyperv.com is being replaced by a new system soon.
Thanks.
- Haiyang
^ permalink raw reply
* Re: [PATCH hyperv-fixes,v2] hv_netvsc: Fix unwanted wakeup after tx_disable
From: David Miller @ 2019-03-28 19:23 UTC (permalink / raw)
To: haiyangz
Cc: sashal, linux-hyperv, haiyangz, kys, sthemmin, olaf, vkuznets,
netdev, linux-kernel
In-Reply-To: <20190328.122214.1269115693598269959.davem@davemloft.net>
Also, your email bounces:
<haiyangz@linuxonhyperv.com>: host mail.linuxonhyperv.com[107.180.71.197] said:
550 No Such User Here (in reply to RCPT TO command)
^ permalink raw reply
* Re: [PATCH hyperv-fixes,v2] hv_netvsc: Fix unwanted wakeup after tx_disable
From: David Miller @ 2019-03-28 19:22 UTC (permalink / raw)
To: haiyangz
Cc: sashal, linux-hyperv, haiyangz, kys, sthemmin, olaf, vkuznets,
netdev, linux-kernel
In-Reply-To: <20190328191637.6698-1-haiyangz@linuxonhyperv.com>
From: Haiyang Zhang <haiyangz@linuxonhyperv.com>
Date: Thu, 28 Mar 2019 19:16:37 +0000
> From: Haiyang Zhang <haiyangz@microsoft.com>
>
> After queue stopped, the wakeup mechanism may wake it up again
> when ring buffer usage is lower than a threshold. This may cause
> send path panic on NULL pointer when we stopped all tx queues in
> netvsc_detach and start removing the netvsc device.
>
> This patch fix it by adding a tx_disable flag to prevent unwanted
> queue wakeup.
>
> Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
> Reported-by: Mohammed Gamal <mgamal@redhat.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Still need to fix the inline issues I mentioned in v1.
^ permalink raw reply
* Re: [PATCH hyperv-fixes] hv_netvsc: Fix unwanted wakeup after tx_disable
From: David Miller @ 2019-03-28 19:21 UTC (permalink / raw)
To: haiyangz
Cc: sashal, linux-hyperv, haiyangz, kys, sthemmin, olaf, vkuznets,
netdev, linux-kernel
In-Reply-To: <20190328174845.4799-1-haiyangz@linuxonhyperv.com>
From: Haiyang Zhang <haiyangz@linuxonhyperv.com>
Date: Thu, 28 Mar 2019 17:48:45 +0000
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 1a08679..0824155 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -109,6 +109,15 @@ static void netvsc_set_rx_mode(struct net_device *net)
> rcu_read_unlock();
> }
>
> +static inline void netvsc_tx_enable(struct netvsc_device *nvscdev,
> + struct net_device *ndev)
Do not use inline in foo.c files, let the compiler decide.
> @@ -184,6 +193,17 @@ static int netvsc_wait_until_empty(struct netvsc_device *nvdev)
> }
> }
>
> +static inline void netvsc_tx_disable(struct netvsc_device *nvscdev,
> + struct net_device *ndev)
> +{
Likewise.
^ permalink raw reply
* RE: [PATCH hyperv-fixes] hv_netvsc: Fix unwanted wakeup after tx_disable
From: Haiyang Zhang @ 2019-03-28 19:00 UTC (permalink / raw)
To: Stephen Hemminger, Haiyang Zhang
Cc: sashal@kernel.org, linux-hyperv@vger.kernel.org, KY Srinivasan,
Stephen Hemminger, olaf@aepfle.de, vkuznets, davem@davemloft.net,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190328113823.3c9b3599@shemminger-XPS-13-9360>
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Thursday, March 28, 2019 2:38 PM
> To: Haiyang Zhang <haiyangz@linuxonhyperv.com>
> Cc: sashal@kernel.org; linux-hyperv@vger.kernel.org; Haiyang Zhang
> <haiyangz@microsoft.com>; KY Srinivasan <kys@microsoft.com>; Stephen
> Hemminger <sthemmin@microsoft.com>; olaf@aepfle.de; vkuznets
> <vkuznets@redhat.com>; davem@davemloft.net; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH hyperv-fixes] hv_netvsc: Fix unwanted wakeup after
> tx_disable
>
> On Thu, 28 Mar 2019 17:48:45 +0000
> Haiyang Zhang <haiyangz@linuxonhyperv.com> wrote:
>
> > +static inline void netvsc_tx_enable(struct netvsc_device *nvscdev,
> > + struct net_device *ndev)
> > +{
> > + nvscdev->tx_disable = false;
> > + mb(); /* ensure queue wake up mechanism is on */
> > +
> > + netif_tx_wake_all_queues(ndev);
> > +}
>
> You don't need a full mb(). virt_wmb() should be sufficient.
I will make this change.
> Could I suggest an alternative approach.
> You don't need to introduce a local tx_disable flag, the only place where a
> wakeup could cause problems is after a send_completion was processed
> during detach state.
>
> Instead, just avoid wakeup in that place.
In netvsc_detach(), after netif_tx_disable(), we call netvsc_wait_until_empty(nvdev);
TX patch should not be waken up again while waiting for in/out ring to becomes empty.
In my tests before this patch, there are wakeup happens before netif_device_detach(),
so netif_device_present(ndev) is still true at that time.
In other places, like netvsc_close(), link_change(), we also don't want wakeup after tx_disable.
Thanks.
- Haiyang
>
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -720,6 +720,7 @@ static void netvsc_send_tx_complete(struct
> net_device *ndev,
> struct netdev_queue *txq = netdev_get_tx_queue(ndev, q_idx);
>
> if (netif_tx_queue_stopped(txq) &&
> + netif_device_present(ndev) &&
> (hv_get_avail_to_write_percent(&channel->outbound) >
> RING_AVAIL_PERCENT_HIWATER || queue_sends < 1)) {
> netif_tx_wake_queue(txq);
^ permalink raw reply
* Re: [PATCH v3 3/3] Drivers: hv: vmbus: Fix race condition with new ring_buffer_info mutex
From: Stephen Hemminger @ 2019-03-28 18:42 UTC (permalink / raw)
To: Kimberly Brown
Cc: Michael Kelley, Long Li, Sasha Levin, Stephen Hemminger,
Dexuan Cui, KY Srinivasan, Haiyang Zhang,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190328043057.GA2258@ubu-Virtual-Machine>
On Thu, 28 Mar 2019 00:30:57 -0400
Kimberly Brown <kimbrownkd@gmail.com> wrote:
> On Thu, Mar 21, 2019 at 04:04:20PM +0000, Michael Kelley wrote:
> > From: Kimberly Brown <kimbrownkd@gmail.com> Sent: Wednesday, March 20, 2019 8:48 PM
> > > > > > Adding more locks will solve the problem but it seems like overkill.
> > > > > > Why not either use a reference count or an RCU style access for the
> > > > > > ring buffer?
> > > > >
> > > > > I agree that a reference count or RCU could also solve this problem.
> > > > > Using mutex locks seemed like the most straightforward solution, but
> > > > > I'll certainly switch to a different approach if it's better!
> > > > >
> > > > > Are you concerned about the extra memory required for the mutex locks,
> > > > > read performance, or something else?
> > > >
> > > > Locks in control path are ok, but my concern is performance of the
> > > > data path which puts packets in/out of rings. To keep reasonable performance,
> > > > no additional locking should be added in those paths.
> > > >
> > > > So if data path is using RCU, can/should the control operations also
> > > > use it?
> > >
>
>
> Hi Stephen,
>
> Do you have any additional questions or suggestions for this race
> condition and the mutex locks? I think that your initial questions were
> addressed in the responses below. If there's anything else, please let
> me know!
>
> Thanks,
> Kim
>
>
> > > The data path doesn't use RCU to protect the ring buffers.
> >
> > My $.02: The mutex is obtained only in the sysfs path and the "delete
> > ringbuffers" path, neither of which is performance or concurrency sensitive.
> > There's no change to any path that reads or writes data from/to the ring
> > buffers. It seems like the mutex is the most straightforward solution to
> > preventing sysfs from accessing the ring buffer info while the memory is
> > being freed as part of "delete ringbuffers".
> >
> > Michael
I have no problems with the patch you did.
My discussion was more around the general issues with ringbuffers being detached
from the device. Not sure if it was even a good design choice but that is
something that is hard to fix now.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox