LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH]->[PATCH v2] kgdb: Removed kmalloc returned value cast
From: tiejun.chen @ 2013-03-11  1:33 UTC (permalink / raw)
  To: Alex Grad, Benjamin Herrenschmidt, Jason Wessel
  Cc: sfr, mikey, daniel.baluta, linux-kernel, penberg, paulus,
	jason.wessel, linuxppc-dev
In-Reply-To: <1362955188-3023-1-git-send-email-alex.grad@gmail.com>

On 03/11/2013 06:39 AM, Alex Grad wrote:
> While at it, check kmalloc return value.
>
> Signed-off-by: Alex Grad <alex.grad@gmail.com>
> ---
>   arch/powerpc/kernel/kgdb.c |    5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
> index 5ca82cd..9e81dd8 100644
> --- a/arch/powerpc/kernel/kgdb.c
> +++ b/arch/powerpc/kernel/kgdb.c
> @@ -159,7 +159,10 @@ static int kgdb_singlestep(struct pt_regs *regs)
>   	if (user_mode(regs))
>   		return 0;
>
> -	backup_current_thread_info = (struct thread_info *)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
> +	backup_current_thread_info = kmalloc(sizeof(struct thread_info), GFP_KERNEL);
> +	if (!backup_current_thread_info)
> +		return -ENOMEM;

I already send a kgdb patchset in "[v3][PATCH 6/6] powerpc/kgdb: remove copying 
the thread_info" to remove these stuff since its unnecessary to copy current 
thread_info now.

Tiejun

^ permalink raw reply

* [PATCH]->[PATCH v2] kgdb: Removed kmalloc returned value cast
From: Alex Grad @ 2013-03-10 22:39 UTC (permalink / raw)
  To: penberg
  Cc: sfr, mikey, daniel.baluta, linux-kernel, tiejun.chen, paulus,
	Alex Grad, jason.wessel, linuxppc-dev

While at it, check kmalloc return value.

Signed-off-by: Alex Grad <alex.grad@gmail.com>
---
 arch/powerpc/kernel/kgdb.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index 5ca82cd..9e81dd8 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -159,7 +159,10 @@ static int kgdb_singlestep(struct pt_regs *regs)
 	if (user_mode(regs))
 		return 0;
 
-	backup_current_thread_info = (struct thread_info *)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
+	backup_current_thread_info = kmalloc(sizeof(struct thread_info), GFP_KERNEL);
+	if (!backup_current_thread_info)
+		return -ENOMEM;
+
 	/*
 	 * On Book E and perhaps other processors, singlestep is handled on
 	 * the critical exception stack.  This causes current_thread_info()
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH] kgdb: Removed kmalloc returned value cast
From: Daniel Baluta @ 2013-03-10 19:05 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: mikey, fr, linux-kernel, tiejun.chen, paulus, Alex Grad,
	jason.wessel, linuxppc-dev
In-Reply-To: <CAOJsxLGD1fXsGMCQbP8aAMDsRiwPD2hHcfWfupO4jy5=JCM=NA@mail.gmail.com>

On Sun, Mar 10, 2013 at 4:10 PM, Pekka Enberg <penberg@kernel.org> wrote:
> On Sun, Mar 10, 2013 at 3:06 PM, Alex Grad <alex.grad@gmail.com> wrote:
>> Signed-off-by: Alex Grad <alex.grad@gmail.com>
>> ---
>>  arch/powerpc/kernel/kgdb.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
>> index 5ca82cd..c1eef24 100644
>> --- a/arch/powerpc/kernel/kgdb.c
>> +++ b/arch/powerpc/kernel/kgdb.c
>> @@ -159,7 +159,7 @@ static int kgdb_singlestep(struct pt_regs *regs)
>>         if (user_mode(regs))
>>                 return 0;
>>
>> -       backup_current_thread_info = (struct thread_info *)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
>> +       backup_current_thread_info = kmalloc(sizeof(struct thread_info), GFP_KERNEL);
>
> Looks good to me.
>
> There's two additional problems in the kgdb_singlestep() function: it
> doesn't check if the kmalloc() call fails nor does it kfree() the
> allocated memory.

Hi Pekka,

Indeed there is no check if kmalloc() fails. This should be fixed.

Anyhow, commit fefd9e6f powerpc: kernel/kgdb.c: Fix memory leakage
fixed the problem with kfree.

thanks,
Daniel.

^ permalink raw reply

* Re: [PATCH] kgdb: Removed kmalloc returned value cast
From: Pekka Enberg @ 2013-03-10 14:10 UTC (permalink / raw)
  To: Alex Grad
  Cc: mikey, fr, linux-kernel, tiejun.chen, paulus, jason.wessel,
	linuxppc-dev
In-Reply-To: <1362920779-25925-1-git-send-email-alex.grad@gmail.com>

On Sun, Mar 10, 2013 at 3:06 PM, Alex Grad <alex.grad@gmail.com> wrote:
> Signed-off-by: Alex Grad <alex.grad@gmail.com>
> ---
>  arch/powerpc/kernel/kgdb.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
> index 5ca82cd..c1eef24 100644
> --- a/arch/powerpc/kernel/kgdb.c
> +++ b/arch/powerpc/kernel/kgdb.c
> @@ -159,7 +159,7 @@ static int kgdb_singlestep(struct pt_regs *regs)
>         if (user_mode(regs))
>                 return 0;
>
> -       backup_current_thread_info = (struct thread_info *)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
> +       backup_current_thread_info = kmalloc(sizeof(struct thread_info), GFP_KERNEL);

Looks good to me.

There's two additional problems in the kgdb_singlestep() function: it
doesn't check if the kmalloc() call fails nor does it kfree() the
allocated memory.

                        Pekka

^ permalink raw reply

* [PATCH] powerpc: place EXPORT_SYMBOL macro right after declaration
From: Valentina Manea @ 2013-03-10 13:22 UTC (permalink / raw)
  To: benh
  Cc: Valentina Manea, agraf, linux-kernel, suzuki, paulus, akpm,
	linuxppc-dev

This fixes the following checkpatch.pl warnings:

WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
+EXPORT_SYMBOL(kmap_prot);

WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
+EXPORT_SYMBOL(kmap_pte);

Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
---
 arch/powerpc/mm/mem.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index f1f7409..056732e 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -66,10 +66,9 @@ unsigned long long memory_limit;
 
 #ifdef CONFIG_HIGHMEM
 pte_t *kmap_pte;
+EXPORT_SYMBOL(kmap_pte);
 pgprot_t kmap_prot;
-
 EXPORT_SYMBOL(kmap_prot);
-EXPORT_SYMBOL(kmap_pte);
 
 static inline pte_t *virt_to_kpte(unsigned long vaddr)
 {
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH] powerpc: Use PTR_RET instead of IS_ERR/PTR_ERR
From: Adrian-Leonard Radu @ 2013-03-10 13:07 UTC (permalink / raw)
  To: benh
  Cc: cbe-oss-dev, geoff, fweisbec, akinobu.mita, rostedt, linux-kernel,
	paulus, anton, john.stultz, ady8radu, linuxppc-dev, cascardo

Signed-off-by: Adrian-Leonard Radu <ady8radu@gmail.com>
---
 arch/powerpc/kernel/iommu.c          |    2 +-
 arch/powerpc/kernel/time.c           |    4 +---
 arch/powerpc/platforms/ps3/time.c    |    4 +---
 arch/powerpc/sysdev/rtc_cmos_setup.c |    5 +----
 4 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 31c4fdc..c0d0dbd 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -102,7 +102,7 @@ static int __init fail_iommu_debugfs(void)
 	struct dentry *dir = fault_create_debugfs_attr("fail_iommu",
 						       NULL, &fail_iommu);
 
-	return IS_ERR(dir) ? PTR_ERR(dir) : 0;
+	return PTR_RET(dir);
 }
 late_initcall(fail_iommu_debugfs);
 
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index f77fa22..5fc29ad 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -1049,10 +1049,8 @@ static int __init rtc_init(void)
 		return -ENODEV;
 
 	pdev = platform_device_register_simple("rtc-generic", -1, NULL, 0);
-	if (IS_ERR(pdev))
-		return PTR_ERR(pdev);
 
-	return 0;
+	return PTR_RET(pdev);
 }
 
 module_init(rtc_init);
diff --git a/arch/powerpc/platforms/ps3/time.c b/arch/powerpc/platforms/ps3/time.c
index 40b5cb4..cba1e6b 100644
--- a/arch/powerpc/platforms/ps3/time.c
+++ b/arch/powerpc/platforms/ps3/time.c
@@ -89,10 +89,8 @@ static int __init ps3_rtc_init(void)
 		return -ENODEV;
 
 	pdev = platform_device_register_simple("rtc-ps3", -1, NULL, 0);
-	if (IS_ERR(pdev))
-		return PTR_ERR(pdev);
 
-	return 0;
+	return PTR_RET(pdev);
 }
 
 module_init(ps3_rtc_init);
diff --git a/arch/powerpc/sysdev/rtc_cmos_setup.c b/arch/powerpc/sysdev/rtc_cmos_setup.c
index 9afba92..af79e1e 100644
--- a/arch/powerpc/sysdev/rtc_cmos_setup.c
+++ b/arch/powerpc/sysdev/rtc_cmos_setup.c
@@ -62,10 +62,7 @@ static int  __init add_rtc(void)
 	pd = platform_device_register_simple("rtc_cmos", -1,
 					     &res[0], num_res);
 
-	if (IS_ERR(pd))
-		return PTR_ERR(pd);
-
-	return 0;
+	return PTR_RET(pd);
 }
 fs_initcall(add_rtc);
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH] kgdb: Removed kmalloc returned value cast
From: Alex Grad @ 2013-03-10 13:06 UTC (permalink / raw)
  To: benh
  Cc: mikey, fr, linux-kernel, tiejun.chen, paulus, Alex Grad,
	jason.wessel, linuxppc-dev

Signed-off-by: Alex Grad <alex.grad@gmail.com>
---
 arch/powerpc/kernel/kgdb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index 5ca82cd..c1eef24 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -159,7 +159,7 @@ static int kgdb_singlestep(struct pt_regs *regs)
 	if (user_mode(regs))
 		return 0;
 
-	backup_current_thread_info = (struct thread_info *)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
+	backup_current_thread_info = kmalloc(sizeof(struct thread_info), GFP_KERNEL);
 	/*
 	 * On Book E and perhaps other processors, singlestep is handled on
 	 * the critical exception stack.  This causes current_thread_info()
-- 
1.7.10.4

^ permalink raw reply related

* Re: Linux kernel 3.x problems on PowerMac G5
From: Benjamin Herrenschmidt @ 2013-03-10 12:52 UTC (permalink / raw)
  To: Phileas Fogg; +Cc: Andreas Schwab, Denis Kirjanov, linuxppc-dev, Aaro Koskinen
In-Reply-To: <513C6641.2010308@mail.ru>

On Sun, 2013-03-10 at 11:53 +0100, Phileas Fogg wrote:
> Good news :) I found the bug.
> MMU features were not set properly for PPC970MP DD1.0 which,
> unfortunately, my machine has.
> Damn, one line fix but one week searching.
> Linux 3.8.2 boots without problems now :)

Nice one ! I didn't think anybody shipped a DD1.0 chip ! :-)

Looks like some typo/thinko in the cputable and you are one of the very
rare victims of it. I'll fix that up. Regarding the IDE problem, can you
shoot a note to Tejun who wrote that patch (and CC me) ? I do recommend
switching to libata but we should still fix the problem with legacy IDE.

Cheers,
Ben.

^ permalink raw reply

* Re: Linux kernel 3.x problems on PowerMac G5
From: Phileas Fogg @ 2013-03-10 10:53 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Andreas Schwab, Denis Kirjanov, linuxppc-dev, Aaro Koskinen
In-Reply-To: <1362876308.6977.42.camel@pasglop>

Good news :) I found the bug.
MMU features were not set properly for PPC970MP DD1.0 which,
unfortunately, my machine has.
Damn, one line fix but one week searching.
Linux 3.8.2 boots without problems now :)


Here is my patch:

--- arch/powerpc/kernel/cputable.c.old	2013-03-10 11:48:56.559480758 +0100
+++ arch/powerpc/kernel/cputable.c	2013-03-10 11:41:07.522786804 +0100
@@ -275,7 +275,7 @@
  		.cpu_features		= CPU_FTRS_PPC970,
  		.cpu_user_features	= COMMON_USER_POWER4 |
  			PPC_FEATURE_HAS_ALTIVEC_COMP,
-		.mmu_features		= MMU_FTR_HPTE_TABLE,
+		.mmu_features		= MMU_FTRS_PPC970,
  		.icache_bsize		= 128,
  		.dcache_bsize		= 128,
  		.num_pmcs		= 8,



Regards

^ permalink raw reply

* Re: Linux kernel 3.x problems on PowerMac G5
From: Phileas Fogg @ 2013-03-10 10:19 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Andreas Schwab, Denis Kirjanov, linuxppc-dev, Aaro Koskinen
In-Reply-To: <1362876308.6977.42.camel@pasglop>

On 03/10/2013 01:45 AM, Benjamin Herrenschmidt wrote:
> On Sun, 2013-03-10 at 01:26 +0100, Phileas Fogg wrote:
>
>> i managed to find the bad commit after a couple of days bisecting.
>
> Thanks !
>
>> ----------------------------
>> 44ae3ab3358e962039c36ad4ae461ae9fb29596c is the first bad commit
>> commit 44ae3ab3358e962039c36ad4ae461ae9fb29596c
>> Author: Matt Evans <matt@ozlabs.org>
>> Date:   Wed Apr 6 19:48:50 2011 +0000
>>
>>       powerpc: Free up some CPU feature bits by moving out MMU-related
>> features
>>
>>       Some of the 64bit PPC CPU features are MMU-related, so this patch moves
>>       them to MMU_FTR_ bits.  All cpu_has_feature()-style tests are moved to
>>       mmu_has_feature(), and seven feature bits are freed as a result.
>>
>>       Signed-off-by: Matt Evans <matt@ozlabs.org>
>>       Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> --------------------------------
>
> Have you verified that if you checkout git at the above commit point, it
> fails and if you then just revert that commit on top, it works again ?
>
> The above should have been mostly a NOP change but I'll have a closer
> look in case a typo of some kind actually broke something.
>
>> Actually, there are 2 problems i found.
>> The first problem occurs when i enable IDE CDROM driver on my machine.
>> The following commit causes hangs on my machine at boot:
>
> Ok. You may want to switch to the new libata instead of the old IDE
> driver too
>
> (CONFIG_IDE off, CONFIG_ATA on, CONFIG_PATA_MACIO on and from there it
> will use the SCSI CDROM driver).
>
>> ----------------------
>> commit 5b03a1b140e13a28ff6be1526892a9dc538ddef6
>> Author: Tejun Heo <tj@kernel.org>
>> Date:   Wed Mar 9 19:54:27 2011 +0100
>>
>>       ide: Convert to bdops->check_events()
>>
>>       Convert ->media_changed() to the new ->check_events() method.  The
>>       conversion is mostly mechanical.  The only notable change is that
>>       cdrom now doesn't generate any event if @slot_nr isn't CDSL_CURRENT.
>>       It used to return -EINVAL which would be treated as media changed.  As
>>       media changer isn't supported anyway, this doesn't make any
>>       difference.
>>
>>       This makes ide emit the standard disk events and allows kernel event
>>       polling.  Currently, only MEDIA_CHANGE event is implemented.  Adding
>>       support for EJECT_REQUEST shouldn't be difficult; however, given that
>>       ide driver is already deprecated, it probably is best to leave it
>>       alone.
>>
>>       Signed-off-by: Tejun Heo <tj@kernel.org>
>> ----------------------------
>>
>>
>>
>>
>> If i disable IDE CDROM driver then the Linux kernel boots again
>> and then it hits the commit 44ae3ab3358e962039c36ad4ae461ae9fb29596c
>> and hangs again :)
>>
>> The commit eca590f402332ab873d13f2d8d00fa0b91cfff36 which is before
>> the commit 44ae3ab3358e962039c36ad4ae461ae9fb29596c works fine,
>> i tested it myself to be on the safe side.
>
> Ok thanks. I'll dig a bit if I get a chance next week.
>
> Cheers,
> Ben.
>>
>>
>> Regards
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>


I'm trying to debug the problem and printed CPU and MMU features
before and after this bad commit. And found i think i found the problem.
At least i got very strange results.

The following code i did add to arch/powerpc/kernel/setup_64.c:setup_system
------------------------------------------------------------
printk("cpu_features %lx\n", cur_cpu_spec->cpu_features);
printk("mmu_features %lx\n", cur_cpu_spec->mmu_features);
------------------------------------------------------------


CPU features before 44ae3ab3358e962039c36ad4ae461ae9fb29596c commit:

cpu_features 0x24000c718100448


CPU and MMU features after 44ae3ab3358e962039c36ad4ae461ae9fb29596c commit:

cpu_features 0x18100448
mmu_features 0x00000001

MMU features in 2nd case have only bit MMU_FTR_HPTE_TABLE set.
Where are the bits MMU_FTR_SLB, MMU_FTR_16M_PAGE and MMU_FTR_TLBIEL 
which introduced in commit 44ae3ab3358e962039c36ad4ae461ae9fb29596c ?
They should be set if i see it right in arch/powerpc/include/asm/mmu.h.

#define MMU_FTR_PPCAS_ARCH_V2 	(MMU_FTR_SLB | MMU_FTR_TLBIEL | \
				 MMU_FTR_16M_PAGE)
/* MMU feature bit sets for various CPUs */
#define MMU_FTRS_DEFAULT_HPTE_ARCH_V2	\
	MMU_FTR_HPTE_TABLE | MMU_FTR_PPCAS_ARCH_V2
#define MMU_FTRS_POWER4		MMU_FTRS_DEFAULT_HPTE_ARCH_V2
#define MMU_FTRS_PPC970		MMU_FTRS_POWER4


Hope it helps to fix the problem.

regards

^ permalink raw reply

* [PATCH v2, part2 07/10] mm/PPC: use free_highmem_page() to free highmem pages into buddy system
From: Jiang Liu @ 2013-03-10  8:01 UTC (permalink / raw)
  To: Andrew Morton, David Rientjes
  Cc: Jiang Liu, Wen Congyang, linux-mm, linux-kernel, Alexander Graf,
	Michal Hocko, Minchan Kim, Paul Mackerras, Mel Gorman,
	Suzuki K. Poulose, linuxppc-dev, KAMEZAWA Hiroyuki, Jianguo Wu
In-Reply-To: <1362902470-25787-1-git-send-email-jiang.liu@huawei.com>

Use helper function free_highmem_page() to free highmem pages into
the buddy system.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Jiang Liu <jiang.liu@huawei.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexander Graf <agraf@suse.de>
Cc: "Suzuki K. Poulose" <suzuki@in.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
---
 arch/powerpc/mm/mem.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index c756713..68f51d0 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -352,13 +352,9 @@ void __init mem_init(void)
 			struct page *page = pfn_to_page(pfn);
 			if (memblock_is_reserved(paddr))
 				continue;
-			ClearPageReserved(page);
-			init_page_count(page);
-			__free_page(page);
-			totalhigh_pages++;
+			free_higmem_page(page);
 			reservedpages--;
 		}
-		totalram_pages += totalhigh_pages;
 		printk(KERN_DEBUG "High memory: %luk\n",
 		       totalhigh_pages << (PAGE_SHIFT-10));
 	}
-- 
1.7.9.5

^ permalink raw reply related

* Re: Linux kernel 3.x problems on PowerMac G5
From: Phileas Fogg @ 2013-03-10  7:53 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Andreas Schwab, Denis Kirjanov, linuxppc-dev, Aaro Koskinen
In-Reply-To: <1362876308.6977.42.camel@pasglop>

On 03/10/2013 01:45 AM, Benjamin Herrenschmidt wrote:

>
> Have you verified that if you checkout git at the above commit point, it
> fails and if you then just revert that commit on top, it works again ?
>
> The above should have been mostly a NOP change but I'll have a closer
> look in case a typo of some kind actually broke something.
>

Verified, the commit 65f47f1339dfcffcd5837a307172fb41aa39e479 hangs too.
After reverting the changes of the commit 
44ae3ab3358e962039c36ad4ae461ae9fb29596c, it is booting fine again.

regards

^ permalink raw reply

* Re: [PATCH] drivers/tty/hvc: fixup original commit: 9276dfd27897a0b29d8b5814f39a1f82f56b6b6b
From: Chen Gang @ 2013-03-10  1:18 UTC (permalink / raw)
  To: David Laight; +Cc: Jiri Slaby, wfp5p, Greg KH, tklauser, linuxppc-dev, alan
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7182@saturn3.aculab.com>

于 2013年03月08日 19:11, David Laight 写道:
> Using strlcpy() also stops someone else having to check it
> again in a few years time.

  yes.

  :-)

-- 
Chen Gang

Asianux Corporation

^ permalink raw reply

* Re: Linux kernel 3.x problems on PowerMac G5
From: Benjamin Herrenschmidt @ 2013-03-10  0:45 UTC (permalink / raw)
  To: Phileas Fogg; +Cc: linuxppc-dev, Denis Kirjanov, Andreas Schwab, Aaro Koskinen
In-Reply-To: <513BD33B.7020305@mail.ru>

On Sun, 2013-03-10 at 01:26 +0100, Phileas Fogg wrote:

> i managed to find the bad commit after a couple of days bisecting.

Thanks !

> ----------------------------
> 44ae3ab3358e962039c36ad4ae461ae9fb29596c is the first bad commit
> commit 44ae3ab3358e962039c36ad4ae461ae9fb29596c
> Author: Matt Evans <matt@ozlabs.org>
> Date:   Wed Apr 6 19:48:50 2011 +0000
> 
>      powerpc: Free up some CPU feature bits by moving out MMU-related 
> features
> 
>      Some of the 64bit PPC CPU features are MMU-related, so this patch moves
>      them to MMU_FTR_ bits.  All cpu_has_feature()-style tests are moved to
>      mmu_has_feature(), and seven feature bits are freed as a result.
> 
>      Signed-off-by: Matt Evans <matt@ozlabs.org>
>      Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> --------------------------------

Have you verified that if you checkout git at the above commit point, it
fails and if you then just revert that commit on top, it works again ?

The above should have been mostly a NOP change but I'll have a closer
look in case a typo of some kind actually broke something.

> Actually, there are 2 problems i found.
> The first problem occurs when i enable IDE CDROM driver on my machine.
> The following commit causes hangs on my machine at boot:

Ok. You may want to switch to the new libata instead of the old IDE
driver too

(CONFIG_IDE off, CONFIG_ATA on, CONFIG_PATA_MACIO on and from there it
will use the SCSI CDROM driver).

> ----------------------
> commit 5b03a1b140e13a28ff6be1526892a9dc538ddef6
> Author: Tejun Heo <tj@kernel.org>
> Date:   Wed Mar 9 19:54:27 2011 +0100
> 
>      ide: Convert to bdops->check_events()
> 
>      Convert ->media_changed() to the new ->check_events() method.  The
>      conversion is mostly mechanical.  The only notable change is that
>      cdrom now doesn't generate any event if @slot_nr isn't CDSL_CURRENT.
>      It used to return -EINVAL which would be treated as media changed.  As
>      media changer isn't supported anyway, this doesn't make any
>      difference.
> 
>      This makes ide emit the standard disk events and allows kernel event
>      polling.  Currently, only MEDIA_CHANGE event is implemented.  Adding
>      support for EJECT_REQUEST shouldn't be difficult; however, given that
>      ide driver is already deprecated, it probably is best to leave it
>      alone.
> 
>      Signed-off-by: Tejun Heo <tj@kernel.org>
> ----------------------------
> 
> 
> 
> 
> If i disable IDE CDROM driver then the Linux kernel boots again
> and then it hits the commit 44ae3ab3358e962039c36ad4ae461ae9fb29596c
> and hangs again :)
> 
> The commit eca590f402332ab873d13f2d8d00fa0b91cfff36 which is before
> the commit 44ae3ab3358e962039c36ad4ae461ae9fb29596c works fine,
> i tested it myself to be on the safe side.

Ok thanks. I'll dig a bit if I get a chance next week.

Cheers,
Ben.
> 
> 
> Regards

^ permalink raw reply

* Re: Linux kernel 3.x problems on PowerMac G5
From: Phileas Fogg @ 2013-03-10  0:26 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linuxppc-dev, Denis Kirjanov, Andreas Schwab, Aaro Koskinen
In-Reply-To: <1362687724.6977.1.camel@pasglop>

On 03/07/2013 09:22 PM, Benjamin Herrenschmidt wrote:
> On Thu, 2013-03-07 at 21:08 +0100, Phileas Fogg wrote:
>> And the bisect couldn't find the commit which causes hangs on my
>> machine.
>> All commits which were provided by the bisect were bad.
>> And the commit before tha last bad bisect commit was bad too.
>> I did bisect several times, and got the same results.
>>
>> Fo testing i used linux-3.0.y branch of
>> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git.
>>
>> Did i miss something or do something wrong here ?
>
> Did git bisect go down a merge commit ? It does for me if I try that and
> asks to test that merge first. If you get that wrong it can get very
> confused.
>
> That's all I can think of... do you have the bisection log just in
> case ?
>
> Also you can use gitk -- arch/powerpc to look at the changes to powerpc
> code and try manually random points before/after that if you think
> bisect isn't doing the right thing.
>
> Cheers,
> Ben.
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>

Hi,

i managed to find the bad commit after a couple of days bisecting.


----------------------------
44ae3ab3358e962039c36ad4ae461ae9fb29596c is the first bad commit
commit 44ae3ab3358e962039c36ad4ae461ae9fb29596c
Author: Matt Evans <matt@ozlabs.org>
Date:   Wed Apr 6 19:48:50 2011 +0000

     powerpc: Free up some CPU feature bits by moving out MMU-related 
features

     Some of the 64bit PPC CPU features are MMU-related, so this patch moves
     them to MMU_FTR_ bits.  All cpu_has_feature()-style tests are moved to
     mmu_has_feature(), and seven feature bits are freed as a result.

     Signed-off-by: Matt Evans <matt@ozlabs.org>
     Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
--------------------------------




Actually, there are 2 problems i found.
The first problem occurs when i enable IDE CDROM driver on my machine.
The following commit causes hangs on my machine at boot:



----------------------
commit 5b03a1b140e13a28ff6be1526892a9dc538ddef6
Author: Tejun Heo <tj@kernel.org>
Date:   Wed Mar 9 19:54:27 2011 +0100

     ide: Convert to bdops->check_events()

     Convert ->media_changed() to the new ->check_events() method.  The
     conversion is mostly mechanical.  The only notable change is that
     cdrom now doesn't generate any event if @slot_nr isn't CDSL_CURRENT.
     It used to return -EINVAL which would be treated as media changed.  As
     media changer isn't supported anyway, this doesn't make any
     difference.

     This makes ide emit the standard disk events and allows kernel event
     polling.  Currently, only MEDIA_CHANGE event is implemented.  Adding
     support for EJECT_REQUEST shouldn't be difficult; however, given that
     ide driver is already deprecated, it probably is best to leave it
     alone.

     Signed-off-by: Tejun Heo <tj@kernel.org>
----------------------------




If i disable IDE CDROM driver then the Linux kernel boots again
and then it hits the commit 44ae3ab3358e962039c36ad4ae461ae9fb29596c
and hangs again :)

The commit eca590f402332ab873d13f2d8d00fa0b91cfff36 which is before
the commit 44ae3ab3358e962039c36ad4ae461ae9fb29596c works fine,
i tested it myself to be on the safe side.



Regards

^ permalink raw reply

* Re: [PATCH net-next] drivers:net: Remove unnecessary OOM messages after netdev_alloc_skb
From: David Miller @ 2013-03-09 21:11 UTC (permalink / raw)
  To: joe
  Cc: netdev, linuxppc-dev, linux-kernel, linux-arm-kernel,
	uclinux-dist-devel
In-Reply-To: <6379afbec3439c45180956b06c13b02ce3c9f802.1362790535.git.joe@perches.com>

From: Joe Perches <joe@perches.com>
Date: Fri,  8 Mar 2013 17:03:25 -0800

> Emitting netdev_alloc_skb and netdev_alloc_skb_ip_align OOM
> messages is unnecessary as there is already a dump_stack
> after allocation failures.
> 
> Other trivial changes around these removals:
> 
> Convert a few comparisons of pointer to 0 to !pointer.
> Change flow to remove unnecessary label.
> Remove now unused variable.
> Hoist assignment from if.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Applied.

^ permalink raw reply

* Re: [PATCH] powerpc: Fix dynamic relocation
From: Alexander Graf @ 2013-03-09 10:12 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev@lists.ozlabs.org, Anton Blanchard
In-Reply-To: <1362814152.6977.32.camel@pasglop>



Am 09.03.2013 um 08:29 schrieb Benjamin Herrenschmidt <benh@kernel.crashing.=
org>:

> On Sat, 2013-03-09 at 02:02 +0100, Alexander Graf wrote:
>> Commit 5ac47f7a introduced dynamic relocation of code by manually
>> relocating TOC entries. However, we need to access the TOC using
>> the physical address that we have for it, not the virtual address
>> that we can't even access yet.
>>=20
>> Drop the offset from the TOC accessing pointer.
>>=20
>> This fixes Linux 3.9 booting on OpenBIOS for me.
>>=20
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>=20
>> ---
>>=20
>> I've also encountered breakage on a G5 Mac, not sure if it's related.
>> I'll have to test whether this is the culprit on that one too.
>=20
> That might fix the mac too.. Anton's patch assumed OF ran in real mode
> in which case the top bits of the address are ignored, meaning we could
> use kernel virtual addresses but that isn't the case with Apple OF which
> has MMU enabled and maps memory 1:1. I suppose OpenBIOS has the same
> problem ? Or is this a 32-bit issue as well ?

That's exactly what OpenBIOS does, yes :).

Alex

>=20
>> Ben, please make sure that this gets into 3.9.
>> ---
>> arch/powerpc/kernel/prom_init.c |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>=20
>> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_i=
nit.c
>> index 7f7fb7f..2bf7cc3 100644
>> --- a/arch/powerpc/kernel/prom_init.c
>> +++ b/arch/powerpc/kernel/prom_init.c
>> @@ -2851,7 +2851,7 @@ static void reloc_toc(void)
>>        (__prom_init_toc_end - __prom_init_toc_start) / sizeof(long);
>>=20
>>    /* Need to add offset to get at __prom_init_toc_start */
>> -    __reloc_toc(__prom_init_toc_start + offset, offset, nr_entries);
>> +    __reloc_toc(__prom_init_toc_start, offset, nr_entries);
>>=20
>>    mb();
>> }
>=20
>=20

^ permalink raw reply

* Re: [PATCH] powerpc: Fix dynamic relocation
From: Andreas Schwab @ 2013-03-09  9:30 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Alexander Graf, Anton Blanchard
In-Reply-To: <1362814152.6977.32.camel__2059.78525705195$1362814216$gmane$org@pasglop>

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> That might fix the mac too..

It does.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: [PATCH] powerpc: Fix dynamic relocation
From: Benjamin Herrenschmidt @ 2013-03-09  7:29 UTC (permalink / raw)
  To: Alexander Graf; +Cc: linuxppc-dev, Anton Blanchard
In-Reply-To: <1362790950-23065-1-git-send-email-agraf@suse.de>

On Sat, 2013-03-09 at 02:02 +0100, Alexander Graf wrote:
> Commit 5ac47f7a introduced dynamic relocation of code by manually
> relocating TOC entries. However, we need to access the TOC using
> the physical address that we have for it, not the virtual address
> that we can't even access yet.
> 
> Drop the offset from the TOC accessing pointer.
> 
> This fixes Linux 3.9 booting on OpenBIOS for me.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> ---
> 
> I've also encountered breakage on a G5 Mac, not sure if it's related.
> I'll have to test whether this is the culprit on that one too.

That might fix the mac too.. Anton's patch assumed OF ran in real mode
in which case the top bits of the address are ignored, meaning we could
use kernel virtual addresses but that isn't the case with Apple OF which
has MMU enabled and maps memory 1:1. I suppose OpenBIOS has the same
problem ? Or is this a 32-bit issue as well ?

> Ben, please make sure that this gets into 3.9.
> ---
>  arch/powerpc/kernel/prom_init.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 7f7fb7f..2bf7cc3 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -2851,7 +2851,7 @@ static void reloc_toc(void)
>  		(__prom_init_toc_end - __prom_init_toc_start) / sizeof(long);
>  
>  	/* Need to add offset to get at __prom_init_toc_start */
> -	__reloc_toc(__prom_init_toc_start + offset, offset, nr_entries);
> +	__reloc_toc(__prom_init_toc_start, offset, nr_entries);
>  
>  	mb();
>  }

^ permalink raw reply

* [PATCH 11/11] Add /proc interface to control topology updates
From: Nathan Fontenot @ 2013-03-09  4:10 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <513AB2E3.6090209@linux.vnet.ibm.com>

There are instances in which we do not want topology updates to occur.
In order to allow this a /proc interface (/proc/powerpc/topology_updates)
is introduced so that topology updates can be enabled and disabled.

This patch also adds a prrn_is_enabled() call so that PRRN events are
handled in the kernel only if topology updating is enabled.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/topology.h |    5 ++
 arch/powerpc/kernel/rtasd.c         |    6 ++-
 arch/powerpc/mm/numa.c              |   62 +++++++++++++++++++++++++++++++++++-
 3 files changed, 70 insertions(+), 3 deletions(-)

Index: powerpc/arch/powerpc/mm/numa.c
===================================================================
--- powerpc.orig/arch/powerpc/mm/numa.c	2013-03-08 19:58:09.000000000 -0600
+++ powerpc/arch/powerpc/mm/numa.c	2013-03-08 19:58:37.000000000 -0600
@@ -23,6 +23,9 @@
 #include <linux/cpuset.h>
 #include <linux/node.h>
 #include <linux/stop_machine.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
+#include <linux/uaccess.h>
 #include <asm/sparsemem.h>
 #include <asm/prom.h>
 #include <asm/smp.h>
@@ -1558,7 +1561,6 @@
 
 	return rc;
 }
-__initcall(start_topology_update);
 
 /*
  * Disable polling for VPHN associativity changes.
@@ -1577,4 +1579,62 @@
 
 	return rc;
 }
+
+inline int prrn_is_enabled(void)
+{
+	return prrn_enabled;
+}
+
+static int topology_read(struct seq_file *file, void *v)
+{
+	if (vphn_enabled || prrn_enabled)
+		seq_puts(file, "on\n");
+	else
+		seq_puts(file, "off\n");
+
+	return 0;
+}
+
+static int topology_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, topology_read, NULL);
+}
+
+static ssize_t topology_write(struct file *file, const char __user *buf,
+			      size_t count, loff_t *off)
+{
+	char kbuf[4]; /* "on" or "off" plus null. */
+	int read_len;
+
+	read_len = count < 3 ? count : 3;
+	if (copy_from_user(kbuf, buf, read_len))
+		return -EINVAL;
+
+	kbuf[read_len] = '\0';
+
+	if (!strncmp(kbuf, "on", 2))
+		start_topology_update();
+	else if (!strncmp(kbuf, "off", 3))
+		stop_topology_update();
+	else
+		return -EINVAL;
+
+	return count;
+}
+
+static const struct file_operations topology_ops = {
+	.read = seq_read,
+	.write = topology_write,
+	.open = topology_open,
+	.release = single_release
+};
+
+static int topology_update_init(void)
+{
+	start_topology_update();
+	proc_create("powerpc/topology_updates", 644, NULL, &topology_ops);
+
+	return 0;
+}
+device_initcall(topology_update_init);
 #endif /* CONFIG_PPC_SPLPAR */
Index: powerpc/arch/powerpc/include/asm/topology.h
===================================================================
--- powerpc.orig/arch/powerpc/include/asm/topology.h	2013-03-08 19:23:06.000000000 -0600
+++ powerpc/arch/powerpc/include/asm/topology.h	2013-03-08 19:58:37.000000000 -0600
@@ -71,6 +71,7 @@
 #if defined(CONFIG_NUMA) && defined(CONFIG_PPC_SPLPAR)
 extern int start_topology_update(void);
 extern int stop_topology_update(void);
+extern inline int prrn_is_enabled(void);
 #else
 static inline int start_topology_update(void)
 {
@@ -80,6 +81,10 @@
 {
 	return 0;
 }
+static inline int prrn_is_enabled(void)
+{
+	return 0;
+}
 #endif /* CONFIG_NUMA && CONFIG_PPC_SPLPAR */
 
 #include <asm-generic/topology.h>
Index: powerpc/arch/powerpc/kernel/rtasd.c
===================================================================
--- powerpc.orig/arch/powerpc/kernel/rtasd.c	2013-03-08 19:56:48.000000000 -0600
+++ powerpc/arch/powerpc/kernel/rtasd.c	2013-03-08 19:58:37.000000000 -0600
@@ -292,11 +292,13 @@
 {
 	pSeries_log_error((char *)log, ERR_TYPE_RTAS_LOG, 0);
 
-	if (log->type == RTAS_TYPE_PRRN)
+	if (log->type == RTAS_TYPE_PRRN) {
 		/* For PRRN Events the extended log length is used to denote
 		 * the scope for calling rtas update-nodes.
 		 */
-		prrn_schedule_update(log->extended_log_length);
+		if (prrn_is_enabled())
+			prrn_schedule_update(log->extended_log_length);
+	}
 
 	return;
 }

^ permalink raw reply

* [PATCH 10/11] Enable PRRN
From: Nathan Fontenot @ 2013-03-09  4:08 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <513AB2E3.6090209@linux.vnet.ibm.com>

The Linux kernel and platform firmware negotiate their mutual support
of the PRRN option via the ibm,client-architecture-support interface.
This patch simply sets the appropriate fields in the client architecture
vector to indicate Linux support and will cause the firmware to begin
sending PRRN events via the RTAS event-scan mechanism.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/prom_init.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: powerpc/arch/powerpc/kernel/prom_init.c
===================================================================
--- powerpc.orig/arch/powerpc/kernel/prom_init.c	2013-03-08 19:57:14.000000000 -0600
+++ powerpc/arch/powerpc/kernel/prom_init.c	2013-03-08 19:58:18.000000000 -0600
@@ -689,7 +689,7 @@
 	OV5_FEAT(OV5_MSI),
 	0,
 	OV5_FEAT(OV5_CMO) | OV5_FEAT(OV5_XCMO),
-	OV5_FEAT(OV5_TYPE1_AFFINITY),
+	OV5_FEAT(OV5_TYPE1_AFFINITY) | OV5_FEAT(OV5_PRRN),
 	0,
 	0,
 	0,

^ permalink raw reply

* [PATCH 9/11] Re-enable Virtual Private Home Node capabilities
From: Nathan Fontenot @ 2013-03-09  4:08 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <513AB2E3.6090209@linux.vnet.ibm.com>

From: Jesse Larrew <jlarrew@linux.vnet.ibm.com>

The new PRRN firmware feature provides a more convenient and event-driven
interface than VPHN for notifying Linux of changes to the NUMA affinity of
platform resources. However, for practical reasons, it may not be feasible
for some customers to update to the latest firmware. For these customers,
the VPHN feature supported on previous firmware versions may still be the
best option.

The VPHN feature was previously disabled due to races with the load
balancing code when accessing the NUMA cpu maps, but the new stop_machine()
approach protects the NUMA cpu maps from these concurrent accesses. It
should be safe to re-enable this feature now.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 arch/powerpc/mm/numa.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: powerpc/arch/powerpc/mm/numa.c
===================================================================
--- powerpc.orig/arch/powerpc/mm/numa.c	2013-03-08 19:57:59.000000000 -0600
+++ powerpc/arch/powerpc/mm/numa.c	2013-03-08 19:58:09.000000000 -0600
@@ -1545,9 +1545,8 @@
 			vphn_enabled = 0;
 			rc = of_reconfig_notifier_register(&dt_update_nb);
 		}
