From: Oleg Nesterov <oleg@redhat.com>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: suravee.suthikulpanit@amd.com, mingo@kernel.org,
mingo@redhat.com, jacob.w.shin@gmail.com, a.p.zijlstra@chello.nl,
acme@ghostprotocols.net, hpa@zytor.com, tgl@domain.invalid,
linux-kernel@vger.kernel.org, sherry.hurwitz@amd.com
Subject: Re: [PATCH 1/3] perf/x86/amd: AMD support for bp_len > HW_BREAKPOINT_LEN_8
Date: Mon, 11 Nov 2013 18:51:31 +0100 [thread overview]
Message-ID: <20131111175131.GA14906@redhat.com> (raw)
In-Reply-To: <20131111154417.GD26853@localhost.localdomain>
On 11/11, Frederic Weisbecker wrote:
>
> On Sat, Nov 09, 2013 at 04:54:28PM +0100, Oleg Nesterov wrote:
> >
> > Up to you and Suravee, but can't we cleanup this later?
> >
> > This series was updated many times to address a lot of (sometimes
> > contradictory) complaints.
>
> Sure. But I'm confident that we can solve the conflicting mask / len issue easily beside.
> I mean, I don't feel confident with merging things as is, otoh it should be easy to fix up.
I do not really understand where do you see the conflict...
I can be easily wrong, but afaics currently mask / len issue is simply
the implementation detail.
> > > I mean, that doesn't look right to me,
> > > it's two units basically measuring the same thing, so that's asking for conflicting troubles.
> >
> > Yes. And we can kill either _len or _mask, not sure what would be more
> > clean.
> >
> > At least with the current implementation (again, iirc) mask == len -1.
> > Although amd supports the more generic masks (but I can't recall the
> > details).
>
> Right. I think len is probably more powerful. Unless the mask could be used to define
> multiple ranges of breakpoints, not sure it's ever going to be used that way though.
Actually, mask is more powerfull. And initial versions of this patches
(iirc) tried to use mask as an argument which comes from the userspace
(tools/perf, perf_event_attr, etc). But one of reviewers nacked this
interfacer, so we still use len.
> But we
> can still extend the interface if we need a mask later.
Yes, agreed.
> > > I'm just not sure how to reuse the len to express breakpoint ranges (that was in fact the
> > > initial purpose of it) without breaking the tools.
> >
> > Confused... user-space still uses len to express the range? Just
> > the kernel "switches" to mask if len > 8 ?
>
> Right but what if we want breakpoints having a size below 8? Like break on instructions
> from 0x1000 to 0x1008 ?
>
> Or should we ignore range instruction breakpoints when len < 8?
In this case the new code has no effect (iirc), we simply use
X86_BREAKPOINT_LEN_* and "tell the hardware about extended range/mask"
code is never called. IIRC, currently we simply check bp_mask != 0
to distinguish.
Oleg.
next prev parent reply other threads:[~2013-11-11 17:50 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-02 16:11 [PATCH V5 0/3] perf/x86/amd: AMD Family 16h Data Breakpoint Extensions suravee.suthikulpanit
2013-10-02 16:11 ` [PATCH 1/3] perf/x86/amd: AMD support for bp_len > HW_BREAKPOINT_LEN_8 suravee.suthikulpanit
2013-10-31 9:58 ` Frederic Weisbecker
2013-10-31 10:48 ` Borislav Petkov
2013-10-31 11:23 ` Frederic Weisbecker
2013-11-02 4:34 ` Borislav Petkov
2013-11-08 21:22 ` Suravee Suthikulpanit
2013-11-08 14:40 ` Borislav Petkov
2013-10-31 16:49 ` Oleg Nesterov
2013-11-08 19:41 ` Frederic Weisbecker
2013-11-09 15:11 ` Oleg Nesterov
2013-11-09 15:32 ` Frederic Weisbecker
2013-11-09 15:54 ` Oleg Nesterov
2013-11-11 15:44 ` Frederic Weisbecker
2013-11-11 17:51 ` Oleg Nesterov [this message]
2013-12-02 23:12 ` Frederic Weisbecker
2013-12-04 13:57 ` Oleg Nesterov
2013-12-10 14:43 ` Frederic Weisbecker
2013-12-10 14:52 ` Frederic Weisbecker
2013-12-10 15:23 ` Frederic Weisbecker
2013-12-10 16:19 ` Oleg Nesterov
2013-12-10 16:30 ` Frederic Weisbecker
2013-12-11 12:05 ` Suravee Suthikulanit
2013-10-02 16:11 ` [PATCH 2/3] perf tools: allow user to specify hardware breakpoint bp_len suravee.suthikulpanit
2013-12-10 15:25 ` Frederic Weisbecker
2013-12-10 16:22 ` Oleg Nesterov
2013-12-10 16:26 ` Frederic Weisbecker
2013-10-02 16:11 ` [PATCH 3/3] perf tools: add hardware breakpoint bp_len test cases suravee.suthikulpanit
2013-10-02 16:15 ` [PATCH V5 0/3] perf/x86/amd: AMD Family 16h Data Breakpoint Extensions Oleg Nesterov
2013-10-02 16:54 ` Suravee Suthikulpanit
2013-10-31 9:43 ` Frederic Weisbecker
-- strict thread matches above, loose matches on Subject: below --
2013-04-28 6:05 [PATCH V4 " Jacob Shin
2013-04-28 6:05 ` [PATCH 1/3] perf/x86/amd: AMD support for bp_len > HW_BREAKPOINT_LEN_8 Jacob Shin
2013-04-26 18:57 [PATCH 0/3] perf/x86/amd: AMD Family 16h Data Breakpoint Extensions Jacob Shin
2013-04-26 18:57 ` [PATCH 1/3] perf/x86/amd: AMD support for bp_len > HW_BREAKPOINT_LEN_8 Jacob Shin
2013-04-27 15:05 ` Oleg Nesterov
2013-04-27 15:14 ` Oleg Nesterov
2013-04-27 15:40 ` Jacob Shin
2013-04-27 16:10 ` Oleg Nesterov
2013-04-28 6:05 ` Jacob Shin
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=20131111175131.GA14906@redhat.com \
--to=oleg@redhat.com \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@ghostprotocols.net \
--cc=fweisbec@gmail.com \
--cc=hpa@zytor.com \
--cc=jacob.w.shin@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=mingo@redhat.com \
--cc=sherry.hurwitz@amd.com \
--cc=suravee.suthikulpanit@amd.com \
--cc=tgl@domain.invalid \
/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).