Re: [PATCH v2 2/3] xfs: fix infinite loop by detaching the group/project hints from user dquot

[Ben Myers <bpm@sgi.com>]

Hey Jeff,

On Fri, Nov 29, 2013 at 05:36:01PM +0800, Jeff Liu wrote:
> On 11/28 2013 18:43 PM, Christoph Hellwig wrote:
> > On Tue, Nov 26, 2013 at 09:38:49PM +0800, Jeff Liu wrote:
> >> +	if (flags & XFS_QMOPT_UQUOTA)
> >> +		return xfs_qm_dqpurge(dqp, NULL);
> > 
> > To me it doesn't make any sense to overload this function for the user
> > quotas that don't have hints.
> To me it would like a silly compromise.
> > 
> > I'd suggest dropping this hunk and keeping a separate walk for
> > releasing the uquots.
> I thought this over and yup, that is an overload if neither group nor project
> are enabled, or we don't want to turn user quota off.
> 
> But even so, we currently also have overloads by checking group/project hints
> before releasing any type of quota in xfs_qm_purge().  In this point, this fix
> can reduce a bit overloads by moving those checkups to xfs_qm_purge_hints() if
> we want to turn group/project quotas off.
> 
> If we considering to drop above hunk to release user quota separately, we finally
> would have to walk through user quota to remove those hints again, i.e,
> 
> /* Remove group/project hints from user dquot */
> STATIC int
> xfs_qm_dqpurge_hints(
> 	struct xfs_dquot        *dqp,
> 	void                    *data)
> {
> 	uint                    flags = *((uint *)data);
> 	struct xfs_dquot        *gdqp;
> 	struct xfs_dquot        *pdqp;
> 
> 	xfs_dqlock(dqp);
> 	if (dqp->dq_flags & XFS_DQ_FREEING) {
> 		xfs_dqunlock(dqp);
> 		return EAGAIN;
> 	}
> 
> 	/* If this quota has a hint attached, prepare for releasing it now */
>         gdqp = dqp->q_gdquot;
> 	if (gdqp)
> 		dqp->q_gdquot = NULL;
> 
> 	pdqp = dqp->q_pdquot;
> 	if (pdqp)
> 		dqp->q_pdquot = NULL;
> 
> 	xfs_dqunlock(dqp);
> 
> 	if (gdqp)
> 		xfs_qm_dqrele(gdqp);
> 	if (pdqp)
> 		xfs_qm_dqrele(pdqp);
> 
> 	return 0;
> }
> 
> void
> xfs_qm_dqpurge_all()
> {
> 	xfs_qm_dquot_walk(mp, XFS_DQ_USER, xfs_qm_dqpurge_hints, NULL);
> 
> 	if (flags & XFS_QMOPT_UQUOTA)
> 		xfs_qm_dquot_walk(mp, XFS_DQ_USER, xfs_qm_dqpurge, NULL);
> 	if (flags & XFS_QMOPT_GQUOTA)
> 		xfs_qm_dquot_walk(mp, XFS_DQ_GROUP, xfs_qm_dqpurge, NULL);
> 	if (flags & XFS_QMOPT_PQUOTA)
> 		xfs_qm_dquot_walk(mp, XFS_DQ_PROJ, xfs_qm_dqpurge, NULL);
> }
> 
> Above code is what I can figured out as per your suggestions for now, but it
> would introduce overheads for walking through user dquots to release hints
> separately if we want to turn user quota off.
> 
> Any thoughts?

I was gonna pull in the single walk version, but now I realize that it is still
under discussion.  I'm happy with either implementation, with maybe a slight
preference for a single user quota walk.  Can you and Christoph come to an
agreement?

Thanks,
	Ben

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