[PATCH 4/4] utrace_release_task: check REAP before utrace_reap()

[Oleg Nesterov]

On 09/07, Roland McGrath wrote:
>
> In the original code, utrace_release_task() would not be called at all when
> there are no engines attached.  IMHO that is the optimization we want to
> get back to.
>
> It may still be too early in the morning for me to be thinking clearly.

and too late for me ;) will reply tomorrow.

Oleg.

> diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
> index e249770..0000000 100644  
> --- a/include/linux/tracehook.h
> +++ b/include/linux/tracehook.h
> @@ -362,7 +362,12 @@ static inline void tracehook_report_vfor
>   */
>  static inline void tracehook_prepare_release_task(struct task_struct *task)
>  {
> -	utrace_release_task(task);
> +	/*
> +	 * See kernel/utrace.c:utrace_attach_task() about this barrier.
> +	 */
> +	smp_mb();
> +	if (task_utrace_flags(task))
> +		utrace_release_task(task);
>  }
>  
>  /**
> diff --git a/kernel/utrace.c b/kernel/utrace.c
> index 76d0d84..0000000 100644  
> --- a/kernel/utrace.c
> +++ b/kernel/utrace.c
> @@ -143,13 +143,13 @@ static int utrace_add_engine(struct task
>  
>  	spin_lock(&utrace->lock);
>  
> -	if (utrace->reap) {
> +	if (unlikely(utrace->reap)) {
>  		/*
>  		 * Already entered utrace_release_task(), cannot attach now.
>  		 */
>  		ret = -ESRCH;
>  	} else if ((flags & UTRACE_ATTACH_EXCLUSIVE) &&
> -	    unlikely(matching_engine(utrace, flags, ops, data))) {
> +		   unlikely(matching_engine(utrace, flags, ops, data))) {
>  		ret = -EEXIST;
>  	} else {
>  		/*
> @@ -216,16 +216,19 @@ struct utrace_engine *utrace_attach_task
>  
>  	utrace = &target->utrace;
>  
> -	if (unlikely(target->exit_state == EXIT_DEAD)) {
> -		/*
> -		 * The target has already been reaped.
> -		 * Check this early, though it's not synchronized.
> -		 * utrace_add_engine() will do the final check.
> -		 */
> -		if (!(flags & UTRACE_ATTACH_CREATE))
> -			return ERR_PTR(-ENOENT);
> -		return ERR_PTR(-ESRCH);
> -	}
> +	/*
> +	 * If it already could be getting reaped, we cannot attach now.
> +	 * The barrier ensures this check precedes utrace_add_engine()
> +	 * setting ->utrace_flags.  tracehook_prepare_release_task() has
> +	 * the corresponding barrier, after setting EXIT_DEAD and before
> +	 * checking ->utrace_flags to decide to call utrace_release_task().
> +	 */
> +	ret = target->exit_state;
> +	smp_mb();
> +
> +	if (unlikely(ret == EXIT_DEAD))
> +		return ERR_PTR((flags & UTRACE_ATTACH_CREATE) ?
> +			       -ESRCH : -ENOENT);
>  
>  	if (!(flags & UTRACE_ATTACH_CREATE)) {
>  		spin_lock(&utrace->lock);