linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nick Piggin <npiggin@kernel.dk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Nick Piggin <npiggin@gmail.com>, Nick Piggin <npiggin@kernel.dk>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [patch 1/6] fs: icache RCU free inodes
Date: Fri, 12 Nov 2010 17:02:02 +1100	[thread overview]
Message-ID: <20101112060202.GB3332@amd> (raw)
In-Reply-To: <AANLkTimn623-N1R_Bn4sF4U6OX55TQZ2w7jGOq+kB5Pz@mail.gmail.com>

On Thu, Nov 11, 2010 at 08:48:38PM -0800, Linus Torvalds wrote:
> On Thu, Nov 11, 2010 at 5:24 PM, Nick Piggin <npiggin@gmail.com> wrote:
> >
> > So this is really not a "oh, maybe someone will see 10-20% slowdown", or even
> > 1-2% slowdown.
> 
> You ignored my bigger issue: the _normal_ way - and the better way -
> to handle these thingsis with SLAB_DESTROY_BY_RCU.

Well I tried to answer that in the other threads.

SLAB_DESTROY_BY_RCU is indeed quite natural for a lot of RCU usages,
because even with standard RCU you almost always have the pattern like

rcu_read_lock();
obj = lookup_data_structure(key);
if (obj) {
  lock(obj);
  verify_obj_in_structure(obj, key);
  /* blah... (eg. take refcount) */
}

And in this pattern, SLAB_DESTROY_BY_RCU takes almost zero work.

OK, but rcu-walk doesn't have that. In rcu-walk, we can't take a lock
or a reference on either the dentry _or_ the inode, because the whole
point is to reduce atomics (for single threaded performance), and
stores into shared cachelines along the path (for scalability).

It gets really interesting when you have crazy stuff going on like
inode->i_ops changing from underneath you while you're trying to do
permission lookups, or inode type changing from link to dir to reg
in the middle of the traversal.


> So what are the advantages of using the inferior approach?  I really
> don't see why you push the whole "free the damn things individually"
> approach.

I'm not pushing _that_ aspect of it. I'm pushing the "don't go away and
come back as something else" aspect.

Yes it may be _possible_ to do store-free walking SLAB_DESTROY_BY_RCU,
and I have some ideas. But it is hairy. More hairy than rcu-walk, by
quite a long shot.

And the complexity and issues that need to be understood are basically
rcu-walk + *more*. So I want basic rcu-walk to be reviewed and basically
verified first, and we could think about the +more part after that.

Basically SLAB_DESTROY_BY_RCU is not exactly natural for a completely
store-free algorithm.


  reply	other threads:[~2010-11-12  6:02 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-09 12:46 [patch 1/6] fs: icache RCU free inodes Nick Piggin
2010-11-09 12:47 ` [patch 2/6] fs: icache avoid RCU freeing for pseudo fs Nick Piggin
2010-11-09 12:58 ` [patch 3/6] fs: dcache documentation cleanup Nick Piggin
2010-11-09 16:24   ` Christoph Hellwig
2010-11-09 22:06     ` Nick Piggin
2010-11-10 16:27       ` Christoph Hellwig
2010-11-09 13:01 ` [patch 4/6] fs: d_delete change Nick Piggin
2010-11-09 16:25   ` Christoph Hellwig
2010-11-09 22:08     ` Nick Piggin
2010-11-10 16:32       ` Christoph Hellwig
2010-11-11  0:27         ` Nick Piggin
2010-11-11 22:07           ` Linus Torvalds
2010-11-09 13:02 ` [patch 5/6] fs: d_compare change for rcu-walk Nick Piggin
2010-11-09 16:25   ` Christoph Hellwig
2010-11-10  1:48     ` Nick Piggin
2010-11-09 13:03 ` [patch 6/6] fs: d_hash " Nick Piggin
2010-11-09 14:19 ` [patch 1/6] fs: icache RCU free inodes Andi Kleen
2010-11-09 21:36   ` Nick Piggin
2010-11-10 14:47     ` Andi Kleen
2010-11-11  4:27       ` Nick Piggin
2010-11-09 16:02 ` Linus Torvalds
2010-11-09 16:21   ` Christoph Hellwig
2010-11-09 21:48     ` Nick Piggin
2010-11-09 16:21   ` Eric Dumazet
2010-11-09 17:08     ` Linus Torvalds
2010-11-09 17:15       ` Christoph Hellwig
2010-11-09 21:55         ` Nick Piggin
2010-11-09 22:05       ` Nick Piggin
2010-11-12  1:24         ` Nick Piggin
2010-11-12  4:48           ` Linus Torvalds
2010-11-12  6:02             ` Nick Piggin [this message]
2010-11-12  6:49               ` Nick Piggin
2010-11-12 17:33                 ` Linus Torvalds
2010-11-12 23:17                   ` Nick Piggin
2010-11-15  1:00           ` Dave Chinner
2010-11-15  4:21             ` Nick Piggin
2010-11-16  3:02               ` Dave Chinner
2010-11-16  3:49                 ` Nick Piggin
2010-11-17  1:12                   ` Dave Chinner
2010-11-17  4:18                     ` Nick Piggin
2010-11-17  5:56                       ` Nick Piggin
2010-11-17  6:04                         ` Nick Piggin
2010-11-09 21:44   ` Nick Piggin

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=20101112060202.GB3332@amd \
    --to=npiggin@kernel.dk \
    --cc=eric.dumazet@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=npiggin@gmail.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /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;
as well as URLs for NNTP newsgroup(s).