Re: [PATCH V2 net-next 2/7] rds: hold a sock ref from rds_message to the rds_sock

[Santosh Shilimkar <santosh.shilimkar@oracle.com>]

On 2/14/2018 2:28 AM, Sowmini Varadhan wrote:
> The existing model holds a reference from the rds_sock to the
> rds_message, but the rds_message does not itself hold a sock_put()
> on the rds_sock. Instead the m_rs field in the rds_message is
> assigned when the message is queued on the sock, and nulled when
> the message is dequeued from the sock.
> 
> We want to be able to notify userspace when the rds_message
> is actually freed (from rds_message_purge(), after the refcounts
> to the rds_message go to 0). At the time that rds_message_purge()
> is called, the message is no longer on the rds_sock retransmit
> queue. Thus the explicit reference for the m_rs is needed to
> send a notification that will signal to userspace that
> it is now safe to free/reuse any pages that may have
> been pinned down for zerocopy.
> 
> This patch manages the m_rs assignment in the rds_message with
> the necessary refcount book-keeping.
> 
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---

[...]


> @@ -756,9 +755,6 @@ void rds_send_drop_to(struct rds_sock *rs, struct sockaddr_in *dest)
>   		 */
>   		if (!test_and_clear_bit(RDS_MSG_ON_CONN, &rm->m_flags)) {
>   			spin_unlock_irqrestore(&cp->cp_lock, flags);
> -			spin_lock_irqsave(&rm->m_rs_lock, flags);
> -			rm->m_rs = NULL;
> -			spin_unlock_irqrestore(&rm->m_rs_lock, flags);
>   			continue;
>   		}
>   		list_del_init(&rm->m_conn_item);
This hunk was clearly wrong so good that you got rid of it as well.
Patch looks fine to me.

Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>