2019-10-15 11:34:58

by Chengguang Xu

[permalink] [raw]
Subject: [PATCH v2] ext4: choose hardlimit when softlimit is larger than hardlimit in ext4_statfs_project()

Setting softlimit larger than hardlimit seems meaningless
for disk quota but currently it is allowed. In this case,
there may be a bit of comfusion for users when they run
df comamnd to directory which has project quota.

For example, we set 20M softlimit and 10M hardlimit of
block usage limit for project quota of test_dir(project id 123).

[root@hades mnt_ext4]# repquota -P -a
*** Report for project quotas on device /dev/loop0
Block grace time: 7days; Inode grace time: 7days
Block limits File limits
Project used soft hard grace used soft hard grace
----------------------------------------------------------------------
0 -- 13 0 0 2 0 0
123 -- 10237 20480 10240 5 200 100

The result of df command as below:

[root@hades mnt_ext4]# df -h test_dir
Filesystem Size Used Avail Use% Mounted on
/dev/loop0 20M 10M 10M 50% /home/cgxu/test/mnt_ext4

Even though it looks like there is another 10M free space to use,
if we write new data to diretory test_dir(inherit project id),
the write will fail with errno(-EDQUOT).

After this patch, the df result looks like below.

[root@hades mnt_ext4]# df -h test_dir
Filesystem Size Used Avail Use% Mounted on
/dev/loop0 10M 10M 3.0K 100% /home/cgxu/test/mnt_ext4

Signed-off-by: Chengguang Xu <[email protected]>
---
- Fix a bug in the limit setting logic.

fs/ext4/super.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index dd654e53ba3d..f24e175ae5e0 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5546,9 +5546,15 @@ static int ext4_statfs_project(struct super_block *sb,
return PTR_ERR(dquot);
spin_lock(&dquot->dq_dqb_lock);

- limit = (dquot->dq_dqb.dqb_bsoftlimit ?
- dquot->dq_dqb.dqb_bsoftlimit :
- dquot->dq_dqb.dqb_bhardlimit) >> sb->s_blocksize_bits;
+ limit = 0;
+ if (dquot->dq_dqb.dqb_bsoftlimit &&
+ (!limit || dquot->dq_dqb.dqb_bsoftlimit < limit))
+ limit = dquot->dq_dqb.dqb_bsoftlimit;
+ if (dquot->dq_dqb.dqb_bhardlimit &&
+ (!limit || dquot->dq_dqb.dqb_bhardlimit < limit))
+ limit = dquot->dq_dqb.dqb_bhardlimit;
+ limit >>= sb->s_blocksize_bits;
+
if (limit && buf->f_blocks > limit) {
curblock = (dquot->dq_dqb.dqb_curspace +
dquot->dq_dqb.dqb_rsvspace) >> sb->s_blocksize_bits;
@@ -5558,9 +5564,14 @@ static int ext4_statfs_project(struct super_block *sb,
(buf->f_blocks - curblock) : 0;
}

- limit = dquot->dq_dqb.dqb_isoftlimit ?
- dquot->dq_dqb.dqb_isoftlimit :
- dquot->dq_dqb.dqb_ihardlimit;
+ limit = 0;
+ if (dquot->dq_dqb.dqb_isoftlimit &&
+ (!limit || dquot->dq_dqb.dqb_isoftlimit < limit))
+ limit = dquot->dq_dqb.dqb_isoftlimit;
+ if (dquot->dq_dqb.dqb_ihardlimit &&
+ (!limit || dquot->dq_dqb.dqb_ihardlimit < limit))
+ limit = dquot->dq_dqb.dqb_ihardlimit;
+
if (limit && buf->f_files > limit) {
buf->f_files = limit;
buf->f_ffree =
--
2.20.1




2019-10-15 12:13:08

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH v2] ext4: choose hardlimit when softlimit is larger than hardlimit in ext4_statfs_project()

On Tue 15-10-19 18:23:27, Chengguang Xu wrote:
> Setting softlimit larger than hardlimit seems meaningless
> for disk quota but currently it is allowed. In this case,
> there may be a bit of comfusion for users when they run
> df comamnd to directory which has project quota.
>
> For example, we set 20M softlimit and 10M hardlimit of
> block usage limit for project quota of test_dir(project id 123).
>
> [root@hades mnt_ext4]# repquota -P -a
> *** Report for project quotas on device /dev/loop0
> Block grace time: 7days; Inode grace time: 7days
> Block limits File limits
> Project used soft hard grace used soft hard grace
> ----------------------------------------------------------------------
> 0 -- 13 0 0 2 0 0
> 123 -- 10237 20480 10240 5 200 100
>
> The result of df command as below:
>
> [root@hades mnt_ext4]# df -h test_dir
> Filesystem Size Used Avail Use% Mounted on
> /dev/loop0 20M 10M 10M 50% /home/cgxu/test/mnt_ext4
>
> Even though it looks like there is another 10M free space to use,
> if we write new data to diretory test_dir(inherit project id),
> the write will fail with errno(-EDQUOT).
>
> After this patch, the df result looks like below.
>
> [root@hades mnt_ext4]# df -h test_dir
> Filesystem Size Used Avail Use% Mounted on
> /dev/loop0 10M 10M 3.0K 100% /home/cgxu/test/mnt_ext4
>
> Signed-off-by: Chengguang Xu <[email protected]>
> ---
> - Fix a bug in the limit setting logic.

Thanks for the patch! It looks good to me. You can add:

Reviewed-by: Jan Kara <[email protected]>

Just one style nit below:


> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index dd654e53ba3d..f24e175ae5e0 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -5546,9 +5546,15 @@ static int ext4_statfs_project(struct super_block *sb,
> return PTR_ERR(dquot);
> spin_lock(&dquot->dq_dqb_lock);
>
> - limit = (dquot->dq_dqb.dqb_bsoftlimit ?
> - dquot->dq_dqb.dqb_bsoftlimit :
> - dquot->dq_dqb.dqb_bhardlimit) >> sb->s_blocksize_bits;
> + limit = 0;
> + if (dquot->dq_dqb.dqb_bsoftlimit &&
> + (!limit || dquot->dq_dqb.dqb_bsoftlimit < limit))

In ext4 we don't indent wrapped condition to the same depth as the
following block. Rather we indent at the start of the condition with spaces
like:

if (dquot->dq_dqb.dqb_bsoftlimit &&
(!limit || dquot->dq_dqb.dqb_bsoftlimit < limit))
do something

Some other subsystems also use:

if (dquot->dq_dqb.dqb_bsoftlimit &&
(!limit || dquot->dq_dqb.dqb_bsoftlimit < limit))
do something.

But indenting at the same depth like you did makes it easy to conflate the
condition with the command block so we don't use that...

Honza

> + limit = dquot->dq_dqb.dqb_bsoftlimit;
> + if (dquot->dq_dqb.dqb_bhardlimit &&
> + (!limit || dquot->dq_dqb.dqb_bhardlimit < limit))
> + limit = dquot->dq_dqb.dqb_bhardlimit;
> + limit >>= sb->s_blocksize_bits;
> +
> if (limit && buf->f_blocks > limit) {
> curblock = (dquot->dq_dqb.dqb_curspace +
> dquot->dq_dqb.dqb_rsvspace) >> sb->s_blocksize_bits;
> @@ -5558,9 +5564,14 @@ static int ext4_statfs_project(struct super_block *sb,
> (buf->f_blocks - curblock) : 0;
> }
>
> - limit = dquot->dq_dqb.dqb_isoftlimit ?
> - dquot->dq_dqb.dqb_isoftlimit :
> - dquot->dq_dqb.dqb_ihardlimit;
> + limit = 0;
> + if (dquot->dq_dqb.dqb_isoftlimit &&
> + (!limit || dquot->dq_dqb.dqb_isoftlimit < limit))
> + limit = dquot->dq_dqb.dqb_isoftlimit;
> + if (dquot->dq_dqb.dqb_ihardlimit &&
> + (!limit || dquot->dq_dqb.dqb_ihardlimit < limit))
> + limit = dquot->dq_dqb.dqb_ihardlimit;
> +
> if (limit && buf->f_files > limit) {
> buf->f_files = limit;
> buf->f_ffree =
> --
> 2.20.1
>
>
>
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2019-11-06 04:39:06

by Chengguang Xu

[permalink] [raw]
Subject: Re: [PATCH v2] ext4: choose hardlimit when softlimit is larger than hardlimit in ext4_statfs_project()

---- 在 星期二, 2019-10-15 19:25:23 Jan Kara <[email protected]> 撰写 ----
> On Tue 15-10-19 18:23:27, Chengguang Xu wrote:
> > Setting softlimit larger than hardlimit seems meaningless
> > for disk quota but currently it is allowed. In this case,
> > there may be a bit of comfusion for users when they run
> > df comamnd to directory which has project quota.
> >
> > For example, we set 20M softlimit and 10M hardlimit of
> > block usage limit for project quota of test_dir(project id 123).
> >
> > [root@hades mnt_ext4]# repquota -P -a
> > *** Report for project quotas on device /dev/loop0
> > Block grace time: 7days; Inode grace time: 7days
> > Block limits File limits
> > Project used soft hard grace used soft hard grace
> > ----------------------------------------------------------------------
> > 0 -- 13 0 0 2 0 0
> > 123 -- 10237 20480 10240 5 200 100
> >
> > The result of df command as below:
> >
> > [root@hades mnt_ext4]# df -h test_dir
> > Filesystem Size Used Avail Use% Mounted on
> > /dev/loop0 20M 10M 10M 50% /home/cgxu/test/mnt_ext4
> >
> > Even though it looks like there is another 10M free space to use,
> > if we write new data to diretory test_dir(inherit project id),
> > the write will fail with errno(-EDQUOT).
> >
> > After this patch, the df result looks like below.
> >
> > [root@hades mnt_ext4]# df -h test_dir
> > Filesystem Size Used Avail Use% Mounted on
> > /dev/loop0 10M 10M 3.0K 100% /home/cgxu/test/mnt_ext4
> >
> > Signed-off-by: Chengguang Xu <[email protected]>
> > ---
> > - Fix a bug in the limit setting logic.
>
> Thanks for the patch! It looks good to me. You can add:
>
> Reviewed-by: Jan Kara <[email protected]>
>

Hi Jan,

I have a proposal for another direction.
Could we add a check for soft limit in quota layer when setting the value?
So that we could not bother with specific file systems on statfs().

Thanks,
Chengguang


2019-11-06 05:05:15

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH v2] ext4: choose hardlimit when softlimit is larger than hardlimit in ext4_statfs_project()

On Wed, Nov 06, 2019 at 12:37:35PM +0800, Chengguang Xu wrote:
> ---- 在 星期二, 2019-10-15 19:25:23 Jan Kara <[email protected]> 撰写 ----
> > On Tue 15-10-19 18:23:27, Chengguang Xu wrote:
> > > Setting softlimit larger than hardlimit seems meaningless
> > > for disk quota but currently it is allowed. In this case,
> > > there may be a bit of comfusion for users when they run
> > > df comamnd to directory which has project quota.
> > >
> > > For example, we set 20M softlimit and 10M hardlimit of
> > > block usage limit for project quota of test_dir(project id 123).
> > >
> > > [root@hades mnt_ext4]# repquota -P -a
> > > *** Report for project quotas on device /dev/loop0
> > > Block grace time: 7days; Inode grace time: 7days
> > > Block limits File limits
> > > Project used soft hard grace used soft hard grace
> > > ----------------------------------------------------------------------
> > > 0 -- 13 0 0 2 0 0
> > > 123 -- 10237 20480 10240 5 200 100
> > >
> > > The result of df command as below:
> > >
> > > [root@hades mnt_ext4]# df -h test_dir
> > > Filesystem Size Used Avail Use% Mounted on
> > > /dev/loop0 20M 10M 10M 50% /home/cgxu/test/mnt_ext4
> > >
> > > Even though it looks like there is another 10M free space to use,
> > > if we write new data to diretory test_dir(inherit project id),
> > > the write will fail with errno(-EDQUOT).
> > >
> > > After this patch, the df result looks like below.
> > >
> > > [root@hades mnt_ext4]# df -h test_dir
> > > Filesystem Size Used Avail Use% Mounted on
> > > /dev/loop0 10M 10M 3.0K 100% /home/cgxu/test/mnt_ext4
> > >
> > > Signed-off-by: Chengguang Xu <[email protected]>
> > > ---
> > > - Fix a bug in the limit setting logic.
> >
> > Thanks for the patch! It looks good to me. You can add:
> >
> > Reviewed-by: Jan Kara <[email protected]>
> >
>
> Hi Jan,
>
> I have a proposal for another direction.
> Could we add a check for soft limit in quota layer when setting the value?
> So that we could not bother with specific file systems on statfs().

How do the other filesystems (e.g. xfs) behave if someone tries to set a
soft limit higher than the hard limit?

--D

> Thanks,
> Chengguang
>
>

2019-11-06 08:36:04

by Chengguang Xu

[permalink] [raw]
Subject: Re: [PATCH v2] ext4: choose hardlimit when softlimit is larger than hardlimit in ext4_statfs_project()

---- 在 星期三, 2019-11-06 13:03:36 Darrick J. Wong <[email protected]> 撰写 ----
> On Wed, Nov 06, 2019 at 12:37:35PM +0800, Chengguang Xu wrote:
> > ---- 在 星期二, 2019-10-15 19:25:23 Jan Kara <[email protected]> 撰写 ----
> > > On Tue 15-10-19 18:23:27, Chengguang Xu wrote:
> > > > Setting softlimit larger than hardlimit seems meaningless
> > > > for disk quota but currently it is allowed. In this case,
> > > > there may be a bit of comfusion for users when they run
> > > > df comamnd to directory which has project quota.
> > > >
> > > > For example, we set 20M softlimit and 10M hardlimit of
> > > > block usage limit for project quota of test_dir(project id 123).
> > > >
> > > > [root@hades mnt_ext4]# repquota -P -a
> > > > *** Report for project quotas on device /dev/loop0
> > > > Block grace time: 7days; Inode grace time: 7days
> > > > Block limits File limits
> > > > Project used soft hard grace used soft hard grace
> > > > ----------------------------------------------------------------------
> > > > 0 -- 13 0 0 2 0 0
> > > > 123 -- 10237 20480 10240 5 200 100
> > > >
> > > > The result of df command as below:
> > > >
> > > > [root@hades mnt_ext4]# df -h test_dir
> > > > Filesystem Size Used Avail Use% Mounted on
> > > > /dev/loop0 20M 10M 10M 50% /home/cgxu/test/mnt_ext4
> > > >
> > > > Even though it looks like there is another 10M free space to use,
> > > > if we write new data to diretory test_dir(inherit project id),
> > > > the write will fail with errno(-EDQUOT).
> > > >
> > > > After this patch, the df result looks like below.
> > > >
> > > > [root@hades mnt_ext4]# df -h test_dir
> > > > Filesystem Size Used Avail Use% Mounted on
> > > > /dev/loop0 10M 10M 3.0K 100% /home/cgxu/test/mnt_ext4
> > > >
> > > > Signed-off-by: Chengguang Xu <[email protected]>
> > > > ---
> > > > - Fix a bug in the limit setting logic.
> > >
> > > Thanks for the patch! It looks good to me. You can add:
> > >
> > > Reviewed-by: Jan Kara <[email protected]>
> > >
> >
> > Hi Jan,
> >
> > I have a proposal for another direction.
> > Could we add a check for soft limit in quota layer when setting the value?
> > So that we could not bother with specific file systems on statfs().
>
> How do the other filesystems (e.g. xfs) behave if someone tries to set a
> soft limit higher than the hard limit?
>

In xfs if (hard && hard < soft), the limit will not be set(or changed) but command xfs_quota does not return error.


Thanks,
Chengguang


2019-11-06 09:51:13

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH v2] ext4: choose hardlimit when softlimit is larger than hardlimit in ext4_statfs_project()

On Wed 06-11-19 12:37:35, Chengguang Xu wrote:
> ---- 在 星期二, 2019-10-15 19:25:23 Jan Kara <[email protected]> 撰写 ----
> > On Tue 15-10-19 18:23:27, Chengguang Xu wrote:
> > > Setting softlimit larger than hardlimit seems meaningless
> > > for disk quota but currently it is allowed. In this case,
> > > there may be a bit of comfusion for users when they run
> > > df comamnd to directory which has project quota.
> > >
> > > For example, we set 20M softlimit and 10M hardlimit of
> > > block usage limit for project quota of test_dir(project id 123).
> > >
> > > [root@hades mnt_ext4]# repquota -P -a
> > > *** Report for project quotas on device /dev/loop0
> > > Block grace time: 7days; Inode grace time: 7days
> > > Block limits File limits
> > > Project used soft hard grace used soft hard grace
> > > ----------------------------------------------------------------------
> > > 0 -- 13 0 0 2 0 0
> > > 123 -- 10237 20480 10240 5 200 100
> > >
> > > The result of df command as below:
> > >
> > > [root@hades mnt_ext4]# df -h test_dir
> > > Filesystem Size Used Avail Use% Mounted on
> > > /dev/loop0 20M 10M 10M 50% /home/cgxu/test/mnt_ext4
> > >
> > > Even though it looks like there is another 10M free space to use,
> > > if we write new data to diretory test_dir(inherit project id),
> > > the write will fail with errno(-EDQUOT).
> > >
> > > After this patch, the df result looks like below.
> > >
> > > [root@hades mnt_ext4]# df -h test_dir
> > > Filesystem Size Used Avail Use% Mounted on
> > > /dev/loop0 10M 10M 3.0K 100% /home/cgxu/test/mnt_ext4
> > >
> > > Signed-off-by: Chengguang Xu <[email protected]>
> > > ---
> > > - Fix a bug in the limit setting logic.
> >
> > Thanks for the patch! It looks good to me. You can add:
> >
> > Reviewed-by: Jan Kara <[email protected]>
> >
>
> Hi Jan,
>
> I have a proposal for another direction.
> Could we add a check for soft limit in quota layer when setting the value?
> So that we could not bother with specific file systems on statfs().

What do you mean exactly? To not allow softlimit to be larger than
hardlimit? That would make some sense but I don't think the risk of
breaking some user that accidentally depends on current behavior is worth
the few checks we can save...

Honza

--
Jan Kara <[email protected]>
SUSE Labs, CR

2019-11-06 10:42:06

by Chengguang Xu

[permalink] [raw]
Subject: Re: [PATCH v2] ext4: choose hardlimit when softlimit is larger than hardlimit in ext4_statfs_project()

---- 在 星期三, 2019-11-06 17:49:24 Jan Kara <[email protected]> 撰写 ----
> On Wed 06-11-19 12:37:35, Chengguang Xu wrote:
> > ---- 在 星期二, 2019-10-15 19:25:23 Jan Kara <[email protected]> 撰写 ----
> > > On Tue 15-10-19 18:23:27, Chengguang Xu wrote:
> > > > Setting softlimit larger than hardlimit seems meaningless
> > > > for disk quota but currently it is allowed. In this case,
> > > > there may be a bit of comfusion for users when they run
> > > > df comamnd to directory which has project quota.
> > > >
> > > > For example, we set 20M softlimit and 10M hardlimit of
> > > > block usage limit for project quota of test_dir(project id 123).
> > > >
> > > > [root@hades mnt_ext4]# repquota -P -a
> > > > *** Report for project quotas on device /dev/loop0
> > > > Block grace time: 7days; Inode grace time: 7days
> > > > Block limits File limits
> > > > Project used soft hard grace used soft hard grace
> > > > ----------------------------------------------------------------------
> > > > 0 -- 13 0 0 2 0 0
> > > > 123 -- 10237 20480 10240 5 200 100
> > > >
> > > > The result of df command as below:
> > > >
> > > > [root@hades mnt_ext4]# df -h test_dir
> > > > Filesystem Size Used Avail Use% Mounted on
> > > > /dev/loop0 20M 10M 10M 50% /home/cgxu/test/mnt_ext4
> > > >
> > > > Even though it looks like there is another 10M free space to use,
> > > > if we write new data to diretory test_dir(inherit project id),
> > > > the write will fail with errno(-EDQUOT).
> > > >
> > > > After this patch, the df result looks like below.
> > > >
> > > > [root@hades mnt_ext4]# df -h test_dir
> > > > Filesystem Size Used Avail Use% Mounted on
> > > > /dev/loop0 10M 10M 3.0K 100% /home/cgxu/test/mnt_ext4
> > > >
> > > > Signed-off-by: Chengguang Xu <[email protected]>
> > > > ---
> > > > - Fix a bug in the limit setting logic.
> > >
> > > Thanks for the patch! It looks good to me. You can add:
> > >
> > > Reviewed-by: Jan Kara <[email protected]>
> > >
> >
> > Hi Jan,
> >
> > I have a proposal for another direction.
> > Could we add a check for soft limit in quota layer when setting the value?
> > So that we could not bother with specific file systems on statfs().
>
> What do you mean exactly? To not allow softlimit to be larger than
> hardlimit? That would make some sense but I don't think the risk of
> breaking some user that accidentally depends on current behavior is worth
> the few checks we can save...
>

Actually, I thought exactly same as you when I wrote my patch for statfs() of ext4.
However, even though softlimit > hardlimit, we cannot allow user to use blocks or inode
more than hardlimit. IOW, the limit is always there and fixed in this situation.
So how about set softlimit to hardlimit when softlimit > hardlimit and return with success?


Thanks,
Chengguang



2019-11-06 10:53:02

by Chengguang Xu

[permalink] [raw]
Subject: Re: [PATCH v2] ext4: choose hardlimit when softlimit is larger than hardlimit in ext4_statfs_project()

---- 在 星期三, 2019-11-06 16:34:24 Chengguang Xu <[email protected]> 撰写 ----
> ---- 在 星期三, 2019-11-06 13:03:36 Darrick J. Wong <[email protected]> 撰写 ----
> > On Wed, Nov 06, 2019 at 12:37:35PM +0800, Chengguang Xu wrote:
> > > ---- 在 星期二, 2019-10-15 19:25:23 Jan Kara <[email protected]> 撰写 ----
> > > > On Tue 15-10-19 18:23:27, Chengguang Xu wrote:
> > > > > Setting softlimit larger than hardlimit seems meaningless
> > > > > for disk quota but currently it is allowed. In this case,
> > > > > there may be a bit of comfusion for users when they run
> > > > > df comamnd to directory which has project quota.
> > > > >
> > > > > For example, we set 20M softlimit and 10M hardlimit of
> > > > > block usage limit for project quota of test_dir(project id 123).
> > > > >
> > > > > [root@hades mnt_ext4]# repquota -P -a
> > > > > *** Report for project quotas on device /dev/loop0
> > > > > Block grace time: 7days; Inode grace time: 7days
> > > > > Block limits File limits
> > > > > Project used soft hard grace used soft hard grace
> > > > > ----------------------------------------------------------------------
> > > > > 0 -- 13 0 0 2 0 0
> > > > > 123 -- 10237 20480 10240 5 200 100
> > > > >
> > > > > The result of df command as below:
> > > > >
> > > > > [root@hades mnt_ext4]# df -h test_dir
> > > > > Filesystem Size Used Avail Use% Mounted on
> > > > > /dev/loop0 20M 10M 10M 50% /home/cgxu/test/mnt_ext4
> > > > >
> > > > > Even though it looks like there is another 10M free space to use,
> > > > > if we write new data to diretory test_dir(inherit project id),
> > > > > the write will fail with errno(-EDQUOT).
> > > > >
> > > > > After this patch, the df result looks like below.
> > > > >
> > > > > [root@hades mnt_ext4]# df -h test_dir
> > > > > Filesystem Size Used Avail Use% Mounted on
> > > > > /dev/loop0 10M 10M 3.0K 100% /home/cgxu/test/mnt_ext4
> > > > >
> > > > > Signed-off-by: Chengguang Xu <[email protected]>
> > > > > ---
> > > > > - Fix a bug in the limit setting logic.
> > > >
> > > > Thanks for the patch! It looks good to me. You can add:
> > > >
> > > > Reviewed-by: Jan Kara <[email protected]>
> > > >
> > >
> > > Hi Jan,
> > >
> > > I have a proposal for another direction.
> > > Could we add a check for soft limit in quota layer when setting the value?
> > > So that we could not bother with specific file systems on statfs().
> >
> > How do the other filesystems (e.g. xfs) behave if someone tries to set a
> > soft limit higher than the hard limit?
> >
>
> In xfs if (hard && hard < soft), the limit will not be set(or changed) but command xfs_quota does not return error.
>

Hi Darrick,

IMO, set nothing and return success seems not reasonable,
is it possible change to set softlimit to hardlimit when softlimit > hardlimit?

Thanks,
Chengguang

2019-11-06 12:07:52

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH v2] ext4: choose hardlimit when softlimit is larger than hardlimit in ext4_statfs_project()

On Wed 06-11-19 18:40:36, Chengguang Xu wrote:
> ---- 在 星期三, 2019-11-06 17:49:24 Jan Kara <[email protected]> 撰写 ----
> > On Wed 06-11-19 12:37:35, Chengguang Xu wrote:
> > > ---- 在 星期二, 2019-10-15 19:25:23 Jan Kara <[email protected]> 撰写 ----
> > > > On Tue 15-10-19 18:23:27, Chengguang Xu wrote:
> > > > > Setting softlimit larger than hardlimit seems meaningless
> > > > > for disk quota but currently it is allowed. In this case,
> > > > > there may be a bit of comfusion for users when they run
> > > > > df comamnd to directory which has project quota.
> > > > >
> > > > > For example, we set 20M softlimit and 10M hardlimit of
> > > > > block usage limit for project quota of test_dir(project id 123).
> > > > >
> > > > > [root@hades mnt_ext4]# repquota -P -a
> > > > > *** Report for project quotas on device /dev/loop0
> > > > > Block grace time: 7days; Inode grace time: 7days
> > > > > Block limits File limits
> > > > > Project used soft hard grace used soft hard grace
> > > > > ----------------------------------------------------------------------
> > > > > 0 -- 13 0 0 2 0 0
> > > > > 123 -- 10237 20480 10240 5 200 100
> > > > >
> > > > > The result of df command as below:
> > > > >
> > > > > [root@hades mnt_ext4]# df -h test_dir
> > > > > Filesystem Size Used Avail Use% Mounted on
> > > > > /dev/loop0 20M 10M 10M 50% /home/cgxu/test/mnt_ext4
> > > > >
> > > > > Even though it looks like there is another 10M free space to use,
> > > > > if we write new data to diretory test_dir(inherit project id),
> > > > > the write will fail with errno(-EDQUOT).
> > > > >
> > > > > After this patch, the df result looks like below.
> > > > >
> > > > > [root@hades mnt_ext4]# df -h test_dir
> > > > > Filesystem Size Used Avail Use% Mounted on
> > > > > /dev/loop0 10M 10M 3.0K 100% /home/cgxu/test/mnt_ext4
> > > > >
> > > > > Signed-off-by: Chengguang Xu <[email protected]>
> > > > > ---
> > > > > - Fix a bug in the limit setting logic.
> > > >
> > > > Thanks for the patch! It looks good to me. You can add:
> > > >
> > > > Reviewed-by: Jan Kara <[email protected]>
> > > >
> > >
> > > Hi Jan,
> > >
> > > I have a proposal for another direction.
> > > Could we add a check for soft limit in quota layer when setting the value?
> > > So that we could not bother with specific file systems on statfs().
> >
> > What do you mean exactly? To not allow softlimit to be larger than
> > hardlimit? That would make some sense but I don't think the risk of
> > breaking some user that accidentally depends on current behavior is worth
> > the few checks we can save...
> >
>
> Actually, I thought exactly same as you when I wrote my patch for
> statfs() of ext4. However, even though softlimit > hardlimit, we cannot
> allow user to use blocks or inode more than hardlimit. IOW, the limit is
> always there and fixed in this situation. So how about set softlimit
> to hardlimit when softlimit > hardlimit and return with success?

Well, the softlimit > hardlimit won't have any effect but if the hardlimit
is then raised (e.g. with a tool like edquota(8)), it may suddently start
having effect. That's why I'm reluctant to just ignore or trim too large
softlimit.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR