cpuset,mm: fix no node to alloc memory when changing cpuset's mems - fix2

- cleanup unnecessary header file
- fix the race between set_mempolicy() and cpuset_change_task_nodemask()

---
 kernel/cpuset.c |    3 +--
 kernel/exit.c   |    1 -
 kernel/fork.c   |    1 -
 3 files changed, 1 insertions(+), 4 deletions(-)

index 30cb9a2..d243a22 100644
@@ -963,7 +963,6 @@ repeat:
 	task_lock(tsk);
 	nodes_or(tsk->mems_allowed, tsk->mems_allowed, *newmems);
 	mpol_rebind_task(tsk, newmems, MPOL_REBIND_STEP1);
-	task_unlock(tsk);
 
 
 	/*

@@ -985,6 +984,7 @@ repeat:
 	 * for the read-side.
 	 */
 	while (ACCESS_ONCE(tsk->mems_allowed_change_disable)) {
+		task_unlock(tsk);
 		if (!task_curr(tsk))
 			yield();
 		goto repeat;

@@ -999,7 +999,6 @@ repeat:
 	 */
 	smp_mb();
 
-	task_lock(tsk);
 	mpol_rebind_task(tsk, newmems, MPOL_REBIND_STEP2);
 	tsk->mems_allowed = *newmems;
 	task_unlock(tsk);

index 41bc5b2..0ecb17b 100644
@@ -16,7 +16,6 @@
 #include <linux/key.h>
 #include <linux/security.h>
 #include <linux/cpu.h>
-#include <linux/cpuset.h>
 #include <linux/acct.h>
 #include <linux/tsacct_kern.h>
 #include <linux/file.h>

index 6e87c95..f4f0951 100644
@@ -31,7 +31,6 @@
 #include <linux/nsproxy.h>
 #include <linux/capability.h>
 #include <linux/cpu.h>
-#include <linux/cpuset.h>
 #include <linux/cgroup.h>
 #include <linux/security.h>
 #include <linux/hugetlb.h>

1.6.5.2


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

Maybe we can use wait_for_completion().

do you means using a real lock(such as: mutex) to protect mempolicy and mem_allowed?

It may cause the performance regression, so I do my best to abstain from using a real lock.

Thanks
Miao

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

That would work.

yes.

Well, the code as-is is pretty exotic with lots of open-coded tricky barriers - it's best to avoid inventing new primitives if possible. For example, there's no lockdep support for this new "lock".

mutex_lock() is pretty quick - basically a simgle atomic op. How frequently do these operations occur?

The code you have at present is fairly similar to sequence locks. I wonder if there's some way of (ab)using sequence locks for this. seqlocks don't have lockdep support either...

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

I didn't find an existing lock that could fix the problem well till now, so I had to design this new "lock" to protect the task's mempolicy and mems_allowed.

There is another problem that I forgot to mention.
besides the performance problem, the read-side may call it in the context in which the task can't sleep. so we can't use mutex.

We can't use sequence locks here, because the read-side may read the data in changing, but it can't put off cleaning the old bits.

Thanks
Miao

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

I don't understand that sentence. Can you expand on it please?

the mempolicy and mems_allowed tell the task that it should allocates the memory space on the specified node. so when allocating the memory space, the memory allocation functions that the task invokes must accesses the mempolicy and mems_allowed to find a node on which it can do memory allocation.

But those memory allocation functions can be used in both the context that the task can sleep and the context that the task can't sleep(etc. disable irq). so the real lock is not suitable.

And it is not a problem that the task allocates the memory space on the old allowed node when the mempolicy and mems_allowed is in changing, because the mempolicy and mems_allowed is not mandatory. So I think we needn't use a real lock to protect the mempolicy and mems_allowed in the read-side, and just use a real lock in the write-side. But there is a serious problem, that is the read -side may find no node to allocate memory and oom occurs, just like the following case(mentioned in the patch's changelog):
(mpol: mempolicy)
task1 task1's mpol task2
alloc page 1
alloc on node0? NO 1
1 change mems from 1 to 0
0 rebind task1's mpol
alloc on node1? NO 0
...
can't alloc page
goto oom

In order to fix this problem, I got an idea that we set the newly allowed nodes first, and then clean the disallowed nodes, But there is still a problem. (mpol: mempolicy)
task1 task1's mpol task2
alloc page 1
alloc on node0? NO 1
1 change mems from 1 to 0
1 rebind task1's mpol
0-1 set new bits
0 clear disallowed bits
alloc on node1? NO 0
...
can't alloc page
goto oom

It is because we cleanup disallowed nodes early, so I use a variable to tell the write-side that the task is accessing the mempolicy and mems_allowed now, the write-side must cleanup disallowed nodes soon after.

And the seq read lock can't provide this function. And besides that, the read-side will goto oom and not go back if it find no node to allcate memory, so it won't check the seq number of lock to find whether the mempolicy and mems_allowed have been changed. so the seq lock is also not suitable, I think.

Thanks
Miao