Re: [PATCH 4/5] locking/lockdep: Reuse free chain_hlocks entries

[Waiman Long]

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