Re: [PATCH 1/2] xfs: reserve quota for dir expansion when linking/unlinking files

[Dave Chinner <david@fromorbit.com>]

On Wed, Mar 09, 2022 at 03:33:02PM -0800, Darrick J. Wong wrote:
> On Thu, Mar 10, 2022 at 08:48:21AM +1100, Dave Chinner wrote:
> > On Wed, Mar 09, 2022 at 11:22:26AM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > >  		if (error)
> > >  			goto error_return;
> > >  	}
> > > @@ -2755,6 +2749,7 @@ xfs_remove(
> > >  	xfs_mount_t		*mp = dp->i_mount;
> > >  	xfs_trans_t             *tp = NULL;
> > >  	int			is_dir = S_ISDIR(VFS_I(ip)->i_mode);
> > > +	int			dontcare;
> > >  	int                     error = 0;
> > >  	uint			resblks;
> > >  
> > > @@ -2781,22 +2776,13 @@ xfs_remove(
> > >  	 * block from the directory.
> > >  	 */
> > >  	resblks = XFS_REMOVE_SPACE_RES(mp);
> > > -	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_remove, resblks, 0, 0, &tp);
> > > -	if (error == -ENOSPC) {
> > > -		resblks = 0;
> > > -		error = xfs_trans_alloc(mp, &M_RES(mp)->tr_remove, 0, 0, 0,
> > > -				&tp);
> > > -	}
> > > +	error = xfs_trans_alloc_dir(dp, &M_RES(mp)->tr_remove, ip, &resblks,
> > > +			&tp, &dontcare);
> > >  	if (error) {
> > >  		ASSERT(error != -ENOSPC);
> > >  		goto std_return;
> > >  	}
> > 
> > So we just ignore -EDQUOT when it is returned in @dontcare? I'd like
> > a comment to explain why we don't care about EDQUOT here, because
> > the next time I look at this I will have forgotten all about this...
> 
> Ok.  How about:
> 
> 	/*
> 	 * We try to get the real space reservation first, allowing for
> 	 * directory btree deletion(s) implying possible bmap insert(s).
> 	 * If we can't get the space reservation then we use 0 instead,
> 	 * and avoid the bmap btree insert(s) in the directory code by,
> 	 * if the bmap insert tries to happen, instead trimming the LAST
> 	 * block from the directory.
> 	 *
> 	 * Ignore EDQUOT and ENOSPC being returned via nospace_error
> 	 * because the directory code can handle a reservationless
> 	 * update and we don't want to prevent a user from trying to
> 	 * free space by deleting things.
> 	 */
> 	error = xfs_trans_alloc_dir(...);

Yeah, that looks good.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com