Add memrlimit controller (v4)

Documentation patch - describes the goals and usage of the memrlimit controller.

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

---

 Documentation/controllers/memrlimit.txt |   29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff -puN /dev/null Documentation/controllers/memrlimit.txt

Documentation/controllers/memrlimit.txt new file +3 -0

@@ -0,0 +1,29 @@
+This controller is enabled by the CONFIG_CGROUP_MEMRLIMIT_CTLR option. Prior
+to reading this documentation please read Documentation/cgroups.txt and
+Documentation/controllers/memory.txt. Several of the principles of this

Documentation/controllers/memrlimit.txt new file +16 -0

+interface. Please see section 3 of the memory resource controller documentation
+for more details on how to use the user interface to get and set values.
+
+The "memrlimit.usage_in_bytes" file provides information about the total address
+space usage of the tasks in the cgroup, in bytes.
+
+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
+   pages can be reclaimed.
+2. It provides better control over how many pages can be swapped out when
+   the cgroup goes over its limit. A badly setup cgroup can cause excessive
+   swapping. Providing control over the address space allocations ensures
+   that the system administrator has control over the total swapping that
+   can take place.

_

Documentation/controllers/memrlimit.txt line 24

Quoted 8 lines · Balbir Singh

Documentation/controllers/memrlimit.txt new file +4 -0

+
+Advantages of providing this feature
+
+1. Control over virtual address space allows for a cgroup to fail

gracefully

Documentation/controllers/memrlimit.txt new file +2 -0

+   i.e., via a malloc or mmap failure as compared to OOM kill when no
+   pages can be reclaimed.

Do you mean by this, limiting the number of pagetable pages (that are pinned in memory), this preventing oom by a cgroup that instantiates many pagetables?

This patch sets up the rlimit cgroup controller. It adds the basic create, destroy and populate functionality. The user interface provided is very similar to the memory resource controller. The rlimit controller can be enhanced easily in the future to control mlocked pages.

Changelog v3->v4

1. Use PAGE_ALIGN()
2. Rename rlimit to memrlimit

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

+
+int memrlimit_cgroup_charge_as(struct mm_struct *mm, unsigned long nr_pages) +{
+ int ret;
+ struct memrlimit_cgroup *memrcg;
+

 +       rcu_read_lock();
 +       memrcg = memrlimit_cgroup_from_task(rcu_dereference(mm->owner));
 +       css_get(&memrcg->css);
 +       rcu_read_unlock();
 +
 +       ret = res_counter_charge(&memrcg->as_res, (nr_pages << PAGE_SHIFT));
 +       css_put(&memrcg->css);
 +       return ret;

+}

+
+void memrlimit_cgroup_uncharge_as(struct mm_struct *mm, unsigned long nr_pages) +{
+ struct memrlimit_cgroup *memrcg;
+

 +       rcu_read_lock();
 +       memrcg = memrlimit_cgroup_from_task(rcu_dereference(mm->owner));
 +       css_get(&memrcg->css);
 +       rcu_read_unlock();
 +
 +       res_counter_uncharge(&memrcg->as_res, (nr_pages << PAGE_SHIFT));
 +       css_put(&memrcg->css);
 +}
 +
  static struct cgroup_subsys_state *
  memrlimit_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cgrp)
  {
 @@ -134,11 +169,70 @@ static int memrlimit_cgroup_populate(str
                                ARRAY_SIZE(memrlimit_cgroup_files));

}

+static void memrlimit_cgroup_move_task(struct cgroup_subsys *ss,

 +                                       struct cgroup *cgrp,
 +                                       struct cgroup *old_cgrp,
 +                                       struct task_struct *p)

+{
+ struct mm_struct *mm;
+ struct memrlimit_cgroup *memrcg, *old_memrcg;
+
+ mm = get_task_mm(p);
+ if (mm == NULL)
+ return;
+
+ rcu_read_lock();
+ if (p != rcu_dereference(mm->owner))
+ goto out;

+
+ memrcg = memrlimit_cgroup_from_cgrp(cgrp);
+ old_memrcg = memrlimit_cgroup_from_cgrp(old_cgrp);
+
+ if (memrcg == old_memrcg)
+ goto out;

+
+ /*
+ * Hold mmap_sem, so that total_vm does not change underneath us
+ */
+ down_read(&mm->mmap_sem);

+ if (res_counter_charge(&memrcg->as_res, (mm->total_vm << PAGE_SHIFT)))
+ goto out;
+ res_counter_uncharge(&old_memrcg->as_res, (mm->total_vm << PAGE_SHIFT));
+out:

 +       up_read(&mm->mmap_sem);
 +       rcu_read_unlock();
 +       mmput(mm);
 +}
 +
 +static void memrlimit_cgroup_mm_owner_changed(struct cgroup_subsys *ss,
 +                                               struct cgroup *cgrp,
 +                                               struct cgroup *old_cgrp,
 +                                               struct task_struct *p)
 +{
 +       struct memrlimit_cgroup *memrcg, *old_memrcg;
 +       struct mm_struct *mm = get_task_mm(p);
 +
 +       BUG_ON(!mm);
 +       memrcg = memrlimit_cgroup_from_cgrp(cgrp);
 +       old_memrcg = memrlimit_cgroup_from_cgrp(old_cgrp);

+
+ down_read(&mm->mmap_sem);

+ if (res_counter_charge(&memrcg->as_res, (mm->total_vm << PAGE_SHIFT)))
+ goto out;
+ res_counter_uncharge(&old_memrcg->as_res, (mm->total_vm << PAGE_SHIFT));
+out:
+ up_read(&mm->mmap_sem);
+
+ mmput(mm);
+}
+

Quoted 14 lines · Balbir Singh

+
+int memrlimit_cgroup_charge_as(struct mm_struct *mm, unsigned long nr_pages) +{
+ int ret;
+ struct memrlimit_cgroup *memrcg;
+

 +       rcu_read_lock();
 +       memrcg = memrlimit_cgroup_from_task(rcu_dereference(mm->owner));
 +       css_get(&memrcg->css);
 +       rcu_read_unlock();
 +
 +       ret = res_counter_charge(&memrcg->as_res, (nr_pages << PAGE_SHIFT));
 +       css_put(&memrcg->css);
 +       return ret;

Quoted 1 line

+}

Assuming that we're holding a write lock on mm->mmap_sem here, and we additionally hold mmap_sem for the whole of mm_update_next_owner(), then maybe we don't need any extra synchronization here? Something like simply:

int memrlimit_cgroup_charge_as(struct mm_struct *mm, unsigned long nr_pages)

{
       struct memrlimit_cgroup *memrcg = memrlimit_cgroup_from_task(mm->owner);
       return res_counter_charge(&memrcg->as_res, (nr_pages << PAGE_SHIFT));
}

Seems good to minimize additional synchronization on the fast path.

The only thing that's still broken is that the task_struct.cgroups pointer gets updated only under the synchronization of task_lock(), so we've still got the race of:

A: attach_task() updates B->cgroups

B: memrlimit_cgroup_charge_as() charges the new res counter and updates mm->total_vm

A: memrlimit_cgroup_move_task() moves mm->total_vm from the old counter to the new counter

Here's one way I see to fix this:

We change attach_task() so that rather than updating the task_struct.cgroups pointer once from the original css_set to the final css_set, it goes through a series of intermediate css_set structures, one for each subsystem in the hierarchy, transitioning from the old set to the final set. Then for each subsystem ss, it would do:

next_css = <old css with pointer for ss updated>
if (ss->attach) {
ss->attach(ss, p, next_css);
} else {
task_lock(p);
rcu_assign_ptr(p->cgroups, next_css);
task_unlock(p);
}

i.e. the subsystem would be free to implement any synchronization it desired in the attach() code. The attach() method's responsibility would be to ensure that p->cgroups was updated to point to next_css before returning. This should make it much simpler for a subsystem to handle potential races between attach() and accounting. The current semantics of can_attach()/update/attach() are sufficient for cpusets, but probably not for systems with more complex accounting. I'd still need to figure out a nice way to get the kind of transactional semantics that you want from can_attach().

Quoted 25 lines

+
+void memrlimit_cgroup_uncharge_as(struct mm_struct *mm, unsigned long nr_pages) +{
+ struct memrlimit_cgroup *memrcg;
+

 +       rcu_read_lock();
 +       memrcg = memrlimit_cgroup_from_task(rcu_dereference(mm->owner));
 +       css_get(&memrcg->css);
 +       rcu_read_unlock();
 +
 +       res_counter_uncharge(&memrcg->as_res, (nr_pages << PAGE_SHIFT));
 +       css_put(&memrcg->css);
 +}
 +
  static struct cgroup_subsys_state *
  memrlimit_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cgrp)
  {
 @@ -134,11 +169,70 @@ static int memrlimit_cgroup_populate(str
                                ARRAY_SIZE(memrlimit_cgroup_files));

}

+static void memrlimit_cgroup_move_task(struct cgroup_subsys *ss,

 +                                       struct cgroup *cgrp,
 +                                       struct cgroup *old_cgrp,
 +                                       struct task_struct *p)

Quoted 11 lines

+{
+ struct mm_struct *mm;
+ struct memrlimit_cgroup *memrcg, *old_memrcg;
+
+ mm = get_task_mm(p);
+ if (mm == NULL)
+ return;
+
+ rcu_read_lock();
+ if (p != rcu_dereference(mm->owner))
+ goto out;

out: does up_read() on mmap_sem, which you don't currently hold.

Can you add more comments about why you're using RCU here?

You have a refcounted pointer on mm, so mm can't go away, and you never dereference the result of the rcu_dereference(mm->owner), so you're not protecting the validity of that pointer. The only way this could help would be if anything that changes mm->owner calls synchronize_rcu() before, say, doing accounting changes, and I don't believe that's the case.

Would it be simpler to use task_lock(p) here to ensure that it doesn't lose its owner status (by exiting or execing) while we're moving it?

i.e., something like: [ this assumes the new semantics of attach that I proposed above ]

static void memrlimit_cgroup_move_task(struct cgroup_subsys *ss,
struct cgroup *cgrp, struct cgroup *old_cgrp, struct task_struct *p, struct css_set new_css_set)

{
        struct mm_struct *mm;
        struct memrlimit_cgroup *memrcg, *old_memrcg;

retry:

        mm = get_task_mm(p);
        if (mm == NULL) {
          task_lock(p);
          rcu_assign_ptr(p->cgroups, new_css_set);

          task_unlock(p);
          return;
        }

        /* Take mmap_sem to prevent address space changes */
        down_read(&mm->mmap_sem);
        /* task_lock(p) to prevent mm ownership changes */
        task_lock(p);
        if (p->mm != mm) {
                /* We raced */

                task_unlock(p);
                up_read(&mm->mmap_sem);
                mmput(mm);
                goto retry;
        }
        if (p != mm->owner)
                goto out_assign;

        memrcg = memrlimit_cgroup_from_cgrp(cgrp);
        old_memrcg = memrlimit_cgroup_from_cgrp(old_cgrp);

        if (memrcg == old_memrcg)
                goto out_assign;

        if (res_counter_charge(&memrcg->as_res, (mm->total_vm << PAGE_SHIFT)))

                goto out_assign;
        res_counter_uncharge(&old_memrcg->as_res, (mm->total_vm << PAGE_SHIFT));
  out_assign:
        rcu_assign_ptr(p->cgroups, new_css_set);
        task_unlock(p);
        up_read(&mm->mmap_sem);
        mmput(mm);
}

Quoted 9 lines

+
+ memrcg = memrlimit_cgroup_from_cgrp(cgrp);
+ old_memrcg = memrlimit_cgroup_from_cgrp(old_cgrp);
+
+ if (memrcg == old_memrcg)
+ goto out;

mmap_sem is also not held here.

Quoted 5 lines

+
+ /*
+ * Hold mmap_sem, so that total_vm does not change underneath us
+ */
+ down_read(&mm->mmap_sem);

You can't block inside rcu_read_lock().

Quoted 20 lines

+ if (res_counter_charge(&memrcg->as_res, (mm->total_vm << PAGE_SHIFT)))
+ goto out;
+ res_counter_uncharge(&old_memrcg->as_res, (mm->total_vm << PAGE_SHIFT));
+out:

 +       up_read(&mm->mmap_sem);
 +       rcu_read_unlock();
 +       mmput(mm);
 +}
 +
 +static void memrlimit_cgroup_mm_owner_changed(struct cgroup_subsys *ss,
 +                                               struct cgroup *cgrp,
 +                                               struct cgroup *old_cgrp,
 +                                               struct task_struct *p)
 +{
 +       struct memrlimit_cgroup *memrcg, *old_memrcg;
 +       struct mm_struct *mm = get_task_mm(p);
 +
 +       BUG_ON(!mm);
 +       memrcg = memrlimit_cgroup_from_cgrp(cgrp);
 +       old_memrcg = memrlimit_cgroup_from_cgrp(old_cgrp);

Quoted 2 lines

+
+ down_read(&mm->mmap_sem);

At this point we're holding p->alloc_lock, so we can't do a blocking down_read().

How about if we down_read(&mm->mmap_sem) in mm_update_next_owner() prior to taking tasklist_lock? That will ensure that mm ownership changes are synchronized against mmap/munmap operations on that mm, and since the cgroups-based owners are likely to be wanting to track the ownership changes for accounting purposes, this seems like an appropriate lock to hold.

Quoted 10 lines

+ if (res_counter_charge(&memrcg->as_res, (mm->total_vm << PAGE_SHIFT)))
+ goto out;
+ res_counter_uncharge(&old_memrcg->as_res, (mm->total_vm << PAGE_SHIFT));
+out:
+ up_read(&mm->mmap_sem);
+
+ mmput(mm);
+}
+

