Messages in this thread | | | Date | Tue, 22 Aug 2023 18:37:14 +0200 | Subject | Re: can: isotp: epoll breaks isotp_sendmsg | From | Oliver Hartkopp <> |
| |
Hi Lukas,
On 22.08.23 08:51, Lukas Magel wrote:
>>> @Oliver I adjusted the exit path for the case where the initial wait is >>> interrupted to return immediately instead of jumping to err_event_drop. >>> Could you please check if you would agree with this change? >> The code has really won with your change! Thanks! >> >> But as you already assumed I have a problem with the handling of the >> cleanup when a signal interrupts the wait_event_interruptible() statement. >> >> I think it should still be: >> >> /* wait for complete transmission of current pdu */ >> err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE); >> if (err) >> goto err_event_drop; >> >> as we need to make sure that the state machine is set to defined values >> and states for the next isotp_sendmsg() attempt. >> >> Best regards, >> Oliver > > > Thank you for the feedback! Can you elaborate why the state needs to be > reset here? For me, the loop is basically a "let's wait until we win > arbitration for the tx.state", which means that the task is allowed > to send. I'm imagining an application that has two threads, both sending > at the same time (because maybe they don't care about reading). So one > would always be waiting in the loop until the send operation of the other > has concluded. My motivation for not going to err_event_drop was that if > one thread was interrupted in its wait_event_interruptible, why would we > need to change tx.state that is currently being occupied by the other > thread? The thread waiting in the loop has not done any state manipulation > of the socket.
Please don't only look at the isotp_sendmsg() function but the other possibilities e.g. from timeouts.
Look for the documentation from the commit 051737439eaee. This patch has been added recently as it was needed.
Best regards, Oliver
| |