Re: can: isotp: epoll breaks isotp_sendmsg
From: Hillf Danton <hidden>
Date: 2023-07-18 10:10:14
On Fri, Jun 30 2023, Maxime Jayat wrote:
Hi,
There is something not clear happening with the non-blocking behavior
of ISO-TP sockets in the TX path, but more importantly, using epoll now
completely breaks isotp_sendmsg.
I believe it is related to
79e19fa79c ("can: isotp: isotp_ops: fix poll() to not report false
EPOLLOUT events"),
but actually is probably deeper than that.
I don't completely understand what is exactly going on, so I am sharing
the problem I face:
With an ISO-TP socket in non-blocking mode, using epoll seems to make
isotp_sendmsg always return -EAGAIN.Problem 1.
By reverting 79e19fa79c, I get better results but still incorrect:
[...]
It is then possible to write on the socket but the write is blocking, which is not the expected behavior for a non-blocking socket.
Problem 2. My two cents with the two problems addressed.
--- x/net/can/isotp.c
+++ y/net/can/isotp.c@@ -954,21 +954,18 @@ static int isotp_sendmsg(struct socket * if (!so->bound || so->tx.state == ISOTP_SHUTDOWN) return -EADDRNOTAVAIL; -wait_free_buffer: /* we do not support multiple buffers - for now */ - if (wq_has_sleeper(&so->wait) && (msg->msg_flags & MSG_DONTWAIT)) - return -EAGAIN; - - /* wait for complete transmission of current pdu */ - err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE); - if (err) - goto err_event_drop; - - if (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE) { + while (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE) { if (so->tx.state == ISOTP_SHUTDOWN) return -EADDRNOTAVAIL; - goto wait_free_buffer; + if (msg->msg_flags & MSG_DONTWAIT) + return -EAGAIN; + + /* wait for complete transmission of current pdu */ + err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE); + if (err) + return err; } /* PDU size > default => try max_pdu_size */ --