Re: [PATCH v2] drm/syncobj: Limit the number of handles and use kvmalloc for user-sized arrays

["Christian König" <christian.koenig@amd.com>]

On 11/6/26 06:35, Yu, Fei wrote:
> From: Fei Yu <fei.yu@intel.com>
> 
> On 11/5/26, Christian König wrote:
>> On 11/5/26 10:12, Tvrtko Ursulin wrote:
>>> On 05/11/2026 08:18, Yu, Fei wrote:
>>>>  - 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.
> 
> Agreed, a fixed number is better.
> 
>> I think I prefer a fixed number here. Something reasonable large enough,
>> like 64k or similar, should do for backporting.
> 
> One concern with 64K: today only wait and timeline_wait break above
> ~64K handles, because of the 64-byte wait entries. The other ioctls
> only allocate 4-8 bytes per handle, so on x86_64 reset, signal, query
> and timeline_signal currently work up to ~512K handles. In my v2
> testing, reset, signal and timeline_signal on 200000 handles succeed on
> the base kernel. With a 64K limit they would start returning -EINVAL
> after this fix, which I would rather avoid in a stable backport.
> 
> So I would suggest SZ_1M instead:
> 
>  - it is above what any of these ioctls can handle today, so nothing
>    that works now starts failing;
>  - 1M wait entries are 64 MiB, far below the kvmalloc() INT_MAX limit;
>  - it is the same on 32-bit and 64-bit.

Work for me and should really cover everything ever needed here.

> 
> If you still prefer 64K, I'll go with that.
> 
>> 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.
> 
> Will do. Something like:
> 
>   #define DRM_SYNCOBJ_MAX_HANDLES	SZ_1M
>   static_assert(DRM_SYNCOBJ_MAX_HANDLES <=
>                 INT_MAX / sizeof(struct syncobj_wait_entry));

Looks good but I have no idea if that really works or not :)

> Plan for v3, for -fixes:
> 
>  - fixed DRM_SYNCOBJ_MAX_HANDLES plus the static_assert, define kept in
>    drm_syncobj.c;
>  - keep the kvmalloc() conversion;
>  - update the commit message accordingly.
> 
> The uAPI documentation of the limit will be a separate patch for -next
> once this has landed.

Yeah work for me as well.

Thanks,
Christian.

> 
> Regards,
> Fei