Virtual address space control for cgroups

/*
+ * Check if the current cgroup exceeds its address space limit.
+ * Returns 0 on success and 1 on failure.
+ */
+int mem_cgroup_update_as(struct mm_struct *mm, long nr_pages) +{

 +       int ret = 0;
 +       struct mem_cgroup *mem;
 +       if (mem_cgroup_subsys.disabled)
 +               return ret;
 +
 +       rcu_read_lock();
 +       mem = rcu_dereference(mm->mem_cgroup);
 +       css_get(&mem->css);
 +       rcu_read_unlock();

+

Quoted 15 lines · Balbir Singh

/*
+ * Check if the current cgroup exceeds its address space limit.
+ * Returns 0 on success and 1 on failure.
+ */
+int mem_cgroup_update_as(struct mm_struct *mm, long nr_pages) +{

 +       int ret = 0;
 +       struct mem_cgroup *mem;
 +       if (mem_cgroup_subsys.disabled)
 +               return ret;
 +
 +       rcu_read_lock();
 +       mem = rcu_dereference(mm->mem_cgroup);
 +       css_get(&mem->css);
 +       rcu_read_unlock();

Quoted 1 line

+

How about if this function avoided charging the root cgroup? You'd save 4 atomic operations on a global data structure on every mmap/munmap when the virtual address limit cgroup wasn't in use, which could be significant on a large system. And I don't see situations where you really need to limit the address space of the root cgroup.

1. We want to be able to support hierarchial accounting and control

2. We want to track usage of the root cgroup and report it back to the user

3. We don't want to treat the root cgroup as a special case.

mm/memcontrol.c renamed from mm/memcontrol.c~memory-controller-virtual-address-space-accounting-and-control +7 -0

+int mem_cgroup_update_as(struct mm_struct *mm, long nr_pages)
+{
+	int ret = 0;
+	struct mem_cgroup *mem;
+	if (mem_cgroup_subsys.disabled)
+		return ret;
+

[snip]

Quoted 7 lines

mm/memcontrol.c renamed from mm/memcontrol.c~memory-controller-virtual-address-space-accounting-and-control +7 -0

+int mem_cgroup_update_as(struct mm_struct *mm, long nr_pages)
+{
+	int ret = 0;
+	struct mem_cgroup *mem;
+	if (mem_cgroup_subsys.disabled)
+		return ret;
+

mm/memcontrol.c unsigned long mem_cgroup_isolate_pages(u · line 547

Quoted 10 lines

mm/memcontrol.c renamed from mm/memcontrol.c~memory-controller-virtual-address-space-accounting-and-control +10 -0

+	rcu_read_lock();
+	mem = rcu_dereference(mm->mem_cgroup);
+	css_get(&mem->css);
+	rcu_read_unlock();
+
+	if (nr_pages > 0) {
+		if (res_counter_charge(&mem->as_res, (nr_pages * PAGE_SIZE)))
+			ret = 1;
+	} else
+		res_counter_uncharge(&mem->as_res, (-nr_pages * PAGE_SIZE));

No, please, no. Let's make two calls - mem_cgroup_charge_as and mem_cgroup_uncharge_as.

[snip]

mm/mmap.c munmap_back: · line 1122

Quoted 7 lines

mm/mmap.c renamed from mm/mmap.c~memory-controller-virtual-address-space-accounting-and-control +3 -0

@@ -1117,6 +1117,9 @@ munmap_back:
 		}
 	}
 
+	if (mem_cgroup_update_as(mm, len >> PAGE_SHIFT))
+		return -ENOMEM;
+

Why not use existintg cap_vm_enough_memory and co?

Quoted 21 lines · Pavel Emelyanov

[snip]

Quoted 7 lines

mm/memcontrol.c renamed from mm/memcontrol.c~memory-controller-virtual-address-space-accounting-and-control +7 -0

+int mem_cgroup_update_as(struct mm_struct *mm, long nr_pages)
+{
+	int ret = 0;
+	struct mem_cgroup *mem;
+	if (mem_cgroup_subsys.disabled)
+		return ret;
+

mm/memcontrol.c unsigned long mem_cgroup_isolate_pages(u · line 547

Quoted 10 lines

mm/memcontrol.c renamed from mm/memcontrol.c~memory-controller-virtual-address-space-accounting-and-control +10 -0

+	rcu_read_lock();
+	mem = rcu_dereference(mm->mem_cgroup);
+	css_get(&mem->css);
+	rcu_read_unlock();
+
+	if (nr_pages > 0) {
+		if (res_counter_charge(&mem->as_res, (nr_pages * PAGE_SIZE)))
+			ret = 1;
+	} else
+		res_counter_uncharge(&mem->as_res, (-nr_pages * PAGE_SIZE));

No, please, no. Let's make two calls - mem_cgroup_charge_as and mem_cgroup_uncharge_as.

Quoted 2 lines

[snip]

Yes, sure :)

Quoted 9 lines

mm/mmap.c munmap_back: · line 1122

Quoted 7 lines

mm/mmap.c renamed from mm/mmap.c~memory-controller-virtual-address-space-accounting-and-control +3 -0

@@ -1117,6 +1117,9 @@ munmap_back:
 		}
 	}
 
+	if (mem_cgroup_update_as(mm, len >> PAGE_SHIFT))
+		return -ENOMEM;
+

Why not use existintg cap_vm_enough_memory and co?

I thought about it and almost used may_expand_vm(), but there is a slight catch there. With cap_vm_enough_memory() or security_vm_enough_memory(), they are called after total_vm has been calculated. In our case we need to keep the cgroups equivalent of total_vm up to date, and we do this in mem_cgorup_update_as.

Quoted 22 lines · Balbir Singh

Quoted 21 lines · Pavel Emelyanov

[snip]

Quoted 7 lines

mm/memcontrol.c renamed from mm/memcontrol.c~memory-controller-virtual-address-space-accounting-and-control +7 -0

+int mem_cgroup_update_as(struct mm_struct *mm, long nr_pages)
+{
+	int ret = 0;
+	struct mem_cgroup *mem;
+	if (mem_cgroup_subsys.disabled)
+		return ret;
+

mm/memcontrol.c unsigned long mem_cgroup_isolate_pages(u · line 547

Quoted 10 lines

mm/memcontrol.c renamed from mm/memcontrol.c~memory-controller-virtual-address-space-accounting-and-control +10 -0

+	rcu_read_lock();
+	mem = rcu_dereference(mm->mem_cgroup);
+	css_get(&mem->css);
+	rcu_read_unlock();
+
+	if (nr_pages > 0) {
+		if (res_counter_charge(&mem->as_res, (nr_pages * PAGE_SIZE)))
+			ret = 1;
+	} else
+		res_counter_uncharge(&mem->as_res, (-nr_pages * PAGE_SIZE));

No, please, no. Let's make two calls - mem_cgroup_charge_as and mem_cgroup_uncharge_as.

Quoted 3 lines

Quoted 2 lines

[snip]

Yes, sure :)

Thanks :)

Quoted 9 lines

Quoted 9 lines

mm/mmap.c munmap_back: · line 1122

Quoted 7 lines

mm/mmap.c renamed from mm/mmap.c~memory-controller-virtual-address-space-accounting-and-control +3 -0

@@ -1117,6 +1117,9 @@ munmap_back:
 		}
 	}
 
+	if (mem_cgroup_update_as(mm, len >> PAGE_SHIFT))
+		return -ENOMEM;
+

Why not use existintg cap_vm_enough_memory and co?

Quoted 4 lines

I thought about it and almost used may_expand_vm(), but there is a slight catch there. With cap_vm_enough_memory() or security_vm_enough_memory(), they are called after total_vm has been calculated. In our case we need to keep the cgroups equivalent of total_vm up to date, and we do this in mem_cgorup_update_as.

So? What prevents us from using these hooks? :)

Quoted 37 lines · Pavel Emelyanov

Quoted 22 lines · Balbir Singh

Quoted 21 lines · Pavel Emelyanov

[snip]

Quoted 7 lines

mm/memcontrol.c renamed from mm/memcontrol.c~memory-controller-virtual-address-space-accounting-and-control +7 -0

+int mem_cgroup_update_as(struct mm_struct *mm, long nr_pages)
+{
+	int ret = 0;
+	struct mem_cgroup *mem;
+	if (mem_cgroup_subsys.disabled)
+		return ret;
+

mm/memcontrol.c unsigned long mem_cgroup_isolate_pages(u · line 547

Quoted 10 lines

mm/memcontrol.c renamed from mm/memcontrol.c~memory-controller-virtual-address-space-accounting-and-control +10 -0

+	rcu_read_lock();
+	mem = rcu_dereference(mm->mem_cgroup);
+	css_get(&mem->css);
+	rcu_read_unlock();
+
+	if (nr_pages > 0) {
+		if (res_counter_charge(&mem->as_res, (nr_pages * PAGE_SIZE)))
+			ret = 1;
+	} else
+		res_counter_uncharge(&mem->as_res, (-nr_pages * PAGE_SIZE));

No, please, no. Let's make two calls - mem_cgroup_charge_as and mem_cgroup_uncharge_as.

Quoted 3 lines

Quoted 2 lines

[snip]

Yes, sure :)

Thanks :)

Quoted 9 lines

Quoted 9 lines

mm/mmap.c munmap_back: · line 1122

Quoted 7 lines

mm/mmap.c renamed from mm/mmap.c~memory-controller-virtual-address-space-accounting-and-control +3 -0

@@ -1117,6 +1117,9 @@ munmap_back:
 		}
 	}
 
+	if (mem_cgroup_update_as(mm, len >> PAGE_SHIFT))
+		return -ENOMEM;
+

Why not use existintg cap_vm_enough_memory and co?

Quoted 5 lines

Quoted 4 lines

I thought about it and almost used may_expand_vm(), but there is a slight catch there. With cap_vm_enough_memory() or security_vm_enough_memory(), they are called after total_vm has been calculated. In our case we need to keep the cgroups equivalent of total_vm up to date, and we do this in mem_cgorup_update_as.

So? What prevents us from using these hooks? :)

1. We need to account total_vm usage of the task anyway. So why have two places, one for accounting and second for control?

2. These hooks are activated for conditionally invoked for vma's with VM_ACCOUNT set.

Quoted 38 lines · Balbir Singh

Quoted 37 lines · Pavel Emelyanov

Quoted 22 lines · Balbir Singh

Quoted 21 lines · Pavel Emelyanov

[snip]

Quoted 7 lines

mm/memcontrol.c renamed from mm/memcontrol.c~memory-controller-virtual-address-space-accounting-and-control +7 -0

+int mem_cgroup_update_as(struct mm_struct *mm, long nr_pages)
+{
+	int ret = 0;
+	struct mem_cgroup *mem;
+	if (mem_cgroup_subsys.disabled)
+		return ret;
+

mm/memcontrol.c unsigned long mem_cgroup_isolate_pages(u · line 547

Quoted 10 lines

mm/memcontrol.c renamed from mm/memcontrol.c~memory-controller-virtual-address-space-accounting-and-control +10 -0

+	rcu_read_lock();
+	mem = rcu_dereference(mm->mem_cgroup);
+	css_get(&mem->css);
+	rcu_read_unlock();
+
+	if (nr_pages > 0) {
+		if (res_counter_charge(&mem->as_res, (nr_pages * PAGE_SIZE)))
+			ret = 1;
+	} else
+		res_counter_uncharge(&mem->as_res, (-nr_pages * PAGE_SIZE));

No, please, no. Let's make two calls - mem_cgroup_charge_as and mem_cgroup_uncharge_as.

Quoted 3 lines

Quoted 2 lines

[snip]

Yes, sure :)

Thanks :)

Quoted 9 lines

Quoted 9 lines

mm/mmap.c munmap_back: · line 1122

Quoted 7 lines

mm/mmap.c renamed from mm/mmap.c~memory-controller-virtual-address-space-accounting-and-control +3 -0

@@ -1117,6 +1117,9 @@ munmap_back:
 		}
 	}
 
