linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [patch 1/2] mm: make swap token dummies static inlines
       [not found] <Pine.LNX.4.64.0906162152250.12770@sister.anvils>
@ 2009-06-16 21:50 ` Johannes Weiner
  2009-06-16 21:55   ` Rik van Riel
  2009-06-16 21:50 ` [patch 2/2] mm: remove task assumptions from swap token Johannes Weiner
  1 sibling, 1 reply; 6+ messages in thread
From: Johannes Weiner @ 2009-06-16 21:50 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Hugh Dickins, Andrea Arcangeli, Izik Eidus, Rik van Riel,
	Nick Piggin, linux-mm, linux-kernel

Make use of the compiler's typechecking on !CONFIG_SWAP as well.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 include/linux/swap.h |   20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index d476aad..3c6e856 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -426,10 +426,22 @@ static inline swp_entry_t get_swap_page(void)
 }
 
 /* linux/mm/thrash.c */
-#define put_swap_token(x) do { } while(0)
-#define grab_swap_token()  do { } while(0)
-#define has_swap_token(x) 0
-#define disable_swap_token() do { } while(0)
+static inline void put_swap_token(struct mm_struct *mm)
+{
+}
+
+static inline void grab_swap_token(void)
+{
+}
+
+static inline int has_swap_token(struct mm_struct *mm)
+{
+	return 0;
+}
+
+static inline void disable_swap_token(struct mm_struct *mm)
+{
+}
 
 static inline int mem_cgroup_cache_charge_swapin(struct page *page,
 			struct mm_struct *mm, gfp_t mask, bool locked)
-- 
1.6.3

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [patch 2/2] mm: remove task assumptions from swap token
       [not found] <Pine.LNX.4.64.0906162152250.12770@sister.anvils>
  2009-06-16 21:50 ` [patch 1/2] mm: make swap token dummies static inlines Johannes Weiner
@ 2009-06-16 21:50 ` Johannes Weiner
  2009-06-16 21:56   ` Rik van Riel
  2009-06-17  2:00   ` Minchan Kim
  1 sibling, 2 replies; 6+ messages in thread
From: Johannes Weiner @ 2009-06-16 21:50 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Hugh Dickins, Andrea Arcangeli, Izik Eidus, Rik van Riel,
	Nick Piggin, linux-mm, linux-kernel

From: Hugh Dickins <hugh.dickins@tiscali.co.uk>

grab_swap_token() should not make any assumptions about the running
process as the swap token is an attribute of the address space and the
faulting mm is not necessarily current->mm.

This fixes get_user_pages() from kernel threads which would blow up
when encountering a swapped out page and grab_swap_token()
dereferencing the unset for kernel threads current->mm.

Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 include/linux/swap.h |    4 ++--
 mm/memory.c          |    2 +-
 mm/thrash.c          |   31 +++++++++++++++----------------
 3 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 3c6e856..e73ea8a 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -315,7 +315,7 @@ struct backing_dev_info;
 
 /* linux/mm/thrash.c */
 extern struct mm_struct * swap_token_mm;
-extern void grab_swap_token(void);
+extern void grab_swap_token(struct mm_struct *);
 extern void __put_swap_token(struct mm_struct *);
 
 static inline int has_swap_token(struct mm_struct *mm)
@@ -430,7 +430,7 @@ static inline void put_swap_token(struct mm_struct *mm)
 {
 }
 
-static inline void grab_swap_token(void)
+static inline void grab_swap_token(struct mm_struct *mm)
 {
 }
 
diff --git a/mm/memory.c b/mm/memory.c
index 4126dd1..862e120 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2466,7 +2466,7 @@ static int do_swap_page(struct mm_struct *mm, struct vm_area_struct *vma,
 	delayacct_set_flag(DELAYACCT_PF_SWAPIN);
 	page = lookup_swap_cache(entry);
 	if (!page) {
-		grab_swap_token(); /* Contend for token _before_ read-in */
+		grab_swap_token(mm); /* Contend for token _before_ read-in */
 		page = swapin_readahead(entry,
 					GFP_HIGHUSER_MOVABLE, vma, address);
 		if (!page) {
diff --git a/mm/thrash.c b/mm/thrash.c
index c4c5205..8b864ae 100644
--- a/mm/thrash.c
+++ b/mm/thrash.c
@@ -26,47 +26,46 @@ static DEFINE_SPINLOCK(swap_token_lock);
 struct mm_struct *swap_token_mm;
 static unsigned int global_faults;
 
-void grab_swap_token(void)
+void grab_swap_token(struct mm_struct *mm)
 {
 	int current_interval;
 
 	global_faults++;
 
-	current_interval = global_faults - current->mm->faultstamp;
+	current_interval = global_faults - mm->faultstamp;
 
 	if (!spin_trylock(&swap_token_lock))
 		return;
 
 	/* First come first served */
 	if (swap_token_mm == NULL) {
-		current->mm->token_priority = current->mm->token_priority + 2;
-		swap_token_mm = current->mm;
+		mm->token_priority = mm->token_priority + 2;
+		swap_token_mm = mm;
 		goto out;
 	}
 
-	if (current->mm != swap_token_mm) {
-		if (current_interval < current->mm->last_interval)
-			current->mm->token_priority++;
+	if (mm != swap_token_mm) {
+		if (current_interval < mm->last_interval)
+			mm->token_priority++;
 		else {
-			if (likely(current->mm->token_priority > 0))
-				current->mm->token_priority--;
+			if (likely(mm->token_priority > 0))
+				mm->token_priority--;
 		}
 		/* Check if we deserve the token */
-		if (current->mm->token_priority >
+		if (mm->token_priority >
 				swap_token_mm->token_priority) {
-			current->mm->token_priority += 2;
-			swap_token_mm = current->mm;
+			mm->token_priority += 2;
+			swap_token_mm = mm;
 		}
 	} else {
 		/* Token holder came in again! */
-		current->mm->token_priority += 2;
+		mm->token_priority += 2;
 	}
 
 out:
-	current->mm->faultstamp = global_faults;
-	current->mm->last_interval = current_interval;
+	mm->faultstamp = global_faults;
+	mm->last_interval = current_interval;
 	spin_unlock(&swap_token_lock);
-return;
 }
 
 /* Called on process exit. */
-- 
1.6.3

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 1/2] mm: make swap token dummies static inlines
  2009-06-16 21:50 ` [patch 1/2] mm: make swap token dummies static inlines Johannes Weiner
