All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	eric.dumazet@gmail.com, xfs@oss.sgi.com
Subject: Re: [PATCH 07/16] xfs: convert inode cache lookups to use RCU locking
Date: Tue, 9 Nov 2010 16:04:17 +1100	[thread overview]
Message-ID: <20101109050417.GI2715@dastard> (raw)
In-Reply-To: <20101109033628.GN4032@linux.vnet.ibm.com>

On Mon, Nov 08, 2010 at 07:36:28PM -0800, Paul E. McKenney wrote:
> On Mon, Nov 08, 2010 at 06:09:29PM -0500, Christoph Hellwig wrote:
> > This patch generally looks good to me, but with so much RCU magic I'd prefer
> > if Paul & Eric could look over it.
> 
> Is there a git tree, tarball, or whatever? 

git://git.kernel.org/pub/scm/linux/kernel/git/dgc/xfsdev.git working

contains the series that this patch is in.

> For example, I don't see
> how this patch handles the case of an inode being freed just as an RCU
> reader gains a reference to it,

XFS_IRECLAIM flag is set on inodes as they transition into the
reclaim state long before they are freed. The XFS_IRECLAIM flag is left there once
freed. Hence lookups in xfs_iget_cache_hit() will see this.

If the inode has been reallocated, the inode number will not yet be
set, or the inode state will have changed to XFS_INEW, both of which
xfs_iget_cache_hit() will also reject.

> but then reallocated as some other inode
> (so that ->ino is nonzero) before the RCU reader gets a chance to actually
> look at the inode.

XFS_INEW is not cleared until well after a new ->i_ino is set, so
the lookup should find trip over XFS_INEW in that case. I think that
I may need to move the inode number check under the i_flags_lock
after validating the flags - more to check that we've got the
correct inode than to validate we have a freed inode.

> But such a check might well be in the code that this
> patch didn't change...

Yeah, most of the XFS code is already in a form compatible with such
RCU use because inodes have always had a quiescent "reclaimable"
state between active and reclaim (XFS_INEW -> active ->
XFS_IRECLAIMABLE -> XFS_IRECLAIM) where the inode can be reused
before being freed. The result is that lookups have always had to
handle races with inodes that have just transitioned into the
XFS_IRECLAIM state and hence cannot be immediately reused...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2010-11-09  5:02 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-08  8:55 [PATCH 00/16] xfs: current patch stack for 2.6.38 window Dave Chinner
2010-11-08  8:55 ` [PATCH 01/16] xfs: fix per-ag reference counting in inode reclaim tree walking Dave Chinner
2010-11-08  9:23   ` Christoph Hellwig
2010-11-08  8:55 ` [PATCH 02/16] xfs: move delayed write buffer trace Dave Chinner
2010-11-08  9:24   ` Christoph Hellwig
2010-11-08  8:55 ` [PATCH 03/16] [RFC] xfs: use generic per-cpu counter infrastructure Dave Chinner
2010-11-08 12:13   ` Christoph Hellwig
2010-11-09  0:20     ` Dave Chinner
2010-11-08  8:55 ` [PATCH 04/16] xfs: dynamic speculative EOF preallocation Dave Chinner
2010-11-08 11:43   ` Christoph Hellwig
2010-11-09  0:08     ` Dave Chinner
2010-11-08  8:55 ` [PATCH 05/16] xfs: don't truncate prealloc from frequently accessed inodes Dave Chinner
2010-11-08 11:36   ` Christoph Hellwig
2010-11-08 23:56     ` Dave Chinner
2010-11-08  8:55 ` [PATCH 06/16] patch xfs-inode-hash-fake Dave Chinner
2010-11-08  9:19   ` Christoph Hellwig
2010-11-08  8:55 ` [PATCH 07/16] xfs: convert inode cache lookups to use RCU locking Dave Chinner
2010-11-08 23:09   ` Christoph Hellwig
2010-11-09  0:24     ` Dave Chinner
2010-11-09  3:36     ` Paul E. McKenney
2010-11-09  5:04       ` Dave Chinner [this message]
2010-11-10  5:12         ` Paul E. McKenney
2010-11-10  6:20           ` Dave Chinner
2010-11-08  8:55 ` [PATCH 08/16] xfs: convert pag_ici_lock to a spin lock Dave Chinner
2010-11-08 23:10   ` Christoph Hellwig
2010-11-08  8:55 ` [PATCH 09/16] xfs: convert xfsbud shrinker to a per-buftarg shrinker Dave Chinner
2010-11-08  8:55 ` [PATCH 10/16] xfs: add a lru to the XFS buffer cache Dave Chinner
2010-11-08 23:19   ` Christoph Hellwig
2010-11-08 23:45     ` Dave Chinner
2010-11-08  8:55 ` [PATCH 11/16] xfs: connect up buffer reclaim priority hooks Dave Chinner
2010-11-08 11:25   ` Christoph Hellwig
2010-11-08 23:50     ` Dave Chinner
2010-11-08  8:55 ` [PATCH 12/16] xfs: bulk AIL insertion during transaction commit Dave Chinner
2010-11-08  8:55 ` [PATCH 13/16] xfs: reduce the number of AIL push wakeups Dave Chinner
2010-11-08 11:32   ` Christoph Hellwig
2010-11-08 23:51     ` Dave Chinner
2010-11-08  8:55 ` [PATCH 14/16] xfs: remove all the inodes on a buffer from the AIL in bulk Dave Chinner
2010-11-08  8:55 ` [PATCH 15/16] xfs: only run xfs_error_test if error injection is active Dave Chinner
2010-11-08 11:33   ` Christoph Hellwig
2010-11-08  8:55 ` [PATCH 16/16] xfs: make xlog_space_left() independent of the grant lock Dave Chinner
2010-11-08 14:17 ` [PATCH 00/16] xfs: current patch stack for 2.6.38 window Christoph Hellwig
2010-11-09  0:21   ` Dave Chinner

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=20101109050417.GI2715@dastard \
    --to=david@fromorbit.com \
    --cc=eric.dumazet@gmail.com \
    --cc=hch@infradead.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=xfs@oss.sgi.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.