All of lore.kernel.org
 help / color / mirror / Atom feed
From: Qais Yousef <qais.yousef@arm.com>
To: Valentin Schneider <valentin.schneider@arm.com>
Cc: Jing-Ting Wu <jing-ting.wu@mediatek.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	wsd_upstream@mediatek.com, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org
Subject: Re: [PATCH 1/1] sched/rt: avoid contend with CFS task
Date: Fri, 30 Aug 2019 15:55:02 +0100	[thread overview]
Message-ID: <20190830145501.zadfv2ffuu7j46ft@e107158-lin.cambridge.arm.com> (raw)
In-Reply-To: <d5100b2d-46c4-5811-8274-8b06710d2594@arm.com>

On 08/29/19 11:38, Valentin Schneider wrote:
> On 29/08/2019 04:15, Jing-Ting Wu wrote:
> > At original linux design, RT & CFS scheduler are independent.
> > Current RT task placement policy will select the first cpu in
> > lowest_mask, even if the first CPU is running a CFS task.
> > This may put RT task to a running cpu and let CFS task runnable.
> > 
> > So we select idle cpu in lowest_mask first to avoid preempting
> > CFS task.
> > 
> 
> Regarding the RT & CFS thing, that's working as intended. RT is a whole
> class above CFS, it shouldn't have to worry about CFS.
> 
> On the other side of things, CFS does worry about RT. We have the concept
> of RT-pressure in the CFS scheduler, where RT tasks will reduce a CPU's
> capacity (see fair.c::scale_rt_capacity()).
> 
> CPU capacity is looked at on CFS wakeup (see wake_cap() and
> find_idlest_cpu()), and the periodic load balancer tries to spread load
> over capacity, so it'll tend to put less things on CPUs that are also
> running RT tasks.
> 
> If RT were to start avoiding rqs with CFS tasks, we'd end up with a nasty
> situation were both are avoiding each other. It's even more striking when
> you see that RT pressure is done with a rq-wide RT util_avg, which
> *doesn't* get migrated when a RT task migrates. So if you decide to move
> a RT task to an idle CPU "B" because CPU "A" had runnable CFS tasks, the
> CFS scheduler will keep seeing CPU "B" as not significantly RT-pressured
> while that util_avg signal ramps up, whereas it would correctly see CPU
> "A" as RT-pressured if the RT task previously ran there.
> 
> So overall I think this is the wrong approach.

I like the idea, but yeah tend to agree the current approach might not be
enough.

I think the major problem here is that on generic systems where CFS is a first
class citizen, RT tasks can be hostile to them - not always necessarily for a
good reason.

To further complicate the matter, even among CFS tasks we can't tell which are
more important than the others - though hopefully latency-nice proposal will
make the situation better.

So I agree we have a problem here, but I think this patch is just a temporary
band aid and we need to do better. Though I have no concrete suggestion yet on
how to do that.

Another thing I couldn't quantify yet how common and how severe this problem is
yet. Jing-Ting, if you can share the details of your use case that'd be great.

Cheers

--
Qais Yousef

WARNING: multiple messages have this Message-ID (diff)
From: Qais Yousef <qais.yousef@arm.com>
To: Valentin Schneider <valentin.schneider@arm.com>
Cc: wsd_upstream@mediatek.com, Peter Zijlstra <peterz@infradead.org>,
	linux-kernel@vger.kernel.org,
	Jing-Ting Wu <jing-ting.wu@mediatek.com>,
	linux-mediatek@lists.infradead.org,
	Matthias Brugger <matthias.bgg@gmail.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/1] sched/rt: avoid contend with CFS task
Date: Fri, 30 Aug 2019 15:55:02 +0100	[thread overview]
Message-ID: <20190830145501.zadfv2ffuu7j46ft@e107158-lin.cambridge.arm.com> (raw)
In-Reply-To: <d5100b2d-46c4-5811-8274-8b06710d2594@arm.com>

