Maybe dmc->df->lock seems not needed to protect "if (ret)
& dev_warn" branch. Maybe this change speed up the code a bit.
Signed-off-by: Bernard Zhao <[email protected]>
----
Changes since v1:
*change release lock before the if statement.
*revert dmc->df->lock mutex lock to protect function
exynos5_dmc_perf_events_check
Link for V1:
*https://lore.kernel.org/patchwork/patch/1238888/
---
drivers/memory/samsung/exynos5422-dmc.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c
index 22a43d662833..25196d6268e2 100644
--- a/drivers/memory/samsung/exynos5422-dmc.c
+++ b/drivers/memory/samsung/exynos5422-dmc.c
@@ -1346,15 +1346,13 @@ static irqreturn_t dmc_irq_thread(int irq, void *priv)
struct exynos5_dmc *dmc = priv;
mutex_lock(&dmc->df->lock);
-
exynos5_dmc_perf_events_check(dmc);
-
res = update_devfreq(dmc->df);
+ mutex_unlock(&dmc->df->lock);
+
if (res)
dev_warn(dmc->dev, "devfreq failed with %d\n", res);
- mutex_unlock(&dmc->df->lock);
-
return IRQ_HANDLED;
}
--
2.26.2
> Maybe dmc->df->lock seems not needed to protect "if (ret)
> & dev_warn" branch. Maybe this change speed up the code a bit.
I suggest to improve also this commit message.
* Please reduce uncertainty.
* An imperative wording is probably preferred, isn't it?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=152036d1379ffd6985262743dcf6b0f9c75f83a4#n151
* Will it be more appropriate to refer to the transformation “Reduce the lock scope”?
* Would you like to add the tag “Fixes” to the change description?
Regards,
Markus
On 5/12/20 12:40 PM, Markus Elfring wrote:
>> Maybe dmc->df->lock seems not needed to protect "if (ret)
>> & dev_warn" branch. Maybe this change speed up the code a bit.
>
> I suggest to improve also this commit message.
>
> * Please reduce uncertainty.
>
> * An imperative wording is probably preferred, isn't it?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=152036d1379ffd6985262743dcf6b0f9c75f83a4#n151
>
> * Will it be more appropriate to refer to the transformation “Reduce the lock scope”?
>
> * Would you like to add the tag “Fixes” to the change description?
>
> Regards,
> Markus
>
Thank you Markus for providing to Bernard helpful suggestions.
@Bernard please read the link above and send v3
Something like: 'memory/samsung: reduce protected code area in IRQ
handler' for the subject header would be better.
Then in the message explain (without 'maybe') that it will speed-up a
bit this IRQ processing and there is no need to protect return value or
printing.
Regards,
Lukasz
> Something like: 'memory/samsung: reduce protected code area in IRQ
> handler' for the subject header would be better.
> Then in the message explain (without 'maybe') that it will speed-up a
> bit this IRQ processing and there is no need to protect return value
> or printing.
Do you tend to prefer smaller lock scopes for nicer run time characteristics?
Regards,
Markus
From: Lukasz Luba <[email protected]>
Date: 2020-05-12 19:56:49
To: Markus Elfring <[email protected]>,Bernard Zhao <[email protected]>,[email protected],[email protected],[email protected]
Cc: [email protected],[email protected],Krzysztof Kozlowski <[email protected]>,Kukjin Kim <[email protected]>
Subject: Re: [PATCH v2] memory/samsung: reduce unnecessary mutex lock area>
>
>On 5/12/20 12:40 PM, Markus Elfring wrote:
>>> Maybe dmc->df->lock seems not needed to protect "if (ret)
>>> & dev_warn" branch. Maybe this change speed up the code a bit.
>>
>> I suggest to improve also this commit message.
>>
>> * Please reduce uncertainty.
>>
>> * An imperative wording is probably preferred, isn't it?
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=152036d1379ffd6985262743dcf6b0f9c75f83a4#n151
>>
>> * Will it be more appropriate to refer to the transformation “Reduce the lock scope”?
>>
>> * Would you like to add the tag “Fixes” to the change description?
>>
>> Regards,
>> Markus
>>
>
>Thank you Markus for providing to Bernard helpful suggestions.
>
>@Bernard please read the link above and send v3
>
>Something like: 'memory/samsung: reduce protected code area in IRQ
>handler' for the subject header would be better.
>Then in the message explain (without 'maybe') that it will speed-up a
>bit this IRQ processing and there is no need to protect return value or
>printing.
>
>Regards,
>Lukasz
Sure, thank you both Markus & Lukasz for your helpful suggestions, I will read the content
of the link in detail and use that preferred format in my future subsequent submissions.
Regards,
Bernard
On 5/12/20 1:32 PM, Markus Elfring wrote:
>> Something like: 'memory/samsung: reduce protected code area in IRQ
>> handler' for the subject header would be better.
>> Then in the message explain (without 'maybe') that it will speed-up a
>> bit this IRQ processing and there is no need to protect return value
>> or printing.
>
> Do you tend to prefer smaller lock scopes for nicer run time characteristics?
Yes and I am probably not the only one. Of course it consumes time, to
make and test this kind of changes (so it must be in my current scope
of work).
Regards,
Lukasz
>
> Regards,
> Markus
>