Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vdev: log waiting of the first vhost-user request #15

Merged
merged 1 commit into from
Feb 4, 2025
+14 −6
Merged

vdev: log waiting of the first vhost-user request #15

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions vdev.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,9 @@ static void arm_msg_handling_timer(struct vhd_vdev *vdev, int secs)
static void vdev_handle_start(struct vhd_vdev *vdev, uint32_t req,
bool ack_pending)
{
/* detach timer_handler attached right after accept, no-op after the first request */
vhd_detach_io_handler(vdev->timer_handler);
Copy link
Contributor Author

@zeil zeil Feb 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Так было сделать проще всего.
Тут остается вот такой редкий кейс: мы задетачили дескриптор из epoll, но таймер все еще возведен и не зафайрился. У нас есть промежуток времени на строках 609-610, когда мы сначала лобавляем timerfd обратно в epoll, а потом переустанавливаем таймер. Теоретически может случиться так, что в этот промежуток времени напишется сообщение Still waiting...
Я думаю, что это почти невозможный кейс и ради него усложнять патч не стоит.
Тем не менее есть такие варианты:

  • поменять местами строки 609-610, т.е. сначала делать arm_msg_handling_timer, а затем vhd_attach_io_handler. Может быть, это и логично. Как раз будет обратный порядок тому, что в vdev_handle_finish. Врядли epoll_ctl сильно много времени займет в процессе хэндлинга.
  • сделать в vhd_detach_io_handler возвращаемое значение, которые бы показывало, был ли реально произведен детач или нет, и по нему сделать if тут

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Звучит так что поменять порядок, в общем-то, разумно. Давай наверное так и сделаем тут


/* do not accept further messages until this one is fully handled */
vhd_detach_io_handler(vdev->conn_handler);

Expand All @@ -603,8 +606,8 @@ static void vdev_handle_start(struct vhd_vdev *vdev, uint32_t req,

VHD_OBJ_DEBUG(vdev, "%s (%u)", vhost_req_name(req), req);

vhd_attach_io_handler(vdev->timer_handler);
arm_msg_handling_timer(vdev, MSG_HANDLING_LOG_INTERVAL);
vhd_attach_io_handler(vdev->timer_handler);
}

static void vdev_handle_finish(struct vhd_vdev *vdev)
Expand Down Expand Up @@ -1978,9 +1981,13 @@ static int timer_read(void *opaque)

elapsed_time(vdev, &elapsed);

VHD_OBJ_WARN(vdev, "long processing %s (%u): elapsed %jd.%03lds",
vhost_req_name(vdev->req), vdev->req,
(intmax_t)elapsed.tv_sec, elapsed.tv_nsec / NSEC_PER_MSEC);
if (likely(vdev->req != VHOST_USER_NONE)) {
VHD_OBJ_WARN(vdev, "long processing %s (%u): elapsed %jd.%03lds",
vhost_req_name(vdev->req), vdev->req,
(intmax_t)elapsed.tv_sec, elapsed.tv_nsec / NSEC_PER_MSEC);
} else {
VHD_OBJ_WARN(vdev, "Still waiting for a vhost-user request...");
}

return 0;
}
Expand Down Expand Up @@ -2018,8 +2025,6 @@ static int server_read(void *opaque)
if (!timer_handler) {
goto close_timer;
}
/* it only needs to be attached during message handling */
vhd_detach_io_handler(timer_handler);

vhd_detach_io_handler(vdev->listen_handler);

Expand All @@ -2030,6 +2035,9 @@ static int server_read(void *opaque)
vdev->negotiated_features = 0;
vdev->negotiated_protocol_features = 0;
VHD_OBJ_INFO(vdev, "Connection established, sock = %d", connfd);

arm_msg_handling_timer(vdev, MSG_HANDLING_LOG_INTERVAL);

return 0;

close_timer:
Expand Down
Loading