From: Gautham R Shenoy <ego@linux.vnet.ibm.com>
To: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
Cc: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>,
Michael Ellerman <mpe@ellerman.id.au>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Michael Neuling <mikey@neuling.org>,
Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>,
Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>,
Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>,
"Oliver O'Halloran" <oohall@gmail.com>,
Nicholas Piggin <npiggin@gmail.com>,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/2] powerpc: Detect the presence of big-cores via "ibm, thread-groups"
Date: Wed, 11 Jul 2018 15:24:18 +0530 [thread overview]
Message-ID: <20180711095418.GC6591@in.ibm.com> (raw)
In-Reply-To: <20180708160334.GA6947@kermit-br-ibm-com>
Hello Murilo,
On Sun, Jul 08, 2018 at 01:03:34PM -0300, Murilo Opsfelder Araujo wrote:
> On Fri, Jul 06, 2018 at 02:35:48PM +0530, Gautham R. Shenoy wrote:
> > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> >
> > On IBM POWER9, the device tree exposes a property array identifed by
> > "ibm,thread-groups" which will indicate which groups of threads share a
> > particular set of resources.
> >
> > As of today we only have one form of grouping identifying the group of
> > threads in the core that share the L1 cache, translation cache and
> > instruction data flow.
> >
> > This patch defines the helper function to parse the contents of
> > "ibm,thread-groups" and a new structure to contain the parsed output.
> >
> > The patch also creates the sysfs file named "small_core_siblings" that
> > returns the physical ids of the threads in the core that share the L1
> > cache, translation cache and instruction data flow.
> >
> > Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> > ---
> > Documentation/ABI/testing/sysfs-devices-system-cpu | 8 ++
> > arch/powerpc/include/asm/cputhreads.h | 22 +++
> > arch/powerpc/kernel/setup-common.c | 154 +++++++++++++++++++++
> > arch/powerpc/kernel/sysfs.c | 35 +++++
> > 4 files changed, 219 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
> > index 9c5e7732..62f24de 100644
> > --- a/Documentation/ABI/testing/sysfs-devices-system-cpu
> > +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
> > @@ -487,3 +487,11 @@ Description: Information about CPU vulnerabilities
> > "Not affected" CPU is not affected by the vulnerability
> > "Vulnerable" CPU is affected and no mitigation in effect
> > "Mitigation: $M" CPU is affected and mitigation $M is in effect
> > +
> > +What: /sys/devices/system/cpu/cpu[0-9]+/small_core_siblings
> > +Date: 05-Jul-2018
> > +KernelVersion: v4.18.0
> > +Contact: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> > +Description: List of Physical ids of CPUs which share the the L1 cache,
> > + translation cache and instruction data-flow with this CPU.
>
> What about this?
>
> Description: List of physical CPU IDs that share a common L1 cache,
> translation cache and instruction data flow with this CPU.
>
> Or perhaps just remove the extra "the".
Oops! Will remove the extra "the". Thanks for spotting it.
>
> > +Values: Comma separated list of decimal integers.
> > diff --git a/arch/powerpc/include/asm/cputhreads.h b/arch/powerpc/include/asm/cputhreads.h
> > index d71a909..33226d7 100644
> > --- a/arch/powerpc/include/asm/cputhreads.h
[..snip..]
> > diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> > index 755dc98..f5717de 100644
> > --- a/arch/powerpc/kernel/sysfs.c
> > +++ b/arch/powerpc/kernel/sysfs.c
> > @@ -18,6 +18,7 @@
> > #include <asm/smp.h>
> > #include <asm/pmc.h>
> > #include <asm/firmware.h>
> > +#include <asm/cputhreads.h>
> >
> > #include "cacheinfo.h"
> > #include "setup.h"
> > @@ -1025,6 +1026,33 @@ static ssize_t show_physical_id(struct device *dev,
> > }
> > static DEVICE_ATTR(physical_id, 0444, show_physical_id, NULL);
> >
> > +static ssize_t show_small_core_siblings(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
>
> Interesting enough, checkpatch.pl warned about this function name:
>
> WARNING: Consider renaming function(s) 'show_small_core_siblings' to 'small_core_siblings_show'
> #354: FILE: arch/powerpc/kernel/sysfs.c:1053:
> +}
Yes I kept it despite the warning, as doing otherwise would introduce
inconsistency with the way the remaining functions are named in the
file.
If Michael Ellerman is ok, I can send a file converting all these
sysfs calls to use DEVICE_ATTR_RO/RW, which will require renaming all
the other functions to XXX_show().
>
> > + struct cpu *cpu = container_of(dev, struct cpu, dev);
> > + struct device_node *dn = of_get_cpu_node(cpu->dev.id, NULL);
> > + struct thread_groups tg;
> > + int i, j;
> > + ssize_t ret = 0;
> > +
> > + if (parse_thread_groups(dn, &tg))
> > + return -ENODATA;
> > +
> > + i = get_cpu_thread_group_start(cpu->dev.id, &tg);
> > +
> > + if (i == -1)
> > + return -ENODATA;
> > +
> > + for (j = 0; j < tg.threads_per_group - 1; j++)
> > + ret += sprintf(buf + ret, "%d,", tg.thread_list[i + j]);
> > +
> > + ret += sprintf(buf + ret, "%d\n", tg.thread_list[i + j]);
> > +
> > + return ret;
> > +}
> > +static DEVICE_ATTR(small_core_siblings, 0444, show_small_core_siblings, NULL);
> > +
> > static int __init topology_init(void)
> > {
> > int cpu, r;
> > @@ -1048,6 +1076,13 @@ static int __init topology_init(void)
> > register_cpu(c, cpu);
> >
> > device_create_file(&c->dev, &dev_attr_physical_id);
> > +
> > + if (has_big_cores) {
> > + const struct device_attribute *attr =
> > + &dev_attr_small_core_siblings;
> > +
> > + device_create_file(&c->dev, attr);
> > + }
> > }
> > }
> > r = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "powerpc/topology:online",
> > --
> > 1.9.4
> >
>
> --
> Murilo
next prev parent reply other threads:[~2018-07-11 9:54 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-06 9:05 [PATCH v3 0/2] powerpc: Detection and scheduler optimization for POWER9 bigcore Gautham R. Shenoy
2018-07-06 9:05 ` [PATCH v3 1/2] powerpc: Detect the presence of big-cores via "ibm,thread-groups" Gautham R. Shenoy
2018-07-08 16:03 ` [PATCH v3 1/2] powerpc: Detect the presence of big-cores via "ibm, thread-groups" Murilo Opsfelder Araujo
2018-07-09 3:52 ` Joe Perches
2018-07-11 9:54 ` Gautham R Shenoy [this message]
2018-07-06 9:05 ` [PATCH v3 2/2] powerpc: Enable CPU_FTR_ASYM_SMT for interleaved big-cores Gautham R. Shenoy
2018-07-11 8:32 ` kbuild test robot
2018-07-16 5:25 ` Gautham R Shenoy
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=20180711095418.GC6591@in.ibm.com \
--to=ego@linux.vnet.ibm.com \
--cc=akshay.adiga@linux.vnet.ibm.com \
--cc=benh@kernel.crashing.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mikey@neuling.org \
--cc=mpe@ellerman.id.au \
--cc=muriloo@linux.ibm.com \
--cc=npiggin@gmail.com \
--cc=oohall@gmail.com \
--cc=shilpa.bhat@linux.vnet.ibm.com \
--cc=svaidy@linux.vnet.ibm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).