* [PATCH hyperv-fixes] hv_netvsc: Fix unwanted wakeup after tx_disable
From: Haiyang Zhang @ 2019-03-28 17:48 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..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)
+{
+ nvscdev->tx_disable = false;
+ mb(); /* 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;
+ mb(); /* 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] hv_netvsc: Fix unwanted wakeup after tx_disable
From: Stephen Hemminger @ 2019-03-28 18:38 UTC (permalink / raw)
To: Haiyang Zhang
Cc: sashal, linux-hyperv, haiyangz, kys, sthemmin, olaf, vkuznets,
davem, netdev, linux-kernel
In-Reply-To: <20190328174845.4799-1-haiyangz@linuxonhyperv.com>
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.
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.
--- 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
* 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 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,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,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: 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
* [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
* [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
* 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
* 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 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 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
* [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
* [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] 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
* 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 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 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
* 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] 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
* [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 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
* 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
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