Re: [PATCH V2 net-next 5/7] rds: zerocopy Tx support.

On 2/14/2018 11:49 AM, Sowmini Varadhan wrote:

On (02/14/18 11:10), Santosh Shilimkar wrote:

quoted

s/RDS_CMSG_ZCOPY_COOKIE/RDS_CMSG_ZMSGCOPY_COOKIE	

Please see https://www.spinics.net/lists/netdev/msg483627.html

Just saw it and responded to Dave.

quoted

quoted

@@ -356,6 +358,53 @@ int rds_message_copy_from_user(struct rds_message *rm, struct iov_iter *from)
  	sg = rm->data.op_sg;
  	sg_off = 0; /* Dear gcc, sg->page will be null from kzalloc. */
+	if (zcopy) {
+		int total_copied = 0;
+		struct sk_buff *skb;
+
+		skb = alloc_skb(SO_EE_ORIGIN_MAX_ZCOOKIES * sizeof(u32),
+				GFP_KERNEL);

This can sleep so you might want to check if you want to use ATOMIC version
here.

I think it should be fine: rds_message_copy_from_user() is called
in process context, and if you notice, the calling function rds_sendmsg()
also has this
    1100         rm = rds_message_alloc(ret, GFP_KERNEL);
    1101         if (!rm) {
    1102                 ret = -ENOMEM;
    1103                 goto out;
    1104         }

     :
    1106         /* Attach data to the rm */
     :
    1113                 ret = rds_message_copy_from_user(rm, &msg->msg_iter);

So using GFP_KERNEL is as safe as the call at line 1100.

Was just asking you to check if it is safe. The path already
does that so we are good.

quoted

quoted

+			return -ENOMEM;
+		}

NOMEM new application visible change but probably the right one for this
particular case. Just need to make sure application can handle this
error.

I think the application already handles this correctly (see line 1102 above)

Indeed. Thanks for checking.

Regards,
Santosh