linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] ext4: add helpers for checking whether quota can be enabled/is journalled
@ 2020-10-18  2:02 Roman Anufriev
  2020-10-18  2:02 ` [PATCH v2 2/2] ext4: print quota journalling mode on (re-)mount Roman Anufriev
  2020-10-19  9:37 ` [PATCH v2 1/2] ext4: add helpers for checking whether quota can be enabled/is journalled Jan Kara
  0 siblings, 2 replies; 9+ messages in thread
From: Roman Anufriev @ 2020-10-18  2:02 UTC (permalink / raw)
  To: linux-ext4; +Cc: tytso, jack, dmtrmonakhov, dotdot

Right now, there are several places, where we check whether fs is
capable of enabling quota or if quota is journalled with quite long
and non-self-descriptive condition statements.

This patch wraps these statements into helpers for better readability
and easier usage.

Signed-off-by: Roman Anufriev <dotdot@yandex-team.ru>
---
Changes in v2:
  - Fix misleading helper name 'ext4_any_quota_enabled()' ->
    'ext4_quota_capable()'.

 fs/ext4/ext4.h      | 15 +++++++++++++++
 fs/ext4/ext4_jbd2.h |  9 +++------
 fs/ext4/super.c     |  5 +----
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 250e905..897df24 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3251,6 +3251,21 @@ static inline void ext4_unlock_group(struct super_block *sb,
 	spin_unlock(ext4_group_lock_ptr(sb, group));
 }
 
+#ifdef CONFIG_QUOTA
+static inline bool ext4_quota_capable(struct super_block *sb)
+{
+	return (test_opt(sb, QUOTA) || ext4_has_feature_quota(sb));
+}
+
+static inline bool ext4_is_quota_journalled(struct super_block *sb)
+{
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
+
+	return (ext4_has_feature_quota(sb) ||
+		sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]);
+}
+#endif
+
 /*
  * Block validity checking
  */
diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
index 00dc668..a124c68 100644
--- a/fs/ext4/ext4_jbd2.h
+++ b/fs/ext4/ext4_jbd2.h
@@ -86,17 +86,14 @@
 #ifdef CONFIG_QUOTA
 /* Amount of blocks needed for quota update - we know that the structure was
  * allocated so we need to update only data block */
-#define EXT4_QUOTA_TRANS_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\
-		ext4_has_feature_quota(sb)) ? 1 : 0)
+#define EXT4_QUOTA_TRANS_BLOCKS(sb) ((ext4_quota_capable(sb)) ? 1 : 0)
 /* Amount of blocks needed for quota insert/delete - we do some block writes
  * but inode, sb and group updates are done only once */
-#define EXT4_QUOTA_INIT_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\
-		ext4_has_feature_quota(sb)) ?\
+#define EXT4_QUOTA_INIT_BLOCKS(sb) ((ext4_quota_capable(sb)) ?\
 		(DQUOT_INIT_ALLOC*(EXT4_SINGLEDATA_TRANS_BLOCKS(sb)-3)\
 		 +3+DQUOT_INIT_REWRITE) : 0)
 
