* [PATCH v2] f2fs: reserve space to be shrunk during resize_fs @ 2026-10-22 16:55 Daeho Jeong 2026-10-29 18:14 ` Daeho Jeong 2026-10-30 12:58 ` Chao Yu 0 siblings, 2 replies; 4+ messages in thread From: Daeho Jeong @ 2026-10-22 16:55 UTC (permalink / raw) To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong, Chao Yu From: Daeho Jeong <daehojeong@google.com> f2fs_resize_fs() drops gc_lock and the cp lock between the free space check and freeze_super(). In that window, others can allocate the free space, so the second check after freeze_super() fails with -ENOSPC. This is easy to hit on a busy filesystem, e.g. at OTA install time. Reserve the space to be shrunk via resize_reserved_blocks once the first check passes, and deduct it in __get_available_block_count() like alias_reserved_blocks. Release it on success or on error. With the reservation, the second check should not fail anymore, so: - allow root reserved blocks, which root may use in the meantime, - recheck DISABLE_CHECKPOINT, which remount may set in the meantime, since freeze_super() fails only when the fs is still dirty, - keep the check only as a WARN_ON_ONCE() safety net. Also exclude the alias/resize reservations when updating current_reserved_blocks via sysfs, otherwise it can underflow __get_available_block_count(). Suggested-by: Chao Yu <chao@kernel.org> Signed-off-by: Daeho Jeong <daehojeong@google.com> --- Changes in v2: - Exclude alias/resize reservations from current_reserved_blocks. - Allow root reserved blocks in the second check, with WARN_ON_ONCE(). - Recheck DISABLE_CHECKPOINT after freeze_super(). --- fs/f2fs/f2fs.h | 18 +++++++++++++---- fs/f2fs/gc.c | 54 ++++++++++++++++++++++++++++++++++++++----------- fs/f2fs/super.c | 1 + fs/f2fs/sysfs.c | 12 ++++++++++- 4 files changed, 68 insertions(+), 17 deletions(-) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 089a62c054e..6b9e1d0e71c 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -1897,6 +1897,7 @@ struct f2fs_sb_info { block_t reserved_blocks; /* configurable reserved blocks */ block_t current_reserved_blocks; /* current reserved blocks */ block_t alias_reserved_blocks; /* reserved blocks for device alias */ + block_t resize_reserved_blocks; /* reserved blocks for resize_fs */ /* Additional tracking for no checkpoint mode */ block_t unusable_block_count; /* # of blocks saved by last cp */ @@ -2666,16 +2667,18 @@ static inline bool __allow_reserved_root(struct f2fs_sb_info *sbi, return false; } -static inline unsigned int get_available_block_count(struct f2fs_sb_info *sbi, - struct inode *inode, bool cap) +static inline unsigned int __get_available_block_count( + struct f2fs_sb_info *sbi, + bool allow_root) { block_t avail_user_block_count; avail_user_block_count = sbi->user_block_count - sbi->current_reserved_blocks - - sbi->alias_reserved_blocks; + sbi->alias_reserved_blocks - + sbi->resize_reserved_blocks; - if (test_opt(sbi, RESERVE_ROOT) && !__allow_reserved_root(sbi, inode, cap)) + if (test_opt(sbi, RESERVE_ROOT) && !allow_root) avail_user_block_count -= F2FS_OPTION(sbi).root_reserved_blocks; if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) { @@ -2688,6 +2691,13 @@ static inline unsigned int get_available_block_count(struct f2fs_sb_info *sbi, return avail_user_block_count; } +static inline unsigned int get_available_block_count(struct f2fs_sb_info *sbi, + struct inode *inode, bool cap) +{ + return __get_available_block_count(sbi, + __allow_reserved_root(sbi, inode, cap)); +} + static inline void f2fs_i_blocks_write(struct inode *, block_t, bool, bool); static inline int inc_valid_block_count(struct f2fs_sb_info *sbi, struct inode *inode, blkcnt_t *count, diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index 926e6976bc2..955224f4166 100644 --- a/fs/f2fs/gc.c +++ b/fs/f2fs/gc.c @@ -2374,6 +2374,7 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) struct f2fs_lock_context glc; struct f2fs_lock_context clc; unsigned int secs; + bool reserved = false; int err = 0; __u32 rem; @@ -2425,10 +2426,17 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) f2fs_lock_op(sbi, &lc); spin_lock(&sbi->stat_lock); - if (shrunk_blocks + valid_user_blocks(sbi) + - sbi->current_reserved_blocks + sbi->unusable_block_count + - F2FS_OPTION(sbi).root_reserved_blocks > sbi->user_block_count) + if (shrunk_blocks + valid_user_blocks(sbi) > + __get_available_block_count(sbi, false)) { err = -ENOSPC; + } else { + /* + * Reserve the space to be shrunk so that no one, including + * root, can allocate it until the shrink is done. + */ + sbi->resize_reserved_blocks += shrunk_blocks; + reserved = true; + } spin_unlock(&sbi->stat_lock); if (err) @@ -2442,29 +2450,45 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) out_drop_write: mnt_drop_write_file(filp); if (err) - return err; + goto out_release; err = freeze_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); if (err) - return err; + goto out_release; if (f2fs_readonly(sbi->sb)) { err = thaw_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); - if (err) - return err; - return -EROFS; + if (!err) + err = -EROFS; + goto out_release; + } + + /* checkpoint can be disabled by remount before freeze_super() */ + if (test_opt(sbi, DISABLE_CHECKPOINT)) { + f2fs_err(sbi, "Checkpoint should be enabled."); + err = thaw_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); + if (!err) + err = -EINVAL; + goto out_release; } f2fs_down_write_trace(&sbi->gc_lock, &glc); f2fs_down_write_trace(&sbi->cp_global_sem, &clc); spin_lock(&sbi->stat_lock); - if (shrunk_blocks + valid_user_blocks(sbi) + - sbi->current_reserved_blocks + sbi->unusable_block_count + - F2FS_OPTION(sbi).root_reserved_blocks > sbi->user_block_count) + /* + * shrunk_blocks is already accounted in resize_reserved_blocks, so + * this should not fail. Allow root reserved blocks here since root + * may have used them after the first check, regardless of resize. + */ + if (WARN_ON_ONCE(valid_user_blocks(sbi) > + __get_available_block_count(sbi, true))) { err = -ENOSPC; - else + } else { sbi->user_block_count -= shrunk_blocks; + sbi->resize_reserved_blocks -= shrunk_blocks; + reserved = false; + } spin_unlock(&sbi->stat_lock); if (err) goto out_err; @@ -2510,5 +2534,11 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) f2fs_up_write_trace(&sbi->cp_global_sem, &clc); f2fs_up_write_trace(&sbi->gc_lock, &glc); thaw_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); +out_release: + if (reserved) { + spin_lock(&sbi->stat_lock); + sbi->resize_reserved_blocks -= shrunk_blocks; + spin_unlock(&sbi->stat_lock); + } return err; } diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index d683240040f..28e49e8d71e 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -5345,6 +5345,7 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc) sbi->reserved_blocks = 0; sbi->current_reserved_blocks = 0; sbi->alias_reserved_blocks = 0; + sbi->resize_reserved_blocks = 0; limit_reserve_root(sbi); adjust_unusable_cap_perc(sbi); diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c index 9749da70089..ce70d81eba3 100644 --- a/fs/f2fs/sysfs.c +++ b/fs/f2fs/sysfs.c @@ -594,6 +594,8 @@ static ssize_t __sbi_store(struct f2fs_attr *a, } #endif if (a->struct_type == RESERVED_BLOCKS) { + block_t avail, valid; + spin_lock(&sbi->stat_lock); if (t > (unsigned long)(sbi->user_block_count - F2FS_OPTION(sbi).root_reserved_blocks)) { @@ -601,8 +603,16 @@ static ssize_t __sbi_store(struct f2fs_attr *a, return -EINVAL; } *ui = t; + /* + * Exclude the space reserved for device aliasing and resize_fs, + * otherwise the reserved blocks can overlap with them and + * __get_available_block_count() can underflow. + */ + avail = sbi->user_block_count - sbi->alias_reserved_blocks - + sbi->resize_reserved_blocks; + valid = valid_user_blocks(sbi); sbi->current_reserved_blocks = min(sbi->reserved_blocks, - sbi->user_block_count - valid_user_blocks(sbi)); + avail > valid ? avail - valid : 0); spin_unlock(&sbi->stat_lock); return count; } -- 2.56.0.218.g1ec904b173-goog ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] f2fs: reserve space to be shrunk during resize_fs 2026-10-22 16:55 [PATCH v2] f2fs: reserve space to be shrunk during resize_fs Daeho Jeong @ 2026-10-29 18:14 ` Daeho Jeong 2026-10-30 12:58 ` Chao Yu 1 sibling, 0 replies; 4+ messages in thread From: Daeho Jeong @ 2026-10-29 18:14 UTC (permalink / raw) To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong, Chao Yu Kindly ping this. On Thu, Oct 22, 2026 at 9:55 AM Daeho Jeong <daeho43@gmail.com> wrote: > > From: Daeho Jeong <daehojeong@google.com> > > f2fs_resize_fs() drops gc_lock and the cp lock between the free space > check and freeze_super(). In that window, others can allocate the free > space, so the second check after freeze_super() fails with -ENOSPC. > This is easy to hit on a busy filesystem, e.g. at OTA install time. > > Reserve the space to be shrunk via resize_reserved_blocks once the > first check passes, and deduct it in __get_available_block_count() like > alias_reserved_blocks. Release it on success or on error. > > With the reservation, the second check should not fail anymore, so: > - allow root reserved blocks, which root may use in the meantime, > - recheck DISABLE_CHECKPOINT, which remount may set in the meantime, > since freeze_super() fails only when the fs is still dirty, > - keep the check only as a WARN_ON_ONCE() safety net. > > Also exclude the alias/resize reservations when updating > current_reserved_blocks via sysfs, otherwise it can underflow > __get_available_block_count(). > > Suggested-by: Chao Yu <chao@kernel.org> > Signed-off-by: Daeho Jeong <daehojeong@google.com> > --- > Changes in v2: > - Exclude alias/resize reservations from current_reserved_blocks. > - Allow root reserved blocks in the second check, with WARN_ON_ONCE(). > - Recheck DISABLE_CHECKPOINT after freeze_super(). > --- > fs/f2fs/f2fs.h | 18 +++++++++++++---- > fs/f2fs/gc.c | 54 ++++++++++++++++++++++++++++++++++++++----------- > fs/f2fs/super.c | 1 + > fs/f2fs/sysfs.c | 12 ++++++++++- > 4 files changed, 68 insertions(+), 17 deletions(-) > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 089a62c054e..6b9e1d0e71c 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -1897,6 +1897,7 @@ struct f2fs_sb_info { > block_t reserved_blocks; /* configurable reserved blocks */ > block_t current_reserved_blocks; /* current reserved blocks */ > block_t alias_reserved_blocks; /* reserved blocks for device alias */ > + block_t resize_reserved_blocks; /* reserved blocks for resize_fs */ > > /* Additional tracking for no checkpoint mode */ > block_t unusable_block_count; /* # of blocks saved by last cp */ > @@ -2666,16 +2667,18 @@ static inline bool __allow_reserved_root(struct f2fs_sb_info *sbi, > return false; > } > > -static inline unsigned int get_available_block_count(struct f2fs_sb_info *sbi, > - struct inode *inode, bool cap) > +static inline unsigned int __get_available_block_count( > + struct f2fs_sb_info *sbi, > + bool allow_root) > { > block_t avail_user_block_count; > > avail_user_block_count = sbi->user_block_count - > sbi->current_reserved_blocks - > - sbi->alias_reserved_blocks; > + sbi->alias_reserved_blocks - > + sbi->resize_reserved_blocks; > > - if (test_opt(sbi, RESERVE_ROOT) && !__allow_reserved_root(sbi, inode, cap)) > + if (test_opt(sbi, RESERVE_ROOT) && !allow_root) > avail_user_block_count -= F2FS_OPTION(sbi).root_reserved_blocks; > > if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) { > @@ -2688,6 +2691,13 @@ static inline unsigned int get_available_block_count(struct f2fs_sb_info *sbi, > return avail_user_block_count; > } > > +static inline unsigned int get_available_block_count(struct f2fs_sb_info *sbi, > + struct inode *inode, bool cap) > +{ > + return __get_available_block_count(sbi, > + __allow_reserved_root(sbi, inode, cap)); > +} > + > static inline void f2fs_i_blocks_write(struct inode *, block_t, bool, bool); > static inline int inc_valid_block_count(struct f2fs_sb_info *sbi, > struct inode *inode, blkcnt_t *count, > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index 926e6976bc2..955224f4166 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -2374,6 +2374,7 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) > struct f2fs_lock_context glc; > struct f2fs_lock_context clc; > unsigned int secs; > + bool reserved = false; > int err = 0; > __u32 rem; > > @@ -2425,10 +2426,17 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) > f2fs_lock_op(sbi, &lc); > > spin_lock(&sbi->stat_lock); > - if (shrunk_blocks + valid_user_blocks(sbi) + > - sbi->current_reserved_blocks + sbi->unusable_block_count + > - F2FS_OPTION(sbi).root_reserved_blocks > sbi->user_block_count) > + if (shrunk_blocks + valid_user_blocks(sbi) > > + __get_available_block_count(sbi, false)) { > err = -ENOSPC; > + } else { > + /* > + * Reserve the space to be shrunk so that no one, including > + * root, can allocate it until the shrink is done. > + */ > + sbi->resize_reserved_blocks += shrunk_blocks; > + reserved = true; > + } > spin_unlock(&sbi->stat_lock); > > if (err) > @@ -2442,29 +2450,45 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) > out_drop_write: > mnt_drop_write_file(filp); > if (err) > - return err; > + goto out_release; > > err = freeze_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); > if (err) > - return err; > + goto out_release; > > if (f2fs_readonly(sbi->sb)) { > err = thaw_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); > - if (err) > - return err; > - return -EROFS; > + if (!err) > + err = -EROFS; > + goto out_release; > + } > + > + /* checkpoint can be disabled by remount before freeze_super() */ > + if (test_opt(sbi, DISABLE_CHECKPOINT)) { > + f2fs_err(sbi, "Checkpoint should be enabled."); > + err = thaw_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); > + if (!err) > + err = -EINVAL; > + goto out_release; > } > > f2fs_down_write_trace(&sbi->gc_lock, &glc); > f2fs_down_write_trace(&sbi->cp_global_sem, &clc); > > spin_lock(&sbi->stat_lock); > - if (shrunk_blocks + valid_user_blocks(sbi) + > - sbi->current_reserved_blocks + sbi->unusable_block_count + > - F2FS_OPTION(sbi).root_reserved_blocks > sbi->user_block_count) > + /* > + * shrunk_blocks is already accounted in resize_reserved_blocks, so > + * this should not fail. Allow root reserved blocks here since root > + * may have used them after the first check, regardless of resize. > + */ > + if (WARN_ON_ONCE(valid_user_blocks(sbi) > > + __get_available_block_count(sbi, true))) { > err = -ENOSPC; > - else > + } else { > sbi->user_block_count -= shrunk_blocks; > + sbi->resize_reserved_blocks -= shrunk_blocks; > + reserved = false; > + } > spin_unlock(&sbi->stat_lock); > if (err) > goto out_err; > @@ -2510,5 +2534,11 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) > f2fs_up_write_trace(&sbi->cp_global_sem, &clc); > f2fs_up_write_trace(&sbi->gc_lock, &glc); > thaw_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); > +out_release: > + if (reserved) { > + spin_lock(&sbi->stat_lock); > + sbi->resize_reserved_blocks -= shrunk_blocks; > + spin_unlock(&sbi->stat_lock); > + } > return err; > } > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index d683240040f..28e49e8d71e 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -5345,6 +5345,7 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc) > sbi->reserved_blocks = 0; > sbi->current_reserved_blocks = 0; > sbi->alias_reserved_blocks = 0; > + sbi->resize_reserved_blocks = 0; > limit_reserve_root(sbi); > adjust_unusable_cap_perc(sbi); > > diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c > index 9749da70089..ce70d81eba3 100644 > --- a/fs/f2fs/sysfs.c > +++ b/fs/f2fs/sysfs.c > @@ -594,6 +594,8 @@ static ssize_t __sbi_store(struct f2fs_attr *a, > } > #endif > if (a->struct_type == RESERVED_BLOCKS) { > + block_t avail, valid; > + > spin_lock(&sbi->stat_lock); > if (t > (unsigned long)(sbi->user_block_count - > F2FS_OPTION(sbi).root_reserved_blocks)) { > @@ -601,8 +603,16 @@ static ssize_t __sbi_store(struct f2fs_attr *a, > return -EINVAL; > } > *ui = t; > + /* > + * Exclude the space reserved for device aliasing and resize_fs, > + * otherwise the reserved blocks can overlap with them and > + * __get_available_block_count() can underflow. > + */ > + avail = sbi->user_block_count - sbi->alias_reserved_blocks - > + sbi->resize_reserved_blocks; > + valid = valid_user_blocks(sbi); > sbi->current_reserved_blocks = min(sbi->reserved_blocks, > - sbi->user_block_count - valid_user_blocks(sbi)); > + avail > valid ? avail - valid : 0); > spin_unlock(&sbi->stat_lock); > return count; > } > -- > 2.56.0.218.g1ec904b173-goog > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] f2fs: reserve space to be shrunk during resize_fs 2026-10-22 16:55 [PATCH v2] f2fs: reserve space to be shrunk during resize_fs Daeho Jeong 2026-10-29 18:14 ` Daeho Jeong @ 2026-10-30 12:58 ` Chao Yu 2026-10-30 21:21 ` Daeho Jeong 1 sibling, 1 reply; 4+ messages in thread From: Chao Yu @ 2026-10-30 12:58 UTC (permalink / raw) To: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team Cc: chao, Daeho Jeong Daeho, sorry for the delay. On 10/23/26 00:55, Daeho Jeong wrote: > From: Daeho Jeong <daehojeong@google.com> > > f2fs_resize_fs() drops gc_lock and the cp lock between the free space > check and freeze_super(). In that window, others can allocate the free > space, so the second check after freeze_super() fails with -ENOSPC. > This is easy to hit on a busy filesystem, e.g. at OTA install time. > > Reserve the space to be shrunk via resize_reserved_blocks once the > first check passes, and deduct it in __get_available_block_count() like > alias_reserved_blocks. Release it on success or on error. > > With the reservation, the second check should not fail anymore, so: > - allow root reserved blocks, which root may use in the meantime, > - recheck DISABLE_CHECKPOINT, which remount may set in the meantime, > since freeze_super() fails only when the fs is still dirty, > - keep the check only as a WARN_ON_ONCE() safety net. > > Also exclude the alias/resize reservations when updating > current_reserved_blocks via sysfs, otherwise it can underflow > __get_available_block_count(). > > Suggested-by: Chao Yu <chao@kernel.org> > Signed-off-by: Daeho Jeong <daehojeong@google.com> > --- > Changes in v2: > - Exclude alias/resize reservations from current_reserved_blocks. > - Allow root reserved blocks in the second check, with WARN_ON_ONCE(). > - Recheck DISABLE_CHECKPOINT after freeze_super(). > --- > fs/f2fs/f2fs.h | 18 +++++++++++++---- > fs/f2fs/gc.c | 54 ++++++++++++++++++++++++++++++++++++++----------- > fs/f2fs/super.c | 1 + > fs/f2fs/sysfs.c | 12 ++++++++++- > 4 files changed, 68 insertions(+), 17 deletions(-) > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 089a62c054e..6b9e1d0e71c 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -1897,6 +1897,7 @@ struct f2fs_sb_info { > block_t reserved_blocks; /* configurable reserved blocks */ > block_t current_reserved_blocks; /* current reserved blocks */ > block_t alias_reserved_blocks; /* reserved blocks for device alias */ > + block_t resize_reserved_blocks; /* reserved blocks for resize_fs */ > > /* Additional tracking for no checkpoint mode */ > block_t unusable_block_count; /* # of blocks saved by last cp */ > @@ -2666,16 +2667,18 @@ static inline bool __allow_reserved_root(struct f2fs_sb_info *sbi, > return false; > } > > -static inline unsigned int get_available_block_count(struct f2fs_sb_info *sbi, > - struct inode *inode, bool cap) > +static inline unsigned int __get_available_block_count( > + struct f2fs_sb_info *sbi, > + bool allow_root) > { > block_t avail_user_block_count; > > avail_user_block_count = sbi->user_block_count - > sbi->current_reserved_blocks - > - sbi->alias_reserved_blocks; > + sbi->alias_reserved_blocks - > + sbi->resize_reserved_blocks; Can you please take a look at comments from sashiko? it seems valid: https://sashiko.dev/#/patchset/20261022165526.1539629-1-daeho43%40gmail.com __get_available_block_count() can underflow here: avail in __sbi_store() doesn't exclude root_reserved_blocks, so once valid_user_blocks() < root_reserved_blocks, current_reserved_blocks can overlap with root reserved space, and the unguarded "avail_user_block_count -= root_reserved_blocks" wraps block_t. e.g. user_block_count = 1000, root_reserved_blocks = 100, valid = 30, reserved_blocks = current_reserved_blocks = alias_reserved_blocks = 0 Thread A Thread B (root) - f2fs_resize_fs(shrunk_blocks = 500) - 500 + 30 <= avail(false) = 900, ok - resize_reserved_blocks = 500 - drop gc_lock/cp lock - echo 500 > /sys/fs/f2fs/<dev>/reserved_blocks - 500 <= 1000 - 100, ok - avail = 1000 - 500 = 500 - current_reserved_blocks = min(500, 500 - 30) = 470 Now for a non-root user: __get_available_block_count(sbi, false) = 1000 - 470 - 0 - 500 = 30, then 30 - 100 => 0xFFFFFFBA Thanks, > > - if (test_opt(sbi, RESERVE_ROOT) && !__allow_reserved_root(sbi, inode, cap)) > + if (test_opt(sbi, RESERVE_ROOT) && !allow_root) > avail_user_block_count -= F2FS_OPTION(sbi).root_reserved_blocks; > > if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) { > @@ -2688,6 +2691,13 @@ static inline unsigned int get_available_block_count(struct f2fs_sb_info *sbi, > return avail_user_block_count; > } > > +static inline unsigned int get_available_block_count(struct f2fs_sb_info *sbi, > + struct inode *inode, bool cap) > +{ > + return __get_available_block_count(sbi, > + __allow_reserved_root(sbi, inode, cap)); > +} > + > static inline void f2fs_i_blocks_write(struct inode *, block_t, bool, bool); > static inline int inc_valid_block_count(struct f2fs_sb_info *sbi, > struct inode *inode, blkcnt_t *count, > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index 926e6976bc2..955224f4166 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -2374,6 +2374,7 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) > struct f2fs_lock_context glc; > struct f2fs_lock_context clc; > unsigned int secs; > + bool reserved = false; > int err = 0; > __u32 rem; > > @@ -2425,10 +2426,17 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) > f2fs_lock_op(sbi, &lc); > > spin_lock(&sbi->stat_lock); > - if (shrunk_blocks + valid_user_blocks(sbi) + > - sbi->current_reserved_blocks + sbi->unusable_block_count + > - F2FS_OPTION(sbi).root_reserved_blocks > sbi->user_block_count) > + if (shrunk_blocks + valid_user_blocks(sbi) > > + __get_available_block_count(sbi, false)) { > err = -ENOSPC; > + } else { > + /* > + * Reserve the space to be shrunk so that no one, including > + * root, can allocate it until the shrink is done. > + */ > + sbi->resize_reserved_blocks += shrunk_blocks; > + reserved = true; > + } > spin_unlock(&sbi->stat_lock); > > if (err) > @@ -2442,29 +2450,45 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) > out_drop_write: > mnt_drop_write_file(filp); > if (err) > - return err; > + goto out_release; > > err = freeze_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); > if (err) > - return err; > + goto out_release; > > if (f2fs_readonly(sbi->sb)) { > err = thaw_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); > - if (err) > - return err; > - return -EROFS; > + if (!err) > + err = -EROFS; > + goto out_release; > + } > + > + /* checkpoint can be disabled by remount before freeze_super() */ > + if (test_opt(sbi, DISABLE_CHECKPOINT)) { > + f2fs_err(sbi, "Checkpoint should be enabled."); > + err = thaw_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); > + if (!err) > + err = -EINVAL; > + goto out_release; > } > > f2fs_down_write_trace(&sbi->gc_lock, &glc); > f2fs_down_write_trace(&sbi->cp_global_sem, &clc); > > spin_lock(&sbi->stat_lock); > - if (shrunk_blocks + valid_user_blocks(sbi) + > - sbi->current_reserved_blocks + sbi->unusable_block_count + > - F2FS_OPTION(sbi).root_reserved_blocks > sbi->user_block_count) > + /* > + * shrunk_blocks is already accounted in resize_reserved_blocks, so > + * this should not fail. Allow root reserved blocks here since root > + * may have used them after the first check, regardless of resize. > + */ > + if (WARN_ON_ONCE(valid_user_blocks(sbi) > > + __get_available_block_count(sbi, true))) { > err = -ENOSPC; > - else > + } else { > sbi->user_block_count -= shrunk_blocks; > + sbi->resize_reserved_blocks -= shrunk_blocks; > + reserved = false; > + } > spin_unlock(&sbi->stat_lock); > if (err) > goto out_err; > @@ -2510,5 +2534,11 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) > f2fs_up_write_trace(&sbi->cp_global_sem, &clc); > f2fs_up_write_trace(&sbi->gc_lock, &glc); > thaw_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); > +out_release: > + if (reserved) { > + spin_lock(&sbi->stat_lock); > + sbi->resize_reserved_blocks -= shrunk_blocks; > + spin_unlock(&sbi->stat_lock); > + } > return err; > } > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index d683240040f..28e49e8d71e 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -5345,6 +5345,7 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc) > sbi->reserved_blocks = 0; > sbi->current_reserved_blocks = 0; > sbi->alias_reserved_blocks = 0; > + sbi->resize_reserved_blocks = 0; > limit_reserve_root(sbi); > adjust_unusable_cap_perc(sbi); > > diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c > index 9749da70089..ce70d81eba3 100644 > --- a/fs/f2fs/sysfs.c > +++ b/fs/f2fs/sysfs.c > @@ -594,6 +594,8 @@ static ssize_t __sbi_store(struct f2fs_attr *a, > } > #endif > if (a->struct_type == RESERVED_BLOCKS) { > + block_t avail, valid; > + > spin_lock(&sbi->stat_lock); > if (t > (unsigned long)(sbi->user_block_count - > F2FS_OPTION(sbi).root_reserved_blocks)) { > @@ -601,8 +603,16 @@ static ssize_t __sbi_store(struct f2fs_attr *a, > return -EINVAL; > } > *ui = t; > + /* > + * Exclude the space reserved for device aliasing and resize_fs, > + * otherwise the reserved blocks can overlap with them and > + * __get_available_block_count() can underflow. > + */ > + avail = sbi->user_block_count - sbi->alias_reserved_blocks - > + sbi->resize_reserved_blocks; > + valid = valid_user_blocks(sbi); > sbi->current_reserved_blocks = min(sbi->reserved_blocks, > - sbi->user_block_count - valid_user_blocks(sbi)); > + avail > valid ? avail - valid : 0); > spin_unlock(&sbi->stat_lock); > return count; > } ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] f2fs: reserve space to be shrunk during resize_fs 2026-10-30 12:58 ` Chao Yu @ 2026-10-30 21:21 ` Daeho Jeong 0 siblings, 0 replies; 4+ messages in thread From: Daeho Jeong @ 2026-10-30 21:21 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong On Fri, Oct 30, 2026 at 5:58 AM Chao Yu <chao@kernel.org> wrote: > > Daeho, sorry for the delay. > > On 10/23/26 00:55, Daeho Jeong wrote: > > From: Daeho Jeong <daehojeong@google.com> > > > > f2fs_resize_fs() drops gc_lock and the cp lock between the free space > > check and freeze_super(). In that window, others can allocate the free > > space, so the second check after freeze_super() fails with -ENOSPC. > > This is easy to hit on a busy filesystem, e.g. at OTA install time. > > > > Reserve the space to be shrunk via resize_reserved_blocks once the > > first check passes, and deduct it in __get_available_block_count() like > > alias_reserved_blocks. Release it on success or on error. > > > > With the reservation, the second check should not fail anymore, so: > > - allow root reserved blocks, which root may use in the meantime, > > - recheck DISABLE_CHECKPOINT, which remount may set in the meantime, > > since freeze_super() fails only when the fs is still dirty, > > - keep the check only as a WARN_ON_ONCE() safety net. > > > > Also exclude the alias/resize reservations when updating > > current_reserved_blocks via sysfs, otherwise it can underflow > > __get_available_block_count(). > > > > Suggested-by: Chao Yu <chao@kernel.org> > > Signed-off-by: Daeho Jeong <daehojeong@google.com> > > --- > > Changes in v2: > > - Exclude alias/resize reservations from current_reserved_blocks. > > - Allow root reserved blocks in the second check, with WARN_ON_ONCE(). > > - Recheck DISABLE_CHECKPOINT after freeze_super(). > > --- > > fs/f2fs/f2fs.h | 18 +++++++++++++---- > > fs/f2fs/gc.c | 54 ++++++++++++++++++++++++++++++++++++++----------- > > fs/f2fs/super.c | 1 + > > fs/f2fs/sysfs.c | 12 ++++++++++- > > 4 files changed, 68 insertions(+), 17 deletions(-) > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > > index 089a62c054e..6b9e1d0e71c 100644 > > --- a/fs/f2fs/f2fs.h > > +++ b/fs/f2fs/f2fs.h > > @@ -1897,6 +1897,7 @@ struct f2fs_sb_info { > > block_t reserved_blocks; /* configurable reserved blocks */ > > block_t current_reserved_blocks; /* current reserved blocks */ > > block_t alias_reserved_blocks; /* reserved blocks for device alias */ > > + block_t resize_reserved_blocks; /* reserved blocks for resize_fs */ > > > > /* Additional tracking for no checkpoint mode */ > > block_t unusable_block_count; /* # of blocks saved by last cp */ > > @@ -2666,16 +2667,18 @@ static inline bool __allow_reserved_root(struct f2fs_sb_info *sbi, > > return false; > > } > > > > -static inline unsigned int get_available_block_count(struct f2fs_sb_info *sbi, > > - struct inode *inode, bool cap) > > +static inline unsigned int __get_available_block_count( > > + struct f2fs_sb_info *sbi, > > + bool allow_root) > > { > > block_t avail_user_block_count; > > > > avail_user_block_count = sbi->user_block_count - > > sbi->current_reserved_blocks - > > - sbi->alias_reserved_blocks; > > + sbi->alias_reserved_blocks - > > + sbi->resize_reserved_blocks; > > Can you please take a look at comments from sashiko? it seems valid: > > https://sashiko.dev/#/patchset/20261022165526.1539629-1-daeho43%40gmail.com > > __get_available_block_count() can underflow here: avail in __sbi_store() doesn't > exclude root_reserved_blocks, so once valid_user_blocks() < root_reserved_blocks, > current_reserved_blocks can overlap with root reserved space, and the unguarded > "avail_user_block_count -= root_reserved_blocks" wraps block_t. > > e.g. > user_block_count = 1000, root_reserved_blocks = 100, valid = 30, > reserved_blocks = current_reserved_blocks = alias_reserved_blocks = 0 > > Thread A Thread B (root) > - f2fs_resize_fs(shrunk_blocks = 500) > - 500 + 30 <= avail(false) = 900, ok > - resize_reserved_blocks = 500 > - drop gc_lock/cp lock > - echo 500 > /sys/fs/f2fs/<dev>/reserved_blocks > - 500 <= 1000 - 100, ok > - avail = 1000 - 500 = 500 > - current_reserved_blocks = > min(500, 500 - 30) = 470 > > Now for a non-root user: > __get_available_block_count(sbi, false) > = 1000 - 470 - 0 - 500 = 30, then 30 - 100 => 0xFFFFFFBA > > Thanks, > Hi Chao, Thanks, it's valid. Actually, the same underflow already exists with device aliasing: f2fs_ioc_reserve_dev_alias() checks the space with root reserved blocks allowed, so alias_reserved_blocks can eat into the root reserved space, e.g. user_block_count = 1000, root_reserved = 100, valid = 30, reserving an alias of 950 blocks passes the check, and then 1000 - 950 - 100 wraps for non-root users. Clamping current_reserved_blocks in sysfs by root_reserved_blocks is not enough either, since dec_valid_{block,node}_count() grows current_reserved_blocks back up to reserved_blocks without considering alias/resize reservations. So, I'll make __get_available_block_count() saturate at 0, like what we already do for unusable_block_count, as a separate fix for device aliasing, and rebase this patch on top of it in v3. WDYT? Thanks, > > > > - if (test_opt(sbi, RESERVE_ROOT) && !__allow_reserved_root(sbi, inode, cap)) > > + if (test_opt(sbi, RESERVE_ROOT) && !allow_root) > > avail_user_block_count -= F2FS_OPTION(sbi).root_reserved_blocks; > > > > if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) { > > @@ -2688,6 +2691,13 @@ static inline unsigned int get_available_block_count(struct f2fs_sb_info *sbi, > > return avail_user_block_count; > > } > > > > +static inline unsigned int get_available_block_count(struct f2fs_sb_info *sbi, > > + struct inode *inode, bool cap) > > +{ > > + return __get_available_block_count(sbi, > > + __allow_reserved_root(sbi, inode, cap)); > > +} > > + > > static inline void f2fs_i_blocks_write(struct inode *, block_t, bool, bool); > > static inline int inc_valid_block_count(struct f2fs_sb_info *sbi, > > struct inode *inode, blkcnt_t *count, > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > > index 926e6976bc2..955224f4166 100644 > > --- a/fs/f2fs/gc.c > > +++ b/fs/f2fs/gc.c > > @@ -2374,6 +2374,7 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) > > struct f2fs_lock_context glc; > > struct f2fs_lock_context clc; > > unsigned int secs; > > + bool reserved = false; > > int err = 0; > > __u32 rem; > > > > @@ -2425,10 +2426,17 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) > > f2fs_lock_op(sbi, &lc); > > > > spin_lock(&sbi->stat_lock); > > - if (shrunk_blocks + valid_user_blocks(sbi) + > > - sbi->current_reserved_blocks + sbi->unusable_block_count + > > - F2FS_OPTION(sbi).root_reserved_blocks > sbi->user_block_count) > > + if (shrunk_blocks + valid_user_blocks(sbi) > > > + __get_available_block_count(sbi, false)) { > > err = -ENOSPC; > > + } else { > > + /* > > + * Reserve the space to be shrunk so that no one, including > > + * root, can allocate it until the shrink is done. > > + */ > > + sbi->resize_reserved_blocks += shrunk_blocks; > > + reserved = true; > > + } > > spin_unlock(&sbi->stat_lock); > > > > if (err) > > @@ -2442,29 +2450,45 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) > > out_drop_write: > > mnt_drop_write_file(filp); > > if (err) > > - return err; > > + goto out_release; > > > > err = freeze_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); > > if (err) > > - return err; > > + goto out_release; > > > > if (f2fs_readonly(sbi->sb)) { > > err = thaw_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); > > - if (err) > > - return err; > > - return -EROFS; > > + if (!err) > > + err = -EROFS; > > + goto out_release; > > + } > > + > > + /* checkpoint can be disabled by remount before freeze_super() */ > > + if (test_opt(sbi, DISABLE_CHECKPOINT)) { > > + f2fs_err(sbi, "Checkpoint should be enabled."); > > + err = thaw_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); > > + if (!err) > > + err = -EINVAL; > > + goto out_release; > > } > > > > f2fs_down_write_trace(&sbi->gc_lock, &glc); > > f2fs_down_write_trace(&sbi->cp_global_sem, &clc); > > > > spin_lock(&sbi->stat_lock); > > - if (shrunk_blocks + valid_user_blocks(sbi) + > > - sbi->current_reserved_blocks + sbi->unusable_block_count + > > - F2FS_OPTION(sbi).root_reserved_blocks > sbi->user_block_count) > > + /* > > + * shrunk_blocks is already accounted in resize_reserved_blocks, so > > + * this should not fail. Allow root reserved blocks here since root > > + * may have used them after the first check, regardless of resize. > > + */ > > + if (WARN_ON_ONCE(valid_user_blocks(sbi) > > > + __get_available_block_count(sbi, true))) { > > err = -ENOSPC; > > - else > > + } else { > > sbi->user_block_count -= shrunk_blocks; > > + sbi->resize_reserved_blocks -= shrunk_blocks; > > + reserved = false; > > + } > > spin_unlock(&sbi->stat_lock); > > if (err) > > goto out_err; > > @@ -2510,5 +2534,11 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) > > f2fs_up_write_trace(&sbi->cp_global_sem, &clc); > > f2fs_up_write_trace(&sbi->gc_lock, &glc); > > thaw_super(sbi->sb, FREEZE_HOLDER_KERNEL, NULL); > > +out_release: > > + if (reserved) { > > + spin_lock(&sbi->stat_lock); > > + sbi->resize_reserved_blocks -= shrunk_blocks; > > + spin_unlock(&sbi->stat_lock); > > + } > > return err; > > } > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > > index d683240040f..28e49e8d71e 100644 > > --- a/fs/f2fs/super.c > > +++ b/fs/f2fs/super.c > > @@ -5345,6 +5345,7 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc) > > sbi->reserved_blocks = 0; > > sbi->current_reserved_blocks = 0; > > sbi->alias_reserved_blocks = 0; > > + sbi->resize_reserved_blocks = 0; > > limit_reserve_root(sbi); > > adjust_unusable_cap_perc(sbi); > > > > diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c > > index 9749da70089..ce70d81eba3 100644 > > --- a/fs/f2fs/sysfs.c > > +++ b/fs/f2fs/sysfs.c > > @@ -594,6 +594,8 @@ static ssize_t __sbi_store(struct f2fs_attr *a, > > } > > #endif > > if (a->struct_type == RESERVED_BLOCKS) { > > + block_t avail, valid; > > + > > spin_lock(&sbi->stat_lock); > > if (t > (unsigned long)(sbi->user_block_count - > > F2FS_OPTION(sbi).root_reserved_blocks)) { > > @@ -601,8 +603,16 @@ static ssize_t __sbi_store(struct f2fs_attr *a, > > return -EINVAL; > > } > > *ui = t; > > + /* > > + * Exclude the space reserved for device aliasing and resize_fs, > > + * otherwise the reserved blocks can overlap with them and > > + * __get_available_block_count() can underflow. > > + */ > > + avail = sbi->user_block_count - sbi->alias_reserved_blocks - > > + sbi->resize_reserved_blocks; > > + valid = valid_user_blocks(sbi); > > sbi->current_reserved_blocks = min(sbi->reserved_blocks, > > - sbi->user_block_count - valid_user_blocks(sbi)); > > + avail > valid ? avail - valid : 0); > > spin_unlock(&sbi->stat_lock); > > return count; > > } > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-30 21:21 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-22 16:55 [PATCH v2] f2fs: reserve space to be shrunk during resize_fs Daeho Jeong 2026-10-29 18:14 ` Daeho Jeong 2026-10-30 12:58 ` Chao Yu 2026-10-30 21:21 ` Daeho Jeong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®