Messages in this thread | | | Date | Thu, 6 Feb 2020 17:03:34 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH v6 6/6] locking/lockdep: Reuse freed chain_hlocks entries |
| |
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; > +}
| |