xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@novell.com>
To: Daniel Stodden <daniel.stodden@citrix.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>,
	"Xen-devel@lists.xensource.com" <Xen-devel@lists.xensource.com>,
	Tom Kopec <tek@acm.org>
Subject: Re: [GIT PULL] Fix lost interrupt race in Xen event channels
Date: Mon, 30 Aug 2010 09:03:39 +0100	[thread overview]
Message-ID: <4C7B81FB0200007800012C16@vpn.id2.novell.com> (raw)
In-Reply-To: <1282941781.26797.386.camel@agari.van.xensource.com>

>>> On 27.08.10 at 22:43, Daniel Stodden <daniel.stodden@citrix.com> wrote:
> So let's come up with a new theory. Right now I'm still
> looking at xen/next. Correct me if I'm mistaken:
> 
> mask_ack_pirq will:
>  1. chip->mask
>  2. chip->ack
> 
> Where chip->ack will:
>  1. move_native_irq
>  2. clear_evtchn.
> 
> Now if you look into move_native_irq, it will:
>  1. chip->mask (gratuitous)
>  2. move
>  3. chip->unmask (aiiiiiie).
> 
> That explains why edge_irq still fixed the problem.

This totals to it having been wrong to break up -mask() and ->ack() -
when using the combined ->mask_ack() (like in XCP etc) it would have
been pretty obvious that move_native_irq() cannot go between
mask() and ack().

For us, using fasteoi, move_native_irq() sits in ->eoi(), before
un-masking. One could, as Jeremy suggests, call move_masked_irq()
here, but I didn't want to duplicate the IRQ_DISABLED check done
in move_native_irq(), mainly to not depend on following potential
future changes (additions) to the set of conditions checked there.

Helpful would be if the function returned whether it actually went
through the mask/unmask pair, as I'm not sure the double
unmasking really is a good idea, especially in the PIRQ case (for
the moment I'm considering putting an already-unmasked check
into both ->eoi() handlers).

Jan

  parent reply	other threads:[~2010-08-30  8:03 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-24 21:35 [GIT PULL] Fix lost interrupt race in Xen event channels Jeremy Fitzhardinge
2010-08-25  7:52 ` [Xen-devel] " Jan Beulich
2010-08-25 10:04   ` Daniel Stodden
2010-08-25 11:24     ` Jan Beulich
2010-08-25 17:54   ` Jeremy Fitzhardinge
2010-08-26  6:46     ` Jan Beulich
2010-08-26 16:32       ` Jeremy Fitzhardinge
2010-08-27  8:56         ` Jan Beulich
2010-08-27 20:43           ` Daniel Stodden
2010-08-27 21:49             ` Daniel Stodden
2010-08-27 23:43             ` Jeremy Fitzhardinge
2010-08-30  8:03             ` Jan Beulich [this message]
2010-08-30  8:43               ` Jan Beulich
2010-08-30  8:48                 ` Keir Fraser
2010-08-30  9:06                   ` Jan Beulich
2010-08-30  9:15                     ` Keir Fraser
2010-08-30  9:22                       ` Jan Beulich
2010-08-30 16:36               ` Jeremy Fitzhardinge
2010-08-31  6:38                 ` Jan Beulich
2010-09-03 18:46   ` Using handle_fasteoi_irq for pirqs Jeremy Fitzhardinge
2010-09-06  7:58     ` Jan Beulich
2010-09-07  1:53       ` Jeremy Fitzhardinge
2010-09-07  6:58         ` Jan Beulich
2010-09-07  8:02           ` Jeremy Fitzhardinge
2010-09-07  8:58             ` Jan Beulich

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=4C7B81FB0200007800012C16@vpn.id2.novell.com \
    --to=jbeulich@novell.com \
    --cc=Xen-devel@lists.xensource.com \
    --cc=daniel.stodden@citrix.com \
    --cc=jeremy@goop.org \
    --cc=tek@acm.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;
as well as URLs for NNTP newsgroup(s).