Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752218AbdLECRb (ORCPT ); Mon, 4 Dec 2017 21:17:31 -0500 Received: from mail-pg0-f68.google.com ([74.125.83.68]:36164 "EHLO mail-pg0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750995AbdLECR3 (ORCPT ); Mon, 4 Dec 2017 21:17:29 -0500 X-Google-Smtp-Source: AGs4zManY9gGG4MAeva6Fs90XYjnFU4tr2VPVMPUnz49yrIM8WnzdAumsMV2ncmcR/79N/R81jDRRw== Subject: Re: [PATCH 1/1] gianfar: fix a flooded alignment reports because of padding issue. To: Claudiu Manoil , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" References: <1512357722-12124-1-git-send-email-zumeng.chen@gmail.com> Cc: "davem@davemloft.net" From: Zumeng Chen Message-ID: <46d11607-d9bc-d24f-5c1d-62eb74c721c2@gmail.com> Date: Tue, 5 Dec 2017 10:18:42 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2997 Lines: 90 On 12/05/2017 12:06 AM, Claudiu Manoil wrote: >> -----Original Message----- >> From: Zumeng Chen [mailto:zumeng.chen@gmail.com] >> Sent: Monday, December 04, 2017 5:22 AM >> To: netdev@vger.kernel.org; linux-kernel@vger.kernel.org >> Cc: Claudiu Manoil ; davem@davemloft.net >> Subject: [PATCH 1/1] gianfar: fix a flooded alignment reports because of padding >> issue. >> >> According to LS1021A RM, the value of PAL can be set so that the start of the >> IP header in the receive data buffer is aligned to a 32-bit boundary. Normally, >> setting PAL = 2 provides minimal padding to ensure such alignment of the IP >> header. >> >> However every incoming packet's 8-byte time stamp will be inserted into the >> packet data buffer as padding alignment bytes when hardware time stamping is >> enabled. >> >> So we set the padding 8+2 here to avoid the flooded alignment faults: >> >> root@128:~# cat /proc/cpu/alignment >> User: 0 >> System: 17539 (inet_gro_receive+0x114/0x2c0) >> Skipped: 0 >> Half: 0 >> Word: 0 >> DWord: 0 >> Multi: 17539 >> User faults: 2 (fixup) >> > [...] >> drivers/net/ethernet/freescale/gianfar.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/freescale/gianfar.c >> b/drivers/net/ethernet/freescale/gianfar.c >> index e616b71..e47945f 100644 >> --- a/drivers/net/ethernet/freescale/gianfar.c >> +++ b/drivers/net/ethernet/freescale/gianfar.c >> @@ -1413,9 +1413,11 @@ static int gfar_probe(struct platform_device *ofdev) >> >> gfar_init_addr_hash_table(priv); >> >> - /* Insert receive time stamps into padding alignment bytes */ >> + /* Insert receive time stamps into padding alignment bytes, and >> + * plus 2 bytes padding to ensure the cpu alignment. >> + */ >> if (priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER) >> - priv->padding = 8; >> + priv->padding = 8 + DEFAULT_PADDING; >> >> if (dev->features & NETIF_F_IP_CSUM || >> priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER) >> -- >> 2.5.0 > Why handle only the rx timestamp path (HAS_TIMER) when the issue, Sorry, missed this one. Because the mis-alignment is only been seen in gfar_clean_rx_ring from padding issue so far. > as presented > by you, should be applicable to the default path as well? > > The code change according to the patch description should be more likely: > > + priv->padding = DEFAULT_PADDING; > /* Insert receive time stamps into padding alignment bytes */ > if (priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER) > - priv->padding = 8; > + priv->padding += 8; I just did a sanity testing on your change, it's OK. root@1021:~# cat /proc/cpu/alignment User: 0 System: 0 ( (null)) Skipped: 0 Half: 0 Word: 0 DWord: 0 Multi: 0 User faults: 2 (fixup) > > > Do you have any performance numbers for this change? > > Thanks, > Claudiu