All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Ingo Molnar <mingo@elte.hu>, Thomas Gleixner <tglx@linutronix.de>,
	Christoph Hellwig <hch@infradead.org>, Andi Kleen <ak@suse.de>
Subject: Re: [PATCH] enable interrupts in user path of page fault.
Date: Thu, 07 Jun 2007 22:06:46 -0400	[thread overview]
Message-ID: <1181268406.10408.26.camel@localhost.localdomain> (raw)
In-Reply-To: <20070607185851.faf36f95.akpm@linux-foundation.org>

On Thu, 2007-06-07 at 18:58 -0700, Andrew Morton wrote:
> On Wed, 06 Jun 2007 23:34:04 -0400

> Interrupts got disabled here because do_page_fault() is an
> interrupt-disabling trap, yes?

Correct.

> 
> The patch looks reasonable to me: a slight reduction in interrupt-off
> latency when really weird things are happening.
> 
> The patch also breaks things, I think: if userspace is running with
> interrupts disabled and tries to access kernel memory it will presently
> whizz through the kernel without ever enabling interrupts.  With this
> change, the kernel will now enable interrupts, which is presumably not what
> the application wanted.

I didn't realize that userspace was allowed to run with interrupts
disabled. If this becomes a problem, we can add the same check that's
above where do_page_fault does enable interrupts, but is skipped because
the faulting address was above PAGE_OFFSET.


ie. (i386)
        if (regs->eflags & (X86_EFLAGS_IF|VM_MASK))
                local_irq_enable();


> 
> However it's surely already the case that most pagefaults will go and
> enable interrupts on this process anyway, so no big loss there.  I'd expect
> the kernel to spit piles of might_sleep() warnings when all this happens, so
> maybe it just doesn't happen for some reason.

Actually it does on the RT kernel. Hence why I found it ;-)

-- Steve



  reply	other threads:[~2007-06-08  2:07 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-06-07  3:34 [PATCH] enable interrupts in user path of page fault Steven Rostedt
2007-06-08  1:58 ` Andrew Morton
2007-06-08  2:06   ` Steven Rostedt [this message]
2007-06-08  2:29     ` Linus Torvalds
2007-06-08  2:28   ` Linus Torvalds

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=1181268406.10408.26.camel@localhost.localdomain \
    --to=rostedt@goodmis.org \
    --cc=ak@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.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.