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, AbinashPreallocating 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
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
