On 10/4/19 4:07 PM, Anssi Hannula wrote:
> On 4.10.2019 16.54, Marc Kleine-Budde wrote:
>> On 10/4/19 11:02 AM, Anssi Hannula wrote:
>>> Userspace can signal with CAN_CTRLMODE_BERR_REPORTING whether they need
>>> reporting of bus errors (CAN_ERR_BUSERROR) or not.
>>>
>>> However, xilinx_can driver currently always sends CAN_ERR_BUSERROR
>>> frames to userspace on bus errors.
>>>
>>> To improve performance on error conditions when bus error reporting is
>>> not needed, avoid sending CAN_ERR_BUSERROR frames unless requested via
>>> CAN_CTRLMODE_BERR_REPORTING.
>>>
>>> The error interrupt is still kept enabled as there is no dedicated state
>>> transition interrupt, but just disabling error frame submission still
>>> yields a significant performance improvement. In a simple test with
>>> continuous bus errors and no userspace programs reading/writing CAN I
>>> saw system CPU load reduced by 1/3.
>>>
>>> Tested on a ZynqMP board with CAN-FD v1.0.
>>>
>>> Signed-off-by: Anssi Hannula <anssi.hannula@xxxxxxxxxx>
>>> ---
>>> drivers/net/can/xilinx_can.c | 84 +++++++++++++++++++-----------------
>>> 1 file changed, 45 insertions(+), 39 deletions(-)
>>>
>>> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
>>> index 911b34316c9d..9b9ec07f7e5b 100644
>>> --- a/drivers/net/can/xilinx_can.c
>>> +++ b/drivers/net/can/xilinx_can.c
>>> @@ -471,6 +471,10 @@ static int xcan_chip_start(struct net_device *ndev)
>>> return err;
>>>
>>> /* Enable interrupts */
>>> + /* We enable the ERROR interrupt even with CAN_CTRLMODE_BERR_REPORTING
>>> + * disabled as there is no dedicated interrupt for a state change to
>>> + * ERROR_WARNING/ERROR_PASSIVE.
>>> + */
>>> ier = XCAN_IXR_TXOK_MASK | XCAN_IXR_BSOFF_MASK |
>>> XCAN_IXR_WKUP_MASK | XCAN_IXR_SLP_MASK |
>>> XCAN_IXR_ERROR_MASK | XCAN_IXR_RXOFLW_MASK |
>>> @@ -981,11 +985,10 @@ static void xcan_err_interrupt(struct net_device *ndev, u32 isr)
>>> {
>>> struct xcan_priv *priv = netdev_priv(ndev);
>>> struct net_device_stats *stats = &ndev->stats;
>>> - struct can_frame *cf;
>>> - struct sk_buff *skb;
>>> + struct can_frame cf;
>>> u32 err_status;
>>>
>>> - skb = alloc_can_err_skb(ndev, &cf);
>>> + memset(&cf, 0, sizeof(cf));
>> This change is unrelated to the one described. Please move these into a
>> seperate patch.
>>
>
> It is related - the SKB allocation has to be delayed as we no longer
> know at this point whether the SKB is needed or not.
> So a temporary can_frame is used instead.
Right, I understand now :)
> Or should I still move it to a separate patch along lines of "delay CAN
> error frame SKB allocation to accommodate a following commit where we do
> not want to always allocate it"?
> I'd side on keeping it together, but your call :)
regards,
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:
signature.asc
Description: OpenPGP digital signature
