On Sun, Feb 05, 2017 at 04:51:50PM -0800, Philip Guenther wrote:
> On Sun, 5 Feb 2017, Ossi Herrala wrote:
> > init(8) is wanted to have process ID 1. It's also the only process which
> > is assigned non-random PID (well, there's also swapper as PID 0).
> >
> > This patch renames fork1() to fork1_to_pid() and introduces new argument
> > "pid" which can be used to select PID for new process. When pid is 0,
> > random PID is assigned. fork1() is then wrapper to fork1_to_pid() with
> > pid argument being 0. No functional change in fork1().
>
> I wouldn't have a problem with this diff...except it adds a *ninth*
> argument to a function.  fork1() is already really bad with eight
> arguments: we need to break up its uses and refactor it to be less
> confusing and overloaded and not add Yet Another Argument With Magic
> Value.
>

After couple of private mails with guenther@ we came up with the
following patch.

Introduce new flag FORK_PID1 for fork1(9).

This flag is special and only for forking init(8) which wants to be
PID 1.

With this flag in place, it's possible to remove global randompid
variable which was used to control if allocpid() returns random PIDs
or PID 1 for init(8). Now allocpid() can also be simplified.


---
 sys/kern/init_main.c |  6 ++----
 sys/kern/kern_fork.c | 22 +++++++++-------------
 sys/sys/proc.h       |  2 +-
 3 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c
index e21a8306854..7324727b415 100644
--- a/sys/kern/init_main.c
+++ b/sys/kern/init_main.c
@@ -437,14 +437,12 @@ main(void *framep)
        {
                struct proc *initproc;

-               if (fork1(p, FORK_FORK, NULL, 0, start_init, NULL, NULL,
-                   &initproc))
+               if (fork1(p, FORK_FORK|FORK_PID1, NULL, 0, start_init, NULL,
+                   NULL, &initproc))
                        panic("fork init");
                initprocess = initproc->p_p;
        }

-       randompid = 1;
-
        /*
         * Create any kernel threads whose creation was deferred because
         * initprocess had not yet been created.
diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index 3ff2085f732..5ed6251d390 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -71,7 +71,6 @@

 int    nprocesses = 1;         /* process 0 */
 int    nthreads = 1;           /* proc 0 */
-int    randompid;              /* when set to 1, pid's go random */
 struct forkstat forkstat;

 void fork_return(void *);
@@ -193,7 +192,11 @@ process_new(struct proc *p, struct process *parent, int 
flags)
            (caddr_t)&pr->ps_endcopy - (caddr_t)&pr->ps_startcopy);

        process_initialize(pr, p);
-       pr->ps_pid = allocpid();
+
+       if (flags & FORK_PID1)
+               pr->ps_pid = 1;
+       else
+               pr->ps_pid = allocpid();

        /* post-copy fixups */
        pr->ps_pptr = parent;
@@ -590,19 +593,12 @@ ispidtaken(pid_t pid)
 pid_t
 allocpid(void)
 {
-       static pid_t lastpid;
        pid_t pid;

-       if (!randompid) {
-               /* only used early on for system processes */
-               pid = ++lastpid;
-       } else {
-               /* Find an unused pid satisfying lastpid < pid <= PID_MAX */
-               do {
-                       pid = arc4random_uniform(PID_MAX - lastpid) + 1 +
-                           lastpid;
-               } while (ispidtaken(pid));
-       }
+       /* Find an unused pid satisfying 1 < pid <= PID_MAX */
+       do {
+               pid = 2 + arc4random_uniform(PID_MAX - 1);
+       } while (ispidtaken(pid));

        return pid;
 }
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 1a1f0966518..5f0b310ff23 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -457,6 +457,7 @@ struct uidinfo *uid_find(uid_t);
 #define FORK_SIGHAND   0x00000200
 #define FORK_PTRACE    0x00000400
 #define FORK_THREAD    0x00000800
+#define FORK_PID1      0x00001000

 #define EXIT_NORMAL            0x00000001
 #define EXIT_THREAD            0x00000002
@@ -478,7 +479,6 @@ extern struct proc proc0;           /* Process slot for 
swapper. */
 extern struct process process0;                /* Process slot for kernel 
threads. */
 extern int nprocesses, maxprocess;     /* Cur and max number of processes. */
 extern int nthreads, maxthread;                /* Cur and max number of 
threads. */
-extern int randompid;                  /* fork() should create random pid's */

 LIST_HEAD(proclist, proc);
 LIST_HEAD(processlist, process);
--
2.11.0



--
Ossi Herrala



--
Ossi Herrala

Reply via email to