public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Nick Piggin <npiggin@suse.de>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 5/6] [XFS] Replace per-ag array with a radix tree
Date: Mon, 14 Dec 2009 15:16:16 +1100	[thread overview]
Message-ID: <20091214041615.GB9208@nick> (raw)
In-Reply-To: <20091211114627.GB5436@infradead.org>

On Fri, Dec 11, 2009 at 06:46:27AM -0500, Christoph Hellwig wrote:
> On Fri, Dec 11, 2009 at 11:43:53AM +1100, Dave Chinner wrote:
> > > > +	spin_lock(&mp->m_perag_lock);
> > > > +	pag = radix_tree_lookup(&mp->m_perag_tree, agno);
> > > > +	spin_unlock(&mp->m_perag_lock);
> > > > +	return pag;
> > > 
> > > Can't we do this as a lock-less (at least for lookups) radix tree?
> > 
> > I think it can be (RCU-based?) , but I think that makes sense as a
> > followup optimisation once we have confidence the code is working
> > as it should.
> 
> Nick, what are the rules for the lock-less radix tree reader side?

OK, well basically we can do a radix_tree_lookup and get the pointer
without locking. In order to be able to follow the pointer to the
object of course the calling code needs to do its own synchronization
(eg. objects might be RCU protected as well so it can be used or a
refcount can be taken on it under rcu_read_lock).

A single lookup basically has concurrency semantics as though it may
have been performed before or after any concurrent modifications. This
is really just the same as:

spin_lock()
ptr = radix_tree_lookup()
spin_unlock()
/* use the ptr */

Because before the lock is taken and after it is released, we can get
any kind of interleaving of modifications anyway, so by the time we
use the pointer then regardless of whether we use lock or RCU, then the
tree may have been modified anyway.

This looks like the pattern used in Dave's patch.

If you need any kind of atomic multiple lookups (including gang lookups)
or atomic lookups and modifications, then you'll need a lock.


> Dave is introducing a radix tree in XFS which has the following access
> pattern:
> 
>  - lots of read side access during normal fs operations
>  - insertations currently only happen during mount before the fs is life
>    and during a very rare operation (filesystem resize)
>  - currently items are never deleted, but we might need that in the
>    future (for filesystem shrink support)
>  - the objects pointed to are kmalloced and refcounted structures,
>    but we don't even strictly need the refcounting until the filesystem
>    shrink support is implemented

This sounds like it would be perfectly suited to RCU lookups.

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

  reply	other threads:[~2009-12-15 10:49 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-02  6:11 [PATCH 0/6] [XFS] Fix growfs deadlocks and per-AG use after free Dave Chinner
2009-12-02  6:11 ` [PATCH 1/6] [XFS] rename xfs_get_perag Dave Chinner
2009-12-10 23:15   ` Christoph Hellwig
2009-12-10 23:25     ` Dave Chinner
2009-12-02  6:11 ` [PATCH 2/6] [XFS] Don't directly reference m_perag in allocation code Dave Chinner
2009-12-10 23:18   ` Christoph Hellwig
2009-12-10 23:41     ` Dave Chinner
2009-12-02  6:11 ` [PATCH 3/6] [XFS] Convert filestreams code to use per-ag get/put routines Dave Chinner
2009-12-10 23:22   ` Christoph Hellwig
2009-12-02  6:11 ` [PATCH 4/6] [XFS] convert remaining direct references to m_perag Dave Chinner
2009-12-10 23:24   ` Christoph Hellwig
2009-12-02  6:11 ` [PATCH 5/6] [XFS] Replace per-ag array with a radix tree Dave Chinner
2009-12-10 23:45   ` Christoph Hellwig
2009-12-11  0:43     ` Dave Chinner
2009-12-11 11:43       ` Christoph Hellwig
2009-12-11 11:46       ` Christoph Hellwig
2009-12-14  4:16         ` Nick Piggin [this message]
2009-12-02  6:11 ` [PATCH 6/6] [XFS] Reference count per-ag structures Dave Chinner
2009-12-10 23:48   ` Christoph Hellwig
2009-12-11  0:50     ` Dave Chinner
  -- strict thread matches above, loose matches on Subject: below --
2009-12-14 23:11 [PATCH 0/6] XFS: Fix growfs deadlocks and per-AG use after free V2 Dave Chinner
2009-12-14 23:11 ` [PATCH 5/6] XFS: Replace per-ag array with a radix tree Dave Chinner
2009-12-15  6:11 [PATCH 0/6] XFS: Fix growfs deadlocks and per-AG use after free V3 Dave Chinner
2009-12-15  6:11 ` [PATCH 5/6] XFS: Replace per-ag array with a radix tree Dave Chinner
2009-12-23 16:02   ` Christoph Hellwig
2009-12-23 22:08   ` Alex Elder
2009-12-26  4:17     ` 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=20091214041615.GB9208@nick \
    --to=npiggin@suse.de \
    --cc=hch@infradead.org \
    --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