[5.10,v2,6/7] xfs: reorder iunlink remove operation in xfs_ifree

Commit Message

Amir Goldstein Sept. 1, 2022, 5:48 a.m. UTC

From: Dave Chinner <[e-mail]>

commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.

[backport for 5.10.y]

The O_TMPFILE creation implementation creates a specific order of
operations for inode allocation/freeing and unlinked list
modification. Currently both are serialised by the AGI, so the order
doesn't strictly matter as long as the are both in the same
transaction.

However, if we want to move the unlinked list insertions largely out
from under the AGI lock, then we have to be concerned about the
order in which we do unlinked list modification operations.
O_TMPFILE creation tells us this order is inode allocation/free,
then unlinked list modification.

Change xfs_ifree() to use this same ordering on unlinked list
removal. This way we always guarantee that when we enter the
iunlinked list removal code from this path, we already have the AGI
locked and we don't have to worry about lock nesting AGI reads
inside unlink list locks because it's already locked and attached to
the transaction.

We can do this safely as the inode freeing and unlinked list removal
are done in the same transaction and hence are atomic operations
with respect to log recovery.

Reported-by: Frank Hofmann <[e-mail]>
Fixes: 298f7bec503f ("xfs: pin inode backing buffer to the inode log item")
Signed-off-by: Dave Chinner <[e-mail]>
Reviewed-by: Darrick J. Wong <[e-mail]>
Signed-off-by: Dave Chinner <[e-mail]>
Signed-off-by: Amir Goldstein <[e-mail]>
Acked-by: Darrick J. Wong <[e-mail]>
---
 fs/xfs/xfs_inode.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

Comments

Frank Hofmann Sept. 1, 2022, 9:04 a.m. UTC | #1

On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein <[e-mail]> wrote:
>
> From: Dave Chinner <[e-mail]>
>
> commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
>
> [backport for 5.10.y]

Hi Amir, hi Dave,

I've got no objections to backporting this change at all. We've been
using the patch on our internal 5.15 tracker branch happily for
several months now.

