public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: mingo@elte.hu, hpa@zytor.com, tglx@linutronix.de,
	akpm@linux-foundation.org, arjan@linux.intel.com,
	andi@firstfloor.org, jbarnes@virtuousgeek.org, steiner@sgi.com,
	linux-kernel@vger.kernel.org
Subject: Re: [patch 00/26] x64, x2apic/intr-remap: Interrupt-remapping and x2apic support
Date: Thu, 10 Jul 2008 13:05:50 -0700	[thread overview]
Message-ID: <m1prplv71d.fsf@frodo.ebiederm.org> (raw)
In-Reply-To: <20080710181634.764954000@linux-os.sc.intel.com> (Suresh Siddha's message of "Thu, 10 Jul 2008 11:16:34 -0700")


At a quick skim nothing looks really bad, nothing looks really bad,
but you are dealing in an area of code that could be made much nicer
and if we are going to support a noticeably different style of irq
management we need to get in some of those pending cleanups so the
code does not fall down under it's own wait.

Suresh Siddha <suresh.b.siddha@intel.com> writes:

> irq migration in the presence of interrupt-remapping is done from the
> process-context as opposed to interrupt-context. Interrupt-remapping
> infrastrucutre allows us to do this migration in a simple fashion (atleast for
> edge triggered interrupts).

Unless I have misread things this irq migration remains racy, as I did not
see any instructions that would guarantee that in flight irqs were flushed
to the cpus local apics before we cleaned up the destination.

You are sizing an array as NR_IRQS this is something there should be sufficient
existing infrastructure to avoid.  Arrays sized by NR_IRQS is a significant
problem both for scaling the system up and down so ultimately we need to kill
this.  For now we should not introduce any new arrays.

A lot of your code is generic, and some of it is for just x86_64.  Since the
cpus are capable of running in 32bit mode.  We really need to implement x86_32
and x86_64 support in the same code base.  Which I believe means factoring out
pieces of io_apic_N.c into things such as msi.c that can be shared between the
two architectures.

Eric


  parent reply	other threads:[~2008-07-10 20:12 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-10 18:16 [patch 00/26] x64, x2apic/intr-remap: Interrupt-remapping and x2apic support Suresh Siddha
2008-07-10 18:16 ` [patch 01/26] x64, x2apic/intr-remap: Intel vt-d, IOMMU code reorganization Suresh Siddha
2008-07-10 18:16 ` [patch 02/26] x64, x2apic/intr-remap: fix the need for sequential array allocation of iommus Suresh Siddha
2008-07-10 18:16 ` [patch 03/26] x64, x2apic/intr-remap: code re-structuring, to be used by both DMA and Interrupt remapping Suresh Siddha
2008-07-10 18:16 ` [patch 04/26] x64, x2apic/intr-remap: use CONFIG_DMAR for DMA-remapping specific code Suresh Siddha
2008-07-10 18:16 ` [patch 05/26] x64, x2apic/intr-remap: Fix the need for RMRR in the DMA-remapping detection Suresh Siddha
2008-07-10 18:16 ` [patch 06/26] x64, x2apic/intr-remap: parse ioapic scope under vt-d structures Suresh Siddha
2008-07-10 18:16 ` [patch 07/26] x64, x2apic/intr-remap: move IOMMU_WAIT_OP() macro to intel-iommu.h Suresh Siddha
2008-07-10 18:16 ` [patch 08/26] x64, x2apic/intr-remap: Queued invalidation infrastructure (part of VT-d) Suresh Siddha
2008-07-10 18:16 ` [patch 09/26] x64, x2apic/intr-remap: Interrupt remapping infrastructure Suresh Siddha
2008-07-10 18:16 ` [patch 10/26] x64, x2apic/intr-remap: routines managing Interrupt remapping table entries Suresh Siddha
2008-07-10 18:16 ` [patch 11/26] x64, x2apic/intr-remap: generic irq migration support from process context Suresh Siddha
2008-07-10 23:08   ` Eric W. Biederman
2008-07-11  5:41     ` Suresh Siddha
2008-07-11  9:19       ` Eric W. Biederman
2008-07-10 18:16 ` [patch 12/26] x64, x2apic/intr-remap: 8259 specific mask/unmask routines Suresh Siddha
2008-07-10 18:16 ` [patch 13/26] x64, x2apic/intr-remap: ioapic routines which deal with initial io-apic RTE setup Suresh Siddha
2008-07-10 18:16 ` [patch 14/26] x64, x2apic/intr-remap: introduce read_apic_id() to genapic routines Suresh Siddha
2008-07-10 18:16 ` [patch 15/26] x64, x2apic/intr-remap: basic apic ops support Suresh Siddha
2008-07-10 18:16 ` [patch 16/26] x64, x2apic/intr-remap: cpuid bits for x2apic feature Suresh Siddha
2008-07-10 18:16 ` [patch 17/26] x64, x2apic/intr-remap: disable DMA-remapping if Interrupt-remapping is detected (temporary quirk) Suresh Siddha
2008-07-10 18:16 ` [patch 18/26] x64, x2apic/intr-remap: x2apic ops for x2apic mode support Suresh Siddha
2008-07-10 18:16 ` [patch 19/26] x64, x2apic/intr-remap: introcude self IPI to genapic routines Suresh Siddha
2008-07-10 23:34   ` Eric W. Biederman
2008-07-11  2:29     ` Mike Travis
2008-07-11  3:50       ` Eric W. Biederman
2008-07-11 13:55         ` Mike Travis
2008-07-10 18:16 ` [patch 20/26] x64, x2apic/intr-remap: x2apic cluster mode support Suresh Siddha
2008-07-10 18:16 ` [patch 21/26] x64, x2apic/intr-remap: setup init_apic_ldr for UV Suresh Siddha
2008-07-11  0:14   ` Andrew Morton
2008-07-11  1:56     ` Suresh Siddha
2008-07-10 18:16 ` [patch 22/26] x64, x2apic/intr-remap: IO-APIC support for interrupt-remapping Suresh Siddha
2008-07-10 18:16 ` [patch 23/26] x64, x2apic/intr-remap: MSI and MSI-X support for interrupt remapping infrastructure Suresh Siddha
2008-07-11  1:22   ` Eric W. Biederman
2008-07-11  6:07     ` Suresh Siddha
2008-07-11  8:59       ` Eric W. Biederman
2008-07-11 23:07         ` Suresh Siddha
2008-07-11 23:50           ` Eric W. Biederman
2008-07-10 18:16 ` [patch 24/26] x64, x2apic/intr-remap: add x2apic support, including enabling interrupt-remapping Suresh Siddha
2008-07-10 18:16 ` [patch 25/26] x64, x2apic/intr-remap: support for x2apic physical mode support Suresh Siddha
2008-07-10 18:17 ` [patch 26/26] x64, x2apic/intr-remap: introduce CONFIG_INTR_REMAP Suresh Siddha
2008-07-10 23:29   ` Eric W. Biederman
2008-07-10 23:37     ` Yong Wang
2008-07-11  1:50       ` Suresh Siddha
2008-07-11  1:53       ` Eric W. Biederman
2008-07-10 19:53 ` [patch 00/26] x64, x2apic/intr-remap: Interrupt-remapping and x2apic support Ingo Molnar
2008-07-10 20:22   ` Suresh Siddha
2008-07-10 21:56   ` Suresh Siddha
2008-07-11 10:28     ` Ingo Molnar
2008-07-11 20:09       ` Ingo Molnar
2008-07-11 20:31         ` Suresh Siddha
2008-07-11 20:42           ` Yinghai Lu
2008-07-11 20:45             ` Ingo Molnar
2008-07-11 21:24               ` Suresh Siddha
2008-07-11 22:02                 ` Yinghai Lu
2008-07-12  3:16                   ` Yinghai Lu
2008-07-12  3:52                     ` Eric W. Biederman
2008-07-12  6:17                       ` Yinghai Lu
2008-07-12  7:02                         ` Eric W. Biederman
2008-07-12  7:49                           ` Yinghai Lu
2008-07-12  8:11                             ` Eric W. Biederman
2008-07-12  8:37                               ` Yinghai Lu
2008-07-12  9:46                                 ` Eric W. Biederman
2008-07-13  1:02                                 ` Suresh Siddha
2008-07-13  1:01                             ` Suresh Siddha
2008-07-13  1:32                           ` Suresh Siddha
2008-07-13  1:00                         ` Suresh Siddha
2008-07-13  0:55                     ` Suresh Siddha
2008-07-12  5:37                   ` Ingo Molnar
2008-07-12  6:06                     ` Yinghai Lu
2008-07-12  6:45                       ` Ingo Molnar
2008-07-11 20:49           ` Ingo Molnar
2008-07-16 14:37   ` Yong Wang
2008-07-16 14:53     ` Ingo Molnar
2008-07-22 20:49   ` Andrew Morton
2008-07-22 21:00     ` Mike Travis
2008-07-22 21:14       ` Andrew Morton
2008-07-24  5:03       ` Ingo Molnar
2008-07-10 20:05 ` Eric W. Biederman [this message]
2008-07-10 20:18   ` Ingo Molnar
2008-07-10 21:07     ` Eric W. Biederman
2008-07-10 21:15   ` Suresh Siddha
2008-07-10 22:52     ` Eric W. Biederman
2008-07-11  2:35       ` Suresh Siddha
2008-07-11  3:15         ` Eric W. Biederman
2008-07-10 22:09   ` Arjan van de Ven
2008-07-10 22:54     ` Eric W. Biederman

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=m1prplv71d.fsf@frodo.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=arjan@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jbarnes@virtuousgeek.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=steiner@sgi.com \
    --cc=suresh.b.siddha@intel.com \
    --cc=tglx@linutronix.de \
    /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