On Wed, Jan 16, 2019 at 01:12:27PM -0800, Matthew Wilcox wrote:
> > I don't think the comment explaining why the aglorithm is so special
> > should be deleted :)
>
> Fair, but I think it's in the wrong place. It should be up with the
> definition of RESERVED_PIDS, and it should probably say why! How about this:
>
> /*
> * We avoid reusing PIDs below this to prevent user processes receiving
> * signals which are intended for system daemons.
> */
> #define RESERVED_PIDS 300
This is better, yes.. I assume this old behavior is to support old
init systems that rely on pid files and might experience a race when a
daemon dies if a new process is re-spawned into the old pid?
> > I think this is clearer as to the intended special case:
> >
> > retval = __xa_alloc_cyclic(&tmp->pids, &pid->numbers[i].nr,
> > NULL, XA_LIMIT(tmp->last_pid < RESERVED_PIDS ?
> > 1 : RESERVED_PIDS, pid_max),
> > &tmp->last_pid, GFP_KERNEL);
>
> That's ... hard to read for me. How about this?
Yah, I wouldn't usually reach for a ?:, just wanted to be concise for
the email
> for (i = ns->level; i >= 0; i--) {
> unsigned int min = RESERVED_PIDS;
>
> xa_lock_irq(&tmp->pids);
>
> /* Allow allocation below RESERVED_PID the first time */
> if (min > tmp->last_pid)
> min = tmp->last_pid;
Not min = 1?
Jason
