linux-kernel - Re:[PATCH v4] block: plug attempts to batch allocate tags multiple times

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20251014083144.5479-1-xue01.he@samsung.com>
Date: Tue, 14 Oct 2025 08:31:44 +0000
From: Xue He <xue01.he@...sung.com>
To: yukuai1@...weicloud.com
Cc: akpm@...ux-foundation.org, axboe@...nel.dk, linux-block@...r.kernel.org,
	linux-kernel@...r.kernel.org, xue01.he@...sung.com, yukuai3@...wei.com
Subject: Re:[PATCH v4] block: plug attempts to batch allocate tags multiple
 times

Hi Yu Kuai,

On 2025/10/14 15:28, Yu Kuai wrote:
>On 2025/10/10 14:35, Xue He wrote:
>>   static unsigned int __map_depth_with_shallow(const struct sbitmap *sb,
>>   					     int index,
>>   					     unsigned int shallow_depth)
>> @@ -534,26 +560,22 @@ unsigned long __sbitmap_queue_get_batch(struct sbitmap_queue *sbq, int nr_tags,
>>   		unsigned int map_depth = __map_depth(sb, index);
>>   		unsigned long val;
>>   
>> -		sbitmap_deferred_clear(map, 0, 0, 0);
>> -		val = READ_ONCE(map->word);
>> -		if (val == (1UL << (map_depth - 1)) - 1)
>> +		nr = sbitmap_find_bits_in_word(map, 0, 0, 0, &val, nr_tags, map_depth);
>> +		if (nr == -1UL)
>>   			goto next;
>>   
>> -		nr = find_first_zero_bit(&val, map_depth);
>> -		if (nr + nr_tags <= map_depth) {
>> -			atomic_long_t *ptr = (atomic_long_t *) &map->word;
>> -
>> -			get_mask = ((1UL << nr_tags) - 1) << nr;
>> -			while (!atomic_long_try_cmpxchg(ptr, &val,
>> -							  get_mask | val))
>> -				;
>> -			get_mask = (get_mask & ~val) >> nr;
>> -			if (get_mask) {
>> -				*offset = nr + (index << sb->shift);
>> -				update_alloc_hint_after_get(sb, depth, hint,
>> -							*offset + nr_tags - 1);
>> -				return get_mask;
>> -			}
>> +		atomic_long_t *ptr = (atomic_long_t *) &map->word;
>> +
>> +		get_mask = ((1UL << nr_tags) - 1) << nr;
>> +		while (!atomic_long_try_cmpxchg(ptr, &val,
>> +						  get_mask | val))
>> +			;
>> +		get_mask = (get_mask & ~val) >> nr;
>> +		if (get_mask) {
>> +			*offset = nr + (index << sb->shift);
>> +			update_alloc_hint_after_get(sb, depth, hint,
>> +						*offset + nr_tags - 1);
>> +			return get_mask;
>
>I feel it's better to fold in above into the helper and return get_mask
>directly.
Hi Yukuai, sorry I'm not sure that my understanding is correct, does it
mean modifying it in a way similar to the following? If so, I will repackage
the patch accordingly.

static unsigned long sbitmap_find_bits_in_word(unsigned long index, 
					struct sbitmap *sb, unsigned int *offset,
					unsigned int hint, unsigned int depth, int nr_tags)
{
	unsigned long val, nr, get_mask;
	struct sbitmap_word *map = &sb->map[index];
	unsigned int map_depth = __map_depth(sb, index);

	while (1) {
		val = READ_ONCE(map->word);
		if (val == (1UL << (map_depth - 1)) - 1) {
			if (!sbitmap_deferred_clear(map, 0, 0, 0))
				return 0;
			continue;
		}

		nr = find_first_zero_bit(&val, map_depth);
		if (nr != map_depth)
			break;

		if (!sbitmap_deferred_clear(map, 0, 0, 0))
			return 0;
	};

	atomic_long_t *ptr = (atomic_long_t *) &map->word;
	
	get_mask = ((1UL << nr_tags) - 1) << nr;
	while (!atomic_long_try_cmpxchg(ptr, &val,
					  get_mask | val))
		;
	get_mask = (get_mask & ~val) >> nr;
	if (get_mask){
		*offset = nr + (index << sb->shift);
		update_alloc_hint_after_get(sb, depth, hint,
					*offset + nr_tags - 1);
	}
	return get_mask;
}

and upper like

unsigned long __sbitmap_queue_get_batch(struct sbitmap_queue *sbq, int nr_tags,
						unsigned int *offset)
{
	struct sbitmap *sb = &sbq->sb;
	unsigned int hint, depth;
	unsigned long get_mask, index;
	int i;

	if (unlikely(sb->round_robin))
		return 0;

	depth = READ_ONCE(sb->depth);
	hint = update_alloc_hint_before_get(sb, depth);

	index = SB_NR_TO_INDEX(sb, hint);

	for (i = 0; i < sb->map_nr; i++) {
		get_mask = sbitmap_find_bits_in_word(index, sb, offset, hint,
							depth, nr_tags);
		if (get_mask) {
			return get_mask;
		}

		/* Jump to next index. */
		if (++index >= sb->map_nr)
			index = 0;
	}

	return 0;
}

Am I missing something? 

Thanks,
Xue
>
>BTW, the sbitmap and blk-mq changes are not quite related, and you can
>split them into seperate patches.
>
>Thanks,
>Kuai
>
>>   		}
>>   next:
>>   		/* Jump to next index. */
>>