* [PATCH net-next 01/13] net: Check skb for NULL in dev_kfree_skb_irq
2013-11-02 13:47 [PATCH net-next 00/13] Protect dev_kfree_skb_irq from NULL and remove unnecessary NULL checks Govindarajulu Varadarajan
@ 2013-11-02 13:47 ` Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 02/13] driver: net: remove unnecessary skb NULL check before calling dev_kfree_skb_irq Govindarajulu Varadarajan
` (11 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Govindarajulu Varadarajan @ 2013-11-02 13:47 UTC (permalink / raw)
To: davem, gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
Cc: Govindarajulu Varadarajan
Signed-off-by: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
---
net/core/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 0054c8c..63bd44d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2146,7 +2146,7 @@ EXPORT_SYMBOL(__netif_schedule);
void dev_kfree_skb_irq(struct sk_buff *skb)
{
- if (atomic_dec_and_test(&skb->users)) {
+ if (skb && atomic_dec_and_test(&skb->users)) {
struct softnet_data *sd;
unsigned long flags;
--
1.8.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 02/13] driver: net: remove unnecessary skb NULL check before calling dev_kfree_skb_irq
2013-11-02 13:47 [PATCH net-next 00/13] Protect dev_kfree_skb_irq from NULL and remove unnecessary NULL checks Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 01/13] net: Check skb for NULL in dev_kfree_skb_irq Govindarajulu Varadarajan
@ 2013-11-02 13:47 ` Govindarajulu Varadarajan
2013-11-04 20:12 ` David Miller
2013-11-02 13:47 ` [PATCH net-next 03/13] driver: atm: " Govindarajulu Varadarajan
` (10 subsequent siblings)
12 siblings, 1 reply; 20+ messages in thread
From: Govindarajulu Varadarajan @ 2013-11-02 13:47 UTC (permalink / raw)
To: davem, gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
Cc: Govindarajulu Varadarajan
dev_kfree_skb_irq is protected from NULL. No need to check for NULL
while calling this function.
Signed-off-by: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
---
drivers/net/ethernet/amd/ni65.c | 6 ++----
drivers/net/ethernet/atheros/atl1c/atl1c_main.c | 10 ++++------
drivers/net/ethernet/atheros/atl1e/atl1e_main.c | 6 ++----
drivers/net/ethernet/atheros/atlx/atl1.c | 6 ++----
drivers/net/ethernet/icplus/ipg.c | 6 ++----
drivers/net/ethernet/intel/e1000e/netdev.c | 3 +--
drivers/net/ethernet/marvell/pxa168_eth.c | 3 +--
drivers/net/ethernet/xilinx/ll_temac_main.c | 3 +--
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 6 ++----
drivers/net/hamradio/scc.c | 10 +++-------
drivers/net/vmxnet3/vmxnet3_drv.c | 3 +--
drivers/net/wireless/ath/ar5523/ar5523.c | 6 ++----
12 files changed, 23 insertions(+), 45 deletions(-)
diff --git a/drivers/net/ethernet/amd/ni65.c b/drivers/net/ethernet/amd/ni65.c
index 1cf33ad..0695ce2 100644
--- a/drivers/net/ethernet/amd/ni65.c
+++ b/drivers/net/ethernet/amd/ni65.c
@@ -1030,10 +1030,8 @@ static void ni65_xmit_intr(struct net_device *dev,int csr0)
}
#ifdef XMT_VIA_SKB
- if(p->tmd_skb[p->tmdlast]) {
- dev_kfree_skb_irq(p->tmd_skb[p->tmdlast]);
- p->tmd_skb[p->tmdlast] = NULL;
- }
+ dev_kfree_skb_irq(p->tmd_skb[p->tmdlast]);
+ p->tmd_skb[p->tmdlast] = NULL;
#endif
p->tmdlast = (p->tmdlast + 1) & (TMDNUM-1);
diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
index a36a760..cf89008 100644
--- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
+++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
@@ -848,12 +848,10 @@ static inline void atl1c_clean_buffer(struct pci_dev *pdev,
pci_unmap_page(pdev, buffer_info->dma,
buffer_info->length, pci_driection);
}
- if (buffer_info->skb) {
- if (in_irq)
- dev_kfree_skb_irq(buffer_info->skb);
- else
- dev_kfree_skb(buffer_info->skb);
- }
+ if (in_irq)
+ dev_kfree_skb_irq(buffer_info->skb);
+ else
+ dev_kfree_skb(buffer_info->skb);
buffer_info->dma = 0;
buffer_info->skb = NULL;
ATL1C_SET_BUFFER_STATE(buffer_info, ATL1C_BUFFER_FREE);
diff --git a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
index 7a73f3a..c3ca8c6 100644
--- a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
+++ b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
@@ -1257,10 +1257,8 @@ static bool atl1e_clean_tx_irq(struct atl1e_adapter *adapter)
tx_buffer->dma = 0;
}
- if (tx_buffer->skb) {
- dev_kfree_skb_irq(tx_buffer->skb);
- tx_buffer->skb = NULL;
- }
+ dev_kfree_skb_irq(tx_buffer->skb);
+ tx_buffer->skb = NULL;
if (++next_to_clean == tx_ring->count)
next_to_clean = 0;
diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index 538211d..b8cc654 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -2082,10 +2082,8 @@ static int atl1_intr_tx(struct atl1_adapter *adapter)
buffer_info->dma = 0;
}
- if (buffer_info->skb) {
- dev_kfree_skb_irq(buffer_info->skb);
- buffer_info->skb = NULL;
- }
+ dev_kfree_skb_irq(buffer_info->skb);
+ buffer_info->skb = NULL;
if (++sw_tpd_next_to_clean == tpd_ring->count)
sw_tpd_next_to_clean = 0;
diff --git a/drivers/net/ethernet/icplus/ipg.c b/drivers/net/ethernet/icplus/ipg.c
index 25045ae..d3a5e1f 100644
--- a/drivers/net/ethernet/icplus/ipg.c
+++ b/drivers/net/ethernet/icplus/ipg.c
@@ -823,10 +823,8 @@ static void init_tfdlist(struct net_device *dev)
txfd->tfc = cpu_to_le64(IPG_TFC_TFDDONE);
- if (sp->tx_buff[i]) {
- dev_kfree_skb_irq(sp->tx_buff[i]);
- sp->tx_buff[i] = NULL;
- }
+ dev_kfree_skb_irq(sp->tx_buff[i]);
+ sp->tx_buff[i] = NULL;
txfd->next_desc = cpu_to_le64(sp->txd_map +
sizeof(struct ipg_tx)*(i + 1));
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 4ef7867..4c22a1a 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1526,8 +1526,7 @@ static bool e1000_clean_jumbo_rx_irq(struct e1000_ring *rx_ring, int *work_done,
/* recycle both page and skb */
buffer_info->skb = skb;
/* an error means any chain goes out the window too */
- if (rx_ring->rx_skb_top)
- dev_kfree_skb_irq(rx_ring->rx_skb_top);
+ dev_kfree_skb_irq(rx_ring->rx_skb_top);
rx_ring->rx_skb_top = NULL;
goto next_desc;
}
diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index fff6246..7443b11 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -749,8 +749,7 @@ static int txq_reclaim(struct net_device *dev, int force)
dev->stats.tx_errors++;
}
dma_unmap_single(NULL, addr, count, DMA_TO_DEVICE);
- if (skb)
- dev_kfree_skb_irq(skb);
+ dev_kfree_skb_irq(skb);
released++;
}
txq_reclaim_end:
diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
index 0029148..3d21741 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -629,8 +629,7 @@ static void temac_start_xmit_done(struct net_device *ndev)
while (stat & STS_CTRL_APP0_CMPLT) {
dma_unmap_single(ndev->dev.parent, cur_p->phys, cur_p->len,
DMA_TO_DEVICE);
- if (cur_p->app4)
- dev_kfree_skb_irq((struct sk_buff *)cur_p->app4);
+ dev_kfree_skb_irq((struct sk_buff *)cur_p->app4);
cur_p->app0 = 0;
cur_p->app1 = 0;
cur_p->app2 = 0;
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index b2ff038..f3bcabb 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -589,8 +589,7 @@ static void axienet_start_xmit_done(struct net_device *ndev)
dma_unmap_single(ndev->dev.parent, cur_p->phys,
(cur_p->cntrl & XAXIDMA_BD_CTRL_LENGTH_MASK),
DMA_TO_DEVICE);
- if (cur_p->app4)
- dev_kfree_skb_irq((struct sk_buff *)cur_p->app4);
+ dev_kfree_skb_irq((struct sk_buff *)cur_p->app4);
/*cur_p->phys = 0;*/
cur_p->app0 = 0;
cur_p->app1 = 0;
@@ -1367,8 +1366,7 @@ static void axienet_dma_err_handler(unsigned long data)
(cur_p->cntrl &
XAXIDMA_BD_CTRL_LENGTH_MASK),
DMA_TO_DEVICE);
- if (cur_p->app4)
- dev_kfree_skb_irq((struct sk_buff *) cur_p->app4);
+ dev_kfree_skb_irq((struct sk_buff *) cur_p->app4);
cur_p->phys = 0;
cur_p->cntrl = 0;
cur_p->status = 0;
diff --git a/drivers/net/hamradio/scc.c b/drivers/net/hamradio/scc.c
index 4bc6ee8..0abd1e3 100644
--- a/drivers/net/hamradio/scc.c
+++ b/drivers/net/hamradio/scc.c
@@ -496,11 +496,8 @@ static inline void scc_exint(struct scc_channel *scc)
scc->stat.tx_under++; /* oops, an underrun! count 'em */
Outb(scc->ctrl, RES_EXT_INT); /* reset ext/status interrupts */
- if (scc->tx_buff != NULL)
- {
- dev_kfree_skb_irq(scc->tx_buff);
- scc->tx_buff = NULL;
- }
+ dev_kfree_skb_irq(scc->tx_buff);
+ scc->tx_buff = NULL;
or(scc,R10,ABUNDER);
scc_start_tx_timer(scc, t_txdelay, 0); /* restart transmission */
@@ -577,8 +574,7 @@ static inline void scc_spint(struct scc_channel *scc)
scc->stat.rx_over++; /* count them */
or(scc,R3,ENT_HM); /* enter hunt mode for next flag */
- if (skb != NULL)
- dev_kfree_skb_irq(skb);
+ dev_kfree_skb_irq(skb);
scc->rx_buff = skb = NULL;
}
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 7e2788c..64ca248 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -1142,8 +1142,7 @@ vmxnet3_rx_error(struct vmxnet3_rx_queue *rq, struct Vmxnet3_RxCompDesc *rcd,
* ctx->skb may be NULL if this is the first and the only one
* desc for the pkt
*/
- if (ctx->skb)
- dev_kfree_skb_irq(ctx->skb);
+ dev_kfree_skb_irq(ctx->skb);
ctx->skb = NULL;
}
diff --git a/drivers/net/wireless/ath/ar5523/ar5523.c b/drivers/net/wireless/ath/ar5523/ar5523.c
index 280fc3d..cb0dd61 100644
--- a/drivers/net/wireless/ath/ar5523/ar5523.c
+++ b/drivers/net/wireless/ath/ar5523/ar5523.c
@@ -601,10 +601,8 @@ static void ar5523_data_rx_cb(struct urb *urb)
data->skb = NULL;
skip:
- if (data->skb) {
- dev_kfree_skb_irq(data->skb);
- data->skb = NULL;
- }
+ dev_kfree_skb_irq(data->skb);
+ data->skb = NULL;
ar5523_rx_data_put(ar, data);
if (atomic_inc_return(&ar->rx_data_free_cnt) >=
--
1.8.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH net-next 02/13] driver: net: remove unnecessary skb NULL check before calling dev_kfree_skb_irq
2013-11-02 13:47 ` [PATCH net-next 02/13] driver: net: remove unnecessary skb NULL check before calling dev_kfree_skb_irq Govindarajulu Varadarajan
@ 2013-11-04 20:12 ` David Miller
2013-11-11 10:31 ` Govindarajulu Varadarajan
0 siblings, 1 reply; 20+ messages in thread
From: David Miller @ 2013-11-04 20:12 UTC (permalink / raw)
To: govindarajulu90
Cc: gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
From: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
Date: Sat, 2 Nov 2013 19:17:43 +0530
> @@ -1030,10 +1030,8 @@ static void ni65_xmit_intr(struct net_device *dev,int csr0)
> }
>
> #ifdef XMT_VIA_SKB
> - if(p->tmd_skb[p->tmdlast]) {
> - dev_kfree_skb_irq(p->tmd_skb[p->tmdlast]);
> - p->tmd_skb[p->tmdlast] = NULL;
> - }
> + dev_kfree_skb_irq(p->tmd_skb[p->tmdlast]);
> + p->tmd_skb[p->tmdlast] = NULL;
> #endif
I absolutely disagree with this kind of change.
There is a non-trivial cost for NULL'ing out that array entry
unconditionally. It's a dirtied cache line and this is in the
fast path of TX SKB reclaim of this driver.
You've made several changes of this kind.
And it sort-of shows that the places that do check for NULL,
are getting something in return for that test, namely avoidance
of an unnecessary cpu store in the fast path of the driver.
I'm throwing away this series, sorry.
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH net-next 02/13] driver: net: remove unnecessary skb NULL check before calling dev_kfree_skb_irq
2013-11-04 20:12 ` David Miller
@ 2013-11-11 10:31 ` Govindarajulu Varadarajan
2013-11-18 19:26 ` Govindarajulu Varadarajan
0 siblings, 1 reply; 20+ messages in thread
From: Govindarajulu Varadarajan @ 2013-11-11 10:31 UTC (permalink / raw)
To: David Miller
Cc: govindarajulu90, gregkh, linux-usb, linux-kernel, schwidefsky,
linville, linux-wireless, netdev, IvDoorn, sbhatewara, samuel,
chas, roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
On Mon, 4 Nov 2013, David Miller wrote:
> From: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
> Date: Sat, 2 Nov 2013 19:17:43 +0530
>
>> @@ -1030,10 +1030,8 @@ static void ni65_xmit_intr(struct net_device *dev,int csr0)
>> }
>>
>> #ifdef XMT_VIA_SKB
>> - if(p->tmd_skb[p->tmdlast]) {
>> - dev_kfree_skb_irq(p->tmd_skb[p->tmdlast]);
>> - p->tmd_skb[p->tmdlast] = NULL;
>> - }
>> + dev_kfree_skb_irq(p->tmd_skb[p->tmdlast]);
>> + p->tmd_skb[p->tmdlast] = NULL;
>> #endif
>
> I absolutely disagree with this kind of change.
>
> There is a non-trivial cost for NULL'ing out that array entry
> unconditionally. It's a dirtied cache line and this is in the
> fast path of TX SKB reclaim of this driver.
>
> You've made several changes of this kind.
>
> And it sort-of shows that the places that do check for NULL,
> are getting something in return for that test, namely avoidance
> of an unnecessary cpu store in the fast path of the driver.
>
True, in case of dev_kfree_skb_irq. If you look at patch 06-12, at many
places we do
if (s->skb) {
dev_kfree_skb_any(s->skb);
s->skb = NULL)
}
This is in fast path. If the code is not running in hardirq,
dev_kfree_skb_any calls dev_kfree_skb. Which again check if skb is NULL.
So we are checking if skb is null twice. That is what this patch is
trying to fix. (sorry I should have mentioned this in cover letter).
I am not sure if you have read my previous mail. I am pasting it below.
>> On Sun, 3 Nov 2013, Brandeburg, Jesse wrote:
>> Thanks for this work, I'm a little concerned that there is a
>> non-trivial
>> overhead to this patch.
>>
>> when doing (for example in the Intel drivers):
>> if (s->skb) {
>> dev_kfree_skb(s->skb);
>> s->skb = NULL;
>> }
>>
>
>In current code, dev_kfree_skb is NULL safe. Which means skb is
>checked for NULL inside dev_kfree_skb. dev_kfree_skb_any is also NULL safe
>when the code is running in non-hardirq.
>
>Lets consider two cases
>
>1. skb is not NULL:
> * Without my patch:
> In the code above, we check for skb!=NULL twice. (once
> before calling dev_kfree_skb, once by dev_kfree_skb). And
> then we do assignment.
> * With this patch:
> we check for skb!=NULL once, And then we do assignment.
>
> To fix the twice NULL check, we either have to remove the check
> which is inside dev_kfree_skb (1). Or do whats done in this
> patch.
>
> (1) is not an option because a lot of kernel code already
> assumes that dev_kfree_skb is NULL safe.
>
>2. skb is NULL:
> * Without this patch:
> One if statement is executed.
> * With this patch:
> One if statement and one assignment is executed.
>
> From my observation most of the dev_kfree_skb calls are from
> e1000_unmap_and_free_tx_resource, e1000_put_txbuf,
> atl1_clean_tx_ring, alx_free_txbuf etc. in clean up functions.
>
> Is is quite unlikely thats skb is NULL. So it comes down to one extra
> if-branching statement or one extra assignment. I would prefer extra
> assignment to branching statement. In my opinion extra assignment is
> very little price we pay.
>
> //govind
Another way to solve the double NULL check is to define a new function
something like this
dev_kfree_skb_NULL(struct sk_buff **skb)
{
if(*skb) {
free_skb(*skb);
*skb=NULL;
}
}
and use this if you want to free a skb and make it NULL.
Is this approach better?
//govind
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH net-next 02/13] driver: net: remove unnecessary skb NULL check before calling dev_kfree_skb_irq
2013-11-11 10:31 ` Govindarajulu Varadarajan
@ 2013-11-18 19:26 ` Govindarajulu Varadarajan
2013-11-18 19:37 ` Johannes Berg
2013-11-18 20:17 ` David Miller
0 siblings, 2 replies; 20+ messages in thread
From: Govindarajulu Varadarajan @ 2013-11-18 19:26 UTC (permalink / raw)
To: David Miller
Cc: Govindarajulu Varadarajan, gregkh, linux-usb, linux-kernel,
schwidefsky, linville, linux-wireless, netdev, IvDoorn,
sbhatewara, samuel, chas, roland, isdn, jcliburn,
Christian Benvenuti (benve), Sujith Sankar (ssujith),
jeffrey.t.kirsher, jesse.brandeburg, shahed.shaikh, joe, apw
Hi Dave
Did you have a chance to look at this? Let me know how you want me to
fix this.
//govind
On Mon, 11 Nov 2013, Govindarajulu Varadarajan wrote:
>
>
> On Mon, 4 Nov 2013, David Miller wrote:
>
>> From: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
>> Date: Sat, 2 Nov 2013 19:17:43 +0530
>>
>>> @@ -1030,10 +1030,8 @@ static void ni65_xmit_intr(struct net_device
>>> *dev,int csr0)
>>> }
>>>
>>> #ifdef XMT_VIA_SKB
>>> - if(p->tmd_skb[p->tmdlast]) {
>>> - dev_kfree_skb_irq(p->tmd_skb[p->tmdlast]);
>>> - p->tmd_skb[p->tmdlast] = NULL;
>>> - }
>>> + dev_kfree_skb_irq(p->tmd_skb[p->tmdlast]);
>>> + p->tmd_skb[p->tmdlast] = NULL;
>>> #endif
>>
>> I absolutely disagree with this kind of change.
>>
>> There is a non-trivial cost for NULL'ing out that array entry
>> unconditionally. It's a dirtied cache line and this is in the
>> fast path of TX SKB reclaim of this driver.
>>
>> You've made several changes of this kind.
>>
>> And it sort-of shows that the places that do check for NULL,
>> are getting something in return for that test, namely avoidance
>> of an unnecessary cpu store in the fast path of the driver.
>>
>
> True, in case of dev_kfree_skb_irq. If you look at patch 06-12, at many
> places we do
>
> if (s->skb) {
> dev_kfree_skb_any(s->skb);
> s->skb = NULL)
> }
>
> This is in fast path. If the code is not running in hardirq,
> dev_kfree_skb_any calls dev_kfree_skb. Which again check if skb is NULL.
> So we are checking if skb is null twice. That is what this patch is
> trying to fix. (sorry I should have mentioned this in cover letter).
>
> I am not sure if you have read my previous mail. I am pasting it below.
>
>>> On Sun, 3 Nov 2013, Brandeburg, Jesse wrote: Thanks for this work, I'm a
>>> little concerned that there is a
>>> non-trivial overhead to this patch.
>>>
>>> when doing (for example in the Intel drivers): if (s->skb) {
>>> dev_kfree_skb(s->skb);
>>> s->skb = NULL; }
>>>
>>
>> In current code, dev_kfree_skb is NULL safe. Which means skb is
>> checked for NULL inside dev_kfree_skb. dev_kfree_skb_any is also NULL safe
>> when the code is running in non-hardirq.
>>
>> Lets consider two cases
>>
>> 1. skb is not NULL:
>> * Without my patch:
>> In the code above, we check for skb!=NULL twice. (once
>> before calling dev_kfree_skb, once by dev_kfree_skb). And
>> then we do assignment.
>> * With this patch:
>> we check for skb!=NULL once, And then we do assignment.
>>
>> To fix the twice NULL check, we either have to remove the check
>> which is inside dev_kfree_skb (1). Or do whats done in this
>> patch.
>>
>> (1) is not an option because a lot of kernel code already
>> assumes that dev_kfree_skb is NULL safe.
>>
>> 2. skb is NULL:
>> * Without this patch:
>> One if statement is executed.
>> * With this patch:
>> One if statement and one assignment is executed.
>>
>> From my observation most of the dev_kfree_skb calls are from
>> e1000_unmap_and_free_tx_resource, e1000_put_txbuf,
>> atl1_clean_tx_ring, alx_free_txbuf etc. in clean up functions.
>>
>> Is is quite unlikely thats skb is NULL. So it comes down to one extra
>> if-branching statement or one extra assignment. I would prefer extra
>> assignment to branching statement. In my opinion extra assignment is
>> very little price we pay.
>>
>> //govind
>
> Another way to solve the double NULL check is to define a new function
> something like this
>
> dev_kfree_skb_NULL(struct sk_buff **skb)
> {
> if(*skb) {
> free_skb(*skb);
> *skb=NULL;
> }
> }
>
> and use this if you want to free a skb and make it NULL.
> Is this approach better?
>
> //govind
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH net-next 02/13] driver: net: remove unnecessary skb NULL check before calling dev_kfree_skb_irq
2013-11-18 19:26 ` Govindarajulu Varadarajan
@ 2013-11-18 19:37 ` Johannes Berg
2013-11-18 20:17 ` David Miller
1 sibling, 0 replies; 20+ messages in thread
From: Johannes Berg @ 2013-11-18 19:37 UTC (permalink / raw)
To: Govindarajulu Varadarajan
Cc: David Miller, gregkh, linux-usb, linux-kernel, schwidefsky,
linville, linux-wireless, netdev, IvDoorn, sbhatewara, samuel,
chas, roland, isdn, jcliburn, Christian Benvenuti (benve),
Sujith Sankar (ssujith),
jeffrey.t.kirsher, jesse.brandeburg, shahed.shaikh, joe, apw
Please don't top-post. You're making a lot of obvious mistakes, to the
likely effect that soon enough people won't even read your email.
> Did you have a chance to look at this? Let me know how you want me to
> fix this.
By not "fixing" anything?
> >> Is is quite unlikely thats skb is NULL. So it comes down to one extra
> >> if-branching statement or one extra assignment. I would prefer extra
> >> assignment to branching statement. In my opinion extra assignment is
> >> very little price we pay.
> >>
> >> //govind
> >
> > Another way to solve the double NULL check is to define a new function
> > something like this
> >
> > dev_kfree_skb_NULL(struct sk_buff **skb)
> > {
> > if(*skb) {
> > free_skb(*skb);
> > *skb=NULL;
> > }
> > }
> >
> > and use this if you want to free a skb and make it NULL.
> > Is this approach better?
That's just ugly imho. Why do you want to "clean up" something that
doesn't need changing?
Anyway, just saying.
johannes
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH net-next 02/13] driver: net: remove unnecessary skb NULL check before calling dev_kfree_skb_irq
2013-11-18 19:26 ` Govindarajulu Varadarajan
2013-11-18 19:37 ` Johannes Berg
@ 2013-11-18 20:17 ` David Miller
1 sibling, 0 replies; 20+ messages in thread
From: David Miller @ 2013-11-18 20:17 UTC (permalink / raw)
To: govindarajulu90
Cc: gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
From: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
Date: Tue, 19 Nov 2013 00:56:48 +0530 (IST)
> Did you have a chance to look at this? Let me know how you want me to
> fix this.
I said clearly that I don't want this change to be made, because in many
cases the "= NULL" assignment in the driver is desirable to elide when it
isn't necessary because the value is already NULL. Therefore the NULL
check in dev_kfree_skb_irq() is basically coming for free.
Yes there are other cases, but it really doesn't matter enough to justify
your change.
Thanks.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH net-next 03/13] driver: atm: remove unnecessary skb NULL check before calling dev_kfree_skb_irq
2013-11-02 13:47 [PATCH net-next 00/13] Protect dev_kfree_skb_irq from NULL and remove unnecessary NULL checks Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 01/13] net: Check skb for NULL in dev_kfree_skb_irq Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 02/13] driver: net: remove unnecessary skb NULL check before calling dev_kfree_skb_irq Govindarajulu Varadarajan
@ 2013-11-02 13:47 ` Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 04/13] driver: staging: " Govindarajulu Varadarajan
` (9 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Govindarajulu Varadarajan @ 2013-11-02 13:47 UTC (permalink / raw)
To: davem, gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
Cc: Govindarajulu Varadarajan
dev_kfree_skb_irq is protected from NULL. No need to check for NULL while
calling this function.
Signed-off-by: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
---
drivers/atm/eni.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/atm/eni.c b/drivers/atm/eni.c
index b1955ba..78a4445 100644
--- a/drivers/atm/eni.c
+++ b/drivers/atm/eni.c
@@ -481,7 +481,7 @@ trouble:
if (paddr)
pci_unmap_single(eni_dev->pci_dev,paddr,skb->len,
PCI_DMA_FROMDEVICE);
- if (skb) dev_kfree_skb_irq(skb);
+ dev_kfree_skb_irq(skb);
return -1;
}
--
1.8.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 04/13] driver: staging: remove unnecessary skb NULL check before calling dev_kfree_skb_irq
2013-11-02 13:47 [PATCH net-next 00/13] Protect dev_kfree_skb_irq from NULL and remove unnecessary NULL checks Govindarajulu Varadarajan
` (2 preceding siblings ...)
2013-11-02 13:47 ` [PATCH net-next 03/13] driver: atm: " Govindarajulu Varadarajan
@ 2013-11-02 13:47 ` Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 05/13] driver: usb/gadget: " Govindarajulu Varadarajan
` (8 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Govindarajulu Varadarajan @ 2013-11-02 13:47 UTC (permalink / raw)
To: davem, gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
Cc: Govindarajulu Varadarajan
dev_kfree_skb_irq is protected from NULL. No need to check for NULL while
calling this function.
Signed-off-by: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
---
drivers/staging/slicoss/slicoss.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index 869dcd3..1073880 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -2616,8 +2616,7 @@ static void slic_xmit_complete(struct adapter *adapter)
address;
/* hcmd = (struct slic_hostcmd *) rspbuf->hosthandle; */
if (hcmd->type == SLIC_CMD_DUMB) {
- if (hcmd->skb)
- dev_kfree_skb_irq(hcmd->skb);
+ dev_kfree_skb_irq(hcmd->skb);
slic_cmdq_putdone_irq(adapter, hcmd);
}
rspbuf->status = 0;
--
1.8.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 05/13] driver: usb/gadget: remove unnecessary skb NULL check before calling dev_kfree_skb_irq
2013-11-02 13:47 [PATCH net-next 00/13] Protect dev_kfree_skb_irq from NULL and remove unnecessary NULL checks Govindarajulu Varadarajan
` (3 preceding siblings ...)
2013-11-02 13:47 ` [PATCH net-next 04/13] driver: staging: " Govindarajulu Varadarajan
@ 2013-11-02 13:47 ` Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 06/13] driver: net: remove unnecessary NULL check before dev_kfree_skb_any Govindarajulu Varadarajan
` (7 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Govindarajulu Varadarajan @ 2013-11-02 13:47 UTC (permalink / raw)
To: davem, gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
Cc: Govindarajulu Varadarajan
dev_kfree_skb_irq is protected from NULL. No need to check for NULL
while calling this function.
Signed-off-by: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
---
drivers/usb/gadget/f_phonet.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/f_phonet.c b/drivers/usb/gadget/f_phonet.c
index eb3aa81..9e220b6 100644
--- a/drivers/usb/gadget/f_phonet.c
+++ b/drivers/usb/gadget/f_phonet.c
@@ -393,10 +393,8 @@ static void __pn_reset(struct usb_function *f)
usb_ep_disable(fp->out_ep);
usb_ep_disable(fp->in_ep);
- if (fp->rx.skb) {
- dev_kfree_skb_irq(fp->rx.skb);
- fp->rx.skb = NULL;
- }
+ dev_kfree_skb_irq(fp->rx.skb);
+ fp->rx.skb = NULL;
}
static int pn_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
--
1.8.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 06/13] driver: net: remove unnecessary NULL check before dev_kfree_skb_any
2013-11-02 13:47 [PATCH net-next 00/13] Protect dev_kfree_skb_irq from NULL and remove unnecessary NULL checks Govindarajulu Varadarajan
` (4 preceding siblings ...)
2013-11-02 13:47 ` [PATCH net-next 05/13] driver: usb/gadget: " Govindarajulu Varadarajan
@ 2013-11-02 13:47 ` Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 07/13] driver: staging: " Govindarajulu Varadarajan
` (6 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Govindarajulu Varadarajan @ 2013-11-02 13:47 UTC (permalink / raw)
To: davem, gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
Cc: Govindarajulu Varadarajan
dev_kfree_skb_any is protected from NULL. No need to check for NULL
while calling this function.
Signed-off-by: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
---
drivers/net/ethernet/amd/lance.c | 9 +++------
drivers/net/ethernet/atheros/alx/main.c | 6 ++----
drivers/net/ethernet/atheros/atl1e/atl1e_main.c | 6 ++----
drivers/net/ethernet/atheros/atlx/atl1.c | 6 ++----
drivers/net/ethernet/cisco/enic/enic_main.c | 3 +--
drivers/net/ethernet/freescale/fec_main.c | 6 ++----
drivers/net/ethernet/hp/hp100.c | 3 +--
drivers/net/ethernet/intel/e1000/e1000_main.c | 6 ++----
drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++--------
drivers/net/ethernet/intel/igb/igb_ptp.c | 6 ++----
drivers/net/ethernet/intel/igbvf/netdev.c | 6 ++----
drivers/net/ethernet/intel/ixgb/ixgb_main.c | 6 ++----
drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c | 6 ++----
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 6 ++----
drivers/net/ethernet/korina.c | 12 ++++--------
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 12 ++++--------
drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c | 9 +++------
drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c | 6 ++----
drivers/net/ethernet/seeq/sgiseeq.c | 6 ++----
drivers/net/ethernet/sgi/ioc3-eth.c | 9 +++------
drivers/net/ethernet/smsc/smsc9420.c | 3 +--
drivers/net/ethernet/sun/sunbmac.c | 12 ++++--------
drivers/net/ethernet/ti/cpmac.c | 6 ++----
drivers/net/hyperv/netvsc_drv.c | 3 +--
drivers/net/irda/vlsi_ir.c | 15 +++++----------
drivers/net/usb/cdc_mbim.c | 3 +--
drivers/net/usb/cdc_ncm.c | 15 +++++----------
drivers/net/usb/lg-vl600.c | 6 ++----
drivers/net/usb/usbnet.c | 3 +--
drivers/net/vmxnet3/vmxnet3_drv.c | 6 ++----
drivers/net/wireless/ath/ath10k/mac.c | 6 ++----
drivers/net/wireless/ath/ath9k/htc_drv_txrx.c | 3 +--
drivers/net/wireless/ath/ath9k/main.c | 6 ++----
drivers/net/wireless/ath/ath9k/recv.c | 6 ++----
drivers/net/wireless/ath/wil6210/txrx.c | 6 ++----
drivers/net/wireless/b43/main.c | 9 +++------
drivers/net/wireless/b43legacy/main.c | 9 +++------
drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c | 3 +--
drivers/net/wireless/brcm80211/brcmsmac/main.c | 12 ++++--------
drivers/net/wireless/ipw2x00/ipw2200.c | 6 ++----
drivers/net/wireless/ipw2x00/libipw_rx.c | 3 +--
drivers/net/wireless/ipw2x00/libipw_tx.c | 3 +--
drivers/net/wireless/mwifiex/init.c | 3 +--
43 files changed, 96 insertions(+), 192 deletions(-)
diff --git a/drivers/net/ethernet/amd/lance.c b/drivers/net/ethernet/amd/lance.c
index 256f590..4b8fc50 100644
--- a/drivers/net/ethernet/amd/lance.c
+++ b/drivers/net/ethernet/amd/lance.c
@@ -845,14 +845,11 @@ lance_purge_ring(struct net_device *dev)
struct sk_buff *skb = lp->rx_skbuff[i];
lp->rx_skbuff[i] = NULL;
lp->rx_ring[i].base = 0; /* Not owned by LANCE chip. */
- if (skb)
- dev_kfree_skb_any(skb);
+ dev_kfree_skb_any(skb);
}
for (i = 0; i < TX_RING_SIZE; i++) {
- if (lp->tx_skbuff[i]) {
- dev_kfree_skb_any(lp->tx_skbuff[i]);
- lp->tx_skbuff[i] = NULL;
- }
+ dev_kfree_skb_any(lp->tx_skbuff[i]);
+ lp->tx_skbuff[i] = NULL;
}
}
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index 5aa5e81..2bee48b 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -64,10 +64,8 @@ static void alx_free_txbuf(struct alx_priv *alx, int entry)
dma_unmap_len_set(txb, size, 0);
}
- if (txb->skb) {
- dev_kfree_skb_any(txb->skb);
- txb->skb = NULL;
- }
+ dev_kfree_skb_any(txb->skb);
+ txb->skb = NULL;
}
static int alx_refill_rx_ring(struct alx_priv *alx, gfp_t gfp)
diff --git a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
index c3ca8c6..b420ead 100644
--- a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
+++ b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
@@ -686,10 +686,8 @@ static void atl1e_clean_tx_ring(struct atl1e_adapter *adapter)
/* second free skb */
for (index = 0; index < ring_count; index++) {
tx_buffer = &tx_ring->tx_buffer[index];
- if (tx_buffer->skb) {
- dev_kfree_skb_any(tx_buffer->skb);
- tx_buffer->skb = NULL;
- }
+ dev_kfree_skb_any(tx_buffer->skb);
+ tx_buffer->skb = NULL;
}
/* Zero out Tx-buffers */
memset(tx_ring->desc, 0, sizeof(struct atl1e_tpd_desc) *
diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index b8cc654..5fd7212 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -1205,10 +1205,8 @@ static void atl1_clean_tx_ring(struct atl1_adapter *adapter)
for (i = 0; i < tpd_ring->count; i++) {
buffer_info = &tpd_ring->buffer_info[i];
- if (buffer_info->skb) {
- dev_kfree_skb_any(buffer_info->skb);
- buffer_info->skb = NULL;
- }
+ dev_kfree_skb_any(buffer_info->skb);
+ buffer_info->skb = NULL;
}
size = sizeof(struct atl1_buffer) * tpd_ring->count;
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index ff78dfa..b436a92 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -107,8 +107,7 @@ static void enic_free_wq_buf(struct vnic_wq *wq, struct vnic_wq_buf *buf)
pci_unmap_page(enic->pdev, buf->dma_addr,
buf->len, PCI_DMA_TODEVICE);
- if (buf->os_buf)
- dev_kfree_skb_any(buf->os_buf);
+ dev_kfree_skb_any(buf->os_buf);
}
static void enic_wq_free_buf(struct vnic_wq *wq,
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index b2793b9..9c6a8a6 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -548,10 +548,8 @@ fec_restart(struct net_device *ndev, int duplex)
for (i = 0; i <= TX_RING_MOD_MASK; i++) {
- if (fep->tx_skbuff[i]) {
- dev_kfree_skb_any(fep->tx_skbuff[i]);
- fep->tx_skbuff[i] = NULL;
- }
+ dev_kfree_skb_any(fep->tx_skbuff[i]);
+ fep->tx_skbuff[i] = NULL;
}
/* Enable MII mode */
diff --git a/drivers/net/ethernet/hp/hp100.c b/drivers/net/ethernet/hp/hp100.c
index 3786009..240d3adb 100644
--- a/drivers/net/ethernet/hp/hp100.c
+++ b/drivers/net/ethernet/hp/hp100.c
@@ -1952,8 +1952,7 @@ static void hp100_rx_bm(struct net_device *dev)
#ifdef HP100_DEBUG
printk("hp100: %s: rx_bm: Received bad packet (length=%d)\n", dev->name, pkt_len);
#endif
- if (ptr->skb != NULL)
- dev_kfree_skb_any(ptr->skb);
+ dev_kfree_skb_any(ptr->skb);
dev->stats.rx_errors++;
}
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index ad6800a..890dcba 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -1968,10 +1968,8 @@ static void e1000_unmap_and_free_tx_resource(struct e1000_adapter *adapter,
DMA_TO_DEVICE);
buffer_info->dma = 0;
}
- if (buffer_info->skb) {
- dev_kfree_skb_any(buffer_info->skb);
- buffer_info->skb = NULL;
- }
+ dev_kfree_skb_any(buffer_info->skb);
+ buffer_info->skb = NULL;
buffer_info->time_stamp = 0;
/* buffer_info must be completely set up in the transmit path */
}
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 4c22a1a..a562f54 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1057,10 +1057,8 @@ static void e1000_put_txbuf(struct e1000_ring *tx_ring,
buffer_info->length, DMA_TO_DEVICE);
buffer_info->dma = 0;
}
- if (buffer_info->skb) {
- dev_kfree_skb_any(buffer_info->skb);
- buffer_info->skb = NULL;
- }
+ dev_kfree_skb_any(buffer_info->skb);
+ buffer_info->skb = NULL;
buffer_info->time_stamp = 0;
}
@@ -6893,10 +6891,8 @@ static void e1000_remove(struct pci_dev *pdev)
if (adapter->flags & FLAG_HAS_HW_TIMESTAMP) {
cancel_work_sync(&adapter->tx_hwtstamp_work);
- if (adapter->tx_hwtstamp_skb) {
- dev_kfree_skb_any(adapter->tx_hwtstamp_skb);
- adapter->tx_hwtstamp_skb = NULL;
- }
+ dev_kfree_skb_any(adapter->tx_hwtstamp_skb);
+ adapter->tx_hwtstamp_skb = NULL;
}
if (!(netdev->flags & IFF_UP))
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 5a54e3d..ed8b232 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -838,10 +838,8 @@ void igb_ptp_stop(struct igb_adapter *adapter)
}
cancel_work_sync(&adapter->ptp_tx_work);
- if (adapter->ptp_tx_skb) {
- dev_kfree_skb_any(adapter->ptp_tx_skb);
- adapter->ptp_tx_skb = NULL;
- }
+ dev_kfree_skb_any(adapter->ptp_tx_skb);
+ adapter->ptp_tx_skb = NULL;
if (adapter->ptp_clock) {
ptp_clock_unregister(adapter->ptp_clock);
diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c
index 9fadbb2..1220dd1 100644
--- a/drivers/net/ethernet/intel/igbvf/netdev.c
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c
@@ -417,10 +417,8 @@ static void igbvf_put_txbuf(struct igbvf_adapter *adapter,
DMA_TO_DEVICE);
buffer_info->dma = 0;
}
- if (buffer_info->skb) {
- dev_kfree_skb_any(buffer_info->skb);
- buffer_info->skb = NULL;
- }
+ dev_kfree_skb_any(buffer_info->skb);
+ buffer_info->skb = NULL;
buffer_info->time_stamp = 0;
}
diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
index 9f6b236..468b7c6 100644
--- a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
+++ b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
@@ -941,10 +941,8 @@ ixgb_unmap_and_free_tx_resource(struct ixgb_adapter *adapter,
buffer_info->dma = 0;
}
- if (buffer_info->skb) {
- dev_kfree_skb_any(buffer_info->skb);
- buffer_info->skb = NULL;
- }
+ dev_kfree_skb_any(buffer_info->skb);
+ buffer_info->skb = NULL;
buffer_info->time_stamp = 0;
/* these fields must always be initialized in tx
* buffer_info->length = 0;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
index 5184e2a..cf4648a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
@@ -908,10 +908,8 @@ void ixgbe_ptp_stop(struct ixgbe_adapter *adapter)
ixgbe_ptp_setup_sdp(adapter);
cancel_work_sync(&adapter->ptp_tx_work);
- if (adapter->ptp_tx_skb) {
- dev_kfree_skb_any(adapter->ptp_tx_skb);
- adapter->ptp_tx_skb = NULL;
- }
+ dev_kfree_skb_any(adapter->ptp_tx_skb);
+ adapter->ptp_tx_skb = NULL;
if (adapter->ptp_clock) {
ptp_clock_unregister(adapter->ptp_clock);
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 038bfc8..8d0fb73 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -159,10 +159,8 @@ static void ixgbevf_unmap_and_free_tx_resource(struct ixgbevf_ring *tx_ring,
DMA_TO_DEVICE);
tx_buffer_info->dma = 0;
}
- if (tx_buffer_info->skb) {
- dev_kfree_skb_any(tx_buffer_info->skb);
- tx_buffer_info->skb = NULL;
- }
+ dev_kfree_skb_any(tx_buffer_info->skb);
+ tx_buffer_info->skb = NULL;
tx_buffer_info->time_stamp = 0;
/* tx_buffer_info must be completely set up in the transmit path */
}
diff --git a/drivers/net/ethernet/korina.c b/drivers/net/ethernet/korina.c
index 4a5e3b0..5efb977 100644
--- a/drivers/net/ethernet/korina.c
+++ b/drivers/net/ethernet/korina.c
@@ -572,10 +572,8 @@ static void korina_tx(struct net_device *dev)
}
/* We must always free the original skb */
- if (lp->tx_skb[lp->tx_next_done]) {
- dev_kfree_skb_any(lp->tx_skb[lp->tx_next_done]);
- lp->tx_skb[lp->tx_next_done] = NULL;
- }
+ dev_kfree_skb_any(lp->tx_skb[lp->tx_next_done]);
+ lp->tx_skb[lp->tx_next_done] = NULL;
lp->td_ring[lp->tx_next_done].control = DMA_DESC_IOF;
lp->td_ring[lp->tx_next_done].devcs = ETH_TX_FD | ETH_TX_LD;
@@ -785,15 +783,13 @@ static void korina_free_ring(struct net_device *dev)
for (i = 0; i < KORINA_NUM_RDS; i++) {
lp->rd_ring[i].control = 0;
- if (lp->rx_skb[i])
- dev_kfree_skb_any(lp->rx_skb[i]);
+ dev_kfree_skb_any(lp->rx_skb[i]);
lp->rx_skb[i] = NULL;
}
for (i = 0; i < KORINA_NUM_TDS; i++) {
lp->td_ring[i].control = 0;
- if (lp->tx_skb[i])
- dev_kfree_skb_any(lp->tx_skb[i]);
+ dev_kfree_skb_any(lp->tx_skb[i]);
lp->tx_skb[i] = NULL;
}
}
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 5a0f04c..5ed4b92 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -953,10 +953,8 @@ static void pch_gbe_unmap_and_free_tx_resource(
buffer_info->length, DMA_TO_DEVICE);
buffer_info->mapped = false;
}
- if (buffer_info->skb) {
- dev_kfree_skb_any(buffer_info->skb);
- buffer_info->skb = NULL;
- }
+ dev_kfree_skb_any(buffer_info->skb);
+ buffer_info->skb = NULL;
}
/**
@@ -973,10 +971,8 @@ static void pch_gbe_unmap_and_free_rx_resource(
buffer_info->length, DMA_FROM_DEVICE);
buffer_info->mapped = false;
}
- if (buffer_info->skb) {
- dev_kfree_skb_any(buffer_info->skb);
- buffer_info->skb = NULL;
- }
+ dev_kfree_skb_any(buffer_info->skb);
+ buffer_info->skb = NULL;
}
/**
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
index 7692dfd..e9710c0 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
@@ -124,8 +124,7 @@ void netxen_release_rx_buffers(struct netxen_adapter *adapter)
rx_buf->dma,
rds_ring->dma_size,
PCI_DMA_FROMDEVICE);
- if (rx_buf->skb != NULL)
- dev_kfree_skb_any(rx_buf->skb);
+ dev_kfree_skb_any(rx_buf->skb);
}
}
}
@@ -154,10 +153,8 @@ void netxen_release_tx_buffers(struct netxen_adapter *adapter)
buffrag->dma = 0ULL;
}
}
- if (cmd_buf->skb) {
- dev_kfree_skb_any(cmd_buf->skb);
- cmd_buf->skb = NULL;
- }
+ dev_kfree_skb_any(cmd_buf->skb);
+ cmd_buf->skb = NULL;
cmd_buf++;
}
}
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
index 66c26cf..d2c23f5 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
@@ -151,10 +151,8 @@ void qlcnic_release_tx_buffers(struct qlcnic_adapter *adapter,
buffrag->dma = 0ULL;
}
}
- if (cmd_buf->skb) {
- dev_kfree_skb_any(cmd_buf->skb);
- cmd_buf->skb = NULL;
- }
+ dev_kfree_skb_any(cmd_buf->skb);
+ cmd_buf->skb = NULL;
cmd_buf++;
}
}
diff --git a/drivers/net/ethernet/seeq/sgiseeq.c b/drivers/net/ethernet/seeq/sgiseeq.c
index c765718..a49ce76 100644
--- a/drivers/net/ethernet/seeq/sgiseeq.c
+++ b/drivers/net/ethernet/seeq/sgiseeq.c
@@ -486,10 +486,8 @@ static inline void sgiseeq_tx(struct net_device *dev, struct sgiseeq_private *sp
sp->tx_old = NEXT_TX(sp->tx_old);
td->tdma.cntinfo &= ~(HPCDMA_XIU | HPCDMA_XIE);
td->tdma.cntinfo |= HPCDMA_EOX;
- if (td->skb) {
- dev_kfree_skb_any(td->skb);
- td->skb = NULL;
- }
+ dev_kfree_skb_any(td->skb);
+ td->skb = NULL;
dma_sync_desc_dev(dev, td);
}
}
diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
index ffa7843..6bd90f2 100644
--- a/drivers/net/ethernet/sgi/ioc3-eth.c
+++ b/drivers/net/ethernet/sgi/ioc3-eth.c
@@ -856,10 +856,8 @@ static inline void ioc3_clean_tx_ring(struct ioc3_private *ip)
for (i=0; i < 128; i++) {
skb = ip->tx_skbs[i];
- if (skb) {
- ip->tx_skbs[i] = NULL;
- dev_kfree_skb_any(skb);
- }
+ ip->tx_skbs[i] = NULL;
+ dev_kfree_skb_any(skb);
ip->txr[i].cmd = 0;
}
ip->tx_pi = 0;
@@ -883,8 +881,7 @@ static void ioc3_free_rings(struct ioc3_private *ip)
while (n_entry != rx_entry) {
skb = ip->rx_skbs[n_entry];
- if (skb)
- dev_kfree_skb_any(skb);
+ dev_kfree_skb_any(skb);
n_entry = (n_entry + 1) & 511;
}
diff --git a/drivers/net/ethernet/smsc/smsc9420.c b/drivers/net/ethernet/smsc/smsc9420.c
index 059bcaf..e45d7c7 100644
--- a/drivers/net/ethernet/smsc/smsc9420.c
+++ b/drivers/net/ethernet/smsc/smsc9420.c
@@ -595,8 +595,7 @@ static void smsc9420_free_rx_ring(struct smsc9420_pdata *pd)
return;
for (i = 0; i < RX_RING_SIZE; i++) {
- if (pd->rx_buffers[i].skb)
- dev_kfree_skb_any(pd->rx_buffers[i].skb);
+ dev_kfree_skb_any(pd->rx_buffers[i].skb);
if (pd->rx_buffers[i].mapping)
pci_unmap_single(pd->pdev, pd->rx_buffers[i].mapping,
diff --git a/drivers/net/ethernet/sun/sunbmac.c b/drivers/net/ethernet/sun/sunbmac.c
index 7217ee5..50eb0f1 100644
--- a/drivers/net/ethernet/sun/sunbmac.c
+++ b/drivers/net/ethernet/sun/sunbmac.c
@@ -195,17 +195,13 @@ static void bigmac_clean_rings(struct bigmac *bp)
int i;
for (i = 0; i < RX_RING_SIZE; i++) {
- if (bp->rx_skbs[i] != NULL) {
- dev_kfree_skb_any(bp->rx_skbs[i]);
- bp->rx_skbs[i] = NULL;
- }
+ dev_kfree_skb_any(bp->rx_skbs[i]);
+ bp->rx_skbs[i] = NULL;
}
for (i = 0; i < TX_RING_SIZE; i++) {
- if (bp->tx_skbs[i] != NULL) {
- dev_kfree_skb_any(bp->tx_skbs[i]);
- bp->tx_skbs[i] = NULL;
- }
+ dev_kfree_skb_any(bp->tx_skbs[i]);
+ bp->tx_skbs[i] = NULL;
}
}
diff --git a/drivers/net/ethernet/ti/cpmac.c b/drivers/net/ethernet/ti/cpmac.c
index 2dc16b6..ba340a8 100644
--- a/drivers/net/ethernet/ti/cpmac.c
+++ b/drivers/net/ethernet/ti/cpmac.c
@@ -728,10 +728,8 @@ static void cpmac_clear_tx(struct net_device *dev)
return;
for (i = 0; i < CPMAC_QUEUES; i++) {
priv->desc_ring[i].dataflags = 0;
- if (priv->desc_ring[i].skb) {
- dev_kfree_skb_any(priv->desc_ring[i].skb);
- priv->desc_ring[i].skb = NULL;
- }
+ dev_kfree_skb_any(priv->desc_ring[i].skb);
+ priv->desc_ring[i].skb = NULL;
}
}
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 524f713..b051c45 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -128,8 +128,7 @@ static void netvsc_xmit_completion(void *context)
kfree(packet);
- if (skb)
- dev_kfree_skb_any(skb);
+ dev_kfree_skb_any(skb);
}
static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
diff --git a/drivers/net/irda/vlsi_ir.c b/drivers/net/irda/vlsi_ir.c
index c5bd58b..49882f7 100644
--- a/drivers/net/irda/vlsi_ir.c
+++ b/drivers/net/irda/vlsi_ir.c
@@ -467,8 +467,7 @@ static int vlsi_free_ring(struct vlsi_ring *r)
for (i = 0; i < r->size; i++) {
rd = r->rd + i;
- if (rd->skb)
- dev_kfree_skb_any(rd->skb);
+ dev_kfree_skb_any(rd->skb);
busaddr = rd_get_addr(rd);
rd_set_addr_status(rd, 0, 0);
if (busaddr)
@@ -703,10 +702,8 @@ static void vlsi_unarm_rx(vlsi_irda_dev_t *idev)
}
rd_set_count(rd, 0);
pci_dma_sync_single_for_cpu(r->pdev, rd_get_addr(rd), r->len, r->dir);
- if (rd->skb) {
- dev_kfree_skb_any(rd->skb);
- rd->skb = NULL;
- }
+ dev_kfree_skb_any(rd->skb);
+ rd->skb = NULL;
}
else
ret = vlsi_process_rx(r, rd);
@@ -1136,10 +1133,8 @@ static void vlsi_unarm_tx(vlsi_irda_dev_t *idev)
rd_set_status(rd, 0);
rd_set_count(rd, 0);
pci_dma_sync_single_for_cpu(r->pdev, rd_get_addr(rd), r->len, r->dir);
- if (rd->skb) {
- dev_kfree_skb_any(rd->skb);
- rd->skb = NULL;
- }
+ dev_kfree_skb_any(rd->skb);
+ rd->skb = NULL;
IRDA_DEBUG(0, "%s - dropping tx packet\n", __func__);
ret = -VLSI_TX_DROP;
}
diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index af76aaf..0986ba0 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -180,8 +180,7 @@ static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb
return skb_out;
error:
- if (skb)
- dev_kfree_skb_any(skb);
+ dev_kfree_skb_any(skb);
return NULL;
}
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 11c7033..160d854 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -311,15 +311,11 @@ static void cdc_ncm_free(struct cdc_ncm_ctx *ctx)
if (ctx == NULL)
return;
- if (ctx->tx_rem_skb != NULL) {
- dev_kfree_skb_any(ctx->tx_rem_skb);
- ctx->tx_rem_skb = NULL;
- }
+ dev_kfree_skb_any(ctx->tx_rem_skb);
+ ctx->tx_rem_skb = NULL;
- if (ctx->tx_curr_skb != NULL) {
- dev_kfree_skb_any(ctx->tx_curr_skb);
- ctx->tx_curr_skb = NULL;
- }
+ dev_kfree_skb_any(ctx->tx_curr_skb);
+ ctx->tx_curr_skb = NULL;
kfree(ctx);
}
@@ -852,8 +848,7 @@ cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
return skb_out;
error:
- if (skb != NULL)
- dev_kfree_skb_any(skb);
+ dev_kfree_skb_any(skb);
return NULL;
}
diff --git a/drivers/net/usb/lg-vl600.c b/drivers/net/usb/lg-vl600.c
index 808d650..7ecf203 100644
--- a/drivers/net/usb/lg-vl600.c
+++ b/drivers/net/usb/lg-vl600.c
@@ -238,10 +238,8 @@ static int vl600_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
}
error:
- if (s->current_rx_buf) {
- dev_kfree_skb_any(s->current_rx_buf);
- s->current_rx_buf = NULL;
- }
+ dev_kfree_skb_any(s->current_rx_buf);
+ s->current_rx_buf = NULL;
dev->net->stats.rx_errors++;
return 0;
}
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 90a429b..b37f812 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1377,8 +1377,7 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
drop:
dev->net->stats.tx_dropped++;
not_drop:
- if (skb)
- dev_kfree_skb_any (skb);
+ dev_kfree_skb_any (skb);
if (urb) {
kfree(urb->sg);
usb_free_urb(urb);
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 64ca248..2fd7034 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -403,10 +403,8 @@ vmxnet3_tq_cleanup(struct vmxnet3_tx_queue *tq,
tbi = tq->buf_info + tq->tx_ring.next2comp;
vmxnet3_unmap_tx_buf(tbi, adapter->pdev);
- if (tbi->skb) {
- dev_kfree_skb_any(tbi->skb);
- tbi->skb = NULL;
- }
+ dev_kfree_skb_any(tbi->skb);
+ tbi->skb = NULL;
vmxnet3_cmd_ring_adv_next2comp(&tq->tx_ring);
}
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 99a9bad..7918569 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2067,10 +2067,8 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
mutex_lock(&ar->conf_mutex);
spin_lock_bh(&ar->data_lock);
- if (arvif->beacon) {
- dev_kfree_skb_any(arvif->beacon);
- arvif->beacon = NULL;
- }
+ dev_kfree_skb_any(arvif->beacon);
+ arvif->beacon = NULL;
spin_unlock_bh(&ar->data_lock);
ar->free_vdev_map |= 1 << (arvif->vdev_id);
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
index c028df7..69d9b44 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
@@ -1200,8 +1200,7 @@ void ath9k_rx_cleanup(struct ath9k_htc_priv *priv)
list_for_each_entry_safe(rxbuf, tbuf, &priv->rx.rxbuf, list) {
list_del(&rxbuf->list);
- if (rxbuf->skb)
- dev_kfree_skb_any(rxbuf->skb);
+ dev_kfree_skb_any(rxbuf->skb);
kfree(rxbuf);
}
}
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 20a2fbc..8125b1b 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -871,10 +871,8 @@ static void ath9k_stop(struct ieee80211_hw *hw)
ath_prepare_reset(sc);
- if (sc->rx.frag) {
- dev_kfree_skb_any(sc->rx.frag);
- sc->rx.frag = NULL;
- }
+ dev_kfree_skb_any(sc->rx.frag);
+ sc->rx.frag = NULL;
if (!ah->curchan)
ah->curchan = ath9k_cmn_get_channel(hw, ah, &hw->conf.chandef);
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 8b788ef..679e05d 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -1454,10 +1454,8 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
ieee80211_rx(hw, skb);
requeue_drop_frag:
- if (sc->rx.frag) {
- dev_kfree_skb_any(sc->rx.frag);
- sc->rx.frag = NULL;
- }
+ dev_kfree_skb_any(sc->rx.frag);
+ sc->rx.frag = NULL;
requeue:
list_add_tail(&bf->list, &sc->rx.rxbuf);
if (flush)
diff --git a/drivers/net/wireless/ath/wil6210/txrx.c b/drivers/net/wireless/ath/wil6210/txrx.c
index d505b26..a6a46d7 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -130,8 +130,7 @@ static void wil_vring_free(struct wil6210_priv *wil, struct vring *vring,
dma_unmap_single(dev, pa, dmalen,
DMA_TO_DEVICE);
}
- if (ctx->skb)
- dev_kfree_skb_any(ctx->skb);
+ dev_kfree_skb_any(ctx->skb);
vring->swtail = wil_vring_next_tail(vring);
} else { /* rx */
struct vring_rx_desc dd, *d = ⅆ
@@ -819,8 +818,7 @@ static int wil_tx_vring(struct wil6210_priv *wil, struct vring *vring,
else
dma_unmap_single(dev, pa, dmalen, DMA_TO_DEVICE);
- if (ctx->skb)
- dev_kfree_skb_any(ctx->skb);
+ dev_kfree_skb_any(ctx->skb);
memset(ctx, 0, sizeof(*ctx));
}
diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
index ccd24f0a..449d98d 100644
--- a/drivers/net/wireless/b43/main.c
+++ b/drivers/net/wireless/b43/main.c
@@ -1755,8 +1755,7 @@ static void b43_update_templates(struct b43_wl *wl)
if (unlikely(!beacon))
return;
- if (wl->current_beacon)
- dev_kfree_skb_any(wl->current_beacon);
+ dev_kfree_skb_any(wl->current_beacon);
wl->current_beacon = beacon;
wl->beacon0_uploaded = false;
wl->beacon1_uploaded = false;
@@ -4662,10 +4661,8 @@ static void b43_wireless_core_exit(struct b43_wldev *dev)
b43_pio_free(dev);
b43_chip_exit(dev);
dev->phy.ops->switch_analog(dev, 0);
- if (dev->wl->current_beacon) {
- dev_kfree_skb_any(dev->wl->current_beacon);
- dev->wl->current_beacon = NULL;
- }
+ dev_kfree_skb_any(dev->wl->current_beacon);
+ dev->wl->current_beacon = NULL;
b43_device_disable(dev, 0);
b43_bus_may_powerdown(dev);
diff --git a/drivers/net/wireless/b43legacy/main.c b/drivers/net/wireless/b43legacy/main.c
index 5726688..4751b22 100644
--- a/drivers/net/wireless/b43legacy/main.c
+++ b/drivers/net/wireless/b43legacy/main.c
@@ -1274,8 +1274,7 @@ static void b43legacy_update_templates(struct b43legacy_wl *wl)
if (unlikely(!beacon))
return;
- if (wl->current_beacon)
- dev_kfree_skb_any(wl->current_beacon);
+ dev_kfree_skb_any(wl->current_beacon);
wl->current_beacon = beacon;
wl->beacon0_uploaded = false;
wl->beacon1_uploaded = false;
@@ -3233,10 +3232,8 @@ static void b43legacy_wireless_core_exit(struct b43legacy_wldev *dev)
kfree(phy->tssi2dbm);
kfree(phy->lo_control);
phy->lo_control = NULL;
- if (dev->wl->current_beacon) {
- dev_kfree_skb_any(dev->wl->current_beacon);
- dev->wl->current_beacon = NULL;
- }
+ dev_kfree_skb_any(dev->wl->current_beacon);
+ dev->wl->current_beacon = NULL;
ssb_device_disable(dev->dev, 0);
ssb_bus_may_powerdown(dev->dev->bus);
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
index 64e9cff..402a642 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
@@ -878,8 +878,7 @@ static int brcmf_net_p2p_do_ioctl(struct net_device *ndev,
static netdev_tx_t brcmf_net_p2p_start_xmit(struct sk_buff *skb,
struct net_device *ndev)
{
- if (skb)
- dev_kfree_skb_any(skb);
+ dev_kfree_skb_any(skb);
return NETDEV_TX_OK;
}
diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c
index 8138f1c..07f63db 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/main.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c
@@ -449,10 +449,8 @@ static void brcms_c_detach_mfree(struct brcms_c_info *wlc)
kfree(wlc->corestate);
kfree(wlc->hw->bandstate[0]);
kfree(wlc->hw);
- if (wlc->beacon)
- dev_kfree_skb_any(wlc->beacon);
- if (wlc->probe_resp)
- dev_kfree_skb_any(wlc->probe_resp);
+ dev_kfree_skb_any(wlc->beacon);
+ dev_kfree_skb_any(wlc->probe_resp);
/* free the wlc */
kfree(wlc);
@@ -7470,8 +7468,7 @@ void brcms_c_set_new_beacon(struct brcms_c_info *wlc, struct sk_buff *beacon,
{
if (!beacon)
return;
- if (wlc->beacon)
- dev_kfree_skb_any(wlc->beacon);
+ dev_kfree_skb_any(wlc->beacon);
wlc->beacon = beacon;
/* add PLCP */
@@ -7486,8 +7483,7 @@ void brcms_c_set_new_probe_resp(struct brcms_c_info *wlc,
{
if (!probe_resp)
return;
- if (wlc->probe_resp)
- dev_kfree_skb_any(wlc->probe_resp);
+ dev_kfree_skb_any(wlc->probe_resp);
wlc->probe_resp = probe_resp;
/* add PLCP */
diff --git a/drivers/net/wireless/ipw2x00/ipw2200.c b/drivers/net/wireless/ipw2x00/ipw2200.c
index 81903e3..f803ca1 100644
--- a/drivers/net/wireless/ipw2x00/ipw2200.c
+++ b/drivers/net/wireless/ipw2x00/ipw2200.c
@@ -8538,10 +8538,8 @@ static void ipw_rx(struct ipw_priv *priv)
/* For now we just don't re-use anything. We can tweak this
* later to try and re-use notification packets and SKBs that
* fail to Rx correctly */
- if (rxb->skb != NULL) {
- dev_kfree_skb_any(rxb->skb);
- rxb->skb = NULL;
- }
+ dev_kfree_skb_any(rxb->skb);
+ rxb->skb = NULL;
pci_unmap_single(priv->pci_dev, rxb->dma_addr,
IPW_RX_BUF_SIZE, PCI_DMA_FROMDEVICE);
diff --git a/drivers/net/wireless/ipw2x00/libipw_rx.c b/drivers/net/wireless/ipw2x00/libipw_rx.c
index 9ffe659..4411888 100644
--- a/drivers/net/wireless/ipw2x00/libipw_rx.c
+++ b/drivers/net/wireless/ipw2x00/libipw_rx.c
@@ -113,8 +113,7 @@ static struct sk_buff *libipw_frag_cache_get(struct libipw_device *ieee,
if (ieee->frag_next_idx >= LIBIPW_FRAG_CACHE_LEN)
ieee->frag_next_idx = 0;
- if (entry->skb != NULL)
- dev_kfree_skb_any(entry->skb);
+ dev_kfree_skb_any(entry->skb);
entry->first_frag_time = jiffies;
entry->seq = seq;
diff --git a/drivers/net/wireless/ipw2x00/libipw_tx.c b/drivers/net/wireless/ipw2x00/libipw_tx.c
index e8c0398..b04243a 100644
--- a/drivers/net/wireless/ipw2x00/libipw_tx.c
+++ b/drivers/net/wireless/ipw2x00/libipw_tx.c
@@ -183,8 +183,7 @@ void libipw_txb_free(struct libipw_txb *txb)
if (unlikely(!txb))
return;
for (i = 0; i < txb->nr_frags; i++)
- if (txb->fragments[i])
- dev_kfree_skb_any(txb->fragments[i]);
+ dev_kfree_skb_any(txb->fragments[i]);
kfree(txb);
}
diff --git a/drivers/net/wireless/mwifiex/init.c b/drivers/net/wireless/mwifiex/init.c
index 6499117..ffd0751 100644
--- a/drivers/net/wireless/mwifiex/init.c
+++ b/drivers/net/wireless/mwifiex/init.c
@@ -395,8 +395,7 @@ mwifiex_adapter_cleanup(struct mwifiex_adapter *adapter)
dev_dbg(adapter->dev, "info: free scan table\n");
- if (adapter->sleep_cfm)
- dev_kfree_skb_any(adapter->sleep_cfm);
+ dev_kfree_skb_any(adapter->sleep_cfm);
}
/*
--
1.8.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 07/13] driver: staging: remove unnecessary NULL check before dev_kfree_skb_any
2013-11-02 13:47 [PATCH net-next 00/13] Protect dev_kfree_skb_irq from NULL and remove unnecessary NULL checks Govindarajulu Varadarajan
` (5 preceding siblings ...)
2013-11-02 13:47 ` [PATCH net-next 06/13] driver: net: remove unnecessary NULL check before dev_kfree_skb_any Govindarajulu Varadarajan
@ 2013-11-02 13:47 ` Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 08/13] driver: isdn: " Govindarajulu Varadarajan
` (5 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Govindarajulu Varadarajan @ 2013-11-02 13:47 UTC (permalink / raw)
To: davem, gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
Cc: Govindarajulu Varadarajan
dev_kfree_skb_any is protected from NULL. No need to check for NULL while
calling this function.
Signed-off-by: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
---
drivers/staging/rtl8187se/ieee80211/ieee80211_rx.c | 3 +--
drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c | 3 +--
drivers/staging/rtl8188eu/core/rtw_recv.c | 6 ++----
drivers/staging/rtl8192e/rtllib_rx.c | 3 +--
drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c | 3 +--
drivers/staging/rtl8712/recv_linux.c | 3 +--
drivers/staging/rtl8712/rtl8712_recv.c | 6 ++----
drivers/staging/rtl8712/xmit_linux.c | 3 +--
drivers/staging/sbe-2t3e3/dc.c | 12 ++++--------
9 files changed, 14 insertions(+), 28 deletions(-)
diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8187se/ieee80211/ieee80211_rx.c
index 10b2210..bbafdcc 100644
--- a/drivers/staging/rtl8187se/ieee80211/ieee80211_rx.c
+++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_rx.c
@@ -138,8 +138,7 @@ ieee80211_frag_cache_get(struct ieee80211_device *ieee,
if (ieee->frag_next_idx[tid] >= IEEE80211_FRAG_CACHE_LEN)
ieee->frag_next_idx[tid] = 0;
- if (entry->skb != NULL)
- dev_kfree_skb_any(entry->skb);
+ dev_kfree_skb_any(entry->skb);
entry->first_frag_time = jiffies;
entry->seq = seq;
diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
index b346653..fe747b1 100644
--- a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
+++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
@@ -234,8 +234,7 @@ void ieee80211_txb_free(struct ieee80211_txb *txb) {
if (unlikely(!txb))
return;
for (i = 0; i < txb->nr_frags; i++)
- if (txb->fragments[i])
- dev_kfree_skb_any(txb->fragments[i]);
+ dev_kfree_skb_any(txb->fragments[i]);
kfree(txb);
}
diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c b/drivers/staging/rtl8188eu/core/rtw_recv.c
index 2011657..93fb9ed 100644
--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
@@ -209,10 +209,8 @@ int rtw_free_recvframe(union recv_frame *precvframe, struct __queue *pfree_recv_
_func_enter_;
- if (precvframe->u.hdr.pkt) {
- dev_kfree_skb_any(precvframe->u.hdr.pkt);/* free skb by driver */
- precvframe->u.hdr.pkt = NULL;
- }
+ dev_kfree_skb_any(precvframe->u.hdr.pkt);/* free skb by driver */
+ precvframe->u.hdr.pkt = NULL;
_enter_critical_bh(&pfree_recv_queue->lock, &irqL);
diff --git a/drivers/staging/rtl8192e/rtllib_rx.c b/drivers/staging/rtl8192e/rtllib_rx.c
index 8aeaed5..0118854 100644
--- a/drivers/staging/rtl8192e/rtllib_rx.c
+++ b/drivers/staging/rtl8192e/rtllib_rx.c
@@ -133,8 +133,7 @@ rtllib_frag_cache_get(struct rtllib_device *ieee,
if (ieee->frag_next_idx[tid] >= RTLLIB_FRAG_CACHE_LEN)
ieee->frag_next_idx[tid] = 0;
- if (entry->skb != NULL)
- dev_kfree_skb_any(entry->skb);
+ dev_kfree_skb_any(entry->skb);
entry->first_frag_time = jiffies;
entry->seq = seq;
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
index 59900bf..70bf7a4 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
@@ -138,8 +138,7 @@ ieee80211_frag_cache_get(struct ieee80211_device *ieee,
if (ieee->frag_next_idx[tid] >= IEEE80211_FRAG_CACHE_LEN)
ieee->frag_next_idx[tid] = 0;
- if (entry->skb != NULL)
- dev_kfree_skb_any(entry->skb);
+ dev_kfree_skb_any(entry->skb);
entry->first_frag_time = jiffies;
entry->seq = seq;
diff --git a/drivers/staging/rtl8712/recv_linux.c b/drivers/staging/rtl8712/recv_linux.c
index 495ee12..c422d1f 100644
--- a/drivers/staging/rtl8712/recv_linux.c
+++ b/drivers/staging/rtl8712/recv_linux.c
@@ -75,8 +75,7 @@ int r8712_os_recvbuf_resource_alloc(struct _adapter *padapter,
int r8712_os_recvbuf_resource_free(struct _adapter *padapter,
struct recv_buf *precvbuf)
{
- if (precvbuf->pskb)
- dev_kfree_skb_any(precvbuf->pskb);
+ dev_kfree_skb_any(precvbuf->pskb);
if (precvbuf->purb) {
usb_kill_urb(precvbuf->purb);
usb_free_urb(precvbuf->purb);
diff --git a/drivers/staging/rtl8712/rtl8712_recv.c b/drivers/staging/rtl8712/rtl8712_recv.c
index d59a74a..9a4e87c 100644
--- a/drivers/staging/rtl8712/rtl8712_recv.c
+++ b/drivers/staging/rtl8712/rtl8712_recv.c
@@ -145,10 +145,8 @@ int r8712_free_recvframe(union recv_frame *precvframe,
struct _adapter *padapter = precvframe->u.hdr.adapter;
struct recv_priv *precvpriv = &padapter->recvpriv;
- if (precvframe->u.hdr.pkt) {
- dev_kfree_skb_any(precvframe->u.hdr.pkt);/*free skb by driver*/
- precvframe->u.hdr.pkt = NULL;
- }
+ dev_kfree_skb_any(precvframe->u.hdr.pkt);/*free skb by driver*/
+ precvframe->u.hdr.pkt = NULL;
spin_lock_irqsave(&pfree_recv_queue->lock, irqL);
list_delete(&(precvframe->u.hdr.list));
list_insert_tail(&(precvframe->u.hdr.list),
diff --git a/drivers/staging/rtl8712/xmit_linux.c b/drivers/staging/rtl8712/xmit_linux.c
index 4d22bb7..13bb27a 100644
--- a/drivers/staging/rtl8712/xmit_linux.c
+++ b/drivers/staging/rtl8712/xmit_linux.c
@@ -156,8 +156,7 @@ void r8712_xmit_resource_free(struct _adapter *padapter,
void r8712_xmit_complete(struct _adapter *padapter, struct xmit_frame *pxframe)
{
- if (pxframe->pkt)
- dev_kfree_skb_any(pxframe->pkt);
+ dev_kfree_skb_any(pxframe->pkt);
pxframe->pkt = NULL;
}
diff --git a/drivers/staging/sbe-2t3e3/dc.c b/drivers/staging/sbe-2t3e3/dc.c
index f207b9e..d8c9464 100644
--- a/drivers/staging/sbe-2t3e3/dc.c
+++ b/drivers/staging/sbe-2t3e3/dc.c
@@ -403,10 +403,8 @@ void dc_clear_descriptor_list(struct channel *sc)
/* free all data buffers on TX ring */
for (i = 0; i < SBE_2T3E3_TX_DESC_RING_SIZE; i++) {
- if (sc->ether.tx_data[i] != NULL) {
- dev_kfree_skb_any(sc->ether.tx_data[i]);
- sc->ether.tx_data[i] = NULL;
- }
+ dev_kfree_skb_any(sc->ether.tx_data[i]);
+ sc->ether.tx_data[i] = NULL;
}
}
@@ -418,10 +416,8 @@ void dc_drop_descriptor_list(struct channel *sc)
/* free all data buffers on RX ring */
for (i = 0; i < SBE_2T3E3_RX_DESC_RING_SIZE; i++) {
- if (sc->ether.rx_data[i] != NULL) {
- dev_kfree_skb_any(sc->ether.rx_data[i]);
- sc->ether.rx_data[i] = NULL;
- }
+ dev_kfree_skb_any(sc->ether.rx_data[i]);
+ sc->ether.rx_data[i] = NULL;
}
kfree(sc->ether.rx_ring);
--
1.8.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 08/13] driver: isdn: remove unnecessary NULL check before dev_kfree_skb_any
2013-11-02 13:47 [PATCH net-next 00/13] Protect dev_kfree_skb_irq from NULL and remove unnecessary NULL checks Govindarajulu Varadarajan
` (6 preceding siblings ...)
2013-11-02 13:47 ` [PATCH net-next 07/13] driver: staging: " Govindarajulu Varadarajan
@ 2013-11-02 13:47 ` Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 09/13] driver: s390: " Govindarajulu Varadarajan
` (4 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Govindarajulu Varadarajan @ 2013-11-02 13:47 UTC (permalink / raw)
To: davem, gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
Cc: Govindarajulu Varadarajan
dev_kfree_skb_any is protected from NULL. No need to check for NULL
while calling this function.
Signed-off-by: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
---
drivers/isdn/gigaset/ser-gigaset.c | 3 +--
drivers/isdn/hisax/hfc_usb.c | 6 ++----
drivers/isdn/hisax/icc.c | 6 ++----
drivers/isdn/hisax/ipacx.c | 6 ++----
drivers/isdn/hisax/isac.c | 6 ++----
drivers/isdn/hisax/st5481_b.c | 6 ++----
drivers/isdn/hisax/w6692.c | 6 ++----
7 files changed, 13 insertions(+), 26 deletions(-)
diff --git a/drivers/isdn/gigaset/ser-gigaset.c b/drivers/isdn/gigaset/ser-gigaset.c
index 8c91fd5..9128526 100644
--- a/drivers/isdn/gigaset/ser-gigaset.c
+++ b/drivers/isdn/gigaset/ser-gigaset.c
@@ -221,8 +221,7 @@ static void flush_send_queue(struct cardstate *cs)
spin_unlock_irqrestore(&cs->cmdlock, flags);
/* data queue */
- if (cs->bcs->tx_skb)
- dev_kfree_skb_any(cs->bcs->tx_skb);
+ dev_kfree_skb_any(cs->bcs->tx_skb);
while ((skb = skb_dequeue(&cs->bcs->squeue)) != NULL)
dev_kfree_skb_any(skb);
}
diff --git a/drivers/isdn/hisax/hfc_usb.c b/drivers/isdn/hisax/hfc_usb.c
index 849a807..a1f8380 100644
--- a/drivers/isdn/hisax/hfc_usb.c
+++ b/drivers/isdn/hisax/hfc_usb.c
@@ -864,10 +864,8 @@ rx_int_complete(struct urb *urb)
fifon, urb->status);
fifo->urb->interval = 0; /* cancel automatic rescheduling */
- if (fifo->skbuff) {
- dev_kfree_skb_any(fifo->skbuff);
- fifo->skbuff = NULL;
- }
+ dev_kfree_skb_any(fifo->skbuff);
+ fifo->skbuff = NULL;
return;
}
len = urb->actual_length;
diff --git a/drivers/isdn/hisax/icc.c b/drivers/isdn/hisax/icc.c
index 51dae91..2c52996 100644
--- a/drivers/isdn/hisax/icc.c
+++ b/drivers/isdn/hisax/icc.c
@@ -549,10 +549,8 @@ ICC_l1hw(struct PStack *st, int pr, void *arg)
case (HW_DEACTIVATE | RESPONSE):
skb_queue_purge(&cs->rq);
skb_queue_purge(&cs->sq);
- if (cs->tx_skb) {
- dev_kfree_skb_any(cs->tx_skb);
- cs->tx_skb = NULL;
- }
+ dev_kfree_skb_any(cs->tx_skb);
+ cs->tx_skb = NULL;
if (test_and_clear_bit(FLG_DBUSY_TIMER, &cs->HW_Flags))
del_timer(&cs->dbusytimer);
if (test_and_clear_bit(FLG_L1_DBUSY, &cs->HW_Flags))
diff --git a/drivers/isdn/hisax/ipacx.c b/drivers/isdn/hisax/ipacx.c
index 5faa5de..55192a4 100644
--- a/drivers/isdn/hisax/ipacx.c
+++ b/drivers/isdn/hisax/ipacx.c
@@ -181,10 +181,8 @@ dch_l2l1(struct PStack *st, int pr, void *arg)
case (HW_DEACTIVATE | RESPONSE):
skb_queue_purge(&cs->rq);
skb_queue_purge(&cs->sq);
- if (cs->tx_skb) {
- dev_kfree_skb_any(cs->tx_skb);
- cs->tx_skb = NULL;
- }
+ dev_kfree_skb_any(cs->tx_skb);
+ cs->tx_skb = NULL;
if (test_and_clear_bit(FLG_DBUSY_TIMER, &cs->HW_Flags))
del_timer(&cs->dbusytimer);
break;
diff --git a/drivers/isdn/hisax/isac.c b/drivers/isdn/hisax/isac.c
index 7fdf78f..d0895f2 100644
--- a/drivers/isdn/hisax/isac.c
+++ b/drivers/isdn/hisax/isac.c
@@ -547,10 +547,8 @@ ISAC_l1hw(struct PStack *st, int pr, void *arg)
case (HW_DEACTIVATE | RESPONSE):
skb_queue_purge(&cs->rq);
skb_queue_purge(&cs->sq);
- if (cs->tx_skb) {
- dev_kfree_skb_any(cs->tx_skb);
- cs->tx_skb = NULL;
- }
+ dev_kfree_skb_any(cs->tx_skb);
+ cs->tx_skb = NULL;
if (test_and_clear_bit(FLG_DBUSY_TIMER, &cs->HW_Flags))
del_timer(&cs->dbusytimer);
if (test_and_clear_bit(FLG_L1_DBUSY, &cs->HW_Flags))
diff --git a/drivers/isdn/hisax/st5481_b.c b/drivers/isdn/hisax/st5481_b.c
index 4098491..b753aca 100644
--- a/drivers/isdn/hisax/st5481_b.c
+++ b/drivers/isdn/hisax/st5481_b.c
@@ -254,10 +254,8 @@ static void st5481B_mode(struct st5481_bcs *bcs, int mode)
} else {
st5481_usb_device_ctrl_msg(adapter, GPIO_OUT, adapter->leds, NULL, NULL);
}
- if (b_out->tx_skb) {
- dev_kfree_skb_any(b_out->tx_skb);
- b_out->tx_skb = NULL;
- }
+ dev_kfree_skb_any(b_out->tx_skb);
+ b_out->tx_skb = NULL;
}
}
diff --git a/drivers/isdn/hisax/w6692.c b/drivers/isdn/hisax/w6692.c
index a858955..dd1fbf4 100644
--- a/drivers/isdn/hisax/w6692.c
+++ b/drivers/isdn/hisax/w6692.c
@@ -653,10 +653,8 @@ W6692_l1hw(struct PStack *st, int pr, void *arg)
case (HW_DEACTIVATE | RESPONSE):
skb_queue_purge(&cs->rq);
skb_queue_purge(&cs->sq);
- if (cs->tx_skb) {
- dev_kfree_skb_any(cs->tx_skb);
- cs->tx_skb = NULL;
- }
+ dev_kfree_skb_any(cs->tx_skb);
+ cs->tx_skb = NULL;
if (test_and_clear_bit(FLG_DBUSY_TIMER, &cs->HW_Flags))
del_timer(&cs->dbusytimer);
if (test_and_clear_bit(FLG_L1_DBUSY, &cs->HW_Flags))
--
1.8.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 09/13] driver: s390: remove unnecessary NULL check before dev_kfree_skb_any
2013-11-02 13:47 [PATCH net-next 00/13] Protect dev_kfree_skb_irq from NULL and remove unnecessary NULL checks Govindarajulu Varadarajan
` (7 preceding siblings ...)
2013-11-02 13:47 ` [PATCH net-next 08/13] driver: isdn: " Govindarajulu Varadarajan
@ 2013-11-02 13:47 ` Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 10/13] driver: infiniband: " Govindarajulu Varadarajan
` (3 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Govindarajulu Varadarajan @ 2013-11-02 13:47 UTC (permalink / raw)
To: davem, gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
Cc: Govindarajulu Varadarajan
dev_kfree_skb_any is protected from NULL. No need to check for NULL
while calling this function.
Signed-off-by: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
---
drivers/s390/net/claw.c | 6 ++----
drivers/s390/net/ctcm_mpc.c | 6 ++----
drivers/s390/net/qeth_core_main.c | 6 ++----
3 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/s390/net/claw.c b/drivers/s390/net/claw.c
index fd7b3bd..66c0b14 100644
--- a/drivers/s390/net/claw.c
+++ b/drivers/s390/net/claw.c
@@ -834,10 +834,8 @@ claw_release(struct net_device *dev)
ccw_check_return_code(privptr->channel[i].cdev, rc);
}
}
- if (privptr->pk_skb != NULL) {
- dev_kfree_skb_any(privptr->pk_skb);
- privptr->pk_skb = NULL;
- }
+ dev_kfree_skb_any(privptr->pk_skb);
+ privptr->pk_skb = NULL;
if(privptr->buffs_alloc != 1) {
CLAW_DBF_TEXT(4, trace, "none2fre");
return 0;
diff --git a/drivers/s390/net/ctcm_mpc.c b/drivers/s390/net/ctcm_mpc.c
index 2dbc77b..2dd53a6 100644
--- a/drivers/s390/net/ctcm_mpc.c
+++ b/drivers/s390/net/ctcm_mpc.c
@@ -964,8 +964,7 @@ void mpc_channel_action(struct channel *ch, int direction, int action)
grp->outstanding_xid2++;
ch->in_mpcgroup = 1;
- if (ch->xid_skb != NULL)
- dev_kfree_skb_any(ch->xid_skb);
+ dev_kfree_skb_any(ch->xid_skb);
ch->xid_skb = __dev_alloc_skb(MPC_BUFSIZE_DEFAULT,
GFP_ATOMIC | GFP_DMA);
@@ -1017,8 +1016,7 @@ void mpc_channel_action(struct channel *ch, int direction, int action)
grp->num_channel_paths--;
grp->active_channels[direction]--;
- if (ch->xid_skb != NULL)
- dev_kfree_skb_any(ch->xid_skb);
+ dev_kfree_skb_any(ch->xid_skb);
ch->xid_skb = NULL;
if (grp->channels_terminating)
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 0a328d0..7d33b94 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -1282,10 +1282,8 @@ static void qeth_free_qdio_buffers(struct qeth_card *card)
qeth_free_cq(card);
cancel_delayed_work_sync(&card->buffer_reclaim_work);
- for (j = 0; j < QDIO_MAX_BUFFERS_PER_Q; ++j) {
- if (card->qdio.in_q->bufs[j].rx_skb)
- dev_kfree_skb_any(card->qdio.in_q->bufs[j].rx_skb);
- }
+ for (j = 0; j < QDIO_MAX_BUFFERS_PER_Q; ++j)
+ dev_kfree_skb_any(card->qdio.in_q->bufs[j].rx_skb);
kfree(card->qdio.in_q);
card->qdio.in_q = NULL;
/* inbound buffer pool */
--
1.8.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 10/13] driver: infiniband: remove unnecessary NULL check before dev_kfree_skb_any
2013-11-02 13:47 [PATCH net-next 00/13] Protect dev_kfree_skb_irq from NULL and remove unnecessary NULL checks Govindarajulu Varadarajan
` (8 preceding siblings ...)
2013-11-02 13:47 ` [PATCH net-next 09/13] driver: s390: " Govindarajulu Varadarajan
@ 2013-11-02 13:47 ` Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 11/13] driver: usb: " Govindarajulu Varadarajan
` (2 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Govindarajulu Varadarajan @ 2013-11-02 13:47 UTC (permalink / raw)
To: davem, gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
Cc: Govindarajulu Varadarajan
dev_kfree_skb_any is protected from NULL. No need to check for NULL
while calling this function.
Signed-off-by: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
---
drivers/infiniband/hw/amso1100/c2.c | 6 ++----
drivers/infiniband/hw/nes/nes_hw.c | 3 +--
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/hw/amso1100/c2.c b/drivers/infiniband/hw/amso1100/c2.c
index d53cf51..30c4d00 100644
--- a/drivers/infiniband/hw/amso1100/c2.c
+++ b/drivers/infiniband/hw/amso1100/c2.c
@@ -312,10 +312,8 @@ static inline int c2_tx_free(struct c2_dev *c2dev, struct c2_element *elem)
pci_unmap_single(c2dev->pcidev, elem->mapaddr, elem->maplen,
PCI_DMA_TODEVICE);
- if (elem->skb) {
- dev_kfree_skb_any(elem->skb);
- elem->skb = NULL;
- }
+ dev_kfree_skb_any(elem->skb);
+ elem->skb = NULL;
return 0;
}
diff --git a/drivers/infiniband/hw/nes/nes_hw.c b/drivers/infiniband/hw/nes/nes_hw.c
index 9020024..2275a8e 100644
--- a/drivers/infiniband/hw/nes/nes_hw.c
+++ b/drivers/infiniband/hw/nes/nes_hw.c
@@ -2856,8 +2856,7 @@ void nes_nic_ce_handler(struct nes_device *nesdev, struct nes_hw_nic_cq *cq)
break;
}
}
- if (skb)
- dev_kfree_skb_any(skb);
+ dev_kfree_skb_any(skb);
nesnic->sq_tail++;
nesnic->sq_tail &= nesnic->sq_size-1;
if (sq_cqes > 128) {
--
1.8.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 11/13] driver: usb: remove unnecessary NULL check before dev_kfree_skb_any
2013-11-02 13:47 [PATCH net-next 00/13] Protect dev_kfree_skb_irq from NULL and remove unnecessary NULL checks Govindarajulu Varadarajan
` (9 preceding siblings ...)
2013-11-02 13:47 ` [PATCH net-next 10/13] driver: infiniband: " Govindarajulu Varadarajan
@ 2013-11-02 13:47 ` Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 12/13] driver: net: fix space before '(' and remove extra variable Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 13/13] scripts/checkpatch.pl: Add dev_kfree_skb*(NULL) check to checkpatch Govindarajulu Varadarajan
12 siblings, 0 replies; 20+ messages in thread
From: Govindarajulu Varadarajan @ 2013-11-02 13:47 UTC (permalink / raw)
To: davem, gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
Cc: Govindarajulu Varadarajan
dev_kfree_skb_any is protected from NULL. No need to check for NULL
while calling this function.
Signed-off-by: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
---
drivers/usb/gadget/u_ether.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/u_ether.c b/drivers/usb/gadget/u_ether.c
index 2aae0d6..3b61ac6 100644
--- a/drivers/usb/gadget/u_ether.c
+++ b/drivers/usb/gadget/u_ether.c
@@ -251,8 +251,7 @@ enomem:
defer_kevent(dev, WORK_RX_MEMORY);
if (retval) {
DBG(dev, "rx submit --> %d\n", retval);
- if (skb)
- dev_kfree_skb_any(skb);
+ dev_kfree_skb_any(skb);
spin_lock_irqsave(&dev->req_lock, flags);
list_add(&req->list, &dev->rx_reqs);
spin_unlock_irqrestore(&dev->req_lock, flags);
@@ -339,8 +338,7 @@ quiesce:
break;
}
- if (skb)
- dev_kfree_skb_any(skb);
+ dev_kfree_skb_any(skb);
if (!netif_running(dev->net)) {
clean:
spin_lock(&dev->req_lock);
--
1.8.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 12/13] driver: net: fix space before '(' and remove extra variable
2013-11-02 13:47 [PATCH net-next 00/13] Protect dev_kfree_skb_irq from NULL and remove unnecessary NULL checks Govindarajulu Varadarajan
` (10 preceding siblings ...)
2013-11-02 13:47 ` [PATCH net-next 11/13] driver: usb: " Govindarajulu Varadarajan
@ 2013-11-02 13:47 ` Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 13/13] scripts/checkpatch.pl: Add dev_kfree_skb*(NULL) check to checkpatch Govindarajulu Varadarajan
12 siblings, 0 replies; 20+ messages in thread
From: Govindarajulu Varadarajan @ 2013-11-02 13:47 UTC (permalink / raw)
To: davem, gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
Cc: Govindarajulu Varadarajan
Signed-off-by: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
---
drivers/net/ethernet/sgi/ioc3-eth.c | 4 +---
drivers/net/usb/usbnet.c | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
index 6bd90f2..80dcfd9 100644
--- a/drivers/net/ethernet/sgi/ioc3-eth.c
+++ b/drivers/net/ethernet/sgi/ioc3-eth.c
@@ -851,13 +851,11 @@ static inline void ioc3_clean_rx_ring(struct ioc3_private *ip)
static inline void ioc3_clean_tx_ring(struct ioc3_private *ip)
{
- struct sk_buff *skb;
int i;
for (i=0; i < 128; i++) {
- skb = ip->tx_skbs[i];
+ dev_kfree_skb_any(ip->tx_skbs[i]);
ip->tx_skbs[i] = NULL;
- dev_kfree_skb_any(skb);
ip->txr[i].cmd = 0;
}
ip->tx_pi = 0;
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index b37f812..bd18155 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1377,7 +1377,7 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
drop:
dev->net->stats.tx_dropped++;
not_drop:
- dev_kfree_skb_any (skb);
+ dev_kfree_skb_any(skb);
if (urb) {
kfree(urb->sg);
usb_free_urb(urb);
--
1.8.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 13/13] scripts/checkpatch.pl: Add dev_kfree_skb*(NULL) check to checkpatch
2013-11-02 13:47 [PATCH net-next 00/13] Protect dev_kfree_skb_irq from NULL and remove unnecessary NULL checks Govindarajulu Varadarajan
` (11 preceding siblings ...)
2013-11-02 13:47 ` [PATCH net-next 12/13] driver: net: fix space before '(' and remove extra variable Govindarajulu Varadarajan
@ 2013-11-02 13:47 ` Govindarajulu Varadarajan
2013-11-04 0:37 ` Joe Perches
12 siblings, 1 reply; 20+ messages in thread
From: Govindarajulu Varadarajan @ 2013-11-02 13:47 UTC (permalink / raw)
To: davem, gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, joe, apw
Cc: Govindarajulu Varadarajan
Signed-off-by: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
---
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 66cad50..e651b8a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3719,7 +3719,7 @@ sub process {
# check for needless "if (<foo>) fn(<foo>)" uses
if ($prevline =~ /\bif\s*\(\s*($Lval)\s*\)/) {
my $expr = '\s*\(\s*' . quotemeta($1) . '\s*\)\s*;';
- if ($line =~ /\b(kfree|usb_free_urb|debugfs_remove(?:_recursive)?)$expr/) {
+ if ($line =~ /\b(kfree|dev_kfree_skb|dev_kfree_skb_any|dev_kfree_skb_irq|usb_free_urb|debugfs_remove(?:_recursive)?)$expr/) {
WARN('NEEDLESS_IF',
"$1(NULL) is safe this check is probably not required\n" . $hereprev);
}
--
1.8.4.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH net-next 13/13] scripts/checkpatch.pl: Add dev_kfree_skb*(NULL) check to checkpatch
2013-11-02 13:47 ` [PATCH net-next 13/13] scripts/checkpatch.pl: Add dev_kfree_skb*(NULL) check to checkpatch Govindarajulu Varadarajan
@ 2013-11-04 0:37 ` Joe Perches
0 siblings, 0 replies; 20+ messages in thread
From: Joe Perches @ 2013-11-04 0:37 UTC (permalink / raw)
To: Govindarajulu Varadarajan
Cc: davem, gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas,
roland, isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, apw
On Sat, 2013-11-02 at 19:17 +0530, Govindarajulu Varadarajan wrote:
> Signed-off-by: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -3719,7 +3719,7 @@ sub process {
> # check for needless "if (<foo>) fn(<foo>)" uses
> if ($prevline =~ /\bif\s*\(\s*($Lval)\s*\)/) {
> my $expr = '\s*\(\s*' . quotemeta($1) . '\s*\)\s*;';
> - if ($line =~ /\b(kfree|usb_free_urb|debugfs_remove(?:_recursive)?)$expr/) {
> + if ($line =~ /\b(kfree|dev_kfree_skb|dev_kfree_skb_any|dev_kfree_skb_irq|usb_free_urb|debugfs_remove(?:_recursive)?)$expr/) {
> WARN('NEEDLESS_IF',
> "$1(NULL) is safe this check is probably not required\n" . $hereprev);
> }
OK, but I think this is easier to read as
dev_kfree_skb(?:_skb|_any|_irq)?
^ permalink raw reply [flat|nested] 20+ messages in thread