Page MenuHomeFreeBSD

Support MSG_DONTWAIT in sendmsg()
ClosedPublic

Authored by greg_unrelenting.technology on Jan 3 2019, 4:10 PM.

Details

Summary

Sometimes Weston (or any Wayland compositor really) would become unresponsive (whole UI frozen, mouse doesn't move) when a client (desktop app) became unresponsive. After some testing with procstat, the cause was found: sendmsg. Turns out libwayland-server was telling the kernel to not block on sendmsg, but the kernel wasn't listening.

https://gitlab.freedesktop.org/wayland/wayland/blob/921d0548035673a1bf6aeb9396b9bc728133411e/src/connection.c#L314-317

sendmsg(connection->fd, &msg, MSG_NOSIGNAL | MSG_DONTWAIT);

As a workaround MSG_NBIO can be used, but according to the comment it's intended for some fifo stuff. Let's support the correct constant that other operating systems support, e.g. OpenBSD does support it.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

zeising added a subscriber: zeising.Jan 3 2019, 4:36 PM
markj accepted this revision.Jan 3 2019, 4:44 PM
markj added subscribers: tuexen, markj.

This seems fine to me. sdp_sosend() needs the same treatment. sctp_sosend() probably does as well, but I don't see where. @tuexen might know.

sys/kern/uipc_socket.c
1525 ↗(On Diff #52506)

You can test both flags at once: (flags & (MSG_DONTWAIT | MSG_NBIO)) != 0

This revision is now accepted and ready to land.Jan 3 2019, 4:44 PM
bcr accepted this revision.Jan 3 2019, 5:16 PM
bcr added a subscriber: bcr.

OK from manpages, too.

sys/kern/uipc_socket.c
1525 ↗(On Diff #52506)

oh, right, with !=0 it won't be a check for both, thanks for the tip

I think that's the right place in sctp. I'm not sure how to test it though, or the sdp thing…

This revision now requires review to proceed.Jan 3 2019, 6:07 PM
greg_unrelenting.technology marked an inline comment as done.Jan 3 2019, 6:08 PM
markj added a comment.Jan 3 2019, 6:19 PM

I think that's the right place in sctp. I'm not sure how to test it though, or the sdp thing…

Thanks. Let's give @tuexen a day or two to weigh in on the SCTP side. sdp_sosend() is quite similar to sosend_generic(); I think it's fine to skip testing there.

sys/kern/uipc_socket.c
1525 ↗(On Diff #52506)

The parens around the second condition are redundant, and the line is longer than 80 columns. I'd write

if ((so->so_state & SS_NBIO) != 0 ||
    (flags & (MSG_DONTWAIT | MSG_NBIO)) != 0) {
...
sys/netinet/sctp_output.c
12839 ↗(On Diff #52513)

Redundant parens.

sys/ofed/drivers/infiniband/ulp/sdp/sdp_main.c
1127 ↗(On Diff #52513)

Ditto.

greg_unrelenting.technology marked 3 inline comments as done.

Updated style.

BTW, would be nice to see this MFC'd into 12

tuexen accepted this revision.Jan 3 2019, 11:48 PM
This revision is now accepted and ready to land.Jan 3 2019, 11:48 PM
Closed by commit rS342768: Support MSG_DONTWAIT in send*(2). (authored by markj, committed by ). · Explain WhyJan 4 2019, 5:32 PM
This revision was automatically updated to reflect the committed changes.