kern/60550: [PATCH] hitting process limits produces sub-optimal error messages

[Bruce Evans]

The following reply was made to PR kern/60550; it has been noted by GNATS.

From: Bruce Evans <bde at zeta.org.au>
To: Lowell Gilbert <freebsd-bugs-local at be-well.no-ip.com>
Cc: FreeBSD-gnats-submit at FreeBSD.org, silby at FreeBSD.org,
	freebsd-gnats-submit at FreeBSD.org
Subject: Re: kern/60550: [PATCH] hitting process limits produces sub-optimal
 error messages
Date: Thu, 25 Dec 2003 16:29:59 +1100 (EST)

 On Wed, 24 Dec 2003, Lowell Gilbert wrote:
 
 > >Description:
 >
 > Recent changes to the fork code (to control boot-time interrupt
 > storms) added some error messages for the cases where process limits
 > are hit.  These list the UID hitting the limit, but not a process ID,
 > which would be useful also.
 
 I agree.
 
 > >Fix:
 >
 > I have a proposed patch which just adds the current PID to the printf
 > statement.  However, I wonder whether it might be even better to
 > remove the printf completely for the case where process limits (as
 > opposed to maxproc) are being hit.  [If a task hits other process
 > limits, there are no comparable messages produced.]
 
 Removing the printf() would also decrease the numbers of printf format
 errors and style bugs instead of increasing them.
 
 > *** kern_fork.c.ORIG    Wed Nov 12 10:40:22 2003
 > --- kern_fork.c Wed Dec 24 18:51:32 2003
 > ***************
 > *** 239,245 ****
 >         uid = p1->p_cred->p_ruid;
 >         if ((nprocs >= maxproc - 10 && uid != 0) || nprocs >= maxproc) {
 >                 if (ppsratecheck(&lastfail, &curfail, 1))
 > !                       printf("maxproc limit exceeded by uid %d, please see tuning(7) and login.conf(5).\n", uid);
 >                 tsleep(&forksleep, PUSER, "fork", hz / 2);
 >                 return (EAGAIN);
 >         }
 > --- 239,245 ----
 >         uid = p1->p_cred->p_ruid;
 >         if ((nprocs >= maxproc - 10 && uid != 0) || nprocs >= maxproc) {
 >                 if (ppsratecheck(&lastfail, &curfail, 1))
 > !                       printf("maxproc limit exceeded by uid %d (pid %d), please see tuning(7) and login.conf(5).\n", uid, p1->p_pid);
 >                 tsleep(&forksleep, PUSER, "fork", hz / 2);
 >                 return (EAGAIN);
 >         }
 > ***************
 > *** 261,267 ****
 >                  */
 >                 nprocs--;
 >                 if (ppsratecheck(&lastfail, &curfail, 1))
 > !                       printf("maxproc limit exceeded by uid %d, please see tuning(7) and login.conf(5).\n", uid);
 >                 tsleep(&forksleep, PUSER, "fork", hz / 2);
 >                 return (EAGAIN);
 >         }
 > --- 261,267 ----
 >                  */
 >                 nprocs--;
 >                 if (ppsratecheck(&lastfail, &curfail, 1))
 > !                       printf("maxproc limit exceeded by uid %d (pid %d), please see tuning(7) and login.conf(5).\n", uid, p1->p_pid);
 >                 tsleep(&forksleep, PUSER, "fork", hz / 2);
 >                 return (EAGAIN);
 >         }
 
 New printf format errors (old ones duplicated)
 - unsigned type uid_t printed using signed format %d
 - typedefed type uid_t printf using a format that is only suitable for
   printing ints.  uid_t actually currently has type uint32_t which is
   never int.
 
 New style bugs:
 - source level: too-long lines longer than before.
 - user-visible level: not-too-long lines now too long.  (The lines were
   shorter than 80 columns assuming that PID_MAX is not much larger than
   its current value of 99999.)
 
 I use the following patches for the old printf format errors and style
 bugs in -current.  Almost all of the messages about tuning(7) have the
 same bugs.  The bugs fixed here are:
 
 Printf format:
 - errors as above.
 - %i is a weird style (AAIK, %i is just an archaic spelling of %d).
 
 Style:
 - English error: comma splice.
 - verboseness.  "please" adds nothing and there is not enough space for it.
 - punctuation.  Kernel messages are conventionally not terminated by a "."
   except possibly for multi-sentence messages, but the one here was.
 - indentation.  The continuation indent in KNF is 4, but 8 was used.
 
 %%%
 Index: kern_fork.c
 ===================================================================
 RCS file: /home/ncvs/src/sys/kern/kern_fork.c,v
 retrieving revision 1.208
 diff -u -2 -r1.208 kern_fork.c
 --- kern_fork.c	29 Oct 2003 15:23:09 -0000	1.208
 +++ kern_fork.c	12 Nov 2003 16:05:21 -0000
 @@ -731,9 +749,10 @@
  	return (0);
  fail:
 -	if (ppsratecheck(&lastfail, &curfail, 1))
 -		printf("maxproc limit exceeded by uid %i, please see tuning(7) and login.conf(5).\n",
 -			uid);
  	sx_xunlock(&allproc_lock);
  	uma_zfree(proc_zone, newproc);
 +	if (ppsratecheck(&lastfail, &curfail, 1))
 +		printf(
 +	"maxproc limit exceeded by uid %lu; see tuning(7) and login.conf(5)\n",
 +		    (u_long)uid);
  	if (p1->p_flag & P_SA) {
  		PROC_LOCK(p1);
 %%%
 
 Bruce