* [PATCH] memory/samsung: reduce unnecessary mutex lock area
@ 2020-05-08 13:13 ` Bernard Zhao
0 siblings, 0 replies; 12+ messages in thread
From: Bernard Zhao @ 2020-05-08 13:13 UTC (permalink / raw)
To: Lukasz Luba, Kukjin Kim, Krzysztof Kozlowski, linux-pm,
linux-samsung-soc, linux-arm-kernel, linux-kernel
Cc: opensource.kernel, Bernard Zhao
Maybe dmc->df->lock is unnecessary to protect function
exynos5_dmc_perf_events_check(dmc). If we have to protect,
dmc->lock is more better and more effective.
Also, it seems not needed to protect "if (ret) & dev_warn"
branch.
Signed-off-by: Bernard Zhao <bernard@vivo.com>
---
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..88e8ac8b5327 100644
--- a/drivers/memory/samsung/exynos5422-dmc.c
+++ b/drivers/memory/samsung/exynos5422-dmc.c
@@ -1345,16 +1345,14 @@ static irqreturn_t dmc_irq_thread(int irq, void *priv)
int res;
struct exynos5_dmc *dmc = priv;
- mutex_lock(&dmc->df->lock);
-
exynos5_dmc_perf_events_check(dmc);
+ mutex_lock(&dmc->df->lock);
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] memory/samsung: reduce unnecessary mutex lock area
@ 2020-05-08 13:13 ` Bernard Zhao
0 siblings, 0 replies; 12+ messages in thread
From: Bernard Zhao @ 2020-05-08 13:13 UTC (permalink / raw)
To: Lukasz Luba, Kukjin Kim, Krzysztof Kozlowski, linux-pm,
linux-samsung-soc, linux-arm-kernel, linux-kernel
Cc: opensource.kernel, Bernard Zhao
Maybe dmc->df->lock is unnecessary to protect function
exynos5_dmc_perf_events_check(dmc). If we have to protect,
dmc->lock is more better and more effective.
Also, it seems not needed to protect "if (ret) & dev_warn"
branch.
Signed-off-by: Bernard Zhao <bernard@vivo.com>
---
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..88e8ac8b5327 100644
--- a/drivers/memory/samsung/exynos5422-dmc.c
+++ b/drivers/memory/samsung/exynos5422-dmc.c
@@ -1345,16 +1345,14 @@ static irqreturn_t dmc_irq_thread(int irq, void *priv)
int res;
struct exynos5_dmc *dmc = priv;
- mutex_lock(&dmc->df->lock);
-
exynos5_dmc_perf_events_check(dmc);
+ mutex_lock(&dmc->df->lock);
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] memory/samsung: reduce unnecessary mutex lock area
2020-05-08 13:13 ` Bernard Zhao
@ 2020-05-12 6:50 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2020-05-12 6:50 UTC (permalink / raw)
To: Bernard Zhao, Lukasz Luba
Cc: Kukjin Kim, linux-pm, linux-samsung-soc, linux-arm-kernel,
linux-kernel, opensource.kernel
On Fri, May 08, 2020 at 06:13:38AM -0700, Bernard Zhao wrote:
> Maybe dmc->df->lock is unnecessary to protect function
> exynos5_dmc_perf_events_check(dmc). If we have to protect,
> dmc->lock is more better and more effective.
> Also, it seems not needed to protect "if (ret) & dev_warn"
> branch.
>
> Signed-off-by: Bernard Zhao <bernard@vivo.com>
> ---
> drivers/memory/samsung/exynos5422-dmc.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
I checked the concurrent accesses and it looks correct.
Lukasz, any review from your side?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] memory/samsung: reduce unnecessary mutex lock area
@ 2020-05-12 6:50 ` Krzysztof Kozlowski
0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2020-05-12 6:50 UTC (permalink / raw)
To: Bernard Zhao, Lukasz Luba
Cc: opensource.kernel, linux-samsung-soc, linux-pm, linux-kernel,
Kukjin Kim, linux-arm-kernel
On Fri, May 08, 2020 at 06:13:38AM -0700, Bernard Zhao wrote:
> Maybe dmc->df->lock is unnecessary to protect function
> exynos5_dmc_perf_events_check(dmc). If we have to protect,
> dmc->lock is more better and more effective.
> Also, it seems not needed to protect "if (ret) & dev_warn"
> branch.
>
> Signed-off-by: Bernard Zhao <bernard@vivo.com>
> ---
> drivers/memory/samsung/exynos5422-dmc.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
I checked the concurrent accesses and it looks correct.
Lukasz, any review from your side?
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] memory/samsung: reduce unnecessary mutex lock area
2020-05-12 6:50 ` Krzysztof Kozlowski
@ 2020-05-12 8:47 ` Lukasz Luba
-1 siblings, 0 replies; 12+ messages in thread
From: Lukasz Luba @ 2020-05-12 8:47 UTC (permalink / raw)
To: Krzysztof Kozlowski, Bernard Zhao
Cc: Kukjin Kim, linux-pm, linux-samsung-soc, linux-arm-kernel,
linux-kernel, opensource.kernel
Hi Krzysztof,
I am sorry, I was a bit busy recently.
On 5/12/20 7:50 AM, Krzysztof Kozlowski wrote:
> On Fri, May 08, 2020 at 06:13:38AM -0700, Bernard Zhao wrote:
>> Maybe dmc->df->lock is unnecessary to protect function
>> exynos5_dmc_perf_events_check(dmc). If we have to protect,
>> dmc->lock is more better and more effective.
>> Also, it seems not needed to protect "if (ret) & dev_warn"
>> branch.
>>
>> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>> ---
>> drivers/memory/samsung/exynos5422-dmc.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> I checked the concurrent accesses and it looks correct.
>
> Lukasz, any review from your side?
The lock from devfreq lock protects from a scenario when
concurrent access from devfreq framework uses internal dmc fields 'load'
and 'total' (which are set to 'busy_time', 'total_time').
The .get_dev_status can be called at any time (even due to thermal
devfreq cooling action) and reads above fields.
That's why the calculation of the new values inside dmc is protected.
This patch should not be taken IMO. Maybe we can release lock before the
if statement, just to speed-up.
Regards,
Lukasz
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] memory/samsung: reduce unnecessary mutex lock area
@ 2020-05-12 8:47 ` Lukasz Luba
0 siblings, 0 replies; 12+ messages in thread
From: Lukasz Luba @ 2020-05-12 8:47 UTC (permalink / raw)
To: Krzysztof Kozlowski, Bernard Zhao
Cc: opensource.kernel, linux-samsung-soc, linux-pm, linux-kernel,
Kukjin Kim, linux-arm-kernel
Hi Krzysztof,
I am sorry, I was a bit busy recently.
On 5/12/20 7:50 AM, Krzysztof Kozlowski wrote:
> On Fri, May 08, 2020 at 06:13:38AM -0700, Bernard Zhao wrote:
>> Maybe dmc->df->lock is unnecessary to protect function
>> exynos5_dmc_perf_events_check(dmc). If we have to protect,
>> dmc->lock is more better and more effective.
>> Also, it seems not needed to protect "if (ret) & dev_warn"
>> branch.
>>
>> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>> ---
>> drivers/memory/samsung/exynos5422-dmc.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> I checked the concurrent accesses and it looks correct.
>
> Lukasz, any review from your side?
The lock from devfreq lock protects from a scenario when
concurrent access from devfreq framework uses internal dmc fields 'load'
and 'total' (which are set to 'busy_time', 'total_time').
The .get_dev_status can be called at any time (even due to thermal
devfreq cooling action) and reads above fields.
That's why the calculation of the new values inside dmc is protected.
This patch should not be taken IMO. Maybe we can release lock before the
if statement, just to speed-up.
Regards,
Lukasz
>
> Best regards,
> Krzysztof
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] memory/samsung: reduce unnecessary mutex lock area
2020-05-12 8:47 ` Lukasz Luba
@ 2020-05-12 9:05 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2020-05-12 9:05 UTC (permalink / raw)
To: Lukasz Luba
Cc: Bernard Zhao, Kukjin Kim, linux-pm, linux-samsung-soc,
linux-arm-kernel, linux-kernel, opensource.kernel
On Tue, 12 May 2020 at 10:47, Lukasz Luba <lukasz.luba@arm.com> wrote:
>
> Hi Krzysztof,
>
> I am sorry, I was a bit busy recently.
>
> On 5/12/20 7:50 AM, Krzysztof Kozlowski wrote:
> > On Fri, May 08, 2020 at 06:13:38AM -0700, Bernard Zhao wrote:
> >> Maybe dmc->df->lock is unnecessary to protect function
> >> exynos5_dmc_perf_events_check(dmc). If we have to protect,
> >> dmc->lock is more better and more effective.
> >> Also, it seems not needed to protect "if (ret) & dev_warn"
> >> branch.
> >>
> >> Signed-off-by: Bernard Zhao <bernard@vivo.com>
> >> ---
> >> drivers/memory/samsung/exynos5422-dmc.c | 6 ++----
> >> 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > I checked the concurrent accesses and it looks correct.
> >
> > Lukasz, any review from your side?
>
> The lock from devfreq lock protects from a scenario when
> concurrent access from devfreq framework uses internal dmc fields 'load'
> and 'total' (which are set to 'busy_time', 'total_time').
> The .get_dev_status can be called at any time (even due to thermal
> devfreq cooling action) and reads above fields.
> That's why the calculation of the new values inside dmc is protected.
I looked at this path (get_dev_status) and currently in devfreq it
will be only called from update_devfreq() -> get_target_freq()... at
least when looking at devfreq core and governors. On the other hand
you are right that this is public function and this call scenario
might change. It could be called directly from other paths sooner or
later.
> This patch should not be taken IMO. Maybe we can release lock before the
> if statement, just to speed-up.
Yep.
Bernard, you can send just this part of the patch.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] memory/samsung: reduce unnecessary mutex lock area
@ 2020-05-12 9:05 ` Krzysztof Kozlowski
0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2020-05-12 9:05 UTC (permalink / raw)
To: Lukasz Luba
Cc: opensource.kernel, linux-samsung-soc, linux-pm, Bernard Zhao,
linux-kernel, Kukjin Kim, linux-arm-kernel
On Tue, 12 May 2020 at 10:47, Lukasz Luba <lukasz.luba@arm.com> wrote:
>
> Hi Krzysztof,
>
> I am sorry, I was a bit busy recently.
>
> On 5/12/20 7:50 AM, Krzysztof Kozlowski wrote:
> > On Fri, May 08, 2020 at 06:13:38AM -0700, Bernard Zhao wrote:
> >> Maybe dmc->df->lock is unnecessary to protect function
> >> exynos5_dmc_perf_events_check(dmc). If we have to protect,
> >> dmc->lock is more better and more effective.
> >> Also, it seems not needed to protect "if (ret) & dev_warn"
> >> branch.
> >>
> >> Signed-off-by: Bernard Zhao <bernard@vivo.com>
> >> ---
> >> drivers/memory/samsung/exynos5422-dmc.c | 6 ++----
> >> 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > I checked the concurrent accesses and it looks correct.
> >
> > Lukasz, any review from your side?
>
> The lock from devfreq lock protects from a scenario when
> concurrent access from devfreq framework uses internal dmc fields 'load'
> and 'total' (which are set to 'busy_time', 'total_time').
> The .get_dev_status can be called at any time (even due to thermal
> devfreq cooling action) and reads above fields.
> That's why the calculation of the new values inside dmc is protected.
I looked at this path (get_dev_status) and currently in devfreq it
will be only called from update_devfreq() -> get_target_freq()... at
least when looking at devfreq core and governors. On the other hand
you are right that this is public function and this call scenario
might change. It could be called directly from other paths sooner or
later.
> This patch should not be taken IMO. Maybe we can release lock before the
> if statement, just to speed-up.
Yep.
Bernard, you can send just this part of the patch.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re:Re: [PATCH] memory/samsung: reduce unnecessary mutex lock area
2020-05-12 9:05 ` Krzysztof Kozlowski
@ 2020-05-12 9:19 ` Bernard
-1 siblings, 0 replies; 12+ messages in thread
From: Bernard @ 2020-05-12 9:19 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lukasz Luba, Kukjin Kim, linux-pm, linux-samsung-soc,
linux-arm-kernel, linux-kernel, opensource.kernel
From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2020-05-12 17:05:28
To: Lukasz Luba <lukasz.luba@arm.com>
Cc: Bernard Zhao <bernard@vivo.com>,Kukjin Kim <kgene@kernel.org>,linux-pm@vger.kernel.org,"linux-samsung-soc@vger.kernel.org" <linux-samsung-soc@vger.kernel.org>,linux-arm-kernel@lists.infradead.org,"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,opensource.kernel@vivo.com
Subject: Re: [PATCH] memory/samsung: reduce unnecessary mutex lock area>On Tue, 12 May 2020 at 10:47, Lukasz Luba <lukasz.luba@arm.com> wrote:
>>
>> Hi Krzysztof,
>>
>> I am sorry, I was a bit busy recently.
>>
>> On 5/12/20 7:50 AM, Krzysztof Kozlowski wrote:
>> > On Fri, May 08, 2020 at 06:13:38AM -0700, Bernard Zhao wrote:
>> >> Maybe dmc->df->lock is unnecessary to protect function
>> >> exynos5_dmc_perf_events_check(dmc). If we have to protect,
>> >> dmc->lock is more better and more effective.
>> >> Also, it seems not needed to protect "if (ret) & dev_warn"
>> >> branch.
>> >>
>> >> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>> >> ---
>> >> drivers/memory/samsung/exynos5422-dmc.c | 6 ++----
>> >> 1 file changed, 2 insertions(+), 4 deletions(-)
>> >
>> > I checked the concurrent accesses and it looks correct.
>> >
>> > Lukasz, any review from your side?
>>
>> The lock from devfreq lock protects from a scenario when
>> concurrent access from devfreq framework uses internal dmc fields 'load'
>> and 'total' (which are set to 'busy_time', 'total_time').
>> The .get_dev_status can be called at any time (even due to thermal
>> devfreq cooling action) and reads above fields.
>> That's why the calculation of the new values inside dmc is protected.
>
>I looked at this path (get_dev_status) and currently in devfreq it
>will be only called from update_devfreq() -> get_target_freq()... at
>least when looking at devfreq core and governors. On the other hand
>you are right that this is public function and this call scenario
>might change. It could be called directly from other paths sooner or
>later.
>
>> This patch should not be taken IMO. Maybe we can release lock before the
>> if statement, just to speed-up.
>
>Yep.
>
>Bernard, you can send just this part of the patch.
>
Sure, I will resubmit a patch in v2.
Best regards,
Bernard
>Best regards,
>Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re:Re: [PATCH] memory/samsung: reduce unnecessary mutex lock area
@ 2020-05-12 9:19 ` Bernard
0 siblings, 0 replies; 12+ messages in thread
From: Bernard @ 2020-05-12 9:19 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: opensource.kernel, linux-samsung-soc, linux-pm, linux-kernel,
Kukjin Kim, Lukasz Luba, linux-arm-kernel
From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2020-05-12 17:05:28
To: Lukasz Luba <lukasz.luba@arm.com>
Cc: Bernard Zhao <bernard@vivo.com>,Kukjin Kim <kgene@kernel.org>,linux-pm@vger.kernel.org,"linux-samsung-soc@vger.kernel.org" <linux-samsung-soc@vger.kernel.org>,linux-arm-kernel@lists.infradead.org,"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,opensource.kernel@vivo.com
Subject: Re: [PATCH] memory/samsung: reduce unnecessary mutex lock area>On Tue, 12 May 2020 at 10:47, Lukasz Luba <lukasz.luba@arm.com> wrote:
>>
>> Hi Krzysztof,
>>
>> I am sorry, I was a bit busy recently.
>>
>> On 5/12/20 7:50 AM, Krzysztof Kozlowski wrote:
>> > On Fri, May 08, 2020 at 06:13:38AM -0700, Bernard Zhao wrote:
>> >> Maybe dmc->df->lock is unnecessary to protect function
>> >> exynos5_dmc_perf_events_check(dmc). If we have to protect,
>> >> dmc->lock is more better and more effective.
>> >> Also, it seems not needed to protect "if (ret) & dev_warn"
>> >> branch.
>> >>
>> >> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>> >> ---
>> >> drivers/memory/samsung/exynos5422-dmc.c | 6 ++----
>> >> 1 file changed, 2 insertions(+), 4 deletions(-)
>> >
>> > I checked the concurrent accesses and it looks correct.
>> >
>> > Lukasz, any review from your side?
>>
>> The lock from devfreq lock protects from a scenario when
>> concurrent access from devfreq framework uses internal dmc fields 'load'
>> and 'total' (which are set to 'busy_time', 'total_time').
>> The .get_dev_status can be called at any time (even due to thermal
>> devfreq cooling action) and reads above fields.
>> That's why the calculation of the new values inside dmc is protected.
>
>I looked at this path (get_dev_status) and currently in devfreq it
>will be only called from update_devfreq() -> get_target_freq()... at
>least when looking at devfreq core and governors. On the other hand
>you are right that this is public function and this call scenario
>might change. It could be called directly from other paths sooner or
>later.
>
>> This patch should not be taken IMO. Maybe we can release lock before the
>> if statement, just to speed-up.
>
>Yep.
>
>Bernard, you can send just this part of the patch.
>
Sure, I will resubmit a patch in v2.
Best regards,
Bernard
>Best regards,
>Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] memory/samsung: reduce unnecessary mutex lock area
2020-05-12 9:05 ` Krzysztof Kozlowski
@ 2020-05-12 9:23 ` Lukasz Luba
-1 siblings, 0 replies; 12+ messages in thread
From: Lukasz Luba @ 2020-05-12 9:23 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Bernard Zhao, Kukjin Kim, linux-pm, linux-samsung-soc,
linux-arm-kernel, linux-kernel, opensource.kernel
On 5/12/20 10:05 AM, Krzysztof Kozlowski wrote:
> On Tue, 12 May 2020 at 10:47, Lukasz Luba <lukasz.luba@arm.com> wrote:
>>
>> Hi Krzysztof,
>>
>> I am sorry, I was a bit busy recently.
>>
>> On 5/12/20 7:50 AM, Krzysztof Kozlowski wrote:
>>> On Fri, May 08, 2020 at 06:13:38AM -0700, Bernard Zhao wrote:
>>>> Maybe dmc->df->lock is unnecessary to protect function
>>>> exynos5_dmc_perf_events_check(dmc). If we have to protect,
>>>> dmc->lock is more better and more effective.
>>>> Also, it seems not needed to protect "if (ret) & dev_warn"
>>>> branch.
>>>>
>>>> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>>>> ---
>>>> drivers/memory/samsung/exynos5422-dmc.c | 6 ++----
>>>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> I checked the concurrent accesses and it looks correct.
>>>
>>> Lukasz, any review from your side?
>>
>> The lock from devfreq lock protects from a scenario when
>> concurrent access from devfreq framework uses internal dmc fields 'load'
>> and 'total' (which are set to 'busy_time', 'total_time').
>> The .get_dev_status can be called at any time (even due to thermal
>> devfreq cooling action) and reads above fields.
>> That's why the calculation of the new values inside dmc is protected.
>
> I looked at this path (get_dev_status) and currently in devfreq it
> will be only called from update_devfreq() -> get_target_freq()... at
> least when looking at devfreq core and governors. On the other hand
> you are right that this is public function and this call scenario
> might change. It could be called directly from other paths sooner or
> later.
Indeed, I am currently changing this while I am adding devfreq devices
to the Energy Model.
>
>> This patch should not be taken IMO. Maybe we can release lock before the
>> if statement, just to speed-up.
>
> Yep.
>
> Bernard, you can send just this part of the patch.
Thank you Bernard and please submit the patch v2.
>
> Best regards,
> Krzysztof
>
Thank you Krzysztof for your time spent on this.
Regards,
Lukasz
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] memory/samsung: reduce unnecessary mutex lock area
@ 2020-05-12 9:23 ` Lukasz Luba
0 siblings, 0 replies; 12+ messages in thread
From: Lukasz Luba @ 2020-05-12 9:23 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: opensource.kernel, linux-samsung-soc, linux-pm, Bernard Zhao,
linux-kernel, Kukjin Kim, linux-arm-kernel
On 5/12/20 10:05 AM, Krzysztof Kozlowski wrote:
> On Tue, 12 May 2020 at 10:47, Lukasz Luba <lukasz.luba@arm.com> wrote:
>>
>> Hi Krzysztof,
>>
>> I am sorry, I was a bit busy recently.
>>
>> On 5/12/20 7:50 AM, Krzysztof Kozlowski wrote:
>>> On Fri, May 08, 2020 at 06:13:38AM -0700, Bernard Zhao wrote:
>>>> Maybe dmc->df->lock is unnecessary to protect function
>>>> exynos5_dmc_perf_events_check(dmc). If we have to protect,
>>>> dmc->lock is more better and more effective.
>>>> Also, it seems not needed to protect "if (ret) & dev_warn"
>>>> branch.
>>>>
>>>> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>>>> ---
>>>> drivers/memory/samsung/exynos5422-dmc.c | 6 ++----
>>>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> I checked the concurrent accesses and it looks correct.
>>>
>>> Lukasz, any review from your side?
>>
>> The lock from devfreq lock protects from a scenario when
>> concurrent access from devfreq framework uses internal dmc fields 'load'
>> and 'total' (which are set to 'busy_time', 'total_time').
>> The .get_dev_status can be called at any time (even due to thermal
>> devfreq cooling action) and reads above fields.
>> That's why the calculation of the new values inside dmc is protected.
>
> I looked at this path (get_dev_status) and currently in devfreq it
> will be only called from update_devfreq() -> get_target_freq()... at
> least when looking at devfreq core and governors. On the other hand
> you are right that this is public function and this call scenario
> might change. It could be called directly from other paths sooner or
> later.
Indeed, I am currently changing this while I am adding devfreq devices
to the Energy Model.
>
>> This patch should not be taken IMO. Maybe we can release lock before the
>> if statement, just to speed-up.
>
> Yep.
>
> Bernard, you can send just this part of the patch.
Thank you Bernard and please submit the patch v2.
>
> Best regards,
> Krzysztof
>
Thank you Krzysztof for your time spent on this.
Regards,
Lukasz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-05-12 9:23 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-08 13:13 [PATCH] memory/samsung: reduce unnecessary mutex lock area Bernard Zhao
2020-05-08 13:13 ` Bernard Zhao
2020-05-12 6:50 ` Krzysztof Kozlowski
2020-05-12 6:50 ` Krzysztof Kozlowski
2020-05-12 8:47 ` Lukasz Luba
2020-05-12 8:47 ` Lukasz Luba
2020-05-12 9:05 ` Krzysztof Kozlowski
2020-05-12 9:05 ` Krzysztof Kozlowski
2020-05-12 9:19 ` Bernard
2020-05-12 9:19 ` Bernard
2020-05-12 9:23 ` Lukasz Luba
2020-05-12 9:23 ` Lukasz Luba
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.