linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH resend 0/3] rtlwifi: don't call kfree_skb() under spin_lock_irqsave()
@ 2022-12-06 13:12 Yang Yingliang
  2022-12-06 13:12 ` [PATCH resend 1/3] rtlwifi: rtl8821ae: " Yang Yingliang
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Yang Yingliang @ 2022-12-06 13:12 UTC (permalink / raw)
  To: pkshih, kvalo; +Cc: linux-wireless, yangyingliang

It is not allowed to call kfree_skb() from hardware interrupt
context or with interrupts being disabled. This patchset is
trying to add all skb to a free list, then free them after
spin_unlock_irqrestore() at once.

Yang Yingliang (3):
  rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
  rtlwifi: rtl8188ee: don't call kfree_skb() under spin_lock_irqsave()
  rtlwifi: rtl8723be: don't call kfree_skb() under spin_lock_irqsave()

 drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c | 6 +++++-
 drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c | 6 +++++-
 drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 6 +++++-
 3 files changed, 15 insertions(+), 3 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH resend 1/3] rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
  2022-12-06 13:12 [PATCH resend 0/3] rtlwifi: don't call kfree_skb() under spin_lock_irqsave() Yang Yingliang
@ 2022-12-06 13:12 ` Yang Yingliang
  2022-12-07  3:31   ` Ping-Ke Shih
  2022-12-06 13:12 ` [PATCH resend 2/3] rtlwifi: rtl8188ee: " Yang Yingliang
  2022-12-06 13:12 ` [PATCH resend 3/3] rtlwifi: rtl8723be: " Yang Yingliang
  2 siblings, 1 reply; 9+ messages in thread
From: Yang Yingliang @ 2022-12-06 13:12 UTC (permalink / raw)
  To: pkshih, kvalo; +Cc: linux-wireless, yangyingliang

It is not allowed to call kfree_skb() from hardware interrupt
context or with interrupts being disabled. So add all skb to
a free list, then free them after spin_unlock_irqrestore() at
once.

Fixes: 5c99f04fec93 ("rtlwifi: rtl8723be: Update driver to match Realtek release of 06/28/14")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
index 7e0f62d59fe1..a7e3250957dc 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
@@ -26,8 +26,10 @@ static void _rtl8821ae_return_beacon_queue_skb(struct ieee80211_hw *hw)
 	struct rtl_priv *rtlpriv = rtl_priv(hw);
 	struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
 	struct rtl8192_tx_ring *ring = &rtlpci->tx_ring[BEACON_QUEUE];
+	struct sk_buff_head free_list;
 	unsigned long flags;
 
+	skb_queue_head_init(&free_list);
 	spin_lock_irqsave(&rtlpriv->locks.irq_th_lock, flags);
 	while (skb_queue_len(&ring->queue)) {
 		struct rtl_tx_desc *entry = &ring->desc[ring->idx];
@@ -37,10 +39,12 @@ static void _rtl8821ae_return_beacon_queue_skb(struct ieee80211_hw *hw)
 				 rtlpriv->cfg->ops->get_desc(hw, (u8 *)entry,
 						true, HW_DESC_TXBUFF_ADDR),
 				 skb->len, DMA_TO_DEVICE);
-		kfree_skb(skb);
+		__skb_queue_tail(&free_list, skb);
 		ring->idx = (ring->idx + 1) % ring->entries;
 	}
 	spin_unlock_irqrestore(&rtlpriv->locks.irq_th_lock, flags);
+
+	__skb_queue_purge(&free_list);
 }
 
 static void _rtl8821ae_set_bcn_ctrl_reg(struct ieee80211_hw *hw,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH resend 2/3] rtlwifi: rtl8188ee: don't call kfree_skb() under spin_lock_irqsave()
  2022-12-06 13:12 [PATCH resend 0/3] rtlwifi: don't call kfree_skb() under spin_lock_irqsave() Yang Yingliang
  2022-12-06 13:12 ` [PATCH resend 1/3] rtlwifi: rtl8821ae: " Yang Yingliang
@ 2022-12-06 13:12 ` Yang Yingliang
  2022-12-06 13:12 ` [PATCH resend 3/3] rtlwifi: rtl8723be: " Yang Yingliang
  2 siblings, 0 replies; 9+ messages in thread
From: Yang Yingliang @ 2022-12-06 13:12 UTC (permalink / raw)
  To: pkshih, kvalo; +Cc: linux-wireless, yangyingliang

It is not allowed to call kfree_skb() from hardware interrupt
context or with interrupts being disabled. So add all skb to
a free list, then free them after spin_unlock_irqrestore() at
once.

Fixes: 7fe3b3abb5da ("rtlwifi: rtl8188ee: rtl8821ae: Fix a queue locking problem")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c
index 58c2ab3d44be..de61c9c0ddec 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c
@@ -68,8 +68,10 @@ static void _rtl88ee_return_beacon_queue_skb(struct ieee80211_hw *hw)
 	struct rtl_priv *rtlpriv = rtl_priv(hw);
 	struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
 	struct rtl8192_tx_ring *ring = &rtlpci->tx_ring[BEACON_QUEUE];
+	struct sk_buff_head free_list;
 	unsigned long flags;
 
+	skb_queue_head_init(&free_list);
 	spin_lock_irqsave(&rtlpriv->locks.irq_th_lock, flags);
 	while (skb_queue_len(&ring->queue)) {
 		struct rtl_tx_desc *entry = &ring->desc[ring->idx];
@@ -79,10 +81,12 @@ static void _rtl88ee_return_beacon_queue_skb(struct ieee80211_hw *hw)
 				 rtlpriv->cfg->ops->get_desc(hw, (u8 *)entry,
 						true, HW_DESC_TXBUFF_ADDR),
 				 skb->len, DMA_TO_DEVICE);
-		kfree_skb(skb);
+		__skb_queue_tail(&free_list, skb);
 		ring->idx = (ring->idx + 1) % ring->entries;
 	}
 	spin_unlock_irqrestore(&rtlpriv->locks.irq_th_lock, flags);
+
+	__skb_queue_purge(&free_list);
 }
 
 static void _rtl88ee_disable_bcn_sub_func(struct ieee80211_hw *hw)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH resend 3/3] rtlwifi: rtl8723be: don't call kfree_skb() under spin_lock_irqsave()
  2022-12-06 13:12 [PATCH resend 0/3] rtlwifi: don't call kfree_skb() under spin_lock_irqsave() Yang Yingliang
  2022-12-06 13:12 ` [PATCH resend 1/3] rtlwifi: rtl8821ae: " Yang Yingliang
  2022-12-06 13:12 ` [PATCH resend 2/3] rtlwifi: rtl8188ee: " Yang Yingliang
@ 2022-12-06 13:12 ` Yang Yingliang
  2 siblings, 0 replies; 9+ messages in thread
From: Yang Yingliang @ 2022-12-06 13:12 UTC (permalink / raw)
  To: pkshih, kvalo; +Cc: linux-wireless, yangyingliang

It is not allowed to call kfree_skb() from hardware interrupt
context or with interrupts being disabled. So add all skb to
a free list, then free them after spin_unlock_irqrestore() at
once.

Fixes: 5c99f04fec93 ("rtlwifi: rtl8723be: Update driver to match Realtek release of 06/28/14")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c
index 189cc6437600..0ba3bbed6ed3 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c
@@ -30,8 +30,10 @@ static void _rtl8723be_return_beacon_queue_skb(struct ieee80211_hw *hw)
 	struct rtl_priv *rtlpriv = rtl_priv(hw);
 	struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
 	struct rtl8192_tx_ring *ring = &rtlpci->tx_ring[BEACON_QUEUE];
+	struct sk_buff_head free_list;
 	unsigned long flags;
 
+	skb_queue_head_init(&free_list);
 	spin_lock_irqsave(&rtlpriv->locks.irq_th_lock, flags);
 	while (skb_queue_len(&ring->queue)) {
 		struct rtl_tx_desc *entry = &ring->desc[ring->idx];
@@ -41,10 +43,12 @@ static void _rtl8723be_return_beacon_queue_skb(struct ieee80211_hw *hw)
 				 rtlpriv->cfg->ops->get_desc(hw, (u8 *)entry,
 						true, HW_DESC_TXBUFF_ADDR),
 				 skb->len, DMA_TO_DEVICE);
-		kfree_skb(skb);
+		__skb_queue_tail(&free_list, skb);
 		ring->idx = (ring->idx + 1) % ring->entries;
 	}
 	spin_unlock_irqrestore(&rtlpriv->locks.irq_th_lock, flags);
+
+	__skb_queue_purge(&free_list);
 }
 
 static void _rtl8723be_set_bcn_ctrl_reg(struct ieee80211_hw *hw,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* RE: [PATCH resend 1/3] rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
  2022-12-06 13:12 ` [PATCH resend 1/3] rtlwifi: rtl8821ae: " Yang Yingliang
@ 2022-12-07  3:31   ` Ping-Ke Shih
  2022-12-07  3:44     ` Yang Yingliang
  0 siblings, 1 reply; 9+ messages in thread
From: Ping-Ke Shih @ 2022-12-07  3:31 UTC (permalink / raw)
  To: Yang Yingliang, kvalo; +Cc: linux-wireless


> -----Original Message-----
> From: Yang Yingliang <yangyingliang@huawei.com>
> Sent: Tuesday, December 6, 2022 9:13 PM
> To: Ping-Ke Shih <pkshih@realtek.com>; kvalo@kernel.org
> Cc: linux-wireless@vger.kernel.org; yangyingliang@huawei.com
> Subject: [PATCH resend 1/3] rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
> 
> It is not allowed to call kfree_skb() from hardware interrupt
> context or with interrupts being disabled. So add all skb to
> a free list, then free them after spin_unlock_irqrestore() at
> once.

The patch doesn't change logic, so it should work. But, I would like to know
if there is a comment about this in kernel code. Could you point it out?

