All of lore.kernel.org
 help / color / mirror / Atom feed
* [Ocfs2-devel] [PATCH] ocfs2: Free sc->sc_page in sc_kref_release()
@ 2013-06-18  3:40 Younger Liu
  2013-06-18  4:20 ` Jeff Liu
  0 siblings, 1 reply; 5+ messages in thread
From: Younger Liu @ 2013-06-18  3:40 UTC (permalink / raw)
  To: ocfs2-devel

There is a memory leak in sc_kref_release().
When free struct o2net_sock_container (sc), 
we should release sc->sc_page.

Signed-off-by: Younger Liu <younger.liu@huawei.com>
---
 fs/ocfs2/cluster/tcp.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
index aa88bd8..f0272b9 100644
--- a/fs/ocfs2/cluster/tcp.c
+++ b/fs/ocfs2/cluster/tcp.c
@@ -406,6 +406,11 @@ static void sc_kref_release(struct kref *kref)
 	sc->sc_node = NULL;
 
 	o2net_debug_del_sc(sc);
+
+	if (sc->sc_page) {
+		__free_page(sc->sc_page);
+		sc->sc_page = NULL;
+	}
 	kfree(sc);
 }
 
-- 1.7.9.7

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

* [Ocfs2-devel] [PATCH] ocfs2: Free sc->sc_page in sc_kref_release()
  2013-06-18  3:40 [Ocfs2-devel] [PATCH] ocfs2: Free sc->sc_page in sc_kref_release() Younger Liu
@ 2013-06-18  4:20 ` Jeff Liu
  2013-06-18  4:29   ` Li Zefan
  0 siblings, 1 reply; 5+ messages in thread
From: Jeff Liu @ 2013-06-18  4:20 UTC (permalink / raw)
  To: ocfs2-devel

On 06/18/2013 11:40 AM, Younger Liu wrote:

> There is a memory leak in sc_kref_release().
> When free struct o2net_sock_container (sc), 
> we should release sc->sc_page.
> 
> Signed-off-by: Younger Liu <younger.liu@huawei.com>

Looks fine to me, thanks!

Reviewed-by: Jie Liu <jeff.liu@oracle.com>

> ---
>  fs/ocfs2/cluster/tcp.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
> index aa88bd8..f0272b9 100644
> --- a/fs/ocfs2/cluster/tcp.c
> +++ b/fs/ocfs2/cluster/tcp.c
> @@ -406,6 +406,11 @@ static void sc_kref_release(struct kref *kref)
>  	sc->sc_node = NULL;
>  
>  	o2net_debug_del_sc(sc);
> +
> +	if (sc->sc_page) {
> +		__free_page(sc->sc_page);
> +		sc->sc_page = NULL;
> +	}
>  	kfree(sc);
>  }
>  
> -- 1.7.9.7

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

* [Ocfs2-devel] [PATCH] ocfs2: Free sc->sc_page in sc_kref_release()
  2013-06-18  4:20 ` Jeff Liu
@ 2013-06-18  4:29   ` Li Zefan
  2013-06-18  4:35     ` Jeff Liu
  0 siblings, 1 reply; 5+ messages in thread
From: Li Zefan @ 2013-06-18  4:29 UTC (permalink / raw)
  To: ocfs2-devel

On 2013/6/18 12:20, Jeff Liu wrote:
> On 06/18/2013 11:40 AM, Younger Liu wrote:
> 
>> There is a memory leak in sc_kref_release().
>> When free struct o2net_sock_container (sc), 
>> we should release sc->sc_page.
>>
>> Signed-off-by: Younger Liu <younger.liu@huawei.com>
> 
> Looks fine to me, thanks!
> 
> Reviewed-by: Jie Liu <jeff.liu@oracle.com>
> 
>> ---
>>  fs/ocfs2/cluster/tcp.c |    5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
>> index aa88bd8..f0272b9 100644
>> --- a/fs/ocfs2/cluster/tcp.c
>> +++ b/fs/ocfs2/cluster/tcp.c
>> @@ -406,6 +406,11 @@ static void sc_kref_release(struct kref *kref)
>>  	sc->sc_node = NULL;
>>  
>>  	o2net_debug_del_sc(sc);
>> +
>> +	if (sc->sc_page) {
>> +		__free_page(sc->sc_page);
>> +		sc->sc_page = NULL;

But why set sc->sc_page to NULL, given sc is to be kfreed.

>> +	}
>>  	kfree(sc);
>>  }

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

