All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Byungchul Park <byungchul.park@lge.com>,
	Chris Metcalf <cmetcalf@ezchip.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Luiz Capitulino <lcapitulino@redhat.com>,
	Christoph Lameter <cl@linux.com>,
	"Paul E . McKenney" <paulmck@linux.vnet.ibm.com>,
	Mike Galbraith <efault@gmx.de>, Rik van Riel <riel@redhat.com>,
	Ingo Molnar <mingo@elte.hu>
Subject: Re: [PATCH 3/3] sched: Optimize !CONFIG_NO_HZ_COMMON cpu load updates
Date: Mon, 11 Apr 2016 15:18:16 +0200	[thread overview]
Message-ID: <20160411131814.GA22628@lerouge> (raw)
In-Reply-To: <20160408174414.GE1087@worktop>

On Fri, Apr 08, 2016 at 07:44:14PM +0200, Peter Zijlstra wrote:
> On Fri, Apr 08, 2016 at 02:55:22PM +0200, Frederic Weisbecker wrote:
> > > > @@ -4540,17 +4568,8 @@ static void cpu_load_update(struct rq *this_rq, unsigned long this_load,
> > > >  
> > > >  		/* scale is effectively 1 << i now, and >> i divides by scale */
> > > >  
> > > > -		old_load = this_rq->cpu_load[i];
> > > #ifdef CONFIG_NO_HZ_COMMON
> > > > -		old_load = decay_load_missed(old_load, pending_updates - 1, i);
> > > > -		if (tickless_load) {
> > > > -			old_load -= decay_load_missed(tickless_load, pending_updates - 1, i);
> > > > -			/*
> > > > -			 * old_load can never be a negative value because a
> > > > -			 * decayed tickless_load cannot be greater than the
> > > > -			 * original tickless_load.
> > > > -			 */
> > > > -			old_load += tickless_load;
> > > > -		}
> > > #endif
> > 
> > Ah sure, if you prefer it that way, I can do that.
> 
> Yes please. I normally favour the thing you did, but here it makes
> tricky code that much harder to read.

So I tried and it warns about the unused variable tickless_load, so I
would need two scattered ifdeffery in the function:

@@ -4528,7 +4529,9 @@ decay_load_missed(unsigned long load, unsigned long missed_updates, int idx)
 static void cpu_load_update(struct rq *this_rq, unsigned long this_load,
 			    unsigned long pending_updates)
 {
+#ifdef CONFIG_NO_HZ_COMMON
 	unsigned long tickless_load = this_rq->cpu_load[0];
+#endif
 	int i, scale;
 
 	this_rq->nr_load_updates++;
@@ -4541,6 +4544,7 @@ static void cpu_load_update(struct rq *this_rq, unsigned long this_load,
 		/* scale is effectively 1 << i now, and >> i divides by scale */
 
 		old_load = this_rq->cpu_load[i];
+#ifdef CONFIG_NO_HZ_COMMON
 		old_load = decay_load_missed(old_load, pending_updates - 1, i);
 		if (tickless_load) {
 			old_load -= decay_load_missed(tickless_load, pending_updates - 1, i);
@@ -4551,6 +4555,7 @@ static void cpu_load_update(struct rq *this_rq, unsigned long this_load,
 			 */
 			old_load += tickless_load;
 		}
+#endif
 		new_load = this_load;
 		/*
 		 * Round up the averaging division if load is increasing. This


Are you still sure you don't want the ifdeffed inline function?

Thanks.

  reply	other threads:[~2016-04-11 13:18 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-08  1:07 [PATCH 0/3] sched: Fix/improve nohz cpu load updates v2 Frederic Weisbecker
2016-04-08  1:07 ` [PATCH 1/3] sched: Gather cpu load functions under a more conventional namespace Frederic Weisbecker
2016-04-08  1:07 ` [PATCH 2/3] sched: Correctly handle nohz ticks cpu load accounting Frederic Weisbecker
2016-04-08  9:41   ` Peter Zijlstra
2016-04-08 12:53     ` Frederic Weisbecker
2016-04-08 17:40       ` Peter Zijlstra
2016-04-08  1:07 ` [PATCH 3/3] sched: Optimize !CONFIG_NO_HZ_COMMON cpu load updates Frederic Weisbecker
2016-04-08 10:48   ` Peter Zijlstra
2016-04-08 12:55     ` Frederic Weisbecker
2016-04-08 17:44       ` Peter Zijlstra
2016-04-11 13:18         ` Frederic Weisbecker [this message]
2016-04-11 14:53           ` Chris Metcalf
2016-04-11 18:21             ` Frederic Weisbecker
2016-04-12 14:23               ` Peter Zijlstra
2016-04-13 13:56 [PATCH 0/3] sched: Fix/improve nohz cpu load updates v3 Frederic Weisbecker
2016-04-13 13:56 ` [PATCH 3/3] sched: Optimize !CONFIG_NO_HZ_COMMON cpu load updates Frederic Weisbecker
2016-04-18 13:36   ` Frederic Weisbecker

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=20160411131814.GA22628@lerouge \
    --to=fweisbec@gmail.com \
    --cc=byungchul.park@lge.com \
    --cc=cl@linux.com \
    --cc=cmetcalf@ezchip.com \
    --cc=efault@gmx.de \
    --cc=lcapitulino@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=tglx@linutronix.de \
    /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.