[patch 3/5] git-ia64 versus genirq

[patch 3/5] git-ia64 versus genirq

[horms]

> commit 1f4c5c1fe2a6a74271989ec079af11e2bb8e2826
> tree a0da63a3dcc3ffd71653ecc039db416dbcaa86d4
> parent beada884dd437b509c26b39f1a0b0c6b31e6f340
> author Andrew Morton <akpm@osdl.org> 1151573360 -0700
> committer Tony Luck <tony.luck@intel.com> 1151607053 -0700
> 
> [IA64] git-ia64 versus genirq
> 
> Fix the git-ia64 tree after genirq merge.
> 
> Signed-off-by: Andrew Morton <akpm@osdl.org>
> Signed-off-by: Tony Luck <tony.luck@intel.com>

Patch from test branch of Tony Luck's ia64 tree.
This is needed for ia64 kexec in Linus's tree.

Cc: Andrew Morton <akpm@osdl.org>
Cc: Tony Luck <tony.luck@intel.com>
Signed-Off-By: Simon Horman <horms@verge.net.au>

 arch/ia64/kernel/crash.c         |    4 ++--
 arch/ia64/kernel/machine_kexec.c |    6 +++---
 kernel/irq/manage.c              |   15 ++++-----------
 3 files changed, 9 insertions(+), 16 deletions(-)

1f4c5c1fe2a6a74271989ec079af11e2bb8e2826
diff --git a/arch/ia64/kernel/crash.c b/arch/ia64/kernel/crash.c
index 03c3118..a1a192c 100644
--- a/arch/ia64/kernel/crash.c
+++ b/arch/ia64/kernel/crash.c
@@ -48,7 +48,7 @@ static void device_shootdown(void)
        u16 pci_command;
 
        list_for_each_entry(dev, &pci_devices, global_list) {
-               desc = irq_descp(dev->irq);
+	       desc = irq_desc + dev->irq;
                if (!desc->action)
                        continue;
                pci_read_config_word(dev, PCI_COMMAND, &pci_command);
@@ -57,7 +57,7 @@ static void device_shootdown(void)
                        pci_write_config_word(dev, PCI_COMMAND, pci_command);
                }
                disable_irq_nosync(dev->irq);
-               desc->handler->end(dev->irq);
+               desc->chip->end(dev->irq);
        }
 }
 
diff --git a/arch/ia64/kernel/machine_kexec.c b/arch/ia64/kernel/machine_kexec.c
index 73fbb26..573a6b8 100644
--- a/arch/ia64/kernel/machine_kexec.c
+++ b/arch/ia64/kernel/machine_kexec.c
@@ -63,13 +63,13 @@ #ifdef CONFIG_PCI
 	while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
 		if (!(dev->is_enabled))
 			continue;
-		idesc = irq_descp(dev->irq);
+		idesc = irq_desc + dev->irq;
 		if (!idesc)
 			continue;
 		cpu_set(0, mask);
 		disable_irq_nosync(dev->irq);
-		idesc->handler->end(dev->irq);
-		idesc->handler->set_affinity(dev->irq, mask);
+		idesc->chip->end(dev->irq);
+		idesc->chip->set_affinity(dev->irq, mask);
 		idesc->action = NULL;
 		pci_disable_device(dev);
 	}
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 7d855d2..4ef0356 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -398,17 +398,10 @@ EXPORT_SYMBOL(request_irq);
  */
 void terminate_irqs(void)
 {
-	struct irqaction * action;
-	irq_desc_t *idesc;
+	irq_desc_t *desc = irq_desc;
 	int i;
 
-	for (i=0; i < NR_IRQS; i++) {
-		idesc = irq_descp(i);
-		action = idesc->action;
-		if (!action)
-			continue;
-		if (idesc->handler->end)
-			idesc->handler->end(i);
-	}
+	for (i = 0; i < NR_IRQS; i++, desc++)
+		if (desc->action && desc->chip->end)
+			desc->chip->end(i);
 }
-

--

-- 
Horms
  H: http://www.vergenet.net/~horms/
  W: http://www.valinux.co.jp/en/

Re: [patch 3/5] git-ia64 versus genirq

[Andrew Morton]

On Thu, 17 Aug 2006 16:07:22 +0900
horms@tabatha.lab.ultramonkey.org wrote:

> > commit 1f4c5c1fe2a6a74271989ec079af11e2bb8e2826
> > tree a0da63a3dcc3ffd71653ecc039db416dbcaa86d4
> > parent beada884dd437b509c26b39f1a0b0c6b31e6f340
> > author Andrew Morton <akpm@osdl.org> 1151573360 -0700
> > committer Tony Luck <tony.luck@intel.com> 1151607053 -0700
> > 
> > [IA64] git-ia64 versus genirq
> > 
> > Fix the git-ia64 tree after genirq merge.
> > 
> > Signed-off-by: Andrew Morton <akpm@osdl.org>
> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> 
> Patch from test branch of Tony Luck's ia64 tree.
> This is needed for ia64 kexec in Linus's tree.
> 

I think you're telling us that Tony needs to get this into mainline, yes?

> 
> 1f4c5c1fe2a6a74271989ec079af11e2bb8e2826
> diff --git a/arch/ia64/kernel/crash.c b/arch/ia64/kernel/crash.c
> index 03c3118..a1a192c 100644
> --- a/arch/ia64/kernel/crash.c
> +++ b/arch/ia64/kernel/crash.c
> @@ -48,7 +48,7 @@ static void device_shootdown(void)
>         u16 pci_command;
>  
>         list_for_each_entry(dev, &pci_devices, global_list) {
> -               desc = irq_descp(dev->irq);
> +	       desc = irq_desc + dev->irq;
>                 if (!desc->action)
>                         continue;
>                 pci_read_config_word(dev, PCI_COMMAND, &pci_command);
> @@ -57,7 +57,7 @@ static void device_shootdown(void)
>                         pci_write_config_word(dev, PCI_COMMAND, pci_command);
>                 }
>                 disable_irq_nosync(dev->irq);
> -               desc->handler->end(dev->irq);
> +               desc->chip->end(dev->irq);
>         }
>  }
>  
> diff --git a/arch/ia64/kernel/machine_kexec.c b/arch/ia64/kernel/machine_kexec.c
> index 73fbb26..573a6b8 100644
> --- a/arch/ia64/kernel/machine_kexec.c
> +++ b/arch/ia64/kernel/machine_kexec.c
> @@ -63,13 +63,13 @@ #ifdef CONFIG_PCI
>  	while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
>  		if (!(dev->is_enabled))
>  			continue;
> -		idesc = irq_descp(dev->irq);
> +		idesc = irq_desc + dev->irq;
>  		if (!idesc)
>  			continue;
>  		cpu_set(0, mask);
>  		disable_irq_nosync(dev->irq);
> -		idesc->handler->end(dev->irq);
> -		idesc->handler->set_affinity(dev->irq, mask);
> +		idesc->chip->end(dev->irq);
> +		idesc->chip->set_affinity(dev->irq, mask);
>  		idesc->action = NULL;
>  		pci_disable_device(dev);
>  	}
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 7d855d2..4ef0356 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -398,17 +398,10 @@ EXPORT_SYMBOL(request_irq);
>   */
>  void terminate_irqs(void)
>  {
> -	struct irqaction * action;
> -	irq_desc_t *idesc;
> +	irq_desc_t *desc = irq_desc;
>  	int i;
>  
> -	for (i=0; i < NR_IRQS; i++) {
> -		idesc = irq_descp(i);
> -		action = idesc->action;
> -		if (!action)
> -			continue;
> -		if (idesc->handler->end)
> -			idesc->handler->end(i);
> -	}
> +	for (i = 0; i < NR_IRQS; i++, desc++)
> +		if (desc->action && desc->chip->end)
> +			desc->chip->end(i);
>  }
> -
> 
> --
> 
> -- 
> Horms
>   H: http://www.vergenet.net/~horms/
>   W: http://www.valinux.co.jp/en/

Re: [patch 3/5] git-ia64 versus genirq

[Eric W. Biederman]

Horms <horms@verge.net.au> writes:

> On Thu, 17 Aug 2006 00:28:53 -0700, Andrew Morton wrote:
>> On Thu, 17 Aug 2006 16:07:22 +0900
>> horms@tabatha.lab.ultramonkey.org wrote:
>> 
>>> > commit 1f4c5c1fe2a6a74271989ec079af11e2bb8e2826
>>> > tree a0da63a3dcc3ffd71653ecc039db416dbcaa86d4
>>> > parent beada884dd437b509c26b39f1a0b0c6b31e6f340
>>> > author Andrew Morton <akpm@osdl.org> 1151573360 -0700
>>> > committer Tony Luck <tony.luck@intel.com> 1151607053 -0700
>>> > 
>>> > [IA64] git-ia64 versus genirq
>>> > 
>>> > Fix the git-ia64 tree after genirq merge.
>>> > 
>>> > Signed-off-by: Andrew Morton <akpm@osdl.org>
>>> > Signed-off-by: Tony Luck <tony.luck@intel.com>
>>> 
>>> Patch from test branch of Tony Luck's ia64 tree.
>>> This is needed for ia64 kexec in Linus's tree.
>>> 
>> 
>> I think you're telling us that Tony needs to get this into mainline, yes?
>
> This would be ia64 kexec.
>
> I was thinking more along the lines that it would be nice if Zou Nan hai
> sent incremental patches against Tony's tree. But he probably gets more
> testing by sending out a apply and forget patch against 2.6.18-rc4. And
> certainly merging ia64 kexec into Linus' tree would be a nice resolution
> to this problem.
>
> At OLS Tony spoke about what state he would like to see kexec in before
> it is merged into Linus' tree. Basically reports that it works on
> at least a cople of different vendor's gear. That is proving harder
> than one might have hoped. But the code is marked as experimental,
> and if pushing it into Linus's tree both makes patch management a bit
> easier, and potentially gives the code better testing, then it seems
> like a good idea to me.

Guys I have a serious problem with this patchset.

It does way to much crap on the crash shutdown path, and none of the
improvements we discussed making at OLS have even been touched.  Even
from Zou Nan hai patchset there was a comment we should use the
startup IPI but he didn't because the firmware implements improperly.

We had a presentation at OLS about how on x86 where we have a fairly minimal
crash_shutdown how that implementation suffers from being nowhere near
paranoid enough.  With this patchset I can almost guarantee your kexec
on panic path is worthless in the face of real failures.

Tony's point on testing is also important, but this is the kind of
code path you can only get right by being paranoid and reviewing the
code carefully.

There is way to much of this patchset where it appears you are
trying to solve things on the shutdown path and not in the init
routines.  We had several discussions at OLS where it was the
indisputed conclusion that there were no short cuts.  Fixing things
right in the init routines was the way to go.

Things like the genirq have no place even affecting the kexec on
panic path.

Do I need to get specific and describe the faults of each individual
function or have I said enough?

Eric

Re: [patch 3/5] git-ia64 versus genirq

[Zou Nan hai]

On Thu, 2006-08-17 at 21:58, Eric W. Biederman wrote:
> Horms <horms@verge.net.au> writes:
> 
> > On Thu, 17 Aug 2006 00:28:53 -0700, Andrew Morton wrote:
> >> On Thu, 17 Aug 2006 16:07:22 +0900
> >> horms@tabatha.lab.ultramonkey.org wrote:
> >> 
> >>> > commit 1f4c5c1fe2a6a74271989ec079af11e2bb8e2826
> >>> > tree a0da63a3dcc3ffd71653ecc039db416dbcaa86d4
> >>> > parent beada884dd437b509c26b39f1a0b0c6b31e6f340
> >>> > author Andrew Morton <akpm@osdl.org> 1151573360 -0700
> >>> > committer Tony Luck <tony.luck@intel.com> 1151607053 -0700
> >>> > 
> >>> > [IA64] git-ia64 versus genirq
> >>> > 
> >>> > Fix the git-ia64 tree after genirq merge.
> >>> > 
> >>> > Signed-off-by: Andrew Morton <akpm@osdl.org>
> >>> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> >>> 
> >>> Patch from test branch of Tony Luck's ia64 tree.
> >>> This is needed for ia64 kexec in Linus's tree.
> >>> 
> >> 
> >> I think you're telling us that Tony needs to get this into mainline, yes?
> >
> > This would be ia64 kexec.
> >
> > I was thinking more along the lines that it would be nice if Zou Nan hai
> > sent incremental patches against Tony's tree. But he probably gets more
> > testing by sending out a apply and forget patch against 2.6.18-rc4. And
> > certainly merging ia64 kexec into Linus' tree would be a nice resolution
> > to this problem.
> >
> > At OLS Tony spoke about what state he would like to see kexec in before
> > it is merged into Linus' tree. Basically reports that it works on
> > at least a cople of different vendor's gear. That is proving harder
> > than one might have hoped. But the code is marked as experimental,
> > and if pushing it into Linus's tree both makes patch management a bit
> > easier, and potentially gives the code better testing, then it seems
> > like a good idea to me.
> 
> Guys I have a serious problem with this patchset.
> 

Hi Eric,
	That based patchset was post days before the OLS....
	I have a much crash path simplified patch set posted a few days before.
	http://www.gelato.unsw.edu.au/archives/linux-ia64/0608/18790.html
	I agree that we should have minmal code in crash path.
	I am trying to simplify that patch set more.

Thanks
Zou Nan hai 

	
	

Re: [patch 3/5] git-ia64 versus genirq

[Horms]

On Thu, 17 Aug 2006 07:58:45 -0600, Eric W. Biederman wrote:
> Horms <horms@verge.net.au> writes:
> 
>> On Thu, 17 Aug 2006 00:28:53 -0700, Andrew Morton wrote:
>>> On Thu, 17 Aug 2006 16:07:22 +0900
>>> horms@tabatha.lab.ultramonkey.org wrote:
>>> 
>>>> > commit 1f4c5c1fe2a6a74271989ec079af11e2bb8e2826
>>>> > tree a0da63a3dcc3ffd71653ecc039db416dbcaa86d4
>>>> > parent beada884dd437b509c26b39f1a0b0c6b31e6f340
>>>> > author Andrew Morton <akpm@osdl.org> 1151573360 -0700
>>>> > committer Tony Luck <tony.luck@intel.com> 1151607053 -0700
>>>> > 
>>>> > [IA64] git-ia64 versus genirq
>>>> > 
>>>> > Fix the git-ia64 tree after genirq merge.
>>>> > 
>>>> > Signed-off-by: Andrew Morton <akpm@osdl.org>
>>>> > Signed-off-by: Tony Luck <tony.luck@intel.com>
>>>> 
>>>> Patch from test branch of Tony Luck's ia64 tree.
>>>> This is needed for ia64 kexec in Linus's tree.
>>>> 
>>> 
>>> I think you're telling us that Tony needs to get this into mainline, yes?
>>
>> This would be ia64 kexec.
>>
>> I was thinking more along the lines that it would be nice if Zou Nan hai
>> sent incremental patches against Tony's tree. But he probably gets more
>> testing by sending out a apply and forget patch against 2.6.18-rc4. And
>> certainly merging ia64 kexec into Linus' tree would be a nice resolution
>> to this problem.
>>
>> At OLS Tony spoke about what state he would like to see kexec in before
>> it is merged into Linus' tree. Basically reports that it works on
>> at least a cople of different vendor's gear. That is proving harder
>> than one might have hoped. But the code is marked as experimental,
>> and if pushing it into Linus's tree both makes patch management a bit
>> easier, and potentially gives the code better testing, then it seems
>> like a good idea to me.
> 
> Guys I have a serious problem with this patchset.
> 
> It does way to much crap on the crash shutdown path, and none of the
> improvements we discussed making at OLS have even been touched.  Even
> from Zou Nan hai patchset there was a comment we should use the
> startup IPI but he didn't because the firmware implements improperly.
> 
> We had a presentation at OLS about how on x86 where we have a fairly minimal
> crash_shutdown how that implementation suffers from being nowhere near
> paranoid enough.  With this patchset I can almost guarantee your kexec
> on panic path is worthless in the face of real failures.
> 
> Tony's point on testing is also important, but this is the kind of
> code path you can only get right by being paranoid and reviewing the
> code carefully.
> 
> There is way to much of this patchset where it appears you are
> trying to solve things on the shutdown path and not in the init
> routines.  We had several discussions at OLS where it was the
> indisputed conclusion that there were no short cuts.  Fixing things
> right in the init routines was the way to go.
> 
> Things like the genirq have no place even affecting the kexec on
> panic path.
> 
> Do I need to get specific and describe the faults of each individual
> function or have I said enough?

Hi Eric,

as you are the kexec maintainer I think that code reviews from you are
highly appropriate.

As to the issue at hand. I believe that the most troublesome code is the
PCI shutdown routines, which I myself am not particularly keen on either.
I even posted a patch to remove them at one stage. However, according to
a recent discussion that I had with Nan hai about this issue, it seems
that they are a work around for some HP boxes. 

Clearly you are right that it would be a lot nicer to resolve the
problem in the init path. Perhaps an interim solution would be to wrap
these bits of code in #ifdef CONFIG_IA64_HP_ZX1 (Nan hai, would that
cover the hardware that you are seeing the problem on?) and/or break it
out into a separate add-on, very-wip patch.

Lastly, Nan hai, do you have remote or local access to HP hardware that
exhibits this problem? I don't, but perhaps someone can provide some
access to help explore better options to this problem.

-- 
Horms
  H: http://www.vergenet.net/~horms/
  W: http://www.valinux.co.jp/en/

RE: [patch 3/5] git-ia64 versus genirq

[Zou, Nanhai]

> -----Original Message-----
> From: Horms [mailto:horms@verge.net.au]
> Sent: 2006Äê8ÔÂ18ÈÕ 8:39
> To: Eric W. Biederman
> Cc: Andrew Morton; fastboot@osdl.org; Zou, Nanhai; Luck, Tony;
> linux-ia64@vger.kernel.org
> Subject: Re: [patch 3/5] git-ia64 versus genirq
> 
> On Thu, 17 Aug 2006 07:58:45 -0600, Eric W. Biederman wrote:
> > Horms <horms@verge.net.au> writes:
> >
> >> On Thu, 17 Aug 2006 00:28:53 -0700, Andrew Morton wrote:
> >>> On Thu, 17 Aug 2006 16:07:22 +0900
> >>> horms@tabatha.lab.ultramonkey.org wrote:
> >>>
> >>>> > commit 1f4c5c1fe2a6a74271989ec079af11e2bb8e2826
> >>>> > tree a0da63a3dcc3ffd71653ecc039db416dbcaa86d4
> >>>> > parent beada884dd437b509c26b39f1a0b0c6b31e6f340
> >>>> > author Andrew Morton <akpm@osdl.org> 1151573360 -0700
> >>>> > committer Tony Luck <tony.luck@intel.com> 1151607053 -0700
> >>>> >
> >>>> > [IA64] git-ia64 versus genirq
> >>>> >
> >>>> > Fix the git-ia64 tree after genirq merge.
> >>>> >
> >>>> > Signed-off-by: Andrew Morton <akpm@osdl.org>
> >>>> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> >>>>
> >>>> Patch from test branch of Tony Luck's ia64 tree.
> >>>> This is needed for ia64 kexec in Linus's tree.
> >>>>
> >>>
> >>> I think you're telling us that Tony needs to get this into mainline, yes?
> >>
> >> This would be ia64 kexec.
> problem.
> >>
> >> At OLS Tony spoke about what state he would like to see kexec in before
> >> it is merged into Linus' tree. Basically reports that it works on
> >> at least a cople of different vendor's gear. That is proving harder
> >> than one might have hoped. But the code is marked as experimental,
> >> and if pushing it into Linus's tree both makes patch management a bit
> >> easier, and potentially gives the code better testing, then it seems
> Hi Eric,
> 
> Clearly you are right that it would be a lot nicer to resolve the
> problem in the init path. Perhaps an interim solution would be to wrap
> these bits of code in #ifdef CONFIG_IA64_HP_ZX1 (Nan hai, would that
> cover the hardware that you are seeing the problem on?) and/or break it
> out into a separate add-on, very-wip patch.


Hi Horms,
	I have little knowledge of the code inside CONFIG_IA64_HP_ZX1....
I think it stop's IOMMU controller on some HP boxes.

Aziz told me it is necessary to have this code to start up the second kernel.
Thanks
Zou Nan hai

> Horms
>   H: http://www.vergenet.net/~horms/
>   W: http://www.valinux.co.jp/en/

Re: [patch 3/5] git-ia64 versus genirq

[Horms]

On Fri, Aug 18, 2006 at 06:55:36AM +0800, Zou Nan hai wrote:
> On Thu, 2006-08-17 at 21:58, Eric W. Biederman wrote:
> > Horms <horms@verge.net.au> writes:
> > 
> > > On Thu, 17 Aug 2006 00:28:53 -0700, Andrew Morton wrote:
> > >> On Thu, 17 Aug 2006 16:07:22 +0900
> > >> horms@tabatha.lab.ultramonkey.org wrote:
> > >> 
> > >>> > commit 1f4c5c1fe2a6a74271989ec079af11e2bb8e2826
> > >>> > tree a0da63a3dcc3ffd71653ecc039db416dbcaa86d4
> > >>> > parent beada884dd437b509c26b39f1a0b0c6b31e6f340
> > >>> > author Andrew Morton <akpm@osdl.org> 1151573360 -0700
> > >>> > committer Tony Luck <tony.luck@intel.com> 1151607053 -0700
> > >>> > 
> > >>> > [IA64] git-ia64 versus genirq
> > >>> > 
> > >>> > Fix the git-ia64 tree after genirq merge.
> > >>> > 
> > >>> > Signed-off-by: Andrew Morton <akpm@osdl.org>
> > >>> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> > >>> 
> > >>> Patch from test branch of Tony Luck's ia64 tree.
> > >>> This is needed for ia64 kexec in Linus's tree.
> > >>> 
> > >> 
> > >> I think you're telling us that Tony needs to get this into mainline, yes?
> > >
> > > This would be ia64 kexec.
> > >
> > > I was thinking more along the lines that it would be nice if Zou Nan hai
> > > sent incremental patches against Tony's tree. But he probably gets more
> > > testing by sending out a apply and forget patch against 2.6.18-rc4. And
> > > certainly merging ia64 kexec into Linus' tree would be a nice resolution
> > > to this problem.
> > >
> > > At OLS Tony spoke about what state he would like to see kexec in before
> > > it is merged into Linus' tree. Basically reports that it works on
> > > at least a cople of different vendor's gear. That is proving harder
> > > than one might have hoped. But the code is marked as experimental,
> > > and if pushing it into Linus's tree both makes patch management a bit
> > > easier, and potentially gives the code better testing, then it seems
> > > like a good idea to me.
> > 
> > Guys I have a serious problem with this patchset.
> > 
> 
> Hi Eric,
> 	That based patchset was post days before the OLS....
> 	I have a much crash path simplified patch set posted a few days before.
> 	http://www.gelato.unsw.edu.au/archives/linux-ia64/0608/18790.html
> 	I agree that we should have minmal code in crash path.
> 	I am trying to simplify that patch set more.

Just for the record, that patch should have been included in
the series that I posted and this thread has spawned off.

When I say I posted, I may have done the posting, but
its just reformating if your patches, so there is really
absolutly nothing new in there.

My real aim in posting the patches in incremental form was firstly to
make it easier to see how the code is evolving.  And secondly to make
any applying and/or merging, particularly into Tony's tree, easier.

-- 
Horms
  H: http://www.vergenet.net/~horms/
  W: http://www.valinux.co.jp/en/