All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Zhang Qiao <zhangqiao22@huawei.com>
Cc: Waiman Long <longman@redhat.com>, Ingo Molnar <mingo@redhat.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	lkml <linux-kernel@vger.kernel.org>
Subject: Re: [bug-report] possible s64 overflow in max_vruntime()
Date: Thu, 22 Dec 2022 13:45:48 +0100	[thread overview]
Message-ID: <Y6RRfF5yRew7rdCp@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <73e639d5-702b-0d03-16d9-a965b1963ef6@huawei.com>

On Wed, Dec 21, 2022 at 11:19:31PM +0800, Zhang Qiao wrote:
> hi folks,
> 
>     I found problem about s64 overflow in max_vruntime().
> 
>     I create a task group GROUPA (path: /system.slice/xxx/yyy/CGROUPA) and run a task in this
> group on each cpu, these tasks is while loop and 100% cpu usage.
> 
>     When unregister net devices, will queue a kwork on system_highpri_wq at flush_all_backlogs()
> and wake up a high-priority kworker thread on each cpu. However, the kworker thread has been
> waiting on the queue and has not been scheduled.
> 
>     After parsing the vmcore, the vruntime of the kworker is 0x918fdb05287da7c3 and the
> cfs_rq->min_vruntime is 0x124b17fd59db8d02.
> 
>     why the difference between the cfs_rq->min_vruntime and kworker's vruntime is so large?
>     1) the kworker of the system_highpri_wq sleep for long long time(about 300 days).
>     2) cfs_rq->curr is the ancestor of the GROUPA, cfs->curr->load.weight is 2494, so when
> the task belonging to the GROUPA run for a long time, its vruntime will increase by 420
> times, cfs_rq->min_vruntime will also grow rapidly.
>     3) when wakeup kworker thread, kworker will be set the maximum value between kworker's
> vruntime and cfs_rq->min_vruntime. But at max_vruntime(), there will be a s64 overflow issue,
> as follow:
> 
> ---------
> 
> static inline u64 min_vruntime(u64 min_vruntime, u64 vruntime)
> {
> 	/*
> 	 * vruntime=0x124b17fd59db8d02
> 	 * min_vruntime=0x918fdb05287da7c3
> 	 * vruntime - min_vruntime = 9276074894177461567 > s64_max, will s64 overflow
> 	 */
> 	s64 delta = (s64)(vruntime - min_vruntime);
> 	if (delta < 0)
> 		min_vruntime = vruntime;
> 
> 	return min_vruntime;
> }
> 
> ----------
> 
> max_vruntime() will return the kworker's old vruntime, it is incorrect and the correct result
> shoud be cfs_rq->minvruntime. This incorrect result is greater than cfs_rq->min_vruntime and
> will cause kworker thread starved.
> 
>     Does anyone have a good suggestion for slove this problem? or bugfix patch.

I don't understand what you tihnk the problem is. Signed overflow is
perfectly fine and works as designed here.

  parent reply	other threads:[~2022-12-22 12:46 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-21 15:19 [bug-report] possible s64 overflow in max_vruntime() Zhang Qiao
2022-12-21 16:10 ` Waiman Long
2022-12-22 12:45 ` Peter Zijlstra [this message]
2022-12-23 13:57   ` Zhang Qiao
2023-01-12  3:01     ` Zhang Qiao
2023-01-25 19:57     ` Roman Kagan
2023-01-25 19:45   ` Roman Kagan
2023-01-26 12:49     ` Peter Zijlstra
2023-01-26 18:31       ` Roman Kagan
2023-01-27 11:44         ` Peter Zijlstra
2023-01-27 16:18           ` Vincent Guittot
2023-01-27 22:10             ` Benjamin Segall
2023-01-27 22:29               ` Vincent Guittot
2023-01-31  3:21             ` Chen Yu
2023-01-31  9:59               ` Roman Kagan
2023-01-31 11:10                 ` Vincent Guittot
2023-02-01 12:52                   ` Chen Yu
2023-02-07 19:37                   ` Roman Kagan
2023-02-08 10:13                     ` Vincent Guittot
2023-02-08 18:09                       ` Roman Kagan
2023-02-09 11:26                         ` Vincent Guittot
2023-02-09 13:33                           ` Roman Kagan
2023-02-09 13:44                             ` Vincent Guittot
2023-02-09 14:34                               ` Roman Kagan

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=Y6RRfF5yRew7rdCp@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=vincent.guittot@linaro.org \
    --cc=zhangqiao22@huawei.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.