LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [Patch v4 08/12] memory-hotplug: remove memmap of sparse-vmemmap
From: Jianguo Wu @ 2012-12-04 12:20 UTC (permalink / raw)
  To: Tang Chen
  Cc: linux-s390, linux-ia64, Wen Congyang, linux-acpi, linux-sh,
	Len Brown, x86, linux-kernel, cmetcalf, linux-mm,
	Yasuaki Ishimatsu, paulus, Minchan Kim, KOSAKI Motohiro,
	David Rientjes, sparclinux, Christoph Lameter, linuxppc-dev,
	Andrew Morton, Jiang Liu
In-Reply-To: <50BDBEB7.3070807@cn.fujitsu.com>

Hi Tang,

Thanks for your review and comments, Please see below for my reply.

On 2012/12/4 17:13, Tang Chen wrote:

> Hi Wu,
> 
> Sorry to make noise here. Please see below. :)
> 
> On 12/03/2012 10:23 AM, Jianguo Wu wrote:
>> Signed-off-by: Jianguo Wu<wujianguo@huawei.com>
>> Signed-off-by: Jiang Liu<jiang.liu@huawei.com>
>> ---
>>   include/linux/mm.h  |    1 +
>>   mm/sparse-vmemmap.c |  231 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   mm/sparse.c         |    3 +-
>>   3 files changed, 234 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 5657670..1f26af5 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -1642,6 +1642,7 @@ int vmemmap_populate(struct page *start_page, unsigned long pages, int node);
>>   void vmemmap_populate_print_last(void);
>>   void register_page_bootmem_memmap(unsigned long section_nr, struct page *map,
>>                     unsigned long size);
>> +void vmemmap_free(struct page *memmap, unsigned long nr_pages);
>>
>>   enum mf_flags {
>>       MF_COUNT_INCREASED = 1<<  0,
>> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
>> index 1b7e22a..748732d 100644
>> --- a/mm/sparse-vmemmap.c
>> +++ b/mm/sparse-vmemmap.c
>> @@ -29,6 +29,10 @@
>>   #include<asm/pgalloc.h>
>>   #include<asm/pgtable.h>
>>
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +#include<asm/tlbflush.h>
>> +#endif
>> +
>>   /*
>>    * Allocate a block of memory to be used to back the virtual memory map
>>    * or to back the page tables that are used to create the mapping.
>> @@ -224,3 +228,230 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
>>           vmemmap_buf_end = NULL;
>>       }
>>   }
>> +
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +
>> +#define PAGE_INUSE 0xFD
>> +
>> +static void vmemmap_free_pages(struct page *page, int order)
>> +{
>> +    struct zone *zone;
>> +    unsigned long magic;
>> +
>> +    magic = (unsigned long) page->lru.next;
>> +    if (magic == SECTION_INFO || magic == MIX_SECTION_INFO) {
>> +        put_page_bootmem(page);
>> +
>> +        zone = page_zone(page);
>> +        zone_span_writelock(zone);
>> +        zone->present_pages++;
>> +        zone_span_writeunlock(zone);
>> +        totalram_pages++;
> 
> Seems that we have different ways to handle pages allocated by bootmem
> or by regular allocator. Is the checking way in [PATCH 09/12] available
> here ?
> 
> +    /* bootmem page has reserved flag */
> +    if (PageReserved(page)) {
> ......
> +    }
> 
> If so, I think we can just merge these two functions.

Hmm, direct mapping table isn't allocated by bootmem allocator such as memblock, can't be free by put_page_bootmem().
But I will try to merge these two functions.

> 
>> +    } else
>> +        free_pages((unsigned long)page_address(page), order);
>> +}
>> +
>> +static void free_pte_table(pmd_t *pmd)
>> +{
>> +    pte_t *pte, *pte_start;
>> +    int i;
>> +
>> +    pte_start = (pte_t *)pmd_page_vaddr(*pmd);
>> +    for (i = 0; i<  PTRS_PER_PTE; i++) {
>> +        pte = pte_start + i;
>> +        if (pte_val(*pte))
>> +            return;
>> +    }
>> +
>> +    /* free a pte talbe */
>> +    vmemmap_free_pages(pmd_page(*pmd), 0);
>> +    spin_lock(&init_mm.page_table_lock);
>> +    pmd_clear(pmd);
>> +    spin_unlock(&init_mm.page_table_lock);
>> +}
>> +
>> +static void free_pmd_table(pud_t *pud)
>> +{
>> +    pmd_t *pmd, *pmd_start;
>> +    int i;
>> +
>> +    pmd_start = (pmd_t *)pud_page_vaddr(*pud);
>> +    for (i = 0; i<  PTRS_PER_PMD; i++) {
>> +        pmd = pmd_start + i;
>> +        if (pmd_val(*pmd))
>> +            return;
>> +    }
>> +
>> +    /* free a pmd talbe */
>> +    vmemmap_free_pages(pud_page(*pud), 0);
>> +    spin_lock(&init_mm.page_table_lock);
>> +    pud_clear(pud);
>> +    spin_unlock(&init_mm.page_table_lock);
>> +}
>> +
>> +static void free_pud_table(pgd_t *pgd)
>> +{
>> +    pud_t *pud, *pud_start;
>> +    int i;
>> +
>> +    pud_start = (pud_t *)pgd_page_vaddr(*pgd);
>> +    for (i = 0; i<  PTRS_PER_PUD; i++) {
>> +        pud = pud_start + i;
>> +        if (pud_val(*pud))
>> +            return;
>> +    }
>> +
>> +    /* free a pud table */
>> +    vmemmap_free_pages(pgd_page(*pgd), 0);
>> +    spin_lock(&init_mm.page_table_lock);
>> +    pgd_clear(pgd);
>> +    spin_unlock(&init_mm.page_table_lock);
>> +}
> 
> All the free_xxx_table() are very similar to the functions in
> [PATCH 09/12]. Could we reuse them anyway ?

yes, we can reuse them.

> 
>> +
>> +static int split_large_page(pte_t *kpte, unsigned long address, pte_t *pbase)
>> +{
>> +    struct page *page = pmd_page(*(pmd_t *)kpte);
>> +    int i = 0;
>> +    unsigned long magic;
>> +    unsigned long section_nr;
>> +
>> +    __split_large_page(kpte, address, pbase);
> 
> Is this patch going to replace [PATCH 08/12] ?
> 

I wish to replace [PATCH 08/12], but need Congyang and Yasuaki to confirm first:)

> If so, __split_large_page() was added and exported in [PATCH 09/12],
> then we should move it here, right ?

yes.

and what do you think about moving vmemmap_pud[pmd/pte]_remove() to arch/x86/mm/init_64.c,
to be consistent with vmemmap_populate() ?

I will rework [PATCH 08/12] and [PATCH 09/12] soon.

Thanks,
Jianguo Wu.

