public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Jan Beulich" <jbeulich@novell.com>
To: "Chuck Ebbert" <76306.1226@compuserve.com>
Cc: "Andi Kleen" <ak@suse.de>, <linux-kernel@vger.kernel.org>
Subject: Re: [patch] i386: annotate the rest of entry.s::nmi
Date: Fri, 11 Aug 2006 09:10:05 +0200	[thread overview]
Message-ID: <44DC496D.76E4.0078.0@novell.com> (raw)
In-Reply-To: <200608101343_MC3-1-C7B1-9E81@compuserve.com>

>> >> The point is that the push-es in FIX_STACK() aren't annotated, so
>> >> things won't be correct at those points anyway.
>> >
>> >I have a patch here that adds that, but it won't compile
>> >because that part of the NMI handler is un-annotated:
>> 
>> But you didn't clarify why you need this piece of code annotated...
>
>Uh, which one didn't I clarify?
>
>FIX_STACK() is already invoked from debug(), which is annotated, but
>FIX_STACK() isn't.  And that messes with the stack, so for a few
>instructions the annotations are all wrong.
>
>When I annotated FIX_STACK(), I found entry.S wouldn't compile
because
>nmi() included FIX_STACK() but was completely missing annotations
>in that piece. So I added them so FIX_STACK()'s annotations would
>compile...

Ah, okay, this means the original sequence of additions was the reverse
of
how I got to see these patches. I understand now, but am still
uncertain
about the need to annotate FIX_STACK() - especially since you use
.cfi_undefined, meaning the return point cannot be established anyway.
If at all I'd annotate the initial pushes with either just the normal
CFI_ADJUST_CFA_OFFSET, and the final one with one setting back the
CFA base to the now adjusted frame. That way, until the pushes are
complete the old frame will be used for determining the call origin,
and
once complete the (full) new state will be used.
Or annotate them so that the new values take effect immediately with
each push, but clearly without any CFI_UNDEFINED. That way, the
frame will be slightly inconsistent in between, which could be of
concern
once we also properly annotate the segment register spills/restores.

>Should I send a combined patch, leave the two patches separate, or
just
>drop it?

Either way, but if you leave them separate you should always send them
as pair, to make the intentions clear.

Jan

  reply	other threads:[~2006-08-11  7:09 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-08-10 17:39 [patch] i386: annotate the rest of entry.s::nmi Chuck Ebbert
2006-08-11  7:10 ` Jan Beulich [this message]
  -- strict thread matches above, loose matches on Subject: below --
2006-08-11 23:13 Chuck Ebbert
2006-08-14  6:55 ` Jan Beulich
2006-08-10 12:48 Chuck Ebbert
2006-08-10 12:58 ` Andi Kleen
2006-08-10 13:39 ` Jan Beulich
2006-08-10  4:59 Chuck Ebbert
2006-08-10  5:20 ` Andi Kleen
2006-08-10  8:23 ` Jan Beulich
2006-08-10  8:26   ` Andi Kleen
2006-08-10  8:34     ` Jan Beulich
2006-08-11 17:46   ` Stas Sergeev
2006-08-14  6:52     ` 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=44DC496D.76E4.0078.0@novell.com \
    --to=jbeulich@novell.com \
    --cc=76306.1226@compuserve.com \
    --cc=ak@suse.de \
    --cc=linux-kernel@vger.kernel.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