| Message ID | 20241209115522.3741093-4-yukuai1@huaweicloud.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | lib/sbitmap: fix shallow_depth tag allocation | expand |
On 12/9/24 7:55 PM, Yu Kuai wrote: > From: Yu Kuai <yukuai3@huawei.com> > > Currently, shallow_depth is used by bfq, kyber and mq-deadline, they both > pass in the value for the whole sbitmap, while sbitmap treate the value > for just one word. Which means, shallow_depth never work as expected, > and there really is no such functional tests to covert it. treate -> treats The above text is incorrect. I have verified that shallow_depth restricts the queue depth of asynchronous requests for mq-deadline if it is reduced from its default value. The function dd_word_to_depth() in mq-deadline converts the value written into the sysfs attribute into a value that is appropriate for the sbitmap implementation. That being said, it seems like a good idea to me to modify how the sbitmap code interprets 'shallow_depth'. Thanks, Bart.
Hi, 在 2024/12/10 2:11, Bart Van Assche 写道: > On 12/9/24 7:55 PM, Yu Kuai wrote: >> From: Yu Kuai <yukuai3@huawei.com> >> >> Currently, shallow_depth is used by bfq, kyber and mq-deadline, they both >> pass in the value for the whole sbitmap, while sbitmap treate the value >> for just one word. Which means, shallow_depth never work as expected, >> and there really is no such functional tests to covert it. > > treate -> treats > > The above text is incorrect. I have verified that shallow_depth > restricts the queue depth of asynchronous requests for mq-deadline if it > is reduced from its default value. The function dd_word_to_depth() in > mq-deadline converts the value written into the sysfs attribute into a > value that is appropriate for the sbitmap implementation. Hi, please notice that patch 1 is reverted in this set, which means dd_word_to_depth() is not existed. :) And for patch 1, dd_word_to_depth() doesn't(and can't) handle the last word, for the case that avaliable bits in the last word is less than 1 << sb->shift. So, in fact it's not convert to the appropriate value. > > That being said, it seems like a good idea to me to modify how the > sbitmap code interprets 'shallow_depth'. Good to know! Thanks, Kuai > > Thanks, > > Bart. > . >
diff --git a/include/linux/sbitmap.h b/include/linux/sbitmap.h index e1730f5fdf9c..ffb9907c7070 100644 --- a/include/linux/sbitmap.h +++ b/include/linux/sbitmap.h @@ -461,7 +461,7 @@ unsigned long __sbitmap_queue_get_batch(struct sbitmap_queue *sbq, int nr_tags, * sbitmap_queue, limiting the depth used from each word, with preemption * already disabled. * @sbq: Bitmap queue to allocate from. - * @shallow_depth: The maximum number of bits to allocate from a single word. + * @shallow_depth: The maximum number of bits to allocate from the queue. * See sbitmap_get_shallow(). * * If you call this, make sure to call sbitmap_queue_min_shallow_depth() after diff --git a/lib/sbitmap.c b/lib/sbitmap.c index 9d4213ce7916..13831c7536a3 100644 --- a/lib/sbitmap.c +++ b/lib/sbitmap.c @@ -208,8 +208,27 @@ static int sbitmap_find_bit_in_word(struct sbitmap_word *map, return nr; } +static unsigned int __map_depth_with_shallow(const struct sbitmap *sb, + int index, + unsigned int shallow_depth) +{ + unsigned int pre_word_bits = 0; + + if (shallow_depth >= sb->depth) + return __map_depth(sb, index); + + if (index > 0) + pre_word_bits += (index - 1) << sb->shift; + + if (shallow_depth <= pre_word_bits) + return 0; + + return min_t(unsigned int, __map_depth(sb, index), + shallow_depth - pre_word_bits); +} + static int sbitmap_find_bit(struct sbitmap *sb, - unsigned int depth, + unsigned int shallow_depth, unsigned int index, unsigned int alloc_hint, bool wrap) @@ -218,12 +237,12 @@ static int sbitmap_find_bit(struct sbitmap *sb, int nr = -1; for (i = 0; i < sb->map_nr; i++) { - nr = sbitmap_find_bit_in_word(&sb->map[index], - min_t(unsigned int, - __map_depth(sb, index), - depth), - alloc_hint, wrap); + unsigned int depth = __map_depth_with_shallow(sb, index, + shallow_depth); + if (depth) + nr = sbitmap_find_bit_in_word(&sb->map[index], depth, + alloc_hint, wrap); if (nr != -1) { nr += index << sb->shift; break;