On 8/1/24 10:36 AM, Willem de Bruijn wrote:

On Thu, Aug 1, 2024 at 1:30 PM Zijian Zhang [off-list ref] wrote:

quoted

quoted

quoted

-static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
+static void add_zcopy_info(struct msghdr *msg)
+{
+    struct zc_info *zc_info;
+    struct cmsghdr *cm;
+
+    if (!msg->msg_control)
+            error(1, errno, "NULL user arg");

Don't add precondition checks for code entirely under your control.
This is not a user API.

Ack.

quoted

quoted

+    cm = (struct cmsghdr *)msg->msg_control;
+    cm->cmsg_len = CMSG_LEN(ZC_INFO_SIZE);
+    cm->cmsg_level = SOL_SOCKET;
+    cm->cmsg_type = SCM_ZC_NOTIFICATION;
+
+    zc_info = (struct zc_info *)CMSG_DATA(cm);
+    zc_info->size = ZC_NOTIFICATION_MAX;
+
+    added_zcopy_info = true;

Just initialize every time? Is this here to reuse the same msg_control
as long as metadata is returned?

Yes, the same msg_control will be reused.

The overall paradiagm is,
start:
    sendmsg(..)
    sendmsg(..)
    ...          sends_since_notify sendmsgs in total

    add_zcopy_info(..)
    sendmsg(.., msg_control)
    do_recv_completions_sendmsg(..)
    goto start;

if (sends_since_notify + 1 >= cfg_notification_limit), add_zcopy_info
will be invoked, and the right next sendmsg will have the msg_control
passed in.

If (added_zcopy_info), do_recv_completions_sendmsg will be invoked,
and added_zcopy_info will be set to false in it.

This does not seem like it would need a global variable?

Agreed, maybe I can use sends_since_notify to check whether we
need to do_recv_completions_sendmsg, then we get rid of
added_zcopy_info.

quoted

Btw, before I put some efforts to solve the current issues, I think
I should wait for comments about api change from linux-api@vger.kernel.org?

I'm not sure whether anyone on that list will give feedback.

I would continue with revisions at a normal schedule, as long as that
stays in the Cc.

Got it, thanks