* [PATCH RFC] xfs: allow adjusting individual quota grace times
@ 2020-05-09 19:47 Eric Sandeen
2020-05-09 19:51 ` Eric Sandeen
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Eric Sandeen @ 2020-05-09 19:47 UTC (permalink / raw)
To: linux-xfs; +Cc: Jan Kara
vfs/ext3/4 quota allows the administrator to push out the grace time
for soft quota with the setquota -T command:
setquota -T [ -u | -g ] [ -F quotaformat ] name block-grace inode-grace -a | filesystem...
-T, --edit-times
Alter times for individual user/group when softlimit is enforced.
Times block-grace and inode-grace are specified in seconds or can
be string 'unset'.
Essentially, if you do "setquota -T -u username 1d 1d" and "username" is
over their soft quotas and into their grace time, it will extend the
grace time expiry to 1d from now.
xfs can't do this, today. The patch below is a first cut at allowing us
to do this, and userspace updates are needed as well (I have those in a
patch stack.)
I'm not looking so much for patch review right now, though, what I'm
wondering is if this is a change we can make from the ABI perspective?
Because today, if you try to pass in a UID other than 0 (i.e. the
default grace period) it just gets ignored by the kernel, not rejected.
So there's no real way to know that the grace period adjustment failed
on an older kernel. We could consider that a bug and fix it, or
consider it a change in behavior that we can't just make without
at least some form of versioning. Thoughts?
Anyway, the patch below moves the disk quota grace period adjustment out
from "if id == 0" and allows the change for any ID; it only sets the
default grace value in the "id == 0" case.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index f48561b7e947..e58ee98f938c 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -555,32 +555,41 @@ 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) {
+ if (!id)
+ defq->btimelimit = newlim->d_spc_timer;
+ ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer);
+ }
+ if (newlim->d_fieldmask & QC_INO_TIMER) {
+ printk("setting inode timer to %d\n", newlim->d_ino_timer);
+ if (!id)
+ defq->itimelimit = newlim->d_ino_timer;
+ ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer);
+ }
+ if (newlim->d_fieldmask & QC_RT_SPC_TIMER) {
+ if (!id)
+ defq->rtbtimelimit = newlim->d_rt_spc_timer;
+ ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer);
+ }
+
+ if (id != 0) {
/*
* If the user is now over quota, start the timelimit.
* The user will not be 'warned'.
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH RFC] xfs: allow adjusting individual quota grace times
2020-05-09 19:47 [PATCH RFC] xfs: allow adjusting individual quota grace times Eric Sandeen
@ 2020-05-09 19:51 ` Eric Sandeen
2020-05-11 15:35 ` Christoph Hellwig
2020-05-12 23:29 ` Darrick J. Wong
2 siblings, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2020-05-09 19:51 UTC (permalink / raw)
To: linux-xfs; +Cc: Jan Kara
The userspace change is basically just modifying timer_f so we can
specify an id; if it's not id 0, we set the grace time to now() + value,
iff the user is over the soft limit (this mirrors what setquota does):
+ /*
+ * If id is specified we are extending grace time by value
+ * Otherwise we are setting the default grace time
+ */
+ if (id) {
+ time_t now;
+
+ if (xfsquotactl(XFS_GETQUOTA, dev, type, id, (void *)&d) < 0) {
+ exitcode = 1;
+ fprintf(stderr, _("%s: cannot get quota: %s\n"),
+ progname, strerror(errno));
+ return;
+ }
+
+ time(&now);
+
+ if (d.d_blk_hardlimit && d.d_bcount > d.d_blk_hardlimit)
+ d.d_btimer = now + value;
+ if (d.d_ino_softlimit && d.d_icount > d.d_ino_softlimit)
+ d.d_itimer = now + value;
+ if (d.d_rtb_softlimit && d.d_rtbcount > d.d_rtb_softlimit)
+ d.d_rtbtimer = now + value;
+ } else {
+ d.d_btimer = value;
+ d.d_itimer = value;
+ d.d_rtbtimer = value;
+ }
+
and then we set the quota via the quotactl as normal.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH RFC] xfs: allow adjusting individual quota grace times
2020-05-09 19:47 [PATCH RFC] xfs: allow adjusting individual quota grace times Eric Sandeen
2020-05-09 19:51 ` Eric Sandeen
@ 2020-05-11 15:35 ` Christoph Hellwig
2020-05-12 23:27 ` Darrick J. Wong
2020-05-12 23:29 ` Darrick J. Wong
2 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2020-05-11 15:35 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs, Jan Kara
On Sat, May 09, 2020 at 02:47:02PM -0500, Eric Sandeen wrote:
> So there's no real way to know that the grace period adjustment failed
> on an older kernel. We could consider that a bug and fix it, or
> consider it a change in behavior that we can't just make without
> at least some form of versioning. Thoughts?
I'd consider a bug, as applications are very unlikely to rely on
these ignored calls (famous last words..).
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] xfs: allow adjusting individual quota grace times
2020-05-11 15:35 ` Christoph Hellwig
@ 2020-05-12 23:27 ` Darrick J. Wong
0 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2020-05-12 23:27 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Eric Sandeen, linux-xfs, Jan Kara
On Mon, May 11, 2020 at 08:35:32AM -0700, Christoph Hellwig wrote:
> On Sat, May 09, 2020 at 02:47:02PM -0500, Eric Sandeen wrote:
> > So there's no real way to know that the grace period adjustment failed
> > on an older kernel. We could consider that a bug and fix it, or
> > consider it a change in behavior that we can't just make without
> > at least some form of versioning. Thoughts?
Well you /could/ roll this new functionality into a new revision of the
ioctl that supports 64bit time values...
> I'd consider a bug, as applications are very unlikely to rely on
> these ignored calls (famous last words..).
...but OTOH that might be a better discussion to have when we start
reviewing the bigtime series. In the meantime, seeing as xfs_quota
never really let you push out soft grace period expiration anyway, I
guess you could just teach userspace to try to make the change and
declare success if an immediate re-query shows the value changed.
--D
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] xfs: allow adjusting individual quota grace times
2020-05-09 19:47 [PATCH RFC] xfs: allow adjusting individual quota grace times Eric Sandeen
2020-05-09 19:51 ` Eric Sandeen
2020-05-11 15:35 ` Christoph Hellwig
@ 2020-05-12 23:29 ` Darrick J. Wong
2020-05-12 23:34 ` Eric Sandeen
2 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2020-05-12 23:29 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs, Jan Kara
On Sat, May 09, 2020 at 02:47:02PM -0500, Eric Sandeen wrote:
> vfs/ext3/4 quota allows the administrator to push out the grace time
> for soft quota with the setquota -T command:
>
> setquota -T [ -u | -g ] [ -F quotaformat ] name block-grace inode-grace -a | filesystem...
>
> -T, --edit-times
> Alter times for individual user/group when softlimit is enforced.
> Times block-grace and inode-grace are specified in seconds or can
> be string 'unset'.
>
> Essentially, if you do "setquota -T -u username 1d 1d" and "username" is
> over their soft quotas and into their grace time, it will extend the
> grace time expiry to 1d from now.
>
> xfs can't do this, today. The patch below is a first cut at allowing us
> to do this, and userspace updates are needed as well (I have those in a
> patch stack.)
>
> I'm not looking so much for patch review right now, though, what I'm
> wondering is if this is a change we can make from the ABI perspective?
>
> Because today, if you try to pass in a UID other than 0 (i.e. the
> default grace period) it just gets ignored by the kernel, not rejected.
>
> So there's no real way to know that the grace period adjustment failed
> on an older kernel. We could consider that a bug and fix it, or
> consider it a change in behavior that we can't just make without
> at least some form of versioning. Thoughts?
>
> Anyway, the patch below moves the disk quota grace period adjustment out
> from "if id == 0" and allows the change for any ID; it only sets the
> default grace value in the "id == 0" case.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index f48561b7e947..e58ee98f938c 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -555,32 +555,41 @@ 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) {
> + if (!id)
> + defq->btimelimit = newlim->d_spc_timer;
> + ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer);
> + }
> + if (newlim->d_fieldmask & QC_INO_TIMER) {
> + printk("setting inode timer to %d\n", newlim->d_ino_timer);
Stray printk here.
> + if (!id)
> + defq->itimelimit = newlim->d_ino_timer;
> + ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer);
> + }
> + if (newlim->d_fieldmask & QC_RT_SPC_TIMER) {
> + if (!id)
> + defq->rtbtimelimit = newlim->d_rt_spc_timer;
> + ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer);
> + }
Otherwise I guess this looks reasonable. It might help to patchbomb all
the kernel/userspace/fstests changes together so I don't have to go
scurrying around to find the pieces(?)
--D
> +
> + if (id != 0) {
> /*
> * If the user is now over quota, start the timelimit.
> * The user will not be 'warned'.
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH RFC] xfs: allow adjusting individual quota grace times
2020-05-12 23:29 ` Darrick J. Wong
@ 2020-05-12 23:34 ` Eric Sandeen
0 siblings, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2020-05-12 23:34 UTC (permalink / raw)
To: Darrick J. Wong, Eric Sandeen; +Cc: linux-xfs, Jan Kara
On 5/12/20 6:29 PM, Darrick J. Wong wrote:
> On Sat, May 09, 2020 at 02:47:02PM -0500, Eric Sandeen wrote:
>> vfs/ext3/4 quota allows the administrator to push out the grace time
>> for soft quota with the setquota -T command:
>>
>> setquota -T [ -u | -g ] [ -F quotaformat ] name block-grace inode-grace -a | filesystem...
>>
>> -T, --edit-times
>> Alter times for individual user/group when softlimit is enforced.
>> Times block-grace and inode-grace are specified in seconds or can
>> be string 'unset'.
>>
>> Essentially, if you do "setquota -T -u username 1d 1d" and "username" is
>> over their soft quotas and into their grace time, it will extend the
>> grace time expiry to 1d from now.
>>
>> xfs can't do this, today. The patch below is a first cut at allowing us
>> to do this, and userspace updates are needed as well (I have those in a
>> patch stack.)
>>
>> I'm not looking so much for patch review right now, though, what I'm
>> wondering is if this is a change we can make from the ABI perspective?
...
>> + * For other IDs, userspace can bump out the grace period if over
>> + * the soft limit.
>> + */
>> + if (newlim->d_fieldmask & QC_SPC_TIMER) {
>> + if (!id)
>> + defq->btimelimit = newlim->d_spc_timer;
>> + ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer);
>> + }
>> + if (newlim->d_fieldmask & QC_INO_TIMER) {
>> + printk("setting inode timer to %d\n", newlim->d_ino_timer);
>
> Stray printk here.
>
>> + if (!id)
>> + defq->itimelimit = newlim->d_ino_timer;
>> + ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer);
>> + }
>> + if (newlim->d_fieldmask & QC_RT_SPC_TIMER) {
>> + if (!id)
>> + defq->rtbtimelimit = newlim->d_rt_spc_timer;
>> + ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer);
>> + }
>
> Otherwise I guess this looks reasonable. It might help to patchbomb all
> the kernel/userspace/fstests changes together so I don't have to go
> scurrying around to find the pieces(?)
yeah I will do that, this was more akin to pseudocode fo the larger
"can I even change this behavior?" question.
Thanks,
-Eric
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-12 23:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-09 19:47 [PATCH RFC] xfs: allow adjusting individual quota grace times Eric Sandeen
2020-05-09 19:51 ` Eric Sandeen
2020-05-11 15:35 ` Christoph Hellwig
2020-05-12 23:27 ` Darrick J. Wong
2020-05-12 23:29 ` Darrick J. Wong
2020-05-12 23:34 ` Eric Sandeen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox