* Re: [RFC PATCH V2 2/4] mm: Add get_user_pages_cma_migrate
From: Aneesh Kumar K.V @ 2018-09-06 13:23 UTC (permalink / raw)
To: Michal Hocko
Cc: akpm, Alexey Kardashevskiy, mpe, linux-mm, linux-kernel,
linuxppc-dev
In-Reply-To: <20180906124504.GW14951@dhcp22.suse.cz>
On 09/06/2018 06:15 PM, Michal Hocko wrote:
> On Thu 06-09-18 11:13:40, Aneesh Kumar K.V wrote:
>> This helper does a get_user_pages_fast and if it find pages in the CMA area
>> it will try to migrate them before taking page reference. This makes sure that
>> we don't keep non-movable pages (due to page reference count) in the CMA area.
>> Not able to move pages out of CMA area result in CMA allocation failures.
>
> Again, there is no user so it is hard to guess the intention completely.
> There is no documentation to describe the expected context and
> assumptions about locking etc.
>
patch 4 is the user for the new helper. I will add the documentation
update.
> As noted in the previous email. You should better describe why you are
> bypassing hugetlb pools. I assume that the reason is to guarantee a
> forward progress because those might be sitting in the CMA pools
> already, right?
>
The reason for that is explained in the code
+ struct hstate *h = page_hstate(page);
+ /*
+ * We don't want to dequeue from the pool because pool pages will
+ * mostly be from the CMA region.
+ */
+ return alloc_migrate_huge_page(h, gfp_mask, nid, NULL);
-aneesh
^ permalink raw reply
* Re: [RFC PATCH 00/29] mm: remove bootmem allocator
From: Mike Rapoport @ 2018-09-06 13:21 UTC (permalink / raw)
To: Pasha Tatashin
Cc: Michal Hocko, linux-mm@kvack.org, Andrew Morton, David S. Miller,
Greg Kroah-Hartman, Ingo Molnar, Michael Ellerman, Paul Burton,
Thomas Gleixner, Tony Luck, linux-ia64@vger.kernel.org,
linux-mips@linux-mips.org, linuxppc-dev@lists.ozlabs.org,
sparclinux@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <46ae5e64-7b1a-afab-bfef-d00183a7ef76@microsoft.com>
On Thu, Sep 06, 2018 at 01:04:47PM +0000, Pasha Tatashin wrote:
>
>
> On 9/6/18 5:15 AM, Michal Hocko wrote:
> > On Wed 05-09-18 18:59:15, Mike Rapoport wrote:
> > [...]
> >> 325 files changed, 846 insertions(+), 2478 deletions(-)
> >> delete mode 100644 include/linux/bootmem.h
> >> delete mode 100644 mm/bootmem.c
> >> delete mode 100644 mm/nobootmem.c
> >
> > This is really impressive! Thanks a lot for working on this. I wish we
> > could simplify the memblock API as well. There are just too many public
> > functions with subtly different semantic and barely any useful
> > documentation.
> >
> > But even this is a great step forward!
>
> This is a great simplification of boot process. Thank you Mike!
>
> I agree, with Michal in the future, once nobootmem kernel stabalizes
> after this effort, we should start simplifying memblock allocator API:
> it won't be as big effort as this one, as I think, that can be done in
> incremental phases, but it will help to make early boot much more stable
> and uniform across arches.
It's not only about the memblock APIs. Every arch has its own way of memory
detection and initialization, all this should be revisited at some point.
But yes, apart from the memblock APIs update which will be quite
disruptive, the arches memory initialization can be updated incrementally.
> Thank you,
> Pavel
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [RFC PATCH 00/29] mm: remove bootmem allocator
From: Mike Rapoport @ 2018-09-06 13:16 UTC (permalink / raw)
To: Michal Hocko
Cc: linux-mm, Andrew Morton, David S. Miller, Greg Kroah-Hartman,
Ingo Molnar, Michael Ellerman, Paul Burton, Thomas Gleixner,
Tony Luck, linux-ia64, linux-mips, linuxppc-dev, sparclinux,
linux-kernel
In-Reply-To: <20180906091538.GN14951@dhcp22.suse.cz>
On Thu, Sep 06, 2018 at 11:15:38AM +0200, Michal Hocko wrote:
> On Wed 05-09-18 18:59:15, Mike Rapoport wrote:
> [...]
> > 325 files changed, 846 insertions(+), 2478 deletions(-)
> > delete mode 100644 include/linux/bootmem.h
> > delete mode 100644 mm/bootmem.c
> > delete mode 100644 mm/nobootmem.c
>
> This is really impressive! Thanks a lot for working on this. I wish we
> could simplify the memblock API as well. There are just too many public
> functions with subtly different semantic and barely any useful
> documentation.
There are also many functions with exactly the same semantic :)
Cleaning up the memblock API would be the next step.
> But even this is a great step forward!
> --
> Michal Hocko
> SUSE Labs
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [RFC PATCH 21/29] memblock: replace alloc_bootmem with memblock_alloc
From: Mike Rapoport @ 2018-09-06 13:14 UTC (permalink / raw)
To: Michal Hocko
Cc: linux-mm, Andrew Morton, David S. Miller, Greg Kroah-Hartman,
Ingo Molnar, Michael Ellerman, Paul Burton, Thomas Gleixner,
Tony Luck, linux-ia64, linux-mips, linuxppc-dev, sparclinux,
linux-kernel
In-Reply-To: <20180906085515.GF14951@dhcp22.suse.cz>
On Thu, Sep 06, 2018 at 10:55:15AM +0200, Michal Hocko wrote:
> On Wed 05-09-18 18:59:36, Mike Rapoport wrote:
> > The conversion is done using the following semantic patch:
> >
> > @@
> > expression e;
> > @@
> > - __alloc_bootmem(e)
>
> Did you mean alloc_bottmem? Anyway the only difference from
> _alloc_bootmem is SMP_CACHE_BYTES and so you can use 0 alignment for
> memblock_virt_alloc. Why do we need memblock_alloc_from at all?
Here again, I've copied the wrong script to the changelog. Will fix.
Should have been
- alloc_bootmem(size)
+ memblock_alloc(size, 0)
> > + memblock_alloc_from(e, 0)
> >
> > Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> > ---
> > arch/alpha/kernel/core_marvel.c | 4 ++--
> > arch/alpha/kernel/pci-noop.c | 4 ++--
> > arch/alpha/kernel/pci.c | 4 ++--
> > arch/alpha/kernel/pci_iommu.c | 4 ++--
> > arch/ia64/kernel/mca.c | 4 ++--
> > arch/ia64/mm/tlb.c | 4 ++--
> > arch/m68k/sun3/sun3dvma.c | 3 ++-
> > arch/microblaze/mm/init.c | 2 +-
> > arch/mips/kernel/setup.c | 2 +-
> > arch/um/drivers/net_kern.c | 2 +-
> > arch/um/drivers/vector_kern.c | 2 +-
> > arch/um/kernel/initrd.c | 2 +-
> > arch/x86/kernel/acpi/boot.c | 3 ++-
> > arch/x86/kernel/apic/io_apic.c | 2 +-
> > arch/x86/kernel/e820.c | 2 +-
> > arch/x86/platform/olpc/olpc_dt.c | 2 +-
> > arch/xtensa/platforms/iss/network.c | 2 +-
> > arch/xtensa/platforms/iss/setup.c | 4 ++--
> > drivers/macintosh/smu.c | 2 +-
> > init/main.c | 4 ++--
> > 20 files changed, 30 insertions(+), 28 deletions(-)
> >
> > diff --git a/arch/alpha/kernel/core_marvel.c b/arch/alpha/kernel/core_marvel.c
> > index bdebb8c2..1f00c94 100644
> > --- a/arch/alpha/kernel/core_marvel.c
> > +++ b/arch/alpha/kernel/core_marvel.c
> > @@ -82,7 +82,7 @@ mk_resource_name(int pe, int port, char *str)
> > char *name;
> >
> > sprintf(tmp, "PCI %s PE %d PORT %d", str, pe, port);
> > - name = alloc_bootmem(strlen(tmp) + 1);
> > + name = memblock_alloc(strlen(tmp) + 1, 0);
> > strcpy(name, tmp);
> >
> > return name;
> > @@ -117,7 +117,7 @@ alloc_io7(unsigned int pe)
> > return NULL;
> > }
> >
> > - io7 = alloc_bootmem(sizeof(*io7));
> > + io7 = memblock_alloc(sizeof(*io7), 0);
> > io7->pe = pe;
> > raw_spin_lock_init(&io7->irq_lock);
> >
> > diff --git a/arch/alpha/kernel/pci-noop.c b/arch/alpha/kernel/pci-noop.c
> > index c7c5879..59cbfc2 100644
> > --- a/arch/alpha/kernel/pci-noop.c
> > +++ b/arch/alpha/kernel/pci-noop.c
> > @@ -33,7 +33,7 @@ alloc_pci_controller(void)
> > {
> > struct pci_controller *hose;
> >
> > - hose = alloc_bootmem(sizeof(*hose));
> > + hose = memblock_alloc(sizeof(*hose), 0);
> >
> > *hose_tail = hose;
> > hose_tail = &hose->next;
> > @@ -44,7 +44,7 @@ alloc_pci_controller(void)
> > struct resource * __init
> > alloc_resource(void)
> > {
> > - return alloc_bootmem(sizeof(struct resource));
> > + return memblock_alloc(sizeof(struct resource), 0);
> > }
> >
> > SYSCALL_DEFINE3(pciconfig_iobase, long, which, unsigned long, bus,
> > diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
> > index c668c3b..4cc3eb9 100644
> > --- a/arch/alpha/kernel/pci.c
> > +++ b/arch/alpha/kernel/pci.c
> > @@ -392,7 +392,7 @@ alloc_pci_controller(void)
> > {
> > struct pci_controller *hose;
> >
> > - hose = alloc_bootmem(sizeof(*hose));
> > + hose = memblock_alloc(sizeof(*hose), 0);
> >
> > *hose_tail = hose;
> > hose_tail = &hose->next;
> > @@ -403,7 +403,7 @@ alloc_pci_controller(void)
> > struct resource * __init
> > alloc_resource(void)
> > {
> > - return alloc_bootmem(sizeof(struct resource));
> > + return memblock_alloc(sizeof(struct resource), 0);
> > }
> >
> >
> > diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
> > index 0c05493..5d178c7 100644
> > --- a/arch/alpha/kernel/pci_iommu.c
> > +++ b/arch/alpha/kernel/pci_iommu.c
> > @@ -79,7 +79,7 @@ iommu_arena_new_node(int nid, struct pci_controller *hose, dma_addr_t base,
> > printk("%s: couldn't allocate arena from node %d\n"
> > " falling back to system-wide allocation\n",
> > __func__, nid);
> > - arena = alloc_bootmem(sizeof(*arena));
> > + arena = memblock_alloc(sizeof(*arena), 0);
> > }
> >
> > arena->ptes = memblock_alloc_node(sizeof(*arena), align, nid);
> > @@ -92,7 +92,7 @@ iommu_arena_new_node(int nid, struct pci_controller *hose, dma_addr_t base,
> >
> > #else /* CONFIG_DISCONTIGMEM */
> >
> > - arena = alloc_bootmem(sizeof(*arena));
> > + arena = memblock_alloc(sizeof(*arena), 0);
> > arena->ptes = memblock_alloc_from(mem_size, align, 0);
> >
> > #endif /* CONFIG_DISCONTIGMEM */
> > diff --git a/arch/ia64/kernel/mca.c b/arch/ia64/kernel/mca.c
> > index 5586926..7120976 100644
> > --- a/arch/ia64/kernel/mca.c
> > +++ b/arch/ia64/kernel/mca.c
> > @@ -361,9 +361,9 @@ static ia64_state_log_t ia64_state_log[IA64_MAX_LOG_TYPES];
> >
> > #define IA64_LOG_ALLOCATE(it, size) \
> > {ia64_state_log[it].isl_log[IA64_LOG_CURR_INDEX(it)] = \
> > - (ia64_err_rec_t *)alloc_bootmem(size); \
> > + (ia64_err_rec_t *)memblock_alloc(size, 0); \
> > ia64_state_log[it].isl_log[IA64_LOG_NEXT_INDEX(it)] = \
> > - (ia64_err_rec_t *)alloc_bootmem(size);}
> > + (ia64_err_rec_t *)memblock_alloc(size, 0);}
> > #define IA64_LOG_LOCK_INIT(it) spin_lock_init(&ia64_state_log[it].isl_lock)
> > #define IA64_LOG_LOCK(it) spin_lock_irqsave(&ia64_state_log[it].isl_lock, s)
> > #define IA64_LOG_UNLOCK(it) spin_unlock_irqrestore(&ia64_state_log[it].isl_lock,s)
> > diff --git a/arch/ia64/mm/tlb.c b/arch/ia64/mm/tlb.c
> > index acf10eb..5554863 100644
> > --- a/arch/ia64/mm/tlb.c
> > +++ b/arch/ia64/mm/tlb.c
> > @@ -59,8 +59,8 @@ struct ia64_tr_entry *ia64_idtrs[NR_CPUS];
> > void __init
> > mmu_context_init (void)
> > {
> > - ia64_ctx.bitmap = alloc_bootmem((ia64_ctx.max_ctx+1)>>3);
> > - ia64_ctx.flushmap = alloc_bootmem((ia64_ctx.max_ctx+1)>>3);
> > + ia64_ctx.bitmap = memblock_alloc((ia64_ctx.max_ctx + 1) >> 3, 0);
> > + ia64_ctx.flushmap = memblock_alloc((ia64_ctx.max_ctx + 1) >> 3, 0);
> > }
> >
> > /*
> > diff --git a/arch/m68k/sun3/sun3dvma.c b/arch/m68k/sun3/sun3dvma.c
> > index 8546922..72d9458 100644
> > --- a/arch/m68k/sun3/sun3dvma.c
> > +++ b/arch/m68k/sun3/sun3dvma.c
> > @@ -267,7 +267,8 @@ void __init dvma_init(void)
> >
> > list_add(&(hole->list), &hole_list);
> >
> > - iommu_use = alloc_bootmem(IOMMU_TOTAL_ENTRIES * sizeof(unsigned long));
> > + iommu_use = memblock_alloc(IOMMU_TOTAL_ENTRIES * sizeof(unsigned long),
> > + 0);
> >
> > dvma_unmap_iommu(DVMA_START, DVMA_SIZE);
> >
> > diff --git a/arch/microblaze/mm/init.c b/arch/microblaze/mm/init.c
> > index df6de7c..8c7f074 100644
> > --- a/arch/microblaze/mm/init.c
> > +++ b/arch/microblaze/mm/init.c
> > @@ -377,7 +377,7 @@ void * __ref zalloc_maybe_bootmem(size_t size, gfp_t mask)
> > if (mem_init_done)
> > p = kzalloc(size, mask);
> > else {
> > - p = alloc_bootmem(size);
> > + p = memblock_alloc(size, 0);
> > if (p)
> > memset(p, 0, size);
> > }
> > diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> > index 08f8251..419dfc42 100644
> > --- a/arch/mips/kernel/setup.c
> > +++ b/arch/mips/kernel/setup.c
> > @@ -901,7 +901,7 @@ static void __init resource_init(void)
> > if (end >= HIGHMEM_START)
> > end = HIGHMEM_START - 1;
> >
> > - res = alloc_bootmem(sizeof(struct resource));
> > + res = memblock_alloc(sizeof(struct resource), 0);
> >
> > res->start = start;
> > res->end = end;
> > diff --git a/arch/um/drivers/net_kern.c b/arch/um/drivers/net_kern.c
> > index 3ef1b48..ef19a39 100644
> > --- a/arch/um/drivers/net_kern.c
> > +++ b/arch/um/drivers/net_kern.c
> > @@ -650,7 +650,7 @@ static int __init eth_setup(char *str)
> > return 1;
> > }
> >
> > - new = alloc_bootmem(sizeof(*new));
> > + new = memblock_alloc(sizeof(*new), 0);
> >
> > INIT_LIST_HEAD(&new->list);
> > new->index = n;
> > diff --git a/arch/um/drivers/vector_kern.c b/arch/um/drivers/vector_kern.c
> > index c84133c..9d77579 100644
> > --- a/arch/um/drivers/vector_kern.c
> > +++ b/arch/um/drivers/vector_kern.c
> > @@ -1575,7 +1575,7 @@ static int __init vector_setup(char *str)
> > str, error);
> > return 1;
> > }
> > - new = alloc_bootmem(sizeof(*new));
> > + new = memblock_alloc(sizeof(*new), 0);
> > INIT_LIST_HEAD(&new->list);
> > new->unit = n;
> > new->arguments = str;
> > diff --git a/arch/um/kernel/initrd.c b/arch/um/kernel/initrd.c
> > index 6f6e789..844056c 100644
> > --- a/arch/um/kernel/initrd.c
> > +++ b/arch/um/kernel/initrd.c
> > @@ -36,7 +36,7 @@ int __init read_initrd(void)
> > return 0;
> > }
> >
> > - area = alloc_bootmem(size);
> > + area = memblock_alloc(size, 0);
> >
> > if (load_initrd(initrd, area, size) == -1)
> > return 0;
> > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> > index 3b20607..fd887c1 100644
> > --- a/arch/x86/kernel/acpi/boot.c
> > +++ b/arch/x86/kernel/acpi/boot.c
> > @@ -932,7 +932,8 @@ static int __init acpi_parse_hpet(struct acpi_table_header *table)
> > * the resource tree during the lateinit timeframe.
> > */
> > #define HPET_RESOURCE_NAME_SIZE 9
> > - hpet_res = alloc_bootmem(sizeof(*hpet_res) + HPET_RESOURCE_NAME_SIZE);
> > + hpet_res = memblock_alloc(sizeof(*hpet_res) + HPET_RESOURCE_NAME_SIZE,
> > + 0);
> >
> > hpet_res->name = (void *)&hpet_res[1];
> > hpet_res->flags = IORESOURCE_MEM;
> > diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> > index e25118f..8c74509 100644
> > --- a/arch/x86/kernel/apic/io_apic.c
> > +++ b/arch/x86/kernel/apic/io_apic.c
> > @@ -2578,7 +2578,7 @@ static struct resource * __init ioapic_setup_resources(void)
> > n = IOAPIC_RESOURCE_NAME_SIZE + sizeof(struct resource);
> > n *= nr_ioapics;
> >
> > - mem = alloc_bootmem(n);
> > + mem = memblock_alloc(n, 0);
> > res = (void *)mem;
> >
> > mem += sizeof(struct resource) * nr_ioapics;
> > diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> > index c88c23c..7ea8748 100644
> > --- a/arch/x86/kernel/e820.c
> > +++ b/arch/x86/kernel/e820.c
> > @@ -1094,7 +1094,7 @@ void __init e820__reserve_resources(void)
> > struct resource *res;
> > u64 end;
> >
> > - res = alloc_bootmem(sizeof(*res) * e820_table->nr_entries);
> > + res = memblock_alloc(sizeof(*res) * e820_table->nr_entries, 0);
> > e820_res = res;
> >
> > for (i = 0; i < e820_table->nr_entries; i++) {
> > diff --git a/arch/x86/platform/olpc/olpc_dt.c b/arch/x86/platform/olpc/olpc_dt.c
> > index d6ee929..140cd76 100644
> > --- a/arch/x86/platform/olpc/olpc_dt.c
> > +++ b/arch/x86/platform/olpc/olpc_dt.c
> > @@ -141,7 +141,7 @@ void * __init prom_early_alloc(unsigned long size)
> > * fast enough on the platforms we care about while minimizing
> > * wasted bootmem) and hand off chunks of it to callers.
> > */
> > - res = alloc_bootmem(chunk_size);
> > + res = memblock_alloc(chunk_size, 0);
> > BUG_ON(!res);
> > prom_early_allocated += chunk_size;
> > memset(res, 0, chunk_size);
> > diff --git a/arch/xtensa/platforms/iss/network.c b/arch/xtensa/platforms/iss/network.c
> > index d027ddd..206b9d4 100644
> > --- a/arch/xtensa/platforms/iss/network.c
> > +++ b/arch/xtensa/platforms/iss/network.c
> > @@ -646,7 +646,7 @@ static int __init iss_net_setup(char *str)
> > return 1;
> > }
> >
> > - new = alloc_bootmem(sizeof(*new));
> > + new = memblock_alloc(sizeof(*new), 0);
> > if (new == NULL) {
> > pr_err("Alloc_bootmem failed\n");
> > return 1;
> > diff --git a/arch/xtensa/platforms/iss/setup.c b/arch/xtensa/platforms/iss/setup.c
> > index f4bbb28..a922511 100644
> > --- a/arch/xtensa/platforms/iss/setup.c
> > +++ b/arch/xtensa/platforms/iss/setup.c
> > @@ -82,8 +82,8 @@ void __init platform_setup(char **p_cmdline)
> > int argv_size = simc_argv_size();
> >
> > if (argc > 1) {
> > - void **argv = alloc_bootmem(argv_size);
> > - char *cmdline = alloc_bootmem(argv_size);
> > + void **argv = memblock_alloc(argv_size, 0);
> > + char *cmdline = memblock_alloc(argv_size, 0);
> > int i;
> >
> > cmdline[0] = 0;
> > diff --git a/drivers/macintosh/smu.c b/drivers/macintosh/smu.c
> > index e8ae2e5..332fcca 100644
> > --- a/drivers/macintosh/smu.c
> > +++ b/drivers/macintosh/smu.c
> > @@ -493,7 +493,7 @@ int __init smu_init (void)
> > goto fail_np;
> > }
> >
> > - smu = alloc_bootmem(sizeof(struct smu_device));
> > + smu = memblock_alloc(sizeof(struct smu_device), 0);
> >
> > spin_lock_init(&smu->lock);
> > INIT_LIST_HEAD(&smu->cmd_list);
> > diff --git a/init/main.c b/init/main.c
> > index d0b92bd..99a9e99 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -768,8 +768,8 @@ static int __init initcall_blacklist(char *str)
> > str_entry = strsep(&str, ",");
> > if (str_entry) {
> > pr_debug("blacklisting initcall %s\n", str_entry);
> > - entry = alloc_bootmem(sizeof(*entry));
> > - entry->buf = alloc_bootmem(strlen(str_entry) + 1);
> > + entry = memblock_alloc(sizeof(*entry), 0);
> > + entry->buf = memblock_alloc(strlen(str_entry) + 1, 0);
> > strcpy(entry->buf, str_entry);
> > list_add(&entry->next, &blacklisted_initcalls);
> > }
> > --
> > 2.7.4
> >
>
> --
> Michal Hocko
> SUSE Labs
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH 3/3] tty: hvc: hvc_write() fix break condition
From: Nicholas Piggin @ 2018-09-06 13:10 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, Michael Ellerman, Matteo Croce, Jason Gunthorpe,
Leon Romanovsky, linuxppc-dev, op
In-Reply-To: <20180905121439.23809-4-npiggin@gmail.com>
On Wed, 5 Sep 2018 22:14:39 +1000
Nicholas Piggin <npiggin@gmail.com> wrote:
> Commit 550ddadcc758 ("tty: hvc: hvc_write() may sleep") broke the
> termination condition in case the driver stops accepting characters.
> This can result in unnecessary polling of the busy driver.
>
> Restore it by testing the hvc_push return code.
>
> Fixes: 550ddadcc758 ("tty: hvc: hvc_write() may sleep")
> Tested-by: Matteo Croce <mcroce@redhat.com>
> Tested-by: Leon Romanovsky <leonro@mellanox.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> drivers/tty/hvc/hvc_console.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
> index bacf9b73ec98..c09a38fb0d66 100644
> --- a/drivers/tty/hvc/hvc_console.c
> +++ b/drivers/tty/hvc/hvc_console.c
> @@ -522,6 +522,8 @@ static int hvc_write(struct tty_struct *tty, const unsigned char *buf, int count
> return -EIO;
>
> while (count > 0) {
> + int ret;
> +
> spin_lock_irqsave(&hp->lock, flags);
>
> rsize = hp->outbuf_size - hp->n_outbuf;
That needs to be 'int ret = 0;' to avoid used uninitialized.
Let me know if I should resend the patches.
Thanks,
Nick
^ permalink raw reply
* Re: [RFC PATCH 00/29] mm: remove bootmem allocator
From: Pasha Tatashin @ 2018-09-06 13:04 UTC (permalink / raw)
To: Michal Hocko, Mike Rapoport
Cc: linux-mm@kvack.org, Andrew Morton, David S. Miller,
Greg Kroah-Hartman, Ingo Molnar, Michael Ellerman, Paul Burton,
Thomas Gleixner, Tony Luck, linux-ia64@vger.kernel.org,
linux-mips@linux-mips.org, linuxppc-dev@lists.ozlabs.org,
sparclinux@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20180906091538.GN14951@dhcp22.suse.cz>
DQoNCk9uIDkvNi8xOCA1OjE1IEFNLCBNaWNoYWwgSG9ja28gd3JvdGU6DQo+IE9uIFdlZCAwNS0w
OS0xOCAxODo1OToxNSwgTWlrZSBSYXBvcG9ydCB3cm90ZToNCj4gWy4uLl0NCj4+ICAzMjUgZmls
ZXMgY2hhbmdlZCwgODQ2IGluc2VydGlvbnMoKyksIDI0NzggZGVsZXRpb25zKC0pDQo+PiAgZGVs
ZXRlIG1vZGUgMTAwNjQ0IGluY2x1ZGUvbGludXgvYm9vdG1lbS5oDQo+PiAgZGVsZXRlIG1vZGUg
MTAwNjQ0IG1tL2Jvb3RtZW0uYw0KPj4gIGRlbGV0ZSBtb2RlIDEwMDY0NCBtbS9ub2Jvb3RtZW0u
Yw0KPiANCj4gVGhpcyBpcyByZWFsbHkgaW1wcmVzc2l2ZSEgVGhhbmtzIGEgbG90IGZvciB3b3Jr
aW5nIG9uIHRoaXMuIEkgd2lzaCB3ZQ0KPiBjb3VsZCBzaW1wbGlmeSB0aGUgbWVtYmxvY2sgQVBJ
IGFzIHdlbGwuIFRoZXJlIGFyZSBqdXN0IHRvbyBtYW55IHB1YmxpYw0KPiBmdW5jdGlvbnMgd2l0
aCBzdWJ0bHkgZGlmZmVyZW50IHNlbWFudGljIGFuZCBiYXJlbHkgYW55IHVzZWZ1bA0KPiBkb2N1
bWVudGF0aW9uLg0KPiANCj4gQnV0IGV2ZW4gdGhpcyBpcyBhIGdyZWF0IHN0ZXAgZm9yd2FyZCEN
Cg0KVGhpcyBpcyBhIGdyZWF0IHNpbXBsaWZpY2F0aW9uIG9mIGJvb3QgcHJvY2Vzcy4gVGhhbmsg
eW91IE1pa2UhDQoNCkkgYWdyZWUsIHdpdGggTWljaGFsIGluIHRoZSBmdXR1cmUsIG9uY2Ugbm9i
b290bWVtIGtlcm5lbCBzdGFiYWxpemVzDQphZnRlciB0aGlzIGVmZm9ydCwgd2Ugc2hvdWxkIHN0
YXJ0IHNpbXBsaWZ5aW5nIG1lbWJsb2NrIGFsbG9jYXRvciBBUEk6DQppdCB3b24ndCBiZSBhcyBi
aWcgZWZmb3J0IGFzIHRoaXMgb25lLCBhcyBJIHRoaW5rLCB0aGF0IGNhbiBiZSBkb25lIGluDQpp
bmNyZW1lbnRhbCBwaGFzZXMsIGJ1dCBpdCB3aWxsIGhlbHAgdG8gbWFrZSBlYXJseSBib290IG11
Y2ggbW9yZSBzdGFibGUNCmFuZCB1bmlmb3JtIGFjcm9zcyBhcmNoZXMuDQoNClRoYW5rIHlvdSwN
ClBhdmVs
^ permalink raw reply
* Re: [RFC PATCH 07/29] memblock: remove _virt from APIs returning virtual address
From: Michal Hocko @ 2018-09-06 13:01 UTC (permalink / raw)
To: Mike Rapoport
Cc: Rob Herring, linux-mm, Andrew Morton, davem, Greg Kroah-Hartman,
mingo, Michael Ellerman, paul.burton, Thomas Gleixner, tony.luck,
linux-ia64, linux-mips, linuxppc-dev, sparclinux,
Linux Kernel Mailing List
In-Reply-To: <20180906124321.GD27492@rapoport-lnx>
On Thu 06-09-18 15:43:21, Mike Rapoport wrote:
> On Thu, Sep 06, 2018 at 09:28:00AM +0200, Michal Hocko wrote:
> > On Wed 05-09-18 20:20:18, Mike Rapoport wrote:
> > > On Wed, Sep 05, 2018 at 12:04:36PM -0500, Rob Herring wrote:
> > > > On Wed, Sep 5, 2018 at 11:00 AM Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
> > > > >
> > > > > The conversion is done using
> > > > >
> > > > > sed -i 's@memblock_virt_alloc@memblock_alloc@g' \
> > > > > $(git grep -l memblock_virt_alloc)
> > > >
> > > > What's the reason to do this? It seems like a lot of churn even if a
> > > > mechanical change.
> > >
> > > I felt that memblock_virt_alloc_ is too long for a prefix, e.g:
> > > memblock_virt_alloc_node_nopanic, memblock_virt_alloc_low_nopanic.
> > >
> > > And for consistency I've changed the memblock_virt_alloc as well.
> >
> > I would keep the current API unless the name is terribly misleading or
> > it can be improved a lot. Neither seems to be the case here. So I would
> > rather stick with the status quo.
>
> I'm ok with the memblock_virt_alloc by itself, but having 'virt' in
> 'memblock_virt_alloc_try_nid_nopanic' and 'memblock_virt_alloc_low_nopanic'
> reduces code readability in my opinion.
Well, is _nopanic really really useful in the name. Do we even need/want
implicit panic/nopanic semantic? The code should rather check for the
return value and decide depending on the code path. I suspect removing
panic/nopanic would make the API slightly lighter.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [RFC PATCH 20/29] memblock: replace __alloc_bootmem with memblock_alloc_from
From: Mike Rapoport @ 2018-09-06 12:58 UTC (permalink / raw)
To: Michal Hocko
Cc: linux-mm, Andrew Morton, David S. Miller, Greg Kroah-Hartman,
Ingo Molnar, Michael Ellerman, Paul Burton, Thomas Gleixner,
Tony Luck, linux-ia64, linux-mips, linuxppc-dev, sparclinux,
linux-kernel
In-Reply-To: <20180906085205.GE14951@dhcp22.suse.cz>
On Thu, Sep 06, 2018 at 10:52:05AM +0200, Michal Hocko wrote:
> On Wed 05-09-18 18:59:35, Mike Rapoport wrote:
> > The conversion is done using the following semantic patch:
> >
> > @@
> > expression e1, e2, e3;
> > @@
> > - __alloc_bootmem(e1, e2, e3)
> > + memblock_alloc(e1, e2, e3)
>
> This is not that straightforward. memblock_virt_alloc with 0 alignment
> uses SMP_CACHE_BYTES implicitly. I do not see this being handled here.
> I do not expect this should cause any problems, it would be worse other
> way around, but it should be at least documented.
Huh, I've copied the contents of the wrong coccinelle script to the
changelog. This should have been
- __alloc_bootmem(e1, e2, e3)
+ memblock_alloc_from(e1, e2, e3)
Or if replace e1, e2, e3 with more meaningful names
- __alloc_bootmem(size, align, goal)
+ memblock_alloc_from(size, align, min_addr)
That said, the parameters have the exact mapping and the same meaning, so
the alignment and the goal requirements will be handled.
I'll update all the scripts in the changelogs with normal names and add
more elaborate descriptions for all patches.
> > Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> > ---
> > arch/alpha/kernel/core_cia.c | 2 +-
> > arch/alpha/kernel/pci_iommu.c | 4 ++--
> > arch/alpha/kernel/setup.c | 2 +-
> > arch/ia64/kernel/mca.c | 4 ++--
> > arch/ia64/mm/contig.c | 5 +++--
> > arch/mips/kernel/traps.c | 2 +-
> > arch/sparc/kernel/prom_32.c | 2 +-
> > arch/sparc/kernel/smp_64.c | 10 +++++-----
> > arch/sparc/mm/init_32.c | 2 +-
> > arch/sparc/mm/init_64.c | 9 ++++++---
> > arch/sparc/mm/srmmu.c | 10 +++++-----
> > include/linux/bootmem.h | 8 ++++++++
> > 12 files changed, 36 insertions(+), 24 deletions(-)
> >
> > diff --git a/arch/alpha/kernel/core_cia.c b/arch/alpha/kernel/core_cia.c
> > index 4b38386..026ee95 100644
> > --- a/arch/alpha/kernel/core_cia.c
> > +++ b/arch/alpha/kernel/core_cia.c
> > @@ -331,7 +331,7 @@ cia_prepare_tbia_workaround(int window)
> > long i;
> >
> > /* Use minimal 1K map. */
> > - ppte = __alloc_bootmem(CIA_BROKEN_TBIA_SIZE, 32768, 0);
> > + ppte = memblock_alloc_from(CIA_BROKEN_TBIA_SIZE, 32768, 0);
> > pte = (virt_to_phys(ppte) >> (PAGE_SHIFT - 1)) | 1;
> >
> > for (i = 0; i < CIA_BROKEN_TBIA_SIZE / sizeof(unsigned long); ++i)
> > diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
> > index b52d76f..0c05493 100644
> > --- a/arch/alpha/kernel/pci_iommu.c
> > +++ b/arch/alpha/kernel/pci_iommu.c
> > @@ -87,13 +87,13 @@ iommu_arena_new_node(int nid, struct pci_controller *hose, dma_addr_t base,
> > printk("%s: couldn't allocate arena ptes from node %d\n"
> > " falling back to system-wide allocation\n",
> > __func__, nid);
> > - arena->ptes = __alloc_bootmem(mem_size, align, 0);
> > + arena->ptes = memblock_alloc_from(mem_size, align, 0);
> > }
> >
> > #else /* CONFIG_DISCONTIGMEM */
> >
> > arena = alloc_bootmem(sizeof(*arena));
> > - arena->ptes = __alloc_bootmem(mem_size, align, 0);
> > + arena->ptes = memblock_alloc_from(mem_size, align, 0);
> >
> > #endif /* CONFIG_DISCONTIGMEM */
> >
> > diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
> > index 4f0d944..64c06a0 100644
> > --- a/arch/alpha/kernel/setup.c
> > +++ b/arch/alpha/kernel/setup.c
> > @@ -294,7 +294,7 @@ move_initrd(unsigned long mem_limit)
> > unsigned long size;
> >
> > size = initrd_end - initrd_start;
> > - start = __alloc_bootmem(PAGE_ALIGN(size), PAGE_SIZE, 0);
> > + start = memblock_alloc_from(PAGE_ALIGN(size), PAGE_SIZE, 0);
> > if (!start || __pa(start) + size > mem_limit) {
> > initrd_start = initrd_end = 0;
> > return NULL;
> > diff --git a/arch/ia64/kernel/mca.c b/arch/ia64/kernel/mca.c
> > index 6115464..5586926 100644
> > --- a/arch/ia64/kernel/mca.c
> > +++ b/arch/ia64/kernel/mca.c
> > @@ -1835,8 +1835,8 @@ format_mca_init_stack(void *mca_data, unsigned long offset,
> > /* Caller prevents this from being called after init */
> > static void * __ref mca_bootmem(void)
> > {
> > - return __alloc_bootmem(sizeof(struct ia64_mca_cpu),
> > - KERNEL_STACK_SIZE, 0);
> > + return memblock_alloc_from(sizeof(struct ia64_mca_cpu),
> > + KERNEL_STACK_SIZE, 0);
> > }
> >
> > /* Do per-CPU MCA-related initialization. */
> > diff --git a/arch/ia64/mm/contig.c b/arch/ia64/mm/contig.c
> > index e2e40bb..9e5c23a 100644
> > --- a/arch/ia64/mm/contig.c
> > +++ b/arch/ia64/mm/contig.c
> > @@ -85,8 +85,9 @@ void *per_cpu_init(void)
> > static inline void
> > alloc_per_cpu_data(void)
> > {
> > - cpu_data = __alloc_bootmem(PERCPU_PAGE_SIZE * num_possible_cpus(),
> > - PERCPU_PAGE_SIZE, __pa(MAX_DMA_ADDRESS));
> > + cpu_data = memblock_alloc_from(PERCPU_PAGE_SIZE * num_possible_cpus(),
> > + PERCPU_PAGE_SIZE,
> > + __pa(MAX_DMA_ADDRESS));
> > }
> >
> > /**
> > diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
> > index 576aeef..31566d5 100644
> > --- a/arch/mips/kernel/traps.c
> > +++ b/arch/mips/kernel/traps.c
> > @@ -2261,7 +2261,7 @@ void __init trap_init(void)
> > phys_addr_t ebase_pa;
> >
> > ebase = (unsigned long)
> > - __alloc_bootmem(size, 1 << fls(size), 0);
> > + memblock_alloc_from(size, 1 << fls(size), 0);
> >
> > /*
> > * Try to ensure ebase resides in KSeg0 if possible.
> > diff --git a/arch/sparc/kernel/prom_32.c b/arch/sparc/kernel/prom_32.c
> > index b51cbb9..4389944 100644
> > --- a/arch/sparc/kernel/prom_32.c
> > +++ b/arch/sparc/kernel/prom_32.c
> > @@ -32,7 +32,7 @@ void * __init prom_early_alloc(unsigned long size)
> > {
> > void *ret;
> >
> > - ret = __alloc_bootmem(size, SMP_CACHE_BYTES, 0UL);
> > + ret = memblock_alloc_from(size, SMP_CACHE_BYTES, 0UL);
> > if (ret != NULL)
> > memset(ret, 0, size);
> >
> > diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c
> > index 83ff88d..337febd 100644
> > --- a/arch/sparc/kernel/smp_64.c
> > +++ b/arch/sparc/kernel/smp_64.c
> > @@ -1588,7 +1588,7 @@ static void * __init pcpu_alloc_bootmem(unsigned int cpu, size_t size,
> > void *ptr;
> >
> > if (!node_online(node) || !NODE_DATA(node)) {
> > - ptr = __alloc_bootmem(size, align, goal);
> > + ptr = memblock_alloc_from(size, align, goal);
> > pr_info("cpu %d has no node %d or node-local memory\n",
> > cpu, node);
> > pr_debug("per cpu data for cpu%d %lu bytes at %016lx\n",
> > @@ -1601,7 +1601,7 @@ static void * __init pcpu_alloc_bootmem(unsigned int cpu, size_t size,
> > }
> > return ptr;
> > #else
> > - return __alloc_bootmem(size, align, goal);
> > + return memblock_alloc_from(size, align, goal);
> > #endif
> > }
> >
> > @@ -1627,7 +1627,7 @@ static void __init pcpu_populate_pte(unsigned long addr)
> > if (pgd_none(*pgd)) {
> > pud_t *new;
> >
> > - new = __alloc_bootmem(PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> > + new = memblock_alloc_from(PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> > pgd_populate(&init_mm, pgd, new);
> > }
> >
> > @@ -1635,7 +1635,7 @@ static void __init pcpu_populate_pte(unsigned long addr)
> > if (pud_none(*pud)) {
> > pmd_t *new;
> >
> > - new = __alloc_bootmem(PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> > + new = memblock_alloc_from(PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> > pud_populate(&init_mm, pud, new);
> > }
> >
> > @@ -1643,7 +1643,7 @@ static void __init pcpu_populate_pte(unsigned long addr)
> > if (!pmd_present(*pmd)) {
> > pte_t *new;
> >
> > - new = __alloc_bootmem(PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> > + new = memblock_alloc_from(PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> > pmd_populate_kernel(&init_mm, pmd, new);
> > }
> > }
> > diff --git a/arch/sparc/mm/init_32.c b/arch/sparc/mm/init_32.c
> > index 92634d4..885dd38 100644
> > --- a/arch/sparc/mm/init_32.c
> > +++ b/arch/sparc/mm/init_32.c
> > @@ -265,7 +265,7 @@ void __init mem_init(void)
> > i = last_valid_pfn >> ((20 - PAGE_SHIFT) + 5);
> > i += 1;
> > sparc_valid_addr_bitmap = (unsigned long *)
> > - __alloc_bootmem(i << 2, SMP_CACHE_BYTES, 0UL);
> > + memblock_alloc_from(i << 2, SMP_CACHE_BYTES, 0UL);
> >
> > if (sparc_valid_addr_bitmap == NULL) {
> > prom_printf("mem_init: Cannot alloc valid_addr_bitmap.\n");
> > diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
> > index 578ec3d..51cd583 100644
> > --- a/arch/sparc/mm/init_64.c
> > +++ b/arch/sparc/mm/init_64.c
> > @@ -1810,7 +1810,8 @@ static unsigned long __ref kernel_map_range(unsigned long pstart,
> > if (pgd_none(*pgd)) {
> > pud_t *new;
> >
> > - new = __alloc_bootmem(PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> > + new = memblock_alloc_from(PAGE_SIZE, PAGE_SIZE,
> > + PAGE_SIZE);
> > alloc_bytes += PAGE_SIZE;
> > pgd_populate(&init_mm, pgd, new);
> > }
> > @@ -1822,7 +1823,8 @@ static unsigned long __ref kernel_map_range(unsigned long pstart,
> > vstart = kernel_map_hugepud(vstart, vend, pud);
> > continue;
> > }
> > - new = __alloc_bootmem(PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> > + new = memblock_alloc_from(PAGE_SIZE, PAGE_SIZE,
> > + PAGE_SIZE);
> > alloc_bytes += PAGE_SIZE;
> > pud_populate(&init_mm, pud, new);
> > }
> > @@ -1835,7 +1837,8 @@ static unsigned long __ref kernel_map_range(unsigned long pstart,
> > vstart = kernel_map_hugepmd(vstart, vend, pmd);
> > continue;
> > }
> > - new = __alloc_bootmem(PAGE_SIZE, PAGE_SIZE, PAGE_SIZE);
> > + new = memblock_alloc_from(PAGE_SIZE, PAGE_SIZE,
> > + PAGE_SIZE);
> > alloc_bytes += PAGE_SIZE;
> > pmd_populate_kernel(&init_mm, pmd, new);
> > }
> > diff --git a/arch/sparc/mm/srmmu.c b/arch/sparc/mm/srmmu.c
> > index be9cb00..b48fea5 100644
> > --- a/arch/sparc/mm/srmmu.c
> > +++ b/arch/sparc/mm/srmmu.c
> > @@ -303,13 +303,13 @@ static void __init srmmu_nocache_init(void)
> >
> > bitmap_bits = srmmu_nocache_size >> SRMMU_NOCACHE_BITMAP_SHIFT;
> >
> > - srmmu_nocache_pool = __alloc_bootmem(srmmu_nocache_size,
> > - SRMMU_NOCACHE_ALIGN_MAX, 0UL);
> > + srmmu_nocache_pool = memblock_alloc_from(srmmu_nocache_size,
> > + SRMMU_NOCACHE_ALIGN_MAX, 0UL);
> > memset(srmmu_nocache_pool, 0, srmmu_nocache_size);
> >
> > srmmu_nocache_bitmap =
> > - __alloc_bootmem(BITS_TO_LONGS(bitmap_bits) * sizeof(long),
> > - SMP_CACHE_BYTES, 0UL);
> > + memblock_alloc_from(BITS_TO_LONGS(bitmap_bits) * sizeof(long),
> > + SMP_CACHE_BYTES, 0UL);
> > bit_map_init(&srmmu_nocache_map, srmmu_nocache_bitmap, bitmap_bits);
> >
> > srmmu_swapper_pg_dir = __srmmu_get_nocache(SRMMU_PGD_TABLE_SIZE, SRMMU_PGD_TABLE_SIZE);
> > @@ -467,7 +467,7 @@ static void __init sparc_context_init(int numctx)
> > unsigned long size;
> >
> > size = numctx * sizeof(struct ctx_list);
> > - ctx_list_pool = __alloc_bootmem(size, SMP_CACHE_BYTES, 0UL);
> > + ctx_list_pool = memblock_alloc_from(size, SMP_CACHE_BYTES, 0UL);
> >
> > for (ctx = 0; ctx < numctx; ctx++) {
> > struct ctx_list *clist;
> > diff --git a/include/linux/bootmem.h b/include/linux/bootmem.h
> > index 3896af2..c97c105 100644
> > --- a/include/linux/bootmem.h
> > +++ b/include/linux/bootmem.h
> > @@ -122,6 +122,14 @@ static inline void * __init memblock_alloc_raw(
> > NUMA_NO_NODE);
> > }
> >
> > +static inline void * __init memblock_alloc_from(
> > + phys_addr_t size, phys_addr_t align, phys_addr_t min_addr)
> > +{
> > + return memblock_alloc_try_nid(size, align, min_addr,
> > + BOOTMEM_ALLOC_ACCESSIBLE,
> > + NUMA_NO_NODE);
> > +}
> > +
> > static inline void * __init memblock_alloc_nopanic(
> > phys_addr_t size, phys_addr_t align)
> > {
> > --
> > 2.7.4
> >
>
> --
> Michal Hocko
> SUSE Labs
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [RFC PATCH V2 4/4] powerpc/mm/iommu: Allow migration of cma allocated pages during mm_iommu_get
From: Michal Hocko @ 2018-09-06 12:53 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: akpm, Alexey Kardashevskiy, mpe, linux-mm, linux-kernel,
linuxppc-dev
In-Reply-To: <20180906054342.25094-4-aneesh.kumar@linux.ibm.com>
On Thu 06-09-18 11:13:42, Aneesh Kumar K.V wrote:
> Current code doesn't do page migration if the page allocated is a compound page.
> With HugeTLB migration support, we can end up allocating hugetlb pages from
> CMA region. Also THP pages can be allocated from CMA region. This patch updates
> the code to handle compound pages correctly.
>
> This use the new helper get_user_pages_cma_migrate. It does one get_user_pages
> with right count, instead of doing one get_user_pages per page. That avoids
> reading page table multiple times.
>
> The patch also convert the hpas member of mm_iommu_table_group_mem_t to a union.
> We use the same storage location to store pointers to struct page. We cannot
> update alll the code path use struct page *, because we access hpas in real mode
> and we can't do that struct page * to pfn conversion in real mode.
I am not fmailiar with this code so bear with me. I am completely
missing the purpose of this patch. The changelog doesn't really explain
that AFAICS. I can only guess that you do not want to establish long
pins on CMA pages, right? So whenever you are about to pin a page that
is in CMA you migrate it away to a different !__GFP_MOVABLE page, right?
If that is the case then how do you handle pins which are already in
zone_movable? I do not see any specific check for those.
Btw. why is this a proper thing to do? Problems with longterm pins are
not only for CMA/ZONE_MOVABLE pages. Pinned pages are not reclaimable as
well so there is a risk of OOMs if there are too many of them. We have
discussed approaches that would allow to force pin invalidation/revocation
at LSF/MM. Isn't that a more appropriate solution to the problem you are
seeing?
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [RFC PATCH 16/29] memblock: replace __alloc_bootmem_node with appropriate memblock_ API
From: Mike Rapoport @ 2018-09-06 12:50 UTC (permalink / raw)
To: Michal Hocko
Cc: linux-mm, Andrew Morton, David S. Miller, Greg Kroah-Hartman,
Ingo Molnar, Michael Ellerman, Paul Burton, Thomas Gleixner,
Tony Luck, linux-ia64, linux-mips, linuxppc-dev, sparclinux,
linux-kernel
In-Reply-To: <20180906083841.GA14951@dhcp22.suse.cz>
On Thu, Sep 06, 2018 at 10:38:41AM +0200, Michal Hocko wrote:
> On Wed 05-09-18 18:59:31, Mike Rapoport wrote:
> > Use memblock_alloc_try_nid whenever goal (i.e. mininal address is
> > specified) and memblock_alloc_node otherwise.
>
> I suspect you wanted to say (i.e. minimal address) is specified
Yep
> > Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
>
> Acked-by: Michal Hocko <mhocko@suse.com>
>
> One note below
>
> > ---
> > arch/ia64/mm/discontig.c | 6 ++++--
> > arch/ia64/mm/init.c | 2 +-
> > arch/powerpc/kernel/setup_64.c | 6 ++++--
> > arch/sparc/kernel/setup_64.c | 10 ++++------
> > arch/sparc/kernel/smp_64.c | 4 ++--
> > 5 files changed, 15 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/ia64/mm/discontig.c b/arch/ia64/mm/discontig.c
> > index 1928d57..918dda9 100644
> > --- a/arch/ia64/mm/discontig.c
> > +++ b/arch/ia64/mm/discontig.c
> > @@ -451,8 +451,10 @@ static void __init *memory_less_node_alloc(int nid, unsigned long pernodesize)
> > if (bestnode == -1)
> > bestnode = anynode;
> >
> > - ptr = __alloc_bootmem_node(pgdat_list[bestnode], pernodesize,
> > - PERCPU_PAGE_SIZE, __pa(MAX_DMA_ADDRESS));
> > + ptr = memblock_alloc_try_nid(pernodesize, PERCPU_PAGE_SIZE,
> > + __pa(MAX_DMA_ADDRESS),
> > + BOOTMEM_ALLOC_ACCESSIBLE,
> > + bestnode);
> >
> > return ptr;
> > }
> > diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> > index ffcc358..2169ca5 100644
> > --- a/arch/ia64/mm/init.c
> > +++ b/arch/ia64/mm/init.c
> > @@ -459,7 +459,7 @@ int __init create_mem_map_page_table(u64 start, u64 end, void *arg)
> > pte = pte_offset_kernel(pmd, address);
> >
> > if (pte_none(*pte))
> > - set_pte(pte, pfn_pte(__pa(memblock_alloc_node(PAGE_SIZE, PAGE_SIZE, node))) >> PAGE_SHIFT,
> > + set_pte(pte, pfn_pte(__pa(memblock_alloc_node(PAGE_SIZE, PAGE_SIZE, node)) >> PAGE_SHIFT,
> > PAGE_KERNEL));
>
> This doesn't seem to belong to the patch, right?
Right, will fix.
> > }
> > return 0;
> --
> Michal Hocko
> SUSE Labs
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [RFC PATCH 14/29] memblock: add align parameter to memblock_alloc_node()
From: Mike Rapoport @ 2018-09-06 12:49 UTC (permalink / raw)
To: Michal Hocko
Cc: linux-mm, Andrew Morton, David S. Miller, Greg Kroah-Hartman,
Ingo Molnar, Michael Ellerman, Paul Burton, Thomas Gleixner,
Tony Luck, linux-ia64, linux-mips, linuxppc-dev, sparclinux,
linux-kernel
In-Reply-To: <20180906080614.GW14951@dhcp22.suse.cz>
On Thu, Sep 06, 2018 at 10:06:14AM +0200, Michal Hocko wrote:
> On Wed 05-09-18 18:59:29, Mike Rapoport wrote:
> > With the align parameter memblock_alloc_node() can be used as drop in
> > replacement for alloc_bootmem_pages_node().
>
> Why do we need an additional translation later? Sparse code which is the
> only one to use it already uses memblock_alloc_try_nid elsewhere
> (sparse_mem_map_populate).
It is also used in later patches to replace alloc_bootmem* in several
places and most of them explicitly set the alignment.
> > Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> > ---
> > include/linux/bootmem.h | 4 ++--
> > mm/sparse.c | 2 +-
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/bootmem.h b/include/linux/bootmem.h
> > index 7d91f0f..3896af2 100644
> > --- a/include/linux/bootmem.h
> > +++ b/include/linux/bootmem.h
> > @@ -157,9 +157,9 @@ static inline void * __init memblock_alloc_from_nopanic(
> > }
> >
> > static inline void * __init memblock_alloc_node(
> > - phys_addr_t size, int nid)
> > + phys_addr_t size, phys_addr_t align, int nid)
> > {
> > - return memblock_alloc_try_nid(size, 0, BOOTMEM_LOW_LIMIT,
> > + return memblock_alloc_try_nid(size, align, BOOTMEM_LOW_LIMIT,
> > BOOTMEM_ALLOC_ACCESSIBLE, nid);
> > }
> >
> > diff --git a/mm/sparse.c b/mm/sparse.c
> > index 04e97af..509828f 100644
> > --- a/mm/sparse.c
> > +++ b/mm/sparse.c
> > @@ -68,7 +68,7 @@ static noinline struct mem_section __ref *sparse_index_alloc(int nid)
> > if (slab_is_available())
> > section = kzalloc_node(array_size, GFP_KERNEL, nid);
> > else
> > - section = memblock_alloc_node(array_size, nid);
> > + section = memblock_alloc_node(array_size, 0, nid);
> >
> > return section;
> > }
> > --
> > 2.7.4
> >
>
> --
> Michal Hocko
> SUSE Labs
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [RFC PATCH V2 2/4] mm: Add get_user_pages_cma_migrate
From: Michal Hocko @ 2018-09-06 12:45 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: akpm, Alexey Kardashevskiy, mpe, linux-mm, linux-kernel,
linuxppc-dev
In-Reply-To: <20180906054342.25094-2-aneesh.kumar@linux.ibm.com>
On Thu 06-09-18 11:13:40, Aneesh Kumar K.V wrote:
> This helper does a get_user_pages_fast and if it find pages in the CMA area
> it will try to migrate them before taking page reference. This makes sure that
> we don't keep non-movable pages (due to page reference count) in the CMA area.
> Not able to move pages out of CMA area result in CMA allocation failures.
Again, there is no user so it is hard to guess the intention completely.
There is no documentation to describe the expected context and
assumptions about locking etc.
As noted in the previous email. You should better describe why you are
bypassing hugetlb pools. I assume that the reason is to guarantee a
forward progress because those might be sitting in the CMA pools
already, right?
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [RFC PATCH 13/29] memblock: replace __alloc_bootmem_nopanic with memblock_alloc_from_nopanic
From: Mike Rapoport @ 2018-09-06 12:44 UTC (permalink / raw)
To: Michal Hocko
Cc: linux-mm, Andrew Morton, David S. Miller, Greg Kroah-Hartman,
Ingo Molnar, Michael Ellerman, Paul Burton, Thomas Gleixner,
Tony Luck, linux-ia64, linux-mips, linuxppc-dev, sparclinux,
linux-kernel
In-Reply-To: <20180906075721.GV14951@dhcp22.suse.cz>
On Thu, Sep 06, 2018 at 09:57:21AM +0200, Michal Hocko wrote:
> On Wed 05-09-18 18:59:28, Mike Rapoport wrote:
> > Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
>
> The translation is simpler here but still a word or two would be nice.
> Empty changelogs suck.
This is one of the things left to sort out :)
> To the change
> Acked-by: Michal Hocko <mhocko@suse.com>
>
> > ---
> > arch/arc/kernel/unwind.c | 4 ++--
> > arch/x86/kernel/setup_percpu.c | 4 ++--
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arc/kernel/unwind.c b/arch/arc/kernel/unwind.c
> > index 183391d..2a01dd1 100644
> > --- a/arch/arc/kernel/unwind.c
> > +++ b/arch/arc/kernel/unwind.c
> > @@ -181,8 +181,8 @@ static void init_unwind_hdr(struct unwind_table *table,
> > */
> > static void *__init unw_hdr_alloc_early(unsigned long sz)
> > {
> > - return __alloc_bootmem_nopanic(sz, sizeof(unsigned int),
> > - MAX_DMA_ADDRESS);
> > + return memblock_alloc_from_nopanic(sz, sizeof(unsigned int),
> > + MAX_DMA_ADDRESS);
> > }
> >
> > static void *unw_hdr_alloc(unsigned long sz)
> > diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
> > index 67d48e26..041663a 100644
> > --- a/arch/x86/kernel/setup_percpu.c
> > +++ b/arch/x86/kernel/setup_percpu.c
> > @@ -106,7 +106,7 @@ static void * __init pcpu_alloc_bootmem(unsigned int cpu, unsigned long size,
> > void *ptr;
> >
> > if (!node_online(node) || !NODE_DATA(node)) {
> > - ptr = __alloc_bootmem_nopanic(size, align, goal);
> > + ptr = memblock_alloc_from_nopanic(size, align, goal);
> > pr_info("cpu %d has no node %d or node-local memory\n",
> > cpu, node);
> > pr_debug("per cpu data for cpu%d %lu bytes at %016lx\n",
> > @@ -121,7 +121,7 @@ static void * __init pcpu_alloc_bootmem(unsigned int cpu, unsigned long size,
> > }
> > return ptr;
> > #else
> > - return __alloc_bootmem_nopanic(size, align, goal);
> > + return memblock_alloc_from_nopanic(size, align, goal);
> > #endif
> > }
> >
> > --
> > 2.7.4
> >
>
> --
> Michal Hocko
> SUSE Labs
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [RFC PATCH 07/29] memblock: remove _virt from APIs returning virtual address
From: Mike Rapoport @ 2018-09-06 12:43 UTC (permalink / raw)
To: Michal Hocko
Cc: Rob Herring, linux-mm, Andrew Morton, davem, Greg Kroah-Hartman,
mingo, Michael Ellerman, paul.burton, Thomas Gleixner, tony.luck,
linux-ia64, linux-mips, linuxppc-dev, sparclinux,
Linux Kernel Mailing List
In-Reply-To: <20180906072800.GN14951@dhcp22.suse.cz>
On Thu, Sep 06, 2018 at 09:28:00AM +0200, Michal Hocko wrote:
> On Wed 05-09-18 20:20:18, Mike Rapoport wrote:
> > On Wed, Sep 05, 2018 at 12:04:36PM -0500, Rob Herring wrote:
> > > On Wed, Sep 5, 2018 at 11:00 AM Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
> > > >
> > > > The conversion is done using
> > > >
> > > > sed -i 's@memblock_virt_alloc@memblock_alloc@g' \
> > > > $(git grep -l memblock_virt_alloc)
> > >
> > > What's the reason to do this? It seems like a lot of churn even if a
> > > mechanical change.
> >
> > I felt that memblock_virt_alloc_ is too long for a prefix, e.g:
> > memblock_virt_alloc_node_nopanic, memblock_virt_alloc_low_nopanic.
> >
> > And for consistency I've changed the memblock_virt_alloc as well.
>
> I would keep the current API unless the name is terribly misleading or
> it can be improved a lot. Neither seems to be the case here. So I would
> rather stick with the status quo.
I'm ok with the memblock_virt_alloc by itself, but having 'virt' in
'memblock_virt_alloc_try_nid_nopanic' and 'memblock_virt_alloc_low_nopanic'
reduces code readability in my opinion.
Besides, from what I've seen, many users of memblock_phys_alloc can be
converted to the virtual variant and then we can just have memblock_alloc
everywhere in the end.
Currently there are ~70 users of memblock_virt_alloc*, but with the
bootmem -> memblock conversion we'll be adding ~140 more.
> --
> Michal Hocko
> SUSE Labs
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [RFC PATCH V2 1/4] mm: Export alloc_migrate_huge_page
From: Michal Hocko @ 2018-09-06 12:35 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: akpm, Alexey Kardashevskiy, mpe, linux-mm, linux-kernel,
linuxppc-dev
In-Reply-To: <20180906123111.GC26069@dhcp22.suse.cz>
On Thu 06-09-18 14:31:11, Michal Hocko wrote:
> On Thu 06-09-18 11:13:39, Aneesh Kumar K.V wrote:
> > We want to use this to support customized huge page migration.
>
> Please be much more specific. Ideally including the user. Btw. why do
> you want to skip the hugetlb pools? In other words alloc_huge_page_node*
> which are intended to an external use?
Ups, I have now found http://lkml.kernel.org/r/20180906054342.25094-2-aneesh.kumar@linux.ibm.com
which ended up in a different email folder so I have missed it. It would
be much better to merge those two to make the user immediately obvious.
There is a good reason to keep newly added functions closer to their
users.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [RFC PATCH V2 1/4] mm: Export alloc_migrate_huge_page
From: Michal Hocko @ 2018-09-06 12:31 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: akpm, Alexey Kardashevskiy, mpe, linux-mm, linux-kernel,
linuxppc-dev
In-Reply-To: <20180906054342.25094-1-aneesh.kumar@linux.ibm.com>
On Thu 06-09-18 11:13:39, Aneesh Kumar K.V wrote:
> We want to use this to support customized huge page migration.
Please be much more specific. Ideally including the user. Btw. why do
you want to skip the hugetlb pools? In other words alloc_huge_page_node*
which are intended to an external use?
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* [PATCH v2 5/5] PCI/powerpc/eeh: Add pcibios hooks for preparing to rescan
From: Sergey Miroshnichenko @ 2018-09-06 11:57 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux, Sergey Miroshnichenko
In-Reply-To: <20180906115752.29316-1-s.miroshnichenko@yadro.com>
Reading an empty slot returns all ones, which triggers a false
EEH error event on PowerNV.
New callbacks pcibios_rescan_prepare/done are introduced to
pause/resume the EEH during rescan.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
arch/powerpc/include/asm/eeh.h | 2 ++
arch/powerpc/kernel/eeh.c | 12 +++++++++++
arch/powerpc/platforms/powernv/eeh-powernv.c | 22 ++++++++++++++++++++
drivers/pci/probe.c | 14 +++++++++++++
include/linux/pci.h | 2 ++
5 files changed, 52 insertions(+)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 219637ea69a1..926c3e31df99 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -219,6 +219,8 @@ struct eeh_ops {
int (*next_error)(struct eeh_pe **pe);
int (*restore_config)(struct pci_dn *pdn);
int (*notify_resume)(struct pci_dn *pdn);
+ int (*pause)(struct pci_bus *bus);
+ int (*resume)(struct pci_bus *bus);
};
extern int eeh_subsystem_flags;
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 6ebba3e48b01..9fb5012f389d 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1831,3 +1831,15 @@ static int __init eeh_init_proc(void)
return 0;
}
__initcall(eeh_init_proc);
+
+void pcibios_rescan_prepare(struct pci_bus *bus)
+{
+ if (eeh_ops && eeh_ops->pause)
+ eeh_ops->pause(bus);
+}
+
+void pcibios_rescan_done(struct pci_bus *bus)
+{
+ if (eeh_ops && eeh_ops->resume)
+ eeh_ops->resume(bus);
+}
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 3c1beae29f2d..9724a58afcd2 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -59,6 +59,26 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
eeh_sysfs_add_device(pdev);
}
+static int pnv_eeh_pause(struct pci_bus *bus)
+{
+ struct pci_controller *hose = pci_bus_to_host(bus);
+ struct pnv_phb *phb = hose->private_data;
+
+ phb->flags &= ~PNV_PHB_FLAG_EEH;
+ disable_irq(eeh_event_irq);
+ return 0;
+}
+
+static int pnv_eeh_resume(struct pci_bus *bus)
+{
+ struct pci_controller *hose = pci_bus_to_host(bus);
+ struct pnv_phb *phb = hose->private_data;
+
+ enable_irq(eeh_event_irq);
+ phb->flags |= PNV_PHB_FLAG_EEH;
+ return 0;
+}
+
static int pnv_eeh_init(void)
{
struct pci_controller *hose;
@@ -1710,6 +1730,8 @@ static struct eeh_ops pnv_eeh_ops = {
.write_config = pnv_eeh_write_config,
.next_error = pnv_eeh_next_error,
.restore_config = pnv_eeh_restore_config,
+ .pause = pnv_eeh_pause,
+ .resume = pnv_eeh_resume,
.notify_resume = NULL
};
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ac876e32de4b..4a9045364809 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2801,6 +2801,14 @@ void __weak pcibios_remove_bus(struct pci_bus *bus)
{
}
+void __weak pcibios_rescan_prepare(struct pci_bus *bus)
+{
+}
+
+void __weak pcibios_rescan_done(struct pci_bus *bus)
+{
+}
+
struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
struct pci_ops *ops, void *sysdata, struct list_head *resources)
{
@@ -3055,9 +3063,15 @@ unsigned int pci_rescan_bus_bridge_resize(struct pci_dev *bridge)
unsigned int pci_rescan_bus(struct pci_bus *bus)
{
unsigned int max;
+ struct pci_bus *root = bus;
+
+ while (!pci_is_root_bus(root))
+ root = root->parent;
+ pcibios_rescan_prepare(root);
max = pci_scan_child_bus(bus);
pci_assign_unassigned_bus_resources(bus);
+ pcibios_rescan_done(root);
pci_bus_add_devices(bus);
return max;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 340029b2fb38..42930731c5a7 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1929,6 +1929,8 @@ void pcibios_penalize_isa_irq(int irq, int active);
int pcibios_alloc_irq(struct pci_dev *dev);
void pcibios_free_irq(struct pci_dev *dev);
resource_size_t pcibios_default_alignment(void);
+void pcibios_rescan_prepare(struct pci_bus *bus);
+void pcibios_rescan_done(struct pci_bus *bus);
#ifdef CONFIG_HIBERNATE_CALLBACKS
extern struct dev_pm_ops pcibios_pm_ops;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 4/5] powerpc/powernv/pci: Enable reassigning the bus numbers
From: Sergey Miroshnichenko @ 2018-09-06 11:57 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux, Sergey Miroshnichenko
In-Reply-To: <20180906115752.29316-1-s.miroshnichenko@yadro.com>
PowerNV doesn't depend on PCIe topology info from DT anymore, and now
it is able to enumerate the fabric and assign the bus numbers.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
arch/powerpc/platforms/powernv/pci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 3f87a2dc6578..ece3f3e2808e 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -1112,6 +1112,7 @@ void __init pnv_pci_init(void)
struct device_node *np;
pci_add_flags(PCI_CAN_SKIP_ISA_ALIGN);
+ pci_add_flags(PCI_REASSIGN_ALL_BUS);
/* If we don't have OPAL, eg. in sim, just skip PCI probe */
if (!firmware_has_feature(FW_FEATURE_OPAL))
--
2.17.1
^ permalink raw reply related
* [PATCH v2 3/5] powerpc/pci: Use DT to create pci_dn for root bridges only
From: Sergey Miroshnichenko @ 2018-09-06 11:57 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux, Sergey Miroshnichenko
In-Reply-To: <20180906115752.29316-1-s.miroshnichenko@yadro.com>
Endpoint's pci_dn can be created dynamically.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
arch/powerpc/kernel/pci_dn.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 48ec16407835..3b4368f07c82 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -537,8 +537,10 @@ void pci_devs_phb_init_dynamic(struct pci_controller *phb)
phb->pci_data = pdn;
}
- /* Update dn->phb ptrs for new phb and children devices */
- pci_traverse_device_nodes(dn, add_pdn, phb);
+ if (!pci_has_flag(PCI_REASSIGN_ALL_BUS)) {
+ /* Update dn->phb ptrs for new phb and children devices */
+ pci_traverse_device_nodes(dn, add_pdn, phb);
+ }
}
/**
--
2.17.1
^ permalink raw reply related
* [PATCH v2 2/5] powerpc/pci: Create pci_dn on demand
From: Sergey Miroshnichenko @ 2018-09-06 11:57 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux, Sergey Miroshnichenko
In-Reply-To: <20180906115752.29316-1-s.miroshnichenko@yadro.com>
The pci_dn structures can be created not only from DT, but also
directly from newly discovered PCIe devices, so allocate them
dynamically.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
arch/powerpc/kernel/pci_dn.c | 76 ++++++++++++++++++++++++++++--------
1 file changed, 59 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index ab147a1909c8..48ec16407835 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -33,6 +33,8 @@
#include <asm/firmware.h>
#include <asm/eeh.h>
+static struct pci_dn *create_pdn(struct pci_dev *pdev, struct pci_dn *parent);
+
/*
* The function is used to find the firmware data of one
* specific PCI device, which is attached to the indicated
@@ -58,6 +60,9 @@ static struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
pbus = pbus->parent;
}
+ if (!pbus->self && !pci_is_root_bus(pbus))
+ return NULL;
+
/*
* Except virtual bus, all PCI buses should
* have device nodes.
@@ -65,13 +70,15 @@ static struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
dn = pci_bus_to_OF_node(pbus);
pdn = dn ? PCI_DN(dn) : NULL;
+ if (!pdn && pbus->self)
+ pdn = pbus->self->dev.archdata.pci_data;
+
return pdn;
}
struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
int devfn)
{
- struct device_node *dn = NULL;
struct pci_dn *parent, *pdn;
struct pci_dev *pdev = NULL;
@@ -80,17 +87,10 @@ struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
if (pdev->devfn == devfn) {
if (pdev->dev.archdata.pci_data)
return pdev->dev.archdata.pci_data;
-
- dn = pci_device_to_OF_node(pdev);
break;
}
}
- /* Fast path: fetch from device node */
- pdn = dn ? PCI_DN(dn) : NULL;
- if (pdn)
- return pdn;
-
/* Slow path: fetch from firmware data hierarchy */
parent = pci_bus_to_pdn(bus);
if (!parent)
@@ -128,16 +128,9 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
if (!parent)
return NULL;
- list_for_each_entry(pdn, &parent->child_list, list) {
- if (pdn->busno == pdev->bus->number &&
- pdn->devfn == pdev->devfn)
- return pdn;
- }
-
- return NULL;
+ return create_pdn(pdev, parent);
}
-#ifdef CONFIG_PCI_IOV
static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
int vf_index,
int busno, int devfn)
@@ -156,7 +149,9 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
pdn->parent = parent;
pdn->busno = busno;
pdn->devfn = devfn;
+ #ifdef CONFIG_PCI_IOV
pdn->vf_index = vf_index;
+ #endif /* CONFIG_PCI_IOV */
pdn->pe_number = IODA_INVALID_PE;
INIT_LIST_HEAD(&pdn->child_list);
INIT_LIST_HEAD(&pdn->list);
@@ -164,7 +159,54 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
return pdn;
}
-#endif
+
+static struct pci_dn *create_pdn(struct pci_dev *pdev, struct pci_dn *parent)
+{
+ struct pci_dn *pdn = NULL;
+
+ pdn = add_one_dev_pci_data(parent, 0, pdev->bus->number, pdev->devfn);
+ dev_info(&pdev->dev, "Create a new pdn for devfn %2x\n", pdev->devfn / 8);
+
+ if (pdn) {
+ #ifdef CONFIG_EEH
+ struct eeh_dev *edev;
+ #endif /* CONFIG_EEH */
+ u32 class_code;
+ u16 device_id;
+ u16 vendor_id;
+
+ #ifdef CONFIG_EEH
+ edev = eeh_dev_init(pdn);
+ if (!edev) {
+ kfree(pdn);
+ dev_err(&pdev->dev, "%s: Failed to allocate edev\n", __func__);
+ return NULL;
+ }
+ #endif /* CONFIG_EEH */
+
+ pdn->busno = pdev->bus->busn_res.start;
+
+ pci_bus_read_config_word(pdev->bus, pdev->devfn,
+ PCI_VENDOR_ID, &vendor_id);
+ pdn->vendor_id = vendor_id;
+
+ pci_bus_read_config_word(pdev->bus, pdev->devfn,
+ PCI_DEVICE_ID, &device_id);
+ pdn->device_id = device_id;
+
+ pci_bus_read_config_dword(pdev->bus, pdev->devfn,
+ PCI_CLASS_REVISION, &class_code);
+ class_code >>= 8;
+ pdn->class_code = class_code;
+
+ pdn->pci_ext_config_space = 0;
+ pdev->dev.archdata.pci_data = pdn;
+ } else {
+ dev_err(&pdev->dev, "%s: Failed to allocate pdn\n", __func__);
+ }
+
+ return pdn;
+}
struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
{
--
2.17.1
^ permalink raw reply related
* [PATCH v2 1/5] powerpc/pci: Access PCI config space directly w/o pci_dn
From: Sergey Miroshnichenko @ 2018-09-06 11:57 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux, Sergey Miroshnichenko
In-Reply-To: <20180906115752.29316-1-s.miroshnichenko@yadro.com>
The pci_dn structures are retrieved from a DT, but hot-plugged PCIe
devices don't have them. Don't stop PCIe I/O in absence of pci_dn, so
it is now possible to discover new devices.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
arch/powerpc/kernel/rtas_pci.c | 97 +++++++++++++++++++---------
arch/powerpc/platforms/powernv/pci.c | 64 ++++++++++++------
2 files changed, 109 insertions(+), 52 deletions(-)
diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c
index c2b148b1634a..0611b46d9b5f 100644
--- a/arch/powerpc/kernel/rtas_pci.c
+++ b/arch/powerpc/kernel/rtas_pci.c
@@ -55,10 +55,26 @@ static inline int config_access_valid(struct pci_dn *dn, int where)
return 0;
}
-int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
+static int rtas_read_raw_config(unsigned long buid, int busno, unsigned int devfn,
+ int where, int size, u32 *val)
{
int returnval = -1;
- unsigned long buid, addr;
+ unsigned long addr = rtas_config_addr(busno, devfn, where);
+ int ret;
+
+ if (buid) {
+ ret = rtas_call(ibm_read_pci_config, 4, 2, &returnval,
+ addr, BUID_HI(buid), BUID_LO(buid), size);
+ } else {
+ ret = rtas_call(read_pci_config, 2, 2, &returnval, addr, size);
+ }
+ *val = returnval;
+
+ return ret;
+}
+
+int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
+{
int ret;
if (!pdn)
@@ -71,16 +87,8 @@ int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
return PCIBIOS_SET_FAILED;
#endif
- addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
- buid = pdn->phb->buid;
- if (buid) {
- ret = rtas_call(ibm_read_pci_config, 4, 2, &returnval,
- addr, BUID_HI(buid), BUID_LO(buid), size);
- } else {
- ret = rtas_call(read_pci_config, 2, 2, &returnval, addr, size);
- }
- *val = returnval;
-
+ ret = rtas_read_raw_config(pdn->phb->buid, pdn->busno, pdn->devfn,
+ where, size, val);
if (ret)
return PCIBIOS_DEVICE_NOT_FOUND;
@@ -98,18 +106,44 @@ static int rtas_pci_read_config(struct pci_bus *bus,
pdn = pci_get_pdn_by_devfn(bus, devfn);
- /* Validity of pdn is checked in here */
- ret = rtas_read_config(pdn, where, size, val);
- if (*val == EEH_IO_ERROR_VALUE(size) &&
- eeh_dev_check_failure(pdn_to_eeh_dev(pdn)))
- return PCIBIOS_DEVICE_NOT_FOUND;
+ if (pdn && eeh_enabled()) {
+ /* Validity of pdn is checked in here */
+ ret = rtas_read_config(pdn, where, size, val);
+
+ if (*val == EEH_IO_ERROR_VALUE(size) &&
+ eeh_dev_check_failure(pdn_to_eeh_dev(pdn)))
+ ret = PCIBIOS_DEVICE_NOT_FOUND;
+ } else {
+ struct pci_controller *phb = pci_bus_to_host(bus);
+
+ ret = rtas_read_raw_config(phb->buid, bus->number, devfn,
+ where, size, val);
+ }
return ret;
}
+static int rtas_write_raw_config(unsigned long buid, int busno, unsigned int devfn,
+ int where, int size, u32 val)
+{
+ unsigned long addr = rtas_config_addr(busno, devfn, where);
+ int ret;
+
+ if (buid) {
+ ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr,
+ BUID_HI(buid), BUID_LO(buid), size, (ulong)val);
+ } else {
+ ret = rtas_call(write_pci_config, 3, 1, NULL, addr, size, (ulong)val);
+ }
+
+ if (ret)
+ return PCIBIOS_DEVICE_NOT_FOUND;
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val)
{
- unsigned long buid, addr;
int ret;
if (!pdn)
@@ -122,15 +156,8 @@ int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val)
return PCIBIOS_SET_FAILED;
#endif
- addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
- buid = pdn->phb->buid;
- if (buid) {
- ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr,
- BUID_HI(buid), BUID_LO(buid), size, (ulong) val);
- } else {
- ret = rtas_call(write_pci_config, 3, 1, NULL, addr, size, (ulong)val);
- }
-
+ ret = rtas_write_raw_config(pdn->phb->buid, pdn->busno, pdn->devfn,
+ where, size, val);
if (ret)
return PCIBIOS_DEVICE_NOT_FOUND;
@@ -141,12 +168,20 @@ static int rtas_pci_write_config(struct pci_bus *bus,
unsigned int devfn,
int where, int size, u32 val)
{
- struct pci_dn *pdn;
+ struct pci_dn *pdn = pci_get_pdn_by_devfn(bus, devfn);
+ int ret;
- pdn = pci_get_pdn_by_devfn(bus, devfn);
+ if (pdn && eeh_enabled()) {
+ /* Validity of pdn is checked in here. */
+ ret = rtas_write_config(pdn, where, size, val);
+ } else {
+ struct pci_controller *phb = pci_bus_to_host(bus);
- /* Validity of pdn is checked in here. */
- return rtas_write_config(pdn, where, size, val);
+ ret = rtas_write_raw_config(phb->buid, bus->number, devfn,
+ where, size, val);
+ }
+
+ return ret;
}
static struct pci_ops rtas_pci_ops = {
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 13aef2323bbc..3f87a2dc6578 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -654,30 +654,29 @@ static void pnv_pci_config_check_eeh(struct pci_dn *pdn)
}
}
-int pnv_pci_cfg_read(struct pci_dn *pdn,
- int where, int size, u32 *val)
+int pnv_pci_cfg_read_raw(u64 phb_id, int busno, unsigned int devfn,
+ int where, int size, u32 *val)
{
- struct pnv_phb *phb = pdn->phb->private_data;
- u32 bdfn = (pdn->busno << 8) | pdn->devfn;
+ u32 bdfn = (busno << 8) | devfn;
s64 rc;
switch (size) {
case 1: {
u8 v8;
- rc = opal_pci_config_read_byte(phb->opal_id, bdfn, where, &v8);
+ rc = opal_pci_config_read_byte(phb_id, bdfn, where, &v8);
*val = (rc == OPAL_SUCCESS) ? v8 : 0xff;
break;
}
case 2: {
__be16 v16;
- rc = opal_pci_config_read_half_word(phb->opal_id, bdfn, where,
- &v16);
+ rc = opal_pci_config_read_half_word(phb_id, bdfn, where,
+ &v16);
*val = (rc == OPAL_SUCCESS) ? be16_to_cpu(v16) : 0xffff;
break;
}
case 4: {
__be32 v32;
- rc = opal_pci_config_read_word(phb->opal_id, bdfn, where, &v32);
+ rc = opal_pci_config_read_word(phb_id, bdfn, where, &v32);
*val = (rc == OPAL_SUCCESS) ? be32_to_cpu(v32) : 0xffffffff;
break;
}
@@ -686,27 +685,28 @@ int pnv_pci_cfg_read(struct pci_dn *pdn,
}
pr_devel("%s: bus: %x devfn: %x +%x/%x -> %08x\n",
- __func__, pdn->busno, pdn->devfn, where, size, *val);
+ __func__, busno, devfn, where, size, *val);
+
return PCIBIOS_SUCCESSFUL;
}
-int pnv_pci_cfg_write(struct pci_dn *pdn,
- int where, int size, u32 val)
+int pnv_pci_cfg_write_raw(u64 phb_id, int busno, unsigned int devfn,
+ int where, int size, u32 val)
{
- struct pnv_phb *phb = pdn->phb->private_data;
- u32 bdfn = (pdn->busno << 8) | pdn->devfn;
+ u32 bdfn = (busno << 8) | devfn;
pr_devel("%s: bus: %x devfn: %x +%x/%x -> %08x\n",
- __func__, pdn->busno, pdn->devfn, where, size, val);
+ __func__, busno, devfn, where, size, val);
+
switch (size) {
case 1:
- opal_pci_config_write_byte(phb->opal_id, bdfn, where, val);
+ opal_pci_config_write_byte(phb_id, bdfn, where, val);
break;
case 2:
- opal_pci_config_write_half_word(phb->opal_id, bdfn, where, val);
+ opal_pci_config_write_half_word(phb_id, bdfn, where, val);
break;
case 4:
- opal_pci_config_write_word(phb->opal_id, bdfn, where, val);
+ opal_pci_config_write_word(phb_id, bdfn, where, val);
break;
default:
return PCIBIOS_FUNC_NOT_SUPPORTED;
@@ -715,6 +715,24 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
return PCIBIOS_SUCCESSFUL;
}
+int pnv_pci_cfg_read(struct pci_dn *pdn,
+ int where, int size, u32 *val)
+{
+ struct pnv_phb *phb = pdn->phb->private_data;
+
+ return pnv_pci_cfg_read_raw(phb->opal_id, pdn->busno, pdn->devfn,
+ where, size, val);
+}
+
+int pnv_pci_cfg_write(struct pci_dn *pdn,
+ int where, int size, u32 val)
+{
+ struct pnv_phb *phb = pdn->phb->private_data;
+
+ return pnv_pci_cfg_write_raw(phb->opal_id, pdn->busno, pdn->devfn,
+ where, size, val);
+}
+
#if CONFIG_EEH
static bool pnv_pci_cfg_check(struct pci_dn *pdn)
{
@@ -750,13 +768,15 @@ static int pnv_pci_read_config(struct pci_bus *bus,
int where, int size, u32 *val)
{
struct pci_dn *pdn;
- struct pnv_phb *phb;
+ struct pci_controller *hose = pci_bus_to_host(bus);
+ struct pnv_phb *phb = hose->private_data;
int ret;
*val = 0xFFFFFFFF;
pdn = pci_get_pdn_by_devfn(bus, devfn);
if (!pdn)
- return PCIBIOS_DEVICE_NOT_FOUND;
+ return pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
+ where, size, val);
if (!pnv_pci_cfg_check(pdn))
return PCIBIOS_DEVICE_NOT_FOUND;
@@ -779,12 +799,14 @@ static int pnv_pci_write_config(struct pci_bus *bus,
int where, int size, u32 val)
{
struct pci_dn *pdn;
- struct pnv_phb *phb;
+ struct pci_controller *hose = pci_bus_to_host(bus);
+ struct pnv_phb *phb = hose->private_data;
int ret;
pdn = pci_get_pdn_by_devfn(bus, devfn);
if (!pdn)
- return PCIBIOS_DEVICE_NOT_FOUND;
+ return pnv_pci_cfg_write_raw(phb->opal_id, bus->number, devfn,
+ where, size, val);
if (!pnv_pci_cfg_check(pdn))
return PCIBIOS_DEVICE_NOT_FOUND;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 0/5] powerpc/powernv/pci: Discover surprise-hotplugged PCIe devices during rescan
From: Sergey Miroshnichenko @ 2018-09-06 11:57 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux, Sergey Miroshnichenko
This patchset allows hotplugged PCIe devices to be enumerated during a bus
rescan being issued via sysfs on PowerNV platforms, when the "Presence
Detect Changed" interrupt is not available.
As a first part of our work on adding support for hotplugging PCIe bridges
full of devices (without special requirement such as Hot-Plug Controller,
reservation of bus numbers and memory regions by firmware, etc.), this
serie is intended to solve the first of two problems from the list below:
I PowerNV doesn't discover new hotplugged PCIe devices
II EEH is falsely triggered when poking empty slots during the PCIe rescan
III The PCI subsystem is not prepared to runtime changes of BAR addresses
IV Device drivers don't track changes of their BAR addresses
V Allow the moving BARs of working devices to make space for new ones
VI Add support for PCIe bridge hotplug
Tested on:
- POWER8 PowerNV+OPAL ppc64le (our Vesnin server) with pci=realloc;
- POWER8 IBM 8247-42L (pSeries);
- POWER8 IBM 8247-42L (PowerNV+OPAL) with pci=realloc.
Changes since v1:
- Fixed build for ppc64le and ppc64be when CONFIG_PCI_IOV is disabled;
- Fixed build for ppc64e when CONFIG_EEH is disabled;
- Fixed code style warnings.
Sergey Miroshnichenko (5):
powerpc/pci: Access PCI config space directly w/o pci_dn
powerpc/pci: Create pci_dn on demand
powerpc/pci: Use DT to create pci_dn for root bridges only
powerpc/powernv/pci: Enable reassigning the bus numbers
PCI/powerpc/eeh: Add pcibios hooks for preparing to rescan
arch/powerpc/include/asm/eeh.h | 2 +
arch/powerpc/kernel/eeh.c | 12 +++
arch/powerpc/kernel/pci_dn.c | 82 +++++++++++++----
arch/powerpc/kernel/rtas_pci.c | 97 +++++++++++++-------
arch/powerpc/platforms/powernv/eeh-powernv.c | 22 +++++
arch/powerpc/platforms/powernv/pci.c | 65 ++++++++-----
drivers/pci/probe.c | 14 +++
include/linux/pci.h | 2 +
8 files changed, 225 insertions(+), 71 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH v2 6/9] kbuild: consolidate Devicetree dtb build rules
From: Will Deacon @ 2018-09-06 10:05 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, linux-kernel, Masahiro Yamada, Michal Marek,
Vineet Gupta, Russell King, Catalin Marinas, Yoshinori Sato,
Michal Simek, Ralf Baechle, Paul Burton, James Hogan,
Ley Foon Tan, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Chris Zankel, Max Filippov, linux-kbuild,
linux-snps-arc, linux-arm-kernel, uclinux-h8-devel, linux-mips,
nios2-dev, linuxppc-dev, linux-xtensa
In-Reply-To: <20180905235327.5996-7-robh@kernel.org>
On Wed, Sep 05, 2018 at 06:53:24PM -0500, Rob Herring wrote:
> There is nothing arch specific about building dtb files other than their
> location under /arch/*/boot/dts/. Keeping each arch aligned is a pain.
> The dependencies and supported targets are all slightly different.
> Also, a cross-compiler for each arch is needed, but really the host
> compiler preprocessor is perfectly fine for building dtbs. Move the
> build rules to a common location and remove the arch specific ones. This
> is done in a single step to avoid warnings about overriding rules.
>
> The build dependencies had been a mixture of 'scripts' and/or 'prepare'.
> These pull in several dependencies some of which need a target compiler
> (specifically devicetable-offsets.h) and aren't needed to build dtbs.
> All that is really needed is dtc, so adjust the dependencies to only be
> dtc.
>
> This change enables support 'dtbs_install' on some arches which were
> missing the target.
>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Michal Marek <michal.lkml@markovi.net>
> Cc: Vineet Gupta <vgupta@synopsys.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Michal Simek <monstr@monstr.eu>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Paul Burton <paul.burton@mips.com>
> Cc: James Hogan <jhogan@kernel.org>
> Cc: Ley Foon Tan <lftan@altera.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Chris Zankel <chris@zankel.net>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Cc: linux-kbuild@vger.kernel.org
> Cc: linux-snps-arc@lists.infradead.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: uclinux-h8-devel@lists.sourceforge.jp
> Cc: linux-mips@linux-mips.org
> Cc: nios2-dev@lists.rocketboards.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-xtensa@linux-xtensa.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> Please ack so I can take the whole series via the DT tree.
For arm64:
Acked-by: Will Deacon <will.deacon@arm.com>
Will
^ permalink raw reply
* Re: [RFC PATCH v1 00/17] ban the use of _PAGE_XXX flags outside platform specific code
From: Aneesh Kumar K.V @ 2018-09-06 9:58 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, npiggin, aneesh.kumar
Cc: linux-kernel, linuxppc-dev
In-Reply-To: <cover.1535913369.git.christophe.leroy@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Today flags like for instance _PAGE_RW or _PAGE_USER are used through
> common parts of code.
> Using those directly in common parts of code have proven to lead to
> mistakes or misbehaviour, because their use is not always as trivial
> as one could think.
>
> For instance, (flags & _PAGE_USER) == 0 isn't enough to tell
> that a page is a kernel page, because some targets are using
> _PAGE_PRIVILEDGED and not _PAGE_USER, so the test has to be
> (flags & (_PAGE_USER | _PAGE_PRIVILEDGED)) == _PAGE_PRIVILEDGED
> This has to (bad) consequences:
>
> - All targets must define every bit, even the unsupported ones,
> leading to a lot of useless #define _PAGE_XXX 0
> - If someone forgets to take into account all possible _PAGE_XXX bits
> for the case, we can get unexpected behaviour on some targets.
>
> This becomes even more complex when we come to using _PAGE_RW.
> Testing (flags & _PAGE_RW) is not enough to test whether a page
> if writable or not, because:
>
> - Some targets have _PAGE_RO instead, which has to be unset to tell
> a page is writable
> - Some targets have _PAGE_R and _PAGE_W, in which case
> _PAGE_RW = _PAGE_R | _PAGE_W
> - Even knowing whether a page is readable is not always trivial because:
> - Some targets requires to check that _PAGE_R is set to ensure page
> is readable
> - Some targets requires to check that _PAGE_NA is not set
> - Some targets requires to check that _PAGE_RO or _PAGE_RW is set
>
> Etc ....
>
> In order to work around all those issues and minimise the risks of errors,
> this serie aims at removing all use of _PAGE_XXX flags from powerpc code
> and always use pte_xxx() and pte_mkxxx() accessors instead. Those accessors
> are then defined in target specific parts of the kernel code.
The series is really good. It also helps in code readability. Few things
i am not sure there is a way to reduce the overhead
- access = _PAGE_EXEC;
+ access = pte_val(pte_mkexec(__pte(0)));
Considering we have multiple big endian to little endian coversion there
for book3s 64.
Other thing is __ioremap_at where we do
+ pte_t pte = __pte(flags);
/* Make sure we have the base flags */
- if ((flags & _PAGE_PRESENT) == 0)
+ if (!pte_present(pte))
- err = map_kernel_page(v+i, p+i, flags);
+ err = map_kernel_page(v + i, p + i, pte_val(pte));
But otherwise for the series.
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
^ permalink raw reply
* [PATCH kernel] KVM: PPC: Remove redundand permission bits removal
From: Alexey Kardashevskiy @ 2018-09-06 9:51 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, David Gibson, kvm-ppc, Paul Mackerras
The kvmppc_gpa_to_ua() helper itself takes care of the permission
bits in the TCE and yet every single caller removes them.
This changes semantics of kvmppc_gpa_to_ua() so it takes TCEs
(which are GPAs + TCE permission bits) to make the callers simpler.
This should cause no behavioural change.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
This is not related to any bug, just noticed this while doing other things.
I can also rename kvmppc_gpa_to_ua() if it makes more sense, does not it?
---
arch/powerpc/kvm/book3s_64_vio.c | 10 +++-------
arch/powerpc/kvm/book3s_64_vio_hv.c | 16 ++++++----------
2 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index 174299d..7207481 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -378,8 +378,7 @@ static long kvmppc_tce_validate(struct kvmppc_spapr_tce_table *stt,
if (iommu_tce_check_gpa(stt->page_shift, gpa))
return H_TOO_HARD;
- if (kvmppc_gpa_to_ua(stt->kvm, tce & ~(TCE_PCI_READ | TCE_PCI_WRITE),
- &ua, NULL))
+ if (kvmppc_gpa_to_ua(stt->kvm, tce, &ua, NULL))
return H_TOO_HARD;
list_for_each_entry_rcu(stit, &stt->iommu_tables, next) {
@@ -553,8 +552,7 @@ long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
idx = srcu_read_lock(&vcpu->kvm->srcu);
- if ((dir != DMA_NONE) && kvmppc_gpa_to_ua(vcpu->kvm,
- tce & ~(TCE_PCI_READ | TCE_PCI_WRITE), &ua, NULL)) {
+ if ((dir != DMA_NONE) && kvmppc_gpa_to_ua(vcpu->kvm, tce, &ua, NULL)) {
ret = H_PARAMETER;
goto unlock_exit;
}
@@ -647,9 +645,7 @@ long kvmppc_h_put_tce_indirect(struct kvm_vcpu *vcpu,
}
tce = be64_to_cpu(tce);
- if (kvmppc_gpa_to_ua(vcpu->kvm,
- tce & ~(TCE_PCI_READ | TCE_PCI_WRITE),
- &ua, NULL))
+ if (kvmppc_gpa_to_ua(vcpu->kvm, tce, &ua, NULL))
return H_PARAMETER;
list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
index 5f810dc..a03cd93 100644
--- a/arch/powerpc/kvm/book3s_64_vio_hv.c
+++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
@@ -110,8 +110,7 @@ static long kvmppc_rm_tce_validate(struct kvmppc_spapr_tce_table *stt,
if (iommu_tce_check_gpa(stt->page_shift, gpa))
return H_PARAMETER;
- if (kvmppc_gpa_to_ua(stt->kvm, tce & ~(TCE_PCI_READ | TCE_PCI_WRITE),
- &ua, NULL))
+ if (kvmppc_gpa_to_ua(stt->kvm, tce, &ua, NULL))
return H_TOO_HARD;
list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
@@ -180,10 +179,10 @@ void kvmppc_tce_put(struct kvmppc_spapr_tce_table *stt,
}
EXPORT_SYMBOL_GPL(kvmppc_tce_put);
-long kvmppc_gpa_to_ua(struct kvm *kvm, unsigned long gpa,
+long kvmppc_gpa_to_ua(struct kvm *kvm, unsigned long tce,
unsigned long *ua, unsigned long **prmap)
{
- unsigned long gfn = gpa >> PAGE_SHIFT;
+ unsigned long gfn = tce >> PAGE_SHIFT;
struct kvm_memory_slot *memslot;
memslot = search_memslots(kvm_memslots(kvm), gfn);
@@ -191,7 +190,7 @@ long kvmppc_gpa_to_ua(struct kvm *kvm, unsigned long gpa,
return -EINVAL;
*ua = __gfn_to_hva_memslot(memslot, gfn) |
- (gpa & ~(PAGE_MASK | TCE_PCI_READ | TCE_PCI_WRITE));
+ (tce & ~(PAGE_MASK | TCE_PCI_READ | TCE_PCI_WRITE));
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
if (prmap)
@@ -366,8 +365,7 @@ long kvmppc_rm_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
return ret;
dir = iommu_tce_direction(tce);
- if ((dir != DMA_NONE) && kvmppc_gpa_to_ua(vcpu->kvm,
- tce & ~(TCE_PCI_READ | TCE_PCI_WRITE), &ua, NULL))
+ if ((dir != DMA_NONE) && kvmppc_gpa_to_ua(vcpu->kvm, tce, &ua, NULL))
return H_PARAMETER;
entry = ioba >> stt->page_shift;
@@ -520,9 +518,7 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
unsigned long tce = be64_to_cpu(((u64 *)tces)[i]);
ua = 0;
- if (kvmppc_gpa_to_ua(vcpu->kvm,
- tce & ~(TCE_PCI_READ | TCE_PCI_WRITE),
- &ua, NULL))
+ if (kvmppc_gpa_to_ua(vcpu->kvm, tce, &ua, NULL))
return H_PARAMETER;
list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
--
2.11.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox