public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jim Meyering <jim@meyering.net>
To: Theodore Tso <tytso@mit.edu>
Cc: Ulrich Drepper <drepper@gmail.com>,
	Ulrich Drepper <drepper@redhat.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	bug-coreutils@gnu.org
Subject: Re: make getdents/readdir POSIX compliant wrt mount-point dirent.d_ino
Date: Fri, 06 Nov 2009 00:28:56 +0100	[thread overview]
Message-ID: <874op8lfev.fsf@meyering.net> (raw)
In-Reply-To: <20091105194858.GP6510@mit.edu> (Theodore Tso's message of "Thu, 5 Nov 2009 14:48:58 -0500")

Theodore Tso wrote:
> On Wed, Nov 04, 2009 at 08:29:00PM +0100, Jim Meyering wrote:
>> One way to accommodate the current automount semantics, is to make fts.c
>> incur, _for every directory traversed_, the cost of an additional
>> stat (fstatat, actually) call just in case this happens to be one of
>> those rare mount points.
>>
>> I would really rather not pessimize most[*] hierarchy-traversing
>> command-line tools by up to 17% (though usually far less) in order
>> to accommodate device-number change semantics that arise
>> for an automountable directory.
>
> I must be missing something.  How do you come up with the 17% penalty
> figure?  And what does this actually mean in real life?

Actually, it can approach 25%.  See below.

> stat() in Linux is fast.  Really fast.

Sure.  But so are chown, rm, du, etc.
And an extra stat can make more than a measurable difference.
On an absolute scale, the difference is not prohibitive,
but wouldn't it be a shame to penalize everyone
for a feature that some of us don't ever use?

> A quick benchmark clocks
> stat() on my system at 0.814 *microseconds* in the warm cache case,
> and if you're stating a directory that you've traversed, odds are
> extremely high that it will still be in the cache.
>
> My entire laptop root filesystem has 53,934 directories, so an extra
> stat() per directory translates to an extra 43 milliseconds, assuming
> I needed to walk my entire root filesystem.  It's really hard to see
> why kernel developers should get worked up into a lather over that
> kind of "performance penalty".

Here's a comparison with fewer than 5000 directories:
Given a directory named z, with 70 subdirs, each containing 70 empty subdirs.
All names are in 0..69.  Hot cache.  On a tmpfs file system.
linux 2.6.31.1-56.fc12.x86_64

Compare chgrp -R applied to "z", with and without the stat-adding patch:

  $ for i in 1 2 3; do for p in prev .; do echo $p; \
    env time $p/chgrp -R group2 z; done; done;
  prev
  0.03user 0.31system 0:00.34elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
  0inputs+0outputs (0major+235minor)pagefaults 0swaps
  .
  0.02user 0.39system 0:00.42elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
  0inputs+0outputs (0major+235minor)pagefaults 0swaps
  prev
  0.03user 0.30system 0:00.34elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
  0inputs+0outputs (0major+235minor)pagefaults 0swaps
  .
  0.04user 0.38system 0:00.43elapsed 96%CPU (0avgtext+0avgdata 0maxresident)k
  0inputs+0outputs (0major+236minor)pagefaults 0swaps
  prev
  0.03user 0.31system 0:00.34elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
  0inputs+0outputs (0major+236minor)pagefaults 0swaps
  .
  0.04user 0.37system 0:00.41elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
  0inputs+0outputs (0major+235minor)pagefaults 0swaps

That's a 23.5% performance hit. (42-34)/34

Sure, it's not even 1/10th of a second, but remember this is a tiny
hierarchy, and it's not just chgrp, but also find, rm, du, etc. that
are affected.  And this is not the sole reason to make a change, but
rather one more reason, in addition to the one that started this thread.

      reply	other threads:[~2009-11-05 23:29 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-01 13:07 make getdents/readdir POSIX compliant wrt mount-point dirent.d_ino Jim Meyering
2009-09-01 20:19 ` Theodore Tso
2009-09-01 22:03   ` Ulrich Drepper
2009-09-03 14:50     ` Eric Blake
2009-11-04 20:22       ` Jeff Layton
2009-11-04 19:29     ` Jim Meyering
2009-11-05 19:48       ` Theodore Tso
2009-11-05 23:28         ` Jim Meyering [this message]

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=874op8lfev.fsf@meyering.net \
    --to=jim@meyering.net \
    --cc=bug-coreutils@gnu.org \
    --cc=drepper@gmail.com \
    --cc=drepper@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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