From: Aaron Lu <ziqianlu@bytedance.com>
To: Hao Jia <jiahao.kernel@gmail.com>
Cc: mingo@redhat.com, peterz@infradead.org, mingo@kernel.org,
juri.lelli@redhat.com, vincent.guittot@linaro.org,
dietmar.eggemann@arm.com, rostedt@goodmis.org,
bsegall@google.com, mgorman@suse.de, vschneid@redhat.com,
kprateek.nayak@amd.com, linux-kernel@vger.kernel.org,
Hao Jia <jiahao1@lixiang.com>
Subject: Re: [PATCH v2] sched/fair: Fix non-empty throttled_limbo_list warning in tg_throttle_down()
Date: Mon, 27 Oct 2025 20:02:11 +0800 [thread overview]
Message-ID: <20251027120211.GB33@bytedance> (raw)
In-Reply-To: <20251027090534.94429-1-jiahao.kernel@gmail.com>
On Mon, Oct 27, 2025 at 05:05:34PM +0800, Hao Jia wrote:
> @@ -6403,7 +6407,7 @@ static void do_sched_cfs_slack_timer(struct cfs_bandwidth *cfs_b)
> * expired/exceeded, otherwise it may be allowed to steal additional ticks of
> * runtime as update_curr() throttling can not trigger until it's on-rq.
> */
> -static void check_enqueue_throttle(struct cfs_rq *cfs_rq)
> +static void check_enqueue_throttle(struct cfs_rq *cfs_rq, int flags)
> {
> if (!cfs_bandwidth_used())
> return;
> @@ -6418,6 +6422,13 @@ static void check_enqueue_throttle(struct cfs_rq *cfs_rq)
>
> /* update runtime allocation */
> account_cfs_rq_runtime(cfs_rq, 0);
> + /*
> + * Do not attempt to throttle on the cfs_rq unthrottle path.
> + * and it must be placed after account_cfs_rq_runtime() to
> + * prevent a possible missed start of the bandwidth timer.
Hi Prateek and Hao,
Does it matter to start the bw timer? If no cfs_rq gets throttled, the
timer doesn't look that useful. Also, account_cfs_rq_runtime() calls
assign_cfs_rq_runtime() and if assign failed, it will do resched_curr()
but since we do not throttle cfs_rq here, that resched would be useless.
> + */
> + if (flags & ENQUEUE_THROTTLE)
> + return;
> if (cfs_rq->runtime_remaining <= 0)
> throttle_cfs_rq(cfs_rq);
> }
> @@ -6724,7 +6735,7 @@ static void sched_fair_update_stop_tick(struct rq *rq, struct task_struct *p)
>
> static void account_cfs_rq_runtime(struct cfs_rq *cfs_rq, u64 delta_exec) {}
> static bool check_cfs_rq_runtime(struct cfs_rq *cfs_rq) { return false; }
> -static void check_enqueue_throttle(struct cfs_rq *cfs_rq) {}
> +static void check_enqueue_throttle(struct cfs_rq *cfs_rq, int flags) {}
> static inline void sync_throttle(struct task_group *tg, int cpu) {}
> static __always_inline void return_cfs_rq_runtime(struct cfs_rq *cfs_rq) {}
> static void task_throttle_setup_work(struct task_struct *p) {}
> @@ -6926,6 +6937,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> int h_nr_runnable = 1;
> int task_new = !(flags & ENQUEUE_WAKEUP);
> int rq_h_nr_queued = rq->cfs.h_nr_queued;
> + int throttle_flag = flags & ENQUEUE_THROTTLE;
> u64 slice = 0;
>
> if (task_is_throttled(p) && enqueue_throttled_task(p))
> @@ -6983,7 +6995,8 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> if (cfs_rq_is_idle(cfs_rq))
> h_nr_idle = 1;
>
> - flags = ENQUEUE_WAKEUP;
> + /* Ensure ENQUEUE_THROTTLE flag can be propagated through the hierarchy */
> + flags = ENQUEUE_WAKEUP | throttle_flag;
> }
>
> for_each_sched_entity(se) {
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index e7718f12bc55..468013d860a6 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -2364,6 +2364,10 @@ extern const u32 sched_prio_to_wmult[40];
> * CLASS - going to update p->sched_class; makes sched_change call the
> * various switch methods.
> *
> + * THROTTLE - invoke in throttle_cfs_rq_work() to ensure task dequeue
> + * during throttling, and in tg_unthrottle_up() to ensure
> + * task enqueue during unthrottling.
Thanks for adding the comment for dequeue_throttle. One nit is:
dequeue_throttle is used for time accounting purpose so it would be
better if you can make that clear, maybe something like this:
invoked in throttle_cfs_rq_work() during throttle for accounting
purpose, and in tg_unthrottle_up() to avoid throttling when enqueuing
tasks.
> + *
> * ENQUEUE_HEAD - place at front of runqueue (tail if not specified)
> * ENQUEUE_REPLENISH - CBS (replenish runtime and postpone deadline)
> * ENQUEUE_MIGRATED - the task was migrated during wakeup
> @@ -2381,9 +2385,9 @@ extern const u32 sched_prio_to_wmult[40];
> #define DEQUEUE_MIGRATING 0x0010 /* Matches ENQUEUE_MIGRATING */
> #define DEQUEUE_DELAYED 0x0020 /* Matches ENQUEUE_DELAYED */
> #define DEQUEUE_CLASS 0x0040 /* Matches ENQUEUE_CLASS */
> +#define DEQUEUE_THROTTLE 0x0080 /* Matches ENQUEUE_THROTTLE */
>
> #define DEQUEUE_SPECIAL 0x00010000
> -#define DEQUEUE_THROTTLE 0x00020000
>
> #define ENQUEUE_WAKEUP 0x0001
> #define ENQUEUE_RESTORE 0x0002
> @@ -2393,6 +2397,7 @@ extern const u32 sched_prio_to_wmult[40];
> #define ENQUEUE_MIGRATING 0x0010
> #define ENQUEUE_DELAYED 0x0020
> #define ENQUEUE_CLASS 0x0040
> +#define ENQUEUE_THROTTLE 0x0080
>
> #define ENQUEUE_HEAD 0x00010000
> #define ENQUEUE_REPLENISH 0x00020000
> --
> 2.34.1
>
next prev parent reply other threads:[~2025-10-27 12:02 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-27 9:05 [PATCH v2] sched/fair: Fix non-empty throttled_limbo_list warning in tg_throttle_down() Hao Jia
2025-10-27 12:02 ` Aaron Lu [this message]
2025-10-28 3:03 ` K Prateek Nayak
2025-10-28 6:39 ` Aaron Lu
2025-10-28 7:31 ` Hao Jia
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=20251027120211.GB33@bytedance \
--to=ziqianlu@bytedance.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=jiahao.kernel@gmail.com \
--cc=jiahao1@lixiang.com \
--cc=juri.lelli@redhat.com \
--cc=kprateek.nayak@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@kernel.org \
--cc=mingo@redhat.com \
--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