[Patch] swapfile.c

[Patch] swapfile.c

[Nathan D. Fabian]

[-- Attachment #1: Type: text/plain, Size: 307 bytes --]

The following diff tries to improve on the efficiency of try_to_unuse().  It 
removes the potential O(|swap_map|^2) business and makes it linear time.  
I'm not sure what this means in terms of overall change, but Linus seemed 
interested in the innefficiency in that code.  Test with caution.

Nathan.

[-- Attachment #2: swapfile.diff --]
[-- Type: text/x-c, Size: 3018 bytes --]

diff -uwrN linux/mm/swapfile.c.old linux/mm/swapfile.c
--- linux/mm/swapfile.c.old	Tue Jun 19 17:15:07 2001
+++ linux/mm/swapfile.c	Tue Jun 19 17:44:24 2001
@@ -328,6 +328,41 @@
 	return;
 }
 
+static int unuse_entry (swp_entry_t entry) {
+	struct page *page;
+	struct task_struct *p;
+
+	/* Get a page for the entry, using the existing swap
+       	cache page if there is one.  Otherwise, get a clean
+       	page and read the swap into it. */
+	page = read_swap_cache_async(entry);
+	if (!page) {
+		swap_free(entry);
+		return -ENOMEM;
+	}
+
+	lock_page (page);
+	if (PageSwapCache(page))
+		delete_from_swap_cache_nolock(page);
+	UnlockPage (page);
+
+	read_lock(&tasklist_lock);
+	for_each_task(p)
+		unuse_process(p->mm, entry, page);
+	read_unlock(&tasklist_lock);
+
+	shmem_unuse(entry, page);
+	/* Now get rid of the extra reference to the temporary
+                 	page we've been using. */
+	page_cache_release(page);
+	/*
+ 	* Check for and clear any overflowed swap map counts.
+ 	*/
+	swap_free(entry);
+
+	return 0;
+}
+
 /*
  * We completely avoid races by reading each swap page in advance,
  * and then search for the process using it.  All the necessary
@@ -336,12 +371,10 @@
 static int try_to_unuse(unsigned int type)
 {
 	struct swap_info_struct * si = &swap_info[type];
-	struct task_struct *p;
-	struct page *page;
 	swp_entry_t entry;
 	int i;
+	int err;
 
-	while (1) {
 		/*
 		 * Find a swap page in use and read it in.
 		 */
@@ -356,42 +389,19 @@
 				 */
 				if (si->swap_map[i] != SWAP_MAP_MAX)
 					si->swap_map[i]++;
+
 				swap_device_unlock(si);
-				goto found_entry;
-			}
-		}
-		swap_device_unlock(si);
-		break;
 
-	found_entry:
 		entry = SWP_ENTRY(type, i);
-
-		/* Get a page for the entry, using the existing swap
-                   cache page if there is one.  Otherwise, get a clean
-                   page and read the swap into it. */
-		page = read_swap_cache_async(entry);
-		if (!page) {
-			swap_free(entry);
-  			return -ENOMEM;
+			err = unuse_entry (entry);
+			if (err < 0) {
+				return err;
 		}
-		lock_page(page);
-		if (PageSwapCache(page))
-			delete_from_swap_cache_nolock(page);
-		UnlockPage(page);
-		read_lock(&tasklist_lock);
-		for_each_task(p)
-			unuse_process(p->mm, entry, page);
-		read_unlock(&tasklist_lock);
-		shmem_unuse(entry, page);
-		/* Now get rid of the extra reference to the temporary
-                   page we've been using. */
-		page_cache_release(page);
-		/*
-		 * Check for and clear any overflowed swap map counts.
-		 */
-		swap_free(entry);
-		swap_list_lock();
+
 		swap_device_lock(si);
+
+			swap_list_lock();
+			/* check this value again after unusing the entry */
 		if (si->swap_map[i] > 0) {
 			if (si->swap_map[i] != SWAP_MAP_MAX)
 				printk("VM: Undead swap entry %08lx\n", 
@@ -399,9 +409,13 @@
 			nr_swap_pages++;
 			si->swap_map[i] = 0;
 		}
-		swap_device_unlock(si);
 		swap_list_unlock();
+
+			/* Leave the device locked for loop */
+		}
 	}
