diff mbox series

xfs: short circuit xfs_growfs_data_private() if delta is zero

Message ID a6a7bfa4-a7bb-4103-9887-63c69356d187@redhat.com (mailing list archive)
State Superseded, archived
Headers show
Series xfs: short circuit xfs_growfs_data_private() if delta is zero | expand

Commit Message

Eric Sandeen Dec. 14, 2023, 7:28 p.m. UTC
Although xfs_growfs_data() doesn't call xfs_growfs_data_private()
if in->newblocks == mp->m_sb.sb_dblocks, xfs_growfs_data_private()
further massages the new block count so that we don't i.e. try
to create a too-small new AG.

This may lead to a delta of "0" in xfs_growfs_data_private(), so
we end up in the shrink case and emit the EXPERIMENTAL warning
even if we're not changing anything at all.

Fix this by returning straightaway if the block delta is zero.

(nb: in older kernels, the result of entering the shrink case
with delta == 0 may actually let an -ENOSPC escape to userspace,
which is confusing for users.)

Fixes: fb2fc1720185 ("xfs: support shrinking unused space in the last AG")
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

Comments

Darrick J. Wong Dec. 14, 2023, 8:14 p.m. UTC | #1
On Thu, Dec 14, 2023 at 01:28:08PM -0600, Eric Sandeen wrote:
> Although xfs_growfs_data() doesn't call xfs_growfs_data_private()
> if in->newblocks == mp->m_sb.sb_dblocks, xfs_growfs_data_private()
> further massages the new block count so that we don't i.e. try
> to create a too-small new AG.
> 
> This may lead to a delta of "0" in xfs_growfs_data_private(), so
> we end up in the shrink case and emit the EXPERIMENTAL warning
> even if we're not changing anything at all.
> 
> Fix this by returning straightaway if the block delta is zero.
> 
> (nb: in older kernels, the result of entering the shrink case
> with delta == 0 may actually let an -ENOSPC escape to userspace,
> which is confusing for users.)
> 
> Fixes: fb2fc1720185 ("xfs: support shrinking unused space in the last AG")
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Seems like a reasonable addition...
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
> 
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 7cb75cb6b8e9..80811d16dde0 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -134,6 +134,10 @@ xfs_growfs_data_private(
>  	if (delta < 0 && nagcount < 2)
>  		return -EINVAL;
> 
> +	/* No work to do */
> +	if (delta == 0)
> +		return 0;
> +
>  	oagcount = mp->m_sb.sb_agcount;
>  	/* allocate the new per-ag structures */
>  	if (nagcount > oagcount) {
> 
>
Christoph Hellwig Dec. 15, 2023, 4:19 a.m. UTC | #2
On Thu, Dec 14, 2023 at 01:28:08PM -0600, Eric Sandeen wrote:
> Although xfs_growfs_data() doesn't call xfs_growfs_data_private()
> if in->newblocks == mp->m_sb.sb_dblocks, xfs_growfs_data_private()
> further massages the new block count so that we don't i.e. try
> to create a too-small new AG.
> 
> This may lead to a delta of "0" in xfs_growfs_data_private(), so
> we end up in the shrink case and emit the EXPERIMENTAL warning
> even if we're not changing anything at all.
> 
> Fix this by returning straightaway if the block delta is zero.
> 
> (nb: in older kernels, the result of entering the shrink case
> with delta == 0 may actually let an -ENOSPC escape to userspace,
> which is confusing for users.)
> 
> Fixes: fb2fc1720185 ("xfs: support shrinking unused space in the last AG")
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good:

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

Patch

diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 7cb75cb6b8e9..80811d16dde0 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -134,6 +134,10 @@  xfs_growfs_data_private(
 	if (delta < 0 && nagcount < 2)
 		return -EINVAL;

+	/* No work to do */
+	if (delta == 0)
+		return 0;
+
 	oagcount = mp->m_sb.sb_agcount;
 	/* allocate the new per-ag structures */
 	if (nagcount > oagcount) {