[PATCH 0/2] xfs: code cleanup and item deletion fix

[PATCH 0/2] xfs: code cleanup and item deletion fix

[Long Li]

This patch series contains the first two patches from the previously
unfinished patch set [1]. I'm resubmitting them separately since patch 2
addresses an corner case issue that should be fixed. At the same time, I
rewrote the commit message and comments of patch 2.

Patch 1 : Simple clean code.
Patch 2 : Fixes a issue where the tail LSN was incorrectly moved forward
	  due to improper deletion of log items from the AIL before log
	  shutdown. 

[1] https://patchwork.kernel.org/project/xfs/patch/20240823110439.1585041-1-leo.lilong@huawei.com/

Long Li (2):
  xfs: remove redundant set null for ip->i_itemp
  xfs: ensure dquot item is deleted from AIL only after log shutdown

 fs/xfs/xfs_dquot.c  | 8 +++++++-
 fs/xfs/xfs_icache.c | 1 -
 2 files changed, 7 insertions(+), 2 deletions(-)

-- 
2.39.2

[PATCH 1/2] xfs: remove redundant set null for ip->i_itemp

[Long Li]

ip->i_itemp has been set null in xfs_inode_item_destroy(), so there is
no need set it null again in xfs_inode_free_callback().

Signed-off-by: Long Li <leo.lilong@huawei.com>
---
 fs/xfs/xfs_icache.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index a7a09e7eec81..2040a9292ee6 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -159,7 +159,6 @@ xfs_inode_free_callback(
 		ASSERT(!test_bit(XFS_LI_IN_AIL,
 				 &ip->i_itemp->ili_item.li_flags));
 		xfs_inode_item_destroy(ip);
-		ip->i_itemp = NULL;
 	}
 
 	kmem_cache_free(xfs_inode_cache, ip);
-- 
2.39.2

Re: [PATCH 1/2] xfs: remove redundant set null for ip->i_itemp

[Christoph Hellwig]

On Thu, Mar 05, 2026 at 04:49:21PM +0800, Long Li wrote:
> ip->i_itemp has been set null in xfs_inode_item_destroy(), so there is
> no need set it null again in xfs_inode_free_callback().

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/2] xfs: remove redundant set null for ip->i_itemp

[Carlos Maiolino]

On Thu, Mar 05, 2026 at 04:49:21PM +0800, Long Li wrote:
> ip->i_itemp has been set null in xfs_inode_item_destroy(), so there is
> no need set it null again in xfs_inode_free_callback().
> 
> Signed-off-by: Long Li <leo.lilong@huawei.com>
> ---

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

>  fs/xfs/xfs_icache.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index a7a09e7eec81..2040a9292ee6 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -159,7 +159,6 @@ xfs_inode_free_callback(
>  		ASSERT(!test_bit(XFS_LI_IN_AIL,
>  				 &ip->i_itemp->ili_item.li_flags));
>  		xfs_inode_item_destroy(ip);
> -		ip->i_itemp = NULL;
>  	}
>  
>  	kmem_cache_free(xfs_inode_cache, ip);
> -- 
> 2.39.2
> 
> 

[PATCH 2/2] xfs: ensure dquot item is deleted from AIL only after log shutdown

[Long Li]

In xfs_qm_dqflush(), when a dquot flush fails due to corruption
(the out_abort error path), the original code removed the dquot log
item from the AIL before calling xfs_force_shutdown(). This ordering
introduces a subtle race condition that can lead to data loss after
a crash.

The AIL tracks the oldest dirty metadata in the journal. The position
of the tail item in the AIL determines the log tail LSN, which is the
oldest LSN that must be preserved for crash recovery. When an item is
removed from the AIL, the log tail can advance past the LSN of that item.

The race window is as follows: if the dquot item happens to be at
the tail of the log, removing it from the AIL allows the log tail
to advance. If a concurrent log write is sampling the tail LSN at
the same time and subsequently writes a complete checkpoint (i.e.,
one containing a commit record) to disk before the shutdown takes
effect, the journal will no longer protect the dquot's last
modification. On the next mount, log recovery will not replay the
dquot changes, even though they were never written back to disk,
resulting in silent data loss.

Fix this by calling xfs_force_shutdown() before xfs_trans_ail_delete()
in the out_abort path. Once the log is shut down, no new log writes
can complete with an updated tail LSN, making it safe to remove the
dquot item from the AIL.

Cc: <stable@vger.kernel.org>
Fixes: b707fffda6a3 ("xfs: abort consistently on dquot flush failure")
Signed-off-by: Long Li <leo.lilong@huawei.com>
---
 fs/xfs/xfs_dquot.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 2b208e2c5264..69e9bc588c8b 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -1439,9 +1439,15 @@ xfs_qm_dqflush(
 	return 0;
 
 out_abort:
+	/*
+	 * Shut down the log before removing the dquot item from the AIL.
+	 * Otherwise, the log tail may advance past this item's LSN while
+	 * log writes are still in progress, making these unflushed changes
+	 * unrecoverable on the next mount.
+	 */
+	xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
 	dqp->q_flags &= ~XFS_DQFLAG_DIRTY;
 	xfs_trans_ail_delete(lip, 0);
-	xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
 	xfs_dqfunlock(dqp);
 	return error;
 }
-- 
2.39.2

Re: [PATCH 2/2] xfs: ensure dquot item is deleted from AIL only after log shutdown

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/2] xfs: ensure dquot item is deleted from AIL only after log shutdown

[Carlos Maiolino]

On Thu, Mar 05, 2026 at 04:49:22PM +0800, Long Li wrote:
> In xfs_qm_dqflush(), when a dquot flush fails due to corruption
> (the out_abort error path), the original code removed the dquot log
> item from the AIL before calling xfs_force_shutdown(). This ordering
> introduces a subtle race condition that can lead to data loss after
> a crash.
> 
> The AIL tracks the oldest dirty metadata in the journal. The position
> of the tail item in the AIL determines the log tail LSN, which is the
> oldest LSN that must be preserved for crash recovery. When an item is
> removed from the AIL, the log tail can advance past the LSN of that item.
> 
> The race window is as follows: if the dquot item happens to be at
> the tail of the log, removing it from the AIL allows the log tail
> to advance. If a concurrent log write is sampling the tail LSN at
> the same time and subsequently writes a complete checkpoint (i.e.,
> one containing a commit record) to disk before the shutdown takes
> effect, the journal will no longer protect the dquot's last
> modification. On the next mount, log recovery will not replay the
> dquot changes, even though they were never written back to disk,
> resulting in silent data loss.
> 
> Fix this by calling xfs_force_shutdown() before xfs_trans_ail_delete()
> in the out_abort path. Once the log is shut down, no new log writes
> can complete with an updated tail LSN, making it safe to remove the
> dquot item from the AIL.
> 
> Cc: <stable@vger.kernel.org>
> Fixes: b707fffda6a3 ("xfs: abort consistently on dquot flush failure")
> Signed-off-by: Long Li <leo.lilong@huawei.com>
> ---
>  fs/xfs/xfs_dquot.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 2b208e2c5264..69e9bc588c8b 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -1439,9 +1439,15 @@ xfs_qm_dqflush(
>  	return 0;
>  
>  out_abort:
> +	/*
> +	 * Shut down the log before removing the dquot item from the AIL.
> +	 * Otherwise, the log tail may advance past this item's LSN while
> +	 * log writes are still in progress, making these unflushed changes
> +	 * unrecoverable on the next mount.
> +	 */
> +	xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
>  	dqp->q_flags &= ~XFS_DQFLAG_DIRTY;
>  	xfs_trans_ail_delete(lip, 0);
> -	xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
>  	xfs_dqfunlock(dqp);
>  	return error;
>  }

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> -- 
> 2.39.2
> 

Re: [PATCH 0/2] xfs: code cleanup and item deletion fix

[Carlos Maiolino]

On Thu, 05 Mar 2026 16:49:20 +0800, Long Li wrote:
> This patch series contains the first two patches from the previously
> unfinished patch set [1]. I'm resubmitting them separately since patch 2
> addresses an corner case issue that should be fixed. At the same time, I
> rewrote the commit message and comments of patch 2.
> 
> Patch 1 : Simple clean code.
> Patch 2 : Fixes a issue where the tail LSN was incorrectly moved forward
> 	  due to improper deletion of log items from the AIL before log
> 	  shutdown.
> 
> [...]

Applied to for-next, thanks!

[1/2] xfs: remove redundant set null for ip->i_itemp
      commit: f1d77b863b414586ee45e10d9837c9ab27d8692d
[2/2] xfs: ensure dquot item is deleted from AIL only after log shutdown
      commit: 186ac39b8a7d3ec7ce9c5dd45e5c2730177f375c

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>