On Thu, Dec 12, 2013 at 08:40:58PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> 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@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