-#define EXT4_QUOTA_DEL_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\
-		ext4_has_feature_quota(sb)) ?\
+#define EXT4_QUOTA_DEL_BLOCKS(sb) ((ext4_quota_capable(sb)) ?\
 		(DQUOT_DEL_ALLOC*(EXT4_SINGLEDATA_TRANS_BLOCKS(sb)-3)\
 		 +3+DQUOT_DEL_REWRITE) : 0)
 #else
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 9d01318..a988cf3 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -6158,11 +6158,8 @@ static int ext4_release_dquot(struct dquot *dquot)
 static int ext4_mark_dquot_dirty(struct dquot *dquot)
 {
 	struct super_block *sb = dquot->dq_sb;
-	struct ext4_sb_info *sbi = EXT4_SB(sb);
 
-	/* Are we journaling quotas? */
-	if (ext4_has_feature_quota(sb) ||
-	    sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) {
+	if (ext4_is_quota_journalled(sb)) {
 		dquot_mark_dquot_dirty(dquot);
 		return ext4_write_dquot(dquot);
 	} else {
-- 
2.7.4


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

* [PATCH v2 2/2] ext4: print quota journalling mode on (re-)mount
  2020-10-18  2:02 [PATCH v2 1/2] ext4: add helpers for checking whether quota can be enabled/is journalled Roman Anufriev
@ 2020-10-18  2:02 ` Roman Anufriev
  2020-10-18  3:22   ` kernel test robot
  2020-10-19  9:52   ` Jan Kara
  2020-10-19  9:37 ` [PATCH v2 1/2] ext4: add helpers for checking whether quota can be enabled/is journalled Jan Kara
  1 sibling, 2 replies; 9+ messages in thread
From: Roman Anufriev @ 2020-10-18  2:02 UTC (permalink / raw)
  To: linux-ext4; +Cc: tytso, jack, dmtrmonakhov, dotdot

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(-)

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


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

* Re: [PATCH v2 2/2] ext4: print quota journalling mode on (re-)mount
  2020-10-18  2:02 ` [PATCH v2 2/2] ext4: print quota journalling mode on (re-)mount Roman Anufriev
@ 2020-10-18  3:22   ` kernel test robot
  2020-10-19  9:46     ` Roman Anufriev
  2020-10-19  9:52   ` Jan Kara
  1 sibling, 1 reply; 9+ messages in thread
From: kernel test robot @ 2020-10-18  3:22 UTC (permalink / raw)
  To: Roman Anufriev, linux-ext4; +Cc: kbuild-all, tytso, jack, dmtrmonakhov, dotdot

[-- Attachment #1: Type: text/plain, Size: 2285 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29784 bytes --]

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

* Re: [PATCH v2 1/2] ext4: add helpers for checking whether quota can be enabled/is journalled
  2020-10-18  2:02 [PATCH v2 1/2] ext4: add helpers for checking whether quota can be enabled/is journalled Roman Anufriev
  2020-10-18  2:02 ` [PATCH v2 2/2] ext4: print quota journalling mode on (re-)mount Roman Anufriev
@ 2020-10-19  9:37 ` Jan Kara
  2020-10-19  9:53   ` Jan Kara
  1 sibling, 1 reply; 9+ messages in thread
From: Jan Kara @ 2020-10-19  9:37 UTC (permalink / raw)
  To: Roman Anufriev; +Cc: linux-ext4, tytso, jack, dmtrmonakhov

On Sun 18-10-20 05:02:26, Roman Anufriev wrote:
> Right now, there are several places, where we check whether fs is
> capable of enabling quota or if quota is journalled with quite long
> and non-self-descriptive condition statements.
> 
> This patch wraps these statements into helpers for better readability
> and easier usage.
> 
> Signed-off-by: Roman Anufriev <dotdot@yandex-team.ru>

Looks good to me. You can add:

Reviewe-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> Changes in v2:
>   - Fix misleading helper name 'ext4_any_quota_enabled()' ->
>     'ext4_quota_capable()'.
> 
>  fs/ext4/ext4.h      | 15 +++++++++++++++
>  fs/ext4/ext4_jbd2.h |  9 +++------
>  fs/ext4/super.c     |  5 +----
>  3 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 250e905..897df24 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3251,6 +3251,21 @@ static inline void ext4_unlock_group(struct super_block *sb,
>  	spin_unlock(ext4_group_lock_ptr(sb, group));
>  }
>  
> +#ifdef CONFIG_QUOTA
> +static inline bool ext4_quota_capable(struct super_block *sb)
> +{
> +	return (test_opt(sb, QUOTA) || ext4_has_feature_quota(sb));
> +}
> +
> +static inline bool ext4_is_quota_journalled(struct super_block *sb)
> +{
> +	struct ext4_sb_info *sbi = EXT4_SB(sb);
> +
> +	return (ext4_has_feature_quota(sb) ||
> +		sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]);
> +}
> +#endif
> +
>  /*
>   * Block validity checking
>   */
> diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
> index 00dc668..a124c68 100644
> --- a/fs/ext4/ext4_jbd2.h
> +++ b/fs/ext4/ext4_jbd2.h
> @@ -86,17 +86,14 @@
>  #ifdef CONFIG_QUOTA
>  /* Amount of blocks needed for quota update - we know that the structure was
>   * allocated so we need to update only data block */
> -#define EXT4_QUOTA_TRANS_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\
> -		ext4_has_feature_quota(sb)) ? 1 : 0)
> +#define EXT4_QUOTA_TRANS_BLOCKS(sb) ((ext4_quota_capable(sb)) ? 1 : 0)
>  /* Amount of blocks needed for quota insert/delete - we do some block writes
>   * but inode, sb and group updates are done only once */
> -#define EXT4_QUOTA_INIT_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\
> -		ext4_has_feature_quota(sb)) ?\
> +#define EXT4_QUOTA_INIT_BLOCKS(sb) ((ext4_quota_capable(sb)) ?\
>  		(DQUOT_INIT_ALLOC*(EXT4_SINGLEDATA_TRANS_BLOCKS(sb)-3)\
>  		 +3+DQUOT_INIT_REWRITE) : 0)
>  
> -#define EXT4_QUOTA_DEL_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\
> -		ext4_has_feature_quota(sb)) ?\
> +#define EXT4_QUOTA_DEL_BLOCKS(sb) ((ext4_quota_capable(sb)) ?\
>  		(DQUOT_DEL_ALLOC*(EXT4_SINGLEDATA_TRANS_BLOCKS(sb)-3)\
>  		 +3+DQUOT_DEL_REWRITE) : 0)
>  #else
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 9d01318..a988cf3 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -6158,11 +6158,8 @@ static int ext4_release_dquot(struct dquot *dquot)
>  static int ext4_mark_dquot_dirty(struct dquot *dquot)
>  {
>  	struct super_block *sb = dquot->dq_sb;
> -	struct ext4_sb_info *sbi = EXT4_SB(sb);
>  
> -	/* Are we journaling quotas? */
> -	if (ext4_has_feature_quota(sb) ||
> -	    sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) {
> +	if (ext4_is_quota_journalled(sb)) {
>  		dquot_mark_dquot_dirty(dquot);
>  		return ext4_write_dquot(dquot);
>  	} else {
> -- 
> 2.7.4
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [PATCH v2 2/2] ext4: print quota journalling mode on (re-)mount
  2020-10-18  3:22   ` kernel test robot
@ 2020-10-19  9:46     ` Roman Anufriev
  0 siblings, 0 replies; 9+ messages in thread
From: Roman Anufriev @ 2020-10-19  9:46 UTC (permalink / raw)
  To: kernel test robot; +Cc: linux-ext4, kbuild-all, tytso, jack, dmtrmonakhov

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

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

* Re: [PATCH v2 2/2] ext4: print quota journalling mode on (re-)mount
  2020-10-18  2:02 ` [PATCH v2 2/2] ext4: print quota journalling mode on (re-)mount Roman Anufriev
  2020-10-18  3:22   ` kernel test robot
@ 2020-10-19  9:52   ` Jan Kara
  2020-10-22  3:29     ` Roman Anufriev
  1 sibling, 1 reply; 9+ messages in thread
From: Jan Kara @ 2020-10-19  9:52 UTC (permalink / raw)
  To: Roman Anufriev; +Cc: linux-ext4, tytso, jack, dmtrmonakhov

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
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [PATCH v2 1/2] ext4: add helpers for checking whether quota can be enabled/is journalled
  2020-10-19  9:37 ` [PATCH v2 1/2] ext4: add helpers for checking whether quota can be enabled/is journalled Jan Kara
@ 2020-10-19  9:53   ` Jan Kara
  2020-10-22  3:33     ` Roman Anufriev
  0 siblings, 1 reply; 9+ messages in thread
From: Jan Kara @ 2020-10-19  9:53 UTC (permalink / raw)
  To: Roman Anufriev; +Cc: linux-ext4, tytso, jack, dmtrmonakhov

On Mon 19-10-20 11:37:06, Jan Kara wrote:
> On Sun 18-10-20 05:02:26, Roman Anufriev wrote:
> > Right now, there are several places, where we check whether fs is
> > capable of enabling quota or if quota is journalled with quite long
> > and non-self-descriptive condition statements.
> > 
> > This patch wraps these statements into helpers for better readability
> > and easier usage.
> > 
> > Signed-off-by: Roman Anufriev <dotdot@yandex-team.ru>
> 
> Looks good to me. You can add:
> 
> Reviewe-by: Jan Kara <jack@suse.cz>

Now I've realized that if we run in nojournal mode, quota won't be
journalled in any case. Probably not a configuration you run in but still
we should get that right.


 								Honza

> > ---
> > Changes in v2:
> >   - Fix misleading helper name 'ext4_any_quota_enabled()' ->
> >     'ext4_quota_capable()'.
> > 
> >  fs/ext4/ext4.h      | 15 +++++++++++++++
> >  fs/ext4/ext4_jbd2.h |  9 +++------
> >  fs/ext4/super.c     |  5 +----
> >  3 files changed, 19 insertions(+), 10 deletions(-)
> > 
> > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> > index 250e905..897df24 100644
> > --- a/fs/ext4/ext4.h
> > +++ b/fs/ext4/ext4.h
> > @@ -3251,6 +3251,21 @@ static inline void ext4_unlock_group(struct super_block *sb,
> >  	spin_unlock(ext4_group_lock_ptr(sb, group));
> >  }
> >  
> > +#ifdef CONFIG_QUOTA
> > +static inline bool ext4_quota_capable(struct super_block *sb)
> > +{
> > +	return (test_opt(sb, QUOTA) || ext4_has_feature_quota(sb));
> > +}
> > +
> > +static inline bool ext4_is_quota_journalled(struct super_block *sb)
> > +{
> > +	struct ext4_sb_info *sbi = EXT4_SB(sb);
> > +
> > +	return (ext4_has_feature_quota(sb) ||
> > +		sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]);
> > +}
> > +#endif
> > +
> >  /*
> >   * Block validity checking
> >   */
> > diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
> > index 00dc668..a124c68 100644
> > --- a/fs/ext4/ext4_jbd2.h
> > +++ b/fs/ext4/ext4_jbd2.h
> > @@ -86,17 +86,14 @@
> >  #ifdef CONFIG_QUOTA
> >  /* Amount of blocks needed for quota update - we know that the structure was
> >   * allocated so we need to update only data block */
> > -#define EXT4_QUOTA_TRANS_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\
> > -		ext4_has_feature_quota(sb)) ? 1 : 0)
> > +#define EXT4_QUOTA_TRANS_BLOCKS(sb) ((ext4_quota_capable(sb)) ? 1 : 0)
> >  /* Amount of blocks needed for quota insert/delete - we do some block writes
> >   * but inode, sb and group updates are done only once */
> > -#define EXT4_QUOTA_INIT_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\
> > -		ext4_has_feature_quota(sb)) ?\
> > +#define EXT4_QUOTA_INIT_BLOCKS(sb) ((ext4_quota_capable(sb)) ?\
> >  		(DQUOT_INIT_ALLOC*(EXT4_SINGLEDATA_TRANS_BLOCKS(sb)-3)\
> >  		 +3+DQUOT_INIT_REWRITE) : 0)
> >  
> > -#define EXT4_QUOTA_DEL_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\
> > -		ext4_has_feature_quota(sb)) ?\
> > +#define EXT4_QUOTA_DEL_BLOCKS(sb) ((ext4_quota_capable(sb)) ?\
> >  		(DQUOT_DEL_ALLOC*(EXT4_SINGLEDATA_TRANS_BLOCKS(sb)-3)\
> >  		 +3+DQUOT_DEL_REWRITE) : 0)
> >  #else
> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > index 9d01318..a988cf3 100644
> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
> > @@ -6158,11 +6158,8 @@ static int ext4_release_dquot(struct dquot *dquot)
> >  static int ext4_mark_dquot_dirty(struct dquot *dquot)
> >  {
> >  	struct super_block *sb = dquot->dq_sb;
> > -	struct ext4_sb_info *sbi = EXT4_SB(sb);
> >  
> > -	/* Are we journaling quotas? */
> > -	if (ext4_has_feature_quota(sb) ||
> > -	    sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) {
> > +	if (ext4_is_quota_journalled(sb)) {
> >  		dquot_mark_dquot_dirty(dquot);
> >  		return ext4_write_dquot(dquot);
> >  	} else {
> > -- 
> > 2.7.4
> > 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [PATCH v2 2/2] ext4: print quota journalling mode on (re-)mount
  2020-10-19  9:52   ` Jan Kara
@ 2020-10-22  3:29     ` Roman Anufriev
  0 siblings, 0 replies; 9+ messages in thread
From: Roman Anufriev @ 2020-10-22  3:29 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-ext4, tytso, dmtrmonakhov

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

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

* Re: [PATCH v2 1/2] ext4: add helpers for checking whether quota can be enabled/is journalled
  2020-10-19  9:53   ` Jan Kara
@ 2020-10-22  3:33     ` Roman Anufriev
  0 siblings, 0 replies; 9+ messages in thread
From: Roman Anufriev @ 2020-10-22  3:33 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-ext4, tytso, dmtrmonakhov

On Mon, 19 Oct 2020, Jan Kara wrote:

> On Mon 19-10-20 11:37:06, Jan Kara wrote:
>> On Sun 18-10-20 05:02:26, Roman Anufriev wrote:
>>> Right now, there are several places, where we check whether fs is
>>> capable of enabling quota or if quota is journalled with quite long
>>> and non-self-descriptive condition statements.
>>>
>>> This patch wraps these statements into helpers for better readability
>>> and easier usage.
>>>
>>> Signed-off-by: Roman Anufriev <dotdot@yandex-team.ru>
>>
>> Looks good to me. You can add:
>>
>> Reviewe-by: Jan Kara <jack@suse.cz>
>
> Now I've realized that if we run in nojournal mode, quota won't be
> journalled in any case. Probably not a configuration you run in but still
> we should get that right.

I forgot about this case. Fixed in v4:
https://lore.kernel.org/linux-ext4/1603336860-16153-1-git-send-email-dotdot@yandex-team.ru/

 								Roman

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

end of thread, other threads:[~2020-10-22  3:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-18  2:02 [PATCH v2 1/2] ext4: add helpers for checking whether quota can be enabled/is journalled Roman Anufriev
2020-10-18  2:02 ` [PATCH v2 2/2] ext4: print quota journalling mode on (re-)mount Roman Anufriev
2020-10-18  3:22   ` kernel test robot
2020-10-19  9:46     ` Roman Anufriev
2020-10-19  9:52   ` Jan Kara
2020-10-22  3:29     ` Roman Anufriev
2020-10-19  9:37 ` [PATCH v2 1/2] ext4: add helpers for checking whether quota can be enabled/is journalled Jan Kara
2020-10-19  9:53   ` Jan Kara
2020-10-22  3:33     ` Roman Anufriev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).