* Re: [PATCH] mm/hugetlb: Fix build failure with HUGETLB_PAGE but not HUGEBTLBFS
From: Christophe Leroy @ 2020-03-17 8:43 UTC (permalink / raw)
To: Baoquan He
Cc: Nick Piggin, Andi Kleen, linux-kernel, linux-mm, Adam Litke,
Nishanth Aravamudan, Andrew Morton, linuxppc-dev, Mike Kravetz
In-Reply-To: <20200317082550.GA3375@MiWiFi-R3L-srv>
Le 17/03/2020 à 09:25, Baoquan He a écrit :
> On 03/17/20 at 08:04am, Christophe Leroy wrote:
>> When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the
>> following build failure is encoutered:
>
> From the definition of HUGETLB_PAGE, isn't it relying on HUGETLBFS?
> I could misunderstand the def_bool, please correct me if I am wrong.
AFAIU, it means that HUGETLBFS rely on HUGETLB_PAGE, by default
HUGETLB_PAGE is not selected when HUGETLBFS is not. But it is still
possible for an arch to select HUGETLB_PAGE without selecting HUGETLBFS
when it uses huge pages for other purpose than hugetlb file system.
Christophe
>
> config HUGETLB_PAGE
> def_bool HUGETLBFS
>
>>
>> In file included from arch/powerpc/mm/fault.c:33:0:
>> ./include/linux/hugetlb.h: In function 'hstate_inode':
>> ./include/linux/hugetlb.h:477:9: error: implicit declaration of function 'HUGETLBFS_SB' [-Werror=implicit-function-declaration]
>> return HUGETLBFS_SB(i->i_sb)->hstate;
>> ^
>> ./include/linux/hugetlb.h:477:30: error: invalid type argument of '->' (have 'int')
>> return HUGETLBFS_SB(i->i_sb)->hstate;
>> ^
>>
>> Gate hstate_inode() with CONFIG_HUGETLBFS instead of CONFIG_HUGETLB_PAGE.
>>
>> Reported-by: kbuild test robot <lkp@intel.com>
>> Link: https://patchwork.ozlabs.org/patch/1255548/#2386036
>> Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>> include/linux/hugetlb.h | 19 ++++++++-----------
>> 1 file changed, 8 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
>> index 1e897e4168ac..dafb3d70ff81 100644
>> --- a/include/linux/hugetlb.h
>> +++ b/include/linux/hugetlb.h
>> @@ -390,7 +390,10 @@ static inline bool is_file_hugepages(struct file *file)
>> return is_file_shm_hugepages(file);
>> }
>>
>> -
>> +static inline struct hstate *hstate_inode(struct inode *i)
>> +{
>> + return HUGETLBFS_SB(i->i_sb)->hstate;
>> +}
>> #else /* !CONFIG_HUGETLBFS */
>>
>> #define is_file_hugepages(file) false
>> @@ -402,6 +405,10 @@ hugetlb_file_setup(const char *name, size_t size, vm_flags_t acctflag,
>> return ERR_PTR(-ENOSYS);
>> }
>>
>> +static inline struct hstate *hstate_inode(struct inode *i)
>> +{
>> + return NULL;
>> +}
>> #endif /* !CONFIG_HUGETLBFS */
>>
>> #ifdef HAVE_ARCH_HUGETLB_UNMAPPED_AREA
>> @@ -472,11 +479,6 @@ extern unsigned int default_hstate_idx;
>>
>> #define default_hstate (hstates[default_hstate_idx])
>>
>> -static inline struct hstate *hstate_inode(struct inode *i)
>> -{
>> - return HUGETLBFS_SB(i->i_sb)->hstate;
>> -}
>> -
>> static inline struct hstate *hstate_file(struct file *f)
>> {
>> return hstate_inode(file_inode(f));
>> @@ -729,11 +731,6 @@ static inline struct hstate *hstate_vma(struct vm_area_struct *vma)
>> return NULL;
>> }
>>
>> -static inline struct hstate *hstate_inode(struct inode *i)
>> -{
>> - return NULL;
>> -}
>> -
>> static inline struct hstate *page_hstate(struct page *page)
>> {
>> return NULL;
>> --
>> 2.25.0
>>
>>
^ permalink raw reply
* Re: [PATCH] mm/hugetlb: Fix build failure with HUGETLB_PAGE but not HUGEBTLBFS
From: Baoquan He @ 2020-03-17 8:25 UTC (permalink / raw)
To: Christophe Leroy
Cc: Nick Piggin, Andi Kleen, linux-kernel, linux-mm, Adam Litke,
Nishanth Aravamudan, Andrew Morton, linuxppc-dev, Mike Kravetz
In-Reply-To: <7e8c3a3c9a587b9cd8a2f146df32a421b961f3a2.1584432148.git.christophe.leroy@c-s.fr>
On 03/17/20 at 08:04am, Christophe Leroy wrote:
> When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the
> following build failure is encoutered:
From the definition of HUGETLB_PAGE, isn't it relying on HUGETLBFS?
I could misunderstand the def_bool, please correct me if I am wrong.
config HUGETLB_PAGE
def_bool HUGETLBFS
>
> In file included from arch/powerpc/mm/fault.c:33:0:
> ./include/linux/hugetlb.h: In function 'hstate_inode':
> ./include/linux/hugetlb.h:477:9: error: implicit declaration of function 'HUGETLBFS_SB' [-Werror=implicit-function-declaration]
> return HUGETLBFS_SB(i->i_sb)->hstate;
> ^
> ./include/linux/hugetlb.h:477:30: error: invalid type argument of '->' (have 'int')
> return HUGETLBFS_SB(i->i_sb)->hstate;
> ^
>
> Gate hstate_inode() with CONFIG_HUGETLBFS instead of CONFIG_HUGETLB_PAGE.
>
> Reported-by: kbuild test robot <lkp@intel.com>
> Link: https://patchwork.ozlabs.org/patch/1255548/#2386036
> Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> include/linux/hugetlb.h | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 1e897e4168ac..dafb3d70ff81 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -390,7 +390,10 @@ static inline bool is_file_hugepages(struct file *file)
> return is_file_shm_hugepages(file);
> }
>
> -
> +static inline struct hstate *hstate_inode(struct inode *i)
> +{
> + return HUGETLBFS_SB(i->i_sb)->hstate;
> +}
> #else /* !CONFIG_HUGETLBFS */
>
> #define is_file_hugepages(file) false
> @@ -402,6 +405,10 @@ hugetlb_file_setup(const char *name, size_t size, vm_flags_t acctflag,
> return ERR_PTR(-ENOSYS);
> }
>
> +static inline struct hstate *hstate_inode(struct inode *i)
> +{
> + return NULL;
> +}
> #endif /* !CONFIG_HUGETLBFS */
>
> #ifdef HAVE_ARCH_HUGETLB_UNMAPPED_AREA
> @@ -472,11 +479,6 @@ extern unsigned int default_hstate_idx;
>
> #define default_hstate (hstates[default_hstate_idx])
>
> -static inline struct hstate *hstate_inode(struct inode *i)
> -{
> - return HUGETLBFS_SB(i->i_sb)->hstate;
> -}
> -
> static inline struct hstate *hstate_file(struct file *f)
> {
> return hstate_inode(file_inode(f));
> @@ -729,11 +731,6 @@ static inline struct hstate *hstate_vma(struct vm_area_struct *vma)
> return NULL;
> }
>
> -static inline struct hstate *hstate_inode(struct inode *i)
> -{
> - return NULL;
> -}
> -
> static inline struct hstate *page_hstate(struct page *page)
> {
> return NULL;
> --
> 2.25.0
>
>
^ permalink raw reply
* [PATCH] mm/hugetlb: Fix build failure with HUGETLB_PAGE but not HUGEBTLBFS
From: Christophe Leroy @ 2020-03-17 8:04 UTC (permalink / raw)
To: Mike Kravetz, Andrew Morton, Nishanth Aravamudan, Nick Piggin,
Adam Litke, Andi Kleen
Cc: linux-mm, linuxppc-dev, linux-kernel
When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the
following build failure is encoutered:
In file included from arch/powerpc/mm/fault.c:33:0:
./include/linux/hugetlb.h: In function 'hstate_inode':
./include/linux/hugetlb.h:477:9: error: implicit declaration of function 'HUGETLBFS_SB' [-Werror=implicit-function-declaration]
return HUGETLBFS_SB(i->i_sb)->hstate;
^
./include/linux/hugetlb.h:477:30: error: invalid type argument of '->' (have 'int')
return HUGETLBFS_SB(i->i_sb)->hstate;
^
Gate hstate_inode() with CONFIG_HUGETLBFS instead of CONFIG_HUGETLB_PAGE.
Reported-by: kbuild test robot <lkp@intel.com>
Link: https://patchwork.ozlabs.org/patch/1255548/#2386036
Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
include/linux/hugetlb.h | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 1e897e4168ac..dafb3d70ff81 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -390,7 +390,10 @@ static inline bool is_file_hugepages(struct file *file)
return is_file_shm_hugepages(file);
}
-
+static inline struct hstate *hstate_inode(struct inode *i)
+{
+ return HUGETLBFS_SB(i->i_sb)->hstate;
+}
#else /* !CONFIG_HUGETLBFS */
#define is_file_hugepages(file) false
@@ -402,6 +405,10 @@ hugetlb_file_setup(const char *name, size_t size, vm_flags_t acctflag,
return ERR_PTR(-ENOSYS);
}
+static inline struct hstate *hstate_inode(struct inode *i)
+{
+ return NULL;
+}
#endif /* !CONFIG_HUGETLBFS */
#ifdef HAVE_ARCH_HUGETLB_UNMAPPED_AREA
@@ -472,11 +479,6 @@ extern unsigned int default_hstate_idx;
#define default_hstate (hstates[default_hstate_idx])
-static inline struct hstate *hstate_inode(struct inode *i)
-{
- return HUGETLBFS_SB(i->i_sb)->hstate;
-}
-
static inline struct hstate *hstate_file(struct file *f)
{
return hstate_inode(file_inode(f));
@@ -729,11 +731,6 @@ static inline struct hstate *hstate_vma(struct vm_area_struct *vma)
return NULL;
}
-static inline struct hstate *hstate_inode(struct inode *i)
-{
- return NULL;
-}
-
static inline struct hstate *page_hstate(struct page *page)
{
return NULL;
--
2.25.0
^ permalink raw reply related
* Re: [RFC 00/11] perf: Enhancing perf to export processor hazard information
From: maddy @ 2020-03-17 6:50 UTC (permalink / raw)
To: Kim Phillips, Ravi Bangoria
Cc: Mark Rutland, Andi Kleen, Peter Zijlstra, Jiri Olsa, LKML,
Stephane Eranian, Adrian Hunter, Alexander Shishkin, yao.jin,
Ingo Molnar, Paul Mackerras, Arnaldo Carvalho de Melo,
Robert Richter, Namhyung Kim, linuxppc-dev, Alexey Budankov,
Liang, Kan
In-Reply-To: <f226f4c5-6310-fd6b-ee76-aebd938ec212@amd.com>
On 3/13/20 4:08 AM, Kim Phillips wrote:
> On 3/11/20 11:00 AM, Ravi Bangoria wrote:
>> Hi Kim,
> Hi Ravi,
>
>> On 3/6/20 3:36 AM, Kim Phillips wrote:
>>>> On 3/3/20 3:55 AM, Kim Phillips wrote:
>>>>> On 3/2/20 2:21 PM, Stephane Eranian wrote:
>>>>>> On Mon, Mar 2, 2020 at 2:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
>>>>>>> On Mon, Mar 02, 2020 at 10:53:44AM +0530, Ravi Bangoria wrote:
>>>>>>>> Modern processors export such hazard data in Performance
>>>>>>>> Monitoring Unit (PMU) registers. Ex, 'Sampled Instruction Event
>>>>>>>> Register' on IBM PowerPC[1][2] and 'Instruction-Based Sampling' on
>>>>>>>> AMD[3] provides similar information.
>>>>>>>>
>>>>>>>> Implementation detail:
>>>>>>>>
>>>>>>>> A new sample_type called PERF_SAMPLE_PIPELINE_HAZ is introduced.
>>>>>>>> If it's set, kernel converts arch specific hazard information
>>>>>>>> into generic format:
>>>>>>>>
>>>>>>>> struct perf_pipeline_haz_data {
>>>>>>>> /* Instruction/Opcode type: Load, Store, Branch .... */
>>>>>>>> __u8 itype;
>>>>>>>> /* Instruction Cache source */
>>>>>>>> __u8 icache;
>>>>>>>> /* Instruction suffered hazard in pipeline stage */
>>>>>>>> __u8 hazard_stage;
>>>>>>>> /* Hazard reason */
>>>>>>>> __u8 hazard_reason;
>>>>>>>> /* Instruction suffered stall in pipeline stage */
>>>>>>>> __u8 stall_stage;
>>>>>>>> /* Stall reason */
>>>>>>>> __u8 stall_reason;
>>>>>>>> __u16 pad;
>>>>>>>> };
>>>>>>> Kim, does this format indeed work for AMD IBS?
>>>>> It's not really 1:1, we don't have these separations of stages
>>>>> and reasons, for example: we have missed in L2 cache, for example.
>>>>> So IBS output is flatter, with more cycle latency figures than
>>>>> IBM's AFAICT.
>>>> AMD IBS captures pipeline latency data incase Fetch sampling like the
>>>> Fetch latency, tag to retire latency, completion to retire latency and
>>>> so on. Yes, Ops sampling do provide more data on load/store centric
>>>> information. But it also captures more detailed data for Branch instructions.
>>>> And we also looked at ARM SPE, which also captures more details pipeline
>>>> data and latency information.
>>>>
>>>>>> Personally, I don't like the term hazard. This is too IBM Power
>>>>>> specific. We need to find a better term, maybe stall or penalty.
>>>>> Right, IBS doesn't have a filter to only count stalled or otherwise
>>>>> bad events. IBS' PPR descriptions has one occurrence of the
>>>>> word stall, and no penalty. The way I read IBS is it's just
>>>>> reporting more sample data than just the precise IP: things like
>>>>> hits, misses, cycle latencies, addresses, types, etc., so words
>>>>> like 'extended', or the 'auxiliary' already used today even
>>>>> are more appropriate for IBS, although I'm the last person to
>>>>> bikeshed.
>>>> We are thinking of using "pipeline" word instead of Hazard.
>>> Hm, the word 'pipeline' occurs 0 times in IBS documentation.
>> NP. We thought pipeline is generic hw term so we proposed "pipeline"
>> word. We are open to term which can be generic enough.
>>
>>> I realize there are a couple of core pipeline-specific pieces
>>> of information coming out of it, but the vast majority
>>> are addresses, latencies of various components in the memory
>>> hierarchy, and various component hit/miss bits.
>> Yes. we should capture core pipeline specific details. For example,
>> IBS generates Branch unit information(IbsOpData1) and Icahce related
>> data(IbsFetchCtl) which is something that shouldn't be extended as
>> part of perf-mem, IMO.
> Sure, IBS Op-side output is more 'perf mem' friendly, and so it
> should populate perf_mem_data_src fields, just like POWER9 can:
>
> union perf_mem_data_src {
> ...
> __u64 mem_rsvd:24,
> mem_snoopx:2, /* snoop mode, ext */
> mem_remote:1, /* remote */
> mem_lvl_num:4, /* memory hierarchy level number */
> mem_dtlb:7, /* tlb access */
> mem_lock:2, /* lock instr */
> mem_snoop:5, /* snoop mode */
> mem_lvl:14, /* memory hierarchy level */
> mem_op:5; /* type of opcode */
>
>
> E.g., SIER[LDST] SIER[A_XLATE_SRC] can be used to populate
> mem_lvl[_num], SIER_TYPE can be used to populate 'mem_op',
> 'mem_lock', and the Reload Bus Source Encoding bits can
> be used to populate mem_snoop, right?
Hi Kim,
Yes. We do expose these data as part of perf-mem for POWER.
> For IBS, I see PERF_SAMPLE_ADDR and PERF_SAMPLE_PHYS_ADDR can be
> used for the ld/st target addresses, too.
>
>>> What's needed here is a vendor-specific extended
>>> sample information that all these technologies gather,
>>> of which things like e.g., 'L1 TLB cycle latency' we
>>> all should have in common.
>> Yes. We will include fields to capture the latency cycles (like Issue
>> latency, Instruction completion latency etc..) along with other pipeline
>> details in the proposed structure.
> Latency figures are just an example, and from what I
> can tell, struct perf_sample_data already has a 'weight' member,
> used with PERF_SAMPLE_WEIGHT, that is used by intel-pt to
> transfer memory access latency figures. Granted, that's
> a bad name given all other vendors don't call latency
> 'weight'.
>
> I didn't see any latency figures coming out of POWER9,
> and do not expect this patchseries to implement those
> of other vendors, e.g., AMD's IBS; leave each vendor
> to amend perf to suit their own h/w output please.
Reference structure proposed in this patchset did not have members
to capture latency info for that exact reason. But idea here is to
abstract as vendor specific as possible. So if we include u16 array,
then this format can also capture data from IBS since it provides
few latency details.
>
> My main point there, however, was that each vendor should
> use streamlined record-level code to just copy the data
> in the proprietary format that their hardware produces,
> and then then perf tooling can synthesize the events
> from the raw data at report/script/etc. time.
>
>>> I'm not sure why a new PERF_SAMPLE_PIPELINE_HAZ is needed
>>> either. Can we use PERF_SAMPLE_AUX instead?
>> We took a look at PERF_SAMPLE_AUX. IIUC, PERF_SAMPLE_AUX is intended when
>> large volume of data needs to be captured as part of perf.data without
>> frequent PMIs. But proposed type is to address the capture of pipeline
> SAMPLE_AUX shouldn't care whether the volume is large, or how frequent
> PMIs are, even though it may be used in those environments.
>
>> information on each sample using PMI at periodic intervals. Hence proposing
>> PERF_SAMPLE_PIPELINE_HAZ.
> And that's fine for any extra bits that POWER9 has to convey
> to its users beyond things already represented by other sample
> types like PERF_SAMPLE_DATA_SRC, but the capturing of both POWER9
> and other vendor e.g., AMD IBS data can be made vendor-independent
> at record time by using SAMPLE_AUX, or SAMPLE_RAW even, which is
> what IBS currently uses.
My bad. Not sure what you mean by this. We are trying to abstract
as much vendor specific data as possible with this (like perf-mem).
Maddy
>
>>> Take a look at
>>> commit 98dcf14d7f9c "perf tools: Add kernel AUX area sampling
>>> definitions". The sample identifier can be used to determine
>>> which vendor's sampling IP's data is in it, and events can
>>> be recorded just by copying the content of the SIER, etc.
>>> registers, and then events get synthesized from the aux
>>> sample at report/inject/annotate etc. time. This allows
>>> for less sample recording overhead, and moves all the vendor
>>> specific decoding and common event conversions for userspace
>>> to figure out.
>> When AUX buffer data is structured, tool side changes added to present the
>> pipeline data can be re-used.
> Not sure I understand: AUX data would be structured on
> each vendor's raw h/w register formats.
>
> Thanks,
>
> Kim
>
>>>>>> Also worth considering is the support of ARM SPE (Statistical
>>>>>> Profiling Extension) which is their version of IBS.
>>>>>> Whatever gets added need to cover all three with no limitations.
>>>>> I thought Intel's various LBR, PEBS, and PT supported providing
>>>>> similar sample data in perf already, like with perf mem/c2c?
>>>> perf-mem is more of data centric in my opinion. It is more towards
>>>> memory profiling. So proposal here is to expose pipeline related
>>>> details like stalls and latencies.
>>> Like I said, I don't see it that way, I see it as "any particular
>>> vendor's event's extended details', and these pipeline details
>>> have overlap with existing infrastructure within perf, e.g., L2
>>> cache misses.
>>>
>>> Kim
>>>
^ permalink raw reply
* [PATCH v5 11/11] perf/tools/pmu-events/powerpc: Add hv_24x7 socket/chip level metric events
From: Kajol Jain @ 2020-03-17 6:23 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200317062333.14555-1-kjain@linux.ibm.com>
The hv_24×7 feature in IBM® POWER9™ processor-based servers provide the
facility to continuously collect large numbers of hardware performance
metrics efficiently and accurately.
This patch adds hv_24x7 metric file for different Socket/chip
resources.
Result:
power9 platform:
command:# ./perf stat --metric-only -M Memory_RD_BW_Chip -C 0 -I 1000
1.000096188 0.9 0.3
2.000285720 0.5 0.1
3.000424990 0.4 0.1
command:# ./perf stat --metric-only -M PowerBUS_Frequency -C 0 -I 1000
1.000097981 2.3 2.3
2.000291713 2.3 2.3
3.000421719 2.3 2.3
4.000550912 2.3 2.3
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
.../arch/powerpc/power9/nest_metrics.json | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json
diff --git a/tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json b/tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json
new file mode 100644
index 000000000000..ac38f5540ac6
--- /dev/null
+++ b/tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json
@@ -0,0 +1,19 @@
+[
+ {
+ "MetricExpr": "(hv_24x7@PM_MCS01_128B_RD_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS01_128B_RD_DISP_PORT23\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_RD_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_RD_DISP_PORT23\\,chip\\=?@)",
+ "MetricName": "Memory_RD_BW_Chip",
+ "MetricGroup": "Memory_BW",
+ "ScaleUnit": "1.6e-2MB"
+ },
+ {
+ "MetricExpr": "(hv_24x7@PM_MCS01_128B_WR_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS01_128B_WR_DISP_PORT23\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_WR_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_WR_DISP_PORT23\\,chip\\=?@ )",
+ "MetricName": "Memory_WR_BW_Chip",
+ "MetricGroup": "Memory_BW",
+ "ScaleUnit": "1.6e-2MB"
+ },
+ {
+ "MetricExpr": "(hv_24x7@PM_PB_CYC\\,chip\\=?@ )",
+ "MetricName": "PowerBUS_Frequency",
+ "ScaleUnit": "2.5e-7GHz"
+ }
+]
--
2.18.1
^ permalink raw reply related
* [PATCH v5 10/11] tools/perf: Enable Hz/hz prinitg for --metric-only option
From: Kajol Jain @ 2020-03-17 6:23 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200317062333.14555-1-kjain@linux.ibm.com>
Commit 54b5091606c18 ("perf stat: Implement --metric-only mode")
added function 'valid_only_metric()' which drops "Hz" or "hz",
if it is part of "ScaleUnit". This patch enable it since hv_24x7
supports couple of frequency events.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
tools/perf/util/stat-display.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 16efdba1973a..ecdebfcdd379 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -237,8 +237,6 @@ static bool valid_only_metric(const char *unit)
if (!unit)
return false;
if (strstr(unit, "/sec") ||
- strstr(unit, "hz") ||
- strstr(unit, "Hz") ||
strstr(unit, "CPUs utilized"))
return false;
return true;
--
2.18.1
^ permalink raw reply related
* [PATCH v5 09/11] perf/tools: Enhance JSON/metric infrastructure to handle "?"
From: Kajol Jain @ 2020-03-17 6:23 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200317062333.14555-1-kjain@linux.ibm.com>
Patch enhances current metric infrastructure to handle "?" in the metric
expression. The "?" can be use for parameters whose value not known while
creating metric events and which can be replace later at runtime to
the proper value. It also add flexibility to create multiple events out
of single metric event added in json file.
Patch adds function 'arch_get_runtimeparam' which is a arch specific
function, returns the count of metric events need to be created.
By default it return 1.
One loop is added in function 'metricgroup__add_metric_runtime_param',
which create multiple events at run time depend on return value of
'arch_get_runtimeparam' and merge that event in 'group_list'.
This infrastructure needed for hv_24x7 socket/chip level events.
"hv_24x7" chip level events needs specific chip-id to which the
data is requested. Function 'arch_get_runtimeparam' implemented
in header.c which extract number of sockets from sysfs file
"sockets" under "/sys/devices/hv_24x7/interface/".
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
| 10 ++++
tools/perf/tests/expr.c | 8 ++--
tools/perf/util/expr.c | 11 +++--
tools/perf/util/expr.h | 5 +-
tools/perf/util/expr.l | 27 ++++++++---
tools/perf/util/metricgroup.c | 66 ++++++++++++++++++++++++++-
tools/perf/util/metricgroup.h | 1 +
tools/perf/util/stat-shadow.c | 12 ++++-
8 files changed, 119 insertions(+), 21 deletions(-)
--git a/tools/perf/arch/powerpc/util/header.c b/tools/perf/arch/powerpc/util/header.c
index 3b4cdfc5efd6..dcc3c6ab2e67 100644
--- a/tools/perf/arch/powerpc/util/header.c
+++ b/tools/perf/arch/powerpc/util/header.c
@@ -7,6 +7,8 @@
#include <string.h>
#include <linux/stringify.h>
#include "header.h"
+#include "metricgroup.h"
+#include <api/fs/fs.h>
#define mfspr(rn) ({unsigned long rval; \
asm volatile("mfspr %0," __stringify(rn) \
@@ -16,6 +18,8 @@
#define PVR_VER(pvr) (((pvr) >> 16) & 0xFFFF) /* Version field */
#define PVR_REV(pvr) (((pvr) >> 0) & 0xFFFF) /* Revison field */
+#define SOCKETS_INFO_FILE_PATH "/devices/hv_24x7/interface/sockets"
+
int
get_cpuid(char *buffer, size_t sz)
{
@@ -44,3 +48,9 @@ get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
return bufp;
}
+
+int arch_get_runtimeparam(void)
+{
+ int count;
+ return sysfs__read_int(SOCKETS_INFO_FILE_PATH, &count) < 0 ? 1 : count;
+}
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index ea10fc4412c4..516504cf0ea5 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -10,7 +10,7 @@ static int test(struct expr_parse_ctx *ctx, const char *e, double val2)
{
double val;
- if (expr__parse(&val, ctx, e))
+ if (expr__parse(&val, ctx, e, 1))
TEST_ASSERT_VAL("parse test failed", 0);
TEST_ASSERT_VAL("unexpected value", val == val2);
return 0;
@@ -44,15 +44,15 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
return ret;
p = "FOO/0";
- ret = expr__parse(&val, &ctx, p);
+ ret = expr__parse(&val, &ctx, p, 1);
TEST_ASSERT_VAL("division by zero", ret == -1);
p = "BAR/";
- ret = expr__parse(&val, &ctx, p);
+ ret = expr__parse(&val, &ctx, p, 1);
TEST_ASSERT_VAL("missing operand", ret == -1);
TEST_ASSERT_VAL("find other",
- expr__find_other("FOO + BAR + BAZ + BOZO", "FOO", &other, &num_other) == 0);
+ expr__find_other("FOO + BAR + BAZ + BOZO", "FOO", &other, &num_other, 1) == 0);
TEST_ASSERT_VAL("find other", num_other == 3);
TEST_ASSERT_VAL("find other", !strcmp(other[0], "BAR"));
TEST_ASSERT_VAL("find other", !strcmp(other[1], "BAZ"));
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index c3382d58cf40..b228b737a5b0 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -27,10 +27,11 @@ void expr__ctx_init(struct expr_parse_ctx *ctx)
static int
__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
- int start)
+ int start, int param)
{
struct expr_scanner_ctx scanner_ctx = {
.start_token = start,
+ .expr__runtimeparam = param,
};
YY_BUFFER_STATE buffer;
void *scanner;
@@ -54,9 +55,9 @@ __expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
return ret;
}
-int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr)
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr, int param)
{
- return __expr__parse(final_val, ctx, expr, EXPR_PARSE) ? -1 : 0;
+ return __expr__parse(final_val, ctx, expr, EXPR_PARSE, param) ? -1 : 0;
}
static bool
@@ -74,13 +75,13 @@ already_seen(const char *val, const char *one, const char **other,
}
int expr__find_other(const char *expr, const char *one, const char ***other,
- int *num_other)
+ int *num_other, int param)
{
int err, i = 0, j = 0;
struct expr_parse_ctx ctx;
expr__ctx_init(&ctx);
- err = __expr__parse(NULL, &ctx, expr, EXPR_OTHER);
+ err = __expr__parse(NULL, &ctx, expr, EXPR_OTHER, param);
if (err)
return -1;
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 0938ad166ece..7786829b048b 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -17,12 +17,13 @@ struct expr_parse_ctx {
struct expr_scanner_ctx {
int start_token;
+ int expr__runtimeparam;
};
void expr__ctx_init(struct expr_parse_ctx *ctx);
void expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val);
-int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr);
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr, int param);
int expr__find_other(const char *expr, const char *one, const char ***other,
- int *num_other);
+ int *num_other, int param);
#endif
diff --git a/tools/perf/util/expr.l b/tools/perf/util/expr.l
index 2582c2464938..bde1e4543829 100644
--- a/tools/perf/util/expr.l
+++ b/tools/perf/util/expr.l
@@ -35,7 +35,7 @@ static int value(yyscan_t scanner, int base)
* Allow @ instead of / to be able to specify pmu/event/ without
* conflicts with normal division.
*/
-static char *normalize(char *str)
+static char *normalize(char *str, int expr__runtimeparam)
{
char *ret = str;
char *dst = str;
@@ -45,6 +45,19 @@ static char *normalize(char *str)
*dst++ = '/';
else if (*str == '\\')
*dst++ = *++str;
+ else if (*str == '?') {
+ char *paramval;
+ int i = 0;
+ int size = asprintf(¶mval, "%d", expr__runtimeparam);
+
+ if (size < 0)
+ *dst++ = '0';
+ else {
+ while (i < size)
+ *dst++ = paramval[i++];
+ free(paramval);
+ }
+ }
else
*dst++ = *str;
str++;
@@ -54,16 +67,16 @@ static char *normalize(char *str)
return ret;
}
-static int str(yyscan_t scanner, int token)
+static int str(yyscan_t scanner, int token, int expr__runtimeparam)
{
YYSTYPE *yylval = expr_get_lval(scanner);
char *text = expr_get_text(scanner);
- yylval->str = normalize(strdup(text));
+ yylval->str = normalize(strdup(text), expr__runtimeparam);
if (!yylval->str)
return EXPR_ERROR;
- yylval->str = normalize(yylval->str);
+ yylval->str = normalize(yylval->str, expr__runtimeparam);
return token;
}
%}
@@ -72,8 +85,8 @@ number [0-9]+
sch [-,=]
spec \\{sch}
-sym [0-9a-zA-Z_\.:@]+
-symbol {spec}*{sym}*{spec}*{sym}*
+sym [0-9a-zA-Z_\.:@?]+
+symbol {spec}*{sym}*{spec}*{sym}*{spec}*{sym}
%%
struct expr_scanner_ctx *sctx = expr_get_extra(yyscanner);
@@ -93,7 +106,7 @@ if { return IF; }
else { return ELSE; }
#smt_on { return SMT_ON; }
{number} { return value(yyscanner, 10); }
-{symbol} { return str(yyscanner, ID); }
+{symbol} { return str(yyscanner, ID, sctx->expr__runtimeparam); }
"|" { return '|'; }
"^" { return '^'; }
"&" { return '&'; }
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index b4919bcfbd8b..ec1742b63f44 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -474,6 +474,66 @@ static bool metricgroup__has_constraint(struct pmu_event *pe)
return false;
}
+int __weak arch_get_runtimeparam(void)
+{
+ return 1;
+}
+
+static int metricgroup__add_metric_runtime_param(struct strbuf *events,
+ struct list_head *group_list, struct pmu_event *pe)
+{
+ int i, count;
+ int ret = -EINVAL;
+
+ count = arch_get_runtimeparam();
+
+ /* This loop is added to create multiple
+ * events depend on count value and add
+ * those events to group_list.
+ */
+
+ for (i = 0; i < count; i++) {
+ const char **ids;
+ int idnum;
+ struct egroup *eg;
+ char value[PATH_MAX];
+
+ if (expr__find_other(pe->metric_expr, NULL, &ids, &idnum, i) < 0)
+ return ret;
+
+ if (events->len > 0)
+ strbuf_addf(events, ",");
+
+ if (metricgroup__has_constraint(pe))
+ metricgroup__add_metric_non_group(events, ids, idnum);
+ else
+ metricgroup__add_metric_weak_group(events, ids, idnum);
+
+ eg = malloc(sizeof(*eg));
+ if (!eg) {
+ ret = -ENOMEM;
+ return ret;
+ }
+
+ sprintf(value, "%s%c%d", pe->metric_name, '_', i);
+ eg->ids = ids;
+ eg->idnum = idnum;
+ eg->metric_name = strdup(value);
+ if (!eg->metric_name) {
+ ret = -ENOMEM;
+ return ret;
+ }
+
+ eg->metric_expr = pe->metric_expr;
+ eg->metric_unit = pe->unit;
+ list_add_tail(&eg->nd, group_list);
+ ret = 0;
+
+ if (ret != 0)
+ break;
+ }
+ return ret;
+}
static int metricgroup__add_metric_param(struct strbuf *events,
struct list_head *group_list, struct pmu_event *pe)
{
@@ -531,7 +591,11 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
- ret = metricgroup__add_metric_param(events,
+ if (strstr(pe->metric_expr, "?"))
+ ret = metricgroup__add_metric_runtime_param(events,
+ group_list, pe);
+ else
+ ret = metricgroup__add_metric_param(events,
group_list, pe);
if (ret == -EINVAL)
continue;
diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
index 475c7f912864..81224ba1270d 100644
--- a/tools/perf/util/metricgroup.h
+++ b/tools/perf/util/metricgroup.h
@@ -34,4 +34,5 @@ int metricgroup__parse_groups(const struct option *opt,
void metricgroup__print(bool metrics, bool groups, char *filter,
bool raw, bool details);
bool metricgroup__has_metric(const char *metric);
+int arch_get_runtimeparam(void);
#endif
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 402af3e8d287..85ac6d913782 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -336,7 +336,7 @@ void perf_stat__collect_metric_expr(struct evlist *evsel_list)
metric_events = counter->metric_events;
if (!metric_events) {
if (expr__find_other(counter->metric_expr, counter->name,
- &metric_names, &num_metric_names) < 0)
+ &metric_names, &num_metric_names, 1) < 0)
continue;
metric_events = calloc(sizeof(struct evsel *),
@@ -777,7 +777,15 @@ static void generic_metric(struct perf_stat_config *config,
}
if (!metric_events[i]) {
- if (expr__parse(&ratio, &pctx, metric_expr) == 0) {
+ int param = 1;
+ if (strstr(metric_expr, "?")) {
+ char *tmp = strrchr(metric_name, '_');
+
+ tmp++;
+ param = strtol(tmp, &tmp, 10);
+ }
+
+ if (expr__parse(&ratio, &pctx, metric_expr, param) == 0) {
char *unit;
char metric_bf[64];
--
2.18.1
^ permalink raw reply related
* [PATCH v5 08/11] perf/tools: Refactoring metricgroup__add_metric function
From: Kajol Jain @ 2020-03-17 6:23 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200317062333.14555-1-kjain@linux.ibm.com>
This patch refactor metricgroup__add_metric function where
some part of it move to function metricgroup__add_metric_param.
No logic change.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
tools/perf/util/metricgroup.c | 63 +++++++++++++++++++++--------------
1 file changed, 38 insertions(+), 25 deletions(-)
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index c3a8c701609a..b4919bcfbd8b 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -474,6 +474,41 @@ static bool metricgroup__has_constraint(struct pmu_event *pe)
return false;
}
+static int metricgroup__add_metric_param(struct strbuf *events,
+ struct list_head *group_list, struct pmu_event *pe)
+{
+
+ const char **ids;
+ int idnum;
+ struct egroup *eg;
+ int ret = -EINVAL;
+
+ if (expr__find_other(pe->metric_expr, NULL, &ids, &idnum, 1) < 0)
+ return ret;
+
+ if (events->len > 0)
+ strbuf_addf(events, ",");
+
+ if (metricgroup__has_constraint(pe))
+ metricgroup__add_metric_non_group(events, ids, idnum);
+ else
+ metricgroup__add_metric_weak_group(events, ids, idnum);
+
+ eg = malloc(sizeof(*eg));
+ if (!eg)
+ ret = -ENOMEM;
+
+ eg->ids = ids;
+ eg->idnum = idnum;
+ eg->metric_name = pe->metric_name;
+ eg->metric_expr = pe->metric_expr;
+ eg->metric_unit = pe->unit;
+ list_add_tail(&eg->nd, group_list);
+ ret = 0;
+
+ return ret;
+}
+
static int metricgroup__add_metric(const char *metric, struct strbuf *events,
struct list_head *group_list)
{
@@ -493,35 +528,13 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
continue;
if (match_metric(pe->metric_group, metric) ||
match_metric(pe->metric_name, metric)) {
- const char **ids;
- int idnum;
- struct egroup *eg;
pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
- if (expr__find_other(pe->metric_expr,
- NULL, &ids, &idnum) < 0)
+ ret = metricgroup__add_metric_param(events,
+ group_list, pe);
+ if (ret == -EINVAL)
continue;
- if (events->len > 0)
- strbuf_addf(events, ",");
-
- if (metricgroup__has_constraint(pe))
- metricgroup__add_metric_non_group(events, ids, idnum);
- else
- metricgroup__add_metric_weak_group(events, ids, idnum);
-
- eg = malloc(sizeof(struct egroup));
- if (!eg) {
- ret = -ENOMEM;
- break;
- }
- eg->ids = ids;
- eg->idnum = idnum;
- eg->metric_name = pe->metric_name;
- eg->metric_expr = pe->metric_expr;
- eg->metric_unit = pe->unit;
- list_add_tail(&eg->nd, group_list);
- ret = 0;
}
}
return ret;
--
2.18.1
^ permalink raw reply related
* [PATCH v5 07/11] powerpc/hv-24x7: Update post_mobility_fixup() to handle migration
From: Kajol Jain @ 2020-03-17 6:23 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200317062333.14555-1-kjain@linux.ibm.com>
Function 'read_sys_info_pseries()' is added to get system parameter
values like number of sockets and chips per socket.
and it gets these details via rtas_call with token
"PROCESSOR_MODULE_INFO".
Incase lpar migrate from one system to another, system
parameter details like chips per sockets or number of sockets might
change. So, it needs to be re-initialized otherwise, these values
corresponds to previous system values.
This patch adds a call to 'read_sys_info_pseries()' from
'post-mobility_fixup()' to re-init the physsockets and physchips values.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/platforms/pseries/mobility.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index b571285f6c14..226accd6218b 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -371,6 +371,18 @@ void post_mobility_fixup(void)
/* Possibly switch to a new RFI flush type */
pseries_setup_rfi_flush();
+ /*
+ * Incase lpar migrate from one system to another, system
+ * parameter details like chips per sockets and number of sockets
+ * might change. So, it needs to be re-initialized otherwise these
+ * values corresponds to previous system.
+ * Here, adding a call to read_sys_info_pseries() declared in
+ * platforms/pseries/pseries.h to re-init the physsockets and
+ * physchips value.
+ */
+ if (IS_ENABLED(CONFIG_HV_PERF_CTRS) && IS_ENABLED(CONFIG_PPC_RTAS))
+ read_sys_info_pseries();
+
return;
}
--
2.18.1
^ permalink raw reply related
* [PATCH v5 06/11] Documentation/ABI: Add ABI documentation for chips and sockets
From: Kajol Jain @ 2020-03-17 6:23 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200317062333.14555-1-kjain@linux.ibm.com>
Add documentation for the following sysfs files:
/sys/devices/hv_24x7/interface/chips,
/sys/devices/hv_24x7/interface/sockets
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
.../testing/sysfs-bus-event_source-devices-hv_24x7 | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7 b/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
index ec27c6c9e737..e17e5b444a1c 100644
--- a/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
+++ b/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
@@ -22,6 +22,20 @@ Description:
Exposes the "version" field of the 24x7 catalog. This is also
extractable from the provided binary "catalog" sysfs entry.
+What: /sys/devices/hv_24x7/interface/sockets
+Date: March 2020
+Contact: Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
+Description: read only
+ This sysfs interface exposes the number of sockets present in the
+ system.
+
+What: /sys/devices/hv_24x7/interface/chips
+Date: March 2020
+Contact: Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
+Description: read only
+ This sysfs interface exposes the number of chips per socket
+ present in the system.
+
What: /sys/bus/event_source/devices/hv_24x7/event_descs/<event-name>
Date: February 2014
Contact: Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
--
2.18.1
^ permalink raw reply related
* [PATCH v5 05/11] powerpc/hv-24x7: Add sysfs files inside hv-24x7 device to show processor details
From: Kajol Jain @ 2020-03-17 6:23 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200317062333.14555-1-kjain@linux.ibm.com>
To expose the system dependent parameter like total number of
sockets and numbers of chips per socket, patch adds two sysfs files.
"sockets" and "chips" are added to /sys/devices/hv_24x7/interface/
of the "hv_24x7" pmu.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/perf/hv-24x7.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 9ae00f29bd21..a31bd5b88f7a 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -454,6 +454,20 @@ static ssize_t device_show_string(struct device *dev,
return sprintf(buf, "%s\n", (char *)d->var);
}
+#ifdef CONFIG_PPC_RTAS
+static ssize_t sockets_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", physsockets);
+}
+
+static ssize_t chips_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%d\n", physchips);
+}
+#endif
+
static struct attribute *device_str_attr_create_(char *name, char *str)
{
struct dev_ext_attribute *attr = kzalloc(sizeof(*attr), GFP_KERNEL);
@@ -1100,6 +1114,10 @@ PAGE_0_ATTR(catalog_len, "%lld\n",
(unsigned long long)be32_to_cpu(page_0->length) * 4096);
static BIN_ATTR_RO(catalog, 0/* real length varies */);
static DEVICE_ATTR_RO(domains);
+#ifdef CONFIG_PPC_RTAS
+static DEVICE_ATTR_RO(sockets);
+static DEVICE_ATTR_RO(chips);
+#endif
static struct bin_attribute *if_bin_attrs[] = {
&bin_attr_catalog,
@@ -1110,6 +1128,10 @@ static struct attribute *if_attrs[] = {
&dev_attr_catalog_len.attr,
&dev_attr_catalog_version.attr,
&dev_attr_domains.attr,
+#ifdef CONFIG_PPC_RTAS
+ &dev_attr_sockets.attr,
+ &dev_attr_chips.attr,
+#endif
NULL,
};
--
2.18.1
^ permalink raw reply related
* [PATCH v5 04/11] powerpc/hv-24x7: Add rtas call in hv-24x7 driver to get processor details
From: Kajol Jain @ 2020-03-17 6:23 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200317062333.14555-1-kjain@linux.ibm.com>
For hv_24x7 socket/chip level events, specific chip-id to which
the data requested should be added as part of pmu events.
But number of chips/socket in the system details are not exposed.
Patch implements read_sys_info_pseries() to get system
parameter values like number of sockets and chips per socket.
Rtas_call with token "PROCESSOR_MODULE_INFO"
is used to get these values.
Sub-sequent patch exports these values via sysfs.
Patch also make these parameters default to 1.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/perf/hv-24x7.c | 72 ++++++++++++++++++++++++
arch/powerpc/platforms/pseries/pseries.h | 3 +
2 files changed, 75 insertions(+)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 48e8f4b17b91..9ae00f29bd21 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -20,6 +20,11 @@
#include <asm/io.h>
#include <linux/byteorder/generic.h>
+#ifdef CONFIG_PPC_RTAS
+#include <asm/rtas.h>
+#include <../../platforms/pseries/pseries.h>
+#endif
+
#include "hv-24x7.h"
#include "hv-24x7-catalog.h"
#include "hv-common.h"
@@ -57,6 +62,69 @@ static bool is_physical_domain(unsigned domain)
}
}
+#ifdef CONFIG_PPC_RTAS
+#define PROCESSOR_MODULE_INFO 43
+#define PROCESSOR_MAX_LENGTH (8 * 1024)
+
+static int strbe16toh(const char *buf, int offset)
+{
+ return (buf[offset] << 8) + buf[offset + 1];
+}
+
+static u32 physsockets; /* Physical sockets */
+static u32 physchips; /* Physical chips */
+
+/*
+ * Function read_sys_info_pseries() make a rtas_call which require
+ * data buffer of size 8K. As standard 'rtas_data_buf' is of size
+ * 4K, we are adding new local buffer 'rtas_local_data_buf'.
+ */
+char rtas_local_data_buf[PROCESSOR_MAX_LENGTH] __cacheline_aligned;
+
+/*
+ * read_sys_info_pseries()
+ * Retrieve the number of sockets and chips per socket details
+ * through the get-system-parameter rtas call.
+ */
+void read_sys_info_pseries(void)
+{
+ int call_status, len, ntypes;
+
+ /*
+ * Making system parameter: chips and sockets default to 1.
+ */
+ physsockets = 1;
+ physchips = 1;
+ memset(rtas_local_data_buf, 0, PROCESSOR_MAX_LENGTH);
+ spin_lock(&rtas_data_buf_lock);
+
+ call_status = rtas_call(rtas_token("ibm,get-system-parameter"), 3, 1,
+ NULL,
+ PROCESSOR_MODULE_INFO,
+ __pa(rtas_local_data_buf),
+ PROCESSOR_MAX_LENGTH);
+
+ spin_unlock(&rtas_data_buf_lock);
+
+ if (call_status != 0) {
+ pr_info("%s %s Error calling get-system-parameter (0x%x)\n",
+ __FILE__, __func__, call_status);
+ } else {
+ rtas_local_data_buf[PROCESSOR_MAX_LENGTH - 1] = '\0';
+ len = strbe16toh(rtas_local_data_buf, 0);
+ if (len < 6)
+ return;
+
+ ntypes = strbe16toh(rtas_local_data_buf, 2);
+
+ if (!ntypes)
+ return;
+ physsockets = strbe16toh(rtas_local_data_buf, 4);
+ physchips = strbe16toh(rtas_local_data_buf, 6);
+ }
+}
+#endif /* CONFIG_PPC_RTAS */
+
/* Domains for which more than one result element are returned for each event. */
static bool domain_needs_aggregation(unsigned int domain)
{
@@ -1605,6 +1673,10 @@ static int hv_24x7_init(void)
if (r)
return r;
+#ifdef CONFIG_PPC_RTAS
+ read_sys_info_pseries();
+#endif
+
return 0;
}
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 13fa370a87e4..1727559ce304 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -19,6 +19,9 @@ extern void request_event_sources_irqs(struct device_node *np,
struct pt_regs;
extern int pSeries_system_reset_exception(struct pt_regs *regs);
+#ifdef CONFIG_PPC_RTAS
+extern void read_sys_info_pseries(void);
+#endif
extern int pSeries_machine_check_exception(struct pt_regs *regs);
extern long pseries_machine_check_realmode(struct pt_regs *regs);
--
2.18.1
^ permalink raw reply related
* [PATCH v5 03/11] powerpc/perf/hv-24x7: Fix inconsistent output values incase multiple hv-24x7 events run
From: Kajol Jain @ 2020-03-17 6:23 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200317062333.14555-1-kjain@linux.ibm.com>
Commit 2b206ee6b0df ("powerpc/perf/hv-24x7: Display change in counter
values")' added to print _change_ in the counter value rather then raw
value for 24x7 counters. Incase of transactions, the event count
is set to 0 at the beginning of the transaction. It also sets
the event's prev_count to the raw value at the time of initialization.
Because of setting event count to 0, we are seeing some weird behaviour,
whenever we run multiple 24x7 events at a time.
For example:
command#: ./perf stat -e "{hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/,
hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/}"
-C 0 -I 1000 sleep 100
1.000121704 120 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
1.000121704 5 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
2.000357733 8 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
2.000357733 10 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
3.000495215 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
3.000495215 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
4.000641884 56 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
4.000641884 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
5.000791887 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
Getting these large values in case we do -I.
As we are setting event_count to 0, for interval case, overall event_count is not
coming in incremental order. As we may can get new delta lesser then previous count.
Because of which when we print intervals, we are getting negative value which create
these large values.
This patch removes part where we set event_count to 0 in function
'h_24x7_event_read'. There won't be much impact as we do set event->hw.prev_count
to the raw value at the time of initialization to print change value.
With this patch
In power9 platform
command#: ./perf stat -e "{hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/,
hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/}"
-C 0 -I 1000 sleep 100
1.000117685 93 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
1.000117685 1 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
2.000349331 98 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
2.000349331 2 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
3.000495900 131 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
3.000495900 4 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
4.000645920 204 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
4.000645920 61 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
4.284169997 22 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Suggested-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
arch/powerpc/perf/hv-24x7.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 573e0b309c0c..48e8f4b17b91 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -1400,16 +1400,6 @@ static void h_24x7_event_read(struct perf_event *event)
h24x7hw = &get_cpu_var(hv_24x7_hw);
h24x7hw->events[i] = event;
put_cpu_var(h24x7hw);
- /*
- * Clear the event count so we can compute the _change_
- * in the 24x7 raw counter value at the end of the txn.
- *
- * Note that we could alternatively read the 24x7 value
- * now and save its value in event->hw.prev_count. But
- * that would require issuing a hcall, which would then
- * defeat the purpose of using the txn interface.
- */
- local64_set(&event->count, 0);
}
put_cpu_var(hv_24x7_reqb);
--
2.18.1
^ permalink raw reply related
* [PATCH v5 02/11] perf expr: Add expr_scanner_ctx object
From: Kajol Jain @ 2020-03-17 6:23 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200317062333.14555-1-kjain@linux.ibm.com>
From: Jiri Olsa <jolsa@kernel.org>
Adding expr_scanner_ctx object to hold user data
for the expr scanner. Currently it holds only
start_token, Kajol Jain will use it to hold 24x7
runtime param.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/expr.c | 6 ++++--
tools/perf/util/expr.h | 4 ++++
tools/perf/util/expr.l | 10 +++++-----
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index c8ccc548a585..c3382d58cf40 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -3,7 +3,6 @@
#include <assert.h>
#include "expr.h"
#include "expr-bison.h"
-#define YY_EXTRA_TYPE int
#include "expr-flex.h"
#ifdef PARSER_DEBUG
@@ -30,11 +29,14 @@ static int
__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
int start)
{
+ struct expr_scanner_ctx scanner_ctx = {
+ .start_token = start,
+ };
YY_BUFFER_STATE buffer;
void *scanner;
int ret;
- ret = expr_lex_init_extra(start, &scanner);
+ ret = expr_lex_init_extra(&scanner_ctx, &scanner);
if (ret)
return ret;
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index b9e53f2b5844..0938ad166ece 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -15,6 +15,10 @@ struct expr_parse_ctx {
struct expr_parse_id ids[MAX_PARSE_ID];
};
+struct expr_scanner_ctx {
+ int start_token;
+};
+
void expr__ctx_init(struct expr_parse_ctx *ctx);
void expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val);
int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr);
diff --git a/tools/perf/util/expr.l b/tools/perf/util/expr.l
index eaad29243c23..2582c2464938 100644
--- a/tools/perf/util/expr.l
+++ b/tools/perf/util/expr.l
@@ -76,13 +76,13 @@ sym [0-9a-zA-Z_\.:@]+
symbol {spec}*{sym}*{spec}*{sym}*
%%
- {
- int start_token;
+ struct expr_scanner_ctx *sctx = expr_get_extra(yyscanner);
- start_token = expr_get_extra(yyscanner);
+ {
+ int start_token = sctx->start_token;
- if (start_token) {
- expr_set_extra(NULL, yyscanner);
+ if (sctx->start_token) {
+ sctx->start_token = 0;
return start_token;
}
}
--
2.18.1
^ permalink raw reply related
* [PATCH v5 01/11] perf expr: Add expr_ prefix for parse_ctx and parse_id
From: Kajol Jain @ 2020-03-17 6:23 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200317062333.14555-1-kjain@linux.ibm.com>
From: Jiri Olsa <jolsa@kernel.org>
Adding expr_ prefix for parse_ctx and parse_id,
to straighten out the expr* namespace.
There's no functional change.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/tests/expr.c | 4 ++--
tools/perf/util/expr.c | 10 +++++-----
tools/perf/util/expr.h | 12 ++++++------
tools/perf/util/expr.y | 6 +++---
tools/perf/util/stat-shadow.c | 2 +-
5 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index 28313e59d6f6..ea10fc4412c4 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -6,7 +6,7 @@
#include <string.h>
#include <linux/zalloc.h>
-static int test(struct parse_ctx *ctx, const char *e, double val2)
+static int test(struct expr_parse_ctx *ctx, const char *e, double val2)
{
double val;
@@ -22,7 +22,7 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
const char **other;
double val;
int i, ret;
- struct parse_ctx ctx;
+ struct expr_parse_ctx ctx;
int num_other;
expr__ctx_init(&ctx);
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index fd192ddf93c1..c8ccc548a585 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -11,7 +11,7 @@ extern int expr_debug;
#endif
/* Caller must make sure id is allocated */
-void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
+void expr__add_id(struct expr_parse_ctx *ctx, const char *name, double val)
{
int idx;
@@ -21,13 +21,13 @@ void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
ctx->ids[idx].val = val;
}
-void expr__ctx_init(struct parse_ctx *ctx)
+void expr__ctx_init(struct expr_parse_ctx *ctx)
{
ctx->num_ids = 0;
}
static int
-__expr__parse(double *val, struct parse_ctx *ctx, const char *expr,
+__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
int start)
{
YY_BUFFER_STATE buffer;
@@ -52,7 +52,7 @@ __expr__parse(double *val, struct parse_ctx *ctx, const char *expr,
return ret;
}
-int expr__parse(double *final_val, struct parse_ctx *ctx, const char *expr)
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr)
{
return __expr__parse(final_val, ctx, expr, EXPR_PARSE) ? -1 : 0;
}
@@ -75,7 +75,7 @@ int expr__find_other(const char *expr, const char *one, const char ***other,
int *num_other)
{
int err, i = 0, j = 0;
- struct parse_ctx ctx;
+ struct expr_parse_ctx ctx;
expr__ctx_init(&ctx);
err = __expr__parse(NULL, &ctx, expr, EXPR_OTHER);
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 9377538f4097..b9e53f2b5844 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -5,19 +5,19 @@
#define EXPR_MAX_OTHER 20
#define MAX_PARSE_ID EXPR_MAX_OTHER
-struct parse_id {
+struct expr_parse_id {
const char *name;
double val;
};
-struct parse_ctx {
+struct expr_parse_ctx {
int num_ids;
- struct parse_id ids[MAX_PARSE_ID];
+ struct expr_parse_id ids[MAX_PARSE_ID];
};
-void expr__ctx_init(struct parse_ctx *ctx);
-void expr__add_id(struct parse_ctx *ctx, const char *id, double val);
-int expr__parse(double *final_val, struct parse_ctx *ctx, const char *expr);
+void expr__ctx_init(struct expr_parse_ctx *ctx);
+void expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val);
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr);
int expr__find_other(const char *expr, const char *one, const char ***other,
int *num_other);
diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index 4720cbe79357..cd17486c1c5d 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -15,7 +15,7 @@
%define api.pure full
%parse-param { double *final_val }
-%parse-param { struct parse_ctx *ctx }
+%parse-param { struct expr_parse_ctx *ctx }
%parse-param {void *scanner}
%lex-param {void* scanner}
@@ -39,14 +39,14 @@
%{
static void expr_error(double *final_val __maybe_unused,
- struct parse_ctx *ctx __maybe_unused,
+ struct expr_parse_ctx *ctx __maybe_unused,
void *scanner,
const char *s)
{
pr_debug("%s\n", s);
}
-static int lookup_id(struct parse_ctx *ctx, char *id, double *val)
+static int lookup_id(struct expr_parse_ctx *ctx, char *id, double *val)
{
int i;
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 0fd713d3674f..402af3e8d287 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -729,7 +729,7 @@ static void generic_metric(struct perf_stat_config *config,
struct runtime_stat *st)
{
print_metric_t print_metric = out->print_metric;
- struct parse_ctx pctx;
+ struct expr_parse_ctx pctx;
double ratio, scale;
int i;
void *ctxp = out->ctx;
--
2.18.1
^ permalink raw reply related
* [PATCH v5 00/11] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events
From: Kajol Jain @ 2020-03-17 6:23 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
Patchset fixes the inconsistent results we are getting when
we run multiple 24x7 events.
Patchset adds json file metric support for the hv_24x7 socket/chip level
events. "hv_24x7" pmu interface events needs system dependent parameter
like socket/chip/core. For example, hv_24x7 chip level events needs
specific chip-id to which the data is requested should be added as part
of pmu events.
So to enable JSON file support to "hv_24x7" interface, patchset expose
total number of sockets and chips per-socket details in sysfs
files (sockets, chips) under "/sys/devices/hv_24x7/interface/".
To get sockets and number of chips per sockets, patchset adds a rtas call
with token "PROCESSOR_MODULE_INFO" to get these details. Patchset also
handles partition migration case to re-init these system depended
parameters by adding proper calls in post_mobility_fixup() (mobility.c).
Second patch of the patchset adds expr_scanner_ctx object to hold user
data for the expr scanner, which can be used to hold runtime parameter.
Patch 9 & 11 of the patchset handles perf tool plumbing needed to replace
the "?" character in the metric expression to proper value and hv_24x7
json metric file for different Socket/chip resources.
Patch set also enable Hz/hz prinitg for --metric-only option to print
metric data for bus frequency.
Applied and tested all these patches cleanly on top of jiri's flex changes
with the changes done by Kan Liang for "Support metric group constraint"
patchset and made required changes.
Changelog:
v4 -> v5
- Using sysfs__read_int instead of sysfs__read_ull while reading
parameter value in powerpc/util/header.c file.
- Using asprintf rather then malloc and sprintf
Suggested by Arnaldo Carvalho de Melo
- Break patch 6 from previous version to two patch,
- One to add refactor current "metricgroup__add_metric" function
and another where actually "?" handling infra added.
- Add expr__runtimeparam as part of 'expr_scanner_ctx' struct
rather then making it global variable. Thanks Jiri for
adding this structure to hold user data for the expr scanner.
- Add runtime param as agrugement to function 'expr__find_other'
and 'expr__parse' and made changes on references accordingly.
v3 -> v4
- Apply these patch on top of Kan liang changes.
As suggested by Jiri.
v2 -> v3
- Remove setting event_count to 0 part in function 'h_24x7_event_read'
with comment rather then adding 0 to event_count value.
Suggested by: Sukadev Bhattiprolu
- Apply tool side changes require to replace "?" on Jiri's flex patch
series and made all require changes to make it compatible with added
flex change.
v1 -> v2
- Rename hv-24x7 metric json file as nest_metrics.json
Jiri Olsa (2):
perf expr: Add expr_ prefix for parse_ctx and parse_id
perf expr: Add expr_scanner_ctx object
Kajol Jain (9):
powerpc/perf/hv-24x7: Fix inconsistent output values incase multiple
hv-24x7 events run
powerpc/hv-24x7: Add rtas call in hv-24x7 driver to get processor
details
powerpc/hv-24x7: Add sysfs files inside hv-24x7 device to show
processor details
Documentation/ABI: Add ABI documentation for chips and sockets
powerpc/hv-24x7: Update post_mobility_fixup() to handle migration
perf/tools: Refactoring metricgroup__add_metric function
perf/tools: Enhance JSON/metric infrastructure to handle "?"
tools/perf: Enable Hz/hz prinitg for --metric-only option
perf/tools/pmu-events/powerpc: Add hv_24x7 socket/chip level metric
events
.../sysfs-bus-event_source-devices-hv_24x7 | 14 ++
arch/powerpc/perf/hv-24x7.c | 104 ++++++++++++--
arch/powerpc/platforms/pseries/mobility.c | 12 ++
arch/powerpc/platforms/pseries/pseries.h | 3 +
tools/perf/arch/powerpc/util/header.c | 10 ++
.../arch/powerpc/power9/nest_metrics.json | 19 +++
tools/perf/tests/expr.c | 12 +-
tools/perf/util/expr.c | 25 ++--
tools/perf/util/expr.h | 19 ++-
tools/perf/util/expr.l | 37 +++--
tools/perf/util/expr.y | 6 +-
tools/perf/util/metricgroup.c | 127 ++++++++++++++----
tools/perf/util/metricgroup.h | 1 +
tools/perf/util/stat-display.c | 2 -
tools/perf/util/stat-shadow.c | 14 +-
15 files changed, 326 insertions(+), 79 deletions(-)
create mode 100644 tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json
--
2.18.1
^ permalink raw reply
* Re: [PATCH 00/15] powerpc/watchpoint: Preparation for more than one watchpoint
From: Christophe Leroy @ 2020-03-17 5:56 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Ravi Bangoria, mikey, apopple, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200316184339.GB22482@gate.crashing.org>
Le 16/03/2020 à 19:43, Segher Boessenkool a écrit :
> On Mon, Mar 16, 2020 at 04:05:01PM +0100, Christophe Leroy wrote:
>> Some book3s (e300 family for instance, I think G2 as well) already have
>> a DABR2 in addition to DABR.
>
> The original "G2" (meaning 603 and 604) do not have DABR2. The newer
> "G2" (meaning e300) does have it. e500 and e600 do not have it either.
>
> Hope I got that right ;-)
>
>
G2 core reference manual says:
Features specific to the G2 core not present on the original MPC603e
(PID6-603e) processors follow:
...
Enhanced debug features
— Addition of three breakpoint registers—IABR2, DABR, and DABR2
— Two new breakpoint control registers—DBCR and IBCR
e500 has DAC1 and DAC2 instead for breakpoints iaw e500 core reference
manual.
Christophe
^ permalink raw reply
* Re: [PATCH V7 09/14] powerpc/vas: Update CSB and notify process for fault CRBs
From: Michael Ellerman @ 2020-03-17 5:28 UTC (permalink / raw)
To: Haren Myneni
Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev, ajd
In-Reply-To: <1583525805.9256.14.camel@hbabu-laptop>
Haren Myneni <haren@linux.ibm.com> writes:
> For each fault CRB, update fault address in CRB (fault_storage_addr)
> and translation error status in CSB so that user space can touch the
> fault address and resend the request. If the user space passed invalid
> CSB address send signal to process with SIGSEGV.
>
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
> arch/powerpc/platforms/powernv/vas-fault.c | 114 +++++++++++++++++++++++++++++
> 1 file changed, 114 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
> index 1c6d5cc..751ce48 100644
> --- a/arch/powerpc/platforms/powernv/vas-fault.c
> +++ b/arch/powerpc/platforms/powernv/vas-fault.c
> @@ -11,6 +11,7 @@
> #include <linux/slab.h>
> #include <linux/uaccess.h>
> #include <linux/kthread.h>
> +#include <linux/sched/signal.h>
> #include <linux/mmu_context.h>
> #include <asm/icswx.h>
>
> @@ -26,6 +27,118 @@
> #define VAS_FAULT_WIN_FIFO_SIZE (4 << 20)
>
> /*
> + * Update the CSB to indicate a translation error.
> + *
> + * If we are unable to update the CSB means copy_to_user failed due to
> + * invalid csb_addr, send a signal to the process.
> + *
> + * Remaining settings in the CSB are based on wait_for_csb() of
> + * NX-GZIP.
> + */
> +static void update_csb(struct vas_window *window,
> + struct coprocessor_request_block *crb)
> +{
> + int rc;
> + struct pid *pid;
> + void __user *csb_addr;
> + struct task_struct *tsk;
> + struct kernel_siginfo info;
> + struct coprocessor_status_block csb;
csb is on the stack, and later copied to user, which is a risk for
creating an infoleak.
Also please use reverse Christmas tree layout for your variables.
> +
> + /*
> + * NX user space windows can not be opened for task->mm=NULL
> + * and faults will not be generated for kernel requests.
> + */
> + if (!window->mm || !window->user_win)
> + return;
If that's a should-never-happen condition then should it do a
WARN_ON_ONCE() rather than silently returning?
> + csb_addr = (void __user *)be64_to_cpu(crb->csb_addr);
> +
> + csb.cc = CSB_CC_TRANSLATION;
> + csb.ce = CSB_CE_TERMINATION;
> + csb.cs = 0;
> + csb.count = 0;
> +
> + /*
> + * NX operates and returns in BE format as defined CRB struct.
> + * So return fault_storage_addr in BE as NX pastes in FIFO and
> + * expects user space to convert to CPU format.
> + */
> + csb.address = crb->stamp.nx.fault_storage_addr;
> + csb.flags = 0;
I'm pretty sure this has initialised all the fields of csb.
But, I'd still be much happier if you zeroed the whole struct to begin
with, that way we know for sure we can't leak any uninitialised bytes to
userspace. It's only 16 bytes so it shouldn't add any noticeable
overhead.
> +
> + pid = window->pid;
> + tsk = get_pid_task(pid, PIDTYPE_PID);
> + /*
> + * Send window will be closed after processing all NX requests
> + * and process exits after closing all windows. In multi-thread
> + * applications, thread may not exists, but does not close FD
> + * (means send window) upon exit. Parent thread (tgid) can use
> + * and close the window later.
> + * pid and mm references are taken when window is opened by
> + * process (pid). So tgid is used only when child thread opens
> + * a window and exits without closing it in multithread tasks.
> + */
> + if (!tsk) {
> + pid = window->tgid;
> + tsk = get_pid_task(pid, PIDTYPE_PID);
> + /*
> + * Parent thread will be closing window during its exit.
> + * So should not get here.
> + */
> + if (!tsk)
> + return;
Similar question on WARN_ON_ONCE()
> + }
> +
> + /* Return if the task is exiting. */
Why? Just because it's no use? It's racy isn't it, so it can't be for
correctness?
> + if (tsk->flags & PF_EXITING) {
> + put_task_struct(tsk);
> + return;
> + }
> +
> + use_mm(window->mm);
There's no check that csb_addr is actually pointing into userspace, but
copy_to_user() does it for you.
> + rc = copy_to_user(csb_addr, &csb, sizeof(csb));
> + /*
> + * User space polls on csb.flags (first byte). So add barrier
> + * then copy first byte with csb flags update.
> + */
> + smp_mb();
You only need to order the stores above vs the store below to csb.flags.
So you should only need an smp_wmb() here.
> + if (!rc) {
> + csb.flags = CSB_V;
> + rc = copy_to_user(csb_addr, &csb, sizeof(u8));
> + }
> + unuse_mm(window->mm);
> + put_task_struct(tsk);
> +
> + /* Success */
> + if (!rc)
> + return;
> +
> + pr_debug("Invalid CSB address 0x%p signalling pid(%d)\n",
> + csb_addr, pid_vnr(pid));
> +
> + clear_siginfo(&info);
> + info.si_signo = SIGSEGV;
> + info.si_errno = EFAULT;
> + info.si_code = SEGV_MAPERR;
> + info.si_addr = csb_addr;
> +
> + /*
> + * process will be polling on csb.flags after request is sent to
> + * NX. So generally CSB update should not fail except when an
> + * application does not follow the process properly. So an error
> + * message will be displayed and leave it to user space whether
> + * to ignore or handle this signal.
> + */
> + rcu_read_lock();
> + rc = kill_pid_info(SIGSEGV, &info, pid);
> + rcu_read_unlock();
Shouldn't this be using force_sig_fault_to_task() or another helper,
rather than open-coding?
> +
> + pr_devel("%s(): pid %d kill_proc_info() rc %d\n", __func__,
> + pid_vnr(pid), rc);
> +}
> +
> +/*
> * Process valid CRBs in fault FIFO.
> */
> irqreturn_t vas_fault_thread_fn(int irq, void *data)
> @@ -111,6 +224,7 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> + update_csb(window, crb);
> } while (true);
> }
>
> --
> 1.8.3.1
cheers
^ permalink raw reply
* Re: [PATCH V7 08/14] powerpc/vas: Take reference to PID and mm for user space windows
From: Michael Ellerman @ 2020-03-17 4:09 UTC (permalink / raw)
To: Haren Myneni
Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev, ajd
In-Reply-To: <1583525764.9256.13.camel@hbabu-laptop>
Haren Myneni <haren@linux.ibm.com> writes:
> Process close windows after its requests are completed. In multi-thread
> applications, child can open a window but release FD will not be called
> upon its exit. Parent thread will be closing it later upon its exit.
What if the parent exits first?
> The parent can also send NX requests with this window and NX can
> generate page faults. After kernel handles the page fault, send
> signal to process by using PID if CSB address is invalid. Parent
> thread will not receive signal since its PID is different from the one
> saved in vas_window. So use tgid in case if the task for the pid saved
> in window is not running and send signal to its parent.
>
> To prevent reusing the pid until the window closed, take reference to
> pid and task mm.
That text is all very dense. Can you please flesh it out and reword it
to clearly spell out what's going on in much more detail.
> diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
> index a45d81d..7587258 100644
> --- a/arch/powerpc/platforms/powernv/vas-window.c
> +++ b/arch/powerpc/platforms/powernv/vas-window.c
> @@ -1266,8 +1300,17 @@ int vas_win_close(struct vas_window *window)
> poll_window_castout(window);
>
> /* if send window, drop reference to matching receive window */
> - if (window->tx_win)
> + if (window->tx_win) {
> + if (window->user_win) {
> + /* Drop references to pid and mm */
> + put_pid(window->pid);
> + if (window->mm) {
> + mmdrop(window->mm);
> + mm_context_remove_copro(window->mm);
That seems backward. Once you drop the reference the mm can be freed
can't it?
> + }
> + }
> put_rx_win(window->rxwin);
> + }
>
> vas_window_free(window);
cheers
^ permalink raw reply
* Re: [PATCH v1 11/46] powerpc/ptdump: Display size of BATs
From: kbuild test robot @ 2020-03-17 3:25 UTC (permalink / raw)
To: Christophe Leroy; +Cc: kbuild-all, linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <755f29975957f229f445b600686b4fba9ee9399a.1584360344.git.christophe.leroy@c-s.fr>
[-- Attachment #1: Type: text/plain, Size: 1654 bytes --]
Hi Christophe,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on next-20200316]
[cannot apply to powerpc/next v5.6-rc6 v5.6-rc5 v5.6-rc4 v5.6-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/Use-hugepages-to-map-kernel-mem-on-8xx/20200317-065610
base: 8548fd2f20ed19b0e8c0585b71fdfde1ae00ae3c
config: powerpc-rhel-kconfig (attached as .config)
compiler: powerpc64le-linux-gcc (GCC) 9.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=9.2.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from arch/powerpc/mm/ptdump/book3s64.c:10:
>> arch/powerpc/mm/ptdump/ptdump.h:21:26: warning: 'struct seq_file' declared inside parameter list will not be visible outside of this definition or declaration
21 | void pt_dump_size(struct seq_file *m, unsigned long delta);
| ^~~~~~~~
vim +21 arch/powerpc/mm/ptdump/ptdump.h
20
> 21 void pt_dump_size(struct seq_file *m, unsigned long delta);
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 15571 bytes --]
^ permalink raw reply
* Re: [PATCH v4 3/3] powerpc/powernv: Parse device tree, population of SPR support
From: Michael Ellerman @ 2020-03-17 3:13 UTC (permalink / raw)
To: Pratik Rajesh Sampat, linux-kernel, linuxppc-dev, svaidy, ego,
linuxram, pratik.sampat, psampat, pratik.r.sampat
In-Reply-To: <1f5138b5080606804162b0a7cf20c134589b96b1.1581505210.git.psampat@linux.ibm.com>
Pratik Rajesh Sampat <psampat@linux.ibm.com> writes:
> Parse the device tree for nodes self-save, self-restore and populate
> support for the preferred SPRs based what was advertised by the device
> tree.
These should be documented in:
Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 97aeb45e897b..27dfadf609e8 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -1436,6 +1436,85 @@ static void __init pnv_probe_idle_states(void)
> supported_cpuidle_states |= pnv_idle_states[i].flags;
> }
>
> +/*
> + * Extracts and populates the self save or restore capabilities
> + * passed from the device tree node
> + */
> +static int extract_save_restore_state_dt(struct device_node *np, int type)
> +{
> + int nr_sprns = 0, i, bitmask_index;
> + int rc = 0;
> + u64 *temp_u64;
> + u64 bit_pos;
> +
> + nr_sprns = of_property_count_u64_elems(np, "sprn-bitmask");
> + if (nr_sprns <= 0)
> + return rc;
Using <= 0 means zero SPRs is treated by success as the caller, is that
intended? If so a comment would be appropriate.
> + temp_u64 = kcalloc(nr_sprns, sizeof(u64), GFP_KERNEL);
> + if (of_property_read_u64_array(np, "sprn-bitmask",
> + temp_u64, nr_sprns)) {
> + pr_warn("cpuidle-powernv: failed to find registers in DT\n");
> + kfree(temp_u64);
> + return -EINVAL;
> + }
> + /*
> + * Populate acknowledgment of support for the sprs in the global vector
> + * gotten by the registers supplied by the firmware.
> + * The registers are in a bitmask, bit index within
> + * that specifies the SPR
> + */
> + for (i = 0; i < nr_preferred_sprs; i++) {
> + bitmask_index = preferred_sprs[i].spr / 64;
> + bit_pos = preferred_sprs[i].spr % 64;
This is basically a hand coded bitmap, see eg. BIT_WORD(), BIT_MASK() etc.
I don't think there's an easy way to convert temp_u64 into a proper
bitmap, so it's probably not worth doing that. But at least use the macros.
> + if ((temp_u64[bitmask_index] & (1UL << bit_pos)) == 0) {
> + if (type == SELF_RESTORE_TYPE)
> + preferred_sprs[i].supported_mode &=
> + ~SELF_RESTORE_STRICT;
> + else
> + preferred_sprs[i].supported_mode &=
> + ~SELF_SAVE_STRICT;
> + continue;
> + }
> + if (type == SELF_RESTORE_TYPE) {
> + preferred_sprs[i].supported_mode |=
> + SELF_RESTORE_STRICT;
> + } else {
> + preferred_sprs[i].supported_mode |=
> + SELF_SAVE_STRICT;
> + }
> + }
> +
> + kfree(temp_u64);
> + return rc;
> +}
> +
> +static int pnv_parse_deepstate_dt(void)
> +{
> + struct device_node *sr_np, *ss_np;
You never use these concurrently AFAICS, so you could just have a single *np.
> + int rc = 0, i;
> +
> + /* Self restore register population */
> + sr_np = of_find_node_by_path("/ibm,opal/power-mgt/self-restore");
I know the existing idle code uses of_find_node_by_path(), but that's
because it's old and crufty. Please don't add new searches by path. You
should be searching by compatible.
> + if (!sr_np) {
> + pr_warn("opal: self restore Node not found");
This warning and the others below will fire on all existing firmware
versions, which is not OK.
> + } else {
> + rc = extract_save_restore_state_dt(sr_np, SELF_RESTORE_TYPE);
> + if (rc != 0)
> + return rc;
> + }
> + /* Self save register population */
> + ss_np = of_find_node_by_path("/ibm,opal/power-mgt/self-save");
> + if (!ss_np) {
> + pr_warn("opal: self save Node not found");
> + pr_warn("Legacy firmware. Assuming default self-restore support");
> + for (i = 0; i < nr_preferred_sprs; i++)
> + preferred_sprs[i].supported_mode &= ~SELF_SAVE_STRICT;
> + } else {
> + rc = extract_save_restore_state_dt(ss_np, SELF_SAVE_TYPE);
> + }
> + return rc;
You're leaking references on all the device_nodes in here, you need
of_node_put() before exiting.
> +}
cheers
^ permalink raw reply
* Re: [PATCH v1 39/46] powerpc/8xx: Add a function to early map kernel via huge pages
From: kbuild test robot @ 2020-03-17 1:39 UTC (permalink / raw)
To: Christophe Leroy; +Cc: kbuild-all, linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <abaf66aaebfd988d835dc9520bccedd546ee95c4.1584360344.git.christophe.leroy@c-s.fr>
[-- Attachment #1: Type: text/plain, Size: 5390 bytes --]
Hi Christophe,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on next-20200316]
[cannot apply to powerpc/next v5.6-rc6 v5.6-rc5 v5.6-rc4 v5.6-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/Use-hugepages-to-map-kernel-mem-on-8xx/20200317-065610
base: 8548fd2f20ed19b0e8c0585b71fdfde1ae00ae3c
config: powerpc-tqm8xx_defconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=9.2.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from arch/powerpc/mm/fault.c:33:
include/linux/hugetlb.h: In function 'hstate_inode':
>> include/linux/hugetlb.h:522:9: error: implicit declaration of function 'HUGETLBFS_SB'; did you mean 'HUGETLBFS_MAGIC'? [-Werror=implicit-function-declaration]
522 | return HUGETLBFS_SB(i->i_sb)->hstate;
| ^~~~~~~~~~~~
| HUGETLBFS_MAGIC
>> include/linux/hugetlb.h:522:30: error: invalid type argument of '->' (have 'int')
522 | return HUGETLBFS_SB(i->i_sb)->hstate;
| ^~
cc1: all warnings being treated as errors
--
In file included from arch/powerpc/mm/mem.c:30:
include/linux/hugetlb.h: In function 'hstate_inode':
>> include/linux/hugetlb.h:522:9: error: implicit declaration of function 'HUGETLBFS_SB' [-Werror=implicit-function-declaration]
522 | return HUGETLBFS_SB(i->i_sb)->hstate;
| ^~~~~~~~~~~~
>> include/linux/hugetlb.h:522:30: error: invalid type argument of '->' (have 'int')
522 | return HUGETLBFS_SB(i->i_sb)->hstate;
| ^~
cc1: all warnings being treated as errors
--
In file included from arch/powerpc/mm/nohash/8xx.c:12:
include/linux/hugetlb.h: In function 'hstate_inode':
>> include/linux/hugetlb.h:522:9: error: implicit declaration of function 'HUGETLBFS_SB' [-Werror=implicit-function-declaration]
522 | return HUGETLBFS_SB(i->i_sb)->hstate;
| ^~~~~~~~~~~~
>> include/linux/hugetlb.h:522:30: error: invalid type argument of '->' (have 'int')
522 | return HUGETLBFS_SB(i->i_sb)->hstate;
| ^~
At top level:
arch/powerpc/mm/nohash/8xx.c:73:18: error: '__early_map_kernel_hugepage' defined but not used [-Werror=unused-function]
73 | static int __ref __early_map_kernel_hugepage(unsigned long va, phys_addr_t pa,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
--
In file included from arch/powerpc//mm/nohash/8xx.c:12:
include/linux/hugetlb.h: In function 'hstate_inode':
>> include/linux/hugetlb.h:522:9: error: implicit declaration of function 'HUGETLBFS_SB' [-Werror=implicit-function-declaration]
522 | return HUGETLBFS_SB(i->i_sb)->hstate;
| ^~~~~~~~~~~~
>> include/linux/hugetlb.h:522:30: error: invalid type argument of '->' (have 'int')
522 | return HUGETLBFS_SB(i->i_sb)->hstate;
| ^~
At top level:
arch/powerpc//mm/nohash/8xx.c:73:18: error: '__early_map_kernel_hugepage' defined but not used [-Werror=unused-function]
73 | static int __ref __early_map_kernel_hugepage(unsigned long va, phys_addr_t pa,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
--
In file included from include/linux/migrate.h:8,
from kernel///sched/sched.h:53,
from kernel///sched/loadavg.c:9:
include/linux/hugetlb.h: In function 'hstate_inode':
>> include/linux/hugetlb.h:522:9: error: implicit declaration of function 'HUGETLBFS_SB'; did you mean 'HUGETLBFS_MAGIC'? [-Werror=implicit-function-declaration]
522 | return HUGETLBFS_SB(i->i_sb)->hstate;
| ^~~~~~~~~~~~
| HUGETLBFS_MAGIC
>> include/linux/hugetlb.h:522:30: error: invalid type argument of '->' (have 'int')
522 | return HUGETLBFS_SB(i->i_sb)->hstate;
| ^~
cc1: some warnings being treated as errors
vim +522 include/linux/hugetlb.h
a5516438959d90 Andi Kleen 2008-07-23 519
a137e1cc6d6e7d Andi Kleen 2008-07-23 520 static inline struct hstate *hstate_inode(struct inode *i)
a5516438959d90 Andi Kleen 2008-07-23 521 {
7fab358d90e6ba Chen Gang 2016-05-20 @522 return HUGETLBFS_SB(i->i_sb)->hstate;
a5516438959d90 Andi Kleen 2008-07-23 523 }
a5516438959d90 Andi Kleen 2008-07-23 524
:::::: The code at line 522 was first introduced by commit
:::::: 7fab358d90e6ba9d9cb702bee0c8a5f5c13bb6df include/linux/hugetlb*.h: clean up code
:::::: TO: Chen Gang <gang.chen.5i5j@gmail.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 10668 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/32: Fix missing NULL pmd check in virt_to_kpte()
From: Michael Ellerman @ 2020-03-16 23:51 UTC (permalink / raw)
To: Nick Desaulniers; +Cc: LKML, Paul Mackerras, Nathan Chancellor, linuxppc-dev
In-Reply-To: <CAKwvOdm6Z+ERUcGXPbuBKmnpBUNKfL8fPOdxK2g+a1gVRWqh-Q@mail.gmail.com>
Nick Desaulniers <ndesaulniers@google.com> writes:
> Hello ppc friends, did this get picked up into -next yet?
Not yet.
It's in my next-test, but it got stuck there because some subsequent
patches caused some CI errors that I had to debug.
I'll push it to next today.
cheers
> On Thu, Mar 12, 2020 at 8:35 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
>>
>> On Sat, Mar 07, 2020 at 10:09:15AM +0000, Christophe Leroy wrote:
>> > Commit 2efc7c085f05 ("powerpc/32: drop get_pteptr()"),
>> > replaced get_pteptr() by virt_to_kpte(). But virt_to_kpte() lacks a
>> > NULL pmd check and returns an invalid non NULL pointer when there
>> > is no page table.
>> >
>> > Reported-by: Nick Desaulniers <ndesaulniers@google.com>
>> > Fixes: 2efc7c085f05 ("powerpc/32: drop get_pteptr()")
>> > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> > ---
>> > arch/powerpc/include/asm/pgtable.h | 4 +++-
>> > 1 file changed, 3 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
>> > index b80bfd41828d..b1f1d5339735 100644
>> > --- a/arch/powerpc/include/asm/pgtable.h
>> > +++ b/arch/powerpc/include/asm/pgtable.h
>> > @@ -54,7 +54,9 @@ static inline pmd_t *pmd_ptr_k(unsigned long va)
>> >
>> > static inline pte_t *virt_to_kpte(unsigned long vaddr)
>> > {
>> > - return pte_offset_kernel(pmd_ptr_k(vaddr), vaddr);
>> > + pmd_t *pmd = pmd_ptr_k(vaddr);
>> > +
>> > + return pmd_none(*pmd) ? NULL : pte_offset_kernel(pmd, vaddr);
>> > }
>> > #endif
>> >
>> > --
>> > 2.25.0
>> >
>>
>> With QEMU 4.2.0, I can confirm this fixes the panic:
>>
>> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
>
>
>
> --
> Thanks,
> ~Nick Desaulniers
^ permalink raw reply
* Re: [PATCH v3 0/9] crypto/nx: Enable GZIP engine and provide userpace API
From: Haren Myneni @ 2020-03-16 23:31 UTC (permalink / raw)
To: Daniel Axtens
Cc: mikey, herbert, npiggin, linux-crypto, sukadev, linuxppc-dev
In-Reply-To: <87y2s0o3i5.fsf@dja-thinkpad.axtens.net>
On Tue, 2020-03-17 at 00:04 +1100, Daniel Axtens wrote:
> Hi Haren,
>
> If I understand correctly, to test these, I need to apply both this
> series and your VAS userspace page fault handling series - is that
> right?
Daniel,
Yes, This patch series enables GZIP engine and provides user space API.
Whereas VAS fault handling series process faults if NX sees fault on
request buffer.
selftest -
https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-March/206035.html
or https://github.com/abalib/power-gzip/tree/develop/selftest
More tests are available - https://github.com/abalib/power-gzip
libnxz - https://github.com/libnxz/power-gzip
Thanks
Haren
>
> Kind regards,
> Daniel
>
> > Power9 processor supports Virtual Accelerator Switchboard (VAS) which
> > allows kernel and userspace to send compression requests to Nest
> > Accelerator (NX) directly. The NX unit comprises of 2 842 compression
> > engines and 1 GZIP engine. Linux kernel already has 842 compression
> > support on kernel. This patch series adds GZIP compression support
> > from user space. The GZIP Compression engine implements the ZLIB and
> > GZIP compression algorithms. No plans of adding NX-GZIP compression
> > support in kernel right now.
> >
> > Applications can send requests to NX directly with COPY/PASTE
> > instructions. But kernel has to establish channel / window on NX-GZIP
> > device for the userspace. So userspace access to the GZIP engine is
> > provided through /dev/crypto/nx-gzip device with several operations.
> >
> > An application must open the this device to obtain a file descriptor (fd).
> > Using the fd, application should issue the VAS_TX_WIN_OPEN ioctl to
> > establish a connection to the engine. Once window is opened, should use
> > mmap() system call to map the hardware address of engine's request queue
> > into the application's virtual address space. Then user space forms the
> > request as co-processor Request Block (CRB) and paste this CRB on the
> > mapped HW address using COPY/PASTE instructions. Application can poll
> > on status flags (part of CRB) with timeout for request completion.
> >
> > For VAS_TX_WIN_OPEN ioctl, if user space passes vas_id = -1 (struct
> > vas_tx_win_open_attr), kernel determines the VAS instance on the
> > corresponding chip based on the CPU on which the process is executing.
> > Otherwise, the specified VAS instance is used if application passes the
> > proper VAS instance (vas_id listed in /proc/device-tree/vas@*/ibm,vas_id).
> >
> > Process can open multiple windows with different FDs or can send several
> > requests to NX on the same window at the same time.
> >
> > A userspace library libnxz is available:
> > https://github.com/abalib/power-gzip
> >
> > Applications that use inflate/deflate calls can link with libNXz and use
> > NX GZIP compression without any modification.
> >
> > Tested the available 842 compression on power8 and power9 system to make
> > sure no regression and tested GZIP compression on power9 with tests
> > available in the above link.
> >
> > Thanks to Bulent Abali for nxz library and tests development.
> >
> > Changelog:
> > V2:
> > - Move user space API code to powerpc as suggested. Also this API
> > can be extended to any other coprocessor type that VAS can support
> > in future. Example: Fast thread wakeup feature from VAS
> > - Rebased to 5.6-rc3
> >
> > V3:
> > - Fix sparse warnings (patches 3&6)
> >
> > Haren Myneni (9):
> > powerpc/vas: Initialize window attributes for GZIP coprocessor type
> > powerpc/vas: Define VAS_TX_WIN_OPEN ioctl API
> > powerpc/vas: Add VAS user space API
> > crypto/nx: Initialize coproc entry with kzalloc
> > crypto/nx: Rename nx-842-powernv file name to nx-common-powernv
> > crypto/NX: Make enable code generic to add new GZIP compression type
> > crypto/nx: Enable and setup GZIP compresstion type
> > crypto/nx: Remove 'pid' in vas_tx_win_attr struct
> > Documentation/powerpc: VAS API
> >
> > Documentation/powerpc/index.rst | 1 +
> > Documentation/powerpc/vas-api.rst | 246 +++++
> > Documentation/userspace-api/ioctl/ioctl-number.rst | 1 +
> > arch/powerpc/include/asm/vas.h | 12 +-
> > arch/powerpc/include/uapi/asm/vas-api.h | 22 +
> > arch/powerpc/platforms/powernv/Makefile | 2 +-
> > arch/powerpc/platforms/powernv/vas-api.c | 290 +++++
> > arch/powerpc/platforms/powernv/vas-window.c | 23 +-
> > arch/powerpc/platforms/powernv/vas.h | 2 +
> > drivers/crypto/nx/Makefile | 2 +-
> > drivers/crypto/nx/nx-842-powernv.c | 1062 ------------------
> > drivers/crypto/nx/nx-common-powernv.c | 1133 ++++++++++++++++++++
> > 12 files changed, 1723 insertions(+), 1073 deletions(-)
> > create mode 100644 Documentation/powerpc/vas-api.rst
> > create mode 100644 arch/powerpc/include/uapi/asm/vas-api.h
> > create mode 100644 arch/powerpc/platforms/powernv/vas-api.c
> > delete mode 100644 drivers/crypto/nx/nx-842-powernv.c
> > create mode 100644 drivers/crypto/nx/nx-common-powernv.c
> >
> > --
> > 1.8.3.1
^ permalink raw reply
* Re: [PATCH] powerpc/32: Fix missing NULL pmd check in virt_to_kpte()
From: Nick Desaulniers @ 2020-03-16 22:51 UTC (permalink / raw)
To: Michael Ellerman; +Cc: LKML, Paul Mackerras, Nathan Chancellor, linuxppc-dev
In-Reply-To: <20200313033517.GA37606@ubuntu-m2-xlarge-x86>
Hello ppc friends, did this get picked up into -next yet?
On Thu, Mar 12, 2020 at 8:35 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Sat, Mar 07, 2020 at 10:09:15AM +0000, Christophe Leroy wrote:
> > Commit 2efc7c085f05 ("powerpc/32: drop get_pteptr()"),
> > replaced get_pteptr() by virt_to_kpte(). But virt_to_kpte() lacks a
> > NULL pmd check and returns an invalid non NULL pointer when there
> > is no page table.
> >
> > Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> > Fixes: 2efc7c085f05 ("powerpc/32: drop get_pteptr()")
> > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > ---
> > arch/powerpc/include/asm/pgtable.h | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
> > index b80bfd41828d..b1f1d5339735 100644
> > --- a/arch/powerpc/include/asm/pgtable.h
> > +++ b/arch/powerpc/include/asm/pgtable.h
> > @@ -54,7 +54,9 @@ static inline pmd_t *pmd_ptr_k(unsigned long va)
> >
> > static inline pte_t *virt_to_kpte(unsigned long vaddr)
> > {
> > - return pte_offset_kernel(pmd_ptr_k(vaddr), vaddr);
> > + pmd_t *pmd = pmd_ptr_k(vaddr);
> > +
> > + return pmd_none(*pmd) ? NULL : pte_offset_kernel(pmd, vaddr);
> > }
> > #endif
> >
> > --
> > 2.25.0
> >
>
> With QEMU 4.2.0, I can confirm this fixes the panic:
>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox