On 03/02/2016 11:08 AM, Ramesh Shanmugasundaram wrote:

quoted

quoted

quoted

I see no locking for the tx-path.

I am not sure why locking is needed in tx-path?

If the tx-path is shared between both channels you need locking as the
networking subsystem may send one frame to each controller at the same
time.

Yes. Tx completion interrupt is shared by both channels but the Tx
FIFO, echo skbs, head, tail are maintained on a per channel basis. 
Yes, net subsystem can send one frame to each channel at the same
time and the tx completion interrupt handler will traverse each
channel & update per channel context accordingly.

Ok. Which instruction starts the transmit? Please add a comment to the
code. You stop the tx_queue after this, so your code is racy, as the
interrupt might happen in between.

quoted

quoted

However, looking at it again, I should move the incrementing of head
after the "sts" handing to be apt. What do you think?

With one producer (one SW instance) and one consumer (the HW) you can
write lockless code (if the HW allows it), but with two producers it's not
possible.

For a channel represented as netdev, there is still one producer (one
SW instance) isn't it? net acquires lock before calling xmit. The
consumer is tx completion interrupt handler which updates per channel
attributes only.

Ok - I haven't looked at the code in depth yet. Now it's clear, that
just the tx interrupt is shared.

Sorry if I am annoying. I have tested this with two channels
transmitting & receiving at the same time and it works fine. If I am
missing lock, I would like to understand it thoroughly before making
the change.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |