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

... (cont..)

quoted

quoted

+		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