--
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>

Quoted 11 lines

Assuming that we're holding a write lock on mm->mmap_sem here, and we additionally hold mmap_sem for the whole of mm_update_next_owner(), then maybe we don't need any extra synchronization here? Something like simply:

int memrlimit_cgroup_charge_as(struct mm_struct *mm, unsigned long nr_pages)

{
       struct memrlimit_cgroup *memrcg = memrlimit_cgroup_from_task(mm->owner);
       return res_counter_charge(&memrcg->as_res, (nr_pages << PAGE_SHIFT));
}

The charge_as routine is not always called with mmap_sem held, since the undo path gets more complicated under the lock. We already have our own locking mechanism for the counters.

We're not really accessing
any member of the mm here except the owner. Do we need to be called with mmap_sem held?

Quoted 5 lines

retry:

        mm = get_task_mm(p);
        if (mm == NULL) {
          task_lock(p);
          rcu_assign_ptr(p->cgroups, new_css_set);

Will each callback assign p->cgroups to new_css_set?

Quoted 3 lines

        task_lock(p);
        if (p->mm != mm) {
                /* We raced */

With exit_mmap() or exec_mmap() right?

If we agree with the assertion/conclusion above, then a simple lock might be able to protect us, assuming that it does not create a interwined locking hierarchy.

Quoted 12 lines · Balbir Singh

Quoted 11 lines

Assuming that we're holding a write lock on mm->mmap_sem here, and we additionally hold mmap_sem for the whole of mm_update_next_owner(), then maybe we don't need any extra synchronization here? Something like simply:

int memrlimit_cgroup_charge_as(struct mm_struct *mm, unsigned long nr_pages)

{
       struct memrlimit_cgroup *memrcg = memrlimit_cgroup_from_task(mm->owner);
       return res_counter_charge(&memrcg->as_res, (nr_pages << PAGE_SHIFT));
}

Quoted 3 lines

The charge_as routine is not always called with mmap_sem held, since the undo path gets more complicated under the lock. We already have our own locking mechanism for the counters.

I'm not worried about the counters themselves being inconsistent - I'm worried about the case where charge_as() is called in the middle of the attach operation, and we account the charge X to the new cgroup's res_counter and update mm->total_vm, and then when we do the move, we charge the whole of mm->total_mm to the new cgroup even though the last charge was already accounted to the new res_counter, not the old one.

That's what I'm hoping to address with the idea of splitting the attach into one update per subsystem, and letting the subsystems control their own synchronization.

Quoted 4 lines

We're not really accessing
any member of the mm here except the owner. Do we need to be called with mmap_sem held?

Not necessarily mmap_sem, but there needs to be something to ensure that the update to mm->total_vm and the charge/uncharge against the res_counter are an atomic pair with respect to the code that shifts an mm between two cgroups, either due to mm->owner change or due to an attach_task(). Since mmap_sem is held for write on almost all the fast path calls to the rlimit_as charge/uncharge functions, using that for the synchronization avoids the need for any additional synchronization in the fast path.

Can you say more about the complications of holding a write lock on mmap_sem in the cleanup calls to uncharge?

Quoted 7 lines

Quoted 5 lines

retry:

        mm = get_task_mm(p);
        if (mm == NULL) {
          task_lock(p);
          rcu_assign_ptr(p->cgroups, new_css_set);

Will each callback assign p->cgroups to new_css_set?

Yes - but new_css_set will be slightly different for each callback. Specifically, it will differ from the existing set pointed to by p->cgroups in the pointer for this particular subsystem. So the task will move over in a staggered fashion, and each subsystem will get to choose its own synchronization.

Quoted 6 lines

Quoted 3 lines

        task_lock(p);
        if (p->mm != mm) {
                /* We raced */

With exit_mmap() or exec_mmap() right?

Yes.

Quoted 4 lines

If we agree with the assertion/conclusion above, then a simple lock might be able to protect us, assuming that it does not create a interwined locking hierarchy.

Right - and if we can make that lock be the mmap_sem of the mm in question, we avoid introducing a new lock into the fast path.

I want to focus on this conclusion/assertion, since it takes care of most of the locking related discussion above, unless I missed something.

My concern with using mmap_sem, is that

1. It's highly contended (every page fault, vma change, etc)

2. It's going to make the locking hierarchy deeper and complex

3. It's not appropriate to call all the accounting callbacks with the mmap_sem() held, since the undo operations _can get_ complicated at the caller.

I would prefer introducing a new lock, so that other subsystems are not affected.

Quoted 5 lines · Balbir Singh

I want to focus on this conclusion/assertion, since it takes care of most of the locking related discussion above, unless I missed something.

Quoted 3 lines

My concern with using mmap_sem, is that

1. It's highly contended (every page fault, vma change, etc)

But the only *new* cases of taking the mmap_sem that this would introduce would be:

- on a failed vm limit charge

- when a task exit/exec causes an mm ownership change

- when a task moves between two cgroups in the memrlimit hierarchy.

All of these should be rare events, so I don't think the additional contention is a worry.

Quoted 2 lines

2. It's going to make the locking hierarchy deeper and complex

Yes, potentially. But if the upside of that is that we eliminate a lock/unlock on a shared lock on every mmap/munmap call, it might well be worth it.

Quoted 4 lines

3. It's not appropriate to call all the accounting callbacks with the mmap_sem() held, since the undo operations _can get_ complicated at the caller.

Can you give an example?

Quoted 4 lines

I would prefer introducing a new lock, so that other subsystems are not affected.

For getting the first cut of the memrlimit controller working this may well make sense. But it would be nice to avoid it longer-term.

Quoted 5 lines

But the only *new* cases of taking the mmap_sem that this would introduce would be:

- on a failed vm limit charge

Why a failed charge? Aren't we talking of moving all charge/uncharge under mmap_sem?

Quoted 3 lines

- when a task moves between two cgroups in the memrlimit hierarchy.

Yes, this would nest cgroup_mutex and mmap_sem. Not sure if that would be a bad side-effect.

Refactor the code to try and use mmap_sem and see what I come up with. Basically use mmap_sem for all charge/uncharge operations as well use mmap_sem in read_mode in the move_task() and mm_owner_changed() callbacks. That should take care of the race conditions discussed, unless I missed something.

Quoted 9 lines · Balbir Singh

Quoted 5 lines

But the only *new* cases of taking the mmap_sem that this would introduce would be:

- on a failed vm limit charge

Why a failed charge? Aren't we talking of moving all charge/uncharge under mmap_sem?

Sorry, I worded that wrongly - I meant "cleaning up a successful charge after an expansion fails for other reasons"

I thought that all the charges and most of the uncharges were already under mmap_sem, and it would just be a few of the cleanup paths that needed to take it.

Quoted 6 lines

Quoted 2 lines

- when a task moves between two cgroups in the memrlimit hierarchy.

Yes, this would nest cgroup_mutex and mmap_sem. Not sure if that would be a bad side-effect.

I think it's already nested that way - e.g. the cpusets code can call various migration functions (which take mmap_sem) while holding cgroup_mutex.

Quoted 5 lines

Refactor the code to try and use mmap_sem and see what I come up with. Basically use mmap_sem for all charge/uncharge operations as well use mmap_sem in read_mode in the move_task() and mm_owner_changed() callbacks. That should take care of the race conditions discussed, unless I missed something.

Sounds good.

Thanks,

Paul

Quoted 9 lines · Balbir Singh

Quoted 5 lines

But the only *new* cases of taking the mmap_sem that this would introduce would be:

- on a failed vm limit charge

Why a failed charge? Aren't we talking of moving all charge/uncharge under mmap_sem?

Sorry, I worded that wrongly - I meant "cleaning up a successful charge after an expansion fails for other reasons"

I thought that all the charges and most of the uncharges were already under mmap_sem, and it would just be a few of the cleanup paths that needed to take it.

Quoted 7 lines

Quoted 3 lines

- when a task moves between two cgroups in the memrlimit hierarchy.

Yes, this would nest cgroup_mutex and mmap_sem. Not sure if that would be a bad side-effect.

I think it's already nested that way - e.g. the cpusets code can call various migration functions (which take mmap_sem) while holding cgroup_mutex.

Quoted 6 lines

Refactor the code to try and use mmap_sem and see what I come up with. Basically use mmap_sem for all charge/uncharge operations as well use mmap_sem in read_mode in the move_task() and mm_owner_changed() callbacks. That should take care of the race conditions discussed, unless I missed something.

Sounds good.

Thanks,

Quoted 37 lines · Paul Menage

Quoted 9 lines · Balbir Singh

Quoted 5 lines

But the only *new* cases of taking the mmap_sem that this would introduce would be:

- on a failed vm limit charge

Why a failed charge? Aren't we talking of moving all charge/uncharge under mmap_sem?

Sorry, I worded that wrongly - I meant "cleaning up a successful charge after an expansion fails for other reasons"

I thought that all the charges and most of the uncharges were already under mmap_sem, and it would just be a few of the cleanup paths that needed to take it.

Quoted 7 lines

Quoted 3 lines

- when a task moves between two cgroups in the memrlimit hierarchy.

Yes, this would nest cgroup_mutex and mmap_sem. Not sure if that would be a bad side-effect.

I think it's already nested that way - e.g. the cpusets code can call various migration functions (which take mmap_sem) while holding cgroup_mutex.

Quoted 6 lines

Refactor the code to try and use mmap_sem and see what I come up with. Basically use mmap_sem for all charge/uncharge operations as well use mmap_sem in read_mode in the move_task() and mm_owner_changed() callbacks. That should take care of the race conditions discussed, unless I missed something.

Quoted 4 lines

Sounds good.

Thanks,

I've revamped the last two patches. Please review

This patch adds support for accounting and control of virtual address space limits. The accounting is done via the rlimit_cgroup_(un)charge_as functions. The core of the accounting takes place during fork time in copy_process(), may_expand_vm(), remove_vma_list() and exit_mmap(). There are some special cases that are handled here as well (arch/ia64/kernel/perform.c, arch/x86/kernel/ptrace.c, insert_special_mapping())

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

---

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

---

 arch/ia64/kernel/perfmon.c      |    6 ++
 arch/x86/kernel/ptrace.c        |   17 +++++--
 fs/exec.c                       |    5 ++
 include/linux/memrlimitcgroup.h |   21 ++++++++
 kernel/fork.c                   |    8 +++
 mm/memrlimitcgroup.c            |   94 ++++++++++++++++++++++++++++++++++++++++
 mm/mmap.c                       |   11 ++++
 7 files changed, 157 insertions(+), 5 deletions(-)

diff -puN arch/ia64/kernel/perfmon.c~memrlimit-controller-address-space-accounting-and-control arch/ia64/kernel/perfmon.c

arch/ia64/kernel/perfmon.c renamed from arch/ia64/kernel/perfmon.c~memrlimit-controller-address-space-accounting-and-control +1 -0

@@ -40,6 +40,7 @@
 #include <linux/capability.h>
 #include <linux/rcupdate.h>
 #include <linux/completion.h>
+#include <linux/memrlimitcgroup.h>
 
 #include <asm/errno.h>
 #include <asm/intrinsics.h>

AFAIS you didn't cover all the cases when VM expands. At least all the arch/ia64/ia32/binfmt_elf32.c is missed.

I'd insert this charge into insert_vm_struct. This would a) cover all of the missed cases and b) reduce the amount of places to patch.

This patch adds support for accounting and control of virtual address space limits.

Quoted 2 lines · On Wed, 14 May 2008 18:39:51 +0530 Balbir Singh <[email protected]>…

This patch adds support for accounting and control of virtual address space limits.

could you plese take a look at today's mmotm and see what needs to be done to salvage it? Most of the code you were altering got moved into arch/x86/kernel/ds.c and got changed rather a lot.

Quoted 2 lines · On Wed, 14 May 2008 18:39:51 +0530 Balbir Singh <[email protected]>…

This patch adds support for accounting and control of virtual address space limits.

Large changes in linux-next's arch/x86/kernel/ptrace.c caused damage to the memrlimit patches.

I decided to retain the patches because it looks repairable. The problem is this reject from memrlimit-add-memrlimit-controller-accounting-and-control.patch:

Quoted 10 lines · Andrew Morton

Quoted 2 lines · On Wed, 14 May 2008 18:39:51 +0530 Balbir Singh <[email protected]>…

This patch adds support for accounting and control of virtual address space limits.

Large changes in linux-next's arch/x86/kernel/ptrace.c caused damage to the memrlimit patches.

Quoted 4 lines

I decided to retain the patches because it looks repairable. The problem is this reject from memrlimit-add-memrlimit-controller-accounting-and-control.patch:

Andrew,

I could not apply mmotm to linux-next (both downloaded right now).

I
applied the patches one-by-one resolving differences starting from #mm in the series file.

Here is my fixed version of the patch, I compiled the patch, but could not run it, since I could not create the full series of applied patches. I compiled arch/x86/kernel/ds.o and ptrace.o. I've included the patch below, please let me know if the code looks OK (via review) and the patch applies. I'll test it once I can resonably resolve all conflicts between linux-next and mmotm.

Quoted 15 lines · On Thu, 18 Sep 2008 23:38:23 -0700 Balbir Singh <[email protected]>…

Quoted 13 lines · Andrew Morton

Quoted 2 lines · On Wed, 14 May 2008 18:39:51 +0530 Balbir Singh <[email protected]>…

This patch adds support for accounting and control of virtual address space limits.

Large changes in linux-next's arch/x86/kernel/ptrace.c caused damage to the memrlimit patches.

I decided to retain the patches because it looks repairable. The problem is this reject from memrlimit-add-memrlimit-controller-accounting-and-control.patch:

Quoted 3 lines

Andrew,

I could not apply mmotm to linux-next (both downloaded right now).

mmotm includes linux-next.patch. mmotm is based upon the most recent 2.6.x-rcy.

OK, we'll give it a shot, thanks.