On Mon, Jan 18, 2021 at 07:39:58PM +0000, Christoph Hellwig wrote:
> On Mon, Jan 18, 2021 at 11:37:18AM -0800, Darrick J. Wong wrote:
> > Ah, I see, you're asking why don't I make xfs_inode_walk responsible for
> > deciding what to do about EAGAIN, instead of open-coding that in the
> > ->execute function. That would be a nice cleanup since the walk
> > function already has special casing for EFSCORRUPTED.
> >
> > If I read you correctly, the relevant part of xfs_inode_walk becomes:
> >
> > error = execute(batch[i]...);
> > xfs_irele(batch[i]);
> > if (error == -EAGAIN) {
> > if (args->flags & EOF_SYNC)
> > skipped++;
> > continue;
> > }
> >
> > and the relevant part of xfs_inode_free_eofblocks becomes:
> >
> > if (!xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL))
> > return -EAGAIN;
> >
> > I think that would work, and afaict it won't cause any serious problems
> > with the deferred inactivation series.
>
> Exactly!
D'oh. I tried making that change, but ran into the problem that *args
isn't necessarily an eofb structure, and xfs_qm_dqrele_all_inodes passes
a uint pointer. I could define a new XFS_INODE_WALK_SYNC flag and
update the blockgc callers to set that if EOF_FLAGS_SYNC is set...
--D
