stmmac: Needs to check dma_map return values (oops when build with CONFIG_DMA_API_DEBUG)

PATCH Applied Hans de Goede 4 participants 7 messages Aug 1, 2014 – Aug 30, 2014

Subsystems (4)

Networking Network drivers Arm STM32 stmmac Ethernet driver

lore

Share & tools

lore

Hans de Goede

author

2014-08-01 09:52 UTC #0 lore raw

Hi,

When booting a Fedora rawhide kernel, which is build with CONFIG_DMA_API_DEBUG enabled, I get the following oops:

[  162.398116] ------------[ cut here ]------------
[  162.402770] WARNING: CPU: 0 PID: 0 at lib/dma-debug.c:1140 check_unmap+0x6ec/0x824()
[  162.410507] stmmaceth 1c50000.ethernet: DMA-API: device driver failed to check map error[device address=0x0000000068a1d002] [size=90 bytes] [mapped as single]
[  162.424480] Modules linked in: ip6t_rpfilter ip6t_REJECT xt_conntrack cfg80211 rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw stmmac ptp pps_core i2c_mv64xxx sun4i_ts ohci_platform sunxi_sid rtc_sunxi phy_sun4i_usb sunxi_wdt leds_gpio mmc_block sunxi_mmc mmc_core ehci_platform ahci_sunxi libahci_platform
[  162.476435] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.16.0-0.rc7.git1.1.fc22.armv7hl #1
[  162.484622] [<c02188dc>] (unwind_backtrace) from [<c0212bb0>] (show_stack+0x18/0x1c)
[  162.492366] [<c0212bb0>] (show_stack) from [<c0938d20>] (dump_stack+0x84/0xb0)
[  162.499590] [<c0938d20>] (dump_stack) from [<c0252444>] (warn_slowpath_common+0x70/0x94)
[  162.507678] [<c0252444>] (warn_slowpath_common) from [<c025249c>] (warn_slowpath_fmt+0x34/0x44)
[  162.516374] [<c025249c>] (warn_slowpath_fmt) from [<c058a65c>] (check_unmap+0x6ec/0x824)
[  162.524463] [<c058a65c>] (check_unmap) from [<c058a7f8>] (debug_dma_unmap_page+0x64/0x70)
[  162.532667] [<c058a7f8>] (debug_dma_unmap_page) from [<bf08dde0>] (stmmac_tx_clean+0x260/0x494 [stmmac])
[  162.542242] [<bf08dde0>] (stmmac_tx_clean [stmmac]) from [<c025f988>] (call_timer_fn+0xd0/0x310)
[  162.551025] [<c025f988>] (call_timer_fn) from [<c0260578>] (run_timer_softirq+0x268/0x360)
[  162.559287] [<c0260578>] (run_timer_softirq) from [<c0258628>] (__do_softirq+0x184/0x4d0)
[  162.567461] [<c0258628>] (__do_softirq) from [<c0258c58>] (irq_exit+0x8c/0xfc)
[  162.574683] [<c0258c58>] (irq_exit) from [<c020f52c>] (handle_IRQ+0x74/0x98)
[  162.581730] [<c020f52c>] (handle_IRQ) from [<c0208920>] (gic_handle_irq+0x4c/0x70)
[  162.589299] [<c0208920>] (gic_handle_irq) from [<c0942b04>] (__irq_svc+0x44/0x58)
[  162.596774] Exception stack(0xc0d95f58 to 0xc0d95fa0)
[  162.601823] 5f40:                                                       00000001 00000004
[  162.609995] 5f60: 00000000 c0da2128 c0d94000 00000000 00000000 c0d915d0 00000000 ffffffed
[  162.618168] 5f80: c0d94000 c0d9de8c 00000000 c0d95fa0 c02a6d4c c020f8bc 200f0013 ffffffff
[  162.626344] [<c0942b04>] (__irq_svc) from [<c020f8bc>] (arch_cpu_idle+0x38/0x44)
[  162.633742] [<c020f8bc>] (arch_cpu_idle) from [<c029e780>] (cpu_startup_entry+0x21c/0x6ac)
[  162.642008] [<c029e780>] (cpu_startup_entry) from [<c0cabb98>] (start_kernel+0x374/0x3dc)
[  162.650176] ---[ end trace 70ec89df0de626f3 ]---
[  162.654787] Mapped at:
[  162.657145]  [<c07ed840>] dev_hard_start_xmit+0x350/0x8a0
[  162.662550]  [<c080fcf8>] sch_direct_xmit+0x74/0x264
[  162.667526]  [<c07ee1ec>] __dev_queue_xmit+0x45c/0x8e0
[  162.672675]  [<c08a8c84>] ip6_finish_output2+0x7cc/0xa80
[  162.677999]  [<c08d3a20>] mld_sendpack+0x3fc/0x808

This is caused by various calls to dma_map_single not being error checked.

A call to dma_map_single should look something like this:

	dma_handle = dma_map_single(dev, addr, size, direction);
	if (dma_mapping_error(dma_handle))
		/* do error */

I believe that the calls to skb_frag_dma_map should be error checked similarly, but I'm not 100% sure on that.

I looked into fixing this myself, but esp. for the jumbo frame paths fixing this looks like it is non trivial.

Regards,

Hans

Denis Kirjanov

2014-08-01 10:11 UTC #1 lore raw

Quoted 72 lines · Hans de Goede

Hi,

When booting a Fedora rawhide kernel, which is build with CONFIG_DMA_API_DEBUG enabled, I get the following oops:

[  162.398116] ------------[ cut here ]------------
[  162.402770] WARNING: CPU: 0 PID: 0 at lib/dma-debug.c:1140
check_unmap+0x6ec/0x824()
[  162.410507] stmmaceth 1c50000.ethernet: DMA-API: device driver failed to
check map error[device address=0x0000000068a1d002] [size=90 bytes] [mapped
as single]
[  162.424480] Modules linked in: ip6t_rpfilter ip6t_REJECT xt_conntrack

cfg80211 rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw stmmac ptp pps_core i2c_mv64xxx sun4i_ts ohci_platform sunxi_sid rtc_sunxi phy_sun4i_usb sunxi_wdt leds_gpio mmc_block sunxi_mmc mmc_core ehci_platform ahci_sunxi libahci_platform

[  162.476435] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
3.16.0-0.rc7.git1.1.fc22.armv7hl #1
[  162.484622] [<c02188dc>] (unwind_backtrace) from [<c0212bb0>]
(show_stack+0x18/0x1c)
[  162.492366] [<c0212bb0>] (show_stack) from [<c0938d20>]
(dump_stack+0x84/0xb0)
[  162.499590] [<c0938d20>] (dump_stack) from [<c0252444>]
(warn_slowpath_common+0x70/0x94)
[  162.507678] [<c0252444>] (warn_slowpath_common) from [<c025249c>]
(warn_slowpath_fmt+0x34/0x44)
[  162.516374] [<c025249c>] (warn_slowpath_fmt) from [<c058a65c>]
(check_unmap+0x6ec/0x824)
[  162.524463] [<c058a65c>] (check_unmap) from [<c058a7f8>]
(debug_dma_unmap_page+0x64/0x70)
[  162.532667] [<c058a7f8>] (debug_dma_unmap_page) from [<bf08dde0>]
(stmmac_tx_clean+0x260/0x494 [stmmac])
[  162.542242] [<bf08dde0>] (stmmac_tx_clean [stmmac]) from [<c025f988>]
(call_timer_fn+0xd0/0x310)
[  162.551025] [<c025f988>] (call_timer_fn) from [<c0260578>]
(run_timer_softirq+0x268/0x360)
[  162.559287] [<c0260578>] (run_timer_softirq) from [<c0258628>]
(__do_softirq+0x184/0x4d0)
[  162.567461] [<c0258628>] (__do_softirq) from [<c0258c58>]
(irq_exit+0x8c/0xfc)
[  162.574683] [<c0258c58>] (irq_exit) from [<c020f52c>]
(handle_IRQ+0x74/0x98)
[  162.581730] [<c020f52c>] (handle_IRQ) from [<c0208920>]
(gic_handle_irq+0x4c/0x70)
[  162.589299] [<c0208920>] (gic_handle_irq) from [<c0942b04>]
(__irq_svc+0x44/0x58)
[  162.596774] Exception stack(0xc0d95f58 to 0xc0d95fa0)
[  162.601823] 5f40:
00000001 00000004
[  162.609995] 5f60: 00000000 c0da2128 c0d94000 00000000 00000000 c0d915d0
00000000 ffffffed
[  162.618168] 5f80: c0d94000 c0d9de8c 00000000 c0d95fa0 c02a6d4c c020f8bc
200f0013 ffffffff
[  162.626344] [<c0942b04>] (__irq_svc) from [<c020f8bc>]
(arch_cpu_idle+0x38/0x44)
[  162.633742] [<c020f8bc>] (arch_cpu_idle) from [<c029e780>]
(cpu_startup_entry+0x21c/0x6ac)
[  162.642008] [<c029e780>] (cpu_startup_entry) from [<c0cabb98>]
(start_kernel+0x374/0x3dc)
[  162.650176] ---[ end trace 70ec89df0de626f3 ]---
[  162.654787] Mapped at:
[  162.657145]  [<c07ed840>] dev_hard_start_xmit+0x350/0x8a0
[  162.662550]  [<c080fcf8>] sch_direct_xmit+0x74/0x264
[  162.667526]  [<c07ee1ec>] __dev_queue_xmit+0x45c/0x8e0
[  162.672675]  [<c08a8c84>] ip6_finish_output2+0x7cc/0xa80
[  162.677999]  [<c08d3a20>] mld_sendpack+0x3fc/0x808

This is caused by various calls to dma_map_single not being error checked.

Quoted 8 lines

A call to dma_map_single should look something like this:
	dma_handle = dma_map_single(dev, addr, size, direction);
	if (dma_mapping_error(dma_handle))
		/* do error */
I believe that the calls to skb_frag_dma_map should be error checked similarly, but I'm not 100% sure on that.

Yes. You should unwind your frags.
It's been done already in many places.

Quoted 12 lines

I looked into fixing this myself, but esp. for the jumbo frame paths fixing this looks like it is non trivial.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Giuseppe CAVALLARO

2014-08-18 05:41 UTC #2 lore raw

Quoted 75 lines · Denis Kirjanov

Quoted 74 lines · Hans de Goede

Hi,

When booting a Fedora rawhide kernel, which is build with CONFIG_DMA_API_DEBUG enabled, I get the following oops:

[  162.398116] ------------[ cut here ]------------
[  162.402770] WARNING: CPU: 0 PID: 0 at lib/dma-debug.c:1140
check_unmap+0x6ec/0x824()
[  162.410507] stmmaceth 1c50000.ethernet: DMA-API: device driver failed to
check map error[device address=0x0000000068a1d002] [size=90 bytes] [mapped
as single]
[  162.424480] Modules linked in: ip6t_rpfilter ip6t_REJECT xt_conntrack

[  162.476435] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
3.16.0-0.rc7.git1.1.fc22.armv7hl #1
[  162.484622] [<c02188dc>] (unwind_backtrace) from [<c0212bb0>]
(show_stack+0x18/0x1c)
[  162.492366] [<c0212bb0>] (show_stack) from [<c0938d20>]
(dump_stack+0x84/0xb0)
[  162.499590] [<c0938d20>] (dump_stack) from [<c0252444>]
(warn_slowpath_common+0x70/0x94)
[  162.507678] [<c0252444>] (warn_slowpath_common) from [<c025249c>]
(warn_slowpath_fmt+0x34/0x44)
[  162.516374] [<c025249c>] (warn_slowpath_fmt) from [<c058a65c>]
(check_unmap+0x6ec/0x824)
[  162.524463] [<c058a65c>] (check_unmap) from [<c058a7f8>]
(debug_dma_unmap_page+0x64/0x70)
[  162.532667] [<c058a7f8>] (debug_dma_unmap_page) from [<bf08dde0>]
(stmmac_tx_clean+0x260/0x494 [stmmac])
[  162.542242] [<bf08dde0>] (stmmac_tx_clean [stmmac]) from [<c025f988>]
(call_timer_fn+0xd0/0x310)
[  162.551025] [<c025f988>] (call_timer_fn) from [<c0260578>]
(run_timer_softirq+0x268/0x360)
[  162.559287] [<c0260578>] (run_timer_softirq) from [<c0258628>]
(__do_softirq+0x184/0x4d0)
[  162.567461] [<c0258628>] (__do_softirq) from [<c0258c58>]
(irq_exit+0x8c/0xfc)
[  162.574683] [<c0258c58>] (irq_exit) from [<c020f52c>]
(handle_IRQ+0x74/0x98)
[  162.581730] [<c020f52c>] (handle_IRQ) from [<c0208920>]
(gic_handle_irq+0x4c/0x70)
[  162.589299] [<c0208920>] (gic_handle_irq) from [<c0942b04>]
(__irq_svc+0x44/0x58)
[  162.596774] Exception stack(0xc0d95f58 to 0xc0d95fa0)
[  162.601823] 5f40:
00000001 00000004
[  162.609995] 5f60: 00000000 c0da2128 c0d94000 00000000 00000000 c0d915d0
00000000 ffffffed
[  162.618168] 5f80: c0d94000 c0d9de8c 00000000 c0d95fa0 c02a6d4c c020f8bc
200f0013 ffffffff
[  162.626344] [<c0942b04>] (__irq_svc) from [<c020f8bc>]
(arch_cpu_idle+0x38/0x44)
[  162.633742] [<c020f8bc>] (arch_cpu_idle) from [<c029e780>]
(cpu_startup_entry+0x21c/0x6ac)
[  162.642008] [<c029e780>] (cpu_startup_entry) from [<c0cabb98>]
(start_kernel+0x374/0x3dc)
[  162.650176] ---[ end trace 70ec89df0de626f3 ]---
[  162.654787] Mapped at:
[  162.657145]  [<c07ed840>] dev_hard_start_xmit+0x350/0x8a0
[  162.662550]  [<c080fcf8>] sch_direct_xmit+0x74/0x264
[  162.667526]  [<c07ee1ec>] __dev_queue_xmit+0x45c/0x8e0
[  162.672675]  [<c08a8c84>] ip6_finish_output2+0x7cc/0xa80
[  162.677999]  [<c08d3a20>] mld_sendpack+0x3fc/0x808

This is caused by various calls to dma_map_single not being error checked.

A call to dma_map_single should look something like this:

Quoted 12 lines

Quoted 6 lines
	dma_handle = dma_map_single(dev, addr, size, direction);
	if (dma_mapping_error(dma_handle))
		/* do error */
I believe that the calls to skb_frag_dma_map should be error checked similarly, but I'm not 100% sure on that.
Yes. You should unwind your frags.
It's been done already in many places.
Quoted 3 lines
I looked into fixing this myself, but esp. for the jumbo frame paths fixing this looks like it is non trivial.

Hello Denis.

I have a patch that must be ported on top of the net.dev kernel and I will provide it asap.

For reference this is the patch I will rework, give me you opinion too.

http://git.stlinux.com/?p=stm/linux-stm.git;a=commit;h=755f606c7efc5d9c111fa2be00b6e9916e101920

br
peppe

Quoted 10 lines

Quoted 9 lines
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Denis Kirjanov

Reviewed-by

2014-08-19 20:56 UTC #3 lore raw

Quoted 98 lines · Giuseppe CAVALLARO

Quoted 84 lines · Denis Kirjanov
Quoted 83 lines · Hans de Goede
Hi,
When booting a Fedora rawhide kernel, which is build with CONFIG_DMA_API_DEBUG enabled, I get the following oops:
[  162.398116] ------------[ cut here ]------------
[  162.402770] WARNING: CPU: 0 PID: 0 at lib/dma-debug.c:1140
check_unmap+0x6ec/0x824()
[  162.410507] stmmaceth 1c50000.ethernet: DMA-API: device driver failed
to
check map error[device address=0x0000000068a1d002] [size=90 bytes] [mapped
as single]
[  162.424480] Modules linked in: ip6t_rpfilter ip6t_REJECT xt_conntrack
cfg80211 rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables
iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack
iptable_mangle iptable_security iptable_raw stmmac ptp pps_core i2c_mv64xxx
sun4i_ts ohci_platform sunxi_sid rtc_sunxi phy_sun4i_usb sunxi_wdt leds_gpio
mmc_block sunxi_mmc mmc_core ehci_platform ahci_sunxi libahci_platform
[  162.476435] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
3.16.0-0.rc7.git1.1.fc22.armv7hl #1
[  162.484622] [<c02188dc>] (unwind_backtrace) from [<c0212bb0>]
(show_stack+0x18/0x1c)
[  162.492366] [<c0212bb0>] (show_stack) from [<c0938d20>]
(dump_stack+0x84/0xb0)
[  162.499590] [<c0938d20>] (dump_stack) from [<c0252444>]
(warn_slowpath_common+0x70/0x94)
[  162.507678] [<c0252444>] (warn_slowpath_common) from [<c025249c>]
(warn_slowpath_fmt+0x34/0x44)
[  162.516374] [<c025249c>] (warn_slowpath_fmt) from [<c058a65c>]
(check_unmap+0x6ec/0x824)
[  162.524463] [<c058a65c>] (check_unmap) from [<c058a7f8>]
(debug_dma_unmap_page+0x64/0x70)
[  162.532667] [<c058a7f8>] (debug_dma_unmap_page) from [<bf08dde0>]
(stmmac_tx_clean+0x260/0x494 [stmmac])
[  162.542242] [<bf08dde0>] (stmmac_tx_clean [stmmac]) from [<c025f988>]
(call_timer_fn+0xd0/0x310)
[  162.551025] [<c025f988>] (call_timer_fn) from [<c0260578>]
(run_timer_softirq+0x268/0x360)
[  162.559287] [<c0260578>] (run_timer_softirq) from [<c0258628>]
(__do_softirq+0x184/0x4d0)
[  162.567461] [<c0258628>] (__do_softirq) from [<c0258c58>]
(irq_exit+0x8c/0xfc)
[  162.574683] [<c0258c58>] (irq_exit) from [<c020f52c>]
(handle_IRQ+0x74/0x98)
[  162.581730] [<c020f52c>] (handle_IRQ) from [<c0208920>]
(gic_handle_irq+0x4c/0x70)
[  162.589299] [<c0208920>] (gic_handle_irq) from [<c0942b04>]
(__irq_svc+0x44/0x58)
[  162.596774] Exception stack(0xc0d95f58 to 0xc0d95fa0)
[  162.601823] 5f40:
00000001 00000004
[  162.609995] 5f60: 00000000 c0da2128 c0d94000 00000000 00000000
c0d915d0
00000000 ffffffed
[  162.618168] 5f80: c0d94000 c0d9de8c 00000000 c0d95fa0 c02a6d4c
c020f8bc
200f0013 ffffffff
[  162.626344] [<c0942b04>] (__irq_svc) from [<c020f8bc>]
(arch_cpu_idle+0x38/0x44)
[  162.633742] [<c020f8bc>] (arch_cpu_idle) from [<c029e780>]
(cpu_startup_entry+0x21c/0x6ac)
[  162.642008] [<c029e780>] (cpu_startup_entry) from [<c0cabb98>]
(start_kernel+0x374/0x3dc)
[  162.650176] ---[ end trace 70ec89df0de626f3 ]---
[  162.654787] Mapped at:
[  162.657145]  [<c07ed840>] dev_hard_start_xmit+0x350/0x8a0
[  162.662550]  [<c080fcf8>] sch_direct_xmit+0x74/0x264
[  162.667526]  [<c07ee1ec>] __dev_queue_xmit+0x45c/0x8e0
[  162.672675]  [<c08a8c84>] ip6_finish_output2+0x7cc/0xa80
[  162.677999]  [<c08d3a20>] mld_sendpack+0x3fc/0x808
This is caused by various calls to dma_map_single not being error checked.
A call to dma_map_single should look something like this:
Quoted 12 lines
Quoted 6 lines
	dma_handle = dma_map_single(dev, addr, size, direction);
	if (dma_mapping_error(dma_handle))
		/* do error */
I believe that the calls to skb_frag_dma_map should be error checked similarly, but I'm not 100% sure on that.
Yes. You should unwind your frags.
It's been done already in many places.
Quoted 3 lines
I looked into fixing this myself, but esp. for the jumbo frame paths fixing this looks like it is non trivial.

Quoted 8 lines

Hello Denis.
I have a patch that must be ported on top of the net.dev kernel and I will provide it asap.
For reference this is the patch I will rework, give me you opinion too.
http://git.stlinux.com/?p=stm/linux-stm.git;a=commit;h=755f606c7efc5d9c111fa2be00b6e9916e101920

Reviewed-by Denis Kirjanov <[email protected]>

Please note that you _do still ignore_ the dma_* return values

Quoted 16 lines

br
peppe
Quoted 10 lines
Quoted 9 lines
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Giuseppe CAVALLARO

2014-08-22 06:53 UTC #4 lore raw

Quoted 104 lines · Denis Kirjanov

Quoted 103 lines · Giuseppe CAVALLARO
Quoted 84 lines · Denis Kirjanov
Quoted 83 lines · Hans de Goede
Hi,
When booting a Fedora rawhide kernel, which is build with CONFIG_DMA_API_DEBUG enabled, I get the following oops:
[  162.398116] ------------[ cut here ]------------
[  162.402770] WARNING: CPU: 0 PID: 0 at lib/dma-debug.c:1140
check_unmap+0x6ec/0x824()
[  162.410507] stmmaceth 1c50000.ethernet: DMA-API: device driver failed
to
check map error[device address=0x0000000068a1d002] [size=90 bytes] [mapped
as single]
[  162.424480] Modules linked in: ip6t_rpfilter ip6t_REJECT xt_conntrack
cfg80211 rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables
iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack
iptable_mangle iptable_security iptable_raw stmmac ptp pps_core i2c_mv64xxx
sun4i_ts ohci_platform sunxi_sid rtc_sunxi phy_sun4i_usb sunxi_wdt leds_gpio
mmc_block sunxi_mmc mmc_core ehci_platform ahci_sunxi libahci_platform
[  162.476435] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
3.16.0-0.rc7.git1.1.fc22.armv7hl #1
[  162.484622] [<c02188dc>] (unwind_backtrace) from [<c0212bb0>]
(show_stack+0x18/0x1c)
[  162.492366] [<c0212bb0>] (show_stack) from [<c0938d20>]
(dump_stack+0x84/0xb0)
[  162.499590] [<c0938d20>] (dump_stack) from [<c0252444>]
(warn_slowpath_common+0x70/0x94)
[  162.507678] [<c0252444>] (warn_slowpath_common) from [<c025249c>]
(warn_slowpath_fmt+0x34/0x44)
[  162.516374] [<c025249c>] (warn_slowpath_fmt) from [<c058a65c>]
(check_unmap+0x6ec/0x824)
[  162.524463] [<c058a65c>] (check_unmap) from [<c058a7f8>]
(debug_dma_unmap_page+0x64/0x70)
[  162.532667] [<c058a7f8>] (debug_dma_unmap_page) from [<bf08dde0>]
(stmmac_tx_clean+0x260/0x494 [stmmac])
[  162.542242] [<bf08dde0>] (stmmac_tx_clean [stmmac]) from [<c025f988>]
(call_timer_fn+0xd0/0x310)
[  162.551025] [<c025f988>] (call_timer_fn) from [<c0260578>]
(run_timer_softirq+0x268/0x360)
[  162.559287] [<c0260578>] (run_timer_softirq) from [<c0258628>]
(__do_softirq+0x184/0x4d0)
[  162.567461] [<c0258628>] (__do_softirq) from [<c0258c58>]
(irq_exit+0x8c/0xfc)
[  162.574683] [<c0258c58>] (irq_exit) from [<c020f52c>]
(handle_IRQ+0x74/0x98)
[  162.581730] [<c020f52c>] (handle_IRQ) from [<c0208920>]
(gic_handle_irq+0x4c/0x70)
[  162.589299] [<c0208920>] (gic_handle_irq) from [<c0942b04>]
(__irq_svc+0x44/0x58)
[  162.596774] Exception stack(0xc0d95f58 to 0xc0d95fa0)
[  162.601823] 5f40:
00000001 00000004
[  162.609995] 5f60: 00000000 c0da2128 c0d94000 00000000 00000000
c0d915d0
00000000 ffffffed
[  162.618168] 5f80: c0d94000 c0d9de8c 00000000 c0d95fa0 c02a6d4c
c020f8bc
200f0013 ffffffff
[  162.626344] [<c0942b04>] (__irq_svc) from [<c020f8bc>]
(arch_cpu_idle+0x38/0x44)
[  162.633742] [<c020f8bc>] (arch_cpu_idle) from [<c029e780>]
(cpu_startup_entry+0x21c/0x6ac)
[  162.642008] [<c029e780>] (cpu_startup_entry) from [<c0cabb98>]
(start_kernel+0x374/0x3dc)
[  162.650176] ---[ end trace 70ec89df0de626f3 ]---
[  162.654787] Mapped at:
[  162.657145]  [<c07ed840>] dev_hard_start_xmit+0x350/0x8a0
[  162.662550]  [<c080fcf8>] sch_direct_xmit+0x74/0x264
[  162.667526]  [<c07ee1ec>] __dev_queue_xmit+0x45c/0x8e0
[  162.672675]  [<c08a8c84>] ip6_finish_output2+0x7cc/0xa80
[  162.677999]  [<c08d3a20>] mld_sendpack+0x3fc/0x808
This is caused by various calls to dma_map_single not being error checked.
A call to dma_map_single should look something like this:
Quoted 12 lines
Quoted 6 lines
	dma_handle = dma_map_single(dev, addr, size, direction);
	if (dma_mapping_error(dma_handle))
		/* do error */
