On Wed, Aug 27, 2025 at 8:57 AM Suren Baghdasaryan [off-list ref] wrote:

On Fri, Aug 22, 2025 at 6:51 PM Xuewen Yan [off-list ref] wrote:

quoted

Hi Suren,

Thanks for your review:)

On Thu, Aug 21, 2025 at 3:51 AM Suren Baghdasaryan [off-list ref] wrote:

quoted

On Wed, Aug 20, 2025 at 4:28 AM Xuewen Yan [off-list ref] wrote:

quoted

Add trace point to psi triggers. This is useful to
observe the psi events in the kernel space.

One use of this is to monitor memory pressure.
When the pressure is too high, we can kill the process
in the kernel space to prevent OOM.

Signed-off-by: Xuewen Yan <redacted>
---
v2:
-fix compilation error;
-export the tp;
-add more commit message;
---
 include/trace/events/sched.h | 5 +++++
 kernel/sched/psi.c           | 4 ++++
 2 files changed, 9 insertions(+)

diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
index 7b2645b50e78..d54db5fcbca2 100644
--- a/include/trace/events/sched.h
+++ b/include/trace/events/sched.h

@@ -896,6 +896,11 @@ DECLARE_TRACE(sched_set_need_resched,
        TP_PROTO(struct task_struct *tsk, int cpu, int tif),
        TP_ARGS(tsk, cpu, tif));

+struct psi_trigger;
+DECLARE_TRACE(psi_event,

DECLARE_TRACE will create a tracepoint but will not export it in the
tracefs. Why should we not have it in the tracefs?

I haven't figured out what content should be displayed in the trace yet.
Until this is fully determined, I think it might be a better option to
just export the tracepoint and let users add their own hooks to print
the content they need.

You can report what you think makes sense today for this tracepoint.
Tracepoints can be enhanced later if needed since their format is
exported to the userspace and well-designed userspace parsers should
be able to parse new fields.

Okay, I would change this to be tracepoint.

quoted

quoted

quoted

+       TP_PROTO(struct psi_trigger *t),
+       TP_ARGS(t));
+
 #endif /* _TRACE_SCHED_H */

 /* This part must be outside protection */

diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
index 59fdb7ebbf22..f06eb91a1250 100644
--- a/kernel/sched/psi.c
+++ b/kernel/sched/psi.c

@@ -141,6 +141,8 @@
 #include <linux/psi.h>
 #include "sched.h"

+EXPORT_TRACEPOINT_SYMBOL_GPL(psi_event_tp);

So, are you planning to attach some handler to this trace event in your driver?

Yes, our modules would attach a handler to observe the memory pressure.

quoted

quoted

+
 static int psi_bug __read_mostly;

 DEFINE_STATIC_KEY_FALSE(psi_disabled);

@@ -509,6 +511,8 @@ static void update_triggers(struct psi_group *group, u64 now,
                if (now < t->last_event_time + t->win.size)
                        continue;

+               trace_psi_event_tp(t);

This should only be done if the below cmpxchg() check is true, right?
Otherwise it will not match with what userspace is receiving.

If we put it below cmpxchg() check, we may lose some event before the
user space repose the signal.
Because the t->event needs to be set to 0 again.
In order to ensure that all events are displayed, it is better to put
it before the cmpxchg.

Huh? Are you modifying the t->event inside your handler?! If so, that
is unacceptable and I will NAK this change. A driver should not
interfere with core kernel mechanisms.

Sorry for missing this, the next patch, I would export some variables
instead of the pointer of t...

Thanks!

quoted

Thanks!

quoted

quoted

+
                /* Generate an event */
                if (cmpxchg(&t->event, 0, 1) == 0) {
                        if (t->of)
--
2.25.1