All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] f2fs: avoid stale FI_COMPRESS_RELEASED on release failure
@ 2026-07-24 13:50 ` Wenjie Qi
  0 siblings, 0 replies; 6+ messages in thread
From: Wenjie Qi @ 2026-07-24 13:50 UTC (permalink / raw)
  To: jaegeuk, chao; +Cc: linux-f2fs-devel, linux-kernel, qiwenjie, qwjhust, stable

F2FS_IOC_RELEASE_COMPRESS_BLOCKS sets FI_COMPRESS_RELEASED before walking
data nodes. If release_compress_blocks() fails before freeing any reserved
block, the inode keeps the flag while i_compr_blocks is still non-zero.

Then F2FS_IOC_RESERVE_COMPRESS_BLOCKS returns success with zero reserved
blocks, and regular writes keep failing with -EPERM.

Set the flag only after the ioctl succeeds, or after it has actually
released some blocks. This preserves the existing partial-release error
handling and leaves a failed zero-release attempt unchanged.

Fixes: ef8d563f184e ("f2fs: introduce F2FS_IOC_RELEASE_COMPRESS_BLOCKS")
Cc: stable@kernel.org
Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
---
QEMU fault-injection test with FAULT_BLKADDR_VALIDITY:
- before: release failed with -EFSCORRUPTED, reserve returned 0 blocks,
  and pwrite kept failing with -EPERM after remount.
- after: release still failed, reserve returned -EINVAL, and pwrite succeeded.
- normal release/reserve still released and reserved 192 blocks.

 fs/f2fs/file.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index c54897a25981..dca1722f92b3 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3916,10 +3916,6 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg)
 		goto out;
 	}
 
-	set_inode_flag(inode, FI_COMPRESS_RELEASED);
-	inode_set_ctime_current(inode);
-	f2fs_mark_inode_dirty_sync(inode, true);
-
 	f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
 	filemap_invalidate_lock(inode->i_mapping);
 
@@ -3963,6 +3959,12 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg)
 
 	filemap_invalidate_unlock(inode->i_mapping);
 	f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
+
+	if (ret >= 0 || released_blocks) {
+		set_inode_flag(inode, FI_COMPRESS_RELEASED);
+		inode_set_ctime_current(inode);
+		f2fs_mark_inode_dirty_sync(inode, true);
+	}
 out:
 	if (released_blocks)
 		f2fs_update_time(sbi, REQ_TIME);
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [f2fs-dev] [PATCH] f2fs: avoid stale FI_COMPRESS_RELEASED on release failure
@ 2026-07-24 13:50 ` Wenjie Qi
  0 siblings, 0 replies; 6+ messages in thread
From: Wenjie Qi @ 2026-07-24 13:50 UTC (permalink / raw)
  To: jaegeuk, chao; +Cc: qwjhust, stable, qiwenjie, linux-kernel, linux-f2fs-devel

F2FS_IOC_RELEASE_COMPRESS_BLOCKS sets FI_COMPRESS_RELEASED before walking
data nodes. If release_compress_blocks() fails before freeing any reserved
block, the inode keeps the flag while i_compr_blocks is still non-zero.

Then F2FS_IOC_RESERVE_COMPRESS_BLOCKS returns success with zero reserved
blocks, and regular writes keep failing with -EPERM.

Set the flag only after the ioctl succeeds, or after it has actually
released some blocks. This preserves the existing partial-release error
handling and leaves a failed zero-release attempt unchanged.

Fixes: ef8d563f184e ("f2fs: introduce F2FS_IOC_RELEASE_COMPRESS_BLOCKS")
Cc: stable@kernel.org
Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
---
QEMU fault-injection test with FAULT_BLKADDR_VALIDITY:
- before: release failed with -EFSCORRUPTED, reserve returned 0 blocks,
  and pwrite kept failing with -EPERM after remount.
- after: release still failed, reserve returned -EINVAL, and pwrite succeeded.
- normal release/reserve still released and reserved 192 blocks.

 fs/f2fs/file.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index c54897a25981..dca1722f92b3 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3916,10 +3916,6 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg)
 		goto out;
 	}
 
-	set_inode_flag(inode, FI_COMPRESS_RELEASED);
-	inode_set_ctime_current(inode);
-	f2fs_mark_inode_dirty_sync(inode, true);
-
 	f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
 	filemap_invalidate_lock(inode->i_mapping);
 
@@ -3963,6 +3959,12 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg)
 
 	filemap_invalidate_unlock(inode->i_mapping);
 	f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
