subject:"RE\: \[PATCH 1\/1\] powerpc\/irq\: tidy up inconsistent context in migrate

RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

2015-12-23 Thread Zhang Zhuoyu



> -Original Message-
> From: Denis Kirjanov [mailto:k...@linux-powerpc.org]
> Sent: Thursday, December 24, 2015 6:04 AM
> To: Zhang Zhuoyu 
> Cc: Michael Ellerman ; linux-kernel@vger.kernel.org;
> pau...@samba.org; t...@linutronix.de; linuxppc-...@lists.ozlabs.org;
> jiang@linux.intel.com; Daniel Axtens 
> Subject: Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in
> migrate_irqs()
> 
> On 12/24/15, Denis Kirjanov  wrote:
> > On 12/23/15, Zhang Zhuoyu  wrote:
> >> Hi, Denis
> >>
> >> Any test result on pmac machine for this patch?
> >
> > Hi,
> >
> > So I ran the tests by writing to cpuN/online.
> >
> > with your change I'm observing lines like the following:
> > [  713.436922] NOHZ: local_softirq_pending 08
> 

Hi, 

It is a NOHZ warning, NOHZ will check for whether there are softirqs pending 
when an online CPU goes
idle, this warning cannot be triggered by offlining CPU, you should check which 
driver's
softirq is preventing CPU goes idle.


> Another bad thing that the current powerpc/next kernel crashed on pmac
> (without this change) while enabling/disabling cpu cores :/ I've attached the
> screenshot
> 
> 
> >
> > Thanks!
> >
> >>
> >> Zhuoyu
> >>
> >>> -Original Message-
> >>> From: Zhang Zhuoyu [mailto:zhangzhu...@cmss.chinamobile.com]
> >>> Sent: Wednesday, December 16, 2015 10:46 PM
> >>> To: 'Denis Kirjanov'; 'Michael Ellerman'
> >>> Cc: 'Daniel Axtens'; 'Zhang Zhuoyu'; 'b...@kernel.crashing.org';
> >>> 'pau...@samba.org'; 't...@linutronix.de';
> >>> 'jiang@linux.intel.com'; 'linuxppc-...@lists.ozlabs.org'; 'linux-
> ker...@vger.kernel.org'
> >>> Subject: RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context
> >>> in
> >>> migrate_irqs()
> >>>
> >>>
> >>> > -Original Message-
> >>> > From: Denis Kirjanov [mailto:k...@linux-powerpc.org]
> >>> > Sent: Wednesday, December 16, 2015 7:55 PM
> >>> > To: Michael Ellerman
> >>> > Cc: Daniel Axtens; Zhang Zhuoyu; b...@kernel.crashing.org;
> >>> > pau...@samba.org; t...@linutronix.de; jiang@linux.intel.com;
> >>> > zhangzhu...@cmss.chinamobile.com; linuxppc-...@lists.ozlabs.org;
> >>> > linux- ker...@vger.kernel.org
> >>> > Subject: Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context
> >>> > in
> >>> > migrate_irqs()
> >>> >
> >>> > On 12/16/15, Michael Ellerman  wrote:
> >>> > > On Wed, 2015-12-16 at 17:08 +1100, Daniel Axtens wrote:
> >>> > >> Hi,
> >>> > >>
> >>> > >> A couple of things.
> >>> > >>
> >>> > >> Firstly, your two email addresses don't match:
> >>> > >>
> >>> > >> Zhang Zhuoyu  writes:
> >>> > >
> >>> > >> > From: Zhang Zhuoyu 
> >>> > >>
> >>>
> >>> Mmm, My mistake, I will correct it next time.
> >>>
> >>> > >> These lines do seem odd! Are they causing a problem?
> >>> > >>
> >>> > >> I'd be more comfortable removing them if I understood why they
> >>> > >> were added. But they've been around since the beginning of git
> >>> > >> history so that could be a bit difficult.
> >>> > >
> >>> > > It's in the fullhist tree, but that doesn't tell us much (below,
> >>> > > named fixup_irqs()).
> >>> > >
> >>> > > But I suspect those lines are actually there very deliberately.
> >>> > >
> >>> > > The function is migrating interrupts off the recently offlined
> >>> > > cpu, because we don't want to take interrupts on an offline cpu.
> >>> > >
> >>> > > After it's finished doing the migration, it wants to make sure
> >>> > > there are no interrupts that have already been latched by the offline
> cpu.
> >>> > > So it briefly enables interrupts, waits a bit for the interrupts
> >>> > > to fire, and the disables them again.
> >>> > >
> >>> > > Whether that actually works I couldn't say, it is very old code,
> >>> > > and it's used on platforms where I don't ever test cpu hotplug
> >>> > > (85xx & powermac).
>

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

2015-12-23 Thread Denis Kirjanov

On 12/23/15, Zhang Zhuoyu  wrote:
> Hi, Denis
>
> Any test result on pmac machine for this patch?

Hi,

So I ran the tests by writing to cpuN/online.

with your change I'm observing lines like the following:
[  713.436922] NOHZ: local_softirq_pending 08

Thanks!

>
> Zhuoyu
>
>> -Original Message-
>> From: Zhang Zhuoyu [mailto:zhangzhu...@cmss.chinamobile.com]
>> Sent: Wednesday, December 16, 2015 10:46 PM
>> To: 'Denis Kirjanov'; 'Michael Ellerman'
>> Cc: 'Daniel Axtens'; 'Zhang Zhuoyu'; 'b...@kernel.crashing.org';
>> 'pau...@samba.org'; 't...@linutronix.de'; 'jiang@linux.intel.com';
>> 'linuxppc-...@lists.ozlabs.org'; 'linux-kernel@vger.kernel.org'
>> Subject: RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in
>> migrate_irqs()
>>
>>
>> > -Original Message-
>> > From: Denis Kirjanov [mailto:k...@linux-powerpc.org]
>> > Sent: Wednesday, December 16, 2015 7:55 PM
>> > To: Michael Ellerman
>> > Cc: Daniel Axtens; Zhang Zhuoyu; b...@kernel.crashing.org;
>> > pau...@samba.org; t...@linutronix.de; jiang....@linux.intel.com;
>> > zhangzhu...@cmss.chinamobile.com; linuxppc-...@lists.ozlabs.org;
>> > linux- ker...@vger.kernel.org
>> > Subject: Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in
>> > migrate_irqs()
>> >
>> > On 12/16/15, Michael Ellerman  wrote:
>> > > On Wed, 2015-12-16 at 17:08 +1100, Daniel Axtens wrote:
>> > >> Hi,
>> > >>
>> > >> A couple of things.
>> > >>
>> > >> Firstly, your two email addresses don't match:
>> > >>
>> > >> Zhang Zhuoyu  writes:
>> > >
>> > >> > From: Zhang Zhuoyu 
>> > >>
>>
>> Mmm, My mistake, I will correct it next time.
>>
>> > >> These lines do seem odd! Are they causing a problem?
>> > >>
>> > >> I'd be more comfortable removing them if I understood why they were
>> > >> added. But they've been around since the beginning of git history
>> > >> so that could be a bit difficult.
>> > >
>> > > It's in the fullhist tree, but that doesn't tell us much (below,
>> > > named fixup_irqs()).
>> > >
>> > > But I suspect those lines are actually there very deliberately.
>> > >
>> > > The function is migrating interrupts off the recently offlined cpu,
>> > > because we don't want to take interrupts on an offline cpu.
>> > >
>> > > After it's finished doing the migration, it wants to make sure there
>> > > are no interrupts that have already been latched by the offline cpu.
>> > > So it briefly enables interrupts, waits a bit for the interrupts to
>> > > fire, and the disables them again.
>> > >
>> > > Whether that actually works I couldn't say, it is very old code, and
>> > > it's used on platforms where I don't ever test cpu hotplug (85xx &
>> > > powermac).
>> >
>> > Yeah, it would be nice to test this change. I'll try it on my
>> > quad-core pmac machine
>> >
>>
>> Thanks Michael for help explaining the code logic, it also resolved my
>> doubts.
>> These snippets are suspected when I did PM benchmark on FSL MPC85xx
>> series(T1040, T4240), For T4240, which has 24 CPUs, it waits 1ms in
>> migrate_irqs()each time a CPU is plugged offline, it seems a waste of
>> time. I
>> also did a test on T1040, after plugging offline/online CPU hundreds of
>> times,
>> system works well. If you have any other suggestion on how to test, I'd
>> like
>> to do more benchmark.
>> (1)for((i=0; i<1000; i++)); do echo 0 >
>> /sys/devices/system/cpu/cpu1/online;
>> sleep 1; echo 1 > /sys/devices/system/cpu/cpu1/online; done
>> (2)root@t1040rdb:~# cat /proc/interrupts
>>CPU0   CPU1   CPU2   CPU3
>>  36:393  1223  1   OpenPIC36 Level
>> serial
>> LOC:   1946   1486   1555   1361   Local timer interrupts
>> DBL:   7371   9707   9390   7568   Doorbell interrupts
>> (3)root@t1040rdb:~# ps
>>   PID TTY  TIME CMD
>>  2751 ttyS000:00:00 sh
>>  2757 ttyS000:00:00 ps
>> root@t1040rdb:~# taskset -pc 1 2751
>> pid 2751's current affinity list: 0-3
>> pid 2751's new affinity list: 1
>> root@t1040rdb:~#
>> root@t1040rdb:~#
>> root@t1040rdb:~#
>> root@t1040rdb:~# echo "hello"

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

2015-12-23 Thread Denis Kirjanov

On 12/23/15, Zhang Zhuoyu <zhangzhu...@cmss.chinamobile.com> wrote:
> Hi, Denis
>
> Any test result on pmac machine for this patch?

Hi,

So I ran the tests by writing to cpuN/online.

with your change I'm observing lines like the following:
[  713.436922] NOHZ: local_softirq_pending 08

Thanks!

>
> Zhuoyu
>
>> -Original Message-
>> From: Zhang Zhuoyu [mailto:zhangzhu...@cmss.chinamobile.com]
>> Sent: Wednesday, December 16, 2015 10:46 PM
>> To: 'Denis Kirjanov'; 'Michael Ellerman'
>> Cc: 'Daniel Axtens'; 'Zhang Zhuoyu'; 'b...@kernel.crashing.org';
>> 'pau...@samba.org'; 't...@linutronix.de'; 'jiang@linux.intel.com';
>> 'linuxppc-...@lists.ozlabs.org'; 'linux-kernel@vger.kernel.org'
>> Subject: RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in
>> migrate_irqs()
>>
>>
>> > -Original Message-
>> > From: Denis Kirjanov [mailto:k...@linux-powerpc.org]
>> > Sent: Wednesday, December 16, 2015 7:55 PM
>> > To: Michael Ellerman
>> > Cc: Daniel Axtens; Zhang Zhuoyu; b...@kernel.crashing.org;
>> > pau...@samba.org; t...@linutronix.de; jiang....@linux.intel.com;
>> > zhangzhu...@cmss.chinamobile.com; linuxppc-...@lists.ozlabs.org;
>> > linux- ker...@vger.kernel.org
>> > Subject: Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in
>> > migrate_irqs()
>> >
>> > On 12/16/15, Michael Ellerman <m...@ellerman.id.au> wrote:
>> > > On Wed, 2015-12-16 at 17:08 +1100, Daniel Axtens wrote:
>> > >> Hi,
>> > >>
>> > >> A couple of things.
>> > >>
>> > >> Firstly, your two email addresses don't match:
>> > >>
>> > >> Zhang Zhuoyu <hellozzy1...@126.com> writes:
>> > >
>> > >> > From: Zhang Zhuoyu <zhangzhu...@cmss.chinamobile.com>
>> > >>
>>
>> Mmm, My mistake, I will correct it next time.
>>
>> > >> These lines do seem odd! Are they causing a problem?
>> > >>
>> > >> I'd be more comfortable removing them if I understood why they were
>> > >> added. But they've been around since the beginning of git history
>> > >> so that could be a bit difficult.
>> > >
>> > > It's in the fullhist tree, but that doesn't tell us much (below,
>> > > named fixup_irqs()).
>> > >
>> > > But I suspect those lines are actually there very deliberately.
>> > >
>> > > The function is migrating interrupts off the recently offlined cpu,
>> > > because we don't want to take interrupts on an offline cpu.
>> > >
>> > > After it's finished doing the migration, it wants to make sure there
>> > > are no interrupts that have already been latched by the offline cpu.
>> > > So it briefly enables interrupts, waits a bit for the interrupts to
>> > > fire, and the disables them again.
>> > >
>> > > Whether that actually works I couldn't say, it is very old code, and
>> > > it's used on platforms where I don't ever test cpu hotplug (85xx &
>> > > powermac).
>> >
>> > Yeah, it would be nice to test this change. I'll try it on my
>> > quad-core pmac machine
>> >
>>
>> Thanks Michael for help explaining the code logic, it also resolved my
>> doubts.
>> These snippets are suspected when I did PM benchmark on FSL MPC85xx
>> series(T1040, T4240), For T4240, which has 24 CPUs, it waits 1ms in
>> migrate_irqs()each time a CPU is plugged offline, it seems a waste of
>> time. I
>> also did a test on T1040, after plugging offline/online CPU hundreds of
>> times,
>> system works well. If you have any other suggestion on how to test, I'd
>> like
>> to do more benchmark.
>> (1)for((i=0; i<1000; i++)); do echo 0 >
>> /sys/devices/system/cpu/cpu1/online;
>> sleep 1; echo 1 > /sys/devices/system/cpu/cpu1/online; done
>> (2)root@t1040rdb:~# cat /proc/interrupts
>>CPU0   CPU1   CPU2   CPU3
>>  36:393  1223  1   OpenPIC36 Level
>> serial
>> LOC:   1946   1486   1555   1361   Local timer interrupts
>> DBL:   7371   9707   9390   7568   Doorbell interrupts
>> (3)root@t1040rdb:~# ps
>>   PID TTY  TIME CMD
>>  2751 ttyS000:00:00 sh
>>  2757 ttyS000:00:00 ps
>> root@t1040rdb:~# taskset -pc 1 2751
>> pid 2751's current affinity list: 0-3
>> pid 2751's new affinity l

RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

2015-12-23 Thread Zhang Zhuoyu



> -Original Message-
> From: Denis Kirjanov [mailto:k...@linux-powerpc.org]
> Sent: Thursday, December 24, 2015 6:04 AM
> To: Zhang Zhuoyu <zhangzhu...@cmss.chinamobile.com>
> Cc: Michael Ellerman <m...@ellerman.id.au>; linux-kernel@vger.kernel.org;
> pau...@samba.org; t...@linutronix.de; linuxppc-...@lists.ozlabs.org;
> jiang@linux.intel.com; Daniel Axtens <d...@axtens.net>
> Subject: Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in
> migrate_irqs()
> 
> On 12/24/15, Denis Kirjanov <k...@linux-powerpc.org> wrote:
> > On 12/23/15, Zhang Zhuoyu <zhangzhu...@cmss.chinamobile.com> wrote:
> >> Hi, Denis
> >>
> >> Any test result on pmac machine for this patch?
> >
> > Hi,
> >
> > So I ran the tests by writing to cpuN/online.
> >
> > with your change I'm observing lines like the following:
> > [  713.436922] NOHZ: local_softirq_pending 08
> 

Hi, 

It is a NOHZ warning, NOHZ will check for whether there are softirqs pending 
when an online CPU goes
idle, this warning cannot be triggered by offlining CPU, you should check which 
driver's
softirq is preventing CPU goes idle.


> Another bad thing that the current powerpc/next kernel crashed on pmac
> (without this change) while enabling/disabling cpu cores :/ I've attached the
> screenshot
> 
> 
> >
> > Thanks!
> >
> >>
> >> Zhuoyu
> >>
> >>> -Original Message-
> >>> From: Zhang Zhuoyu [mailto:zhangzhu...@cmss.chinamobile.com]
> >>> Sent: Wednesday, December 16, 2015 10:46 PM
> >>> To: 'Denis Kirjanov'; 'Michael Ellerman'
> >>> Cc: 'Daniel Axtens'; 'Zhang Zhuoyu'; 'b...@kernel.crashing.org';
> >>> 'pau...@samba.org'; 't...@linutronix.de';
> >>> 'jiang@linux.intel.com'; 'linuxppc-...@lists.ozlabs.org'; 'linux-
> ker...@vger.kernel.org'
> >>> Subject: RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context
> >>> in
> >>> migrate_irqs()
> >>>
> >>>
> >>> > -Original Message-
> >>> > From: Denis Kirjanov [mailto:k...@linux-powerpc.org]
> >>> > Sent: Wednesday, December 16, 2015 7:55 PM
> >>> > To: Michael Ellerman
> >>> > Cc: Daniel Axtens; Zhang Zhuoyu; b...@kernel.crashing.org;
> >>> > pau...@samba.org; t...@linutronix.de; jiang@linux.intel.com;
> >>> > zhangzhu...@cmss.chinamobile.com; linuxppc-...@lists.ozlabs.org;
> >>> > linux- ker...@vger.kernel.org
> >>> > Subject: Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context
> >>> > in
> >>> > migrate_irqs()
> >>> >
> >>> > On 12/16/15, Michael Ellerman <m...@ellerman.id.au> wrote:
> >>> > > On Wed, 2015-12-16 at 17:08 +1100, Daniel Axtens wrote:
> >>> > >> Hi,
> >>> > >>
> >>> > >> A couple of things.
> >>> > >>
> >>> > >> Firstly, your two email addresses don't match:
> >>> > >>
> >>> > >> Zhang Zhuoyu <hellozzy1...@126.com> writes:
> >>> > >
> >>> > >> > From: Zhang Zhuoyu <zhangzhu...@cmss.chinamobile.com>
> >>> > >>
> >>>
> >>> Mmm, My mistake, I will correct it next time.
> >>>
> >>> > >> These lines do seem odd! Are they causing a problem?
> >>> > >>
> >>> > >> I'd be more comfortable removing them if I understood why they
> >>> > >> were added. But they've been around since the beginning of git
> >>> > >> history so that could be a bit difficult.
> >>> > >
> >>> > > It's in the fullhist tree, but that doesn't tell us much (below,
> >>> > > named fixup_irqs()).
> >>> > >
> >>> > > But I suspect those lines are actually there very deliberately.
> >>> > >
> >>> > > The function is migrating interrupts off the recently offlined
> >>> > > cpu, because we don't want to take interrupts on an offline cpu.
> >>> > >
> >>> > > After it's finished doing the migration, it wants to make sure
> >>> > > there are no interrupts that have already been latched by the offline
> cpu.
> >>> > > So it briefly enables interrupts, waits a bit for the interrupts
> >>> > > to fire, and the disables them again.
> >>> &g

RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

2015-12-22 Thread Zhang Zhuoyu

Hi, Denis

Any test result on pmac machine for this patch? 

Zhuoyu

> -Original Message-
> From: Zhang Zhuoyu [mailto:zhangzhu...@cmss.chinamobile.com]
> Sent: Wednesday, December 16, 2015 10:46 PM
> To: 'Denis Kirjanov'; 'Michael Ellerman'
> Cc: 'Daniel Axtens'; 'Zhang Zhuoyu'; 'b...@kernel.crashing.org';
> 'pau...@samba.org'; 't...@linutronix.de'; 'jiang@linux.intel.com';
> 'linuxppc-...@lists.ozlabs.org'; 'linux-kernel@vger.kernel.org'
> Subject: RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in
> migrate_irqs()
> 
> 
> > -Original Message-
> > From: Denis Kirjanov [mailto:k...@linux-powerpc.org]
> > Sent: Wednesday, December 16, 2015 7:55 PM
> > To: Michael Ellerman
> > Cc: Daniel Axtens; Zhang Zhuoyu; b...@kernel.crashing.org;
> > pau...@samba.org; t...@linutronix.de; jiang@linux.intel.com;
> > zhangzhu...@cmss.chinamobile.com; linuxppc-...@lists.ozlabs.org;
> > linux- ker...@vger.kernel.org
> > Subject: Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in
> > migrate_irqs()
> >
> > On 12/16/15, Michael Ellerman  wrote:
> > > On Wed, 2015-12-16 at 17:08 +1100, Daniel Axtens wrote:
> > >> Hi,
> > >>
> > >> A couple of things.
> > >>
> > >> Firstly, your two email addresses don't match:
> > >>
> > >> Zhang Zhuoyu  writes:
> > >
> > >> > From: Zhang Zhuoyu 
> > >>
> 
> Mmm, My mistake, I will correct it next time.
> 
> > >> These lines do seem odd! Are they causing a problem?
> > >>
> > >> I'd be more comfortable removing them if I understood why they were
> > >> added. But they've been around since the beginning of git history
> > >> so that could be a bit difficult.
> > >
> > > It's in the fullhist tree, but that doesn't tell us much (below,
> > > named fixup_irqs()).
> > >
> > > But I suspect those lines are actually there very deliberately.
> > >
> > > The function is migrating interrupts off the recently offlined cpu,
> > > because we don't want to take interrupts on an offline cpu.
> > >
> > > After it's finished doing the migration, it wants to make sure there
> > > are no interrupts that have already been latched by the offline cpu.
> > > So it briefly enables interrupts, waits a bit for the interrupts to
> > > fire, and the disables them again.
> > >
> > > Whether that actually works I couldn't say, it is very old code, and
> > > it's used on platforms where I don't ever test cpu hotplug (85xx &
> > > powermac).
> >
> > Yeah, it would be nice to test this change. I'll try it on my
> > quad-core pmac machine
> >
> 
> Thanks Michael for help explaining the code logic, it also resolved my doubts.
> These snippets are suspected when I did PM benchmark on FSL MPC85xx
> series(T1040, T4240), For T4240, which has 24 CPUs, it waits 1ms in
> migrate_irqs()each time a CPU is plugged offline, it seems a waste of time. I
> also did a test on T1040, after plugging offline/online CPU hundreds of times,
> system works well. If you have any other suggestion on how to test, I'd like
> to do more benchmark.
> (1)for((i=0; i<1000; i++)); do echo 0 > /sys/devices/system/cpu/cpu1/online;
> sleep 1; echo 1 > /sys/devices/system/cpu/cpu1/online; done
> (2)root@t1040rdb:~# cat /proc/interrupts
>CPU0   CPU1   CPU2   CPU3
>  36:393  1223  1   OpenPIC36 Level 
> serial
> LOC:   1946   1486   1555   1361   Local timer interrupts
> DBL:   7371   9707   9390   7568   Doorbell interrupts
> (3)root@t1040rdb:~# ps
>   PID TTY  TIME CMD
>  2751 ttyS000:00:00 sh
>  2757 ttyS000:00:00 ps
> root@t1040rdb:~# taskset -pc 1 2751
> pid 2751's current affinity list: 0-3
> pid 2751's new affinity list: 1
> root@t1040rdb:~#
> root@t1040rdb:~#
> root@t1040rdb:~#
> root@t1040rdb:~# echo "hello"
> hello
> root@t1040rdb:~#
> 
> > >
> > > cheers
> > >
> > >
> > > commit d58830b9a740ad1c3b089196d4afdaea251dc701
> > > Author: Zwane Mwaikambo 
> > > Date:   Fri Mar 4 17:34:00 2005 -0800
> > >
> > > [PATCH] ppc64: generic hotplug cpu support
> > >
> > > Patch provides a generic hotplug cpu implementation, with the
> > > only current
> > > user being pmac.  This doesn't replace real hotplug code as is 
> > > currently
> > > used by LPAR systems.  Ben i can add

RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

2015-12-22 Thread Zhang Zhuoyu

Hi, Denis

Any test result on pmac machine for this patch? 

Zhuoyu

> -Original Message-
> From: Zhang Zhuoyu [mailto:zhangzhu...@cmss.chinamobile.com]
> Sent: Wednesday, December 16, 2015 10:46 PM
> To: 'Denis Kirjanov'; 'Michael Ellerman'
> Cc: 'Daniel Axtens'; 'Zhang Zhuoyu'; 'b...@kernel.crashing.org';
> 'pau...@samba.org'; 't...@linutronix.de'; 'jiang@linux.intel.com';
> 'linuxppc-...@lists.ozlabs.org'; 'linux-kernel@vger.kernel.org'
> Subject: RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in
> migrate_irqs()
> 
> 
> > -Original Message-
> > From: Denis Kirjanov [mailto:k...@linux-powerpc.org]
> > Sent: Wednesday, December 16, 2015 7:55 PM
> > To: Michael Ellerman
> > Cc: Daniel Axtens; Zhang Zhuoyu; b...@kernel.crashing.org;
> > pau...@samba.org; t...@linutronix.de; jiang@linux.intel.com;
> > zhangzhu...@cmss.chinamobile.com; linuxppc-...@lists.ozlabs.org;
> > linux- ker...@vger.kernel.org
> > Subject: Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in
> > migrate_irqs()
> >
> > On 12/16/15, Michael Ellerman <m...@ellerman.id.au> wrote:
> > > On Wed, 2015-12-16 at 17:08 +1100, Daniel Axtens wrote:
> > >> Hi,
> > >>
> > >> A couple of things.
> > >>
> > >> Firstly, your two email addresses don't match:
> > >>
> > >> Zhang Zhuoyu <hellozzy1...@126.com> writes:
> > >
> > >> > From: Zhang Zhuoyu <zhangzhu...@cmss.chinamobile.com>
> > >>
> 
> Mmm, My mistake, I will correct it next time.
> 
> > >> These lines do seem odd! Are they causing a problem?
> > >>
> > >> I'd be more comfortable removing them if I understood why they were
> > >> added. But they've been around since the beginning of git history
> > >> so that could be a bit difficult.
> > >
> > > It's in the fullhist tree, but that doesn't tell us much (below,
> > > named fixup_irqs()).
> > >
> > > But I suspect those lines are actually there very deliberately.
> > >
> > > The function is migrating interrupts off the recently offlined cpu,
> > > because we don't want to take interrupts on an offline cpu.
> > >
> > > After it's finished doing the migration, it wants to make sure there
> > > are no interrupts that have already been latched by the offline cpu.
> > > So it briefly enables interrupts, waits a bit for the interrupts to
> > > fire, and the disables them again.
> > >
> > > Whether that actually works I couldn't say, it is very old code, and
> > > it's used on platforms where I don't ever test cpu hotplug (85xx &
> > > powermac).
> >
> > Yeah, it would be nice to test this change. I'll try it on my
> > quad-core pmac machine
> >
> 
> Thanks Michael for help explaining the code logic, it also resolved my doubts.
> These snippets are suspected when I did PM benchmark on FSL MPC85xx
> series(T1040, T4240), For T4240, which has 24 CPUs, it waits 1ms in
> migrate_irqs()each time a CPU is plugged offline, it seems a waste of time. I
> also did a test on T1040, after plugging offline/online CPU hundreds of times,
> system works well. If you have any other suggestion on how to test, I'd like
> to do more benchmark.
> (1)for((i=0; i<1000; i++)); do echo 0 > /sys/devices/system/cpu/cpu1/online;
> sleep 1; echo 1 > /sys/devices/system/cpu/cpu1/online; done
> (2)root@t1040rdb:~# cat /proc/interrupts
>CPU0   CPU1   CPU2   CPU3
>  36:393  1223  1   OpenPIC36 Level 
> serial
> LOC:   1946   1486   1555   1361   Local timer interrupts
> DBL:   7371   9707   9390   7568   Doorbell interrupts
> (3)root@t1040rdb:~# ps
>   PID TTY  TIME CMD
>  2751 ttyS000:00:00 sh
>  2757 ttyS000:00:00 ps
> root@t1040rdb:~# taskset -pc 1 2751
> pid 2751's current affinity list: 0-3
> pid 2751's new affinity list: 1
> root@t1040rdb:~#
> root@t1040rdb:~#
> root@t1040rdb:~#
> root@t1040rdb:~# echo "hello"
> hello
> root@t1040rdb:~#
> 
> > >
> > > cheers
> > >
> > >
> > > commit d58830b9a740ad1c3b089196d4afdaea251dc701
> > > Author: Zwane Mwaikambo <zw...@arm.linux.org.uk>
> > > Date:   Fri Mar 4 17:34:00 2005 -0800
> > >
> > > [PATCH] ppc64: generic hotplug cpu support
> > >
> > > Patch provides a generic hotplug cpu implementation, with the
> > > only current
> > > user being pmac.  Thi

RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

2015-12-16 Thread Zhang Zhuoyu


> -Original Message-
> From: Denis Kirjanov [mailto:k...@linux-powerpc.org]
> Sent: Wednesday, December 16, 2015 7:55 PM
> To: Michael Ellerman
> Cc: Daniel Axtens; Zhang Zhuoyu; b...@kernel.crashing.org;
> pau...@samba.org; t...@linutronix.de; jiang@linux.intel.com;
> zhangzhu...@cmss.chinamobile.com; linuxppc-...@lists.ozlabs.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in
> migrate_irqs()
> 
> On 12/16/15, Michael Ellerman  wrote:
> > On Wed, 2015-12-16 at 17:08 +1100, Daniel Axtens wrote:
> >> Hi,
> >>
> >> A couple of things.
> >>
> >> Firstly, your two email addresses don't match:
> >>
> >> Zhang Zhuoyu  writes:
> >
> >> > From: Zhang Zhuoyu 
> >>

Mmm, My mistake, I will correct it next time.

> >> These lines do seem odd! Are they causing a problem?
> >>
> >> I'd be more comfortable removing them if I understood why they were
> >> added. But they've been around since the beginning of git history so
> >> that could be a bit difficult.
> >
> > It's in the fullhist tree, but that doesn't tell us much (below, named
> > fixup_irqs()).
> >
> > But I suspect those lines are actually there very deliberately.
> >
> > The function is migrating interrupts off the recently offlined cpu,
> > because we don't want to take interrupts on an offline cpu.
> >
> > After it's finished doing the migration, it wants to make sure there
> > are no interrupts that have already been latched by the offline cpu.
> > So it briefly enables interrupts, waits a bit for the interrupts to
> > fire, and the disables them again.
> >
> > Whether that actually works I couldn't say, it is very old code, and
> > it's used on platforms where I don't ever test cpu hotplug (85xx &
> > powermac).
> 
> Yeah, it would be nice to test this change. I'll try it on my quad-core pmac
> machine
> 

Thanks Michael for help explaining the code logic, it also resolved my doubts.
These snippets are suspected when I did PM benchmark on FSL MPC85xx 
series(T1040, T4240), 
For T4240, which has 24 CPUs, it waits 1ms in migrate_irqs()each time a CPU is 
plugged offline,
it seems a waste of time. I also did a test on T1040, after plugging 
offline/online CPU hundreds of times,
system works well. If you have any other suggestion on how to test, I'd like to 
do more benchmark.
(1)for((i=0; i<1000; i++)); do echo 0 > /sys/devices/system/cpu/cpu1/online; 
sleep 1; echo 1 > /sys/devices/system/cpu/cpu1/online; done
(2)root@t1040rdb:~# cat /proc/interrupts
   CPU0   CPU1   CPU2   CPU3
 36:393  1223  1   OpenPIC36 Level 
serial
LOC:   1946   1486   1555   1361   Local timer interrupts
DBL:   7371   9707   9390   7568   Doorbell interrupts
(3)root@t1040rdb:~# ps
  PID TTY  TIME CMD
 2751 ttyS000:00:00 sh
 2757 ttyS000:00:00 ps
root@t1040rdb:~# taskset -pc 1 2751
pid 2751's current affinity list: 0-3
pid 2751's new affinity list: 1
root@t1040rdb:~#
root@t1040rdb:~#
root@t1040rdb:~#
root@t1040rdb:~# echo "hello"
hello
root@t1040rdb:~#

> >
> > cheers
> >
> >
> > commit d58830b9a740ad1c3b089196d4afdaea251dc701
> > Author: Zwane Mwaikambo 
> > Date:   Fri Mar 4 17:34:00 2005 -0800
> >
> > [PATCH] ppc64: generic hotplug cpu support
> >
> > Patch provides a generic hotplug cpu implementation, with the only
> > current
> > user being pmac.  This doesn't replace real hotplug code as is currently
> > used by LPAR systems.  Ben i can add the additional pmac specific
> > code to
> > put the processor into a sleeping state seperately.  Thanks to
> > Nathan for
> > testing.
> >
> > Signed-off-by: Zwane Mwaikambo 
> > Signed-off-by: Andrew Morton 
> > Signed-off-by: Linus Torvalds 
> >
> > diff --git a/arch/ppc64/Kconfig b/arch/ppc64/Kconfig index
> > a7933ab62e98..861f4460ad02 100644
> > --- a/arch/ppc64/Kconfig
> > +++ b/arch/ppc64/Kconfig
> > @@ -313,7 +313,7 @@ source "drivers/pci/Kconfig"
> >
> >  config HOTPLUG_CPU
> > bool "Support for hot-pluggable CPUs"
> > -   depends on SMP && EXPERIMENTAL && PPC_PSERIES
> > +   depends on SMP && EXPERIMENTAL && (PPC_PSERIES ||
> PPC_PMAC)
> > select HOTPLUG
> > ---help---
> >   Say Y here to be able to turn CPUs off and on.
> > diff --git a/arch/ppc64/kernel/idle.c b/arch/ppc64/kernel/idle.c index
> > 398b4682127b..51e

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

2015-12-16 Thread Denis Kirjanov

On 12/16/15, Michael Ellerman  wrote:
> On Wed, 2015-12-16 at 17:08 +1100, Daniel Axtens wrote:
>> Hi,
>>
>> A couple of things.
>>
>> Firstly, your two email addresses don't match:
>>
>> Zhang Zhuoyu  writes:
>
>> > From: Zhang Zhuoyu 
>>
>> These lines do seem odd! Are they causing a problem?
>>
>> I'd be more comfortable removing them if I understood why they were
>> added. But they've been around since the beginning of git history so
>> that could be a bit difficult.
>
> It's in the fullhist tree, but that doesn't tell us much (below, named
> fixup_irqs()).
>
> But I suspect those lines are actually there very deliberately.
>
> The function is migrating interrupts off the recently offlined cpu, because
> we
> don't want to take interrupts on an offline cpu.
>
> After it's finished doing the migration, it wants to make sure there are no
> interrupts that have already been latched by the offline cpu. So it briefly
> enables interrupts, waits a bit for the interrupts to fire, and the disables
> them again.
>
> Whether that actually works I couldn't say, it is very old code, and it's
> used
> on platforms where I don't ever test cpu hotplug (85xx & powermac).

Yeah, it would be nice to test this change. I'll try it on my quad-core pmac
machine

>
> cheers
>
>
> commit d58830b9a740ad1c3b089196d4afdaea251dc701
> Author: Zwane Mwaikambo 
> Date:   Fri Mar 4 17:34:00 2005 -0800
>
> [PATCH] ppc64: generic hotplug cpu support
>
> Patch provides a generic hotplug cpu implementation, with the only
> current
> user being pmac.  This doesn't replace real hotplug code as is currently
> used by LPAR systems.  Ben i can add the additional pmac specific code
> to
> put the processor into a sleeping state seperately.  Thanks to Nathan
> for
> testing.
>
> Signed-off-by: Zwane Mwaikambo 
> Signed-off-by: Andrew Morton 
> Signed-off-by: Linus Torvalds 
>
> diff --git a/arch/ppc64/Kconfig b/arch/ppc64/Kconfig
> index a7933ab62e98..861f4460ad02 100644
> --- a/arch/ppc64/Kconfig
> +++ b/arch/ppc64/Kconfig
> @@ -313,7 +313,7 @@ source "drivers/pci/Kconfig"
>
>  config HOTPLUG_CPU
>   bool "Support for hot-pluggable CPUs"
> - depends on SMP && EXPERIMENTAL && PPC_PSERIES
> + depends on SMP && EXPERIMENTAL && (PPC_PSERIES || PPC_PMAC)
>   select HOTPLUG
>   ---help---
> Say Y here to be able to turn CPUs off and on.
> diff --git a/arch/ppc64/kernel/idle.c b/arch/ppc64/kernel/idle.c
> index 398b4682127b..51eb6af14a8f 100644
> --- a/arch/ppc64/kernel/idle.c
> +++ b/arch/ppc64/kernel/idle.c
> @@ -293,6 +293,10 @@ static int native_idle(void)
>   power4_idle();
>   if (need_resched())
>   schedule();
> +
> + if (cpu_is_offline(smp_processor_id()) &&
> + system_state == SYSTEM_RUNNING)
> + cpu_die();
>   }
>   return 0;
>  }
> diff --git a/arch/ppc64/kernel/irq.c b/arch/ppc64/kernel/irq.c
> index 0ea8016146a2..4fd7f203c1e3 100644
> --- a/arch/ppc64/kernel/irq.c
> +++ b/arch/ppc64/kernel/irq.c
> @@ -116,6 +116,35 @@ skip:
>   return 0;
>  }
>
> +#ifdef CONFIG_HOTPLUG_CPU
> +void fixup_irqs(cpumask_t map)
> +{
> + unsigned int irq;
> + static int warned;
> +
> + for_each_irq(irq) {
> + cpumask_t mask;
> +
> + if (irq_desc[irq].status & IRQ_PER_CPU)
> + continue;
> +
> + cpus_and(mask, irq_affinity[irq], map);
> + if (any_online_cpu(mask) == NR_CPUS) {
> + printk("Breaking affinity for irq %i\n", irq);
> + mask = map;
> + }
> + if (irq_desc[irq].handler->set_affinity)
> + irq_desc[irq].handler->set_affinity(irq, mask);
> + else if (irq_desc[irq].action && !(warned++))
> + printk("Cannot set affinity for irq %i\n", irq);
> + }
> +
> + local_irq_enable();
> + mdelay(1);
> + local_irq_disable();
> +}
> +#endif
> +
>  extern int noirqdebug;
>
>  /*
> diff --git a/arch/ppc64/kernel/pSeries_setup.c
> b/arch/ppc64/kernel/pSeries_setup.c
> index f603397b7b04..0426892749c6 100644
> --- a/arch/ppc64/kernel/pSeries_setup.c
> +++ b/arch/ppc64/kernel/pSeries_setup.c
> @@ -320,8 +320,9 @@ static  void __init pSeries_discover_pic(void)
>   }
>  }
>
> -static void pSeries_cpu_die(void)
> +static void pSeries_mach_cpu_die(void)
>  {
> + idle_task_exit();
>   local_irq_disable();
>   /* Some hardware requires clearing the CPPR, while other hardware does 
> not
>* it is safe either way
> @@ -599,7 +600,7 @@ struct machdep_calls __initdata pSeries_md = {
>   .power_off  = rtas_power_off,
>   .halt   = rtas_halt,
>   .panic  = rtas_os_term,
> - .cpu_die= pSeries_cpu_die,
> + .cpu_die= pSeries_mach_cpu_die,
>   .get_boot_time  =

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

2015-12-16 Thread Michael Ellerman

On Wed, 2015-12-16 at 17:08 +1100, Daniel Axtens wrote:
> Hi,
> 
> A couple of things.
> 
> Firstly, your two email addresses don't match:
> 
> Zhang Zhuoyu  writes:

> > From: Zhang Zhuoyu 
> 
> These lines do seem odd! Are they causing a problem?
> 
> I'd be more comfortable removing them if I understood why they were
> added. But they've been around since the beginning of git history so
> that could be a bit difficult.

It's in the fullhist tree, but that doesn't tell us much (below, named
fixup_irqs()).

But I suspect those lines are actually there very deliberately.

The function is migrating interrupts off the recently offlined cpu, because we
don't want to take interrupts on an offline cpu.

After it's finished doing the migration, it wants to make sure there are no
interrupts that have already been latched by the offline cpu. So it briefly
enables interrupts, waits a bit for the interrupts to fire, and the disables
them again.

Whether that actually works I couldn't say, it is very old code, and it's used
on platforms where I don't ever test cpu hotplug (85xx & powermac).

cheers


commit d58830b9a740ad1c3b089196d4afdaea251dc701
Author: Zwane Mwaikambo 
Date:   Fri Mar 4 17:34:00 2005 -0800

[PATCH] ppc64: generic hotplug cpu support

Patch provides a generic hotplug cpu implementation, with the only current
user being pmac.  This doesn't replace real hotplug code as is currently
used by LPAR systems.  Ben i can add the additional pmac specific code to
put the processor into a sleeping state seperately.  Thanks to Nathan for
testing.

Signed-off-by: Zwane Mwaikambo 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 

diff --git a/arch/ppc64/Kconfig b/arch/ppc64/Kconfig
index a7933ab62e98..861f4460ad02 100644
--- a/arch/ppc64/Kconfig
+++ b/arch/ppc64/Kconfig
@@ -313,7 +313,7 @@ source "drivers/pci/Kconfig"
 
 config HOTPLUG_CPU
bool "Support for hot-pluggable CPUs"
-   depends on SMP && EXPERIMENTAL && PPC_PSERIES
+   depends on SMP && EXPERIMENTAL && (PPC_PSERIES || PPC_PMAC)
select HOTPLUG
---help---
  Say Y here to be able to turn CPUs off and on.
diff --git a/arch/ppc64/kernel/idle.c b/arch/ppc64/kernel/idle.c
index 398b4682127b..51eb6af14a8f 100644
--- a/arch/ppc64/kernel/idle.c
+++ b/arch/ppc64/kernel/idle.c
@@ -293,6 +293,10 @@ static int native_idle(void)
power4_idle();
if (need_resched())
schedule();
+
+   if (cpu_is_offline(smp_processor_id()) &&
+   system_state == SYSTEM_RUNNING)
+   cpu_die();
}
return 0;
 }
diff --git a/arch/ppc64/kernel/irq.c b/arch/ppc64/kernel/irq.c
index 0ea8016146a2..4fd7f203c1e3 100644
--- a/arch/ppc64/kernel/irq.c
+++ b/arch/ppc64/kernel/irq.c
@@ -116,6 +116,35 @@ skip:
return 0;
 }
 
+#ifdef CONFIG_HOTPLUG_CPU
+void fixup_irqs(cpumask_t map)
+{
+   unsigned int irq;
+   static int warned;
+
+   for_each_irq(irq) {
+   cpumask_t mask;
+
+   if (irq_desc[irq].status & IRQ_PER_CPU)
+   continue;
+
+   cpus_and(mask, irq_affinity[irq], map);
+   if (any_online_cpu(mask) == NR_CPUS) {
+   printk("Breaking affinity for irq %i\n", irq);
+   mask = map;
+   }
+   if (irq_desc[irq].handler->set_affinity)
+   irq_desc[irq].handler->set_affinity(irq, mask);
+   else if (irq_desc[irq].action && !(warned++))
+   printk("Cannot set affinity for irq %i\n", irq);
+   }
+
+   local_irq_enable();
+   mdelay(1);
+   local_irq_disable();
+}
+#endif
+
 extern int noirqdebug;
 
 /*
diff --git a/arch/ppc64/kernel/pSeries_setup.c 
b/arch/ppc64/kernel/pSeries_setup.c
index f603397b7b04..0426892749c6 100644
--- a/arch/ppc64/kernel/pSeries_setup.c
+++ b/arch/ppc64/kernel/pSeries_setup.c
@@ -320,8 +320,9 @@ static  void __init pSeries_discover_pic(void)
}
 }
 
-static void pSeries_cpu_die(void)
+static void pSeries_mach_cpu_die(void)
 {
+   idle_task_exit();
local_irq_disable();
/* Some hardware requires clearing the CPPR, while other hardware does 
not
 * it is safe either way
@@ -599,7 +600,7 @@ struct machdep_calls __initdata pSeries_md = {
.power_off  = rtas_power_off,
.halt   = rtas_halt,
.panic  = rtas_os_term,
-   .cpu_die= pSeries_cpu_die,
+   .cpu_die= pSeries_mach_cpu_die,
.get_boot_time  = pSeries_get_boot_time,
.get_rtc_time   = pSeries_get_rtc_time,
.set_rtc_time   = pSeries_set_rtc_time,
diff --git a/arch/ppc64/kernel/pmac_setup.c b/arch/ppc64/kernel/pmac_setup.c
index 41fa6e95a06f..5c56fc956245 100644
--- a/arch/ppc64/kernel/pmac_setup.c
+++

RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

2015-12-16 Thread Zhang Zhuoyu


> -Original Message-
> From: Denis Kirjanov [mailto:k...@linux-powerpc.org]
> Sent: Wednesday, December 16, 2015 7:55 PM
> To: Michael Ellerman
> Cc: Daniel Axtens; Zhang Zhuoyu; b...@kernel.crashing.org;
> pau...@samba.org; t...@linutronix.de; jiang@linux.intel.com;
> zhangzhu...@cmss.chinamobile.com; linuxppc-...@lists.ozlabs.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in
> migrate_irqs()
> 
> On 12/16/15, Michael Ellerman <m...@ellerman.id.au> wrote:
> > On Wed, 2015-12-16 at 17:08 +1100, Daniel Axtens wrote:
> >> Hi,
> >>
> >> A couple of things.
> >>
> >> Firstly, your two email addresses don't match:
> >>
> >> Zhang Zhuoyu <hellozzy1...@126.com> writes:
> >
> >> > From: Zhang Zhuoyu <zhangzhu...@cmss.chinamobile.com>
> >>

Mmm, My mistake, I will correct it next time.

> >> These lines do seem odd! Are they causing a problem?
> >>
> >> I'd be more comfortable removing them if I understood why they were
> >> added. But they've been around since the beginning of git history so
> >> that could be a bit difficult.
> >
> > It's in the fullhist tree, but that doesn't tell us much (below, named
> > fixup_irqs()).
> >
> > But I suspect those lines are actually there very deliberately.
> >
> > The function is migrating interrupts off the recently offlined cpu,
> > because we don't want to take interrupts on an offline cpu.
> >
> > After it's finished doing the migration, it wants to make sure there
> > are no interrupts that have already been latched by the offline cpu.
> > So it briefly enables interrupts, waits a bit for the interrupts to
> > fire, and the disables them again.
> >
> > Whether that actually works I couldn't say, it is very old code, and
> > it's used on platforms where I don't ever test cpu hotplug (85xx &
> > powermac).
> 
> Yeah, it would be nice to test this change. I'll try it on my quad-core pmac
> machine
> 

Thanks Michael for help explaining the code logic, it also resolved my doubts.
These snippets are suspected when I did PM benchmark on FSL MPC85xx 
series(T1040, T4240), 
For T4240, which has 24 CPUs, it waits 1ms in migrate_irqs()each time a CPU is 
plugged offline,
it seems a waste of time. I also did a test on T1040, after plugging 
offline/online CPU hundreds of times,
system works well. If you have any other suggestion on how to test, I'd like to 
do more benchmark.
(1)for((i=0; i<1000; i++)); do echo 0 > /sys/devices/system/cpu/cpu1/online; 
sleep 1; echo 1 > /sys/devices/system/cpu/cpu1/online; done
(2)root@t1040rdb:~# cat /proc/interrupts
   CPU0   CPU1   CPU2   CPU3
 36:393  1223  1   OpenPIC36 Level 
serial
LOC:   1946   1486   1555   1361   Local timer interrupts
DBL:   7371   9707   9390   7568   Doorbell interrupts
(3)root@t1040rdb:~# ps
  PID TTY  TIME CMD
 2751 ttyS000:00:00 sh
 2757 ttyS000:00:00 ps
root@t1040rdb:~# taskset -pc 1 2751
pid 2751's current affinity list: 0-3
pid 2751's new affinity list: 1
root@t1040rdb:~#
root@t1040rdb:~#
root@t1040rdb:~#
root@t1040rdb:~# echo "hello"
hello
root@t1040rdb:~#

> >
> > cheers
> >
> >
> > commit d58830b9a740ad1c3b089196d4afdaea251dc701
> > Author: Zwane Mwaikambo <zw...@arm.linux.org.uk>
> > Date:   Fri Mar 4 17:34:00 2005 -0800
> >
> > [PATCH] ppc64: generic hotplug cpu support
> >
> > Patch provides a generic hotplug cpu implementation, with the only
> > current
> > user being pmac.  This doesn't replace real hotplug code as is currently
> > used by LPAR systems.  Ben i can add the additional pmac specific
> > code to
> > put the processor into a sleeping state seperately.  Thanks to
> > Nathan for
> > testing.
> >
> > Signed-off-by: Zwane Mwaikambo <zw...@arm.linux.org.uk>
> > Signed-off-by: Andrew Morton <a...@osdl.org>
> > Signed-off-by: Linus Torvalds <torva...@osdl.org>
> >
> > diff --git a/arch/ppc64/Kconfig b/arch/ppc64/Kconfig index
> > a7933ab62e98..861f4460ad02 100644
> > --- a/arch/ppc64/Kconfig
> > +++ b/arch/ppc64/Kconfig
> > @@ -313,7 +313,7 @@ source "drivers/pci/Kconfig"
> >
> >  config HOTPLUG_CPU
> > bool "Support for hot-pluggable CPUs"
> > -   depends on SMP && EXPERIMENTAL && PPC_PSERIES
> > +   depends on SMP && EXPERIMENTAL && (PPC_PSERIES ||
> PPC_PMAC)
> > s

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

2015-12-16 Thread Michael Ellerman

On Wed, 2015-12-16 at 17:08 +1100, Daniel Axtens wrote:
> Hi,
> 
> A couple of things.
> 
> Firstly, your two email addresses don't match:
> 
> Zhang Zhuoyu  writes:

> > From: Zhang Zhuoyu 
> 
> These lines do seem odd! Are they causing a problem?
> 
> I'd be more comfortable removing them if I understood why they were
> added. But they've been around since the beginning of git history so
> that could be a bit difficult.

It's in the fullhist tree, but that doesn't tell us much (below, named
fixup_irqs()).

But I suspect those lines are actually there very deliberately.

The function is migrating interrupts off the recently offlined cpu, because we
don't want to take interrupts on an offline cpu.

After it's finished doing the migration, it wants to make sure there are no
interrupts that have already been latched by the offline cpu. So it briefly
enables interrupts, waits a bit for the interrupts to fire, and the disables
them again.

Whether that actually works I couldn't say, it is very old code, and it's used
on platforms where I don't ever test cpu hotplug (85xx & powermac).

cheers


commit d58830b9a740ad1c3b089196d4afdaea251dc701
Author: Zwane Mwaikambo 
Date:   Fri Mar 4 17:34:00 2005 -0800

[PATCH] ppc64: generic hotplug cpu support

Patch provides a generic hotplug cpu implementation, with the only current
user being pmac.  This doesn't replace real hotplug code as is currently
used by LPAR systems.  Ben i can add the additional pmac specific code to
put the processor into a sleeping state seperately.  Thanks to Nathan for
testing.

Signed-off-by: Zwane Mwaikambo 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 

diff --git a/arch/ppc64/Kconfig b/arch/ppc64/Kconfig
index a7933ab62e98..861f4460ad02 100644
--- a/arch/ppc64/Kconfig
+++ b/arch/ppc64/Kconfig
@@ -313,7 +313,7 @@ source "drivers/pci/Kconfig"
 
 config HOTPLUG_CPU
bool "Support for hot-pluggable CPUs"
-   depends on SMP && EXPERIMENTAL && PPC_PSERIES
+   depends on SMP && EXPERIMENTAL && (PPC_PSERIES || PPC_PMAC)
select HOTPLUG
---help---
  Say Y here to be able to turn CPUs off and on.
diff --git a/arch/ppc64/kernel/idle.c b/arch/ppc64/kernel/idle.c
index 398b4682127b..51eb6af14a8f 100644
--- a/arch/ppc64/kernel/idle.c
+++ b/arch/ppc64/kernel/idle.c
@@ -293,6 +293,10 @@ static int native_idle(void)
power4_idle();
if (need_resched())
schedule();
+
+   if (cpu_is_offline(smp_processor_id()) &&
+   system_state == SYSTEM_RUNNING)
+   cpu_die();
}
return 0;
 }
diff --git a/arch/ppc64/kernel/irq.c b/arch/ppc64/kernel/irq.c
index 0ea8016146a2..4fd7f203c1e3 100644
--- a/arch/ppc64/kernel/irq.c
+++ b/arch/ppc64/kernel/irq.c
@@ -116,6 +116,35 @@ skip:
return 0;
 }
 
+#ifdef CONFIG_HOTPLUG_CPU
+void fixup_irqs(cpumask_t map)
+{
+   unsigned int irq;
+   static int warned;
+
+   for_each_irq(irq) {
+   cpumask_t mask;
+
+   if (irq_desc[irq].status & IRQ_PER_CPU)
+   continue;
+
+   cpus_and(mask, irq_affinity[irq], map);
+   if (any_online_cpu(mask) == NR_CPUS) {
+   printk("Breaking affinity for irq %i\n", irq);
+   mask = map;
+   }
+   if (irq_desc[irq].handler->set_affinity)
+   irq_desc[irq].handler->set_affinity(irq, mask);
+   else if (irq_desc[irq].action && !(warned++))
+   printk("Cannot set affinity for irq %i\n", irq);
+   }
+
+   local_irq_enable();
+   mdelay(1);
+   local_irq_disable();
+}
+#endif
+
 extern int noirqdebug;
 
 /*
diff --git a/arch/ppc64/kernel/pSeries_setup.c 
b/arch/ppc64/kernel/pSeries_setup.c
index f603397b7b04..0426892749c6 100644
--- a/arch/ppc64/kernel/pSeries_setup.c
+++ b/arch/ppc64/kernel/pSeries_setup.c
@@ -320,8 +320,9 @@ static  void __init pSeries_discover_pic(void)
}
 }
 
-static void pSeries_cpu_die(void)
+static void pSeries_mach_cpu_die(void)
 {
+   idle_task_exit();
local_irq_disable();
/* Some hardware requires clearing the CPPR, while other hardware does 
not
 * it is safe either way
@@ -599,7 +600,7 @@ struct machdep_calls __initdata pSeries_md = {
.power_off  = rtas_power_off,
.halt   = rtas_halt,
.panic  = rtas_os_term,
-   .cpu_die= pSeries_cpu_die,
+   .cpu_die= pSeries_mach_cpu_die,
.get_boot_time  = pSeries_get_boot_time,
.get_rtc_time   = pSeries_get_rtc_time,
.set_rtc_time   = pSeries_set_rtc_time,
diff --git

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

2015-12-16 Thread Denis Kirjanov

On 12/16/15, Michael Ellerman  wrote:
> On Wed, 2015-12-16 at 17:08 +1100, Daniel Axtens wrote:
>> Hi,
>>
>> A couple of things.
>>
>> Firstly, your two email addresses don't match:
>>
>> Zhang Zhuoyu  writes:
>
>> > From: Zhang Zhuoyu 
>>
>> These lines do seem odd! Are they causing a problem?
>>
>> I'd be more comfortable removing them if I understood why they were
>> added. But they've been around since the beginning of git history so
>> that could be a bit difficult.
>
> It's in the fullhist tree, but that doesn't tell us much (below, named
> fixup_irqs()).
>
> But I suspect those lines are actually there very deliberately.
>
> The function is migrating interrupts off the recently offlined cpu, because
> we
> don't want to take interrupts on an offline cpu.
>
> After it's finished doing the migration, it wants to make sure there are no
> interrupts that have already been latched by the offline cpu. So it briefly
> enables interrupts, waits a bit for the interrupts to fire, and the disables
> them again.
>
> Whether that actually works I couldn't say, it is very old code, and it's
> used
> on platforms where I don't ever test cpu hotplug (85xx & powermac).

Yeah, it would be nice to test this change. I'll try it on my quad-core pmac
machine

>
> cheers
>
>
> commit d58830b9a740ad1c3b089196d4afdaea251dc701
> Author: Zwane Mwaikambo 
> Date:   Fri Mar 4 17:34:00 2005 -0800
>
> [PATCH] ppc64: generic hotplug cpu support
>
> Patch provides a generic hotplug cpu implementation, with the only
> current
> user being pmac.  This doesn't replace real hotplug code as is currently
> used by LPAR systems.  Ben i can add the additional pmac specific code
> to
> put the processor into a sleeping state seperately.  Thanks to Nathan
> for
> testing.
>
> Signed-off-by: Zwane Mwaikambo 
> Signed-off-by: Andrew Morton 
> Signed-off-by: Linus Torvalds 
>
> diff --git a/arch/ppc64/Kconfig b/arch/ppc64/Kconfig
> index a7933ab62e98..861f4460ad02 100644
> --- a/arch/ppc64/Kconfig
> +++ b/arch/ppc64/Kconfig
> @@ -313,7 +313,7 @@ source "drivers/pci/Kconfig"
>
>  config HOTPLUG_CPU
>   bool "Support for hot-pluggable CPUs"
> - depends on SMP && EXPERIMENTAL && PPC_PSERIES
> + depends on SMP && EXPERIMENTAL && (PPC_PSERIES || PPC_PMAC)
>   select HOTPLUG
>   ---help---
> Say Y here to be able to turn CPUs off and on.
> diff --git a/arch/ppc64/kernel/idle.c b/arch/ppc64/kernel/idle.c
> index 398b4682127b..51eb6af14a8f 100644
> --- a/arch/ppc64/kernel/idle.c
> +++ b/arch/ppc64/kernel/idle.c
> @@ -293,6 +293,10 @@ static int native_idle(void)
>   power4_idle();
>   if (need_resched())
>   schedule();
> +
> + if (cpu_is_offline(smp_processor_id()) &&
> + system_state == SYSTEM_RUNNING)
> + cpu_die();
>   }
>   return 0;
>  }
> diff --git a/arch/ppc64/kernel/irq.c b/arch/ppc64/kernel/irq.c
> index 0ea8016146a2..4fd7f203c1e3 100644
> --- a/arch/ppc64/kernel/irq.c
> +++ b/arch/ppc64/kernel/irq.c
> @@ -116,6 +116,35 @@ skip:
>   return 0;
>  }
>
> +#ifdef CONFIG_HOTPLUG_CPU
> +void fixup_irqs(cpumask_t map)
> +{
> + unsigned int irq;
> + static int warned;
> +
> + for_each_irq(irq) {
> + cpumask_t mask;
> +
> + if (irq_desc[irq].status & IRQ_PER_CPU)
> + continue;
> +
> + cpus_and(mask, irq_affinity[irq], map);
> + if (any_online_cpu(mask) == NR_CPUS) {
> + printk("Breaking affinity for irq %i\n", irq);
> + mask = map;
> + }
> + if (irq_desc[irq].handler->set_affinity)
> + irq_desc[irq].handler->set_affinity(irq, mask);
> + else if (irq_desc[irq].action && !(warned++))
> + printk("Cannot set affinity for irq %i\n", irq);
> + }
> +
> + local_irq_enable();
> + mdelay(1);
> + local_irq_disable();
> +}
> +#endif
> +
>  extern int noirqdebug;
>
>  /*
> diff --git a/arch/ppc64/kernel/pSeries_setup.c
> b/arch/ppc64/kernel/pSeries_setup.c
> index f603397b7b04..0426892749c6 100644
> --- a/arch/ppc64/kernel/pSeries_setup.c
> +++ b/arch/ppc64/kernel/pSeries_setup.c
> @@ -320,8 +320,9 @@ static  void __init pSeries_discover_pic(void)
>   }
>  }
>
> -static void pSeries_cpu_die(void)
> +static void pSeries_mach_cpu_die(void)
>  {
> + idle_task_exit();
>   local_irq_disable();
>   /* Some hardware requires clearing the CPPR, while other hardware does 
> not
>* it is safe either way
> @@ -599,7 +600,7 @@ struct machdep_calls __initdata pSeries_md = {
>   .power_off  = rtas_power_off,
>   .halt   = rtas_halt,
>   .panic  = rtas_os_term,

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

2015-12-15 Thread Daniel Axtens


Hi,

A couple of things.

Firstly, your two email addresses don't match:

Zhang Zhuoyu  writes:
> From: Zhang Zhuoyu 

These lines do seem odd! Are they causing a problem?

I'd be more comfortable removing them if I understood why they were
added. But they've been around since the beginning of git history so
that could be a bit difficult.

Instead, would it be possible to make sure this doesn't break anything
by including some test results?

Thanks,
Daniel

> Remove paradoxical and unnecessary lines before disable local interrupt.
>
> Signed-off-by: Zhang Zhuoyu 
> ---
>  arch/powerpc/kernel/irq.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> index 290559d..03fa686 100644
> --- a/arch/powerpc/kernel/irq.c
> +++ b/arch/powerpc/kernel/irq.c
> @@ -454,8 +454,6 @@ void migrate_irqs(void)
>  
>   free_cpumask_var(mask);
>  
> - local_irq_enable();
> - mdelay(1);
>   local_irq_disable();
>  }
>  #endif
> -- 
> 1.8.3.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

2015-12-15 Thread Daniel Axtens


Hi,

A couple of things.

Firstly, your two email addresses don't match:

Zhang Zhuoyu  writes:
> From: Zhang Zhuoyu 

These lines do seem odd! Are they causing a problem?

I'd be more comfortable removing them if I understood why they were
added. But they've been around since the beginning of git history so
that could be a bit difficult.

Instead, would it be possible to make sure this doesn't break anything
by including some test results?

Thanks,
Daniel

> Remove paradoxical and unnecessary lines before disable local interrupt.
>
> Signed-off-by: Zhang Zhuoyu 
> ---
>  arch/powerpc/kernel/irq.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> index 290559d..03fa686 100644
> --- a/arch/powerpc/kernel/irq.c
> +++ b/arch/powerpc/kernel/irq.c
> @@ -454,8 +454,6 @@ void migrate_irqs(void)
>  
>   free_cpumask_var(mask);
>  
> - local_irq_enable();
> - mdelay(1);
>   local_irq_disable();
>  }
>  #endif
> -- 
> 1.8.3.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

RE: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

Re: [PATCH 1/1] powerpc/irq: tidy up inconsistent context in migrate_irqs()

14 matches

Site Navigation

Mail list logo

Footer information