lkml.org 
[lkml]   [2012]   [Feb]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 0/2] [GIT PULL][v3.3] x86: Fix up CFI for the nested NMI

* Steven Rostedt <rostedt@goodmis.org> wrote:

> Hi Ingo,
>
> On Mon, 2012-02-27 at 08:39 +0100, Ingo Molnar wrote:
> > * Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > >
> > > Ingo,
> > >
> > > Jan posted a patch that fixes the CFI annotations. I recommend getting
> > > this into 3.3 as this is new code and it would be nice to have CFI
> > > correct. It also does a little simplification of it as well.
> > >
> > > The second patch is comment changes only (very low impact on messing
> > > anything up). I realized that the comments had some references to
> > > previous approaches that I tried, and I fixed them to reflect what
> > > the final result was. I also added some more comments to describe
> > > the code a bit better.
> > >
> > > Thanks,
> > >
> > > -- Steve
> > >
> > > Please pull the latest tip/x86/urgent tree, which can be found at:
> > >
> > > git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
> > > tip/x86/urgent
> > >
> > > Head SHA1: 79fb4ad63e8266ffac1f69bbb45a6f86570493e7
> > >
> > >
> > > Jan Beulich (1):
> > > x86-64: Fix CFI annotations for NMI nesting code
> > >
> > > Steven Rostedt (1):
> > > x86: Fix the NMI nesting comments
> > >
> > > ----
> > > arch/x86/kernel/entry_64.S | 64 ++++++++++++++++++++++++--------------------
> > > 1 files changed, 35 insertions(+), 29 deletions(-)
> >
> > I don't think we want a 30+ lines diffstat to this rather
>
> Some of that was just movement of code.

That's why I said 30+ lines, not 60.

> > non-trivial NMI codepath - and it changes real instructions,
> > not just the CFI annotations.
>
> The changes to the real code made the CFI code easier to fix.
> But if you are nervous about the code change (which actually
> simplifies the code), I can ask Jan (Cc'd) to break out the
> patch into two changes if possible.
>
> I'm not sure how the CFI can handle the current trampoline,
> but perhaps we can just fix the main part of the code and
> leave the trampoline part broken? Then we can add the rest of
> the CFI changes and the movement of the trampoline back into
> the function for the next release.
>
> >
> > Also, the 'update comments' commit does not belong into
> > x86/urgent either.
>
> Hmm, I didn't know that fixing comments was for a merge window
> only. [...]

We try to avoid them for later -rc's, and we are now in -rc5
territory already.

> [...] Some of the comments are currently wrong and I didn't
> think we would want those in a main release. While reading the
> code again I realized that I could also add more comments to
> make it easier to understand. I would think that comments
> would be fine for the -rc releases because they have almost no
> chance of introducing bugs.

By that argument we could be doing mechanic API renames in later
-rc's as well.

In general we don't want "noise" around the really relevant
changes to make them *really* obvious regression fixes - even if
this noise is obviously correct code.

This is also code that *everyone* uses so if it breaks then
everyone suffers (and seeing people suffer, even hypothetically,
makes me sad), so the maintenance filter is quite a bit
stricter. I wouldn't worry nearly as much about
drivers/some/obscure/piece/of/hw.c.

> > So either you do an obviously trivial patch that only adds
> > CFI annotations and nothing else, or I can pull these bits
> > into tip:x86/debug, for a v3.4 merge.
>
> I'm fine with waiting for v3.4 before these changes get in.

Jan, is that fine with you?

Thanks,

Ingo


\
 
 \ /
  Last update: 2012-02-28 10:09    [W:0.999 / U:0.196 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site