From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 4/3] xfs: xfs_qm_dqrele mostly doesn't need locking
Date: Fri, 13 Dec 2013 05:28:07 -0800 [thread overview]
Message-ID: <20131213132807.GB13689@infradead.org> (raw)
In-Reply-To: <20131212102507.GX10988@dastard>
On Thu, Dec 12, 2013 at 09:25:07PM +1100, Dave Chinner wrote:
>
> From: Dave Chinner <dchinner@redhat.com>
>
> Now that we have an atomic variable for the reference count, we
> don't need to take the dquot lock if we are not removing the last
> reference count. The dquot lock is a mutex, so we can't use
> atomic_dec_and_lock(), but we can open code it in xfs_qm_dqrele and
> hence avoid the dquot lock for most of the cases where we drop a
> reference count.
>
> The result is that concurrent file creates jump from 24,000/s to
> 28,000/s, and the entire workload is now serialised on the dquot
> being locked during transaction commit. Another significant win,
> even though it's not the big one...
Maybe I'm missing something, but shou;dn't the following be enough to
be a valid dqput (plus asserts & tracing):
if (atomic_dec_and_test(&dqp->q_nrefs)) {
if (list_lru_add(&mp->m_quotainfo->qi_lru, &dqp->q_lru))
XFS_STATS_INC(xs_qm_dquot_unused);
}
given that the only locking we need is the internal lru lock?
>
> While there, rename xfs_qm_dqrele to xfs_dqrele - the "qm" part of
> the name means nothing and just makes the code harder to read.
Please keep that out of the patch. I don't mind dropping the
qm_ part, but there's a lot of functions that have it, and it should
be done for all of them at the same time.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2013-12-13 13:30 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-12 9:40 [PATCH 0/3] xfs: dquot modification scalability Dave Chinner
2013-12-12 9:40 ` [PATCH 1/3] xfs: remote dquot hints Dave Chinner
2013-12-12 18:33 ` Christoph Hellwig
2013-12-12 9:40 ` [PATCH 2/3] xfs: dquot refcounting by atomics Dave Chinner
2013-12-13 13:23 ` Christoph Hellwig
2013-12-12 9:40 ` [PATCH 3/3] xfs: xfs_trans_dqresv() can be made lockless Dave Chinner
2013-12-13 13:37 ` Christoph Hellwig
2013-12-16 0:11 ` Dave Chinner
2013-12-12 10:25 ` [PATCH 4/3] xfs: xfs_qm_dqrele mostly doesn't need locking Dave Chinner
2013-12-13 13:28 ` Christoph Hellwig [this message]
2013-12-13 21:30 ` Dave Chinner
2013-12-16 18:21 ` Christoph Hellwig
2013-12-13 16:30 ` [PATCH 5/3] xfs: return unlocked dquots from xfs_qm_dqqet Christoph Hellwig
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=20131213132807.GB13689@infradead.org \
--to=hch@infradead.org \
--cc=david@fromorbit.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