public inbox for linux-xfs@vger.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox