* Re: [RFC 00/11] perf: Enhancing perf to export processor hazard information
From: Peter Zijlstra @ 2020-03-02 10:13 UTC (permalink / raw)
To: Ravi Bangoria
Cc: mark.rutland, ak, maddy, alexander.shishkin, jolsa, linux-kernel,
eranian, adrian.hunter, robert.richter, yao.jin, mingo, paulus,
acme, namhyung, kim.phillips, linuxppc-dev, alexey.budankov,
kan.liang
In-Reply-To: <20200302052355.36365-1-ravi.bangoria@linux.ibm.com>
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?
^ permalink raw reply
* Re: [RFC 02/11] perf/core: Data structure to present hazard data
From: Peter Zijlstra @ 2020-03-02 9:55 UTC (permalink / raw)
To: Ravi Bangoria
Cc: mark.rutland, ak, maddy, alexander.shishkin, jolsa,
Madhavan Srinivasan, linux-kernel, eranian, adrian.hunter,
robert.richter, yao.jin, mingo, paulus, acme, namhyung,
kim.phillips, linuxppc-dev, alexey.budankov, kan.liang
In-Reply-To: <20200302052355.36365-3-ravi.bangoria@linux.ibm.com>
On Mon, Mar 02, 2020 at 10:53:46AM +0530, Ravi Bangoria wrote:
> From: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>
> Introduce new perf sample_type PERF_SAMPLE_PIPELINE_HAZ to request kernel
> to provide cpu pipeline hazard data. Also, introduce arch independent
> structure 'perf_pipeline_haz_data' to pass hazard data to userspace. This
> is generic structure and arch specific data needs to be converted to this
> format.
>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> include/linux/perf_event.h | 7 ++++++
> include/uapi/linux/perf_event.h | 32 ++++++++++++++++++++++++++-
> kernel/events/core.c | 6 +++++
> tools/include/uapi/linux/perf_event.h | 32 ++++++++++++++++++++++++++-
> 4 files changed, 75 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 547773f5894e..d5b606e3c57d 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -1001,6 +1001,7 @@ struct perf_sample_data {
> u64 stack_user_size;
>
> u64 phys_addr;
> + struct perf_pipeline_haz_data pipeline_haz;
> } ____cacheline_aligned;
>
> /* default value for data source */
> @@ -1021,6 +1022,12 @@ static inline void perf_sample_data_init(struct perf_sample_data *data,
> data->weight = 0;
> data->data_src.val = PERF_MEM_NA;
> data->txn = 0;
> + data->pipeline_haz.itype = PERF_HAZ__ITYPE_NA;
> + data->pipeline_haz.icache = PERF_HAZ__ICACHE_NA;
> + data->pipeline_haz.hazard_stage = PERF_HAZ__PIPE_STAGE_NA;
> + data->pipeline_haz.hazard_reason = PERF_HAZ__HREASON_NA;
> + data->pipeline_haz.stall_stage = PERF_HAZ__PIPE_STAGE_NA;
> + data->pipeline_haz.stall_reason = PERF_HAZ__SREASON_NA;
> }
NAK, Don't touch anything outside of the first cacheline here.
^ permalink raw reply
* Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable
From: 王文虎 @ 2020-03-02 9:51 UTC (permalink / raw)
To: Scott Wood
Cc: Rai Harninder, trivial, linux-kernel, wangwenhu, Paul Mackerras,
linuxppc-dev
In-Reply-To: <2a7cf26aa11adf43ec4f29ab733afc695039633c.camel@buserror.net>
发件人：Scott Wood <oss@buserror.net>
发送日期：2020-03-02 16:58:52
收件人："王文虎" <wenhu.wang@vivo.com>
抄送人：wangwenhu <wenhu.pku@gmail.com>,Kumar Gala <galak@kernel.crashing.org>,Benjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org,trivial@kernel.org,Rai Harninder <harninder.rai@nxp.com>
主题：Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable>On Mon, 2020-03-02 at 12:42 +0800, 王文虎 wrote:
>> 发件人：Scott Wood <oss@buserror.net>
>> 发送日期：2020-03-01 07:12:58
>> 收件人："王文虎" <wenhu.wang@vivo.com>
>> 抄送人：wangwenhu <wenhu.pku@gmail.com>,Kumar Gala <galak@kernel.crashing.org>,B
>> enjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <
>> paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,
>> linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org,
>> trivial@kernel.org,Rai Harninder <harninder.rai@nxp.com>
>> 主题：Re: Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable>On
>> Tue, 2020-01-21 at 14:38 +0800, 王文虎 wrote:
>> > > 发件人：Scott Wood <oss@buserror.net>
>> > > 发送日期：2020-01-21 13:49:59
>> > > 收件人："王文虎" <wenhu.wang@vivo.com>
>> > > 抄送人：wangwenhu <wenhu.pku@gmail.com>,Kumar Gala <
>> > > galak@kernel.crashing.org>,B
>> > > enjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <
>> > > paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,
>> > > linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org,
>> > > trivial@kernel.org,Rai Harninder <harninder.rai@nxp.com>
>> > > 主题：Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable>On
>> > > Tue, 2020-01-21 at 13:20 +0800, 王文虎 wrote:
>> > > > > From: Scott Wood <oss@buserror.net>
>> > > > > Date: 2020-01-21 11:25:25
>> > > > > To: wangwenhu <wenhu.pku@gmail.com>,Kumar Gala <
>> > > > > galak@kernel.crashing.org>,
>> > > > > Benjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <
>> > > > > paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,
>> > > > > linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org
>> > > > > Cc: trivial@kernel.org,wenhu.wang@vivo.com,Rai Harninder <
>> > > > > harninder.rai@nxp.com>
>> > > > > Subject: Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM
>> > > > > configurable>On Mon, 2020-01-20 at 06:43 -0800, wangwenhu wrote:
>> > > > > > > From: wangwenhu <wenhu.wang@vivo.com>
>> > > > > > >
>> > > > > > > When generating .config file with menuconfig on Freescale BOOKE
>> > > > > > > SOC, FSL_85XX_CACHE_SRAM is not configurable for the lack of
>> > > > > > > description in the Kconfig field, which makes it impossible
>> > > > > > > to support L2Cache-Sram driver. Add a description to make it
>> > > > > > > configurable.
>> > > > > > >
>> > > > > > > Signed-off-by: wangwenhu <wenhu.wang@vivo.com>
>> > > > > >
>> > > > > > The intent was that drivers using the SRAM API would select the
>> > > > > > symbol. What
>> > > > > > is the use case for selecting it manually?
>> > > > > >
>> > > > >
>> > > > > With a repository of multiple products(meaning different defconfigs)
>> > > > > and
>> > > > > multiple
>> > > > > developers, the Kconfigs of the Kernel Source Tree change
>> > > > > frequently. So
>> > > > > the
>> > > > > "make menuconfig"
>> > > > > process is needed for defconfigs' re-generating or updating for the
>> > > > > complexity of dependencies
>> > > > > between different features defined in the Kconfigs.
>> > > >
>> > > > That doesn't answer my question of how the SRAM code would be useful
>> > > > other
>> > > > than to some other driver that uses the API (which would use
>> > > > "select"). There
>> > > > is no userspace API. You could use the kernel command line to
>> > > > configure
>> > > > the
>> > > > SRAM but you need to get the address of it for it to be useful.
>> > > >
>> > >
>> > > Like you've asked below, via /dev/mem or direct calling within the
>> > > Kernel.
>> > > And they are not submitted yes, under development.
>> >
>> > If they are calling within the kernel, then whatever driver that is should
>> > select FSL_85XX_CACHE_SRAM. Directly accessing /dev/mem without any way
>> > for
>> > the kernel to advertise where it is or which parts of SRAM are available
>> > for
>> > use sounds like a bad idea.
>> >
>>
>> Yes, definitely. So like we enable the moulde which should selet
>> FSL_85XX_CACHE_SRAM to build vmlinux, FSL_85XX_CACHE_SRAM
>> could not be seleted because of the Kconfig definition problem
>> which I am trying to fix now. So would you please merge the patch
>> for the convenience of later works depending on the driver.
>
>Sorry, I don't think it's something that should be enabled by itself with
>nothing using the allocators. Suppose we took this patch, and people enabled
>it and accessed it via /dev/mem. Then suppose a driver is patched to allocate
>some sram and use it. They'd be stepping on each others' toes undetected.
>
Right, and maybe i did not explain it clear: I mean that we are developing
modules both in kernel which call the interfaces of FSL_85XX_CACHE_SRAM
directly, and in user space which is a further consideration upon the work
we have done. Cause we have not exported the code under developing, it
seems like that nothing uses FSL_85XX_CACHE_SRAM.
>If you want to expose it to userspace, add code that allocates some or all of
>the sram and make it something userspace can mmap. Or, if nothing's going to
>use them, remove the allocators and export the entire thing to userspace
>(again via an sram-specific mappable rather than /dev/mem).
As for /dev/mem, it was one of our choices ever but now a user-space
driver is preferred for further consideration. But currently, the functionality
that directly calls the insterfaces of FSL_85XX_CACHE_SRAM have been
under developing for couples of days and would be exported in the future.
They would be used on hw-platforms like PPCe500.
Just some time it takes.
Do you mean the exporting is a pre-condition? If not, the merge would
do a favor for the convenience.
Wenhu
>
>-Scott
>
>
^ permalink raw reply
* Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
From: Jason Yan @ 2020-03-02 9:37 UTC (permalink / raw)
To: Scott Wood, Daniel Axtens, mpe, linuxppc-dev, diana.craciun,
christophe.leroy, benh, paulus, npiggin, keescook,
kernel-hardening, me
Cc: linux-kernel, zhaohongjiang
In-Reply-To: <7f608c18250c509ff091990d4bb460846fae11a0.camel@buserror.net>
在 2020/3/2 16:47, Scott Wood 写道:
> On Mon, 2020-03-02 at 15:12 +0800, Jason Yan wrote:
>>
>> 在 2020/3/2 11:24, Scott Wood 写道:
>>> On Mon, 2020-03-02 at 10:17 +0800, Jason Yan wrote:
>>>>
>>>> 在 2020/3/1 6:54, Scott Wood 写道:
>>>>> On Sat, 2020-02-29 at 15:27 +0800, Jason Yan wrote:
>>>>>>
>>>>>> Turnning to %p may not be a good idea in this situation. So
>>>>>> for the REG logs printed when dumping stack, we can disable it when
>>>>>> KASLR is open. For the REG logs in other places like show_regs(),
>>>>>> only
>>>>>> privileged can trigger it, and they are not combind with a symbol,
>>>>>> so
>>>>>> I think it's ok to keep them.
>>>>>>
>>>>>> diff --git a/arch/powerpc/kernel/process.c
>>>>>> b/arch/powerpc/kernel/process.c
>>>>>> index fad50db9dcf2..659c51f0739a 100644
>>>>>> --- a/arch/powerpc/kernel/process.c
>>>>>> +++ b/arch/powerpc/kernel/process.c
>>>>>> @@ -2068,7 +2068,10 @@ void show_stack(struct task_struct *tsk,
>>>>>> unsigned
>>>>>> long *stack)
>>>>>> newsp = stack[0];
>>>>>> ip = stack[STACK_FRAME_LR_SAVE];
>>>>>> if (!firstframe || ip != lr) {
>>>>>> - printk("["REG"] ["REG"] %pS", sp, ip, (void
>>>>>> *)ip);
>>>>>> + if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
>>>>>> + printk("%pS", (void *)ip);
>>>>>> + else
>>>>>> + printk("["REG"] ["REG"] %pS", sp,
>>>>>> ip,
>>>>>> (void *)ip);
>>>>>
>>>>> This doesn't deal with "nokaslr" on the kernel command line. It also
>>>>> doesn't
>>>>> seem like something that every callsite should have to opencode,
>>>>> versus
>>>>> having
>>>>> an appropriate format specifier behaves as I described above (and I
>>>>> still
>>>>> don't see why that format specifier should not be "%p").
>>>>>
>>>>
>>>> Actually I still do not understand why we should print the raw value
>>>> here. When KALLSYMS is enabled we have symbol name and offset like
>>>> put_cred_rcu+0x108/0x110, and when KALLSYMS is disabled we have the raw
>>>> address.
>>>
>>> I'm more concerned about the stack address for wading through a raw stack
>>> dump
>>> (to find function call arguments, etc). The return address does help
>>> confirm
>>> that I'm on the right stack frame though, and also makes looking up a line
>>> number slightly easier than having to look up a symbol address and then
>>> add
>>> the offset (at least for non-module addresses).
>>>
>>> As a random aside, the mismatch between Linux printing a hex offset and
>>> GDB
>>> using decimal in disassembly is annoying...
>>>
>>
>> OK, I will send a RFC patch to add a new format specifier such as "%pk"
>> or change the exsiting "%pK" to print raw value of addresses when KASLR
>> is disabled and print hash value of addresses when KASLR is enabled.
>> Let's see what the printk guys would say :)
>
> I'm not sure that a new format specifier is needed versus changing the
> behavior of "%p", and "%pK" definitely doesn't seem suitable given that it's
> intended to be more restricted than "%p" (see commit ef0010a30935de4). The
> question is whether there is a legitimate reason to hash in the absence of
> kaslr.
>
The problem is that if we change the behavior of "%p", we have to turn
all exsiting "%p" to "%pK". Hashing is still reasonable when there is no
kaslr because some architectures support randomize at build time such as
arm64.
> -Scott
>
>
>
> .
>
^ permalink raw reply
* Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable
From: Scott Wood @ 2020-03-02 8:58 UTC (permalink / raw)
To: 王文虎
Cc: Rai Harninder, trivial, linux-kernel, wangwenhu, Paul Mackerras,
linuxppc-dev
In-Reply-To: <AKMA9QAgCCx9CEivazbGIap-.3.1583124159966.Hmail.wenhu.wang@vivo.com>
On Mon, 2020-03-02 at 12:42 +0800, 王文虎 wrote:
> 发件人：Scott Wood <oss@buserror.net>
> 发送日期：2020-03-01 07:12:58
> 收件人："王文虎" <wenhu.wang@vivo.com>
> 抄送人：wangwenhu <wenhu.pku@gmail.com>,Kumar Gala <galak@kernel.crashing.org>,B
> enjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <
> paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,
> linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org,
> trivial@kernel.org,Rai Harninder <harninder.rai@nxp.com>
> 主题：Re: Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable>On
> Tue, 2020-01-21 at 14:38 +0800, 王文虎 wrote:
> > > 发件人：Scott Wood <oss@buserror.net>
> > > 发送日期：2020-01-21 13:49:59
> > > 收件人："王文虎" <wenhu.wang@vivo.com>
> > > 抄送人：wangwenhu <wenhu.pku@gmail.com>,Kumar Gala <
> > > galak@kernel.crashing.org>,B
> > > enjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <
> > > paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,
> > > linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org,
> > > trivial@kernel.org,Rai Harninder <harninder.rai@nxp.com>
> > > 主题：Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable>On
> > > Tue, 2020-01-21 at 13:20 +0800, 王文虎 wrote:
> > > > > From: Scott Wood <oss@buserror.net>
> > > > > Date: 2020-01-21 11:25:25
> > > > > To: wangwenhu <wenhu.pku@gmail.com>,Kumar Gala <
> > > > > galak@kernel.crashing.org>,
> > > > > Benjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <
> > > > > paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,
> > > > > linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org
> > > > > Cc: trivial@kernel.org,wenhu.wang@vivo.com,Rai Harninder <
> > > > > harninder.rai@nxp.com>
> > > > > Subject: Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM
> > > > > configurable>On Mon, 2020-01-20 at 06:43 -0800, wangwenhu wrote:
> > > > > > > From: wangwenhu <wenhu.wang@vivo.com>
> > > > > > >
> > > > > > > When generating .config file with menuconfig on Freescale BOOKE
> > > > > > > SOC, FSL_85XX_CACHE_SRAM is not configurable for the lack of
> > > > > > > description in the Kconfig field, which makes it impossible
> > > > > > > to support L2Cache-Sram driver. Add a description to make it
> > > > > > > configurable.
> > > > > > >
> > > > > > > Signed-off-by: wangwenhu <wenhu.wang@vivo.com>
> > > > > >
> > > > > > The intent was that drivers using the SRAM API would select the
> > > > > > symbol. What
> > > > > > is the use case for selecting it manually?
> > > > > >
> > > > >
> > > > > With a repository of multiple products(meaning different defconfigs)
> > > > > and
> > > > > multiple
> > > > > developers, the Kconfigs of the Kernel Source Tree change
> > > > > frequently. So
> > > > > the
> > > > > "make menuconfig"
> > > > > process is needed for defconfigs' re-generating or updating for the
> > > > > complexity of dependencies
> > > > > between different features defined in the Kconfigs.
> > > >
> > > > That doesn't answer my question of how the SRAM code would be useful
> > > > other
> > > > than to some other driver that uses the API (which would use
> > > > "select"). There
> > > > is no userspace API. You could use the kernel command line to
> > > > configure
> > > > the
> > > > SRAM but you need to get the address of it for it to be useful.
> > > >
> > >
> > > Like you've asked below, via /dev/mem or direct calling within the
> > > Kernel.
> > > And they are not submitted yes, under development.
> >
> > If they are calling within the kernel, then whatever driver that is should
> > select FSL_85XX_CACHE_SRAM. Directly accessing /dev/mem without any way
> > for
> > the kernel to advertise where it is or which parts of SRAM are available
> > for
> > use sounds like a bad idea.
> >
>
> Yes, definitely. So like we enable the moulde which should selet
> FSL_85XX_CACHE_SRAM to build vmlinux, FSL_85XX_CACHE_SRAM
> could not be seleted because of the Kconfig definition problem
> which I am trying to fix now. So would you please merge the patch
> for the convenience of later works depending on the driver.
Sorry, I don't think it's something that should be enabled by itself with
nothing using the allocators. Suppose we took this patch, and people enabled
it and accessed it via /dev/mem. Then suppose a driver is patched to allocate
some sram and use it. They'd be stepping on each others' toes undetected.
If you want to expose it to userspace, add code that allocates some or all of
the sram and make it something userspace can mmap. Or, if nothing's going to
use them, remove the allocators and export the entire thing to userspace
(again via an sram-specific mappable rather than /dev/mem).
-Scott
^ permalink raw reply
* Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
From: Scott Wood @ 2020-03-02 8:47 UTC (permalink / raw)
To: Jason Yan, Daniel Axtens, mpe, linuxppc-dev, diana.craciun,
christophe.leroy, benh, paulus, npiggin, keescook,
kernel-hardening, me
Cc: linux-kernel, zhaohongjiang
In-Reply-To: <17658c2b-9eb8-cee9-e9a2-93d316a401b1@huawei.com>
On Mon, 2020-03-02 at 15:12 +0800, Jason Yan wrote:
>
> 在 2020/3/2 11:24, Scott Wood 写道:
> > On Mon, 2020-03-02 at 10:17 +0800, Jason Yan wrote:
> > >
> > > 在 2020/3/1 6:54, Scott Wood 写道:
> > > > On Sat, 2020-02-29 at 15:27 +0800, Jason Yan wrote:
> > > > >
> > > > > Turnning to %p may not be a good idea in this situation. So
> > > > > for the REG logs printed when dumping stack, we can disable it when
> > > > > KASLR is open. For the REG logs in other places like show_regs(),
> > > > > only
> > > > > privileged can trigger it, and they are not combind with a symbol,
> > > > > so
> > > > > I think it's ok to keep them.
> > > > >
> > > > > diff --git a/arch/powerpc/kernel/process.c
> > > > > b/arch/powerpc/kernel/process.c
> > > > > index fad50db9dcf2..659c51f0739a 100644
> > > > > --- a/arch/powerpc/kernel/process.c
> > > > > +++ b/arch/powerpc/kernel/process.c
> > > > > @@ -2068,7 +2068,10 @@ void show_stack(struct task_struct *tsk,
> > > > > unsigned
> > > > > long *stack)
> > > > > newsp = stack[0];
> > > > > ip = stack[STACK_FRAME_LR_SAVE];
> > > > > if (!firstframe || ip != lr) {
> > > > > - printk("["REG"] ["REG"] %pS", sp, ip, (void
> > > > > *)ip);
> > > > > + if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
> > > > > + printk("%pS", (void *)ip);
> > > > > + else
> > > > > + printk("["REG"] ["REG"] %pS", sp,
> > > > > ip,
> > > > > (void *)ip);
> > > >
> > > > This doesn't deal with "nokaslr" on the kernel command line. It also
> > > > doesn't
> > > > seem like something that every callsite should have to opencode,
> > > > versus
> > > > having
> > > > an appropriate format specifier behaves as I described above (and I
> > > > still
> > > > don't see why that format specifier should not be "%p").
> > > >
> > >
> > > Actually I still do not understand why we should print the raw value
> > > here. When KALLSYMS is enabled we have symbol name and offset like
> > > put_cred_rcu+0x108/0x110, and when KALLSYMS is disabled we have the raw
> > > address.
> >
> > I'm more concerned about the stack address for wading through a raw stack
> > dump
> > (to find function call arguments, etc). The return address does help
> > confirm
> > that I'm on the right stack frame though, and also makes looking up a line
> > number slightly easier than having to look up a symbol address and then
> > add
> > the offset (at least for non-module addresses).
> >
> > As a random aside, the mismatch between Linux printing a hex offset and
> > GDB
> > using decimal in disassembly is annoying...
> >
>
> OK, I will send a RFC patch to add a new format specifier such as "%pk"
> or change the exsiting "%pK" to print raw value of addresses when KASLR
> is disabled and print hash value of addresses when KASLR is enabled.
> Let's see what the printk guys would say :)
I'm not sure that a new format specifier is needed versus changing the
behavior of "%p", and "%pK" definitely doesn't seem suitable given that it's
intended to be more restricted than "%p" (see commit ef0010a30935de4). The
question is whether there is a legitimate reason to hash in the absence of
kaslr.
-Scott
^ permalink raw reply
* Re: [RFC 1/3] mm/vma: Define a default value for VM_DATA_DEFAULT_FLAGS
From: Geert Uytterhoeven @ 2020-03-02 8:22 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Rich Felker, linux-ia64@vger.kernel.org, Linux-sh list,
Catalin Marinas, Heiko Carstens, linux-mips, James E.J. Bottomley,
Linux MM, Guo Ren, sparclinux, open list:QUALCOMM HEXAGON...,
linux-riscv, Jonas Bonn, linux-s390, arcml, linux-c6x-dev,
Brian Cain, Russell King, Ley Foon Tan, Parisc List, Mark Salter,
Paul Burton, moderated list:H8/300 ARCHITECTURE,
open list:TENSILICA XTENSA PORT (xtensa), Jeff Dike, linux-um,
linux-m68k, Openrisc, Paul Walmsley, Thomas Gleixner, Guan Xuetao,
Linux ARM, Richard Henderson, Chris Zankel, Michal Simek,
Tony Luck, Yoshinori Sato, Nick Hu, Vineet Gupta,
Linux Kernel Mailing List, Ralf Baechle, alpha, nios2-dev,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <1583131666-15531-2-git-send-email-anshuman.khandual@arm.com>
On Mon, Mar 2, 2020 at 7:48 AM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
> There are many platforms with exact same value for VM_DATA_DEFAULT_FLAGS
> This creates a default value for VM_DATA_DEFAULT_FLAGS in line with the
> existing VM_STACK_DEFAULT_FLAGS. While here, also define some more macros
> with standard VMA access flag combinations that are used frequently across
> many platforms. Apart from simplification, this reduces code duplication
> as well.
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> arch/m68k/include/asm/page.h | 3 ---
For m68k:
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents
From: Hari Bathini @ 2020-03-02 8:07 UTC (permalink / raw)
To: Pingfan Liu, linuxppc-dev; +Cc: Paul Mackerras, kexec
In-Reply-To: <1581389982-5701-2-git-send-email-kernelfans@gmail.com>
On 11/02/20 8:29 AM, Pingfan Liu wrote:
> A bug is observed on pseries by taking the following steps on rhel:
> -1. drmgr -c mem -r -q 5
> -2. echo c > /proc/sysrq-trigger
>
> And then, the failure looks like:
> kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/
> kdump: saving vmcore-dmesg.txt
> kdump: saving vmcore-dmesg.txt complete
> kdump: saving vmcore
> Checking for memory holes : [ 0.0 %] / Checking for memory holes : [100.0 %] | Excluding unnecessary pages : [100.0 %] \ Copying data : [ 0.3 %] - eta: 38s[ 44.337636] hash-mmu: mm: Hashing failure ! EA=0x7fffba400000 access=0x8000000000000004 current=makedumpfile
> [ 44.337663] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc000000050000504
> [ 44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba400000 access=0x8000000000000004 current=makedumpfile
> [ 44.337692] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc000000050000504
> [ 44.337708] makedumpfile[469]: unhandled signal 7 at 00007fffba400000 nip 00007fffbbc4d7fc lr 000000011356ca3c code 2
> [ 44.338548] Core dump to |/bin/false pipe failed
> /lib/kdump-lib-initramfs.sh: line 98: 469 Bus error $CORE_COLLECTOR /proc/vmcore $_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete
> kdump: saving vmcore failed
>
> * Root cause *
> After analyzing, it turns out that in the current implementation,
> when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt updating as
> the code __remove_memory() comes before drmem_update_dt().
>
> From a viewpoint of listener and publisher, the publisher notifies the
> listener before data is ready. This introduces a problem where udev
> launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before
> updating. And in capture kernel, makedumpfile will access the memory based
> on the stale dt info, and hit a SIGBUS error due to an un-existed lmb.
>
> * Fix *
> In order to fix this issue, update dt before __remove_memory(), and
> accordingly the same rule in hot-add path.
>
> This will introduce extra dt updating payload for each involved lmb when hotplug.
> But it should be fine since drmem_update_dt() is memory based operation and
> hotplug is not a hot path.
>
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> To: linuxppc-dev@lists.ozlabs.org
> Cc: kexec@lists.infradead.org
KDump fails to capture vmcore as we end up looking at a stale elfcore hdr
with udev event happening before DT update. Resolved with these patches.
For the series:
Tested-by: Hari Bathini <hbathini@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH v3 26/27] powerpc/powernv/pmem: Expose the firmware version in sysfs
From: Andrew Donnellan @ 2020-03-02 7:35 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200221032720.33893-27-alastair@au1.ibm.com>
On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> This information will be used by ndctl in userspace to help users identify
> the device.
You should include the information from the subject line in the body of
the commit message too.
I think this patch could probably be squashed in with the last one.
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Reg: KASLR backporting to 4.9 kernels for ppc platform.
From: Bala murugan @ 2020-03-02 5:57 UTC (permalink / raw)
To: linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 136 bytes --]
Hi,
Any plans to back port KASLR for 4.9 kernel. How feasible is to back port
to 4.9 kernel for PPC platforms.
Regards
S Balamurugan.
[-- Attachment #2: Type: text/html, Size: 235 bytes --]
^ permalink raw reply
* Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
From: Jason Yan @ 2020-03-02 7:12 UTC (permalink / raw)
To: Scott Wood, Daniel Axtens, mpe, linuxppc-dev, diana.craciun,
christophe.leroy, benh, paulus, npiggin, keescook,
kernel-hardening, me
Cc: linux-kernel, zhaohongjiang
In-Reply-To: <31b5966ba579ef246176a7d8ad18c2c02788dd27.camel@buserror.net>
在 2020/3/2 11:24, Scott Wood 写道:
> On Mon, 2020-03-02 at 10:17 +0800, Jason Yan wrote:
>>
>> 在 2020/3/1 6:54, Scott Wood 写道:
>>> On Sat, 2020-02-29 at 15:27 +0800, Jason Yan wrote:
>>>>
>>>> Turnning to %p may not be a good idea in this situation. So
>>>> for the REG logs printed when dumping stack, we can disable it when
>>>> KASLR is open. For the REG logs in other places like show_regs(), only
>>>> privileged can trigger it, and they are not combind with a symbol, so
>>>> I think it's ok to keep them.
>>>>
>>>> diff --git a/arch/powerpc/kernel/process.c
>>>> b/arch/powerpc/kernel/process.c
>>>> index fad50db9dcf2..659c51f0739a 100644
>>>> --- a/arch/powerpc/kernel/process.c
>>>> +++ b/arch/powerpc/kernel/process.c
>>>> @@ -2068,7 +2068,10 @@ void show_stack(struct task_struct *tsk, unsigned
>>>> long *stack)
>>>> newsp = stack[0];
>>>> ip = stack[STACK_FRAME_LR_SAVE];
>>>> if (!firstframe || ip != lr) {
>>>> - printk("["REG"] ["REG"] %pS", sp, ip, (void
>>>> *)ip);
>>>> + if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
>>>> + printk("%pS", (void *)ip);
>>>> + else
>>>> + printk("["REG"] ["REG"] %pS", sp, ip,
>>>> (void *)ip);
>>>
>>> This doesn't deal with "nokaslr" on the kernel command line. It also
>>> doesn't
>>> seem like something that every callsite should have to opencode, versus
>>> having
>>> an appropriate format specifier behaves as I described above (and I still
>>> don't see why that format specifier should not be "%p").
>>>
>>
>> Actually I still do not understand why we should print the raw value
>> here. When KALLSYMS is enabled we have symbol name and offset like
>> put_cred_rcu+0x108/0x110, and when KALLSYMS is disabled we have the raw
>> address.
>
> I'm more concerned about the stack address for wading through a raw stack dump
> (to find function call arguments, etc). The return address does help confirm
> that I'm on the right stack frame though, and also makes looking up a line
> number slightly easier than having to look up a symbol address and then add
> the offset (at least for non-module addresses).
>
> As a random aside, the mismatch between Linux printing a hex offset and GDB
> using decimal in disassembly is annoying...
>
OK, I will send a RFC patch to add a new format specifier such as "%pk"
or change the exsiting "%pK" to print raw value of addresses when KASLR
is disabled and print hash value of addresses when KASLR is enabled.
Let's see what the printk guys would say :)
> -Scott
>
>
>
> .
>
^ permalink raw reply
* [RFC 2/3] mm/vma: Introduce VM_ACCESS_FLAGS
From: Anshuman Khandual @ 2020-03-02 6:47 UTC (permalink / raw)
To: linux-mm
Cc: linux-sh, Catalin Marinas, Dave Hansen, Heiko Carstens, devel,
linux-s390, linux-c6x-dev, Yoshinori Sato, Russell King,
Ley Foon Tan, Mark Salter, Anshuman Khandual, Rob Springer,
Thomas Gleixner, Guan Xuetao, linux-arm-kernel, Nick Hu,
Greg Kroah-Hartman, linux-kernel, nios2-dev, Andrew Morton,
linuxppc-dev
In-Reply-To: <1583131666-15531-1-git-send-email-anshuman.khandual@arm.com>
There are many places where all basic VMA access flags (read, write, exec)
are initialized or checked against as a group. One such example is during
page fault. Existing vma_is_accessible() wrapper already creates the notion
of VMA accessibility as a group access permissions. Hence lets just create
VM_ACCESS_FLAGS (VM_READ|VM_WRITE|VM_EXEC) which will not only reduce code
duplication but also extend the VMA accessibility concept in general.
Cc: Russell King <linux@armlinux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Mark Salter <msalter@redhat.com>
Cc: Nick Hu <nickhu@andestech.com>
CC: Ley Foon Tan <ley.foon.tan@intel.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Guan Xuetao <gxt@pku.edu.cn>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rob Springer <rspringer@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-c6x-dev@linux-c6x.org
Cc: nios2-dev@lists.rocketboards.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: devel@driverdev.osuosl.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
arch/arm/mm/fault.c | 2 +-
arch/arm64/mm/fault.c | 2 +-
arch/c6x/include/asm/processor.h | 2 +-
arch/nds32/mm/fault.c | 2 +-
arch/nios2/include/asm/processor.h | 2 +-
arch/powerpc/mm/book3s64/pkeys.c | 2 +-
arch/s390/mm/fault.c | 2 +-
arch/sh/include/asm/processor_64.h | 2 +-
arch/unicore32/mm/fault.c | 2 +-
arch/x86/mm/pkeys.c | 2 +-
drivers/staging/gasket/gasket_core.c | 2 +-
include/linux/mm.h | 4 +++-
mm/mmap.c | 4 ++--
mm/mprotect.c | 7 +++----
14 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index bd0f4821f7e1..2c71028d9d6b 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -189,7 +189,7 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
*/
static inline bool access_error(unsigned int fsr, struct vm_area_struct *vma)
{
- unsigned int mask = VM_READ | VM_WRITE | VM_EXEC;
+ unsigned int mask = VM_ACCESS_FLAGS;
if ((fsr & FSR_WRITE) && !(fsr & FSR_CM))
mask = VM_WRITE;
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 85566d32958f..63f31206a12e 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -445,7 +445,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
const struct fault_info *inf;
struct mm_struct *mm = current->mm;
vm_fault_t fault, major = 0;
- unsigned long vm_flags = VM_READ | VM_WRITE | VM_EXEC;
+ unsigned long vm_flags = VM_ACCESS_FLAGS;
unsigned int mm_flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
if (kprobe_page_fault(regs, esr))
diff --git a/arch/c6x/include/asm/processor.h b/arch/c6x/include/asm/processor.h
index 1456f5e11de3..77372b8c28d7 100644
--- a/arch/c6x/include/asm/processor.h
+++ b/arch/c6x/include/asm/processor.h
@@ -57,7 +57,7 @@ struct thread_struct {
}
#define INIT_MMAP { \
- &init_mm, 0, 0, NULL, PAGE_SHARED, VM_READ | VM_WRITE | VM_EXEC, 1, \
+ &init_mm, 0, 0, NULL, PAGE_SHARED, VM_ACCESS_FLAGS, 1, \
NULL, NULL }
#define task_pt_regs(task) \
diff --git a/arch/nds32/mm/fault.c b/arch/nds32/mm/fault.c
index 906dfb25353c..55387a31bf42 100644
--- a/arch/nds32/mm/fault.c
+++ b/arch/nds32/mm/fault.c
@@ -79,7 +79,7 @@ void do_page_fault(unsigned long entry, unsigned long addr,
struct vm_area_struct *vma;
int si_code;
vm_fault_t fault;
- unsigned int mask = VM_READ | VM_WRITE | VM_EXEC;
+ unsigned int mask = VM_ACCESS_FLAGS;
unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
error_code = error_code & (ITYPE_mskINST | ITYPE_mskETYPE);
diff --git a/arch/nios2/include/asm/processor.h b/arch/nios2/include/asm/processor.h
index 94bcb86f679f..fbfb3ab14cfc 100644
--- a/arch/nios2/include/asm/processor.h
+++ b/arch/nios2/include/asm/processor.h
@@ -51,7 +51,7 @@ struct thread_struct {
};
#define INIT_MMAP \
- { &init_mm, (0), (0), __pgprot(0x0), VM_READ | VM_WRITE | VM_EXEC }
+ { &init_mm, (0), (0), __pgprot(0x0), VM_ACCESS_FLAGS }
# define INIT_THREAD { \
.kregs = NULL, \
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index 59e0ebbd8036..11fd52b24f68 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -315,7 +315,7 @@ int __execute_only_pkey(struct mm_struct *mm)
static inline bool vma_is_pkey_exec_only(struct vm_area_struct *vma)
{
/* Do this check first since the vm_flags should be hot */
- if ((vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)) != VM_EXEC)
+ if ((vma->vm_flags & VM_ACCESS_FLAGS) != VM_EXEC)
return false;
return (vma_pkey(vma) == vma->vm_mm->context.execute_only_pkey);
diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index 7b0bb475c166..b2cb3c0d0e1a 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -584,7 +584,7 @@ void do_dat_exception(struct pt_regs *regs)
int access;
vm_fault_t fault;
- access = VM_READ | VM_EXEC | VM_WRITE;
+ access = VM_ACCESS_FLAGS;
fault = do_exception(regs, access);
if (unlikely(fault))
do_fault_error(regs, access, fault);
diff --git a/arch/sh/include/asm/processor_64.h b/arch/sh/include/asm/processor_64.h
index 53efc9f51ef1..3b8187284e3f 100644
--- a/arch/sh/include/asm/processor_64.h
+++ b/arch/sh/include/asm/processor_64.h
@@ -121,7 +121,7 @@ struct thread_struct {
};
#define INIT_MMAP \
-{ &init_mm, 0, 0, NULL, PAGE_SHARED, VM_READ | VM_WRITE | VM_EXEC, 1, NULL, NULL }
+{ &init_mm, 0, 0, NULL, PAGE_SHARED, VM_ACCESS_FLAGS, 1, NULL, NULL }
#define INIT_THREAD { \
.sp = sizeof(init_stack) + \
diff --git a/arch/unicore32/mm/fault.c b/arch/unicore32/mm/fault.c
index 76342de9cf8c..fc27c274d358 100644
--- a/arch/unicore32/mm/fault.c
+++ b/arch/unicore32/mm/fault.c
@@ -149,7 +149,7 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
*/
static inline bool access_error(unsigned int fsr, struct vm_area_struct *vma)
{
- unsigned int mask = VM_READ | VM_WRITE | VM_EXEC;
+ unsigned int mask = VM_ACCESS_FLAGS;
if (!(fsr ^ 0x12)) /* write? */
mask = VM_WRITE;
diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c
index c6f84c0b5d7a..8873ed1438a9 100644
--- a/arch/x86/mm/pkeys.c
+++ b/arch/x86/mm/pkeys.c
@@ -63,7 +63,7 @@ int __execute_only_pkey(struct mm_struct *mm)
static inline bool vma_is_pkey_exec_only(struct vm_area_struct *vma)
{
/* Do this check first since the vm_flags should be hot */
- if ((vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)) != VM_EXEC)
+ if ((vma->vm_flags & VM_ACCESS_FLAGS) != VM_EXEC)
return false;
if (vma_pkey(vma) != vma->vm_mm->context.execute_only_pkey)
return false;
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index be6b50f454b4..81bb7d58dc49 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -689,7 +689,7 @@ static bool gasket_mmap_has_permissions(struct gasket_dev *gasket_dev,
/* Make sure that no wrong flags are set. */
requested_permissions =
- (vma->vm_flags & (VM_WRITE | VM_READ | VM_EXEC));
+ (vma->vm_flags & VM_ACCESS_FLAGS);
if (requested_permissions & ~(bar_permissions)) {
dev_dbg(gasket_dev->dev,
"Attempting to map a region with requested permissions "
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 7a764ae6ab68..525026df1e58 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -368,6 +368,8 @@ extern unsigned int kobjsize(const void *objp);
#endif
#define VM_STACK_FLAGS (VM_STACK | VM_STACK_DEFAULT_FLAGS | VM_ACCOUNT)
+#define VM_ACCESS_FLAGS (VM_READ | VM_WRITE | VM_EXEC)
+
/*
* Special vmas that are non-mergable, non-mlock()able.
@@ -558,7 +560,7 @@ static inline bool vma_is_anonymous(struct vm_area_struct *vma)
static inline bool vma_is_accessible(struct vm_area_struct *vma)
{
- return vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
+ return vma->vm_flags & VM_ACCESS_FLAGS;
}
#ifdef CONFIG_SHMEM
diff --git a/mm/mmap.c b/mm/mmap.c
index 0d295f49b24d..f9a01763857b 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -106,7 +106,7 @@ static inline pgprot_t arch_filter_pgprot(pgprot_t prot)
pgprot_t vm_get_page_prot(unsigned long vm_flags)
{
pgprot_t ret = __pgprot(pgprot_val(protection_map[vm_flags &
- (VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
+ (VM_ACCESS_FLAGS | VM_SHARED)]) |
pgprot_val(arch_vm_get_page_prot(vm_flags)));
return arch_filter_pgprot(ret);
@@ -1221,7 +1221,7 @@ static int anon_vma_compatible(struct vm_area_struct *a, struct vm_area_struct *
return a->vm_end == b->vm_start &&
mpol_equal(vma_policy(a), vma_policy(b)) &&
a->vm_file == b->vm_file &&
- !((a->vm_flags ^ b->vm_flags) & ~(VM_READ|VM_WRITE|VM_EXEC|VM_SOFTDIRTY)) &&
+ !((a->vm_flags ^ b->vm_flags) & ~(VM_ACCESS_FLAGS | VM_SOFTDIRTY)) &&
b->vm_pgoff == a->vm_pgoff + ((b->vm_start - a->vm_start) >> PAGE_SHIFT);
}
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 7a8e84f86831..4921a4211c6b 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -359,7 +359,7 @@ mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
*/
if (arch_has_pfn_modify_check() &&
(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP)) &&
- (newflags & (VM_READ|VM_WRITE|VM_EXEC)) == 0) {
+ (newflags & VM_ACCESS_FLAGS) == 0) {
pgprot_t new_pgprot = vm_get_page_prot(newflags);
error = walk_page_range(current->mm, start, end,
@@ -530,15 +530,14 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
* If a permission is not passed to mprotect(), it must be
* cleared from the VMA.
*/
- mask_off_old_flags = VM_READ | VM_WRITE | VM_EXEC |
- VM_FLAGS_CLEAR;
+ mask_off_old_flags = VM_ACCESS_FLAGS | VM_FLAGS_CLEAR;
new_vma_pkey = arch_override_mprotect_pkey(vma, prot, pkey);
newflags = calc_vm_prot_bits(prot, new_vma_pkey);
newflags |= (vma->vm_flags & ~mask_off_old_flags);
/* newflags >> 4 shift VM_MAY% in place of VM_% */
- if ((newflags & ~(newflags >> 4)) & (VM_READ | VM_WRITE | VM_EXEC)) {
+ if ((newflags & ~(newflags >> 4)) & VM_ACCESS_FLAGS) {
error = -EACCES;
goto out;
}
--
2.20.1
^ permalink raw reply related
* [RFC 1/3] mm/vma: Define a default value for VM_DATA_DEFAULT_FLAGS
From: Anshuman Khandual @ 2020-03-02 6:47 UTC (permalink / raw)
To: linux-mm
Cc: Rich Felker, linux-ia64, linux-sh, Catalin Marinas,
Heiko Carstens, linux-mips, James E.J. Bottomley, Guo Ren,
sparclinux, linux-hexagon, linux-riscv, Jonas Bonn, linux-s390,
linux-snps-arc, linux-c6x-dev, Brian Cain, Russell King,
Ley Foon Tan, Geert Uytterhoeven, linux-parisc, Mark Salter,
Paul Burton, uclinux-h8-devel, linux-xtensa, Anshuman Khandual,
Jeff Dike, linux-um, linux-m68k, openrisc, Paul Walmsley,
Thomas Gleixner, Guan Xuetao, linux-arm-kernel, Richard Henderson,
Chris Zankel, Michal Simek, Tony Luck, Yoshinori Sato, Nick Hu,
Vineet Gupta, linux-kernel, Ralf Baechle, linux-alpha, nios2-dev,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <1583131666-15531-1-git-send-email-anshuman.khandual@arm.com>
There are many platforms with exact same value for VM_DATA_DEFAULT_FLAGS
This creates a default value for VM_DATA_DEFAULT_FLAGS in line with the
existing VM_STACK_DEFAULT_FLAGS. While here, also define some more macros
with standard VMA access flag combinations that are used frequently across
many platforms. Apart from simplification, this reduces code duplication
as well.
Cc: Richard Henderson <rth@twiddle.net>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: Guo Ren <guoren@kernel.org>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Brian Cain <bcain@codeaurora.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Michal Simek <monstr@monstr.eu>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Nick Hu <nickhu@andestech.com>
Cc: Ley Foon Tan <ley.foon.tan@intel.com>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Rich Felker <dalias@libc.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Guan Xuetao <gxt@pku.edu.cn>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Jeff Dike <jdike@addtoit.com>
Cc: Chris Zankel <chris@zankel.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-alpha@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-c6x-dev@linux-c6x.org
Cc: uclinux-h8-devel@lists.sourceforge.jp
Cc: linux-hexagon@vger.kernel.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Cc: linux-mips@vger.kernel.org
Cc: nios2-dev@lists.rocketboards.org
Cc: openrisc@lists.librecores.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-um@lists.infradead.org
Cc: linux-xtensa@linux-xtensa.org
Cc: linux-mm@kvack.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
arch/alpha/include/asm/page.h | 3 ---
arch/arc/include/asm/page.h | 2 +-
arch/arm/include/asm/page.h | 4 +---
arch/arm64/include/asm/page.h | 4 +---
arch/c6x/include/asm/page.h | 5 +----
arch/csky/include/asm/page.h | 3 ---
arch/h8300/include/asm/page.h | 2 --
arch/hexagon/include/asm/page.h | 3 +--
arch/ia64/include/asm/page.h | 5 +----
arch/m68k/include/asm/page.h | 3 ---
arch/microblaze/include/asm/page.h | 2 --
arch/mips/include/asm/page.h | 5 +----
arch/nds32/include/asm/page.h | 3 ---
arch/nios2/include/asm/page.h | 3 +--
arch/openrisc/include/asm/page.h | 5 -----
arch/parisc/include/asm/page.h | 3 ---
arch/powerpc/include/asm/page.h | 9 ++-------
arch/powerpc/include/asm/page_64.h | 7 ++-----
arch/riscv/include/asm/page.h | 3 +--
arch/s390/include/asm/page.h | 3 +--
arch/sh/include/asm/page.h | 3 ---
arch/sparc/include/asm/page_32.h | 3 ---
arch/sparc/include/asm/page_64.h | 3 ---
arch/unicore32/include/asm/page.h | 3 ---
arch/x86/include/asm/page_types.h | 4 +---
arch/x86/um/asm/vm-flags.h | 10 ++--------
arch/xtensa/include/asm/page.h | 3 ---
include/linux/mm.h | 15 +++++++++++++++
28 files changed, 32 insertions(+), 89 deletions(-)
diff --git a/arch/alpha/include/asm/page.h b/arch/alpha/include/asm/page.h
index f3fb2848470a..e241bd88880f 100644
--- a/arch/alpha/include/asm/page.h
+++ b/arch/alpha/include/asm/page.h
@@ -90,9 +90,6 @@ typedef struct page *pgtable_t;
#define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
#endif /* CONFIG_DISCONTIGMEM */
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/arc/include/asm/page.h b/arch/arc/include/asm/page.h
index 0a32e8cfd074..b0dfed0f12be 100644
--- a/arch/arc/include/asm/page.h
+++ b/arch/arc/include/asm/page.h
@@ -102,7 +102,7 @@ typedef pte_t * pgtable_t;
#define virt_addr_valid(kaddr) pfn_valid(virt_to_pfn(kaddr))
/* Default Permissions for stack/heaps pages (Non Executable) */
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_NON_EXEC
#define WANT_PAGE_VIRTUAL 1
diff --git a/arch/arm/include/asm/page.h b/arch/arm/include/asm/page.h
index c2b75cba26df..11b058a72a5b 100644
--- a/arch/arm/include/asm/page.h
+++ b/arch/arm/include/asm/page.h
@@ -161,9 +161,7 @@ extern int pfn_valid(unsigned long);
#endif /* !__ASSEMBLY__ */
-#define VM_DATA_DEFAULT_FLAGS \
- (((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
- VM_READ | VM_WRITE | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
#include <asm-generic/getorder.h>
diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h
index d39ddb258a04..cb4e1e6ca385 100644
--- a/arch/arm64/include/asm/page.h
+++ b/arch/arm64/include/asm/page.h
@@ -32,9 +32,7 @@ extern int pfn_valid(unsigned long);
#endif /* !__ASSEMBLY__ */
-#define VM_DATA_DEFAULT_FLAGS \
- (((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
- VM_READ | VM_WRITE | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
#include <asm-generic/getorder.h>
diff --git a/arch/c6x/include/asm/page.h b/arch/c6x/include/asm/page.h
index 70db1e7632bc..40079899084d 100644
--- a/arch/c6x/include/asm/page.h
+++ b/arch/c6x/include/asm/page.h
@@ -2,10 +2,7 @@
#ifndef _ASM_C6X_PAGE_H
#define _ASM_C6X_PAGE_H
-#define VM_DATA_DEFAULT_FLAGS \
- (VM_READ | VM_WRITE | \
- ((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
#include <asm-generic/page.h>
diff --git a/arch/csky/include/asm/page.h b/arch/csky/include/asm/page.h
index 9738eacefdc7..9b98bf31d57c 100644
--- a/arch/csky/include/asm/page.h
+++ b/arch/csky/include/asm/page.h
@@ -85,9 +85,6 @@ extern unsigned long va_pa_offset;
PHYS_OFFSET_OFFSET)
#define virt_to_page(x) (mem_map + MAP_NR(x))
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#define pfn_to_kaddr(x) __va(PFN_PHYS(x))
#include <asm-generic/memory_model.h>
diff --git a/arch/h8300/include/asm/page.h b/arch/h8300/include/asm/page.h
index 8da5124ad344..53e037544239 100644
--- a/arch/h8300/include/asm/page.h
+++ b/arch/h8300/include/asm/page.h
@@ -6,8 +6,6 @@
#include <linux/types.h>
#define MAP_NR(addr) (((uintptr_t)(addr)-PAGE_OFFSET) >> PAGE_SHIFT)
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
#ifndef __ASSEMBLY__
extern unsigned long rom_length;
diff --git a/arch/hexagon/include/asm/page.h b/arch/hexagon/include/asm/page.h
index ee31f36f48f3..7cbf719c578e 100644
--- a/arch/hexagon/include/asm/page.h
+++ b/arch/hexagon/include/asm/page.h
@@ -93,8 +93,7 @@ struct page;
#define virt_to_page(kaddr) pfn_to_page(PFN_DOWN(__pa(kaddr)))
/* Default vm area behavior is non-executable. */
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_NON_EXEC
#define pfn_valid(pfn) ((pfn) < max_mapnr)
#define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
diff --git a/arch/ia64/include/asm/page.h b/arch/ia64/include/asm/page.h
index 5798bd2b462c..b69a5499d75b 100644
--- a/arch/ia64/include/asm/page.h
+++ b/arch/ia64/include/asm/page.h
@@ -218,10 +218,7 @@ get_order (unsigned long size)
#define PAGE_OFFSET RGN_BASE(RGN_KERNEL)
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC | \
- (((current->personality & READ_IMPLIES_EXEC) != 0) \
- ? VM_EXEC : 0))
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
#define GATE_ADDR RGN_BASE(RGN_GATE)
diff --git a/arch/m68k/include/asm/page.h b/arch/m68k/include/asm/page.h
index 05e1e1e77a9a..9d2ef0a51720 100644
--- a/arch/m68k/include/asm/page.h
+++ b/arch/m68k/include/asm/page.h
@@ -55,9 +55,6 @@ extern unsigned long _ramend;
#define __phys_to_pfn(paddr) ((unsigned long)((paddr) >> PAGE_SHIFT))
#define __pfn_to_phys(pfn) PFN_PHYS(pfn)
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/getorder.h>
#endif /* _M68K_PAGE_H */
diff --git a/arch/microblaze/include/asm/page.h b/arch/microblaze/include/asm/page.h
index f4b44b24b02e..43bd560b7b67 100644
--- a/arch/microblaze/include/asm/page.h
+++ b/arch/microblaze/include/asm/page.h
@@ -197,8 +197,6 @@ extern int page_is_ram(unsigned long pfn);
#ifdef CONFIG_MMU
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
#endif /* CONFIG_MMU */
#endif /* __KERNEL__ */
diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
index 0ba4ce6e2bf3..e2f503fc7a84 100644
--- a/arch/mips/include/asm/page.h
+++ b/arch/mips/include/asm/page.h
@@ -253,10 +253,7 @@ extern bool __virt_addr_valid(const volatile void *kaddr);
#define virt_addr_valid(kaddr) \
__virt_addr_valid((const volatile void *) (kaddr))
-#define VM_DATA_DEFAULT_FLAGS \
- (VM_READ | VM_WRITE | \
- ((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/nds32/include/asm/page.h b/arch/nds32/include/asm/page.h
index 86b32014c5f9..add33a7f02c8 100644
--- a/arch/nds32/include/asm/page.h
+++ b/arch/nds32/include/asm/page.h
@@ -59,9 +59,6 @@ typedef struct page *pgtable_t;
#endif /* !__ASSEMBLY__ */
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#endif /* __KERNEL__ */
#endif
diff --git a/arch/nios2/include/asm/page.h b/arch/nios2/include/asm/page.h
index 79fcac61f6ef..6a989819a7c1 100644
--- a/arch/nios2/include/asm/page.h
+++ b/arch/nios2/include/asm/page.h
@@ -98,8 +98,7 @@ static inline bool pfn_valid(unsigned long pfn)
# define virt_to_page(vaddr) pfn_to_page(PFN_DOWN(virt_to_phys(vaddr)))
# define virt_addr_valid(vaddr) pfn_valid(PFN_DOWN(virt_to_phys(vaddr)))
-# define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+# define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_NON_EXEC
#include <asm-generic/memory_model.h>
diff --git a/arch/openrisc/include/asm/page.h b/arch/openrisc/include/asm/page.h
index 01069db59454..aab6e64d6db4 100644
--- a/arch/openrisc/include/asm/page.h
+++ b/arch/openrisc/include/asm/page.h
@@ -86,11 +86,6 @@ typedef struct page *pgtable_t;
#endif /* __ASSEMBLY__ */
-
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
-
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/parisc/include/asm/page.h b/arch/parisc/include/asm/page.h
index 796ae29e9b9a..6b3f6740a6a6 100644
--- a/arch/parisc/include/asm/page.h
+++ b/arch/parisc/include/asm/page.h
@@ -180,9 +180,6 @@ extern int npmem_ranges;
#define page_to_phys(page) (page_to_pfn(page) << PAGE_SHIFT)
#define virt_to_page(kaddr) pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
#include <asm/pdc.h>
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 080a0bf8e54b..3ee8df0f66e0 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -240,13 +240,8 @@ static inline bool pfn_valid(unsigned long pfn)
* and needs to be executable. This means the whole heap ends
* up being executable.
*/
-#define VM_DATA_DEFAULT_FLAGS32 \
- (((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
- VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
-#define VM_DATA_DEFAULT_FLAGS64 (VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS32 VM_DATA_FLAGS_TSK_EXEC
+#define VM_DATA_DEFAULT_FLAGS64 VM_DATA_FLAGS_NON_EXEC
#ifdef __powerpc64__
#include <asm/page_64.h>
diff --git a/arch/powerpc/include/asm/page_64.h b/arch/powerpc/include/asm/page_64.h
index 5962797f784a..79a9b7c6a132 100644
--- a/arch/powerpc/include/asm/page_64.h
+++ b/arch/powerpc/include/asm/page_64.h
@@ -94,11 +94,8 @@ extern u64 ppc64_pft_size;
* stack by default, so in the absence of a PT_GNU_STACK program header
* we turn execute permission off.
*/
-#define VM_STACK_DEFAULT_FLAGS32 (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
-#define VM_STACK_DEFAULT_FLAGS64 (VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_STACK_DEFAULT_FLAGS32 VM_DATA_FLAGS_EXEC
+#define VM_STACK_DEFAULT_FLAGS64 VM_DATA_FLAGS_NON_EXEC
#define VM_STACK_DEFAULT_FLAGS \
(is_32bit_task() ? \
diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
index 8ca1930caa44..2d50f76efe48 100644
--- a/arch/riscv/include/asm/page.h
+++ b/arch/riscv/include/asm/page.h
@@ -137,8 +137,7 @@ extern phys_addr_t __phys_addr_symbol(unsigned long x);
#define virt_addr_valid(vaddr) (pfn_valid(virt_to_pfn(vaddr)))
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_NON_EXEC
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/s390/include/asm/page.h b/arch/s390/include/asm/page.h
index 1019efd85b9d..089f49230966 100644
--- a/arch/s390/include/asm/page.h
+++ b/arch/s390/include/asm/page.h
@@ -176,8 +176,7 @@ static inline int devmem_is_allowed(unsigned long pfn)
#define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_NON_EXEC
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/sh/include/asm/page.h b/arch/sh/include/asm/page.h
index 5eef8be3e59f..ea8d68f58e39 100644
--- a/arch/sh/include/asm/page.h
+++ b/arch/sh/include/asm/page.h
@@ -182,9 +182,6 @@ typedef struct page *pgtable_t;
#endif
#define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/sparc/include/asm/page_32.h b/arch/sparc/include/asm/page_32.h
index b76d59edec8c..478260002836 100644
--- a/arch/sparc/include/asm/page_32.h
+++ b/arch/sparc/include/asm/page_32.h
@@ -133,9 +133,6 @@ extern unsigned long pfn_base;
#define pfn_valid(pfn) (((pfn) >= (pfn_base)) && (((pfn)-(pfn_base)) < max_mapnr))
#define virt_addr_valid(kaddr) ((((unsigned long)(kaddr)-PAGE_OFFSET)>>PAGE_SHIFT) < max_mapnr)
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/sparc/include/asm/page_64.h b/arch/sparc/include/asm/page_64.h
index e80f2d5bf62f..254dffd85fb1 100644
--- a/arch/sparc/include/asm/page_64.h
+++ b/arch/sparc/include/asm/page_64.h
@@ -158,9 +158,6 @@ extern unsigned long PAGE_OFFSET;
#endif /* !(__ASSEMBLY__) */
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/getorder.h>
#endif /* _SPARC64_PAGE_H */
diff --git a/arch/unicore32/include/asm/page.h b/arch/unicore32/include/asm/page.h
index 8a89335673f9..96d6bdf180bd 100644
--- a/arch/unicore32/include/asm/page.h
+++ b/arch/unicore32/include/asm/page.h
@@ -69,9 +69,6 @@ extern int pfn_valid(unsigned long);
#endif /* !__ASSEMBLY__ */
-#define VM_DATA_DEFAULT_FLAGS \
- (VM_READ | VM_WRITE | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/getorder.h>
#endif
diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
index c85e15010f48..e27aa6be6320 100644
--- a/arch/x86/include/asm/page_types.h
+++ b/arch/x86/include/asm/page_types.h
@@ -35,9 +35,7 @@
#define PAGE_OFFSET ((unsigned long)__PAGE_OFFSET)
-#define VM_DATA_DEFAULT_FLAGS \
- (((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0 ) | \
- VM_READ | VM_WRITE | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
#define __PHYSICAL_START ALIGN(CONFIG_PHYSICAL_START, \
CONFIG_PHYSICAL_ALIGN)
diff --git a/arch/x86/um/asm/vm-flags.h b/arch/x86/um/asm/vm-flags.h
index 7c297e9e2413..df7a3896f5dd 100644
--- a/arch/x86/um/asm/vm-flags.h
+++ b/arch/x86/um/asm/vm-flags.h
@@ -9,17 +9,11 @@
#ifdef CONFIG_X86_32
-#define VM_DATA_DEFAULT_FLAGS \
- (VM_READ | VM_WRITE | \
- ((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0 ) | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
#else
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-#define VM_STACK_DEFAULT_FLAGS (VM_GROWSDOWN | VM_READ | VM_WRITE | \
- VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_STACK_DEFAULT_FLAGS (VM_GROWSDOWN | VM_DATA_FLAGS_EXEC)
#endif
#endif
diff --git a/arch/xtensa/include/asm/page.h b/arch/xtensa/include/asm/page.h
index f4771c29c7e9..37ce25ef92d6 100644
--- a/arch/xtensa/include/asm/page.h
+++ b/arch/xtensa/include/asm/page.h
@@ -203,8 +203,5 @@ static inline unsigned long ___pa(unsigned long va)
#endif /* __ASSEMBLY__ */
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/memory_model.h>
#endif /* _XTENSA_PAGE_H */
diff --git a/include/linux/mm.h b/include/linux/mm.h
index b0e53ef13ff1..7a764ae6ab68 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -342,6 +342,21 @@ extern unsigned int kobjsize(const void *objp);
/* Bits set in the VMA until the stack is in its final location */
#define VM_STACK_INCOMPLETE_SETUP (VM_RAND_READ | VM_SEQ_READ)
+#define TASK_EXEC ((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0)
+
+/* Common data flag combinations */
+#define VM_DATA_FLAGS_TSK_EXEC (VM_READ | VM_WRITE | TASK_EXEC | \
+ VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_FLAGS_NON_EXEC (VM_READ | VM_WRITE | VM_MAYREAD | \
+ VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_FLAGS_EXEC (VM_READ | VM_WRITE | VM_EXEC | \
+ VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+
+#ifndef VM_DATA_DEFAULT_FLAGS /* arch can override this */
+#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
+ VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#endif
+
#ifndef VM_STACK_DEFAULT_FLAGS /* arch can override this */
#define VM_STACK_DEFAULT_FLAGS VM_DATA_DEFAULT_FLAGS
#endif
--
2.20.1
^ permalink raw reply related
* [RFC 0/3] mm/vma: some new flags and helpers
From: Anshuman Khandual @ 2020-03-02 6:47 UTC (permalink / raw)
To: linux-mm
Cc: uclinux-h8-devel, linux-s390, linux-ia64, linux-c6x-dev,
linux-parisc, Anshuman Khandual, linux-hexagon, linux-xtensa,
linux-sh, linux-um, linux-kernel, linux-mips, linux-m68k,
openrisc, linux-alpha, sparclinux, nios2-dev, linux-riscv,
linux-snps-arc, linuxppc-dev, linux-arm-kernel
The motivation here is to consolidate VMA flag combinations commonly used
across platforms and reduce code duplication while making it uncluttered
in general.
This first introduces a default VM_DATA_DEFAULT_FLAGS which platforms can
easily fall back on without requiring to define any similar data flag
combinations as they currently do. This also adds some more common data
flag combinations which are generally used when the platforms decide to
override the default.
The second patch consolidates VM_READ, VM_WRITE, VM_EXEC as VM_ACCESS_FLAGS
extending the existing VMA accessibility concept via vma_is_accessibility().
VM_ACCESS_FLAGS replaces many other instances which used check all three
VMA access flags simultaneously.
While here, this also adds some more special VMA flag based helpers which
wraps around similar checks at various places thus improving readability.
This series intentionally limits these new helpers which are applicable
only for special purpose VM flags than the more common ones like VM_READ,
VM_WRITE, VM_EXEC, VM_SHARED etc just to limit code churn. But if there is
common agreement that every flag should have it's own wrapper here, we can
do that as well. Otherwise if this patch seems really unnecessary with much
code churn, will be happy to drop it.
Reviews, comments, suggestions and concerns welcome. Thank you.
This series is based on v5.6-r4 after applying these patches.
1. https://patchwork.kernel.org/cover/11399319/
2. https://patchwork.kernel.org/patch/11399379/
This series is build tested across multiple architectures but boot tested
only on arm64 and x86 platforms.
Cc: linux-alpha@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-c6x-dev@linux-c6x.org
Cc: uclinux-h8-devel@lists.sourceforge.jp
Cc: linux-hexagon@vger.kernel.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Cc: linux-mips@vger.kernel.org
Cc: nios2-dev@lists.rocketboards.org
Cc: openrisc@lists.librecores.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-um@lists.infradead.org
Cc: linux-xtensa@linux-xtensa.org
Cc: linux-mm@kvack.org
Anshuman Khandual (3):
mm/vma: Define a default value for VM_DATA_DEFAULT_FLAGS
mm/vma: Introduce VM_ACCESS_FLAGS
mm/vma: Introduce some more VMA flag wrappers
arch/alpha/include/asm/page.h | 3 --
arch/arc/include/asm/page.h | 2 +-
arch/arm/include/asm/page.h | 4 +-
arch/arm/mm/fault.c | 2 +-
arch/arm64/include/asm/page.h | 4 +-
arch/arm64/mm/fault.c | 2 +-
arch/c6x/include/asm/page.h | 5 +--
arch/c6x/include/asm/processor.h | 2 +-
arch/csky/include/asm/page.h | 3 --
arch/h8300/include/asm/page.h | 2 -
arch/hexagon/include/asm/page.h | 3 +-
arch/ia64/include/asm/page.h | 5 +--
arch/m68k/include/asm/page.h | 3 --
arch/microblaze/include/asm/page.h | 2 -
arch/mips/include/asm/page.h | 5 +--
arch/nds32/include/asm/page.h | 3 --
arch/nds32/mm/fault.c | 2 +-
arch/nios2/include/asm/page.h | 3 +-
arch/nios2/include/asm/processor.h | 2 +-
arch/openrisc/include/asm/page.h | 5 ---
arch/parisc/include/asm/page.h | 3 --
arch/powerpc/include/asm/page.h | 9 +----
arch/powerpc/include/asm/page_64.h | 7 +---
arch/powerpc/mm/book3s64/pkeys.c | 2 +-
arch/riscv/include/asm/page.h | 3 +-
arch/s390/include/asm/page.h | 3 +-
arch/s390/mm/fault.c | 2 +-
arch/sh/include/asm/page.h | 3 --
arch/sh/include/asm/processor_64.h | 2 +-
arch/sparc/include/asm/mman.h | 2 +-
arch/sparc/include/asm/page_32.h | 3 --
arch/sparc/include/asm/page_64.h | 3 --
arch/unicore32/include/asm/page.h | 3 --
arch/unicore32/mm/fault.c | 2 +-
arch/x86/include/asm/page_types.h | 4 +-
arch/x86/mm/pkeys.c | 2 +-
arch/x86/um/asm/vm-flags.h | 10 +----
arch/xtensa/include/asm/page.h | 3 --
drivers/staging/gasket/gasket_core.c | 2 +-
fs/binfmt_elf.c | 2 +-
fs/proc/task_mmu.c | 14 +++----
include/linux/huge_mm.h | 4 +-
include/linux/mm.h | 58 +++++++++++++++++++++++++++-
kernel/events/core.c | 2 +-
kernel/events/uprobes.c | 2 +-
mm/gup.c | 2 +-
mm/huge_memory.c | 6 +--
mm/hugetlb.c | 4 +-
mm/ksm.c | 8 ++--
mm/madvise.c | 4 +-
mm/memory.c | 4 +-
mm/migrate.c | 4 +-
mm/mlock.c | 4 +-
mm/mmap.c | 20 +++++-----
mm/mprotect.c | 9 ++---
mm/mremap.c | 4 +-
mm/msync.c | 3 +-
mm/rmap.c | 6 +--
mm/shmem.c | 8 ++--
59 files changed, 140 insertions(+), 158 deletions(-)
--
2.20.1
^ permalink raw reply
* Re: [PATCH] powerpc/sysdev: fix compile errors
From: Christophe Leroy @ 2020-03-02 6:38 UTC (permalink / raw)
To: WANG Wenhu, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Kate Stewart, Greg Kroah-Hartman,
Allison Randal, Thomas Gleixner, linuxppc-dev, linux-kernel
Cc: wenhu.pku, trivial
In-Reply-To: <20200302053801.26027-1-wenhu.wang@vivo.com>
Le 02/03/2020 à 06:37, WANG Wenhu a écrit :
> Include linux/io.h into fsl_85xx_cache_sram.c to fix the
> implicit-declaration compile errors when building Cache-Sram.
>
> arch/powerpc/sysdev/fsl_85xx_cache_sram.c: In function ‘instantiate_cache_sram’:
> arch/powerpc/sysdev/fsl_85xx_cache_sram.c:97:26: error: implicit declaration of function ‘ioremap_coherent’; did you mean ‘bitmap_complement’? [-Werror=implicit-function-declaration]
> cache_sram->base_virt = ioremap_coherent(cache_sram->base_phys,
> ^~~~~~~~~~~~~~~~
> bitmap_complement
> arch/powerpc/sysdev/fsl_85xx_cache_sram.c:97:24: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
> cache_sram->base_virt = ioremap_coherent(cache_sram->base_phys,
> ^
> arch/powerpc/sysdev/fsl_85xx_cache_sram.c:123:2: error: implicit declaration of function ‘iounmap’; did you mean ‘roundup’? [-Werror=implicit-function-declaration]
> iounmap(cache_sram->base_virt);
> ^~~~~~~
> roundup
> cc1: all warnings being treated as errors
>
> Fixed: commit 6db92cc9d07d ("powerpc/85xx: add cache-sram support")
> Signed-off-by: WANG Wenhu <wenhu.wang@vivo.com>
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/sysdev/fsl_85xx_cache_sram.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/sysdev/fsl_85xx_cache_sram.c b/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
> index f6c665dac725..be3aef4229d7 100644
> --- a/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
> +++ b/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
> @@ -17,6 +17,7 @@
> #include <linux/of_platform.h>
> #include <asm/pgtable.h>
> #include <asm/fsl_85xx_cache_sram.h>
> +#include <linux/io.h>
>
> #include "fsl_85xx_cache_ctlr.h"
>
>
^ permalink raw reply
* Re: [PATCH v3 16/27] powerpc/powernv/pmem: Register a character device for userspace to interact with
From: Alastair D'Silva @ 2020-03-02 6:05 UTC (permalink / raw)
To: Andrew Donnellan
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <1e980dc7-109a-d96f-1329-1c38918e2bba@linux.ibm.com>
On Mon, 2020-03-02 at 16:34 +1100, Andrew Donnellan wrote:
> On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > This patch introduces a character device (/dev/ocxl-scmX) which
> > further
> > patches will use to interact with userspace.
>
> As with the comments on other patches in this series, this commit
> message is lacking in explanation. What's the purpose of this device?
>
I'll reword this for v4.
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> > arch/powerpc/platforms/powernv/pmem/ocxl.c | 116
> > +++++++++++++++++-
> > .../platforms/powernv/pmem/ocxl_internal.h | 2 +
> > 2 files changed, 116 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > index b8bd7e703b19..63109a870d2c 100644
> > --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > @@ -10,6 +10,7 @@
> > #include <misc/ocxl.h>
> > #include <linux/delay.h>
> > #include <linux/ndctl.h>
> > +#include <linux/fs.h>
> > #include <linux/mm_types.h>
> > #include <linux/memory_hotplug.h>
> > #include "ocxl_internal.h"
> > @@ -339,6 +340,9 @@ static void free_ocxlpmem(struct ocxlpmem
> > *ocxlpmem)
> >
> > free_minor(ocxlpmem);
> >
> > + if (ocxlpmem->cdev.owner)
> > + cdev_del(&ocxlpmem->cdev);
> > +
> > if (ocxlpmem->metadata_addr)
> > devm_memunmap(&ocxlpmem->dev, ocxlpmem->metadata_addr);
> >
> > @@ -396,6 +400,70 @@ static int ocxlpmem_register(struct ocxlpmem
> > *ocxlpmem)
> > return device_register(&ocxlpmem->dev);
> > }
> >
> > +static void ocxlpmem_put(struct ocxlpmem *ocxlpmem)
> > +{
> > + put_device(&ocxlpmem->dev);
> > +}
> > +
> > +static struct ocxlpmem *ocxlpmem_get(struct ocxlpmem *ocxlpmem)
> > +{
> > + return (get_device(&ocxlpmem->dev) == NULL) ? NULL : ocxlpmem;
> > +}
> > +
> > +static struct ocxlpmem *find_and_get_ocxlpmem(dev_t devno)
> > +{
> > + struct ocxlpmem *ocxlpmem;
> > + int minor = MINOR(devno);
> > + /*
> > + * We don't declare an RCU critical section here, as our AFU
> > + * is protected by a re0ference counter on the device. By the
> > time the
> > + * minor number of a device is removed from the idr, the ref
> > count of
> > + * the device is already at 0, so no user API will access that
> > AFU and
> > + * this function can't return it.
> > + */
> > + ocxlpmem = idr_find(&minors_idr, minor);
> > + if (ocxlpmem)
> > + ocxlpmem_get(ocxlpmem);
> > + return ocxlpmem;
> > +}
> > +
> > +static int file_open(struct inode *inode, struct file *file)
> > +{
> > + struct ocxlpmem *ocxlpmem;
> > +
> > + ocxlpmem = find_and_get_ocxlpmem(inode->i_rdev);
> > + if (!ocxlpmem)
> > + return -ENODEV;
> > +
> > + file->private_data = ocxlpmem;
> > + return 0;
> > +}
> > +
> > +static int file_release(struct inode *inode, struct file *file)
> > +{
> > + struct ocxlpmem *ocxlpmem = file->private_data;
> > +
> > + ocxlpmem_put(ocxlpmem);
> > + return 0;
> > +}
> > +
> > +static const struct file_operations fops = {
> > + .owner = THIS_MODULE,
> > + .open = file_open,
> > + .release = file_release,
> > +};
> > +
> > +/**
> > + * create_cdev() - Create the chardev in /dev for the device
> > + * @ocxlpmem: the SCM metadata
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int create_cdev(struct ocxlpmem *ocxlpmem)
> > +{
> > + cdev_init(&ocxlpmem->cdev, &fops);
> > + return cdev_add(&ocxlpmem->cdev, ocxlpmem->dev.devt, 1);
> > +}
> > +
> > /**
> > * ocxlpmem_remove() - Free an OpenCAPI persistent memory device
> > * @pdev: the PCI device information struct
> > @@ -572,6 +640,11 @@ static int probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> > goto err;
> > }
> >
> > + if (create_cdev(ocxlpmem)) {
> > + dev_err(&pdev->dev, "Could not create character
> > device\n");
> > + goto err;
> > + }
> > +
> > elapsed = 0;
> > timeout = ocxlpmem->readiness_timeout + ocxlpmem-
> > >memory_available_timeout;
> > while (!is_usable(ocxlpmem, false)) {
> > @@ -613,20 +686,59 @@ static struct pci_driver pci_driver = {
> > .shutdown = ocxlpmem_remove,
> > };
> >
> > +static int file_init(void)
> > +{
> > + int rc;
> > +
> > + mutex_init(&minors_idr_lock);
> > + idr_init(&minors_idr);
> > +
> > + rc = alloc_chrdev_region(&ocxlpmem_dev, 0, NUM_MINORS, "ocxl-
> > pmem");
>
> If the driver is going to be called "ocxlpmem" can we standardise on
> that without the extra hyphen?
Ok
> > + if (rc) {
> > + idr_destroy(&minors_idr);
> > + pr_err("Unable to allocate OpenCAPI persistent memory
> > major number: %d\n", rc);
> > + return rc;
> > + }
> > +
> > + ocxlpmem_class = class_create(THIS_MODULE, "ocxl-pmem");
> > + if (IS_ERR(ocxlpmem_class)) {
> > + idr_destroy(&minors_idr);
> > + pr_err("Unable to create ocxl-pmem class\n");
> > + unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
> > + return PTR_ERR(ocxlpmem_class);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void file_exit(void)
> > +{
> > + class_destroy(ocxlpmem_class);
> > + unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
> > + idr_destroy(&minors_idr);
> > +}
> > +
> > static int __init ocxlpmem_init(void)
> > {
> > - int rc = 0;
> > + int rc;
> >
> > - rc = pci_register_driver(&pci_driver);
> > + rc = file_init();
> > if (rc)
> > return rc;
> >
> > + rc = pci_register_driver(&pci_driver);
> > + if (rc) {
> > + file_exit();
> > + return rc;
> > + }
> > +
> > return 0;
> > }
> >
> > static void ocxlpmem_exit(void)
> > {
> > pci_unregister_driver(&pci_driver);
> > + file_exit();
> > }
> >
> > module_init(ocxlpmem_init);
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* [PATCH] powerpc/sysdev: fix compile errors
From: WANG Wenhu @ 2020-03-02 5:37 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Kate Stewart, Greg Kroah-Hartman, Allison Randal, Thomas Gleixner,
wangwenhu, linuxppc-dev, linux-kernel
Cc: wenhu.pku, trivial
Include linux/io.h into fsl_85xx_cache_sram.c to fix the
implicit-declaration compile errors when building Cache-Sram.
arch/powerpc/sysdev/fsl_85xx_cache_sram.c: In function ‘instantiate_cache_sram’:
arch/powerpc/sysdev/fsl_85xx_cache_sram.c:97:26: error: implicit declaration of function ‘ioremap_coherent’; did you mean ‘bitmap_complement’? [-Werror=implicit-function-declaration]
cache_sram->base_virt = ioremap_coherent(cache_sram->base_phys,
^~~~~~~~~~~~~~~~
bitmap_complement
arch/powerpc/sysdev/fsl_85xx_cache_sram.c:97:24: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
cache_sram->base_virt = ioremap_coherent(cache_sram->base_phys,
^
arch/powerpc/sysdev/fsl_85xx_cache_sram.c:123:2: error: implicit declaration of function ‘iounmap’; did you mean ‘roundup’? [-Werror=implicit-function-declaration]
iounmap(cache_sram->base_virt);
^~~~~~~
roundup
cc1: all warnings being treated as errors
Fixed: commit 6db92cc9d07d ("powerpc/85xx: add cache-sram support")
Signed-off-by: WANG Wenhu <wenhu.wang@vivo.com>
---
arch/powerpc/sysdev/fsl_85xx_cache_sram.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/sysdev/fsl_85xx_cache_sram.c b/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
index f6c665dac725..be3aef4229d7 100644
--- a/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
+++ b/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
@@ -17,6 +17,7 @@
#include <linux/of_platform.h>
#include <asm/pgtable.h>
#include <asm/fsl_85xx_cache_sram.h>
+#include <linux/io.h>
#include "fsl_85xx_cache_ctlr.h"
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v3 21/27] powerpc/powernv/pmem: Add an IOCTL to request controller health & perf data
From: Alastair D'Silva @ 2020-03-02 5:40 UTC (permalink / raw)
To: Andrew Donnellan
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <fdc5faec-d03d-3cba-4a9c-add7e522ad13@linux.ibm.com>
On Fri, 2020-02-28 at 17:12 +1100, Andrew Donnellan wrote:
> On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > When health & performance data is requested from the controller,
> > it responds with an error log containing the requested information.
> >
> > This patch allows the request to me issued via an IOCTL.
>
> A better explanation would be good - this IOCTL triggers a request
> to
> the controller to collect controller health/perf data, and the
> controller will later respond with an error log that can be picked
> up
> via the error log IOCTL that you've defined earlier.
>
>
Ok
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* RE: [PATCH v3 25/27] powerpc/powernv/pmem: Expose the serial number in sysfs
From: Alastair D'Silva @ 2020-03-02 5:38 UTC (permalink / raw)
To: Greg Kroah-Hartman, Andrew Donnellan
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
linux-kernel, Vishal Verma, Frederic Barrat, Paul Mackerras,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <4075f48568fee61123579d4edea0e7939b4b2e6c.camel@au1.ibm.com>
On Mon, 2020-03-02 at 10:42 +1100, Alastair D'Silva wrote:
> On Fri, 2020-02-28 at 08:15 +0100, Greg Kroah-Hartman wrote:
> > On Fri, Feb 28, 2020 at 05:25:31PM +1100, Andrew Donnellan wrote:
> > > On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> > > > +int ocxlpmem_sysfs_add(struct ocxlpmem *ocxlpmem)
> > > > +{
> > > > + int i, rc;
> > > > +
> > > > + for (i = 0; i < ARRAY_SIZE(attrs); i++) {
> > > > + rc = device_create_file(&ocxlpmem->dev,
> > > > &attrs[i]);
> > > > + if (rc) {
> > > > + for (; --i >= 0;)
> > > > + device_remove_file(&ocxlpmem-
> > > > >dev,
> > > > &attrs[i]);
> > >
> > > I'd rather avoid weird for loop constructs if possible.
> > >
> > > Is it actually dangerous to call device_remove_file() on an attr
> > > that hasn't
> > > been added? If not then I'd rather define an err: label and loop
> > > over the
> > > whole array there.
> >
> > None of this should be used at all, just use attribute groups
> > properly
> > and the driver core will handle this all for you.
> >
> > device_create/remove_file should never be called by anyone anymore
> > if
> > at all
> > possible.
> >
> > thanks,
> >
> > greg k-h
>
> Thanks, I'll rework it to use the .groups member of struct
> pci_driver.
>
I ended up making these available as DIMM attributes instead.
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH v3 16/27] powerpc/powernv/pmem: Register a character device for userspace to interact with
From: Andrew Donnellan @ 2020-03-02 5:34 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200221032720.33893-17-alastair@au1.ibm.com>
On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> This patch introduces a character device (/dev/ocxl-scmX) which further
> patches will use to interact with userspace.
As with the comments on other patches in this series, this commit
message is lacking in explanation. What's the purpose of this device?
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
> arch/powerpc/platforms/powernv/pmem/ocxl.c | 116 +++++++++++++++++-
> .../platforms/powernv/pmem/ocxl_internal.h | 2 +
> 2 files changed, 116 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> index b8bd7e703b19..63109a870d2c 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> @@ -10,6 +10,7 @@
> #include <misc/ocxl.h>
> #include <linux/delay.h>
> #include <linux/ndctl.h>
> +#include <linux/fs.h>
> #include <linux/mm_types.h>
> #include <linux/memory_hotplug.h>
> #include "ocxl_internal.h"
> @@ -339,6 +340,9 @@ static void free_ocxlpmem(struct ocxlpmem *ocxlpmem)
>
> free_minor(ocxlpmem);
>
> + if (ocxlpmem->cdev.owner)
> + cdev_del(&ocxlpmem->cdev);
> +
> if (ocxlpmem->metadata_addr)
> devm_memunmap(&ocxlpmem->dev, ocxlpmem->metadata_addr);
>
> @@ -396,6 +400,70 @@ static int ocxlpmem_register(struct ocxlpmem *ocxlpmem)
> return device_register(&ocxlpmem->dev);
> }
>
> +static void ocxlpmem_put(struct ocxlpmem *ocxlpmem)
> +{
> + put_device(&ocxlpmem->dev);
> +}
> +
> +static struct ocxlpmem *ocxlpmem_get(struct ocxlpmem *ocxlpmem)
> +{
> + return (get_device(&ocxlpmem->dev) == NULL) ? NULL : ocxlpmem;
> +}
> +
> +static struct ocxlpmem *find_and_get_ocxlpmem(dev_t devno)
> +{
> + struct ocxlpmem *ocxlpmem;
> + int minor = MINOR(devno);
> + /*
> + * We don't declare an RCU critical section here, as our AFU
> + * is protected by a re0ference counter on the device. By the time the
> + * minor number of a device is removed from the idr, the ref count of
> + * the device is already at 0, so no user API will access that AFU and
> + * this function can't return it.
> + */
> + ocxlpmem = idr_find(&minors_idr, minor);
> + if (ocxlpmem)
> + ocxlpmem_get(ocxlpmem);
> + return ocxlpmem;
> +}
> +
> +static int file_open(struct inode *inode, struct file *file)
> +{
> + struct ocxlpmem *ocxlpmem;
> +
> + ocxlpmem = find_and_get_ocxlpmem(inode->i_rdev);
> + if (!ocxlpmem)
> + return -ENODEV;
> +
> + file->private_data = ocxlpmem;
> + return 0;
> +}
> +
> +static int file_release(struct inode *inode, struct file *file)
> +{
> + struct ocxlpmem *ocxlpmem = file->private_data;
> +
> + ocxlpmem_put(ocxlpmem);
> + return 0;
> +}
> +
> +static const struct file_operations fops = {
> + .owner = THIS_MODULE,
> + .open = file_open,
> + .release = file_release,
> +};
> +
> +/**
> + * create_cdev() - Create the chardev in /dev for the device
> + * @ocxlpmem: the SCM metadata
> + * Return: 0 on success, negative on failure
> + */
> +static int create_cdev(struct ocxlpmem *ocxlpmem)
> +{
> + cdev_init(&ocxlpmem->cdev, &fops);
> + return cdev_add(&ocxlpmem->cdev, ocxlpmem->dev.devt, 1);
> +}
> +
> /**
> * ocxlpmem_remove() - Free an OpenCAPI persistent memory device
> * @pdev: the PCI device information struct
> @@ -572,6 +640,11 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> goto err;
> }
>
> + if (create_cdev(ocxlpmem)) {
> + dev_err(&pdev->dev, "Could not create character device\n");
> + goto err;
> + }
> +
> elapsed = 0;
> timeout = ocxlpmem->readiness_timeout + ocxlpmem->memory_available_timeout;
> while (!is_usable(ocxlpmem, false)) {
> @@ -613,20 +686,59 @@ static struct pci_driver pci_driver = {
> .shutdown = ocxlpmem_remove,
> };
>
> +static int file_init(void)
> +{
> + int rc;
> +
> + mutex_init(&minors_idr_lock);
> + idr_init(&minors_idr);
> +
> + rc = alloc_chrdev_region(&ocxlpmem_dev, 0, NUM_MINORS, "ocxl-pmem");
If the driver is going to be called "ocxlpmem" can we standardise on
that without the extra hyphen?
> + if (rc) {
> + idr_destroy(&minors_idr);
> + pr_err("Unable to allocate OpenCAPI persistent memory major number: %d\n", rc);
> + return rc;
> + }
> +
> + ocxlpmem_class = class_create(THIS_MODULE, "ocxl-pmem");
> + if (IS_ERR(ocxlpmem_class)) {
> + idr_destroy(&minors_idr);
> + pr_err("Unable to create ocxl-pmem class\n");
> + unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
> + return PTR_ERR(ocxlpmem_class);
> + }
> +
> + return 0;
> +}
> +
> +static void file_exit(void)
> +{
> + class_destroy(ocxlpmem_class);
> + unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
> + idr_destroy(&minors_idr);
> +}
> +
> static int __init ocxlpmem_init(void)
> {
> - int rc = 0;
> + int rc;
>
> - rc = pci_register_driver(&pci_driver);
> + rc = file_init();
> if (rc)
> return rc;
>
> + rc = pci_register_driver(&pci_driver);
> + if (rc) {
> + file_exit();
> + return rc;
> + }
> +
> return 0;
> }
>
> static void ocxlpmem_exit(void)
> {
> pci_unregister_driver(&pci_driver);
> + file_exit();
> }
>
> module_init(ocxlpmem_init);
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* [RFC 05/11] perf tools: Enable record and script to record and show hazard data
From: Ravi Bangoria @ 2020-03-02 5:23 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: mark.rutland, ravi.bangoria, ak, maddy, peterz, alexey.budankov,
Madhavan Srinivasan, adrian.hunter, acme, alexander.shishkin,
yao.jin, mingo, paulus, eranian, robert.richter, namhyung,
kim.phillips, jolsa, kan.liang
In-Reply-To: <20200302052355.36365-1-ravi.bangoria@linux.ibm.com>
From: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Introduce new perf record option "--hazard" to capture cpu pipeline
hazard data. Also enable perf script -D to dump raw values of it.
Sample o/p:
$ ./perf record -e r4010e --hazard -- ls
$ ./perf script -D
... PERF_RECORD_SAMPLE(IP, 0x2): ...
hazard information:
Inst Type 0x1
Inst Cache 0x1
Hazard Stage 0x4
Hazard Reason 0x3
Stall Stage 0x4
Stall Reason 0x2
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
tools/perf/Documentation/perf-record.txt | 3 +++
tools/perf/builtin-record.c | 1 +
tools/perf/util/event.h | 1 +
tools/perf/util/evsel.c | 10 ++++++++++
tools/perf/util/perf_event_attr_fprintf.c | 1 +
tools/perf/util/record.h | 1 +
tools/perf/util/session.c | 16 ++++++++++++++++
7 files changed, 33 insertions(+)
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index b23a4012a606..e7bd1b6938ce 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -283,6 +283,9 @@ OPTIONS
--phys-data::
Record the sample physical addresses.
+--hazard::
+ Record processor pipeline hazard and stall information.
+
-T::
--timestamp::
Record the sample timestamps. Use it with 'perf report -D' to see the
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 4c301466101b..6bd32d7bc4e9 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2301,6 +2301,7 @@ static struct option __record_options[] = {
OPT_BOOLEAN('s', "stat", &record.opts.inherit_stat,
"per thread counts"),
OPT_BOOLEAN('d', "data", &record.opts.sample_address, "Record the sample addresses"),
+ OPT_BOOLEAN(0, "hazard", &record.opts.hazard, "Record processor pipeline hazard and stall information"),
OPT_BOOLEAN(0, "phys-data", &record.opts.sample_phys_addr,
"Record the sample physical addresses"),
OPT_BOOLEAN(0, "sample-cpu", &record.opts.sample_cpu, "Record the sample cpu"),
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 85223159737c..ff0f03253a95 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -148,6 +148,7 @@ struct perf_sample {
struct stack_dump user_stack;
struct sample_read read;
struct aux_sample aux_sample;
+ struct perf_pipeline_haz_data *pipeline_haz;
};
#define PERF_MEM_DATA_SRC_NONE \
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index c8dc4450884c..e37ed7929c2c 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1080,6 +1080,9 @@ void perf_evsel__config(struct evsel *evsel, struct record_opts *opts,
if (opts->sample_phys_addr)
perf_evsel__set_sample_bit(evsel, PHYS_ADDR);
+ if (opts->hazard)
+ perf_evsel__set_sample_bit(evsel, PIPELINE_HAZ);
+
if (opts->no_buffering) {
attr->watermark = 0;
attr->wakeup_events = 1;
@@ -2265,6 +2268,13 @@ int perf_evsel__parse_sample(struct evsel *evsel, union perf_event *event,
array = (void *)array + sz;
}
+ if (type & PERF_SAMPLE_PIPELINE_HAZ) {
+ sz = sizeof(struct perf_pipeline_haz_data);
+ OVERFLOW_CHECK(array, sz, max_size);
+ data->pipeline_haz = (struct perf_pipeline_haz_data *)array;
+ array = (void *)array + sz;
+ }
+
return 0;
}
diff --git a/tools/perf/util/perf_event_attr_fprintf.c b/tools/perf/util/perf_event_attr_fprintf.c
index 651203126c71..d97e755c886b 100644
--- a/tools/perf/util/perf_event_attr_fprintf.c
+++ b/tools/perf/util/perf_event_attr_fprintf.c
@@ -35,6 +35,7 @@ static void __p_sample_type(char *buf, size_t size, u64 value)
bit_name(BRANCH_STACK), bit_name(REGS_USER), bit_name(STACK_USER),
bit_name(IDENTIFIER), bit_name(REGS_INTR), bit_name(DATA_SRC),
bit_name(WEIGHT), bit_name(PHYS_ADDR), bit_name(AUX),
+ bit_name(PIPELINE_HAZ),
{ .name = NULL, }
};
#undef bit_name
diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
index 5421fd2ad383..f1678a0bc8ce 100644
--- a/tools/perf/util/record.h
+++ b/tools/perf/util/record.h
@@ -67,6 +67,7 @@ struct record_opts {
int affinity;
int mmap_flush;
unsigned int comp_level;
+ bool hazard;
};
extern const char * const *record_usage;
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index d0d7d25b23e3..834ca7df2349 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1153,6 +1153,19 @@ static void stack_user__printf(struct stack_dump *dump)
dump->size, dump->offset);
}
+static void pipeline_hazard__printf(struct perf_sample *sample)
+{
+ struct perf_pipeline_haz_data *haz = sample->pipeline_haz;
+
+ printf("... hazard information:\n");
+ printf(".... Inst Type 0x%" PRIx32 "\n", haz->itype);
+ printf(".... Inst Cache 0x%" PRIx32 "\n", haz->icache);
+ printf(".... Hazard Stage 0x%" PRIx32 "\n", haz->hazard_stage);
+ printf(".... Hazard Reason 0x%" PRIx32 "\n", haz->hazard_reason);
+ printf(".... Stall Stage 0x%" PRIx32 "\n", haz->stall_stage);
+ printf(".... Stall Reason 0x%" PRIx32 "\n", haz->stall_reason);
+}
+
static void perf_evlist__print_tstamp(struct evlist *evlist,
union perf_event *event,
struct perf_sample *sample)
@@ -1251,6 +1264,9 @@ static void dump_sample(struct evsel *evsel, union perf_event *event,
if (sample_type & PERF_SAMPLE_STACK_USER)
stack_user__printf(&sample->user_stack);
+ if (sample_type & PERF_SAMPLE_PIPELINE_HAZ)
+ pipeline_hazard__printf(sample);
+
if (sample_type & PERF_SAMPLE_WEIGHT)
printf("... weight: %" PRIu64 "\n", sample->weight);
--
2.21.1
^ permalink raw reply related
* [RFC 11/11] perf annotate: Show hazard data in tui mode
From: Ravi Bangoria @ 2020-03-02 5:23 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: mark.rutland, ravi.bangoria, ak, maddy, peterz, alexey.budankov,
adrian.hunter, acme, alexander.shishkin, yao.jin, mingo, paulus,
eranian, robert.richter, namhyung, kim.phillips, jolsa, kan.liang
In-Reply-To: <20200302052355.36365-1-ravi.bangoria@linux.ibm.com>
Enable perf report->annotate tui mode to show hazard information. By
default they are hidden, but user can unhide them by pressing hot key
'S'. Sample o/p:
│ Disassembly of section .text:
│
│ 0000000010001cf8 <compare>:
│ compare():
│ return NULL;
│ }
│
│ static int
│ compare(const void *p1, const void *p2)
│ {
33.23 │ std r31,-8(r1)
│ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
│ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
│ {haz_stage: LSU, haz_reason: Load Hit Store, stall_stage: LSU, stall_reason: -, icache: L3 hit}
│ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: -, stall_reason: -, icache: L1 hit}
│ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
│ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
0.84 │ stdu r1,-64(r1)
│ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: -, stall_reason: -, icache: L1 hit}
0.24 │ mr r31,r1
│ {haz_stage: -, haz_reason: -, stall_stage: -, stall_reason: -, icache: L1 hit}
21.18 │ std r3,32(r31)
│ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
│ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
│ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
tools/perf/builtin-annotate.c | 5 ++
tools/perf/ui/browsers/annotate.c | 124 ++++++++++++++++++++++++++----
tools/perf/util/annotate.c | 51 +++++++++++-
tools/perf/util/annotate.h | 18 ++++-
4 files changed, 178 insertions(+), 20 deletions(-)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 78552a9428a6..a51313a6b019 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -472,6 +472,7 @@ static const char * const annotate_usage[] = {
int cmd_annotate(int argc, const char **argv)
{
+ bool annotate_haz = false;
struct perf_annotate annotate = {
.tool = {
.sample = process_sample_event,
@@ -531,6 +532,8 @@ int cmd_annotate(int argc, const char **argv)
symbol__config_symfs),
OPT_BOOLEAN(0, "source", &annotate.opts.annotate_src,
"Interleave source code with assembly code (default)"),
+ OPT_BOOLEAN(0, "hazard", &annotate_haz,
+ "Interleave CPU pileline hazard/stall data with assembly code"),
OPT_BOOLEAN(0, "asm-raw", &annotate.opts.show_asm_raw,
"Display raw encoding of assembly instructions (default)"),
OPT_STRING('M', "disassembler-style", &annotate.opts.disassembler_style, "disassembler style",
@@ -583,6 +586,8 @@ int cmd_annotate(int argc, const char **argv)
if (annotate_check_args(&annotate.opts) < 0)
return -EINVAL;
+ annotate.opts.hide_haz_data = !annotate_haz;
+
if (symbol_conf.show_nr_samples && annotate.use_gtk) {
pr_err("--show-nr-samples is not available in --gtk mode at this time\n");
return ret;
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 2e4db8216b3b..b04d825cee50 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -190,9 +190,15 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser)
return;
}
- if (notes->options->hide_src_code) {
+ if (notes->options->hide_src_code && notes->options->hide_haz_data) {
from = cursor->al.idx_asm;
to = target->idx_asm;
+ } else if (!notes->options->hide_src_code && notes->options->hide_haz_data) {
+ from = cursor->al.idx_asm + cursor->al.idx_src + 1;
+ to = target->idx_asm + target->idx_src + 1;
+ } else if (notes->options->hide_src_code && !notes->options->hide_haz_data) {
+ from = cursor->al.idx_asm + cursor->al.idx_haz + 1;
+ to = target->idx_asm + target->idx_haz + 1;
} else {
from = (u64)cursor->al.idx;
to = (u64)target->idx;
@@ -293,8 +299,13 @@ static void annotate_browser__set_rb_top(struct annotate_browser *browser,
struct annotation_line * pos = rb_entry(nd, struct annotation_line, rb_node);
u32 idx = pos->idx;
- if (notes->options->hide_src_code)
+ if (notes->options->hide_src_code && notes->options->hide_haz_data)
idx = pos->idx_asm;
+ else if (!notes->options->hide_src_code && notes->options->hide_haz_data)
+ idx = pos->idx_asm + pos->idx_src + 1;
+ else if (notes->options->hide_src_code && !notes->options->hide_haz_data)
+ idx = pos->idx_asm + pos->idx_haz + 1;
+
annotate_browser__set_top(browser, pos, idx);
browser->curr_hot = nd;
}
@@ -348,44 +359,117 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
struct annotation *notes = browser__annotation(&browser->b);
struct annotation_line *al;
off_t offset = browser->b.index - browser->b.top_idx;
+ u32 curr_idx;
browser->b.seek(&browser->b, offset, SEEK_CUR);
al = list_entry(browser->b.top, struct annotation_line, node);
if (notes->options->hide_src_code) {
- if (al->idx_asm < offset)
- offset = al->idx;
+ if (notes->options->hide_haz_data) {
+ curr_idx = al->idx_asm + al->idx_src + 1;
+ browser->b.nr_entries = notes->nr_asm_entries +
+ notes->nr_src_entries;
+ } else {
+ curr_idx = al->idx;
+ browser->b.nr_entries = notes->nr_entries;
+ }
- browser->b.nr_entries = notes->nr_entries;
notes->options->hide_src_code = false;
browser->b.seek(&browser->b, -offset, SEEK_CUR);
- browser->b.top_idx = al->idx - offset;
- browser->b.index = al->idx;
+ browser->b.top_idx = curr_idx - offset;
+ browser->b.index = curr_idx;
} else {
- if (al->idx_asm < 0) {
+ if (al->type != AL_TYPE_ASM) {
ui_helpline__puts("Only available for assembly lines.");
browser->b.seek(&browser->b, -offset, SEEK_CUR);
return false;
}
- if (al->idx_asm < offset)
- offset = al->idx_asm;
+ if (notes->options->hide_haz_data) {
+ curr_idx = al->idx_asm;
+ browser->b.nr_entries = notes->nr_asm_entries;
+ } else {
+ curr_idx = al->idx_asm + al->idx_haz + 1;
+ browser->b.nr_entries = notes->nr_asm_entries +
+ notes->nr_haz_entries;
+ }
+
+ if (curr_idx < offset)
+ offset = curr_idx;
- browser->b.nr_entries = notes->nr_asm_entries;
notes->options->hide_src_code = true;
browser->b.seek(&browser->b, -offset, SEEK_CUR);
- browser->b.top_idx = al->idx_asm - offset;
- browser->b.index = al->idx_asm;
+ browser->b.top_idx = curr_idx - offset;
+ browser->b.index = curr_idx;
+ }
+
+ return true;
+}
+
+static bool annotate_browser__toggle_hazard(struct annotate_browser *browser)
+{
+ struct annotation *notes = browser__annotation(&browser->b);
+ struct annotation_line *al;
+ off_t offset = browser->b.index - browser->b.top_idx;
+ u32 curr_idx;
+
+ browser->b.seek(&browser->b, offset, SEEK_CUR);
+ al = list_entry(browser->b.top, struct annotation_line, node);
+
+ if (notes->options->hide_haz_data) {
+ if (notes->options->hide_src_code) {
+ curr_idx = al->idx_asm + al->idx_haz + 1;
+ browser->b.nr_entries = notes->nr_asm_entries +
+ notes->nr_haz_entries;
+ } else {
+ curr_idx = al->idx;
+ browser->b.nr_entries = notes->nr_entries;
+ }
+
+ notes->options->hide_haz_data = false;
+ browser->b.seek(&browser->b, -offset, SEEK_CUR);
+ browser->b.top_idx = curr_idx - offset;
+ browser->b.index = curr_idx;
+ } else {
+ if (al->type != AL_TYPE_ASM) {
+ ui_helpline__puts("Only available for assembly lines.");
+ browser->b.seek(&browser->b, -offset, SEEK_CUR);
+ return false;
+ }
+
+ if (notes->options->hide_src_code) {
+ curr_idx = al->idx_asm;
+ browser->b.nr_entries = notes->nr_asm_entries;
+ } else {
+ curr_idx = al->idx_asm + al->idx_src + 1;
+ browser->b.nr_entries = notes->nr_asm_entries +
+ notes->nr_src_entries;
+ }
+
+ if (curr_idx < offset)
+ offset = curr_idx;
+
+ notes->options->hide_haz_data = true;
+ browser->b.seek(&browser->b, -offset, SEEK_CUR);
+ browser->b.top_idx = curr_idx - offset;
+ browser->b.index = curr_idx;
}
return true;
}
-static void ui_browser__init_asm_mode(struct ui_browser *browser)
+static void ui_browser__hide_src_code(struct ui_browser *browser)
+{
+ struct annotation *notes = browser__annotation(browser);
+ ui_browser__reset_index(browser);
+ browser->nr_entries -= notes->nr_src_entries;
+}
+
+static void ui_browser__hide_haz_data(struct ui_browser *browser)
{
struct annotation *notes = browser__annotation(browser);
ui_browser__reset_index(browser);
- browser->nr_entries = notes->nr_asm_entries;
+ browser->nr_entries -= notes->nr_haz_entries;
}
#define SYM_TITLE_MAX_SIZE (PATH_MAX + 64)
@@ -743,6 +827,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
"o Toggle disassembler output/simplified view\n"
"O Bump offset level (jump targets -> +call -> all -> cycle thru)\n"
"s Toggle source code view\n"
+ "S Toggle pipeline hazard/stall view\n"
"t Circulate percent, total period, samples view\n"
"c Show min/max cycle\n"
"/ Search string\n"
@@ -767,6 +852,10 @@ static int annotate_browser__run(struct annotate_browser *browser,
if (annotate_browser__toggle_source(browser))
ui_helpline__puts(help);
continue;
+ case 'S':
+ if (annotate_browser__toggle_hazard(browser))
+ ui_helpline__puts(help);
+ continue;
case 'o':
notes->options->use_offset = !notes->options->use_offset;
annotation__update_column_widths(notes);
@@ -932,7 +1021,10 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
browser.b.width += 18; /* Percentage */
if (notes->options->hide_src_code)
- ui_browser__init_asm_mode(&browser.b);
+ ui_browser__hide_src_code(&browser.b);
+
+ if (notes->options->hide_haz_data)
+ ui_browser__hide_haz_data(&browser.b);
ret = annotate_browser__run(&browser, evsel, hbt);
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 766934b0f36d..d782ee193345 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -66,6 +66,7 @@ struct annotation_options annotation__default_options = {
.annotate_src = true,
.offset_level = ANNOTATION__OFFSET_JUMP_TARGETS,
.percent_type = PERCENT_PERIOD_LOCAL,
+ .hide_haz_data = true,
};
static regex_t file_lineno;
@@ -1268,7 +1269,7 @@ static struct disasm_line *disasm_line__new(struct annotate_args *args)
struct disasm_line *dl = NULL;
int nr = 1;
- if (perf_evsel__is_group_event(args->evsel))
+ if (args->type != AL_TYPE_HAZ && perf_evsel__is_group_event(args->evsel))
nr = args->evsel->core.nr_members;
dl = zalloc(disasm_line_size(nr));
@@ -1509,6 +1510,7 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
} else if (max_lines && printed >= max_lines)
return 1;
else {
+ /* TODO: Hazard specific changes */
int width = symbol_conf.show_total_period ? 12 : 8;
if (queue)
@@ -1526,6 +1528,34 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
return 0;
}
+static int annotation_add_hazard_detail(struct annotation *notes, s64 offset)
+{
+ int ret;
+ struct hazard_hist *hh;
+ struct disasm_line *dl;
+ struct annotate_args args = {
+ .type = AL_TYPE_HAZ,
+ .offset = -1,
+ };
+
+ list_for_each_entry(hh, ¬es->src->haz_hist[offset].list, list) {
+ ret = asprintf(&(args.line), " {haz_stage: %s, "
+ "haz_reason: %s, stall_stage: %s, stall_reason: %s, "
+ "icache: %s}", hh->haz_stage, hh->haz_reason,
+ hh->stall_stage, hh->stall_reason, hh->icache);
+ if (ret == -1)
+ return -ENOMEM;
+
+ dl = disasm_line__new(&args);
+ free(args.line);
+ if (!dl)
+ return -ENOMEM;
+
+ annotation_line__add(&dl->al, ¬es->src->source);
+ }
+ return 0;
+}
+
/*
* symbol__parse_objdump_line() parses objdump output (with -d --no-show-raw)
* which looks like following
@@ -1608,6 +1638,10 @@ static int symbol__parse_objdump_line(struct symbol *sym,
annotation_line__add(&dl->al, ¬es->src->source);
+ if (offset != -1 && notes->src->haz_hist &&
+ !list_empty(¬es->src->haz_hist[offset].list))
+ return annotation_add_hazard_detail(notes, offset);
+
return 0;
}
@@ -2701,6 +2735,8 @@ static void annotation__set_offsets(struct annotation *notes, s64 size)
notes->max_line_len = 0;
notes->nr_entries = 0;
notes->nr_asm_entries = 0;
+ notes->nr_src_entries = 0;
+ notes->nr_haz_entries = 0;
list_for_each_entry(al, ¬es->src->source, node) {
size_t line_len = strlen(al->line);
@@ -2710,6 +2746,8 @@ static void annotation__set_offsets(struct annotation *notes, s64 size)
al->idx = notes->nr_entries++;
if (al->type == AL_TYPE_ASM) {
al->idx_asm = notes->nr_asm_entries++;
+ al->idx_src = notes->nr_src_entries - 1;
+ al->idx_haz = notes->nr_haz_entries - 1;
/*
* FIXME: short term bandaid to cope with assembly
* routines that comes with labels in the same column
@@ -2719,8 +2757,15 @@ static void annotation__set_offsets(struct annotation *notes, s64 size)
*/
if (al->offset < size)
notes->offsets[al->offset] = al;
- } else
+ } else if (al->type == AL_TYPE_SRC) {
al->idx_asm = -1;
+ al->idx_src = notes->nr_src_entries++;
+ al->idx_haz = notes->nr_haz_entries - 1;
+ } else if (al->type == AL_TYPE_HAZ) {
+ al->idx_asm = -1;
+ al->idx_haz = notes->nr_haz_entries++;
+ al->idx_src = notes->nr_src_entries - 1;
+ }
}
}
@@ -3051,6 +3096,8 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
printed = scnprintf(bf, sizeof(bf), "%-*s ", notes->widths.addr, " ");
obj__printf(obj, bf);
obj__printf(obj, "%-*s", width - printed - pcnt_width - cycles_width + 1, al->line);
+ } else if (al->type == AL_TYPE_HAZ) {
+ obj__printf(obj, "%-*s", width - pcnt_width - cycles_width, al->line);
} else {
u64 addr = al->offset;
int color = -1;
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index a3803f89b8fc..3a082c13bdad 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -85,7 +85,8 @@ struct annotation_options {
show_nr_jumps,
show_minmax_cycle,
show_asm_raw,
- annotate_src;
+ annotate_src,
+ hide_haz_data;
u8 offset_level;
int min_pcnt;
int max_lines;
@@ -131,8 +132,16 @@ struct annotation_data {
enum annotation_line_type {
AL_TYPE_ASM = 0,
AL_TYPE_SRC,
+ AL_TYPE_HAZ,
};
+/*
+ * @idx_asm, @idx_src and @idx_haz starts from -1 and increments in
+ * chronological order. For _all_ non-assembly lines, @idx_asm is -1.
+ * OTOH, for non-source line, @idx_src contains prev value. Similarly
+ * for non-hazard lines @idx_haz also contains prev value.
+ * And @idx = @idx_asm + (@idx_src + 1) + (@idx_haz + 1);
+ */
struct annotation_line {
struct list_head node;
struct rb_node rb_node;
@@ -148,6 +157,8 @@ struct annotation_line {
char *path;
u32 idx;
int idx_asm;
+ int idx_src;
+ int idx_haz;
int data_nr;
struct annotation_data data[0];
};
@@ -298,6 +309,8 @@ struct annotation {
int max_jump_sources;
int nr_entries;
int nr_asm_entries;
+ int nr_src_entries;
+ int nr_haz_entries;
u16 max_line_len;
struct {
u8 addr;
@@ -326,7 +339,8 @@ static inline int annotation__pcnt_width(struct annotation *notes)
static inline bool annotation_line__filter(struct annotation_line *al, struct annotation *notes)
{
- return notes->options->hide_src_code && al->type == AL_TYPE_SRC;
+ return ((notes->options->hide_src_code && al->type == AL_TYPE_SRC) ||
+ (notes->options->hide_haz_data && al->type == AL_TYPE_HAZ));
}
void annotation__update_column_widths(struct annotation *notes);
--
2.21.1
^ permalink raw reply related
* [RFC 09/11] perf annotate: Introduce type for annotation_line
From: Ravi Bangoria @ 2020-03-02 5:23 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: mark.rutland, ravi.bangoria, ak, maddy, peterz, alexey.budankov,
adrian.hunter, acme, alexander.shishkin, yao.jin, mingo, paulus,
eranian, robert.richter, namhyung, kim.phillips, jolsa, kan.liang
In-Reply-To: <20200302052355.36365-1-ravi.bangoria@linux.ibm.com>
struct annotation_line can contain either assembly instruction or
a source code line. To distinguish between them we currently use
offset. If offset is -1, it's a source otherwise it's assembly.
This is bit cryptic when you first read the code. Introduce new
field 'type' that denotes type of the data the annotation_line
object contains.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
tools/perf/ui/browsers/annotate.c | 4 ++--
tools/perf/ui/gtk/annotate.c | 6 +++---
tools/perf/util/annotate.c | 27 ++++++++++++++++-----------
tools/perf/util/annotate.h | 8 +++++++-
4 files changed, 28 insertions(+), 17 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 9023267e5643..2e4db8216b3b 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -317,7 +317,7 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
double max_percent = 0.0;
int i;
- if (pos->al.offset == -1) {
+ if (pos->al.type != AL_TYPE_ASM) {
RB_CLEAR_NODE(&pos->al.rb_node);
continue;
}
@@ -816,7 +816,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
if (browser->selection == NULL)
ui_helpline__puts("Huh? No selection. Report to linux-kernel@vger.kernel.org");
- else if (browser->selection->offset == -1)
+ else if (browser->selection->type != AL_TYPE_ASM)
ui_helpline__puts("Actions are only available for assembly lines.");
else if (!dl->ins.ops)
goto show_sup_ins;
diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
index 35f9641bf670..71c792a0b17d 100644
--- a/tools/perf/ui/gtk/annotate.c
+++ b/tools/perf/ui/gtk/annotate.c
@@ -35,7 +35,7 @@ static int perf_gtk__get_percent(char *buf, size_t size, struct symbol *sym,
strcpy(buf, "");
- if (dl->al.offset == (s64) -1)
+ if (dl->al.type != AL_TYPE_ASM)
return 0;
symhist = annotation__histogram(symbol__annotation(sym), evidx);
@@ -61,7 +61,7 @@ static int perf_gtk__get_offset(char *buf, size_t size, struct map_symbol *ms,
strcpy(buf, "");
- if (dl->al.offset == (s64) -1)
+ if (dl->al.type != AL_TYPE_ASM)
return 0;
return scnprintf(buf, size, "%"PRIx64, start + dl->al.offset);
@@ -78,7 +78,7 @@ static int perf_gtk__get_line(char *buf, size_t size, struct disasm_line *dl)
if (!line)
return 0;
- if (dl->al.offset != (s64) -1)
+ if (dl->al.type == AL_TYPE_ASM)
markup = NULL;
if (markup)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 4e2706274d85..8aef60a6ffea 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1148,6 +1148,7 @@ struct annotate_args {
struct evsel *evsel;
struct annotation_options *options;
s64 offset;
+ u8 type;
char *line;
int line_nr;
};
@@ -1156,6 +1157,7 @@ static void annotation_line__init(struct annotation_line *al,
struct annotate_args *args,
int nr)
{
+ al->type = args->type;
al->offset = args->offset;
al->line = strdup(args->line);
al->line_nr = args->line_nr;
@@ -1202,7 +1204,7 @@ static struct disasm_line *disasm_line__new(struct annotate_args *args)
if (dl->al.line == NULL)
goto out_delete;
- if (args->offset != -1) {
+ if (dl->al.type == AL_TYPE_ASM) {
if (disasm_line__parse(dl->al.line, &dl->ins.name, &dl->ops.raw) < 0)
goto out_free_line;
@@ -1246,7 +1248,7 @@ struct annotation_line *
annotation_line__next(struct annotation_line *pos, struct list_head *head)
{
list_for_each_entry_continue(pos, head, node)
- if (pos->offset >= 0)
+ if (pos->type == AL_TYPE_ASM)
return pos;
return NULL;
@@ -1357,7 +1359,7 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
static const char *prev_line;
static const char *prev_color;
- if (al->offset != -1) {
+ if (al->type == AL_TYPE_ASM) {
double max_percent = 0.0;
int i, nr_percent = 1;
const char *color;
@@ -1500,6 +1502,7 @@ static int symbol__parse_objdump_line(struct symbol *sym,
}
args->offset = offset;
+ args->type = (offset != -1) ? AL_TYPE_ASM : AL_TYPE_SRC;
args->line = parsed_line;
args->line_nr = *line_nr;
args->ms.sym = sym;
@@ -1783,6 +1786,7 @@ static int symbol__disassemble_bpf(struct symbol *sym,
fflush(s);
if (!opts->hide_src_code && srcline) {
+ args->type = AL_TYPE_SRC;
args->offset = -1;
args->line = strdup(srcline);
args->line_nr = 0;
@@ -1794,6 +1798,7 @@ static int symbol__disassemble_bpf(struct symbol *sym,
}
}
+ args->type = AL_TYPE_ASM;
args->offset = pc;
args->line = buf + prev_buf_size;
args->line_nr = 0;
@@ -2099,7 +2104,7 @@ static void annotation__calc_percent(struct annotation *notes,
s64 end;
int i = 0;
- if (al->offset == -1)
+ if (al->type != AL_TYPE_ASM)
continue;
next = annotation_line__next(al, ¬es->src->source);
@@ -2309,7 +2314,7 @@ static int annotated_source__addr_fmt_width(struct list_head *lines, u64 start)
struct annotation_line *line;
list_for_each_entry_reverse(line, lines, node) {
- if (line->offset != -1)
+ if (line->type == AL_TYPE_ASM)
return scnprintf(bf, sizeof(bf), "%" PRIx64, start + line->offset);
}
@@ -2549,7 +2554,7 @@ static size_t disasm_line__fprintf(struct disasm_line *dl, FILE *fp)
{
size_t printed;
- if (dl->al.offset == -1)
+ if (dl->al.type == AL_TYPE_SRC)
return fprintf(fp, "%s\n", dl->al.line);
printed = fprintf(fp, "%#" PRIx64 " %s", dl->al.offset, dl->ins.name);
@@ -2628,7 +2633,7 @@ static void annotation__set_offsets(struct annotation *notes, s64 size)
if (notes->max_line_len < line_len)
notes->max_line_len = line_len;
al->idx = notes->nr_entries++;
- if (al->offset != -1) {
+ if (al->type == AL_TYPE_ASM) {
al->idx_asm = notes->nr_asm_entries++;
/*
* FIXME: short term bandaid to cope with assembly
@@ -2659,7 +2664,7 @@ static int annotation__max_ins_name(struct annotation *notes)
struct annotation_line *al;
list_for_each_entry(al, ¬es->src->source, node) {
- if (al->offset == -1)
+ if (al->type != AL_TYPE_ASM)
continue;
len = strlen(disasm_line(al)->ins.name);
@@ -2875,7 +2880,7 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
char bf[256];
int printed;
- if (first_line && (al->offset == -1 || percent_max == 0.0)) {
+ if (first_line && (al->type != AL_TYPE_ASM || percent_max == 0.0)) {
if (notes->have_cycles) {
if (al->ipc == 0.0 && al->cycles == 0)
show_title = true;
@@ -2883,7 +2888,7 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
show_title = true;
}
- if (al->offset != -1 && percent_max != 0.0) {
+ if (al->type == AL_TYPE_ASM && percent_max != 0.0) {
int i;
for (i = 0; i < notes->nr_events; i++) {
@@ -2964,7 +2969,7 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
if (!*al->line)
obj__printf(obj, "%-*s", width - pcnt_width - cycles_width, " ");
- else if (al->offset == -1) {
+ else if (al->type == AL_TYPE_SRC) {
if (al->line_nr && notes->options->show_linenr)
printed = scnprintf(bf, sizeof(bf), "%-*d ", notes->widths.addr + 1, al->line_nr);
else
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index d9b5bb105056..89839713d242 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -128,9 +128,15 @@ struct annotation_data {
struct sym_hist_entry he;
};
+enum annotation_line_type {
+ AL_TYPE_ASM = 0,
+ AL_TYPE_SRC,
+};
+
struct annotation_line {
struct list_head node;
struct rb_node rb_node;
+ u8 type;
s64 offset;
char *line;
int line_nr;
@@ -310,7 +316,7 @@ static inline int annotation__pcnt_width(struct annotation *notes)
static inline bool annotation_line__filter(struct annotation_line *al, struct annotation *notes)
{
- return notes->options->hide_src_code && al->offset == -1;
+ return notes->options->hide_src_code && al->type == AL_TYPE_SRC;
}
void annotation__update_column_widths(struct annotation *notes);
--
2.21.1
^ permalink raw reply related
* [RFC 10/11] perf annotate: Preparation for hazard
From: Ravi Bangoria @ 2020-03-02 5:23 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: mark.rutland, ravi.bangoria, ak, maddy, peterz, alexey.budankov,
adrian.hunter, acme, alexander.shishkin, yao.jin, mingo, paulus,
eranian, robert.richter, namhyung, kim.phillips, jolsa, kan.liang
In-Reply-To: <20200302052355.36365-1-ravi.bangoria@linux.ibm.com>
Introduce 'struct hazard_hist' that will contain hazard specific
information for annotate. Add Array of list of 'struct hazard_hist'
into 'struct annotated_source' where array length = symbol size and
each member of list contain hazard info from associated perf sample.
This information is prepared while parsing samples in perf report.
Also, this is just a preparation step for annotate and followup
patch does actual annotate ui changes.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
tools/perf/builtin-report.c | 1 +
tools/perf/util/annotate.c | 75 +++++++++++++++++++++++++++++++++++++
tools/perf/util/annotate.h | 14 +++++++
tools/perf/util/hist.c | 13 +++++++
tools/perf/util/hist.h | 4 ++
tools/perf/util/machine.c | 6 +++
tools/perf/util/machine.h | 3 ++
7 files changed, 116 insertions(+)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index a47542a12da1..ff950ff8dd51 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -301,6 +301,7 @@ static int process_sample_event(struct perf_tool *tool,
hist__account_cycles(sample->branch_stack, &al, sample,
rep->nonany_branch_mode,
&rep->total_cycles);
+ hist__capture_haz_info(&al, sample, evsel);
}
ret = hist_entry_iter__add(&iter, &al, rep->max_stack, rep);
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 8aef60a6ffea..766934b0f36d 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -36,6 +36,7 @@
#include "string2.h"
#include "util/event.h"
#include "arch/common.h"
+#include "hazard.h"
#include <regex.h>
#include <pthread.h>
#include <linux/bitops.h>
@@ -800,6 +801,21 @@ static int symbol__alloc_hist_cycles(struct symbol *sym)
return 0;
}
+static int symbol__alloc_hist_hazard(struct symbol *sym)
+{
+ struct annotation *notes = symbol__annotation(sym);
+ const size_t size = symbol__size(sym);
+ size_t i;
+
+ notes->src->haz_hist = calloc(size, sizeof(struct hazard_hist));
+ if (notes->src->haz_hist == NULL)
+ return -1;
+
+ for (i = 0; i < size; i++)
+ INIT_LIST_HEAD(¬es->src->haz_hist[i].list);
+ return 0;
+}
+
void symbol__annotate_zero_histograms(struct symbol *sym)
{
struct annotation *notes = symbol__annotation(sym);
@@ -920,6 +936,25 @@ static struct cyc_hist *symbol__cycles_hist(struct symbol *sym)
return notes->src->cycles_hist;
}
+static struct hazard_hist *symbol__hazard_hist(struct symbol *sym)
+{
+ struct annotation *notes = symbol__annotation(sym);
+
+ if (notes->src == NULL) {
+ notes->src = annotated_source__new();
+ if (notes->src == NULL)
+ return NULL;
+ goto alloc_haz_hist;
+ }
+
+ if (!notes->src->haz_hist) {
+alloc_haz_hist:
+ symbol__alloc_hist_hazard(sym);
+ }
+
+ return notes->src->haz_hist;
+}
+
struct annotated_source *symbol__hists(struct symbol *sym, int nr_hists)
{
struct annotation *notes = symbol__annotation(sym);
@@ -1014,6 +1049,46 @@ int addr_map_symbol__account_cycles(struct addr_map_symbol *ams,
return err;
}
+int symbol__capture_haz_info(struct addr_map_symbol *ams,
+ struct perf_sample *sample,
+ struct evsel *evsel)
+{
+ struct hazard_hist *hh, *tmp;
+ u64 offset;
+ const char *arch = perf_env__arch(perf_evsel__env(evsel));
+
+ if (ams->ms.sym == NULL)
+ return 0;
+
+ hh = symbol__hazard_hist(ams->ms.sym);
+ if (!hh)
+ return -ENOMEM;
+
+ if (ams->al_addr < ams->ms.sym->start || ams->al_addr >= ams->ms.sym->end)
+ return -ERANGE;
+
+ offset = ams->al_addr - ams->ms.sym->start;
+
+ tmp = zalloc(sizeof(*tmp));
+ if (!tmp)
+ return -ENOMEM;
+
+ tmp->icache = perf_haz__icache_str(sample->pipeline_haz->icache, arch);
+ tmp->haz_stage = perf_haz__hstage_str(sample->pipeline_haz->hazard_stage,
+ arch);
+ tmp->haz_reason = perf_haz__hreason_str(sample->pipeline_haz->hazard_stage,
+ sample->pipeline_haz->hazard_reason,
+ arch);
+ tmp->stall_stage = perf_haz__sstage_str(sample->pipeline_haz->stall_stage,
+ arch);
+ tmp->stall_reason = perf_haz__sreason_str(sample->pipeline_haz->stall_stage,
+ sample->pipeline_haz->stall_reason,
+ arch);
+
+ list_add(&tmp->list, &hh[offset].list);
+ return 0;
+}
+
static unsigned annotation__count_insn(struct annotation *notes, u64 start, u64 end)
{
unsigned n_insn = 0;
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 89839713d242..a3803f89b8fc 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -249,6 +249,15 @@ struct cyc_hist {
u16 reset;
};
+struct hazard_hist {
+ struct list_head list;
+ const char *icache;
+ const char *haz_stage;
+ const char *haz_reason;
+ const char *stall_stage;
+ const char *stall_reason;
+};
+
/** struct annotated_source - symbols with hits have this attached as in sannotation
*
* @histograms: Array of addr hit histograms per event being monitored
@@ -271,6 +280,7 @@ struct annotated_source {
int nr_histograms;
size_t sizeof_sym_hist;
struct cyc_hist *cycles_hist;
+ struct hazard_hist *haz_hist; /* Array of list. Array length = symbol size */
struct sym_hist *histograms;
};
@@ -343,6 +353,10 @@ int addr_map_symbol__account_cycles(struct addr_map_symbol *ams,
struct addr_map_symbol *start,
unsigned cycles);
+int symbol__capture_haz_info(struct addr_map_symbol *ams,
+ struct perf_sample *sample,
+ struct evsel *evsel);
+
int hist_entry__inc_addr_samples(struct hist_entry *he, struct perf_sample *sample,
struct evsel *evsel, u64 addr);
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index d690d08b0f64..3cbfebb80f68 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -2702,6 +2702,19 @@ void hist__account_cycles(struct branch_stack *bs, struct addr_location *al,
}
}
+void hist__capture_haz_info(struct addr_location *al,
+ struct perf_sample *sample,
+ struct evsel *evsel)
+{
+ struct addr_map_symbol ams;
+
+ if (!sample->pipeline_haz)
+ return;
+
+ sample__resolve_haz(al, &ams, sample);
+ symbol__capture_haz_info(&ams, sample, evsel);
+}
+
size_t perf_evlist__fprintf_nr_events(struct evlist *evlist, FILE *fp)
{
struct evsel *pos;
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 063d65985a11..086c1dfde21f 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -554,6 +554,10 @@ void hist__account_cycles(struct branch_stack *bs, struct addr_location *al,
struct perf_sample *sample, bool nonany_branch_mode,
u64 *total_cycles);
+void hist__capture_haz_info(struct addr_location *al,
+ struct perf_sample *sample,
+ struct evsel *evsel);
+
struct option;
int parse_filter_percentage(const struct option *opt, const char *arg, int unset);
int perf_hist_config(const char *var, const char *value);
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index fb5c2cd44d30..e575488a1390 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2094,6 +2094,12 @@ struct branch_info *sample__resolve_bstack(struct perf_sample *sample,
return bi;
}
+void sample__resolve_haz(struct addr_location *al, struct addr_map_symbol *ams,
+ struct perf_sample *sample)
+{
+ ip__resolve_ams(al->thread, ams, sample->ip);
+}
+
static void save_iterations(struct iterations *iter,
struct branch_entry *be, int nr)
{
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index be0a930eca89..e9a298b5430c 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -173,6 +173,9 @@ struct branch_info *sample__resolve_bstack(struct perf_sample *sample,
struct mem_info *sample__resolve_mem(struct perf_sample *sample,
struct addr_location *al);
+void sample__resolve_haz(struct addr_location *al, struct addr_map_symbol *ams,
+ struct perf_sample *sample);
+
struct callchain_cursor;
int thread__resolve_callchain(struct thread *thread,
--
2.21.1
^ permalink raw reply related
* [RFC 08/11] perf report: Enable hazard mode
From: Ravi Bangoria @ 2020-03-02 5:23 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: mark.rutland, ravi.bangoria, ak, maddy, peterz, alexey.budankov,
Madhavan Srinivasan, adrian.hunter, acme, alexander.shishkin,
yao.jin, mingo, paulus, eranian, robert.richter, namhyung,
kim.phillips, jolsa, kan.liang
In-Reply-To: <20200302052355.36365-1-ravi.bangoria@linux.ibm.com>
From: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Introduce --hazard with perf report to show perf report with hazard
data. Hazard mode columns are Instruction Type, Hazard Stage, Hazard
Reason, Stall Stage, Stall Reason and Icache access. Default sort
order is sym, dso, inst type, hazard stage, hazard reason, stall
stage, stall reason, inst cache.
Sample o/p on IBM PowerPC machine:
Overhead Symbol Shared Instruction Type Hazard Stage Hazard Reason Stall Stage Stall Reason ICache access
36.58% [.] thread_run ebizzy Load LSU Mispredict LSU Load fin L1 hit
9.46% [.] thread_run ebizzy Load LSU Mispredict LSU Dcache_miss L1 hit
1.76% [.] thread_run ebizzy Fixed point - - - - L1 hit
1.31% [.] thread_run ebizzy Load LSU ERAT Miss LSU Load fin L1 hit
1.27% [.] thread_run ebizzy Load LSU Mispredict - - L1 hit
1.16% [.] thread_run ebizzy Fixed point - - FXU Fixed cycle L1 hit
0.50% [.] thread_run ebizzy Fixed point ISU Source Unavailable FXU Fixed cycle L1 hit
0.30% [.] thread_run ebizzy Load LSU LMQ Full, DERAT Miss LSU Load fin L1 hit
0.24% [.] thread_run ebizzy Load LSU ERAT Miss - - L1 hit
0.08% [.] thread_run ebizzy - - - BRU Fixed cycle L1 hit
0.05% [.] thread_run ebizzy Branch - - BRU Fixed cycle L1 hit
0.04% [.] thread_run ebizzy Fixed point ISU Source Unavailable - - L1 hit
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
tools/perf/builtin-report.c | 28 +++++
tools/perf/util/hist.c | 77 ++++++++++++
tools/perf/util/hist.h | 7 ++
tools/perf/util/sort.c | 230 ++++++++++++++++++++++++++++++++++++
tools/perf/util/sort.h | 22 ++++
5 files changed, 364 insertions(+)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 72a12b69f120..a47542a12da1 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -77,6 +77,7 @@ struct report {
bool show_threads;
bool inverted_callchain;
bool mem_mode;
+ bool hazard;
bool stats_mode;
bool tasks_mode;
bool mmaps_mode;
@@ -285,6 +286,8 @@ static int process_sample_event(struct perf_tool *tool,
iter.ops = &hist_iter_branch;
} else if (rep->mem_mode) {
iter.ops = &hist_iter_mem;
+ } else if (rep->hazard) {
+ iter.ops = &hist_iter_haz;
} else if (symbol_conf.cumulate_callchain) {
iter.ops = &hist_iter_cumulative;
} else {
@@ -396,6 +399,14 @@ static int report__setup_sample_type(struct report *rep)
}
}
+ if (sort__mode == SORT_MODE__HAZARD) {
+ if (!is_pipe && !(sample_type & PERF_SAMPLE_PIPELINE_HAZ)) {
+ ui__error("Selected --hazard but no hazard data. "
+ "Did you call perf record without --hazard?\n");
+ return -1;
+ }
+ }
+
if (symbol_conf.use_callchain || symbol_conf.cumulate_callchain) {
if ((sample_type & PERF_SAMPLE_REGS_USER) &&
(sample_type & PERF_SAMPLE_STACK_USER)) {
@@ -484,6 +495,9 @@ static size_t hists__fprintf_nr_sample_events(struct hists *hists, struct report
if (rep->mem_mode) {
ret += fprintf(fp, "\n# Total weight : %" PRIu64, nr_events);
ret += fprintf(fp, "\n# Sort order : %s", sort_order ? : default_mem_sort_order);
+ } else if (rep->hazard) {
+ ret += fprintf(fp, "\n# Event count (approx.): %" PRIu64, nr_events);
+ ret += fprintf(fp, "\n# Sort order: %s", sort_order ? : default_haz_sort_order);
} else
ret += fprintf(fp, "\n# Event count (approx.): %" PRIu64, nr_events);
@@ -1228,6 +1242,7 @@ int cmd_report(int argc, const char **argv)
OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
"Enable kernel symbol demangling"),
OPT_BOOLEAN(0, "mem-mode", &report.mem_mode, "mem access profile"),
+ OPT_BOOLEAN(0, "hazard", &report.hazard, "Processor pipeline hazard and stalls"),
OPT_INTEGER(0, "samples", &symbol_conf.res_sample,
"Number of samples to save per histogram entry for individual browsing"),
OPT_CALLBACK(0, "percent-limit", &report, "percent",
@@ -1394,6 +1409,19 @@ int cmd_report(int argc, const char **argv)
symbol_conf.cumulate_callchain = false;
}
+ if (report.hazard) {
+ if (sort__mode == SORT_MODE__BRANCH) {
+ pr_err("branch and hazard incompatible\n");
+ goto error;
+ }
+ if (sort__mode == SORT_MODE__MEMORY) {
+ pr_err("mem-mode and hazard incompatible\n");
+ goto error;
+ }
+ sort__mode = SORT_MODE__HAZARD;
+ symbol_conf.cumulate_callchain = false;
+ }
+
if (symbol_conf.report_hierarchy) {
/* disable incompatible options */
symbol_conf.cumulate_callchain = false;
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 6d23efaa52c8..d690d08b0f64 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -203,6 +203,12 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
hists__new_col_len(hists, HISTC_MEM_LVL, 21 + 3);
hists__new_col_len(hists, HISTC_LOCAL_WEIGHT, 12);
hists__new_col_len(hists, HISTC_GLOBAL_WEIGHT, 12);
+ hists__new_col_len(hists, HISTC_HAZ_ITYPE, 31);
+ hists__new_col_len(hists, HISTC_HAZ_ICACHE, 14);
+ hists__new_col_len(hists, HISTC_HAZ_HSTAGE, 13);
+ hists__new_col_len(hists, HISTC_HAZ_HREASON, 27);
+ hists__new_col_len(hists, HISTC_HAZ_SSTAGE, 12);
+ hists__new_col_len(hists, HISTC_HAZ_SREASON, 22);
if (symbol_conf.nanosecs)
hists__new_col_len(hists, HISTC_TIME, 16);
else
@@ -864,6 +870,69 @@ iter_finish_mem_entry(struct hist_entry_iter *iter,
return err;
}
+static int iter_prepare_haz_entry(struct hist_entry_iter *iter,
+ struct addr_location *al __maybe_unused)
+{
+ struct perf_sample *sample = iter->sample;
+ struct perf_pipeline_haz_data *haz_data;
+
+ if (!sample->pipeline_haz) {
+ iter->priv = NULL;
+ return 0;
+ }
+
+ haz_data = zalloc(sizeof(*haz_data));
+ if (!haz_data)
+ return -ENOMEM;
+
+ memcpy(haz_data, sample->pipeline_haz, sizeof(*haz_data));
+ iter->priv = haz_data;
+ return 0;
+}
+
+static int iter_add_single_haz_entry(struct hist_entry_iter *iter,
+ struct addr_location *al __maybe_unused)
+{
+ struct perf_pipeline_haz_data *haz_data = iter->priv;
+ struct hists *hists = evsel__hists(iter->evsel);
+ struct perf_sample *sample = iter->sample;
+ struct hist_entry *he;
+
+ he = hists__add_entry(hists, al, iter->parent, NULL, NULL, haz_data,
+ sample, true);
+ if (!he)
+ return -ENOMEM;
+
+ iter->he = he;
+ return 0;
+}
+
+static int iter_finish_haz_entry(struct hist_entry_iter *iter,
+ struct addr_location *al __maybe_unused)
+{
+ struct evsel *evsel = iter->evsel;
+ struct hists *hists = evsel__hists(evsel);
+ struct hist_entry *he = iter->he;
+ int err = -EINVAL;
+
+ if (he == NULL)
+ goto out;
+
+ hists__inc_nr_samples(hists, he->filtered);
+
+ err = hist_entry__append_callchain(he, iter->sample);
+
+out:
+ /*
+ * We don't need to free iter->priv (perf_pipeline_haz_data) here since
+ * the hazard info was either already freed in hists__findnew_entry() or
+ * passed to a new hist entry by hist_entry__new().
+ */
+ iter->priv = NULL;
+ iter->he = NULL;
+ return err;
+}
+
static int
iter_prepare_branch_entry(struct hist_entry_iter *iter, struct addr_location *al)
{
@@ -1128,6 +1197,14 @@ iter_finish_cumulative_entry(struct hist_entry_iter *iter,
return 0;
}
+const struct hist_iter_ops hist_iter_haz = {
+ .prepare_entry = iter_prepare_haz_entry,
+ .add_single_entry = iter_add_single_haz_entry,
+ .next_entry = iter_next_nop_entry,
+ .add_next_entry = iter_add_next_nop_entry,
+ .finish_entry = iter_finish_haz_entry,
+};
+
const struct hist_iter_ops hist_iter_mem = {
.prepare_entry = iter_prepare_mem_entry,
.add_single_entry = iter_add_single_mem_entry,
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index a4d12a503126..063d65985a11 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -69,6 +69,12 @@ enum hist_column {
HISTC_SYM_SIZE,
HISTC_DSO_SIZE,
HISTC_SYMBOL_IPC,
+ HISTC_HAZ_ITYPE,
+ HISTC_HAZ_ICACHE,
+ HISTC_HAZ_HSTAGE,
+ HISTC_HAZ_HREASON,
+ HISTC_HAZ_SSTAGE,
+ HISTC_HAZ_SREASON,
HISTC_NR_COLS, /* Last entry */
};
@@ -132,6 +138,7 @@ struct hist_entry_iter {
extern const struct hist_iter_ops hist_iter_normal;
extern const struct hist_iter_ops hist_iter_branch;
extern const struct hist_iter_ops hist_iter_mem;
+extern const struct hist_iter_ops hist_iter_haz;
extern const struct hist_iter_ops hist_iter_cumulative;
struct hist_entry *hists__add_entry(struct hists *hists,
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index ab0cfd790ad0..b6d47e7160af 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -23,6 +23,7 @@
#include "strbuf.h"
#include <traceevent/event-parse.h>
#include "mem-events.h"
+#include "hazard.h"
#include "annotate.h"
#include "time-utils.h"
#include <linux/kernel.h>
@@ -34,6 +35,7 @@ const char *parent_pattern = default_parent_pattern;
const char *default_sort_order = "comm,dso,symbol";
const char default_branch_sort_order[] = "comm,dso_from,symbol_from,symbol_to,cycles";
const char default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked";
+const char default_haz_sort_order[] = "sym,dso,itype,hstage,hreason,sstage,sreason,icache";
const char default_top_sort_order[] = "dso,symbol";
const char default_diff_sort_order[] = "dso,symbol";
const char default_tracepoint_sort_order[] = "trace";
@@ -1377,6 +1379,190 @@ struct sort_entry sort_mem_dcacheline = {
.se_width_idx = HISTC_MEM_DCACHELINE,
};
+static int64_t sort__itype_cmp(struct hist_entry *l, struct hist_entry *r)
+{
+ u8 itype_l, itype_r;
+
+ itype_l = l->haz_data ? l->haz_data->itype : PERF_HAZ__ITYPE_NA;
+ itype_r = r->haz_data ? r->haz_data->itype : PERF_HAZ__ITYPE_NA;
+
+ return (int64_t)(itype_r - itype_l);
+}
+
+static int hist_entry__itype_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width)
+{
+ const char *arch;
+ const char *itype = "-";
+
+ arch = hist_entry__get_arch(he);
+ if (he->haz_data)
+ itype = perf_haz__itype_str(he->haz_data->itype, arch);
+
+ return repsep_snprintf(bf, size, "%-*s", width, itype);
+}
+
+struct sort_entry sort_haz_inst_type = {
+ .se_header = "Instruction Type",
+ .se_cmp = sort__itype_cmp,
+ .se_snprintf = hist_entry__itype_snprintf,
+ .se_width_idx = HISTC_HAZ_ITYPE,
+};
+
+static int64_t sort__icache_cmp(struct hist_entry *l, struct hist_entry *r)
+{
+ u8 icache_l, icache_r;
+
+ icache_l = l->haz_data ? l->haz_data->icache : PERF_HAZ__ICACHE_NA;
+ icache_r = r->haz_data ? r->haz_data->icache : PERF_HAZ__ICACHE_NA;
+
+ return (int64_t)(icache_r - icache_l);
+}
+
+static int hist_entry__icache_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width)
+{
+ const char *arch;
+ const char *icache = "-";
+
+ arch = hist_entry__get_arch(he);
+ if (he->haz_data)
+ icache = perf_haz__icache_str(he->haz_data->icache, arch);
+
+ return repsep_snprintf(bf, size, "%-*s", width, icache);
+}
+
+struct sort_entry sort_haz_inst_cache = {
+ .se_header = "ICache access",
+ .se_cmp = sort__icache_cmp,
+ .se_snprintf = hist_entry__icache_snprintf,
+ .se_width_idx = HISTC_HAZ_ICACHE,
+};
+
+static int64_t sort__hstage_cmp(struct hist_entry *l, struct hist_entry *r)
+{
+ u8 hstage_l, hstage_r;
+
+ hstage_l = l->haz_data ? l->haz_data->hazard_stage : PERF_HAZ__PIPE_STAGE_NA;
+ hstage_r = r->haz_data ? r->haz_data->hazard_stage : PERF_HAZ__PIPE_STAGE_NA;
+
+ return (int64_t)(hstage_r - hstage_l);
+}
+
+static int hist_entry__hstage_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width)
+{
+ const char *arch;
+ const char *hstage = "-";
+
+ arch = hist_entry__get_arch(he);
+ if (he->haz_data)
+ hstage = perf_haz__hstage_str(he->haz_data->hazard_stage, arch);
+
+ return repsep_snprintf(bf, size, "%-*s", width, hstage);
+}
+
+struct sort_entry sort_haz_hazard_stage = {
+ .se_header = "Hazard Stage",
+ .se_cmp = sort__hstage_cmp,
+ .se_snprintf = hist_entry__hstage_snprintf,
+ .se_width_idx = HISTC_HAZ_HSTAGE,
+};
+
+static int64_t sort__hreason_cmp(struct hist_entry *l, struct hist_entry *r)
+{
+ u8 hreason_l, hreason_r;
+
+ hreason_l = l->haz_data ? l->haz_data->hazard_reason : PERF_HAZ__HREASON_NA;
+ hreason_r = r->haz_data ? r->haz_data->hazard_reason : PERF_HAZ__HREASON_NA;
+
+ return (int64_t)(hreason_r - hreason_l);
+}
+
+static int hist_entry__hreason_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width)
+{
+ const char *arch;
+ const char *hreason = "-";
+
+ arch = hist_entry__get_arch(he);
+ if (he->haz_data) {
+ hreason = perf_haz__hreason_str(he->haz_data->hazard_stage,
+ he->haz_data->hazard_reason, arch);
+ }
+
+ return repsep_snprintf(bf, size, "%-*s", width, hreason);
+}
+
+struct sort_entry sort_haz_hazard_reason = {
+ .se_header = "Hazard Reason",
+ .se_cmp = sort__hreason_cmp,
+ .se_snprintf = hist_entry__hreason_snprintf,
+ .se_width_idx = HISTC_HAZ_HREASON,
+};
+
+static int64_t sort__sstage_cmp(struct hist_entry *l, struct hist_entry *r)
+{
+ u8 sstage_l, sstage_r;
+
+ sstage_l = l->haz_data ? l->haz_data->stall_stage : PERF_HAZ__PIPE_STAGE_NA;
+ sstage_r = r->haz_data ? r->haz_data->stall_stage : PERF_HAZ__PIPE_STAGE_NA;
+
+ return (int64_t)(sstage_r - sstage_l);
+}
+
+static int hist_entry__sstage_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width)
+{
+ const char *arch;
+ const char *sstage = "-";
+
+ arch = hist_entry__get_arch(he);
+ if (he->haz_data)
+ sstage = perf_haz__sstage_str(he->haz_data->stall_stage, arch);
+
+ return repsep_snprintf(bf, size, "%-*s", width, sstage);
+}
+
+struct sort_entry sort_haz_stall_stage = {
+ .se_header = "Stall Stage",
+ .se_cmp = sort__sstage_cmp,
+ .se_snprintf = hist_entry__sstage_snprintf,
+ .se_width_idx = HISTC_HAZ_SSTAGE,
+};
+
+static int64_t sort__sreason_cmp(struct hist_entry *l, struct hist_entry *r)
+{
+ u8 sreason_l, sreason_r;
+
+ sreason_l = l->haz_data ? l->haz_data->stall_reason : PERF_HAZ__SREASON_NA;
+ sreason_r = r->haz_data ? r->haz_data->stall_reason : PERF_HAZ__SREASON_NA;
+
+ return (int64_t)(sreason_r - sreason_l);
+}
+
+static int hist_entry__sreason_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width)
+{
+ const char *arch;
+ const char *sreason = "-";
+
+ arch = hist_entry__get_arch(he);
+ if (he->haz_data) {
+ sreason = perf_haz__sreason_str(he->haz_data->stall_stage,
+ he->haz_data->stall_reason, arch);
+ }
+
+ return repsep_snprintf(bf, size, "%-*s", width, sreason);
+}
+
+struct sort_entry sort_haz_stall_reason = {
+ .se_header = "Stall Reason",
+ .se_cmp = sort__sreason_cmp,
+ .se_snprintf = hist_entry__sreason_snprintf,
+ .se_width_idx = HISTC_HAZ_SREASON,
+};
+
static int64_t
sort__phys_daddr_cmp(struct hist_entry *left, struct hist_entry *right)
{
@@ -1699,6 +1885,19 @@ static struct sort_dimension memory_sort_dimensions[] = {
#undef DIM
+#define DIM(d, n, func) [d - __SORT_HAZARD_MODE] = { .name = n, .entry = &(func) }
+
+static struct sort_dimension hazard_sort_dimensions[] = {
+ DIM(SORT_HAZ_ITYPE, "itype", sort_haz_inst_type),
+ DIM(SORT_HAZ_ICACHE, "icache", sort_haz_inst_cache),
+ DIM(SORT_HAZ_HSTAGE, "hstage", sort_haz_hazard_stage),
+ DIM(SORT_HAZ_HREASON, "hreason", sort_haz_hazard_reason),
+ DIM(SORT_HAZ_SSTAGE, "sstage", sort_haz_stall_stage),
+ DIM(SORT_HAZ_SREASON, "sreason", sort_haz_stall_reason),
+};
+
+#undef DIM
+
struct hpp_dimension {
const char *name;
struct perf_hpp_fmt *fmt;
@@ -2643,6 +2842,19 @@ int sort_dimension__add(struct perf_hpp_list *list, const char *tok,
return 0;
}
+ for (i = 0; i < ARRAY_SIZE(hazard_sort_dimensions); i++) {
+ struct sort_dimension *sd = &hazard_sort_dimensions[i];
+
+ if (strncasecmp(tok, sd->name, strlen(tok)))
+ continue;
+
+ if (sort__mode != SORT_MODE__HAZARD)
+ return -EINVAL;
+
+ __sort_dimension__add(sd, list, level);
+ return 0;
+ }
+
if (!add_dynamic_entry(evlist, tok, level))
return 0;
@@ -2705,6 +2917,7 @@ static const char *get_default_sort_order(struct evlist *evlist)
default_top_sort_order,
default_diff_sort_order,
default_tracepoint_sort_order,
+ default_haz_sort_order,
};
bool use_trace = true;
struct evsel *evsel;
@@ -2976,6 +3189,18 @@ int output_field_add(struct perf_hpp_list *list, char *tok)
return __sort_dimension__add_output(list, sd);
}
+ for (i = 0; i < ARRAY_SIZE(hazard_sort_dimensions); i++) {
+ struct sort_dimension *sd = &hazard_sort_dimensions[i];
+
+ if (strncasecmp(tok, sd->name, strlen(tok)))
+ continue;
+
+ if (sort__mode != SORT_MODE__HAZARD)
+ return -EINVAL;
+
+ return __sort_dimension__add_output(list, sd);
+ }
+
return -ESRCH;
}
@@ -3014,6 +3239,9 @@ void reset_dimensions(void)
for (i = 0; i < ARRAY_SIZE(memory_sort_dimensions); i++)
memory_sort_dimensions[i].taken = 0;
+
+ for (i = 0; i < ARRAY_SIZE(hazard_sort_dimensions); i++)
+ hazard_sort_dimensions[i].taken = 0;
}
bool is_strict_order(const char *order)
@@ -3148,6 +3376,8 @@ const char *sort_help(const char *prefix)
ARRAY_SIZE(bstack_sort_dimensions), &len);
add_sort_string(&sb, memory_sort_dimensions,
ARRAY_SIZE(memory_sort_dimensions), &len);
+ add_sort_string(&sb, hazard_sort_dimensions,
+ ARRAY_SIZE(hazard_sort_dimensions), &len);
s = strbuf_detach(&sb, NULL);
strbuf_release(&sb);
return s;
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 55eb65fd593f..3c18ece0aa4f 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -12,6 +12,7 @@
#include "hist.h"
#include "stat.h"
#include "spark.h"
+#include "env.h"
struct option;
struct thread;
@@ -36,6 +37,7 @@ extern struct sort_entry sort_sym_to;
extern struct sort_entry sort_srcline;
extern enum sort_type sort__first_dimension;
extern const char default_mem_sort_order[];
+extern const char default_haz_sort_order[];
struct res_sample {
u64 time;
@@ -199,6 +201,16 @@ static inline float hist_entry__get_percent_limit(struct hist_entry *he)
return period * 100.0 / total_period;
}
+static inline struct perf_env *hist_entry__env(struct hist_entry *he)
+{
+ return perf_evsel__env(hists_to_evsel(he->hists));
+}
+
+static inline const char *hist_entry__get_arch(struct hist_entry *he)
+{
+ return perf_env__arch(hist_entry__env(he));
+}
+
enum sort_mode {
SORT_MODE__NORMAL,
SORT_MODE__BRANCH,
@@ -206,6 +218,7 @@ enum sort_mode {
SORT_MODE__TOP,
SORT_MODE__DIFF,
SORT_MODE__TRACEPOINT,
+ SORT_MODE__HAZARD,
};
enum sort_type {
@@ -254,6 +267,15 @@ enum sort_type {
SORT_MEM_DCACHELINE,
SORT_MEM_IADDR_SYMBOL,
SORT_MEM_PHYS_DADDR,
+
+ /* hazard mode specific sort keys */
+ __SORT_HAZARD_MODE,
+ SORT_HAZ_ITYPE = __SORT_HAZARD_MODE,
+ SORT_HAZ_ICACHE,
+ SORT_HAZ_HSTAGE,
+ SORT_HAZ_HREASON,
+ SORT_HAZ_SSTAGE,
+ SORT_HAZ_SREASON,
};
/*
--
2.21.1
^ permalink raw reply related
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