All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Patrick Bellasi <patrick.bellasi@arm.com>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Quentin Perret <quentin.perret@arm.com>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Morten Rasmussen <morten.rasmussen@arm.com>,
	Juri Lelli <juri.lelli@redhat.com>, Todd Kjos <tkjos@google.com>,
	Steve Muckle <smuckle@google.com>,
	Suren Baghdasaryan <surenb@google.com>
Subject: Re: [PATCH v2 3/3] sched/fair: add lsub_positive and use it consistently
Date: Mon, 12 Nov 2018 05:16:56 +0100	[thread overview]
Message-ID: <20181112041656.GA47272@gmail.com> (raw)
In-Reply-To: <20181105145400.935-4-patrick.bellasi@arm.com>


* Patrick Bellasi <patrick.bellasi@arm.com> wrote:

> The following pattern:
> 
>    var -= min_t(typeof(var), var, val);
> 
> is used multiple times in fair.c.
> 
> The existing sub_positive() already capture that pattern but it adds
> also explicit load-sotre to properly support lockless observations.
> In other cases, the patter above is used to update local, and/or not
> concurrently accessed, variables.

> Let's add a simpler version of sub_positive, targeted to local variables
> updates, which gives the same readability benefits at calling sites
> without enforcing {READ,WRITE}_ONCE barriers.

> +/*
> + * Remove and clamp on negative, from a local variable.
> + *
> + * A variant of sub_positive which do not use explicit load-store
> + * and thus optimized for local variable updates.

I fixed up the two typos ('load-sotre', 'patter'), and fixed eight 
grammar mistakes (!) in the changelog and in the code comments, but 
*please* read the changelogs and code you are writing, this is scheduler 
code after all ...

( Please also use the fn() notation in changelogs consistently in the 
  future: first you use 'sub_positive()' correctly then it becomes 
  'sub_positive'. )

Anyway, the fix looks sane so I've applied it to sched/urgent.

Thanks,

	Ingo

      reply	other threads:[~2018-11-12  4:17 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-05 14:53 [PATCH v2 0/3] util_est regression fixup and cleanups Patrick Bellasi
2018-11-05 14:53 ` [PATCH v2 1/3] sched/fair: util_est: fix cpu_util_wake for execl Patrick Bellasi
2018-11-05 14:53 ` [PATCH v2 2/3] sched/fair: util_est: mask UTIL_AVG_UNCHANGED usages Patrick Bellasi
2018-11-12  6:03   ` [tip:sched/core] sched/fair: Mask " tip-bot for Patrick Bellasi
2018-11-05 14:54 ` [PATCH v2 3/3] sched/fair: add lsub_positive and use it consistently Patrick Bellasi
2018-11-12  4:16   ` Ingo Molnar [this message]

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=20181112041656.GA47272@gmail.com \
    --to=mingo@kernel.org \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=morten.rasmussen@arm.com \
    --cc=patrick.bellasi@arm.com \
    --cc=peterz@infradead.org \
    --cc=quentin.perret@arm.com \
    --cc=smuckle@google.com \
    --cc=surenb@google.com \
    --cc=tkjos@google.com \
    --cc=vincent.guittot@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.