Thread (44 messages) 44 messages, 6 authors, 2011-09-28

Re: [PATCH v3 1/7] Basic kernel memory functionality for the Memory Controller

From: Glauber Costa <hidden>
Date: 2011-09-26 22:45:21
Also in: lkml, netdev

quoted
  #endif

-
+#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
+int do_kmem_account __read_mostly = 1;
+#else
+#define do_kmem_account		0
+#endif

Hmm, do we really need this boot option ?
 From my experience to have swap-accounting boot option,
this scares us ;) I think config is enough.
If no one else wants it, I can remove it. I personally
don't need it, just wanted to follow the convention laid down by swap here.

quoted
  /*
   * Statistics for memory cgroup.
   */
@@ -270,6 +274,10 @@ struct mem_cgroup {
  	 */
  	struct res_counter memsw;
  	/*
+	 * the counter to account for kmem usage.
+	 */
+	struct res_counter kmem;
+	/*
  	 * Per cgroup active and inactive list, similar to the
  	 * per zone LRU lists.
  	 */
@@ -321,6 +329,11 @@ struct mem_cgroup {
  	 */
  	unsigned long 	move_charge_at_immigrate;
  	/*
+	 * Should kernel memory limits be stabilished independently
+	 * from user memory ?
+	 */
+	int		kmem_independent;
+	/*
  	 * percpu counter.
  	 */
  	struct mem_cgroup_stat_cpu *stat;
@@ -388,9 +401,14 @@ enum charge_type {
  };

  /* for encoding cft->private value on file */
-#define _MEM			(0)
-#define _MEMSWAP		(1)
-#define _OOM_TYPE		(2)
+
+enum mem_type {
+	_MEM = 0,
+	_MEMSWAP,
+	_OOM_TYPE,
+	_KMEM,
+};
+
ok, nice clean up.

quoted
  #define MEMFILE_PRIVATE(x, val)	(((x)<<  16) | (val))
  #define MEMFILE_TYPE(val)	(((val)>>  16)&  0xffff)
  #define MEMFILE_ATTR(val)	((val)&  0xffff)
@@ -3943,10 +3961,15 @@ static inline u64 mem_cgroup_usage(struct mem_cgroup *mem, bool swap)
  	u64 val;

  	if (!mem_cgroup_is_root(mem)) {
+		val = 0;
+		if (!mem->kmem_independent)
+			val = res_counter_read_u64(&mem->kmem, RES_USAGE);
quoted
  		if (!swap)
-			return res_counter_read_u64(&mem->res, RES_USAGE);
+			val += res_counter_read_u64(&mem->res, RES_USAGE);
  		else
-			return res_counter_read_u64(&mem->memsw, RES_USAGE);
+			val += res_counter_read_u64(&mem->memsw, RES_USAGE);
+
+		return val;
  	}

  	val = mem_cgroup_recursive_stat(mem, MEM_CGROUP_STAT_CACHE);
@@ -3979,6 +4002,10 @@ static u64 mem_cgroup_read(struct cgroup *cont, struct cftype *cft)
  		else
  			val = res_counter_read_u64(&mem->memsw, name);
  		break;
+	case _KMEM:
+		val = res_counter_read_u64(&mem->kmem, name);
+		break;
+
  	default:
  		BUG();
  		break;
@@ -4756,6 +4783,21 @@ static int mem_cgroup_reset_vmscan_stat(struct cgroup *cgrp,
  	return 0;
  }

+#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
+static u64 kmem_limit_independent_read(struct cgroup *cont, struct cftype *cft)
+{
+	return mem_cgroup_from_cont(cont)->kmem_independent;
+}
+
+static int kmem_limit_independent_write(struct cgroup *cont, struct cftype *cft,
+					u64 val)
+{
+	cgroup_lock();
+	mem_cgroup_from_cont(cont)->kmem_independent = !!val;
+	cgroup_unlock();
Hm. This code allows that parent/child can have different settings.
Could you add parent-child check as..

"If parent sets use_hierarchy==1, children must have the same kmem_independent value
with parant's one."
Agree.
How do you think ? I think a hierarchy must have the same config.
Yes, I think this is reasonable.
BTW...I don't like naming a little ;)

memory->consolidated/shared/?????_kmem_accounting ?
Or
memory->kmem_independent_accounting ?

or some better naming ?
I can go with kmem_independent_accounting if you like, it is fine
by me.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help