[PATCH] cputime_t patches broke RLIMIT_CPU

[PATCH] cputime_t patches broke RLIMIT_CPU

[Roland McGrath]

The RLIMIT_CPU limit is in seconds, not in jiffies.

Signed-off-by: Roland McGrath <roland@redhat.com>

--- 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);
 	}

Re: [PATCH] cputime_t patches broke RLIMIT_CPU

[Linus Torvalds]

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

Re: [PATCH] cputime_t patches broke RLIMIT_CPU

[Roland McGrath]

> 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);
 	}
 }

Re: [PATCH] cputime_t patches broke RLIMIT_CPU

[Linus Torvalds]

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