Re: [PATCH v6 6/6] locking/lockdep: Reuse freed chain_hlocks entries

[Peter Zijlstra]

On Thu, Feb 06, 2020 at 10:24:08AM -0500, Waiman Long wrote:
> +#define for_each_chain_block(bucket, prev, curr)		\
> +	for ((prev) = -1, (curr) = chain_block_buckets[bucket];	\
> +	     (curr) >= 0;					\
> +	     (prev) = (curr), (curr) = chain_block_next(curr))

> +static inline void add_chain_block(int offset, int size)
> +{
> +	int bucket = size_to_bucket(size);
> +	int next = chain_block_buckets[bucket];
> +	int prev, curr;
> +
> +	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.
> +		 */
> +		if (size)
> +			nr_lost_chain_hlocks++;
> +		return;
> +	}
> +
> +	nr_free_chain_hlocks += size;
> +	if (!bucket) {
> +		nr_large_chain_blocks++;
> +
> +		if (unlikely(next >= 0)) {

I was surprised by this condition..

> +			/*
> +			 * Variable sized, sort large to small.
> +			 */
> +			for_each_chain_block(0, prev, curr) {
> +				if (size >= chain_block_size(curr))
> +					break;
> +			}

Because if that is not so, then here:
	@curr == @next
	@prev == -1

> +			init_chain_block(offset, curr, 0, size);

and this will be identical to:

	init_chain_block(offset, next, bucket, size);


> +			if (prev < 0)
> +				chain_block_buckets[0] = offset;
> +			else
> +				init_chain_block(prev, offset, 0, 0);

and this will be:

	chain_block_buckets[bucket] = offset;


Or am I missing something?

> +			return;
> +		}
> +	}
> +	/*
> +	 * Fixed size or bucket[0] empty, add to head.
> +	 */
> +	init_chain_block(offset, next, bucket, size);
> +	chain_block_buckets[bucket] = offset;
> +}