Netdev List
 help / color / mirror / Atom feed
From: Shrikanth Hegde <sshegde@linux.ibm.com>
To: Srikar Dronamraju <srikar@linux.ibm.com>,
	LKML <linux-kernel@vger.kernel.org>,
	netdev@vger.kernel.org, David S Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Dust Li <dust.li@linux.alibaba.com>,
	D Wythe <alibuda@linux.alibaba.com>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Jon Maloy <jmaloy@redhat.com>,
	Kuniyuki Iwashima <kuniyu@google.com>,
	linux-sctp@vger.kernel.org,
	Mahanta Jambigi <mjambigi@linux.ibm.com>,
	Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
	Paolo Abeni <pabeni@redhat.com>,
	Sidraya Jayagond <sidraya@linux.ibm.com>,
	Simon Horman <horms@kernel.org>,
	Tony Lu <tonylu@linux.alibaba.com>,
	Wen Gu <guwen@linux.alibaba.com>,
	Wenjia Zhang <wenjia@linux.ibm.com>,
	Willem de Bruijn <willemb@google.com>,
	Xin Long <lucien.xin@gmail.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	K Prateek Nayak <kprateek.nayak@amd.com>
Subject: Re: [PATCH 2/2] net/sock: Propagate WF_SYNC only when requested
Date: Tue, 21 Jul 2026 10:20:03 +0530	[thread overview]
Message-ID: <5bf27f85-6d07-4f40-8604-8b9ac9777db9@linux.ibm.com> (raw)
In-Reply-To: <20260714013940.4068189-6-srikar@linux.ibm.com>

Hi Srikar,

On 7/14/26 7:09 AM, Srikar Dronamraju wrote:
> Use SOCK_SYNC_WAKEUP to select between synchronous and asynchronous wakeup
> wakeup APIs. This avoids propagating WF_SYNC when no blocking waiter is
> expected. All wakeup locations in networking code that currently issue
> synchronous poll-style wakeups unconditionally are updated.
> 

You can also add the performance data in the cover-letter to this patch.

> Signed-off-by: Srikar Dronamraju <srikar@linux.ibm.com>
> ---
>   net/core/sock.c    | 31 ++++++++++++++++++++++++-------
>   net/sctp/socket.c  | 10 ++++++++--
>   net/smc/af_smc.c   |  4 ++--
>   net/smc/smc_rx.c   | 10 ++++++++--
>   net/tipc/socket.c  | 22 +++++++++++++++++-----
>   net/unix/af_unix.c | 26 ++++++++++++++++++--------
>   6 files changed, 77 insertions(+), 26 deletions(-)
> 
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 8a59bfaa8096..a214e883b14b 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -3652,9 +3652,15 @@ void sock_def_readable(struct sock *sk)
>   
>   	rcu_read_lock();
>   	wq = rcu_dereference(sk->sk_wq);
> -	if (skwq_has_sleeper(wq))
> -		wake_up_interruptible_sync_poll(&wq->wait, EPOLLIN | EPOLLPRI |
> +	if (skwq_has_sleeper(wq)) {
> +		if (sock_flag(sk, SOCK_SYNC_WAKEUP)) {
> +			wake_up_interruptible_sync_poll(&wq->wait, EPOLLIN | EPOLLPRI |
> +						EPOLLRDNORM | EPOLLRDBAND);
> +		} else {
> +			wake_up_interruptible_poll(&wq->wait, EPOLLIN | EPOLLPRI |
>   						EPOLLRDNORM | EPOLLRDBAND);
> +		}
> +	}
>   	sk_wake_async_rcu(sk, SOCK_WAKE_WAITD, POLL_IN);
>   	rcu_read_unlock();
>   }
> @@ -3670,9 +3676,15 @@ static void sock_def_write_space(struct sock *sk)
>   	 */
>   	if (sock_writeable(sk)) {
>   		wq = rcu_dereference(sk->sk_wq);
> -		if (skwq_has_sleeper(wq))
> -			wake_up_interruptible_sync_poll(&wq->wait, EPOLLOUT |
> +		if (skwq_has_sleeper(wq)) {
> +			if (sock_flag(sk, SOCK_SYNC_WAKEUP)) {
> +				wake_up_interruptible_sync_poll(&wq->wait, EPOLLOUT |
> +						EPOLLWRNORM | EPOLLWRBAND);
> +			} else {
> +				wake_up_interruptible_poll(&wq->wait, EPOLLOUT |
>   						EPOLLWRNORM | EPOLLWRBAND);
> +			}
> +		}
>   
>   		/* Should agree with poll, otherwise some programs break */
>   		sk_wake_async_rcu(sk, SOCK_WAKE_SPACE, POLL_OUT);
> @@ -3695,10 +3707,15 @@ static void sock_def_write_space_wfree(struct sock *sk, int wmem_alloc)
>   
>   		/* rely on refcount_sub from sock_wfree() */
>   		smp_mb__after_atomic();
> -		if (wq && waitqueue_active(&wq->wait))
> -			wake_up_interruptible_sync_poll(&wq->wait, EPOLLOUT |
> +		if (wq && waitqueue_active(&wq->wait)) {
> +			if (sock_flag(sk, SOCK_SYNC_WAKEUP)) {
> +				wake_up_interruptible_sync_poll(&wq->wait, EPOLLOUT |
>   						EPOLLWRNORM | EPOLLWRBAND);
> -
> +			} else {
> +				wake_up_interruptible_poll(&wq->wait, EPOLLOUT |
> +						EPOLLWRNORM | EPOLLWRBAND);
> +			}
> +		}
>   		/* Should agree with poll, otherwise some programs break */
>   		sk_wake_async_rcu(sk, SOCK_WAKE_SPACE, POLL_OUT);
>   	}
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index c7b9e325ec1c..9cb3432f065a 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -9348,9 +9348,15 @@ void sctp_data_ready(struct sock *sk)
>   
>   	rcu_read_lock();
>   	wq = rcu_dereference(sk->sk_wq);
> -	if (skwq_has_sleeper(wq))
> -		wake_up_interruptible_sync_poll(&wq->wait, EPOLLIN |
> +	if (skwq_has_sleeper(wq)) {
> +		if (sock_flag(sk, SOCK_SYNC_WAKEUP)) {
> +			wake_up_interruptible_sync_poll(&wq->wait, EPOLLIN |
> +					EPOLLRDNORM | EPOLLRDBAND);
> +		} else {
> +			wake_up_interruptible_poll(&wq->wait, EPOLLIN |
>   						EPOLLRDNORM | EPOLLRDBAND);
> +		}
> +	}
>   	sk_wake_async_rcu(sk, SOCK_WAKE_WAITD, POLL_IN);
>   	rcu_read_unlock();
>   }
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index b5db69073e20..1a6ea2e30769 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -819,10 +819,10 @@ static void smc_fback_wakeup_waitqueue(struct smc_sock *smc, void *key)
>   		wake_up_interruptible_all(&wq->wait);
>   	} else {
>   		flags = key_to_poll(key);
> -		if (flags & (EPOLLIN | EPOLLOUT))
> +		if (flags & (EPOLLIN | EPOLLOUT) && sock_flag(&smc->sk, SOCK_SYNC_WAKEUP))
>   			/* sk_data_ready or sk_write_space */
>   			wake_up_interruptible_sync_poll(&wq->wait, flags);
> -		else if (flags & EPOLLERR)
> +		else
>   			/* sk_error_report */
>   			wake_up_interruptible_poll(&wq->wait, flags);
>   	}
> diff --git a/net/smc/smc_rx.c b/net/smc/smc_rx.c
> index c1d9b923938d..4e288a2364d2 100644
> --- a/net/smc/smc_rx.c
> +++ b/net/smc/smc_rx.c
> @@ -39,9 +39,15 @@ static void smc_rx_wake_up(struct sock *sk)
>   	/* called already in smc_listen_work() */
>   	rcu_read_lock();
>   	wq = rcu_dereference(sk->sk_wq);
> -	if (skwq_has_sleeper(wq))
> -		wake_up_interruptible_sync_poll(&wq->wait, EPOLLIN | EPOLLPRI |
> +	if (skwq_has_sleeper(wq)) {
> +		if (sock_flag(sk, SOCK_SYNC_WAKEUP)) {
> +			wake_up_interruptible_sync_poll(&wq->wait, EPOLLIN | EPOLLPRI |
>   						EPOLLRDNORM | EPOLLRDBAND);
> +		} else {
> +			wake_up_interruptible_poll(&wq->wait, EPOLLIN | EPOLLPRI |
> +						EPOLLRDNORM | EPOLLRDBAND);
> +		}
> +	}
>   	sk_wake_async_rcu(sk, SOCK_WAKE_WAITD, POLL_IN);
>   	if ((sk->sk_shutdown == SHUTDOWN_MASK) ||
>   	    (sk->sk_state == SMC_CLOSED))
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index e564341e0216..9fa83a89882c 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -2116,9 +2116,15 @@ static void tipc_write_space(struct sock *sk)
>   
>   	rcu_read_lock();
>   	wq = rcu_dereference(sk->sk_wq);
> -	if (skwq_has_sleeper(wq))
> -		wake_up_interruptible_sync_poll(&wq->wait, EPOLLOUT |
> +	if (skwq_has_sleeper(wq)) {
> +		if (sock_flag(sk, SOCK_SYNC_WAKEUP)) {
> +			wake_up_interruptible_sync_poll(&wq->wait, EPOLLOUT |
>   						EPOLLWRNORM | EPOLLWRBAND);
> +		} else {
> +			wake_up_interruptible_poll(&wq->wait, EPOLLOUT |
> +						EPOLLWRNORM | EPOLLWRBAND);
> +		}
> +	}
>   	rcu_read_unlock();
>   }
>   
> @@ -2134,9 +2140,15 @@ static void tipc_data_ready(struct sock *sk)
>   
>   	rcu_read_lock();
>   	wq = rcu_dereference(sk->sk_wq);
> -	if (skwq_has_sleeper(wq))
> -		wake_up_interruptible_sync_poll(&wq->wait, EPOLLIN |
> -						EPOLLRDNORM | EPOLLRDBAND);
> +	if (skwq_has_sleeper(wq)) {
> +		if (sock_flag(sk, SOCK_SYNC_WAKEUP)) {
> +			wake_up_interruptible_sync_poll(&wq->wait, EPOLLIN |
> +							EPOLLRDNORM | EPOLLRDBAND);
> +		} else {
> +			wake_up_interruptible_poll(&wq->wait, EPOLLIN |
> +							EPOLLRDNORM | EPOLLRDBAND);
> +		}
> +	}
>   	rcu_read_unlock();
>   }
>   
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index f7a9d55eee8a..15ebcc2d9d58 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -601,9 +601,15 @@ static void unix_write_space(struct sock *sk)
>   	rcu_read_lock();
>   	if (unix_writable(sk, READ_ONCE(sk->sk_state))) {
>   		wq = rcu_dereference(sk->sk_wq);
> -		if (skwq_has_sleeper(wq))
> -			wake_up_interruptible_sync_poll(&wq->wait,
> -				EPOLLOUT | EPOLLWRNORM | EPOLLWRBAND);
> +		if (skwq_has_sleeper(wq)) {
> +			if (sock_flag(sk, SOCK_SYNC_WAKEUP)) {
> +				wake_up_interruptible_sync_poll(&wq->wait,
> +					EPOLLOUT | EPOLLWRNORM | EPOLLWRBAND);
> +			} else {
> +				wake_up_interruptible_poll(&wq->wait,
> +					EPOLLOUT | EPOLLWRNORM | EPOLLWRBAND);
> +			}
> +		}
>   		sk_wake_async_rcu(sk, SOCK_WAKE_SPACE, POLL_OUT);
>   	}
>   	rcu_read_unlock();
> @@ -2603,11 +2609,15 @@ int __unix_dgram_recvmsg(struct sock *sk, struct msghdr *msg, size_t size,
>   		goto out;
>   	}
>   
> -	if (wq_has_sleeper(&u->peer_wait))
> -		wake_up_interruptible_sync_poll(&u->peer_wait,
> -						EPOLLOUT | EPOLLWRNORM |
> -						EPOLLWRBAND);
> -
> +	if (wq_has_sleeper(&u->peer_wait)) {
> +		if (sock_flag(sk, SOCK_SYNC_WAKEUP)) {
> +			wake_up_interruptible_sync_poll(&u->peer_wait,
> +						EPOLLOUT | EPOLLWRNORM | EPOLLWRBAND);
> +		} else {
> +			wake_up_interruptible_poll(&u->peer_wait,
> +						EPOLLOUT | EPOLLWRNORM | EPOLLWRBAND);
> +		}
> +	}
>   	if (msg->msg_name) {
>   		unix_copy_addr(msg, skb->sk);
>   


Would it make sense to write a macro or a wrapper function do the
same instead of sprinkling the same at all the places?

similar comment for patch 1.

IMHO, it would make it easier to read.


      reply	other threads:[~2026-07-21  4:50 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-14  1:39 [PATCH 0/2] net: Use synchronous wakeups selectively Srikar Dronamraju
2026-07-14  1:39 ` [PATCH 1/2] net/socket: Record preference for synchronous wakeups Srikar Dronamraju
2026-07-21  5:00   ` Shrikanth Hegde
2026-07-21  8:00   ` Willem de Bruijn
2026-07-14  1:39 ` [PATCH 2/2] net/sock: Propagate WF_SYNC only when requested Srikar Dronamraju
2026-07-21  4:50   ` Shrikanth Hegde [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5bf27f85-6d07-4f40-8604-8b9ac9777db9@linux.ibm.com \
    --to=sshegde@linux.ibm.com \
    --cc=alibuda@linux.alibaba.com \
    --cc=bsegall@google.com \
    --cc=davem@davemloft.net \
    --cc=dietmar.eggemann@arm.com \
    --cc=dust.li@linux.alibaba.com \
    --cc=edumazet@google.com \
    --cc=guwen@linux.alibaba.com \
    --cc=horms@kernel.org \
    --cc=jmaloy@redhat.com \
    --cc=kprateek.nayak@amd.com \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sctp@vger.kernel.org \
    --cc=lucien.xin@gmail.com \
    --cc=marcelo.leitner@gmail.com \
    --cc=mgorman@suse.de \
    --cc=mingo@kernel.org \
    --cc=mjambigi@linux.ibm.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=sidraya@linux.ibm.com \
    --cc=srikar@linux.ibm.com \
    --cc=tonylu@linux.alibaba.com \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=wenjia@linux.ibm.com \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox