[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b402252a-91de-4983-abc1-65f78e7e6ae7@igalia.com>
Date: Wed, 9 Apr 2025 16:49:45 -0300
From: Maíra Canal <mcanal@...lia.com>
To: Dan Carpenter <dan.carpenter@...aro.org>, oe-kbuild@...ts.linux.dev,
Christian König <christian.koenig@....com>
Cc: lkp@...el.com, oe-kbuild-all@...ts.linux.dev,
linux-kernel@...r.kernel.org, Melissa Wen <mwen@...lia.com>
Subject: Re: drivers/gpu/drm/vc4/vc4_gem.c:604 vc4_lock_bo_reservations()
error: uninitialized symbol 'ret'.
+ König
Hi Dan,
On 02/04/25 05:43, Dan Carpenter wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: acc4d5ff0b61eb1715c498b6536c38c1feb7f3c1
> commit: 04630796c437a9285643097825cbd3cd06603f47 drm/vc4: Use DRM Execution Contexts
> date: 2 months ago
> config: arm64-randconfig-r073-20250402 (https://download.01.org/0day-ci/archive/20250402/202504021500.3AM1hKKS-lkp@intel.com/config)
> compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@...el.com>
> | Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
> | Closes: https://lore.kernel.org/r/202504021500.3AM1hKKS-lkp@intel.com/
>
> smatch warnings:
> drivers/gpu/drm/vc4/vc4_gem.c:604 vc4_lock_bo_reservations() error: uninitialized symbol 'ret'.
>
> vim +/ret +604 drivers/gpu/drm/vc4/vc4_gem.c
>
> cdec4d3613230f Eric Anholt 2017-04-12 589 static int
> 04630796c437a9 Maíra Canal 2024-12-20 590 vc4_lock_bo_reservations(struct vc4_exec_info *exec,
> 04630796c437a9 Maíra Canal 2024-12-20 591 struct drm_exec *exec_ctx)
> cdec4d3613230f Eric Anholt 2017-04-12 592 {
> 04630796c437a9 Maíra Canal 2024-12-20 593 int ret;
> cdec4d3613230f Eric Anholt 2017-04-12 594
> cdec4d3613230f Eric Anholt 2017-04-12 595 /* Reserve space for our shared (read-only) fence references,
> cdec4d3613230f Eric Anholt 2017-04-12 596 * before we commit the CL to the hardware.
> cdec4d3613230f Eric Anholt 2017-04-12 597 */
> 04630796c437a9 Maíra Canal 2024-12-20 598 drm_exec_init(exec_ctx, DRM_EXEC_INTERRUPTIBLE_WAIT, exec->bo_count);
> 04630796c437a9 Maíra Canal 2024-12-20 599 drm_exec_until_all_locked(exec_ctx) {
> 04630796c437a9 Maíra Canal 2024-12-20 600 ret = drm_exec_prepare_array(exec_ctx, exec->bo,
> 04630796c437a9 Maíra Canal 2024-12-20 601 exec->bo_count, 1);
>
> This is a false positive in Smatch. I can silence the warning on my
> end easily enough to say that we always enter the drm_exec_until_all_locked()
> loop. But the question is why do we only test the last "ret" instead of
> testing all of them?
AFAIU `drm_exec_until_all_locked` will loop until all GEM objects are
locked and no more contention exists. As we have a single operation
inside the loop, we don't need to check "ret" for every iteration.
I believe Christian will possibly give you a more precise answer as he
designed the API.
Best Regards,
- Maíra
>
> 04630796c437a9 Maíra Canal 2024-12-20 602 }
> cdec4d3613230f Eric Anholt 2017-04-12 603
> cdec4d3613230f Eric Anholt 2017-04-12 @604 if (ret) {
> 04630796c437a9 Maíra Canal 2024-12-20 605 drm_exec_fini(exec_ctx);
> cdec4d3613230f Eric Anholt 2017-04-12 606 return ret;
> 7edabee06a5622 Eric Anholt 2016-09-27 607 }
> d5b1a78a772f1e Eric Anholt 2015-11-30 608
> cdec4d3613230f Eric Anholt 2017-04-12 609 return 0;
> cdec4d3613230f Eric Anholt 2017-04-12 610 }
>
Powered by blists - more mailing lists