Re: [PATCH v1 net-next] af_unix: Try not to hold unix_gc_lock during accept().

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 10 Apr 2024 13:19:29 -0700 Kuniyuki Iwashima wrote:
>  void unix_update_edges(struct unix_sock *receiver)
>  {
> -	spin_lock(&unix_gc_lock);
> -	unix_update_graph(unix_sk(receiver->listener)->vertex);
> +	/* nr_unix_fds is only updated under unix_state_lock().
> +	 * If it's 0 here, the embryo socket is not part of the
> +	 * inflight graph, and GC will not see it.
> +	 */
> +	bool need_lock = !!receiver->scm_stat.nr_unix_fds;
> +
> +	if (need_lock) {
> +		spin_lock(&unix_gc_lock);
> +		unix_update_graph(unix_sk(receiver->listener)->vertex);
> +	}
> +
>  	receiver->listener = NULL;
> -	spin_unlock(&unix_gc_lock);
> +
> +	if (need_lock)
> +		spin_unlock(&unix_gc_lock);
>  }

Are you planning to add more code here? I feel like the sharing of 
a single line is outweighted by the conditionals.. I mean:

	/* ...
	 */
	if (!receiver->scm_stat.nr_unix_fd) {
		receiver->listener = NULL;
	} else {
		spin_lock(&unix_gc_lock);
		unix_update_graph(unix_sk(receiver->listener)->vertex);
		receiver->listener = NULL;
		spin_unlock(&unix_gc_lock);
	}

no?