Re: [PATCH v5 07/15] locking/lockdep: Free lock classes that are no longer in use

From: Peter Zijlstra
Date: Thu Jan 10 2019 - 10:21:01 EST


Hi Bart,

Sorry for the delay; real-life and holidays got in the way :/

On Mon, Dec 17, 2018 at 01:29:54PM -0800, Bart Van Assche wrote:
> +/* Remove a class from a lock chain. Must be called with the graph lock held. */
> +static void remove_class_from_lock_chain(struct lock_chain *chain,
> + struct lock_class *class)
> +{
> +#ifdef CONFIG_PROVE_LOCKING
> + struct lock_chain *new_chain;
> + u64 chain_key;
> + int i;
> +
> + for (i = chain->base; i < chain->base + chain->depth; i++) {
> + if (chain_hlocks[i] != class - lock_classes)
> + continue;
> + /* The code below leaks one chain_hlock[] entry. */
> + if (--chain->depth > 0)
> + memmove(&chain_hlocks[i], &chain_hlocks[i + 1],
> + (chain->base + chain->depth - i) *
> + sizeof(chain_hlocks[0]));
> + /*
> + * Each lock class occurs at most once in a lock chain so once
> + * we found a match we can break out of this loop.
> + */
> + goto recalc;
> + }
> + /* Since the chain has not been modified, return. */
> + return;
> +
> +recalc:
> + chain_key = 0;
> + for (i = chain->base; i < chain->base + chain->depth; i++)
> + chain_key = iterate_chain_key(chain_key, chain_hlocks[i] + 1);
> + if (chain->depth && chain->chain_key == chain_key)
> + return;
> + /* Overwrite the chain key for concurrent RCU readers. */
> + WRITE_ONCE(chain->chain_key, chain_key);
> + /*
> + * Note: calling hlist_del_rcu() from inside a
> + * hlist_for_each_entry_rcu() loop is safe.
> + */
> + hlist_del_rcu(&chain->entry);
> + if (chain->depth == 0)
> + return;
> + /*
> + * If the modified lock chain matches an existing lock chain, drop
> + * the modified lock chain.
> + */
> + if (lookup_chain_cache(chain_key))
> + return;
> + if (WARN_ON_ONCE(nr_lock_chains >= MAX_LOCKDEP_CHAINS)) {
> + debug_locks_off();
> + return;
> + }
> + /*
> + * Leak *chain because it is not safe to reinsert it before an RCU
> + * grace period has expired.
> + */
> + new_chain = lock_chains + nr_lock_chains++;
> + *new_chain = *chain;
> + hlist_add_head_rcu(&new_chain->entry, chainhashentry(chain_key));
> +#endif
> +}

Leaking a chain_hlocks[] and lock_chains[] entry is pretty gross, but
yes that should work.