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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <cgxu519@xxxxxxxxxxxx>
> ---
> - Fix a bug in the limit setting logic.

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

Reviewed-by: Jan Kara <jack@xxxxxxx>

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 <jack@xxxxxxxx>
SUSE Labs, CR



[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux