All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Williams <pwil3058@bigpond.net.au>
To: Con Kolivas <kernel@kolivas.org>
Cc: linux list <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@osdl.org>, Ingo Molnar <mingo@elte.hu>,
	ck list <ck@vds.kolivas.org>
Subject: Re: [PATCH][2/4] sched: add discrete weighted cpu load function
Date: Wed, 15 Mar 2006 09:45:37 +1100	[thread overview]
Message-ID: <44174791.7090905@bigpond.net.au> (raw)
In-Reply-To: <200603150926.52064.kernel@kolivas.org>

Con Kolivas wrote:
> On Wednesday 15 March 2006 09:09, Peter Williams wrote:
> 
>>Con Kolivas wrote:
>>
>>>Peter Williams writes:
>>>
>>>>Con Kolivas wrote:
>>>>
>>>>>+unsigned long weighted_cpuload(const int cpu)
>>>>>+{
>>>>>+    return (cpu_rq(cpu)->raw_weighted_load);
>>>>>+}
>>>>>+
>>>>
>>>>Wouldn't this be a candidate for inlining?
>>>
>>>That would make it unsuitable for exporting via sched.h.
>>
>>If above_background_load() were implemented inside sched.c instead of in
>>sched.h there would be no need to export weighted_cpuload() would there?
>>  This would allow weighted_cpuload() to be inline and the efficiency
>>would be better as above_background_load() doesn't gain a lot by being
>>inline
> 
> 
> I don't care about above_background_load() being inline; that's done because 
> all functions in header files need to be static inline to not become a mess.
> 
> 
>>as having weighted_cpulpad() non inline means that it's doing a 
>>function call several times in a loop i.e. it may save one function call
>>by being inline but requires (up to) one function call for every CPU.
> 
> 
> I haven't checked but gcc may well inline weighted_cpuload anyway?

It may be doing so for internal uses inside sched.c but I'm pretty sure 
that it won't for external calls.

> We're 
> moving away from inlining most things manually since the compiler is doing it 
> well these days.
> 

OK.  Even without explicit inlining you need to give the compiler a 
chance to optimize function call overhead away.  It cant do that with 
the present arrangement.

> 
>>The other way around the cost would be just one function call.
> 
> 
> The way you're suggesting adds a function that is never used by anything but 
> swap prefetch which would then need to be 'ifdef'ed out to not be needlessly 
> built on every system. Adding ifdefs is frowned upon already, and to have an 
> mm/ specific ifdef in sched.c would be rather ugly.

Sometimes ugliness is the best option.

Peter
-- 
Peter Williams                                   pwil3058@bigpond.net.au

"Learning, n. The kind of ignorance distinguishing the studious."
  -- Ambrose Bierce

  reply	other threads:[~2006-03-14 22:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-03-13  8:06 [PATCH][2/4] sched: add discrete weighted cpu load function Con Kolivas
2006-03-13  9:06 ` Ingo Molnar
2006-03-13  9:14   ` Con Kolivas
2006-03-13 22:39 ` Peter Williams
2006-03-13 22:52   ` Con Kolivas
2006-03-13 23:16     ` Peter Williams
2006-03-14 22:09     ` Peter Williams
2006-03-14 22:26       ` Con Kolivas
2006-03-14 22:45         ` Peter Williams [this message]
2006-03-14 22:50           ` Con Kolivas

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=44174791.7090905@bigpond.net.au \
    --to=pwil3058@bigpond.net.au \
    --cc=akpm@osdl.org \
    --cc=ck@vds.kolivas.org \
    --cc=kernel@kolivas.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    /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.