All of lore.kernel.org
 help / color / mirror / Atom feed
From: Saravana Kannan <saravanak@google.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Saravana Kannan <saravanak@google.com>
Cc: linux-imx@nxp.com, linux-arm-kernel@lists.infradead.org,
	Dong Aisheng <aisheng.dong@nxp.com>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <kernel@pengutronix.de>,
	linux@roeck-us.net, m.szyprowski@samsung.com,
	naresh.kamboju@linaro.org, Peng Fan <peng.fan@nxp.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Dong Aisheng <dongas86@gmail.com>,
	kernel-team@android.com, linux-kernel@vger.kernel.org
Subject: [PATCH v2] driver core: Fix device_pm_lock() locking for device links
Date: Tue,  1 Sep 2020 11:44:44 -0700	[thread overview]
Message-ID: <20200901184445.1736658-1-saravanak@google.com> (raw)

This commit fixes two issues:

1. The lockdep warning reported by Dong Aisheng <dongas86@gmail.com> [1].

It is a warning about a cycle (dpm_list_mtx --> kn->active#3 --> fw_lock)
that was introduced when device-link devices were added to expose device
link information in sysfs.

The patch that "introduced" this cycle can't be reverted because it's fixes
a real SRCU issue and also ensures that the device-link device is deleted
as soon as the device-link is deleted. This is important to avoid sysfs
name collisions if the device-link is create again immediately (this can
happen a lot with deferred probing).

2. Inconsistency in grabbing device_pm_lock() during device link deletion

Some device link deletion code paths grab device_pm_lock(), while others
don't.  The device_pm_lock() is grabbed during device_link_add() because it
checks if the supplier is in the dpm_list and also reorders the dpm_list.
However, when a device link is deleted, it does not do either of those and
therefore device_pm_lock() is not necessary. Dropping the device_pm_lock()
in all the device link deletion paths removes the inconsistency in locking.

Thanks to Stephen Boyd for helping me understand the lockdep splat.

Fixes: 843e600b8a2b ("driver core: Fix sleeping in invalid context during device link deletion")
[1] - https://lore.kernel.org/lkml/CAA+hA=S4eAreb7vo69LAXSk2t5=DEKNxHaiY1wSpk4xTp9urLg@mail.gmail.com/
Reported-by: Dong Aisheng <dongas86@gmail.com>
Signed-off-by: Saravana Kannan <saravanak@google.com>
---

Cc'ing everyone from the original thread [1]

-Saravana

 drivers/base/core.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index f6f620aa9408..07e5ceb40bb1 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -807,9 +807,7 @@ static void device_link_put_kref(struct device_link *link)
 void device_link_del(struct device_link *link)
 {
 	device_links_write_lock();
-	device_pm_lock();
 	device_link_put_kref(link);
-	device_pm_unlock();
 	device_links_write_unlock();
 }
 EXPORT_SYMBOL_GPL(device_link_del);
@@ -830,7 +828,6 @@ void device_link_remove(void *consumer, struct device *supplier)
 		return;
 
 	device_links_write_lock();
-	device_pm_lock();
 
 	list_for_each_entry(link, &supplier->links.consumers, s_node) {
 		if (link->consumer == consumer) {
@@ -839,7 +836,6 @@ void device_link_remove(void *consumer, struct device *supplier)
 		}
 	}
 
-	device_pm_unlock();
 	device_links_write_unlock();
 }
 EXPORT_SYMBOL_GPL(device_link_remove);
-- 
2.28.0.402.g5ffc5be6b7-goog


WARNING: multiple messages have this Message-ID (diff)
From: Saravana Kannan <saravanak@google.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	 Saravana Kannan <saravanak@google.com>
Cc: Dong Aisheng <aisheng.dong@nxp.com>, Peng Fan <peng.fan@nxp.com>,
	Dong Aisheng <dongas86@gmail.com>,
	Stephen Boyd <sboyd@kernel.org>,
	kernel-team@android.com, naresh.kamboju@linaro.org,
	linux-kernel@vger.kernel.org, linux-imx@nxp.com,
	Sascha Hauer <kernel@pengutronix.de>,
	m.szyprowski@samsung.com, Shawn Guo <shawnguo@kernel.org>,
	linux@roeck-us.net, linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] driver core: Fix device_pm_lock() locking for device links
Date: Tue,  1 Sep 2020 11:44:44 -0700	[thread overview]
Message-ID: <20200901184445.1736658-1-saravanak@google.com> (raw)

This commit fixes two issues:

1. The lockdep warning reported by Dong Aisheng <dongas86@gmail.com> [1].

It is a warning about a cycle (dpm_list_mtx --> kn->active#3 --> fw_lock)
that was introduced when device-link devices were added to expose device
link information in sysfs.

The patch that "introduced" this cycle can't be reverted because it's fixes
a real SRCU issue and also ensures that the device-link device is deleted
as soon as the device-link is deleted. This is important to avoid sysfs
name collisions if the device-link is create again immediately (this can
happen a lot with deferred probing).

2. Inconsistency in grabbing device_pm_lock() during device link deletion

Some device link deletion code paths grab device_pm_lock(), while others
don't.  The device_pm_lock() is grabbed during device_link_add() because it
checks if the supplier is in the dpm_list and also reorders the dpm_list.
However, when a device link is deleted, it does not do either of those and
therefore device_pm_lock() is not necessary. Dropping the device_pm_lock()
in all the device link deletion paths removes the inconsistency in locking.

Thanks to Stephen Boyd for helping me understand the lockdep splat.

Fixes: 843e600b8a2b ("driver core: Fix sleeping in invalid context during device link deletion")
[1] - https://lore.kernel.org/lkml/CAA+hA=S4eAreb7vo69LAXSk2t5=DEKNxHaiY1wSpk4xTp9urLg@mail.gmail.com/
Reported-by: Dong Aisheng <dongas86@gmail.com>
Signed-off-by: Saravana Kannan <saravanak@google.com>
---

Cc'ing everyone from the original thread [1]

-Saravana

 drivers/base/core.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index f6f620aa9408..07e5ceb40bb1 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -807,9 +807,7 @@ static void device_link_put_kref(struct device_link *link)
 void device_link_del(struct device_link *link)
 {
 	device_links_write_lock();
-	device_pm_lock();
 	device_link_put_kref(link);
-	device_pm_unlock();
 	device_links_write_unlock();
 }
 EXPORT_SYMBOL_GPL(device_link_del);
@@ -830,7 +828,6 @@ void device_link_remove(void *consumer, struct device *supplier)
 		return;
 
 	device_links_write_lock();
-	device_pm_lock();
 
 	list_for_each_entry(link, &supplier->links.consumers, s_node) {
 		if (link->consumer == consumer) {
@@ -839,7 +836,6 @@ void device_link_remove(void *consumer, struct device *supplier)
 		}
 	}
 
-	device_pm_unlock();
 	device_links_write_unlock();
 }
 EXPORT_SYMBOL_GPL(device_link_remove);
-- 
2.28.0.402.g5ffc5be6b7-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

             reply	other threads:[~2020-09-01 18:44 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-01 18:44 Saravana Kannan [this message]
2020-09-01 18:44 ` [PATCH v2] driver core: Fix device_pm_lock() locking for device links Saravana Kannan
2020-09-02  1:11 ` Peng Fan
2020-09-02  1:11   ` Peng Fan
2020-09-02  8:10 ` Stephen Boyd

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200901184445.1736658-1-saravanak@google.com \
    --to=saravanak@google.com \
    --cc=aisheng.dong@nxp.com \
    --cc=dongas86@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel-team@android.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=m.szyprowski@samsung.com \
    --cc=naresh.kamboju@linaro.org \
    --cc=peng.fan@nxp.com \
    --cc=rafael@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=shawnguo@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.