On 15.08.24 16:23, Danilo Krummrich wrote:
> On Thu, Aug 15, 2024 at 01:44:27PM +0000, Benno Lossin wrote:
>> On 15.08.24 14:29, Danilo Krummrich wrote:
>>> On Thu, Aug 15, 2024 at 06:48:19AM +0000, Benno Lossin wrote:
>>>> On 15.08.24 01:20, Danilo Krummrich wrote:
>>>>> On Thu, Aug 15, 2024 at 12:13:06AM +0200, Danilo Krummrich wrote:
>>>>>>> How difficult will it be to support this? (it is a weird requirement,
>>>>>>> but I dislike just returning an error...)
>>>>>>
>>>>>> It's not difficult to support at all. But it requires a C API taking an
>>>>>> alignment argument (same for `KVmalloc`).
>>>>
>>>> I see, that's good to know.
>>>>
>>>>>> Coming up with a vrealloc_aligned() is rather trivial. kvrealloc_aligned() would
>>>>>> be a bit weird though, because the alignment argument could only be really
>>>>>> honored if we run into the vrealloc() case. For the krealloc() case it'd still
>>>>>> depend on the bucket size that is selected for the requested size.
>>>>
>>>> Yeah... Maybe some more logic on the Rust side can help with that.
>>>
>>> Only if we reimplement `KVmalloc` in Rust, However, there are quite some special
>>> cases in __kvmalloc_node_noprof(), i.e. fixup page flags, sanity check the size
>>> on kmalloc failure, fail on certain page flags, etc.
>>>
>>> I don't really want to duplicate this code, unless we absolutely have to.
>>
>> I am under the (probably wrong) impression that kvmalloc has some size
>> check and selects vmalloc or kmalloc depending on that.
>
> Basically, yes. But as mentioned above, there are quite some corner cases [1].
>
>> I think that we
>> could check the size and if it is going to allocate via kmalloc, then we
>> adjust the size for alignment as usual
>
> We don't need this adjustment any longer, see commit ad59baa31695 ("slab, rust:
> extend kmalloc() alignment guarantees to remove Rust padding").
>
>> and if it is going to select
>> vmalloc, then we can just pass the alignment (if the vmalloc alignment
>> patch is done first).
>
> Yeah, but as mentioned, I'd prefer to do this in C, such that we don't need to
> open code everything the C code already does.
>
> [1] https://elixir.bootlin.com/linux/v6.11-rc3/source/mm/util.c#L628
I see, then it's probably better to just add an align parameter variant
on the C side. Instead of rebuilding it in Rust.
---
Cheers,
Benno
