Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752043AbdLEAXh (ORCPT ); Mon, 4 Dec 2017 19:23:37 -0500 Received: from mail-pg0-f65.google.com ([74.125.83.65]:46142 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750995AbdLEAXd (ORCPT ); Mon, 4 Dec 2017 19:23:33 -0500 X-Google-Smtp-Source: AGs4zMaOHSRemKFxn2UWvCG90uDnvX+zbeaBWD52N2NVuLT9jGTYrV88F/cgEEP61AOBED/+5SN7Vg== 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: <06a8b0a9-c8fc-ee9b-6f82-09a594892f28@gmail.com> Date: Tue, 5 Dec 2017 08:24:46 +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: 2848 Lines: 82 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, 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; En, seems more reasonable and clear, feel free to merge it if convenience, thanks :-) > > Do you have any performance numbers for this change? No performance testing since just padding issue but with the clear sky of /proc/cpu/alignment in arm side. Cheers, Zumeng > > Thanks, > Claudiu