From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
Namhyung Kim <namhyung.kim@lge.com>,
Frederic Weisbecker <fweisbec@gmail.com>,
Ingo Molnar <mingo@redhat.com>, Jiri Olsa <jolsa@redhat.com>,
linux-kernel@vger.kernel.org
Subject: Re: Re: [PATCH 1/2] tracing: Add $cpu and $current probe-vars
Date: Thu, 28 Nov 2013 11:51:18 +0900 [thread overview]
Message-ID: <5296AFA6.3060509@hitachi.com> (raw)
In-Reply-To: <20131127170550.GB26138@redhat.com>
(2013/11/28 2:05), Oleg Nesterov wrote:
> On 11/27, Masami Hiramatsu wrote:
>>
>> (2013/11/27 2:23), Oleg Nesterov wrote:
>>> On 11/26, Masami Hiramatsu wrote:
>>>>
>>>> (2013/11/26 4:29), Oleg Nesterov wrote:
>>>>> +#define PSEUDO_REG_OFFSET 4096 /* arbitrary value > MAX_REG_OFFSET */
>>>>> +
>>>>> +static unsigned long probe_get_register(struct pt_regs *regs, unsigned long offset)
>>>>> +{
>>>>> + if (offset < PSEUDO_REG_OFFSET)
>>>>> + return regs_get_register(regs, offset);
>>>>> +
>>>>> + return pseudo_reg_table[offset - PSEUDO_REG_OFFSET].fetch();
>>>>> +}
>>>>
>>>>
>>>> Hmm, I don't like this, since fetch_reg is the most frequently
>>>> used fetch method, and it actually uses the offset in different
>>>> way.
>>>
>>> Sure, this overloads the usage of FETCH_MTD_reg/offset.
>>>
>>> And btw, yes, the naming is ugly (I mean pseudo_*). But why this
>>> is bad? This is cheap and simple.
>>
>> I think it's not simple. The code looks short, but not well
>> self-described. It is "hidden" in the structure, and double-meaning.
>
> Yes, I agree. And the ugly naming doesn't make it more clear/clean.
>
> I'll try to cleanup this somehow and resend. Perhaps I should start
> with 2/2 which generalizes FETCH_MTD_reg and kills FETCH_MTD_retval.
> Or at least generalizes FETCH_MTD_retval.
Sounds good for me :)
>>>> Why don't you make another fetch function for vars?
>>>
>>> This is what I tried to avoid ;) I do not want to add another
>>> FETCH_MTD_. To me, this looks like unnecessary complication and
>>> bloat (but see below).
>>
>> I see, perhaps, it is a time to introduce independent fetch
>> method implementation. Current one is not sophisticated
>> nor generic
>
> Yes, yes, I agree. But until then I do not want to complicate/blow
> this code to implement $current.
Yeah, that is another topic. Let's focus on making it more useful. :)
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
next prev parent reply other threads:[~2013-11-28 2:51 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-23 20:15 [PATCH 0/1] tracing: Introduce "pseudo registers" for FETCH_MTD_reg Oleg Nesterov
2013-11-23 20:16 ` [PATCH 1/1] " Oleg Nesterov
2013-11-24 7:32 ` Masami Hiramatsu
2013-11-25 8:04 ` Namhyung Kim
2013-11-25 14:35 ` Oleg Nesterov
2013-11-25 17:21 ` [RFC PATCH 0/3] tracing: Teach FETCH_MTD_{symbol,deref} to handle per-cpu data Oleg Nesterov
2013-11-25 17:21 ` [RFC PATCH 1/3] tracing: Don't mangle sc->addr in update_symbol_cache() Oleg Nesterov
2013-11-25 17:21 ` [RFC PATCH 2/3] tracing: introduce {calc,parse}_probe_offset() for FETCH_MTD_{symbol,deref} Oleg Nesterov
2013-11-25 17:22 ` [RFC PATCH 3/3] tracing: Teach FETCH_MTD_{symbol,deref} to handle per-cpu data Oleg Nesterov
2013-11-26 9:34 ` Masami Hiramatsu
2013-11-26 17:43 ` [RFC PATCH 0/2] tracing: Teach FETCH_MTD_symbol " Oleg Nesterov
2013-11-26 17:44 ` [RFC PATCH 1/2] tracing: Don't update sc->addr in update_symbol_cache() Oleg Nesterov
2013-11-26 17:44 ` [RFC PATCH 2/2] tracing: Teach FETCH_MTD_symbol to handle per-cpu data Oleg Nesterov
2013-11-26 17:50 ` modules, add_kallsyms() && DEFINE_PER_CPU Oleg Nesterov
2013-11-27 2:23 ` Masami Hiramatsu
2013-11-27 8:20 ` Namhyung Kim
2013-11-27 11:22 ` Masami Hiramatsu
2013-11-27 13:35 ` Oleg Nesterov
2013-11-28 2:02 ` Masami Hiramatsu
2013-11-27 11:30 ` [RFC PATCH 2/2] tracing: Teach FETCH_MTD_symbol to handle per-cpu data Masami Hiramatsu
2013-11-27 0:37 ` [RFC PATCH 0/2] " Namhyung Kim
2013-11-27 10:01 ` Masami Hiramatsu
2013-11-27 17:41 ` Oleg Nesterov
2013-11-28 2:55 ` Masami Hiramatsu
2013-11-25 19:29 ` [PATCH 0/2] tracing: Add $cpu and $current probe-vars Oleg Nesterov
2013-11-25 19:29 ` [PATCH 1/2] " Oleg Nesterov
2013-11-26 2:21 ` Masami Hiramatsu
2013-11-26 17:23 ` Oleg Nesterov
2013-11-27 8:22 ` Masami Hiramatsu
2013-11-27 17:05 ` Oleg Nesterov
2013-11-28 2:51 ` Masami Hiramatsu [this message]
2013-11-25 19:30 ` [PATCH 2/2] tracing: Kill FETCH_MTD_retval, reimplement $retval via pseudo_reg_retval() Oleg Nesterov
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=5296AFA6.3060509@hitachi.com \
--to=masami.hiramatsu.pt@hitachi.com \
--cc=fweisbec@gmail.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung.kim@lge.com \
--cc=oleg@redhat.com \
--cc=rostedt@goodmis.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.