From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 3/3] xfs: xfs_trans_dqresv() can be made lockless
Date: Fri, 13 Dec 2013 05:37:45 -0800 [thread overview]
Message-ID: <20131213133745.GC13689@infradead.org> (raw)
In-Reply-To: <1386841258-22183-4-git-send-email-david@fromorbit.com>
On Thu, Dec 12, 2013 at 08:40:58PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> xfs_trans_dqresv() serialises dquot modifications by taking the
> dquot lock while it is doing reservations. The thing is, nothing it
> does really requires exclusive access to the dquot except for the
> reservation accounting. We can do that locklessly with cmpxchg.
Can you split the various refactorings into separate patches to make
this more readable?
> +do_ninos:
> + if (ninos == 0)
> + goto do_trans;
> +
> + smp_mb();
> + timer = be32_to_cpu(dqp->q_core.d_itimer);
> + warns = be16_to_cpu(dqp->q_core.d_iwarns);
> + warnlimit = dqp->q_mount->m_quotainfo->qi_iwarnlimit;
> + hardlimit = be64_to_cpu(dqp->q_core.d_ino_hardlimit);
> + if (!hardlimit)
> + hardlimit = q->qi_ihardlimit;
> + softlimit = be64_to_cpu(dqp->q_core.d_ino_softlimit);
> + if (!softlimit)
> + softlimit = q->qi_isoftlimit;
> + resbcountp = &dqp->q_res_icount;
> +
> + oldcnt = xfs_dqresv_cmpxchg(mp, dqp, resbcountp, ninos, false, enforce,
> + hardlimit, softlimit, timer, warns,
> + warnlimit);
> + if (oldcnt == (xfs_qcnt_t)-1ULL)
> + goto error_undo_nblks;
> +
> +do_trans:
Instead of having all these goto labels maye this should be factored
into helpers for each of the stages?
> if (udqp) {
> + enforce = !(flags & XFS_QMOPT_FORCE_RES) &&
> + udqp->q_core.d_id && XFS_IS_UQUOTA_ENFORCED(mp);
> error = xfs_trans_dqresv(tp, mp, udqp, nblks, ninos,
> - (flags & ~XFS_QMOPT_ENOSPC));
> + (flags & ~XFS_QMOPT_ENOSPC), enforce);
I have to say I'd much prefer having the enforcement decision hidden
inside xfs_trans_dqresv.
> if (error)
> return error;
> }
>
> if (gdqp) {
> - error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos, flags);
> + enforce = !(flags & XFS_QMOPT_FORCE_RES) &&
> + gdqp->q_core.d_id && XFS_IS_GQUOTA_ENFORCED(mp);
> + error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos, flags,
> + enforce);
> if (error)
Unrelated to the patch: why do we clear XFS_QMOPT_ENOSPC for user
quotas, but not for project quotas here?
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2013-12-13 13:38 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 [this message]
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
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=20131213133745.GC13689@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