On Wed, Nov 13, 2024 at 09:05:15AM +1100, Dave Chinner wrote:
> From: Supriya Wickrematillake <sup@xxxxxxx>
Wow, there are still people working at SGI??
> I've been seeing this failure on during xfs/050 recently:
>
> XFS: Assertion failed: dst->d_spc_timer != 0, file: fs/xfs/xfs_qm_syscalls.c, line: 435
> ....
> Call Trace:
> <TASK>
> xfs_qm_scall_getquota_fill_qc+0x2a2/0x2b0
> xfs_qm_scall_getquota_next+0x69/0xa0
> xfs_fs_get_nextdqblk+0x62/0xf0
> quota_getnextxquota+0xbf/0x320
> do_quotactl+0x1a1/0x410
> __se_sys_quotactl+0x126/0x310
> __x64_sys_quotactl+0x21/0x30
> x64_sys_call+0x2819/0x2ee0
> do_syscall_64+0x68/0x130
> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> It turns out that the _qmount call has silently been failing to
> unmount and mount the filesystem, so when the softlimit is pushed
> past with a buffered write, it is not getting synced to disk before
> the next quota report is being run.
>
> Hence when the quota report runs, we have 300 blocks of delalloc
> data on an inode, with a soft limit of 200 blocks. XFS dquots
> account delalloc reservations as used space, hence the dquot is over
> the soft limit.
>
> However, we don't update the soft limit timers until we do a
> transactional update of the dquot. That is, the dquot sits over the
> soft limit without a softlimit timer being started until writeback
> occurs and the allocation modifies the dquot and we call
> xfs_qm_adjust_dqtimers() from xfs_trans_apply_dquot_deltas() in
> xfs_trans_commit() context.
>
> This isn't really a problem, except for this debug code in
> xfs_qm_scall_getquota_fill_qc():
>
> #ifdef DEBUG
> if (xfs_dquot_is_enforced(dqp) && dqp->q_id != 0) {
> if ((dst->d_space > dst->d_spc_softlimit) &&
> (dst->d_spc_softlimit > 0)) {
> ASSERT(dst->d_spc_timer != 0);
> }
> ....
>
> It asserts taht if the used block count is over the soft limit,
> it *must* have a soft limit timer running. This is clearly not
> the case, because we haven't committed the delalloc space to disk
> yet. Hence the soft limit is only exceeded temporarily in memory
> (which isn't an issue) and we start the timer the moment we exceed
> the soft limit in journalled metadata.
>
> This debug was introduced in:
>
> commit 0d5ad8383061fbc0a9804fbb98218750000fe032
> Author: Supriya Wickrematillake <sup@xxxxxxx>
> Date: Wed May 15 22:44:44 1996 +0000
>
> initial checkin
> quotactl syscall functions.
>
> The very first quota support commit back in 1996. This is zero-day
> debug for Irix and, as it turns out, a zero-day bug in the debug
> code because the delalloc code on Irix didn't update the softlimit
> timers, either.
>
> IOWs, this issue has been in the code for 28 years.
>
> We obviously don't care if soft limit timers are a bit rubbery when
> we have delalloc reservations in memory. Production systems running
> quota reports have been exposed to this situation for 28 years and
> nobody has noticed it, so the debug code is essentially worthless at
> this point in time.
>
> We also have the on-disk dquot verifiers checking that the soft
> limit timer is running whenever the dquot is over the soft limit
> before we write it to disk and after we read it from disk. These
> aren't firing, so it is clear the issue is purely a temporary
> in-memory incoherency that I never would have noticed had the test
> not silently failed to unmount the filesystem.
>
> Hence I'm simply going to trash this runtime debug because it isn't
> useful in the slightest for catching quota bugs.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Agreed! I've hit this once in a blue moon and didn't think it was
especially useful either.
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_qm_syscalls.c | 13 -------------
> 1 file changed, 13 deletions(-)
>
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index 4eda50ae2d1c..0c78f30fa4a3 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -427,19 +427,6 @@ xfs_qm_scall_getquota_fill_qc(
> dst->d_ino_timer = 0;
> dst->d_rt_spc_timer = 0;
> }
> -
> -#ifdef DEBUG
> - if (xfs_dquot_is_enforced(dqp) && dqp->q_id != 0) {
> - if ((dst->d_space > dst->d_spc_softlimit) &&
> - (dst->d_spc_softlimit > 0)) {
> - ASSERT(dst->d_spc_timer != 0);
> - }
> - if ((dst->d_ino_count > dqp->q_ino.softlimit) &&
> - (dqp->q_ino.softlimit > 0)) {
> - ASSERT(dst->d_ino_timer != 0);
> - }
> - }
> -#endif
> }
>
> /* Return the quota information for the dquot matching id. */
> --
> 2.45.2
>
>
