[Devel] [PATCH] kernel: call task_work_run() before exit_task_namespaces()

[Andrey Ryabinin]

On 07/19/2017 04:14 AM, Andrei Vagin wrote:
> From: Andrei Vagin <avagin at virtuozzo.com>
> 
> task_work_run() has to be called before exit_task_namespaces(),
> because fuse_abort_conn() is called from __fput(). If it will not
> be executed, we can hang in request_wait_answer(). We have seen this
> situation when a process was the last member of a mount namespace
> and the mount namespace has a vstorage fuse mount.
> 

Can we pleas have a changelog that doesn't look like an output of random text generator?
The fact that "fuse_abort_conn() is called from __fput()" doesn't really explain why
task_work_run() needs to be called before exit_task_namespaces.



> https://jira.sw.ru/browse/PSBM-68266
> Signed-off-by: Andrei Vagin <avagin at virtuozzo.com>
> ---
>  include/linux/task_work.h | 9 +++++++--
>  kernel/exit.c             | 9 +++++++++
>  kernel/task_work.c        | 4 ++--
>  3 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/task_work.h b/include/linux/task_work.h
> index ca5a1cf..b3af76d 100644
> --- a/include/linux/task_work.h
> +++ b/include/linux/task_work.h
> @@ -14,11 +14,16 @@ init_task_work(struct callback_head *twork, task_work_func_t func)
>  
>  int task_work_add(struct task_struct *task, struct callback_head *twork, bool);
>  struct callback_head *task_work_cancel(struct task_struct *, task_work_func_t);
> -void task_work_run(void);
> +void __task_work_run(bool exiting);
> +
> +static inline void task_work_run(void)
> +{
> +	return __task_work_run(false);
> +}
>  
>  static inline void exit_task_work(struct task_struct *task)
>  {
> -	task_work_run();
> +	__task_work_run(true);
>  }
>  
>  #endif	/* _LINUX_TASK_WORK_H */
> diff --git a/kernel/exit.c b/kernel/exit.c
> index 3c83db2..ea54a73 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -827,6 +827,15 @@ void do_exit(long code)
>  	exit_fs(tsk);
>  	if (group_dead)
>  		disassociate_ctty(1);
> +
> +	/*
> +	 * task_work_run() has to be called before exit_task_namespaces(),
> +	 * because fuse_abort_conn() is called from __fput(). If it will not
> +	 * be executed, we can hang in request_wait_answer(). We have seen this
> +	 * situation when a process was the last member of a mount namespace
> +	 * and the mount namespace has a vstorage fuse mount.
> +	 */
> +	task_work_run();

Given that this is purely fuse's problem, maybe request_wait_answer() could just call task_work_run()?

Or maybe we can just call exit_task_work(tsk) before exit_task_namespaces(tsk). This seems fine to me,
but perhaps I'm missing something.

>  	exit_task_namespaces(tsk);
>  	exit_task_work(tsk);
>  
> diff --git a/kernel/task_work.c b/kernel/task_work.c
> index 65bd3c9..f0000c4 100644
> --- a/kernel/task_work.c
> +++ b/kernel/task_work.c
> @@ -46,7 +46,7 @@ task_work_cancel(struct task_struct *task, task_work_func_t func)
>  	return work;
>  }
>  
> -void task_work_run(void)
> +void __task_work_run(bool exiting)
>  {
>  	struct task_struct *task = current;
>  	struct callback_head *work, *head, *next;
> @@ -58,7 +58,7 @@ void task_work_run(void)
>  		 */
>  		do {
>  			work = ACCESS_ONCE(task->task_works);
> -			head = !work && (task->flags & PF_EXITING) ?
> +			head = !work && exiting ?

Why we need this change? AFAIU this will allow to add more task_works in exit_task_namespaces()
before final exit_task_work(). What's the point of this?

>  				&work_exited : NULL;
>  		} while (cmpxchg(&task->task_works, work, head) != work);
>  
>