xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@novell.com>
To: Keir Fraser <keir@xen.org>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>
Subject: Re: [PATCH] reduce side effects of handling '*' debug key
Date: Wed, 15 Dec 2010 10:59:05 +0000	[thread overview]
Message-ID: <4D08AD890200007800028153@vpn.id2.novell.com> (raw)
In-Reply-To: <C92E4CE3.28FF6%keir@xen.org>

>>> On 15.12.10 at 11:45, Keir Fraser <keir@xen.org> wrote:
> On 14/12/2010 14:46, "Jan Beulich" <JBeulich@novell.com> wrote:
> 
>> NMI watchdog should be suppressed, and softirqs should be handled at
>> least in the non-IRQ handler portion (they obviously must not be
>> processed in IRQ context).
> 
> Why do you add calls to watchdog_disable() --- The process_softirqs() call
> you add is intended to be sufficient to keep timers being handled in a
> timely fashion, is it not?
> 
> I can see why you remove console_start_log_everything(), since that is
> handled from handle_keypress(), but why do you add calls to
> console_start_sync()?

I didn't actually pay attention to the duplication with
handle_keypress(). Instead, I was simply following what the 'd'
handler does, noting that calling console_start_sync() makes
calling console_start_log_everything() pointless.

> Personally I think that if you have justifiable reason to place the console
> in synchronous mode, and protect yourself from the NMI watchdog, these calls
> should be added to handle_keypress() for all keyhandlers to enjoy.

Protection from the NMI watchdog shouldn't be needed for
handlers not taking long, so I'm not certain it'd be useful to add
generally.

Similarly, for handlers not printing much, forcing the console
into synchronous mode doesn't seem appropriate to me.
Basically, each handler should know for itself (and
handle_keypress() should, as it does, only do what all
handler want in common).

Jan

      reply	other threads:[~2010-12-15 10:59 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-14 14:46 [PATCH] reduce side effects of handling '*' debug key Jan Beulich
2010-12-15 10:45 ` Keir Fraser
2010-12-15 10:59   ` Jan Beulich [this message]

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=4D08AD890200007800028153@vpn.id2.novell.com \
    --to=jbeulich@novell.com \
    --cc=keir@xen.org \
    --cc=xen-devel@lists.xensource.com \
    /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).