| Message ID | 20171109104628.63979-1-mika.westerberg@linux.intel.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next] net: thunderbolt: Clear finished Tx frame bus address in tbnet_tx_callback() | expand |
From: Mika Westerberg <mika.westerberg@linux.intel.com> Date: Thu, 9 Nov 2017 13:46:28 +0300 > When Thunderbolt network interface is disabled or when the cable is > unplugged the driver releases all allocated buffers by calling > tbnet_free_buffers() for each ring. This function then calls > dma_unmap_page() for each buffer it finds where bus address is non-zero. > Now, we only clear this bus address when the Tx buffer is sent to the > hardware so it is possible that the function finds an entry that has > already been unmapped. > > Enabling DMA-API debugging catches this as well: > > thunderbolt 0000:06:00.0: DMA-API: device driver tries to free DMA > memory it has not allocated [device address=0x0000000068321000] [size=4096 bytes] > > Fix this by clearing the bus address of a Tx frame right after we have > unmapped the buffer. > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> Applied, but assuming zero is a non-valid DMA address is never a good idea. That's why we have the DMA error code signaling abstracted.
On Sat, Nov 11, 2017 at 07:21:24PM +0900, David Miller wrote: > From: Mika Westerberg <mika.westerberg@linux.intel.com> > Date: Thu, 9 Nov 2017 13:46:28 +0300 > > > When Thunderbolt network interface is disabled or when the cable is > > unplugged the driver releases all allocated buffers by calling > > tbnet_free_buffers() for each ring. This function then calls > > dma_unmap_page() for each buffer it finds where bus address is non-zero. > > Now, we only clear this bus address when the Tx buffer is sent to the > > hardware so it is possible that the function finds an entry that has > > already been unmapped. > > > > Enabling DMA-API debugging catches this as well: > > > > thunderbolt 0000:06:00.0: DMA-API: device driver tries to free DMA > > memory it has not allocated [device address=0x0000000068321000] [size=4096 bytes] > > > > Fix this by clearing the bus address of a Tx frame right after we have > > unmapped the buffer. > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > Applied, but assuming zero is a non-valid DMA address is never a good > idea. That's why we have the DMA error code signaling abstracted. There does not seem to be a way to mark DMA address invalid in a driver so we probably need to add a flag to struct tbnet_frame instead.
From: Mika Westerberg > Sent: 13 November 2017 10:22 > To: David Miller > Cc: michael.jamet@intel.com; yehezkel.bernat@intel.com; netdev@vger.kernel.org > Subject: Re: [PATCH net-next] net: thunderbolt: Clear finished Tx frame bus address in > tbnet_tx_callback() > > On Sat, Nov 11, 2017 at 07:21:24PM +0900, David Miller wrote: > > From: Mika Westerberg <mika.westerberg@linux.intel.com> > > Date: Thu, 9 Nov 2017 13:46:28 +0300 > > > > > When Thunderbolt network interface is disabled or when the cable is > > > unplugged the driver releases all allocated buffers by calling > > > tbnet_free_buffers() for each ring. This function then calls > > > dma_unmap_page() for each buffer it finds where bus address is non-zero. > > > Now, we only clear this bus address when the Tx buffer is sent to the > > > hardware so it is possible that the function finds an entry that has > > > already been unmapped. > > > > > > Enabling DMA-API debugging catches this as well: > > > > > > thunderbolt 0000:06:00.0: DMA-API: device driver tries to free DMA > > > memory it has not allocated [device address=0x0000000068321000] [size=4096 bytes] > > > > > > Fix this by clearing the bus address of a Tx frame right after we have > > > unmapped the buffer. > > > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > > Applied, but assuming zero is a non-valid DMA address is never a good > > idea. That's why we have the DMA error code signaling abstracted. > > There does not seem to be a way to mark DMA address invalid in a driver > so we probably need to add a flag to struct tbnet_frame instead. Can you use the length? David
On Mon, Nov 20, 2017 at 02:46:53PM +0000, David Laight wrote: > > There does not seem to be a way to mark DMA address invalid in a driver > > so we probably need to add a flag to struct tbnet_frame instead. > > Can you use the length? Unfortunately no because size is 12-bit field in struct ring_frame and 0 means 4096 bytes.
diff --git a/drivers/net/thunderbolt.c b/drivers/net/thunderbolt.c index 435854688a7a..228d4aa6d9ae 100644 --- a/drivers/net/thunderbolt.c +++ b/drivers/net/thunderbolt.c @@ -536,6 +536,7 @@ static void tbnet_tx_callback(struct tb_ring *ring, struct ring_frame *frame, dma_unmap_page(dma_dev, tf->frame.buffer_phy, tbnet_frame_size(tf), DMA_TO_DEVICE); + tf->frame.buffer_phy = 0; /* Return buffer to the ring */ net->tx_ring.prod++;
When Thunderbolt network interface is disabled or when the cable is unplugged the driver releases all allocated buffers by calling tbnet_free_buffers() for each ring. This function then calls dma_unmap_page() for each buffer it finds where bus address is non-zero. Now, we only clear this bus address when the Tx buffer is sent to the hardware so it is possible that the function finds an entry that has already been unmapped. Enabling DMA-API debugging catches this as well: thunderbolt 0000:06:00.0: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0x0000000068321000] [size=4096 bytes] Fix this by clearing the bus address of a Tx frame right after we have unmapped the buffer. Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/net/thunderbolt.c | 1 + 1 file changed, 1 insertion(+)