> 
> If not, free_map_bootmem() and __kfree_section_memmap() were changed in
> [PATCH 08/12], and we need to handle this.
> 
>> +    __flush_tlb_all();
>> +
>> +    magic = (unsigned long) page->lru.next;
>> +    if (magic == SECTION_INFO) {
>> +        section_nr = pfn_to_section_nr(page_to_pfn(page));
>> +        while (i<  PTRS_PER_PMD) {
>> +            page++;
>> +            i++;
>> +            get_page_bootmem(section_nr, page, SECTION_INFO);
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void vmemmap_pte_remove(pmd_t *pmd, unsigned long addr, unsigned long end)
>> +{
>> +    pte_t *pte;
>> +    unsigned long next;
>> +    void *page_addr;
>> +
>> +    pte = pte_offset_kernel(pmd, addr);
>> +    for (; addr<  end; pte++, addr += PAGE_SIZE) {
>> +        next = (addr + PAGE_SIZE)&  PAGE_MASK;
>> +        if (next>  end)
>> +            next = end;
>> +
>> +        if (pte_none(*pte))
>> +            continue;
>> +        if (IS_ALIGNED(addr, PAGE_SIZE)&&
>> +            IS_ALIGNED(next, PAGE_SIZE)) {
>> +            vmemmap_free_pages(pte_page(*pte), 0);
>> +            spin_lock(&init_mm.page_table_lock);
>> +            pte_clear(&init_mm, addr, pte);
>> +            spin_unlock(&init_mm.page_table_lock);
>> +        } else {
>> +            /*
>> +             * Removed page structs are filled with 0xFD.
>> +             */
>> +            memset((void *)addr, PAGE_INUSE, next - addr);
>> +            page_addr = page_address(pte_page(*pte));
>> +
>> +            if (!memchr_inv(page_addr, PAGE_INUSE, PAGE_SIZE)) {
>> +                spin_lock(&init_mm.page_table_lock);
>> +                pte_clear(&init_mm, addr, pte);
>> +                spin_unlock(&init_mm.page_table_lock);
>> +            }
>> +        }
>> +    }
>> +
>> +    free_pte_table(pmd);
>> +    __flush_tlb_all();
>> +}
>> +
>> +static void vmemmap_pmd_remove(pud_t *pud, unsigned long addr, unsigned long end)
>> +{
>> +    unsigned long next;
>> +    pmd_t *pmd;
>> +
>> +    pmd = pmd_offset(pud, addr);
>> +    for (; addr<  end; addr = next, pmd++) {
>> +        next = pmd_addr_end(addr, end);
>> +        if (pmd_none(*pmd))
>> +            continue;
>> +
>> +        if (cpu_has_pse) {
>> +            unsigned long pte_base;
>> +
>> +            if (IS_ALIGNED(addr, PMD_SIZE)&&
>> +                IS_ALIGNED(next, PMD_SIZE)) {
>> +                vmemmap_free_pages(pmd_page(*pmd),
>> +                           get_order(PMD_SIZE));
>> +                spin_lock(&init_mm.page_table_lock);
>> +                pmd_clear(pmd);
>> +                spin_unlock(&init_mm.page_table_lock);
>> +                continue;
>> +            }
>> +
>> +            /*
>> +             * We use 2M page, but we need to remove part of them,
>> +             * so split 2M page to 4K page.
>> +             */
>> +            pte_base = get_zeroed_page(GFP_ATOMIC | __GFP_NOTRACK);
>> +            if (!pte_base) {
>> +                WARN_ON(1);
>> +                continue;
>> +            }
>> +
>> +            split_large_page((pte_t *)pmd, addr, (pte_t *)pte_base);
>> +            __flush_tlb_all();
>> +
>> +            spin_lock(&init_mm.page_table_lock);
>> +            pmd_populate_kernel(&init_mm, pmd, (pte_t *)pte_base);
>> +            spin_unlock(&init_mm.page_table_lock);
>> +        }
>> +
>> +        vmemmap_pte_remove(pmd, addr, next);
>> +    }
>> +
>> +    free_pmd_table(pud);
>> +    __flush_tlb_all();
>> +}
>> +
>> +static void vmemmap_pud_remove(pgd_t *pgd, unsigned long addr, unsigned long end)
>> +{
>> +    unsigned long next;
>> +    pud_t *pud;
>> +
>> +    pud = pud_offset(pgd, addr);
>> +    for (; addr<  end; addr = next, pud++) {
>> +        next = pud_addr_end(addr, end);
>> +        if (pud_none(*pud))
>> +            continue;
>> +
>> +        vmemmap_pmd_remove(pud, addr, next);
>> +    }
>> +
>> +    free_pud_table(pgd);
>> +    __flush_tlb_all();
>> +}
>> +
>> +void vmemmap_free(struct page *memmap, unsigned long nr_pages)
>> +{
>> +    unsigned long addr = (unsigned long)memmap;
>> +    unsigned long end = (unsigned long)(memmap + nr_pages);
>> +    unsigned long next;
>> +
>> +    for (; addr<  end; addr = next) {
>> +        pgd_t *pgd = pgd_offset_k(addr);
>> +
>> +        next = pgd_addr_end(addr, end);
>> +        if (!pgd_present(*pgd))
>> +            continue;
>> +
>> +        vmemmap_pud_remove(pgd, addr, next);
>> +        sync_global_pgds(addr, next - 1);
>> +    }
>> +}
>> +#endif
>> diff --git a/mm/sparse.c b/mm/sparse.c
>> index fac95f2..4060229 100644
>> --- a/mm/sparse.c
>> +++ b/mm/sparse.c
>> @@ -615,10 +615,11 @@ static inline struct page *kmalloc_section_memmap(unsigned long pnum, int nid,
>>   }
>>   static void __kfree_section_memmap(struct page *memmap, unsigned long nr_pages)
>>   {
>> -    return; /* XXX: Not implemented yet */
>> +    vmemmap_free(memmap, nr_pages);
>>   }
>>   static void free_map_bootmem(struct page *page, unsigned long nr_pages)
> 
> In the latest kernel, this line was:
> static void free_map_bootmem(struct page *memmap, unsigned long nr_pages)
> 
>>   {
>> +    vmemmap_free(page, nr_pages);
>>   }
>>   #else
>>   static struct page *__kmalloc_section_memmap(unsigned long nr_pages)
> 
> 
> .
> 

^ permalink raw reply

* RE: [PATCH 4/4 v6] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
From: Sethi Varun-B16395 @ 2012-12-04 12:11 UTC (permalink / raw)
  To: Tabi Timur-B04825
  Cc: joerg.roedel@amd.com, iommu@lists.linux-foundation.org,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Wood Scott-B07421
In-Reply-To: <50BD056F.7070701@freescale.com>



> -----Original Message-----
> From: Tabi Timur-B04825
> Sent: Tuesday, December 04, 2012 1:33 AM
> To: Sethi Varun-B16395
> Cc: joerg.roedel@amd.com; iommu@lists.linux-foundation.org; linuxppc-
> dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Wood Scott-B07421
> Subject: Re: [PATCH 4/4 v6] iommu/fsl: Freescale PAMU driver and IOMMU
> API implementation.
>=20
> Varun Sethi wrote:
>=20
> > +	out_be32(&pamu_regs->ppbah, ((u64)ppaact_phys) >> 32);
> > +	out_be32(&pamu_regs->ppbal, ppaact_phys);
> > +	ppaact_phys =3D ppaact_phys + PAACT_SIZE;
> > +	out_be32(&pamu_regs->pplah, ((u64)ppaact_phys) >> 32);
> > +	out_be32(&pamu_regs->pplal, ppaact_phys);
>=20
> Instead of  ((u64)ppaact_phys) >> 32, use upper_32_bits() and
> lower_32_bits().
>=20
> > +#define PAACE_NUMBER_ENTRIES    0xFF
>=20
> This is going to break with large LIODNs.  Instead of hard-coding the
> size of the PPAACT, you need to scan the device tree for the largest
> LIODN, and make the array dynamically sized.
This would in any case change with the new LIODN allocation scheme. I inten=
d on introducing the new scheme as a separate patch.

-Varun

^ permalink raw reply

* RE: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver.
From: Sethi Varun-B16395 @ 2012-12-04 11:53 UTC (permalink / raw)
  To: Wood Scott-B07421
  Cc: Tabi Timur-B04825, Joerg Roedel, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org
In-Reply-To: <1354554223.2960.7@snotra>



> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Monday, December 03, 2012 10:34 PM
> To: Sethi Varun-B16395
> Cc: Joerg Roedel; linux-kernel@vger.kernel.org; iommu@lists.linux-
> foundation.org; Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; Tabi
> Timur-B04825
> Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes
> required by fsl PAMU driver.
>=20
> On 12/03/2012 10:57:29 AM, Sethi Varun-B16395 wrote:
> >
> >
> > > -----Original Message-----
> > > From: iommu-bounces@lists.linux-foundation.org [mailto:iommu-
> > > bounces@lists.linux-foundation.org] On Behalf Of Joerg Roedel
> > > Sent: Sunday, December 02, 2012 7:33 PM
> > > To: Sethi Varun-B16395
> > > Cc: linux-kernel@vger.kernel.org; iommu@lists.linux-foundation.org;
> > Wood
> > > Scott-B07421; linuxppc-dev@lists.ozlabs.org; Tabi Timur-B04825
> > > Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes
> > > required by fsl PAMU driver.
> > >
> > > Hmm, we need to work out a good abstraction for this.
> > >
> > > On Tue, Nov 20, 2012 at 07:24:56PM +0530, Varun Sethi wrote:
> > > > Added the following domain attributes required by FSL PAMU driver:
> > > > 1. Subwindows field added to the iommu domain geometry attribute.
> > >
> > > Are the Subwindows mapped with full size or do you map only parts
> > of the
> > > subwindows?
> > >
> > [Sethi Varun-B16395] It's possible to map a part of the subwindow i.e.
> > size of the mapping can be less than the sub window size.
> >
> > > > +	 * This attribute indicates number of DMA subwindows
> supported
> > by
> > > > +	 * the geometry. If there is a single window that maps the
> > entire
> > > > +	 * geometry, attribute must be set to "1". A value of "0"
> > implies
> > > > +	 * that this mechanism is not used at all(normal paging is
> > used).
> > > > +	 * Value other than* "0" or "1" indicates the actual number
> of
> > > > +	 * subwindows.
> > > > +	 */
> > >
> > > This semantic is ugly, how about a feature detection mechanism?
> > >
> > [Sethi Varun-B16395] A feature mechanism to query the type of IOMMU?
>=20
> A feature mechanism to determine whether this subwindow mechanism is
> available, and what the limits are.
>=20
So, we use the IOMMU capability interface to find out if IOMMU supports sub=
 windows or not, right? But still number of sub windows would be specified =
as a part of the geometry and the valid value for sub windows would  0,1 or=
 actual number of sub windows.

-Varun
=20

^ permalink raw reply

* Re: [PATCH 2/4] powerpc: enable ARCH_USE_BUILTIN_BSWAP
From: Stephen Rothwell @ 2012-12-04 11:02 UTC (permalink / raw)
  To: David Woodhouse; +Cc: linux-arch, x86, linuxppc-dev
In-Reply-To: <1354616130-21587-3-git-send-email-dwmw2@infradead.org>

[-- Attachment #1: Type: text/plain, Size: 605 bytes --]

Hi David,

On Tue,  4 Dec 2012 10:15:28 +0000 David Woodhouse <dwmw2@infradead.org> wrote:
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index a902a5c..b4ea516 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -78,6 +78,9 @@ config ARCH_HAS_ILOG2_U64
>  	bool
>  	default y if 64BIT
>  
> +config ARCH_USE_BUILTIN_BSWAP
> +       def_bool y
> +

This should be defined as bool in arch/Kconfig (probably in the previous
patch) and then selected from appropriate architectures.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [Patch v4 03/12] memory-hotplug: remove redundant codes
From: Tang Chen @ 2012-12-04 10:31 UTC (permalink / raw)
  To: Wen Congyang
  Cc: linux-s390, linux-ia64, Len Brown, linux-acpi, linux-sh, x86,
	linux-kernel, cmetcalf, Jianguo Wu, linux-mm, Yasuaki Ishimatsu,
	paulus, Minchan Kim, KOSAKI Motohiro, David Rientjes, sparclinux,
	Christoph Lameter, linuxppc-dev, Andrew Morton, Jiang Liu
In-Reply-To: <50BDC0DE.4010103@cn.fujitsu.com>

On 12/04/2012 05:22 PM, Tang Chen wrote:
> On 11/27/2012 06:00 PM, Wen Congyang wrote:
>> offlining memory blocks and checking whether memory blocks are offlined
>> are very similar. This patch introduces a new function to remove
>> redundant codes.
>>
>> CC: David Rientjes<rientjes@google.com>
>> CC: Jiang Liu<liuj97@gmail.com>
>> CC: Len Brown<len.brown@intel.com>
>> CC: Christoph Lameter<cl@linux.com>
>> Cc: Minchan Kim<minchan.kim@gmail.com>
>> CC: Andrew Morton<akpm@linux-foundation.org>
>> CC: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com>
>> CC: Yasuaki Ishimatsu<isimatu.yasuaki@jp.fujitsu.com>
>> Signed-off-by: Wen Congyang<wency@cn.fujitsu.com>
>
> Can we merge this patch with [PATCH 03/12] ?

Sorry, I think we can merge this patch into [PATCH 02/12].
Thanks. :)

>
> Reviewed-by: Tang Chen <tangchen@cn.fujitsu.com>
>

^ permalink raw reply

* [PATCH 2/4] powerpc: enable ARCH_USE_BUILTIN_BSWAP
From: David Woodhouse @ 2012-12-04 10:15 UTC (permalink / raw)
  To: dwmw2; +Cc: linux-arch, x86, linuxppc-dev
In-Reply-To: <1354616130-21587-1-git-send-email-dwmw2@infradead.org>

From: David Woodhouse <David.Woodhouse@intel.com>

By using the compiler intrinsics instead of hand-crafted opaque inline
assembler for byte-swapping, we let the compiler see what's actually
happening and it gets to use lwbrx/stwbrx instructions instead of a
normal load/store coupled with a sequence of rlwimi instructions to
move bits around.

Compiled-tested only. It gave a code size reduction of almost 4% for
ext2, and more like 2.5% for ext3/ext4.

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 arch/powerpc/Kconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index a902a5c..b4ea516 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -78,6 +78,9 @@ config ARCH_HAS_ILOG2_U64
 	bool
 	default y if 64BIT
 
+config ARCH_USE_BUILTIN_BSWAP
+       def_bool y
+
 config GENERIC_HWEIGHT
 	bool
 	default y
-- 
1.8.0

^ permalink raw reply related

* [PATCH 1/4] byteorder: allow arch to opt to use GCC intrinsics for byteswapping
From: David Woodhouse @ 2012-12-04 10:15 UTC (permalink / raw)
  To: dwmw2; +Cc: linux-arch, x86, linuxppc-dev
In-Reply-To: <1354616130-21587-1-git-send-email-dwmw2@infradead.org>

From: David Woodhouse <David.Woodhouse@intel.com>

Since GCC 4.4, there have been __builtin_bswap32() and __builtin_bswap16()
intrinsics. A __builtin_bswap16() came a little later (4.6 for PowerPC,
48 for other platforms).

By using these instead of the inline assembler that most architectures
have in their __arch_swabXX() macros, we let the compiler see what's
actually happening. The resulting code should be at least as good, and
much *better* in the cases where it can be combined with a nearby load
or store, using a load-and-byteswap or store-and-byteswap instruction
(e.g. lwbrx/stwbrx on PowerPC, movbe on Atom).

When GCC is sufficiently recent *and* the architecture opts in to using
the intrinsics by setting CONFIG_ARCH_USE_BUILTIN_BSWAP, they will be
used in preference to the __arch_swabXX() macros. An architecture which
does not set ARCH_USE_BUILTIN_BSWAP will continue to use its own
hand-crafted macros.

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 include/linux/compiler-gcc4.h  | 10 ++++++++++
 include/linux/compiler-intel.h |  7 +++++++
 include/uapi/linux/swab.h      | 12 +++++++++---
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
index 412bc6c..dc16a85 100644
--- a/include/linux/compiler-gcc4.h
+++ b/include/linux/compiler-gcc4.h
@@ -63,3 +63,13 @@
 #define __compiletime_warning(message) __attribute__((warning(message)))
 #define __compiletime_error(message) __attribute__((error(message)))
 #endif
+
+#ifdef CONFIG_ARCH_USE_BUILTIN_BSWAP
+#if __GNUC_MINOR__ >= 4
+#define __HAVE_BUILTIN_BSWAP32__
+#define __HAVE_BUILTIN_BSWAP64__
+#endif
+#if __GNUC_MINOR__ >= 8 || (defined(__powerpc__) && __GNUC_MINOR__ >= 6)
+#define __HAVE_BUILTIN_BSWAP16__
+#endif
+#endif
diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h
index d8e636e..973ce10 100644
--- a/include/linux/compiler-intel.h
+++ b/include/linux/compiler-intel.h
@@ -29,3 +29,10 @@
 #endif
 
 #define uninitialized_var(x) x
+
+#ifndef __HAVE_BUILTIN_BSWAP16__
+/* icc has this, but it's called _bswap16 */
+#define __HAVE_BUILTIN_BSWAP16__
+#define __builtin_bswap16 _bswap16
+#endif
+
diff --git a/include/uapi/linux/swab.h b/include/uapi/linux/swab.h
index e811474..0e011eb 100644
--- a/include/uapi/linux/swab.h
+++ b/include/uapi/linux/swab.h
@@ -45,7 +45,9 @@
 
 static inline __attribute_const__ __u16 __fswab16(__u16 val)
 {
-#ifdef __arch_swab16
+#ifdef __HAVE_BUILTIN_BSWAP16__
+	return __builtin_bswap16(val);
+#elif defined (__arch_swab16)
 	return __arch_swab16(val);
 #else
 	return ___constant_swab16(val);
@@ -54,7 +56,9 @@ static inline __attribute_const__ __u16 __fswab16(__u16 val)
 
 static inline __attribute_const__ __u32 __fswab32(__u32 val)
 {
-#ifdef __arch_swab32
+#ifdef __HAVE_BUILTIN_BSWAP32__
+	return __builtin_bswap32(val);
+#elif defined(__arch_swab32)
 	return __arch_swab32(val);
 #else
 	return ___constant_swab32(val);
@@ -63,7 +67,9 @@ static inline __attribute_const__ __u32 __fswab32(__u32 val)
 
 static inline __attribute_const__ __u64 __fswab64(__u64 val)
 {
-#ifdef __arch_swab64
+#ifdef __HAVE_BUILTIN_BSWAP64__
+	return __builtin_bswap64(val);
+#elif defined (__arch_swab64)
 	return __arch_swab64(val);
 #elif defined(__SWAB_64_THRU_32__)
 	__u32 h = val >> 32;
-- 
1.8.0

^ permalink raw reply related

* [PATCH 4/4] x86: add CONFIG_X86_MOVBE option
From: David Woodhouse @ 2012-12-04 10:15 UTC (permalink / raw)
  To: dwmw2; +Cc: linux-arch, x86, linuxppc-dev
In-Reply-To: <1354616130-21587-1-git-send-email-dwmw2@infradead.org>

From: David Woodhouse <David.Woodhouse@intel.com>

Currently depends only on CONFIG_MATOM. This will change because big-core
CPUs are getting movbe too...

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 arch/x86/Kconfig.cpu | 4 ++++
 arch/x86/Makefile    | 1 +
 2 files changed, 5 insertions(+)

diff --git a/arch/x86/Kconfig.cpu b/arch/x86/Kconfig.cpu
index f3b86d0..969f7a6 100644
--- a/arch/x86/Kconfig.cpu
+++ b/arch/x86/Kconfig.cpu
@@ -353,6 +353,10 @@ config X86_BSWAP
 	def_bool y
 	depends on X86_32 && !M386
 
+config X86_MOVBE
+	def_bool y
+	depends on MATOM
+
 config X86_POPAD_OK
 	def_bool y
 	depends on X86_32 && !M386
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 05afcca..0e71d76 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -64,6 +64,7 @@ else
                 $(call cc-option,-march=core2,$(call cc-option,-mtune=generic))
 	cflags-$(CONFIG_MATOM) += $(call cc-option,-march=atom) \
 		$(call cc-option,-mtune=atom,$(call cc-option,-mtune=generic))
+        cflags-$(CONFIG_X86_MOVBE) += $(call cc-option,-mmovbe)
         cflags-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=generic)
         KBUILD_CFLAGS += $(cflags-y)
 
-- 
1.8.0

^ permalink raw reply related

* [PATCH 0/4] Use compiler intrinsics for byteswapping
From: David Woodhouse @ 2012-12-04 10:15 UTC (permalink / raw)
  To: dwmw2; +Cc: linux-arch, x86, linuxppc-dev

This series of patches enables the __builtin_bswapXX() series of functions
that have been supported since GCC 4.4. It allows GCC to emit load-and-swap
or store-and-swap instructions on architectures which support that.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

^ permalink raw reply

* [PATCH 3/4] x86: enable ARCH_USE_BUILTIN_BSWAP
From: David Woodhouse @ 2012-12-04 10:15 UTC (permalink / raw)
  To: dwmw2; +Cc: linux-arch, x86, linuxppc-dev
In-Reply-To: <1354616130-21587-1-git-send-email-dwmw2@infradead.org>

From: David Woodhouse <David.Woodhouse@intel.com>

With -mmovbe enabled (implicit with -march=atom), this allows the
compiler to use the movbe instruction. This doesn't have a significant
effect on code size (unlike on PowerPC), because the movbe instruction
actually takes as many bytes to encode as a simple mov and a bswap. But
for Atom in particular I believe it should give a performance win over
the mov+bswap alternative. That was kind of why movbe was invented in
the first place, after all...

I've done basic functionality testing with IPv6 and Legacy IP, but no
performance testing. The EFI firmware on my test box unfortunately no
longer starts up.

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 arch/x86/Kconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 46c3bff..238f2ea 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -194,6 +194,9 @@ config ARCH_HAS_CACHE_LINE_SIZE
 config ARCH_HAS_CPU_AUTOPROBE
 	def_bool y
 
+config ARCH_USE_BUILTIN_BSWAP
+	def_bool y
+
 config HAVE_SETUP_PER_CPU_AREA
 	def_bool y
 
-- 
1.8.0

^ permalink raw reply related

* Re: [Patch v4 12/12] memory-hotplug: free node_data when a node is offlined
From: Tang Chen @ 2012-12-04 10:10 UTC (permalink / raw)
  To: Wen Congyang
  Cc: linux-s390, linux-ia64, Len Brown, linux-acpi, linux-sh, x86,
	linux-kernel, cmetcalf, Jianguo Wu, linux-mm, Yasuaki Ishimatsu,
	paulus, Minchan Kim, KOSAKI Motohiro, David Rientjes, sparclinux,
	Christoph Lameter, linuxppc-dev, Andrew Morton, Jiang Liu
In-Reply-To: <1354010422-19648-13-git-send-email-wency@cn.fujitsu.com>

On 11/27/2012 06:00 PM, Wen Congyang wrote:
> We call hotadd_new_pgdat() to allocate memory to store node_data. So we
> should free it when removing a node.
>
> CC: David Rientjes<rientjes@google.com>
> CC: Jiang Liu<liuj97@gmail.com>
> CC: Len Brown<len.brown@intel.com>
> CC: Benjamin Herrenschmidt<benh@kernel.crashing.org>
> CC: Paul Mackerras<paulus@samba.org>
> CC: Christoph Lameter<cl@linux.com>
> Cc: Minchan Kim<minchan.kim@gmail.com>
> CC: Andrew Morton<akpm@linux-foundation.org>
> CC: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com>
> CC: Yasuaki Ishimatsu<isimatu.yasuaki@jp.fujitsu.com>
> Signed-off-by: Wen Congyang<wency@cn.fujitsu.com>

Reviewed-by: Tang Chen <tangchen@cn.fujitsu.com>

> ---
>   mm/memory_hotplug.c | 20 +++++++++++++++++++-
>   1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 449663e..d1451ab 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1309,9 +1309,12 @@ static int check_cpu_on_node(void *data)
>   /* offline the node if all memory sections of this node are removed */
>   static void try_offline_node(int nid)
>   {
> +	pg_data_t *pgdat = NODE_DATA(nid);
>   	unsigned long start_pfn = NODE_DATA(nid)->node_start_pfn;
> -	unsigned long end_pfn = start_pfn + NODE_DATA(nid)->node_spanned_pages;
> +	unsigned long end_pfn = start_pfn + pgdat->node_spanned_pages;
>   	unsigned long pfn;
> +	struct page *pgdat_page = virt_to_page(pgdat);
> +	int i;
>
>   	for (pfn = start_pfn; pfn<  end_pfn; pfn += PAGES_PER_SECTION) {
>   		unsigned long section_nr = pfn_to_section_nr(pfn);
> @@ -1338,6 +1341,21 @@ static void try_offline_node(int nid)
>   	 */
>   	node_set_offline(nid);
>   	unregister_one_node(nid);
> +
> +	if (!PageSlab(pgdat_page)&&  !PageCompound(pgdat_page))
> +		/* node data is allocated from boot memory */
> +		return;
> +
> +	/* free waittable in each zone */
> +	for (i = 0; i<  MAX_NR_ZONES; i++) {
> +		struct zone *zone = pgdat->node_zones + i;
> +
> +		if (zone->wait_table)
> +			vfree(zone->wait_table);
> +	}
> +
> +	arch_refresh_nodedata(nid, NULL);
> +	arch_free_nodedata(pgdat);
>   }
>
>   int __ref remove_memory(int nid, u64 start, u64 size)

^ permalink raw reply

* Re: [Patch v4 11/12] memory-hotplug: remove sysfs file of node
From: Tang Chen @ 2012-12-04 10:10 UTC (permalink / raw)
  To: Wen Congyang
  Cc: linux-s390, linux-ia64, Len Brown, linux-acpi, linux-sh, x86,
	linux-kernel, cmetcalf, Jianguo Wu, linux-mm, Yasuaki Ishimatsu,
	paulus, Minchan Kim, KOSAKI Motohiro, David Rientjes, sparclinux,
	Christoph Lameter, linuxppc-dev, Andrew Morton, Jiang Liu
In-Reply-To: <1354010422-19648-12-git-send-email-wency@cn.fujitsu.com>

On 11/27/2012 06:00 PM, Wen Congyang wrote:
> This patch introduces a new function try_offline_node() to
> remove sysfs file of node when all memory sections of this
> node are removed. If some memory sections of this node are
> not removed, this function does nothing.
>
> CC: David Rientjes<rientjes@google.com>
> CC: Jiang Liu<liuj97@gmail.com>
> CC: Len Brown<len.brown@intel.com>
> CC: Christoph Lameter<cl@linux.com>
> Cc: Minchan Kim<minchan.kim@gmail.com>
> CC: Andrew Morton<akpm@linux-foundation.org>
> CC: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com>
> CC: Yasuaki Ishimatsu<isimatu.yasuaki@jp.fujitsu.com>
> Signed-off-by: Wen Congyang<wency@cn.fujitsu.com>

Reviewed-by: Tang Chen <tangchen@cn.fujitsu.com>

> ---
>   drivers/acpi/acpi_memhotplug.c |  8 +++++-
>   include/linux/memory_hotplug.h |  2 +-
>   mm/memory_hotplug.c            | 58 ++++++++++++++++++++++++++++++++++++++++--
>   3 files changed, 64 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index 24c807f..0780f99 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -310,7 +310,9 @@ static int acpi_memory_disable_device(struct acpi_memory_device *mem_device)
>   {
>   	int result;
>   	struct acpi_memory_info *info, *n;
> +	int node;
>
> +	node = acpi_get_node(mem_device->device->handle);
>
>   	/*
>   	 * Ask the VM to offline this memory range.
> @@ -318,7 +320,11 @@ static int acpi_memory_disable_device(struct acpi_memory_device *mem_device)
>   	 */
>   	list_for_each_entry_safe(info, n,&mem_device->res_list, list) {
>   		if (info->enabled) {
> -			result = remove_memory(info->start_addr, info->length);
> +			if (node<  0)
> +				node = memory_add_physaddr_to_nid(
> +					info->start_addr);
> +			result = remove_memory(node, info->start_addr,
> +				info->length);
>   			if (result)
>   				return result;
>   		}
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index d4c4402..7b4cfe6 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -231,7 +231,7 @@ extern int arch_add_memory(int nid, u64 start, u64 size);
>   extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
>   extern int offline_memory_block(struct memory_block *mem);
>   extern bool is_memblock_offlined(struct memory_block *mem);
> -extern int remove_memory(u64 start, u64 size);
> +extern int remove_memory(int node, u64 start, u64 size);
>   extern int sparse_add_one_section(struct zone *zone, unsigned long start_pfn,
>   								int nr_pages);
>   extern void sparse_remove_one_section(struct zone *zone, struct mem_section *ms);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index aa97d56..449663e 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -29,6 +29,7 @@
>   #include<linux/suspend.h>
>   #include<linux/mm_inline.h>
>   #include<linux/firmware-map.h>
> +#include<linux/stop_machine.h>
>
>   #include<asm/tlbflush.h>
>
> @@ -1288,7 +1289,58 @@ static int is_memblock_offlined_cb(struct memory_block *mem, void *arg)
>   	return ret;
>   }
>
> -int __ref remove_memory(u64 start, u64 size)
> +static int check_cpu_on_node(void *data)
> +{
> +	struct pglist_data *pgdat = data;
> +	int cpu;
> +
> +	for_each_present_cpu(cpu) {
> +		if (cpu_to_node(cpu) == pgdat->node_id)
> +			/*
> +			 * the cpu on this node isn't removed, and we can't
> +			 * offline this node.
> +			 */
> +			return -EBUSY;
> +	}
> +
> +	return 0;
> +}
> +
> +/* offline the node if all memory sections of this node are removed */
> +static void try_offline_node(int nid)
> +{
> +	unsigned long start_pfn = NODE_DATA(nid)->node_start_pfn;
> +	unsigned long end_pfn = start_pfn + NODE_DATA(nid)->node_spanned_pages;
> +	unsigned long pfn;
> +
> +	for (pfn = start_pfn; pfn<  end_pfn; pfn += PAGES_PER_SECTION) {
> +		unsigned long section_nr = pfn_to_section_nr(pfn);
> +
> +		if (!present_section_nr(section_nr))
> +			continue;
> +
> +		if (pfn_to_nid(pfn) != nid)
> +			continue;
> +
> +		/*
> +		 * some memory sections of this node are not removed, and we
> +		 * can't offline node now.
> +		 */
> +		return;
> +	}
> +
> +	if (stop_machine(check_cpu_on_node, NODE_DATA(nid), NULL))
> +		return;
> +
> +	/*
> +	 * all memory/cpu of this node are removed, we can offline this
> +	 * node now.
> +	 */
> +	node_set_offline(nid);
> +	unregister_one_node(nid);
> +}
> +
> +int __ref remove_memory(int nid, u64 start, u64 size)
>   {
>   	unsigned long start_pfn, end_pfn;
>   	int ret = 0;
> @@ -1335,6 +1387,8 @@ repeat:
>
>   	arch_remove_memory(start, size);
>
> +	try_offline_node(nid);
> +
>   	unlock_memory_hotplug();
>
>   	return 0;
> @@ -1344,7 +1398,7 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages)
>   {
>   	return -EINVAL;
>   }
> -int remove_memory(u64 start, u64 size)
> +int remove_memory(int nid, u64 start, u64 size)
>   {
>   	return -EINVAL;
>   }

^ permalink raw reply

* Re: [Patch v4 10/12] memory-hotplug: memory_hotplug: clear zone when removing the memory
From: Tang Chen @ 2012-12-04 10:09 UTC (permalink / raw)
  To: Wen Congyang
  Cc: linux-s390, linux-ia64, Len Brown, linux-acpi, linux-sh, x86,
	linux-kernel, cmetcalf, Jianguo Wu, linux-mm, Yasuaki Ishimatsu,
	paulus, Minchan Kim, KOSAKI Motohiro, David Rientjes, sparclinux,
	Christoph Lameter, linuxppc-dev, Andrew Morton, Jiang Liu
In-Reply-To: <1354010422-19648-11-git-send-email-wency@cn.fujitsu.com>

On 11/27/2012 06:00 PM, Wen Congyang wrote:
> From: Yasuaki Ishimatsu<isimatu.yasuaki@jp.fujitsu.com>
>
> When a memory is added, we update zone's and pgdat's start_pfn and
> spanned_pages in the function __add_zone(). So we should revert them
> when the memory is removed.
>
> The patch adds a new function __remove_zone() to do this.
>
> CC: David Rientjes<rientjes@google.com>
> CC: Jiang Liu<liuj97@gmail.com>
> CC: Len Brown<len.brown@intel.com>
> CC: Christoph Lameter<cl@linux.com>
> Cc: Minchan Kim<minchan.kim@gmail.com>
> CC: Andrew Morton<akpm@linux-foundation.org>
> CC: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com>
> Signed-off-by: Yasuaki Ishimatsu<isimatu.yasuaki@jp.fujitsu.com>
> Signed-off-by: Wen Congyang<wency@cn.fujitsu.com>

Reviewed-by: Tang Chen <tangchen@cn.fujitsu.com>

> ---
>   mm/memory_hotplug.c | 207 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 207 insertions(+)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 7797e91..aa97d56 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -301,10 +301,213 @@ static int __meminit __add_section(int nid, struct zone *zone,
>   	return register_new_memory(nid, __pfn_to_section(phys_start_pfn));
>   }
>
> +/* find the smallest valid pfn in the range [start_pfn, end_pfn) */
> +static int find_smallest_section_pfn(int nid, struct zone *zone,
> +				     unsigned long start_pfn,
> +				     unsigned long end_pfn)
> +{
> +	struct mem_section *ms;
> +
> +	for (; start_pfn<  end_pfn; start_pfn += PAGES_PER_SECTION) {
> +		ms = __pfn_to_section(start_pfn);
> +
> +		if (unlikely(!valid_section(ms)))
> +			continue;
> +
> +		if (unlikely(pfn_to_nid(start_pfn) != nid))
> +			continue;
> +
> +		if (zone&&  zone != page_zone(pfn_to_page(start_pfn)))
> +			continue;
> +
> +		return start_pfn;
> +	}
> +
> +	return 0;
> +}
> +
> +/* find the biggest valid pfn in the range [start_pfn, end_pfn). */
> +static int find_biggest_section_pfn(int nid, struct zone *zone,
> +				    unsigned long start_pfn,
> +				    unsigned long end_pfn)
> +{
> +	struct mem_section *ms;
> +	unsigned long pfn;
> +
> +	/* pfn is the end pfn of a memory section. */
> +	pfn = end_pfn - 1;
> +	for (; pfn>= start_pfn; pfn -= PAGES_PER_SECTION) {
> +		ms = __pfn_to_section(pfn);
> +
> +		if (unlikely(!valid_section(ms)))
> +			continue;
> +
> +		if (unlikely(pfn_to_nid(pfn) != nid))
> +			continue;
> +
> +		if (zone&&  zone != page_zone(pfn_to_page(pfn)))
> +			continue;
> +
> +		return pfn;
> +	}
> +
> +	return 0;
> +}
> +
> +static void shrink_zone_span(struct zone *zone, unsigned long start_pfn,
> +			     unsigned long end_pfn)
> +{
> +	unsigned long zone_start_pfn =  zone->zone_start_pfn;
> +	unsigned long zone_end_pfn = zone->zone_start_pfn + zone->spanned_pages;
> +	unsigned long pfn;
> +	struct mem_section *ms;
> +	int nid = zone_to_nid(zone);
> +
> +	zone_span_writelock(zone);
> +	if (zone_start_pfn == start_pfn) {
> +		/*
> +		 * If the section is smallest section in the zone, it need
> +		 * shrink zone->zone_start_pfn and zone->zone_spanned_pages.
> +		 * In this case, we find second smallest valid mem_section
> +		 * for shrinking zone.
> +		 */
> +		pfn = find_smallest_section_pfn(nid, zone, end_pfn,
> +						zone_end_pfn);
> +		if (pfn) {
> +			zone->zone_start_pfn = pfn;
> +			zone->spanned_pages = zone_end_pfn - pfn;
> +		}
> +	} else if (zone_end_pfn == end_pfn) {
> +		/*
> +		 * If the section is biggest section in the zone, it need
> +		 * shrink zone->spanned_pages.
> +		 * In this case, we find second biggest valid mem_section for
> +		 * shrinking zone.
> +		 */
> +		pfn = find_biggest_section_pfn(nid, zone, zone_start_pfn,
> +					       start_pfn);
> +		if (pfn)
> +			zone->spanned_pages = pfn - zone_start_pfn + 1;
> +	}
> +
> +	/*
> +	 * The section is not biggest or smallest mem_section in the zone, it
> +	 * only creates a hole in the zone. So in this case, we need not
> +	 * change the zone. But perhaps, the zone has only hole data. Thus
> +	 * it check the zone has only hole or not.
> +	 */
> +	pfn = zone_start_pfn;
> +	for (; pfn<  zone_end_pfn; pfn += PAGES_PER_SECTION) {
> +		ms = __pfn_to_section(pfn);
> +
> +		if (unlikely(!valid_section(ms)))
> +			continue;
> +
> +		if (page_zone(pfn_to_page(pfn)) != zone)
> +			continue;
> +
> +		 /* If the section is current section, it continues the loop */
> +		if (start_pfn == pfn)
> +			continue;
> +
> +		/* If we find valid section, we have nothing to do */
> +		zone_span_writeunlock(zone);
> +		return;
> +	}
> +
> +	/* The zone has no valid section */
> +	zone->zone_start_pfn = 0;
> +	zone->spanned_pages = 0;
> +	zone_span_writeunlock(zone);
> +}
> +
> +static void shrink_pgdat_span(struct pglist_data *pgdat,
> +			      unsigned long start_pfn, unsigned long end_pfn)
> +{
> +	unsigned long pgdat_start_pfn =  pgdat->node_start_pfn;
> +	unsigned long pgdat_end_pfn =
> +		pgdat->node_start_pfn + pgdat->node_spanned_pages;
> +	unsigned long pfn;
> +	struct mem_section *ms;
> +	int nid = pgdat->node_id;
> +
> +	if (pgdat_start_pfn == start_pfn) {
> +		/*
> +		 * If the section is smallest section in the pgdat, it need
> +		 * shrink pgdat->node_start_pfn and pgdat->node_spanned_pages.
> +		 * In this case, we find second smallest valid mem_section
> +		 * for shrinking zone.
> +		 */
> +		pfn = find_smallest_section_pfn(nid, NULL, end_pfn,
> +						pgdat_end_pfn);
> +		if (pfn) {
> +			pgdat->node_start_pfn = pfn;
> +			pgdat->node_spanned_pages = pgdat_end_pfn - pfn;
> +		}
> +	} else if (pgdat_end_pfn == end_pfn) {
> +		/*
> +		 * If the section is biggest section in the pgdat, it need
> +		 * shrink pgdat->node_spanned_pages.
> +		 * In this case, we find second biggest valid mem_section for
> +		 * shrinking zone.
> +		 */
> +		pfn = find_biggest_section_pfn(nid, NULL, pgdat_start_pfn,
> +					       start_pfn);
> +		if (pfn)
> +			pgdat->node_spanned_pages = pfn - pgdat_start_pfn + 1;
> +	}
> +
> +	/*
> +	 * If the section is not biggest or smallest mem_section in the pgdat,
> +	 * it only creates a hole in the pgdat. So in this case, we need not
> +	 * change the pgdat.
> +	 * But perhaps, the pgdat has only hole data. Thus it check the pgdat
> +	 * has only hole or not.
> +	 */
> +	pfn = pgdat_start_pfn;
> +	for (; pfn<  pgdat_end_pfn; pfn += PAGES_PER_SECTION) {
> +		ms = __pfn_to_section(pfn);
> +
> +		if (unlikely(!valid_section(ms)))
> +			continue;
> +
> +		if (pfn_to_nid(pfn) != nid)
> +			continue;
> +
> +		 /* If the section is current section, it continues the loop */
> +		if (start_pfn == pfn)
> +			continue;
> +
> +		/* If we find valid section, we have nothing to do */
> +		return;
> +	}
> +
> +	/* The pgdat has no valid section */
> +	pgdat->node_start_pfn = 0;
> +	pgdat->node_spanned_pages = 0;
> +}
> +
> +static void __remove_zone(struct zone *zone, unsigned long start_pfn)
> +{
> +	struct pglist_data *pgdat = zone->zone_pgdat;
> +	int nr_pages = PAGES_PER_SECTION;
> +	int zone_type;
> +	unsigned long flags;
> +
> +	zone_type = zone - pgdat->node_zones;
> +
> +	pgdat_resize_lock(zone->zone_pgdat,&flags);
> +	shrink_zone_span(zone, start_pfn, start_pfn + nr_pages);
> +	shrink_pgdat_span(pgdat, start_pfn, start_pfn + nr_pages);
> +	pgdat_resize_unlock(zone->zone_pgdat,&flags);
> +}
> +
>   static int __remove_section(struct zone *zone, struct mem_section *ms)
>   {
>   	unsigned long flags;
>   	struct pglist_data *pgdat = zone->zone_pgdat;
> +	unsigned long start_pfn;
> +	int scn_nr;
>   	int ret = -EINVAL;
>
>   	if (!valid_section(ms))
> @@ -314,6 +517,10 @@ static int __remove_section(struct zone *zone, struct mem_section *ms)
>   	if (ret)
>   		return ret;
>
> +	scn_nr = __section_nr(ms);
> +	start_pfn = section_nr_to_pfn(scn_nr);
> +	__remove_zone(zone, start_pfn);
> +
>   	pgdat_resize_lock(pgdat,&flags);
>   	sparse_remove_one_section(zone, ms);
>   	pgdat_resize_unlock(pgdat,&flags);

^ permalink raw reply

* Re: [Patch v4 08/12] memory-hotplug: remove memmap of sparse-vmemmap
From: Tang Chen @ 2012-12-04  9:47 UTC (permalink / raw)
  To: Wen Congyang
  Cc: linux-s390, linux-ia64, Len Brown, linux-acpi, linux-sh, x86,
	linux-kernel, cmetcalf, Jianguo Wu, linux-mm, Yasuaki Ishimatsu,
	paulus, Minchan Kim, KOSAKI Motohiro, David Rientjes, sparclinux,
	Christoph Lameter, linuxppc-dev, Andrew Morton, Jiang Liu
In-Reply-To: <1354010422-19648-9-git-send-email-wency@cn.fujitsu.com>

On 11/27/2012 06:00 PM, Wen Congyang wrote:
>   static int __remove_section(struct zone *zone, struct mem_section *ms)
>   {
>   	unsigned long flags;
> @@ -330,9 +317,9 @@ static int __remove_section(struct zone *zone, struct mem_section *ms)
>   	pgdat_resize_lock(pgdat,&flags);
>   	sparse_remove_one_section(zone, ms);
>   	pgdat_resize_unlock(pgdat,&flags);
> -	return 0;
> +
> +	return ret;

I think we don't need to change this line. :)

Reviewed-by: Tang Chen <tangchen@cn.fujitsu.com>

^ permalink raw reply

* Re: [Patch v4 06/12] memory-hotplug: unregister memory section on SPARSEMEM_VMEMMAP
From: Tang Chen @ 2012-12-04  9:34 UTC (permalink / raw)
  To: Wen Congyang
  Cc: linux-s390, linux-ia64, Len Brown, linux-acpi, linux-sh, x86,
	linux-kernel, cmetcalf, Jianguo Wu, linux-mm, Yasuaki Ishimatsu,
	paulus, Minchan Kim, KOSAKI Motohiro, David Rientjes, sparclinux,
	Christoph Lameter, linuxppc-dev, Andrew Morton, Jiang Liu
In-Reply-To: <1354010422-19648-7-git-send-email-wency@cn.fujitsu.com>

On 11/27/2012 06:00 PM, Wen Congyang wrote:
> From: Yasuaki Ishimatsu<isimatu.yasuaki@jp.fujitsu.com>
>
> Currently __remove_section for SPARSEMEM_VMEMMAP does nothing. But even if
> we use SPARSEMEM_VMEMMAP, we can unregister the memory_section.
>
> So the patch add unregister_memory_section() into __remove_section().
>
> CC: David Rientjes<rientjes@google.com>
> CC: Jiang Liu<liuj97@gmail.com>
> CC: Len Brown<len.brown@intel.com>
> CC: Christoph Lameter<cl@linux.com>
> Cc: Minchan Kim<minchan.kim@gmail.com>
> CC: Andrew Morton<akpm@linux-foundation.org>
> CC: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com>
> Signed-off-by: Yasuaki Ishimatsu<isimatu.yasuaki@jp.fujitsu.com>
> Signed-off-by: Wen Congyang<wency@cn.fujitsu.com>

__remove_section() of CONFIG_SPARSEMEM_VMEMMAP will be integrated
into one in [PATCH 08/12], so I think we can merge this patch into
[PATCH 08/12].

Reviewed-by: Tang Chen <tangchen@cn.fujitsu.com>

> ---
>   mm/memory_hotplug.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index e741732..171610d 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -279,11 +279,14 @@ static int __meminit __add_section(int nid, struct zone *zone,
>   #ifdef CONFIG_SPARSEMEM_VMEMMAP
>   static int __remove_section(struct zone *zone, struct mem_section *ms)
>   {
> -	/*
> -	 * XXX: Freeing memmap with vmemmap is not implement yet.
> -	 *      This should be removed later.
> -	 */
> -	return -EBUSY;
> +	int ret = -EINVAL;
> +
> +	if (!valid_section(ms))
> +		return ret;
> +
> +	ret = unregister_memory_section(ms);
> +
> +	return ret;
>   }
>   #else
>   static int __remove_section(struct zone *zone, struct mem_section *ms)

^ permalink raw reply

* Re: [Patch v4 05/12] memory-hotplug: introduce new function arch_remove_memory() for removing page table depends on architecture
From: Tang Chen @ 2012-12-04  9:30 UTC (permalink / raw)
  To: Wen Congyang
  Cc: linux-s390, linux-ia64, Len Brown, linux-acpi, linux-sh, x86,
	linux-kernel, cmetcalf, Jianguo Wu, linux-mm, Yasuaki Ishimatsu,
	paulus, Minchan Kim, KOSAKI Motohiro, David Rientjes, sparclinux,
	Christoph Lameter, linuxppc-dev, Andrew Morton, Jiang Liu
In-Reply-To: <1354010422-19648-6-git-send-email-wency@cn.fujitsu.com>

On 11/27/2012 06:00 PM, Wen Congyang wrote:
> For removing memory, we need to remove page table. But it depends
> on architecture. So the patch introduce arch_remove_memory() for
> removing page table. Now it only calls __remove_pages().
>
> Note: __remove_pages() for some archtecuture is not implemented
>        (I don't know how to implement it for s390).
>
> CC: David Rientjes<rientjes@google.com>
> CC: Jiang Liu<liuj97@gmail.com>
> CC: Len Brown<len.brown@intel.com>
> CC: Benjamin Herrenschmidt<benh@kernel.crashing.org>
> CC: Paul Mackerras<paulus@samba.org>
> CC: Christoph Lameter<cl@linux.com>
> Cc: Minchan Kim<minchan.kim@gmail.com>
> CC: Andrew Morton<akpm@linux-foundation.org>
> CC: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com>
> CC: Yasuaki Ishimatsu<isimatu.yasuaki@jp.fujitsu.com>
> Signed-off-by: Wen Congyang<wency@cn.fujitsu.com>
> ---
>   arch/ia64/mm/init.c            | 18 ++++++++++++++++++
>   arch/powerpc/mm/mem.c          | 12 ++++++++++++
>   arch/s390/mm/init.c            | 12 ++++++++++++
>   arch/sh/mm/init.c              | 17 +++++++++++++++++
>   arch/tile/mm/init.c            |  8 ++++++++
>   arch/x86/mm/init_32.c          | 12 ++++++++++++
>   arch/x86/mm/init_64.c          | 15 +++++++++++++++
>   include/linux/memory_hotplug.h |  1 +
>   mm/memory_hotplug.c            |  2 ++
>   9 files changed, 97 insertions(+)
>
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 082e383..e333822 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -689,6 +689,24 @@ int arch_add_memory(int nid, u64 start, u64 size)
>
>   	return ret;
>   }
> +
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +int arch_remove_memory(u64 start, u64 size)
> +{
> +	unsigned long start_pfn = start>>  PAGE_SHIFT;
> +	unsigned long nr_pages = size>>  PAGE_SHIFT;
> +	struct zone *zone;
> +	int ret;
> +
> +	zone = page_zone(pfn_to_page(start_pfn));
> +	ret = __remove_pages(zone, start_pfn, nr_pages);
> +	if (ret)
> +		pr_warn("%s: Problem encountered in __remove_pages() as"
> +			" ret=%d\n", __func__,  ret);
> +
> +	return ret;

Just a little question, why do we have different handlers for ret on
different platforms ?  Sometimes we print a msg, sometimes we just
return, and sometimes we give a WARN_ON(). But no big deal. :)

Reviewed-by: Tang Chen <tangchen@cn.fujitsu.com>

> +}
> +#endif
>   #endif
>
>   /*
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 0dba506..09c6451 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -133,6 +133,18 @@ int arch_add_memory(int nid, u64 start, u64 size)
>
>   	return __add_pages(nid, zone, start_pfn, nr_pages);
>   }
> +
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +int arch_remove_memory(u64 start, u64 size)
> +{
> +	unsigned long start_pfn = start>>  PAGE_SHIFT;
> +	unsigned long nr_pages = size>>  PAGE_SHIFT;
> +	struct zone *zone;
> +
> +	zone = page_zone(pfn_to_page(start_pfn));
> +	return __remove_pages(zone, start_pfn, nr_pages);
> +}
> +#endif
>   #endif /* CONFIG_MEMORY_HOTPLUG */
>
>   /*
> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
> index 81e596c..b565190 100644
> --- a/arch/s390/mm/init.c
> +++ b/arch/s390/mm/init.c
> @@ -257,4 +257,16 @@ int arch_add_memory(int nid, u64 start, u64 size)
>   		vmem_remove_mapping(start, size);
>   	return rc;
>   }
> +
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +int arch_remove_memory(u64 start, u64 size)
> +{
> +	/*
> +	 * There is no hardware or firmware interface which could trigger a
> +	 * hot memory remove on s390. So there is nothing that needs to be
> +	 * implemented.
> +	 */
> +	return -EBUSY;
> +}
> +#endif
>   #endif /* CONFIG_MEMORY_HOTPLUG */
> diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
> index 82cc576..1057940 100644
> --- a/arch/sh/mm/init.c
> +++ b/arch/sh/mm/init.c
> @@ -558,4 +558,21 @@ int memory_add_physaddr_to_nid(u64 addr)
>   EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
>   #endif
>
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +int arch_remove_memory(u64 start, u64 size)
> +{
> +	unsigned long start_pfn = start>>  PAGE_SHIFT;
> +	unsigned long nr_pages = size>>  PAGE_SHIFT;
> +	struct zone *zone;
> +	int ret;
> +
> +	zone = page_zone(pfn_to_page(start_pfn));
> +	ret = __remove_pages(zone, start_pfn, nr_pages);
> +	if (unlikely(ret))
> +		pr_warn("%s: Failed, __remove_pages() == %d\n", __func__,
> +			ret);
> +
> +	return ret;
> +}
> +#endif
>   #endif /* CONFIG_MEMORY_HOTPLUG */
> diff --git a/arch/tile/mm/init.c b/arch/tile/mm/init.c
> index ef29d6c..2749515 100644
> --- a/arch/tile/mm/init.c
> +++ b/arch/tile/mm/init.c
> @@ -935,6 +935,14 @@ int remove_memory(u64 start, u64 size)
>   {
>   	return -EINVAL;
>   }
> +
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +int arch_remove_memory(u64 start, u64 size)
> +{
> +	/* TODO */
> +	return -EBUSY;
> +}
> +#endif
>   #endif
>
>   struct kmem_cache *pgd_cache;
> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
> index 11a5800..b19eba4 100644
> --- a/arch/x86/mm/init_32.c
> +++ b/arch/x86/mm/init_32.c
> @@ -839,6 +839,18 @@ int arch_add_memory(int nid, u64 start, u64 size)
>
>   	return __add_pages(nid, zone, start_pfn, nr_pages);
>   }
> +
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +int arch_remove_memory(u64 start, u64 size)
> +{
> +	unsigned long start_pfn = start>>  PAGE_SHIFT;
> +	unsigned long nr_pages = size>>  PAGE_SHIFT;
> +	struct zone *zone;
> +
> +	zone = page_zone(pfn_to_page(start_pfn));
> +	return __remove_pages(zone, start_pfn, nr_pages);
> +}
> +#endif
>   #endif
>
>   /*
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index 3baff25..5675335 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -680,6 +680,21 @@ int arch_add_memory(int nid, u64 start, u64 size)
>   }
>   EXPORT_SYMBOL_GPL(arch_add_memory);
>
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +int __ref arch_remove_memory(u64 start, u64 size)
> +{
> +	unsigned long start_pfn = start>>  PAGE_SHIFT;
> +	unsigned long nr_pages = size>>  PAGE_SHIFT;
> +	struct zone *zone;
> +	int ret;
> +
> +	zone = page_zone(pfn_to_page(start_pfn));
> +	ret = __remove_pages(zone, start_pfn, nr_pages);
> +	WARN_ON_ONCE(ret);
> +
> +	return ret;
> +}
> +#endif
>   #endif /* CONFIG_MEMORY_HOTPLUG */
>
>   static struct kcore_list kcore_vsyscall;
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 38675e9..191b2d9 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -85,6 +85,7 @@ extern void __online_page_free(struct page *page);
>
>   #ifdef CONFIG_MEMORY_HOTREMOVE
>   extern bool is_pageblock_removable_nolock(struct page *page);
> +extern int arch_remove_memory(u64 start, u64 size);
>   #endif /* CONFIG_MEMORY_HOTREMOVE */
>
>   /* reasonably generic interface to expand the physical pages in a zone  */
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 63d5388..e741732 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1111,6 +1111,8 @@ repeat:
>   	/* remove memmap entry */
>   	firmware_map_remove(start, start + size, "System RAM");
>
> +	arch_remove_memory(start, size);
> +
>   	unlock_memory_hotplug();
>
>   	return 0;

^ permalink raw reply

* Re: [Patch v4 03/12] memory-hotplug: remove redundant codes
From: Tang Chen @ 2012-12-04  9:22 UTC (permalink / raw)
  To: Wen Congyang
  Cc: linux-s390, linux-ia64, Len Brown, linux-acpi, linux-sh, x86,
	linux-kernel, cmetcalf, Jianguo Wu, linux-mm, Yasuaki Ishimatsu,
	paulus, Minchan Kim, KOSAKI Motohiro, David Rientjes, sparclinux,
	Christoph Lameter, linuxppc-dev, Andrew Morton, Jiang Liu
In-Reply-To: <1354010422-19648-4-git-send-email-wency@cn.fujitsu.com>

On 11/27/2012 06:00 PM, Wen Congyang wrote:
> offlining memory blocks and checking whether memory blocks are offlined
> are very similar. This patch introduces a new function to remove
> redundant codes.
>
> CC: David Rientjes<rientjes@google.com>
> CC: Jiang Liu<liuj97@gmail.com>
> CC: Len Brown<len.brown@intel.com>
> CC: Christoph Lameter<cl@linux.com>
> Cc: Minchan Kim<minchan.kim@gmail.com>
> CC: Andrew Morton<akpm@linux-foundation.org>
> CC: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com>
> CC: Yasuaki Ishimatsu<isimatu.yasuaki@jp.fujitsu.com>
> Signed-off-by: Wen Congyang<wency@cn.fujitsu.com>

Can we merge this patch with [PATCH 03/12] ?

Reviewed-by: Tang Chen <tangchen@cn.fujitsu.com>

> ---
>   mm/memory_hotplug.c | 101 ++++++++++++++++++++++++++++------------------------
>   1 file changed, 55 insertions(+), 46 deletions(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index b6d1101..6d06488 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1005,20 +1005,14 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages)
>   	return __offline_pages(start_pfn, start_pfn + nr_pages, 120 * HZ);
>   }
>
> -int remove_memory(u64 start, u64 size)
> +static int walk_memory_range(unsigned long start_pfn, unsigned long end_pfn,
> +		void *arg, int (*func)(struct memory_block *, void *))
>   {
>   	struct memory_block *mem = NULL;
>   	struct mem_section *section;
> -	unsigned long start_pfn, end_pfn;
>   	unsigned long pfn, section_nr;
>   	int ret;
> -	int return_on_error = 0;
> -	int retry = 0;
> -
> -	start_pfn = PFN_DOWN(start);
> -	end_pfn = start_pfn + PFN_DOWN(size);
>
> -repeat:
>   	for (pfn = start_pfn; pfn<  end_pfn; pfn += PAGES_PER_SECTION) {
>   		section_nr = pfn_to_section_nr(pfn);
>   		if (!present_section_nr(section_nr))
> @@ -1035,22 +1029,61 @@ repeat:
>   		if (!mem)
>   			continue;
>
> -		ret = offline_memory_block(mem);
> +		ret = func(mem, arg);
>   		if (ret) {
> -			if (return_on_error) {
> -				kobject_put(&mem->dev.kobj);
> -				return ret;
> -			} else {
> -				retry = 1;
> -			}
> +			kobject_put(&mem->dev.kobj);
> +			return ret;
>   		}
>   	}
>
>   	if (mem)
>   		kobject_put(&mem->dev.kobj);
>
> -	if (retry) {
> -		return_on_error = 1;
> +	return 0;
> +}
> +
> +static int offline_memory_block_cb(struct memory_block *mem, void *arg)
> +{
> +	int *ret = arg;
> +	int error = offline_memory_block(mem);
> +
> +	if (error != 0&&  *ret == 0)
> +		*ret = error;
> +
> +	return 0;
> +}
> +
> +static int is_memblock_offlined_cb(struct memory_block *mem, void *arg)
> +{
> +	int ret = !is_memblock_offlined(mem);
> +
> +	if (unlikely(ret))
> +		pr_warn("removing memory fails, because memory "
> +			"[%#010llx-%#010llx] is onlined\n",
> +			PFN_PHYS(section_nr_to_pfn(mem->start_section_nr)),
> +			PFN_PHYS(section_nr_to_pfn(mem->end_section_nr + 1))-1);
> +
> +	return ret;
> +}
> +
> +int remove_memory(u64 start, u64 size)
> +{
> +	unsigned long start_pfn, end_pfn;
> +	int ret = 0;
> +	int retry = 1;
> +
> +	start_pfn = PFN_DOWN(start);
> +	end_pfn = start_pfn + PFN_DOWN(size);
> +
> +repeat:
> +	walk_memory_range(start_pfn, end_pfn,&ret,
> +			  offline_memory_block_cb);
> +	if (ret) {
> +		if (!retry)
> +			return ret;
> +
> +		retry = 0;
> +		ret = 0;
>   		goto repeat;
>   	}
>
> @@ -1068,37 +1101,13 @@ repeat:
>   	 * memory blocks are offlined.
>   	 */
>
> -	for (pfn = start_pfn; pfn<  end_pfn; pfn += PAGES_PER_SECTION) {
> -		section_nr = pfn_to_section_nr(pfn);
> -		if (!present_section_nr(section_nr))
> -			continue;
> -
> -		section = __nr_to_section(section_nr);
> -		/* same memblock? */
> -		if (mem)
> -			if ((section_nr>= mem->start_section_nr)&&
> -			    (section_nr<= mem->end_section_nr))
> -				continue;
> -
> -		mem = find_memory_block_hinted(section, mem);
> -		if (!mem)
> -			continue;
> -
> -		ret = is_memblock_offlined(mem);
> -		if (!ret) {
> -			pr_warn("removing memory fails, because memory "
> -				"[%#010llx-%#010llx] is onlined\n",
> -				PFN_PHYS(section_nr_to_pfn(mem->start_section_nr)),
> -				PFN_PHYS(section_nr_to_pfn(mem->end_section_nr + 1)) - 1);
> -
> -			kobject_put(&mem->dev.kobj);
> -			unlock_memory_hotplug();
> -			return ret;
> -		}
> +	ret = walk_memory_range(start_pfn, end_pfn, NULL,
> +				is_memblock_offlined_cb);
> +	if (ret) {
> +		unlock_memory_hotplug();
> +		return ret;
>   	}
>
> -	if (mem)
> -		kobject_put(&mem->dev.kobj);
>   	unlock_memory_hotplug();
>
>   	return 0;

^ permalink raw reply

* Re: [Patch v4 02/12] memory-hotplug: check whether all memory blocks are offlined or not when removing memory
From: Tang Chen @ 2012-12-04  9:22 UTC (permalink / raw)
  To: Wen Congyang
  Cc: linux-s390, linux-ia64, Len Brown, linux-acpi, linux-sh, x86,
	linux-kernel, cmetcalf, Jianguo Wu, linux-mm, Yasuaki Ishimatsu,
	paulus, Minchan Kim, KOSAKI Motohiro, David Rientjes, sparclinux,
	Christoph Lameter, linuxppc-dev, Andrew Morton, Jiang Liu
In-Reply-To: <1354010422-19648-3-git-send-email-wency@cn.fujitsu.com>

On 11/27/2012 06:00 PM, Wen Congyang wrote:
> From: Yasuaki Ishimatsu<isimatu.yasuaki@jp.fujitsu.com>
>
> We remove the memory like this:
> 1. lock memory hotplug
> 2. offline a memory block
> 3. unlock memory hotplug
> 4. repeat 1-3 to offline all memory blocks
> 5. lock memory hotplug
> 6. remove memory(TODO)
> 7. unlock memory hotplug
>
> All memory blocks must be offlined before removing memory. But we don't hold
> the lock in the whole operation. So we should check whether all memory blocks
> are offlined before step6. Otherwise, kernel maybe panicked.
>
> CC: David Rientjes<rientjes@google.com>
> CC: Jiang Liu<liuj97@gmail.com>
> CC: Len Brown<len.brown@intel.com>
> CC: Christoph Lameter<cl@linux.com>
> Cc: Minchan Kim<minchan.kim@gmail.com>
> CC: Andrew Morton<akpm@linux-foundation.org>
> CC: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com>
> Signed-off-by: Wen Congyang<wency@cn.fujitsu.com>
> Signed-off-by: Yasuaki Ishimatsu<isimatu.yasuaki@jp.fujitsu.com>

Reviewed-by: Tang Chen <tangchen@cn.fujitsu.com>

> ---
>   drivers/base/memory.c          |  6 ++++++
>   include/linux/memory_hotplug.h |  1 +
>   mm/memory_hotplug.c            | 47 ++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 54 insertions(+)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 86c8821..badb025 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -675,6 +675,12 @@ int offline_memory_block(struct memory_block *mem)
>   	return ret;
>   }
>
> +/* return true if the memory block is offlined, otherwise, return false */
> +bool is_memblock_offlined(struct memory_block *mem)
> +{
> +	return mem->state == MEM_OFFLINE;
> +}
> +
>   /*
>    * Initialize the sysfs support for memory devices...
>    */
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 95573ec..38675e9 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -236,6 +236,7 @@ extern int add_memory(int nid, u64 start, u64 size);
>   extern int arch_add_memory(int nid, u64 start, u64 size);
>   extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
>   extern int offline_memory_block(struct memory_block *mem);
> +extern bool is_memblock_offlined(struct memory_block *mem);
>   extern int remove_memory(u64 start, u64 size);
>   extern int sparse_add_one_section(struct zone *zone, unsigned long start_pfn,
>   								int nr_pages);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index b825dbc..b6d1101 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1054,6 +1054,53 @@ repeat:
>   		goto repeat;
>   	}
>
> +	lock_memory_hotplug();
> +
> +	/*
> +	 * we have offlined all memory blocks like this:
> +	 *   1. lock memory hotplug
> +	 *   2. offline a memory block
> +	 *   3. unlock memory hotplug
> +	 *
> +	 * repeat step1-3 to offline the memory block. All memory blocks
> +	 * must be offlined before removing memory. But we don't hold the
> +	 * lock in the whole operation. So we should check whether all
> +	 * memory blocks are offlined.
> +	 */
> +
> +	for (pfn = start_pfn; pfn<  end_pfn; pfn += PAGES_PER_SECTION) {
> +		section_nr = pfn_to_section_nr(pfn);
> +		if (!present_section_nr(section_nr))
> +			continue;
> +
> +		section = __nr_to_section(section_nr);
> +		/* same memblock? */
> +		if (mem)
> +			if ((section_nr>= mem->start_section_nr)&&
> +			    (section_nr<= mem->end_section_nr))
> +				continue;
> +
> +		mem = find_memory_block_hinted(section, mem);
> +		if (!mem)
> +			continue;
> +
> +		ret = is_memblock_offlined(mem);
> +		if (!ret) {
> +			pr_warn("removing memory fails, because memory "
> +				"[%#010llx-%#010llx] is onlined\n",
> +				PFN_PHYS(section_nr_to_pfn(mem->start_section_nr)),
> +				PFN_PHYS(section_nr_to_pfn(mem->end_section_nr + 1)) - 1);
> +
> +			kobject_put(&mem->dev.kobj);
> +			unlock_memory_hotplug();
> +			return ret;
> +		}
> +	}
> +
> +	if (mem)
> +		kobject_put(&mem->dev.kobj);
> +	unlock_memory_hotplug();
> +
>   	return 0;
>   }
>   #else

^ permalink raw reply

* Re: [Patch v4 01/12] memory-hotplug: try to offline the memory twice to avoid dependence
From: Tang Chen @ 2012-12-04  9:17 UTC (permalink / raw)
  To: Wen Congyang
  Cc: linux-s390, linux-ia64, Len Brown, linux-acpi, linux-sh, x86,
	linux-kernel, cmetcalf, Jianguo Wu, linux-mm, Yasuaki Ishimatsu,
	paulus, Minchan Kim, KOSAKI Motohiro, David Rientjes, sparclinux,
	Christoph Lameter, linuxppc-dev, Andrew Morton, Jiang Liu
In-Reply-To: <1354010422-19648-2-git-send-email-wency@cn.fujitsu.com>

On 11/27/2012 06:00 PM, Wen Congyang wrote:
> memory can't be offlined when CONFIG_MEMCG is selected.
> For example: there is a memory device on node 1. The address range
> is [1G, 1.5G). You will find 4 new directories memory8, memory9, memory10,
> and memory11 under the directory /sys/devices/system/memory/.
>
> If CONFIG_MEMCG is selected, we will allocate memory to store page cgroup
> when we online pages. When we online memory8, the memory stored page cgroup
> is not provided by this memory device. But when we online memory9, the memory
> stored page cgroup may be provided by memory8. So we can't offline memory8
> now. We should offline the memory in the reversed order.
>
> When the memory device is hotremoved, we will auto offline memory provided
> by this memory device. But we don't know which memory is onlined first, so
> offlining memory may fail. In such case, iterate twice to offline the memory.
> 1st iterate: offline every non primary memory block.
> 2nd iterate: offline primary (i.e. first added) memory block.
>
> This idea is suggested by KOSAKI Motohiro.
>
> CC: David Rientjes<rientjes@google.com>
> CC: Jiang Liu<liuj97@gmail.com>
> CC: Len Brown<len.brown@intel.com>
> CC: Christoph Lameter<cl@linux.com>
> Cc: Minchan Kim<minchan.kim@gmail.com>
> CC: Andrew Morton<akpm@linux-foundation.org>
> CC: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com>
> CC: Yasuaki Ishimatsu<isimatu.yasuaki@jp.fujitsu.com>
> Signed-off-by: Wen Congyang<wency@cn.fujitsu.com>

Reviewed-by: Tang Chen <tangchen@cn.fujitsu.com>

> ---
>   mm/memory_hotplug.c | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index e4eeaca..b825dbc 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1012,10 +1012,13 @@ int remove_memory(u64 start, u64 size)
>   	unsigned long start_pfn, end_pfn;
>   	unsigned long pfn, section_nr;
>   	int ret;
> +	int return_on_error = 0;
> +	int retry = 0;
>
>   	start_pfn = PFN_DOWN(start);
>   	end_pfn = start_pfn + PFN_DOWN(size);
>
> +repeat:
>   	for (pfn = start_pfn; pfn<  end_pfn; pfn += PAGES_PER_SECTION) {
>   		section_nr = pfn_to_section_nr(pfn);
>   		if (!present_section_nr(section_nr))
> @@ -1034,14 +1037,23 @@ int remove_memory(u64 start, u64 size)
>
>   		ret = offline_memory_block(mem);
>   		if (ret) {
> -			kobject_put(&mem->dev.kobj);
> -			return ret;
> +			if (return_on_error) {
> +				kobject_put(&mem->dev.kobj);
> +				return ret;
> +			} else {
> +				retry = 1;
> +			}
>   		}
>   	}
>
>   	if (mem)
>   		kobject_put(&mem->dev.kobj);
>
> +	if (retry) {
> +		return_on_error = 1;
> +		goto repeat;
> +	}
> +
>   	return 0;
>   }
>   #else

^ permalink raw reply

* Re: [Patch v4 08/12] memory-hotplug: remove memmap of sparse-vmemmap
From: Tang Chen @ 2012-12-04  9:13 UTC (permalink / raw)
  To: Jianguo Wu
  Cc: linux-s390, linux-ia64, Wen Congyang, linux-acpi, linux-sh,
	Len Brown, x86, linux-kernel, cmetcalf, linux-mm,
	Yasuaki Ishimatsu, paulus, Minchan Kim, KOSAKI Motohiro,
	David Rientjes, sparclinux, Christoph Lameter, linuxppc-dev,
	Andrew Morton, Jiang Liu
In-Reply-To: <50BC0D2D.8040008@huawei.com>

Hi Wu,

Sorry to make noise here. Please see below. :)

On 12/03/2012 10:23 AM, Jianguo Wu wrote:
> Signed-off-by: Jianguo Wu<wujianguo@huawei.com>
> Signed-off-by: Jiang Liu<jiang.liu@huawei.com>
> ---
>   include/linux/mm.h  |    1 +
>   mm/sparse-vmemmap.c |  231 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   mm/sparse.c         |    3 +-
>   3 files changed, 234 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 5657670..1f26af5 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1642,6 +1642,7 @@ int vmemmap_populate(struct page *start_page, unsigned long pages, int node);
>   void vmemmap_populate_print_last(void);
>   void register_page_bootmem_memmap(unsigned long section_nr, struct page *map,
>   				  unsigned long size);
> +void vmemmap_free(struct page *memmap, unsigned long nr_pages);
>
>   enum mf_flags {
>   	MF_COUNT_INCREASED = 1<<  0,
> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
> index 1b7e22a..748732d 100644
> --- a/mm/sparse-vmemmap.c
> +++ b/mm/sparse-vmemmap.c
> @@ -29,6 +29,10 @@
>   #include<asm/pgalloc.h>
>   #include<asm/pgtable.h>
>
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +#include<asm/tlbflush.h>
> +#endif
> +
>   /*
>    * Allocate a block of memory to be used to back the virtual memory map
>    * or to back the page tables that are used to create the mapping.
> @@ -224,3 +228,230 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
>   		vmemmap_buf_end = NULL;
>   	}
>   }
> +
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +
> +#define PAGE_INUSE 0xFD
> +
> +static void vmemmap_free_pages(struct page *page, int order)
> +{
> +	struct zone *zone;
> +	unsigned long magic;
> +
> +	magic = (unsigned long) page->lru.next;
> +	if (magic == SECTION_INFO || magic == MIX_SECTION_INFO) {
> +		put_page_bootmem(page);
> +
> +		zone = page_zone(page);
> +		zone_span_writelock(zone);
> +		zone->present_pages++;
> +		zone_span_writeunlock(zone);
> +		totalram_pages++;

Seems that we have different ways to handle pages allocated by bootmem
or by regular allocator. Is the checking way in [PATCH 09/12] available
here ?

+	/* bootmem page has reserved flag */
+	if (PageReserved(page)) {
......
+	}

If so, I think we can just merge these two functions.

> +	} else
> +		free_pages((unsigned long)page_address(page), order);
> +}
> +
> +static void free_pte_table(pmd_t *pmd)
> +{
> +	pte_t *pte, *pte_start;
> +	int i;
> +
> +	pte_start = (pte_t *)pmd_page_vaddr(*pmd);
> +	for (i = 0; i<  PTRS_PER_PTE; i++) {
> +		pte = pte_start + i;
> +		if (pte_val(*pte))
> +			return;
> +	}
> +
> +	/* free a pte talbe */
> +	vmemmap_free_pages(pmd_page(*pmd), 0);
> +	spin_lock(&init_mm.page_table_lock);
> +	pmd_clear(pmd);
> +	spin_unlock(&init_mm.page_table_lock);
> +}
> +
> +static void free_pmd_table(pud_t *pud)
> +{
> +	pmd_t *pmd, *pmd_start;
> +	int i;
> +
> +	pmd_start = (pmd_t *)pud_page_vaddr(*pud);
> +	for (i = 0; i<  PTRS_PER_PMD; i++) {
> +		pmd = pmd_start + i;
> +		if (pmd_val(*pmd))
> +			return;
> +	}
> +
> +	/* free a pmd talbe */
> +	vmemmap_free_pages(pud_page(*pud), 0);
> +	spin_lock(&init_mm.page_table_lock);
> +	pud_clear(pud);
> +	spin_unlock(&init_mm.page_table_lock);
> +}
> +
> +static void free_pud_table(pgd_t *pgd)
> +{
> +	pud_t *pud, *pud_start;
> +	int i;
> +
> +	pud_start = (pud_t *)pgd_page_vaddr(*pgd);
> +	for (i = 0; i<  PTRS_PER_PUD; i++) {
> +		pud = pud_start + i;
> +		if (pud_val(*pud))
> +			return;
> +	}
> +
> +	/* free a pud table */
> +	vmemmap_free_pages(pgd_page(*pgd), 0);
> +	spin_lock(&init_mm.page_table_lock);
> +	pgd_clear(pgd);
> +	spin_unlock(&init_mm.page_table_lock);
> +}

All the free_xxx_table() are very similar to the functions in
[PATCH 09/12]. Could we reuse them anyway ?

> +
> +static int split_large_page(pte_t *kpte, unsigned long address, pte_t *pbase)
> +{
> +	struct page *page = pmd_page(*(pmd_t *)kpte);
> +	int i = 0;
> +	unsigned long magic;
> +	unsigned long section_nr;
> +
> +	__split_large_page(kpte, address, pbase);

Is this patch going to replace [PATCH 08/12] ?

If so, __split_large_page() was added and exported in [PATCH 09/12],
then we should move it here, right ?

If not, free_map_bootmem() and __kfree_section_memmap() were changed in
[PATCH 08/12], and we need to handle this.

> +	__flush_tlb_all();
> +
> +	magic = (unsigned long) page->lru.next;
> +	if (magic == SECTION_INFO) {
> +		section_nr = pfn_to_section_nr(page_to_pfn(page));
> +		while (i<  PTRS_PER_PMD) {
> +			page++;
> +			i++;
> +			get_page_bootmem(section_nr, page, SECTION_INFO);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void vmemmap_pte_remove(pmd_t *pmd, unsigned long addr, unsigned long end)
> +{
> +	pte_t *pte;
> +	unsigned long next;
> +	void *page_addr;
> +
> +	pte = pte_offset_kernel(pmd, addr);
> +	for (; addr<  end; pte++, addr += PAGE_SIZE) {
> +		next = (addr + PAGE_SIZE)&  PAGE_MASK;
> +		if (next>  end)
> +			next = end;
> +
> +		if (pte_none(*pte))
> +			continue;
> +		if (IS_ALIGNED(addr, PAGE_SIZE)&&
> +		    IS_ALIGNED(next, PAGE_SIZE)) {
> +			vmemmap_free_pages(pte_page(*pte), 0);
> +			spin_lock(&init_mm.page_table_lock);
> +			pte_clear(&init_mm, addr, pte);
> +			spin_unlock(&init_mm.page_table_lock);
> +		} else {
> +			/*
> +			 * Removed page structs are filled with 0xFD.
> +			 */
> +			memset((void *)addr, PAGE_INUSE, next - addr);
> +			page_addr = page_address(pte_page(*pte));
> +
> +			if (!memchr_inv(page_addr, PAGE_INUSE, PAGE_SIZE)) {
> +				spin_lock(&init_mm.page_table_lock);
> +				pte_clear(&init_mm, addr, pte);
> +				spin_unlock(&init_mm.page_table_lock);
> +			}
> +		}
> +	}
> +
> +	free_pte_table(pmd);
> +	__flush_tlb_all();
> +}
> +
> +static void vmemmap_pmd_remove(pud_t *pud, unsigned long addr, unsigned long end)
> +{
> +	unsigned long next;
> +	pmd_t *pmd;
> +
> +	pmd = pmd_offset(pud, addr);
> +	for (; addr<  end; addr = next, pmd++) {
> +		next = pmd_addr_end(addr, end);
> +		if (pmd_none(*pmd))
> +			continue;
> +
> +		if (cpu_has_pse) {
> +			unsigned long pte_base;
> +
> +			if (IS_ALIGNED(addr, PMD_SIZE)&&
> +			    IS_ALIGNED(next, PMD_SIZE)) {
> +				vmemmap_free_pages(pmd_page(*pmd),
> +						   get_order(PMD_SIZE));
> +				spin_lock(&init_mm.page_table_lock);
> +				pmd_clear(pmd);
> +				spin_unlock(&init_mm.page_table_lock);
> +				continue;
> +			}
> +
> +			/*
> +			 * We use 2M page, but we need to remove part of them,
> +			 * so split 2M page to 4K page.
> +			 */
> +			pte_base = get_zeroed_page(GFP_ATOMIC | __GFP_NOTRACK);
> +			if (!pte_base) {
> +				WARN_ON(1);
> +				continue;
> +			}
> +
> +			split_large_page((pte_t *)pmd, addr, (pte_t *)pte_base);
> +			__flush_tlb_all();
> +
> +			spin_lock(&init_mm.page_table_lock);
> +			pmd_populate_kernel(&init_mm, pmd, (pte_t *)pte_base);
> +			spin_unlock(&init_mm.page_table_lock);
> +		}
> +
> +		vmemmap_pte_remove(pmd, addr, next);
> +	}
> +
> +	free_pmd_table(pud);
> +	__flush_tlb_all();
> +}
> +
> +static void vmemmap_pud_remove(pgd_t *pgd, unsigned long addr, unsigned long end)
> +{
> +	unsigned long next;
> +	pud_t *pud;
> +
> +	pud = pud_offset(pgd, addr);
> +	for (; addr<  end; addr = next, pud++) {
> +		next = pud_addr_end(addr, end);
> +		if (pud_none(*pud))
> +			continue;
> +
> +		vmemmap_pmd_remove(pud, addr, next);
> +	}
> +
> +	free_pud_table(pgd);
> +	__flush_tlb_all();
> +}
> +
> +void vmemmap_free(struct page *memmap, unsigned long nr_pages)
> +{
> +	unsigned long addr = (unsigned long)memmap;
> +	unsigned long end = (unsigned long)(memmap + nr_pages);
> +	unsigned long next;
> +
> +	for (; addr<  end; addr = next) {
> +		pgd_t *pgd = pgd_offset_k(addr);
> +
> +		next = pgd_addr_end(addr, end);
> +		if (!pgd_present(*pgd))
> +			continue;
> +
> +		vmemmap_pud_remove(pgd, addr, next);
> +		sync_global_pgds(addr, next - 1);
> +	}
> +}
> +#endif
> diff --git a/mm/sparse.c b/mm/sparse.c
> index fac95f2..4060229 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -615,10 +615,11 @@ static inline struct page *kmalloc_section_memmap(unsigned long pnum, int nid,
>   }
>   static void __kfree_section_memmap(struct page *memmap, unsigned long nr_pages)
>   {
> -	return; /* XXX: Not implemented yet */
> +	vmemmap_free(memmap, nr_pages);
>   }
>   static void free_map_bootmem(struct page *page, unsigned long nr_pages)

In the latest kernel, this line was:
static void free_map_bootmem(struct page *memmap, unsigned long nr_pages)

>   {
> +	vmemmap_free(page, nr_pages);
>   }
>   #else
>   static struct page *__kmalloc_section_memmap(unsigned long nr_pages)

^ permalink raw reply

* Re: [PATCH 1/2] vfio powerpc: enabled on powernv platform
From: Alexey Kardashevskiy @ 2012-12-04  8:12 UTC (permalink / raw)
  To: Alex Williamson
  Cc: kvm, linux-kernel, Paul Mackerras, linuxppc-dev, David Gibson
In-Reply-To: <1354556112.1809.369.camel@bling.home>

On 04/12/12 04:35, Alex Williamson wrote:
> On Mon, 2012-12-03 at 13:52 +1100, Alexey Kardashevskiy wrote:
>> This patch initializes IOMMU groups based on the IOMMU
>> configuration discovered during the PCI scan on POWERNV
>> (POWER non virtualized) platform. The IOMMU groups are
>> to be used later by VFIO driver (PCI pass through).
>>
>> It also implements an API for mapping/unmapping pages for
>> guest PCI drivers and providing DMA window properties.
>> This API is going to be used later by QEMU-VFIO to handle
>> h_put_tce hypercalls from the KVM guest.
>>
>> Although this driver has been tested only on the POWERNV
>> platform, it should work on any platform which supports
>> TCE tables.
>>
>> To enable VFIO on POWER, enable SPAPR_TCE_IOMMU config
>> option and configure VFIO as required.
>>
>> Cc: David Gibson <david@gibson.dropbear.id.au>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>   arch/powerpc/include/asm/iommu.h     |    9 ++
>>   arch/powerpc/kernel/iommu.c          |  186 ++++++++++++++++++++++++++++++++++
>>   arch/powerpc/platforms/powernv/pci.c |  135 ++++++++++++++++++++++++
>>   drivers/iommu/Kconfig                |    8 ++
>>   4 files changed, 338 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
>> index cbfe678..5c7087a 100644
>> --- a/arch/powerpc/include/asm/iommu.h
>> +++ b/arch/powerpc/include/asm/iommu.h
>> @@ -76,6 +76,9 @@ struct iommu_table {
>>   	struct iommu_pool large_pool;
>>   	struct iommu_pool pools[IOMMU_NR_POOLS];
>>   	unsigned long *it_map;       /* A simple allocation bitmap for now */
>> +#ifdef CONFIG_IOMMU_API
>> +	struct iommu_group *it_group;
>> +#endif
>>   };
>>
>>   struct scatterlist;
>> @@ -147,5 +150,11 @@ static inline void iommu_restore(void)
>>   }
>>   #endif
>>
>> +extern long iommu_clear_tces(struct iommu_table *tbl, unsigned long entry,
>> +		unsigned long pages);
>> +extern long iommu_put_tces(struct iommu_table *tbl, unsigned long entry,
>> +		uint64_t tce, enum dma_data_direction direction,
>> +		unsigned long pages);
>> +
>>   #endif /* __KERNEL__ */
>>   #endif /* _ASM_IOMMU_H */
>> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
>> index ff5a6ce..2738aa4 100644
>> --- a/arch/powerpc/kernel/iommu.c
>> +++ b/arch/powerpc/kernel/iommu.c
>> @@ -44,6 +44,7 @@
>>   #include <asm/kdump.h>
>>   #include <asm/fadump.h>
>>   #include <asm/vio.h>
>> +#include <asm/tce.h>
>>
>>   #define DBG(...)
>>
>> @@ -856,3 +857,188 @@ void iommu_free_coherent(struct iommu_table *tbl, size_t size,
>>   		free_pages((unsigned long)vaddr, get_order(size));
>>   	}
>>   }
>> +
>> +#ifdef CONFIG_IOMMU_API
>> +/*
>> + * SPAPR TCE API
>> + */
>> +
>> +/*
>> + * Returns the number of used IOMMU pages (4K) within
>> + * the same system page (4K or 64K).
>> + * bitmap_weight is not used as it does not support bigendian maps.
>> + */
>> +static int syspage_weight(unsigned long *map, unsigned long entry)
>> +{
>> +	int ret = 0, nbits = PAGE_SIZE/IOMMU_PAGE_SIZE;
>> +
>> +	/* Aligns TCE entry number to system page boundary */
>> +	entry &= PAGE_MASK >> IOMMU_PAGE_SHIFT;
>> +
>> +	/* Count used 4K pages */
>> +	while (nbits--)
>> +		ret += (test_bit(entry++, map) == 0) ? 0 : 1;
>> +
>> +	return ret;
>> +}
>> +
>> +static void tce_flush(struct iommu_table *tbl)
>> +{
>> +	/* Flush/invalidate TLB caches if necessary */
>> +	if (ppc_md.tce_flush)
>> +		ppc_md.tce_flush(tbl);
>> +
>> +	/* Make sure updates are seen by hardware */
>> +	mb();
>> +}
>> +
>> +/*
>> + * iommu_clear_tces clears tces and returned the number of system pages
>> + * which it called put_page() on
>> + */
>> +static long clear_tces_nolock(struct iommu_table *tbl, unsigned long entry,
>> +		unsigned long pages)
>> +{
>> +	int i, retpages = 0;
>> +	unsigned long oldtce, oldweight;
>> +	struct page *page;
>> +
>> +	for (i = 0; i < pages; ++i) {
>> +		oldtce = ppc_md.tce_get(tbl, entry + i);
>> +		ppc_md.tce_free(tbl, entry + i, 1);
>> +
>> +		oldweight = syspage_weight(tbl->it_map, entry);
>> +		__clear_bit(entry - tbl->it_offset, tbl->it_map);
>> +
>> +		if (!(oldtce & (TCE_PCI_WRITE | TCE_PCI_READ)))
>> +			continue;
>
> Could this happen earlier, above syspage_weight() and __clear_bit()?


Want to clear it anyway if it is not cleared by some reason. Added WARN_ON.


>> +
>> +		page = pfn_to_page(oldtce >> PAGE_SHIFT);
>> +
>> +		WARN_ON(!page);
>> +		if (!page)
>> +			continue;
>> +
>> +		if (oldtce & TCE_PCI_WRITE)
>> +			SetPageDirty(page);
>> +
>> +		put_page(page);
>> +
>> +		/* That was the last IOMMU page within the system page */
>> +		if ((oldweight == 1) && !syspage_weight(tbl->it_map, entry))
>> +			++retpages;
>
> If you used __test_and_clear_bit() above I think you could avoid this
> 2nd call to syspage_weight.  A minor optimization though.
>
>> +	}
>> +
>> +	return retpages;
>> +}
>> +
>> +/*
>> + * iommu_clear_tces clears tces and returned the number
>> + / of released system pages
>> + */
>
> Something bad happened to your comments here.
>
>> +long iommu_clear_tces(struct iommu_table *tbl, unsigned long entry,
>> +		unsigned long pages)
>> +{
>> +	int ret;
>> +	struct iommu_pool *pool = get_pool(tbl, entry);
>> +
>> +	spin_lock(&(pool->lock));
>> +	ret = clear_tces_nolock(tbl, entry, pages);
>> +	tce_flush(tbl);
>> +	spin_unlock(&(pool->lock));
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_clear_tces);
>> +
>> +static int put_tce(struct iommu_table *tbl, unsigned long entry,
>> +		uint64_t tce, enum dma_data_direction direction)
>> +{
>> +	int ret;
>> +	struct page *page = NULL;
>> +	unsigned long kva, offset, oldweight;
>> +
>> +	/* Map new TCE */
>> +	offset = (tce & IOMMU_PAGE_MASK) - (tce & PAGE_MASK);
>
> Maybe the compiler will figure this out, but isn't this the same as tce
> & (IOMMU_PAGE_MASK & PAGE_MASK)?


it is rather (tce & (IOMMU_PAGE_MASK & ~PAGE_MASK)) but I cannot see how it 
is simpler and I doubt that it is faster enough to notice it anyhow :)


>> +	ret = get_user_pages_fast(tce & PAGE_MASK, 1,
>> +			direction != DMA_TO_DEVICE, &page);
>> +	if (ret < 1) {
>
> Probably (ret != 1) here or else we never get to your >1 case below.
>
>> +		printk(KERN_ERR "tce_vfio: get_user_pages_fast failed tce=%llx ioba=%lx ret=%d\n",
>> +				tce, entry << IOMMU_PAGE_SHIFT, ret);
>
> Use pr_err
 >
>> +		if (!ret || (ret > 1))
>
> Then (ret >= 0) here.  Or return (ret >= 0) ? -EFAULT : ret
>
>> +			ret = -EFAULT;
>> +		return ret;
>> +	}
>
> You're missing the code from x86 that handles mapping mmap'd ranges.
> This is intended to allow peer-to-peer DMA between devices.  Is that
> intentional?

I am not following you here. What code exactly are talking about? We do not 
track ranges at all and I do not see how it helps with p2p dma.


>> +
>> +	kva = (unsigned long) page_address(page);
>> +	kva += offset;
>> +
>> +	/* tce_build receives a virtual address */
>> +	entry += tbl->it_offset; /* Offset into real TCE table */
>
> Here's what makes me call the entry "relative" rather than zero-based.

This is the bug actually, I overlooked it and I removed it now. Thanks for 
being so picky :)


> The iova is relative to the start of dma32_window_start, ie. if the
> window starts at bus address 512MB and I want to create a translation at
> bus address 512MB, I pass in an iova of 0, right?  The above adds the
> window offset.  So you've removed dma64 window, but we really need to
> define iova better.




>> +	ret = ppc_md.tce_build(tbl, entry, 1, kva, direction, NULL);
>> +
>> +	/* tce_build() only returns non-zero for transient errors */
>> +	if (unlikely(ret)) {
>> +		printk(KERN_ERR "tce_vfio: tce_put failed on tce=%llx ioba=%lx kva=%lx ret=%d\n",
>> +				tce, entry << IOMMU_PAGE_SHIFT, kva, ret);
>
> Use pr_err
>
>> +		put_page(page);
>> +		return -EIO;
>> +	}
>> +
>> +	/* Calculate if new system page has been locked */
>> +	oldweight = syspage_weight(tbl->it_map, entry);
>> +	__set_bit(entry - tbl->it_offset, tbl->it_map);
>> +
>> +	return (oldweight == 0) ? 1 : 0;
>> +}
>> +
>> +/*
>> + * iommu_put_tces builds tces and returned the number of actually
>> + * locked system pages
>> + */
>> +long iommu_put_tces(struct iommu_table *tbl, unsigned long entry,
>> +		uint64_t tce, enum dma_data_direction direction,
>> +		unsigned long pages)
>> +{
>> +	int i, ret = 0, retpages = 0;
>> +	struct iommu_pool *pool = get_pool(tbl, entry);
>> +
>> +	BUILD_BUG_ON(PAGE_SIZE < IOMMU_PAGE_SIZE);
>> +	BUG_ON(direction == DMA_NONE);
>
> This doesn't seem BUG worthy, -EINVAL?  We can't assume tce_iommu_ioctl
> will always be the only caller of this function.


This is what other function does in this file.


>> +
>> +	spin_lock(&(pool->lock));
>> +
>> +	/* Check if any is in use */
>> +	for (i = 0; i < pages; ++i) {
>> +		unsigned long oldtce = ppc_md.tce_get(tbl, entry + i);
>> +		if ((oldtce & (TCE_PCI_WRITE | TCE_PCI_READ)) ||
>> +				test_bit(entry + i, tbl->it_map)) {
>> +			WARN_ON(test_bit(entry + i, tbl->it_map));
>
> The WARN_ON seems to confirm that these are redundant tests, does that
> imply we don't trust it_map?  It would be a lot faster if we could rely
> on it_map exclusively here.


As for me, pretty minor optimization. I'm testing it now to see if I do not 
miss bits.



>> +			spin_unlock(&(pool->lock));
>> +			return -EBUSY;
>> +		}
>> +	}
>> +
>> +	/* Put tces to the table */
>> +	for (i = 0; (i < pages) && (ret >= 0); ++i, tce += IOMMU_PAGE_SIZE) {
>> +		ret = put_tce(tbl, entry + i, tce, direction);
>> +		if (ret == 1)
>> +			++retpages;
>> +	}
>> +
>> +	/*
>> +	 * If failed, release locked pages, otherwise return the number
>> +	 * of locked system pages
>> +	 */
>> +	if (ret < 0)
>> +		clear_tces_nolock(tbl, entry, i);
>> +	else
>> +		ret = retpages;
>> +
>> +	tce_flush(tbl);
>> +	spin_unlock(&(pool->lock));
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_put_tces);
>> +#endif /* CONFIG_IOMMU_API */
>> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
>> index 05205cf..21250ef 100644
>> --- a/arch/powerpc/platforms/powernv/pci.c
>> +++ b/arch/powerpc/platforms/powernv/pci.c
>> @@ -20,6 +20,7 @@
>>   #include <linux/irq.h>
>>   #include <linux/io.h>
>>   #include <linux/msi.h>
>> +#include <linux/iommu.h>
>>
>>   #include <asm/sections.h>
>>   #include <asm/io.h>
>> @@ -613,3 +614,137 @@ void __init pnv_pci_init(void)
>>   	ppc_md.teardown_msi_irqs = pnv_teardown_msi_irqs;
>>   #endif
>>   }
>> +
>> +#ifdef CONFIG_IOMMU_API
>> +/*
>> + * IOMMU groups support required by VFIO
>> + */
>> +static int add_device(struct device *dev)
>> +{
>> +	struct iommu_table *tbl;
>> +	int ret = 0;
>> +
>> +	if (WARN_ON(dev->iommu_group)) {
>> +		printk(KERN_WARNING "tce_vfio: device %s is already in iommu group %d, skipping\n",
>> +				dev_name(dev),
>> +				iommu_group_id(dev->iommu_group));
>
> Use pr_warn
>
>> +		return -EBUSY;
>> +	}
>> +
>> +	tbl = get_iommu_table_base(dev);
>> +	if (!tbl) {
>> +		pr_debug("tce_vfio: skipping device %s with no tbl\n",
>> +				dev_name(dev));
>> +		return 0;
>> +	}
>> +
>> +	pr_debug("tce_vfio: adding %s to iommu group %d\n",
>> +			dev_name(dev), iommu_group_id(tbl->it_group));
>> +
>> +	ret = iommu_group_add_device(tbl->it_group, dev);
>> +	if (ret < 0)
>> +		printk(KERN_ERR "tce_vfio: %s has not been added, ret=%d\n",
>> +				dev_name(dev), ret);
>
> Use pr_err
>
>> +
>> +	return ret;
>> +}
>> +
>> +static void del_device(struct device *dev)
>> +{
>> +	iommu_group_remove_device(dev);
>> +}
>> +
>> +static int iommu_bus_notifier(struct notifier_block *nb,
>> +			      unsigned long action, void *data)
>> +{
>> +	struct device *dev = data;
>> +
>> +	switch (action) {
>> +	case BUS_NOTIFY_ADD_DEVICE:
>> +		return add_device(dev);
>> +	case BUS_NOTIFY_DEL_DEVICE:
>> +		del_device(dev);
>> +		return 0;
>> +	default:
>> +		return 0;
>> +	}
>> +}
>> +
>> +static struct notifier_block tce_iommu_bus_nb = {
>> +	.notifier_call = iommu_bus_notifier,
>> +};
>> +
>> +static void group_release(void *iommu_data)
>> +{
>> +	struct iommu_table *tbl = iommu_data;
>> +	tbl->it_group = NULL;
>> +}
>> +
>> +static int __init tce_iommu_init(void)
>> +{
>> +	struct pci_dev *pdev = NULL;
>> +	struct iommu_table *tbl;
>> +	struct iommu_group *grp;
>> +
>> +	/* Allocate and initialize IOMMU groups */
>> +	for_each_pci_dev(pdev) {
>> +		tbl = get_iommu_table_base(&pdev->dev);
>> +		if (!tbl)
>> +			continue;
>> +
>> +		/* Skip already initialized */
>> +		if (tbl->it_group)
>> +			continue;
>> +
>> +		grp = iommu_group_alloc();
>> +		if (IS_ERR(grp)) {
>> +			printk(KERN_INFO "tce_vfio: cannot create "
>> +					"new IOMMU group, ret=%ld\n",
>> +					PTR_ERR(grp));
>
> Use pr_info
>
>> +			return PTR_ERR(grp);
>> +		}
>> +		tbl->it_group = grp;
>> +		iommu_group_set_iommudata(grp, tbl, group_release);
>> +	}
>> +
>> +	bus_register_notifier(&pci_bus_type, &tce_iommu_bus_nb);
>> +
>> +	/* Add PCI devices to VFIO groups */
>> +	for_each_pci_dev(pdev)
>> +		add_device(&pdev->dev);
>> +
>> +	return 0;
>> +}
>> +
>> +static void __exit tce_iommu_cleanup(void)
>> +{
>> +	struct pci_dev *pdev = NULL;
>> +	struct iommu_table *tbl;
>> +	struct iommu_group *grp = NULL;
>> +
>> +	bus_unregister_notifier(&pci_bus_type, &tce_iommu_bus_nb);
>> +
>> +	/* Delete PCI devices from VFIO groups */
>> +	for_each_pci_dev(pdev)
>> +		del_device(&pdev->dev);
>> +
>> +	/* Release VFIO groups */
>> +	for_each_pci_dev(pdev) {
>> +		tbl = get_iommu_table_base(&pdev->dev);
>> +		if (!tbl)
>> +			continue;
>> +		grp = tbl->it_group;
>> +
>> +		/* Skip (already) uninitialized */
>> +		if (!grp)
>> +			continue;
>> +
>> +		/* Do actual release, group_release() is expected to work */
>> +		iommu_group_put(grp);
>> +		BUG_ON(tbl->it_group);
>> +	}
>> +}
>> +
>> +module_init(tce_iommu_init);
>> +module_exit(tce_iommu_cleanup);
>> +#endif /* CONFIG_IOMMU_API */
>> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
>> index 9f69b56..29d11dc 100644
>> --- a/drivers/iommu/Kconfig
>> +++ b/drivers/iommu/Kconfig
>> @@ -187,4 +187,12 @@ config EXYNOS_IOMMU_DEBUG
>>
>>   	  Say N unless you need kernel log message for IOMMU debugging
>>
>> +config SPAPR_TCE_IOMMU
>> +	bool "sPAPR TCE IOMMU Support"
>> +	depends on PPC_POWERNV
>> +	select IOMMU_API
>> +	help
>> +	  Enables bits of IOMMU API required by VFIO. The iommu_ops is
>> +	  still not implemented.
>> +
>>   endif # IOMMU_SUPPORT
>
> Thanks,
>
> Alex
>


