Message ID | 1602986547-15886-2-git-send-email-dotdot@yandex-team.ru |
---|---|
State | Superseded |
Headers | show |
Series | [v2,1/2] ext4: add helpers for checking whether quota can be enabled/is journalled | expand |
Hi Roman, Thank you for the patch! Yet something to improve: [auto build test ERROR on ext4/dev] [also build test ERROR on v5.9 next-20201016] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Roman-Anufriev/ext4-add-helpers-for-checking-whether-quota-can-be-enabled-is-journalled/20201018-100410 base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev config: i386-randconfig-m021-20201018 (attached as .config) compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/9ee2e9dad32135b665f733b714c75ff22731bbcd git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Roman-Anufriev/ext4-add-helpers-for-checking-whether-quota-can-be-enabled-is-journalled/20201018-100410 git checkout 9ee2e9dad32135b665f733b714c75ff22731bbcd # save the attached .config to linux build tree make W=1 ARCH=i386 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): fs/ext4/super.c: In function 'ext4_quota_mode': >> fs/ext4/super.c:3999:19: error: expected ';' before '}' token 3999 | return "disabled" | ^ | ; 4000 | #endif 4001 | } | ~ fs/ext4/super.c: In function 'ext4_remount': fs/ext4/super.c:5738:6: warning: variable 'enable_quota' set but not used [-Wunused-but-set-variable] 5738 | int enable_quota = 0; | ^~~~~~~~~~~~ vim +3999 fs/ext4/super.c 3987 3988 static const char *ext4_quota_mode(struct super_block *sb) 3989 { 3990 #ifdef CONFIG_QUOTA 3991 if (!ext4_quota_capable(sb)) 3992 return "none"; 3993 3994 if (ext4_is_quota_journalled(sb)) 3995 return "journalled"; 3996 else 3997 return "writeback"; 3998 #else > 3999 return "disabled" 4000 #endif 4001 } 4002 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Sun, 18 Oct 2020, kernel test robot wrote: > Hi Roman, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on ext4/dev] > [also build test ERROR on v5.9 next-20201016] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch] > > url: https://github.com/0day-ci/linux/commits/Roman-Anufriev/ext4-add-helpers-for-checking-whether-quota-can-be-enabled-is-journalled/20201018-100410 > base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev > config: i386-randconfig-m021-20201018 (attached as .config) > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 > reproduce (this is a W=1 build): > # https://github.com/0day-ci/linux/commit/9ee2e9dad32135b665f733b714c75ff22731bbcd > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review Roman-Anufriev/ext4-add-helpers-for-checking-whether-quota-can-be-enabled-is-journalled/20201018-100410 > git checkout 9ee2e9dad32135b665f733b714c75ff22731bbcd > # save the attached .config to linux build tree > make W=1 ARCH=i386 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > > All errors (new ones prefixed by >>): > > fs/ext4/super.c: In function 'ext4_quota_mode': >>> fs/ext4/super.c:3999:19: error: expected ';' before '}' token > 3999 | return "disabled" > | ^ > | ; > 4000 | #endif > 4001 | } > | ~ > fs/ext4/super.c: In function 'ext4_remount': > fs/ext4/super.c:5738:6: warning: variable 'enable_quota' set but not used [-Wunused-but-set-variable] > 5738 | int enable_quota = 0; > | ^~~~~~~~~~~~ > > vim +3999 fs/ext4/super.c > > 3987 > 3988 static const char *ext4_quota_mode(struct super_block *sb) > 3989 { > 3990 #ifdef CONFIG_QUOTA > 3991 if (!ext4_quota_capable(sb)) > 3992 return "none"; > 3993 > 3994 if (ext4_is_quota_journalled(sb)) > 3995 return "journalled"; > 3996 else > 3997 return "writeback"; > 3998 #else >> 3999 return "disabled" > 4000 #endif > 4001 } > 4002 > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Fixed missing semicolon in v3: https://lore.kernel.org/linux-ext4/1603099162-25028-1-git-send-email-dotdot@yandex-team.ru/ Roman
On Sun 18-10-20 05:02:27, Roman Anufriev wrote: > Right now, it is hard to understand what quota journalling type is enabled: > you need to be quite familiar with kernel code and trace it or really > understand what different combinations of fs flags/mount options lead to. > > This patch adds printing of current quota jounalling mode on each > mount/remount, thus making it easier to check it at a glance/in autotests. > The semantics is similar to ext4 data journalling modes: > > * journalled - quota accounting and journaling are enabled > * writeback - quota accounting is enabled, but journalling is disabled The above two descriptions are still somewhat misleading - in fact we don't know whether accounting is enabled or not. Just *if* it is enabled, quota will be journalled / non-journalled. So I'd probably describe it like: * journalled - quota configured, journalling will be enabled * writeback - quota configured, journalling will be disabled We've talked with Ted on last ext4 conf call and we agreed that it's probably time to deprecate old style quotas in external quota files and transition everybody to using quotas with quota feature. That way things will get simpler again. But before we can disable that functionality, it will take a few years of deprecation warnings etc. so that's not directly related to your patch here. JFYI. Honza > * none - quota accounting is disabled > * disabled - kernel compiled without CONFIG_QUOTA feature > > Signed-off-by: Roman Anufriev <dotdot@yandex-team.ru> > --- > Changes in v2: > - Print quota journalling mode instead of exporting it via sysfs. > > fs/ext4/super.c | 23 ++++++++++++++++++++--- > 1 file changed, 20 insertions(+), 3 deletions(-) > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index a988cf3..09b5645 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -3985,6 +3985,21 @@ static void ext4_set_resv_clusters(struct super_block *sb) > atomic64_set(&sbi->s_resv_clusters, resv_clusters); > } > > +static const char *ext4_quota_mode(struct super_block *sb) > +{ > +#ifdef CONFIG_QUOTA > + if (!ext4_quota_capable(sb)) > + return "none"; > + > + if (ext4_is_quota_journalled(sb)) > + return "journalled"; > + else > + return "writeback"; > +#else > + return "disabled" > +#endif > +} > + > static int ext4_fill_super(struct super_block *sb, void *data, int silent) > { > struct dax_device *dax_dev = fs_dax_get_by_bdev(sb->s_bdev); > @@ -5039,10 +5054,11 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) > > if (___ratelimit(&ext4_mount_msg_ratelimit, "EXT4-fs mount")) > ext4_msg(sb, KERN_INFO, "mounted filesystem with%s. " > - "Opts: %.*s%s%s", descr, > + "Opts: %.*s%s%s. Quota mode: %s.", descr, > (int) sizeof(sbi->s_es->s_mount_opts), > sbi->s_es->s_mount_opts, > - *sbi->s_es->s_mount_opts ? "; " : "", orig_data); > + *sbi->s_es->s_mount_opts ? "; " : "", orig_data, > + ext4_quota_mode(sb)); > > if (es->s_error_count) > mod_timer(&sbi->s_err_report, jiffies + 300*HZ); /* 5 minutes */ > @@ -5979,7 +5995,8 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data) > */ > *flags = (*flags & ~vfs_flags) | (sb->s_flags & vfs_flags); > > - ext4_msg(sb, KERN_INFO, "re-mounted. Opts: %s", orig_data); > + ext4_msg(sb, KERN_INFO, "re-mounted. Opts: %s. Quota mode: %s.", > + orig_data, ext4_quota_mode(sb)); > kfree(orig_data); > return 0; > > -- > 2.7.4 >
On Mon, 19 Oct 2020, Jan Kara wrote: > On Sun 18-10-20 05:02:27, Roman Anufriev wrote: >> Right now, it is hard to understand what quota journalling type is enabled: >> you need to be quite familiar with kernel code and trace it or really >> understand what different combinations of fs flags/mount options lead to. >> >> This patch adds printing of current quota jounalling mode on each >> mount/remount, thus making it easier to check it at a glance/in autotests. >> The semantics is similar to ext4 data journalling modes: >> >> * journalled - quota accounting and journaling are enabled >> * writeback - quota accounting is enabled, but journalling is disabled > > The above two descriptions are still somewhat misleading - in fact we don't > know whether accounting is enabled or not. Just *if* it is enabled, quota > will be journalled / non-journalled. So I'd probably describe it like: > * journalled - quota configured, journalling will be enabled > * writeback - quota configured, journalling will be disabled Yeah, you are right, I'll fix this in v4. > We've talked with Ted on last ext4 conf call and we agreed that it's > probably time to deprecate old style quotas in external quota files and > transition everybody to using quotas with quota feature. That way things > will get simpler again. But before we can disable that functionality, it > will take a few years of deprecation warnings etc. so that's not directly > related to your patch here. JFYI. It will be great! Roman
diff --git a/fs/ext4/super.c b/fs/ext4/super.c index a988cf3..09b5645 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -3985,6 +3985,21 @@ static void ext4_set_resv_clusters(struct super_block *sb) atomic64_set(&sbi->s_resv_clusters, resv_clusters); } +static const char *ext4_quota_mode(struct super_block *sb) +{ +#ifdef CONFIG_QUOTA + if (!ext4_quota_capable(sb)) + return "none"; + + if (ext4_is_quota_journalled(sb)) + return "journalled"; + else + return "writeback"; +#else + return "disabled" +#endif +} + static int ext4_fill_super(struct super_block *sb, void *data, int silent) { struct dax_device *dax_dev = fs_dax_get_by_bdev(sb->s_bdev); @@ -5039,10 +5054,11 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) if (___ratelimit(&ext4_mount_msg_ratelimit, "EXT4-fs mount")) ext4_msg(sb, KERN_INFO, "mounted filesystem with%s. " - "Opts: %.*s%s%s", descr, + "Opts: %.*s%s%s. Quota mode: %s.", descr, (int) sizeof(sbi->s_es->s_mount_opts), sbi->s_es->s_mount_opts, - *sbi->s_es->s_mount_opts ? "; " : "", orig_data); + *sbi->s_es->s_mount_opts ? "; " : "", orig_data, + ext4_quota_mode(sb)); if (es->s_error_count) mod_timer(&sbi->s_err_report, jiffies + 300*HZ); /* 5 minutes */ @@ -5979,7 +5995,8 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data) */ *flags = (*flags & ~vfs_flags) | (sb->s_flags & vfs_flags); - ext4_msg(sb, KERN_INFO, "re-mounted. Opts: %s", orig_data); + ext4_msg(sb, KERN_INFO, "re-mounted. Opts: %s. Quota mode: %s.", + orig_data, ext4_quota_mode(sb)); kfree(orig_data); return 0;
Right now, it is hard to understand what quota journalling type is enabled: you need to be quite familiar with kernel code and trace it or really understand what different combinations of fs flags/mount options lead to. This patch adds printing of current quota jounalling mode on each mount/remount, thus making it easier to check it at a glance/in autotests. The semantics is similar to ext4 data journalling modes: * journalled - quota accounting and journaling are enabled * writeback - quota accounting is enabled, but journalling is disabled * none - quota accounting is disabled * disabled - kernel compiled without CONFIG_QUOTA feature Signed-off-by: Roman Anufriev <dotdot@yandex-team.ru> --- Changes in v2: - Print quota journalling mode instead of exporting it via sysfs. fs/ext4/super.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)