Re: [PATCH] memcg: Free spare array to avoid memory leak

On 03/08/2012 06:35 PM, Kirill A. Shutemov wrote:

On Thu, Mar 08, 2012 at 10:11:32AM +0800, Sha Zhengju wrote:

quoted

On 03/08/2012 07:08 AM, Kirill A. Shutemov wrote:

quoted

On Tue, Mar 06, 2012 at 08:13:24PM +0800, Sha Zhengju wrote:

quoted

From: Sha Zhengju<redacted>

When the last event is unregistered, there is no need to keep the spare
array anymore. So free it to avoid memory leak.

It's not a leak. It will be freed on next event register.

Yeah, I noticed that. But what if it is just the last one and no more
event registering ?

See my question below. ;)

quoted

quoted

Yeah, we don't have to keep spare if primary is empty. But is it worth to
make code more complicated to save few bytes of memory?

If we unregister the last event and *don't* register a new event anymore,
the primary is freed but the spare is still kept which has no chance to
free.

IMHO, it's obvious not a problem of saving bytes but *memory leak*.

quoted

quoted

quoted

Signed-off-by: Sha Zhengju<redacted>

---
  mm/memcontrol.c |    6 ++++++
  1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 22d94f5..3c09a84 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c

@@ -4412,6 +4412,12 @@ static void mem_cgroup_usage_unregister_event(struct cgroup *cgrp,
  swap_buffers:
  	/* Swap primary and spare array */
  	thresholds->spare = thresholds->primary;
+	/* If all events are unregistered, free the spare array */
+	if (!new) {
+		kfree(thresholds->spare);
+		thresholds->spare = NULL;
+	}
+
  	rcu_assign_pointer(thresholds->primary, new);

  	/* To be sure that nobody uses thresholds */

-- 
1.7.4.1