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:
...
> > > > @@ -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
>
> > Thanks,
> > Gao Xiang
> >
> > >
> > > Brian
> >
>
