[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <m17hp4wiz0.fsf@fess.ebiederm.org>
Date: Mon, 22 Mar 2010 07:04:35 -0700
From: ebiederm@...ssion.com (Eric W. Biederman)
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Yinghai Lu <yinghai@...nel.org>, Ingo Molnar <mingo@...e.hu>,
"H. Peter Anvin" <hpa@...or.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Jesse Barnes <jbarnes@...tuousgeek.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 05/10] x86: use vector_desc instead of vector_irq
Thomas Gleixner <tglx@...utronix.de> writes:
> On Sun, 21 Mar 2010, Yinghai Lu wrote:
>
>> Eric pointed out that radix tree version of irq_to_desc will magnify delay on
>> the path of handle_irq.
>>
>> use vector_desc to reduce the calling of irq_to_desc.
>>
>> next step: need to change all ack, mask, umask, eoi for all irq_chip to take irq_desc
>
> That's not relevant for this change.
>
>>
>> -typedef int vector_irq_t[NR_VECTORS];
>> -DECLARE_PER_CPU(vector_irq_t, vector_irq);
>> -extern void setup_vector_irq(int cpu);
>> +typedef struct irq_desc *vector_desc_t[NR_VECTORS];
>
> Why do we need that typedef ? Please use plain struct irq_desc *
Well at least originally DECLARE_PER_CPU chocked when given a complex
type. Does:
DECLARE_PER_CPU(struct irq_desc *[NR_VECTORS], vector_desc);
work?
>> +DECLARE_PER_CPU(vector_desc_t, vector_desc);
>> +extern void setup_vector_desc(int cpu);
> ...
>> void destroy_irq(unsigned int irq)
>> {
>> unsigned long flags;
>> + struct irq_desc *desc;
>> + struct irq_cfg *cfg;
>>
>> dynamic_irq_cleanup_keep_chip_data(irq);
>>
>> free_irte(irq);
>> raw_spin_lock_irqsave(&vector_lock, flags);
>> - __clear_irq_vector(irq, get_irq_chip_data(irq));
>> + desc = irq_to_desc(irq);
>> + cfg = desc->chip_data;
>> + __clear_irq_vector(desc, cfg);
>
> __clear_irq_vector(desc, desc->chip_data);
>
> should be sufficient, right ?
You want to deliberately loose a modicum of type safety?
>
>> raw_spin_unlock_irqrestore(&vector_lock, flags);
>> }
>>
>> @@ -3377,6 +3376,7 @@ void destroy_irq(unsigned int irq)
>> static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
>> struct msi_msg *msg, u8 hpet_id)
>> {
>> + struct irq_desc *desc;
>> struct irq_cfg *cfg;
>> int err;
>> unsigned dest;
>> @@ -3384,8 +3384,9 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
>> if (disable_apic)
>> return -ENXIO;
>>
>> - cfg = irq_cfg(irq);
>> - err = assign_irq_vector(irq, cfg, apic->target_cpus());
>> + desc = irq_to_desc(irq);
>> + cfg = desc->chip_data;
>> + err = assign_irq_vector(desc, cfg, apic->target_cpus());
>
> Ditto
>
>> if (err)
>> return err;
>>
>> @@ -3876,14 +3877,16 @@ static struct irq_chip ht_irq_chip = {
>>
>> int arch_setup_ht_irq(unsigned int irq, struct pci_dev *dev)
>> {
>> + struct irq_desc *desc;
>> struct irq_cfg *cfg;
>> int err;
>>
>> if (disable_apic)
>> return -ENXIO;
>>
>> - cfg = irq_cfg(irq);
>> - err = assign_irq_vector(irq, cfg, apic->target_cpus());
>> + desc = irq_to_desc(irq);
>> + cfg = desc->chip_data;
>> + err = assign_irq_vector(desc, cfg, apic->target_cpus());
>
> Ditto
>
>> if (!err) {
>> struct ht_irq_msg msg;
>> unsigned dest;
>> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
>> index 91fd0c7..f71625c 100644
>> --- a/arch/x86/kernel/irq.c
>> +++ b/arch/x86/kernel/irq.c
>> @@ -229,19 +229,19 @@ unsigned int __irq_entry do_IRQ(struct pt_regs *regs)
>>
>> /* high bit used in ret_from_ code */
>> unsigned vector = ~regs->orig_ax;
>> - unsigned irq;
>> + struct irq_desc *desc;
>>
>> exit_idle();
>> irq_enter();
>>
>> - irq = __get_cpu_var(vector_irq)[vector];
>> + desc = __get_cpu_var(vector_desc)[vector];
>>
>> - if (!handle_irq(irq, regs)) {
>> + if (!handle_irq(desc, regs)) {
>> ack_APIC_irq();
>>
>> if (printk_ratelimit())
>> - pr_emerg("%s: %d.%d No irq handler for vector (irq %d)\n",
>> - __func__, smp_processor_id(), vector, irq);
>> + pr_emerg("%s: %d.%d No irq handler for vector\n",
>
> That printk is confusing. It's not lacking an irq handler. The
> vector is simply not assigned.
Long evolution. Do you have a suggestion of better wording?
Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists