Re: [PATCH] staging:r8723bs: remove wrappers around skb_clone()
On 5/31/20 8:36 PM, Joe Perches wrote:
On Sun, 2020-05-31 at 20:28 +0300, Ivan Safonov wrote:
On 5/31/20 7:15 PM, Joe Perches wrote:
On Sun, 2020-05-31 at 19:08 +0300, Ivan Safonov wrote:
Wrappers around skb_clone() do not simplify the driver code.
[]
-inline struct sk_buff *_rtw_skb_clone(struct sk_buff *skb)
-{
- return skb_clone(skb, in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);
-}
-
[]
diff --git a/drivers/staging/rtl8723bs/os_dep/recv_linux.c
b/drivers/staging/rtl8723bs/os_dep/recv_linux.c
[]
@@ -110,7 +110,7 @@ void rtw_os_recv_indicate_pkt(struct adapter *padapter,
_pkt *pkt, struct rx_pkt
if (memcmp(pattrib->dst, myid(>eeprompriv),
ETH_ALEN)) {
if (bmcast) {
psta = rtw_get_bcmc_stainfo(padapter);
- pskb2 = rtw_skb_clone(pkt);
+ pskb2 = skb_clone(pkt, GFP_ATOMIC);
Why make every clone allocation GFP_ATOMIC ?
The rtw_os_recv_indicate_pkt() is always called from an interrupt handler.
It'd be better to indicate you know that in the changelog
as the subject and changelog just shows removing wrappers
and the patch code does not agree with that.
Yes, it's right.
Re: [PATCH] staging:r8723bs: remove wrappers around skb_clone()
On Sun, 2020-05-31 at 20:28 +0300, Ivan Safonov wrote:
> On 5/31/20 7:15 PM, Joe Perches wrote:
> > On Sun, 2020-05-31 at 19:08 +0300, Ivan Safonov wrote:
> > > Wrappers around skb_clone() do not simplify the driver code.
> > []
> > > -inline struct sk_buff *_rtw_skb_clone(struct sk_buff *skb)
> > > -{
> > > - return skb_clone(skb, in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);
> > > -}
> > > -
> > []
> > > diff --git a/drivers/staging/rtl8723bs/os_dep/recv_linux.c
> > > b/drivers/staging/rtl8723bs/os_dep/recv_linux.c
> > []
> > > @@ -110,7 +110,7 @@ void rtw_os_recv_indicate_pkt(struct adapter
> > > *padapter, _pkt *pkt, struct rx_pkt
> > > if (memcmp(pattrib->dst,
> > > myid(>eeprompriv), ETH_ALEN)) {
> > > if (bmcast) {
> > > psta =
> > > rtw_get_bcmc_stainfo(padapter);
> > > - pskb2 = rtw_skb_clone(pkt);
> > > + pskb2 = skb_clone(pkt, GFP_ATOMIC);
> >
> > Why make every clone allocation GFP_ATOMIC ?
>
> The rtw_os_recv_indicate_pkt() is always called from an interrupt handler.
It'd be better to indicate you know that in the changelog
as the subject and changelog just shows removing wrappers
and the patch code does not agree with that.
Re: [PATCH] staging:r8723bs: remove wrappers around skb_clone()
On 5/31/20 7:15 PM, Joe Perches wrote:
On Sun, 2020-05-31 at 19:08 +0300, Ivan Safonov wrote:
Wrappers around skb_clone() do not simplify the driver code.
[]
-inline struct sk_buff *_rtw_skb_clone(struct sk_buff *skb)
-{
- return skb_clone(skb, in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);
-}
-
[]
diff --git a/drivers/staging/rtl8723bs/os_dep/recv_linux.c
b/drivers/staging/rtl8723bs/os_dep/recv_linux.c
[]
@@ -110,7 +110,7 @@ void rtw_os_recv_indicate_pkt(struct adapter *padapter,
_pkt *pkt, struct rx_pkt
if (memcmp(pattrib->dst, myid(>eeprompriv),
ETH_ALEN)) {
if (bmcast) {
psta = rtw_get_bcmc_stainfo(padapter);
- pskb2 = rtw_skb_clone(pkt);
+ pskb2 = skb_clone(pkt, GFP_ATOMIC);
Why make every clone allocation GFP_ATOMIC ?
The rtw_os_recv_indicate_pkt() is always called from an interrupt handler.
Re: [PATCH] staging:r8723bs: remove wrappers around skb_clone()
On Sun, 2020-05-31 at 19:08 +0300, Ivan Safonov wrote:
> Wrappers around skb_clone() do not simplify the driver code.
[]
> -inline struct sk_buff *_rtw_skb_clone(struct sk_buff *skb)
> -{
> - return skb_clone(skb, in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);
> -}
> -
[]
> diff --git a/drivers/staging/rtl8723bs/os_dep/recv_linux.c
> b/drivers/staging/rtl8723bs/os_dep/recv_linux.c
[]
> @@ -110,7 +110,7 @@ void rtw_os_recv_indicate_pkt(struct adapter *padapter,
> _pkt *pkt, struct rx_pkt
> if (memcmp(pattrib->dst, myid(>eeprompriv),
> ETH_ALEN)) {
> if (bmcast) {
> psta = rtw_get_bcmc_stainfo(padapter);
> - pskb2 = rtw_skb_clone(pkt);
> + pskb2 = skb_clone(pkt, GFP_ATOMIC);
Why make every clone allocation GFP_ATOMIC ?
[PATCH] staging:r8723bs: remove wrappers around skb_clone()
Wrappers around skb_clone() do not simplify the driver code.
Signed-off-by: Ivan Safonov
---
drivers/staging/rtl8723bs/include/osdep_service.h | 3 ---
drivers/staging/rtl8723bs/os_dep/osdep_service.c | 5 -
drivers/staging/rtl8723bs/os_dep/recv_linux.c | 2 +-
3 files changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/staging/rtl8723bs/include/osdep_service.h
b/drivers/staging/rtl8723bs/include/osdep_service.h
index 5f681899bbec..be34e279670b 100644
--- a/drivers/staging/rtl8723bs/include/osdep_service.h
+++ b/drivers/staging/rtl8723bs/include/osdep_service.h
@@ -94,7 +94,6 @@ void _kfree(u8 *pbuf, u32 sz);
struct sk_buff *_rtw_skb_alloc(u32 sz);
struct sk_buff *_rtw_skb_copy(const struct sk_buff *skb);
-struct sk_buff *_rtw_skb_clone(struct sk_buff *skb);
int _rtw_netif_rx(_nic_hdl ndev, struct sk_buff *skb);
#define rtw_malloc(sz) _rtw_malloc((sz))
@@ -103,9 +102,7 @@ int _rtw_netif_rx(_nic_hdl ndev, struct sk_buff *skb);
#define rtw_skb_alloc(size) _rtw_skb_alloc((size))
#define rtw_skb_alloc_f(size, mstat_f) _rtw_skb_alloc((size))
#define rtw_skb_copy(skb) _rtw_skb_copy((skb))
-#define rtw_skb_clone(skb) _rtw_skb_clone((skb))
#define rtw_skb_copy_f(skb, mstat_f) _rtw_skb_copy((skb))
-#define rtw_skb_clone_f(skb, mstat_f) _rtw_skb_clone((skb))
#define rtw_netif_rx(ndev, skb) _rtw_netif_rx(ndev, skb)
extern void _rtw_init_queue(struct __queue *pqueue);
diff --git a/drivers/staging/rtl8723bs/os_dep/osdep_service.c
b/drivers/staging/rtl8723bs/os_dep/osdep_service.c
index 4238209ec175..6d443197a0cf 100644
--- a/drivers/staging/rtl8723bs/os_dep/osdep_service.c
+++ b/drivers/staging/rtl8723bs/os_dep/osdep_service.c
@@ -47,11 +47,6 @@ inline struct sk_buff *_rtw_skb_copy(const struct sk_buff
*skb)
return skb_copy(skb, in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);
}
-inline struct sk_buff *_rtw_skb_clone(struct sk_buff *skb)
-{
- return skb_clone(skb, in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);
-}
-
inline int _rtw_netif_rx(_nic_hdl ndev, struct sk_buff *skb)
{
skb->dev = ndev;
diff --git a/drivers/staging/rtl8723bs/os_dep/recv_linux.c
b/drivers/staging/rtl8723bs/os_dep/recv_linux.c
index eb4d1c3008fe..b2a1bbb30df6 100644
--- a/drivers/staging/rtl8723bs/os_dep/recv_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/recv_linux.c
@@ -110,7 +110,7 @@ void rtw_os_recv_indicate_pkt(struct adapter *padapter,
_pkt *pkt, struct rx_pkt
if (memcmp(pattrib->dst, myid(>eeprompriv),
ETH_ALEN)) {
if (bmcast) {
psta = rtw_get_bcmc_stainfo(padapter);
- pskb2 = rtw_skb_clone(pkt);
+ pskb2 = skb_clone(pkt, GFP_ATOMIC);
} else {
psta = rtw_get_stainfo(pstapriv,
pattrib->dst);
}
--
2.26.2
