Re: [PATCH] lib/sbitmap: kill 'depth' from sbitmap_word

[Jens Axboe <axboe@kernel.dk>]

On 1/9/22 7:47 PM, Ming Lei wrote:
> On Sun, Jan 09, 2022 at 07:43:26PM -0700, Jens Axboe wrote:
>> On 1/9/22 7:38 PM, Ming Lei wrote:
>>> On Sun, Jan 09, 2022 at 06:54:21PM -0700, Jens Axboe wrote:
>>>> On 1/9/22 6:50 PM, Ming Lei wrote:
>>>>> Only the last sbitmap_word can have different depth, and all the others
>>>>> must have same depth of 1U << sb->shift, so not necessary to store it in
>>>>> sbitmap_word, and it can be retrieved easily and efficiently by adding
>>>>> one internal helper of __map_depth(sb, index).
>>>>>
>>>>> Not see performance effect when running iops test on null_blk.
>>>>>
>>>>> This way saves us one cacheline(usually 64 words) per each sbitmap_word.
>>>>
>>>> We probably want to kill the ____cacheline_aligned_in_smp from 'word' as
>>>> well.
>>>
>>> But sbitmap_deferred_clear_bit() is called in put fast path, then the
>>> cacheline becomes shared with get path, and I guess this way isn't
>>> expected.
>>
>> Just from 'word', not from 'cleared'. They will still be in separate
>> cache lines, but usually doesn't make sense to have the leading member
>> marked as cacheline aligned, that's a whole struct property at that
>> point.
> 
> OK, got it, it is just sort of code style thing, and not any functional
> change.

Right, it's more of a cleanup. It would be a bit misleading to have
it on the first member and thinking you could rely on it, when
external factors come into play there.

> Will include it in V2.

Thanks!

-- 
Jens Axboe