On Thu, Nov 30, 2023 at 02:10:15PM -0800, Darrick J. Wong wrote:
> On Wed, Nov 29, 2023 at 09:33:41PM -0800, Christoph Hellwig wrote:
> > > @@ -328,7 +328,6 @@ xchk_quota(
> > > if (error)
> > > break;
> > > }
> > > - xchk_ilock(sc, XFS_ILOCK_EXCL);
> > > if (error == -ECANCELED)
> > > error = 0;
> > > if (!xchk_fblock_process_error(sc, XFS_DATA_FORK,
> >
> > What is the replacement for this lock? The call in xrep_quota_item?
>
> The replacement is the conditional re-lock at the start of xrep_quota.
Hmm. but not all scrub calls do even end up in the repair callbacks,
do they? Ok, I guess the xchk_iunlock call in xchk_teardown would have
just released it a bit later and we skip the cycle. Would have been
a lot easier to understand if this was in a well-explained
self-contained patch..
> Not sure what you meant about "we used just lock the exclusive lock
> directly without tracking it" -- both files call xchk_{ilock,iunlock}.
> The telemetry data I've collected shows that quota file checking is
> sorta slow, so perhaps it would be justified to create a special
> no-alloc dqget function where the caller is allowed to pre-acquire the
> ILOCK.
My confusions was more about checking/using sc->ilock_flags in the
callers, while it is maintained by the locking helpers. Probably not
*THAT* unusual, but I might have simply been too tired to understand it.
