Return-path: Received: from mail-oa0-f54.google.com ([209.85.219.54]:59214 "EHLO mail-oa0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752362AbaBJWuu (ORCPT ); Mon, 10 Feb 2014 17:50:50 -0500 Received: by mail-oa0-f54.google.com with SMTP id i4so8339791oah.13 for ; Mon, 10 Feb 2014 14:50:50 -0800 (PST) Message-ID: <52F957C6.40407@lwfinger.net> (sfid-20140210_235053_828450_6AF19594) Date: Mon, 10 Feb 2014 16:50:46 -0600 From: Larry Finger MIME-Version: 1.0 To: Stanislaw Gruszka , linux-wireless@vger.kernel.org CC: Herton Ronaldo Krzesinski , Hin-Tak Leung , linux-mips@linux-mips.org Subject: Re: [PATCH] rtl8187: fix regression on MIPS without coherent DMA References: <20140210213828.GA15903@localhost.localdomain> In-Reply-To: <20140210213828.GA15903@localhost.localdomain> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Sender: linux-wireless-owner@vger.kernel.org List-ID: On 02/10/2014 03:38 PM, Stanislaw Gruszka wrote: > This patch fixes regression caused by commit a16dad77634 "MIPS: Fix > potencial corruption". That commit fixes one corruption scenario in > cost of adding another one, which actually start to cause crashes > on Yeeloong laptop when rtl8187 driver is used. > > For correct DMA read operation on machines without DMA coherence, kernel > have to invalidate cache, such it will refill later with new data that > device wrote to memory, when that data is needed to process. We can only > invalidate full cache line. Hence when cache line includes both dma > buffer and some other data (written in cache, but not yet in main > memory), the other data can not hit memory due to invalidation. That > happen on rtl8187 where struct rtl8187_priv fields are located just > before and after small buffers that are passed to USB layer and DMA > is performed on them. > > To fix the problem we align buffers and reserve space after them to make > them match cache line. > > This patch does not resolve all possible MIPS problems entirely, for > that we have to assure that we always map cache aligned buffers for DMA, > what can be complex or even not possible. But patch fixes visible and > reproducible regression and seems other possible corruptions do not > happen in practice, since Yeeloong laptop works stable without rtl8187 > driver. > > Bug report: > https://bugzilla.kernel.org/show_bug.cgi?id=54391 > > Reported-by: Petr Pisar > Bisected-by: Tom Li > Reported-and-tested-by: Tom Li > Cc: stable@vger.kernel.org > Signed-off-by: Stanislaw Gruszka > --- Congratulations to all for sorting this out. It could not have been too easy. The only effect I see on architectures with DMA coherence is that the private data area has grown a little. Certainly, my RTL8187B device still works on x86_64. Acked-by: Larry Finger Larry > drivers/net/wireless/rtl818x/rtl8187/rtl8187.h | 10 ++++++++-- > 1 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h b/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h > index 56aee06..a6ad79f 100644 > --- a/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h > +++ b/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h > @@ -15,6 +15,8 @@ > #ifndef RTL8187_H > #define RTL8187_H > > +#include > + > #include "rtl818x.h" > #include "leds.h" > > @@ -139,7 +141,10 @@ struct rtl8187_priv { > u8 aifsn[4]; > u8 rfkill_mask; > struct { > - __le64 buf; > + union { > + __le64 buf; > + u8 dummy1[L1_CACHE_BYTES]; > + } ____cacheline_aligned; > struct sk_buff_head queue; > } b_tx_status; /* This queue is used by both -b and non-b devices */ > struct mutex io_mutex; > @@ -147,7 +152,8 @@ struct rtl8187_priv { > u8 bits8; > __le16 bits16; > __le32 bits32; > - } *io_dmabuf; > + u8 dummy2[L1_CACHE_BYTES]; > + } *io_dmabuf ____cacheline_aligned; > bool rfkill_off; > u16 seqno; > }; >