All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Robert Richter <robert.richter@amd.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"oprofile-list@lists.sourceforge.net" 
	<oprofile-list@lists.sourceforge.net>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: Re: [GIT PULL] oprofile fixes for v2.6.40
Date: Fri, 27 May 2011 14:23:01 +0200	[thread overview]
Message-ID: <20110527122301.GA32754@elte.hu> (raw)
In-Reply-To: <20110526133948.GX20052@erda.amd.com>


* Robert Richter <robert.richter@amd.com> wrote:

> Robert Richter (1):
>       oprofile, x86: Enable preemption during pci device setup in IBS init

I'm wondering about this bit, as from a preempt_enable()/disable() 
pattern POV it looks bogus. It does:

        preempt_disable();
        for (offset = 1; offset < APIC_EILVT_NR_MAX; offset++) {
                if (get_eilvt(offset))
                        break;
        }
        preempt_enable();

	...


	ret = setup_ibs_ctl(offset);

So whatever condition it established at the get_eilvt() stage might 
not be valid at the setup_ibs_ctl() point: we might be on a different 
CPU.

Or are the IBS MSRs (which get twiddled in get_eilvt() absolutely 
symmetric across all CPUs, thus making this a preempt invariant 
thing? In that case we'd at least need a big fat comment in the code 
as it's not obvious at all ...

But i really do not see how this can work. get_eilvt() twiddles the 
APIC LVT of this current CPU. Does IBS only activate on a single CPU? 
How will this LVT entry be cleared on deinit?

Mysteries, mysteries!

Thanks,

	Ingo

  reply	other threads:[~2011-05-27 12:23 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-26 13:39 [GIT PULL] oprofile fixes for v2.6.40 Robert Richter
2011-05-27 12:23 ` Ingo Molnar [this message]
2011-05-30 10:00   ` Robert Richter
2011-05-30 10:09     ` Ingo Molnar
2011-05-30 10:16       ` Robert Richter
2011-05-30 14:41         ` Robert Richter
2011-05-30 14:44           ` Ingo Molnar
2011-05-30 14:43         ` [PATCH] oprofile, x86: Add comments to IBS LVT offset initialization Robert Richter

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=20110527122301.GA32754@elte.hu \
    --to=mingo@elte.hu \
    --cc=a.p.zijlstra@chello.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oprofile-list@lists.sourceforge.net \
    --cc=robert.richter@amd.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 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.