Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-13 Thread Alexander Duyck
On Thu, Oct 13, 2016 at 4:00 AM, Nikita Yushchenko wrote: It would make more sense to update the DMA API for __dma_page_cpu_to_dev on ARM so that you don't invalidate the cache if the direction is DMA_FROM_DEVICE. >>> >>> No, in generic case it's

Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-13 Thread Nikita Yushchenko
>>> It would make more sense to update the DMA API for >>> __dma_page_cpu_to_dev on ARM so that you don't invalidate the cache if >>> the direction is DMA_FROM_DEVICE. >> >> No, in generic case it's unsafe. >> >> If CPU issued a write to a location, and sometime later that location is >> used as

RE: igb driver can cause cache invalidation of non-owned memory?

2016-10-13 Thread David Laight
From: Alexander Duyck > Sent: 12 October 2016 19:30 > On Wed, Oct 12, 2016 at 11:12 AM, Nikita Yushchenko > wrote: > >> It would make more sense to update the DMA API for > >> __dma_page_cpu_to_dev on ARM so that you don't invalidate the cache if > >> the

Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-12 Thread Alexander Duyck
On Wed, Oct 12, 2016 at 11:12 AM, Nikita Yushchenko wrote: >> It would make more sense to update the DMA API for >> __dma_page_cpu_to_dev on ARM so that you don't invalidate the cache if >> the direction is DMA_FROM_DEVICE. > > No, in generic case it's unsafe. > >

Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-12 Thread Nikita Yushchenko
> It would make more sense to update the DMA API for > __dma_page_cpu_to_dev on ARM so that you don't invalidate the cache if > the direction is DMA_FROM_DEVICE. No, in generic case it's unsafe. If CPU issued a write to a location, and sometime later that location is used as DMA buffer, there is

Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-12 Thread Alexander Duyck
On Wed, Oct 12, 2016 at 9:11 AM, Nikita Yushchenko wrote: To get some throughput improvement, I propose removal of that sync_for_device() before reusing buffer. Will you accept such a patch ;) >>> >>> Not one that gets rid of sync_for_device() in the

Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-12 Thread Nikita Yushchenko
>>> To get some throughput improvement, I propose removal of that >>> sync_for_device() before reusing buffer. Will you accept such a patch ;) >> >> Not one that gets rid of sync_for_device() in the driver. From what I >> can tell there are some DMA APIs that use that to perform the >>

RE: igb driver can cause cache invalidation of non-owned memory?

2016-10-12 Thread David Laight
From: Alexander Duyck > Sent: 12 October 2016 16:33 ... > > To get some throughput improvement, I propose removal of that > > sync_for_device() before reusing buffer. Will you accept such a patch ;) > > Not one that gets rid of sync_for_device() in the driver. From what I > can tell there are

Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-12 Thread Alexander Duyck
On Tue, Oct 11, 2016 at 11:55 PM, Nikita Yushchenko wrote: The main reason why this isn't a concern for the igb driver is because we currently pass the page up as read-only. We don't allow the stack to write into the page by keeping the page count

Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-12 Thread Nikita Yushchenko
>>> The main reason why this isn't a concern for the igb driver is because >>> we currently pass the page up as read-only. We don't allow the stack >>> to write into the page by keeping the page count greater than 1 which >>> means that the page is shared. It isn't until we unmap the page that

Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-10 Thread Alexander Duyck
On Mon, Oct 10, 2016 at 10:00 AM, Nikita Yushchenko wrote: > Hi Alexander > > Thanks for your explanation. > >> The main reason why this isn't a concern for the igb driver is because >> we currently pass the page up as read-only. We don't allow the stack >> to

Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-10 Thread Nikita Yushchenko
Hi Alexander Thanks for your explanation. > The main reason why this isn't a concern for the igb driver is because > we currently pass the page up as read-only. We don't allow the stack > to write into the page by keeping the page count greater than 1 which > means that the page is shared. It

Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-10 Thread Alexander Duyck
On Mon, Oct 10, 2016 at 5:27 AM, Nikita Yushchenko wrote: >>> Hmm... I'm not about device writing to memory. >> >> This absolutely is about whether the device wrote into the >> area or not. > > Not only. > >>> Sequence in igb driver is: >>> >>> dma_map(full_page)

Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-10 Thread Nikita Yushchenko
>> All the data has been synced > > Non-synced data is write done by CPU executing upper layers of network stack, Upper layers shall never get area that is still dma_map()ed and will be dma_unmap()ed in future. But with igb, this is exactly what happens.

Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-10 Thread Nikita Yushchenko
>> Hmm... I'm not about device writing to memory. > > This absolutely is about whether the device wrote into the > area or not. Not only. >> Sequence in igb driver is: >> >> dma_map(full_page) >> >> sync_to_cpu(half_page); >> skb_add_rx_frag(skb, half_page); >> napi_gro_receive(skb); >>

Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-10 Thread David Miller
From: Nikita Yushchenko Date: Mon, 10 Oct 2016 12:51:28 +0300 > Hmm... I'm not about device writing to memory. This absolutely is about whether the device wrote into the area or not. > Sequence in igb driver is: > > dma_map(full_page) > >

Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-10 Thread Nikita Yushchenko
>> With this scheme, page used for Rx is completely dma_map()ed at >> allocation time, split into two buffers, and individual buffer is >> sync_to_cpu()ed AND PASSED TO NETWORK STACK via skb_add_rx_frag() - >> while driver driver still uses other buffer. Later, when driver decides >> to no longer

Re: igb driver can cause cache invalidation of non-owned memory?

2016-10-10 Thread David Miller
From: Nikita Yushchenko Date: Mon, 10 Oct 2016 11:52:06 +0300 > With this scheme, page used for Rx is completely dma_map()ed at > allocation time, split into two buffers, and individual buffer is > sync_to_cpu()ed AND PASSED TO NETWORK STACK via skb_add_rx_frag()

igb driver can cause cache invalidation of non-owned memory?

2016-10-10 Thread Nikita Yushchenko
Hi DMA mapping scheme introduced in commit cbc8e55f6fda ('igb: Map entire page and sync half instead of mapping and unmapping half pages') back in 2012, and used up to now, can probably cause breakage of unrelated code on archs with non-coherent caches. With this scheme, page used for Rx is