2005-01-19 20:55:33

by Roland McGrath

[permalink] [raw]
Subject: [PATCH] cputime_t patches broke RLIMIT_CPU

The RLIMIT_CPU limit is in seconds, not in jiffies.

Signed-off-by: Roland McGrath <[email protected]>

--- linux-2.6/kernel/sched.c
+++ linux-2.6/kernel/sched.c
@@ -2301,14 +2327,14 @@ static void check_rlimit(struct task_str
cputime_t total, tmp;

total = cputime_add(p->utime, p->stime);
- tmp = jiffies_to_cputime(p->signal->rlim[RLIMIT_CPU].rlim_cur);
+ tmp = secs_to_cputime(p->signal->rlim[RLIMIT_CPU].rlim_cur);
if (unlikely(cputime_gt(total, tmp))) {
/* Send SIGXCPU every second. */
tmp = cputime_sub(total, cputime);
if (cputime_to_secs(tmp) < cputime_to_secs(total))
send_sig(SIGXCPU, p, 1);
/* and SIGKILL when we go over max.. */
- tmp = jiffies_to_cputime(p->signal->rlim[RLIMIT_CPU].rlim_max);
+ tmp = secs_to_cputime(p->signal->rlim[RLIMIT_CPU].rlim_max);
if (cputime_gt(total, tmp))
send_sig(SIGKILL, p, 1);
}


2005-01-20 00:14:22

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] cputime_t patches broke RLIMIT_CPU



On Wed, 19 Jan 2005, Roland McGrath wrote:
>
> The RLIMIT_CPU limit is in seconds, not in jiffies.

This patch would seem to have its own problems, though. See:

#define secs_to_cputime(__secs) (msecs_to_jiffies(__secs * HZ))

which means that since there is no overflow checking (not in the current
tree, and not in the fixed one that uses proper parenthesis and *1000, you
can easily end up overflowing in the *1000 case, and causing nasty things.

So would it not be nicer to just keep everything in seconds instead?
Alternatively, seriously fix "secs_to_cputime()" to do the proper thing?
Or did I miss a patch and you already did that?

Linus

2005-01-20 00:28:35

by Roland McGrath

[permalink] [raw]
Subject: Re: [PATCH] cputime_t patches broke RLIMIT_CPU

> So would it not be nicer to just keep everything in seconds instead?

Yes, that's how it was done before. The patch I just posted was intended
to fix the apparent typo without getting any deeper. Below is an untested
alternate patch to restore the old behavior under the new macro regime.

> Alternatively, seriously fix "secs_to_cputime()" to do the proper thing?
> Or did I miss a patch and you already did that?

I wasn't really on a cputime-cleanup rampage, just fixing the obvious bugs
I saw. I hope Martin would like to do some cleanup.


Thanks,
Roland


--- linux-2.6/kernel/sched.c
+++ linux-2.6/kernel/sched.c
@@ -2299,17 +2325,17 @@ static void account_it_prof(struct task_
static void check_rlimit(struct task_struct *p, cputime_t cputime)
{
cputime_t total, tmp;
+ unsigned long secs;

total = cputime_add(p->utime, p->stime);
- tmp = jiffies_to_cputime(p->signal->rlim[RLIMIT_CPU].rlim_cur);
- if (unlikely(cputime_gt(total, tmp))) {
+ secs = cputime_to_secs(total);
+ if (unlikely(secs >= p->signal->rlim[RLIMIT_CPU].rlim_cur)) {
/* Send SIGXCPU every second. */
tmp = cputime_sub(total, cputime);
- if (cputime_to_secs(tmp) < cputime_to_secs(total))
+ if (cputime_to_secs(tmp) < secs)
send_sig(SIGXCPU, p, 1);
/* and SIGKILL when we go over max.. */
- tmp = jiffies_to_cputime(p->signal->rlim[RLIMIT_CPU].rlim_max);
- if (cputime_gt(total, tmp))
+ if (secs >= p->signal->rlim[RLIMIT_CPU].rlim_max)
send_sig(SIGKILL, p, 1);
}
}

2005-01-20 00:48:00

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] cputime_t patches broke RLIMIT_CPU



On Wed, 19 Jan 2005, Roland McGrath wrote:
>
> Yes, that's how it was done before. The patch I just posted was intended
> to fix the apparent typo without getting any deeper. Below is an untested
> alternate patch to restore the old behavior under the new macro regime.

Thanks, this one looks good. I have this nagging feeling that the test for
"every second" should be doable by using a multiply (ie do a
secs_to_cputime(secs) and see if it's smaller than "total - cputime")
rather than doing the divide that is implied by "cputime_to_secs()", but I
can't really bring myself to care, and if anything, that test for "did we
go into the next whole second" really is pretty obscure anyway.

So I can't see anything wrong with this. Anybody else? Going, going..

Linus "almost gone" Torvalds