Kernel KVM virtualization development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Wanpeng Li" <kernellwp@gmail.com>
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH v3 03/10] sched/fair: Credit queued next-buddy via canonical requeue
Date: Fri, 12 Jun 2026 01:55:39 +0000	[thread overview]
Message-ID: <20260612015539.93D351F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260612013355.59231-4-kernellwp@gmail.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Unconditional setting of `se->rel_deadline = 1` ignores `sched_feat(PLACE_REL_DEADLINE)`, leaking relative deadline flags and corrupting absolute deadlines on reweighting.

Pre-existing issues:
- [High] `__enqueue_entity` fails to reset `se->max_slice = se->slice`, causing stale subtree maximums to incorrectly inflate the augmented rbtree.
--

commit 61176c863b2b0bee3f465f1148e81251b1cf5bda
Author: Wanpeng Li <wanpengli@tencent.com>

sched/fair: Credit queued next-buddy via canonical requeue

This commit extends eevdf_credit_entity_vlag() to handle a queued
entity. It uses the canonical requeue pattern to keep sum_w_vruntime
consistent with entity_key(), preserving the slice and shifting the
deadline.

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c

[ ... ]

> @@ -9422,7 +9424,22 @@ eevdf_credit_entity_vlag(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  		return;
>  	}
>  
> -	/* Queued entities are left unchanged by this helper path. */
> +	/*
> +	 * Canonical place_entity()-paired requeue: see dequeue_entity() and
> +	 * requeue_delayed_entity(). place_entity() restores the deadline via
> +	 * "se->deadline += se->vruntime", so make the deadline relative to
> +	 * se->vruntime here (not avg_vruntime). This preserves the slice and
> +	 * shifts the deadline by exactly -credit, mirroring the curr branch
> +	 * and keeping deadline > vruntime.
> +	 */
> +	se->vlag = vlag + (s64)credit;
> +	se->deadline -= se->vruntime;
> +	se->rel_deadline = 1;

[Severity: High]
Does setting the relative deadline flag unconditionally here cause an issue
if the PLACE_REL_DEADLINE feature is disabled?

Looking at place_entity(), the flag is only cleared if the feature is enabled:

place_entity() {
	...
	if (sched_feat(PLACE_REL_DEADLINE) && se->rel_deadline) {
		se->deadline += se->vruntime;
		se->rel_deadline = 0;
		return;
	}
	...
	se->deadline = se->vruntime + vslice;
}

If the feature is disabled, place_entity() computes an absolute deadline but
leaves se->rel_deadline set to 1. If the task then sleeps and its nice value
changes, rescale_entity() checks se->rel_deadline and will incorrectly apply
relative scaling to the absolute deadline:

rescale_entity() {
	...
	if (se->rel_deadline)
		se->deadline = div64_long(se->deadline * old_weight, weight);
	...
}

Could this arithmetic overflow and corrupt the deadline, potentially breaking
the rbtree priority ordering?

> +	cfs_rq->nr_queued--;
> +	__dequeue_entity(cfs_rq, se);
> +	place_entity(cfs_rq, se, 0);
> +	__enqueue_entity(cfs_rq, se);

[Severity: High]
This is a pre-existing issue, but does the call to __enqueue_entity()
expose a stale max_slice issue in the augmented rbtree?

When an entity is inserted as a leaf node, __enqueue_entity() resets the
min_vruntime and min_slice, but appears to leave max_slice untouched:

__enqueue_entity() {
	sum_w_vruntime_add(cfs_rq, se);
	se->min_vruntime = se->vruntime;
	se->min_slice = se->slice;
	rb_add_augmented_cached(&se->run_node, &cfs_rq->tasks_timeline,
				__entity_less, &min_vruntime_cb);
}

If the task previously had a large max_slice (for instance, when it was a
parent of tasks with large slices), does that stale max_slice propagate up
the tree during rb_add_augmented_cached()?

Could this artificially inflate cfs_rq_max_slice(cfs_rq) and circumvent the
lag clamping bounds in entity_lag(), allowing tasks to hoard excessive lag?

> +	cfs_rq->nr_queued++;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612013355.59231-1-kernellwp@gmail.com?part=3

  reply	other threads:[~2026-06-12  1:55 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-12  1:33 [PATCH v3 00/10] sched/fair, KVM: Semantics-aware directed yield for oversubscribed KVM Wanpeng Li
2026-06-12  1:33 ` [PATCH v3 01/10] sched/fair: Add EEVDF lag credit primitive for nominated next-buddy Wanpeng Li
2026-06-12  1:49   ` sashiko-bot
2026-06-12  5:34   ` K Prateek Nayak
2026-06-12  1:33 ` [PATCH v3 02/10] sched/fair: Credit a persistent, queue-depth-scaled vlag margin Wanpeng Li
2026-06-12  1:53   ` sashiko-bot
2026-06-12  6:07   ` K Prateek Nayak
2026-06-12  1:33 ` [PATCH v3 03/10] sched/fair: Credit queued next-buddy via canonical requeue Wanpeng Li
2026-06-12  1:55   ` sashiko-bot [this message]
2026-06-12  1:33 ` [PATCH v3 04/10] sched/fair: Credit nominated next-buddy in yield_to_task_fair() Wanpeng Li
2026-06-12  1:54   ` sashiko-bot
2026-06-12  1:33 ` [PATCH v3 05/10] sched/fair: Force a local resched on yield_to() so the buddy is picked Wanpeng Li
2026-06-12  1:50   ` sashiko-bot
2026-06-12  1:33 ` [PATCH v3 06/10] KVM: x86: Add IPI tracking infrastructure for directed yield Wanpeng Li
2026-06-12  1:33 ` [PATCH v3 07/10] KVM: x86/lapic: Track unicast fixed IPI delivery Wanpeng Li
2026-06-12  1:33 ` [PATCH v3 08/10] KVM: x86/lapic: Clear IPI tracking on matching-vector EOI Wanpeng Li
2026-06-12  3:46   ` sashiko-bot
2026-06-12  1:33 ` [PATCH v3 09/10] KVM: Add IPI-aware directed-yield candidate selection Wanpeng Li
2026-06-12  1:48   ` sashiko-bot
2026-06-12  1:33 ` [PATCH v3 10/10] KVM: Add relaxed preempted-only fallback for directed yield Wanpeng Li
2026-06-12  5:17 ` [PATCH v3 00/10] sched/fair, KVM: Semantics-aware directed yield for oversubscribed KVM K Prateek Nayak
2026-06-12  9:43 ` Shrikanth Hegde

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=20260612015539.93D351F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=kernellwp@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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