All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Bellasi <patrick.bellasi@arm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	Ingo Molnar <mingo@redhat.com>,
	"Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Morten Rasmussen <morten.rasmussen@arm.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Joel Fernandes <joelaf@google.com>,
	Steve Muckle <smuckle@google.com>
Subject: Re: [PATCH 3/3] sched/fair: schedutil: explicit update only when required
Date: Thu, 10 May 2018 17:54:14 +0100	[thread overview]
Message-ID: <20180510165414.GB30654@e110439-lin> (raw)
In-Reply-To: <20180510161515.GH12217@hirez.programming.kicks-ass.net>

On 10-May 18:15, Peter Zijlstra wrote:
> On Thu, May 10, 2018 at 04:05:53PM +0100, Patrick Bellasi wrote:
> > All the above considered, let's make schedutil updates more explicit in
> > fair.c by removing the cfs_rq_util_change() wrapper function in favour
> > of the existing cpufreq_update_util() public API.
> > This can be done by calling cpufreq_update_util() explicitly in the few
> > call sites where it really makes sense and when all the (potentially)
> > required cfs_rq's information have been updated.
> 
> Aside from having to redraw my ever stale diagrams _again_ I don't think
> I object too much here. As you write tracking the exact point where we
> did do the update was fairly tedious.

Maybe we can simplify even better these diagrams if we get to the
point where schedutil updates are tirggered just from core.c...

This patch could be a first step on this directions, not entirely sure
we can get there... but, having some well defined callbacks maybe it
can make it easier to factorize up the updates in the core scheduler.

> > @@ -5397,9 +5366,27 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> >  		update_cfs_group(se);
> >  	}
> >  
> > +	/* The task is visible from the root cfs_rq */
> > +	if (!se) {
> > +		unsigned int flags = 0;
> 
> That one shadows the @flags argument. Some checker is bound to complain
> about it.

Ohps... right... I'll s/flags/sc_flags/ "sc_" standing for SCHED_CPUFREQ_.

> > +
> >  		add_nr_running(rq, 1);
> >  
> > +		if (p->in_iowait)
> > +			flags |= SCHED_CPUFREQ_IOWAIT;
> > +
> > +		/*
> > +		 * !last_update_time means we've passed through
> > +		 * migrate_task_rq_fair() indicating we migrated.
> > +		 *
> > +		 * IOW we're enqueueing a task on a new CPU.
> > +		 */
> > +		if (!p->se.avg.last_update_time)
> > +			flags |= SCHED_CPUFREQ_MIGRATION;
> > +
> > +		cpufreq_update_util(rq, flags);
> > +	}
> > +
> >  	hrtick_update(rq);
> >  }

-- 
#include <best/regards.h>

Patrick Bellasi

  reply	other threads:[~2018-05-10 16:54 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-10 15:05 [PATCH 0/3] Improve schedutil integration for FAIR tasks Patrick Bellasi
2018-05-10 15:05 ` [PATCH 1/3] sched/cpufreq: always consider blocked FAIR utilization Patrick Bellasi
2018-05-11  5:44   ` Viresh Kumar
2018-05-11  9:12     ` Patrick Bellasi
2018-05-14  9:18       ` Vincent Guittot
2018-05-14 16:33         ` Patrick Bellasi
2018-05-10 15:05 ` [PATCH 2/3] sched/fair: util_est: update before schedutil Patrick Bellasi
2018-05-10 15:34   ` Peter Zijlstra
2018-05-11  5:44   ` Viresh Kumar
2018-05-11  8:41     ` Patrick Bellasi
2018-05-10 15:05 ` [PATCH 3/3] sched/fair: schedutil: explicit update only when required Patrick Bellasi
2018-05-10 16:15   ` Peter Zijlstra
2018-05-10 16:54     ` Patrick Bellasi [this message]
2018-05-11  5:43   ` Viresh Kumar
2018-05-11  8:42     ` Patrick Bellasi
2018-05-13  6:04   ` Joel Fernandes
2018-05-13  6:25     ` Joel Fernandes
2018-05-14 16:32       ` Patrick Bellasi
2018-05-15 10:19         ` Vincent Guittot
2018-05-15 14:53           ` Patrick Bellasi
2018-05-15 16:53             ` Peter Zijlstra
2018-05-15 17:25               ` Patrick Bellasi
2018-05-16  7:13               ` Vincent Guittot
2018-05-16  7:12             ` Vincent Guittot
2018-05-16 10:45               ` Patrick Bellasi
2018-05-17 15:17         ` Joel Fernandes
2018-05-24 13:42           ` Patrick Bellasi

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=20180510165414.GB30654@e110439-lin \
    --to=patrick.bellasi@arm.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=joelaf@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=morten.rasmussen@arm.com \
    --cc=peterz@infradead.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=smuckle@google.com \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.org \
    /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.