Messages in this thread | | | From | Michal Sojka <> | Subject | Re: can: isotp: epoll breaks isotp_sendmsg | Date | Sat, 01 Jul 2023 00:35:18 +0200 |
| |
Hi Maxime,
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.
That's definitely not expected behavior. I tested the patch only with poll, hoping that epoll would behave the same.
[...]
> > 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.
Yes, incorrect behavior was why we made the commit in question, however we saw write() returning -EAGAIN when it shouldn't.
> I don't know how to solve the problem. To me, using wq_has_sleeper seems > weird.
Agreed. I've never tried to understand how synchronization works here. Hopefully, Oliver knows more.
> The implementation of isotp_poll feels weird too (calling both > datagram_poll and > poll_wait?). But I am not sure what would be the correct > implementation.
I understand it as follows (which might be wrong - someone, please correct me), isotp_poll() should register the file with all waitqueues it can wait on. so->wait is one and sock->sq.wait (used by datagram_poll) is another. The former is definitely used for TX, the latter is probably used because skb_recv_datagram() is called for RX. But so->wait is also used for RX and there might proabbly be be some inconsistency between those.
> My actual use-case is in Async Rust using tokio.
Our initial motivation was also Rust and tokio however than I did testing only with simple C programs. I'm definitely interested in having this working.
I'll try to look at this in more detail during the weekend. It's too late for me today.
Best regards, -Michal
| |