@ 2009-06-16 21:55   ` Rik van Riel
  0 siblings, 0 replies; 6+ messages in thread
From: Rik van Riel @ 2009-06-16 21:55 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Andrew Morton, Hugh Dickins, Andrea Arcangeli, Izik Eidus,
	Nick Piggin, linux-mm, linux-kernel

Johannes Weiner wrote:
> Make use of the compiler's typechecking on !CONFIG_SWAP as well.
> 
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

Reviewed-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 2/2] mm: remove task assumptions from swap token
  2009-06-16 21:50 ` [patch 2/2] mm: remove task assumptions from swap token Johannes Weiner
@ 2009-06-16 21:56   ` Rik van Riel
  2009-06-17  2:00   ` Minchan Kim
  1 sibling, 0 replies; 6+ messages in thread
From: Rik van Riel @ 2009-06-16 21:56 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Andrew Morton, Hugh Dickins, Andrea Arcangeli, Izik Eidus,
	Nick Piggin, linux-mm, linux-kernel

Johannes Weiner wrote:
> From: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> 
> grab_swap_token() should not make any assumptions about the running
> process as the swap token is an attribute of the address space and the
> faulting mm is not necessarily current->mm.
> 
> This fixes get_user_pages() from kernel threads which would blow up
> when encountering a swapped out page and grab_swap_token()
> dereferencing the unset for kernel threads current->mm.
> 
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

Reviewed-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 2/2] mm: remove task assumptions from swap token
  2009-06-16 21:50 ` [patch 2/2] mm: remove task assumptions from swap token Johannes Weiner
  2009-06-16 21:56   ` Rik van Riel
@ 2009-06-17  2:00   ` Minchan Kim
  2009-06-17  8:31     ` Johannes Weiner
  1 sibling, 1 reply; 6+ messages in thread
From: Minchan Kim @ 2009-06-17  2:00 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Andrew Morton, Hugh Dickins, Andrea Arcangeli, Izik Eidus,
	Rik van Riel, Nick Piggin, linux-mm, linux-kernel

Hi, Hannes. 

How about adding Hugh's comment ?

I think that is more straightforward and easy.
And it explained even real example like KSM. 

So I suggest following as.. 

==
grab_swap_token() should not make any assumptions about the running
process as the swap token is an attribute of the address space and the
faulting mm is not necessarily current->mm.

If a kthread happens to use get_user_pages() on an mm (as KSM does),
there's a chance that it will end up trying to read in a swap page,
then oops in grab_swap_token() because the kthread has no mm: GUP
passes down the right mm, so grab_swap_token() ought to be using it.
==

Anyway, It looks good to me. 
It might be just nitpick :)
If you feel it, ignore me. 
Anyway I am OK. 

On Tue, 16 Jun 2009 23:50:37 +0200
Johannes Weiner <hannes@cmpxchg.org> wrote:

> From: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> 
> grab_swap_token() should not make any assumptions about the running
> process as the swap token is an attribute of the address space and the
> faulting mm is not necessarily current->mm.
> 
> This fixes get_user_pages() from kernel threads which would blow up
> when encountering a swapped out page and grab_swap_token()
> dereferencing the unset for kernel threads current->mm.
> 
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

-- 
Kinds 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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 2/2] mm: remove task assumptions from swap token
  2009-06-17  2:00   ` Minchan Kim
@ 2009-06-17  8:31     ` Johannes Weiner
  0 siblings, 0 replies; 6+ messages in thread
From: Johannes Weiner @ 2009-06-17  8:31 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, Hugh Dickins, Andrea Arcangeli, Izik Eidus,
	Rik van Riel, Nick Piggin, linux-mm, linux-kernel

On Wed, Jun 17, 2009 at 11:00:34AM +0900, Minchan Kim wrote:
> Hi, Hannes. 
> 
> How about adding Hugh's comment ?

Sorry, I misinterpreted Hugh's reply to me that got into my inbox
directly and totally missed that he had already done the exact same
thing in the lkml thread until much later.

We should probably just use his version.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-06-17  8:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <Pine.LNX.4.64.0906162152250.12770@sister.anvils>
2009-06-16 21:50 ` [patch 1/2] mm: make swap token dummies static inlines Johannes Weiner
2009-06-16 21:55   ` Rik van Riel
2009-06-16 21:50 ` [patch 2/2] mm: remove task assumptions from swap token Johannes Weiner
2009-06-16 21:56   ` Rik van Riel
2009-06-17  2:00   ` Minchan Kim
2009-06-17  8:31     ` Johannes Weiner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox