All of lore.kernel.org
 help / color / mirror / Atom feed
From: "J. R. Okajima" <hooanon05@yahoo.co.jp>
To: Nick Piggin <npiggin@gmail.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>,
	Mark Brown <broonie@opensource.wolfsonmicro.com>,
	Trond Myklebust <Trond.Myklebust@netapp.com>,
	Nick Piggin <npiggin@kernel.dk>,
	linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: vfs-scale, general questions (Re: NFS root lockups with -next 20110113)
Date: Wed, 19 Jan 2011 15:43:03 +0900	[thread overview]
Message-ID: <909.1295419383@jrobl> (raw)
In-Reply-To: <AANLkTim=VY7+fo6d_nUXVxs+iZ_f79qWu_eYMUjvhVJO@mail.gmail.com>


Hi,

Nick Piggin:
> Thanks for your help, can you see how I've fixed it in my vfs-scale
> tree? What do you think?

Your fix is great. I have no objection at all.
Other than the fix, here are more generic questions about vfs-scale work.
I am happy if you reply when you have time.

- getcwd(2) needs d_lock?
  It acquires rename_lock and then tests whether the pwd is removed by
  d_unhashed(). If a race condition between vfs_rename_dir() which may
  unhash/rehash the dentry happens, then getcwd() may return the wrong
  result due to unprotected d_unhashed() call, I am afraid. rename_lock
  doesn't help this case.

- what is the right order of dget() and mntget()?
  If I remember correctly, someone said "mntget() first and then
  dget(). when putting, do in reverse" in the discussion when
  path_{get,put}() were born. So it is called "the right order" in the
  commit log.
  It was many years ago. Is it still true? And should rcu-walk follow it
  too? The current implementation doesn't seem to care about this order.

- d_move() and rename_lock
  This may be out of rcu-walk work, but rename_lock in d_move() looks
  outstanding since it surely kills concurrency. It is a pity that two
  unrelated but concurrent d_move-s are serialized when we run rename(2)
  on two different filesystems. Even if all of dentries, parents and
  hash buckets are different from each other, d_move() never run
  concurrently.


J. R. Okajima

WARNING: multiple messages have this Message-ID (diff)
From: "J. R. Okajima" <hooanon05-/E1597aS9LR3+QwDJ9on6Q@public.gmane.org>
To: Nick Piggin <npiggin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Santosh Shilimkar
	<santosh.shilimkar-l0cyMroinI0@public.gmane.org>,
	Mark Brown
	<broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>,
	Trond Myklebust
	<Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>,
	Nick Piggin <npiggin-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: vfs-scale, general questions (Re: NFS root lockups with -next 20110113)
Date: Wed, 19 Jan 2011 15:43:03 +0900	[thread overview]
Message-ID: <909.1295419383@jrobl> (raw)
In-Reply-To: <AANLkTim=VY7+fo6d_nUXVxs+iZ_f79qWu_eYMUjvhVJO-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>


Hi,

Nick Piggin:
> Thanks for your help, can you see how I've fixed it in my vfs-scale
> tree? What do you think?

Your fix is great. I have no objection at all.
Other than the fix, here are more generic questions about vfs-scale work.
I am happy if you reply when you have time.

- getcwd(2) needs d_lock?
  It acquires rename_lock and then tests whether the pwd is removed by
  d_unhashed(). If a race condition between vfs_rename_dir() which may
  unhash/rehash the dentry happens, then getcwd() may return the wrong
  result due to unprotected d_unhashed() call, I am afraid. rename_lock
  doesn't help this case.

- what is the right order of dget() and mntget()?
  If I remember correctly, someone said "mntget() first and then
  dget(). when putting, do in reverse" in the discussion when
  path_{get,put}() were born. So it is called "the right order" in the
  commit log.
  It was many years ago. Is it still true? And should rcu-walk follow it
  too? The current implementation doesn't seem to care about this order.

- d_move() and rename_lock
  This may be out of rcu-walk work, but rename_lock in d_move() looks
  outstanding since it surely kills concurrency. It is a pity that two
  unrelated but concurrent d_move-s are serialized when we run rename(2)
  on two different filesystems. Even if all of dentries, parents and
  hash buckets are different from each other, d_move() never run
  concurrently.


J. R. Okajima
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2011-01-19  6:43 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-13 12:06 NFS root lockups with -next 20110113 Mark Brown
2011-01-13 13:22 ` J. R. Okajima
2011-01-13 13:28   ` Santosh Shilimkar
2011-01-13 13:28     ` Santosh Shilimkar
2011-01-13 13:45     ` J. R. Okajima
2011-01-13 13:45       ` J. R. Okajima
2011-01-14  3:59       ` Nick Piggin
2011-01-14  3:59         ` Nick Piggin
2011-01-14  4:41         ` J. R. Okajima
2011-01-14  4:41           ` J. R. Okajima
2011-01-19  6:43         ` J. R. Okajima [this message]
2011-01-19  6:43           ` vfs-scale, general questions (Re: NFS root lockups with -next 20110113) J. R. Okajima
2011-01-19  7:21           ` Nick Piggin
2011-01-19  7:21             ` Nick Piggin
2011-01-20  9:05             ` vfs-scale, general questions J. R. Okajima
2011-01-20 11:15               ` Miklos Szeredi
2011-01-21  6:38                 ` J. R. Okajima
2011-02-11  3:49           ` vfs-scale, general questions (Re: NFS root lockups with -next 20110113) Ian Kent
2011-02-11  3:49             ` Ian Kent
2011-02-13  2:19             ` J. R. Okajima
2011-01-13 13:35   ` NFS root lockups with -next 20110113 Mark Brown
2011-01-13 13:35     ` Mark Brown
2011-01-13 13:35     ` Mark Brown
2011-01-13 13:41   ` Santosh Shilimkar
2011-01-13 13:41     ` Santosh Shilimkar
2011-01-13 13:41     ` Santosh Shilimkar
2011-01-13 13:41     ` Santosh Shilimkar

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=909.1295419383@jrobl \
    --to=hooanon05@yahoo.co.jp \
    --cc=Trond.Myklebust@netapp.com \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=npiggin@gmail.com \
    --cc=npiggin@kernel.dk \
    --cc=santosh.shilimkar@ti.com \
    /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.