> 
> Fixes: 5c99f04fec93 ("rtlwifi: rtl8723be: Update driver to match Realtek release of 06/28/14")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>  drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
> b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
> index 7e0f62d59fe1..a7e3250957dc 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
> @@ -26,8 +26,10 @@ static void _rtl8821ae_return_beacon_queue_skb(struct ieee80211_hw *hw)
>  	struct rtl_priv *rtlpriv = rtl_priv(hw);
>  	struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
>  	struct rtl8192_tx_ring *ring = &rtlpci->tx_ring[BEACON_QUEUE];
> +	struct sk_buff_head free_list;
>  	unsigned long flags;
> 
> +	skb_queue_head_init(&free_list);
>  	spin_lock_irqsave(&rtlpriv->locks.irq_th_lock, flags);
>  	while (skb_queue_len(&ring->queue)) {
>  		struct rtl_tx_desc *entry = &ring->desc[ring->idx];
> @@ -37,10 +39,12 @@ static void _rtl8821ae_return_beacon_queue_skb(struct ieee80211_hw *hw)
>  				 rtlpriv->cfg->ops->get_desc(hw, (u8 *)entry,
>  						true, HW_DESC_TXBUFF_ADDR),
>  				 skb->len, DMA_TO_DEVICE);
> -		kfree_skb(skb);
> +		__skb_queue_tail(&free_list, skb);
>  		ring->idx = (ring->idx + 1) % ring->entries;
>  	}
>  	spin_unlock_irqrestore(&rtlpriv->locks.irq_th_lock, flags);
> +
> +	__skb_queue_purge(&free_list);
>  }
> 
>  static void _rtl8821ae_set_bcn_ctrl_reg(struct ieee80211_hw *hw,
> --
> 2.25.1
> 
> 
> ------Please consider the environment before printing this e-mail.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH resend 1/3] rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
  2022-12-07  3:31   ` Ping-Ke Shih
@ 2022-12-07  3:44     ` Yang Yingliang
  2022-12-07  3:52       ` Ping-Ke Shih
  0 siblings, 1 reply; 9+ messages in thread
From: Yang Yingliang @ 2022-12-07  3:44 UTC (permalink / raw)
  To: Ping-Ke Shih, kvalo; +Cc: linux-wireless, yangyingliang


On 2022/12/7 11:31, Ping-Ke Shih wrote:
>> -----Original Message-----
>> From: Yang Yingliang <yangyingliang@huawei.com>
>> Sent: Tuesday, December 6, 2022 9:13 PM
>> To: Ping-Ke Shih <pkshih@realtek.com>; kvalo@kernel.org
>> Cc: linux-wireless@vger.kernel.org; yangyingliang@huawei.com
>> Subject: [PATCH resend 1/3] rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
>>
>> It is not allowed to call kfree_skb() from hardware interrupt
>> context or with interrupts being disabled. So add all skb to
>> a free list, then free them after spin_unlock_irqrestore() at
>> once.
> The patch doesn't change logic, so it should work. But, I would like to know
> if there is a comment about this in kernel code. Could you point it out?
You can see comment of dev_kfree_skb_irq() in include/linux/netdevice.h
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/netdevice.h?h=v6.1-rc8

Thanks,
Yang
>
>> Fixes: 5c99f04fec93 ("rtlwifi: rtl8723be: Update driver to match Realtek release of 06/28/14")
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>> ---
>>   drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
>> b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
>> index 7e0f62d59fe1..a7e3250957dc 100644
>> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
>> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
>> @@ -26,8 +26,10 @@ static void _rtl8821ae_return_beacon_queue_skb(struct ieee80211_hw *hw)
>>   	struct rtl_priv *rtlpriv = rtl_priv(hw);
>>   	struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
>>   	struct rtl8192_tx_ring *ring = &rtlpci->tx_ring[BEACON_QUEUE];
>> +	struct sk_buff_head free_list;
>>   	unsigned long flags;
>>
>> +	skb_queue_head_init(&free_list);
>>   	spin_lock_irqsave(&rtlpriv->locks.irq_th_lock, flags);
>>   	while (skb_queue_len(&ring->queue)) {
>>   		struct rtl_tx_desc *entry = &ring->desc[ring->idx];
>> @@ -37,10 +39,12 @@ static void _rtl8821ae_return_beacon_queue_skb(struct ieee80211_hw *hw)
>>   				 rtlpriv->cfg->ops->get_desc(hw, (u8 *)entry,
>>   						true, HW_DESC_TXBUFF_ADDR),
>>   				 skb->len, DMA_TO_DEVICE);
>> -		kfree_skb(skb);
>> +		__skb_queue_tail(&free_list, skb);
>>   		ring->idx = (ring->idx + 1) % ring->entries;
>>   	}
>>   	spin_unlock_irqrestore(&rtlpriv->locks.irq_th_lock, flags);
>> +
>> +	__skb_queue_purge(&free_list);
>>   }
>>
>>   static void _rtl8821ae_set_bcn_ctrl_reg(struct ieee80211_hw *hw,
>> --
>> 2.25.1
>>
>>
>> ------Please consider the environment before printing this e-mail.
> .

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH resend 1/3] rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
  2022-12-07  3:44     ` Yang Yingliang
@ 2022-12-07  3:52       ` Ping-Ke Shih
  2022-12-07  4:41         ` Yang Yingliang
  0 siblings, 1 reply; 9+ messages in thread
From: Ping-Ke Shih @ 2022-12-07  3:52 UTC (permalink / raw)
  To: Yang Yingliang, kvalo; +Cc: linux-wireless


> -----Original Message-----
> From: Yang Yingliang <yangyingliang@huawei.com>
> Sent: Wednesday, December 7, 2022 11:44 AM
> To: Ping-Ke Shih <pkshih@realtek.com>; kvalo@kernel.org
> Cc: linux-wireless@vger.kernel.org; yangyingliang@huawei.com
> Subject: Re: [PATCH resend 1/3] rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
> 
> 
> On 2022/12/7 11:31, Ping-Ke Shih wrote:
> >> -----Original Message-----
> >> From: Yang Yingliang <yangyingliang@huawei.com>
> >> Sent: Tuesday, December 6, 2022 9:13 PM
> >> To: Ping-Ke Shih <pkshih@realtek.com>; kvalo@kernel.org
> >> Cc: linux-wireless@vger.kernel.org; yangyingliang@huawei.com
> >> Subject: [PATCH resend 1/3] rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
> >>
> >> It is not allowed to call kfree_skb() from hardware interrupt
> >> context or with interrupts being disabled. So add all skb to
> >> a free list, then free them after spin_unlock_irqrestore() at
> >> once.
> > The patch doesn't change logic, so it should work. But, I would like to know
> > if there is a comment about this in kernel code. Could you point it out?
> You can see comment of dev_kfree_skb_irq() in include/linux/netdevice.h
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/netdevice.h?h=v6
> .1-rc8
> 

It seems like we can replace kfree_skb() by dev_kfree_skb_irq(), right?
But your method is more efficient. Is that your point?

Ping-Ke


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH resend 1/3] rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
  2022-12-07  3:52       ` Ping-Ke Shih
@ 2022-12-07  4:41         ` Yang Yingliang
  2022-12-07  5:07           ` Ping-Ke Shih
  0 siblings, 1 reply; 9+ messages in thread
From: Yang Yingliang @ 2022-12-07  4:41 UTC (permalink / raw)
  To: Ping-Ke Shih, kvalo; +Cc: linux-wireless, yangyingliang


On 2022/12/7 11:52, Ping-Ke Shih wrote:
>> -----Original Message-----
>> From: Yang Yingliang <yangyingliang@huawei.com>
>> Sent: Wednesday, December 7, 2022 11:44 AM
>> To: Ping-Ke Shih <pkshih@realtek.com>; kvalo@kernel.org
>> Cc: linux-wireless@vger.kernel.org; yangyingliang@huawei.com
>> Subject: Re: [PATCH resend 1/3] rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
>>
>>
>> On 2022/12/7 11:31, Ping-Ke Shih wrote:
>>>> -----Original Message-----
>>>> From: Yang Yingliang <yangyingliang@huawei.com>
>>>> Sent: Tuesday, December 6, 2022 9:13 PM
>>>> To: Ping-Ke Shih <pkshih@realtek.com>; kvalo@kernel.org
>>>> Cc: linux-wireless@vger.kernel.org; yangyingliang@huawei.com
>>>> Subject: [PATCH resend 1/3] rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
>>>>
>>>> It is not allowed to call kfree_skb() from hardware interrupt
>>>> context or with interrupts being disabled. So add all skb to
>>>> a free list, then free them after spin_unlock_irqrestore() at
>>>> once.
>>> The patch doesn't change logic, so it should work. But, I would like to know
>>> if there is a comment about this in kernel code. Could you point it out?
>> You can see comment of dev_kfree_skb_irq() in include/linux/netdevice.h
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/netdevice.h?h=v6
>> .1-rc8
>>
> It seems like we can replace kfree_skb() by dev_kfree_skb_irq(), right?
> But your method is more efficient. Is that your point?
Yes, the SKBs have already been dequeued from the queue, so they can be 
freed together at once.

Thanks,
Yang
>
> Ping-Ke
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH resend 1/3] rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
  2022-12-07  4:41         ` Yang Yingliang
