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

[Sowmini Varadhan]

On (02/14/18 11:10), Santosh Shilimkar wrote:
> s/RDS_CMSG_ZCOPY_COOKIE/RDS_CMSG_ZMSGCOPY_COOKIE	
> 

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

> >@@ -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.


> >+			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)

Thanks for taking a look.

--Sowmini