* [Ocfs2-devel] [PATCH] ocfs2: Free sc->sc_page in sc_kref_release()
  2013-06-18  4:29   ` Li Zefan
@ 2013-06-18  4:35     ` Jeff Liu
  2013-06-18  5:57       ` Younger Liu
  0 siblings, 1 reply; 5+ messages in thread
From: Jeff Liu @ 2013-06-18  4:35 UTC (permalink / raw)
  To: ocfs2-devel

On 06/18/2013 12:29 PM, Li Zefan wrote:

> On 2013/6/18 12:20, Jeff Liu wrote:
>> On 06/18/2013 11:40 AM, Younger Liu wrote:
>>
>>> There is a memory leak in sc_kref_release().
>>> When free struct o2net_sock_container (sc), 
>>> we should release sc->sc_page.
>>>
>>> Signed-off-by: Younger Liu <younger.liu@huawei.com>
>>
>> Looks fine to me, thanks!
>>
>> Reviewed-by: Jie Liu <jeff.liu@oracle.com>
>>
>>> ---
>>>  fs/ocfs2/cluster/tcp.c |    5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
>>> index aa88bd8..f0272b9 100644
>>> --- a/fs/ocfs2/cluster/tcp.c
>>> +++ b/fs/ocfs2/cluster/tcp.c
>>> @@ -406,6 +406,11 @@ static void sc_kref_release(struct kref *kref)
>>>  	sc->sc_node = NULL;
>>>  
>>>  	o2net_debug_del_sc(sc);
>>> +
>>> +	if (sc->sc_page) {
>>> +		__free_page(sc->sc_page);
>>> +		sc->sc_page = NULL;
> 
> But why set sc->sc_page to NULL, given sc is to be kfreed.

Nice catch. I have not reacted from another thing in process at that time.

Thanks,
-Jeff

> 
>>> +	}
>>>  	kfree(sc);
>>>  }
> 

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

* [Ocfs2-devel] [PATCH] ocfs2: Free sc->sc_page in sc_kref_release()
  2013-06-18  4:35     ` Jeff Liu
@ 2013-06-18  5:57       ` Younger Liu
  0 siblings, 0 replies; 5+ messages in thread
From: Younger Liu @ 2013-06-18  5:57 UTC (permalink / raw)
  To: ocfs2-devel

Thanks for your review comments.
I will resend this patch.

On 2013/6/18 12:35, Jeff Liu wrote:
> On 06/18/2013 12:29 PM, Li Zefan wrote:
> 
>> On 2013/6/18 12:20, Jeff Liu wrote:
>>> On 06/18/2013 11:40 AM, Younger Liu wrote:
>>>
>>>> There is a memory leak in sc_kref_release().
>>>> When free struct o2net_sock_container (sc), 
>>>> we should release sc->sc_page.
>>>>
>>>> Signed-off-by: Younger Liu <younger.liu@huawei.com>
>>>
>>> Looks fine to me, thanks!
>>>
>>> Reviewed-by: Jie Liu <jeff.liu@oracle.com>
>>>
>>>> ---
>>>>  fs/ocfs2/cluster/tcp.c |    5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
>>>> index aa88bd8..f0272b9 100644
>>>> --- a/fs/ocfs2/cluster/tcp.c
>>>> +++ b/fs/ocfs2/cluster/tcp.c
>>>> @@ -406,6 +406,11 @@ static void sc_kref_release(struct kref *kref)
>>>>  	sc->sc_node = NULL;
>>>>  
>>>>  	o2net_debug_del_sc(sc);
>>>> +
>>>> +	if (sc->sc_page) {
>>>> +		__free_page(sc->sc_page);
>>>> +		sc->sc_page = NULL;
>>
>> But why set sc->sc_page to NULL, given sc is to be kfreed.
> 
> Nice catch. I have not reacted from another thing in process at that time.
> 
> Thanks,
> -Jeff
> 
>>
>>>> +	}
>>>>  	kfree(sc);
>>>>  }
>>
> 
> 
> 
> 

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

end of thread, other threads:[~2013-06-18  5:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-18  3:40 [Ocfs2-devel] [PATCH] ocfs2: Free sc->sc_page in sc_kref_release() Younger Liu
2013-06-18  4:20 ` Jeff Liu
2013-06-18  4:29   ` Li Zefan
2013-06-18  4:35     ` Jeff Liu
2013-06-18  5:57       ` Younger Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.