Re: [PATCH V2] xfs: Fix agi&agf ABBA deadlock when performing rename with RENAME_WHITEOUT flag

Brian Foster
2019-08-20T12:23:05Z

On Tue, Aug 20, 2019 at 09:23:04PM +1000, Dave Chinner wrote:
> On Tue, Aug 20, 2019 at 06:51:01AM -0400, Brian Foster wrote:
> > On Tue, Aug 20, 2019 at 04:53:22PM +0800, kaixuxia wrote:
> > FWIW if we do take that approach, then IMO it's worth reconsidering the
> > 1-2 liner I originally proposed to fix the locking. It's slightly hacky,
> > but really all three options are hacky in slightly different ways. The
> > flipside is it's trivial to implement, review and backport and now would
> > be removed shortly thereafter when we replace the on-disk whiteout with
> > the in-core fake whiteout thing. Just my .02 though..
>
> We've got to keep the existing whiteout method around for,
> essentially, forever, because we have to support kernels that don't
> do in-memory translations of DT_WHT to a magic chardev inode and
> vice versa (i.e. via mknod). IOWs, we'll need a feature bit to
> indicate that we actually have DT_WHT based whiteouts on disk.
>

I'm not quite following (probably just because I'm not terribly familiar
with the use case). If current kernels know how to fake up whiteout
inodes in memory based on a dentry, why do we need to continue to create
new on-disk whiteout inodes just because a filesystem might already have
such inodes on disk? Wouldn't the old format whiteouts just continue to
work as expected without any extra handling?

I can see needing a feature bit to restrict a filesystem from being used
on an unsupported, older kernel, but is there a reason we wouldn't just
enable that by default anyways?

Brian

> So we may as well fix this properly now by restructuring the code as
> we will still have to maintain this functionality for a long time to
> come.
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com