On Mon, May 04, 2009 at 01:17:40AM -0700, [email protected] wrote: > From: Sukadev Bhattiprolu <[email protected]> > > alloc_pidmap() can fail either because all pid numbers are in use or > we can't allocate memory. With support for setting a specific pid > number, alloc_pidmap() would also fail if either the given pid > number is invalid or in use. > > Rather than have caller assume -ENOMEM, have alloc_pidmap() return > the actual error.
I think generating actual "errors" should be moved into alloc_pidmap_page(). > > Signed-off-by: Sukadev Bhattiprolu <[email protected]> > --- > kernel/fork.c | 5 +++-- > kernel/pid.c | 9 ++++++--- > 2 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/kernel/fork.c b/kernel/fork.c > index b9e2edd..f8411a8 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -1119,10 +1119,11 @@ static struct task_struct *copy_process(unsigned long > clone_flags, > goto bad_fork_cleanup_io; > > if (pid != &init_struct_pid) { > - retval = -ENOMEM; > pid = alloc_pid(p->nsproxy->pid_ns); > - if (!pid) > + if (IS_ERR(pid)) { > + retval = PTR_ERR(pid); > goto bad_fork_cleanup_io; > + } > > if (clone_flags & CLONE_NEWPID) { > retval = pid_ns_prepare_proc(p->nsproxy->pid_ns); > diff --git a/kernel/pid.c b/kernel/pid.c > index c0aaebe..fd72ad9 100644 > --- a/kernel/pid.c > +++ b/kernel/pid.c > @@ -151,6 +151,7 @@ static int alloc_pidmap(struct pid_namespace *pid_ns) > { > int i, offset, max_scan, pid, last = pid_ns->last_pid; > struct pidmap *map; > + int rc = -EAGAIN; > > pid = last + 1; > if (pid >= pid_max) > @@ -159,8 +160,10 @@ static int alloc_pidmap(struct pid_namespace *pid_ns) > map = &pid_ns->pidmap[pid/BITS_PER_PAGE]; > max_scan = (pid_max + BITS_PER_PAGE - 1)/BITS_PER_PAGE - !offset; > for (i = 0; i <= max_scan; ++i) { > - if (alloc_pidmap_page(map)) > + if (alloc_pidmap_page(map)) { > + rc = -ENOMEM; You could return -ENOMEM from alloc_pidmap_page(map) and then just: rc = alloc_pidmap_page(map); if (rc) break; ENOMEM here is not necessarily correct -- you could have alloc'd pages just fine and failed due to the last race-checking if () in alloc_pidmap_page(). All the more reason to move generation of the error to alloc_pidmap_page() IMHO. Cheers, -Matt Helsley _______________________________________________ Containers mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/containers _______________________________________________ Devel mailing list [email protected] https://openvz.org/mailman/listinfo/devel