-- 
Alexey

^ permalink raw reply

* Re: [PATCH powerpc] Fix MAX_STACK_TRACE_ENTRIES too low warning for ppc32
From: Li Zhong @ 2012-12-04  5:41 UTC (permalink / raw)
  To: Denis Kirjanov; +Cc: Christian Kujau, Paul Mackerras, PowerPC email list
In-Reply-To: <CAHj3AVmF_NV4YoG9TOgWjJ34gCz=+9qtvAKA3o00gxnsRJQdFg@mail.gmail.com>

On Mon, 2012-12-03 at 11:03 +0300, Denis Kirjanov wrote:
> Could you please provide a more verbose patch description

Sorry about the missing background. You may check

[0] http://lkml.indiana.edu/hypermail/linux/kernel/1211.0/03025.html
http://patchwork.ozlabs.org/patch/193414/
http://patchwork.ozlabs.org/patch/202385/

And as it is very similar to that of commit 12660b17, so I think it's
not needed to repeat that in the change log. 

Thanks, Zhong

> 
> Thanks.
> 
> On 12/3/12, Li Zhong <zhong@linux.vnet.ibm.com> wrote:
> > This patch fixes MAX_STACK_TRACE_ENTRIES too low warning for ppc32,
> > which is similar to commit 12660b17.
> >
> > Reported-by: Christian Kujau <lists@nerdbynature.de>
> > Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
> > Tested-by: Christian Kujau <lists@nerdbynature.de>
> > ---
> >  arch/powerpc/kernel/entry_32.S |    2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/powerpc/kernel/entry_32.S
> > b/arch/powerpc/kernel/entry_32.S
> > index d22e73e..e514de5 100644
> > --- a/arch/powerpc/kernel/entry_32.S
> > +++ b/arch/powerpc/kernel/entry_32.S
> > @@ -439,6 +439,8 @@ ret_from_fork:
> >  ret_from_kernel_thread:
> >  	REST_NVGPRS(r1)
> >  	bl	schedule_tail
> > +	li	r3,0
> > +	stw	r3,0(r1)
> >  	mtlr	r14
> >  	mr	r3,r15
> >  	PPC440EP_ERR42
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/linuxppc-dev
> >
> 
> 

