linux-kernel - Re: [RFC PATCH] xen/gntdev: reduce stack usage by dynamically allocating gntdev_copy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f8bde276-9d4e-47d0-9841-fd8724ef5275@suse.com>
Date: Thu, 3 Jul 2025 07:22:56 +0200
From: Jürgen Groß <jgross@...e.com>
To: Tu Dinh <ngoc-tu.dinh@...es.tech>, Abinash <abinashlalotra@...il.com>
Cc: sstabellini@...nel.org, oleksandr_tyshchenko@...m.com,
 xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org,
 Abinash Singh <abinashsinghlalotra@...il.com>
Subject: Re: [RFC PATCH] xen/gntdev: reduce stack usage by dynamically
 allocating gntdev_copy_batch

On 03.07.25 00:42, Tu Dinh wrote:
> On 01/07/2025 23:53, Abinash wrote:
>> Hi ,
>>
>> Thanks for pointing that out.
>>
>> I haven’t measured the performance impact yet — my main focus was on
>> getting rid of the stack usage warning triggered by LLVM due to
>> inlining. But you're right, gntdev_ioctl_grant_copy() is on a hot
>> path, and calling kmalloc() there could definitely slow things down,
>> especially under memory pressure.
>>
>> I’ll run some benchmarks to compare the current approach with the
>> dynamic allocation, and also look into alternatives — maybe
>> pre-allocating the struct or limiting inlining instead. If you have
>> any ideas or suggestions on how best to approach this, I’d be happy to
>> hear them.
>>
>> Do you have any suggestions on how to test the performance?
>>
>> Best,
>> Abinash
>>
>>
> 
> Preallocating may work but I'd be wary of synchronization if the
> preallocated struct is shared.
> 
> I'd look at optimizing status[] which should save quite a few bytes.
> 
> Reducing GNTDEV_COPY_BATCH could be a last resort, but that may also
> impact performance.

IMHO the most promising way would be to dynamically allocate the struct, but
don't free it at the end of the ioctl. Instead it could be put into a list
anchored in struct gntdev_priv, so freeing would be done only at close() time.

Synchronization would be minimal (just for taking a free struct from the list
or putting it back again), while memory usage would be basically just as needed,
depending on the number of concurrent threads using the same file descriptor
for the ioctl.

This approach would even allow to raise GNTDEV_COPY_BATCH, maybe resulting even
in a gain of performance.

I'll write a patch implementing the allocation scheme.


Juergen

Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3684 bytes)

Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (496 bytes)