Re: [BUG] net: sungem: device driver frees DMA memory with wrong function

2019-01-02 Thread Corentin Labbe
On Fri, Dec 28, 2018 at 12:36:21AM -0800, Christoph Hellwig wrote:
> Please try this patch:
> 

The error type change to "DMA-API: gem :00:0f.0: device driver failed to 
check map error" (I will send patch for fixing this).
Note that I used the patch from your just sent DMA series (since the patch 
below is included in).

So you can add my
Tested-by: LABBE Corentin 

Thanks

> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index a52c6409bdc2..f454e0ed1398 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -284,32 +284,25 @@ static inline void dma_direct_sync_sg_for_cpu(struct 
> device *dev,
>  }
>  #endif
>  
> -static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
> -   size_t size,
> -   enum dma_data_direction dir,
> -   unsigned long attrs)
> +static inline dma_addr_t dma_map_page_attrs(struct device *dev,
> + struct page *page, size_t offset, size_t size,
> + enum dma_data_direction dir, unsigned long attrs)
>  {
>   const struct dma_map_ops *ops = get_dma_ops(dev);
>   dma_addr_t addr;
>  
>   BUG_ON(!valid_dma_direction(dir));
> - debug_dma_map_single(dev, ptr, size);
>   if (dma_is_direct(ops))
> - addr = dma_direct_map_page(dev, virt_to_page(ptr),
> - offset_in_page(ptr), size, dir, attrs);
> + addr = dma_direct_map_page(dev, page, offset, size, dir, attrs);
>   else
> - addr = ops->map_page(dev, virt_to_page(ptr),
> - offset_in_page(ptr), size, dir, attrs);
> - debug_dma_map_page(dev, virt_to_page(ptr),
> -offset_in_page(ptr), size,
> -dir, addr, true);
> + addr = ops->map_page(dev, page, offset, size, dir, attrs);
> + debug_dma_map_page(dev, page, offset, size, dir, addr, false);
> +
>   return addr;
>  }
>  
> -static inline void dma_unmap_single_attrs(struct device *dev, dma_addr_t 
> addr,
> -   size_t size,
> -   enum dma_data_direction dir,
> -   unsigned long attrs)
> +static inline void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr,
> + size_t size, enum dma_data_direction dir, unsigned long attrs)
>  {
>   const struct dma_map_ops *ops = get_dma_ops(dev);
>  
> @@ -321,12 +314,6 @@ static inline void dma_unmap_single_attrs(struct device 
> *dev, dma_addr_t addr,
>   debug_dma_unmap_page(dev, addr, size, dir, true);
>  }
>  
> -static inline void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr,
> - size_t size, enum dma_data_direction dir, unsigned long attrs)
> -{
> - return dma_unmap_single_attrs(dev, addr, size, dir, attrs);
> -}
> -
>  /*
>   * dma_maps_sg_attrs returns 0 on error and > 0 on success.
>   * It should never return a value < 0.
> @@ -363,25 +350,6 @@ static inline void dma_unmap_sg_attrs(struct device 
> *dev, struct scatterlist *sg
>   ops->unmap_sg(dev, sg, nents, dir, attrs);
>  }
>  
> -static inline dma_addr_t dma_map_page_attrs(struct device *dev,
> - struct page *page,
> - size_t offset, size_t size,
> - enum dma_data_direction dir,
> - unsigned long attrs)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> - dma_addr_t addr;
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (dma_is_direct(ops))
> - addr = dma_direct_map_page(dev, page, offset, size, dir, attrs);
> - else
> - addr = ops->map_page(dev, page, offset, size, dir, attrs);
> - debug_dma_map_page(dev, page, offset, size, dir, addr, false);
> -
> - return addr;
> -}
> -
>  static inline dma_addr_t dma_map_resource(struct device *dev,
> phys_addr_t phys_addr,
> size_t size,
> @@ -488,6 +456,19 @@ dma_sync_sg_for_device(struct device *dev, struct 
> scatterlist *sg,
>  
>  }
>  
> +static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
> + size_t size, enum dma_data_direction dir, unsigned long attrs)
> +{
> + return dma_map_page_attrs(dev, virt_to_page(ptr), offset_in_page(ptr),
> + size, dir, attrs);
> +}
> +
> +static inline void dma_unmap_single_attrs(struct device *dev, dma_addr_t 
> addr,
> + size_t size, enum dma_data_direction dir, unsigned long attrs)
> +{
> + return dma_unmap_page_attrs(dev, addr, size, dir, attrs);
> +}
> +
>  #define dma_map_single(d, a, s, r) dma_map_single_attrs(d, a, s, r, 0)
>  #define dma_unmap_single(d, a, s, r) 

Re: [BUG] net: sungem: device driver frees DMA memory with wrong function

2018-12-28 Thread Christoph Hellwig
Please try this patch:

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index a52c6409bdc2..f454e0ed1398 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -284,32 +284,25 @@ static inline void dma_direct_sync_sg_for_cpu(struct 
device *dev,
 }
 #endif
 
-static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
- size_t size,
- enum dma_data_direction dir,
- unsigned long attrs)
+static inline dma_addr_t dma_map_page_attrs(struct device *dev,
+   struct page *page, size_t offset, size_t size,
+   enum dma_data_direction dir, unsigned long attrs)
 {
const struct dma_map_ops *ops = get_dma_ops(dev);
dma_addr_t addr;
 
BUG_ON(!valid_dma_direction(dir));
-   debug_dma_map_single(dev, ptr, size);
if (dma_is_direct(ops))
-   addr = dma_direct_map_page(dev, virt_to_page(ptr),
-   offset_in_page(ptr), size, dir, attrs);
+   addr = dma_direct_map_page(dev, page, offset, size, dir, attrs);
else
-   addr = ops->map_page(dev, virt_to_page(ptr),
-   offset_in_page(ptr), size, dir, attrs);
-   debug_dma_map_page(dev, virt_to_page(ptr),
-  offset_in_page(ptr), size,
-  dir, addr, true);
+   addr = ops->map_page(dev, page, offset, size, dir, attrs);
+   debug_dma_map_page(dev, page, offset, size, dir, addr, false);
+
return addr;
 }
 
-static inline void dma_unmap_single_attrs(struct device *dev, dma_addr_t addr,
- size_t size,
- enum dma_data_direction dir,
- unsigned long attrs)
+static inline void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr,
+   size_t size, enum dma_data_direction dir, unsigned long attrs)
 {
const struct dma_map_ops *ops = get_dma_ops(dev);
 
@@ -321,12 +314,6 @@ static inline void dma_unmap_single_attrs(struct device 
*dev, dma_addr_t addr,
debug_dma_unmap_page(dev, addr, size, dir, true);
 }
 
-static inline void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr,
-   size_t size, enum dma_data_direction dir, unsigned long attrs)
-{
-   return dma_unmap_single_attrs(dev, addr, size, dir, attrs);
-}
-
 /*
  * dma_maps_sg_attrs returns 0 on error and > 0 on success.
  * It should never return a value < 0.
@@ -363,25 +350,6 @@ static inline void dma_unmap_sg_attrs(struct device *dev, 
struct scatterlist *sg
ops->unmap_sg(dev, sg, nents, dir, attrs);
 }
 
-static inline dma_addr_t dma_map_page_attrs(struct device *dev,
-   struct page *page,
-   size_t offset, size_t size,
-   enum dma_data_direction dir,
-   unsigned long attrs)
-{
-   const struct dma_map_ops *ops = get_dma_ops(dev);
-   dma_addr_t addr;
-
-   BUG_ON(!valid_dma_direction(dir));
-   if (dma_is_direct(ops))
-   addr = dma_direct_map_page(dev, page, offset, size, dir, attrs);
-   else
-   addr = ops->map_page(dev, page, offset, size, dir, attrs);
-   debug_dma_map_page(dev, page, offset, size, dir, addr, false);
-
-   return addr;
-}
-
 static inline dma_addr_t dma_map_resource(struct device *dev,
  phys_addr_t phys_addr,
  size_t size,
@@ -488,6 +456,19 @@ dma_sync_sg_for_device(struct device *dev, struct 
scatterlist *sg,
 
 }
 
+static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
+   size_t size, enum dma_data_direction dir, unsigned long attrs)
+{
+   return dma_map_page_attrs(dev, virt_to_page(ptr), offset_in_page(ptr),
+   size, dir, attrs);
+}
+
+static inline void dma_unmap_single_attrs(struct device *dev, dma_addr_t addr,
+   size_t size, enum dma_data_direction dir, unsigned long attrs)
+{
+   return dma_unmap_page_attrs(dev, addr, size, dir, attrs);
+}
+
 #define dma_map_single(d, a, s, r) dma_map_single_attrs(d, a, s, r, 0)
 #define dma_unmap_single(d, a, s, r) dma_unmap_single_attrs(d, a, s, r, 0)
 #define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)


Re: [BUG] net: sungem: device driver frees DMA memory with wrong function

2018-12-24 Thread David Miller
From: Corentin Labbe 
Date: Sun, 23 Dec 2018 20:38:21 +0100

> During a boot on a qemu machine, I hit the following problem:
 ...
> After some pr_info, I found that the function triggering this is the 
> pci_unmap_page() in gem_tx().
> So clearly this WARNING() is strange since it says "unmapped as single".

I also went through the code paths and this makes no sense to me either.


[BUG] net: sungem: device driver frees DMA memory with wrong function

2018-12-23 Thread Corentin Labbe
Hello

During a boot on a qemu machine, I hit the following problem:
[   21.613659] [ cut here ]
[   21.614039] DMA-API: gem :00:0f.0: device driver frees DMA memory with 
wrong function [device address=0x185c5402] [size=408 bytes] [mapped as 
page] [unmapped as single]
[   21.615960] WARNING: CPU: 0 PID: 206 at /linux-next/kernel/dma/debug.c:1085 
check_unmap+0x1b0/0xd10
[   21.616599] Modules linked in:
[   21.617344] CPU: 0 PID: 206 Comm: dhcpcd Not tainted 
4.20.0-rc6-next-20181217+ #12
[   21.617735] NIP:  c00ad4e0 LR: c00ad4e0 CTR: c058a710
[   21.618068] REGS: dfff7cb0 TRAP: 0700   Not tainted  
(4.20.0-rc6-next-20181217+)
[   21.618404] MSR:  00021032   CR: 28008264  XER: 
[   21.618789] 
[   21.618789] GPR00: c00ad4e0 dfff7d60 d85924c0 00a8 0102 0101 
00fe 5d205b6d 
[   21.618789] GPR08: 0007   00fe 22008864 100581b4 
dfff7f1c de019508 
[   21.618789] GPR16:  de019000 0001 c0a67f00 0004 c0b44018 
c0a31fc4 c0b43bdc 
[   21.618789] GPR24: 9032 c0dadedc c0db c0da999c c0b43bdc c0c0b178 
dfff7de0 de076ce8 
[   21.620219] NIP [c00ad4e0] check_unmap+0x1b0/0xd10
[   21.620478] LR [c00ad4e0] check_unmap+0x1b0/0xd10
[   21.620703] Call Trace:
[   21.620963] [dfff7d60] [c00ad4e0] check_unmap+0x1b0/0xd10 (unreliable)
[   21.621379] [dfff7dd0] [c00ae128] debug_dma_unmap_page+0xe8/0x120
[   21.621656] [dfff7e40] [c05a865c] gem_poll+0x1000/0x18fc
[   21.621863] [dfff7f00] [c071f044] net_rx_action+0x1b8/0x41c
[   21.622242] [dfff7f80] [c08a287c] __do_softirq+0x17c/0x3b0
[   21.622495] [dfff7ff0] [c0011378] call_do_softirq+0x24/0x3c
[   21.622697] [d86c9c20] [c000724c] do_softirq_own_stack+0x3c/0x7c
[   21.623008] [d86c9c40] [c004e800] do_softirq.part.1+0x64/0x7c
[   21.623292] [d86c9c60] [c004e8d4] __local_bh_enable_ip+0xbc/0x138
[   21.623526] [d86c9c80] [c071c9b4] __dev_queue_xmit+0x28c/0x874
[   21.623776] [d86c9cf0] [c083ebe8] packet_snd+0x4f8/0x988
[   21.624057] [d86c9d80] [c06eddb8] sock_sendmsg+0x20/0x40
[   21.624410] [d86c9d90] [c06ede94] sock_write_iter+0xbc/0x138
[   21.624636] [d86c9df0] [c018f50c] do_iter_readv_writev+0x1dc/0x1f4
[   21.624872] [d86c9e40] [c01925a8] do_iter_write+0xb4/0x350
[   21.625143] [d86c9e80] [c01928f4] vfs_writev+0x88/0x140
[   21.625446] [d86c9f00] [c0192a1c] do_writev+0x70/0x104
[   21.625621] [d86c9f40] [c001912c] ret_from_syscall+0x0/0x38
[   21.626001] --- interrupt: c01 at 0xfedbd08
[   21.626001] LR = 0xffbc1f8
[   21.626357] Instruction dump:
[   21.626719] 7eb5ba14 7e95a214 2b940014 419d0970 92c10008 7efcba14 3c60c0a1 
7e649b78 
[   21.627152] 38637cac 80d7043c 90c1000c 4bf9d36d <0fe0> 8261003c 82810040 
82a10044 
[   21.627665] ---[ end trace fd53714bd2a5fd06 ]---

After some pr_info, I found that the function triggering this is the 
pci_unmap_page() in gem_tx().
So clearly this WARNING() is strange since it says "unmapped as single".

The qemu is ran with:
qemu-system-ppc -machine mac99,via=pmu -nographic -net 
nic,model=sungem,macaddr=52:54:00:12:34:58 -net tap -m 512 -monitor none 
-append "console=ttyPZ0 root=/dev/ram0" -kernel vmlinux -initrd rootfs.cpio.gz 
-drive format=qcow2,file=lava-guest.qcow2,media=disk,id=test,if=ide

Regards