+
+	if (ret >= 0 || released_blocks) {
+		set_inode_flag(inode, FI_COMPRESS_RELEASED);
+		inode_set_ctime_current(inode);
+		f2fs_mark_inode_dirty_sync(inode, true);
+	}
 out:
 	if (released_blocks)
 		f2fs_update_time(sbi, REQ_TIME);
-- 
2.43.0


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: avoid stale FI_COMPRESS_RELEASED on release failure
  2026-07-24 13:50 ` [f2fs-dev] " Wenjie Qi
@ 2026-08-03  8:31   ` Chao Yu
  -1 siblings, 0 replies; 6+ messages in thread
From: Chao Yu via Linux-f2fs-devel @ 2026-08-03  8:31 UTC (permalink / raw)
  To: Wenjie Qi, jaegeuk; +Cc: stable, qiwenjie, linux-kernel, linux-f2fs-devel

On 7/24/26 21:50, Wenjie Qi wrote:
> F2FS_IOC_RELEASE_COMPRESS_BLOCKS sets FI_COMPRESS_RELEASED before walking
> data nodes. If release_compress_blocks() fails before freeing any reserved
> block, the inode keeps the flag while i_compr_blocks is still non-zero.
> 
> Then F2FS_IOC_RESERVE_COMPRESS_BLOCKS returns success with zero reserved
> blocks, and regular writes keep failing with -EPERM.
> 
> Set the flag only after the ioctl succeeds, or after it has actually
> released some blocks. This preserves the existing partial-release error
> handling and leaves a failed zero-release attempt unchanged.
> 
> Fixes: ef8d563f184e ("f2fs: introduce F2FS_IOC_RELEASE_COMPRESS_BLOCKS")
> Cc: stable@kernel.org
> Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> ---
> QEMU fault-injection test with FAULT_BLKADDR_VALIDITY:
> - before: release failed with -EFSCORRUPTED, reserve returned 0 blocks,
>    and pwrite kept failing with -EPERM after remount.
> - after: release still failed, reserve returned -EINVAL, and pwrite succeeded.
> - normal release/reserve still released and reserved 192 blocks.
> 
>   fs/f2fs/file.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index c54897a25981..dca1722f92b3 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -3916,10 +3916,6 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg)
>   		goto out;
>   	}
>   
> -	set_inode_flag(inode, FI_COMPRESS_RELEASED);
> -	inode_set_ctime_current(inode);
> -	f2fs_mark_inode_dirty_sync(inode, true);
> -
>   	f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>   	filemap_invalidate_lock(inode->i_mapping);
>   
> @@ -3963,6 +3959,12 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg)
>   
>   	filemap_invalidate_unlock(inode->i_mapping);
>   	f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
> +
> +	if (ret >= 0 || released_blocks) {

We will set FI_COMPRESS_RELEASED if ret < 0 and released_blocks > 0?

Thanks,

> +		set_inode_flag(inode, FI_COMPRESS_RELEASED);
> +		inode_set_ctime_current(inode);
> +		f2fs_mark_inode_dirty_sync(inode, true);
> +	}
>   out:
>   	if (released_blocks)
>   		f2fs_update_time(sbi, REQ_TIME);



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] f2fs: avoid stale FI_COMPRESS_RELEASED on release failure
@ 2026-08-03  8:31   ` Chao Yu
  0 siblings, 0 replies; 6+ messages in thread
From: Chao Yu @ 2026-08-03  8:31 UTC (permalink / raw)
  To: Wenjie Qi, jaegeuk; +Cc: chao, linux-f2fs-devel, linux-kernel, qiwenjie, stable

On 7/24/26 21:50, Wenjie Qi wrote:
> F2FS_IOC_RELEASE_COMPRESS_BLOCKS sets FI_COMPRESS_RELEASED before walking
> data nodes. If release_compress_blocks() fails before freeing any reserved
> block, the inode keeps the flag while i_compr_blocks is still non-zero.
> 
> Then F2FS_IOC_RESERVE_COMPRESS_BLOCKS returns success with zero reserved
> blocks, and regular writes keep failing with -EPERM.
> 
> Set the flag only after the ioctl succeeds, or after it has actually
> released some blocks. This preserves the existing partial-release error
> handling and leaves a failed zero-release attempt unchanged.
> 
> Fixes: ef8d563f184e ("f2fs: introduce F2FS_IOC_RELEASE_COMPRESS_BLOCKS")
> Cc: stable@kernel.org
> Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> ---
> QEMU fault-injection test with FAULT_BLKADDR_VALIDITY:
> - before: release failed with -EFSCORRUPTED, reserve returned 0 blocks,
>    and pwrite kept failing with -EPERM after remount.
> - after: release still failed, reserve returned -EINVAL, and pwrite succeeded.
> - normal release/reserve still released and reserved 192 blocks.
> 
>   fs/f2fs/file.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index c54897a25981..dca1722f92b3 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -3916,10 +3916,6 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg)
>   		goto out;
>   	}
>   
> -	set_inode_flag(inode, FI_COMPRESS_RELEASED);
> -	inode_set_ctime_current(inode);
> -	f2fs_mark_inode_dirty_sync(inode, true);
> -
>   	f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>   	filemap_invalidate_lock(inode->i_mapping);
>   
> @@ -3963,6 +3959,12 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg)
>   
>   	filemap_invalidate_unlock(inode->i_mapping);
>   	f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
> +
> +	if (ret >= 0 || released_blocks) {

We will set FI_COMPRESS_RELEASED if ret < 0 and released_blocks > 0?

Thanks,

> +		set_inode_flag(inode, FI_COMPRESS_RELEASED);
> +		inode_set_ctime_current(inode);
> +		f2fs_mark_inode_dirty_sync(inode, true);
> +	}
>   out:
>   	if (released_blocks)
>   		f2fs_update_time(sbi, REQ_TIME);


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] f2fs: avoid stale FI_COMPRESS_RELEASED on release failure
  2026-08-03  8:31   ` Chao Yu
