| Message ID | 20250329-work-freeze-v2-5-a47af37ecc3d@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Extend freeze support to suspend and hibernate | expand |
On Sat 29-03-25 09:42:18, Christian Brauner wrote: > Use a common iterator for all callbacks. We could go for something even > more elaborate (advance step-by-step similar to iov_iter) but I really > don't think this is warranted. > > Signed-off-by: Christian Brauner <brauner@kernel.org> Looks good, one nit below. With that fixed feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> > +#define invalid_super list_entry_is_head Why do you have this invalid_super define? I find it rather confusing in the loop below and list_entry_is_head() would be much more understandable... Honza > + > +static void __iterate_supers(void (*f)(struct super_block *, void *), void *arg, > + enum super_iter_flags_t flags) > { > struct super_block *sb, *p = NULL; > + bool excl = flags & SUPER_ITER_EXCL; > > - spin_lock(&sb_lock); > - list_for_each_entry(sb, &super_blocks, s_list) { > - bool locked; > + guard(spinlock)(&sb_lock); > > + for (sb = first_super(flags); !invalid_super(sb, &super_blocks, s_list); > + sb = next_super(sb, flags)) { > if (super_flags(sb, SB_DYING)) > continue; > sb->s_count++; > spin_unlock(&sb_lock); > > - locked = super_lock(sb, excl); > - if (locked) { > + if (flags & SUPER_ITER_UNLOCKED) { > + f(sb, arg); > + } else if (super_lock(sb, excl)) { > f(sb, arg); > super_unlock(sb, excl); > }
On Mon, Mar 31, 2025 at 12:07:12PM +0200, Jan Kara wrote: > On Sat 29-03-25 09:42:18, Christian Brauner wrote: > > Use a common iterator for all callbacks. We could go for something even > > more elaborate (advance step-by-step similar to iov_iter) but I really > > don't think this is warranted. > > > > Signed-off-by: Christian Brauner <brauner@kernel.org> > > Looks good, one nit below. With that fixed feel free to add: > > Reviewed-by: Jan Kara <jack@suse.cz> > > > +#define invalid_super list_entry_is_head > > Why do you have this invalid_super define? I find it rather confusing in > the loop below and list_entry_is_head() would be much more > understandable... Fair, I just wanted a shorter name but I'll change it to list_entry_is_head() and push it out. > > Honza > > > + > > +static void __iterate_supers(void (*f)(struct super_block *, void *), void *arg, > > + enum super_iter_flags_t flags) > > { > > struct super_block *sb, *p = NULL; > > + bool excl = flags & SUPER_ITER_EXCL; > > > > - spin_lock(&sb_lock); > > - list_for_each_entry(sb, &super_blocks, s_list) { > > - bool locked; > > + guard(spinlock)(&sb_lock); > > > > + for (sb = first_super(flags); !invalid_super(sb, &super_blocks, s_list); > > + sb = next_super(sb, flags)) { > > if (super_flags(sb, SB_DYING)) > > continue; > > sb->s_count++; > > spin_unlock(&sb_lock); > > > > - locked = super_lock(sb, excl); > > - if (locked) { > > + if (flags & SUPER_ITER_UNLOCKED) { > > + f(sb, arg); > > + } else if (super_lock(sb, excl)) { > > f(sb, arg); > > super_unlock(sb, excl); > > } > -- > Jan Kara <jack@suse.com> > SUSE Labs, CR
diff --git a/fs/super.c b/fs/super.c index 0dd208804a74..666a2a16df87 100644 --- a/fs/super.c +++ b/fs/super.c @@ -887,21 +887,47 @@ void drop_super_exclusive(struct super_block *sb) } EXPORT_SYMBOL(drop_super_exclusive); -void __iterate_supers(void (*f)(struct super_block *, void *), void *arg, bool excl) +enum super_iter_flags_t { + SUPER_ITER_EXCL = (1U << 0), + SUPER_ITER_UNLOCKED = (1U << 1), + SUPER_ITER_REVERSE = (1U << 2), +}; + +static inline struct super_block *first_super(enum super_iter_flags_t flags) +{ + if (flags & SUPER_ITER_REVERSE) + return list_last_entry(&super_blocks, struct super_block, s_list); + return list_first_entry(&super_blocks, struct super_block, s_list); +} + +static inline struct super_block *next_super(struct super_block *sb, + enum super_iter_flags_t flags) +{ + if (flags & SUPER_ITER_REVERSE) + return list_prev_entry(sb, s_list); + return list_next_entry(sb, s_list); +} + +#define invalid_super list_entry_is_head + +static void __iterate_supers(void (*f)(struct super_block *, void *), void *arg, + enum super_iter_flags_t flags) { struct super_block *sb, *p = NULL; + bool excl = flags & SUPER_ITER_EXCL; - spin_lock(&sb_lock); - list_for_each_entry(sb, &super_blocks, s_list) { - bool locked; + guard(spinlock)(&sb_lock); + for (sb = first_super(flags); !invalid_super(sb, &super_blocks, s_list); + sb = next_super(sb, flags)) { if (super_flags(sb, SB_DYING)) continue; sb->s_count++; spin_unlock(&sb_lock); - locked = super_lock(sb, excl); - if (locked) { + if (flags & SUPER_ITER_UNLOCKED) { + f(sb, arg); + } else if (super_lock(sb, excl)) { f(sb, arg); super_unlock(sb, excl); } @@ -913,7 +939,11 @@ void __iterate_supers(void (*f)(struct super_block *, void *), void *arg, bool e } if (p) __put_super(p); - spin_unlock(&sb_lock); +} + +void iterate_supers(void (*f)(struct super_block *, void *), void *arg) +{ + __iterate_supers(f, arg, 0); } /** @@ -1097,7 +1127,8 @@ static void do_emergency_remount_callback(struct super_block *sb, void *unused) static void do_emergency_remount(struct work_struct *work) { - __iterate_supers(do_emergency_remount_callback, NULL, true); + __iterate_supers(do_emergency_remount_callback, NULL, + SUPER_ITER_EXCL | SUPER_ITER_REVERSE); kfree(work); printk("Emergency Remount complete\n"); } @@ -1124,7 +1155,7 @@ static void do_thaw_all_callback(struct super_block *sb, void *unused) static void do_thaw_all(struct work_struct *work) { - __iterate_supers(do_thaw_all_callback, NULL, true); + __iterate_supers(do_thaw_all_callback, NULL, SUPER_ITER_EXCL); kfree(work); printk(KERN_WARNING "Emergency Thaw complete\n"); } diff --git a/include/linux/fs.h b/include/linux/fs.h index 0351500b71d2..c475fa874055 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -3515,11 +3515,7 @@ extern void put_filesystem(struct file_system_type *fs); extern struct file_system_type *get_fs_type(const char *name); extern void drop_super(struct super_block *sb); extern void drop_super_exclusive(struct super_block *sb); -void __iterate_supers(void (*f)(struct super_block *, void *), void *arg, bool excl); -static inline void iterate_supers(void (*f)(struct super_block *, void *), void *arg) -{ - __iterate_supers(f, arg, false); -} +extern void iterate_supers(void (*f)(struct super_block *, void *), void *arg); extern void iterate_supers_type(struct file_system_type *, void (*)(struct super_block *, void *), void *);
Use a common iterator for all callbacks. We could go for something even more elaborate (advance step-by-step similar to iov_iter) but I really don't think this is warranted. Signed-off-by: Christian Brauner <brauner@kernel.org> --- fs/super.c | 49 ++++++++++++++++++++++++++++++++++++++++--------- include/linux/fs.h | 6 +----- 2 files changed, 41 insertions(+), 14 deletions(-)