diff mbox

E100 RX ring buffers continued...

Message ID m3k50utg5q.fsf@intrepid.localdomain
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Krzysztof Halasa Aug. 23, 2009, 8:43 p.m. UTC
Walt Holman <walt@holmansrus.com> writes:

> I've tested this under 2.6.31-rc7 (which was also broken to begin
> with) and this appears to have fixed it.  It's a fairly limited test
> at this point, as I've just been downloading a large file for the past
> 15 mins. or so, but this was normally enough to have multiple
> stoppages.  Do you still need 2.6.30.5 tested as well?

No, these tests and the drivers in both versions are equivalent. Thanks
a lot.


David, I think it's safe to apply this. Probably to "stable" series,
too.

I think at least this should theoretically be changed as well:

e100_rx_indicate()
        ...
        /* Need to sync before taking a peek at cb_complete bit */
        pci_dma_sync_single_for_cpu(nic->pdev, rx->dma_addr,
                sizeof(struct rfd), PCI_DMA_BIDIRECTIONAL);
        rfd_status = le16_to_cpu(rfd->status);

but since it appears to work I wouldn't touch it ATM. I guess the
swiotlb code is smart enough to not write to device-owner memory on
for_cpu() call.

...

E100: fix interaction with swiotlb on X86.

E100 places it's RX packet descriptors inside skb->data and uses them
with bidirectional streaming DMA mapping. Data in descriptors is
accessed simultaneously by the chip (writing status and size when
a packet is received) and CPU (reading to check if the packet was
received). This isn't a valid usage of PCI DMA API, which requires use
of the coherent (consistent) memory for such purpose. Unfortunately e100
chips working in "simplified" RX mode have to store received data
directly after the descriptor. Fixing the driver to conform to the API
would require using unsupported "flexible" RX mode or receiving data
into a coherent memory and using CPU to copy it to network buffers.

This patch, while not yet making the driver conform to the PCI DMA API,
allows it to work correctly on X86 with swiotlb (while not breaking
other architectures).

Signed-off-by: Krzysztof HaƂasa <khc@pm.waw.pl>

Comments

David Miller Aug. 24, 2009, 2:03 a.m. UTC | #1
From: Krzysztof Halasa <khc@pm.waw.pl>
Date: Sun, 23 Aug 2009 22:43:29 +0200

> David, I think it's safe to apply this.

Yep, I've applied this to net-2.6, thanks!

> Probably to "stable" series, too.

That would depend upon applying your original patch to -stable,
which I had no intentions of doing.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Halasa Aug. 24, 2009, 10:49 a.m. UTC | #2
David Miller <davem@davemloft.net> writes:

>> Probably to "stable" series, too.
>
> That would depend upon applying your original patch to -stable,
> which I had no intentions of doing.

Still you sent it for both 2.6.27 and .30 anyway :-)
David Miller Aug. 24, 2009, 8:10 p.m. UTC | #3
From: Krzysztof Halasa <khc@pm.waw.pl>
Date: Mon, 24 Aug 2009 12:49:01 +0200

> David Miller <davem@davemloft.net> writes:
> 
>>> Probably to "stable" series, too.
>>
>> That would depend upon applying your original patch to -stable,
>> which I had no intentions of doing.
> 
> Still you sent it for both 2.6.27 and .30 anyway :-)

Did I?  Aha, then I indeed have to send your fix :-)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index 014dfb6..53e8252 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -1764,7 +1764,7 @@  static int e100_rx_indicate(struct nic *nic, struct rx *rx,
 				nic->ru_running = RU_SUSPENDED;
 		pci_dma_sync_single_for_device(nic->pdev, rx->dma_addr,
 					       sizeof(struct rfd),
-					       PCI_DMA_BIDIRECTIONAL);
+					       PCI_DMA_FROMDEVICE);
 		return -ENODATA;
 	}