On Mon, 2026-05-18 at 19:20 -0400, Vineeth Remanan Pillai wrote: > On Mon, May 18, 2026 at 11:01 AM Philipp Stanner <[email protected]> wrote: > > > > On Fri, 2026-05-15 at 09:59 -0400, Vineeth Pillai (Google) wrote: > > > From: Vineeth Pillai <[email protected]> > > > > > > Replace trace_foo() with the new trace_call__foo() at sites already > > > guarded by trace_foo_enabled(), avoiding a redundant > > > static_branch_unlikely() re-evaluation inside the tracepoint. > > > trace_call__foo() calls the tracepoint callbacks directly without > > > utilizing the static branch again. > > > > The "foo" terminology is unusual I think? I always wrote it with regex, > > like "trace_*()". > > > Sorry about the terminology. Part of the patches got merged this way, > so is it okay to continue the terminology to have consistency?
Sure, no big deal. > > > > > > > > > > > Original v2 series: > > > https://lore.kernel.org/linux-trace-kernel/[email protected]/ > > > > I'd put this in a Link: tag section below. > > > Makes sense, will do. Steve also suggested to put this whole section > after "---" because it isn't relevant to the changes. Will fix this in > next iteration. I agree with Steve. We have had some tendency in the past to have all sorts of versioning information in the git log, which IMO is not useful to anyone a few months after merging. The commit message should detail the why, how and what, not the history. > > > > > > > Parts of the original v2 series have already been merged in mainline. > > > This patch is being reposted as a follow-up cleanup for the remaining > > > unmerged pieces. > > > > So this v3 series as a whole is a followup to that v2? > > > v3 is a follow up to remaining patches that were not merged with the > previous cycle. The core api and couple of patches went in the > previous cycle, so this is for rest of it. > > The intention was to send this v3 as a direct patch to individual > subsystem maintainers but forgot to remove the numbering and hence > there might be a confusion. Will remove the numbering and send it as > stand alone patch in the next iteration. OK, cool. Thanks, Philipp > > > > > > > Suggested-by: Steven Rostedt <[email protected]> > > > Suggested-by: Peter Zijlstra <[email protected]> > > > Signed-off-by: Vineeth Pillai (Google) <[email protected]> > > > Assisted-by: Claude:claude-sonnet-4-6 > > > --- > > > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +- > > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 ++-- > > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 +++++----- > > > drivers/gpu/drm/scheduler/sched_entity.c | 5 +++-- > > > 4 files changed, 11 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > > > index b24d5d21be5f..cb0b5cb07d57 100644 > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > > > @@ -1004,7 +1004,7 @@ static void trace_amdgpu_cs_ibs(struct > > > amdgpu_cs_parser *p) > > > struct amdgpu_job *job = p->jobs[i]; > > > > > > for (j = 0; j < job->num_ibs; ++j) > > > - trace_amdgpu_cs(p, job, &job->ibs[j]); > > > + trace_call__amdgpu_cs(p, job, &job->ibs[j]); > > > } > > > } > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > > index 9ba9de16a27a..a36ae94c425f 100644 > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > > @@ -1415,7 +1415,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, > > > struct amdgpu_bo_va *bo_va, > > > > > > if (trace_amdgpu_vm_bo_mapping_enabled()) { > > > list_for_each_entry(mapping, &bo_va->valids, list) > > > - trace_amdgpu_vm_bo_mapping(mapping); > > > + trace_call__amdgpu_vm_bo_mapping(mapping); > > > } > > > > > > error_free: > > > @@ -2183,7 +2183,7 @@ void amdgpu_vm_bo_trace_cs(struct amdgpu_vm *vm, > > > struct ww_acquire_ctx *ticket) > > > continue; > > > } > > > > > > - trace_amdgpu_vm_bo_cs(mapping); > > > + trace_call__amdgpu_vm_bo_cs(mapping); > > > } > > > } > > > > > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > index 5fc5d5608506..fbdc12cdd6bb 100644 > > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > @@ -5263,11 +5263,11 @@ static void amdgpu_dm_backlight_set_level(struct > > > amdgpu_display_manager *dm, > > > } > > > > > > if (trace_amdgpu_dm_brightness_enabled()) { > > > - trace_amdgpu_dm_brightness(__builtin_return_address(0), > > > - user_brightness, > > > - brightness, > > > - caps->aux_support, > > > - > > > power_supply_is_system_supplied() > 0); > > > + > > > trace_call__amdgpu_dm_brightness(__builtin_return_address(0), > > > + user_brightness, > > > + brightness, > > > + caps->aux_support, > > > + > > > power_supply_is_system_supplied() > 0); > > > } > > > > > > if (caps->aux_support) { > > > diff --git a/drivers/gpu/drm/scheduler/sched_entity.c > > > b/drivers/gpu/drm/scheduler/sched_entity.c > > > index fe174a4857be..185a2636b599 100644 > > > --- a/drivers/gpu/drm/scheduler/sched_entity.c > > > +++ b/drivers/gpu/drm/scheduler/sched_entity.c > > > @@ -429,7 +429,8 @@ static bool drm_sched_entity_add_dependency_cb(struct > > > drm_sched_entity *entity, > > > > > > if (trace_drm_sched_job_unschedulable_enabled() && > > > !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, > > > &entity->dependency->flags)) > > > - trace_drm_sched_job_unschedulable(sched_job, > > > entity->dependency); > > > + trace_call__drm_sched_job_unschedulable(sched_job, > > > + entity->dependency); > > > > I would be more happy if you sacrifice a bit of space here and keep it > > a single line since the if condition is already quite convoluted and > > challenging to read. > > > I understand, will fix it in next iteration. > > Thanks, > Vineeth
