Messages in this thread | | | Subject | Re: [PATCH 4/5] locking/lockdep: Reuse free chain_hlocks entries | From | Waiman Long <> | Date | Sun, 15 Dec 2019 12:06:04 -0500 |
| |
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
| |