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

[Yu, Fei]

From: Fei Yu <fei.yu@xxxxxxxxx>

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.

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));

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.

Regards,
Fei