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, OliverThank 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
