On Wed, Feb 03, 2021 at 01:01:40PM -0500, Brian Foster wrote:

On Wed, Feb 03, 2021 at 11:01:32PM +0800, Gao Xiang wrote:

...

quoted

quoted

quoted

@@ -559,6 +560,10 @@ xfs_ag_shrink_space(
 	be32_add_cpu(&agf->agf_length, -len);
 
 	err2 = xfs_ag_resv_init(agibp->b_pag, *tpp);
+
+	if (XFS_TEST_ERROR(false, mp, XFS_ERRTAG_SHRINKFS_AG_RESV_FAIL))
+		err2 = -ENOSPC;
+

Seems reasonable, but I feel like this could be broadened to serve as a
generic perag reservation error tag. I suppose we might not be able to
use it on a clean mount, but perhaps it could be reused for growfs and
remount. Hm?

I think it could be done in that way, yet currently the logic is just to
verify the shrink error handling case above rather than extend to actually
error inject per-AG reservation for now... I could rename the errortag
for later reuse (some better naming? I'm not good at this...) in advance
yet real per-AG reservation error injection might be more complicated
than just error out with -ENOSPC, and it's somewhat out of scope of this
patchset for now...

I don't think it needs to be any more complicated than the logic you
have here. Just bury it further down in in the perag res init code,
rename it to something like ERRTAG_AG_RESV_FAIL, and use it the exact
same way for shrink testing. For example, maybe drop it into
__xfs_ag_resv_init() near the xfs_mod_fdblocks() call so we can also
take advantage of the tracepoint that triggers on -ENOSPC for
informational purposes:

	error = xfs_mod_fdblocks(...);
	if (!error && XFS_TEST_ERROR(false, mp, XFS_ERRTAG_AG_RESV_FAIL))
		error = -ENOSPC;
	if (error) {
		...
	}

Ok, I didn't look into much more about it since it's out of scope. I'd
try in the next version.

Thanks,
Gao Xiang

Brian

quoted

Thanks,
Gao Xiang

quoted

Brian