On 11/5/26 10:12, Tvrtko Ursulin wrote: > On 05/11/2026 08:18, Yu, Fei wrote: >> From: Fei Yu <[email protected]> >> On 04/11/2026 10:27, Tvrtko Ursulin wrote: >>> On 04/11/2026 09:44, Christian König wrote: >>>> On 11/4/26 04:09, Yu, Fei wrote: >>>>> +#define DRM_SYNCOBJ_MAX_HANDLES (INT_MAX / sizeof(struct >>>>> syncobj_wait_entry)) >>>> >>>> I really wonder if we should put that into the uAPI files? >>>> >>>> Not that userspace is potentially using it, but this is part of the uAPI >>>> and makes sense for documentation purposes. >> >> I thought about that, but I'd prefer to keep it kernel internal, at >> least for this fix: >> >> - The value is derived from the size of a kernel internal struct and >> from the kvmalloc() size limit, so it is not the same on 32-bit and >> 64-bit.
Oh that is a really good point, but most certainly a bad idea. This means that code working on 32bit then suddenly fails on 64bit, not something we should do. >> To put it in drm.h I would have to replace it with a fixed >> number, and that number becomes ABI. We could not easily raise or >> lower it later. I think I prefer a fixed number here. Something reasonable large enough, like 64k or similar, should do for backporting. >> >> - Tvrtko's series below avoids most of these allocations. Once that >> lands, the right limit may be different, and an internal define is >> easier to change. >> >> - The patch has a Fixes tag, and keeping it out of the uAPI header >> keeps the stable backport small. >> >> For documentation, I could send a follow-up that adds a sentence to the >> uAPI struct comments, saying that count_handles above an implementation >> defined limit is rejected with -EINVAL. That documents the behavior >> without fixing a number. Would that work for you? With Tvrko's patch to avoid most of the allocations in the future that approach sounds reasonable. But I would still use a fixed number and maybe a static assert that we don't hit the kvmalloc limit when we allocate that fixed number of structures. >> >>>> That kvmalloc_array() + the following copy_from_user should probably be >>>> replaced by using vmemdup_array_user(). But that should of course be a >>>> separate patch. >>> >>> Btw I tried to do that some time ago: >>> >>> https://lore.kernel.org/dri-devel/[email protected]/#t >>> >>> I have as local, more recent version which applies, but apparently never >>> sent it. >>> >>> Interesting part should also be the other patches from the above linked >>> series which avoid allocations altogether for the vast majority of cases. >>> >>> If there is interest I can re-base and re-send after this is merged. >> >> Thanks, I wasn't aware of that series. I'll leave the >> vmemdup_array_user() and points array cleanups to it, so we don't >> duplicate work. Happy to test it once this fix is in. > > Thanks - just to clarify though - I understand I wait for your patch to land > and then rebase my series, right? Right. This here is for -fixes, cleanups are for -next. Christian. > > Regards, > > Tvrtko >
