From: Bjorn Andersson <[email protected]>
The recently introduced commit '635ce0db8956 ("soc: qcom: pmic_glink:
don't traverse clients list without a lock")' ensured that the clients
list is not modified while traversed.
But the callback is made from the GLINK IRQ handler and as such this
mutual exclusion can not be provided by a (sleepable) mutex.
Replace the mutex with a spinlock.
Fixes: 635ce0db8956 ("soc: qcom: pmic_glink: don't traverse clients list without a lock")
Signed-off-by: Bjorn Andersson <[email protected]>
---
drivers/soc/qcom/pmic_glink.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
index 9bdbd8944c4e..40fb09d69014 100644
--- a/drivers/soc/qcom/pmic_glink.c
+++ b/drivers/soc/qcom/pmic_glink.c
@@ -11,6 +11,7 @@
#include <linux/slab.h>
#include <linux/soc/qcom/pdr.h>
#include <linux/soc/qcom/pmic_glink.h>
+#include <linux/spinlock.h>
enum {
PMIC_GLINK_CLIENT_BATT = 0,
@@ -36,7 +37,7 @@ struct pmic_glink {
unsigned int pdr_state;
/* serializing clients list updates */
- struct mutex client_lock;
+ spinlock_t client_lock;
struct list_head clients;
};
@@ -58,10 +59,11 @@ static void _devm_pmic_glink_release_client(struct device *dev, void *res)
{
struct pmic_glink_client *client = (struct pmic_glink_client *)res;
struct pmic_glink *pg = client->pg;
+ unsigned long flags;
- mutex_lock(&pg->client_lock);
+ spin_lock_irqsave(&pg->client_lock, flags);
list_del(&client->node);
- mutex_unlock(&pg->client_lock);
+ spin_unlock_irqrestore(&pg->client_lock, flags);
}
struct pmic_glink_client *devm_pmic_glink_register_client(struct device *dev,
@@ -72,6 +74,7 @@ struct pmic_glink_client *devm_pmic_glink_register_client(struct device *dev,
{
struct pmic_glink_client *client;
struct pmic_glink *pg = dev_get_drvdata(dev->parent);
+ unsigned long flags;
client = devres_alloc(_devm_pmic_glink_release_client, sizeof(*client), GFP_KERNEL);
if (!client)
@@ -84,12 +87,12 @@ struct pmic_glink_client *devm_pmic_glink_register_client(struct device *dev,
client->priv = priv;
mutex_lock(&pg->state_lock);
- mutex_lock(&pg->client_lock);
+ spin_lock_irqsave(&pg->client_lock, flags);
list_add(&client->node, &pg->clients);
client->pdr_notify(client->priv, pg->client_state);
- mutex_unlock(&pg->client_lock);
+ spin_unlock_irqrestore(&pg->client_lock, flags);
mutex_unlock(&pg->state_lock);
devres_add(dev, client);
@@ -112,6 +115,7 @@ static int pmic_glink_rpmsg_callback(struct rpmsg_device *rpdev, void *data,
struct pmic_glink_client *client;
struct pmic_glink_hdr *hdr;
struct pmic_glink *pg = dev_get_drvdata(&rpdev->dev);
+ unsigned long flags;
if (len < sizeof(*hdr)) {
dev_warn(pg->dev, "ignoring truncated message\n");
@@ -120,12 +124,12 @@ static int pmic_glink_rpmsg_callback(struct rpmsg_device *rpdev, void *data,
hdr = data;
- mutex_lock(&pg->client_lock);
+ spin_lock_irqsave(&pg->client_lock, flags);
list_for_each_entry(client, &pg->clients, node) {
if (client->id == le32_to_cpu(hdr->owner))
client->cb(data, len, client->priv);
}
- mutex_unlock(&pg->client_lock);
+ spin_unlock_irqrestore(&pg->client_lock, flags);
return 0;
}
@@ -165,6 +169,7 @@ static void pmic_glink_state_notify_clients(struct pmic_glink *pg)
{
struct pmic_glink_client *client;
unsigned int new_state = pg->client_state;
+ unsigned long flags;
if (pg->client_state != SERVREG_SERVICE_STATE_UP) {
if (pg->pdr_state == SERVREG_SERVICE_STATE_UP && pg->ept)
@@ -175,10 +180,10 @@ static void pmic_glink_state_notify_clients(struct pmic_glink *pg)
}
if (new_state != pg->client_state) {
- mutex_lock(&pg->client_lock);
+ spin_lock_irqsave(&pg->client_lock, flags);
list_for_each_entry(client, &pg->clients, node)
client->pdr_notify(client->priv, new_state);
- mutex_unlock(&pg->client_lock);
+ spin_unlock_irqrestore(&pg->client_lock, flags);
pg->client_state = new_state;
}
}
@@ -265,7 +270,7 @@ static int pmic_glink_probe(struct platform_device *pdev)
pg->dev = &pdev->dev;
INIT_LIST_HEAD(&pg->clients);
- mutex_init(&pg->client_lock);
+ spin_lock_init(&pg->client_lock);
mutex_init(&pg->state_lock);
match_data = (unsigned long *)of_device_get_match_data(&pdev->dev);
---
base-commit: bb7a2467e6beef44a80a17d45ebf2931e7631083
change-id: 20240430-pmic-glink-sleep-while-atomic-d43c42c5add0
Best regards,
--
Bjorn Andersson <[email protected]>
On Wed, 1 May 2024 at 06:33, Bjorn Andersson <[email protected]> wrote:
>
> From: Bjorn Andersson <[email protected]>
>
> The recently introduced commit '635ce0db8956 ("soc: qcom: pmic_glink:
> don't traverse clients list without a lock")' ensured that the clients
> list is not modified while traversed.
>
> But the callback is made from the GLINK IRQ handler and as such this
> mutual exclusion can not be provided by a (sleepable) mutex.
>
> Replace the mutex with a spinlock.
>
> Fixes: 635ce0db8956 ("soc: qcom: pmic_glink: don't traverse clients list without a lock")
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
> drivers/soc/qcom/pmic_glink.c | 25 +++++++++++++++----------
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Tue, Apr 30, 2024 at 08:38:57PM -0700, Bjorn Andersson wrote:
> From: Bjorn Andersson <[email protected]>
>
> The recently introduced commit '635ce0db8956 ("soc: qcom: pmic_glink:
> don't traverse clients list without a lock")' ensured that the clients
> list is not modified while traversed.
>
> But the callback is made from the GLINK IRQ handler and as such this
> mutual exclusion can not be provided by a (sleepable) mutex.
>
> Replace the mutex with a spinlock.
>
> Fixes: 635ce0db8956 ("soc: qcom: pmic_glink: don't traverse clients list without a lock")
> Signed-off-by: Bjorn Andersson <[email protected]>
Reviewed-by: Andrew Halaney <[email protected]>
On Tue, 30 Apr 2024 20:38:57 -0700, Bjorn Andersson wrote:
> The recently introduced commit '635ce0db8956 ("soc: qcom: pmic_glink:
> don't traverse clients list without a lock")' ensured that the clients
> list is not modified while traversed.
>
> But the callback is made from the GLINK IRQ handler and as such this
> mutual exclusion can not be provided by a (sleepable) mutex.
>
> [...]
Applied, thanks!
[1/1] soc: qcom: pmic_glink: Make client-lock non-sleeping
commit: 9329933699b32d467a99befa20415c4b2172389a
Best regards,
--
Bjorn Andersson <[email protected]>