Messages in this thread | | | Date | Sun, 4 Dec 2011 16:45:43 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCH 1/1][v2] Add reboot_pid_ns to handle the reboot syscall |
| |
On 12/04, Daniel Lezcano wrote: > > @@ -32,6 +32,8 @@ struct pid_namespace { > #endif > gid_t pid_gid; > int hide_pid; > + int reboot; > + spinlock_t reboot_lock; > };
Well. I was thinking about the serialization too, but this ->reboot_lock asks for v3 imho ;)
First of all, do we really care? force_sig(SIGKILL, child_reaper) can't race with itself, it does nothing if init is already killed.
So why it is important to protect pid_ns->reboot? Yes, it is possible to change it again if two callers do sys_reboot() "at the same time". But in this case we can't predict which caller wins anyway, so why should we worry?
IOW. Say, we have the 2 tasks doing HALT and RESTART in parallel. It is possible that HALT sets ->reboot and kills init first, then RESTART changes ->reboot and the second force_sig() does nothing. In this case we can simply pretend that RESTART wins and the dying init kills HALT before it calls sys_reboot().
In any case. Even if you want to serialize, instead of adding the new lock reboot_pid_ns() can simply do:
if (cmpxchg(&pid_ns->reboot, 0, reboot) != 0) return -EBUSY;
this looks much simpler to me.
Oleg.
| |