public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Milton Miller <miltonm@bga•com>
To: michael@ellerman•id.au
Cc: ppcdev <linuxppc-dev@ozlabs•org>,
	Paul Mackerras <paulus@samba•org>,
	Anton Blanchard <anton@samba•org>,
	fastboot@lists•osdl.org
Subject: Re: [PATCH] Fix interrupt distribution in ppc970
Date: Tue, 10 Apr 2007 11:59:28 -0500	[thread overview]
Message-ID: <eeb3b19029c753682f476e97dcfaa820@bga.com> (raw)
In-Reply-To: <1176188763.9836.16.camel@concordia.ozlabs.ibm.com>


On Apr 10, 2007, at 2:06 AM, Michael Ellerman wrote:

> On Mon, 2007-04-09 at 14:27 +0530, Mohan Kumar M wrote:
>> On Wed, Mar 07, 2007 at 11:52:32AM +0100, Michael Ellerman wrote:
>>> There's already maxcpus in init/main.c, that would probably be 
>>> better,
>>> though still ugly.
>>>
>> Based on Mike's suggestions, I modified the patch. The attached patch
>> refers max_cpus variable to check whether the kernel is booted with
>> maxcpus=1 parameter and if maxcpus=1 is specified the patch assigns 
>> only
>> the current boot cpu to be the default distribution server.
>
> So the core of the problem is that if we haven't onlined all cpus then
> we can't use the default_distrib_server value given to us by firmware,
> because some of the cpus in that queue won't be online.
>
> We can detect this situation by comparing the number of cpus that are
> online vs the number that are present (not possible). This might even
> work if you boot with maxcpus=1 and then hotplug the rest in.
>
> How about this:
>
> Index: powerpc/arch/powerpc/platforms/pseries/xics.c
> ===================================================================
> --- powerpc.orig/arch/powerpc/platforms/pseries/xics.c
> +++ powerpc/arch/powerpc/platforms/pseries/xics.c
> @@ -167,7 +167,10 @@ static int get_irq_server(unsigned int v
>  		return default_server;
>
>  	if (cpus_equal(cpumask, CPU_MASK_ALL)) {
> -		server = default_distrib_server;
> +		if (num_online_cpus() == num_present_cpus())
> +			server = default_distrib_server;
> +		else
> +			server = default_server;
>  	} else {
>  		cpus_and(tmp, cpu_online_map, cpumask);

This means we are doing population counts of two masks, when we really
just care that they are the same.   How about using
cpus_equal(cpu_online_mask, cpu_present_mask)?

>
> @@ -415,7 +418,10 @@ static void xics_set_affinity(unsigned i
>
>  	/* For the moment only implement delivery to all cpus or one cpu */
>  	if (cpus_equal(cpumask, CPU_MASK_ALL)) {
> -		newmask = default_distrib_server;
> +		if (num_online_cpus() == num_present_cpus())
> +			newmask = default_distrib_server;
> +		else
> +			newmask = default_server;
>  	} else {
>  		cpus_and(tmp, cpu_online_map, cpumask);
>  		if (cpus_empty(tmp))

This shows how close these two functions are.  The difference is
what happens when cpus_empty is true -- we default in one and
ignore in the other.

How about adding another arg to get_server, that says to fail
or default for the empty case, with failure being -1?

I'll try to code this up, but it might be a day or two until i get
the time.

milton

  parent reply	other threads:[~2007-04-10 16:59 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-08  4:55 [PATCH] Fix interrupt distribution in ppc970 Mohan Kumar M
2006-12-18  4:37 ` Paul Mackerras
2006-12-18  5:14   ` Mohan Kumar M
2006-12-18 10:57   ` Mohan Kumar M
2007-01-02 11:42     ` [Fastboot] " Mohan Kumar M
2007-01-02 15:07       ` Doug Maxey
2007-03-06 13:57     ` Mohan Kumar M
2007-03-06 14:16       ` Michael Ellerman
2007-03-06 16:55         ` Mohan Kumar M
2007-03-06 17:37           ` Michael Ellerman
2007-03-07  4:53             ` Mohan Kumar M
2007-03-07 10:52               ` Michael Ellerman
2007-04-09  8:57                 ` Mohan Kumar M
2007-04-10  7:06                   ` Michael Ellerman
2007-04-10 12:54                     ` Mohan Kumar M
2007-04-10 16:59                     ` Milton Miller [this message]
2007-04-11  1:16                       ` Michael Ellerman
2007-04-19 11:52                         ` Mohan Kumar M
2007-04-20  5:45                           ` Milton Miller
2007-04-26  9:24                             ` Mohan Kumar M
2007-04-26 14:42                               ` Milton Miller
2007-05-03 14:47                                 ` Mohan Kumar M
2007-05-06  6:52                                   ` Milton Miller
2007-06-04 10:54                                     ` Mohan Kumar M
2007-06-06  9:43                                       ` Milton Miller
2007-06-06 11:31                                         ` Mohan Kumar M
2007-06-11  1:58                                           ` Milton Miller
2007-06-11 18:07                                             ` Mohan Kumar M
2007-06-12 14:51                                             ` Mohan Kumar M
2007-06-15 16:35                                               ` Milton Miller
2007-03-07  6:06         ` [Fastboot] " Vivek Goyal
2007-03-07 10:46           ` Michael Ellerman
2007-03-06 22:05       ` Nathan Lynch
2007-03-07  5:01         ` Mohan Kumar M
2007-03-07  8:52         ` Benjamin Herrenschmidt
2007-03-07  9:10           ` Mohan Kumar M

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=eeb3b19029c753682f476e97dcfaa820@bga.com \
    --to=miltonm@bga$(echo .)com \
    --cc=anton@samba$(echo .)org \
    --cc=fastboot@lists$(echo .)osdl.org \
    --cc=linuxppc-dev@ozlabs$(echo .)org \
    --cc=michael@ellerman$(echo .)id.au \
    --cc=paulus@samba$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox