diff mbox series

[6/6] xfs: allow individual quota grace period extension

Message ID 868cac51-800e-2051-1322-aa77302a65c2@redhat.com (mailing list archive)
State Superseded
Headers show
Series xfs: quota timer enhancements | expand

Commit Message

Eric Sandeen May 18, 2020, 6:52 p.m. UTC
The only grace period which can be set in the kernel today is for id 0,
i.e. the default grace period for all users.  However, setting an
individual grace period is useful; for example:

 Alice has a soft quota of 100 inodes, and a hard quota of 200 inodes
 Alice uses 150 inodes, and enters a short grace period
 Alice really needs to use those 150 inodes past the grace period
 The administrator extends Alice's grace period until next Monday

vfs quota users such as ext4 can do this today, with setquota -T

To enable this for XFS, we simply move the timelimit assignment out
from under the (id == 0) test.  Default setting remains under (id == 0).
Note that this now is consistent with how we set warnings.

(Userspace requires updates to enable this as well; xfs_quota needs to
parse new options, and setquota needs to set appropriate field flags.)

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/xfs_qm_syscalls.c | 48 +++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 20 deletions(-)

Comments

Darrick J. Wong May 19, 2020, 4:39 p.m. UTC | #1
On Mon, May 18, 2020 at 01:52:16PM -0500, Eric Sandeen wrote:
> The only grace period which can be set in the kernel today is for id 0,
> i.e. the default grace period for all users.  However, setting an
> individual grace period is useful; for example:
> 
>  Alice has a soft quota of 100 inodes, and a hard quota of 200 inodes
>  Alice uses 150 inodes, and enters a short grace period
>  Alice really needs to use those 150 inodes past the grace period
>  The administrator extends Alice's grace period until next Monday
> 
> vfs quota users such as ext4 can do this today, with setquota -T

Does setquota -T work on an XFS filesystem?  If so, does that mean that
xfs had a functionality gap where the admin could extend someone's grace
period on ext4 but trying the exact same command on xfs would do
nothing?  Or would it at least error out?

> To enable this for XFS, we simply move the timelimit assignment out
> from under the (id == 0) test.  Default setting remains under (id == 0).
> Note that this now is consistent with how we set warnings.
> 
> (Userspace requires updates to enable this as well; xfs_quota needs to
> parse new options, and setquota needs to set appropriate field flags.)

So ... xfs_quota simply never had the ability to do this, but what does
"setquota needs to set appropriate field flags" mean exactly?

--D

> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  fs/xfs/xfs_qm_syscalls.c | 48 +++++++++++++++++++++++-----------------
>  1 file changed, 28 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index 29c1d5d4104d..94d374820c7e 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -555,32 +555,40 @@ xfs_qm_scall_setqlim(
>  		ddq->d_rtbwarns = cpu_to_be16(newlim->d_rt_spc_warns);
>  
>  	if (id == 0) {
> -		/*
> -		 * Timelimits for the super user set the relative time
> -		 * the other users can be over quota for this file system.
> -		 * If it is zero a default is used.  Ditto for the default
> -		 * soft and hard limit values (already done, above), and
> -		 * for warnings.
> -		 */
> -		if (newlim->d_fieldmask & QC_SPC_TIMER) {
> -			defq->btimelimit = newlim->d_spc_timer;
> -			ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer);
> -		}
> -		if (newlim->d_fieldmask & QC_INO_TIMER) {
> -			defq->itimelimit = newlim->d_ino_timer;
> -			ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer);
> -		}
> -		if (newlim->d_fieldmask & QC_RT_SPC_TIMER) {
> -			defq->rtbtimelimit = newlim->d_rt_spc_timer;
> -			ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer);
> -		}
>  		if (newlim->d_fieldmask & QC_SPC_WARNS)
>  			defq->bwarnlimit = newlim->d_spc_warns;
>  		if (newlim->d_fieldmask & QC_INO_WARNS)
>  			defq->iwarnlimit = newlim->d_ino_warns;
>  		if (newlim->d_fieldmask & QC_RT_SPC_WARNS)
>  			defq->rtbwarnlimit = newlim->d_rt_spc_warns;
> -	} else {
> +	}
> +
> +	/*
> +	 * Timelimits for the super user set the relative time the other users
> +	 * can be over quota for this file system. If it is zero a default is
> +	 * used.  Ditto for the default soft and hard limit values (already
> +	 * done, above), and for warnings.
> +	 *
> +	 * For other IDs, userspace can bump out the grace period if over
> +	 * the soft limit.
> +	 */
> +	if (newlim->d_fieldmask & QC_SPC_TIMER)
> +		ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer);
> +	if (newlim->d_fieldmask & QC_INO_TIMER)
> +		ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer);
> +	if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
> +		ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer);
> +
> +	if (id == 0) {
> +		if (newlim->d_fieldmask & QC_SPC_TIMER)
> +			defq->btimelimit = newlim->d_spc_timer;
> +		if (newlim->d_fieldmask & QC_INO_TIMER)
> +			defq->itimelimit = newlim->d_ino_timer;
> +		if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
> +			defq->rtbtimelimit = newlim->d_rt_spc_timer;
> +	}
> +
> +	if (id != 0) {
>  		/*
>  		 * If the user is now over quota, start the timelimit.
>  		 * The user will not be 'warned'.
> -- 
> 2.17.0
> 
>
Eric Sandeen May 19, 2020, 5:21 p.m. UTC | #2
On 5/19/20 11:39 AM, Darrick J. Wong wrote:
> On Mon, May 18, 2020 at 01:52:16PM -0500, Eric Sandeen wrote:
>> The only grace period which can be set in the kernel today is for id 0,
>> i.e. the default grace period for all users.  However, setting an
>> individual grace period is useful; for example:
>>
>>  Alice has a soft quota of 100 inodes, and a hard quota of 200 inodes
>>  Alice uses 150 inodes, and enters a short grace period
>>  Alice really needs to use those 150 inodes past the grace period
>>  The administrator extends Alice's grace period until next Monday
>>
>> vfs quota users such as ext4 can do this today, with setquota -T
> 
> Does setquota -T work on an XFS filesystem? 

With 

[PATCH] quota-tools: Set FS_DQ_TIMER_MASK for individual xfs grace times

it does.

> If so, does that mean that
> xfs had a functionality gap where the admin could extend someone's grace
> period on ext4 but trying the exact same command on xfs would do
> nothing?  Or would it at least error out?

The former; no error.  quota-tools didn't set the timer mask, so quotactl
wasn't set up to change it.  In addition, we ignored the change for id !=0.
 
>> To enable this for XFS, we simply move the timelimit assignment out
>> from under the (id == 0) test.  Default setting remains under (id == 0).
>> Note that this now is consistent with how we set warnings.
>>
>> (Userspace requires updates to enable this as well; xfs_quota needs to
>> parse new options, and setquota needs to set appropriate field flags.)
> 
> So ... xfs_quota simply never had the ability to do this, but what does
> "setquota needs to set appropriate field flags" mean exactly?

It means:

[PATCH] quota-tools: Set FS_DQ_TIMER_MASK for individual xfs grace times

+		if (flags & COMMIT_TIMES) /* indiv grace period */
+			xdqblk.d_fieldmask |= FS_DQ_TIMER_MASK;

-Eric
diff mbox series

Patch

diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 29c1d5d4104d..94d374820c7e 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -555,32 +555,40 @@  xfs_qm_scall_setqlim(
 		ddq->d_rtbwarns = cpu_to_be16(newlim->d_rt_spc_warns);
 
 	if (id == 0) {
-		/*
-		 * Timelimits for the super user set the relative time
-		 * the other users can be over quota for this file system.
-		 * If it is zero a default is used.  Ditto for the default
-		 * soft and hard limit values (already done, above), and
-		 * for warnings.
-		 */
-		if (newlim->d_fieldmask & QC_SPC_TIMER) {
-			defq->btimelimit = newlim->d_spc_timer;
-			ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer);
-		}
-		if (newlim->d_fieldmask & QC_INO_TIMER) {
-			defq->itimelimit = newlim->d_ino_timer;
-			ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer);
-		}
-		if (newlim->d_fieldmask & QC_RT_SPC_TIMER) {
-			defq->rtbtimelimit = newlim->d_rt_spc_timer;
-			ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer);
-		}
 		if (newlim->d_fieldmask & QC_SPC_WARNS)
 			defq->bwarnlimit = newlim->d_spc_warns;
 		if (newlim->d_fieldmask & QC_INO_WARNS)
 			defq->iwarnlimit = newlim->d_ino_warns;
 		if (newlim->d_fieldmask & QC_RT_SPC_WARNS)
 			defq->rtbwarnlimit = newlim->d_rt_spc_warns;
-	} else {
+	}
+
+	/*
+	 * Timelimits for the super user set the relative time the other users
+	 * can be over quota for this file system. If it is zero a default is
+	 * used.  Ditto for the default soft and hard limit values (already
+	 * done, above), and for warnings.
+	 *
+	 * For other IDs, userspace can bump out the grace period if over
+	 * the soft limit.
+	 */
+	if (newlim->d_fieldmask & QC_SPC_TIMER)
+		ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer);
+	if (newlim->d_fieldmask & QC_INO_TIMER)
+		ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer);
+	if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
+		ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer);
+
+	if (id == 0) {
+		if (newlim->d_fieldmask & QC_SPC_TIMER)
+			defq->btimelimit = newlim->d_spc_timer;
+		if (newlim->d_fieldmask & QC_INO_TIMER)
+			defq->itimelimit = newlim->d_ino_timer;
+		if (newlim->d_fieldmask & QC_RT_SPC_TIMER)
+			defq->rtbtimelimit = newlim->d_rt_spc_timer;
+	}
+
+	if (id != 0) {
 		/*
 		 * If the user is now over quota, start the timelimit.
 		 * The user will not be 'warned'.