From: Larry Finger <Larry.Finger@lwfinger.net>
To: Alexander Duyck <alexander.duyck@gmail.com>,
Wang YanQing <udknight@gmail.com>,
chaoming_li@realsil.com.cn, kvalo@codeaurora.org,
linux-wireless@vger.kernel.org, Netdev <netdev@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] rtlwifi: pci: use dev_kfree_skb_irq instead of kfree_skb in rtl_pci_reset_trx_ring
Date: Fri, 6 May 2016 13:01:20 -0500 [thread overview]
Message-ID: <572CDBF0.9000607@lwfinger.net> (raw)
In-Reply-To: <CAKgT0Uff5dc3_iEjK6GScqgP5_1kcg3YQAv7AH3bzuVDguXiPw@mail.gmail.com>
On 05/06/2016 12:13 PM, Alexander Duyck wrote:
> On Fri, May 6, 2016 at 9:33 AM, Wang YanQing <udknight@gmail.com> wrote:
>> We can't use kfree_skb in irq disable context, because spin_lock_irqsave
>> make sure we are always in irq disable context, use dev_kfree_skb_irq
>> instead of kfree_skb is better than dev_kfree_skb_any.
>>
>> This patch fix below kernel warning:
>> [ 7612.095528] ------------[ cut here ]------------
>> [ 7612.095546] WARNING: CPU: 3 PID: 4460 at kernel/softirq.c:150 __local_bh_enable_ip+0x58/0x80()
>> [ 7612.095550] Modules linked in: rtl8723be x86_pkg_temp_thermal btcoexist rtl_pci rtlwifi rtl8723_common
>> [ 7612.095567] CPU: 3 PID: 4460 Comm: ifconfig Tainted: G W 4.4.0+ #4
>> [ 7612.095570] Hardware name: LENOVO 20DFA04FCD/20DFA04FCD, BIOS J5ET48WW (1.19 ) 08/27/2015
>> [ 7612.095574] 00000000 00000000 da37fc70 c12ce7c5 00000000 da37fca0 c104cc59 c19d4454
>> [ 7612.095584] 00000003 0000116c c19d4784 00000096 c10508a8 c10508a8 00000200 c1b42400
>> [ 7612.095594] f29be780 da37fcb0 c104ccad 00000009 00000000 da37fcbc c10508a8 f21f08b8
>> [ 7612.095604] Call Trace:
>> [ 7612.095614] [<c12ce7c5>] dump_stack+0x41/0x5c
>> [ 7612.095620] [<c104cc59>] warn_slowpath_common+0x89/0xc0
>> [ 7612.095628] [<c10508a8>] ? __local_bh_enable_ip+0x58/0x80
>> [ 7612.095634] [<c10508a8>] ? __local_bh_enable_ip+0x58/0x80
>> [ 7612.095640] [<c104ccad>] warn_slowpath_null+0x1d/0x20
>> [ 7612.095646] [<c10508a8>] __local_bh_enable_ip+0x58/0x80
>> [ 7612.095653] [<c16b7d34>] destroy_conntrack+0x64/0xa0
>> [ 7612.095660] [<c16b300f>] nf_conntrack_destroy+0xf/0x20
>> [ 7612.095665] [<c1677565>] skb_release_head_state+0x55/0xa0
>> [ 7612.095670] [<c16775bb>] skb_release_all+0xb/0x20
>> [ 7612.095674] [<c167760b>] __kfree_skb+0xb/0x60
>> [ 7612.095679] [<c16776f0>] kfree_skb+0x30/0x70
>> [ 7612.095686] [<f81b869d>] ? rtl_pci_reset_trx_ring+0x22d/0x370 [rtl_pci]
>> [ 7612.095692] [<f81b869d>] rtl_pci_reset_trx_ring+0x22d/0x370 [rtl_pci]
>> [ 7612.095698] [<f81b87f9>] rtl_pci_start+0x19/0x190 [rtl_pci]
>> [ 7612.095705] [<f81970e6>] rtl_op_start+0x56/0x90 [rtlwifi]
>> [ 7612.095712] [<c17e3f16>] drv_start+0x36/0xc0
>> [ 7612.095717] [<c17f5ab3>] ieee80211_do_open+0x2d3/0x890
>> [ 7612.095725] [<c16820fe>] ? call_netdevice_notifiers_info+0x2e/0x60
>> [ 7612.095730] [<c17f60bd>] ieee80211_open+0x4d/0x50
>> [ 7612.095736] [<c16891b3>] __dev_open+0xa3/0x130
>> [ 7612.095742] [<c183fa53>] ? _raw_spin_unlock_bh+0x13/0x20
>> [ 7612.095748] [<c1689499>] __dev_change_flags+0x89/0x140
>> [ 7612.095753] [<c127c70d>] ? selinux_capable+0xd/0x10
>> [ 7612.095759] [<c1689589>] dev_change_flags+0x29/0x60
>> [ 7612.095765] [<c1700b93>] devinet_ioctl+0x553/0x670
>> [ 7612.095772] [<c12db758>] ? _copy_to_user+0x28/0x40
>> [ 7612.095777] [<c17018b5>] inet_ioctl+0x85/0xb0
>> [ 7612.095783] [<c166e647>] sock_ioctl+0x67/0x260
>> [ 7612.095788] [<c166e5e0>] ? sock_fasync+0x80/0x80
>> [ 7612.095795] [<c115c99b>] do_vfs_ioctl+0x6b/0x550
>> [ 7612.095800] [<c127c812>] ? selinux_file_ioctl+0x102/0x1e0
>> [ 7612.095807] [<c10a8914>] ? timekeeping_suspend+0x294/0x320
>> [ 7612.095813] [<c10a256a>] ? __hrtimer_run_queues+0x14a/0x210
>> [ 7612.095820] [<c1276e24>] ? security_file_ioctl+0x34/0x50
>> [ 7612.095827] [<c115cef0>] SyS_ioctl+0x70/0x80
>> [ 7612.095832] [<c1001804>] do_fast_syscall_32+0x84/0x120
>> [ 7612.095839] [<c183ff91>] sysenter_past_esp+0x36/0x55
>> [ 7612.095844] ---[ end trace 97e9c637a20e8348 ]---
>>
>> Signed-off-by: Wang YanQing <udknight@gmail.com>
>> Cc: Stable <stable@vger.kernel.org>
>> ---
>> Changes:
>> v1-v2:
>> 1: add a Cc to stable.
>>
>> drivers/net/wireless/realtek/rtlwifi/pci.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
>> index 1ac41b8..99a3a03 100644
>> --- a/drivers/net/wireless/realtek/rtlwifi/pci.c
>> +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
>> @@ -1572,7 +1572,7 @@ int rtl_pci_reset_trx_ring(struct ieee80211_hw *hw)
>> true,
>> HW_DESC_TXBUFF_ADDR),
>> skb->len, PCI_DMA_TODEVICE);
>> - kfree_skb(skb);
>> + dev_kfree_skb_irq(skb);
>> ring->idx = (ring->idx + 1) % ring->entries;
>> }
>> ring->idx = 0;
>
> Is this always called in IRQ context? You might be better off using
> dev_kfree_skb_any instead if this is something that can be called from
> net_device_ops since that way you avoid having to call into the Tx
> softirq cleanup routine to free the buffers later unless you really
> need it.
>
> - Alex
>
Alex,
Six lines below the change is a spin_unlock_irqrestore(), which is always
called. I believe that the patch is correct.
Larry
next prev parent reply other threads:[~2016-05-06 18:01 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-06 16:33 [PATCH v2] rtlwifi: pci: use dev_kfree_skb_irq instead of kfree_skb in rtl_pci_reset_trx_ring Wang YanQing
2016-05-06 17:04 ` Larry Finger
2016-05-06 17:13 ` Alexander Duyck
2016-05-06 18:01 ` Larry Finger [this message]
2016-05-06 18:05 ` Alexander Duyck
2016-05-06 18:05 ` Alexander Duyck
2016-05-11 18:59 ` [v2] rtlwifi: pci: use dev_kfree_skb_irq instead of kfree_skb inrtl_pci_reset_trx_ring Kalle Valo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=572CDBF0.9000607@lwfinger.net \
--to=larry.finger@lwfinger.net \
--cc=alexander.duyck@gmail.com \
--cc=chaoming_li@realsil.com.cn \
--cc=kvalo@codeaurora.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=udknight@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.