I believe that the calls to skb_frag_dma_map should be error checked similarly, but I'm not 100% sure on that.
Yes. You should unwind your frags.
It's been done already in many places.
Quoted 3 lines
I looked into fixing this myself, but esp. for the jumbo frame paths fixing this looks like it is non trivial.
Hello Denis.
I have a patch that must be ported on top of the net.dev kernel and I will provide it asap.

Quoted 7 lines

Quoted 3 lines
For reference this is the patch I will rework, give me you opinion too.
http://git.stlinux.com/?p=stm/linux-stm.git;a=commit;h=755f606c7efc5d9c111fa2be00b6e9916e101920
Reviewed-by Denis Kirjanov <[email protected]>
Please note that you _do still ignore_ the dma_* return values

Ok Denis, I will take care of this too.

I will rework the patch and send it to this mailing list to be reviewed again.

br
Peppe

Quoted 18 lines

Quoted 16 lines
br
peppe
Quoted 10 lines
Quoted 9 lines
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Giuseppe Cavallaro

2014-08-27 09:27 UTC #5 lore raw

PATCH 1/1 stmmac: fix dma api misuse

Enabling DMA_API_DEBUG, warnings are reported at runtime because the device driver frees DMA memory with wrong functions and it does not call dma_mapping_error after mapping dma memory.