+	swap_device_unlock (si);
+
 	return 0;
 }
 

Re: [Patch] swapfile.c

[Nathan D. Fabian]

On Tuesday 19 June 2001 06:24 pm, Nathan D. Fabian wrote:
> The following diff tries to improve on the efficiency of try_to_unuse(). 
> It removes the potential O(|swap_map|^2) business and makes it linear time.
> I'm not sure what this means in terms of overall change, but Linus seemed
> interested in the innefficiency in that code.  Test with caution.

I suppose I should mention that this is against a vanilla 2.4.5 kernel, but 
should patches cleanly against 2.4.5-ac16.

[PATCH] swapfile.c

[Andries.Brouwer]

In 2.5.2 swapfile.c was broken:
In sys_swapon() we see

	swap_file = filp_open(name, O_RDWR, 0);
	if (IS_ERR(swap_file))
		goto bad_swap_2;

bad_swap_2:
	...
	if (swap_file)
		filp_close(swap_file, NULL);

and this oopses the kernel.
Below a trivial fix. Somebody with more time may come
back and polish stuff a little.

Andries

--- swapfile.c~	Sat Mar  2 18:23:19 2002
+++ swapfile.c	Sun Mar  3 23:08:48 2002
@@ -905,8 +905,10 @@
 	swap_file = filp_open(name, O_RDWR, 0);
 	putname(name);
 	error = PTR_ERR(swap_file);
-	if (IS_ERR(swap_file))
+	if (IS_ERR(swap_file)) {
+		swap_file = NULL;
 		goto bad_swap_2;
+	}
 
 	p->swap_file = swap_file;
 

[this was a patch relative to 2.5.6-pre2]

Re: [PATCH] swapfile.c

[Andrey Panin]

[-- Attachment #1: Type: text/plain, Size: 494 bytes --]

On Сбт, Мар 02, 2002 at 09:25:16 +0000, Andries.Brouwer@cwi.nl wrote:
> In 2.5.2 swapfile.c was broken:
> In sys_swapon() we see
> 
> 	swap_file = filp_open(name, O_RDWR, 0);
> 	if (IS_ERR(swap_file))
> 		goto bad_swap_2;
> 
> bad_swap_2:
> 	...
> 	if (swap_file)
> 		filp_close(swap_file, NULL);
> 
> and this oopses the kernel.

Fixed in -dj tree.

-- 
Andrey Panin            | Embedded systems software engineer
pazke@orbita1.ru        | PGP key: wwwkeys.eu.pgp.net

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: [PATCH] swapfile.c

[Robert Love]

On Mon, 2002-03-04 at 06:28, Andrey Panin wrote:
> On Сбт, Мар 02, 2002 at 09:25:16 +0000, Andries.Brouwer@cwi.nl wrote:
> > In 2.5.2 swapfile.c was broken:
> > In sys_swapon() we see
> > 
> > 	swap_file = filp_open(name, O_RDWR, 0);
> > 	if (IS_ERR(swap_file))
> > 		goto bad_swap_2;
> > 
> > bad_swap_2:
> > 	...
> > 	if (swap_file)
> > 		filp_close(swap_file, NULL);
> > 
> > and this oopses the kernel.
> 
> Fixed in -dj tree.

Eww, nice spotting Andries.  If it is in the -dj tree, someone want to
push that bit to Linus?

	Robert Love

Re: [PATCH] swapfile.c

[Dave Jones]

On Mon, Mar 04, 2002 at 01:48:08PM -0500, Robert Love wrote:
 > > Fixed in -dj tree.
 > Eww, nice spotting Andries.  If it is in the -dj tree, someone want to
 > push that bit to Linus?

 Actually, I only just applied that to my tree last night.
 Possibly Andrey was mixing it up to the IS_ERR fix that went in
 a while ago..

-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs