Messages in this thread | ![/](/images/icornerl.gif) | | Subject | Re: [PATCH v3 6/8] locking/lockdep: Reuse freed chain_hlocks entries | From | Waiman Long <> | Date | Sun, 19 Jan 2020 23:22:38 -0500 |
| |
On 1/16/20 4:13 PM, Peter Zijlstra wrote: > On Wed, Jan 15, 2020 at 04:43:11PM -0500, Waiman Long wrote: >> +static inline int alloc_chain_hlocks_from_buckets(int size) >> +{ >> + int prev, curr, next; >> + >> + if (!nr_free_chain_hlocks) >> + return -1; >> + >> + if (size <= MAX_CHAIN_BUCKETS) { >> + curr = chain_block_buckets[size - 1]; >> + if (curr < 0) >> + return -1; >> + >> + chain_block_buckets[size - 1] = next_chain_block(curr); >> + nr_free_chain_hlocks -= size; >> + return curr; >> + } >> + >> + /* >> + * Look for a free chain block of the given size >> + * >> + * It is rare to have a lock chain with depth > MAX_CHAIN_BUCKETS. >> + * It is also more expensive as we may iterate the whole list >> + * without finding one. >> + */ >> + for_each_chain_block(0, prev, curr, next) { >> + next = next_chain_block(curr); >> + if (chain_block_size(curr) == size) { >> + set_chain_block(prev, 0, next); >> + nr_free_chain_hlocks -= size; >> + nr_large_chain_blocks--; >> + return curr; >> + } >> + } >> + return -1; >> +} >> +static int alloc_chain_hlocks(int size) >> +{ >> + int curr; >> + >> + if (size < 2) >> + size = 2; >> + >> + curr = alloc_chain_hlocks_from_buckets(size); >> + if (curr >= 0) >> + return curr; >> + >> + BUILD_BUG_ON((1UL << 24) <= ARRAY_SIZE(chain_hlocks)); >> + BUILD_BUG_ON((1UL << 6) <= ARRAY_SIZE(current->held_locks)); >> + BUILD_BUG_ON((1UL << 8*sizeof(chain_hlocks[0])) <= >> + ARRAY_SIZE(lock_classes)); >> + >> + /* >> + * Allocate directly from chain_hlocks. >> + */ >> + if (likely(nr_chain_hlocks + size <= MAX_LOCKDEP_CHAIN_HLOCKS)) { >> + curr = nr_chain_hlocks; >> + nr_chain_hlocks += size; >> + return curr; >> + } >> + if (!debug_locks_off_graph_unlock()) >> + return -1; >> + >> + print_lockdep_off("BUG: MAX_LOCKDEP_CHAIN_HLOCKS too low!"); >> + dump_stack(); >> + return -1; >> +} > Argh, that's still _two_ half allocators. > > Here, please try this one, it seems to boot. It compiles with some > noise, but that is because GCC is stupid and I'm too tired. > > --- > > --- a/kernel/locking/lockdep.c > +++ b/kernel/locking/lockdep.c > @@ -1071,15 +1071,22 @@ static inline void check_data_structures > > #endif /* CONFIG_DEBUG_LOCKDEP */ > > +static void init_chain_block_buckets(void); > + > /* > * Initialize the lock_classes[] array elements, the free_lock_classes list > * and also the delayed_free structure. > */ > static void init_data_structures_once(void) > { > - static bool ds_initialized, rcu_head_initialized; > + static bool ds_initialized, rcu_head_initialized, chain_block_initialized; > int i; > > + if (!chain_block_initialized) { > + chain_block_initialized = true; > + init_chain_block_buckets(); > + } > + > if (likely(rcu_head_initialized)) > return;
Oh, I was not aware that there is such a init_data_structure_once() function. I don't think we need a chain_block_initialized. The ds_initialized should be enough and the init_chain_block_buckets() can be put to the end of the function.
Other than that, the rests look OK to me so far. I will try it out tomorrow.
Thanks, Longman
| ![\](/images/icornerr.gif) |