All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeremy Fitzhardinge <jeremy@goop.org>
To: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Cc: Steven Rostedt <rostedt@goodmis.org>, Ingo Molnar <mingo@elte.hu>,
	linux-kernel@vger.kernel.org,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [patch 2/3] RCU move trace defines to rcupdate_types.h
Date: Fri, 17 Apr 2009 09:14:27 -0700	[thread overview]
Message-ID: <49E8AAE3.9060005@goop.org> (raw)
In-Reply-To: <20090417154228.GB15046@Krystal>

Mathieu Desnoyers wrote:
> * Jeremy Fitzhardinge (jeremy@goop.org) wrote:
>   
>> Mathieu Desnoyers wrote:
>>     
>>> Given the simplicity of the preempt_disable/enable_notrace found in
>>> preempt.h, we could move them to 
>>>
>>> include/preempt_types.h too, and that would solve all problems, wouldn't
>>> it ?
>>>   
>>>       
>> No, it still needs linux/thread_info.h -> asm/thread_info.h, which in  
>> turn gets quite a lot of things on x86 (and would need to be audited in  
>> each architecture).
>>
>>    J
>>     
>
> Well, I think it's a good time to do some cleanup then. Why on earth
> would thread_info.h be anything else than a "_types"-like header ?
>   

Why indeed?  Because it includes a number of other headers to get the 
definitions it needs, and defines various functions needed to operate on 
the thread_info structure (including the all-important 
current_thread_info()).

Yes, it can be refactored into thread_info.h and thread_info_types.h, 
and all the headers it includes can be similarly refactored, and 
linux/thread_info.h can also be split, and all the asm/*/thread_info.hs 
can be split too, and it can be made to work for all arches under all 
configs... 

But that's going to take a long time, and if its a pre-requisite for 
getting tracing going, then we're not going to see it merged this year.

> If headers has become in such a state in the kernel, then IMHO the
> solution is not to shove more out-of-line functions under the carpet,
> but rather to do the cleanup.
>   

Besides, I'm still not convinced that putting the code inline is a good 
idea.  Direct call/return are not inherently expensive, and they're 
something that CPU vendors have a lot of motivation to optimise for.  In 
particular, the call itself is no more expensive than a jmp other than 
the return-address push, and the ret is also cheap because it will use 
the return address cache rather than having to be a full indirect jmp.

And it would be much easier to justify leaving tracing compile-time 
enabled all the time if each tracepoint really does have a minimal 
icache profile when not enabled.

    J

  reply	other threads:[~2009-04-17 16:14 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-17  0:37 [patch 0/3] Tracepoints kill rcupdate header dependency (v2) mathieu.desnoyers
2009-04-17  0:37 ` [patch 1/3] rcupdate header remove whitespace mathieu.desnoyers
2009-04-17  0:37 ` [patch 2/3] RCU move trace defines to rcupdate_types.h mathieu.desnoyers
2009-04-17  1:10   ` Jeremy Fitzhardinge
2009-04-17  1:22     ` Steven Rostedt
2009-04-17  1:42       ` Mathieu Desnoyers
2009-04-17  1:47         ` Mathieu Desnoyers
2009-04-17  6:23           ` Jeremy Fitzhardinge
2009-04-17  5:57         ` Jeremy Fitzhardinge
2009-04-17 15:16           ` Mathieu Desnoyers
2009-04-17 15:26             ` Jeremy Fitzhardinge
2009-04-17 15:42               ` Mathieu Desnoyers
2009-04-17 16:14                 ` Jeremy Fitzhardinge [this message]
2009-04-17 16:38                   ` Steven Rostedt
2009-04-17 17:09                     ` Jeremy Fitzhardinge
2009-04-17  2:38       ` [patch 2/3] RCU move trace defines to rcupdate_types.h (update) Mathieu Desnoyers
2009-04-17  0:37 ` [patch 3/3] tracepoints : remove rcupdate.h dependency mathieu.desnoyers

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=49E8AAE3.9060005@goop.org \
    --to=jeremy@goop.org \
    --cc=akpm@linux-foundation.org \
    --cc=hch@lst.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@polymtl.ca \
    --cc=mingo@elte.hu \
    --cc=paulmck@linux.vnet.ibm.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.