All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mempolicy: reduce references to the current
@ 2011-04-15  4:33 ` Namhyung Kim
  0 siblings, 0 replies; 14+ messages in thread
From: Namhyung Kim @ 2011-04-15  4:33 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-mm, linux-kernel

Remove duplicated reference to the 'current' task using a local
variable. Since refering the current can be a burden, it'd better
cache the reference, IMHO. At least this saves some bytes on x86_64.

  $ size mempolicy-{old,new}.o
     text    data    bss     dec     hex filename
    25203    2448   9176   36827    8fdb mempolicy-old.o
    25136    2448   9184   36768    8fa0 mempolicy-new.o

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 mm/mempolicy.c |   58 +++++++++++++++++++++++++++++--------------------------
 1 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 959a8b8c7350..37cc80ce5054 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -304,6 +304,7 @@ static void mpol_rebind_nodemask(struct mempolicy *pol, const nodemask_t *nodes,
 				 enum mpol_rebind_step step)
 {
 	nodemask_t tmp;
+	struct task_struct *curr = current;
 
 	if (pol->flags & MPOL_F_STATIC_NODES)
 		nodes_and(tmp, pol->w.user_nodemask, *nodes);
@@ -335,12 +336,12 @@ static void mpol_rebind_nodemask(struct mempolicy *pol, const nodemask_t *nodes,
 	else
 		BUG();
 
-	if (!node_isset(current->il_next, tmp)) {
-		current->il_next = next_node(current->il_next, tmp);
-		if (current->il_next >= MAX_NUMNODES)
-			current->il_next = first_node(tmp);
-		if (current->il_next >= MAX_NUMNODES)
-			current->il_next = numa_node_id();
+	if (!node_isset(curr->il_next, tmp)) {
+		curr->il_next = next_node(curr->il_next, tmp);
+		if (curr->il_next >= MAX_NUMNODES)
+			curr->il_next = first_node(tmp);
+		if (curr->il_next >= MAX_NUMNODES)
+			curr->il_next = numa_node_id();
 	}
 }
 
@@ -714,7 +715,8 @@ static long do_set_mempolicy(unsigned short mode, unsigned short flags,
 			     nodemask_t *nodes)
 {
 	struct mempolicy *new, *old;
-	struct mm_struct *mm = current->mm;
+	struct task_struct *curr = current;
+	struct mm_struct *mm = curr->mm;
 	NODEMASK_SCRATCH(scratch);
 	int ret;
 
@@ -734,22 +736,22 @@ static long do_set_mempolicy(unsigned short mode, unsigned short flags,
 	 */
 	if (mm)
 		down_write(&mm->mmap_sem);
-	task_lock(current);
+	task_lock(curr);
 	ret = mpol_set_nodemask(new, nodes, scratch);
 	if (ret) {
-		task_unlock(current);
+		task_unlock(curr);
 		if (mm)
 			up_write(&mm->mmap_sem);
 		mpol_put(new);
 		goto out;
 	}
-	old = current->mempolicy;
-	current->mempolicy = new;
+	old = curr->mempolicy;
+	curr->mempolicy = new;
 	mpol_set_task_struct_flag();
 	if (new && new->mode == MPOL_INTERLEAVE &&
 	    nodes_weight(new->v.nodes))
-		current->il_next = first_node(new->v.nodes);
-	task_unlock(current);
+		curr->il_next = first_node(new->v.nodes);
+	task_unlock(curr);
 	if (mm)
 		up_write(&mm->mmap_sem);
 
@@ -805,9 +807,10 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 			     unsigned long addr, unsigned long flags)
 {
 	int err;
-	struct mm_struct *mm = current->mm;
+	struct task_struct *curr = current;
+	struct mm_struct *mm = curr->mm;
 	struct vm_area_struct *vma = NULL;
-	struct mempolicy *pol = current->mempolicy;
+	struct mempolicy *pol = curr->mempolicy;
 
 	if (flags &
 		~(unsigned long)(MPOL_F_NODE|MPOL_F_ADDR|MPOL_F_MEMS_ALLOWED))
@@ -817,9 +820,9 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 		if (flags & (MPOL_F_NODE|MPOL_F_ADDR))
 			return -EINVAL;
 		*policy = 0;	/* just so it's initialized */
-		task_lock(current);
+		task_lock(curr);
 		*nmask  = cpuset_current_mems_allowed;
-		task_unlock(current);
+		task_unlock(curr);
 		return 0;
 	}
 
@@ -851,9 +854,9 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 			if (err < 0)
 				goto out;
 			*policy = err;
-		} else if (pol == current->mempolicy &&
+		} else if (pol == curr->mempolicy &&
 				pol->mode == MPOL_INTERLEAVE) {
-			*policy = current->il_next;
+			*policy = curr->il_next;
 		} else {
 			err = -EINVAL;
 			goto out;
@@ -869,7 +872,7 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 	}
 
 	if (vma) {
-		up_read(&current->mm->mmap_sem);
+		up_read(&mm->mmap_sem);
 		vma = NULL;
 	}
 
@@ -878,16 +881,16 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 		if (mpol_store_user_nodemask(pol)) {
 			*nmask = pol->w.user_nodemask;
 		} else {
-			task_lock(current);
+			task_lock(curr);
 			get_policy_nodemask(pol, nmask);
-			task_unlock(current);
+			task_unlock(curr);
 		}
 	}
 
  out:
 	mpol_cond_put(pol);
 	if (vma)
-		up_read(&current->mm->mmap_sem);
+		up_read(&mm->mmap_sem);
 	return err;
 }
 
@@ -1912,22 +1915,23 @@ EXPORT_SYMBOL(alloc_pages_current);
 /* Slow path of a mempolicy duplicate */
 struct mempolicy *__mpol_dup(struct mempolicy *old)
 {
+	struct task_struct *curr = current;
 	struct mempolicy *new = kmem_cache_alloc(policy_cache, GFP_KERNEL);
 
 	if (!new)
 		return ERR_PTR(-ENOMEM);
 
 	/* task's mempolicy is protected by alloc_lock */
-	if (old == current->mempolicy) {
-		task_lock(current);
+	if (old == curr->mempolicy) {
+		task_lock(curr);
 		*new = *old;
-		task_unlock(current);
+		task_unlock(curr);
 	} else
 		*new = *old;
 
 	rcu_read_lock();
 	if (current_cpuset_is_being_rebound()) {
-		nodemask_t mems = cpuset_mems_allowed(current);
+		nodemask_t mems = cpuset_mems_allowed(curr);
 		if (new->flags & MPOL_F_REBINDING)
 			mpol_rebind_policy(new, &mems, MPOL_REBIND_STEP2);
 		else
-- 
1.7.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH] mempolicy: reduce references to the current
@ 2011-04-15  4:33 ` Namhyung Kim
  0 siblings, 0 replies; 14+ messages in thread
From: Namhyung Kim @ 2011-04-15  4:33 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-mm, linux-kernel

Remove duplicated reference to the 'current' task using a local
variable. Since refering the current can be a burden, it'd better
cache the reference, IMHO. At least this saves some bytes on x86_64.

  $ size mempolicy-{old,new}.o
     text    data    bss     dec     hex filename
    25203    2448   9176   36827    8fdb mempolicy-old.o
    25136    2448   9184   36768    8fa0 mempolicy-new.o

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 mm/mempolicy.c |   58 +++++++++++++++++++++++++++++--------------------------
 1 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 959a8b8c7350..37cc80ce5054 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -304,6 +304,7 @@ static void mpol_rebind_nodemask(struct mempolicy *pol, const nodemask_t *nodes,
 				 enum mpol_rebind_step step)
 {
 	nodemask_t tmp;
+	struct task_struct *curr = current;
 
 	if (pol->flags & MPOL_F_STATIC_NODES)
 		nodes_and(tmp, pol->w.user_nodemask, *nodes);
@@ -335,12 +336,12 @@ static void mpol_rebind_nodemask(struct mempolicy *pol, const nodemask_t *nodes,
 	else
 		BUG();
 
-	if (!node_isset(current->il_next, tmp)) {
-		current->il_next = next_node(current->il_next, tmp);
-		if (current->il_next >= MAX_NUMNODES)
-			current->il_next = first_node(tmp);
-		if (current->il_next >= MAX_NUMNODES)
-			current->il_next = numa_node_id();
+	if (!node_isset(curr->il_next, tmp)) {
+		curr->il_next = next_node(curr->il_next, tmp);
+		if (curr->il_next >= MAX_NUMNODES)
+			curr->il_next = first_node(tmp);
+		if (curr->il_next >= MAX_NUMNODES)
+			curr->il_next = numa_node_id();
 	}
 }
 
@@ -714,7 +715,8 @@ static long do_set_mempolicy(unsigned short mode, unsigned short flags,
 			     nodemask_t *nodes)
 {
 	struct mempolicy *new, *old;
-	struct mm_struct *mm = current->mm;
+	struct task_struct *curr = current;
+	struct mm_struct *mm = curr->mm;
 	NODEMASK_SCRATCH(scratch);
 	int ret;
 
@@ -734,22 +736,22 @@ static long do_set_mempolicy(unsigned short mode, unsigned short flags,
 	 */
 	if (mm)
 		down_write(&mm->mmap_sem);
-	task_lock(current);
+	task_lock(curr);
 	ret = mpol_set_nodemask(new, nodes, scratch);
 	if (ret) {
-		task_unlock(current);
+		task_unlock(curr);
 		if (mm)
 			up_write(&mm->mmap_sem);
 		mpol_put(new);
 		goto out;
 	}
-	old = current->mempolicy;
-	current->mempolicy = new;
+	old = curr->mempolicy;
+	curr->mempolicy = new;
 	mpol_set_task_struct_flag();
 	if (new && new->mode == MPOL_INTERLEAVE &&
 	    nodes_weight(new->v.nodes))
-		current->il_next = first_node(new->v.nodes);
-	task_unlock(current);
+		curr->il_next = first_node(new->v.nodes);
+	task_unlock(curr);
 	if (mm)
 		up_write(&mm->mmap_sem);
 
@@ -805,9 +807,10 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 			     unsigned long addr, unsigned long flags)
 {
 	int err;
-	struct mm_struct *mm = current->mm;
+	struct task_struct *curr = current;
+	struct mm_struct *mm = curr->mm;
 	struct vm_area_struct *vma = NULL;
-	struct mempolicy *pol = current->mempolicy;
+	struct mempolicy *pol = curr->mempolicy;
 
 	if (flags &
 		~(unsigned long)(MPOL_F_NODE|MPOL_F_ADDR|MPOL_F_MEMS_ALLOWED))
@@ -817,9 +820,9 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 		if (flags & (MPOL_F_NODE|MPOL_F_ADDR))
 			return -EINVAL;
 		*policy = 0;	/* just so it's initialized */
-		task_lock(current);
+		task_lock(curr);
 		*nmask  = cpuset_current_mems_allowed;
-		task_unlock(current);
+		task_unlock(curr);
 		return 0;
 	}
 
@@ -851,9 +854,9 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 			if (err < 0)
 				goto out;
 			*policy = err;
-		} else if (pol == current->mempolicy &&
+		} else if (pol == curr->mempolicy &&
 				pol->mode == MPOL_INTERLEAVE) {
-			*policy = current->il_next;
+			*policy = curr->il_next;
 		} else {
 			err = -EINVAL;
 			goto out;
@@ -869,7 +872,7 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 	}
 
 	if (vma) {
-		up_read(&current->mm->mmap_sem);
+		up_read(&mm->mmap_sem);
 		vma = NULL;
 	}
 
@@ -878,16 +881,16 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 		if (mpol_store_user_nodemask(pol)) {
 			*nmask = pol->w.user_nodemask;
 		} else {
-			task_lock(current);
+			task_lock(curr);
 			get_policy_nodemask(pol, nmask);
-			task_unlock(current);
+			task_unlock(curr);
 		}
 	}
 
  out:
 	mpol_cond_put(pol);
 	if (vma)
-		up_read(&current->mm->mmap_sem);
+		up_read(&mm->mmap_sem);
 	return err;
 }
 