@ 2022-12-07  5:07           ` Ping-Ke Shih
  0 siblings, 0 replies; 9+ messages in thread
From: Ping-Ke Shih @ 2022-12-07  5:07 UTC (permalink / raw)
  To: Yang Yingliang, kvalo; +Cc: linux-wireless



> -----Original Message-----
> From: Yang Yingliang <yangyingliang@huawei.com>
> Sent: Wednesday, December 7, 2022 12:41 PM
> To: Ping-Ke Shih <pkshih@realtek.com>; kvalo@kernel.org
> Cc: linux-wireless@vger.kernel.org; yangyingliang@huawei.com
> Subject: Re: [PATCH resend 1/3] rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
> 
> On 2022/12/7 11:52, Ping-Ke Shih wrote:
> >> -----Original Message-----
> >> From: Yang Yingliang <yangyingliang@huawei.com>
> >> Sent: Wednesday, December 7, 2022 11:44 AM
> >> To: Ping-Ke Shih <pkshih@realtek.com>; kvalo@kernel.org
> >> Cc: linux-wireless@vger.kernel.org; yangyingliang@huawei.com
> >> Subject: Re: [PATCH resend 1/3] rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
> >>
> >>
> >> On 2022/12/7 11:31, Ping-Ke Shih wrote:
> >>>> -----Original Message-----
> >>>> From: Yang Yingliang <yangyingliang@huawei.com>
> >>>> Sent: Tuesday, December 6, 2022 9:13 PM
> >>>> To: Ping-Ke Shih <pkshih@realtek.com>; kvalo@kernel.org
> >>>> Cc: linux-wireless@vger.kernel.org; yangyingliang@huawei.com
> >>>> Subject: [PATCH resend 1/3] rtlwifi: rtl8821ae: don't call kfree_skb() under spin_lock_irqsave()
> >>>>
> >>>> It is not allowed to call kfree_skb() from hardware interrupt
> >>>> context or with interrupts being disabled. So add all skb to
> >>>> a free list, then free them after spin_unlock_irqrestore() at
> >>>> once.
> >>> The patch doesn't change logic, so it should work. But, I would like to know
> >>> if there is a comment about this in kernel code. Could you point it out?
> >> You can see comment of dev_kfree_skb_irq() in include/linux/netdevice.h
> >>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/netdevice.h?h=v6
> >> .1-rc8
> >>
> > It seems like we can replace kfree_skb() by dev_kfree_skb_irq(), right?
> > But your method is more efficient. Is that your point?
> Yes, the SKBs have already been dequeued from the queue, so they can be
> freed together at once.
> 

Thanks. This patchset is good to me. But, subject prefix should be "wifi: rtlwifi: ..."
if v2 is not a hard work to you. I suppose v2 can change nothing, so add my Acked-by:

Acked-by: Ping-Ke Shih <pkshih@realtek.com>



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-12-07  5:08 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-06 13:12 [PATCH resend 0/3] rtlwifi: don't call kfree_skb() under spin_lock_irqsave() Yang Yingliang
2022-12-06 13:12 ` [PATCH resend 1/3] rtlwifi: rtl8821ae: " Yang Yingliang
2022-12-07  3:31   ` Ping-Ke Shih
2022-12-07  3:44     ` Yang Yingliang
2022-12-07  3:52       ` Ping-Ke Shih
2022-12-07  4:41         ` Yang Yingliang
2022-12-07  5:07           ` Ping-Ke Shih
2022-12-06 13:12 ` [PATCH resend 2/3] rtlwifi: rtl8188ee: " Yang Yingliang
2022-12-06 13:12 ` [PATCH resend 3/3] rtlwifi: rtl8723be: " Yang Yingliang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).