* Re: [PATCH v3 2/7] mm/memory_hotplug: Rename mhp_restrictions to mhp_params
From: David Hildenbrand @ 2020-02-24 9:11 UTC (permalink / raw)
To: Logan Gunthorpe, linux-kernel, linux-arm-kernel, linux-ia64,
linuxppc-dev, linux-s390, linux-sh, platform-driver-x86, linux-mm,
Dan Williams, Michal Hocko, Andrew Morton
Cc: Eric Badger, Peter Zijlstra, Catalin Marinas, Dave Hansen,
Ingo Molnar, Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
Will Deacon, Christoph Hellwig
In-Reply-To: <20200221182503.28317-3-logang@deltatee.com>
On 21.02.20 19:24, Logan Gunthorpe wrote:
> The mhp_restrictions struct really doesn't specify anything resembling
> a restriction anymore so rename it to be mhp_params as it is a list
> of extended parameters.
>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
From: Michal Hocko @ 2020-02-24 8:58 UTC (permalink / raw)
To: Christopher Lameter
Cc: Sachin Sant, Pekka Enberg, Kirill Tkhai, Linux-Next Mailing List,
David Rientjes, Joonsoo Kim, linuxppc-dev
In-Reply-To: <alpine.DEB.2.21.2002220337030.2000@www.lameter.com>
On Sat 22-02-20 03:38:11, Cristopher Lameter wrote:
> On Tue, 18 Feb 2020, Michal Hocko wrote:
>
> > Anyway, I do not think it is expected that kmalloc_node just blows up
> > on those nodes. The page allocator simply falls back to the closest
> > node. Something for kmalloc maintainers I believe.
>
> That is the case for an unconstrained allocation. kmalloc_node means that
> you want memory from that node. And If there is no such node then it is an
> error.
Hmm, nasty. Is there any reason why kmalloc_node behaves differently
from the page allocator?
> > A short summary. kmalloc_node blows up when trying to allocate from a
> > memory less node.
>
> Use kmalloc instead? And set a memory allocation policy?
The current code (memcg_expand_one_shrinker_map resp. memcg_alloc_shrinker_maps)
already use kvmalloc. Kirill's patch wanted to make those data structure
on the respective node and kvmalloc_node sounded like the right thing to
do. It comes as a surprise that the kernel simply blows up on a memory
less node rather than falling back to a close node gracefully. I suspect
this already happens when the target node is out of memory, right?
How would a memory allocation policy help in this case btw.?
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v6 2/3] arm64: dts: ls1028a: Add PCIe controller DT nodes
From: Michael Walle @ 2020-02-24 8:53 UTC (permalink / raw)
To: Shawn Guo
Cc: mark.rutland, devicetree, lorenzo.pieralisi, xiaowei.bao,
roy.zang, linux-pci, Zhiqiang.Hou, linux-kernel, leoyang.li,
minghuan.Lian, robh+dt, mingkai.hu, bhelgaas, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20200224084307.GD27688@dragon>
Hi Shawn, all,
Am 2020-02-24 09:43, schrieb Shawn Guo:
> On Mon, Feb 24, 2020 at 09:11:05AM +0100, Michael Walle wrote:
>> Hi Xiaowei, Hi Shawn,
>>
>> > LS1028a implements 2 PCIe 3.0 controllers.
>>
>> Patch 1/3 and 3/3 are in Linus' tree but nobody seems to care about
>> this patch
>> anymore :(
>>
>> This doesn't work well with the IOMMU, because the iommu-map property
>> is
>> missing. The bootloader needs the &smmu phandle to fixup the entry.
>> See
>> below.
>>
>> Shawn, will you add this patch to your tree once its fixed,
>> considering it
>> just adds the device tree node for the LS1028A?
>
> The patch/thread is a bit aged. You may want to send an updated patch
> for discussion.
So should I just pick up the patch add my two fixes and send it again?
What about
the Signed-off-by tags? Leave them? Replace them? Add mine?
-michael
^ permalink raw reply
* Re: [PATCH v2 3/3] ASoC: fsl_easrc: Add EASRC ASoC CPU DAI and platform drivers
From: S.j. Wang @ 2020-02-24 8:53 UTC (permalink / raw)
To: Nicolin Chen
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
alsa-devel@alsa-project.org, timur@kernel.org,
Xiubo.Lee@gmail.com, linuxppc-dev@lists.ozlabs.org,
tiwai@suse.com, lgirdwood@gmail.com, robh+dt@kernel.org,
perex@perex.cz, broonie@kernel.org, festevam@gmail.com,
linux-kernel@vger.kernel.org
Hi
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> > sound/soc/fsl/Kconfig | 10 +
> > sound/soc/fsl/Makefile | 2 +
> > sound/soc/fsl/fsl_asrc_common.h | 1 +
> > sound/soc/fsl/fsl_easrc.c | 2265 +++++++++++++++++++++++++++++++
> > sound/soc/fsl/fsl_easrc.h | 668 +++++++++
> > sound/soc/fsl/fsl_easrc_dma.c | 440 ++++++
>
> I see a 90% similarity between fsl_asrc_dma and fsl_easrc_dma files.
> Would it be possible reuse the existing code? Could share structures from
> my point of view, just like it reuses "enum asrc_pair_index", I know
> differentiating "pair" and "context" is a big point here though.
>
> A possible quick solution for that, off the top of my head, could be:
>
> 1) in fsl_asrc_common.h
>
> struct fsl_asrc {
> ....
> };
>
> struct fsl_asrc_pair {
> ....
> };
>
> 2) in fsl_easrc.h
>
> /* Renaming shared structures */
> #define fsl_easrc fsl_asrc
> #define fsl_easrc_context fsl_asrc_pair
>
> May be a good idea to see if others have some opinion too.
>
We need to modify the fsl_asrc and fsl_asrc_pair, let them
To be used by both driver, also we need to put the specific
Definition for each module to same struct, right?
>
> > +static const struct regmap_config fsl_easrc_regmap_config = {
> > + .readable_reg = fsl_easrc_readable_reg,
> > + .volatile_reg = fsl_easrc_volatile_reg,
> > + .writeable_reg = fsl_easrc_writeable_reg,
>
> Can we use regmap_range and regmap_access_table?
>
Can the regmap_range support discontinuous registers? The
reg_stride = 4.
Best regards
Wang shengjiu
^ permalink raw reply
* Re: [PATCH v6 2/3] arm64: dts: ls1028a: Add PCIe controller DT nodes
From: Shawn Guo @ 2020-02-24 8:43 UTC (permalink / raw)
To: Michael Walle
Cc: mark.rutland, devicetree, lorenzo.pieralisi, xiaowei.bao,
roy.zang, linux-pci, Zhiqiang.Hou, linux-kernel, leoyang.li,
minghuan.Lian, robh+dt, mingkai.hu, bhelgaas, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20200224081105.13878-1-michael@walle.cc>
On Mon, Feb 24, 2020 at 09:11:05AM +0100, Michael Walle wrote:
> Hi Xiaowei, Hi Shawn,
>
> > LS1028a implements 2 PCIe 3.0 controllers.
>
> Patch 1/3 and 3/3 are in Linus' tree but nobody seems to care about this patch
> anymore :(
>
> This doesn't work well with the IOMMU, because the iommu-map property is
> missing. The bootloader needs the &smmu phandle to fixup the entry. See
> below.
>
> Shawn, will you add this patch to your tree once its fixed, considering it
> just adds the device tree node for the LS1028A?
The patch/thread is a bit aged. You may want to send an updated patch
for discussion.
Shawn
>
> >
> > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
^ permalink raw reply
* Re: [PATCH v6 2/3] arm64: dts: ls1028a: Add PCIe controller DT nodes
From: Michael Walle @ 2020-02-24 8:11 UTC (permalink / raw)
To: xiaowei.bao
Cc: mark.rutland, devicetree, lorenzo.pieralisi, roy.zang, linux-pci,
Zhiqiang.Hou, linux-kernel, leoyang.li, minghuan.Lian,
Michael Walle, robh+dt, mingkai.hu, bhelgaas, shawnguo,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20190902034319.14026-2-xiaowei.bao@nxp.com>
Hi Xiaowei, Hi Shawn,
> LS1028a implements 2 PCIe 3.0 controllers.
Patch 1/3 and 3/3 are in Linus' tree but nobody seems to care about this patch
anymore :(
This doesn't work well with the IOMMU, because the iommu-map property is
missing. The bootloader needs the &smmu phandle to fixup the entry. See
below.
Shawn, will you add this patch to your tree once its fixed, considering it
just adds the device tree node for the LS1028A?
>
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
> v2:
> - Fix up the legacy INTx allocate failed issue.
> v3:
> - No change.
> v4:
> - Remove the num-lanes property.
> v5:
> - Add the num-viewport property.
> v6:
> - move num-viewport to 8.
>
> arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 52 ++++++++++++++++++++++++++
> 1 file changed, 52 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> index 72b9a75..c043b1d 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> @@ -625,6 +625,58 @@
> };
> };
>
> + pcie@3400000 {
> + compatible = "fsl,ls1028a-pcie";
> + reg = <0x00 0x03400000 0x0 0x00100000 /* controller registers */
> + 0x80 0x00000000 0x0 0x00002000>; /* configuration space */
> + reg-names = "regs", "config";
> + interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>, /* PME interrupt */
> + <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>; /* aer interrupt */
> + interrupt-names = "pme", "aer";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + device_type = "pci";
> + dma-coherent;
> + num-viewport = <8>;
> + bus-range = <0x0 0xff>;
> + ranges = <0x81000000 0x0 0x00000000 0x80 0x00010000 0x0 0x00010000 /* downstream I/O */
> + 0x82000000 0x0 0x40000000 0x80 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
> + msi-parent = <&its>;
iommu-map = <0 &smmu 0 0>; /* fixed up by bootloader */
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0 0 0 7>;
> + interrupt-map = <0000 0 0 1 &gic 0 0 GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>,
> + <0000 0 0 2 &gic 0 0 GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>,
> + <0000 0 0 3 &gic 0 0 GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>,
> + <0000 0 0 4 &gic 0 0 GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
> + };
> +
> + pcie@3500000 {
> + compatible = "fsl,ls1028a-pcie";
> + reg = <0x00 0x03500000 0x0 0x00100000 /* controller registers */
> + 0x88 0x00000000 0x0 0x00002000>; /* configuration space */
> + reg-names = "regs", "config";
> + interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "pme", "aer";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + device_type = "pci";
> + dma-coherent;
> + num-viewport = <8>;
> + bus-range = <0x0 0xff>;
> + ranges = <0x81000000 0x0 0x00000000 0x88 0x00010000 0x0 0x00010000 /* downstream I/O */
> + 0x82000000 0x0 0x40000000 0x88 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
> + msi-parent = <&its>;
likewise
With these two fixes:
Tested-by: Michael Walle <michael@walle.cc>
-michael
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0 0 0 7>;
> + interrupt-map = <0000 0 0 1 &gic 0 0 GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>,
> + <0000 0 0 2 &gic 0 0 GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
> + <0000 0 0 3 &gic 0 0 GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>,
> + <0000 0 0 4 &gic 0 0 GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
> + };
> +
> pcie@1f0000000 { /* Integrated Endpoint Root Complex */
> compatible = "pci-host-ecam-generic";
> reg = <0x01 0xf0000000 0x0 0x100000>;
> --
> 2.9.5
>
>
^ permalink raw reply
* Re: [PATCH v3 00/27] Add support for OpenCAPI Persistent Memory devices
From: Oliver O'Halloran @ 2020-02-24 6:51 UTC (permalink / raw)
To: Alastair D'Silva
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Paul Mackerras, Krzysztof Kozlowski, Mauro Carvalho Chehab,
Ira Weiny, Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Matthew Wilcox, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, Linux MM, Greg Kroah-Hartman,
Linux Kernel Mailing List, Vishal Verma, Frederic Barrat,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <83034494d5c3da1fa63b172e844f85d0fec7910a.camel@au1.ibm.com>
On Mon, Feb 24, 2020 at 3:43 PM Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> On Sun, 2020-02-23 at 20:37 -0800, Matthew Wilcox wrote:
> > On Mon, Feb 24, 2020 at 03:34:07PM +1100, Alastair D'Silva wrote:
> > > V3:
> > > - Rebase against next/next-20200220
> > > - Move driver to arch/powerpc/platforms/powernv, we now expect
> > > this
> > > driver to go upstream via the powerpc tree
> >
> > That's rather the opposite direction of normal; mostly drivers live
> > under
> > drivers/ and not in arch/. It's easier for drivers to get overlooked
> > when doing tree-wide changes if they're hiding.
>
> This is true, however, given that it was not all that desirable to have
> it under drivers/nvdimm, it's sister driver (for the same hardware) is
> also under arch, and that we don't expect this driver to be used on any
> platform other than powernv, we think this was the most reasonable
> place to put it.
Historically powernv specific platform drivers go in their respective
subsystem trees rather than in arch/ and I'd prefer we kept it that
way. When I added the papr_scm driver I put it in the pseries platform
directory because most of the pseries paravirt code lives there for
some reason; I don't know why. Luckily for me that followed the same
model that Dan used when he put the NFIT driver in drivers/acpi/ and
the libnvdimm core in drivers/nvdimm/ so we didn't have anything to
argue about. However, as Matthew pointed out, it is at odds with how
most subsystems operate. Is there any particular reason we're doing
things this way or should we think about moving libnvdimm users to
drivers/nvdimm/?
Oliver
^ permalink raw reply
* Re: [PATCH v3 08/27] ocxl: Emit a log message showing how much LPC memory was detected
From: Andrew Donnellan @ 2020-02-24 6:13 UTC (permalink / raw)
To: Alastair D'Silva
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: <44e31df282f4550efebc7625bbaeae1566acc481.camel@au1.ibm.com>
On 24/2/20 5:10 pm, Alastair D'Silva wrote:
>> Printing this at info level for every single AFU seems a bit noisy.
>> Perhaps we can print it only if LPC memory is > 0?
>>
>
> There is an early exit before this if there is no LPC memory.
>
Noted, I'd missed that amidst all the early returns for errors.
In that case
Acked-by: Andrew Donnellan <ajd@linux.ibm.com>
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH v3 08/27] ocxl: Emit a log message showing how much LPC memory was detected
From: Alastair D'Silva @ 2020-02-24 6:10 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: <d833b6bd-abad-a419-616a-14687032cd09@linux.ibm.com>
On Mon, 2020-02-24 at 17:06 +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 emits a message showing how much LPC memory & special
> > purpose
> > memory was detected on an OCXL device.
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> > drivers/misc/ocxl/config.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/misc/ocxl/config.c
> > b/drivers/misc/ocxl/config.c
> > index a62e3d7db2bf..701ae6216abf 100644
> > --- a/drivers/misc/ocxl/config.c
> > +++ b/drivers/misc/ocxl/config.c
> > @@ -568,6 +568,10 @@ static int read_afu_lpc_memory_info(struct
> > pci_dev *dev,
> > afu->special_purpose_mem_size =
> > total_mem_size - lpc_mem_size;
> > }
> > +
> > + dev_info(&dev->dev, "Probed LPC memory of %#llx bytes and
> > special purpose memory of %#llx bytes\n",
> > + afu->lpc_mem_size, afu->special_purpose_mem_size);
> > +
>
> Printing this at info level for every single AFU seems a bit noisy.
> Perhaps we can print it only if LPC memory is > 0?
>
There is an early exit before this if there is no LPC memory.
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH v3 07/27] ocxl: Add functions to map/unmap LPC memory
From: Alastair D'Silva @ 2020-02-24 6:08 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: <ebd1c438-1164-8c7c-7067-2389d64740e7@linux.ibm.com>
On Mon, 2020-02-24 at 17:02 +1100, Andrew Donnellan wrote:
> On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > Add functions to map/unmap LPC memory
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> > drivers/misc/ocxl/core.c | 51
> > +++++++++++++++++++++++++++++++
> > drivers/misc/ocxl/ocxl_internal.h | 3 ++
> > include/misc/ocxl.h | 21 +++++++++++++
> > 3 files changed, 75 insertions(+)
> >
> > diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
> > index 2531c6cf19a0..75ff14e3882a 100644
> > --- a/drivers/misc/ocxl/core.c
> > +++ b/drivers/misc/ocxl/core.c
> > @@ -210,6 +210,56 @@ static void unmap_mmio_areas(struct ocxl_afu
> > *afu)
> > release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> > }
> >
> > +int ocxl_afu_map_lpc_mem(struct ocxl_afu *afu)
> > +{
> > + struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> > +
> > + if ((afu->config.lpc_mem_size + afu-
> > >config.special_purpose_mem_size) == 0)
> > + return 0;
>
> I'd prefer the comparison here to be:
>
> afu->config.lpc_mem_size == 0 &&
> afu->config.special_purpose_mem_size == 0
>
> so a reader doesn't have to think about what this means.
>
Ok
> > +
> > + afu->lpc_base_addr = ocxl_link_lpc_map(afu->fn->link, dev);
> > + if (afu->lpc_base_addr == 0)
> > + return -EINVAL;
> > +
> > + if (afu->config.lpc_mem_size > 0) {
> > + afu->lpc_res.start = afu->lpc_base_addr + afu-
> > >config.lpc_mem_offset;
>
> Maybe not for this series - hmm, I wonder if we should print a
> warning
> somewhere (maybe in read_afu_lpc_memory_info()?) if we see the case
> where (lpc_mem_offset > 0 && lpc_mem_size == 0). Likewise for
> special
> purpose?
>
Sounds reasonable, might as well add it here since there are other LPC
changes.
> > + afu->lpc_res.end = afu->lpc_res.start + afu-
> > >config.lpc_mem_size - 1;
> > + }
> > +
> > + if (afu->config.special_purpose_mem_size > 0) {
> > + afu->special_purpose_res.start = afu->lpc_base_addr +
> > + afu-
> > >config.special_purpose_mem_offset;
> > + afu->special_purpose_res.end = afu-
> > >special_purpose_res.start +
> > + afu-
> > >config.special_purpose_mem_size - 1;
> > + }
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(ocxl_afu_map_lpc_mem);
> > +
> > +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu)
> > +{
> > + return &afu->lpc_res;
> > +}
> > +EXPORT_SYMBOL_GPL(ocxl_afu_lpc_mem);
>
> What's the point of this function? A layer of indirection just in
> case
> we need it in future?
>
struct ocxl_afu is opaque outsite the ocxl driver.
> > +
> > +static void unmap_lpc_mem(struct ocxl_afu *afu)
> > +{
> > + struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> > +
> > + if (afu->lpc_res.start || afu->special_purpose_res.start) {
> > + void *link = afu->fn->link;
> > +
> > + // only release the link when the the last consumer
> > calls release
> > + ocxl_link_lpc_release(link, dev);
> > +
> > + afu->lpc_res.start = 0;
> > + afu->lpc_res.end = 0;
> > + afu->special_purpose_res.start = 0;
> > + afu->special_purpose_res.end = 0;
> > + }
> > +}
> > +
> > static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct
> > pci_dev *dev)
> > {
> > int rc;
> > @@ -251,6 +301,7 @@ static int configure_afu(struct ocxl_afu *afu,
> > u8 afu_idx, struct pci_dev *dev)
> >
> > static void deconfigure_afu(struct ocxl_afu *afu)
> > {
> > + unmap_lpc_mem(afu);
> > unmap_mmio_areas(afu);
> > reclaim_afu_pasid(afu);
> > reclaim_afu_actag(afu);
> > diff --git a/drivers/misc/ocxl/ocxl_internal.h
> > b/drivers/misc/ocxl/ocxl_internal.h
> > index d0c8c4838f42..ce0cac1da416 100644
> > --- a/drivers/misc/ocxl/ocxl_internal.h
> > +++ b/drivers/misc/ocxl/ocxl_internal.h
> > @@ -52,6 +52,9 @@ struct ocxl_afu {
> > void __iomem *global_mmio_ptr;
> > u64 pp_mmio_start;
> > void *private;
> > + u64 lpc_base_addr; /* Covers both LPC & special purpose memory
> > */
> > + struct resource lpc_res;
> > + struct resource special_purpose_res;
> > };
> >
> > enum ocxl_context_status {
> > diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> > index 357ef1aadbc0..d8b0b4d46bfb 100644
> > --- a/include/misc/ocxl.h
> > +++ b/include/misc/ocxl.h
> > @@ -203,6 +203,27 @@ int ocxl_irq_set_handler(struct ocxl_context
> > *ctx, int irq_id,
> >
> > // AFU Metadata
> >
> > +/**
> > + * ocxl_afu_map_lpc_mem() - Map the LPC system & special purpose
> > memory for an AFU
> > + * Do not call this during device discovery, as there may me
> > multiple
>
> be
>
> > + * devices on a link, and the memory is mapped for the whole link,
> > not
> > + * just one device. It should only be called after all devices
> > have
> > + * registered their memory on the link.
> > + *
> > + * @afu: The AFU that has the LPC memory to map
> > + *
> > + * Returns 0 on success, negative on failure
> > + */
> > +int ocxl_afu_map_lpc_mem(struct ocxl_afu *afu);
> > +
> > +/**
> > + * ocxl_afu_lpc_mem() - Get the physical address range of LPC
> > memory for an AFU
> > + * @afu: The AFU associated with the LPC memory
> > + *
> > + * Returns a pointer to the resource struct for the physical
> > address range
> > + */
> > +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu);
> > +
> > /**
> > * ocxl_afu_config() - Get a pointer to the config for an AFU
> > * @afu: a pointer to the AFU to get the config for
> >
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH v3 08/27] ocxl: Emit a log message showing how much LPC memory was detected
From: Andrew Donnellan @ 2020-02-24 6:06 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-9-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 emits a message showing how much LPC memory & special purpose
> memory was detected on an OCXL device.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
> drivers/misc/ocxl/config.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
> index a62e3d7db2bf..701ae6216abf 100644
> --- a/drivers/misc/ocxl/config.c
> +++ b/drivers/misc/ocxl/config.c
> @@ -568,6 +568,10 @@ static int read_afu_lpc_memory_info(struct pci_dev *dev,
> afu->special_purpose_mem_size =
> total_mem_size - lpc_mem_size;
> }
> +
> + dev_info(&dev->dev, "Probed LPC memory of %#llx bytes and special purpose memory of %#llx bytes\n",
> + afu->lpc_mem_size, afu->special_purpose_mem_size);
> +
Printing this at info level for every single AFU seems a bit noisy.
Perhaps we can print it only if LPC memory is > 0?
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH v3 07/27] ocxl: Add functions to map/unmap LPC memory
From: Andrew Donnellan @ 2020-02-24 6:02 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-8-alastair@au1.ibm.com>
On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Add functions to map/unmap LPC memory
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
> drivers/misc/ocxl/core.c | 51 +++++++++++++++++++++++++++++++
> drivers/misc/ocxl/ocxl_internal.h | 3 ++
> include/misc/ocxl.h | 21 +++++++++++++
> 3 files changed, 75 insertions(+)
>
> diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
> index 2531c6cf19a0..75ff14e3882a 100644
> --- a/drivers/misc/ocxl/core.c
> +++ b/drivers/misc/ocxl/core.c
> @@ -210,6 +210,56 @@ static void unmap_mmio_areas(struct ocxl_afu *afu)
> release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> }
>
> +int ocxl_afu_map_lpc_mem(struct ocxl_afu *afu)
> +{
> + struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> +
> + if ((afu->config.lpc_mem_size + afu->config.special_purpose_mem_size) == 0)
> + return 0;
I'd prefer the comparison here to be:
afu->config.lpc_mem_size == 0 &&
afu->config.special_purpose_mem_size == 0
so a reader doesn't have to think about what this means.
> +
> + afu->lpc_base_addr = ocxl_link_lpc_map(afu->fn->link, dev);
> + if (afu->lpc_base_addr == 0)
> + return -EINVAL;
> +
> + if (afu->config.lpc_mem_size > 0) {
> + afu->lpc_res.start = afu->lpc_base_addr + afu->config.lpc_mem_offset;
Maybe not for this series - hmm, I wonder if we should print a warning
somewhere (maybe in read_afu_lpc_memory_info()?) if we see the case
where (lpc_mem_offset > 0 && lpc_mem_size == 0). Likewise for special
purpose?
> + afu->lpc_res.end = afu->lpc_res.start + afu->config.lpc_mem_size - 1;
> + }
> +
> + if (afu->config.special_purpose_mem_size > 0) {
> + afu->special_purpose_res.start = afu->lpc_base_addr +
> + afu->config.special_purpose_mem_offset;
> + afu->special_purpose_res.end = afu->special_purpose_res.start +
> + afu->config.special_purpose_mem_size - 1;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_afu_map_lpc_mem);
> +
> +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu)
> +{
> + return &afu->lpc_res;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_afu_lpc_mem);
What's the point of this function? A layer of indirection just in case
we need it in future?
> +
> +static void unmap_lpc_mem(struct ocxl_afu *afu)
> +{
> + struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> +
> + if (afu->lpc_res.start || afu->special_purpose_res.start) {
> + void *link = afu->fn->link;
> +
> + // only release the link when the the last consumer calls release
> + ocxl_link_lpc_release(link, dev);
> +
> + afu->lpc_res.start = 0;
> + afu->lpc_res.end = 0;
> + afu->special_purpose_res.start = 0;
> + afu->special_purpose_res.end = 0;
> + }
> +}
> +
> static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
> {
> int rc;
> @@ -251,6 +301,7 @@ static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
>
> static void deconfigure_afu(struct ocxl_afu *afu)
> {
> + unmap_lpc_mem(afu);
> unmap_mmio_areas(afu);
> reclaim_afu_pasid(afu);
> reclaim_afu_actag(afu);
> diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
> index d0c8c4838f42..ce0cac1da416 100644
> --- a/drivers/misc/ocxl/ocxl_internal.h
> +++ b/drivers/misc/ocxl/ocxl_internal.h
> @@ -52,6 +52,9 @@ struct ocxl_afu {
> void __iomem *global_mmio_ptr;
> u64 pp_mmio_start;
> void *private;
> + u64 lpc_base_addr; /* Covers both LPC & special purpose memory */
> + struct resource lpc_res;
> + struct resource special_purpose_res;
> };
>
> enum ocxl_context_status {
> diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> index 357ef1aadbc0..d8b0b4d46bfb 100644
> --- a/include/misc/ocxl.h
> +++ b/include/misc/ocxl.h
> @@ -203,6 +203,27 @@ int ocxl_irq_set_handler(struct ocxl_context *ctx, int irq_id,
>
> // AFU Metadata
>
> +/**
> + * ocxl_afu_map_lpc_mem() - Map the LPC system & special purpose memory for an AFU
> + * Do not call this during device discovery, as there may me multiple
be
> + * devices on a link, and the memory is mapped for the whole link, not
> + * just one device. It should only be called after all devices have
> + * registered their memory on the link.
> + *
> + * @afu: The AFU that has the LPC memory to map
> + *
> + * Returns 0 on success, negative on failure
> + */
> +int ocxl_afu_map_lpc_mem(struct ocxl_afu *afu);
> +
> +/**
> + * ocxl_afu_lpc_mem() - Get the physical address range of LPC memory for an AFU
> + * @afu: The AFU associated with the LPC memory
> + *
> + * Returns a pointer to the resource struct for the physical address range
> + */
> +struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu);
> +
> /**
> * ocxl_afu_config() - Get a pointer to the config for an AFU
> * @afu: a pointer to the AFU to get the config for
>
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [RFC PATCH v2 00/12] Reduce ifdef mess in ptrace
From: Christophe Leroy @ 2020-02-24 5:58 UTC (permalink / raw)
To: Michael Neuling; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <7b86733f81c7e15d81ab14b98c8998011ed54880.camel@neuling.org>
Le 24/02/2020 à 03:15, Michael Neuling a écrit :
> Christophe,
>
>> Le 28/06/2019 à 17:47, Christophe Leroy a écrit :
>>> The purpose of this series is to reduce the amount of #ifdefs
>>> in ptrace.c
>>>
>>
>> Any feedback on this series which aims at fixing the issue you opened at
>> https://github.com/linuxppc/issues/issues/128 ?
>
> Yeah, sorry my bad. You did all the hard work and I ignored it.
>
> I like the approach and is a long the lines I was thinking. Putting it in a
> ptrace subdir, splitting out adv_debug_regs, TM, SPE, Alitivec, VSX.
> ppc_gethwdinfo() looks a lot nicer now too (that was some of the worst of it).
>
> I've not gone through it with a fine tooth comb though. There is (rightly) a lot
> of code moved around which could have introduced some issues.
>
> It applies on v5.2 but are you planning on updating it to a newer base?
>
As you noticed there is a lot of code moved around, and rebasing
produces a lot of conflicts. So I didn't want to spend hours to rebase
and rebase without being sure it was the right approach.
Now that I got a positive feedback I'll consider rebasing it, hopping
that Michael will pick it up.
Christophe
^ permalink raw reply
* Re: [PATCH v3 01/27] powerpc: Add OPAL calls for LPC memory alloc/release
From: Alastair D'Silva @ 2020-02-24 5:50 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: <6386dbcd-dd0f-18da-cec7-50208abba591@linux.ibm.com>
On Mon, 2020-02-24 at 16:49 +1100, Andrew Donnellan wrote:
> On 21/2/20 2:26 pm, Alastair D'Silva wrote:
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > Add OPAL calls for LPC memory alloc/release
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > Acked-by: Andrew Donnellan <ajd@linux.ibm.com>
> > Acked-by: Frederic Barrat <fbarrat@linux.ibm.com>
>
> Summary line should be "powerpc/powernv".
>
>
Ok
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH v3 03/27] powerpc: Map & release OpenCAPI LPC memory
From: Andrew Donnellan @ 2020-02-24 5:49 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: <61e2b75b-334e-9eec-d14d-7dfdb4654415@linux.ibm.com>
On 24/2/20 1:51 pm, Andrew Donnellan wrote:
> On 21/2/20 2:26 pm, Alastair D'Silva wrote:
>> From: Alastair D'Silva <alastair@d-silva.org>
>>
>> This patch adds platform support to map & release LPC memory.
>>
>> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
>
> Nothing seems obviously wrong here.
>
> Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
Oh, commit message nitpick :)
Summary should be powerpc/powernv. Commit message should explain that
this is for the powernv platform and presents an interface that drivers
can use to make use of the new OPAL calls.
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH v3 01/27] powerpc: Add OPAL calls for LPC memory alloc/release
From: Andrew Donnellan @ 2020-02-24 5:49 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-2-alastair@au1.ibm.com>
On 21/2/20 2:26 pm, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Add OPAL calls for LPC memory alloc/release
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> Acked-by: Andrew Donnellan <ajd@linux.ibm.com>
> Acked-by: Frederic Barrat <fbarrat@linux.ibm.com>
Summary line should be "powerpc/powernv".
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH v3 06/27] ocxl: Tally up the LPC memory on a link & allow it to be mapped
From: Alastair D'Silva @ 2020-02-24 5:36 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: <8a6eaedd-d806-9111-84ac-c4961227d69c@linux.ibm.com>
On Mon, 2020-02-24 at 16:25 +1100, Andrew Donnellan wrote:
> On 21/2/20 2:26 pm, Alastair D'Silva wrote:
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > Tally up the LPC memory on an OpenCAPI link & allow it to be mapped
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
>
> This commit message is a bit short and could do with some further
> explanation.
>
> In particular - it's worth explaining why the tracking of available
> LPC
> memory needs to be done at a link level, because a single OpenCAPI
> card
> can have multiple PCI functions, each with multiple AFUs which define
> an
> amount of LPC memory they have, even if the common case is expected
> to
> be a single function with a single AFU and thus one LPC area per
> link.
Ok
>
> Snowpatch has a few checkpatch issues to report:
>
> https://openpower.xyz/job/snowpatch/job/snowpatch-linux-checkpatch/11800//artifact/linux/checkpatch.log
>
Gah, I could have sworn I ran checkpatch against this :/
> The code generally looks okay to me.
>
> > diff --git a/drivers/misc/ocxl/ocxl_internal.h
> > b/drivers/misc/ocxl/ocxl_internal.h
> > index 198e4e4bc51d..d0c8c4838f42 100644
> > --- a/drivers/misc/ocxl/ocxl_internal.h
> > +++ b/drivers/misc/ocxl/ocxl_internal.h
> > @@ -142,4 +142,37 @@ int ocxl_irq_offset_to_id(struct ocxl_context
> > *ctx, u64 offset);
> > u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id);
> > void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
> >
> > +/**
> > + * ocxl_link_add_lpc_mem() - Increment the amount of memory
> > required by an OpenCAPI link
> > + *
> > + * @link_handle: The OpenCAPI link handle
> > + * @offset: The offset of the memory to add
> > + * @size: The amount of memory to increment by
> > + *
> > + * Returns 0 on success, negative on overflow
> > + */
>
> I think "amount of memory required" isn't the best way to express
> this.
>
> Might as well explicitly say -EINVAL on overflow.
>
Ok
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH v3 06/27] ocxl: Tally up the LPC memory on a link & allow it to be mapped
From: Andrew Donnellan @ 2020-02-24 5:25 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-7-alastair@au1.ibm.com>
On 21/2/20 2:26 pm, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Tally up the LPC memory on an OpenCAPI link & allow it to be mapped
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
This commit message is a bit short and could do with some further
explanation.
In particular - it's worth explaining why the tracking of available LPC
memory needs to be done at a link level, because a single OpenCAPI card
can have multiple PCI functions, each with multiple AFUs which define an
amount of LPC memory they have, even if the common case is expected to
be a single function with a single AFU and thus one LPC area per link.
Snowpatch has a few checkpatch issues to report:
https://openpower.xyz/job/snowpatch/job/snowpatch-linux-checkpatch/11800//artifact/linux/checkpatch.log
The code generally looks okay to me.
> diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
> index 198e4e4bc51d..d0c8c4838f42 100644
> --- a/drivers/misc/ocxl/ocxl_internal.h
> +++ b/drivers/misc/ocxl/ocxl_internal.h
> @@ -142,4 +142,37 @@ int ocxl_irq_offset_to_id(struct ocxl_context *ctx, u64 offset);
> u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id);
> void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
>
> +/**
> + * ocxl_link_add_lpc_mem() - Increment the amount of memory required by an OpenCAPI link
> + *
> + * @link_handle: The OpenCAPI link handle
> + * @offset: The offset of the memory to add
> + * @size: The amount of memory to increment by
> + *
> + * Returns 0 on success, negative on overflow
> + */
I think "amount of memory required" isn't the best way to express this.
Might as well explicitly say -EINVAL on overflow.
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH v2 1/8] powerpc/perf/hv-24x7: Fix inconsistent output values incase multiple hv-24x7 events run
From: kajoljain @ 2020-02-24 5:17 UTC (permalink / raw)
To: Sukadev Bhattiprolu
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, acme, jmario,
namhyung, tglx, mpetlan, gregkh, linux-perf-users, jolsa,
linuxppc-dev
In-Reply-To: <20200223025119.GA17174@us.ibm.com>
On 2/23/20 8:21 AM, Sukadev Bhattiprolu wrote:
> Kajol Jain [kjain@linux.ibm.com] wrote:
>> Commit 2b206ee6b0df ("powerpc/perf/hv-24x7: Display change in counter
>> values")' added to print _change_ in the counter value rather then raw
>> value for 24x7 counters. Incase of transactions, the event count
>> is set to 0 at the beginning of the transaction. It also sets
>> the event's prev_count to the raw value at the time of initialization.
>> Because of setting event count to 0, we are seeing some weird behaviour,
>> whenever we run multiple 24x7 events at a time.
>
> Interesting. Are we taking delta of a delta and ending up with large
> negative values in the -I case? However...
>
Hi Sukadev,
That's right, we are ending up in calculating delta of delta which may give us negative values
because of which we are getting these large values in -I case.
> <snip>
>
>>
>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
>> ---
>> arch/powerpc/perf/hv-24x7.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
>> index 573e0b309c0c..6dbbf70232aa 100644
>> --- a/arch/powerpc/perf/hv-24x7.c
>> +++ b/arch/powerpc/perf/hv-24x7.c
>> @@ -1409,7 +1409,7 @@ static void h_24x7_event_read(struct perf_event *event)
>> * that would require issuing a hcall, which would then
>> * defeat the purpose of using the txn interface.
>> */
>> - local64_set(&event->count, 0);
>> + local64_add(0, &event->count);
>
> ... not sure, how adding zero to the count helps. Should we just remove the
> line (and the comment block above it)? Or does it help to clear the event
> count in ->start_txn() rather than on read()?
Its not impacting much as we are just adding and not setting event count.I think we can remove that line
with the comment added. Will it be ok if I remove that whole part?
>
> How does the change impact the counts when run without the -I?
There won't be much impact because from my understanding you did add
`(void)local64_xchg(&event->hw.prev_count, ct);`, to print change value in your
Commit 2b206ee6b0df in function 'h_24x7_event_init()'.
And we will go through this path in case of 'without -I'. Please let me know if my
understanding is correct.
Thanks,
Kajol
>
> Thanks for chasing this down.
>
> Sukadev
>
^ permalink raw reply
* Re: [PATCH v2 5/5] Documentation: Document sysfs interfaces purr, spurr, idle_purr, idle_spurr
From: Gautham R Shenoy @ 2020-02-24 5:15 UTC (permalink / raw)
To: Nathan Lynch
Cc: Gautham R. Shenoy, Tyrel Datwyler, linux-kernel, Kamalesh Babulal,
Naveen N. Rao, Vaidyanathan Srinivasan, linuxppc-dev
In-Reply-To: <87blprubh0.fsf@linux.ibm.com>
On Fri, Feb 21, 2020 at 10:55:07AM -0600, Nathan Lynch wrote:
> "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> writes:
> > diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
> > index 2e0e3b4..799dc737a 100644
> > --- a/Documentation/ABI/testing/sysfs-devices-system-cpu
> > +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
> > @@ -580,3 +580,42 @@ Description: Secure Virtual Machine
> > If 1, it means the system is using the Protected Execution
> > Facility in POWER9 and newer processors. i.e., it is a Secure
> > Virtual Machine.
> > +
> > +What: /sys/devices/system/cpu/cpuX/purr
> > +Date: Apr 2005
> > +Contact: Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
> > +Description: PURR ticks for this CPU since the system boot.
> > +
> > + The Processor Utilization Resources Register (PURR) is
> > + a 64-bit counter which provides an estimate of the
> > + resources used by the CPU thread. The contents of this
> > + register increases monotonically. This sysfs interface
> > + exposes the number of PURR ticks for cpuX.
> > +
> > +What: /sys/devices/system/cpu/cpuX/spurr
> > +Date: Dec 2006
> > +Contact: Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
> > +Description: SPURR ticks for this CPU since the system boot.
> > +
> > + The Scaled Processor Utilization Resources Register
> > + (SPURR) is a 64-bit counter that provides a frequency
> > + invariant estimate of the resources used by the CPU
> > + thread. The contents of this register increases
> > + monotonically. This sysfs interface exposes the number
> > + of SPURR ticks for cpuX.
> > +
> > +What: /sys/devices/system/cpu/cpuX/idle_purr
> > +Date: Nov 2019
> > +Contact: Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
> > +Description: PURR ticks for cpuX when it was idle.
> > +
> > + This sysfs interface exposes the number of PURR ticks
> > + for cpuX when it was idle.
> > +
> > +What: /sys/devices/system/cpu/cpuX/spurr
>
> Copy-paste error? This should be:
Yes, this should have been idle_spurr. Will fix it in the next
version.
>
> /sys/devices/system/cpu/cpuX/idle_spurr
>
> > +Date: Nov 2019
>
> And I suppose Nov 2019 is no longer accurate.
My bad. I will resend this with the updated date.
>
>
> > +Contact: Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
> > +Description: SPURR ticks for cpuX when it was idle.
> > +
> > + This sysfs interface exposes the number of SPURR ticks
> > + for cpuX when it was idle.
> > --
> > 1.9.4
^ permalink raw reply
* Re: [PATCH v2 4/5] powerpc/sysfs: Show idle_purr and idle_spurr for every CPU
From: Gautham R Shenoy @ 2020-02-24 5:14 UTC (permalink / raw)
To: Nathan Lynch
Cc: Gautham R. Shenoy, Tyrel Datwyler, linux-kernel, Kamalesh Babulal,
Naveen N. Rao, Vaidyanathan Srinivasan, linuxppc-dev
In-Reply-To: <87eeunubp7.fsf@linux.ibm.com>
On Fri, Feb 21, 2020 at 10:50:12AM -0600, Nathan Lynch wrote:
> "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> writes:
> > diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> > index 80a676d..5b4b450 100644
> > --- a/arch/powerpc/kernel/sysfs.c
> > +++ b/arch/powerpc/kernel/sysfs.c
> > @@ -19,6 +19,7 @@
> > #include <asm/smp.h>
> > #include <asm/pmc.h>
> > #include <asm/firmware.h>
> > +#include <asm/idle.h>
> > #include <asm/svm.h>
> >
> > #include "cacheinfo.h"
> > @@ -733,6 +734,42 @@ static void create_svm_file(void)
> > }
> > #endif /* CONFIG_PPC_SVM */
> >
> > +static void read_idle_purr(void *val)
> > +{
> > + u64 *ret = (u64 *)val;
>
> No cast from void* needed.
Will fix this. Thanks.
>
>
> > +
> > + *ret = read_this_idle_purr();
> > +}
> > +
> > +static ssize_t idle_purr_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct cpu *cpu = container_of(dev, struct cpu, dev);
> > + u64 val;
> > +
> > + smp_call_function_single(cpu->dev.id, read_idle_purr, &val, 1);
> > + return sprintf(buf, "%llx\n", val);
> > +}
> > +static DEVICE_ATTR(idle_purr, 0400, idle_purr_show, NULL);
> > +
> > +static void read_idle_spurr(void *val)
> > +{
> > + u64 *ret = (u64 *)val;
> > +
> > + *ret = read_this_idle_spurr();
> > +}
> > +
> > +static ssize_t idle_spurr_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct cpu *cpu = container_of(dev, struct cpu, dev);
> > + u64 val;
> > +
> > + smp_call_function_single(cpu->dev.id, read_idle_spurr, &val, 1);
> > + return sprintf(buf, "%llx\n", val);
> > +}
> > +static DEVICE_ATTR(idle_spurr, 0400, idle_spurr_show, NULL);
>
> It's regrettable that we have to wake up potentially idle CPUs in order
> to derive correct idle statistics for them, but I suppose the main user
> (lparstat) of these interfaces already is causing this to happen by
> polling the existing per-cpu purr and spurr attributes.
>
> So now lparstat will incur at minimum four syscalls and four IPIs per
> CPU per polling interval -- one for each of purr, spurr, idle_purr and
> idle_spurr. Correct?
Yes, it is unforunate that we will end up making four syscalls and
generating IPI noise, and this is something that I discussed with
Naveen and Kamalesh. We have the following two constraints:
1) These values of PURR and SPURR required are per-cpu. Hence putting
them in lparcfg is not an option.
2) sysfs semantics encourages a single value per key, the key being
the sysfs-file. Something like the following would have made far more
sense.
cat /sys/devices/system/cpu/cpuX/purr_spurr_accounting
purr:A
idle_purr:B
spurr:C
idle_spurr:D
There are some sysfs files which allow something like this. Eg:
/sys/devices/system/cpu/cpu0/cpufreq/stats/time_in_state
Thoughts on any other alternatives?
>
> At some point it's going to make sense to batch sampling of remote CPUs'
> SPRs.
>
>
> > static int register_cpu_online(unsigned int cpu)
> > {
> > struct cpu *c = &per_cpu(cpu_devices, cpu);
> > @@ -794,10 +831,15 @@ static int register_cpu_online(unsigned int cpu)
> > if (!firmware_has_feature(FW_FEATURE_LPAR))
> > add_write_permission_dev_attr(&dev_attr_purr);
> > device_create_file(s, &dev_attr_purr);
> > + if (firmware_has_feature(FW_FEATURE_LPAR))
> > + device_create_file(s, &dev_attr_idle_purr);
> > }
> >
> > - if (cpu_has_feature(CPU_FTR_SPURR))
> > + if (cpu_has_feature(CPU_FTR_SPURR)) {
> > device_create_file(s, &dev_attr_spurr);
> > + if (firmware_has_feature(FW_FEATURE_LPAR))
> > + device_create_file(s, &dev_attr_idle_spurr);
> > + }
> >
> > if (cpu_has_feature(CPU_FTR_DSCR))
> > device_create_file(s, &dev_attr_dscr);
> > @@ -879,11 +921,17 @@ static int unregister_cpu_online(unsigned int cpu)
> > if (cpu_has_feature(CPU_FTR_MMCRA))
> > device_remove_file(s, &dev_attr_mmcra);
> >
> > - if (cpu_has_feature(CPU_FTR_PURR))
> > + if (cpu_has_feature(CPU_FTR_PURR)) {
> > device_remove_file(s, &dev_attr_purr);
> > + if (firmware_has_feature(FW_FEATURE_LPAR))
> > + device_remove_file(s, &dev_attr_idle_purr);
> > + }
> >
> > - if (cpu_has_feature(CPU_FTR_SPURR))
> > + if (cpu_has_feature(CPU_FTR_SPURR)) {
> > device_remove_file(s, &dev_attr_spurr);
> > + if (firmware_has_feature(FW_FEATURE_LPAR))
> > + device_remove_file(s, &dev_attr_idle_spurr);
> > + }
> >
> > if (cpu_has_feature(CPU_FTR_DSCR))
> > device_remove_file(s, &dev_attr_dscr);
>
> The cpu register/unregister stuff here looks correct.
Thanks for reviewing the patch.
--
Thanks and Regards
gautham.
^ permalink raw reply
* Re: [PATCH v2 3/5] powerpc/pseries: Account for SPURR ticks on idle CPUs
From: Gautham R Shenoy @ 2020-02-24 5:05 UTC (permalink / raw)
To: Nathan Lynch
Cc: Gautham R. Shenoy, Tyrel Datwyler, linux-kernel, Kamalesh Babulal,
Naveen N. Rao, Vaidyanathan Srinivasan, linuxppc-dev
In-Reply-To: <87ftf3ubte.fsf@linux.ibm.com>
Hello Nathan,
On Fri, Feb 21, 2020 at 10:47:41AM -0600, Nathan Lynch wrote:
> "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> writes:
> > +static inline void snapshot_spurr_idle_entry(void)
> > +{
> > + *this_cpu_ptr(&idle_entry_spurr_snap) = mfspr(SPRN_SPURR);
> > +}
> > +
>
> [...]
>
> > +static inline void update_idle_spurr_accounting(void)
> > +{
> > + u64 *idle_spurr_cycles_ptr = this_cpu_ptr(&idle_spurr_cycles);
> > + u64 in_spurr = *this_cpu_ptr(&idle_entry_spurr_snap);
> > +
> > + *idle_spurr_cycles_ptr += mfspr(SPRN_SPURR) - in_spurr;
> > +}
>
> [...]
>
> > +static inline u64 read_this_idle_spurr(void)
> > +{
> > + /*
> > + * If we are reading from an idle context, update the
> > + * idle-spurr cycles corresponding to the last idle period.
> > + * Since the idle context is not yet over, take a fresh
> > + * snapshot of the idle-spurr.
> > + */
> > + if (get_lppaca()->idle == 1) {
> > + update_idle_spurr_accounting();
> > + snapshot_spurr_idle_entry();
>
> This samples spurr twice when it could do with just one. I don't know
> the performance implications, but will the results be coherent?
We would have taken the snapshot in idle_loop_prolog() just before
entering idle. That fact that the "if" condition is true above in
read_this_idle_spurr() implies that we are reading the idle_spurr
value from an interrupt context and since get_lppaca()->idle == 1, we
haven't yet called idle_loop_epilog(), where we would have updated the
idle_spurr ticks for the last idle period.
Hence, in this function, we first update the idle_spurr accounting
from the time of the last snapshot to now. We update the snapshot to
the current SPURR value so that when we eventually call
idle_loop_epilog(), we will account for the remaining idle duration,
i.e from the read_this_idle_spurr() call to idle_loop_epilog()
The results are therefore coherant, in that we do not perform double
accounting the second time we invoke update_idle_spurr_accounting()
from idle_loop_epilog(), but only add the spurr ticks from
read_this_idle_spurr() to idle_loop_epilog().
--
Thanks and Regards
gautham.
^ permalink raw reply
* [PATCH V2 3/4] mm/vma: Replace all remaining open encodings with is_vm_hugetlb_page()
From: Anshuman Khandual @ 2020-02-24 5:03 UTC (permalink / raw)
To: linux-mm, linux-kernel
Cc: kvm-ppc, Aneesh Kumar K.V, linux-arch, Arnd Bergmann,
Anshuman Khandual, Peter Zijlstra, linuxppc-dev, Nick Piggin,
Arnaldo Carvalho de Melo, Ingo Molnar, Alexander Viro,
linux-fsdevel, Andrew Morton, Will Deacon
In-Reply-To: <1582520593-30704-1-git-send-email-anshuman.khandual@arm.com>
This replaces all remaining open encodings with is_vm_hugetlb_page().
Cc: Paul Mackerras <paulus@ozlabs.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Will Deacon <will@kernel.org>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Nick Piggin <npiggin@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-mm@kvack.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
arch/powerpc/kvm/e500_mmu_host.c | 2 +-
fs/binfmt_elf.c | 3 ++-
include/asm-generic/tlb.h | 3 ++-
kernel/events/core.c | 3 ++-
4 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 425d13806645..df9989cf7ba3 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -422,7 +422,7 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
break;
}
} else if (vma && hva >= vma->vm_start &&
- (vma->vm_flags & VM_HUGETLB)) {
+ is_vm_hugetlb_page(vma)) {
unsigned long psize = vma_kernel_pagesize(vma);
tsize = (gtlbe->mas1 & MAS1_TSIZE_MASK) >>
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index f4713ea76e82..1eb63867e266 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -27,6 +27,7 @@
#include <linux/highuid.h>
#include <linux/compiler.h>
#include <linux/highmem.h>
+#include <linux/hugetlb.h>
#include <linux/pagemap.h>
#include <linux/vmalloc.h>
#include <linux/security.h>
@@ -1317,7 +1318,7 @@ static unsigned long vma_dump_size(struct vm_area_struct *vma,
}
/* Hugetlb memory check */
- if (vma->vm_flags & VM_HUGETLB) {
+ if (is_vm_hugetlb_page(vma)) {
if ((vma->vm_flags & VM_SHARED) && FILTER(HUGETLB_SHARED))
goto whole;
if (!(vma->vm_flags & VM_SHARED) && FILTER(HUGETLB_PRIVATE))
diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index f391f6b500b4..3f1649a8cf55 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -13,6 +13,7 @@
#include <linux/mmu_notifier.h>
#include <linux/swap.h>
+#include <linux/hugetlb_inline.h>
#include <asm/pgalloc.h>
#include <asm/tlbflush.h>
#include <asm/cacheflush.h>
@@ -398,7 +399,7 @@ tlb_update_vma_flags(struct mmu_gather *tlb, struct vm_area_struct *vma)
* We rely on tlb_end_vma() to issue a flush, such that when we reset
* these values the batch is empty.
*/
- tlb->vma_huge = !!(vma->vm_flags & VM_HUGETLB);
+ tlb->vma_huge = is_vm_hugetlb_page(vma);
tlb->vma_exec = !!(vma->vm_flags & VM_EXEC);
}
diff --git a/kernel/events/core.c b/kernel/events/core.c
index e453589da97c..ef5be3ed0580 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -28,6 +28,7 @@
#include <linux/export.h>
#include <linux/vmalloc.h>
#include <linux/hardirq.h>
+#include <linux/hugetlb.h>
#include <linux/rculist.h>
#include <linux/uaccess.h>
#include <linux/syscalls.h>
@@ -7693,7 +7694,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
flags |= MAP_EXECUTABLE;
if (vma->vm_flags & VM_LOCKED)
flags |= MAP_LOCKED;
- if (vma->vm_flags & VM_HUGETLB)
+ if (is_vm_hugetlb_page(vma))
flags |= MAP_HUGETLB;
if (file) {
--
2.20.1
^ permalink raw reply related
* [PATCH V2 2/4] mm/vma: Make vma_is_accessible() available for general use
From: Anshuman Khandual @ 2020-02-24 5:03 UTC (permalink / raw)
To: linux-mm, linux-kernel
Cc: Rich Felker, linux-sh, Peter Zijlstra, Dave Hansen, Guo Ren,
Yoshinori Sato, Ingo Molnar, Geert Uytterhoeven, Mel Gorman,
Anshuman Khandual, Steven Rostedt, linux-m68k, Andy Lutomirski,
Thomas Gleixner, Paul Burton, linux-mips, Ralf Baechle,
Paul Mackerras, Andrew Morton, linuxppc-dev
In-Reply-To: <1582520593-30704-1-git-send-email-anshuman.khandual@arm.com>
Lets move vma_is_accessible() helper to include/linux/mm.h which makes it
available for general use. While here, this replaces all remaining open
encodings for VMA access check with vma_is_accessible().
Cc: Guo Ren <guoren@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Mel Gorman <mgorman@suse.de>
Cc: linux-kernel@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Cc: linux-mips@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-sh@vger.kernel.org
Cc: linux-mm@kvack.org
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Acked-by: Guo Ren <guoren@kernel.org>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
arch/csky/mm/fault.c | 2 +-
arch/m68k/mm/fault.c | 2 +-
arch/mips/mm/fault.c | 2 +-
arch/powerpc/mm/fault.c | 2 +-
arch/sh/mm/fault.c | 2 +-
arch/x86/mm/fault.c | 2 +-
include/linux/mm.h | 5 +++++
kernel/sched/fair.c | 2 +-
mm/gup.c | 2 +-
mm/memory.c | 5 -----
mm/mempolicy.c | 3 +--
mm/mmap.c | 5 ++---
12 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/arch/csky/mm/fault.c b/arch/csky/mm/fault.c
index f76618b630f9..4b3511b8298d 100644
--- a/arch/csky/mm/fault.c
+++ b/arch/csky/mm/fault.c
@@ -137,7 +137,7 @@ asmlinkage void do_page_fault(struct pt_regs *regs, unsigned long write,
if (!(vma->vm_flags & VM_WRITE))
goto bad_area;
} else {
- if (!(vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)))
+ if (!vma_is_accessible(vma))
goto bad_area;
}
diff --git a/arch/m68k/mm/fault.c b/arch/m68k/mm/fault.c
index e9b1d7585b43..d5131ec5d923 100644
--- a/arch/m68k/mm/fault.c
+++ b/arch/m68k/mm/fault.c
@@ -125,7 +125,7 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
case 1: /* read, present */
goto acc_err;
case 0: /* read, not present */
- if (!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)))
+ if (!vma_is_accessible(vma))
goto acc_err;
}
diff --git a/arch/mips/mm/fault.c b/arch/mips/mm/fault.c
index 1e8d00793784..5b9f947bfa32 100644
--- a/arch/mips/mm/fault.c
+++ b/arch/mips/mm/fault.c
@@ -142,7 +142,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write,
goto bad_area;
}
} else {
- if (!(vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)))
+ if (!vma_is_accessible(vma))
goto bad_area;
}
}
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 8db0507619e2..71a3658c516b 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -314,7 +314,7 @@ static bool access_error(bool is_write, bool is_exec,
return false;
}
- if (unlikely(!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE))))
+ if (unlikely(!vma_is_accessible(vma)))
return true;
/*
* We should ideally do the vma pkey access check here. But in the
diff --git a/arch/sh/mm/fault.c b/arch/sh/mm/fault.c
index 5f51456f4fc7..a8c4253f37d7 100644
--- a/arch/sh/mm/fault.c
+++ b/arch/sh/mm/fault.c
@@ -355,7 +355,7 @@ static inline int access_error(int error_code, struct vm_area_struct *vma)
return 1;
/* read, not present: */
- if (unlikely(!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE))))
+ if (unlikely(!vma_is_accessible(vma)))
return 1;
return 0;
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index fa4ea09593ab..c461eaab0306 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1200,7 +1200,7 @@ access_error(unsigned long error_code, struct vm_area_struct *vma)
return 1;
/* read, not present: */
- if (unlikely(!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE))))
+ if (unlikely(!vma_is_accessible(vma)))
return 1;
return 0;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 52269e56c514..b0e53ef13ff1 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -541,6 +541,11 @@ static inline bool vma_is_anonymous(struct vm_area_struct *vma)
return !vma->vm_ops;
}
+static inline bool vma_is_accessible(struct vm_area_struct *vma)
+{
+ return vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
+}
+
#ifdef CONFIG_SHMEM
/*
* The vma_is_shmem is not inline because it is used only by slow
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 3c8a379c357e..bf15cc72695e 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2573,7 +2573,7 @@ static void task_numa_work(struct callback_head *work)
* Skip inaccessible VMAs to avoid any confusion between
* PROT_NONE and NUMA hinting ptes
*/
- if (!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)))
+ if (!vma_is_accessible(vma))
continue;
do {
diff --git a/mm/gup.c b/mm/gup.c
index 1b521e0ac1de..c8ffe2e61f03 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1171,7 +1171,7 @@ long populate_vma_page_range(struct vm_area_struct *vma,
* We want mlock to succeed for regions that have any permissions
* other than PROT_NONE.
*/
- if (vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC))
+ if (vma_is_accessible(vma))
gup_flags |= FOLL_FORCE;
/*
diff --git a/mm/memory.c b/mm/memory.c
index 0bccc622e482..2f07747612b7 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3942,11 +3942,6 @@ static inline vm_fault_t wp_huge_pmd(struct vm_fault *vmf, pmd_t orig_pmd)
return VM_FAULT_FALLBACK;
}
-static inline bool vma_is_accessible(struct vm_area_struct *vma)
-{
- return vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE);
-}
-
static vm_fault_t create_huge_pud(struct vm_fault *vmf)
{
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 977c641f78cf..91c1ad6ab8ea 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -649,8 +649,7 @@ static int queue_pages_test_walk(unsigned long start, unsigned long end,
if (flags & MPOL_MF_LAZY) {
/* Similar to task_numa_work, skip inaccessible VMAs */
- if (!is_vm_hugetlb_page(vma) &&
- (vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)) &&
+ if (!is_vm_hugetlb_page(vma) && vma_is_accessible(vma) &&
!(vma->vm_flags & VM_MIXEDMAP))
change_prot_numa(vma, start, endvma);
return 1;
diff --git a/mm/mmap.c b/mm/mmap.c
index d681a20eb4ea..0d295f49b24d 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2334,8 +2334,7 @@ int expand_upwards(struct vm_area_struct *vma, unsigned long address)
gap_addr = TASK_SIZE;
next = vma->vm_next;
- if (next && next->vm_start < gap_addr &&
- (next->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
+ if (next && next->vm_start < gap_addr && vma_is_accessible(next)) {
if (!(next->vm_flags & VM_GROWSUP))
return -ENOMEM;
/* Check that both stack segments have the same anon_vma? */
@@ -2416,7 +2415,7 @@ int expand_downwards(struct vm_area_struct *vma,
prev = vma->vm_prev;
/* Check that both stack segments have the same anon_vma? */
if (prev && !(prev->vm_flags & VM_GROWSDOWN) &&
- (prev->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
+ vma_is_accessible(prev)) {
if (address - prev->vm_end < stack_guard_gap)
return -ENOMEM;
}
--
2.20.1
^ permalink raw reply related
* [PATCH V2 0/4] mm/vma: Use all available wrappers when possible
From: Anshuman Khandual @ 2020-02-24 5:03 UTC (permalink / raw)
To: linux-mm, linux-kernel
Cc: linux-arch, linux-sh, Anshuman Khandual, linux-mips, kvm-ppc,
linux-m68k, linux-fsdevel, linuxppc-dev
Apart from adding a VMA flag readable name for trace purpose, this series
does some open encoding replacements with availabe VMA specific wrappers.
This skips VM_HUGETLB check in vma_migratable() as its already being done
with another patch (https://patchwork.kernel.org/patch/11347831/) which
is yet to be merged.
This series applies on 5.6-rc3. This has been build tested on multiple
platforms, though boot and runtime testing was limited to arm64 and x86.
Cc: linux-kernel@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Cc: linux-mips@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-sh@vger.kernel.org
Cc: kvm-ppc@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-mm@kvack.org
Changes in V2:
- Dropped patch [PATCH 4/5] mm/vma: Replace....vma_set_anonymous() per Kirril
- Dropped braces around is_vm_hugetlb_page() in kvmppc_e500_shadow_map()
- Replaced two open encodings in mm/mmap.c with vma_is_accessible()
- Added hugetlb headers to prevent build failures wrt is_vm_hugetlb_page()
Changes in V1: (https://patchwork.kernel.org/cover/11385219/)
Anshuman Khandual (4):
mm/vma: Add missing VMA flag readable name for VM_SYNC
mm/vma: Make vma_is_accessible() available for general use
mm/vma: Replace all remaining open encodings with is_vm_hugetlb_page()
mm/vma: Replace all remaining open encodings with vma_is_anonymous()
arch/csky/mm/fault.c | 2 +-
arch/m68k/mm/fault.c | 2 +-
arch/mips/mm/fault.c | 2 +-
arch/powerpc/kvm/e500_mmu_host.c | 2 +-
arch/powerpc/mm/fault.c | 2 +-
arch/sh/mm/fault.c | 2 +-
arch/x86/mm/fault.c | 2 +-
fs/binfmt_elf.c | 3 ++-
include/asm-generic/tlb.h | 3 ++-
include/linux/mm.h | 5 +++++
include/trace/events/mmflags.h | 1 +
kernel/events/core.c | 3 ++-
kernel/sched/fair.c | 2 +-
mm/gup.c | 5 +++--
mm/memory.c | 5 -----
mm/mempolicy.c | 3 +--
mm/mmap.c | 5 ++---
17 files changed, 26 insertions(+), 23 deletions(-)
--
2.20.1
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox