All of lore.kernel.org
 help / color / mirror / Atom feed
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 00/15] multi-cluster power management
Date: Tue, 23 Apr 2013 21:04:29 +0100	[thread overview]
Message-ID: <20130423200429.GF5316@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <1360041732-17936-1-git-send-email-nicolas.pitre@linaro.org>

On Tue, Feb 05, 2013 at 12:21:57AM -0500, Nicolas Pitre wrote:
> This is version 4 of the patch series required to safely power up
> and down CPUs in a cluster as can be found in b.L systems.  If nothing
> major is reported, I'll send a pull request to Russell for this set
> very soon.

Okay, so from spending an hour or so quickly scanning these patches,
on the face of it there are only a few comments I have which I'm very
surprised that your exhaustive team of apparant reviewers didn't
already catch.

The biggest disappointment so far in this patch set is finding the
re-use of the repeated cache handling stuff - when I've already made
my feelings _well_ known towards re-using existing cache APIs (which
are designed to handle a _purpose_) for different purposes "just
because" they appear to do what you need on a particular CPU.  I've
stated my objection to this kind of behaviour many times in the past,
I've even _purposely_ broken stuff (mtd stuff) when that's happened in
the past, and I've not changed my views on this one bit.  It's really
surprising to me that someone who has been around for such a long time,
and who knows my views on this, is pushing a patch set which goes
against that, and somehow thinks that I won't have a problem with
it... really?

What I suggest for the time being is to provide new inline function(s)
in arch/arm/include/cacheflush.h which are purposed for your application,
document them in that file, and call the implementation you're currently
using.  That means if we do have to change it in the future (for example,
we don't need to do anything in the dcache flushing stuff) we don't have
to hunt through all the code to find _your_ different use of that function
and fix it - we can just fix it in one place and we have the reference
there for what your code expects.

Again, this is nothing new - I've made my position on this kind of stuff
_exceedingly_ plain in the past.


That all said, my biggest concern so far from this set is the independent
issue of moving the cache handling into generic code, so that we avoid
the existing problem where every platform ends up implementing that stuff
time and time again - and the impact that will have on this MCPM code.

Currently as I see it, the two things are mutually incompatible with
each other - and having discussed with Will, we'd come to the conclusion
that I'd merge what I had because the comments alone on how the cpu
hotplug stuff is supposed to work are a valuable improvement, even
though the code changes don't completely solve all the issues.

However, MCPMm gets in the way of that.  So... that presents a dilema...
it's a case of this or that but not both.  Or do you think MCPM can
survive with additional LOUIS flushing before the cpu die callback is
called?

  parent reply	other threads:[~2013-04-23 20:04 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-05  5:21 [PATCH v4 00/15] multi-cluster power management Nicolas Pitre
2013-02-05  5:21 ` [PATCH v4 01/15] ARM: multi-cluster PM: secondary kernel entry code Nicolas Pitre
2013-04-23 19:19   ` Russell King - ARM Linux
2013-04-23 19:34     ` Nicolas Pitre
2013-04-23 20:09       ` Russell King - ARM Linux
2013-04-23 20:19         ` Nicolas Pitre
2013-02-05  5:21 ` [PATCH v4 02/15] ARM: mcpm: introduce the CPU/cluster power API Nicolas Pitre
2013-02-05  5:22 ` [PATCH v4 03/15] ARM: mcpm: introduce helpers for platform coherency exit/setup Nicolas Pitre
2013-04-05 23:00   ` Olof Johansson
2013-04-06 13:41     ` Nicolas Pitre
2013-04-24  9:10     ` Dave Martin
2013-02-05  5:22 ` [PATCH v4 04/15] ARM: mcpm: Add baremetal voting mutexes Nicolas Pitre
2013-02-05  5:22 ` [PATCH v4 05/15] ARM: mcpm_head.S: vlock-based first man election Nicolas Pitre
2013-02-05  5:22 ` [PATCH v4 06/15] ARM: mcpm: generic SMP secondary bringup and hotplug support Nicolas Pitre
2013-04-23 19:31   ` Russell King - ARM Linux
2013-04-23 19:36     ` Nicolas Pitre
2013-02-05  5:22 ` [PATCH v4 07/15] ARM: introduce common set_auxcr/get_auxcr functions Nicolas Pitre
2013-02-05  5:22 ` [PATCH v4 08/15] ARM: vexpress: introduce DCSCB support Nicolas Pitre
2013-02-07 18:14   ` Catalin Marinas
2013-02-07 18:56     ` Nicolas Pitre
2013-02-05  5:22 ` [PATCH v4 09/15] ARM: vexpress/dcscb: add CPU use counts to the power up/down API implementation Nicolas Pitre
2013-02-05  5:22 ` [PATCH v4 10/15] ARM: vexpress/dcscb: do not hardcode number of CPUs per cluster Nicolas Pitre
2013-02-05  5:22 ` [PATCH v4 11/15] drivers/bus: add ARM CCI support Nicolas Pitre
2013-04-23 19:38   ` Russell King - ARM Linux
2013-04-23 19:53     ` Nicolas Pitre
2013-02-05  5:22 ` [PATCH v4 12/15] ARM: CCI: ensure powerdown-time data is flushed from cache Nicolas Pitre
2013-04-23 19:40   ` Russell King - ARM Linux
2013-02-05  5:22 ` [PATCH v4 13/15] ARM: vexpress/dcscb: handle platform coherency exit/setup and CCI Nicolas Pitre
2013-02-05  5:22 ` [PATCH v4 14/15] ARM: Enable selection of SMP operations at boot time Nicolas Pitre
2013-04-05 22:43   ` Olof Johansson
2013-04-06 13:43     ` Nicolas Pitre
2013-04-09 16:30   ` Nicolas Pitre
2013-04-09 16:55     ` Jon Medhurst (Tixy)
2013-02-05  5:22 ` [PATCH v4 15/15] ARM: vexpress: Select multi-cluster SMP operation if required Nicolas Pitre
2013-02-06 16:38   ` Pawel Moll
2013-02-06 17:55     ` Nicolas Pitre
2013-04-05 22:48   ` Olof Johansson
2013-04-06 14:02     ` Nicolas Pitre
2013-04-08  9:10       ` Jon Medhurst (Tixy)
2013-04-09  5:41         ` Nicolas Pitre
2013-04-09  6:00           ` Jon Medhurst (Tixy)
2013-04-09 16:34             ` Nicolas Pitre
2013-04-09 17:28               ` Jon Medhurst (Tixy)
2013-04-23 19:42   ` Russell King - ARM Linux
2013-04-23 19:56     ` Nicolas Pitre
2013-04-23 20:04 ` Russell King - ARM Linux [this message]
2013-04-23 21:03   ` [PATCH v4 00/15] multi-cluster power management Nicolas Pitre
2013-04-23 21:46     ` Russell King - ARM Linux
2013-04-23 21:56       ` Nicolas Pitre
2013-04-23 22:44         ` Russell King - ARM Linux
2013-04-24  4:11           ` Nicolas Pitre
2013-04-24 20:25             ` Russell King - ARM Linux
2013-04-24 23:31               ` Nicolas Pitre
2013-04-24 14:25     ` Dave Martin
2013-04-23 21:11   ` Nicolas Pitre

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=20130423200429.GF5316@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=linux-arm-kernel@lists.infradead.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.