Messages in this thread | | | Date | Tue, 10 Sep 2013 16:01:23 +0200 | From | Oleg Nesterov <> | Subject | [PATCH 0/1] pidns: fix free_pid() to handle the first fork failure |
| |
On 09/09, Eric W. Biederman wrote: > > Oleg Nesterov <oleg@redhat.com> writes: > > > > Agreed, it also makes sense to clear ->nr_hashed. But I still think > > that WARN_ON(ns->child_reaper) makes sense too. > > I don't know that I like warnings for impossible conditions.
But WARN_ON() should only check for "impossible" conditions ;)
> How could > we even make a mistake that gets us there?
I do not know! I mean, this should not happen, that is why it adds a warning.
And note that "ns->nr_hashed = 0" is not really needed, still I agree it makes sense.
However I won't mind to remove this warning if you really dislike it.
> >> At which point I ask myself what of the pathlogocical case where the > >> first fork fails but because we created the pid namespace with unshare > >> there is a concurrent fork from another process into the pid namespace > >> that succeeds. Resulting in one pid in the pid namespace that is not > >> the reaper. > > > > But how can setns() work before the first fork() succeeds and makes the > > ->child_reaper visible in /proc ? > > > > Probably I missed something obvious, I didn't sleep today... > > Actually that is a very good point. That is an accidental feature but > one I very much appreciate today.
OK. Please review v2 then. I also shamelessly stole your comment.
Oleg.
| |