[PATCH] stop swapoff 1/3 vm_enough_memory?

[PATCH] stop swapoff 1/3 vm_enough_memory?

[Hugh Dickins]

First of three small "stop swapoff" patches based on 2.5.67-mm3:

 include/linux/sched.h |    1 +
 mm/oom_kill.c         |    2 ++
 mm/swapfile.c         |   21 +++++++++++++++++----
 3 files changed, 20 insertions(+), 4 deletions(-)

stop swapoff 1/3 vm_enough_memory?

Before embarking upon swapoff, check vm_enough_memory.  Mainly
for consistency in the overcommit_memory 2 (strict accounting) case:
fail with -ENOMEM if it wouldn't let the amount removed be committed.

Will always succeed in the overcommit_memory 1 case, as it should in
root-shoot-foot mode.  In the overcommit_memory 0 case, well, I don't
care much either way, so opted for the simplest code: no special case.
Which means it could now fail at the start; but that's unlikely (case 0
is over-generous) and only when it would have got stuck later on anyway.

--- 2.5.67-mm3/mm/swapfile.c	Mon Apr 14 13:05:36 2003
+++ swapoff1/mm/swapfile.c	Thu Apr 17 18:32:40 2003
@@ -7,6 +7,7 @@
 
 #include <linux/config.h>
 #include <linux/mm.h>
+#include <linux/mman.h>
 #include <linux/slab.h>
 #include <linux/kernel_stat.h>
 #include <linux/swap.h>
@@ -1030,12 +1031,18 @@
 		}
 		prev = type;
 	}
-	err = -EINVAL;
 	if (type < 0) {
+		err = -EINVAL;
+		swap_list_unlock();
+		goto out_dput;
+	}
+	if (vm_enough_memory(p->pages))
+		vm_unacct_memory(p->pages);
+	else {
+		err = -ENOMEM;
 		swap_list_unlock();
 		goto out_dput;
 	}
-
 	if (prev < 0) {
 		swap_list.head = p->next;
 	} else {

[PATCH] stop swapoff 2/3 EINTR

[Hugh Dickins]

Sometimes you start a swapoff and, seeing how long it takes,
wish you had not: allow signal to interrupt and stop swapoff.

--- swapoff1/mm/swapfile.c	Thu Apr 17 18:32:40 2003
+++ swapoff2/mm/swapfile.c	Thu Apr 17 18:32:50 2003
@@ -592,6 +592,11 @@
 	 * to swapoff for a while, then reappear - but that is rare.
 	 */
 	while ((i = find_next_to_unuse(si, i))) {
+		if (signal_pending(current)) {
+			retval = -EINTR;
+			break;
+		}
+
 		/* 
 		 * Get a page for the entry, using the existing swap
 		 * cache page if there is one.  Otherwise, get a clean
@@ -761,8 +766,7 @@
 
 		/*
 		 * Make sure that we aren't completely killing
-		 * interactive performance.  Interruptible check on
-		 * signal_pending() would be nice, but changes the spec?
+		 * interactive performance.
 		 */
 		cond_resched();
 	}

[PATCH] stop swapoff 3/3 OOMkill

[Hugh Dickins]

Current behaviour is that once swapoff has filled memory, other tasks
get OOMkilled one by one until it completes, or more likely hangs.
Better that swapoff be the first choice for OOMkill.

I feel a little ashamed of adding another PF_flag for this, and it
may be done in other ways; but I've found none compellingly elegant.
And it would be enough for sys_swapoff to return failure rather than
be killed (along with any other tasks sharing its mm); but the OOMkill
mechanism gets the job done, I don't think we need complicate it.

--- swapoff2/include/linux/sched.h	Mon Apr 14 13:05:34 2003
+++ swapoff3/include/linux/sched.h	Thu Apr 17 18:33:00 2003
@@ -477,6 +477,7 @@
 #define PF_FROZEN	0x00010000	/* frozen for system suspend */
 #define PF_FSTRANS	0x00020000	/* inside a filesystem transaction */
 #define PF_KSWAPD	0x00040000	/* I am kswapd */
+#define PF_SWAPOFF	0x00080000	/* I am in swapoff */
 
 #if CONFIG_SMP
 extern void set_cpus_allowed(task_t *p, unsigned long new_mask);
--- swapoff2/mm/oom_kill.c	Tue Mar 25 08:06:56 2003
+++ swapoff3/mm/oom_kill.c	Thu Apr 17 18:33:00 2003
@@ -129,6 +129,8 @@
 				chosen = p;
 				maxpoints = points;
 			}
+			if (p->flags & PF_SWAPOFF)
+				return p;
 		}
 	while_each_thread(g, p);
 	return chosen;
--- swapoff2/mm/swapfile.c	Thu Apr 17 18:32:50 2003
+++ swapoff3/mm/swapfile.c	Thu Apr 17 18:33:00 2003
@@ -1060,7 +1060,9 @@
 	total_swap_pages -= p->pages;
 	p->flags &= ~SWP_WRITEOK;
 	swap_list_unlock();
+	current->flags |= PF_SWAPOFF;
 	err = try_to_unuse(type);
+	current->flags &= ~PF_SWAPOFF;
 	if (err) {
 		/* re-insert swap space back into swap_list */
 		swap_list_lock();

Re: [PATCH] stop swapoff 3/3 OOMkill

[Andrew Morton]

Hugh Dickins <hugh@veritas.com> wrote:
>
> Current behaviour is that once swapoff has filled memory, other tasks
> get OOMkilled one by one until it completes, or more likely hangs.
> Better that swapoff be the first choice for OOMkill.

This calls for __GFP_NORETRY.  It will disable the oom-kill and the infinite
retry in the page allocator.  So we will have:

__GFP_REPEAT:

	retry the allocation, but the caller can handle a failure.
	eg: pte_alloc_one().

	__GFP_REPEAT _may_ end up returning NULL.  It depends on the VM
	implemention - eg it would in -aa kernels.  

__GFP_NOFAIL:

	retry the allocation inifinitely, regardless of the VM implementation. 
	For jbd_kmalloc() and others.

__GFP_NORETRY:

	Don't oom-kill and don't retry.   For swapoff.

I've implemented a __GFP_REPEAT, and don't like it, because it blurs the
__GFP_REPEAT and __GFP_NOFAIL requirements.  I'll add __GFP_NORETRY and we
can then pass that into read_swap_cache_async() and handle the error.

Sound good?

Re: [PATCH] stop swapoff 3/3 OOMkill

[Hugh Dickins]

On Thu, 17 Apr 2003, Andrew Morton wrote:
> 
> __GFP_NORETRY:
> 
> 	Don't oom-kill and don't retry.   For swapoff.
> 
> I've implemented a __GFP_REPEAT, and don't like it, because it blurs the
> __GFP_REPEAT and __GFP_NOFAIL requirements.  I'll add __GFP_NORETRY and we
> can then pass that into read_swap_cache_async() and handle the error.
> 
> Sound good?

Probably not.  I did try something like that over a year ago, and it
didn't work as well as I'd expected.  One problem, I think, is that
while it's going through pages already in the swap cache, swapoff
may not need to allocate memory itself; but meanwhile other tasks
are trying to allocate memory and getting OOMkilled.  I think that
argues that swapoff does need to register itself for the duration
with the OOMkiller: a PF_ flag achieves that but a __GFP flag does not.

Hugh