* [Qemu-devel] [PATCH-trivial] arch_init.c: Free 'cache' in cache_fini() to avoid memory leak
@ 2014-06-02 12:16 Chen Gang
2014-06-03 7:17 ` ChenLiang
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Chen Gang @ 2014-06-02 12:16 UTC (permalink / raw)
To: Michael Tokarev, quintela, arei.gonglei, Eric Blake, dgilbert, owasserm
Cc: QEMU Trivial, QEMU Developers
Call g_free() after cache_fini() in migration_end(), but do not call
g_free() after call cache_fini() in xbzrle_cache_resize() which will
cause memory leak.
cache_init() and cache_fini() are pair, so need let cache_fini() call
g_free(cache) to match cache_init(), then fix current issue too.
Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
arch_init.c | 1 -
page_cache.c | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch_init.c b/arch_init.c
index 9f1a174..23044c1 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -739,7 +739,6 @@ static void migration_end(void)
XBZRLE_cache_lock();
if (XBZRLE.cache) {
cache_fini(XBZRLE.cache);
- g_free(XBZRLE.cache);
g_free(XBZRLE.encoded_buf);
g_free(XBZRLE.current_buf);
XBZRLE.cache = NULL;
diff --git a/page_cache.c b/page_cache.c
index b033681..89bb1ec 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -109,6 +109,7 @@ void cache_fini(PageCache *cache)
g_free(cache->page_cache);
cache->page_cache = NULL;
+ g_free(cache);
}
static size_t cache_get_cache_pos(const PageCache *cache,
--
1.7.11.7
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH-trivial] arch_init.c: Free 'cache' in cache_fini() to avoid memory leak
2014-06-02 12:16 [Qemu-devel] [PATCH-trivial] arch_init.c: Free 'cache' in cache_fini() to avoid memory leak Chen Gang
@ 2014-06-03 7:17 ` ChenLiang
2014-06-04 8:16 ` Markus Armbruster
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: ChenLiang @ 2014-06-03 7:17 UTC (permalink / raw)
To: Chen Gang
Cc: quintela, QEMU Trivial, Michael Tokarev, dgilbert,
QEMU Developers, owasserm, arei.gonglei
On 2014/6/2 20:16, Chen Gang wrote:
> Call g_free() after cache_fini() in migration_end(), but do not call
> g_free() after call cache_fini() in xbzrle_cache_resize() which will
> cause memory leak.
>
> cache_init() and cache_fini() are pair, so need let cache_fini() call
> g_free(cache) to match cache_init(), then fix current issue too.
>
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
> arch_init.c | 1 -
> page_cache.c | 1 +
> 2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 9f1a174..23044c1 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -739,7 +739,6 @@ static void migration_end(void)
> XBZRLE_cache_lock();
> if (XBZRLE.cache) {
> cache_fini(XBZRLE.cache);
> - g_free(XBZRLE.cache);
> g_free(XBZRLE.encoded_buf);
> g_free(XBZRLE.current_buf);
> XBZRLE.cache = NULL;
> diff --git a/page_cache.c b/page_cache.c
> index b033681..89bb1ec 100644
> --- a/page_cache.c
> +++ b/page_cache.c
> @@ -109,6 +109,7 @@ void cache_fini(PageCache *cache)
>
> g_free(cache->page_cache);
> cache->page_cache = NULL;
> + g_free(cache);
> }
>
> static size_t cache_get_cache_pos(const PageCache *cache,
Reviewed-by: ChenLiang <chenliang88@huawei.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH-trivial] arch_init.c: Free 'cache' in cache_fini() to avoid memory leak
2014-06-02 12:16 [Qemu-devel] [PATCH-trivial] arch_init.c: Free 'cache' in cache_fini() to avoid memory leak Chen Gang
2014-06-03 7:17 ` ChenLiang
@ 2014-06-04 8:16 ` Markus Armbruster
2014-06-04 10:28 ` Chen Gang
2014-06-04 9:35 ` Dr. David Alan Gilbert
2014-06-08 17:53 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
3 siblings, 1 reply; 8+ messages in thread
From: Markus Armbruster @ 2014-06-04 8:16 UTC (permalink / raw)
To: Chen Gang
Cc: quintela, QEMU Trivial, Michael Tokarev, dgilbert,
QEMU Developers, owasserm, arei.gonglei
Chen Gang <gang.chen.5i5j@gmail.com> writes:
> Call g_free() after cache_fini() in migration_end(), but do not call
> g_free() after call cache_fini() in xbzrle_cache_resize() which will
> cause memory leak.
>
> cache_init() and cache_fini() are pair, so need let cache_fini() call
> g_free(cache) to match cache_init(), then fix current issue too.
I'm not sure I get you. Is the following accurate?
migration: Plug memory leak in migrate-set-cache-size command
We call g_free() after cache_fini() in migration_end(), but we don't
call it after cache_fini() in xbzrle_cache_resize(), leaking the
memory.
cache_init() and cache_fini() are a pair. Since cache_init()
allocates the cache, let cache_fini() free it. This plugs the leak.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH-trivial] arch_init.c: Free 'cache' in cache_fini() to avoid memory leak
2014-06-02 12:16 [Qemu-devel] [PATCH-trivial] arch_init.c: Free 'cache' in cache_fini() to avoid memory leak Chen Gang
2014-06-03 7:17 ` ChenLiang
2014-06-04 8:16 ` Markus Armbruster
@ 2014-06-04 9:35 ` Dr. David Alan Gilbert
2014-06-08 17:53 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
3 siblings, 0 replies; 8+ messages in thread
From: Dr. David Alan Gilbert @ 2014-06-04 9:35 UTC (permalink / raw)
To: Chen Gang
Cc: quintela, QEMU Trivial, Michael Tokarev, QEMU Developers, arei.gonglei
* Chen Gang (gang.chen.5i5j@gmail.com) wrote:
> Call g_free() after cache_fini() in migration_end(), but do not call
> g_free() after call cache_fini() in xbzrle_cache_resize() which will
> cause memory leak.
>
> cache_init() and cache_fini() are pair, so need let cache_fini() call
> g_free(cache) to match cache_init(), then fix current issue too.
>
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
Yes, good catch.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> arch_init.c | 1 -
> page_cache.c | 1 +
> 2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 9f1a174..23044c1 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -739,7 +739,6 @@ static void migration_end(void)
> XBZRLE_cache_lock();
> if (XBZRLE.cache) {
> cache_fini(XBZRLE.cache);
> - g_free(XBZRLE.cache);
> g_free(XBZRLE.encoded_buf);
> g_free(XBZRLE.current_buf);
> XBZRLE.cache = NULL;
> diff --git a/page_cache.c b/page_cache.c
> index b033681..89bb1ec 100644
> --- a/page_cache.c
> +++ b/page_cache.c
> @@ -109,6 +109,7 @@ void cache_fini(PageCache *cache)
>
> g_free(cache->page_cache);
> cache->page_cache = NULL;
> + g_free(cache);
> }
>
> static size_t cache_get_cache_pos(const PageCache *cache,
> --
> 1.7.11.7
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH-trivial] arch_init.c: Free 'cache' in cache_fini() to avoid memory leak
2014-06-04 8:16 ` Markus Armbruster
@ 2014-06-04 10:28 ` Chen Gang
2014-06-04 10:54 ` Markus Armbruster
0 siblings, 1 reply; 8+ messages in thread
From: Chen Gang @ 2014-06-04 10:28 UTC (permalink / raw)
To: Markus Armbruster
Cc: quintela, QEMU Trivial, Michael Tokarev, dgilbert,
QEMU Developers, owasserm, arei.gonglei
On 06/04/2014 04:16 PM, Markus Armbruster wrote:
> Chen Gang <gang.chen.5i5j@gmail.com> writes:
>
>> Call g_free() after cache_fini() in migration_end(), but do not call
>> g_free() after call cache_fini() in xbzrle_cache_resize() which will
>> cause memory leak.
>>
>> cache_init() and cache_fini() are pair, so need let cache_fini() call
>> g_free(cache) to match cache_init(), then fix current issue too.
>
> I'm not sure I get you. Is the following accurate?
>
> migration: Plug memory leak in migrate-set-cache-size command
>
> We call g_free() after cache_fini() in migration_end(), but we don't
> call it after cache_fini() in xbzrle_cache_resize(), leaking the
> memory.
>
> cache_init() and cache_fini() are a pair. Since cache_init()
> allocates the cache, let cache_fini() free it. This plugs the leak.
>
Yeah, and excuse me for my poor English.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH-trivial] arch_init.c: Free 'cache' in cache_fini() to avoid memory leak
2014-06-04 10:28 ` Chen Gang
@ 2014-06-04 10:54 ` Markus Armbruster
2014-06-04 11:30 ` Chen Gang
0 siblings, 1 reply; 8+ messages in thread
From: Markus Armbruster @ 2014-06-04 10:54 UTC (permalink / raw)
To: Chen Gang
Cc: quintela, QEMU Trivial, Michael Tokarev, QEMU Developers,
dgilbert, owasserm, arei.gonglei
Chen Gang <gang.chen.5i5j@gmail.com> writes:
> On 06/04/2014 04:16 PM, Markus Armbruster wrote:
>> Chen Gang <gang.chen.5i5j@gmail.com> writes:
>>
>>> Call g_free() after cache_fini() in migration_end(), but do not call
>>> g_free() after call cache_fini() in xbzrle_cache_resize() which will
>>> cause memory leak.
>>>
>>> cache_init() and cache_fini() are pair, so need let cache_fini() call
>>> g_free(cache) to match cache_init(), then fix current issue too.
>>
>> I'm not sure I get you. Is the following accurate?
>>
>> migration: Plug memory leak in migrate-set-cache-size command
>>
>> We call g_free() after cache_fini() in migration_end(), but we don't
>> call it after cache_fini() in xbzrle_cache_resize(), leaking the
>> memory.
>>
>> cache_init() and cache_fini() are a pair. Since cache_init()
>> allocates the cache, let cache_fini() free it. This plugs the leak.
>>
>
> Yeah, and excuse me for my poor English.
No problem, we figured it out together :)
Would be nice to use the above text as commit message. Perhaps it can
be done on commit.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH-trivial] arch_init.c: Free 'cache' in cache_fini() to avoid memory leak
2014-06-04 10:54 ` Markus Armbruster
@ 2014-06-04 11:30 ` Chen Gang
0 siblings, 0 replies; 8+ messages in thread
From: Chen Gang @ 2014-06-04 11:30 UTC (permalink / raw)
To: Markus Armbruster
Cc: quintela, QEMU Trivial, Michael Tokarev, QEMU Developers,
dgilbert, owasserm, arei.gonglei
On 06/04/2014 06:54 PM, Markus Armbruster wrote:
> Chen Gang <gang.chen.5i5j@gmail.com> writes:
>
>> On 06/04/2014 04:16 PM, Markus Armbruster wrote:
>>> Chen Gang <gang.chen.5i5j@gmail.com> writes:
>>>
>>>> Call g_free() after cache_fini() in migration_end(), but do not call
>>>> g_free() after call cache_fini() in xbzrle_cache_resize() which will
>>>> cause memory leak.
>>>>
>>>> cache_init() and cache_fini() are pair, so need let cache_fini() call
>>>> g_free(cache) to match cache_init(), then fix current issue too.
>>>
>>> I'm not sure I get you. Is the following accurate?
>>>
>>> migration: Plug memory leak in migrate-set-cache-size command
>>>
>>> We call g_free() after cache_fini() in migration_end(), but we don't
>>> call it after cache_fini() in xbzrle_cache_resize(), leaking the
>>> memory.
>>>
>>> cache_init() and cache_fini() are a pair. Since cache_init()
>>> allocates the cache, let cache_fini() free it. This plugs the leak.
>>>
>>
>> Yeah, and excuse me for my poor English.
>
> No problem, we figured it out together :)
>
> Would be nice to use the above text as commit message. Perhaps it can
> be done on commit.
>
That is fine to me.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH-trivial] arch_init.c: Free 'cache' in cache_fini() to avoid memory leak
2014-06-02 12:16 [Qemu-devel] [PATCH-trivial] arch_init.c: Free 'cache' in cache_fini() to avoid memory leak Chen Gang
` (2 preceding siblings ...)
2014-06-04 9:35 ` Dr. David Alan Gilbert
@ 2014-06-08 17:53 ` Michael Tokarev
3 siblings, 0 replies; 8+ messages in thread
From: Michael Tokarev @ 2014-06-08 17:53 UTC (permalink / raw)
To: Chen Gang, quintela, arei.gonglei, Eric Blake, dgilbert, owasserm
Cc: QEMU Trivial, QEMU Developers
02.06.2014 16:16, Chen Gang wrote:
> Call g_free() after cache_fini() in migration_end(), but do not call
> g_free() after call cache_fini() in xbzrle_cache_resize() which will
> cause memory leak.
>
> cache_init() and cache_fini() are pair, so need let cache_fini() call
> g_free(cache) to match cache_init(), then fix current issue too.
Applied to -trivial, with the following commit message:
migration: Plug memory leak in migrate-set-cache-size command
We call g_free() after cache_fini() in migration_end(), but we don't
call it after cache_fini() in xbzrle_cache_resize(), leaking the
memory.
cache_init() and cache_fini() are a pair. Since cache_init()
allocates the cache, let cache_fini() free it. This plugs the leak.
Thanks,
/mjt
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-06-08 17:53 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-02 12:16 [Qemu-devel] [PATCH-trivial] arch_init.c: Free 'cache' in cache_fini() to avoid memory leak Chen Gang
2014-06-03 7:17 ` ChenLiang
2014-06-04 8:16 ` Markus Armbruster
2014-06-04 10:28 ` Chen Gang
2014-06-04 10:54 ` Markus Armbruster
2014-06-04 11:30 ` Chen Gang
2014-06-04 9:35 ` Dr. David Alan Gilbert
2014-06-08 17:53 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
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.