@ 2026-08-03  9:38     ` Wenjie Qi
  -1 siblings, 0 replies; 6+ messages in thread
From: Wenjie Qi @ 2026-08-03  9:38 UTC (permalink / raw)
  To: Chao Yu; +Cc: jaegeuk, linux-f2fs-devel, linux-kernel, qiwenjie, stable

  Yes, that is intentional.

  If ret < 0 but released_blocks > 0, the ioctl has already released some
  compressed reserved blocks and updated the inode/block accounting
  partially. At that point the inode is no longer in the original
  pre-release state, so keeping FI_COMPRESS_RELEASED is deliberate.

  The case this patch is trying to fix is only ret < 0 &&
  released_blocks == 0, where no release-side state change happened but
  the old code still left FI_COMPRESS_RELEASED set.

On Mon, Aug 3, 2026 at 4:31 PM Chao Yu <chao@kernel.org> wrote:
>
> On 7/24/26 21:50, Wenjie Qi wrote:
> > F2FS_IOC_RELEASE_COMPRESS_BLOCKS sets FI_COMPRESS_RELEASED before walking
> > data nodes. If release_compress_blocks() fails before freeing any reserved
> > block, the inode keeps the flag while i_compr_blocks is still non-zero.
> >
> > Then F2FS_IOC_RESERVE_COMPRESS_BLOCKS returns success with zero reserved
> > blocks, and regular writes keep failing with -EPERM.
> >
> > Set the flag only after the ioctl succeeds, or after it has actually
> > released some blocks. This preserves the existing partial-release error
> > handling and leaves a failed zero-release attempt unchanged.
> >
> > Fixes: ef8d563f184e ("f2fs: introduce F2FS_IOC_RELEASE_COMPRESS_BLOCKS")
> > Cc: stable@kernel.org
> > Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> > ---
> > QEMU fault-injection test with FAULT_BLKADDR_VALIDITY:
> > - before: release failed with -EFSCORRUPTED, reserve returned 0 blocks,
> >    and pwrite kept failing with -EPERM after remount.
> > - after: release still failed, reserve returned -EINVAL, and pwrite succeeded.
> > - normal release/reserve still released and reserved 192 blocks.
> >
> >   fs/f2fs/file.c | 10 ++++++----
> >   1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index c54897a25981..dca1722f92b3 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -3916,10 +3916,6 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg)
> >               goto out;
> >       }
> >
> > -     set_inode_flag(inode, FI_COMPRESS_RELEASED);
> > -     inode_set_ctime_current(inode);
> > -     f2fs_mark_inode_dirty_sync(inode, true);
> > -
> >       f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
> >       filemap_invalidate_lock(inode->i_mapping);
> >
> > @@ -3963,6 +3959,12 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg)
> >
> >       filemap_invalidate_unlock(inode->i_mapping);
> >       f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
> > +
> > +     if (ret >= 0 || released_blocks) {
>
> We will set FI_COMPRESS_RELEASED if ret < 0 and released_blocks > 0?
>
> Thanks,
>
> > +             set_inode_flag(inode, FI_COMPRESS_RELEASED);
> > +             inode_set_ctime_current(inode);
> > +             f2fs_mark_inode_dirty_sync(inode, true);
> > +     }
> >   out:
> >       if (released_blocks)
> >               f2fs_update_time(sbi, REQ_TIME);
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: avoid stale FI_COMPRESS_RELEASED on release failure
@ 2026-08-03  9:38     ` Wenjie Qi
  0 siblings, 0 replies; 6+ messages in thread
