[Devel] [PATCH] spfs: Main process wakes and kills its children on exit

[Stanislav Kinsburskiy]

23.01.2018 11:09, Kirill Tkhai пишет:
> On 23.01.2018 13:07, Stanislav Kinsburskiy wrote:
>>
>>
>> 23.01.2018 10:51, Kirill Tkhai пишет:
>>> On 23.01.2018 12:48, Stanislav Kinsburskiy wrote:
>>>> Please, see a couple of nits below
>>>>
>>>> 23.01.2018 10:41, Kirill Tkhai пишет:
>>>>> Stanislav Kinsburskiy says:
>>>>>
>>>>> "SPFS manager has a special "--exit-with-spfs" options, which is used by CRIU.
>>>>>  The idea of the option is simple: force SPFS manager to exit, when it has some
>>>>>  SPFS processes among its children (i.e. spfs was mounted at least once),
>>>>>  but all these processes have exited for whatever reason (which usually happens
>>>>>  when restore has failed and spfs mounts where unmounted).
>>>>>  Although it works in overall (main SPFS manager process exits), its children
>>>>>  (responsible to SPFS replacement) may wait on FUTEX for "release" command
>>>>>  for corresponding SPFS mount and thus never stop until they are killed".
>>>>>
>>>>> 1 spfs-manager
>>>>> 2   \_ spfs
>>>>> 3   \_ spfs-manager
>>>>> 4   \_ spfs
>>>>> 5   \_ spfs-manager
>>>>>
>>>>> 2 and 3 are pair of a mount, and 4 and 5 are pair of another mount.
>>>>> The patch makes spfs-manager 1 kill 3 in case of 2 exited.
>>>>>
>>>>> https://jira.sw.ru/browse/PSBM-80055
>>>>>
>>>>> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
>>>>> ---
>>>>>  manager/context.c |    4 ++++
>>>>>  manager/spfs.c    |    1 +
>>>>>  2 files changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/manager/context.c b/manager/context.c
>>>>> index 1eb37c9..4464a23 100644
>>>>> --- a/manager/context.c
>>>>> +++ b/manager/context.c
>>>>> @@ -53,6 +53,9 @@ static void cleanup_spfs_mount(struct spfs_manager_context_s *ctx,
>>>>>  		/* SPFS master was killed. We need to release the reference */
>>>>>  		spfs_release_mnt(info);
>>>>>  
>>>>> +	if (killed || WEXITSTATUS(status))
>>>>> +		kill(info->replacer, SIGKILL);
>>>>> +
>>>>
>>>> There is "if (killed)" check above.
>>>> Could you please move this hunk there?
>>>
>>> There is logical OR (||) in the hunk. How should I move it to unconditional check?
>>>
>>
>> Ah, ok. Then let the check be as it is.
>> Could you please add warning message for this kill with the process pid being killed and the reason why (spfs was killed of exited with error)?
> 
> Maybe we should call spfs_release_mnt() in case of exit status != 0 too?
> 

Well, this makes sense.
If spfs exited with non-zero result (either it was killed or exited due to some error), then there is no need in replacer. And there is no need in the reference.
So, probably this check you proposed should be used for both spfs_release_mnt(info) and kill(info->replacer, SIGKILL).