On Thu, Jan 4, 2024 at 9:46 AM Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
> > > > diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
> > > > index 80c8f851a2f3..c5f4a170eef1 100644
> > > > --- a/fs/xfs/xfs_dquot.h
> > > > +++ b/fs/xfs/xfs_dquot.h
> > > > @@ -183,6 +183,19 @@ xfs_dquot_is_enforced(
> > > > return false;
> > > > }
> > > >
> > > > +static inline bool
> > > > +xfs_dquot_is_enospc(
> > >
> > > I don't like encoding error codes in a function name, especially since
> > > EDQUOT is used for more dquot types than ENOSPC.
> > >
> > > "xfs_dquot_hardlimit_exceeded" ?
> > >
> > > > + struct xfs_dquot *dqp)
> > > > +{
> > > > + if (!dqp)
> > > > + return false;
> > > > + if (!xfs_dquot_is_enforced(dqp))
> > > > + return false;
> > > > + if (dqp->q_blk.hardlimit - dqp->q_blk.reserved > 0)
> > > > + return false;
> > >
> > > return q_blk.reserved > dqp->q_blk.hardlimit; ?
> > >
> > > hardlimit == reserved shouldn't be considered an edquot condition.
> > >
> > > Also, locking is needed here.
>
> Any response to this?
I will address it in v4.
> > >
> > > Also, a question for Dave: What happens if xfs_trans_dqresv detects a
> > > fatal overage in the project dquot, but the overage condition clears by
> > > the time this caller rechecks the dquot? Is it ok that we then return
> > > EDQUOT whereas the current code would return ENOSPC?
The v3 patch will return EDQUOT if the condition clears. e.g.
STATIC ssize_t
xfs_file_buffered_write(
...
if (ret == -EDQUOT && xfs_dquot_is_enospc(ip->i_pdquot))
ret = -ENOSPC;
ret will not be translated from EDQUOT to ENOSPC.
>
> I think this question is still relevant, though. Or perhaps we should
> define our own code for project quota exceeded, and translate that to
> ENOSPC in the callers?
>
> I wonder, what about the xfs_trans_reserve_quota_nblks in
> xfs_reflink_remap_extent? Does it need to filter EDQUOT?
Yes, it does. I will address it in v4.
>
> Just looking through the list, I think xfs_ioctl_setattr_get_trans and
> xfs_setattr_nonsize also need to check for EDQUOT and project dquots
> being over, don't they?
Yes, xfs_trans_alloc_ichange has got them covered.