On 08/29/19 11:38, Valentin Schneider wrote:
> On 29/08/2019 04:15, Jing-Ting Wu wrote:
> > At original linux design, RT & CFS scheduler are independent.
> > Current RT task placement policy will select the first cpu in
> > lowest_mask, even if the first CPU is running a CFS task.
> > This may put RT task to a running cpu and let CFS task runnable.
> > 
> > So we select idle cpu in lowest_mask first to avoid preempting
> > CFS task.
> > 
> 
> Regarding the RT & CFS thing, that's working as intended. RT is a whole
> class above CFS, it shouldn't have to worry about CFS.
> 
> On the other side of things, CFS does worry about RT. We have the concept
> of RT-pressure in the CFS scheduler, where RT tasks will reduce a CPU's
> capacity (see fair.c::scale_rt_capacity()).
> 
> CPU capacity is looked at on CFS wakeup (see wake_cap() and
> find_idlest_cpu()), and the periodic load balancer tries to spread load
> over capacity, so it'll tend to put less things on CPUs that are also
> running RT tasks.
> 
> If RT were to start avoiding rqs with CFS tasks, we'd end up with a nasty
> situation were both are avoiding each other. It's even more striking when
> you see that RT pressure is done with a rq-wide RT util_avg, which
> *doesn't* get migrated when a RT task migrates. So if you decide to move
> a RT task to an idle CPU "B" because CPU "A" had runnable CFS tasks, the
> CFS scheduler will keep seeing CPU "B" as not significantly RT-pressured
> while that util_avg signal ramps up, whereas it would correctly see CPU
> "A" as RT-pressured if the RT task previously ran there.
> 
> So overall I think this is the wrong approach.

I like the idea, but yeah tend to agree the current approach might not be
enough.

I think the major problem here is that on generic systems where CFS is a first
class citizen, RT tasks can be hostile to them - not always necessarily for a
good reason.

To further complicate the matter, even among CFS tasks we can't tell which are
more important than the others - though hopefully latency-nice proposal will
make the situation better.

So I agree we have a problem here, but I think this patch is just a temporary
band aid and we need to do better. Though I have no concrete suggestion yet on
how to do that.

Another thing I couldn't quantify yet how common and how severe this problem is
yet. Jing-Ting, if you can share the details of your use case that'd be great.

Cheers

--
Qais Yousef

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-08-30 14:55 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-29  3:15 [PATCH 1/1] sched/rt: avoid contend with CFS task Jing-Ting Wu
2019-08-29  3:15 ` Jing-Ting Wu
2019-08-29  3:15 ` Jing-Ting Wu
2019-08-29 10:38 ` Valentin Schneider
2019-08-29 10:38   ` Valentin Schneider
2019-08-30 14:55   ` Qais Yousef [this message]
2019-08-30 14:55     ` Qais Yousef
2019-09-05 13:26     ` Jing-Ting Wu
2019-09-05 13:26       ` Jing-Ting Wu
2019-09-05 13:26       ` Jing-Ting Wu
2019-09-05 14:01       ` Vincent Guittot
2019-09-05 14:01         ` Vincent Guittot
2019-09-19 11:22         ` Jing-Ting Wu
2019-09-19 11:22           ` Jing-Ting Wu
2019-09-19 11:22           ` Jing-Ting Wu
2019-09-19 12:27           ` Vincent Guittot
2019-09-19 12:27             ` Vincent Guittot
2019-09-19 14:23             ` Qais Yousef
2019-09-19 14:23               ` Qais Yousef
2019-09-19 14:23               ` Qais Yousef
2019-09-19 14:32               ` Vincent Guittot
2019-09-19 14:32                 ` Vincent Guittot
2019-09-19 14:37                 ` Vincent Guittot
2019-09-19 14:37                   ` Vincent Guittot
2019-09-19 15:11                   ` Qais Yousef
2019-09-19 15:11                     ` Qais Yousef
2019-10-02  1:20                     ` Jing-Ting Wu
2019-10-02  1:20                       ` Jing-Ting Wu
2019-10-02  1:20                       ` Jing-Ting Wu
2019-10-09 10:56                       ` Qais Yousef
2019-10-09 10:56                         ` Qais Yousef
2019-10-03 16:25 ` Dietmar Eggemann
2019-10-03 16:25   ` Dietmar Eggemann
2019-10-03 16:25   ` Dietmar Eggemann

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=20190830145501.zadfv2ffuu7j46ft@e107158-lin.cambridge.arm.com \
    --to=qais.yousef@arm.com \
    --cc=jing-ting.wu@mediatek.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=peterz@infradead.org \
    --cc=valentin.schneider@arm.com \
    --cc=wsd_upstream@mediatek.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.