1. Patchew
  2. Xen
  3. View series

[PATCH 5.4] xen-netback: move removal of "hotplug-status" to the right place

Pratyush Yadav posted 1 patch 1 year, 4 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
drivers/net/xen-netback/xenbus.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH 5.4] xen-netback: move removal of "hotplug-status" to the right place
Posted by Pratyush Yadav 1 year, 4 months ago 
The removal of "hotplug-status" has moved around a bit. First it was
moved from netback_remove() to hotplug_status_changed() in upstream
commit 1f2565780e9b ("xen-netback: remove 'hotplug-status' once it has
served its purpose"). Then the change was reverted in upstream commit
0f4558ae9187 ("Revert "xen-netback: remove 'hotplug-status' once it has
served its purpose""), but it moved the removal to backend_disconnect().
Then the upstream commit c55f34b6aec2 ("xen-netback: only remove
'hotplug-status' when the vif is actually destroyed") moved it finally
back to netback_remove(). The thing to note being it is removed
unconditionally this time around.

The story on v5.4.y adds to this confusion. Commit 60e4e3198ce8 ("Revert
"xen-netback: remove 'hotplug-status' once it has served its purpose"")
is backported to v5.4.y but the original commit that it tries to revert
was never present on 5.4. So the backport incorrectly ends up just
adding another xenbus_rm() of "hotplug-status" in backend_disconnect().

Now in v5.4.y it is removed in both backend_disconnect() and
netback_remove(). But it should only be removed in netback_remove(), as
the upstream version does.

Removing "hotplug-status" in backend_disconnect() causes problems when
the frontend unilaterally disconnects, as explained in
c55f34b6aec2 ("xen-netback: only remove 'hotplug-status' when the vif is
actually destroyed").

Remove "hotplug-status" in the same place as it is done on the upstream
version to ensure unilateral re-connection of frontend continues to
work.

Fixes: 60e4e3198ce8 ("Revert "xen-netback: remove 'hotplug-status' once it has served its purpose"")
Signed-off-by: Pratyush Yadav <ptyadav@amazon.de>
---
 drivers/net/xen-netback/xenbus.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 44e353dd2ba1..43bd881ab3dd 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -202,10 +202,10 @@ static int netback_remove(struct xenbus_device *dev)
 	set_backend_state(be, XenbusStateClosed);

 	unregister_hotplug_status_watch(be);
+	xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
 	if (be->vif) {
 		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
 		xen_unregister_watchers(be->vif);
-		xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
 		xenvif_free(be->vif);
 		be->vif = NULL;
 	}
@@ -435,7 +435,6 @@ static void backend_disconnect(struct backend_info *be)
 		unsigned int queue_index;

 		xen_unregister_watchers(vif);
-		xenbus_rm(XBT_NIL, be->dev->nodename, "hotplug-status");
 #ifdef CONFIG_DEBUG_FS
 		xenvif_debugfs_delif(vif);
 #endif /* CONFIG_DEBUG_FS */
--
2.38.1
Re: [PATCH 5.4] xen-netback: move removal of "hotplug-status" to the right place
Posted by Greg Kroah-Hartman 1 year, 4 months ago 
On Mon, Dec 19, 2022 at 04:37:10PM +0100, Pratyush Yadav wrote:
> The removal of "hotplug-status" has moved around a bit. First it was
> moved from netback_remove() to hotplug_status_changed() in upstream
> commit 1f2565780e9b ("xen-netback: remove 'hotplug-status' once it has
> served its purpose"). Then the change was reverted in upstream commit
> 0f4558ae9187 ("Revert "xen-netback: remove 'hotplug-status' once it has
> served its purpose""), but it moved the removal to backend_disconnect().
> Then the upstream commit c55f34b6aec2 ("xen-netback: only remove
> 'hotplug-status' when the vif is actually destroyed") moved it finally
> back to netback_remove(). The thing to note being it is removed
> unconditionally this time around.
> 
> The story on v5.4.y adds to this confusion. Commit 60e4e3198ce8 ("Revert
> "xen-netback: remove 'hotplug-status' once it has served its purpose"")
> is backported to v5.4.y but the original commit that it tries to revert
> was never present on 5.4. So the backport incorrectly ends up just
> adding another xenbus_rm() of "hotplug-status" in backend_disconnect().
> 
> Now in v5.4.y it is removed in both backend_disconnect() and
> netback_remove(). But it should only be removed in netback_remove(), as
> the upstream version does.
> 
> Removing "hotplug-status" in backend_disconnect() causes problems when
> the frontend unilaterally disconnects, as explained in
> c55f34b6aec2 ("xen-netback: only remove 'hotplug-status' when the vif is
> actually destroyed").
> 
> Remove "hotplug-status" in the same place as it is done on the upstream
> version to ensure unilateral re-connection of frontend continues to
> work.
> 
> Fixes: 60e4e3198ce8 ("Revert "xen-netback: remove 'hotplug-status' once it has served its purpose"")
> Signed-off-by: Pratyush Yadav <ptyadav@amazon.de>
> ---
>  drivers/net/xen-netback/xenbus.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Now queued up, thanks.

greg k-h
Patch "xen-netback: move removal of "hotplug-status" to the right place" has been added to the 5.4-stable tree
Posted by gregkh@linuxfoundation.org 1 year, 4 months ago 

This is a note to let you know that I've just added the patch titled

    xen-netback: move removal of "hotplug-status" to the right place

to the 5.4-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     xen-netback-move-removal-of-hotplug-status-to-the-right-place.patch
and it can be found in the queue-5.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


From ptyadav@amazon.de  Mon Dec 19 16:41:51 2022
From: Pratyush Yadav <ptyadav@amazon.de>
Date: Mon, 19 Dec 2022 16:37:10 +0100
Subject: xen-netback: move removal of "hotplug-status" to the right place
To: <stable@vger.kernel.org>
Cc: "Pratyush Yadav" <ptyadav@amazon.de>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Wei Liu" <wei.liu@kernel.org>, "Paul Durrant" <paul@xen.org>, "David S. Miller" <davem@davemloft.net>, "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>, "Jakub Kicinski" <kuba@kernel.org>, "Sasha Levin" <sashal@kernel.org>, "Puranjay Mohan" <pjy@amazon.de>, "Maximilian Heyne" <mheyne@amazon.de>, "Julien Grall" <julien@xen.org>, xen-devel@lists.xenproject.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Message-ID: <20221219153710.23782-1-ptyadav@amazon.de>

From: Pratyush Yadav <ptyadav@amazon.de>

The removal of "hotplug-status" has moved around a bit. First it was
moved from netback_remove() to hotplug_status_changed() in upstream
commit 1f2565780e9b ("xen-netback: remove 'hotplug-status' once it has
served its purpose"). Then the change was reverted in upstream commit
0f4558ae9187 ("Revert "xen-netback: remove 'hotplug-status' once it has
served its purpose""), but it moved the removal to backend_disconnect().
Then the upstream commit c55f34b6aec2 ("xen-netback: only remove
'hotplug-status' when the vif is actually destroyed") moved it finally
back to netback_remove(). The thing to note being it is removed
unconditionally this time around.

The story on v5.4.y adds to this confusion. Commit 60e4e3198ce8 ("Revert
"xen-netback: remove 'hotplug-status' once it has served its purpose"")
is backported to v5.4.y but the original commit that it tries to revert
was never present on 5.4. So the backport incorrectly ends up just
adding another xenbus_rm() of "hotplug-status" in backend_disconnect().

Now in v5.4.y it is removed in both backend_disconnect() and
netback_remove(). But it should only be removed in netback_remove(), as
the upstream version does.

Removing "hotplug-status" in backend_disconnect() causes problems when
the frontend unilaterally disconnects, as explained in
c55f34b6aec2 ("xen-netback: only remove 'hotplug-status' when the vif is
actually destroyed").

Remove "hotplug-status" in the same place as it is done on the upstream
version to ensure unilateral re-connection of frontend continues to
work.

Fixes: 60e4e3198ce8 ("Revert "xen-netback: remove 'hotplug-status' once it has served its purpose"")
Signed-off-by: Pratyush Yadav <ptyadav@amazon.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/net/xen-netback/xenbus.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -202,10 +202,10 @@ static int netback_remove(struct xenbus_
 	set_backend_state(be, XenbusStateClosed);
 
 	unregister_hotplug_status_watch(be);
+	xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
 	if (be->vif) {
 		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
 		xen_unregister_watchers(be->vif);
-		xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
 		xenvif_free(be->vif);
 		be->vif = NULL;
 	}
@@ -435,7 +435,6 @@ static void backend_disconnect(struct ba
 		unsigned int queue_index;
 
 		xen_unregister_watchers(vif);
-		xenbus_rm(XBT_NIL, be->dev->nodename, "hotplug-status");
 #ifdef CONFIG_DEBUG_FS
 		xenvif_debugfs_delif(vif);
 #endif /* CONFIG_DEBUG_FS */


Patches currently in stable-queue which might be from ptyadav@amazon.de are

queue-5.4/xen-netback-move-removal-of-hotplug-status-to-the-right-place.patch
queue-5.4/tracing-ring-buffer-only-do-full-wait-when-cpu-ring_buffer_all_cpus.patch

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.