On 8/1/24 10:36 AM, Willem de Bruijn wrote:
On Thu, Aug 1, 2024 at 1:30 PM Zijian Zhang zijianzhang@bytedance.com wrote:
-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.
- 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.
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