+	if (mem_cgroup_update_as(mm, len >> PAGE_SHIFT))
+		return -ENOMEM;
+

Why not use existintg cap_vm_enough_memory and co?

Quoted 7 lines

Quoted 5 lines

Quoted 4 lines

I thought about it and almost used may_expand_vm(), but there is a slight catch there. With cap_vm_enough_memory() or security_vm_enough_memory(), they are called after total_vm has been calculated. In our case we need to keep the cgroups equivalent of total_vm up to date, and we do this in mem_cgorup_update_as.

So? What prevents us from using these hooks? :)

1. We need to account total_vm usage of the task anyway. So why have two places, one for accounting and second for control?

We still have two of them even placing hooks in each place manually.

Besides, putting the mem_cgroup_(un)charge_as() in these vm hooks will
1. save the number of places to patch
2. help keeping memcgroup consistent in case someone adds more places that expand tasks vm (arches, drivers) - in case we have our hooks celled from inside vm ones, we won't have to patch more.

Quoted 2 lines

2. These hooks are activated for conditionally invoked for vma's with VM_ACCOUNT set.

This is a good point against. But, wrt my previous comment, can we handle this somehow?

Quoted 4 lines · Balbir Singh

arch/x86/kernel/ptrace.c renamed from arch/x86/kernel/ptrace.c~memory-controller-virtual-address-space-accounting-and-control +0 -0

