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()