[CRIU] [PATCH 3/4] remap: add a dead pid /proc remap

[Pavel Emelyanov]

On 09/18/2014 05:41 PM, Tycho Andersen wrote:
> On Thu, Sep 18, 2014 at 03:03:15PM +0400, Pavel Emelyanov wrote:
>> On 09/18/2014 12:32 AM, Tycho Andersen wrote:
>>> If a file like /proc/20/mountinfo is open, but 20 is a zombie (or doesn't exist
>>> any more), we can't read this file at all, so a link remap won't work. Instead,
>>> we add a new remap, called the dead process remap, which forks a TASK_HELPER as
>>> that dead pid so that the restore task can open the new /proc/20/mountinfo
>>> instead.
>>>
>>> This commit also adds a new stage CR_STATE_RESTORE_SHARED. Since new
>>> TASK_HELPERS are added when loading the shared resource images, we need to wait
>>> to start forking tasks until after these resources are loaded.
>>
>> I don't quite understand this explanation. CRIU forks root task, which calls
>> root_prepare_shared() in which all the remaps are collected. At that point any
>> proc remap is lined into the pstree. After this root task starts forking
>> whatever entries it finds in there. Why do we need one more stage?
> 
> Because of restore_root_task. Think of the case when after the
> CR_STATE_RESTORE_NS, the restore_root_task process runs until it
> starts waiting for CR_STATE_FORKING. It enters that wait looking for
> task_entries->nr_tasks + task_entries->nr_helpers. But then the init
> task runs, parses the shared state and adds some more helpers. The
> first thread was waiting on the wrong number of helpers.
> 
> Another option would be to get rid of the CR_STATE_RESTORE_NS point
> and just synchronize after restoring the shared state. Then the
> ACT_SETUP_NS scripts wouldn't necessarily run before the tasks are
> forked, though. I didn't know if that was a problem or not, so I left
> it the way it was.

I think we can just move the call to root_prepare_shared() to be above
the _NS synchronization point. IIRC there's nothing that requires namespaces
to be set up.

Can you try that?

Thanks,
Pavel