Re: [PATCH v2 2/2] mm: vmscan: add PIDs to vmscan tracepoints

[Thomas Ballasi <tballasi@linux.microsoft.com>]

On Tue, Dec 16, 2025 at 01:03:02PM -0500, Steven Rostedt wrote:
> On Tue, 16 Dec 2025 06:02:52 -0800
> Thomas Ballasi <tballasi@linux.microsoft.com> wrote:
> 
> > The changes aims at adding additionnal tracepoints variables to help
> > debuggers attribute them to specific processes.
> > 
> > The PID field uses in_task() to reliably detect when we're in process
> > context and can safely access current->pid.  When not in process
> > context (such as in interrupt or in an asynchronous RCU context), the
> > field is set to -1 as a sentinel value.
> > 
> > Signed-off-by: Thomas Ballasi <tballasi@linux.microsoft.com>
> 
> Is this really needed? The trace events already show if you are in
> interrupt context or not.
> 
> # tracer: nop
> #
> # entries-in-buffer/entries-written: 25817/25817   #P:8
> #
> #                                _-----=> irqs-off/BH-disabled
> #                               / _----=> need-resched
> #                              | / _---=> hardirq/softirq   <<<<------ Shows irq context
> #                              || / _--=> preempt-depth
> #                              ||| / _-=> migrate-disable
> #                              |||| /     delay
> #           TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
> #              | |         |   |||||     |         |
>           <idle>-0       [002] d..1. 11429.293552: rcu_watching: Startirq 0 1 0x74c
>           <idle>-0       [000] d.H1. 11429.293564: rcu_utilization: Start scheduler-tick
>           <idle>-0       [000] d.H1. 11429.293566: rcu_utilization: End scheduler-tick
>           <idle>-0       [002] dN.1. 11429.293567: rcu_watching: Endirq 1 0 0x74c
>           <idle>-0       [002] dN.1. 11429.293568: rcu_watching: Start 0 1 0x754
>           <idle>-0       [000] d.s1. 11429.293577: rcu_watching: --= 3 1 0xdf4
>           <idle>-0       [002] dN.1. 11429.293579: rcu_utilization: Start context switch
>           <idle>-0       [002] dN.1. 11429.293580: rcu_utilization: End context switch
>        rcu_sched-15      [002] d..1. 11429.293589: rcu_grace_period: rcu_sched 132685 start
>           <idle>-0       [000] dN.1. 11429.293592: rcu_watching: Endirq 1 0 0xdf4
>        rcu_sched-15      [002] d..1. 11429.293592: rcu_grace_period: rcu_sched 132685 cpustart
>        rcu_sched-15      [002] d..1. 11429.293592: rcu_grace_period_init: rcu_sched 132685 0 0 7 ff
>           <idle>-0       [000] dN.1. 11429.293593: rcu_watching: Start 0 1 0xdfc
> 
> Thus, you can already tell if you are in interrupt context or not, and you
> always get the current pid. The 'H', 'h' or 's' means you are in a
> interrupt type context. ('H' for hard interrupt interrupting a softirq, 'h'
> for just a hard interrupt, and 's' for a softirq).
> 
> What's the point of adding another field to cover the same information
> that's already available?
> 
> -- Steve

(re-sending the reply as I believe I missed the reply all)

It indeed shows whether or not we're in an IRQ, but I believe the
kernel shouldn't show erronous debugging values. Even though it can be
obvious that we're in an interrupt, some people might look directly at
the garbage PID value without having second thoughts and taking it for
granted. On the other hand, it takes just a small check to mark the
debugging information as clearly invalid, which complements the IRQ
context flag.

If we shouldn't put that check there, I'd happily remove it, but I'd
tend to think it's a trivial addition that can only be for the best.

Thomas