Re: [PATCH] can: rcar_canfd: Add Renesas R-Car CAN FD driver

Marc Kleine-Budde
2016-03-02T10:21:32Z

[-- Attachment #1: Type: text/plain, Size: 1993 bytes --]

On 03/02/2016 11:08 AM, Ramesh Shanmugasundaram wrote:
>>>> 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.

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]