On 23.01.2018 13:14, Stanislav Kinsburskiy wrote: > > > 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 <ktk...@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).
Are you OK with this change sent in the only patch, or should we use one more patch? _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel
