Re: [PATCH v5 4/5] xfs: support shrinking unused space in the last AG

[Gao Xiang <hsiangkao@redhat.com>]

On Thu, Jan 21, 2021 at 04:22:59AM +0800, Gao Xiang wrote:

... (cont..)

> 
> > 
> > > +		err2 = xfs_ag_resv_init(agibp->b_pag, tp);
> > > +		if (err2)
> > > +			goto resv_err;
> > > +		return error;
> > > +	}
> > > +
> > > +	/*
> > > +	 * if successfully deleted from freespace btrees, need to confirm
> > > +	 * per-AG reservation works as expected.
> > > +	 */
> > > +	be32_add_cpu(&agi->agi_length, -len);
> > > +	be32_add_cpu(&agf->agf_length, -len);
> > > +
> > > +	err2 = xfs_ag_resv_init(agibp->b_pag, tp);
> > > +	if (err2) {
> > > +		be32_add_cpu(&agi->agi_length, len);
> > > +		be32_add_cpu(&agf->agf_length, len);
> > > +		if (err2 != -ENOSPC)
> > > +			goto resv_err;
> > 
> > If we've just undone reducing ag[if]_length, don't we need to call
> > xfs_ag_resv_init here to (try to) recreate the former per-ag
> > reservations?
> 
> If my understanding is correct, xfs_fs_reserve_ag_blocks() in
> xfs_growfs_data_private() would do that for all AGs... Do we
> need to xfs_ag_resv_init() in advance here?
> 
> I thought xfs_ag_resv_init() here is mainly used to guarantee the
> per-AG reservation for resized size is fine... if ag{i,f}_length
> don't change, leave such normal reservation to
> xfs_fs_reserve_ag_blocks() would be okay?
>

Although When xfs_fs_reserve_ag_blocks(), the transaction has already
been  commited, the last AG is unlocked. So there might be some race
window here... So I will update it, thanks for this!

Thanks,
Gao Xiang