From: Koki Sanagi <sanagi.koki@jp•fujitsu.com>
To: Frederic Weisbecker <fweisbec@gmail•com>
Cc: Steven Rostedt <rostedt@goodmis•org>,
netdev@vger•kernel.org, linux-kernel@vger•kernel.org,
davem@davemloft•net, kaneshige.kenji@jp•fujitsu.com,
izumi.taku@jp•fujitsu.com, kosaki.motohiro@jp•fujitsu.com,
nhorman@tuxdriver•com, laijs@cn•fujitsu.com,
scott.a.mcmillan@intel•com, eric.dumazet@gmail•com,
mathieu.desnoyers@polymtl•ca
Subject: Re: [PATCH v4 1/5] irq: add tracepoint to softirq_raise
Date: Mon, 06 Sep 2010 10:46:12 +0900 [thread overview]
Message-ID: <4C8447E4.6050606@jp.fujitsu.com> (raw)
In-Reply-To: <20100903155008.GC5352@nowhere>
(2010/09/04 0:50), Frederic Weisbecker wrote:
> On Fri, Sep 03, 2010 at 11:43:12AM -0400, Steven Rostedt wrote:
>> On Fri, 2010-09-03 at 17:29 +0200, Frederic Weisbecker wrote:
>>
>>>> /*
>>>> * These correspond to the IORESOURCE_IRQ_* defines in
>>>> @@ -407,7 +408,12 @@ asmlinkage void do_softirq(void);
>>>> asmlinkage void __do_softirq(void);
>>>> extern void open_softirq(int nr, void (*action)(struct softirq_action *));
>>>> extern void softirq_init(void);
>>>> -#define __raise_softirq_irqoff(nr) do { or_softirq_pending(1UL << (nr)); } while (0)
>>>> +static inline void __raise_softirq_irqoff(unsigned int nr)
>>>> +{
>>>> + trace_softirq_raise((struct softirq_action *)&nr, NULL);
>>
>> Perhaps doing:
>>
>> trace_softirq_raise((struct softirq_action *)((unsigend long)nr),
>> NULL);
>>
>> and ...
>>
>>>> + or_softirq_pending(1UL << nr);
>>>> +}
>>>> +
>>>> extern void raise_softirq_irqoff(unsigned int nr);
>>>> extern void raise_softirq(unsigned int nr);
>>>> extern void wakeup_softirqd(void);
>>>> diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
>>>> index 0e4cfb6..3ddda02 100644
>>>> --- a/include/trace/events/irq.h
>>>> +++ b/include/trace/events/irq.h
>>>> @@ -5,7 +5,9 @@
>>>> #define _TRACE_IRQ_H
>>>>
>>>> #include <linux/tracepoint.h>
>>>> -#include <linux/interrupt.h>
>>>> +
>>>> +struct irqaction;
>>>> +struct softirq_action;
>>>>
>>>> #define softirq_name(sirq) { sirq##_SOFTIRQ, #sirq }
>>>> #define show_softirq_name(val) \
>>>> @@ -93,7 +95,10 @@ DECLARE_EVENT_CLASS(softirq,
>>>> ),
>>>>
>>>> TP_fast_assign(
>>>> - __entry->vec = (int)(h - vec);
>>>> + if (vec)
>>>> + __entry->vec = (int)(h - vec);
>>>> + else
>>>> + __entry->vec = *((int *)h);
>>
>> __entry->vec = (int)h;
>>
>> would be better.
>>
>>
>>>> ),
>>>
>>>
>>>
>>> It seems that this will break softirq_entry/exit tracepoints.
>>> __entry->vec will deref vec->action() for these two, which is not
>>> what we want.
>>>
>>> If you can't have the same tracepoint signature for the three, just
>>> split the new one in a seperate TRACE_EVENT().
>>
>> Doing the above will at least be a bit safer.
>
>
> Agreed, I'm going to change that in the patch.
>
> Thanks.
>
I agree.
Thanks,
Koki Sanagi.
next prev parent reply other threads:[~2010-09-06 1:46 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-23 9:41 [PATCH v4 0/5] netdev: show a process of packets Koki Sanagi
2010-08-23 9:42 ` [PATCH v4 1/5] irq: add tracepoint to softirq_raise Koki Sanagi
2010-09-03 15:29 ` Frederic Weisbecker
2010-09-03 15:39 ` Steven Rostedt
2010-09-03 15:42 ` Frederic Weisbecker
2010-09-03 15:43 ` Steven Rostedt
2010-09-03 15:50 ` Frederic Weisbecker
2010-09-06 1:46 ` Koki Sanagi [this message]
2010-08-23 9:43 ` [PATCH v4 2/5] napi: convert trace_napi_poll to TRACE_EVENT Koki Sanagi
2010-08-24 3:52 ` David Miller
2010-08-23 9:45 ` [PATCH v4 3/5] netdev: add tracepoints to netdev layer Koki Sanagi
2010-08-24 3:53 ` David Miller
2010-08-23 9:46 ` [PATCH v4 4/5] skb: add tracepoints to freeing skb Koki Sanagi
2010-08-24 3:53 ` David Miller
2010-08-23 9:47 ` [PATCH v4 5/5] perf:add a script shows a process of packet Koki Sanagi
2010-08-24 3:53 ` David Miller
2010-09-07 16:57 ` Frederic Weisbecker
2010-08-30 23:50 ` [PATCH v4 0/5] netdev: show a process of packets Steven Rostedt
2010-09-03 2:10 ` Koki Sanagi
2010-09-03 2:17 ` David Miller
2010-09-03 2:55 ` Koki Sanagi
2010-09-03 4:46 ` Frederic Weisbecker
2010-09-03 5:12 ` Koki Sanagi
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=4C8447E4.6050606@jp.fujitsu.com \
--to=sanagi.koki@jp$(echo .)fujitsu.com \
--cc=davem@davemloft$(echo .)net \
--cc=eric.dumazet@gmail$(echo .)com \
--cc=fweisbec@gmail$(echo .)com \
--cc=izumi.taku@jp$(echo .)fujitsu.com \
--cc=kaneshige.kenji@jp$(echo .)fujitsu.com \
--cc=kosaki.motohiro@jp$(echo .)fujitsu.com \
--cc=laijs@cn$(echo .)fujitsu.com \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=mathieu.desnoyers@polymtl$(echo .)ca \
--cc=netdev@vger$(echo .)kernel.org \
--cc=nhorman@tuxdriver$(echo .)com \
--cc=rostedt@goodmis$(echo .)org \
--cc=scott.a.mcmillan@intel$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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