patch "driver core: Update device link status correctly for SYNC_STATE_ONLY" added to driver-core-next

DISCUSSION Saravana Kannan 2 participants 2 messages May 30, 2020 1 message not in the archive Stable branch

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Saravana Kannan

author

2020-05-30 03:12 UTC #1 lore raw

Quoted 10 lines · On Thu, May 28, 2020 at 12:19 AM <[email protected]> wrote:

This is a note to let you know that I've just added the patch titled
driver core: Update device link status correctly for SYNC_STATE_ONLY
to my driver-core git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
in the driver-core-next branch.

Quoted 7 lines

The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)
The patch will also be merged in the next major kernel release during the merge window.
If you have any questions about this process, please let me know.

Not sure if this is already/automatically queued, but this needs to go to stable@ too. Cc-ing the list to make sure it's picked up.

-Saravana

Quoted 118 lines

From 8c3e315d4296421cd26b3300ee0ac117f0877f20 Mon Sep 17 00:00:00 2001
From: Saravana Kannan <[email protected]>
Date: Tue, 26 May 2020 15:09:27 -0700
Subject: driver core: Update device link status correctly for SYNC_STATE_ONLY
links
When SYNC_STATE_ONLY support was added in commit 05ef983e0d65 ("driver
core: Add device link support for SYNC_STATE_ONLY flag"), SYNC_STATE_ONLY links were treated similar to STATELESS links in terms of not blocking consumer probe if the supplier hasn't probed yet.
That caused a SYNC_STATE_ONLY device link's status to not get updated. Since SYNC_STATE_ONLY device link is no longer useful once the consumer probes, commit 21c27f06587d ("driver core: Fix SYNC_STATE_ONLY device link implementation") addresses the status update issue by deleting the SYNC_STATE_ONLY device link instead of complicating the status update code.
However, there are still some cases where we need to update the status of a SYNC_STATE_ONLY device link. This is because a SYNC_STATE_ONLY device link can later get converted into a normal MANAGED device link when a normal MANAGED device link is created between a supplier and consumer that already have a SYNC_STATE_ONLY device link between them.
If a SYNC_STATE_ONLY device link's status isn't maintained correctly till it's converted to a normal MANAGED device link, then the normal MANAGED device link will end up with a wrong link status. This can cause a warning stack trace[1] when the consumer device probes successfully.
This commit fixes the SYNC_STATE_ONLY device link status update issue where it wouldn't transition correctly from DL_STATE_DORMANT or DL_STATE_AVAILABLE to DL_STATE_CONSUMER_PROBE. It also resets the status back to DL_STATE_DORMANT or DL_STATE_AVAILABLE if the consumer probe fails.
[1] - https://lore.kernel.org/lkml/20200522204120.3b3c9ed6@apollo/
Fixes 05ef983e0d65 ("driver core: Add device link support for SYNC_STATE_ONLY flag")
Fixes 21c27f06587d ("driver core: Fix SYNC_STATE_ONLY device link implementation")
Reported-by Michael Walle <[email protected]>
Tested-by Michael Walle <[email protected]>
Signed-off-by Saravana Kannan <[email protected]>
Reviewed-by Rafael J. Wysocki <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/base/core.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)
drivers/base/core.c +26 -9
index 791b7530599f..9a76dd44cb37 100644
@@ -646,9 +646,17 @@ static void device_links_missing_supplier(struct device *dev)
 {
        struct device_link *link;
 
-       list_for_each_entry(link, &dev->links.suppliers, c_node)
-               if (link->status == DL_STATE_CONSUMER_PROBE)
+       list_for_each_entry(link, &dev->links.suppliers, c_node) {
+               if (link->status != DL_STATE_CONSUMER_PROBE)
+                       continue;
+
+               if (link->supplier->links.status == DL_DEV_DRIVER_BOUND) {
                        WRITE_ONCE(link->status, DL_STATE_AVAILABLE);
+               } else {
+                       WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY));
+                       WRITE_ONCE(link->status, DL_STATE_DORMANT);
+               }
+       }
 }
 
 /**
@@ -687,11 +695,11 @@ int device_links_check_suppliers(struct device *dev)
        device_links_write_lock();
 
        list_for_each_entry(link, &dev->links.suppliers, c_node) {
-               if (!(link->flags & DL_FLAG_MANAGED) ||
-                   link->flags & DL_FLAG_SYNC_STATE_ONLY)
+               if (!(link->flags & DL_FLAG_MANAGED))
                        continue;
 
-               if (link->status != DL_STATE_AVAILABLE) {
+               if (link->status != DL_STATE_AVAILABLE &&
+                   !(link->flags & DL_FLAG_SYNC_STATE_ONLY)) {
                        device_links_missing_supplier(dev);
                        ret = -EPROBE_DEFER;
                        break;
@@ -952,11 +960,21 @@ static void __device_links_no_driver(struct device *dev)
                if (!(link->flags & DL_FLAG_MANAGED))
                        continue;
 
-               if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER)
+               if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) {
                        device_link_drop_managed(link);
-               else if (link->status == DL_STATE_CONSUMER_PROBE ||
-                        link->status == DL_STATE_ACTIVE)
+                       continue;
+               }
+
+               if (link->status != DL_STATE_CONSUMER_PROBE &&
+                   link->status != DL_STATE_ACTIVE)
+                       continue;
+
+               if (link->supplier->links.status == DL_DEV_DRIVER_BOUND) {
                        WRITE_ONCE(link->status, DL_STATE_AVAILABLE);
+               } else {
+                       WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY));
+                       WRITE_ONCE(link->status, DL_STATE_DORMANT);
+               }
        }
 
        dev->links.status = DL_DEV_NO_DRIVER;
--
2.26.2

Greg Kroah-Hartman

maintainer

2020-05-30 06:02 UTC #2 lore raw

Quoted 14 lines · Saravana Kannan

Quoted 13 lines · On Thu, May 28, 2020 at 12:19 AM <[email protected]> wrote:
This is a note to let you know that I've just added the patch titled
driver core: Update device link status correctly for SYNC_STATE_ONLY
to my driver-core git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
in the driver-core-next branch.
The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)

Quoted 7 lines

Quoted 4 lines
The patch will also be merged in the next major kernel release during the merge window.
If you have any questions about this process, please let me know.
Not sure if this is already/automatically queued, but this needs to go to stable@ too. Cc-ing the list to make sure it's picked up.

I will make sure it goes there too, thanks.

greg k-h