^ permalink raw reply

* [PATCH -next] powerpc/82xx: use for_each_compatible_node() macro
From: Wei Yongjun @ 2012-12-04  5:20 UTC (permalink / raw)
  To: benh, paulus; +Cc: yongjun_wei, linuxppc-dev, linux-kernel

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Use for_each_compatible_node() macro instead of open coding it.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 arch/powerpc/platforms/82xx/pq2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/82xx/pq2.c b/arch/powerpc/platforms/82xx/pq2.c
index fb94d10..fc8b2d6 100644
--- a/arch/powerpc/platforms/82xx/pq2.c
+++ b/arch/powerpc/platforms/82xx/pq2.c
@@ -71,11 +71,11 @@ err:
 
 void __init pq2_init_pci(void)
 {
-	struct device_node *np = NULL;
+	struct device_node *np;
 
 	ppc_md.pci_exclude_device = pq2_pci_exclude_device;
 
-	while ((np = of_find_compatible_node(np, NULL, "fsl,pq2-pci")))
+	for_each_compatible_node(np, NULL, "fsl,pq2-pci")
 		pq2_pci_add_bridge(np);
 }
 #endif

^ permalink raw reply related

* [PATCH -next] [POWERPC] celleb: use for_each_compatible_node() macro
From: Wei Yongjun @ 2012-12-04  5:17 UTC (permalink / raw)
  To: arnd, benh, paulus; +Cc: cbe-oss-dev, yongjun_wei, linuxppc-dev, linux-kernel

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Use for_each_compatible_node() macro instead of open coding it.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 arch/powerpc/platforms/cell/celleb_scc_sio.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/cell/celleb_scc_sio.c b/arch/powerpc/platforms/cell/celleb_scc_sio.c
index 3a16c5b..75d71a7 100644
--- a/arch/powerpc/platforms/cell/celleb_scc_sio.c
+++ b/arch/powerpc/platforms/cell/celleb_scc_sio.c
@@ -42,14 +42,13 @@ static struct {
 static int __init txx9_serial_init(void)
 {
 	extern int early_serial_txx9_setup(struct uart_port *port);
-	struct device_node *node = NULL;
+	struct device_node *node;
 	int i;
 	struct uart_port req;
 	struct of_irq irq;
 	struct resource res;
 
-	while ((node = of_find_compatible_node(node,
-				"serial", "toshiba,sio-scc")) != NULL) {
+	for_each_compatible_node(node, "serial", "toshiba,sio-scc") {
 		for (i = 0; i < ARRAY_SIZE(txx9_scc_tab); i++) {
 			if (!(txx9_serial_bitmap & (1<<i)))
 				continue;

^ permalink raw reply related

* [PATCH -next] TTY: hvsi: use for_each_compatible_node() macro
From: Wei Yongjun @ 2012-12-04  5:16 UTC (permalink / raw)
  To: gregkh, grant.likely, rob.herring
  Cc: yongjun_wei, linuxppc-dev, devicetree-discuss, linux-kernel

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Use for_each_compatible_node() macro instead of open coding it.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/tty/hvc/hvsi.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
index 5b95b4f..70e0ef7 100644
--- a/drivers/tty/hvc/hvsi.c
+++ b/drivers/tty/hvc/hvsi.c
@@ -1187,9 +1187,7 @@ static int __init hvsi_console_init(void)
 	hvsi_wait = poll_for_state; /* no irqs yet; must poll */
 
 	/* search device tree for vty nodes */
-	for (vty = of_find_compatible_node(NULL, "serial", "hvterm-protocol");
-			vty != NULL;
-			vty = of_find_compatible_node(vty, "serial", "hvterm-protocol")) {
+	for_each_compatible_node(vty, "serial", "hvterm-protocol") {
 		struct hvsi_struct *hp;
 		const uint32_t *vtermno, *irq;
 

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox