All of lore.kernel.org
 help / color / mirror / Atom feed
From: Keir Fraser <keir@xen.org>
To: Jan Beulich <JBeulich@suse.com>,
	Sander Eikelenboom <linux@eikelenboom.it>
Cc: xen-devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH] x86/HPET: mask interrupt while changing affinity
Date: Wed, 20 Mar 2013 14:19:37 +0000	[thread overview]
Message-ID: <CD6F75F9.5EE55%keir@xen.org> (raw)
In-Reply-To: <5149CA0202000078000C72BD@nat28.tlf.novell.com>

On 20/03/2013 13:38, "Jan Beulich" <JBeulich@suse.com> wrote:

>>>> On 20.03.13 at 12:55, Sander Eikelenboom <linux@eikelenboom.it> wrote:
>> Close but not entirely ;)
> 
> Close to not crashing, maybe, but whether this really helps with your
> problem is still entirely unclear.
> 
>> See attached serial log
> 
> Okay, I wasn't even aware of that assertion in _spin_lock_irq().
> 
> Keir, do you really think this is necessary?

You are more cunning than some others. ;) I'm pretty confident those
spinlock assertions save us from real bugs. Also I'd rather have blindingly
obvious code here than slightly faster code.

 -- Keir

> In the prior patch
> version, handle_hpet_broadcast() had a flow like this
> 
>     spin_lock_irqsave();
>     ...
>     spin_unlock_irqrestore();
>     ...
>     if ( next_event != STIME_MAX )
>     {
>         spin_lock_irq();
>         ...
>         spin_unlock_irqrestore();
>     }
> 
> avoiding the saving of the flags in the second lock acquire. Said
> assertion makes it impossible to do this.
> 
> Sander, in any case, attached a fixed version of the patch (I had
> to guess which of the two spin_lock_irq() calls it was, as the log
> was incomplete in that the stack trace got dropped, but am pretty
> positive that it was the one in handle_hpet_broadcast()).
> 
> Jan
> 

  parent reply	other threads:[~2013-03-20 14:19 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-18 11:12 [PATCH] x86/HPET: mask interrupt while changing affinity Jan Beulich
2013-03-18 12:09 ` Keir Fraser
2013-03-19 15:53 ` Sander Eikelenboom
2013-03-19 16:00   ` Jan Beulich
2013-03-19 22:48     ` Sander Eikelenboom
2013-03-19 23:24       ` Andrew Cooper
2013-03-20  8:31         ` Jan Beulich
2013-03-20  8:22       ` Jan Beulich
2013-03-20 10:13         ` Sander Eikelenboom
2013-03-20 11:02           ` Jan Beulich
2013-03-20 11:55             ` Sander Eikelenboom
2013-03-20 13:38               ` Jan Beulich
2013-03-20 13:46                 ` Andrew Cooper
2013-03-20 14:19                 ` Keir Fraser [this message]
2013-03-20 14:35                 ` Sander Eikelenboom
2013-03-20 14:41                   ` Jan Beulich
2013-03-20 14:44                     ` Sander Eikelenboom

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=CD6F75F9.5EE55%keir@xen.org \
    --to=keir@xen.org \
    --cc=JBeulich@suse.com \
    --cc=linux@eikelenboom.it \
    --cc=xen-devel@lists.xen.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.