mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: K Prateek Nayak <kprateek.nayak@amd.com>
To: Nam Cao <namcao@linutronix.de>, Gabriele Monaco <gmonaco@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	<linux-trace-kernel@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>
Subject: Re: [PATCH 4/5] sched: Add rt task enqueue/dequeue trace points
Date: Fri, 1 Aug 2025 09:12:08 +0530	[thread overview]
Message-ID: <179674c6-f82a-4718-ace2-67b5e672fdee@amd.com> (raw)
In-Reply-To: <20250731073520.ktIOaGts@linutronix.de>

On 7/31/2025 1:05 PM, Nam Cao wrote:
> Something like the untested patch below?
> 
> Will you have a use case for it too? Then I will try to accommodate your
> use case, otherwise I will do just enough for my case.
> 
> Nam
> 
> diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
> index c38f12f7f903..b50668052f99 100644
> --- a/include/trace/events/sched.h
> +++ b/include/trace/events/sched.h
> @@ -906,6 +906,14 @@ DECLARE_TRACE(dequeue_task_rt,
>  	TP_PROTO(int cpu, struct task_struct *task),
>  	TP_ARGS(cpu, task));
>  
> +DECLARE_TRACE(enqueue_task,
> +	TP_PROTO(int cpu, struct task_struct *task),
> +	TP_ARGS(cpu, task));
> +
> +DECLARE_TRACE(dequeue_task,
> +	TP_PROTO(int cpu, struct task_struct *task),
> +	TP_ARGS(cpu, task));
> +
>  #endif /* _TRACE_SCHED_H */
>  
>  /* This part must be outside protection */
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index b485e0639616..2af90532982a 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2077,6 +2077,8 @@ unsigned long get_wchan(struct task_struct *p)
>  
>  void enqueue_task(struct rq *rq, struct task_struct *p, int flags)
>  {
> +	trace_enqueue_task_tp(rq->cpu, p);
> +
>  	if (!(flags & ENQUEUE_NOCLOCK))
>  		update_rq_clock(rq);
>  
> @@ -2103,6 +2105,8 @@ void enqueue_task(struct rq *rq, struct task_struct *p, int flags)
>   */
>  inline bool dequeue_task(struct rq *rq, struct task_struct *p, int flags)
>  {
> +	trace_dequeue_task_tp(rq->cpu, p);

Just thinking out loud, putting this tracepoint here can lead to a
"dequeued -> dequeued" transition for fair task when they are in delayed
dequeue state.

    dequeue_task(p)
      trace_dequeue_task_tp(p) # First time
      dequeue_task_fair(p)
        p->se.delayed = 1
    ...
    <sched_switch> # p is still delayed
    ...
    sched_setscheduler(p)
      if (prev_class != next_class && p->se.sched_delayed)
        dequeue_task(p, DEQUEUE_DELAYED);
          trace_dequeue_task_tp(p) # Second time

It is not an issue as such but it might come as a surprise if users are
expecting a behavior like below which would be the case for !fair task
currently (and for all tasks before v6.12):

    digraph state_automaton {
        center = true;
        size = "7,11";
        {node [shape = plaintext, style=invis, label=""] "__init_enqueue_dequeue_cycle"};
        {node [shape = ellipse] "enqueued"};
        {node [shape = ellipse] "dequeued"};
        "__init_enqueue_dequeue_cycle" -> "enqueued";
        "__init_enqueue_dequeue_cycle" -> "dequeued";
        "enqueued" [label = "enqueued", color = green3];
        "enqueued" -> "dequeued" [ label = "dequeue_task" ];
        "dequeued" [label = "dequeued", color = red];
        "dequeued" -> "enqueued" [ label = "enqueue_task" ];
        { rank = min ;
            "__init_enqueue_dequeue_cycle";
            "dequeued";
            "enqueued";
        }
    }


Another:

   "dequeued" -> "dequeued" [ label = "dequeue_task" ];

edge would be needed in that case for >= v6.12. It is probably nothing
and can be easily handled by the users if they run into it but just
putting it out there for the record in case you only want to consider a
complete dequeue as "dequeued". Feel free to ignore since I'm completely
out of my depth when it comes to the usage of RV in the field :)

> +
>  	if (sched_core_enabled(rq))
>  		sched_core_dequeue(rq, p, flags);
>  
> 

-- 
Thanks and Regards,
Prateek


  parent reply	other threads:[~2025-08-01  3:42 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-30 12:45 [PATCH 0/5] rv: LTL per-cpu monitor type and real-time scheduling monitor Nam Cao
2025-07-30 12:45 ` [PATCH 1/5] rv/ltl: Prepare for other monitor types Nam Cao
2025-07-31  9:04   ` Gabriele Monaco
2025-07-31  9:28     ` Nam Cao
2025-07-31 10:14       ` Gabriele Monaco
2025-07-30 12:45 ` [PATCH 2/5] rv/ltl: Support per-cpu monitors Nam Cao
2025-07-31  8:02   ` Gabriele Monaco
2025-08-01  6:26     ` Nam Cao
2025-07-30 12:45 ` [PATCH 3/5] verification/rvgen/ltl: Support per-cpu monitor generation Nam Cao
2025-07-30 12:45 ` [PATCH 4/5] sched: Add rt task enqueue/dequeue trace points Nam Cao
2025-07-30 13:53   ` Gabriele Monaco
2025-07-30 15:18     ` Nam Cao
2025-07-30 16:18       ` Gabriele Monaco
2025-07-31  7:35         ` Nam Cao
2025-07-31  8:39           ` Gabriele Monaco
2025-08-01  3:42           ` K Prateek Nayak [this message]
2025-08-01  7:29             ` Nam Cao
2025-08-01  9:56               ` K Prateek Nayak
2025-08-01 11:04                 ` Gabriele Monaco
2025-08-04  3:07                   ` K Prateek Nayak
2025-08-04  5:49                     ` Nam Cao
2025-07-30 12:45 ` [PATCH 5/5] rv: Add rts monitor Nam Cao
2025-07-31  7:47   ` Gabriele Monaco
2025-08-01  7:58     ` Nam Cao
2025-08-01  9:14       ` Gabriele Monaco
2025-08-04  6:05         ` Nam Cao
2025-08-05  8:40   ` Gabriele Monaco
2025-08-05 12:22     ` Nam Cao
2025-08-05 15:45       ` Nam Cao
2025-08-06  8:15         ` Gabriele Monaco
2025-08-06  8:46           ` Nam Cao
2025-08-06  9:03             ` Gabriele Monaco

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=179674c6-f82a-4718-ace2-67b5e672fdee@amd.com \
    --to=kprateek.nayak@amd.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=gmonaco@redhat.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mgorman@suse.de \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=namcao@linutronix.de \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®