Re: [PATCH] firewire: fix panic in handle_at_packet

[Stefan Richter]

Jarod Wilson wrote:
> Panics hooking up to an x86 mac mini in target disk mode are gone on my end 
> with this patch added, and the fix makes sense -- assuming I've got it right, 
> in my head, of course. ;)
>
> As I understand it, we'll now simply bail in handle_at_packet when we see 
> packet == NULL, rather than trying to play with already freed memory, and 
> cancelling an AT packet here should always be perfectly safe, because we're 
> already onto the AR side of this transaction, and in most cases, the AT 
> handler already fired anyway.

To expand on this a little bit:

An outbound transaction goes like this (possible retries and failures 
neglected):
  - fw-core submits a request packet to fw-ohci.
  - fw-ohci creates the DMA program and hands it over to the controller.
  - Controller sends the request packet to the remote node.
  - Remote node sends an ACK.
  - Controller generates an interrupt.
  - fw-ohci schedules the AT handler tasklet.
  - If this was a so-called unified transaction, the transaction is now
    complete.  The AT handler tasklet will eventually call into fw-core
    to let it finish the transaction.
  - If this was a so-called split transaction, the remote node will send
    a response packet.
  - Controller ACKs it and generates an interrupt.
  - fw-ohci schedules the AR handler tasklet.
  - The previously scheduled AT handler tasklet, *if* it is actually
    executed for the sent and ACKed request packet *before* the AR
    handler tasklet for the received response packet, will call into
    fw-core which simply sets a timestamp for the request packet.  (This
    timestamp should be used as the basis for split transaction timeout,
    but fw-core doesn't really look at that timestamp yet.  We discussed
    this recently.)
  - The scheduled AR handler calls into fw-core which completes the
    transaction.

We schedule the request AT handler tasklet before the response AR 
handler tasklet, but under some circumstances the kernel may decide to 
defer execution of the former tasklet but to immediately execute the 
latter tasklet.  That's why they can be reordered.

This reordering is, from fw-core's point of view, an implementation 
detail of fw-ohci and fw-core shouldn't be bothered with this.  However, 
fw-ohci is unable to figure out which responses belong to which 
requests.  (Matching requests and respnses is exclusively fw-core's 
transaction layer's job.)  Hence fw-ohci needs to be told by the 
transaction layer about which requests don't need any handling anymore 
because their transaction was already completed.

And yes, the call to cancel_packet is not only necessary, it is also safe:
  - We call it only for request packets whose response we already
    received.
  - There is nothing useful to be done by the AT handler for these
    request packets anymore at this point.
--
Stefan Richter
-=====-==--- --== =--==
http://arcgraph.de/sr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/