Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

Re: (Off-Topic) Re: Bug in ulimit ?

On Wed, Apr 18, 2007 at 10:46:56AM +0300, Tom Alsberg wrote:
> (Repost top zsh-workers in case it may be of interest, original
>  address was misspelled)
[...]

Hi Tom.

Have you got trouble posting to the list as well? At least one
of the emails I posted yesterday didn't make it (the one
attached). Did you receive it (as I think you were in copy)? I
eventually received the email attachment with the patch the
lkml, but as a separate email!

Cheers,
Stéphane
--- Begin Message --- 
On Tue, Apr 17, 2007 at 09:59:25PM +0300, Tom Alsberg wrote:
> On Tue, Apr 17, 2007 at 04:15:01PM +0100, Stephane Chazelas wrote:
> > Hi Tom,
> > 
> > I couldn't find the attachment,
> 
> That is weird - I checked and did attach it, and it even appears in
> the zsh-workers archives at:

Could very well be on my side. I think I've already had issues
like that in the past. Could be yahoo or fetchmail or mutt, I'll
investigate.

Sorry about that.

[...]
> > I don't have time to try it, but I think an easy fix would be to
> > change the sys_setrlimit() code to still keep the value of the
> > cpu-limit timer as 0 as before, but to schedule the processing of
> > CPU timers for the next tick instead of for the next second (which
> > was wrong as it would delay an itimer),
> 
> Since the scheduling is done somewhere else in the code, that may
> require saving some other piece of information to differentiate
> between a zero value meaning "limit not set" and a zero value meaning
> "break immediately".  Thinking of it now, I am not quite sure why a
> value of zero is necessary at all to denote the limit was never set
> (where is that information necessary?), and whether the initial value
> could not be set to infinity or the hard limit.  But again, this logic
> was there already, and is beyond the scope of this small fix.

Sorry, I wasn't very clear. I didn't mean scheduling as in
process/task scheduling, but as in sceduling of the function
that will check the timers to be fired. The "cheat" wasn't
modifying the current value of the rlimit from 0 to 1 second, it
was just scheduling the next checking for whether that limit is
reached or not to 1 second from now. So that the process would
keep running for 1 second of CPU time with a pending SIGXCPU
largely overdue.

That's why I mentionned the setitimer(ITIMER_PROF). If you
install such a timer, you rescedule the function that checks the
timers to be fired (at least the timers that work in CPU time
such as those "PROF" timers and the cpu rlimit). If you install
such an itimer with a very small timeout just after a ulimit -t
0, you'll find that the process receives the SIGXCPU upon the
expiry of that timer, not after the 1 second delay, I don't know
if that was intentional or not.

> 
> > and at the time of the fork, make sure the it_prof_expirer in
> > copy_signal is not set to zero (for instance, schedule it for the
> > first tick, as in the code in copy_process).
> 
> I have very little with the profiling code in Linux, so I am not sure
> whether this alone will do.
[...]

My understanding is that if one does ulimit -t 0, he wants the
process to get a SIGCPU as soon as it uses the CPU.

The fix I had in mind is below:

--- kernel/fork.c~	2007-04-17 20:23:29.000000000 +0100
+++ kernel/fork.c	2007-04-17 20:14:32.000000000 +0100
@@ -830,6 +830,7 @@ static inline int copy_signal(unsigned l
 {
 	struct signal_struct *sig;
 	int ret;
+	unsigned long cpu_rlimit;
 
 	if (clone_flags & CLONE_THREAD) {
 		atomic_inc(&current->signal->count);
@@ -884,13 +885,16 @@ static inline int copy_signal(unsigned l
 	memcpy(sig->rlim, current->signal->rlim, sizeof sig->rlim);
 	task_unlock(current->group_leader);
 
-	if (sig->rlim[RLIMIT_CPU].rlim_cur != RLIM_INFINITY) {
+	cpu_rlimit = sig->rlim[RLIMIT_CPU].rlim_cur;
+	if (cpu_rlimit != RLIM_INFINITY) {
 		/*
 		 * New sole thread in the process gets an expiry time
 		 * of the whole CPU time limit.
 		 */
-		tsk->it_prof_expires =
-			secs_to_cputime(sig->rlim[RLIMIT_CPU].rlim_cur);
+		if (cpu_rlimit == 0)
+			tsk->it_prof_expires = jiffies_to_cputime(1);
+		else
+			tsk->it_prof_expires = secs_to_cputime(cpu_rlimit);
 	}
 	acct_init_pacct(&sig->pacct);
 
--- kernel/sys.c~	2007-04-17 20:09:28.000000000 +0100
+++ kernel/sys.c	2007-04-17 20:16:02.000000000 +0100
@@ -1941,19 +1941,13 @@ asmlinkage long sys_setrlimit(unsigned i
 
 	it_prof_secs = cputime_to_secs(current->signal->it_prof_expires);
 	if (it_prof_secs == 0 || new_rlim.rlim_cur <= it_prof_secs) {
-		unsigned long rlim_cur = new_rlim.rlim_cur;
 		cputime_t cputime;
 
-		if (rlim_cur == 0) {
-			/*
-			 * The caller is asking for an immediate RLIMIT_CPU
-			 * expiry.  But we use the zero value to mean "it was
-			 * never set".  So let's cheat and make it one second
-			 * instead
-			 */
-			rlim_cur = 1;
-		}
-		cputime = secs_to_cputime(rlim_cur);
+		if (new_rlim.rlim_cur == 0)
+			cputime = jiffies_to_cputime(1);
+		else
+			cputime = secs_to_cputime(new_rlim.rlim_cur);
+		
 		read_lock(&tasklist_lock);
 		spin_lock_irq(&current->sighand->siglock);
 		set_process_cpu_timer(current, CPUCLOCK_PROF, &cputime, NULL);

instead of scheduling it for in one seconds, it schedules it for
the next tick (jiffy), which is kind of more consistent with my
idea of what ulimit -t does, and doesn't explicitely modify what
the user requested.

To get back on topic (slightly :)), with that patch applied, I
get:

$ zsh -c 'float SECONDS; trap "" XCPU; ulimit -t 0
  (trap "echo \$SECONDS" XCPU; while :; do :; done); echo done'
3.253000000e-03
1.009181000e+00
2.019183000e+00
....


(when SIGXCPU is intercepted, Linux resends it every second
until the hard limit is reached).

-- 
Stéphane

--- End Message ---
Messages sorted by: Reverse Date, Date, Thread, Author