* Re: use generic DMA mapping code in powerpc V4
From: Christoph Hellwig @ 2019-02-12 19:52 UTC (permalink / raw)
To: Christian Zigotzky
Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <f57e6d8f-d43f-e9f8-b092-e43f5019f86f@xenosoft.de>
Great! I owe you a night worth of beers at a conference or if
you come anywhere near Innsbruck!
^ permalink raw reply
* Re: [PATCH v2 2/2] locking/rwsem: Optimize down_read_trylock()
From: Linus Torvalds @ 2019-02-12 19:58 UTC (permalink / raw)
To: Waiman Long
Cc: linux-arch, linux-xtensa, Davidlohr Bueso, linux-ia64, Tim Chen,
Arnd Bergmann, Linux-sh list, Peter Zijlstra, linux-hexagon,
the arch/x86 maintainers, Will Deacon, Linux List Kernel Mailing,
Ingo Molnar, Borislav Petkov, H. Peter Anvin, linux-alpha,
sparclinux, Thomas Gleixner, linuxppc-dev, Andrew Morton,
linux-arm-kernel
In-Reply-To: <1549913486-16799-3-git-send-email-longman@redhat.com>
On Mon, Feb 11, 2019 at 11:31 AM Waiman Long <longman@redhat.com> wrote:
>
> Modify __down_read_trylock() to make it generate slightly better code
> (smaller and maybe a tiny bit faster).
This looks good, but I would ask you to try one slightly different approach.
Instead of this:
> long tmp = atomic_long_read(&sem->count);
>
> while (tmp >= 0) {
> if (atomic_long_try_cmpxchg_acquire(&sem->count, &tmp,
> tmp + RWSEM_ACTIVE_READ_BIAS)) {
> return 1;
> }
> }
try doing this instead:
long tmp = 0;
do {
if (atomic_long_try_cmpxchg_acquire(&sem->count, &tmp,
tmp + RWSEM_ACTIVE_READ_BIAS)) {
return 1;
} while (tmp >= 0);
return 0;
because especially when it comes to locking, it's usually better to
just *guess* that the lock is unlocked, than it is to actually read
from the line to see what the state is.
Often - but certainly not always - the lock is the first access to the
target cacheline, and assuming the trylock is successful (which I
think is the case we want to optimize for), we're much better off
causing that first access to be a read-for-ownership, rather than a
read-for-sharing.
Because if you first read from the line, and then do a cmpxchg, and if
the line was not in the cache, your cache coherency protocol will
generally go through two states: first shared (for the initial read)
and then exclusive-dirty (for the cmpxchg).
Now, this is obviously very micro-architecture dependent, and in fact
the microarchitecture could even see the "predict fallthrough to a
cmpxchg with the same address" and turn the first read into a
read-for-ownership, but we've done this at some point before, and the
"guess unlocked" was actually the one that performed better.
Of course, the downside is that it might be worse when the guess is
incorrect - either because of a nested read lock or due to an actual
conflict with a write, but on the whole those *should* be the rare
cases, and not the cases where we necessarily optimize for latency of
the operation.
Hmm?
Linus
^ permalink raw reply
* Re: [PATCH 04/12] of: select OF_RESERVED_MEM automatically
From: Rob Herring @ 2019-02-12 20:11 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-xtensa, Lee Jones, SH-Linux, Greg Kroah-Hartman, x86,
linux-mips, linux-kernel@vger.kernel.org, Linux IOMMU,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-riscv, arcml, linuxppc-dev, devicetree
In-Reply-To: <20190211133554.30055-5-hch@lst.de>
On Mon, Feb 11, 2019 at 7:37 AM Christoph Hellwig <hch@lst.de> wrote:
>
> The OF_RESERVED_MEM can be used if we have either CMA or the generic
> declare coherent code built and we support the early flattened DT.
>
> So don't bother making it a user visible options that is selected
> by most configs that fit the above category, but just select it when
> the requirements are met.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/arc/Kconfig | 1 -
> arch/arm/Kconfig | 1 -
> arch/arm64/Kconfig | 1 -
> arch/csky/Kconfig | 1 -
> arch/powerpc/Kconfig | 1 -
> arch/xtensa/Kconfig | 1 -
> drivers/of/Kconfig | 5 ++---
> 7 files changed, 2 insertions(+), 9 deletions(-)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 03/12] of: mark early_init_dt_alloc_reserved_memory_arch static
From: Rob Herring @ 2019-02-12 20:24 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-xtensa, linuxppc-dev, SH-Linux, devicetree,
Greg Kroah-Hartman, x86, linux-mips, linux-kernel@vger.kernel.org,
Linux IOMMU, linux-riscv, arcml, Lee Jones,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20190211133554.30055-4-hch@lst.de>
On Mon, Feb 11, 2019 at 7:36 AM Christoph Hellwig <hch@lst.de> wrote:
>
> This function is only used in of_reserved_mem.c, and never overridden
> despite the __weak marker.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/of/of_reserved_mem.c | 2 +-
> include/linux/of_reserved_mem.h | 7 -------
> 2 files changed, 1 insertion(+), 8 deletions(-)
Reviewed-by: Rob Herring <robh@kernel.org>
Looks like this one isn't a dependency, so I can take it if you want.
Rob
^ permalink raw reply
* Re: [PATCH 06/12] dma-mapping: improve selection of dma_declare_coherent availability
From: Rob Herring @ 2019-02-12 20:40 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-xtensa, linuxppc-dev, SH-Linux, devicetree,
Greg Kroah-Hartman, x86, linux-mips, linux-kernel@vger.kernel.org,
Linux IOMMU, linux-riscv, arcml, Lee Jones,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20190211133554.30055-7-hch@lst.de>
On Mon, Feb 11, 2019 at 7:37 AM Christoph Hellwig <hch@lst.de> wrote:
>
> This API is primarily used through DT entries, but two architectures
> and two drivers call it directly. So instead of selecting the config
> symbol for random architectures pull it in implicitly for the actual
> users. Also rename the Kconfig option to describe the feature better.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/arc/Kconfig | 1 -
> arch/arm/Kconfig | 2 +-
> arch/arm64/Kconfig | 1 -
> arch/csky/Kconfig | 1 -
> arch/mips/Kconfig | 1 -
> arch/riscv/Kconfig | 1 -
> arch/sh/Kconfig | 2 +-
> arch/unicore32/Kconfig | 1 -
> arch/x86/Kconfig | 1 -
> drivers/mfd/Kconfig | 2 ++
> drivers/of/Kconfig | 3 ++-
> include/linux/device.h | 2 +-
> include/linux/dma-mapping.h | 8 ++++----
> kernel/dma/Kconfig | 2 +-
> kernel/dma/Makefile | 2 +-
> 15 files changed, 13 insertions(+), 17 deletions(-)
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index 3607fd2810e4..f8c66a9472a4 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -43,6 +43,7 @@ config OF_FLATTREE
>
> config OF_EARLY_FLATTREE
> bool
> + select DMA_DECLARE_COHERENT
Is selecting DMA_DECLARE_COHERENT okay on UML? We run the unittests with UML.
Maybe we should just get rid of OF_RESERVED_MEM. If we support booting
from DT, then it should always be enabled anyways.
> select OF_FLATTREE
>
> config OF_PROMTREE
> @@ -83,7 +84,7 @@ config OF_MDIO
> config OF_RESERVED_MEM
> bool
> depends on OF_EARLY_FLATTREE
> - default y if HAVE_GENERIC_DMA_COHERENT || DMA_CMA
> + default y if DMA_DECLARE_COHERENT || DMA_CMA
>
> config OF_RESOLVE
> bool
^ permalink raw reply
* Re: [PATCH kernel] vfio/spapr_tce: Skip unsetting already unset table
From: Alex Williamson @ 2019-02-12 20:52 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: linuxppc-dev, kvm, kvm-ppc, David Gibson
In-Reply-To: <20190211074917.125723-1-aik@ozlabs.ru>
On Mon, 11 Feb 2019 18:49:17 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> VFIO TCE IOMMU v2 owns IOMMU tables so when detach a IOMMU group from
> a container, we need to unset those from a group so we call unset_window()
> so do we unconditionally. We also unset tables when removing a DMA window
Patch looks ok, but this first sentence trails off into a bit of a word
salad. Care to refine a bit? Thanks,
Alex
> via the VFIO_IOMMU_SPAPR_TCE_REMOVE ioctl.
>
> The window removal checks if the table actually exists (hidden inside
> tce_iommu_find_table()) but the group detaching does not so the user
> may see duplicating messages:
> pci 0009:03 : [PE# fd] Removing DMA window #0
> pci 0009:03 : [PE# fd] Removing DMA window #1
> pci 0009:03 : [PE# fd] Removing DMA window #0
> pci 0009:03 : [PE# fd] Removing DMA window #1
>
> At the moment this is not a problem as the second invocation
> of unset_window() writes zeroes to the HW registers again and exits early
> as there is no table.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>
> When doing VFIO PCI hot unplug, first we remove the DMA window and
> set container->tables[num] - this is a first couple of messages.
> Then we detach the group and then we see another couple of the same
> messages which confused myself.
> ---
> drivers/vfio/vfio_iommu_spapr_tce.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
> index c424913..8dbb270 100644
> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -1235,7 +1235,8 @@ static void tce_iommu_release_ownership_ddw(struct tce_container *container,
> }
>
> for (i = 0; i < IOMMU_TABLE_GROUP_MAX_TABLES; ++i)
> - table_group->ops->unset_window(table_group, i);
> + if (container->tables[i])
> + table_group->ops->unset_window(table_group, i);
>
> table_group->ops->release_ownership(table_group);
> }
^ permalink raw reply
* Re: [PATCH v2 2/2] locking/rwsem: Optimize down_read_trylock()
From: Waiman Long @ 2019-02-12 21:21 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-arch, linux-xtensa, Davidlohr Bueso, linux-ia64, Tim Chen,
Arnd Bergmann, Linux-sh list, Peter Zijlstra, linux-hexagon,
the arch/x86 maintainers, Will Deacon, Linux List Kernel Mailing,
Ingo Molnar, Borislav Petkov, H. Peter Anvin, linux-alpha,
sparclinux, Thomas Gleixner, linuxppc-dev, Andrew Morton,
linux-arm-kernel
In-Reply-To: <CAHk-=wjTmSg6zX=xKFSPZor7FWi3s4D0dy-hz_M=yeF_6478QA@mail.gmail.com>
On 02/12/2019 02:58 PM, Linus Torvalds wrote:
> On Mon, Feb 11, 2019 at 11:31 AM Waiman Long <longman@redhat.com> wrote:
>> Modify __down_read_trylock() to make it generate slightly better code
>> (smaller and maybe a tiny bit faster).
> This looks good, but I would ask you to try one slightly different approach.
>
> Instead of this:
>
>> long tmp = atomic_long_read(&sem->count);
>>
>> while (tmp >= 0) {
>> if (atomic_long_try_cmpxchg_acquire(&sem->count, &tmp,
>> tmp + RWSEM_ACTIVE_READ_BIAS)) {
>> return 1;
>> }
>> }
> try doing this instead:
>
> long tmp = 0;
>
> do {
> if (atomic_long_try_cmpxchg_acquire(&sem->count, &tmp,
> tmp + RWSEM_ACTIVE_READ_BIAS)) {
> return 1;
> } while (tmp >= 0);
> return 0;
>
> because especially when it comes to locking, it's usually better to
> just *guess* that the lock is unlocked, than it is to actually read
> from the line to see what the state is.
>
> Often - but certainly not always - the lock is the first access to the
> target cacheline, and assuming the trylock is successful (which I
> think is the case we want to optimize for), we're much better off
> causing that first access to be a read-for-ownership, rather than a
> read-for-sharing.
>
> Because if you first read from the line, and then do a cmpxchg, and if
> the line was not in the cache, your cache coherency protocol will
> generally go through two states: first shared (for the initial read)
> and then exclusive-dirty (for the cmpxchg).
>
> Now, this is obviously very micro-architecture dependent, and in fact
> the microarchitecture could even see the "predict fallthrough to a
> cmpxchg with the same address" and turn the first read into a
> read-for-ownership, but we've done this at some point before, and the
> "guess unlocked" was actually the one that performed better.
>
> Of course, the downside is that it might be worse when the guess is
> incorrect - either because of a nested read lock or due to an actual
> conflict with a write, but on the whole those *should* be the rare
> cases, and not the cases where we necessarily optimize for latency of
> the operation.
>
> Hmm?
I looked at the assembly code in arch/x86/include/asm/rwsem.h. For both
trylocks (read & write), the count is read first before attempting to
lock it. We did the same for all trylock functions in other locks.
Depending on how the trylock is used and how contended the lock is, it
may help or hurt performance. Changing down_read_trylock to do an
unconditional cmpxchg will change the performance profile of existing
code. So I would prefer keeping the current code.
I do notice now that the generic down_write_trylock() code is doing an
unconditional compxchg. So I wonder if we should change it to read the
lock first like other trylocks or just leave it as it is.
Cheers,
Longman
^ permalink raw reply
* Re: [PATCH] powerpc/85xx: Activate Cyrus disk-activity LED
From: Scott Wood @ 2019-02-12 21:35 UTC (permalink / raw)
To: Darren Stevens, linuxppc-dev; +Cc: chzigotzky
In-Reply-To: <4d531fb8828.51b92386@auth.smtp.1and1.co.uk>
On Sat, 2019-02-09 at 23:14 +0000, Darren Stevens wrote:
> The disk activity LED on the Cyrus board is attached to a gpio pin,
> add the required device-tree node for the kernel driver to use.
>
> Signed-off-by: Darren Stevens <darren@stevens-zone.net>
>
> ---
>
> diff --git a/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
> b/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
> index 15e8440..348cfdb 100644
> --- a/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
> +++ b/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
> @@ -82,6 +82,15 @@
> gpios = <&gpio0 2 1>;
> };
>
> + leds {
> + compatible = "gpio-leds";
> +
> + hdd {
> + label = "Disk Activity";
> + gpios = <&gpio0 5 0>;
> + linux,default-trigger = "disk-activity";
> + };
> + };
> };
This patch is whitespace-mangled (as is your other one). Could you try
sending with git send-email?
-Scott
^ permalink raw reply
* Re: [PATCH 06/19] KVM: PPC: Book3S HV: add a GET_ESB_FD control to the XIVE native device
From: Cédric Le Goater @ 2019-02-12 22:07 UTC (permalink / raw)
To: Benjamin Herrenschmidt, David Gibson; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <9dc53c6bccd6fd303481217f768ec173992431e2.camel@kernel.crashing.org>
On 2/11/19 7:42 AM, Benjamin Herrenschmidt wrote:
> On Mon, 2019-02-11 at 13:38 +1100, David Gibson wrote:
>>
>> 1) All in kernel
>>
>> The offset always maps directly to guest irq number and the kernel
>> somehow binds it either to an IPI or a host irq as necessary.
>> Cédric's original code attempts this, but the mechanism of keeping a
>> pointer to the VMA can't work.
>
> Why do you need a pointer to the VMA anyway ? unmap_mapping_range()
> doesn't need a VMA for the unmap part, and faults/mmaps have the VMA.
>
>> But.. remapping the irqs should be sufficiently infrequent that it
>> might be ok to consider simply stepping through all the hosting
>> process's VMAs to do this.
>
> Which unmap_mapping_range() does for you as I explained previously. You
> only need the address space. See how spufs does it (among others).
and the different CAPI drivers. This is much better and it works fine.
On the same topic, the XIVE IC on P10 will use IPI ESB pages for the
PHB interrupts sources. We will still need this kind of remapping but
the pages will be from the same controller.
Thanks,
C.
^ permalink raw reply
* [PATCH] powerpc/mm: Check secondary hash page table
From: Rashmica Gupta @ 2019-02-12 23:29 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Rashmica Gupta, paulus
We were always calling base_hpte_find() with primary = true,
even when we wanted to check the secondary table...
Fixes: 1515ab932156 powerpc/mm: Dump hash table
Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
---
Thanks Paul for spotting this :)
arch/powerpc/mm/dump_hashpagetable.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/dump_hashpagetable.c b/arch/powerpc/mm/dump_hashpagetable.c
index 869294695048..b430e4e08af6 100644
--- a/arch/powerpc/mm/dump_hashpagetable.c
+++ b/arch/powerpc/mm/dump_hashpagetable.c
@@ -342,7 +342,7 @@ static unsigned long hpte_find(struct pg_state *st, unsigned long ea, int psize)
/* Look in secondary table */
if (slot == -1)
- slot = base_hpte_find(ea, psize, true, &v, &r);
+ slot = base_hpte_find(ea, psize, false, &v, &r);
/* No entry found */
if (slot == -1)
--
2.17.2
^ permalink raw reply related
* Re: [PATCH kernel] powerpc/powernv/ioda: Store correct amount of memory used for table
From: Alexey Kardashevskiy @ 2019-02-12 23:57 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <9e1930e2-3cf8-1233-4366-1c1f64cd3c2a@ozlabs.ru>
On 12/02/2019 18:33, Alexey Kardashevskiy wrote:
>
>
> On 12/02/2019 11:20, David Gibson wrote:
>> On Mon, Feb 11, 2019 at 06:48:01PM +1100, Alexey Kardashevskiy wrote:
>>> We store 2 multilevel tables in iommu_table - one for the hardware and
>>> one with the corresponding userspace addresses. Before allocating
>>> the tables, the iommu_table_group_ops::get_table_size() hook returns
>>> the combined size of the two and VFIO SPAPR TCE IOMMU driver adjusts
>>> the locked_vm counter correctly. When the table is actually allocated,
>>> the amount of allocated memory is stored in iommu_table::it_allocated_size
>>> and used to adjust the locked_vm counter when we release the memory used
>>> by the table; .get_table_size() and .create_table() calculate it
>>> independently but the result is expected to be the same.
>>
>> Any way we can remove that redundant calculation? That seems like
>> begging for bugs.
>
>
> I do not see an easy way. One way could be adding a "dryrun" flag to
> pnv_pci_ioda2_table_alloc_pages(), count allocated memory there and call
> it from .get_table_size() but for multilevel TCEs it only allocates
> first level...
byyyyyy the way even this it_allocated_size is buggy as it is what was
actually allocated so it does not include any indirect levels which
might be allocated later although it should.
I'll simply make it tbl->it_allocated_size=.get_table_size() or just
ditch it_allocated_size, let me see...
>
>
>>> Unfortunately the allocator does not add the userspace table size to
>>> ::it_allocated_size so when we destroy the table because of VFIO PCI
>>> unplug (i.e. VFIO container is gone but the userspace keeps running),
>>> we decrement locked_vm by just a half of size of memory we are releasing.
>>> As the result, we leak locked_vm and may not be able to allocate more
>>> IOMMU tables after few iterations of hotplug/unplug.
>>>
>>> This adjusts it_allocated_size if the userspace addresses table was
>>> requested (total_allocated_uas is initialized by zero).
>>>
>>> Fixes: 090bad39b "powerpc/powernv: Add indirect levels to it_userspace"
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>
>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>>
>>> ---
>>> arch/powerpc/platforms/powernv/pci-ioda-tce.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda-tce.c b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
>>> index 697449a..58146e1 100644
>>> --- a/arch/powerpc/platforms/powernv/pci-ioda-tce.c
>>> +++ b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
>>> @@ -313,7 +313,7 @@ long pnv_pci_ioda2_table_alloc_pages(int nid, __u64 bus_offset,
>>> page_shift);
>>> tbl->it_level_size = 1ULL << (level_shift - 3);
>>> tbl->it_indirect_levels = levels - 1;
>>> - tbl->it_allocated_size = total_allocated;
>>> + tbl->it_allocated_size = total_allocated + total_allocated_uas;
>>> tbl->it_userspace = uas;
>>> tbl->it_nid = nid;
>>>
>>
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 2/2] mm: be more verbose about zonelist initialization
From: kbuild test robot @ 2019-02-13 0:12 UTC (permalink / raw)
To: Michal Hocko
Cc: Tony Luck, linux-ia64, Dave Hansen, Peter Zijlstra, x86, LKML,
Pingfan Liu, linux-mm, Michal Hocko, kbuild-all, Ingo Molnar,
linuxppc-dev
In-Reply-To: <20190212095343.23315-3-mhocko@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 5852 bytes --]
Hi Michal,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.0-rc4 next-20190212]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Michal-Hocko/x86-numa-always-initialize-all-possible-nodes/20190213-071628
config: x86_64-randconfig-x016-201906 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
In file included from include/linux/gfp.h:6,
from include/linux/mm.h:10,
from mm/page_alloc.c:18:
mm/page_alloc.c: In function 'build_zonelists':
>> mm/page_alloc.c:5423:31: error: 'z' undeclared (first use in this function)
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^
include/linux/mmzone.h:1036:7: note: in definition of macro 'for_each_zone_zonelist_nodemask'
for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
^
mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^~~~~~~~~~~~~~~~~~~~~~
mm/page_alloc.c:5423:31: note: each undeclared identifier is reported only once for each function it appears in
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^
include/linux/mmzone.h:1036:7: note: in definition of macro 'for_each_zone_zonelist_nodemask'
for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
^
mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^~~~~~~~~~~~~~~~~~~~~~
>> mm/page_alloc.c:5423:25: error: 'zone' undeclared (first use in this function)
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^~~~
include/linux/mmzone.h:1036:59: note: in definition of macro 'for_each_zone_zonelist_nodemask'
for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
^~~~
mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^~~~~~~~~~~~~~~~~~~~~~
include/linux/mmzone.h:1036:57: warning: left-hand operand of comma expression has no effect [-Wunused-value]
for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
^
include/linux/mmzone.h:1058:2: note: in expansion of macro 'for_each_zone_zonelist_nodemask'
for_each_zone_zonelist_nodemask(zone, z, zlist, highidx, NULL)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^~~~~~~~~~~~~~~~~~~~~~
include/linux/mmzone.h:1038:50: warning: left-hand operand of comma expression has no effect [-Wunused-value]
z = next_zones_zonelist(++z, highidx, nodemask), \
^
include/linux/mmzone.h:1058:2: note: in expansion of macro 'for_each_zone_zonelist_nodemask'
for_each_zone_zonelist_nodemask(zone, z, zlist, highidx, NULL)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^~~~~~~~~~~~~~~~~~~~~~
vim +/z +5423 mm/page_alloc.c
5382
5383 /*
5384 * Build zonelists ordered by zone and nodes within zones.
5385 * This results in conserving DMA zone[s] until all Normal memory is
5386 * exhausted, but results in overflowing to remote node while memory
5387 * may still exist in local DMA zone.
5388 */
5389
5390 static void build_zonelists(pg_data_t *pgdat)
5391 {
5392 static int node_order[MAX_NUMNODES];
5393 int node, load, nr_nodes = 0;
5394 nodemask_t used_mask;
5395 int local_node, prev_node;
5396
5397 /* NUMA-aware ordering of nodes */
5398 local_node = pgdat->node_id;
5399 load = nr_online_nodes;
5400 prev_node = local_node;
5401 nodes_clear(used_mask);
5402
5403 memset(node_order, 0, sizeof(node_order));
5404 while ((node = find_next_best_node(local_node, &used_mask)) >= 0) {
5405 /*
5406 * We don't want to pressure a particular node.
5407 * So adding penalty to the first node in same
5408 * distance group to make it round-robin.
5409 */
5410 if (node_distance(local_node, node) !=
5411 node_distance(local_node, prev_node))
5412 node_load[node] = load;
5413
5414 node_order[nr_nodes++] = node;
5415 prev_node = node;
5416 load--;
5417 }
5418
5419 build_zonelists_in_node_order(pgdat, node_order, nr_nodes);
5420 build_thisnode_zonelists(pgdat);
5421
5422 pr_info("node[%d] zonelist: ", pgdat->node_id);
> 5423 for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
5424 pr_cont("%d:%s ", zone_to_nid(zone), zone->name);
5425 pr_cont("\n");
5426 }
5427
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26045 bytes --]
^ permalink raw reply
* Re: [PATCH kernel] vfio/spapr_tce: Skip unsetting already unset table
From: Alexey Kardashevskiy @ 2019-02-13 0:18 UTC (permalink / raw)
To: Alex Williamson; +Cc: linuxppc-dev, kvm, kvm-ppc, David Gibson
In-Reply-To: <20190212135242.78b988d0@w520.home>
On 13/02/2019 07:52, Alex Williamson wrote:
> On Mon, 11 Feb 2019 18:49:17 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
>> VFIO TCE IOMMU v2 owns IOMMU tables so when detach a IOMMU group from
>> a container, we need to unset those from a group so we call unset_window()
>> so do we unconditionally. We also unset tables when removing a DMA window
>
> Patch looks ok, but this first sentence trails off into a bit of a word
> salad. Care to refine a bit? Thanks,
Fair comment, sorry for the salad. How about this?
===
VFIO TCE IOMMU v2 owns IOMMU tables. When we detach an IOMMU group from
a container, we need to unset these tables from the group which we do by
calling unset_window(). We also unset tables when removing a DMA window
via the VFIO_IOMMU_SPAPR_TCE_REMOVE ioctl.
===
>
> Alex
>
>> via the VFIO_IOMMU_SPAPR_TCE_REMOVE ioctl.
>>
>> The window removal checks if the table actually exists (hidden inside
>> tce_iommu_find_table()) but the group detaching does not so the user
>> may see duplicating messages:
>> pci 0009:03 : [PE# fd] Removing DMA window #0
>> pci 0009:03 : [PE# fd] Removing DMA window #1
>> pci 0009:03 : [PE# fd] Removing DMA window #0
>> pci 0009:03 : [PE# fd] Removing DMA window #1
>>
>> At the moment this is not a problem as the second invocation
>> of unset_window() writes zeroes to the HW registers again and exits early
>> as there is no table.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>
>> When doing VFIO PCI hot unplug, first we remove the DMA window and
>> set container->tables[num] - this is a first couple of messages.
>> Then we detach the group and then we see another couple of the same
>> messages which confused myself.
>> ---
>> drivers/vfio/vfio_iommu_spapr_tce.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
>> index c424913..8dbb270 100644
>> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
>> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
>> @@ -1235,7 +1235,8 @@ static void tce_iommu_release_ownership_ddw(struct tce_container *container,
>> }
>>
>> for (i = 0; i < IOMMU_TABLE_GROUP_MAX_TABLES; ++i)
>> - table_group->ops->unset_window(table_group, i);
>> + if (container->tables[i])
>> + table_group->ops->unset_window(table_group, i);
>>
>> table_group->ops->release_ownership(table_group);
>> }
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 1/5] vfio/type1: use pinned_vm instead of locked_vm to account pinned pages
From: Daniel Jordan @ 2019-02-13 0:26 UTC (permalink / raw)
To: Alex Williamson
Cc: dave, jack, kvm, linux-mm, aik, linux-fpga, atull, linux-kernel,
kvm-ppc, Daniel Jordan, Jason Gunthorpe, peterz, mdf, akpm,
linuxppc-dev, cl, hao.wu
In-Reply-To: <20190212114110.17bc8a14@w520.home>
On Tue, Feb 12, 2019 at 11:41:10AM -0700, Alex Williamson wrote:
> Daniel Jordan <daniel.m.jordan@oracle.com> wrote:
> > On Mon, Feb 11, 2019 at 03:56:20PM -0700, Jason Gunthorpe wrote:
> > > I haven't looked at this super closely, but how does this stuff work?
> > >
> > > do_mlock doesn't touch pinned_vm, and this doesn't touch locked_vm...
> > >
> > > Shouldn't all this be 'if (locked_vm + pinned_vm < RLIMIT_MEMLOCK)' ?
> > >
> > > Otherwise MEMLOCK is really doubled..
> >
> > So this has been a problem for some time, but it's not as easy as adding them
> > together, see [1][2] for a start.
> >
> > The locked_vm/pinned_vm issue definitely needs fixing, but all this series is
> > trying to do is account to the right counter.
Thanks for taking a look, Alex.
> This still makes me nervous because we have userspace dependencies on
> setting process locked memory.
Could you please expand on this? Trying to get more context.
> There's a user visible difference if we
> account for them in the same bucket vs separate. Perhaps we're
> counting in the wrong bucket now, but if we "fix" that and userspace
> adapts, how do we ever go back to accounting both mlocked and pinned
> memory combined against rlimit? Thanks,
PeterZ posted an RFC that addresses this point[1]. It kept pinned_vm and
locked_vm accounting separate, but allowed the two to be added safely to be
compared against RLIMIT_MEMLOCK.
Anyway, until some solution is agreed on, are there objections to converting
locked_vm to an atomic, to avoid user-visible changes, instead of switching
locked_vm users to pinned_vm?
Daniel
[1] http://lkml.kernel.org/r/20130524140114.GK23650@twins.programming.kicks-ass.net
^ permalink raw reply
* Re: [PATCH 2/5] vfio/spapr_tce: use pinned_vm instead of locked_vm to account pinned pages
From: Alexey Kardashevskiy @ 2019-02-13 0:34 UTC (permalink / raw)
To: Alex Williamson
Cc: dave, jack, kvm, linux-mm, linux-fpga, atull, linux-kernel,
kvm-ppc, Daniel Jordan, jgg, mdf, akpm, linuxppc-dev, cl, hao.wu
In-Reply-To: <20190212115652.6cf9a20b@w520.home>
On 13/02/2019 05:56, Alex Williamson wrote:
> On Tue, 12 Feb 2019 17:56:18 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
>> On 12/02/2019 09:44, Daniel Jordan wrote:
>>> Beginning with bc3e53f682d9 ("mm: distinguish between mlocked and pinned
>>> pages"), locked and pinned pages are accounted separately. The SPAPR
>>> TCE VFIO IOMMU driver accounts pinned pages to locked_vm; use pinned_vm
>>> instead.
>>>
>>> pinned_vm recently became atomic and so no longer relies on mmap_sem
>>> held as writer: delete.
>>>
>>> Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
>>> ---
>>> Documentation/vfio.txt | 6 +--
>>> drivers/vfio/vfio_iommu_spapr_tce.c | 64 ++++++++++++++---------------
>>> 2 files changed, 33 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
>>> index f1a4d3c3ba0b..fa37d65363f9 100644
>>> --- a/Documentation/vfio.txt
>>> +++ b/Documentation/vfio.txt
>>> @@ -308,7 +308,7 @@ This implementation has some specifics:
>>> currently there is no way to reduce the number of calls. In order to make
>>> things faster, the map/unmap handling has been implemented in real mode
>>> which provides an excellent performance which has limitations such as
>>> - inability to do locked pages accounting in real time.
>>> + inability to do pinned pages accounting in real time.
>>>
>>> 4) According to sPAPR specification, A Partitionable Endpoint (PE) is an I/O
>>> subtree that can be treated as a unit for the purposes of partitioning and
>>> @@ -324,7 +324,7 @@ This implementation has some specifics:
>>> returns the size and the start of the DMA window on the PCI bus.
>>>
>>> VFIO_IOMMU_ENABLE
>>> - enables the container. The locked pages accounting
>>> + enables the container. The pinned pages accounting
>>> is done at this point. This lets user first to know what
>>> the DMA window is and adjust rlimit before doing any real job.
>
> I don't know of a ulimit only covering pinned pages, so for
> documentation it seems more correct to continue referring to this as
> locked page accounting.
>
>>> @@ -454,7 +454,7 @@ This implementation has some specifics:
>>>
>>> PPC64 paravirtualized guests generate a lot of map/unmap requests,
>>> and the handling of those includes pinning/unpinning pages and updating
>>> - mm::locked_vm counter to make sure we do not exceed the rlimit.
>>> + mm::pinned_vm counter to make sure we do not exceed the rlimit.
>>> The v2 IOMMU splits accounting and pinning into separate operations:
>>>
>>> - VFIO_IOMMU_SPAPR_REGISTER_MEMORY/VFIO_IOMMU_SPAPR_UNREGISTER_MEMORY ioctls
>>> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
>>> index c424913324e3..f47e020dc5e4 100644
>>> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
>>> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
>>> @@ -34,9 +34,11 @@
>>> static void tce_iommu_detach_group(void *iommu_data,
>>> struct iommu_group *iommu_group);
>>>
>>> -static long try_increment_locked_vm(struct mm_struct *mm, long npages)
>>> +static long try_increment_pinned_vm(struct mm_struct *mm, long npages)
>>> {
>>> - long ret = 0, locked, lock_limit;
>>> + long ret = 0;
>>> + s64 pinned;
>>> + unsigned long lock_limit;
>>>
>>> if (WARN_ON_ONCE(!mm))
>>> return -EPERM;
>>> @@ -44,39 +46,33 @@ static long try_increment_locked_vm(struct mm_struct *mm, long npages)
>>> if (!npages)
>>> return 0;
>>>
>>> - down_write(&mm->mmap_sem);
>>> - locked = mm->locked_vm + npages;
>>> + pinned = atomic64_add_return(npages, &mm->pinned_vm);
>>> lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>>> - if (locked > lock_limit && !capable(CAP_IPC_LOCK))
>>> + if (pinned > lock_limit && !capable(CAP_IPC_LOCK)) {
>>> ret = -ENOMEM;
>>> - else
>>> - mm->locked_vm += npages;
>>> + atomic64_sub(npages, &mm->pinned_vm);
>>> + }
>>>
>>> - pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%ld%s\n", current->pid,
>>> + pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%lu%s\n", current->pid,
>>> npages << PAGE_SHIFT,
>>> - mm->locked_vm << PAGE_SHIFT,
>>> - rlimit(RLIMIT_MEMLOCK),
>>> - ret ? " - exceeded" : "");
>>> -
>>> - up_write(&mm->mmap_sem);
>>> + atomic64_read(&mm->pinned_vm) << PAGE_SHIFT,
>>> + rlimit(RLIMIT_MEMLOCK), ret ? " - exceeded" : "");
>>>
>>> return ret;
>>> }
>>>
>>> -static void decrement_locked_vm(struct mm_struct *mm, long npages)
>>> +static void decrement_pinned_vm(struct mm_struct *mm, long npages)
>>> {
>>> if (!mm || !npages)
>>> return;
>>>
>>> - down_write(&mm->mmap_sem);
>>> - if (WARN_ON_ONCE(npages > mm->locked_vm))
>>> - npages = mm->locked_vm;
>>> - mm->locked_vm -= npages;
>>> - pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%ld\n", current->pid,
>>> + if (WARN_ON_ONCE(npages > atomic64_read(&mm->pinned_vm)))
>>> + npages = atomic64_read(&mm->pinned_vm);
>>> + atomic64_sub(npages, &mm->pinned_vm);
>>> + pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%lu\n", current->pid,
>>> npages << PAGE_SHIFT,
>>> - mm->locked_vm << PAGE_SHIFT,
>>> + atomic64_read(&mm->pinned_vm) << PAGE_SHIFT,
>>> rlimit(RLIMIT_MEMLOCK));
>>> - up_write(&mm->mmap_sem);
>>
>>
>> So it used to be down_write+up_write and stuff in between.
>>
>> Now it is 3 independent accesses (actually 4 but the last one is
>> diagnostic) with no locking around them. Why do not we need a lock
>> anymore precisely? Thanks,
>
> The first 2 look pretty sketchy to me, is there a case where you don't
> know how many pages you've pinned to unpin them?
No case like this, this is why WARN_ON_ONCE(). At the time I could have
been under impression that pinned_vm is system-global, hence that
adjustment but we do not really need it there.
> And can it ever
> really be correct to just unpin whatever remains? The last access is
> diagnostic, which leaves 1. Daniel's rework to warn on a negative
> result looks more sane. Thanks,
Yes it does look sane.
--
Alexey
^ permalink raw reply
* Re: [PATCH 2/5] vfio/spapr_tce: use pinned_vm instead of locked_vm to account pinned pages
From: Alexey Kardashevskiy @ 2019-02-13 0:37 UTC (permalink / raw)
To: Daniel Jordan, Christopher Lameter
Cc: dave, jack, kvm, linux-mm, linux-fpga, atull, linux-kernel,
kvm-ppc, jgg, alex.williamson, mdf, akpm, linuxppc-dev, hao.wu
In-Reply-To: <20190212171839.env4rnjwdjyips6z@ca-dmjordan1.us.oracle.com>
On 13/02/2019 04:18, Daniel Jordan wrote:
> On Tue, Feb 12, 2019 at 04:50:11PM +0000, Christopher Lameter wrote:
>> On Tue, 12 Feb 2019, Alexey Kardashevskiy wrote:
>>
>>> Now it is 3 independent accesses (actually 4 but the last one is
>>> diagnostic) with no locking around them. Why do not we need a lock
>>> anymore precisely? Thanks,
>>
>> Updating a regular counter is racy and requires a lock. It was converted
>> to be an atomic which can be incremented without a race.
>
> Yes, though Alexey may have meant that the multiple reads of the atomic in
> decrement_pinned_vm are racy.
Yes, I meant this race, thanks for clarifying this.
> It only matters when there's a bug that would
> make the counter go negative, but it's there.
>
> And FWIW the debug print in try_increment_pinned_vm is also racy.
>
> This fixes all that. It doesn't try to correct the negative pinned_vm as the
> old code did because it's already a bug and adjusting the value by the negative
> amount seems to do nothing but make debugging harder.
>
> If it's ok, I'll respin the whole series this way (another point for common
> helper)
This looks good, thanks for fixing this.
>
> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
> index f47e020dc5e4..b79257304de6 100644
> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -53,25 +53,24 @@ static long try_increment_pinned_vm(struct mm_struct *mm, long npages)
> atomic64_sub(npages, &mm->pinned_vm);
> }
>
> - pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%lu%s\n", current->pid,
> - npages << PAGE_SHIFT,
> - atomic64_read(&mm->pinned_vm) << PAGE_SHIFT,
> - rlimit(RLIMIT_MEMLOCK), ret ? " - exceeded" : "");
> + pr_debug("[%d] RLIMIT_MEMLOCK +%ld %lld/%lu%s\n", current->pid,
> + npages << PAGE_SHIFT, pinned << PAGE_SHIFT,
> + lock_limit, ret ? " - exceeded" : "");
>
> return ret;
> }
>
> static void decrement_pinned_vm(struct mm_struct *mm, long npages)
> {
> + s64 pinned;
> +
> if (!mm || !npages)
> return;
>
> - if (WARN_ON_ONCE(npages > atomic64_read(&mm->pinned_vm)))
> - npages = atomic64_read(&mm->pinned_vm);
> - atomic64_sub(npages, &mm->pinned_vm);
> - pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%lu\n", current->pid,
> - npages << PAGE_SHIFT,
> - atomic64_read(&mm->pinned_vm) << PAGE_SHIFT,
> + pinned = atomic64_sub_return(npages, &mm->pinned_vm);
> + WARN_ON_ONCE(pinned < 0);
> + pr_debug("[%d] RLIMIT_MEMLOCK -%ld %lld/%lu\n", current->pid,
> + npages << PAGE_SHIFT, pinned << PAGE_SHIFT,
> rlimit(RLIMIT_MEMLOCK));
> }
>
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 4/5] powerpc/mmu: use pinned_vm instead of locked_vm to account pinned pages
From: kbuild test robot @ 2019-02-13 1:14 UTC (permalink / raw)
To: Daniel Jordan
Cc: dave, jack, kvm, linux-mm, aik, linux-fpga, atull, linux-kernel,
kvm-ppc, daniel.m.jordan, jgg, alex.williamson, mdf, kbuild-all,
cl, linuxppc-dev, akpm, hao.wu
In-Reply-To: <20190211224437.25267-5-daniel.m.jordan@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 11118 bytes --]
Hi Daniel,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on vfio/next]
[also build test ERROR on v5.0-rc4]
[cannot apply to next-20190212]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Daniel-Jordan/use-pinned_vm-instead-of-locked_vm-to-account-pinned-pages/20190213-070458
base: https://github.com/awilliam/linux-vfio.git next
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
In file included from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/sched/signal.h:5,
from arch/powerpc/mm/mmu_context_iommu.c:13:
arch/powerpc/mm/mmu_context_iommu.c: In function 'mm_iommu_adjust_pinned_vm':
>> arch/powerpc/mm/mmu_context_iommu.c:55:43: error: passing argument 2 of 'atomic64_add_return_relaxed' from incompatible pointer type [-Werror=incompatible-pointer-types]
pinned_vm = atomic64_add_return(npages, &mm->pinned_vm);
^~~~~~~~~~~~~~
include/linux/atomic.h:75:22: note: in definition of macro '__atomic_op_fence'
typeof(op##_relaxed(args)) __ret; \
^~~~
arch/powerpc/mm/mmu_context_iommu.c:55:15: note: in expansion of macro 'atomic64_add_return'
pinned_vm = atomic64_add_return(npages, &mm->pinned_vm);
^~~~~~~~~~~~~~~~~~~
In file included from include/linux/atomic.h:7,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/sched/signal.h:5,
from arch/powerpc/mm/mmu_context_iommu.c:13:
arch/powerpc/include/asm/atomic.h:331:52: note: expected 'atomic64_t *' {aka 'struct <anonymous> *'} but argument is of type 'long unsigned int *'
atomic64_##op##_return_relaxed(long a, atomic64_t *v) \
~~~~~~~~~~~~^
arch/powerpc/include/asm/atomic.h:367:2: note: in expansion of macro 'ATOMIC64_OP_RETURN_RELAXED'
ATOMIC64_OP_RETURN_RELAXED(op, asm_op) \
^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/atomic.h:370:1: note: in expansion of macro 'ATOMIC64_OPS'
ATOMIC64_OPS(add, add)
^~~~~~~~~~~~
In file included from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/sched/signal.h:5,
from arch/powerpc/mm/mmu_context_iommu.c:13:
>> arch/powerpc/mm/mmu_context_iommu.c:55:43: error: passing argument 2 of 'atomic64_add_return_relaxed' from incompatible pointer type [-Werror=incompatible-pointer-types]
pinned_vm = atomic64_add_return(npages, &mm->pinned_vm);
^~~~~~~~~~~~~~
include/linux/atomic.h:77:23: note: in definition of macro '__atomic_op_fence'
__ret = op##_relaxed(args); \
^~~~
arch/powerpc/mm/mmu_context_iommu.c:55:15: note: in expansion of macro 'atomic64_add_return'
pinned_vm = atomic64_add_return(npages, &mm->pinned_vm);
^~~~~~~~~~~~~~~~~~~
In file included from include/linux/atomic.h:7,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/sched/signal.h:5,
from arch/powerpc/mm/mmu_context_iommu.c:13:
arch/powerpc/include/asm/atomic.h:331:52: note: expected 'atomic64_t *' {aka 'struct <anonymous> *'} but argument is of type 'long unsigned int *'
atomic64_##op##_return_relaxed(long a, atomic64_t *v) \
~~~~~~~~~~~~^
arch/powerpc/include/asm/atomic.h:367:2: note: in expansion of macro 'ATOMIC64_OP_RETURN_RELAXED'
ATOMIC64_OP_RETURN_RELAXED(op, asm_op) \
^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/atomic.h:370:1: note: in expansion of macro 'ATOMIC64_OPS'
ATOMIC64_OPS(add, add)
^~~~~~~~~~~~
>> arch/powerpc/mm/mmu_context_iommu.c:58:25: error: passing argument 2 of 'atomic64_sub' from incompatible pointer type [-Werror=incompatible-pointer-types]
atomic64_sub(npages, &mm->pinned_vm);
^~~~~~~~~~~~~~
In file included from include/linux/atomic.h:7,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/sched/signal.h:5,
from arch/powerpc/mm/mmu_context_iommu.c:13:
arch/powerpc/include/asm/atomic.h:315:58: note: expected 'atomic64_t *' {aka 'struct <anonymous> *'} but argument is of type 'long unsigned int *'
static __inline__ void atomic64_##op(long a, atomic64_t *v) \
~~~~~~~~~~~~^
arch/powerpc/include/asm/atomic.h:366:2: note: in expansion of macro 'ATOMIC64_OP'
ATOMIC64_OP(op, asm_op) \
^~~~~~~~~~~
arch/powerpc/include/asm/atomic.h:371:1: note: in expansion of macro 'ATOMIC64_OPS'
ATOMIC64_OPS(sub, subf)
^~~~~~~~~~~~
>> arch/powerpc/mm/mmu_context_iommu.c:61:29: error: passing argument 1 of 'atomic64_read' from incompatible pointer type [-Werror=incompatible-pointer-types]
pinned_vm = atomic64_read(&mm->pinned_vm);
^~~~~~~~~~~~~~
In file included from include/linux/atomic.h:7,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/sched/signal.h:5,
from arch/powerpc/mm/mmu_context_iommu.c:13:
arch/powerpc/include/asm/atomic.h:300:56: note: expected 'const atomic64_t *' {aka 'const struct <anonymous> *'} but argument is of type 'long unsigned int *'
static __inline__ long atomic64_read(const atomic64_t *v)
~~~~~~~~~~~~~~~~~~^
arch/powerpc/mm/mmu_context_iommu.c:64:24: error: passing argument 2 of 'atomic64_sub' from incompatible pointer type [-Werror=incompatible-pointer-types]
atomic64_sub(npages, &mm->pinned_vm);
^~~~~~~~~~~~~~
In file included from include/linux/atomic.h:7,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/sched/signal.h:5,
from arch/powerpc/mm/mmu_context_iommu.c:13:
arch/powerpc/include/asm/atomic.h:315:58: note: expected 'atomic64_t *' {aka 'struct <anonymous> *'} but argument is of type 'long unsigned int *'
static __inline__ void atomic64_##op(long a, atomic64_t *v) \
~~~~~~~~~~~~^
arch/powerpc/include/asm/atomic.h:366:2: note: in expansion of macro 'ATOMIC64_OP'
ATOMIC64_OP(op, asm_op) \
^~~~~~~~~~~
arch/powerpc/include/asm/atomic.h:371:1: note: in expansion of macro 'ATOMIC64_OPS'
ATOMIC64_OPS(sub, subf)
^~~~~~~~~~~~
In file included from include/linux/kernel.h:14,
from include/linux/list.h:9,
from include/linux/rculist.h:10,
from include/linux/sched/signal.h:5,
from arch/powerpc/mm/mmu_context_iommu.c:13:
arch/powerpc/mm/mmu_context_iommu.c:70:18: error: passing argument 1 of 'atomic64_read' from incompatible pointer type [-Werror=incompatible-pointer-types]
atomic64_read(&mm->pinned_vm) << PAGE_SHIFT,
^~~~~~~~~~~~~~
include/linux/printk.h:136:17: note: in definition of macro 'no_printk'
printk(fmt, ##__VA_ARGS__); \
^~~~~~~~~~~
arch/powerpc/mm/mmu_context_iommu.c:67:2: note: in expansion of macro 'pr_debug'
pr_debug("[%d] RLIMIT_MEMLOCK HASH64 %c%lu %ld/%lu\n",
^~~~~~~~
In file included from include/linux/atomic.h:7,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:11,
from include/linux/sched/signal.h:5,
from arch/powerpc/mm/mmu_context_iommu.c:13:
arch/powerpc/include/asm/atomic.h:300:56: note: expected 'const atomic64_t *' {aka 'const struct <anonymous> *'} but argument is of type 'long unsigned int *'
static __inline__ long atomic64_read(const atomic64_t *v)
~~~~~~~~~~~~~~~~~~^
cc1: all warnings being treated as errors
vim +/atomic64_add_return_relaxed +55 arch/powerpc/mm/mmu_context_iommu.c
> 13 #include <linux/sched/signal.h>
14 #include <linux/slab.h>
15 #include <linux/rculist.h>
16 #include <linux/vmalloc.h>
17 #include <linux/mutex.h>
18 #include <linux/migrate.h>
19 #include <linux/hugetlb.h>
20 #include <linux/swap.h>
21 #include <linux/sizes.h>
22 #include <asm/mmu_context.h>
23 #include <asm/pte-walk.h>
24
25 static DEFINE_MUTEX(mem_list_mutex);
26
27 #define MM_IOMMU_TABLE_GROUP_PAGE_DIRTY 0x1
28 #define MM_IOMMU_TABLE_GROUP_PAGE_MASK ~(SZ_4K - 1)
29
30 struct mm_iommu_table_group_mem_t {
31 struct list_head next;
32 struct rcu_head rcu;
33 unsigned long used;
34 atomic64_t mapped;
35 unsigned int pageshift;
36 u64 ua; /* userspace address */
37 u64 entries; /* number of entries in hpas[] */
38 u64 *hpas; /* vmalloc'ed */
39 #define MM_IOMMU_TABLE_INVALID_HPA ((uint64_t)-1)
40 u64 dev_hpa; /* Device memory base address */
41 };
42
43 static long mm_iommu_adjust_pinned_vm(struct mm_struct *mm,
44 unsigned long npages, bool incr)
45 {
46 long ret = 0;
47 unsigned long lock_limit;
48 s64 pinned_vm;
49
50 if (!npages)
51 return 0;
52
53 if (incr) {
54 lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> 55 pinned_vm = atomic64_add_return(npages, &mm->pinned_vm);
56 if (pinned_vm > lock_limit && !capable(CAP_IPC_LOCK)) {
57 ret = -ENOMEM;
> 58 atomic64_sub(npages, &mm->pinned_vm);
59 }
60 } else {
> 61 pinned_vm = atomic64_read(&mm->pinned_vm);
62 if (WARN_ON_ONCE(npages > pinned_vm))
63 npages = pinned_vm;
64 atomic64_sub(npages, &mm->pinned_vm);
65 }
66
67 pr_debug("[%d] RLIMIT_MEMLOCK HASH64 %c%lu %ld/%lu\n",
68 current ? current->pid : 0, incr ? '+' : '-',
69 npages << PAGE_SHIFT,
70 atomic64_read(&mm->pinned_vm) << PAGE_SHIFT,
71 rlimit(RLIMIT_MEMLOCK));
72
73 return ret;
74 }
75
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24035 bytes --]
^ permalink raw reply
* Re: [PATCH 5/5] kvm/book3s: use pinned_vm instead of locked_vm to account pinned pages
From: kbuild test robot @ 2019-02-13 1:43 UTC (permalink / raw)
To: Daniel Jordan
Cc: dave, jack, kvm, linux-mm, aik, linux-fpga, atull, linux-kernel,
kvm-ppc, daniel.m.jordan, jgg, alex.williamson, mdf, kbuild-all,
cl, linuxppc-dev, akpm, hao.wu
In-Reply-To: <20190211224437.25267-6-daniel.m.jordan@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 13126 bytes --]
Hi Daniel,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on vfio/next]
[also build test ERROR on v5.0-rc4]
[cannot apply to next-20190212]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Daniel-Jordan/use-pinned_vm-instead-of-locked_vm-to-account-pinned-pages/20190213-070458
base: https://github.com/awilliam/linux-vfio.git next
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
In file included from include/linux/llist.h:63,
from include/linux/smp.h:15,
from include/linux/percpu.h:7,
from include/linux/context_tracking_state.h:5,
from include/linux/vtime.h:5,
from include/linux/hardirq.h:8,
from include/linux/kvm_host.h:10,
from arch/powerpc/kvm/book3s_64_vio.c:23:
arch/powerpc/kvm/book3s_64_vio.c: In function 'kvmppc_account_memlimit':
>> arch/powerpc/kvm/book3s_64_vio.c:70:42: error: passing argument 2 of 'atomic64_add_return_relaxed' from incompatible pointer type [-Werror=incompatible-pointer-types]
pinned_vm = atomic64_add_return(pages, ¤t->mm->pinned_vm);
^~~~~~~~~~~~~~~~~~~~~~~
include/linux/atomic.h:75:22: note: in definition of macro '__atomic_op_fence'
typeof(op##_relaxed(args)) __ret; \
^~~~
arch/powerpc/kvm/book3s_64_vio.c:70:15: note: in expansion of macro 'atomic64_add_return'
pinned_vm = atomic64_add_return(pages, ¤t->mm->pinned_vm);
^~~~~~~~~~~~~~~~~~~
In file included from include/linux/atomic.h:7,
from include/linux/llist.h:63,
from include/linux/smp.h:15,
from include/linux/percpu.h:7,
from include/linux/context_tracking_state.h:5,
from include/linux/vtime.h:5,
from include/linux/hardirq.h:8,
from include/linux/kvm_host.h:10,
from arch/powerpc/kvm/book3s_64_vio.c:23:
arch/powerpc/include/asm/atomic.h:331:52: note: expected 'atomic64_t *' {aka 'struct <anonymous> *'} but argument is of type 'long unsigned int *'
atomic64_##op##_return_relaxed(long a, atomic64_t *v) \
~~~~~~~~~~~~^
arch/powerpc/include/asm/atomic.h:367:2: note: in expansion of macro 'ATOMIC64_OP_RETURN_RELAXED'
ATOMIC64_OP_RETURN_RELAXED(op, asm_op) \
^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/atomic.h:370:1: note: in expansion of macro 'ATOMIC64_OPS'
ATOMIC64_OPS(add, add)
^~~~~~~~~~~~
In file included from include/linux/llist.h:63,
from include/linux/smp.h:15,
from include/linux/percpu.h:7,
from include/linux/context_tracking_state.h:5,
from include/linux/vtime.h:5,
from include/linux/hardirq.h:8,
from include/linux/kvm_host.h:10,
from arch/powerpc/kvm/book3s_64_vio.c:23:
>> arch/powerpc/kvm/book3s_64_vio.c:70:42: error: passing argument 2 of 'atomic64_add_return_relaxed' from incompatible pointer type [-Werror=incompatible-pointer-types]
pinned_vm = atomic64_add_return(pages, ¤t->mm->pinned_vm);
^~~~~~~~~~~~~~~~~~~~~~~
include/linux/atomic.h:77:23: note: in definition of macro '__atomic_op_fence'
__ret = op##_relaxed(args); \
^~~~
arch/powerpc/kvm/book3s_64_vio.c:70:15: note: in expansion of macro 'atomic64_add_return'
pinned_vm = atomic64_add_return(pages, ¤t->mm->pinned_vm);
^~~~~~~~~~~~~~~~~~~
In file included from include/linux/atomic.h:7,
from include/linux/llist.h:63,
from include/linux/smp.h:15,
from include/linux/percpu.h:7,
from include/linux/context_tracking_state.h:5,
from include/linux/vtime.h:5,
from include/linux/hardirq.h:8,
from include/linux/kvm_host.h:10,
from arch/powerpc/kvm/book3s_64_vio.c:23:
arch/powerpc/include/asm/atomic.h:331:52: note: expected 'atomic64_t *' {aka 'struct <anonymous> *'} but argument is of type 'long unsigned int *'
atomic64_##op##_return_relaxed(long a, atomic64_t *v) \
~~~~~~~~~~~~^
arch/powerpc/include/asm/atomic.h:367:2: note: in expansion of macro 'ATOMIC64_OP_RETURN_RELAXED'
ATOMIC64_OP_RETURN_RELAXED(op, asm_op) \
^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/atomic.h:370:1: note: in expansion of macro 'ATOMIC64_OPS'
ATOMIC64_OPS(add, add)
^~~~~~~~~~~~
>> arch/powerpc/kvm/book3s_64_vio.c:73:24: error: passing argument 2 of 'atomic64_sub' from incompatible pointer type [-Werror=incompatible-pointer-types]
atomic64_sub(pages, ¤t->mm->pinned_vm);
^~~~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/atomic.h:7,
from include/linux/llist.h:63,
from include/linux/smp.h:15,
from include/linux/percpu.h:7,
from include/linux/context_tracking_state.h:5,
from include/linux/vtime.h:5,
from include/linux/hardirq.h:8,
from include/linux/kvm_host.h:10,
from arch/powerpc/kvm/book3s_64_vio.c:23:
arch/powerpc/include/asm/atomic.h:315:58: note: expected 'atomic64_t *' {aka 'struct <anonymous> *'} but argument is of type 'long unsigned int *'
static __inline__ void atomic64_##op(long a, atomic64_t *v) \
~~~~~~~~~~~~^
arch/powerpc/include/asm/atomic.h:366:2: note: in expansion of macro 'ATOMIC64_OP'
ATOMIC64_OP(op, asm_op) \
^~~~~~~~~~~
arch/powerpc/include/asm/atomic.h:371:1: note: in expansion of macro 'ATOMIC64_OPS'
ATOMIC64_OPS(sub, subf)
^~~~~~~~~~~~
>> arch/powerpc/kvm/book3s_64_vio.c:76:29: error: passing argument 1 of 'atomic64_read' from incompatible pointer type [-Werror=incompatible-pointer-types]
pinned_vm = atomic64_read(¤t->mm->pinned_vm);
^~~~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/atomic.h:7,
from include/linux/llist.h:63,
from include/linux/smp.h:15,
from include/linux/percpu.h:7,
from include/linux/context_tracking_state.h:5,
from include/linux/vtime.h:5,
from include/linux/hardirq.h:8,
from include/linux/kvm_host.h:10,
from arch/powerpc/kvm/book3s_64_vio.c:23:
arch/powerpc/include/asm/atomic.h:300:56: note: expected 'const atomic64_t *' {aka 'const struct <anonymous> *'} but argument is of type 'long unsigned int *'
static __inline__ long atomic64_read(const atomic64_t *v)
~~~~~~~~~~~~~~~~~~^
arch/powerpc/kvm/book3s_64_vio.c:80:23: error: passing argument 2 of 'atomic64_sub' from incompatible pointer type [-Werror=incompatible-pointer-types]
atomic64_sub(pages, ¤t->mm->pinned_vm);
^~~~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/atomic.h:7,
from include/linux/llist.h:63,
from include/linux/smp.h:15,
from include/linux/percpu.h:7,
from include/linux/context_tracking_state.h:5,
from include/linux/vtime.h:5,
from include/linux/hardirq.h:8,
from include/linux/kvm_host.h:10,
from arch/powerpc/kvm/book3s_64_vio.c:23:
arch/powerpc/include/asm/atomic.h:315:58: note: expected 'atomic64_t *' {aka 'struct <anonymous> *'} but argument is of type 'long unsigned int *'
static __inline__ void atomic64_##op(long a, atomic64_t *v) \
~~~~~~~~~~~~^
arch/powerpc/include/asm/atomic.h:366:2: note: in expansion of macro 'ATOMIC64_OP'
ATOMIC64_OP(op, asm_op) \
^~~~~~~~~~~
arch/powerpc/include/asm/atomic.h:371:1: note: in expansion of macro 'ATOMIC64_OPS'
ATOMIC64_OPS(sub, subf)
^~~~~~~~~~~~
In file included from include/linux/kernel.h:14,
from include/linux/list.h:9,
from include/linux/preempt.h:11,
from include/linux/hardirq.h:5,
from include/linux/kvm_host.h:10,
from arch/powerpc/kvm/book3s_64_vio.c:23:
arch/powerpc/kvm/book3s_64_vio.c:85:18: error: passing argument 1 of 'atomic64_read' from incompatible pointer type [-Werror=incompatible-pointer-types]
atomic64_read(¤t->mm->pinned_vm) << PAGE_SHIFT,
^~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:136:17: note: in definition of macro 'no_printk'
printk(fmt, ##__VA_ARGS__); \
^~~~~~~~~~~
arch/powerpc/kvm/book3s_64_vio.c:83:2: note: in expansion of macro 'pr_debug'
pr_debug("[%d] RLIMIT_MEMLOCK KVM %c%lu %ld/%lu%s\n", current->pid,
^~~~~~~~
In file included from include/linux/atomic.h:7,
from include/linux/llist.h:63,
from include/linux/smp.h:15,
from include/linux/percpu.h:7,
from include/linux/context_tracking_state.h:5,
from include/linux/vtime.h:5,
from include/linux/hardirq.h:8,
from include/linux/kvm_host.h:10,
from arch/powerpc/kvm/book3s_64_vio.c:23:
arch/powerpc/include/asm/atomic.h:300:56: note: expected 'const atomic64_t *' {aka 'const struct <anonymous> *'} but argument is of type 'long unsigned int *'
static __inline__ long atomic64_read(const atomic64_t *v)
~~~~~~~~~~~~~~~~~~^
cc1: all warnings being treated as errors
vim +/atomic64_add_return_relaxed +70 arch/powerpc/kvm/book3s_64_vio.c
> 23 #include <linux/kvm_host.h>
24 #include <linux/highmem.h>
25 #include <linux/gfp.h>
26 #include <linux/slab.h>
27 #include <linux/sched/signal.h>
28 #include <linux/hugetlb.h>
29 #include <linux/list.h>
30 #include <linux/anon_inodes.h>
31 #include <linux/iommu.h>
32 #include <linux/file.h>
33
34 #include <asm/kvm_ppc.h>
35 #include <asm/kvm_book3s.h>
36 #include <asm/book3s/64/mmu-hash.h>
37 #include <asm/hvcall.h>
38 #include <asm/synch.h>
39 #include <asm/ppc-opcode.h>
40 #include <asm/kvm_host.h>
41 #include <asm/udbg.h>
42 #include <asm/iommu.h>
43 #include <asm/tce.h>
44 #include <asm/mmu_context.h>
45
46 static unsigned long kvmppc_tce_pages(unsigned long iommu_pages)
47 {
48 return ALIGN(iommu_pages * sizeof(u64), PAGE_SIZE) / PAGE_SIZE;
49 }
50
51 static unsigned long kvmppc_stt_pages(unsigned long tce_pages)
52 {
53 unsigned long stt_bytes = sizeof(struct kvmppc_spapr_tce_table) +
54 (tce_pages * sizeof(struct page *));
55
56 return tce_pages + ALIGN(stt_bytes, PAGE_SIZE) / PAGE_SIZE;
57 }
58
59 static long kvmppc_account_memlimit(unsigned long pages, bool inc)
60 {
61 long ret = 0;
62 s64 pinned_vm;
63
64 if (!current || !current->mm)
65 return ret; /* process exited */
66
67 if (inc) {
68 unsigned long lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
69
> 70 pinned_vm = atomic64_add_return(pages, ¤t->mm->pinned_vm);
71 if (pinned_vm > lock_limit && !capable(CAP_IPC_LOCK)) {
72 ret = -ENOMEM;
> 73 atomic64_sub(pages, ¤t->mm->pinned_vm);
74 }
75 } else {
> 76 pinned_vm = atomic64_read(¤t->mm->pinned_vm);
77 if (WARN_ON_ONCE(pages > pinned_vm))
78 pages = pinned_vm;
79
80 atomic64_sub(pages, ¤t->mm->pinned_vm);
81 }
82
83 pr_debug("[%d] RLIMIT_MEMLOCK KVM %c%lu %ld/%lu%s\n", current->pid,
84 inc ? '+' : '-', pages << PAGE_SHIFT,
85 atomic64_read(¤t->mm->pinned_vm) << PAGE_SHIFT,
86 rlimit(RLIMIT_MEMLOCK), ret ? " - exceeded" : "");
87
88 return ret;
89 }
90
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24035 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] mm: be more verbose about zonelist initialization
From: kbuild test robot @ 2019-02-13 2:13 UTC (permalink / raw)
To: Michal Hocko
Cc: Tony Luck, linux-ia64, Dave Hansen, Peter Zijlstra, x86, LKML,
Pingfan Liu, linux-mm, Michal Hocko, kbuild-all, Ingo Molnar,
linuxppc-dev
In-Reply-To: <20190212095343.23315-3-mhocko@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 5864 bytes --]
Hi Michal,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.0-rc4 next-20190212]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Michal-Hocko/x86-numa-always-initialize-all-possible-nodes/20190213-071628
config: x86_64-kexec (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from include/linux/gfp.h:6:0,
from include/linux/mm.h:10,
from mm/page_alloc.c:18:
mm/page_alloc.c: In function 'build_zonelists':
mm/page_alloc.c:5423:31: error: 'z' undeclared (first use in this function)
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^
include/linux/mmzone.h:1036:7: note: in definition of macro 'for_each_zone_zonelist_nodemask'
for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
^
>> mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^~~~~~~~~~~~~~~~~~~~~~
mm/page_alloc.c:5423:31: note: each undeclared identifier is reported only once for each function it appears in
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^
include/linux/mmzone.h:1036:7: note: in definition of macro 'for_each_zone_zonelist_nodemask'
for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
^
>> mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^~~~~~~~~~~~~~~~~~~~~~
mm/page_alloc.c:5423:25: error: 'zone' undeclared (first use in this function)
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^
include/linux/mmzone.h:1036:59: note: in definition of macro 'for_each_zone_zonelist_nodemask'
for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
^~~~
>> mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^~~~~~~~~~~~~~~~~~~~~~
include/linux/mmzone.h:1036:57: warning: left-hand operand of comma expression has no effect [-Wunused-value]
for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
^
>> include/linux/mmzone.h:1058:2: note: in expansion of macro 'for_each_zone_zonelist_nodemask'
for_each_zone_zonelist_nodemask(zone, z, zlist, highidx, NULL)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^~~~~~~~~~~~~~~~~~~~~~
include/linux/mmzone.h:1038:50: warning: left-hand operand of comma expression has no effect [-Wunused-value]
z = next_zones_zonelist(++z, highidx, nodemask), \
^
>> include/linux/mmzone.h:1058:2: note: in expansion of macro 'for_each_zone_zonelist_nodemask'
for_each_zone_zonelist_nodemask(zone, z, zlist, highidx, NULL)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
^~~~~~~~~~~~~~~~~~~~~~
vim +/for_each_zone_zonelist +5423 mm/page_alloc.c
5382
5383 /*
5384 * Build zonelists ordered by zone and nodes within zones.
5385 * This results in conserving DMA zone[s] until all Normal memory is
5386 * exhausted, but results in overflowing to remote node while memory
5387 * may still exist in local DMA zone.
5388 */
5389
5390 static void build_zonelists(pg_data_t *pgdat)
5391 {
5392 static int node_order[MAX_NUMNODES];
5393 int node, load, nr_nodes = 0;
5394 nodemask_t used_mask;
5395 int local_node, prev_node;
5396
5397 /* NUMA-aware ordering of nodes */
5398 local_node = pgdat->node_id;
5399 load = nr_online_nodes;
5400 prev_node = local_node;
5401 nodes_clear(used_mask);
5402
5403 memset(node_order, 0, sizeof(node_order));
5404 while ((node = find_next_best_node(local_node, &used_mask)) >= 0) {
5405 /*
5406 * We don't want to pressure a particular node.
5407 * So adding penalty to the first node in same
5408 * distance group to make it round-robin.
5409 */
5410 if (node_distance(local_node, node) !=
5411 node_distance(local_node, prev_node))
5412 node_load[node] = load;
5413
5414 node_order[nr_nodes++] = node;
5415 prev_node = node;
5416 load--;
5417 }
5418
5419 build_zonelists_in_node_order(pgdat, node_order, nr_nodes);
5420 build_thisnode_zonelists(pgdat);
5421
5422 pr_info("node[%d] zonelist: ", pgdat->node_id);
> 5423 for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
5424 pr_cont("%d:%s ", zone_to_nid(zone), zone->name);
5425 pr_cont("\n");
5426 }
5427
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26383 bytes --]
^ permalink raw reply
* [PATCH AUTOSEL 4.20 022/105] soc/fsl/qe: fix err handling of ucc_of_parse_tdm
From: Sasha Levin @ 2019-02-13 2:32 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, netdev, Wen Yang, Julia Lawall, linuxppc-dev,
David S . Miller, Zhao Qiang
In-Reply-To: <20190213023336.19019-1-sashal@kernel.org>
From: Wen Yang <wen.yang99@zte.com.cn>
[ Upstream commit 8d68100ab4ad92560a16a68b72e068613ac4d573 ]
Currently there are some issues with the ucc_of_parse_tdm function:
1, a possible null pointer dereference in ucc_of_parse_tdm,
detected by the semantic patch deref_null.cocci,
with the following warning:
drivers/soc/fsl/qe/qe_tdm.c:177:21-24: ERROR: pdev is NULL but dereferenced.
2, dev gets modified, so in any case that devm_iounmap() will fail
even when the new pdev is valid, because the iomap was done with a
different pdev.
3, there is no driver bind with the "fsl,t1040-qe-si" or
"fsl,t1040-qe-siram" device. So allocating resources using devm_*()
with these devices won't provide a cleanup path for these resources
when the caller fails.
This patch fixes them.
Suggested-by: Li Yang <leoyang.li@nxp.com>
Suggested-by: Christophe LEROY <christophe.leroy@c-s.fr>
Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
Reviewed-by: Peng Hao <peng.hao2@zte.com.cn>
CC: Julia Lawall <julia.lawall@lip6.fr>
CC: Zhao Qiang <qiang.zhao@nxp.com>
CC: David S. Miller <davem@davemloft.net>
CC: netdev@vger.kernel.org
CC: linuxppc-dev@lists.ozlabs.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/wan/fsl_ucc_hdlc.c | 62 +++++++++++++++++++++++++++++++++-
drivers/soc/fsl/qe/qe_tdm.c | 55 ------------------------------
2 files changed, 61 insertions(+), 56 deletions(-)
diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 4d6409605207..af13d8cf94ad 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -1049,6 +1049,54 @@ static const struct net_device_ops uhdlc_ops = {
.ndo_tx_timeout = uhdlc_tx_timeout,
};
+static int hdlc_map_iomem(char *name, int init_flag, void __iomem **ptr)
+{
+ struct device_node *np;
+ struct platform_device *pdev;
+ struct resource *res;
+ static int siram_init_flag;
+ int ret = 0;
+
+ np = of_find_compatible_node(NULL, NULL, name);
+ if (!np)
+ return -EINVAL;
+
+ pdev = of_find_device_by_node(np);
+ if (!pdev) {
+ pr_err("%pOFn: failed to lookup pdev\n", np);
+ of_node_put(np);
+ return -EINVAL;
+ }
+
+ of_node_put(np);
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ ret = -EINVAL;
+ goto error_put_device;
+ }
+ *ptr = ioremap(res->start, resource_size(res));
+ if (!*ptr) {
+ ret = -ENOMEM;
+ goto error_put_device;
+ }
+
+ /* We've remapped the addresses, and we don't need the device any
+ * more, so we should release it.
+ */
+ put_device(&pdev->dev);
+
+ if (init_flag && siram_init_flag == 0) {
+ memset_io(*ptr, 0, resource_size(res));
+ siram_init_flag = 1;
+ }
+ return 0;
+
+error_put_device:
+ put_device(&pdev->dev);
+
+ return ret;
+}
+
static int ucc_hdlc_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
@@ -1143,6 +1191,15 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
ret = ucc_of_parse_tdm(np, utdm, ut_info);
if (ret)
goto free_utdm;
+
+ ret = hdlc_map_iomem("fsl,t1040-qe-si", 0,
+ (void __iomem **)&utdm->si_regs);
+ if (ret)
+ goto free_utdm;
+ ret = hdlc_map_iomem("fsl,t1040-qe-siram", 1,
+ (void __iomem **)&utdm->siram);
+ if (ret)
+ goto unmap_si_regs;
}
if (of_property_read_u16(np, "fsl,hmask", &uhdlc_priv->hmask))
@@ -1151,7 +1208,7 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
ret = uhdlc_init(uhdlc_priv);
if (ret) {
dev_err(&pdev->dev, "Failed to init uhdlc\n");
- goto free_utdm;
+ goto undo_uhdlc_init;
}
dev = alloc_hdlcdev(uhdlc_priv);
@@ -1181,6 +1238,9 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
free_dev:
free_netdev(dev);
undo_uhdlc_init:
+ iounmap(utdm->siram);
+unmap_si_regs:
+ iounmap(utdm->si_regs);
free_utdm:
if (uhdlc_priv->tsa)
kfree(utdm);
diff --git a/drivers/soc/fsl/qe/qe_tdm.c b/drivers/soc/fsl/qe/qe_tdm.c
index f78c34647ca2..76480df195a8 100644
--- a/drivers/soc/fsl/qe/qe_tdm.c
+++ b/drivers/soc/fsl/qe/qe_tdm.c
@@ -44,10 +44,6 @@ int ucc_of_parse_tdm(struct device_node *np, struct ucc_tdm *utdm,
const char *sprop;
int ret = 0;
u32 val;
- struct resource *res;
- struct device_node *np2;
- static int siram_init_flag;
- struct platform_device *pdev;
sprop = of_get_property(np, "fsl,rx-sync-clock", NULL);
if (sprop) {
@@ -124,57 +120,6 @@ int ucc_of_parse_tdm(struct device_node *np, struct ucc_tdm *utdm,
utdm->siram_entry_id = val;
set_si_param(utdm, ut_info);
-
- np2 = of_find_compatible_node(NULL, NULL, "fsl,t1040-qe-si");
- if (!np2)
- return -EINVAL;
-
- pdev = of_find_device_by_node(np2);
- if (!pdev) {
- pr_err("%pOFn: failed to lookup pdev\n", np2);
- of_node_put(np2);
- return -EINVAL;
- }
-
- of_node_put(np2);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- utdm->si_regs = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(utdm->si_regs)) {
- ret = PTR_ERR(utdm->si_regs);
- goto err_miss_siram_property;
- }
-
- np2 = of_find_compatible_node(NULL, NULL, "fsl,t1040-qe-siram");
- if (!np2) {
- ret = -EINVAL;
- goto err_miss_siram_property;
- }
-
- pdev = of_find_device_by_node(np2);
- if (!pdev) {
- ret = -EINVAL;
- pr_err("%pOFn: failed to lookup pdev\n", np2);
- of_node_put(np2);
- goto err_miss_siram_property;
- }
-
- of_node_put(np2);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- utdm->siram = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(utdm->siram)) {
- ret = PTR_ERR(utdm->siram);
- goto err_miss_siram_property;
- }
-
- if (siram_init_flag == 0) {
- memset_io(utdm->siram, 0, resource_size(res));
- siram_init_flag = 1;
- }
-
- return ret;
-
-err_miss_siram_property:
- devm_iounmap(&pdev->dev, utdm->si_regs);
return ret;
}
EXPORT_SYMBOL(ucc_of_parse_tdm);
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.20 051/105] powerpc/8xx: fix setting of pagetable for Abatron BDI debug tool.
From: Sasha Levin @ 2019-02-13 2:32 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: linuxppc-dev, Sasha Levin
In-Reply-To: <20190213023336.19019-1-sashal@kernel.org>
From: Christophe Leroy <christophe.leroy@c-s.fr>
[ Upstream commit fb0bdec51a4901b7dd088de0a1e365e1b9f5cd21 ]
Commit 8c8c10b90d88 ("powerpc/8xx: fix handling of early NULL pointer
dereference") moved the loading of r6 earlier in the code. As some
functions are called inbetween, r6 needs to be loaded again with the
address of swapper_pg_dir in order to set PTE pointers for
the Abatron BDI.
Fixes: 8c8c10b90d88 ("powerpc/8xx: fix handling of early NULL pointer dereference")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/kernel/head_8xx.S | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 3b67b9533c82..438512759e82 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -927,11 +927,12 @@ start_here:
/* set up the PTE pointers for the Abatron bdiGDB.
*/
- tovirt(r6,r6)
lis r5, abatron_pteptrs@h
ori r5, r5, abatron_pteptrs@l
stw r5, 0xf0(0) /* Must match your Abatron config file */
tophys(r5,r5)
+ lis r6, swapper_pg_dir@h
+ ori r6, r6, swapper_pg_dir@l
stw r6, 0(r5)
/* Now turn on the MMU for real! */
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 43/83] powerpc/8xx: fix setting of pagetable for Abatron BDI debug tool.
From: Sasha Levin @ 2019-02-13 2:36 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: linuxppc-dev, Sasha Levin
In-Reply-To: <20190213023701.20286-1-sashal@kernel.org>
From: Christophe Leroy <christophe.leroy@c-s.fr>
[ Upstream commit fb0bdec51a4901b7dd088de0a1e365e1b9f5cd21 ]
Commit 8c8c10b90d88 ("powerpc/8xx: fix handling of early NULL pointer
dereference") moved the loading of r6 earlier in the code. As some
functions are called inbetween, r6 needs to be loaded again with the
address of swapper_pg_dir in order to set PTE pointers for
the Abatron BDI.
Fixes: 8c8c10b90d88 ("powerpc/8xx: fix handling of early NULL pointer dereference")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/kernel/head_8xx.S | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 81d4574d1f37..9fd2ff28b8ff 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -919,11 +919,12 @@ start_here:
/* set up the PTE pointers for the Abatron bdiGDB.
*/
- tovirt(r6,r6)
lis r5, abatron_pteptrs@h
ori r5, r5, abatron_pteptrs@l
stw r5, 0xf0(0) /* Must match your Abatron config file */
tophys(r5,r5)
+ lis r6, swapper_pg_dir@h
+ ori r6, r6, swapper_pg_dir@l
stw r6, 0(r5)
/* Now turn on the MMU for real! */
--
2.19.1
^ permalink raw reply related
* [PATCH kernel v2] powerpc/powernv/ioda: Fix locked_vm counting for memory used by IOMMU tables
From: Alexey Kardashevskiy @ 2019-02-13 3:38 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, David Gibson
We store 2 multilevel tables in iommu_table - one for the hardware and
one with the corresponding userspace addresses. Before allocating
the tables, the iommu_table_group_ops::get_table_size() hook returns
the combined size of the two and VFIO SPAPR TCE IOMMU driver adjusts
the locked_vm counter correctly. When the table is actually allocated,
the amount of allocated memory is stored in iommu_table::it_allocated_size
and used to decrement the locked_vm counter when we release the memory
used by the table; .get_table_size() and .create_table() calculate it
independently but the result is expected to be the same.
However the allocator does not add the userspace table size to
.it_allocated_size so when we destroy the table because of VFIO PCI
unplug (i.e. VFIO container is gone but the userspace keeps running),
we decrement locked_vm by just a half of size of memory we are releasing.
To make things worse, since we enabled on-demain allocation of
indirect levels, it_allocated_size contains only the amount of memory
actually allocated at the table creation time which can just be
a fraction. It is not a problem with incrementing locked_vm (as
get_table_size() value is used) but it is with decrementing.
As the result, we leak locked_vm and may not be able to allocate more
IOMMU tables after few iterations of hotplug/unplug.
This sets it_allocated_size in the pnv_pci_ioda2_ops::create_table()
hook to what pnv_pci_ioda2_get_table_size() returns so from now on
we have a single place which calculates the maximum memory a table can
occupy. The original meaning of it_allocated_size is somewhat lost now
though.
We do not ditch it_allocated_size whatsoever here and we do not call
get_table_size() from vfio_iommu_spapr_tce.c when decrementing locked_vm
as we may have multiple IOMMU groups per container and even though they
all are supposed to have the same get_table_size() implementation,
there is a small chance for failure or confusion.
Fixes: 090bad39b "powerpc/powernv: Add indirect levels to it_userspace"
Fixes: a68bd1267 "powerpc/powernv/ioda: Allocate indirect TCE levels on demand"
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v2:
* this is reworked "[PATCH kernel] powerpc/powernv/ioda: Store correct amount of memory used for table"
---
arch/powerpc/platforms/powernv/pci-ioda-tce.c | 1 -
arch/powerpc/platforms/powernv/pci-ioda.c | 7 ++++++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda-tce.c b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
index 697449a..e28f03e 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda-tce.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
@@ -313,7 +313,6 @@ long pnv_pci_ioda2_table_alloc_pages(int nid, __u64 bus_offset,
page_shift);
tbl->it_level_size = 1ULL << (level_shift - 3);
tbl->it_indirect_levels = levels - 1;
- tbl->it_allocated_size = total_allocated;
tbl->it_userspace = uas;
tbl->it_nid = nid;
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 7db3119..d415739 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -2592,8 +2592,13 @@ static long pnv_pci_ioda2_create_table_userspace(
int num, __u32 page_shift, __u64 window_size, __u32 levels,
struct iommu_table **ptbl)
{
- return pnv_pci_ioda2_create_table(table_group,
+ long ret = pnv_pci_ioda2_create_table(table_group,
num, page_shift, window_size, levels, true, ptbl);
+
+ if (!ret)
+ (*ptbl)->it_allocated_size = pnv_pci_ioda2_get_table_size(
+ page_shift, window_size, levels);
+ return ret;
}
static void pnv_ioda2_take_ownership(struct iommu_table_group *table_group)
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 7/7] powerpc/eeh: Add eeh_force_recover to debugfs
From: Sam Bobroff @ 2019-02-13 4:37 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linuxppc-dev
In-Reply-To: <20190208030802.10805-7-oohall@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5488 bytes --]
On Fri, Feb 08, 2019 at 02:08:02PM +1100, Oliver O'Halloran wrote:
> This patch adds a debugfs interface to force scheduling a recovery event.
> This can be used to recover a specific PE or schedule a "special" recovery
> even that checks for errors at the PHB level.
> To force a recovery of a normal PE, use:
>
> echo '<#pe>:<#phb>' > /sys/kernel/debug/powerpc/eeh_force_recover
How about placing these in the per-PHB debugfs directory?
echo '<#pe>' > /sys/kernel/debug/powerpc/PCI0000/eeh_force_recover
> To force a scan broken PHBs:
>
> echo 'null' > /sys/kernel/debug/powerpc/eeh_force_recover
And keep this one where it is, and just trigger with any write (or a '1'
or whatever)?
Sam.
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> arch/powerpc/include/asm/eeh_event.h | 1 +
> arch/powerpc/kernel/eeh.c | 60 ++++++++++++++++++++++++++++
> arch/powerpc/kernel/eeh_event.c | 25 +++++++-----
> 3 files changed, 76 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/eeh_event.h b/arch/powerpc/include/asm/eeh_event.h
> index 9884e872686f..6d0412b846ac 100644
> --- a/arch/powerpc/include/asm/eeh_event.h
> +++ b/arch/powerpc/include/asm/eeh_event.h
> @@ -33,6 +33,7 @@ struct eeh_event {
>
> int eeh_event_init(void);
> int eeh_send_failure_event(struct eeh_pe *pe);
> +int __eeh_send_failure_event(struct eeh_pe *pe);
> void eeh_remove_event(struct eeh_pe *pe, bool force);
> void eeh_handle_normal_event(struct eeh_pe *pe);
> void eeh_handle_special_event(void);
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 92809b137e39..63b91a4918c9 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -1805,6 +1805,63 @@ static int eeh_enable_dbgfs_get(void *data, u64 *val)
>
> DEFINE_DEBUGFS_ATTRIBUTE(eeh_enable_dbgfs_ops, eeh_enable_dbgfs_get,
> eeh_enable_dbgfs_set, "0x%llx\n");
> +
> +static ssize_t eeh_force_recover_write(struct file *filp,
> + const char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + struct pci_controller *hose;
> + uint32_t phbid, pe_no;
> + struct eeh_pe *pe;
> + char buf[20];
> + int ret;
> +
> + ret = simple_write_to_buffer(buf, sizeof(buf), ppos, user_buf, count);
> + if (!ret)
> + return -EFAULT;
> +
> + /*
> + * When PE is NULL the event is a "special" event. Rather than
> + * recovering a specific PE it forces the EEH core to scan for failed
> + * PHBs and recovers each. This needs to be done before any device
> + * recoveries can occur.
> + */
> + if (!strncmp(buf, "null", 4)) {
> + pr_err("sending failure event\n");
> + __eeh_send_failure_event(NULL);
> + return count;
> + }
> +
> + ret = sscanf(buf, "%x:%x", &phbid, &pe_no);
> + if (ret != 2)
> + return -EINVAL;
> +
> + hose = pci_find_hose_for_domain(phbid);
> + if (!hose)
> + return -ENODEV;
> +
> + /* Retrieve PE */
> + pe = eeh_pe_get(hose, pe_no, 0);
> + if (!pe)
> + return -ENODEV;
> +
> + /*
> + * We don't do any state checking here since the detection
> + * process is async to the recovery process. The recovery
> + * thread *should* not break even if we schedule a recovery
> + * from an odd state (e.g. PE removed, or recovery of a
> + * non-isolated PE)
> + */
> + __eeh_send_failure_event(pe);
> +
> + return ret < 0 ? ret : count;
> +}
> +
> +static const struct file_operations eeh_force_recover_fops = {
> + .open = simple_open,
> + .llseek = no_llseek,
> + .write = eeh_force_recover_write,
> +};
> #endif
>
> static int __init eeh_init_proc(void)
> @@ -1820,6 +1877,9 @@ static int __init eeh_init_proc(void)
> debugfs_create_bool("eeh_disable_recovery", 0600,
> powerpc_debugfs_root,
> &eeh_debugfs_no_recover);
> + debugfs_create_file_unsafe("eeh_force_recover", 0600,
> + powerpc_debugfs_root, NULL,
> + &eeh_force_recover_fops);
> eeh_cache_debugfs_init();
> #endif
> }
> diff --git a/arch/powerpc/kernel/eeh_event.c b/arch/powerpc/kernel/eeh_event.c
> index 19837798bb1d..539aca055d70 100644
> --- a/arch/powerpc/kernel/eeh_event.c
> +++ b/arch/powerpc/kernel/eeh_event.c
> @@ -121,20 +121,11 @@ int eeh_event_init(void)
> * the actual event will be delivered in a normal context
> * (from a workqueue).
> */
> -int eeh_send_failure_event(struct eeh_pe *pe)
> +int __eeh_send_failure_event(struct eeh_pe *pe)
> {
> unsigned long flags;
> struct eeh_event *event;
>
> - /*
> - * If we've manually supressed recovery events via debugfs
> - * then just drop it on the floor.
> - */
> - if (eeh_debugfs_no_recover) {
> - pr_err("EEH: Event dropped due to no_recover setting\n");
> - return 0;
> - }
> -
> event = kzalloc(sizeof(*event), GFP_ATOMIC);
> if (!event) {
> pr_err("EEH: out of memory, event not handled\n");
> @@ -153,6 +144,20 @@ int eeh_send_failure_event(struct eeh_pe *pe)
> return 0;
> }
>
> +int eeh_send_failure_event(struct eeh_pe *pe)
> +{
> + /*
> + * If we've manually supressed recovery events via debugfs
> + * then just drop it on the floor.
> + */
> + if (eeh_debugfs_no_recover) {
> + pr_err("EEH: Event dropped due to no_recover setting\n");
> + return 0;
> + }
> +
> + return __eeh_send_failure_event(pe);
> +}
> +
> /**
> * eeh_remove_event - Remove EEH event from the queue
> * @pe: Event binding to the PE
> --
> 2.20.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH kernel v2] powerpc/powernv/ioda: Fix locked_vm counting for memory used by IOMMU tables
From: David Gibson @ 2019-02-13 4:48 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: linuxppc-dev
In-Reply-To: <20190213033818.51452-1-aik@ozlabs.ru>
[-- Attachment #1: Type: text/plain, Size: 4363 bytes --]
On Wed, Feb 13, 2019 at 02:38:18PM +1100, Alexey Kardashevskiy wrote:
> We store 2 multilevel tables in iommu_table - one for the hardware and
> one with the corresponding userspace addresses. Before allocating
> the tables, the iommu_table_group_ops::get_table_size() hook returns
> the combined size of the two and VFIO SPAPR TCE IOMMU driver adjusts
> the locked_vm counter correctly. When the table is actually allocated,
> the amount of allocated memory is stored in iommu_table::it_allocated_size
> and used to decrement the locked_vm counter when we release the memory
> used by the table; .get_table_size() and .create_table() calculate it
> independently but the result is expected to be the same.
>
> However the allocator does not add the userspace table size to
> .it_allocated_size so when we destroy the table because of VFIO PCI
> unplug (i.e. VFIO container is gone but the userspace keeps running),
> we decrement locked_vm by just a half of size of memory we are releasing.
>
> To make things worse, since we enabled on-demain allocation of
s/demain/demand/
> indirect levels, it_allocated_size contains only the amount of memory
> actually allocated at the table creation time which can just be
> a fraction. It is not a problem with incrementing locked_vm (as
> get_table_size() value is used) but it is with decrementing.
>
> As the result, we leak locked_vm and may not be able to allocate more
> IOMMU tables after few iterations of hotplug/unplug.
>
> This sets it_allocated_size in the pnv_pci_ioda2_ops::create_table()
> hook to what pnv_pci_ioda2_get_table_size() returns so from now on
> we have a single place which calculates the maximum memory a table can
> occupy. The original meaning of it_allocated_size is somewhat lost now
> though.
>
> We do not ditch it_allocated_size whatsoever here and we do not call
> get_table_size() from vfio_iommu_spapr_tce.c when decrementing locked_vm
> as we may have multiple IOMMU groups per container and even though they
> all are supposed to have the same get_table_size() implementation,
> there is a small chance for failure or confusion.
>
> Fixes: 090bad39b "powerpc/powernv: Add indirect levels to it_userspace"
> Fixes: a68bd1267 "powerpc/powernv/ioda: Allocate indirect TCE levels on demand"
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Apart from the typo above,
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> Changes:
> v2:
> * this is reworked "[PATCH kernel] powerpc/powernv/ioda: Store correct amount of memory used for table"
>
> ---
> arch/powerpc/platforms/powernv/pci-ioda-tce.c | 1 -
> arch/powerpc/platforms/powernv/pci-ioda.c | 7 ++++++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda-tce.c b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
> index 697449a..e28f03e 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda-tce.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
> @@ -313,7 +313,6 @@ long pnv_pci_ioda2_table_alloc_pages(int nid, __u64 bus_offset,
> page_shift);
> tbl->it_level_size = 1ULL << (level_shift - 3);
> tbl->it_indirect_levels = levels - 1;
> - tbl->it_allocated_size = total_allocated;
> tbl->it_userspace = uas;
> tbl->it_nid = nid;
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 7db3119..d415739 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -2592,8 +2592,13 @@ static long pnv_pci_ioda2_create_table_userspace(
> int num, __u32 page_shift, __u64 window_size, __u32 levels,
> struct iommu_table **ptbl)
> {
> - return pnv_pci_ioda2_create_table(table_group,
> + long ret = pnv_pci_ioda2_create_table(table_group,
> num, page_shift, window_size, levels, true, ptbl);
> +
> + if (!ret)
> + (*ptbl)->it_allocated_size = pnv_pci_ioda2_get_table_size(
> + page_shift, window_size, levels);
> + return ret;
> }
>
> static void pnv_ioda2_take_ownership(struct iommu_table_group *table_group)
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ 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