On Tue, Mar 17, 2026 at 09:58:19AM +0900, Alexandre Courbot wrote:
> On Tue Mar 17, 2026 at 3:43 AM JST, Joel Fernandes wrote:
> <snip>
> >>> +//! ptr::Alignment,
> >>> +//! sizes::*, //
> >>> +//! };
> >>> +//!
> >>> +//! // Create a 1GB buddy allocator with 4KB minimum chunk size.
> >>> +//! let buddy = GpuBuddy::new(GpuBuddyParams {
> >>> +//! base_offset: 0,
> >>> +//! physical_memory_size: SZ_1G as u64,
> >>> +//! chunk_size: SZ_4K,
> >>
> >> `chunk_size` is an interesting case. The C API uses a `u64`, but I think
> >> we can reasonably consider that we won't ever need chunks larger than
> >> 4GB (or can we :O). I'm actually ok with using a `usize` for this one.
> >>
> >> One of the first things the C code does is throwing an error if it is
> >> not a power of 2, so maybe we can even request an `Alignment`?
> >>
> >> I'm a bit torn as to whether we should use a `u64` to conform with the C
> >> API, but doing so would mean we cannot use an `Alignment`...
> >
> > I prefer to keep it simple and use `usize` for now. I cannot imagine
> > chunk_size ever exceeding 4GB, and given our stance on rejecting invalid
> > inputs, this sounds reasonable. Regarding `Alignment`, I still prefer
> > `usize` here since it makes the caller-side simpler and as you noted the
> > C code already does error-checking. Let's revisit if needed once this
> > lands.
>
> I would like to insist a bit here re: Alignment. We are not trying to
> make the caller side simpler - we are trying to make it correct and to
> turn runtime failures into build-time ones as much as possible. This is
> a good case for that.
>
> The additional burden, if you can call it so, to the caller is just in
> the initial call to `GpuBuddy::new` - i.e. typically once per driver.
> The most important API, `alloc_blocks`, will be unaffected - and
> actually this one already has one `Alignment` as a parameter, for the
> minimal block size! So if anything it would be illogical not to follow
> suit on the buddy's `block_size` parameter.
Right, I was on the fence about it, I changed it one place but not the other.
After our recent discussion, I will change it all to Alignment considering
robustness argument, sounds good :)
>
> <snip>
> >>> +//! let (mut count, mut total) = (0u32, 0usize);
> >>> +//! for block in fragmented.iter() {
> >>> +//! assert_eq!(block.size(), SZ_4M);
> >>> +//! total += block.size();
> >>> +//! count += 1;
> >>> +//! }
> >>
> >> Note that we can avoid mutable variables with this:
> >>
> >> //! let total_size: usize = fragmented.iter()
> >> //! .inspect(|block| assert_eq!(block.size(), SZ_4M))
> >> //! .map(|block| block.size())
> >> //! .sum();
> >> //! assert_eq!(total_size, SZ_8M);
> >> //! assert_eq!(fragmented.iter().count(), 2);
> >>
> >> But your call as to whether this is an improvement.
> >
> > I feel the current for-loop version is slightly more readable,
> > especially in a doc example aimed at new users, so I'd like to keep
> > it as-is.
>
> Sounds good.
>
> <snip>
> >> For this parameter I am pretty sure we want to conform to the C API and
> >> use a `u64` - there is no benefit in not doing so, and buffers larger
> >> than 4GB *are* a reality nowadays, (maybe not for graphics, but this
> >> will also be used in compute scenarios).
> >
> > Agreed. Though, note this adds 7 more `as` usages, but I guess there's
> > nothing we can do till the IntoSafe stuff is moved to core rust, I think.
>
> How so? This parameter is just passed to the C function.
>
> If you are referring to the examples, then yes that's unfortunate but
> there are at least two ways where this could be eventually fixed (John's
> SZ_* rework and the IntoSafe stuff), so we can update these when either
> lands.
>
Yes, referring to the examples. Yes we'll need to adapt it later. For now
I'll leave it with the 'as' usage.
--
Joel Fernandes
