On 11/6/26 06:35, Yu, Fei wrote: > From: Fei Yu <[email protected]> > > 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
