[PATCH] Doc: lockdep: add information about performance impact

[PATCH] Doc: lockdep: add information about performance impact

[Lukasz Luba]

This patch add some warning related to performance drop.
It should be mentioned that this is not for free
and the platfrom resources (cache, bus interconnect, etc.)
will be used more frequently.

Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
---
 Documentation/locking/lockdep-design.txt | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/locking/lockdep-design.txt b/Documentation/locking/lockdep-design.txt
index 49f58a0..1af3686 100644
--- a/Documentation/locking/lockdep-design.txt
+++ b/Documentation/locking/lockdep-design.txt
@@ -331,3 +331,22 @@ Run the command and save the output, then compare against the output from
 a later run of this command to identify the leakers.  This same output
 can also help you find situations where runtime lock initialization has
 been omitted.
+
+This feature can have performance impact, which affects context
+switching time, cache invalidations, delays on bus transactions.
+System performance in some use cases can drop x3-x4 times.
+Tested on ARM Exynos5422 and ARM64 Exynos5433 big.LITTLE architectures
+(overhead is really big).
+The overhead can be measures using hackbench which will show different
+finish time (11sec -> 3.4sec(no lockdep)).
+Use 'perf' with enabled events showing cache usage, and bus usage
+(it is architecture specyfic and if needed on ARM enable CCI to check
+bus transactions).
+When you check transaction on cache or buses, the results are way higher
+than normal for the same hackbench test:
+L1d cache invalidations: 26mln vs 4mln
+L2u cache invalidations: 42mln vs 12mln
+bus cyc/access: 30cyc/access vs. 20cyc/access
+context switch is x3 times cheaper.
+Apart from hackbench issue, there is dhrystone performance drop,
+1.05 DMIPS/MHz vs. 1.45 DMIPS/MHz (no lockdep) on 'big' core.
-- 
2.7.4

[PATCH] Doc: lockdep: add information about performance impact

[Lukasz Luba]

This patch add some warning related to performance drop.
It should be mentioned that this is not for free
and the platfrom resources (cache, bus interconnect, etc.)
will be used more frequently.

Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
---
 Documentation/locking/lockdep-design.txt | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/locking/lockdep-design.txt b/Documentation/locking/lockdep-design.txt
index 49f58a0..1af3686 100644
--- a/Documentation/locking/lockdep-design.txt
+++ b/Documentation/locking/lockdep-design.txt
@@ -331,3 +331,22 @@ Run the command and save the output, then compare against the output from
 a later run of this command to identify the leakers.  This same output
 can also help you find situations where runtime lock initialization has
 been omitted.
+
+This feature can have performance impact, which affects context
+switching time, cache invalidations, delays on bus transactions.
+System performance in some use cases can drop x3-x4 times.
+Tested on ARM Exynos5422 and ARM64 Exynos5433 big.LITTLE architectures
+(overhead is really big).
+The overhead can be measures using hackbench which will show different
+finish time (11sec -> 3.4sec(no lockdep)).
+Use 'perf' with enabled events showing cache usage, and bus usage
+(it is architecture specyfic and if needed on ARM enable CCI to check
+bus transactions).
+When you check transaction on cache or buses, the results are way higher
+than normal for the same hackbench test:
+L1d cache invalidations: 26mln vs 4mln
+L2u cache invalidations: 42mln vs 12mln
+bus cyc/access: 30cyc/access vs. 20cyc/access
+context switch is x3 times cheaper.
+Apart from hackbench issue, there is dhrystone performance drop,
+1.05 DMIPS/MHz vs. 1.45 DMIPS/MHz (no lockdep) on 'big' core.
-- 
2.7.4

[PATCH] Doc: lockdep: add information about performance impact

[Peter Zijlstra]

On Tue, Oct 09, 2018 at 05:39:27PM +0200, Lukasz Luba wrote:
> This patch add some warning related to performance drop.
> It should be mentioned that this is not for free
> and the platfrom resources (cache, bus interconnect, etc.)
> will be used more frequently.

To me this reads a bit like: water is wet.

Is this really needed?

Re: [PATCH] Doc: lockdep: add information about performance impact

[Peter Zijlstra]

On Tue, Oct 09, 2018 at 05:39:27PM +0200, Lukasz Luba wrote:
> This patch add some warning related to performance drop.
> It should be mentioned that this is not for free
> and the platfrom resources (cache, bus interconnect, etc.)
> will be used more frequently.

To me this reads a bit like: water is wet.

Is this really needed?

[PATCH] Doc: lockdep: add information about performance impact

[Will Deacon]

On Tue, Oct 09, 2018 at 05:43:59PM +0200, Peter Zijlstra wrote:
> On Tue, Oct 09, 2018 at 05:39:27PM +0200, Lukasz Luba wrote:
> > This patch add some warning related to performance drop.
> > It should be mentioned that this is not for free
> > and the platfrom resources (cache, bus interconnect, etc.)
> > will be used more frequently.
> 
> To me this reads a bit like: water is wet.
> 
> Is this really needed?

I don't think so -- this is a debug option under "kernel hacking". Surely
the perf hit comes with the territory.

Will

Re: [PATCH] Doc: lockdep: add information about performance impact

[Will Deacon]

On Tue, Oct 09, 2018 at 05:43:59PM +0200, Peter Zijlstra wrote:
> On Tue, Oct 09, 2018 at 05:39:27PM +0200, Lukasz Luba wrote:
> > This patch add some warning related to performance drop.
> > It should be mentioned that this is not for free
> > and the platfrom resources (cache, bus interconnect, etc.)
> > will be used more frequently.
> 
> To me this reads a bit like: water is wet.
> 
> Is this really needed?

I don't think so -- this is a debug option under "kernel hacking". Surely
the perf hit comes with the territory.

Will

[PATCH] Doc: lockdep: add information about performance impact

[Russell King - ARM Linux]

On Tue, Oct 09, 2018 at 04:58:18PM +0100, Will Deacon wrote:
> On Tue, Oct 09, 2018 at 05:43:59PM +0200, Peter Zijlstra wrote:
> > On Tue, Oct 09, 2018 at 05:39:27PM +0200, Lukasz Luba wrote:
> > > This patch add some warning related to performance drop.
> > > It should be mentioned that this is not for free
> > > and the platfrom resources (cache, bus interconnect, etc.)
> > > will be used more frequently.
> > 
> > To me this reads a bit like: water is wet.
> > 
> > Is this really needed?
> 
> I don't think so -- this is a debug option under "kernel hacking". Surely
> the perf hit comes with the territory.

Indeed it does - since adding debug code means additional instructions,
more cache pressure and, therefore, slower execution.

I've already said to people... turn on all debug options for development,
and when you move to performance evaluation and optimisation for the
production version, evaluate and turn off debug options.

Unfortunately, turning off PROVE_LOCKING in the defconfigs is going
to have a negative impact on the automated build/boot testing that
systems like kernelci.org do for us - these are primarily based
around the defconfigs, which means we're going to end up building
with PROVE_LOCKING disabled.  This means kernelci.org will be less
likely to catch locking issues.

Forcing PROVE_LOCKING on also doesn't make sense - there may be (rare)
bugs, eg race conditions, uncovered by having that disabled.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH] Doc: lockdep: add information about performance impact

[Russell King - ARM Linux]

On Tue, Oct 09, 2018 at 04:58:18PM +0100, Will Deacon wrote:
> On Tue, Oct 09, 2018 at 05:43:59PM +0200, Peter Zijlstra wrote:
> > On Tue, Oct 09, 2018 at 05:39:27PM +0200, Lukasz Luba wrote:
> > > This patch add some warning related to performance drop.
> > > It should be mentioned that this is not for free
> > > and the platfrom resources (cache, bus interconnect, etc.)
> > > will be used more frequently.
> > 
> > To me this reads a bit like: water is wet.
> > 
> > Is this really needed?
> 
> I don't think so -- this is a debug option under "kernel hacking". Surely
> the perf hit comes with the territory.

Indeed it does - since adding debug code means additional instructions,
more cache pressure and, therefore, slower execution.

I've already said to people... turn on all debug options for development,
and when you move to performance evaluation and optimisation for the
production version, evaluate and turn off debug options.

Unfortunately, turning off PROVE_LOCKING in the defconfigs is going
to have a negative impact on the automated build/boot testing that
systems like kernelci.org do for us - these are primarily based
around the defconfigs, which means we're going to end up building
with PROVE_LOCKING disabled.  This means kernelci.org will be less
likely to catch locking issues.

Forcing PROVE_LOCKING on also doesn't make sense - there may be (rare)
bugs, eg race conditions, uncovered by having that disabled.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

[PATCH] Doc: lockdep: add information about performance impact

[Lukasz Luba]

Hi Peter,

On 10/09/2018 05:43 PM, Peter Zijlstra wrote:
> On Tue, Oct 09, 2018 at 05:39:27PM +0200, Lukasz Luba wrote:
>> This patch add some warning related to performance drop.
>> It should be mentioned that this is not for free
>> and the platfrom resources (cache, bus interconnect, etc.)
>> will be used more frequently.
> 
> To me this reads a bit like: water is wet.
> 
> Is this really needed?
> 
> 
Well, it would be good to know what is the performance drop
(10% or 20% or x3 times) when you are enabling different debug options.
I have spent some time analyzing these cache and bus strange behavior.
Now the developers would know that LOCKDEP might cause constant trashing 
of your cache in some use cases.

Regards,
Lukasz Luba

Re: [PATCH] Doc: lockdep: add information about performance impact

[Lukasz Luba]

Hi Peter,

On 10/09/2018 05:43 PM, Peter Zijlstra wrote:
> On Tue, Oct 09, 2018 at 05:39:27PM +0200, Lukasz Luba wrote:
>> This patch add some warning related to performance drop.
>> It should be mentioned that this is not for free
>> and the platfrom resources (cache, bus interconnect, etc.)
>> will be used more frequently.
> 
> To me this reads a bit like: water is wet.
> 
> Is this really needed?
> 
> 
Well, it would be good to know what is the performance drop
(10% or 20% or x3 times) when you are enabling different debug options.
I have spent some time analyzing these cache and bus strange behavior.
Now the developers would know that LOCKDEP might cause constant trashing 
of your cache in some use cases.

Regards,
Lukasz Luba

[PATCH] Doc: lockdep: add information about performance impact

[Robin Murphy]

On 09/10/18 17:06, Lukasz Luba wrote:
> Hi Peter,
> 
> On 10/09/2018 05:43 PM, Peter Zijlstra wrote:
>> On Tue, Oct 09, 2018 at 05:39:27PM +0200, Lukasz Luba wrote:
>>> This patch add some warning related to performance drop.
>>> It should be mentioned that this is not for free
>>> and the platfrom resources (cache, bus interconnect, etc.)
>>> will be used more frequently.
>>
>> To me this reads a bit like: water is wet.
>>
>> Is this really needed?
>>
>>
> Well, it would be good to know what is the performance drop
> (10% or 20% or x3 times) when you are enabling different debug options.
> I have spent some time analyzing these cache and bus strange behavior.
> Now the developers would know that LOCKDEP might cause constant trashing
> of your cache in some use cases.

Fair enough, but this is the wrong place for that. Anyone who's got as 
far as reading how the internals of lockdep work can probably already 
figure out that that brings a non-trivial overhead, whereas Joe 
Developer wondering why his kernel is slow seems unlikely to happen 
across this document by chance. And the people shipping devices with 
PROVE_LOCKING enabled because it happened to mask some tricky bug, well, 
they know what they did ;)

If you want to highlight to unwitting users that a tweaking a particular 
config knob has a significant performance hit, at least put the warning 
next to said knob, i.e. in the Kconfig help. For an example, DMA debug 
comes to mind.

Robin.

Re: [PATCH] Doc: lockdep: add information about performance impact

[Robin Murphy]

On 09/10/18 17:06, Lukasz Luba wrote:
> Hi Peter,
> 
> On 10/09/2018 05:43 PM, Peter Zijlstra wrote:
>> On Tue, Oct 09, 2018 at 05:39:27PM +0200, Lukasz Luba wrote:
>>> This patch add some warning related to performance drop.
>>> It should be mentioned that this is not for free
>>> and the platfrom resources (cache, bus interconnect, etc.)
>>> will be used more frequently.
>>
>> To me this reads a bit like: water is wet.
>>
>> Is this really needed?
>>
>>
> Well, it would be good to know what is the performance drop
> (10% or 20% or x3 times) when you are enabling different debug options.
> I have spent some time analyzing these cache and bus strange behavior.
> Now the developers would know that LOCKDEP might cause constant trashing
> of your cache in some use cases.

Fair enough, but this is the wrong place for that. Anyone who's got as 
far as reading how the internals of lockdep work can probably already 
figure out that that brings a non-trivial overhead, whereas Joe 
Developer wondering why his kernel is slow seems unlikely to happen 
across this document by chance. And the people shipping devices with 
PROVE_LOCKING enabled because it happened to mask some tricky bug, well, 
they know what they did ;)

If you want to highlight to unwitting users that a tweaking a particular 
config knob has a significant performance hit, at least put the warning 
next to said knob, i.e. in the Kconfig help. For an example, DMA debug 
comes to mind.

Robin.

[PATCH] Doc: lockdep: add information about performance impact

[Lukasz Luba]

Hi Robin,

On 10/10/2018 12:31 PM, Robin Murphy wrote:
> On 09/10/18 17:06, Lukasz Luba wrote:
>> Hi Peter,
>>
>> On 10/09/2018 05:43 PM, Peter Zijlstra wrote:
>>> On Tue, Oct 09, 2018 at 05:39:27PM +0200, Lukasz Luba wrote:
>>>> This patch add some warning related to performance drop.
>>>> It should be mentioned that this is not for free
>>>> and the platfrom resources (cache, bus interconnect, etc.)
>>>> will be used more frequently.
>>>
>>> To me this reads a bit like: water is wet.
>>>
>>> Is this really needed?
>>>
>>>
>> Well, it would be good to know what is the performance drop
>> (10% or 20% or x3 times) when you are enabling different debug options.
>> I have spent some time analyzing these cache and bus strange behavior.
>> Now the developers would know that LOCKDEP might cause constant trashing
>> of your cache in some use cases.
> 
> Fair enough, but this is the wrong place for that. Anyone who's got as 
> far as reading how the internals of lockdep work can probably already 
> figure out that that brings a non-trivial overhead, whereas Joe 
> Developer wondering why his kernel is slow seems unlikely to happen 
> across this document by chance. And the people shipping devices with 
> PROVE_LOCKING enabled because it happened to mask some tricky bug, well, 
> they know what they did ;)
> 
> If you want to highlight to unwitting users that a tweaking a particular 
> config knob has a significant performance hit, at least put the warning 
> next to said knob, i.e. in the Kconfig help. For an example, DMA debug 
> comes to mind.
> 
> Robin.
> 
> 
Thank for the hint.
I agree, it would not be easy to find in this location.
I've found it during porting EAS where the
PROVE_LOCKING is used for arm and arm64
http://linux-arm.org/git?p=linux-power.git;a=blobdiff;f=arch/arm/configs/multi_v7_defconfig;h=e83c6e5bcfff97f8e9aacc4f2086ed506df63e59;hp=2721877d5a11f7fc41b8eb42d040ccde10eb0f3e;hb=737c15bf13632504e94509f0b9508122c664f3f2;hpb=288ae1d294d314b0aad5e0471605089da0336a77

I just wanted to give some rough performance impact measurements on
a real platform.

Regards,
Lukasz

Re: [PATCH] Doc: lockdep: add information about performance impact

[Lukasz Luba]

Hi Robin,

On 10/10/2018 12:31 PM, Robin Murphy wrote:
> On 09/10/18 17:06, Lukasz Luba wrote:
>> Hi Peter,
>>
>> On 10/09/2018 05:43 PM, Peter Zijlstra wrote:
>>> On Tue, Oct 09, 2018 at 05:39:27PM +0200, Lukasz Luba wrote:
>>>> This patch add some warning related to performance drop.
>>>> It should be mentioned that this is not for free
>>>> and the platfrom resources (cache, bus interconnect, etc.)
>>>> will be used more frequently.
>>>
>>> To me this reads a bit like: water is wet.
>>>
>>> Is this really needed?
>>>
>>>
>> Well, it would be good to know what is the performance drop
>> (10% or 20% or x3 times) when you are enabling different debug options.
>> I have spent some time analyzing these cache and bus strange behavior.
>> Now the developers would know that LOCKDEP might cause constant trashing
>> of your cache in some use cases.
> 
> Fair enough, but this is the wrong place for that. Anyone who's got as 
> far as reading how the internals of lockdep work can probably already 
> figure out that that brings a non-trivial overhead, whereas Joe 
> Developer wondering why his kernel is slow seems unlikely to happen 
> across this document by chance. And the people shipping devices with 
> PROVE_LOCKING enabled because it happened to mask some tricky bug, well, 
> they know what they did ;)
> 
> If you want to highlight to unwitting users that a tweaking a particular 
> config knob has a significant performance hit, at least put the warning 
> next to said knob, i.e. in the Kconfig help. For an example, DMA debug 
> comes to mind.
> 
> Robin.
> 
> 
Thank for the hint.
I agree, it would not be easy to find in this location.
I've found it during porting EAS where the
PROVE_LOCKING is used for arm and arm64
http://linux-arm.org/git?p=linux-power.git;a=blobdiff;f=arch/arm/configs/multi_v7_defconfig;h=e83c6e5bcfff97f8e9aacc4f2086ed506df63e59;hp=2721877d5a11f7fc41b8eb42d040ccde10eb0f3e;hb=737c15bf13632504e94509f0b9508122c664f3f2;hpb=288ae1d294d314b0aad5e0471605089da0336a77

I just wanted to give some rough performance impact measurements on
a real platform.

Regards,
Lukasz