@@ -1912,22 +1915,23 @@ EXPORT_SYMBOL(alloc_pages_current);
 /* Slow path of a mempolicy duplicate */
 struct mempolicy *__mpol_dup(struct mempolicy *old)
 {
+	struct task_struct *curr = current;
 	struct mempolicy *new = kmem_cache_alloc(policy_cache, GFP_KERNEL);
 
 	if (!new)
 		return ERR_PTR(-ENOMEM);
 
 	/* task's mempolicy is protected by alloc_lock */
-	if (old == current->mempolicy) {
-		task_lock(current);
+	if (old == curr->mempolicy) {
+		task_lock(curr);
 		*new = *old;
-		task_unlock(current);
+		task_unlock(curr);
 	} else
 		*new = *old;
 
 	rcu_read_lock();
 	if (current_cpuset_is_being_rebound()) {
-		nodemask_t mems = cpuset_mems_allowed(current);
+		nodemask_t mems = cpuset_mems_allowed(curr);
 		if (new->flags & MPOL_F_REBINDING)
 			mpol_rebind_policy(new, &mems, MPOL_REBIND_STEP2);
 		else
-- 
1.7.4

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH] mempolicy: reduce references to the current
  2011-04-15  4:33 ` Namhyung Kim
@ 2011-04-15  5:41   ` Minchan Kim
  -1 siblings, 0 replies; 14+ messages in thread
From: Minchan Kim @ 2011-04-15  5:41 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Andrew Morton, linux-mm, linux-kernel

On Fri, Apr 15, 2011 at 1:33 PM, Namhyung Kim <namhyung@gmail.com> wrote:
> Remove duplicated reference to the 'current' task using a local
> variable. Since refering the current can be a burden, it'd better
> cache the reference, IMHO. At least this saves some bytes on x86_64.
>
>  $ size mempolicy-{old,new}.o
>     text    data    bss     dec     hex filename
>    25203    2448   9176   36827    8fdb mempolicy-old.o
>    25136    2448   9184   36768    8fa0 mempolicy-new.o
>
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>

Hi Namhyung,

The patch looks good to me. :)
But I have a nitpick. "curr" is rather awkward to me.
We have been used "tsk" and "p" instead of "curr" for task_struct.
But I don't like "p" since it has no meaning. So for consistency,
could you change it with "tsk"?

Thanks.
-- 
Kind regards,
Minchan Kim

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH] mempolicy: reduce references to the current
@ 2011-04-15  5:41   ` Minchan Kim
  0 siblings, 0 replies; 14+ messages in thread
From: Minchan Kim @ 2011-04-15  5:41 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Andrew Morton, linux-mm, linux-kernel

On Fri, Apr 15, 2011 at 1:33 PM, Namhyung Kim <namhyung@gmail.com> wrote:
> Remove duplicated reference to the 'current' task using a local
> variable. Since refering the current can be a burden, it'd better
> cache the reference, IMHO. At least this saves some bytes on x86_64.
>
>  $ size mempolicy-{old,new}.o
>     text    data    bss     dec     hex filename
>    25203    2448   9176   36827    8fdb mempolicy-old.o
>    25136    2448   9184   36768    8fa0 mempolicy-new.o
>
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>

Hi Namhyung,

The patch looks good to me. :)
But I have a nitpick. "curr" is rather awkward to me.
We have been used "tsk" and "p" instead of "curr" for task_struct.
But I don't like "p" since it has no meaning. So for consistency,
could you change it with "tsk"?

Thanks.
-- 
Kind regards,
Minchan Kim

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

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH v2] mempolicy: reduce references to the current
  2011-04-15  5:41   ` Minchan Kim
@ 2011-04-15  6:08     ` Namhyung Kim
  -1 siblings, 0 replies; 14+ messages in thread
From: Namhyung Kim @ 2011-04-15  6:08 UTC (permalink / raw)
  To: Minchan Kim; +Cc: Andrew Morton, linux-mm, linux-kernel

Remove duplicated reference to the 'current' task using a local
variable. Since refering the current can be a burden, it'd better
cache the reference, IMHO. At least this saves some bytes on x86_64.

  $ size mempolicy-{old,new}.o
     text    data    bss     dec     hex filename
    25203    2448   9176   36827    8fdb mempolicy-old.o
    25136    2448   9184   36768    8fa0 mempolicy-new.o

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 mm/mempolicy.c |   58 +++++++++++++++++++++++++++++--------------------------
 1 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 959a8b8c7350..5a30065590aa 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -304,6 +304,7 @@ static void mpol_rebind_nodemask(struct mempolicy *pol, const nodemask_t *nodes,
 				 enum mpol_rebind_step step)
 {
 	nodemask_t tmp;
+	struct task_struct *tsk = current;
 
 	if (pol->flags & MPOL_F_STATIC_NODES)
 		nodes_and(tmp, pol->w.user_nodemask, *nodes);
@@ -335,12 +336,12 @@ static void mpol_rebind_nodemask(struct mempolicy *pol, const nodemask_t *nodes,
 	else
 		BUG();
 
-	if (!node_isset(current->il_next, tmp)) {
-		current->il_next = next_node(current->il_next, tmp);
-		if (current->il_next >= MAX_NUMNODES)
-			current->il_next = first_node(tmp);
-		if (current->il_next >= MAX_NUMNODES)
-			current->il_next = numa_node_id();
+	if (!node_isset(tsk->il_next, tmp)) {
+		tsk->il_next = next_node(tsk->il_next, tmp);
+		if (tsk->il_next >= MAX_NUMNODES)
+			tsk->il_next = first_node(tmp);
+		if (tsk->il_next >= MAX_NUMNODES)
+			tsk->il_next = numa_node_id();
 	}
 }
 
@@ -714,7 +715,8 @@ static long do_set_mempolicy(unsigned short mode, unsigned short flags,
 			     nodemask_t *nodes)
 {
 	struct mempolicy *new, *old;
-	struct mm_struct *mm = current->mm;
+	struct task_struct *tsk = current;
+	struct mm_struct *mm = tsk->mm;
 	NODEMASK_SCRATCH(scratch);
 	int ret;
 
@@ -734,22 +736,22 @@ static long do_set_mempolicy(unsigned short mode, unsigned short flags,
 	 */
 	if (mm)
 		down_write(&mm->mmap_sem);
-	task_lock(current);
+	task_lock(tsk);
 	ret = mpol_set_nodemask(new, nodes, scratch);
 	if (ret) {
-		task_unlock(current);
+		task_unlock(tsk);
 		if (mm)
 			up_write(&mm->mmap_sem);
 		mpol_put(new);
 		goto out;
 	}
-	old = current->mempolicy;
-	current->mempolicy = new;
+	old = tsk->mempolicy;
+	tsk->mempolicy = new;
 	mpol_set_task_struct_flag();
 	if (new && new->mode == MPOL_INTERLEAVE &&
 	    nodes_weight(new->v.nodes))
-		current->il_next = first_node(new->v.nodes);
-	task_unlock(current);
+		tsk->il_next = first_node(new->v.nodes);
+	task_unlock(tsk);
 	if (mm)
 		up_write(&mm->mmap_sem);
 
@@ -805,9 +807,10 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 			     unsigned long addr, unsigned long flags)
 {
 	int err;
-	struct mm_struct *mm = current->mm;
+	struct task_struct *tsk = current;
+	struct mm_struct *mm = tsk->mm;
 	struct vm_area_struct *vma = NULL;
-	struct mempolicy *pol = current->mempolicy;
+	struct mempolicy *pol = tsk->mempolicy;
 
 	if (flags &
 		~(unsigned long)(MPOL_F_NODE|MPOL_F_ADDR|MPOL_F_MEMS_ALLOWED))
@@ -817,9 +820,9 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 		if (flags & (MPOL_F_NODE|MPOL_F_ADDR))
 			return -EINVAL;
 		*policy = 0;	/* just so it's initialized */
-		task_lock(current);
+		task_lock(tsk);
 		*nmask  = cpuset_current_mems_allowed;
-		task_unlock(current);
+		task_unlock(tsk);
 		return 0;
 	}
 
@@ -851,9 +854,9 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 			if (err < 0)
 				goto out;
 			*policy = err;
-		} else if (pol == current->mempolicy &&
+		} else if (pol == tsk->mempolicy &&
 				pol->mode == MPOL_INTERLEAVE) {
-			*policy = current->il_next;
+			*policy = tsk->il_next;
 		} else {
 			err = -EINVAL;
 			goto out;
@@ -869,7 +872,7 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 	}
 
 	if (vma) {
-		up_read(&current->mm->mmap_sem);
+		up_read(&mm->mmap_sem);
 		vma = NULL;
 	}
 
@@ -878,16 +881,16 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 		if (mpol_store_user_nodemask(pol)) {
 			*nmask = pol->w.user_nodemask;
 		} else {
-			task_lock(current);
+			task_lock(tsk);
 			get_policy_nodemask(pol, nmask);
-			task_unlock(current);
+			task_unlock(tsk);
 		}
 	}
 
  out:
 	mpol_cond_put(pol);
 	if (vma)
-		up_read(&current->mm->mmap_sem);
+		up_read(&mm->mmap_sem);
 	return err;
 }
 
@@ -1912,22 +1915,23 @@ EXPORT_SYMBOL(alloc_pages_current);
 /* Slow path of a mempolicy duplicate */
 struct mempolicy *__mpol_dup(struct mempolicy *old)
 {
+	struct task_struct *tsk = current;
 	struct mempolicy *new = kmem_cache_alloc(policy_cache, GFP_KERNEL);
 
 	if (!new)
 		return ERR_PTR(-ENOMEM);
 
 	/* task's mempolicy is protected by alloc_lock */
-	if (old == current->mempolicy) {
-		task_lock(current);
+	if (old == tsk->mempolicy) {
+		task_lock(tsk);
 		*new = *old;
-		task_unlock(current);
+		task_unlock(tsk);
 	} else
 		*new = *old;
 
 	rcu_read_lock();
 	if (current_cpuset_is_being_rebound()) {
-		nodemask_t mems = cpuset_mems_allowed(current);
+		nodemask_t mems = cpuset_mems_allowed(tsk);
 		if (new->flags & MPOL_F_REBINDING)
 			mpol_rebind_policy(new, &mems, MPOL_REBIND_STEP2);
 		else
-- 
1.7.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH v2] mempolicy: reduce references to the current
@ 2011-04-15  6:08     ` Namhyung Kim
  0 siblings, 0 replies; 14+ messages in thread