Would like to highlight though that it's currently not yet merged in
linux-stable 5.15 branch either (it's in 5.19 and mainline alright).
If this gets queued for 5.10 then maybe it also should be for 5.15 ?

Thank you,
FrankH.


>
> The O_TMPFILE creation implementation creates a specific order of
> operations for inode allocation/freeing and unlinked list
> modification. Currently both are serialised by the AGI, so the order
> doesn't strictly matter as long as the are both in the same
> transaction.
>
> However, if we want to move the unlinked list insertions largely out
> from under the AGI lock, then we have to be concerned about the
> order in which we do unlinked list modification operations.
> O_TMPFILE creation tells us this order is inode allocation/free,
> then unlinked list modification.
>
> Change xfs_ifree() to use this same ordering on unlinked list
> removal. This way we always guarantee that when we enter the
> iunlinked list removal code from this path, we already have the AGI
> locked and we don't have to worry about lock nesting AGI reads
> inside unlink list locks because it's already locked and attached to
> the transaction.
>
> We can do this safely as the inode freeing and unlinked list removal
> are done in the same transaction and hence are atomic operations
> with respect to log recovery.
>
> Reported-by: Frank Hofmann <[e-mail]>
> Fixes: 298f7bec503f ("xfs: pin inode backing buffer to the inode log item")
> Signed-off-by: Dave Chinner <[e-mail]>
> Reviewed-by: Darrick J. Wong <[e-mail]>
> Signed-off-by: Dave Chinner <[e-mail]>
> Signed-off-by: Amir Goldstein <[e-mail]>
> Acked-by: Darrick J. Wong <[e-mail]>
> ---
>  fs/xfs/xfs_inode.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 1f61e085676b..929ed3bc5619 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2669,14 +2669,13 @@ xfs_ifree_cluster(
>  }
>
>  /*
> - * This is called to return an inode to the inode free list.
> - * The inode should already be truncated to 0 length and have
> - * no pages associated with it.  This routine also assumes that
> - * the inode is already a part of the transaction.
> + * This is called to return an inode to the inode free list.  The inode should
> + * already be truncated to 0 length and have no pages associated with it.  This
> + * routine also assumes that the inode is already a part of the transaction.
>   *
> - * The on-disk copy of the inode will have been added to the list
> - * of unlinked inodes in the AGI. We need to remove the inode from
> - * that list atomically with respect to freeing it here.
> + * The on-disk copy of the inode will have been added to the list of unlinked
> + * inodes in the AGI. We need to remove the inode from that list atomically with
> + * respect to freeing it here.
>   */
>  int
>  xfs_ifree(
> @@ -2694,13 +2693,16 @@ xfs_ifree(
>         ASSERT(ip->i_d.di_nblocks == 0);
>
>         /*
> -        * Pull the on-disk inode from the AGI unlinked list.
> +        * Free the inode first so that we guarantee that the AGI lock is going
> +        * to be taken before we remove the inode from the unlinked list. This
> +        * makes the AGI lock -> unlinked list modification order the same as
> +        * used in O_TMPFILE creation.
>          */
> -       error = xfs_iunlink_remove(tp, ip);
> +       error = xfs_difree(tp, ip->i_ino, &xic);
>         if (error)
>                 return error;
>
> -       error = xfs_difree(tp, ip->i_ino, &xic);
> +       error = xfs_iunlink_remove(tp, ip);
>         if (error)
>                 return error;
>
> --
> 2.25.1
>

Amir Goldstein Sept. 1, 2022, 9:30 a.m. UTC | #2

On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann <[e-mail]> wrote:
>
> On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein <[e-mail]> wrote:
> >
> > From: Dave Chinner <[e-mail]>
> >
> > commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
> >
> > [backport for 5.10.y]
>
> Hi Amir, hi Dave,
>
> I've got no objections to backporting this change at all. We've been
> using the patch on our internal 5.15 tracker branch happily for
> several months now.
>
> Would like to highlight though that it's currently not yet merged in
> linux-stable 5.15 branch either (it's in 5.19 and mainline alright).
> If this gets queued for 5.10 then maybe it also should be for 5.15 ?
>

Hi Frank,

Quoting from my cover letter:

Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet.
I pointed Leah's attention to these patches and she said she will
include them in a following 5.15.y update.

Thanks,
Amir.

Greg Kroah-Hartman Sept. 1, 2022, 9:41 a.m. UTC | #3

On Thu, Sep 01, 2022 at 12:30:13PM +0300, Amir Goldstein wrote:
> On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann <[e-mail]> wrote:
> >
> > On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein <[e-mail]> wrote:
> > >
> > > From: Dave Chinner <[e-mail]>
> > >
> > > commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
> > >
> > > [backport for 5.10.y]
> >
> > Hi Amir, hi Dave,
> >
> > I've got no objections to backporting this change at all. We've been
> > using the patch on our internal 5.15 tracker branch happily for
> > several months now.
> >
> > Would like to highlight though that it's currently not yet merged in
> > linux-stable 5.15 branch either (it's in 5.19 and mainline alright).
> > If this gets queued for 5.10 then maybe it also should be for 5.15 ?
> >
> 
> Hi Frank,
> 
> Quoting from my cover letter:
> 
> Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet.
> I pointed Leah's attention to these patches and she said she will
> include them in a following 5.15.y update.

And as you know, this means I can't take this series at all until that
series is ready, so to help us out, in the future, just don't even send
them until they are all ready together.

thanks,

greg k-h

Amir Goldstein Sept. 1, 2022, 10:16 a.m. UTC | #4

On Thu, Sep 1, 2022 at 12:41 PM Greg Kroah-Hartman
<[e-mail]> wrote:
>
> On Thu, Sep 01, 2022 at 12:30:13PM +0300, Amir Goldstein wrote:
> > On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann <[e-mail]> wrote:
> > >
> > > On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein <[e-mail]> wrote:
> > > >
> > > > From: Dave Chinner <[e-mail]>
> > > >
> > > > commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
> > > >
> > > > [backport for 5.10.y]
> > >
> > > Hi Amir, hi Dave,
> > >
> > > I've got no objections to backporting this change at all. We've been
> > > using the patch on our internal 5.15 tracker branch happily for
> > > several months now.
> > >
> > > Would like to highlight though that it's currently not yet merged in
> > > linux-stable 5.15 branch either (it's in 5.19 and mainline alright).
> > > If this gets queued for 5.10 then maybe it also should be for 5.15 ?
> > >
> >
> > Hi Frank,
> >
> > Quoting from my cover letter:
> >
> > Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet.
> > I pointed Leah's attention to these patches and she said she will
> > include them in a following 5.15.y update.
>
> And as you know, this means I can't take this series at all until that
> series is ready, so to help us out, in the future, just don't even send
> them until they are all ready together.
>

What?

You cannot take backports to 5.10.y before they are applied to 5.15.y?
Since when?

Thanks,
Amir.

Greg Kroah-Hartman Sept. 1, 2022, 10:26 a.m. UTC | #5

On Thu, Sep 01, 2022 at 01:16:33PM +0300, Amir Goldstein wrote:
> On Thu, Sep 1, 2022 at 12:41 PM Greg Kroah-Hartman
> <[e-mail]> wrote:
> >
> > On Thu, Sep 01, 2022 at 12:30:13PM +0300, Amir Goldstein wrote:
> > > On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann <[e-mail]> wrote:
> > > >
> > > > On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein <[e-mail]> wrote:
> > > > >
> > > > > From: Dave Chinner <[e-mail]>
> > > > >
> > > > > commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
> > > > >
> > > > > [backport for 5.10.y]
> > > >
> > > > Hi Amir, hi Dave,
> > > >
> > > > I've got no objections to backporting this change at all. We've been
> > > > using the patch on our internal 5.15 tracker branch happily for
> > > > several months now.
> > > >
> > > > Would like to highlight though that it's currently not yet merged in
> > > > linux-stable 5.15 branch either (it's in 5.19 and mainline alright).
> > > > If this gets queued for 5.10 then maybe it also should be for 5.15 ?
> > > >
> > >
> > > Hi Frank,
> > >
> > > Quoting from my cover letter:
> > >
> > > Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet.
> > > I pointed Leah's attention to these patches and she said she will
> > > include them in a following 5.15.y update.
> >
> > And as you know, this means I can't take this series at all until that
> > series is ready, so to help us out, in the future, just don't even send
> > them until they are all ready together.
> >
> 
> What?
> 
> You cannot take backports to 5.10.y before they are applied to 5.15.y?
> Since when?

Since always.

Why would you ever want someone to upgrade from an older tree (like
5.10.y) to a newer one (5.15.y) and have a regression?

So we always try to make sure patches are always applied to newer trees
first.  Yes, sometimes we miss this and make mistakes, but it's always
been this way and we fix that whenever it happens accidentally.

I'll drop this series from my review queue for now until the 5.15.y
series shows up.

thanks,

greg k-h

Frank Hofmann Sept. 1, 2022, 11:43 a.m. UTC | #6

On Thu, Sep 1, 2022 at 10:30 AM Amir Goldstein <[e-mail]> wrote:
>
> On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann <[e-mail]> wrote:
> >
> > On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein <[e-mail]> wrote:
> > >
> > > From: Dave Chinner <[e-mail]>
> > >
> > > commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
> > >
> > > [backport for 5.10.y]
> >
> > Hi Amir, hi Dave,
> >
> > I've got no objections to backporting this change at all. We've been
> > using the patch on our internal 5.15 tracker branch happily for
> > several months now.
> >
> > Would like to highlight though that it's currently not yet merged in
> > linux-stable 5.15 branch either (it's in 5.19 and mainline alright).
> > If this gets queued for 5.10 then maybe it also should be for 5.15 ?
> >
>
> Hi Frank,
>
> Quoting from my cover letter:
>
> Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet.
> I pointed Leah's attention to these patches and she said she will
> include them in a following 5.15.y update.
>
> Thanks,
> Amir.

Apologies missing that one ... I've only been directly cc:'ed on 6/7 though.

In any case, for 6/7, whether applied to 5.10.y or 5.15.y,

Acked-by: Frank Hofmann <[e-mail]>

And yes, I hope to see it show up in both LTS branches.

FrankH.

Amir Goldstein Sept. 1, 2022, 12:37 p.m. UTC | #7

On Thu, Sep 1, 2022 at 1:26 PM Greg Kroah-Hartman
<[e-mail]> wrote:
>
> On Thu, Sep 01, 2022 at 01:16:33PM +0300, Amir Goldstein wrote:
> > On Thu, Sep 1, 2022 at 12:41 PM Greg Kroah-Hartman
> > <[e-mail]> wrote:
> > >
> > > On Thu, Sep 01, 2022 at 12:30:13PM +0300, Amir Goldstein wrote:
> > > > On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann <[e-mail]> wrote:
> > > > >
> > > > > On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein <[e-mail]> wrote:
> > > > > >
> > > > > > From: Dave Chinner <[e-mail]>
> > > > > >
> > > > > > commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
> > > > > >
> > > > > > [backport for 5.10.y]
> > > > >
> > > > > Hi Amir, hi Dave,
> > > > >
> > > > > I've got no objections to backporting this change at all. We've been
> > > > > using the patch on our internal 5.15 tracker branch happily for
> > > > > several months now.
> > > > >
> > > > > Would like to highlight though that it's currently not yet merged in
> > > > > linux-stable 5.15 branch either (it's in 5.19 and mainline alright).
> > > > > If this gets queued for 5.10 then maybe it also should be for 5.15 ?
> > > > >
> > > >
> > > > Hi Frank,
> > > >
> > > > Quoting from my cover letter:
> > > >
> > > > Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet.
> > > > I pointed Leah's attention to these patches and she said she will
> > > > include them in a following 5.15.y update.
> > >
> > > And as you know, this means I can't take this series at all until that
> > > series is ready, so to help us out, in the future, just don't even send
> > > them until they are all ready together.
> > >
> >
> > What?
> >
> > You cannot take backports to 5.10.y before they are applied to 5.15.y?
> > Since when?
>
> Since always.
>
> Why would you ever want someone to upgrade from an older tree (like
> 5.10.y) to a newer one (5.15.y) and have a regression?
>

That is certainly not a goal when backporting fixes to 5.10.y, but it
can happen as a by-product of the decentralized nature of testing
backports.

But it did not bother you when xfs patches were applied to 5.4.y and
no xfs patches at all applied to 5.10.y for two years?

> So we always try to make sure patches are always applied to newer trees
> first.  Yes, sometimes we miss this and make mistakes, but it's always
> been this way and we fix that whenever it happens accidentally.
>

That is my intention.
I will try to keep to that rule in the future.
I would have waited for the patches to land in 5.15.y, but
Leah got distracted by another task so I decided to not wait,
knowing that the patches are already in her queue.

> I'll drop this series from my review queue for now until the 5.15.y
> series shows up.

Please don't drop the series.
Please drop patches 6-7 if you must
Or if you insist I can re-post patches 1-5.

Thanks,
Amir.

Greg Kroah-Hartman Sept. 1, 2022, 1:07 p.m. UTC | #8

On Thu, Sep 01, 2022 at 03:37:59PM +0300, Amir Goldstein wrote:
> On Thu, Sep 1, 2022 at 1:26 PM Greg Kroah-Hartman
> <[e-mail]> wrote:
> >
> > On Thu, Sep 01, 2022 at 01:16:33PM +0300, Amir Goldstein wrote:
> > > On Thu, Sep 1, 2022 at 12:41 PM Greg Kroah-Hartman
> > > <[e-mail]> wrote:
> > > >
> > > > On Thu, Sep 01, 2022 at 12:30:13PM +0300, Amir Goldstein wrote:
> > > > > On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann <[e-mail]> wrote:
> > > > > >
> > > > > > On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein <[e-mail]> wrote:
> > > > > > >
> > > > > > > From: Dave Chinner <[e-mail]>
> > > > > > >
> > > > > > > commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
> > > > > > >
> > > > > > > [backport for 5.10.y]
> > > > > >
> > > > > > Hi Amir, hi Dave,
> > > > > >
> > > > > > I've got no objections to backporting this change at all. We've been
> > > > > > using the patch on our internal 5.15 tracker branch happily for
> > > > > > several months now.
> > > > > >
> > > > > > Would like to highlight though that it's currently not yet merged in
> > > > > > linux-stable 5.15 branch either (it's in 5.19 and mainline alright).
> > > > > > If this gets queued for 5.10 then maybe it also should be for 5.15 ?
> > > > > >
> > > > >
> > > > > Hi Frank,
> > > > >
> > > > > Quoting from my cover letter:
> > > > >
> > > > > Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet.
> > > > > I pointed Leah's attention to these patches and she said she will
> > > > > include them in a following 5.15.y update.
> > > >
> > > > And as you know, this means I can't take this series at all until that
> > > > series is ready, so to help us out, in the future, just don't even send
> > > > them until they are all ready together.
> > > >
> > >
> > > What?
> > >
> > > You cannot take backports to 5.10.y before they are applied to 5.15.y?
> > > Since when?
> >
> > Since always.
> >
> > Why would you ever want someone to upgrade from an older tree (like
> > 5.10.y) to a newer one (5.15.y) and have a regression?
> >
> 
> That is certainly not a goal when backporting fixes to 5.10.y, but it
> can happen as a by-product of the decentralized nature of testing
> backports.
> 
> But it did not bother you when xfs patches were applied to 5.4.y and
> no xfs patches at all applied to 5.10.y for two years?

I've been bothered by xfs patches for so long that really, I didn't care
as the maintainers didn't seem bothered either :)

But now that everything is working properly, let's do it correctly
please.

> > So we always try to make sure patches are always applied to newer trees
> > first.  Yes, sometimes we miss this and make mistakes, but it's always
> > been this way and we fix that whenever it happens accidentally.
> >
> 
> That is my intention.
> I will try to keep to that rule in the future.
> I would have waited for the patches to land in 5.15.y, but
> Leah got distracted by another task so I decided to not wait,
> knowing that the patches are already in her queue.
> 
> > I'll drop this series from my review queue for now until the 5.15.y
> > series shows up.
> 
> Please don't drop the series.
> Please drop patches 6-7 if you must
> Or if you insist I can re-post patches 1-5.

Please resend as just the correct ones that you want so I know what to
pick exactly.

thanks,

greg k-h

Amir Goldstein Sept. 1, 2022, 3:19 p.m. UTC | #9

On Thu, Sep 1, 2022 at 4:07 PM Greg Kroah-Hartman
<[e-mail]> wrote:
>
> On Thu, Sep 01, 2022 at 03:37:59PM +0300, Amir Goldstein wrote:
> > On Thu, Sep 1, 2022 at 1:26 PM Greg Kroah-Hartman
> > <[e-mail]> wrote:
> > >
> > > On Thu, Sep 01, 2022 at 01:16:33PM +0300, Amir Goldstein wrote:
> > > > On Thu, Sep 1, 2022 at 12:41 PM Greg Kroah-Hartman
> > > > <[e-mail]> wrote:
> > > > >
> > > > > On Thu, Sep 01, 2022 at 12:30:13PM +0300, Amir Goldstein wrote:
> > > > > > On Thu, Sep 1, 2022 at 12:04 PM Frank Hofmann <[e-mail]> wrote:
> > > > > > >
> > > > > > > On Thu, Sep 1, 2022 at 6:49 AM Amir Goldstein <[e-mail]> wrote:
> > > > > > > >
> > > > > > > > From: Dave Chinner <[e-mail]>
> > > > > > > >
> > > > > > > > commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
> > > > > > > >
> > > > > > > > [backport for 5.10.y]
> > > > > > >
> > > > > > > Hi Amir, hi Dave,
> > > > > > >
> > > > > > > I've got no objections to backporting this change at all. We've been
> > > > > > > using the patch on our internal 5.15 tracker branch happily for
> > > > > > > several months now.
> > > > > > >
> > > > > > > Would like to highlight though that it's currently not yet merged in
> > > > > > > linux-stable 5.15 branch either (it's in 5.19 and mainline alright).
> > > > > > > If this gets queued for 5.10 then maybe it also should be for 5.15 ?
> > > > > > >
> > > > > >
> > > > > > Hi Frank,
> > > > > >
> > > > > > Quoting from my cover letter:
> > > > > >
> > > > > > Patches 6-7 in this 5.10.y update have not been applied to 5.15.y yet.
> > > > > > I pointed Leah's attention to these patches and she said she will
> > > > > > include them in a following 5.15.y update.
> > > > >
> > > > > And as you know, this means I can't take this series at all until that
> > > > > series is ready, so to help us out, in the future, just don't even send
> > > > > them until they are all ready together.
> > > > >
> > > >
> > > > What?
> > > >
> > > > You cannot take backports to 5.10.y before they are applied to 5.15.y?
> > > > Since when?
> > >
> > > Since always.
> > >
> > > Why would you ever want someone to upgrade from an older tree (like
> > > 5.10.y) to a newer one (5.15.y) and have a regression?
> > >
> >
> > That is certainly not a goal when backporting fixes to 5.10.y, but it
> > can happen as a by-product of the decentralized nature of testing
> > backports.
> >
> > But it did not bother you when xfs patches were applied to 5.4.y and
> > no xfs patches at all applied to 5.10.y for two years?
>
> I've been bothered by xfs patches for so long that really, I didn't care
> as the maintainers didn't seem bothered either :)
>
> But now that everything is working properly, let's do it correctly
> please.
>
> > > So we always try to make sure patches are always applied to newer trees
> > > first.  Yes, sometimes we miss this and make mistakes, but it's always
> > > been this way and we fix that whenever it happens accidentally.
> > >
> >
> > That is my intention.
> > I will try to keep to that rule in the future.
> > I would have waited for the patches to land in 5.15.y, but
> > Leah got distracted by another task so I decided to not wait,
> > knowing that the patches are already in her queue.
> >
> > > I'll drop this series from my review queue for now until the 5.15.y
> > > series shows up.
> >
> > Please don't drop the series.
> > Please drop patches 6-7 if you must
> > Or if you insist I can re-post patches 1-5.
>
> Please resend as just the correct ones that you want so I know what to
> pick exactly.
>

Posted v3 with patches 1-5.

Thanks,
Amir.

Patch

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 1f61e085676b..929ed3bc5619 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2669,14 +2669,13 @@  xfs_ifree_cluster(
 }
 
 /*
- * This is called to return an inode to the inode free list.
- * The inode should already be truncated to 0 length and have
- * no pages associated with it.  This routine also assumes that
- * the inode is already a part of the transaction.
+ * This is called to return an inode to the inode free list.  The inode should
+ * already be truncated to 0 length and have no pages associated with it.  This
+ * routine also assumes that the inode is already a part of the transaction.
  *
- * The on-disk copy of the inode will have been added to the list
- * of unlinked inodes in the AGI. We need to remove the inode from
- * that list atomically with respect to freeing it here.
+ * The on-disk copy of the inode will have been added to the list of unlinked
+ * inodes in the AGI. We need to remove the inode from that list atomically with
+ * respect to freeing it here.
  */
 int
 xfs_ifree(
@@ -2694,13 +2693,16 @@  xfs_ifree(
 	ASSERT(ip->i_d.di_nblocks == 0);
 
 	/*
-	 * Pull the on-disk inode from the AGI unlinked list.
+	 * Free the inode first so that we guarantee that the AGI lock is going
+	 * to be taken before we remove the inode from the unlinked list. This
+	 * makes the AGI lock -> unlinked list modification order the same as
+	 * used in O_TMPFILE creation.
 	 */
-	error = xfs_iunlink_remove(tp, ip);
+	error = xfs_difree(tp, ip->i_ino, &xic);
 	if (error)
 		return error;
 
-	error = xfs_difree(tp, ip->i_ino, &xic);
+	error = xfs_iunlink_remove(tp, ip);
 	if (error)
 		return error;