All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Mason <chris.mason@oracle.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Jeff Moyer <jmoyer@redhat.com>,
	npiggin@kernel.dk
Subject: Re: aio: bump i_count instead of using igrab
Date: Mon, 23 Aug 2010 11:00:23 -0400	[thread overview]
Message-ID: <20100823150023.GR21975@think> (raw)
In-Reply-To: <20100823145031.GA1279@infradead.org>

On Mon, Aug 23, 2010 at 10:50:31AM -0400, Christoph Hellwig wrote:
> On Mon, Aug 23, 2010 at 10:47:55AM -0400, Chris Mason wrote:
> > The aio batching code is using igrab to get an extra reference on the
> > inode so it can safely batch.  igrab will go ahead and take the global
> > inode spinlock, which can be a bottleneck on large machines doing lots
> > of AIO.
> > 
> > In this case, igrab isn't required because we already have a reference
> > on the file handle.  It is safe to just bump the i_count directly
> > on the inode.
> > 
> > Benchmarking shows this patch brings IOP/s on tons of flash up by about
> > 2.5X.
> 
> There's some places in XFS where we do the same, and it showed up as a
> bottle neck before.  Instead of open coding the increment we have
> a wrapper that includes and assert that the numbers is always positive.
> 
> I think we really want a proper helper for general use instead of
> completly opencoding it.
> 

Nick, this is about a 1 liner to fs/aio.c replacing igrab with
atomic_inc directly on the inode reference count.

I know your scalability tree gets rid of the global, but in this case I
think it still makes sense to avoid the locking completely when the
caller knows it is safe.  Do you already have something similar hiding
in the scalability tree?

-chris

  reply	other threads:[~2010-08-23 15:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-23 14:47 aio: bump i_count instead of using igrab Chris Mason
2010-08-23 14:50 ` Christoph Hellwig
2010-08-23 15:00   ` Chris Mason [this message]
2010-08-23 17:26     ` Jeff Moyer
2010-08-24  7:33     ` Nick Piggin
2010-08-23 21:18   ` Jeff Moyer

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=20100823150023.GR21975@think \
    --to=chris.mason@oracle.com \
    --cc=hch@infradead.org \
    --cc=jmoyer@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=npiggin@kernel.dk \
    /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.