From: Namhyung Kim @ 2011-04-15  6:08 UTC (permalink / raw)
  To: Minchan Kim; +Cc: Andrew Morton, linux-mm, linux-kernel

Remove duplicated reference to the 'current' task using a local
variable. Since refering the current can be a burden, it'd better
cache the reference, IMHO. At least this saves some bytes on x86_64.

  $ size mempolicy-{old,new}.o
     text    data    bss     dec     hex filename
    25203    2448   9176   36827    8fdb mempolicy-old.o
    25136    2448   9184   36768    8fa0 mempolicy-new.o

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 mm/mempolicy.c |   58 +++++++++++++++++++++++++++++--------------------------
 1 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 959a8b8c7350..5a30065590aa 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -304,6 +304,7 @@ static void mpol_rebind_nodemask(struct mempolicy *pol, const nodemask_t *nodes,
 				 enum mpol_rebind_step step)
 {
 	nodemask_t tmp;
+	struct task_struct *tsk = current;
 
 	if (pol->flags & MPOL_F_STATIC_NODES)
 		nodes_and(tmp, pol->w.user_nodemask, *nodes);
@@ -335,12 +336,12 @@ static void mpol_rebind_nodemask(struct mempolicy *pol, const nodemask_t *nodes,
 	else
 		BUG();
 
-	if (!node_isset(current->il_next, tmp)) {
-		current->il_next = next_node(current->il_next, tmp);
-		if (current->il_next >= MAX_NUMNODES)
-			current->il_next = first_node(tmp);
-		if (current->il_next >= MAX_NUMNODES)
-			current->il_next = numa_node_id();
+	if (!node_isset(tsk->il_next, tmp)) {
+		tsk->il_next = next_node(tsk->il_next, tmp);
+		if (tsk->il_next >= MAX_NUMNODES)
+			tsk->il_next = first_node(tmp);
+		if (tsk->il_next >= MAX_NUMNODES)
+			tsk->il_next = numa_node_id();
 	}
 }
 
@@ -714,7 +715,8 @@ static long do_set_mempolicy(unsigned short mode, unsigned short flags,
 			     nodemask_t *nodes)
 {
 	struct mempolicy *new, *old;
-	struct mm_struct *mm = current->mm;
+	struct task_struct *tsk = current;
+	struct mm_struct *mm = tsk->mm;
 	NODEMASK_SCRATCH(scratch);
 	int ret;
 
@@ -734,22 +736,22 @@ static long do_set_mempolicy(unsigned short mode, unsigned short flags,
 	 */
 	if (mm)
 		down_write(&mm->mmap_sem);
-	task_lock(current);
+	task_lock(tsk);
 	ret = mpol_set_nodemask(new, nodes, scratch);
 	if (ret) {
-		task_unlock(current);
+		task_unlock(tsk);
 		if (mm)
 			up_write(&mm->mmap_sem);
 		mpol_put(new);
 		goto out;
 	}
-	old = current->mempolicy;
-	current->mempolicy = new;
+	old = tsk->mempolicy;
+	tsk->mempolicy = new;
 	mpol_set_task_struct_flag();
 	if (new && new->mode == MPOL_INTERLEAVE &&
 	    nodes_weight(new->v.nodes))
-		current->il_next = first_node(new->v.nodes);
-	task_unlock(current);
+		tsk->il_next = first_node(new->v.nodes);
+	task_unlock(tsk);
 	if (mm)
 		up_write(&mm->mmap_sem);
 
@@ -805,9 +807,10 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 			     unsigned long addr, unsigned long flags)
 {
 	int err;
-	struct mm_struct *mm = current->mm;
+	struct task_struct *tsk = current;
+	struct mm_struct *mm = tsk->mm;
 	struct vm_area_struct *vma = NULL;
-	struct mempolicy *pol = current->mempolicy;
+	struct mempolicy *pol = tsk->mempolicy;
 
 	if (flags &
 		~(unsigned long)(MPOL_F_NODE|MPOL_F_ADDR|MPOL_F_MEMS_ALLOWED))
@@ -817,9 +820,9 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 		if (flags & (MPOL_F_NODE|MPOL_F_ADDR))
 			return -EINVAL;
 		*policy = 0;	/* just so it's initialized */
-		task_lock(current);
+		task_lock(tsk);
 		*nmask  = cpuset_current_mems_allowed;
-		task_unlock(current);
+		task_unlock(tsk);
 		return 0;
 	}
 
@@ -851,9 +854,9 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 			if (err < 0)
 				goto out;
 			*policy = err;
-		} else if (pol == current->mempolicy &&
+		} else if (pol == tsk->mempolicy &&
 				pol->mode == MPOL_INTERLEAVE) {
-			*policy = current->il_next;
+			*policy = tsk->il_next;
 		} else {
 			err = -EINVAL;
 			goto out;
@@ -869,7 +872,7 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 	}
 
 	if (vma) {
-		up_read(&current->mm->mmap_sem);
+		up_read(&mm->mmap_sem);
 		vma = NULL;
 	}
 
@@ -878,16 +881,16 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 		if (mpol_store_user_nodemask(pol)) {
 			*nmask = pol->w.user_nodemask;
 		} else {
-			task_lock(current);
+			task_lock(tsk);
 			get_policy_nodemask(pol, nmask);
-			task_unlock(current);
+			task_unlock(tsk);
 		}
 	}
 
  out:
 	mpol_cond_put(pol);
 	if (vma)
-		up_read(&current->mm->mmap_sem);
+		up_read(&mm->mmap_sem);
 	return err;
 }
 
@@ -1912,22 +1915,23 @@ EXPORT_SYMBOL(alloc_pages_current);
 /* Slow path of a mempolicy duplicate */
 struct mempolicy *__mpol_dup(struct mempolicy *old)
 {
+	struct task_struct *tsk = current;
 	struct mempolicy *new = kmem_cache_alloc(policy_cache, GFP_KERNEL);
 
 	if (!new)
 		return ERR_PTR(-ENOMEM);
 
 	/* task's mempolicy is protected by alloc_lock */
-	if (old == current->mempolicy) {
-		task_lock(current);
+	if (old == tsk->mempolicy) {
+		task_lock(tsk);
 		*new = *old;
-		task_unlock(current);
+		task_unlock(tsk);
 	} else
 		*new = *old;
 
 	rcu_read_lock();
 	if (current_cpuset_is_being_rebound()) {
-		nodemask_t mems = cpuset_mems_allowed(current);
+		nodemask_t mems = cpuset_mems_allowed(tsk);
 		if (new->flags & MPOL_F_REBINDING)
 			mpol_rebind_policy(new, &mems, MPOL_REBIND_STEP2);
 		else
-- 
1.7.4

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH v2] mempolicy: reduce references to the current
  2011-04-15  6:08     ` Namhyung Kim
@ 2011-04-15  6:23       ` Minchan Kim
  -1 siblings, 0 replies; 14+ messages in thread
From: Minchan Kim @ 2011-04-15  6:23 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Andrew Morton, linux-mm, linux-kernel

On Fri, Apr 15, 2011 at 3:08 PM, Namhyung Kim <namhyung@gmail.com> wrote:
> Remove duplicated reference to the 'current' task using a local
> variable. Since refering the current can be a burden, it'd better
> cache the reference, IMHO. At least this saves some bytes on x86_64.
>
>  $ size mempolicy-{old,new}.o
>     text    data    bss     dec     hex filename
>    25203    2448   9176   36827    8fdb mempolicy-old.o
>    25136    2448   9184   36768    8fa0 mempolicy-new.o
>
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

-- 
Kind regards,
Minchan Kim

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v2] mempolicy: reduce references to the current
@ 2011-04-15  6:23       ` Minchan Kim
  0 siblings, 0 replies; 14+ messages in thread
From: Minchan Kim @ 2011-04-15  6:23 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Andrew Morton, linux-mm, linux-kernel

On Fri, Apr 15, 2011 at 3:08 PM, Namhyung Kim <namhyung@gmail.com> wrote:
> Remove duplicated reference to the 'current' task using a local
> variable. Since refering the current can be a burden, it'd better
> cache the reference, IMHO. At least this saves some bytes on x86_64.
>
>  $ size mempolicy-{old,new}.o
>     text    data    bss     dec     hex filename
>    25203    2448   9176   36827    8fdb mempolicy-old.o
>    25136    2448   9184   36768    8fa0 mempolicy-new.o
>
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

-- 
Kind regards,
Minchan Kim

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

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v2] mempolicy: reduce references to the current
  2011-04-15  6:08     ` Namhyung Kim
@ 2011-04-15 23:35       ` David Rientjes
  -1 siblings, 0 replies; 14+ messages in thread
From: David Rientjes @ 2011-04-15 23:35 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Minchan Kim, Andrew Morton, linux-mm, linux-kernel

On Fri, 15 Apr 2011, Namhyung Kim wrote:

> Remove duplicated reference to the 'current' task using a local
> variable. Since refering the current can be a burden, it'd better
> cache the reference, IMHO. At least this saves some bytes on x86_64.
> 
>   $ size mempolicy-{old,new}.o
>      text    data    bss     dec     hex filename
>     25203    2448   9176   36827    8fdb mempolicy-old.o
>     25136    2448   9184   36768    8fa0 mempolicy-new.o
> 

So this is the opposite of what Andrew did in c06b1fca18c3 
(mm/page_alloc.c: don't cache `current' in a local) for the page 
allocator?

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v2] mempolicy: reduce references to the current
@ 2011-04-15 23:35       ` David Rientjes
  0 siblings, 0 replies; 14+ messages in thread
From: David Rientjes @ 2011-04-15 23:35 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Minchan Kim, Andrew Morton, linux-mm, linux-kernel

On Fri, 15 Apr 2011, Namhyung Kim wrote:

> Remove duplicated reference to the 'current' task using a local
> variable. Since refering the current can be a burden, it'd better
> cache the reference, IMHO. At least this saves some bytes on x86_64.
> 
>   $ size mempolicy-{old,new}.o
>      text    data    bss     dec     hex filename
>     25203    2448   9176   36827    8fdb mempolicy-old.o
>     25136    2448   9184   36768    8fa0 mempolicy-new.o
> 

So this is the opposite of what Andrew did in c06b1fca18c3 
(mm/page_alloc.c: don't cache `current' in a local) for the page 
allocator?

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

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v2] mempolicy: reduce references to the current
  2011-04-15  6:08     ` Namhyung Kim
@ 2011-04-18 19:34       ` Andrew Morton
  -1 siblings, 0 replies; 14+ messages in thread
