linux-kernel - Re: [PATCH 4/5] locking/lockdep: Reuse free chain

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1f680c85-c338-6a9e-01f4-d1f6a36e0c6f@redhat.com>
Date:   Sun, 15 Dec 2019 12:06:04 -0500
From:   Waiman Long <longman@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Ingo Molnar <mingo@...hat.com>, Will Deacon <will.deacon@....com>,
        linux-kernel@...r.kernel.org, Bart Van Assche <bvanassche@....org>
Subject: Re: [PATCH 4/5] locking/lockdep: Reuse free chain_hlocks entries

On 12/13/19 3:08 PM, Waiman Long wrote:
> On 12/13/19 1:47 PM, Peter Zijlstra wrote:
>> On Fri, Dec 13, 2019 at 01:35:05PM -0500, Waiman Long wrote:
>>> On 12/13/19 1:12 PM, Peter Zijlstra wrote:
>>>>> In this way, the wasted space will be k bytes where k is the number of
>>>>> 1-entry chains. I don't think merging adjacent blocks will be that
>>>>> useful at this point. We can always add this capability later on if it
>>>>> is found to be useful.
>>>> I'm thinking 1 entry isn't much of a chain. My brain is completely fried
>>>> atm, but are we really storing single entry 'chains' ? It seems to me we
>>>> could skip that.
>>>>
>>> Indeed, the current code can produce a 1-entry chain. I also thought
>>> that a chain had to be at least 2 entries. I got tripped up assuming
>>> that. It could be a bug somewhere that allow a 1-entry chain to happen,
>>> but I am not focusing on that right now.
>> If we need the minimum 2 entry granularity, it might make sense to spend
>> a little time on that. If we can get away with single entry markers,
>> then maybe write a comment so we'll not forget about it.
>>
> I will take a look at why an 1-entry chain happes and see if it is a bug
> that need to be fixed.

New lock chains are stored as part of the validate_chain() call from
__lock_acquire(). So for a n-entry lock chain, all previous n-1, n-2,
... 1 entry lock chains are stored as well. That may not be the most
efficient way to store the information, but it is simple. When booting
up a 2-socket x86-64 system, I saw about 800 1-entry lock chains being
stored.

Since I am planning to enforce a minimum of 2 chain_hlocks entry
allocation, we can theoretically allow a 1-entry chain to share the same
storage with a 2-entry chains with the same starting lock. That will add
a bit more code in the allocation and freeing path. I am not planning to
do that for this patchset, but may consider it as a follow up patch.

Cheers,
Longman