Messages in this thread |  | | Subject | Re: [PATCH v5 6/7] locking/lockdep: Reuse freed chain_hlocks entries | From | Waiman Long <> | Date | Tue, 4 Feb 2020 09:54:06 -0500 |
| |
On 2/4/20 7:36 AM, Peter Zijlstra wrote: > On Mon, Feb 03, 2020 at 11:41:46AM -0500, Waiman Long wrote: >> + if (unlikely(size < 2)) >> + return; // XXX leak! > Stuck this on top... > > --- a/kernel/locking/lockdep.c > +++ b/kernel/locking/lockdep.c > @@ -2631,7 +2631,8 @@ struct lock_chain lock_chains[MAX_LOCKDE > static DECLARE_BITMAP(lock_chains_in_use, MAX_LOCKDEP_CHAINS); > static u16 chain_hlocks[MAX_LOCKDEP_CHAIN_HLOCKS]; > unsigned long nr_zapped_lock_chains; > -unsigned int nr_free_chain_hlocks; /* Free cfhain_hlocks in buckets */ > +unsigned int nr_free_chain_hlocks; /* Free chain_hlocks in buckets */ > +unsigned int nr_lost_chain_hlocks; /* Lost chain_hlocks */ > unsigned int nr_large_chain_blocks; /* size > MAX_CHAIN_BUCKETS */ > > /* > @@ -2718,8 +2719,17 @@ static inline void add_chain_block(int o > int bucket = size_to_bucket(size); > int next = chain_block_buckets[bucket]; > > - if (unlikely(size < 2)) > - return; // XXX leak! > + if (unlikely(size < 2)) { > + /* > + * We can't store single entries on the freelist. Leak them. > + * > + * One possible way out would be to uniquely mark them, other > + * than with CHAIN_BLK_FLAG, such that we can recover them when > + * the block before it is re-added. > + */ > + nr_lost_chain_hlocks++; > + return; > + } > > init_chain_block(offset, next, bucket, size); > chain_block_buckets[bucket] = offset; > @@ -2798,8 +2808,8 @@ static int alloc_chain_hlocks(int req) > > search: > /* > - * linear search in the 'dump' bucket; look for an exact match or the > - * largest block. > + * linear search of the variable sized freelist; look for an exact > + * match or the largest block. > */ > for_each_chain_block(0, prev, curr, next) { > size = chain_block_size(curr); > --- a/kernel/locking/lockdep_internals.h > +++ b/kernel/locking/lockdep_internals.h > @@ -141,6 +141,7 @@ extern unsigned int nr_hardirq_chains; > extern unsigned int nr_softirq_chains; > extern unsigned int nr_process_chains; > extern unsigned int nr_free_chain_hlocks; > +extern unsigned int nr_lost_chain_hlocks; > extern unsigned int nr_large_chain_blocks; > > extern unsigned int max_lockdep_depth; > --- a/kernel/locking/lockdep_proc.c > +++ b/kernel/locking/lockdep_proc.c > @@ -278,9 +278,11 @@ static int lockdep_stats_show(struct seq > #ifdef CONFIG_PROVE_LOCKING > seq_printf(m, " dependency chains: %11lu [max: %lu]\n", > lock_chain_count(), MAX_LOCKDEP_CHAINS); > - seq_printf(m, " dependency chain hlocks: %11lu [max: %lu]\n", > - MAX_LOCKDEP_CHAIN_HLOCKS - nr_free_chain_hlocks, > + seq_printf(m, " dependency chain hlocks used: %11lu [max: %lu]\n", > + MAX_LOCKDEP_CHAIN_HLOCKS - (nr_free_chain_hlocks - nr_lost_chain_hlocks), > MAX_LOCKDEP_CHAIN_HLOCKS); > + seq_printf(m, " dependency chain hlocks free: %11lu\n", nr_free_chain_hlocks); > + seq_printf(m, " dependency chain hlocks lost: %11lu\n", nr_lost_chain_hlocks); > #endif > > #ifdef CONFIG_TRACE_IRQFLAGS > Sure. Will do that.
Thanks for the suggestion.
Cheers, Longman
|  |