From: Andrew Morton @ 2011-04-18 19:34 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Minchan Kim, linux-mm, linux-kernel, Tejun Heo, Christoph Lameter

On Fri, 15 Apr 2011 15:08:08 +0900
Namhyung Kim <namhyung@gmail.com> wrote:

> Remove duplicated reference to the 'current' task using a local
> variable. Since refering the current can be a burden, it'd better
> cache the reference, IMHO. At least this saves some bytes on x86_64.
> 
>   $ size mempolicy-{old,new}.o
>      text    data    bss     dec     hex filename
>     25203    2448   9176   36827    8fdb mempolicy-old.o
>     25136    2448   9184   36768    8fa0 mempolicy-new.o
> 
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
> ---
>  mm/mempolicy.c |   58 +++++++++++++++++++++++++++++--------------------------
>  1 files changed, 31 insertions(+), 27 deletions(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 959a8b8c7350..5a30065590aa 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -304,6 +304,7 @@ static void mpol_rebind_nodemask(struct mempolicy *pol, const nodemask_t *nodes,
>  				 enum mpol_rebind_step step)
>  {
>  	nodemask_t tmp;
> +	struct task_struct *tsk = current;
>  
>  	if (pol->flags & MPOL_F_STATIC_NODES)
>  		nodes_and(tmp, pol->w.user_nodemask, *nodes);
> @@ -335,12 +336,12 @@ static void mpol_rebind_nodemask(struct mempolicy *pol, const nodemask_t *nodes,
>  	else
>  		BUG();
>  
> -	if (!node_isset(current->il_next, tmp)) {
> -		current->il_next = next_node(current->il_next, tmp);
> -		if (current->il_next >= MAX_NUMNODES)
> -			current->il_next = first_node(tmp);
> -		if (current->il_next >= MAX_NUMNODES)
> -			current->il_next = numa_node_id();
> +	if (!node_isset(tsk->il_next, tmp)) {
> +		tsk->il_next = next_node(tsk->il_next, tmp);
> +		if (tsk->il_next >= MAX_NUMNODES)
> +			tsk->il_next = first_node(tmp);
> +		if (tsk->il_next >= MAX_NUMNODES)
> +			tsk->il_next = numa_node_id();
>  	}
>  }

Odd.  The new(ish) percpu_read_stable() stuff produces very efficient
code for `current' and usually means that caching `current' in a local
is unneeded, often an overall loss.

So... what is going wrong in mempolicy.c?

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v2] mempolicy: reduce references to the current
@ 2011-04-18 19:34       ` Andrew Morton
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Morton @ 2011-04-18 19:34 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Minchan Kim, linux-mm, linux-kernel, Tejun Heo, Christoph Lameter

On Fri, 15 Apr 2011 15:08:08 +0900
Namhyung Kim <namhyung@gmail.com> wrote:

> Remove duplicated reference to the 'current' task using a local
> variable. Since refering the current can be a burden, it'd better
> cache the reference, IMHO. At least this saves some bytes on x86_64.
> 
>   $ size mempolicy-{old,new}.o
>      text    data    bss     dec     hex filename
>     25203    2448   9176   36827    8fdb mempolicy-old.o
>     25136    2448   9184   36768    8fa0 mempolicy-new.o
> 
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
> ---
>  mm/mempolicy.c |   58 +++++++++++++++++++++++++++++--------------------------
>  1 files changed, 31 insertions(+), 27 deletions(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 959a8b8c7350..5a30065590aa 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -304,6 +304,7 @@ static void mpol_rebind_nodemask(struct mempolicy *pol, const nodemask_t *nodes,
>  				 enum mpol_rebind_step step)
>  {
>  	nodemask_t tmp;
> +	struct task_struct *tsk = current;
>  
>  	if (pol->flags & MPOL_F_STATIC_NODES)
>  		nodes_and(tmp, pol->w.user_nodemask, *nodes);
> @@ -335,12 +336,12 @@ static void mpol_rebind_nodemask(struct mempolicy *pol, const nodemask_t *nodes,
>  	else
>  		BUG();
>  
> -	if (!node_isset(current->il_next, tmp)) {
> -		current->il_next = next_node(current->il_next, tmp);
> -		if (current->il_next >= MAX_NUMNODES)
> -			current->il_next = first_node(tmp);
> -		if (current->il_next >= MAX_NUMNODES)
> -			current->il_next = numa_node_id();
> +	if (!node_isset(tsk->il_next, tmp)) {
> +		tsk->il_next = next_node(tsk->il_next, tmp);
> +		if (tsk->il_next >= MAX_NUMNODES)
> +			tsk->il_next = first_node(tmp);
> +		if (tsk->il_next >= MAX_NUMNODES)
> +			tsk->il_next = numa_node_id();
>  	}
>  }

Odd.  The new(ish) percpu_read_stable() stuff produces very efficient
code for `current' and usually means that caching `current' in a local
is unneeded, often an overall loss.

So... what is going wrong in mempolicy.c?

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

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v2] mempolicy: reduce references to the current
  2011-04-15  6:08     ` Namhyung Kim
@ 2011-04-19  0:34       ` KOSAKI Motohiro
  -1 siblings, 0 replies; 14+ messages in thread
From: KOSAKI Motohiro @ 2011-04-19  0:34 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: kosaki.motohiro, Minchan Kim, Andrew Morton, linux-mm, linux-kernel

> Remove duplicated reference to the 'current' task using a local
> variable. Since refering the current can be a burden, it'd better
> cache the reference, IMHO. At least this saves some bytes on x86_64.
> 
>   $ size mempolicy-{old,new}.o
>      text    data    bss     dec     hex filename
>     25203    2448   9176   36827    8fdb mempolicy-old.o
>     25136    2448   9184   36768    8fa0 mempolicy-new.o
> 
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>

But, dense stack usage is also performance good thing. Therefore your
patch benefit is not obvious. I have two request.

1) Please don't increase mess into no hot path. It's no worth.
2) Please mesure performance your box instead size command.