-	} else if (0 && firmware_has_feature(FW_FEATURE_VPHN) &&
+	} else if (firmware_has_feature(FW_FEATURE_VPHN) &&
 		   get_lppaca()->shared_proc) {
-		/* Disabled until races with load balancing are fixed */
 		if (!vphn_enabled) {
 			prrn_enabled = 0;
 			vphn_enabled = 1;

^ permalink raw reply

* [PATCH 8/11] Update numa cpu vdso info
From: Nathan Fontenot @ 2013-03-09  4:07 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <513AB2E3.6090209@linux.vnet.ibm.com>

From: Jesse Larrew <jlarrew@linux.vnet.ibm.com>

The following patch adds vdso_getcpu_init(), which stores the NUMA node for
a cpu in SPRG3:

http://patchwork.ozlabs.org/patch/169070/

This patch ensures that this information is also updated when the NUMA
affinity of a cpu changes.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 arch/powerpc/mm/numa.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Index: powerpc/arch/powerpc/mm/numa.c
===================================================================
--- powerpc.orig/arch/powerpc/mm/numa.c	2013-03-08 19:57:47.000000000 -0600
+++ powerpc/arch/powerpc/mm/numa.c	2013-03-08 19:57:59.000000000 -0600
@@ -30,6 +30,7 @@
 #include <asm/paca.h>
 #include <asm/hvcall.h>
 #include <asm/setup.h>
+#include <asm/vdso.h>
 
 static int numa_enabled = 1;
 
@@ -1426,6 +1427,7 @@
 	unregister_cpu_under_node(update->cpu, update->old_nid);
 	unmap_cpu_from_node(update->cpu);
 	map_cpu_to_node(update->cpu, update->new_nid);
+	vdso_getcpu_init();
 	register_cpu_under_node(update->cpu, update->new_nid);
 
 	return 0;
@@ -1440,8 +1442,11 @@
 	int cpu, changed = 0;
 	struct topology_update_data update;
 	unsigned int associativity[VPHN_ASSOC_BUFSIZE] = {0};
+	cpumask_t updated_cpu;
 	struct device *dev;
 
+	cpumask_clear(&updated_cpu);
+
 	for_each_cpu(cpu, &cpu_associativity_changes_mask) {
 		update.cpu = cpu;
 		vphn_get_associativity(cpu, associativity);
@@ -1451,7 +1456,8 @@
 			update.new_nid = first_online_node;
 
 		update.old_nid = numa_cpu_lookup_table[cpu];
-		stop_machine(update_cpu_topology, &update, cpu_online_mask);
+		cpumask_set_cpu(cpu, &updated_cpu);
+		stop_machine(update_cpu_topology, &update, &updated_cpu);
 		dev = get_cpu_device(cpu);
 		if (dev)
 			kobject_uevent(&dev->kobj, KOBJ_CHANGE);

^ permalink raw reply

* [PATCH 7/11] Use stop machine to update cpu maps
From: Nathan Fontenot @ 2013-03-09  4:05 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <513AB2E3.6090209@linux.vnet.ibm.com>

From: Jesse Larrew <jlarrew@linux.vnet.ibm.com>

The new PRRN firmware feature allows CPU and memory resources to be
transparently reassigned across NUMA boundaries. When this happens, the
kernel must update the node maps to reflect the new affinity
information.

Although the NUMA maps can be protected by locking primitives during the
update itself, this is insufficient to prevent concurrent accesses to these
structures. Since cpumask_of_node() hands out a pointer to these
structures, they can still be modified outside of the lock. Furthermore,
tracking down each usage of these pointers and adding locks would be quite
invasive and difficult to maintain.

Situations like these are best handled using stop_machine(). Since the NUMA
affinity updates are exceptionally rare events, this approach has the
benefit of not adding any overhead while accessing the NUMA maps during
normal operation.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 arch/powerpc/mm/numa.c |   51 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 35 insertions(+), 16 deletions(-)

Index: powerpc/arch/powerpc/mm/numa.c
===================================================================
--- powerpc.orig/arch/powerpc/mm/numa.c	2013-03-08 19:57:38.000000000 -0600
+++ powerpc/arch/powerpc/mm/numa.c	2013-03-08 19:57:47.000000000 -0600
@@ -22,6 +22,7 @@
 #include <linux/pfn.h>
 #include <linux/cpuset.h>
 #include <linux/node.h>
+#include <linux/stop_machine.h>
 #include <asm/sparsemem.h>
 #include <asm/prom.h>
 #include <asm/smp.h>
@@ -1254,6 +1255,12 @@
 
 /* Virtual Processor Home Node (VPHN) support */
 #ifdef CONFIG_PPC_SPLPAR
+struct topology_update_data {
+	int cpu;
+	int old_nid;
+	int new_nid;
+};
+
 static u8 vphn_cpu_change_counts[NR_CPUS][MAX_DISTANCE_REF_POINTS];
 static cpumask_t cpu_associativity_changes_mask;
 static int vphn_enabled;
@@ -1405,34 +1412,46 @@
 }
 
 /*
+ * Update the CPU maps and sysfs entries for a single CPU when its NUMA
+ * characteristics change. This function doesn't perform any locking and is
+ * only safe to call from stop_machine().
+ */
+static int update_cpu_topology(void *data)
+{
+	struct topology_update_data *update = data;
+
+	if (!update)
+		return -EINVAL;
+
+	unregister_cpu_under_node(update->cpu, update->old_nid);
+	unmap_cpu_from_node(update->cpu);
+	map_cpu_to_node(update->cpu, update->new_nid);
+	register_cpu_under_node(update->cpu, update->new_nid);
+
+	return 0;
+}
+
+/*
  * Update the node maps and sysfs entries for each cpu whose home node
  * has changed. Returns 1 when the topology has changed, and 0 otherwise.
  */
 int arch_update_cpu_topology(void)
 {
-	int cpu, nid, old_nid, changed = 0;
+	int cpu, changed = 0;
+	struct topology_update_data update;
 	unsigned int associativity[VPHN_ASSOC_BUFSIZE] = {0};
 	struct device *dev;
 
 	for_each_cpu(cpu, &cpu_associativity_changes_mask) {
+		update.cpu = cpu;
 		vphn_get_associativity(cpu, associativity);
-		nid = associativity_to_nid(associativity);
-
-		if (nid < 0 || !node_online(nid))
-			nid = first_online_node;
+		update.new_nid = associativity_to_nid(associativity);
 
-		old_nid = numa_cpu_lookup_table[cpu];
-
-		/* Disable hotplug while we update the cpu
-		 * masks and sysfs.
-		 */
-		get_online_cpus();
-		unregister_cpu_under_node(cpu, old_nid);
-		unmap_cpu_from_node(cpu);
-		map_cpu_to_node(cpu, nid);
-		register_cpu_under_node(cpu, nid);
-		put_online_cpus();
+		if (update.new_nid < 0 || !node_online(update.new_nid))
+			update.new_nid = first_online_node;
 
+		update.old_nid = numa_cpu_lookup_table[cpu];
+		stop_machine(update_cpu_topology, &update, cpu_online_mask);
 		dev = get_cpu_device(cpu);
 		if (dev)
 			kobject_uevent(&dev->kobj, KOBJ_CHANGE);

^ permalink raw reply

* [PATCH 6/11] Update CPU maps
From: Nathan Fontenot @ 2013-03-09  4:04 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <513AB2E3.6090209@linux.vnet.ibm.com>

From: Jesse Larrew <jlarrew@linux.vnet.ibm.com>

Platform events such as partition migration or the new PRRN firmware
feature can cause the NUMA characteristics of a CPU to change, and these
changes will be reflected in the device tree nodes for the affected
CPUs.

This patch registers a handler for Open Firmware device tree updates
and reconfigures the CPU and node maps whenever the associativity
changes. Currently, this is accomplished by marking the affected CPUs in
the cpu_associativity_changes_mask and allowing
arch_update_cpu_topology() to retrieve the new associativity information
using hcall_vphn().

Protecting the NUMA cpu maps from concurrent access during an update
operation will be addressed in a subsequent patch in this series.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---

 arch/powerpc/include/asm/prom.h |    1 
 arch/powerpc/mm/numa.c          |   99 ++++++++++++++++++++++++++++++----------
 2 files changed, 76 insertions(+), 24 deletions(-)

Index: powerpc/arch/powerpc/include/asm/prom.h
===================================================================
--- powerpc.orig/arch/powerpc/include/asm/prom.h	2013-03-08 19:57:14.000000000 -0600
+++ powerpc/arch/powerpc/include/asm/prom.h	2013-03-08 19:57:38.000000000 -0600
@@ -138,6 +138,7 @@
 #define OV5_XCMO		0x0400
 #endif
 #define OV5_TYPE1_AFFINITY	0x0580	/* Type 1 NUMA affinity */
+#define OV5_PRRN		0x0540	/* Platform Resource Reassignment */
 #define OV5_PFO_HW_RNG		0x0E80	/* PFO Random Number Generator */
 #define OV5_PFO_HW_842		0x0E40	/* PFO Compression Accelerator */
 #define OV5_PFO_HW_ENCR		0x0E20	/* PFO Encryption Accelerator */
Index: powerpc/arch/powerpc/mm/numa.c
===================================================================
--- powerpc.orig/arch/powerpc/mm/numa.c	2013-03-08 19:57:27.000000000 -0600
+++ powerpc/arch/powerpc/mm/numa.c	2013-03-08 19:57:38.000000000 -0600
@@ -1257,7 +1257,8 @@
 static u8 vphn_cpu_change_counts[NR_CPUS][MAX_DISTANCE_REF_POINTS];
 static cpumask_t cpu_associativity_changes_mask;
 static int vphn_enabled;
-static void set_topology_timer(void);
+static int prrn_enabled;
+static void reset_topology_timer(void);
 
 /*
  * Store the current values of the associativity change counters in the
@@ -1293,11 +1294,9 @@
  */
 static int update_cpu_associativity_changes_mask(void)
 {
-	int cpu, nr_cpus = 0;
+	int cpu;
 	cpumask_t *changes = &cpu_associativity_changes_mask;
 
-	cpumask_clear(changes);
-
 	for_each_possible_cpu(cpu) {
 		int i, changed = 0;
 		u8 *counts = vphn_cpu_change_counts[cpu];
@@ -1311,11 +1310,10 @@
 		}
 		if (changed) {
 			cpumask_set_cpu(cpu, changes);
-			nr_cpus++;
 		}
 	}
 
-	return nr_cpus;
+	return cpumask_weight(changes);
 }
 
 /*
@@ -1416,7 +1414,7 @@
 	unsigned int associativity[VPHN_ASSOC_BUFSIZE] = {0};
 	struct device *dev;
 
-	for_each_cpu(cpu,&cpu_associativity_changes_mask) {
+	for_each_cpu(cpu, &cpu_associativity_changes_mask) {
 		vphn_get_associativity(cpu, associativity);
 		nid = associativity_to_nid(associativity);
 
@@ -1438,6 +1436,7 @@
 		dev = get_cpu_device(cpu);
 		if (dev)
 			kobject_uevent(&dev->kobj, KOBJ_CHANGE);
+		cpumask_clear_cpu(cpu, &cpu_associativity_changes_mask);
 		changed = 1;
 	}
 
@@ -1457,37 +1456,80 @@
 
 static void topology_timer_fn(unsigned long ignored)
 {
-	if (!vphn_enabled)
-		return;
-	if (update_cpu_associativity_changes_mask() > 0)
+	if (prrn_enabled && cpumask_weight(&cpu_associativity_changes_mask))
 		topology_schedule_update();
-	set_topology_timer();
+	else if (vphn_enabled) {
+		if (update_cpu_associativity_changes_mask() > 0)
+			topology_schedule_update();
+		reset_topology_timer();
+	}
 }
 static struct timer_list topology_timer =
 	TIMER_INITIALIZER(topology_timer_fn, 0, 0);
 
-static void set_topology_timer(void)
+static void reset_topology_timer(void)
 {
 	topology_timer.data = 0;
 	topology_timer.expires = jiffies + 60 * HZ;
-	add_timer(&topology_timer);
+	mod_timer(&topology_timer, topology_timer.expires);
+}
+
+static void stage_topology_update(int core_id)
+{
+	cpumask_or(&cpu_associativity_changes_mask,
+		&cpu_associativity_changes_mask, cpu_sibling_mask(core_id));
+	reset_topology_timer();
 }
 
+static int dt_update_callback(struct notifier_block *nb,
+				unsigned long action, void *data)
+{
+	struct of_prop_reconfig *update;
+	int rc = NOTIFY_DONE;
+
+	switch (action) {
+	case OF_RECONFIG_ADD_PROPERTY:
+	case OF_RECONFIG_UPDATE_PROPERTY:
+		update = (struct of_prop_reconfig *)data;
+		if (!of_prop_cmp(update->dn->type, "cpu")) {
+			u32 core_id;
+			of_property_read_u32(update->dn, "reg", &core_id);
+			stage_topology_update(core_id);
+			rc = NOTIFY_OK;
+		}
+		break;
+	}
+
+	return rc;
+}
+
+static struct notifier_block dt_update_nb = {
+	.notifier_call = dt_update_callback,
+};
+
 /*
- * Start polling for VPHN associativity changes.
+ * Start polling for associativity changes.
  */
 int start_topology_update(void)
 {
 	int rc = 0;
 
-	/* Disabled until races with load balancing are fixed */
-	if (0 && firmware_has_feature(FW_FEATURE_VPHN) &&
-	    get_lppaca()->shared_proc) {
-		vphn_enabled = 1;
-		setup_cpu_associativity_change_counters();
-		init_timer_deferrable(&topology_timer);
-		set_topology_timer();
-		rc = 1;
+	if (platform_has_feature(OV5_PRRN)) {
+		if (!prrn_enabled) {
+			prrn_enabled = 1;
+			vphn_enabled = 0;
+			rc = of_reconfig_notifier_register(&dt_update_nb);
+		}
+	} else if (0 && firmware_has_feature(FW_FEATURE_VPHN) &&
+		   get_lppaca()->shared_proc) {
+		/* Disabled until races with load balancing are fixed */
+		if (!vphn_enabled) {
+			prrn_enabled = 0;
+			vphn_enabled = 1;
+			setup_cpu_associativity_change_counters();
+			init_timer_deferrable(&topology_timer);
+			reset_topology_timer();
+		}
 	}
 
 	return rc;
@@ -1499,7 +1541,16 @@
  */
 int stop_topology_update(void)
 {
-	vphn_enabled = 0;
-	return del_timer_sync(&topology_timer);
+	int rc = 0;
+
+	if (prrn_enabled) {
+		prrn_enabled = 0;
+		rc = of_reconfig_notifier_unregister(&dt_update_nb);
+	} else if (vphn_enabled) {
+		vphn_enabled = 0;
+		rc = del_timer_sync(&topology_timer);
+	}
+
+	return rc;
 }
 #endif /* CONFIG_PPC_SPLPAR */

^ permalink raw reply


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