From: Milton Miller <miltonm@bga•com>
To: mohan@in•ibm.com
Cc: kexec@lists•infradead.org, fastboot@lists•osdl.org,
ppcdev <linuxppc-dev@ozlabs•org>,
Paul Mackerras <paulus@samba•org>,
Anton Blanchard <anton@samba•org>
Subject: Re: [PATCH] Fix interrupt distribution in ppc970
Date: Sun, 6 May 2007 01:52:44 -0500 [thread overview]
Message-ID: <ca045e7e6fde9345b3461214777d8102@bga.com> (raw)
In-Reply-To: <20070503144721.GA28460@in.ibm.com>
On May 3, 2007, at 9:47 AM, Mohan Kumar M wrote:
> On Thu, Apr 26, 2007 at 09:42:50AM -0500, Milton Miller wrote:
>> Yes. The whole point of
>>> -static int get_irq_server(unsigned int virq)
>>> +static int get_irq_server(unsigned int virq, unsigned int
>>> strict_check)
>> was to factor out the common code in this function.
>
> Milton,
>
> How about this patch?
Getting closer, still not right.
>
> Index: linux-2.6.21.1/arch/powerpc/platforms/pseries/xics.c
> ===================================================================
> --- linux-2.6.21.1.orig/arch/powerpc/platforms/pseries/xics.c
> +++ linux-2.6.21.1/arch/powerpc/platforms/pseries/xics.c
> @@ -156,9 +156,9 @@ static inline void lpar_qirr_info(int n_
>
>
> #ifdef CONFIG_SMP
> -static int get_irq_server(unsigned int virq)
> +static int get_irq_server(unsigned int virq, unsigned int
> strict_check)
> {
> - unsigned int server;
> + int server;
> /* For the moment only implement delivery to all cpus or one cpu */
> cpumask_t cpumask = irq_desc[virq].affinity;
> cpumask_t tmp = CPU_MASK_NONE;
> @@ -166,22 +166,28 @@ static int get_irq_server(unsigned int v
> if (!distribute_irqs)
> return default_server;
>
> - if (cpus_equal(cpumask, CPU_MASK_ALL)) {
> - server = default_distrib_server;
> - } else {
> + if (!cpus_equal(cpumask, CPU_MASK_ALL)) {
> cpus_and(tmp, cpu_online_map, cpumask);
>
> - if (cpus_empty(tmp))
> - server = default_distrib_server;
> + server = first_cpu(tmp);
> +
> + if (server < NR_CPUS)
> + return get_hard_smp_processor_id(server);
> + else {
> + if (strict_check)
> + return -1;
> + else
> + return default_distrib_server;
> + }
> + } else {
Take out the above 4 lines, so that the return always has cpu_online
vs cpu_present factored in, by falling through from specific mask to
default server selection.
> + if (cpus_equal(cpu_online_map, cpu_present_map))
> + return default_distrib_server;
> else
> - server = get_hard_smp_processor_id(first_cpu(tmp));
> + return default_server;
> }
[This matching brace will go and indent will change.]
> -
> - return server;
> -
> }
...
> @@ -398,8 +404,7 @@ static void xics_set_affinity(unsigned i
> unsigned int irq;
> int status;
> int xics_status[2];
> - unsigned long newmask;
> - cpumask_t tmp = CPU_MASK_NONE;
> + int irq_server;
>
> irq = (unsigned int)irq_map[virq].hwirq;
> if (irq == XICS_IPI || irq == XICS_IRQ_SPURIOUS)
> @@ -413,18 +418,28 @@ static void xics_set_affinity(unsigned i
> return;
> }
>
> - /* For the moment only implement delivery to all cpus or one cpu */
> - if (cpus_equal(cpumask, CPU_MASK_ALL)) {
> - newmask = default_distrib_server;
> - } else {
> - cpus_and(tmp, cpu_online_map, cpumask);
> - if (cpus_empty(tmp))
> + /* Get current irq_server for the given irq */
> + irq_server = get_irq_server(irq, 1);
> + if (irq_server == -1) {
> + printk(KERN_ERR "xics_set_affinity: Invalid cpumask\n");
WARNING or NOTICE would be fine. The interrupt number should be
printed.
and maybe "No online cpus in <cpumask> for irq %d" would be better (I
think there is a print cpumask helper).
> + return;
> + }
> +
> + /* For the moment only implement delivery to all cpus or one cpu.
> + * Compare the irq_server with the new cpumask. If the irq_server
> + * is specified in cpumask, do the required rtas_call, otherwise
> + * return by printing an error message
> + */
> + if (!cpus_equal(cpumask, CPU_MASK_ALL)) {
> + if (!cpu_isset(irq_server, cpumask)) {
> + printk(KERN_ERR "xics_set_affinity: Invalid "
> + "cpumask\n");
> return;
I don't understand what you are trying to do here. We already
chose the mask, and printed an error if we got -1 due to strict.
I think this can just be dropped?
> - newmask = get_hard_smp_processor_id(first_cpu(tmp));
> + }
> }
>
> status = rtas_call(ibm_set_xive, 3, 1, NULL,
> - irq, newmask, xics_status[1]);
> + irq, irq_server, xics_status[1]);
>
> if (status) {
> printk(KERN_ERR "xics_set_affinity: irq=%u ibm,set-xive "
milton
next prev parent reply other threads:[~2007-05-06 6:53 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
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 [this message]
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=ca045e7e6fde9345b3461214777d8102@bga.com \
--to=miltonm@bga$(echo .)com \
--cc=anton@samba$(echo .)org \
--cc=fastboot@lists$(echo .)osdl.org \
--cc=kexec@lists$(echo .)infradead.org \
--cc=linuxppc-dev@ozlabs$(echo .)org \
--cc=mohan@in$(echo .)ibm.com \
--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