thanks.



^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v2] mempolicy: reduce references to the current
@ 2011-04-19  0:34       ` KOSAKI Motohiro
  0 siblings, 0 replies; 14+ messages in thread
From: KOSAKI Motohiro @ 2011-04-19  0:34 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: kosaki.motohiro, Minchan Kim, Andrew Morton, linux-mm, linux-kernel

> Remove duplicated reference to the 'current' task using a local
> variable. Since refering the current can be a burden, it'd better
> cache the reference, IMHO. At least this saves some bytes on x86_64.
> 
>   $ size mempolicy-{old,new}.o
>      text    data    bss     dec     hex filename
>     25203    2448   9176   36827    8fdb mempolicy-old.o
>     25136    2448   9184   36768    8fa0 mempolicy-new.o
> 
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>

But, dense stack usage is also performance good thing. Therefore your
patch benefit is not obvious. I have two request.

1) Please don't increase mess into no hot path. It's no worth.
2) Please mesure performance your box instead size command.

thanks.


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

^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-04-19  0:34 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-15  4:33 [PATCH] mempolicy: reduce references to the current Namhyung Kim
2011-04-15  4:33 ` Namhyung Kim
2011-04-15  5:41 ` Minchan Kim
2011-04-15  5:41   ` Minchan Kim
2011-04-15  6:08   ` [PATCH v2] " Namhyung Kim
2011-04-15  6:08     ` Namhyung Kim
2011-04-15  6:23     ` Minchan Kim
2011-04-15  6:23       ` Minchan Kim
2011-04-15 23:35     ` David Rientjes
2011-04-15 23:35       ` David Rientjes
2011-04-18 19:34     ` Andrew Morton
2011-04-18 19:34       ` Andrew Morton
2011-04-19  0:34     ` KOSAKI Motohiro
2011-04-19  0:34       ` KOSAKI Motohiro

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.