public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Shailabh Nagar <nagar@watson.ibm.com>
To: Arjan van de Ven <arjan@infradead.org>
Cc: linux-kernel <linux-kernel@vger.kernel.org>,
	Andi Kleen <ak@suse.de>, Greg KH <greg@kroah.com>
Subject: Re: [Patch 3/9] Block I/O accounting initialization
Date: Wed, 15 Mar 2006 11:27:33 -0500	[thread overview]
Message-ID: <44184075.6080000@watson.ibm.com> (raw)
In-Reply-To: <1142418436.3021.13.camel@laptopd505.fenrus.org>

Arjan van de Ven wrote:

>> 
>>+static inline void delayacct_blkio_start(void)
>>+{
>>+	if (unlikely(delayacct_on))
>>+		__delayacct_blkio_start();
>>+}
>>    
>>
>
>
>I still think the unlikely() makes no sense here; at runtime it's either
>going to be always on or off (in the sense that switching it will be
>RARE). The cpus branch predictor will get that right; while if you force
>it unlikely you can even run the risk of getting a 100% miss on some
>architectures (not x86/x86-64) when you enable the accounting
>
>  
>
I don't understand why the fact that delayacct_on is not going to vary
dynamically once a kernel has been booted should prevent the usage of 
"unlikely/likely"
Perhaps you can help me understand. Here's the logic I was using:

There are two kinds of users

1. Those who will not turn delay accounting on at boot time for a number of
reasons (don't care about the functionality, don't want to incur any 
overhead
however small).

2. The ones who do turn it on at boot time are willing to pay some overhead
for the benefits such functionality brings. What that overhead exactly 
is will be known
when we finish running some tests but we can safely assume it'll be 
non-zero.

A distro will need to keep delay accounting configured to accomodate 
both kinds
of users but because 1 is the common case,  the default boot time option 
will be off and
there is a strong incentive to reduce overhead for non-users.

Using unlikely reduces the overhead for the common case 1 and increases 
overhead for
case 2 (because of what you pointed out of 100% wrong decisions by the 
branch predictor).

I don't know how much of overhead reduction is achieved by using 
unlikely (I'm assuming its
non-trivial) or how much penalty is imposed by a 100% wrong prediction 
(I'm assuming its not
very high).

But under these assumptions, its better to use unlikely, make type 2 
users pay extra overhead
and save type 1 users from any.

Is this reasoning accurate ? If not, we could easily switch the unlikely 
off.

Regards,
Shailabh

(cc'ing Greg since he'd brought up the overhead for real benchmarks etc.)

  reply	other threads:[~2006-03-15 16:27 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-03-14  0:40 [Patch 0/9] Per-task delay accounting Shailabh Nagar
2006-03-14  0:42 ` [Patch 1/9] timestamp diff Shailabh Nagar
2006-03-14  1:01   ` Lee Revell
2006-03-14  1:05     ` Shailabh Nagar
2006-03-14  1:12       ` Lee Revell
2006-03-14  3:42         ` Balbir Singh
2006-03-14  4:26           ` Shailabh Nagar
2006-03-14  6:50             ` Balbir Singh
2006-03-15 10:23   ` Arjan van de Ven
2006-03-15 10:28     ` Balbir Singh
2006-03-14  0:45 ` Patch 2/9] Initialization Shailabh Nagar
2006-03-14 10:54   ` Jes Sorensen
2006-03-14 15:20     ` Shailabh Nagar
2006-03-15 10:24   ` Arjan van de Ven
2006-03-15 12:37     ` Alan Cox
2006-03-15 15:53       ` Shailabh Nagar
2006-03-14  0:47 ` [Patch 3/9] Block I/O accounting initialization Shailabh Nagar
2006-03-15 10:27   ` Arjan van de Ven
2006-03-15 16:27     ` Shailabh Nagar [this message]
2006-03-14  0:48 ` [Patch 4/9] Block I/O accounting collection Shailabh Nagar
2006-03-14  0:51 ` [Patch 5/9] Swapin delays Shailabh Nagar
2006-03-14  0:53 ` [Patch 7/9] /proc interface for all I/O delays Shailabh Nagar
2006-03-14  0:55 ` [Patch 8/9] generic netlink utility functions Shailabh Nagar
2006-03-26 16:44   ` Balbir Singh
2006-03-26 17:06     ` jamal
2006-03-14  0:56 ` [Patch 9/9] Generic netlink interface for delay accounting Shailabh Nagar
2006-03-14  2:29   ` jamal
2006-03-14  2:33   ` Matt Helsley
2006-03-14  2:48     ` jamal
2006-03-14  4:18       ` Shailabh Nagar
2006-03-22  7:49       ` [RFC][UPDATED PATCH 2.6.16] " Balbir Singh
2006-03-23 14:04         ` jamal
2006-03-23 15:41           ` Balbir Singh
2006-03-24 14:04             ` jamal
2006-03-24 14:54               ` Balbir Singh
2006-03-25  1:19                 ` jamal
2006-03-25  9:41                   ` Balbir Singh
2006-03-25 12:52                     ` jamal
2006-03-25 15:36                       ` Balbir Singh
2006-03-25 17:48                         ` jamal
2006-03-25 18:22                           ` Balbir Singh
2006-03-26 14:05                             ` jamal
2006-03-26 16:40                               ` Balbir Singh
2006-03-24  1:32           ` Balbir Singh
2006-03-24 14:11             ` jamal
2006-03-24 14:19               ` jamal
2006-03-24 14:59               ` Balbir Singh
2006-03-14  4:29     ` Shailabh Nagar
2006-03-14  1:01 ` [Patch 6/9] cpu delay collection Shailabh Nagar
2006-03-14 19:28 ` [Patch 0/9] Per-task delay accounting Greg KH
2006-03-14 20:49   ` Shailabh Nagar
2006-03-14 21:24     ` Greg KH
2006-03-14 21:59       ` Shailabh Nagar
2006-03-23 15:16   ` [Patch 0/9] Performance Shailabh Nagar
2006-03-25  2:38     ` Greg KH
2006-03-27 18:28       ` Shailabh Nagar

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=44184075.6080000@watson.ibm.com \
    --to=nagar@watson.ibm.com \
    --cc=ak@suse.de \
    --cc=arjan@infradead.org \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox