Re: [PATCH v2 0/4] rust: add `Alignment` type

["Alexandre Courbot" <acourbot@nvidia.com>]

On Mon Aug 4, 2025 at 11:16 PM JST, Miguel Ojeda wrote:
> On Mon, Aug 4, 2025 at 1:45 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>>
>> - The `last_set_bit` function is dropped, with the recommendation to use
>>   the standard library's `checked_ilog2` which does essentially the same
>>   thing.
>
> Yeah, let's see what people think about this one on the kernel side.
>
> I don't mind either way, i.e. to have a few wrappers with slightly
> different semantics if that is more common/understandable.
>
>> The upstream `Alignment` is more constrained than the `PowerOfTwo` of
>> the last revision: it uses `usize` internally instead of a generic
>> value, and does not provide `align_down` or `align_up` methods.
>
> `PowerOfTwo` seemed fine to me as well (or even implementing one in
> terms of the other).

`PowerOfTwo` has little prospect of existing upstream, and I think we
should be able to live pretty well with `Alignment` thanks to the
suggestions you make below.

>
>> These two shortcomings come together very nicely to gift us with a nice
>> headache: we need to align values potentially larger than `usize`, thus
>> need to make `align_down` and `align_up` generic. The generic parameter
>> needs to be constrained on the operations used to perform the alignment
>> (e.g. `BitAnd`, `Not`, etc) and there is one essential operation for
>> which no trait exists in the standard library: `checked_add`. Thus the
>> first patch of this series introduces a trait for it in the `num` module
>> and implements it for all integer types. I suspect we will need
>> something alongside these lines for other purposes anyway, and probably
>> other traits too.
>
> This part could be avoided implementing them the other way around,
> right? i.e. as an extension trait on the other side.
>
> It may also be also a bit easier to understand on the call site, too,
> since value would be first.

Yes! This is much better and more intuitive.

>
>> This generic nature also restricts these methods to being non-const,
>> unfortunately. I have tried to implement them as macros instead, but
>> quickly hit a wall due to the inability to convert `Alignment`'s `usize`
>> into the type of the value to align.
>
> I guess we could also just have one per type like for other ones to
> have them `const`, like we do for other similar things like
> `bit`/`genmask`.

This leaves us with two viable solutions: `Alignable` extension trait
with `align_up` and `align_down` operations that take an `Alignment` as
parameter (with the caveat that they could not be const for now), or a
set of per-type functions defined using a macro, similar to bit/genmask.
I am fine with both but don't know which one would be preferred, can the
R4L leadership provide some guidance? :)

Thanks,
Alex.