[rds-devel] [External] : Re: [PATCH net 1/1] net/rds: handle zerocopy send cleanup before the message is queued

[Allison Henderson]

On Fri, 2026-05-01 at 09:08 +0800, Ren Wei wrote:
> From: Nan Li <tonanli66 at gmail.com>
> 
> A zerocopy send can fail after user pages have been pinned but before
> the message is attached to the sending socket.
> 
> The purge path currently infers zerocopy state from rm->m_rs, so an
> unqueued message can be cleaned up as if it owned normal payload pages.
> However, zerocopy ownership is really determined by the presence of
> op_mmp_znotifier, regardless of whether the message has reached the
> socket queue.
> 
> Capture op_mmp_znotifier up front in rds_message_purge() and use it as
> the cleanup discriminator. If the message is already associated with a
> socket, keep the existing completion path. Otherwise, drop the pinned
> page accounting directly and release the notifier before putting the
> payload pages.
> 
> This keeps early send failure cleanup consistent with the zerocopy
> lifetime rules without changing the normal queued completion path.
> 
> Fixes: 0cebaccef3ac ("rds: zerocopy Tx support.")
> Cc: stable at kernel.org
> Reported-by: Yuan Tan <yuantan098 at gmail.com>
> Reported-by: Yifan Wu <yifanwucs at gmail.com>
> Reported-by: Juefei Pu <tomapufckgml at gmail.com>
> Reported-by: Xin Liu <bird at lzu.edu.cn>
> Co-developed-by: Xiao Liu <lx24 at stu.ynu.edu.cn>
> Signed-off-by: Xiao Liu <lx24 at stu.ynu.edu.cn>
> Signed-off-by: Nan Li <tonanli66 at gmail.com>
> Signed-off-by: Ren Wei <n05ec at lzu.edu.cn>

This fix looks fine to me.  Thanks Ren Wei!
Reviewed-by: Allison Henderson <achender at kernel.org>

Allison

> ---
>  net/rds/message.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/net/rds/message.c b/net/rds/message.c
> index eaa6f22601a4..25fedcb3cd00 100644
> --- a/net/rds/message.c
> +++ b/net/rds/message.c
> @@ -131,24 +131,34 @@ static void rds_rm_zerocopy_callback(struct rds_sock *rs,
>   */
>  static void rds_message_purge(struct rds_message *rm)
>  {
> +	struct rds_znotifier *znotifier;
>  	unsigned long i, flags;
> -	bool zcopy = false;
> +	bool zcopy;
>  
>  	if (unlikely(test_bit(RDS_MSG_PAGEVEC, &rm->m_flags)))
>  		return;
>  
>  	spin_lock_irqsave(&rm->m_rs_lock, flags);
> +	znotifier = rm->data.op_mmp_znotifier;
> +	rm->data.op_mmp_znotifier = NULL;
> +	zcopy = !!znotifier;
> +
>  	if (rm->m_rs) {
>  		struct rds_sock *rs = rm->m_rs;
>  
> -		if (rm->data.op_mmp_znotifier) {
> -			zcopy = true;
> -			rds_rm_zerocopy_callback(rs, rm->data.op_mmp_znotifier);
> +		if (znotifier) {
> +			rds_rm_zerocopy_callback(rs, znotifier);
>  			rds_wake_sk_sleep(rs);
> -			rm->data.op_mmp_znotifier = NULL;
>  		}
>  		sock_put(rds_rs_to_sk(rs));
>  		rm->m_rs = NULL;
> +	} else if (znotifier) {
> +		/*
> +		 * Zerocopy can fail before the message is queued on the
> +		 * socket, so there is no rs to carry the notification.
> +		 */
> +		mm_unaccount_pinned_pages(&znotifier->z_mmp);
> +		kfree(rds_info_from_znotifier(znotifier));
>  	}
>  	spin_unlock_irqrestore(&rm->m_rs_lock, flags);
>