diff mbox series

[V15,05/16] xfs: Check for extent overflow when removing dir entries

Message ID 20210126063232.3648053-6-chandanrlinux@gmail.com (mailing list archive)
State Accepted, archived
Headers show
Series Bail out if transaction can cause extent count to overflow | expand

Commit Message

Chandan Babu R Jan. 26, 2021, 6:32 a.m. UTC
Directory entry removal must always succeed; Hence XFS does the
following during low disk space scenario:
1. Data/Free blocks linger until a future remove operation.
2. Dabtree blocks would be swapped with the last block in the leaf space
   and then the new last block will be unmapped.

This facility is reused during low inode extent count scenario i.e. this
commit causes xfs_bmap_del_extent_real() to return -ENOSPC error code so
that the above mentioned behaviour is exercised causing no change to the
directory's extent count.

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Allison Henderson Jan. 27, 2021, 12:39 a.m. UTC | #1
On 1/25/21 11:32 PM, Chandan Babu R wrote:
> Directory entry removal must always succeed; Hence XFS does the
> following during low disk space scenario:
> 1. Data/Free blocks linger until a future remove operation.
> 2. Dabtree blocks would be swapped with the last block in the leaf space
>     and then the new last block will be unmapped.
> 
> This facility is reused during low inode extent count scenario i.e. this
> commit causes xfs_bmap_del_extent_real() to return -ENOSPC error code so
> that the above mentioned behaviour is exercised causing no change to the
> directory's extent count.
> 
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>

Ok, makes sense.  Thanks for the commentary!
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
> ---
>   fs/xfs/libxfs/xfs_bmap.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 32aeacf6f055..6c8f17a0e247 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5151,6 +5151,24 @@ xfs_bmap_del_extent_real(
>   		/*
>   		 * Deleting the middle of the extent.
>   		 */
> +
> +		/*
> +		 * For directories, -ENOSPC is returned since a directory entry
> +		 * remove operation must not fail due to low extent count
> +		 * availability. -ENOSPC will be handled by higher layers of XFS
> +		 * by letting the corresponding empty Data/Free blocks to linger
> +		 * until a future remove operation. Dabtree blocks would be
> +		 * swapped with the last block in the leaf space and then the
> +		 * new last block will be unmapped.
> +		 */
> +		error = xfs_iext_count_may_overflow(ip, whichfork, 1);
> +		if (error) {
> +			ASSERT(S_ISDIR(VFS_I(ip)->i_mode) &&
> +				whichfork == XFS_DATA_FORK);
> +			error = -ENOSPC;
> +			goto done;
> +		}
> +
>   		old = got;
>   
>   		got.br_blockcount = del->br_startoff - got.br_startoff;
>
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 32aeacf6f055..6c8f17a0e247 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5151,6 +5151,24 @@  xfs_bmap_del_extent_real(
 		/*
 		 * Deleting the middle of the extent.
 		 */
+
+		/*
+		 * For directories, -ENOSPC is returned since a directory entry
+		 * remove operation must not fail due to low extent count
+		 * availability. -ENOSPC will be handled by higher layers of XFS
+		 * by letting the corresponding empty Data/Free blocks to linger
+		 * until a future remove operation. Dabtree blocks would be
+		 * swapped with the last block in the leaf space and then the
+		 * new last block will be unmapped.
+		 */
+		error = xfs_iext_count_may_overflow(ip, whichfork, 1);
+		if (error) {
+			ASSERT(S_ISDIR(VFS_I(ip)->i_mode) &&
+				whichfork == XFS_DATA_FORK);
+			error = -ENOSPC;
+			goto done;
+		}
+
 		old = got;
 
 		got.br_blockcount = del->br_startoff - got.br_startoff;