public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
	linux-arch@vger.kernel.org, Linus Torvalds <torvalds@osdl.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	x86@kernel.org, Peter Zijlstra <peterz@infradead.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mundt <lethal@linux-sh.org>,
	Russell King <linux@arm.linux.org.uk>,
	David Woodhouse <dwmw2@infradead.org>,
	Jesse Barnes <jbarnes@virtuousgeek.org>,
	Yinghai Lu <yinghai@kernel.org>,
	Grant Likely <grant.likely@secretlab.ca>
Subject: Re: [patch 00/47] Sparse irq rework
Date: Sun, 03 Oct 2010 17:49:51 -0700	[thread overview]
Message-ID: <m162xibx3k.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <alpine.LFD.2.00.1010032034420.14550@localhost6.localdomain6> (Thomas Gleixner's message of "Sun, 3 Oct 2010 21:16:47 +0200 (CEST)")

Thomas Gleixner <tglx@linutronix.de> writes:

> On Sun, 3 Oct 2010, Eric W. Biederman wrote:
>> Thomas Gleixner <tglx@linutronix.de> writes:
>> > Rationale:
>> > ----------
>> >
>> > The current sparse_irq allocator has several short comings due to
>> > failures in the design or the lack of it:
>> >
>> >  - Requires iteration over the number of active irqs to find a free slot
>> >    Some architectures have grown their own workarounds for this.
>> >
>> >  - Freeing of irq descriptors is not possible
>> >
>> >  - Racy between create_irq_nr and destroy_irq plugged by horrible
>> >    callbacks
>> >
>> >  - Migration of active irq descriptors is not possible
>> 
>> I believe you have distored the design when aiming for migration
>> of active irq descriptors (which you have not even implemented yet).
>> 
>> How do you plan to remove the radix tree lookup from the irq
>> handling path?
>
> Not at all and it's not even even a requirement to remove the lookup
> for implementing live migration.

It sounds like it is a requirement to *keep* the lookup for supporting
live migration.  *Keeping* the lookup I see as a serious problem.  If we
do this right the only users of the radix tree will be drivers using the
functions in interrupt.h.

>> Those files provide the genirq irq chip implementation especially
>> drivers/pci/msi.c.  Of course they will do what every other irq_chip
>> implementation does to get access to data.  There is an unpleasant
>> difference between which generic irq data field htirq.c uses and msi.c
>> which may be worth cleaning up.  But otherwise I don't see any
>> fundamental problems.
>
> The fundamental problem I hit, was the hack which handed down irq_desc
> to avoid the lookup. If it had been msi_desc in the first place, then
> I would not even need to touch the msi code to cleanup x86.

Just because you intend to rename the irq_desc irq_data...

It isn't a hack for an irq method to look at irq_desc.  At least not
until your irq_data changes go through.  This has nothing to do with
how x86 is structured and everything to do with your irq_data
``cleanup'' which appears to be mostly about code churn, for very little
apparent benefit.

In the current state of the kernel I find it very hard to swallow that
having a genirq client using irq_desc (which is the only way to
implement somethings) is a hack.

Eric

  parent reply	other threads:[~2010-10-04  0:50 UTC|newest]

Thread overview: 111+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-30 23:14 [patch 00/47] Sparse irq rework Thomas Gleixner
2010-09-30 23:14 ` [patch 01/47] x86: Plug memory leak in sparse irq Thomas Gleixner
2010-09-30 23:14 ` [patch 02/47] x86: Hpet: Fix bogus error check in hpet_assign_irq() Thomas Gleixner
2010-09-30 23:14 ` [patch 03/47] genirq: Provide status modifier Thomas Gleixner
2010-09-30 23:14 ` [patch 04/47] arm: Use irq " Thomas Gleixner
2010-09-30 23:14 ` [patch 05/47] genirq-sanitize-irq-data-accessors.patch Thomas Gleixner
2010-09-30 23:15 ` [patch 06/47] genirq: Distangle kernel/irq/handle.c Thomas Gleixner
2010-09-30 23:15 ` [patch 07/47] genirq: Remove early_init_irq_lock_class() Thomas Gleixner
2010-09-30 23:15 ` [patch 08/47] genirq: Move core only inlines to kernel/irq Thomas Gleixner
2010-09-30 23:15 ` [patch 09/47] isdn: hisax: Replace the bogus access to irq stats Thomas Gleixner
2010-09-30 23:15 ` [patch 10/47] genirq: Remove export of kstat_irqs_cpu Thomas Gleixner
2010-09-30 23:15 ` [patch 11/47] genirq: Provide default irq init flags Thomas Gleixner
2010-09-30 23:15 ` [patch 12/47] arm: Use ARCH_IRQ_INIT_FLAGS Thomas Gleixner
2010-09-30 23:15 ` [patch 13/47] powerpc: " Thomas Gleixner
2010-09-30 23:15 ` [patch 14/47] genirq: Implement a sane sparse_irq allocator Thomas Gleixner
2010-10-01  5:28   ` Yinghai Lu
2010-10-01 20:36     ` Thomas Gleixner
2010-09-30 23:15 ` [patch 15/47] genirq: Prepare proc for real sparse irq support Thomas Gleixner
2010-09-30 23:15 ` [patch 16/47] genirq: Implement sane enumeration Thomas Gleixner
2010-10-03 10:55   ` Grant Likely
2010-09-30 23:15 ` [patch 17/47] genirq-update-kerneldoc.patch Thomas Gleixner
2010-09-30 23:16 ` [patch 18/47] genirq: Use sane sparse allocator Thomas Gleixner
2010-09-30 23:16 ` [patch 19/47] genirq: Query arch for number of early descriptors Thomas Gleixner
2010-09-30 23:16 ` [patch 20/47] x86: Remove useless reinitialization of irq descriptors Thomas Gleixner
2010-10-03 15:21   ` Eric W. Biederman
2010-10-03 18:26     ` Thomas Gleixner
2010-09-30 23:16 ` [patch 21/47] x86: Sanitize apb timer interrupt handling Thomas Gleixner
2010-09-30 23:16 ` [patch 22/47] x86: lguest: Convert to new irq chip functions Thomas Gleixner
2010-09-30 23:16 ` [patch 23/47] x86: Cleanup visws interrupt handling Thomas Gleixner
2010-09-30 23:16 ` [patch 24/47] x86: i8259: Convert to new irq_chip functions Thomas Gleixner
2010-09-30 23:16 ` [patch 25/47] x86: Cleanup io_apic Thomas Gleixner
2010-09-30 23:16 ` [patch 26/47] x86: io_apic: Convert startup to new irq_chip function Thomas Gleixner
2010-09-30 23:16 ` [patch 27/47] x86: ioapic: Convert mask " Thomas Gleixner
2010-09-30 23:16 ` [patch 28/47] x86: ioapic/hpet: Convert to new chip functions Thomas Gleixner
2010-09-30 23:16 ` [patch 29/47] pci: Convert msi to new irq_chip functions Thomas Gleixner
2010-10-11 17:09   ` Jesse Barnes
2010-09-30 23:16 ` [patch 30/47] dmar: Convert to new irq chip functions Thomas Gleixner
2010-09-30 23:17 ` [patch 31/47] ht: Convert to new irq_chip functions Thomas Gleixner
2010-09-30 23:17 ` [patch 32/47] x86: ioapic: Clean up the direct access to irq_desc Thomas Gleixner
2010-09-30 23:17 ` [patch 33/47] pci: Cleanup the irq_desc mess in msi Thomas Gleixner
2010-10-11 17:08   ` Jesse Barnes
2010-09-30 23:17 ` [patch 34/47] x86: ioapic: Convert irq affinity to new chip functions Thomas Gleixner
2010-09-30 23:17 ` [patch 35/47] x86: ioapic: Cleanup some more Thomas Gleixner
2010-09-30 23:17 ` [patch 36/47] x86: ioapic: Cleanup sparse irq code Thomas Gleixner
2010-09-30 23:17 ` [patch 37/47] x86: uv: Clean up the direct access to irq_desc Thomas Gleixner
2010-09-30 23:17 ` [patch 38/47] x86: Use sane enumeration Thomas Gleixner
2010-09-30 23:17 ` [patch 39/47] genirq: Remove arch_init_chip_data() Thomas Gleixner
2010-09-30 23:17 ` [patch 40/47] genirq: Sanitize dynamic irq handling Thomas Gleixner
2010-10-01  5:47   ` Yinghai Lu
2010-09-30 23:18 ` [patch 41/47] arm: davinci: Cleanup irq_desc access Thomas Gleixner
2010-09-30 23:18 ` [patch 42/47] genirq: Remove the now unused sparse irq leftovers Thomas Gleixner
2010-09-30 23:18 ` [patch 43/47] x86: xen: Sanitise sparse_irq handling Thomas Gleixner
2010-09-30 23:18 ` [patch 44/47] sh: Sanitize sparse irq Thomas Gleixner
2010-09-30 23:18 ` [patch 45/47] x86: lguest: Use new irq allocator Thomas Gleixner
2010-09-30 23:18 ` [patch 46/47] powerpc: " Thomas Gleixner
2010-10-01  0:42   ` Benjamin Herrenschmidt
2010-10-01 13:07     ` Thomas Gleixner
2010-10-01 20:46       ` Benjamin Herrenschmidt
2010-10-01 21:11         ` Grant Likely
2010-10-01 21:17           ` Benjamin Herrenschmidt
2010-10-03 16:53       ` Eric W. Biederman
2010-10-03 18:34         ` Thomas Gleixner
2010-10-03 20:04           ` Thomas Gleixner
2010-10-03 22:54         ` Benjamin Herrenschmidt
2010-10-04  0:15           ` Eric W. Biederman
2010-10-04  0:37             ` Benjamin Herrenschmidt
2010-10-04 16:46           ` Grant Likely
2010-09-30 23:18 ` [patch 47/47] genirq: Remove the old sparse irq allocator function Thomas Gleixner
2010-10-01  3:32 ` [patch 00/47] Sparse irq rework Linus Torvalds
2010-10-01  5:54 ` Yinghai Lu
2010-10-01 20:35   ` Thomas Gleixner
2010-10-03 11:23 ` Grant Likely
2010-10-03 11:29   ` Russell King - ARM Linux
2010-10-03 11:57     ` Grant Likely
2010-10-03 13:48       ` Thomas Gleixner
2010-10-05 10:22         ` Thomas Gleixner
2010-10-06 22:45           ` Yinghai Lu
2010-10-06 22:52             ` Thomas Gleixner
2010-10-06 23:37               ` Yinghai Lu
2010-10-07  0:16                 ` Yinghai Lu
2010-10-07  4:01                   ` Thomas Gleixner
2010-10-07  4:38                     ` Yinghai Lu
2010-10-08 21:50                       ` Thomas Gleixner
2010-10-08 21:54                         ` Thomas Gleixner
2010-10-09  4:26                           ` Yinghai Lu
2010-10-09  5:44                             ` Yinghai Lu
2010-10-09  6:34                               ` Thomas Gleixner
2010-10-09  7:08                                 ` Yinghai Lu
2010-10-09 12:08                                   ` Thomas Gleixner
2010-10-10  9:32                                   ` Thomas Gleixner
2010-10-10 13:30                                     ` Anca Emanuel
2010-10-11  2:20                                     ` Yinghai Lu
2010-10-11  3:50                                     ` Yinghai Lu
2010-10-11  8:16                                       ` Thomas Gleixner
2010-10-11 11:34                                         ` Benjamin Herrenschmidt
2010-10-11 16:19                                         ` Yinghai Lu
2010-10-12 20:23                               ` [tip:irq/core] x86: Don't setup ioapic irq for sci twice tip-bot for Yinghai Lu
2010-10-09  6:10                             ` [patch 00/47] Sparse irq rework Thomas Gleixner
2010-10-09  7:03                               ` Yinghai Lu
2010-10-09 12:12                                 ` Thomas Gleixner
2010-10-10  2:32                                   ` Yinghai Lu
2010-10-10  5:11                                   ` Yinghai Lu
2010-10-10  8:20                                     ` Thomas Gleixner
2010-10-03 16:41 ` Eric W. Biederman
2010-10-03 19:16   ` Thomas Gleixner
2010-10-03 22:57     ` Benjamin Herrenschmidt
2010-10-04 16:31       ` Grant Likely
2010-10-04  0:49     ` Eric W. Biederman [this message]
2010-10-04  8:05       ` Thomas Gleixner
2010-10-04  1:13     ` Eric W. Biederman
2010-10-04  6:36       ` Ingo Molnar

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=m162xibx3k.fsf@fess.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=dwmw2@infradead.org \
    --cc=grant.likely@secretlab.ca \
    --cc=jbarnes@virtuousgeek.org \
    --cc=lethal@linux-sh.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@osdl.org \
    --cc=x86@kernel.org \
    --cc=yinghai@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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