---- 在 星期二, 2019-10-15 16:20:01 Jan Kara <jack@xxxxxxx> 撰写 ----
> On Thu 10-10-19 13:14:26, 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 <cgxu519@xxxxxxxxxxxx>
>
> Good spotting! But the patch has a bug:
>
> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > index dd654e53ba3d..08d4f993b365 100644
> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
> > @@ -5546,9 +5546,11 @@ 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 = ((dquot->dq_dqb.dqb_bhardlimit &&
> > + (dquot->dq_dqb.dqb_bhardlimit < dquot->dq_dqb.dqb_bsoftlimit)) ?
> > + dquot->dq_dqb.dqb_bhardlimit :
> > + dquot->dq_dqb.dqb_bsoftlimit) >> sb->s_blocksize_bits;
> > +
>
> This is wrong in case softlimit isn't set and hardlimit is. In that case
> you'd have 'limit' equal to 0, which is wrong. Also the formula is rather
> hard to parse already. So I'd rather go with something like:
Ah, you are right, I overlooked it.
I'll fix in next version. Thanks for your review.
Thanks,
Chengguang
>
> 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;
>
> and similarly for inode limit...
>
> Honza
>
> > if (limit && buf->f_blocks > limit) {
> > curblock = (dquot->dq_dqb.dqb_curspace +
> > dquot->dq_dqb.dqb_rsvspace) >> sb->s_blocksize_bits;
> > @@ -5558,9 +5560,11 @@ 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 = (dquot->dq_dqb.dqb_ihardlimit &&
> > + (dquot->dq_dqb.dqb_ihardlimit < dquot->dq_dqb.dqb_isoftlimit)) ?
> > + dquot->dq_dqb.dqb_ihardlimit :
> > + dquot->dq_dqb.dqb_isoftlimit;
> > +
> > if (limit && buf->f_files > limit) {
> > buf->f_files = limit;
> > buf->f_ffree =
> > --
> > 2.20.1
> >
> >
> >
> --
> Jan Kara <jack@xxxxxxxx>
> SUSE Labs, CR
>
