Re: [PATCH] rtl8187: fix regression on MIPS without coherent DMA

* Re: [PATCH] rtl8187: fix regression on MIPS without coherent DMA
@ 2014-02-10 23:13 Hin-Tak Leung
  0 siblings, 0 replies; 4+ messages in thread
From: Hin-Tak Leung @ 2014-02-10 23:13 UTC (permalink / raw)
  To: larry.finger, stf_xl, linux-wireless; +Cc: herton, linux-mips


------------------------------
On Mon, Feb 10, 2014 10:50 PM GMT Larry Finger wrote:

>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 <petr.pisar@atlas.cz>
>> Bisected-by: Tom Li <biergaizi2009@gmail.com>
>> Reported-and-tested-by: Tom Li <biergaizi2009@gmail.com>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Stanislaw Gruszka <stf_xl@wp.pl>
>> ---
>
>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.Finger@lwfinger.next>
>
>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 <linux/cache.h>
>> +
>>   #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;
>>   };
>>
>


^ permalink raw reply	[flat|nested] 4+ messages in thread