From: Wenjie Qi @ 2026-08-03  9:38 UTC (permalink / raw)
  To: Chao Yu; +Cc: jaegeuk, qiwenjie, linux-kernel, stable, linux-f2fs-devel

  Yes, that is intentional.

  If ret < 0 but released_blocks > 0, the ioctl has already released some
  compressed reserved blocks and updated the inode/block accounting
  partially. At that point the inode is no longer in the original
  pre-release state, so keeping FI_COMPRESS_RELEASED is deliberate.

  The case this patch is trying to fix is only ret < 0 &&
  released_blocks == 0, where no release-side state change happened but
  the old code still left FI_COMPRESS_RELEASED set.

On Mon, Aug 3, 2026 at 4:31 PM Chao Yu <chao@kernel.org> wrote:
>
> On 7/24/26 21:50, Wenjie Qi wrote:
> > F2FS_IOC_RELEASE_COMPRESS_BLOCKS sets FI_COMPRESS_RELEASED before walking
> > data nodes. If release_compress_blocks() fails before freeing any reserved
> > block, the inode keeps the flag while i_compr_blocks is still non-zero.
> >
> > Then F2FS_IOC_RESERVE_COMPRESS_BLOCKS returns success with zero reserved
> > blocks, and regular writes keep failing with -EPERM.
> >
> > Set the flag only after the ioctl succeeds, or after it has actually
> > released some blocks. This preserves the existing partial-release error
> > handling and leaves a failed zero-release attempt unchanged.
> >
> > Fixes: ef8d563f184e ("f2fs: introduce F2FS_IOC_RELEASE_COMPRESS_BLOCKS")
> > Cc: stable@kernel.org
> > Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> > ---
> > QEMU fault-injection test with FAULT_BLKADDR_VALIDITY:
> > - before: release failed with -EFSCORRUPTED, reserve returned 0 blocks,
> >    and pwrite kept failing with -EPERM after remount.
> > - after: release still failed, reserve returned -EINVAL, and pwrite succeeded.
> > - normal release/reserve still released and reserved 192 blocks.
> >
> >   fs/f2fs/file.c | 10 ++++++----
> >   1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index c54897a25981..dca1722f92b3 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -3916,10 +3916,6 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg)
> >               goto out;
> >       }
> >
> > -     set_inode_flag(inode, FI_COMPRESS_RELEASED);
> > -     inode_set_ctime_current(inode);
> > -     f2fs_mark_inode_dirty_sync(inode, true);
> > -
> >       f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
> >       filemap_invalidate_lock(inode->i_mapping);
> >
> > @@ -3963,6 +3959,12 @@ static int f2fs_release_compress_blocks(struct file *filp, unsigned long arg)
> >
> >       filemap_invalidate_unlock(inode->i_mapping);
> >       f2fs_up_write(&fi->i_gc_rwsem[WRITE]);
> > +
> > +     if (ret >= 0 || released_blocks) {
>
> We will set FI_COMPRESS_RELEASED if ret < 0 and released_blocks > 0?
>
> Thanks,
>
> > +             set_inode_flag(inode, FI_COMPRESS_RELEASED);
> > +             inode_set_ctime_current(inode);
> > +             f2fs_mark_inode_dirty_sync(inode, true);
> > +     }
> >   out:
> >       if (released_blocks)
> >               f2fs_update_time(sbi, REQ_TIME);
>


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-03  9:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-24 13:50 [PATCH] f2fs: avoid stale FI_COMPRESS_RELEASED on release failure Wenjie Qi
2026-07-24 13:50 ` [f2fs-dev] " Wenjie Qi
2026-08-03  8:31 ` Chao Yu via Linux-f2fs-devel
2026-08-03  8:31   ` Chao Yu
2026-08-03  9:38   ` Wenjie Qi
2026-08-03  9:38     ` [f2fs-dev] " Wenjie Qi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.