The first problem is fixed by of introducing a flag that helps us keeping track which mapping technique was used, so that we can use the right API for unmap.
This approach was inspired by the e1000 driver, which uses a similar technique.

Signed-off-by Andre Draszik <[email protected]>

Signed-off-by Giuseppe Cavallaro <[email protected]>

Reviewed-by Denis Kirjanov <[email protected]>

Cc Hans de Goede <[email protected]>

---
 drivers/net/ethernet/stmicro/stmmac/chain_mode.c  |   14 +++-
 drivers/net/ethernet/stmicro/stmmac/common.h      |    2 +-
 drivers/net/ethernet/stmicro/stmmac/ring_mode.c   |   15 +++-
 drivers/net/ethernet/stmicro/stmmac/stmmac.h      |    7 ++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |   68 +++++++++++++++-----
 5 files changed, 79 insertions(+), 27 deletions(-)

drivers/net/ethernet/stmicro/stmmac/chain_mode.c +10 -4

index c553f6b..cf28dab 100644
@@ -28,7 +28,7 @@
 
 #include "stmmac.h"
 
-static unsigned int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
+static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 {
 	struct stmmac_priv *priv = (struct stmmac_priv *)p;
 	unsigned int txsize = priv->dma_tx_size;

@@ -47,7 +47,9 @@ static unsigned int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 
 	desc->des2 = dma_map_single(priv->device, skb->data,
 				    bmax, DMA_TO_DEVICE);
-	priv->tx_skbuff_dma[entry] = desc->des2;
+	if (dma_mapping_error(priv->device, desc->des2))
+		return -1;
+	priv->tx_skbuff_dma[entry].buf = desc->des2;
 	priv->hw->desc->prepare_tx_desc(desc, 1, bmax, csum, STMMAC_CHAIN_MODE);
 
 	while (len != 0) {

@@ -59,7 +61,9 @@ static unsigned int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 			desc->des2 = dma_map_single(priv->device,
 						    (skb->data + bmax * i),
 						    bmax, DMA_TO_DEVICE);
-			priv->tx_skbuff_dma[entry] = desc->des2;
+			if (dma_mapping_error(priv->device, desc->des2))
+				return -1;
+			priv->tx_skbuff_dma[entry].buf = desc->des2;
 			priv->hw->desc->prepare_tx_desc(desc, 0, bmax, csum,
 							STMMAC_CHAIN_MODE);
 			priv->hw->desc->set_tx_owner(desc);

@@ -69,7 +73,9 @@ static unsigned int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 			desc->des2 = dma_map_single(priv->device,
 						    (skb->data + bmax * i), len,
 						    DMA_TO_DEVICE);
-			priv->tx_skbuff_dma[entry] = desc->des2;
+			if (dma_mapping_error(priv->device, desc->des2))
+				return -1;
+			priv->tx_skbuff_dma[entry].buf = desc->des2;
 			priv->hw->desc->prepare_tx_desc(desc, 0, len, csum,
 							STMMAC_CHAIN_MODE);
 			priv->hw->desc->set_tx_owner(desc);

drivers/net/ethernet/stmicro/stmmac/common.h +1 -1

index de507c3..bd54238 100644
@@ -425,7 +425,7 @@ struct stmmac_mode_ops {
 	void (*init) (void *des, dma_addr_t phy_addr, unsigned int size,
 		      unsigned int extend_desc);
 	unsigned int (*is_jumbo_frm) (int len, int ehn_desc);
-	unsigned int (*jumbo_frm) (void *priv, struct sk_buff *skb, int csum);
+	int (*jumbo_frm)(void *priv, struct sk_buff *skb, int csum);
 	int (*set_16kib_bfsize)(int mtu);
 	void (*init_desc3)(struct dma_desc *p);
 	void (*refill_desc3) (void *priv, struct dma_desc *p);

drivers/net/ethernet/stmicro/stmmac/ring_mode.c +11 -4

index 650a4be..5dd50c6 100644
@@ -28,7 +28,7 @@
 
 #include "stmmac.h"
 
-static unsigned int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
+static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 {
 	struct stmmac_priv *priv = (struct stmmac_priv *)p;
 	unsigned int txsize = priv->dma_tx_size;

@@ -53,7 +53,10 @@ static unsigned int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 
 		desc->des2 = dma_map_single(priv->device, skb->data,
 					    bmax, DMA_TO_DEVICE);
-		priv->tx_skbuff_dma[entry] = desc->des2;
+		if (dma_mapping_error(priv->device, desc->des2))
+			return -1;
+
+		priv->tx_skbuff_dma[entry].buf = desc->des2;
 		desc->des3 = desc->des2 + BUF_SIZE_4KiB;
 		priv->hw->desc->prepare_tx_desc(desc, 1, bmax, csum,
 						STMMAC_RING_MODE);

@@ -68,7 +71,9 @@ static unsigned int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 
 		desc->des2 = dma_map_single(priv->device, skb->data + bmax,
 					    len, DMA_TO_DEVICE);
-		priv->tx_skbuff_dma[entry] = desc->des2;
+		if (dma_mapping_error(priv->device, desc->des2))
+			return -1;
+		priv->tx_skbuff_dma[entry].buf = desc->des2;
 		desc->des3 = desc->des2 + BUF_SIZE_4KiB;
 		priv->hw->desc->prepare_tx_desc(desc, 0, len, csum,
 						STMMAC_RING_MODE);

@@ -77,7 +82,9 @@ static unsigned int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 	} else {
 		desc->des2 = dma_map_single(priv->device, skb->data,
 					    nopaged_len, DMA_TO_DEVICE);
-		priv->tx_skbuff_dma[entry] = desc->des2;
+		if (dma_mapping_error(priv->device, desc->des2))
+			return -1;
+		priv->tx_skbuff_dma[entry].buf = desc->des2;
 		desc->des3 = desc->des2 + BUF_SIZE_4KiB;
 		priv->hw->desc->prepare_tx_desc(desc, 1, nopaged_len, csum,
 						STMMAC_RING_MODE);

drivers/net/ethernet/stmicro/stmmac/stmmac.h +6 -1

index ca01035..b69c156 100644
@@ -34,6 +34,11 @@
 #include <linux/ptp_clock_kernel.h>
 #include <linux/reset.h>
 
+struct stmmac_tx_info {
+	dma_addr_t buf;
+	bool map_as_page;
+};
+
 struct stmmac_priv {
 	/* Frequently used values are kept adjacent for cache effect */
 	struct dma_extended_desc *dma_etx ____cacheline_aligned_in_smp;

@@ -45,7 +50,7 @@ struct stmmac_priv {
 	u32 tx_count_frames;
 	u32 tx_coal_frames;
 	u32 tx_coal_timer;
-	dma_addr_t *tx_skbuff_dma;
+	struct stmmac_tx_info *tx_skbuff_dma;
 	dma_addr_t dma_tx_phy;
 	int tx_coalesce;
 	int hwts_tx_en;

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +51 ~~-17~~

127 lines not on this page · raw message

Signature · 1 line

1.7.4.4

David Miller

maintainer Applied

2014-08-30 02:56 UTC #6 lore raw

Re: [PATCH (net.git)] stmmac: fix dma api misuse

From: Giuseppe Cavallaro <[email protected]>
Date: Wed, 27 Aug 2014 11:27:00 +0200

Quoted 10 lines

Enabling DMA_API_DEBUG, warnings are reported at runtime because the device driver frees DMA memory with wrong functions and it does not call dma_mapping_error after mapping dma memory.
The first problem is fixed by of introducing a flag that helps us keeping track which mapping technique was used, so that we can use the right API for unmap.
This approach was inspired by the e1000 driver, which uses a similar technique.

Quoted 4 lines

Signed-off-by Andre Draszik <[email protected]>
Signed-off-by Giuseppe Cavallaro <[email protected]>
Reviewed-by Denis Kirjanov <[email protected]>
Cc Hans de Goede <[email protected]>

Applied.