On 3/16/26 08:04, SHANMUGAM, SRINIVASAN wrote: > [AMD Official Use Only - AMD Internal Distribution Only] > >> -----Original Message----- >> From: Lazar, Lijo <[email protected]> >> Sent: Monday, March 16, 2026 12:19 PM >> To: SHANMUGAM, SRINIVASAN <[email protected]>; >> Koenig, Christian <[email protected]>; Deucher, Alexander >> <[email protected]> >> Cc: [email protected]; Dan Carpenter <[email protected]>; >> Kuehling, Felix <[email protected]> >> Subject: Re: [PATCH] drm/amdgpu: Make emit_vm_flush() check explicit in >> amdgpu_vm_flush() >> >> >> >> On 15-Mar-26 12:25 PM, Srinivasan Shanmugam wrote: >>> amdgpu_vm_flush() sends commands to the GPU to update the VM page >>> tables for a job. >>> >>> When a job uses a GPU virtual address space, the GPU needs to refresh >>> its address translations after the driver updates the page tables. >>> A VM flush tells the GPU to forget old address translations and use >>> the updated page table mappings. >>> >>> This flush command is not supported on all rings. Only rings that >>> implement the emit_vm_flush() callback know how to emit the correct >>> hardware command for this operation. >>> >>> The function already gates vm_flush_needed on the presence of >>> ring->funcs->emit_vm_flush earlier in the logic. However, static >>> analysis tools such as Smatch may not track this relationship through >>> the vm_flush_needed boolean and warn that emit_vm_flush() could be >>> NULL when the VM flush command is emitted later. >>> >>> Fixes the below: >>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:826 amdgpu_vm_flush() error: we >>> previously assumed 'ring->funcs->emit_vm_flush' could be null (see >>> line 788) >>> >> >> I think the code logic is correct and the check is implicit in >> vm_flush_needed. We >> don't need to write code to eliminate 'false warnings'. Moreover, it is >> inappropriate to >> use a Fixes tag for this. > > > My initial thought was that the earlier gating of vm_flush_needed on > ring->funcs->emit_vm_flush would make the later call safe, but Smatch > still reports the following warning: > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:826 amdgpu_vm_flush() > error: we previously assumed 'ring->funcs->emit_vm_flush' could be null
Yeah that looks like smatch is either not analyzing the code correctly or we are missing something. Regards, Christian. > > Dan, could you please confirm whether this is expected to be a false > positive from Smatch due to the vm_flush_needed boolean, or if there > is a recommended pattern to help Smatch track this relationship? > > Best regards, > Srini > >> >> Thanks, >> Lijo