@@ -787,6 +788,8 @@ static int ptrace_bts_realloc(struct tas
        current->mm->total_vm  -= old_size;
        current->mm->locked_vm -= old_size;
 

arch/x86/kernel/ptrace.c static int ptrace_bts_realloc(struct tas · line 794

Quoted 4 lines

arch/x86/kernel/ptrace.c renamed from arch/x86/kernel/ptrace.c~memory-controller-virtual-address-space-accounting-and-control +2 -0

+       mem_cgroup_update_as(current->mm, -old_size);
+
        if (size == 0)
                goto out;

I think splattering these things all over is probably a bad idea.

If you're going to do this, I think you need a couple of phases.

1. update the vm_(un)acct_memory() functions to take an mm

2. start using them (or some other abstracted functions in place)
3. update the new functions for cgroups

It's a bit non-obvious why you do the mem_cgroup_update_as() calls in the places that you do from context.

Having some other vm-abstracted functions will also keep you from splattering mem_cgroup_update_as() across the tree. That's a pretty bad name. :) ...update_mapped() or ...update_vm() might be a wee bit better.

Quoted 3 lines

If you're going to do this, I think you need a couple of phases.

1. update the vm_(un)acct_memory() functions to take an mm

There are other problems

1. vm_(un)acct_memory is conditionally dependent on VM_ACCOUNT. Look at shmem_(un)acct_size for example

2. These routines are not called from all contexts that we care about (look at insert_special_mapping())

Quoted 7 lines · Balbir Singh

Quoted 3 lines

If you're going to do this, I think you need a couple of phases.

1. update the vm_(un)acct_memory() functions to take an mm

There are other problems

1. vm_(un)acct_memory is conditionally dependent on VM_ACCOUNT. Look at shmem_(un)acct_size for example

Yeah, but if VM_ACCOUNT isn't set, do you really want the controller accounting for them? It's there for a reason. :)

The shmem_acct_size() helpers look good. I wonder if we should be using that kind of things more generically.

Quoted 2 lines

2. These routines are not called from all contexts that we care about (look at insert_special_mapping())

Could you explain why "we" care about it and why it isn't accounted for now?

diff -puN mm/swapfile.c~memory-controller-virtual-address-space-accounting-and-control mm/swapfile.c diff -puN mm/memory.c~memory-controller-virtual-address-space-accounting-and-control mm/memory.c

mm/memory.c renamed from mm/memory.c~memory-controller-virtual-address-space-accounting-and-control +0 -0

@@ -838,6 +838,11 @@ unsigned long unmap_vmas(struct mmu_gath
 

Quoted 14 lines

diff -puN mm/swapfile.c~memory-controller-virtual-address-space-accounting-and-control mm/swapfile.c diff -puN mm/memory.c~memory-controller-virtual-address-space-accounting-and-control mm/memory.c

mm/memory.c renamed from mm/memory.c~memory-controller-virtual-address-space-accounting-and-control +5 -0

@@ -838,6 +838,11 @@ unsigned long unmap_vmas(struct mmu_gath
 
 		if (vma->vm_flags & VM_ACCOUNT)
 			*nr_accounted += (end - start) >> PAGE_SHIFT;
+		/*
+		 * Unaccount used virtual memory for cgroups
+		 */
+		mem_cgroup_update_as(vma->vm_mm,
+					((long)(start - end)) >> PAGE_SHIFT);
 

mm/memory.c unsigned long unmap_vmas(struct mmu_gath · line 848

Quoted 2 lines

mm/memory.c renamed from mm/memory.c~memory-controller-virtual-address-space-accounting-and-control +0 -0

 		while (start != end) {
 			if (!tlb_start_valid) {

i think you can sum and uncharge it with a single call.

This patch adds documentation for virtual address space control.

Signed-off-by Balbir Singh <[email protected]>

---

 Documentation/controllers/memory.txt |   26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff -puN Documentation/controllers/memory.txt~memory-controller-virtual-address-control-documentation Documentation/controllers/memory.txt

Documentation/controllers/memory.txt renamed from Documentation/controllers/memory.txt~memory-controller-virtual-address-control-documentation +0 -0

@@ -237,7 +237,31 @@ cgroup might have some charge associated
 tasks have migrated away from it. Such charges are automatically dropped at

Documentation/controllers/memory.txt renamed from Documentation/controllers/memory.txt~memory-controller-virtual-address-control-documentation +5 -0

+   swapping. Providing control over the address space allocations ensures
+   that the system administrator has control over the total swapping that
+   can take place.
+
+6. TODO
 
 1. Add support for accounting huge pages (as a separate controller)
 2. Make per-cgroup scanner reclaim not-shared pages first

_

Quoted 13 lines · On Sun, 16 Mar 2008 23:00:17 +0530 Balbir Singh wrote:

This patch adds documentation for virtual address space control.

Signed-off-by Balbir Singh <[email protected]>

---

 Documentation/controllers/memory.txt |   26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff -puN Documentation/controllers/memory.txt~memory-controller-virtual-address-control-documentation Documentation/controllers/memory.txt

Documentation/controllers/memory.txt renamed from Documentation/controllers/memory.txt~memory-controller-virtual-address-control-documentation +0 -0

@@ -237,7 +237,31 @@ cgroup might have some charge associated
 tasks have migrated away from it. Such charges are automatically dropped at

Documentation/controllers/memory.txt cgroup might have some charge associated · line 246

Quoted 10 lines

Documentation/controllers/memory.txt renamed from Documentation/controllers/memory.txt~memory-controller-virtual-address-control-documentation +7 -1

 rmdir() if there are no tasks.
 
-5. TODO
+5. Virtual address space accounting
+
+A new resource counter controls the address space expansion of the tasks in
+the cgroup. Address space control is provided along the same lines as
+RLIMIT_AS control, which is available via getrlimit(2)/setrlimit(2).
+The interface for controlling address space is provided through
+"as_limit_in_bytes". The file is similar to "limit_in_bytes" w.r.t the user

                                                                w.r.t.

or even spelled out.

Documentation/controllers/memory.txt cgroup might have some charge associated · line 256

Quoted 10 lines

Documentation/controllers/memory.txt renamed from Documentation/controllers/memory.txt~memory-controller-virtual-address-control-documentation +10 -0

+interface. Please see section 3 for more details on how to use the user
+interface to get and set values.
+
+The "as_usage_in_bytes" file provides information about the total address
+space usage of the cgroup in bytes.
+
+5.1 Advantages of providing this feature
+
+1. Control over virtual address space allows for a cgroup to fail gracefully
+   i.e, via a malloc or mmap failure as compared to OOM kill when no

      i.e.,

Documentation/controllers/memory.txt cgroup might have some charge associated · line 257

Quoted 1 line

Documentation/controllers/memory.txt renamed from Documentation/controllers/memory.txt~memory-controller-virtual-address-control-documentation +1 -0

+   pages can be reclaimed

end with period.

Documentation/controllers/memory.txt cgroup might have some charge associated · line 259

Quoted 3 lines

Documentation/controllers/memory.txt renamed from Documentation/controllers/memory.txt~memory-controller-virtual-address-control-documentation +2 -0

+2. It provides better control over how many pages can be swapped out when
+   the cgroup goes over it's limit. A badly setup cgroup can cause excessive

its (not "it is")

Quoted 9 lines

Documentation/controllers/memory.txt renamed from Documentation/controllers/memory.txt~memory-controller-virtual-address-control-documentation +5 -0

+   swapping. Providing control over the address space allocations ensures
+   that the system administrator has control over the total swapping that
+   can take place.
+
+6. TODO
 
 1. Add support for accounting huge pages (as a separate controller)
 2. Make per-cgroup scanner reclaim not-shared pages first

_

---
~Randy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

This is an early patchset for virtual address space control for cgroups.
The patches are against 2.6.25-rc5-mm1 and have been tested on top of User Mode Linux.

Quoted 3 lines · Balbir Singh

This is an early patchset for virtual address space control for cgroups.
The patches are against 2.6.25-rc5-mm1 and have been tested on top of User Mode Linux.

What's the performance hit of doing these accounting checks on every mmap/munmap? If it's not totally lost in the noise, couldn't it be made a separate control group, so that it could be just enabled (and the performance hit taken) for users that actually want it?

Paul

It will be code duplication to make it a new subsystem,

Quoted 1 line · Li Zefan

It will be code duplication to make it a new subsystem,

Would it? Other than the basic cgroup boilerplate, the only real duplication that I could see would be that there'd need to be an additional per-mm pointer back to the cgroup. (Which could be avoided if we added a single per-mm pointer back to the "owning" task, which would generally be the mm's thread group leader, so that you could go quickly from an mm to a set of cgroup subsystems).

And the advantage would that you'd be able to more easily pick/choose which bits of control you use (and pay for).

I understand the per-mm pointer overhead back to the cgroup. I don't understand the part about adding a per-mm pointer back to the "owning" task. We already have task->mm.

Quoted 3 lines · Balbir Singh

I understand the per-mm pointer overhead back to the cgroup. I don't understand the part about adding a per-mm pointer back to the "owning" task. We already have task->mm.

Yes, but we don't have mm->owner, which is what I was proposing - mm->owner would be a pointer typically to the mm's thread group leader. It would remove the need to have to have pointers for the various different cgroup subsystems that need to act on an mm rather than a task_struct, since then you could use mm->owner->cgroups[subsys_id].

But this is kind of orthogonal to whether virtual address space limits should be a separate cgroup subsystem.

Quoted 3 lines · Balbir Singh

This is an early patchset for virtual address space control for cgroups.
The patches are against 2.6.25-rc5-mm1 and have been tested on top of User Mode Linux.

What's the performance hit of doing these accounting checks on every mmap/munmap? If it's not totally lost in the noise, couldn't it be made a separate control group, so that it could be just enabled (and the performance hit taken) for users that actually want it?

I am yet to measure the performance overhead of the accounting checks. I'll try and get started on that today. I did not consider making it a separate system, because I suspect that anybody wanting memory control would also want address space control (for the advantages listed in the documentation).

Quoted 4 lines · Balbir Singh

I am yet to measure the performance overhead of the accounting checks. I'll try and get started on that today. I did not consider making it a separate system, because I suspect that anybody wanting memory control would also want address space control (for the advantages listed in the documentation).

I'm a counter-example to your suspicion :-)

Trying to control virtual address space is a complete nightmare in the presence of anything that uses large sparsely-populated mappings (mmaps of large files, or large sparse heaps such as the JVM uses.)

If we want to control the effect of swapping, the right way to do it is to control disk I/O, and ensure that the swapping is accounted to that. Or simply just not give apps much swap space.