diff mbox series

[net,v2] ethernet: s2io: don't call dev_kfree_skb() under spin_lock_irqsave()

Message ID 20221207012540.2717379-1-yangyingliang@huawei.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net,v2] ethernet: s2io: don't call dev_kfree_skb() under spin_lock_irqsave() | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 17 this patch: 17
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1 this patch: 1
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Yang Yingliang Dec. 7, 2022, 1:25 a.m. UTC
It is not allowed to call consume_skb() from hardware interrupt context
or with interrupts being disabled. So replace dev_kfree_skb() with
dev_consume_skb_irq() under spin_lock_irqsave().

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
v1 -> v2:
  Add fix tag.
---
 drivers/net/ethernet/neterion/s2io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Leon Romanovsky Dec. 8, 2022, 7:56 a.m. UTC | #1
On Wed, Dec 07, 2022 at 09:25:40AM +0800, Yang Yingliang wrote:
> It is not allowed to call consume_skb() from hardware interrupt context
> or with interrupts being disabled. So replace dev_kfree_skb() with
> dev_consume_skb_irq() under spin_lock_irqsave().
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
> v1 -> v2:
>   Add fix tag.
> ---
>  drivers/net/ethernet/neterion/s2io.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/neterion/s2io.c b/drivers/net/ethernet/neterion/s2io.c
> index 1d3c4474b7cb..a83d61d45936 100644
> --- a/drivers/net/ethernet/neterion/s2io.c
> +++ b/drivers/net/ethernet/neterion/s2io.c
> @@ -2386,7 +2386,7 @@ static void free_tx_buffers(struct s2io_nic *nic)
>  			skb = s2io_txdl_getskb(&mac_control->fifos[i], txdp, j);
>  			if (skb) {
>  				swstats->mem_freed += skb->truesize;
> -				dev_kfree_skb(skb);
> +				dev_consume_skb_irq(skb);

And why did you use dev_consume_skb_irq() and not dev_kfree_skb_irq()?

Thanks

>  				cnt++;
>  			}
>  		}
> -- 
> 2.25.1
>
Yang Yingliang Dec. 8, 2022, 8:40 a.m. UTC | #2
On 2022/12/8 15:56, Leon Romanovsky wrote:
> On Wed, Dec 07, 2022 at 09:25:40AM +0800, Yang Yingliang wrote:
>> It is not allowed to call consume_skb() from hardware interrupt context
>> or with interrupts being disabled. So replace dev_kfree_skb() with
>> dev_consume_skb_irq() under spin_lock_irqsave().
>>
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>> ---
>> v1 -> v2:
>>    Add fix tag.
>> ---
>>   drivers/net/ethernet/neterion/s2io.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/neterion/s2io.c b/drivers/net/ethernet/neterion/s2io.c
>> index 1d3c4474b7cb..a83d61d45936 100644
>> --- a/drivers/net/ethernet/neterion/s2io.c
>> +++ b/drivers/net/ethernet/neterion/s2io.c
>> @@ -2386,7 +2386,7 @@ static void free_tx_buffers(struct s2io_nic *nic)
>>   			skb = s2io_txdl_getskb(&mac_control->fifos[i], txdp, j);
>>   			if (skb) {
>>   				swstats->mem_freed += skb->truesize;
>> -				dev_kfree_skb(skb);
>> +				dev_consume_skb_irq(skb);
> And why did you use dev_consume_skb_irq() and not dev_kfree_skb_irq()?
I chose dev_consume_skb_irq(), because dev_kfree_skb() is consume_skb().

Thanks,
Yang
>
> Thanks
>
>>   				cnt++;
>>   			}
>>   		}
>> -- 
>> 2.25.1
>>
> .
Leon Romanovsky Dec. 8, 2022, 8:51 a.m. UTC | #3
On Thu, Dec 08, 2022 at 04:40:35PM +0800, Yang Yingliang wrote:
> 
> On 2022/12/8 15:56, Leon Romanovsky wrote:
> > On Wed, Dec 07, 2022 at 09:25:40AM +0800, Yang Yingliang wrote:
> > > It is not allowed to call consume_skb() from hardware interrupt context
> > > or with interrupts being disabled. So replace dev_kfree_skb() with
> > > dev_consume_skb_irq() under spin_lock_irqsave().
> > > 
> > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> > > ---
> > > v1 -> v2:
> > >    Add fix tag.
> > > ---
> > >   drivers/net/ethernet/neterion/s2io.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/ethernet/neterion/s2io.c b/drivers/net/ethernet/neterion/s2io.c
> > > index 1d3c4474b7cb..a83d61d45936 100644
> > > --- a/drivers/net/ethernet/neterion/s2io.c
> > > +++ b/drivers/net/ethernet/neterion/s2io.c
> > > @@ -2386,7 +2386,7 @@ static void free_tx_buffers(struct s2io_nic *nic)
> > >   			skb = s2io_txdl_getskb(&mac_control->fifos[i], txdp, j);
> > >   			if (skb) {
> > >   				swstats->mem_freed += skb->truesize;
> > > -				dev_kfree_skb(skb);
> > > +				dev_consume_skb_irq(skb);
> > And why did you use dev_consume_skb_irq() and not dev_kfree_skb_irq()?
> I chose dev_consume_skb_irq(), because dev_kfree_skb() is consume_skb().

Your commit message, title and actual change are totally misleading.
You replaced *_kfree_* with *_consume_* while talking about running it
in interrupts disabled context.

Thanks

> 
> Thanks,
> Yang
> > 
> > Thanks
> > 
> > >   				cnt++;
> > >   			}
> > >   		}
> > > -- 
> > > 2.25.1
> > > 
> > .
Yang Yingliang Dec. 8, 2022, 9:03 a.m. UTC | #4
On 2022/12/8 16:51, Leon Romanovsky wrote:
> On Thu, Dec 08, 2022 at 04:40:35PM +0800, Yang Yingliang wrote:
>> On 2022/12/8 15:56, Leon Romanovsky wrote:
>>> On Wed, Dec 07, 2022 at 09:25:40AM +0800, Yang Yingliang wrote:
>>>> It is not allowed to call consume_skb() from hardware interrupt context
>>>> or with interrupts being disabled. So replace dev_kfree_skb() with
>>>> dev_consume_skb_irq() under spin_lock_irqsave().
>>>>
>>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>>>> ---
>>>> v1 -> v2:
>>>>     Add fix tag.
>>>> ---
>>>>    drivers/net/ethernet/neterion/s2io.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/neterion/s2io.c b/drivers/net/ethernet/neterion/s2io.c
>>>> index 1d3c4474b7cb..a83d61d45936 100644
>>>> --- a/drivers/net/ethernet/neterion/s2io.c
>>>> +++ b/drivers/net/ethernet/neterion/s2io.c
>>>> @@ -2386,7 +2386,7 @@ static void free_tx_buffers(struct s2io_nic *nic)
>>>>    			skb = s2io_txdl_getskb(&mac_control->fifos[i], txdp, j);
>>>>    			if (skb) {
>>>>    				swstats->mem_freed += skb->truesize;
>>>> -				dev_kfree_skb(skb);
>>>> +				dev_consume_skb_irq(skb);
>>> And why did you use dev_consume_skb_irq() and not dev_kfree_skb_irq()?
>> I chose dev_consume_skb_irq(), because dev_kfree_skb() is consume_skb().
> Your commit message, title and actual change are totally misleading.
> You replaced *_kfree_* with *_consume_* while talking about running it
> in interrupts disabled context.
I didn't mention dev_kfree_skb() is same as consume_skb(), I can add it 
to my
commit message and send a new version.

Thanks,
Yang
>
> Thanks
>
>> Thanks,
>> Yang
>>> Thanks
>>>
>>>>    				cnt++;
>>>>    			}
>>>>    		}
>>>> -- 
>>>> 2.25.1
>>>>
>>> .
> .
Leon Romanovsky Dec. 8, 2022, 9:27 a.m. UTC | #5
On Thu, Dec 08, 2022 at 05:03:20PM +0800, Yang Yingliang wrote:
> 
> On 2022/12/8 16:51, Leon Romanovsky wrote:
> > On Thu, Dec 08, 2022 at 04:40:35PM +0800, Yang Yingliang wrote:
> > > On 2022/12/8 15:56, Leon Romanovsky wrote:
> > > > On Wed, Dec 07, 2022 at 09:25:40AM +0800, Yang Yingliang wrote:
> > > > > It is not allowed to call consume_skb() from hardware interrupt context
> > > > > or with interrupts being disabled. So replace dev_kfree_skb() with
> > > > > dev_consume_skb_irq() under spin_lock_irqsave().
> > > > > 
> > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > > > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> > > > > ---
> > > > > v1 -> v2:
> > > > >     Add fix tag.
> > > > > ---
> > > > >    drivers/net/ethernet/neterion/s2io.c | 2 +-
> > > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/net/ethernet/neterion/s2io.c b/drivers/net/ethernet/neterion/s2io.c
> > > > > index 1d3c4474b7cb..a83d61d45936 100644
> > > > > --- a/drivers/net/ethernet/neterion/s2io.c
> > > > > +++ b/drivers/net/ethernet/neterion/s2io.c
> > > > > @@ -2386,7 +2386,7 @@ static void free_tx_buffers(struct s2io_nic *nic)
> > > > >    			skb = s2io_txdl_getskb(&mac_control->fifos[i], txdp, j);
> > > > >    			if (skb) {
> > > > >    				swstats->mem_freed += skb->truesize;
> > > > > -				dev_kfree_skb(skb);
> > > > > +				dev_consume_skb_irq(skb);
> > > > And why did you use dev_consume_skb_irq() and not dev_kfree_skb_irq()?
> > > I chose dev_consume_skb_irq(), because dev_kfree_skb() is consume_skb().
> > Your commit message, title and actual change are totally misleading.
> > You replaced *_kfree_* with *_consume_* while talking about running it
> > in interrupts disabled context.
> I didn't mention dev_kfree_skb() is same as consume_skb(), I can add it to
> my
> commit message and send a new version.

I need you to send patch with commit message which aligns to the actual change.
Right now, it is not.

Thanks

> 
> Thanks,
> Yang
> > 
> > Thanks
> > 
> > > Thanks,
> > > Yang
> > > > Thanks
> > > > 
> > > > >    				cnt++;
> > > > >    			}
> > > > >    		}
> > > > > -- 
> > > > > 2.25.1
> > > > > 
> > > > .
> > .
diff mbox series

Patch

diff --git a/drivers/net/ethernet/neterion/s2io.c b/drivers/net/ethernet/neterion/s2io.c
index 1d3c4474b7cb..a83d61d45936 100644
--- a/drivers/net/ethernet/neterion/s2io.c
+++ b/drivers/net/ethernet/neterion/s2io.c
@@ -2386,7 +2386,7 @@  static void free_tx_buffers(struct s2io_nic *nic)
 			skb = s2io_txdl_getskb(&mac_control->fifos[i], txdp, j);
 			if (skb) {
 				swstats->mem_freed += skb->truesize;
-				dev_kfree_skb(skb);
+				dev_consume_skb_irq(skb);
 				cnt++;
 			}
 		}