Re: [PATCH 3/3] xfs: only iget the file once when doing vectored scrub-by-handle

Darrick J. Wong
2024-04-11T05:21:07Z

On Wed, Apr 10, 2024 at 10:02:23PM -0700, Christoph Hellwig wrote:
> On Wed, Apr 10, 2024 at 09:56:45PM -0700, Darrick J. Wong wrote:
> > > Well, someone needs to own it, it's just not just ext4 but could us.
> >
> > Er... I don't understand this? ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> If we set current->journal and take a page faul we could not just
> recurse into ext4 but into any fs including XFS. Any everyone
> blindly dereferences is as only one fs can own it.

Well back before we ripped it out I had said that XFS should just set
current->journal to 1 to prevent memory corruption but then Jan Kara
noted that ext4 changes its behavior wrt jbd2 if it sees nonzero
current->journal. That's why Dave dropped it entirely.

> > > > Alloc transaction -> lock rmap btree for repairs -> iscan filesystem to
> > > > find rmap records -> iget/irele.
> > >
> > > So this is just the magic empty transaction?
> >
> > No, that's the fully featured repair transaction that will eventually be
> > used to write/commit the new rmap tree.
>
> That seems a bit dangerous to me. I guess we rely on the code inside
> the transaction context to never race with unmount as lack of SB_ACTIVE
> will make the VFS ignore the dontcache flag.

That and we have an open fd to call the ioctl so any unmount will fail,
and we can't enter scrub if unmount already starte.

--D