On 10/12/26 13:38, Julian Orth wrote: > On Mon, Oct 12, 2026 at 9:45 AM Christian König > <[email protected]> wrote: >> >> >> >> On 10/10/26 20:07, Julian Orth wrote: >>> If dma_fence_chain_alloc() fails, the function returns -ENOMEM without >>> dropping the references to the imported fence and the syncobj. >>> >>> Fixes: c2d3a7300695 ("drm/syncobj: Extend EXPORT_SYNC_FILE for timeline >>> syncobjs") >>> Assisted-by: Claude:claude-opus-5-5 >>> Signed-off-by: Julian Orth <[email protected]> >>> --- >>> drivers/gpu/drm/drm_syncobj.c | 10 +++++++--- >>> 1 file changed, 7 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c >>> index c23a5de27..5ca5163d0 100644 >>> --- a/drivers/gpu/drm/drm_syncobj.c >>> +++ b/drivers/gpu/drm/drm_syncobj.c >>> @@ -730,6 +730,7 @@ static int drm_syncobj_import_sync_file_fence(struct >>> drm_file *file_private, >>> { >>> struct dma_fence *fence = sync_file_get_fence(fd); >>> struct drm_syncobj *syncobj; >>> + int ret = 0; >> >> Please don't initialize return values on declaration that is often >> considered bad coding style. >> >>> >>> if (!fence) >>> return -EINVAL; >>> @@ -743,17 +744,20 @@ static int drm_syncobj_import_sync_file_fence(struct >>> drm_file *file_private, >>> if (point) { >> >>> struct dma_fence_chain *chain = dma_fence_chain_alloc(); >>> >>> - if (!chain) >>> - return -ENOMEM; >>> + if (!chain) { >>> + ret = -ENOMEM; >>> + goto out; >>> + } >>> >>> drm_syncobj_add_point(syncobj, chain, fence, point); >> >> Allocation and adding the point is intentionally separated so that >> dma_fence_chain_alloc() can be called before looking up the syncobj or >> previous fence. >> >> I suggest to make use of that. > > This is the fourth time that someone has tried to fix this trivial bug > for over a year.
I was already wondering as well if we haven't fixed that before. > The previous 3 cases were: > > https://lore.kernel.org/all/[email protected]/ > https://lore.kernel.org/all/[email protected]/ > https://lore.kernel.org/all/[email protected]/ > > The last of which didn't even receive a reply. And there have been > more cases in the past where I encountered a dri-devel-adjacent issue, > googled it, and found that patches had already been sent in years ago > to fix it, but were ignored. The problem here is clearly that nobody feels responsible for pushing that fix. Regards, Christian. > > Which is all to explain why I had all of the work for this series done by AI. > > I heard some chatter about dri-devel adopting a gitlab-based workflow, > and I hope that works out and I didn't misunderstand. Because the > current system is clearly broken. Nobody keeps track of which bugs > have been reported, nobody triages existing bugs, nobody keeps track > of attempts to fix bugs. Which is how 3 people ended up doing repeat > work of discovering, analyzing, and fixing the same bug. > > The same pattern applies to my patch two. I clearly don't know enough > about the (probably) undocumented semantics of where and when which > structure of fences can be used. Which is why I sent in a bug report > about this issue in May instead of trying to fix it on my own: > https://lore.kernel.org/all/CAHijbEUKuXNeSnT82P-ojOhzU5siNHjxUW+qEDOqx07Ys9m=x...@mail.gmail.com/ > > I would have hoped that a bug report amounting to "this new feature > causes the kernel to spam dmesg at 60 fps" would get some attention. > But no. The only way to get this issue fixed seems to be me acting as > a meat proxy to fix an issue that I know very little about. > > Thanks for reading my blog post. > > >> >> Regards, >> Christian. >> >>> } else { >>> drm_syncobj_replace_fence(syncobj, fence); >>> } >>> >>> +out: >>> dma_fence_put(fence); >>> drm_syncobj_put(syncobj); >>> - return 0; >>> + return ret; >>> } >>> >>> static int drm_syncobj_export_sync_file(struct drm_file *file_private, >>
