Re: [PATCH 3/6] bio: add allocation cache abstraction

[Jens Axboe <axboe@kernel.dk>]

On 8/12/21 10:32 AM, Christoph Hellwig wrote:
> [adding Thomas for a cpu hotplug questions]
> 
>> +static void bio_alloc_cache_destroy(struct bio_set *bs)
>> +{
>> +	int cpu;
>> +
>> +	if (!bs->cache)
>> +		return;
>> +
>> +	preempt_disable();
>> +	cpuhp_state_remove_instance_nocalls(CPUHP_BIO_DEAD, &bs->cpuhp_dead);
>> +	for_each_possible_cpu(cpu) {
>> +		struct bio_alloc_cache *cache;
>> +
>> +		cache = per_cpu_ptr(bs->cache, cpu);
>> +		bio_alloc_cache_prune(cache, -1U);
>> +	}
>> +	preempt_enable();
> 
> If I understand the cpu hotplug state machine we should not get any new
> cpu down callbacks after cpuhp_state_remove_instance_nocalls returned,
> so what do we need the preempt disable here for?

I don't think we strictly need it. I can kill it.

>> +	/*
>> +	 * Hot un-plug notifier for the per-cpu cache, if used
>> +	 */
>> +	struct hlist_node cpuhp_dead;
> 
> Nit, even if we don't need the cpu up notifaction the node actually
> provides both.  So I'd reword the comment drop the _dead from the
> member name.

Right, but we only sign up for the down call.

-- 
Jens Axboe