Re: + fork-use-this_cpu_try_cmpxchg-in-try_release_thread_stack_to_cache.patch added to mm-nonmm-unstable branch

[Oleg Nesterov <oleg@redhat.com>]

> From: Uros Bizjak <ubizjak@gmail.com>
> Subject: fork: use this_cpu_try_cmpxchg() in try_release_thread_stack_to_cache()
> Date: Thu, 23 May 2024 09:35:14 +0200
>
> Use this_cpu_try_cmpxchg() instead of this_cpu_cmpxchg (*ptr, old, new) ==
> old in try_release_thread_stack_to_cache.  x86 CMPXCHG instruction returns
> success in ZF flag, so this change saves a compare after cmpxchg (and
> related move instruction in front of cmpxchg).
>
> No functional change intended.

Sorry, cosmetic nit...

> @@ -205,7 +205,9 @@ static bool try_release_thread_stack_to_
>  	unsigned int i;
>  
>  	for (i = 0; i < NR_CACHED_STACKS; i++) {
> -		if (this_cpu_cmpxchg(cached_stacks[i], NULL, vm) != NULL)
> +		struct vm_struct *tmp = NULL;
> +
> +		if (!this_cpu_try_cmpxchg(cached_stacks[i], &tmp, vm))
>  			continue;
>  		return true;
>  	}

Why not

	static bool try_release_thread_stack_to_cache(struct vm_struct *vm)
	{
		unsigned int i;

		for (i = 0; i < NR_CACHED_STACKS; i++) {
			struct vm_struct *tmp = NULL;

			if (this_cpu_try_cmpxchg(cached_stacks[i], &tmp, vm))
				return true;
		}
		return false;
	}

?

To me this "continue" just adds the unnecessary confusion with or without
this patch, but I won't insist.

Oleg.