Patchwork net : fix for dst_gc_task not getting scheduled if __dst_free() is called consistently

login
register
mail settings
Submitter b15745@freescale.com
Date May 19, 2012, 9:44 a.m.
Message ID <1337420654-28200-1-git-send-email-b15745@freescale.com>
Download mbox | patch
Permalink /patch/160179/
State Rejected
Delegated to: David Miller
Headers show

Comments

b15745@freescale.com - May 19, 2012, 9:44 a.m.
From: Rajan Gupta <b15745@freescale.com>

dst_gc_work is cancelled and again rescheduled in __ds_free(). In case
__dsf_free() is consistently called dst_gc_work will never get called resulting in
memory not getting freed at all until one stops calling __dst_free
Signed-off-by: Rajan Gupta <b15745@freescale.com>
---
 net/core/dst.c |    1 -
 1 file changed, 1 deletion(-)
Eric Dumazet - May 19, 2012, 10:04 a.m.
On Sat, 2012-05-19 at 04:44 -0500, b15745@freescale.com wrote:
> From: Rajan Gupta <b15745@freescale.com>
> 
> dst_gc_work is cancelled and again rescheduled in __ds_free(). In case
> __dsf_free() is consistently called dst_gc_work will never get called resulting in
> memory not getting freed at all until one stops calling __dst_free
> Signed-off-by: Rajan Gupta <b15745@freescale.com>
> ---
>  net/core/dst.c |    1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/net/core/dst.c b/net/core/dst.c
> index 8246d47..6820206 100644
> --- a/net/core/dst.c
> +++ b/net/core/dst.c
> @@ -215,7 +215,6 @@ void __dst_free(struct dst_entry *dst)
>  	if (dst_garbage.timer_inc > DST_GC_INC) {
>  		dst_garbage.timer_inc = DST_GC_INC;
>  		dst_garbage.timer_expires = DST_GC_MIN;
> -		cancel_delayed_work(&dst_gc_work);
>  		schedule_delayed_work(&dst_gc_work, dst_garbage.timer_expires);
>  	}
>  	spin_unlock_bh(&dst_garbage.lock);

Strange, I never met this....

Since "if (dst_garbage.timer_inc > DST_GC_INC)" will be false if
timer_inc is DST_GC_INC.



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
b15745@freescale.com - May 19, 2012, 2:48 p.m.
Ignore the patch. Issue is dst_gc_task is not getting
scheduled because of scheduling issue at 100% cpu utilization.

> -----Original Message-----

> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]

> Sent: Saturday, May 19, 2012 3:34 PM

> To: Gupta Rajan-B15745

> Cc: netdev@vger.kernel.org

> Subject: Re: [PATCH] net : fix for dst_gc_task not getting scheduled if

> __dst_free() is called consistently

> 

> On Sat, 2012-05-19 at 04:44 -0500, b15745@freescale.com wrote:

> > From: Rajan Gupta <b15745@freescale.com>

> >

> > dst_gc_work is cancelled and again rescheduled in __ds_free(). In case

> > __dsf_free() is consistently called dst_gc_work will never get called

> > resulting in memory not getting freed at all until one stops calling

> > __dst_free

> > Signed-off-by: Rajan Gupta <b15745@freescale.com>

> > ---

> >  net/core/dst.c |    1 -

> >  1 file changed, 1 deletion(-)

> >

> > diff --git a/net/core/dst.c b/net/core/dst.c index 8246d47..6820206

> > 100644

> > --- a/net/core/dst.c

> > +++ b/net/core/dst.c

> > @@ -215,7 +215,6 @@ void __dst_free(struct dst_entry *dst)

> >  	if (dst_garbage.timer_inc > DST_GC_INC) {

> >  		dst_garbage.timer_inc = DST_GC_INC;

> >  		dst_garbage.timer_expires = DST_GC_MIN;

> > -		cancel_delayed_work(&dst_gc_work);

> >  		schedule_delayed_work(&dst_gc_work, dst_garbage.timer_expires);

> >  	}

> >  	spin_unlock_bh(&dst_garbage.lock);

> 

> Strange, I never met this....

> 

> Since "if (dst_garbage.timer_inc > DST_GC_INC)" will be false if timer_inc

> is DST_GC_INC.

> 

> 

>
David Miller - May 19, 2012, 6:36 p.m.
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 19 May 2012 12:04:23 +0200

> On Sat, 2012-05-19 at 04:44 -0500, b15745@freescale.com wrote:
>> From: Rajan Gupta <b15745@freescale.com>
>> 
>> dst_gc_work is cancelled and again rescheduled in __ds_free(). In case
>> __dsf_free() is consistently called dst_gc_work will never get called resulting in
>> memory not getting freed at all until one stops calling __dst_free
>> Signed-off-by: Rajan Gupta <b15745@freescale.com>
>> ---
>>  net/core/dst.c |    1 -
>>  1 file changed, 1 deletion(-)
>> 
>> diff --git a/net/core/dst.c b/net/core/dst.c
>> index 8246d47..6820206 100644
>> --- a/net/core/dst.c
>> +++ b/net/core/dst.c
>> @@ -215,7 +215,6 @@ void __dst_free(struct dst_entry *dst)
>>  	if (dst_garbage.timer_inc > DST_GC_INC) {
>>  		dst_garbage.timer_inc = DST_GC_INC;
>>  		dst_garbage.timer_expires = DST_GC_MIN;
>> -		cancel_delayed_work(&dst_gc_work);
>>  		schedule_delayed_work(&dst_gc_work, dst_garbage.timer_expires);
>>  	}
>>  	spin_unlock_bh(&dst_garbage.lock);
> 
> Strange, I never met this....
> 
> Since "if (dst_garbage.timer_inc > DST_GC_INC)" will be false if
> timer_inc is DST_GC_INC.

I'm not considering this patch at all until we figure out what
the real situation is here.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/core/dst.c b/net/core/dst.c
index 8246d47..6820206 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -215,7 +215,6 @@  void __dst_free(struct dst_entry *dst)
 	if (dst_garbage.timer_inc > DST_GC_INC) {
 		dst_garbage.timer_inc = DST_GC_INC;
 		dst_garbage.timer_expires = DST_GC_MIN;
-		cancel_delayed_work(&dst_gc_work);
 		schedule_delayed_work(&dst_gc_work, dst_garbage.timer_expires);
 	}
 	spin_unlock_bh(&dst_garbage.lock);