Re: [PATCH] xfs: improve handling of prjquot ENOSPC

[Jian Wen <wenjianhn@gmail.com>]

On Fri, Dec 15, 2023 at 5:13 AM Dave Chinner <david@fromorbit.com> wrote:
>
> On Thu, Dec 14, 2023 at 11:07:08PM +0800, Jian Wen wrote:
> > Don't clear space of the whole fs when the project quota limit is
> > reached, since it affects the writing performance of files of
> > the directories that are under quota.
> >
> > Only run cow/eofblocks scans on the quota attached to the inode.
> >
> > Signed-off-by: Jian Wen <wenjian1@xiaomi.com>
> > ---
> >  fs/xfs/xfs_file.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > index e33e5e13b95f..4fbe262d33cc 100644
> > --- a/fs/xfs/xfs_file.c
> > +++ b/fs/xfs/xfs_file.c
> > @@ -24,6 +24,9 @@
> >  #include "xfs_pnfs.h"
> >  #include "xfs_iomap.h"
> >  #include "xfs_reflink.h"
> > +#include "xfs_quota.h"
> > +#include "xfs_dquot_item.h"
> > +#include "xfs_dquot.h"
> >
> >  #include <linux/dax.h>
> >  #include <linux/falloc.h>
> > @@ -803,8 +806,18 @@ xfs_file_buffered_write(
> >               goto write_retry;
> >       } else if (ret == -ENOSPC && !cleared_space) {
> >               struct xfs_icwalk       icw = {0};
> > +             struct xfs_dquot        *pdqp = ip->i_pdquot;
> >
> >               cleared_space = true;
> > +             if (XFS_IS_PQUOTA_ENFORCED(ip->i_mount) &&
> > +                     pdqp && xfs_dquot_lowsp(pdqp)) {
> > +                     xfs_iunlock(ip, iolock);
> > +                     icw.icw_prid = pdqp->q_id;
> > +                     icw.icw_flags |= XFS_ICWALK_FLAG_PRID;
> > +                     xfs_blockgc_free_space(ip->i_mount, &icw);
> > +                     goto write_retry;
> > +             }
>
> This is just duplicating the EDQUOT error handling path for the
> specific case that project quota exhaustion returns ENOSPC instead
> of EDQUOT.  i.e. the root cause of the problem is that project
> quotas are returning ENOSPC rather than EDQUOT, right?
>
Yes, it is.
> Perhaps we should look at having project quotas return EDQUOT like
> the other quotas so we get the project quota block scan done in the
> correct places, then convert the error to ENOSPC if we get a second
> EDQUOT from the project quota on retry?
>
I did so by only returning EDQUOT in xfs_trans_dqresv(), it made error
handling more complex.

And after we get a second EDQUOT, we still need to check if it is
project quota that is over limit.
> -Dave.
> --
> Dave Chinner
> david@fromorbit.com



-- 
Best,

Jian