Re: [PATCH] mm/memory.c, 2.4.1 : memory leak with swap cache (updated)

Richard Jerrell
2001-03-27T22:52:57Z

[-- Attachment #1: Type: TEXT/PLAIN, Size: 718 bytes --]

> 1. we take an extra reference on the page, how does that
> affect the test for if the page is shared or not ?

is_page_shared expects us to have our own reference to the page.

> 2. we call delete_from_swap_cache with the pagemap_lru_lock
> held, since this tries to grab the pagecache_lock we can
> easily deadlock with the rest of the kernel (where the
> locking order is opposite)

You're right. Oversight on my part. Here is another version of the
patch.

> 3. there are no comments in the code explaining what this
> suspicious-looking piece of code does ;)

Oops... I sent out the wrong version of the patch the first time. This
one has comments, promise. And it has one less bug. :)

Rich

[-- Attachment #2: Type: TEXT/PLAIN, Size: 3485 bytes --]

diff -rNu linux-2.4.1/include/linux/swap.h linux-2.4.1-paging-fix/include/linux/swap.h
--- linux-2.4.1/include/linux/swap.h Tue Jan 30 02:24:56 2001
+++ linux-2.4.1-paging-fix/include/linux/swap.h Tue Mar 27 14:00:17 2001
@@ -69,6 +69,7 @@
FASTCALL(unsigned int nr_free_buffer_pages(void));
extern int nr_active_pages;
extern int nr_inactive_dirty_pages;
+extern int nr_swap_cache_pages;
extern atomic_t nr_async_pages;
extern struct address_space swapper_space;
extern atomic_t page_cache_size;
diff -rNu linux-2.4.1/mm/mmap.c linux-2.4.1-paging-fix/mm/mmap.c
--- linux-2.4.1/mm/mmap.c Mon Jan 29 11:10:41 2001
+++ linux-2.4.1-paging-fix/mm/mmap.c Tue Mar 27 10:38:17 2001
@@ -63,6 +63,7 @@
free += atomic_read(&page_cache_size);
free += nr_free_pages();
free += nr_swap_pages;
+ free += nr_swap_cache_pages;
return free > pages;
}

diff -rNu linux-2.4.1/mm/swap_state.c linux-2.4.1-paging-fix/mm/swap_state.c
--- linux-2.4.1/mm/swap_state.c Fri Dec 29 18:04:27 2000
+++ linux-2.4.1-paging-fix/mm/swap_state.c Tue Mar 27 10:41:04 2001
@@ -17,6 +17,8 @@

#include <asm/pgtable.h>

+int nr_swap_cache_pages = 0;
+
static int swap_writepage(struct page *page)
{
rw_swap_page(WRITE, page, 0);
@@ -58,6 +60,7 @@
#ifdef SWAP_CACHE_INFO
swap_cache_add_total++;
#endif
+ nr_swap_cache_pages++;
if (!PageLocked(page))
BUG();
if (PageTestandSetSwapCache(page))
@@ -96,6 +99,7 @@
#ifdef SWAP_CACHE_INFO
swap_cache_del_total++;
#endif
+ nr_swap_cache_pages--;
remove_from_swap_cache(page);
swap_free(entry);
}
diff -rNu linux-2.4.1/mm/vmscan.c linux-2.4.1-paging-fix/mm/vmscan.c
--- linux-2.4.1/mm/vmscan.c Mon Jan 15 15:36:49 2001
+++ linux-2.4.1-paging-fix/mm/vmscan.c Tue Mar 27 14:37:18 2001
@@ -394,6 +394,21 @@
return page;
}

+/**
+ * Short-circuits the dead swap cache page to prevent
+ * unnecessary disk IO
+ */
+static inline void delete_dead_swap_cache_page(struct page *page) {
+ if (block_flushpage(page, 0))
+ lru_cache_del(page);
+
+ ClearPageDirty(page);
+ spin_unlock(&pagemap_lru_lock);
+ __delete_from_swap_cache(page);
+ spin_lock(&pagemap_lru_lock);
+ page_cache_release(page);
+}
+
/**
* page_launder - clean dirty inactive pages, move to inactive_clean list
* @gfp_mask: what operations we are allowed to do
@@ -467,6 +482,24 @@
}

/*
+ * Prevent a dead process's pages from being
+ * written to swap when no one else needs
+ * them. Lazy form of removing these pages
+ * at exit time.
+ */
+ if (PageSwapCache(page)) {
+ page_cache_get(page);
+ if(!is_page_shared(page)) {
+ delete_dead_swap_cache_page(page);
+
+ UnlockPage(page);
+ page_cache_release(page);
+ continue;
+ }
+ page_cache_release(page);
+ }
+
+ /*
* Dirty swap-cache page? Write it out if
* last copy..
*/
@@ -650,6 +683,24 @@
list_del(page_lru);
nr_active_pages--;
continue;
+ }
+
+ /*
+ * Prevent a dead process's pages from being
+ * written to swap when no one else needs
+ * them. Lazy form of removing these pages
+ * at exit time.
+ */
+ if (PageSwapCache(page)) {
+ page_cache_get(page);
+ if(!is_page_shared(page) && !TryLockPage(page)) {
+ delete_dead_swap_cache_page(page);
+
+ UnlockPage(page);
+ page_cache_release(page);
+ continue;
+ }
+ page_cache_release(page);
}

/* Do aging on the pages. */