lkml.org 
[lkml]   [1998]   [Jul]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subjectupdated patch for 2.1.108 swap problems
The attached patch is a simplification to a previously posted patch to
fix a race in read_swap_page, and includes a change to downgrade an
error message that was triggering a cascade of messages.

The message "VM: Removing page cache on unshared page" in
remove_from_swap_cache() could be triggered by the perfectly legal case
of removing an unshared page from the swap cache, and would result in a
cascade of messages as repeated attempts were made to remove the page.
Since the swap entry itself was being freed, this would result in
further messages about the swap map being bad. The patch changes this to
a warning and allows the page to be removed.

In swap_state.c I've added a swap_check_entry() routine to ensure that
the entry is still in use, and have changed swap_duplicate to return a
success indication if the entry is still valid. The lookup_swap_page()
routine calls swap_check_entry() before trying to find the page, which
allows the read_swap_page() to return failure if the swap entry has
become unused. Previously the code would fail ungracefully and leave a
page locked and an invalid swap cache page.

In try_to_ususe_page I've added a test following a failure of
read_swap_page() to continue searching if the swap entry is no longer in
use. The only other client of read_swap_page() is in swap_in, and it
already has a suitable test for the swap entry becoming unused.

In page_io.c the patch adds a warning in case the page count is unused
while swapping the page, and replaces an atomic_dec with a free_page()
for safety.

I've also removed some dead code from swap_state.c, and moved the
definition of the swapper_inode outside the SWAP_CACHE_INFO condtional.

Regards,
Bill--- linux-2.1.108/include/linux/swap.h.old Fri Jul 3 10:31:40 1998
+++ linux-2.1.108/include/linux/swap.h Thu Jul 9 11:54:57 1998
@@ -64,10 +64,17 @@
/* linux/mm/swap_state.c */
extern void show_swap_cache_info(void);
extern int add_to_swap_cache(struct page *, unsigned long);
-extern void swap_duplicate(unsigned long);
+extern int swap_duplicate(unsigned long);
+extern int swap_check_entry(unsigned long);
extern void swap_after_unlock_page (unsigned long entry);
extern struct page * read_swap_cache_async(unsigned long, int);
#define read_swap_cache(entry) read_swap_cache_async(entry, 1);
+/*
+ * Make these inline later once they are working properly.
+ */
+extern int delete_from_swap_cache(struct page *page);
+extern void remove_from_swap_cache(struct page *page);
+extern void free_page_and_swap_cache(unsigned long addr);

/* linux/mm/swapfile.c */
extern unsigned int nr_swapfiles;
@@ -80,6 +87,8 @@
int next; /* swapfile to be used next */
};
extern struct swap_list_t swap_list;
+int sys_swapoff(const char *);
+int sys_swapon(const char *, int);

/*
* vm_ops not present page codes for shared memory.
@@ -142,14 +151,6 @@
}
return retval;
}
-
-/*
- * Make these inline later once they are working properly.
- */
-extern long find_in_swap_cache(struct page *page);
-extern int delete_from_swap_cache(struct page *page);
-extern void remove_from_swap_cache(struct page *page);
-extern void free_page_and_swap_cache(unsigned long addr);

#endif /* __KERNEL__*/

--- linux-2.1.108/mm/swap_state.c.old Sun Mar 22 11:30:40 1998
+++ linux-2.1.108/mm/swap_state.c Thu Jul 9 12:15:07 1998
@@ -24,14 +24,6 @@
#include <asm/bitops.h>
#include <asm/pgtable.h>

-#ifdef SWAP_CACHE_INFO
-unsigned long swap_cache_add_total = 0;
-unsigned long swap_cache_add_success = 0;
-unsigned long swap_cache_del_total = 0;
-unsigned long swap_cache_del_success = 0;
-unsigned long swap_cache_find_total = 0;
-unsigned long swap_cache_find_success = 0;
-
/*
* Keep a reserved false inode which we will use to mark pages in the
* page cache are acting as swap cache instead of file cache.
@@ -43,6 +35,13 @@
*/
struct inode swapper_inode;

+#ifdef SWAP_CACHE_INFO
+unsigned long swap_cache_add_total = 0;
+unsigned long swap_cache_add_success = 0;
+unsigned long swap_cache_del_total = 0;
+unsigned long swap_cache_del_success = 0;
+unsigned long swap_cache_find_total = 0;
+unsigned long swap_cache_find_success = 0;

void show_swap_cache_info(void)
{
@@ -87,10 +86,11 @@
/*
* If swap_map[] reaches 127, the entries are treated as "permanent".
*/
-void swap_duplicate(unsigned long entry)
+int swap_duplicate(unsigned long entry)
{
struct swap_info_struct * p;
unsigned long offset, type;
+ int result = 0;

if (!entry)
goto out;
@@ -105,27 +105,32 @@
goto bad_offset;
if (!p->swap_map[offset])
goto bad_unused;
+ /*
+ * Entry is valid, so increment the map count.
+ */
if (p->swap_map[offset] < 126)
p->swap_map[offset]++;
else {
static int overflow = 0;
if (overflow++ < 5)
- printk("swap_duplicate: entry %08lx map count=%d\n",
+ printk(KERN_WARNING
+ "swap_duplicate: entry %08lx map count=%d\n",
entry, p->swap_map[offset]);
p->swap_map[offset] = 127;
}
+ result = 1;
#ifdef DEBUG_SWAP
printk("DebugVM: swap_duplicate(entry %08lx, count now %d)\n",
entry, p->swap_map[offset]);
#endif
out:
- return;
+ return result;

bad_file:
- printk("swap_duplicate: Trying to duplicate nonexistent swap-page\n");
+ printk("swap_duplicate: entry %08lx, nonexistent swap file\n", entry);
goto out;
bad_offset:
- printk("swap_duplicate: offset exceeds max\n");
+ printk("swap_duplicate: entry %08lx, offset exceeds max\n", entry);
goto out;
bad_unused:
printk("swap_duplicate at %8p: unused page\n",
@@ -133,6 +138,17 @@
goto out;
}

+/*
+ * Verify that a previously valid swap entry is still in use.
+ */
+int swap_check_entry(unsigned long entry)
+{
+ unsigned long type = SWP_TYPE(entry);
+ unsigned long offset = SWP_OFFSET(entry);
+ struct swap_info_struct * p = type + swap_info;
+
+ return (p->swap_map[offset] != 0);
+}

void remove_from_swap_cache(struct page *page)
{
@@ -146,12 +162,12 @@
"on page %08lx\n", page_address(page));
}
/*
- * This will be a legal case once we have a more mature swap cache.
+ * This case is legal, but warn about it for a while.
*/
if (atomic_read(&page->count) == 1) {
- printk ("VM: Removing page cache on unshared page %08lx\n",
+ printk (KERN_WARNING
+ "VM: Removing page cache on unshared page %08lx\n",
page_address(page));
- return;
}


@@ -165,23 +181,6 @@
__free_page (page);
}

-
-long find_in_swap_cache(struct page *page)
-{
-#ifdef SWAP_CACHE_INFO
- swap_cache_find_total++;
-#endif
- if (PageSwapCache (page)) {
- long entry = page->offset;
-#ifdef SWAP_CACHE_INFO
- swap_cache_find_success++;
-#endif
- remove_from_swap_cache (page);
- return entry;
- }
- return 0;
-}
-
int delete_from_swap_cache(struct page *page)
{
#ifdef SWAP_CACHE_INFO
@@ -234,60 +233,73 @@
struct page *found;

while (1) {
+ /* Make sure the swap entry is still in use */
+ if (!swap_check_entry(entry))
+ return 0;
found = find_page(&swapper_inode, entry);
if (!found)
return 0;
- if (found->inode != &swapper_inode
- || !PageSwapCache(found)) {
- __free_page(found);
- printk ("VM: Found a non-swapper swap page!\n");
- return 0;
- }
+ if (found->inode != &swapper_inode || !PageSwapCache(found))
+ goto out_bad;
if (!PageLocked(found))
return found;
__free_page(found);
__wait_on_page(found);
}
+
+out_bad:
+ printk ("VM: Found a non-swapper swap page!\n");
+ __free_page(found);
+ return 0;
}

/*
* Locate a page of swap in physical memory, reserving swap cache space
* and reading the disk if it is not already cached. If wait==0, we are
* only doing readahead, so don't worry if the page is already locked.
+ *
+ * A failure return means that either the page allocation failed or that
+ * the swap entry is no longer in use.
*/

struct page * read_swap_cache_async(unsigned long entry, int wait)
{
- struct page *found_page, *new_page = 0;
- unsigned long new_page_addr = 0;
+ struct page *found_page, *new_page;
+ unsigned long new_page_addr;

#ifdef DEBUG_SWAP
printk("DebugVM: read_swap_cache_async entry %08lx%s\n",
entry, wait ? ", wait" : "");
#endif
-repeat:
+ /*
+ * Look for the page in the swap cache.
+ */
found_page = lookup_swap_cache(entry);
- if (found_page) {
- if (new_page)
- __free_page(new_page);
- return found_page;
- }
+ if (found_page)
+ goto out;
+
+ new_page_addr = __get_free_page(GFP_KERNEL);
+ if (!new_page_addr)
+ goto out; /* Out of memory */
+ new_page = mem_map + MAP_NR(new_page_addr);
+
+ /*
+ * Check the swap cache again, in case we stalled above.
+ */
+ found_page = lookup_swap_cache(entry);
+ if (found_page)
+ goto out_free_page;
+ /*
+ * Make sure the swap entry is still in use.
+ */
+ if (!swap_duplicate(entry)) /* Account for the swap cache */
+ goto out_free_page;
+ /*
+ * Add it to the swap cache and read its contents.
+ */
+ if (!add_to_swap_cache(new_page, entry))
+ goto out_free_page;

- /* The entry is not present. Lock down a new page, add it to
- * the swap cache and read its contents. */
- if (!new_page) {
- new_page_addr = __get_free_page(GFP_KERNEL);
- if (!new_page_addr)
- return 0; /* Out of memory */
- new_page = mem_map + MAP_NR(new_page_addr);
- goto repeat; /* We might have stalled */
- }
-
- if (!add_to_swap_cache(new_page, entry)) {
- free_page(new_page_addr);
- return 0;
- }
- swap_duplicate(entry); /* Account for the swap cache */
set_bit(PG_locked, &new_page->flags);
rw_swap_page(READ, entry, (char *) new_page_addr, wait);
#ifdef DEBUG_SWAP
@@ -296,5 +308,9 @@
entry, (char *) page_address(new_page));
#endif
return new_page;
-}

+out_free_page:
+ __free_page(new_page);
+out:
+ return found_page;
+}
--- linux-2.1.108/mm/swapfile.c.old Fri Jul 3 10:32:34 1998
+++ linux-2.1.108/mm/swapfile.c Thu Jul 9 11:10:10 1998
@@ -299,30 +299,35 @@
{
struct swap_info_struct * si = &swap_info[type];
struct task_struct *p;
- unsigned long page = 0;
struct page *page_map;
- unsigned long entry;
+ unsigned long page, entry;
int i;

while (1) {
/*
* Find a swap page in use and read it in.
*/
- for (i = 1 , entry = 0; i < si->max ; i++) {
- if (si->swap_map[i] > 0 && si->swap_map[i] != 0x80) {
- entry = SWP_ENTRY(type, i);
- break;
- }
+ for (i = 1; i < si->max ; i++) {
+ if (si->swap_map[i] > 0 && si->swap_map[i] != 0x80)
+ goto found_entry;
}
- if (!entry)
- break;
+ 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_map = read_swap_cache(entry);
- if (!page_map)
+ if (!page_map) {
+ /*
+ * Continue searching if the entry became unused.
+ */
+ if (si->swap_map[i] == 0)
+ continue;
return -ENOMEM;
+ }
page = page_address(page_map);
read_lock(&tasklist_lock);
for_each_task(p)
@@ -332,11 +337,15 @@
page we've been using. */
if (PageSwapCache(page_map))
delete_from_swap_cache(page_map);
- free_page(page);
+ __free_page(page_map);
+ /*
+ * Check for and clear any overflowed swap map counts.
+ */
if (si->swap_map[i] != 0) {
if (si->swap_map[i] != 127)
- printk("try_to_unuse: entry %08lx "
- "not in use\n", entry);
+ printk(KERN_ERR
+ "try_to_unuse: entry %08lx count=%d\n",
+ entry, si->swap_map[i]);
si->swap_map[i] = 0;
nr_swap_pages++;
}
@@ -377,10 +386,9 @@
prev = type;
}
err = -EINVAL;
- if (type < 0){
- dput(dentry);
- goto out;
- }
+ if (type < 0)
+ goto out_dput;
+
if (prev < 0) {
swap_list.head = p->next;
} else {
@@ -393,7 +401,6 @@
p->flags = SWP_USED;
err = try_to_unuse(type);
if (err) {
- dput(dentry);
/* re-insert swap space back into swap_list */
for (prev = -1, i = swap_list.head; i >= 0; prev = i, i = swap_info[i].next)
if (p->prio >= swap_info[i].prio)
@@ -404,7 +411,7 @@
else
swap_info[prev].next = p - swap_info;
p->flags = SWP_WRITEOK;
- goto out;
+ goto out_dput;
}
if(p->swap_device){
memset(&filp, 0, sizeof(filp));
@@ -419,9 +426,9 @@
}
dput(dentry);

- nr_swap_pages -= p->pages;
- dput(p->swap_file);
+ dentry = p->swap_file;
p->swap_file = NULL;
+ nr_swap_pages -= p->pages;
p->swap_device = 0;
vfree(p->swap_map);
p->swap_map = NULL;
@@ -429,6 +436,9 @@
p->swap_lockmap = NULL;
p->flags = 0;
err = 0;
+
+out_dput:
+ dput(dentry);
out:
unlock_kernel();
return err;
--- linux-2.1.108/mm/page_io.c.old Fri Jul 3 10:32:34 1998
+++ linux-2.1.108/mm/page_io.c Thu Jul 9 12:21:32 1998
@@ -74,13 +74,13 @@
return;
}
if (p->swap_map && !p->swap_map[offset]) {
- printk("Hmm.. Trying to %s unallocated swap (%08lx)\n",
+ printk("rw_swap_page: Trying to %s unallocated swap (%08lx)\n",
(rw == READ) ? "read" : "write",
entry);
return;
}
if (!(p->flags & SWP_USED)) {
- printk("Trying to swap to unused swap-device\n");
+ printk("rw_swap_page: Trying to swap to unused swap-device\n");
return;
}

@@ -115,7 +115,7 @@
return;
}
if (page->offset != entry) {
- printk ("swap entry mismatch");
+ printk ("VM: swap entry mismatch\n");
return;
}

@@ -181,7 +181,12 @@
} else
printk("rw_swap_page: no swap file or device\n");

- atomic_dec(&page->count);
+#if 1 /* see how often this actually happens ... */
+ if (atomic_read(&page->count) == 1)
+ printk("rw_swap_page: page unused while waiting!\n");
+#endif
+ __free_page(page);
+
if (offset && !test_and_clear_bit(offset,p->swap_lockmap))
printk("rw_swap_page: lock already cleared\n");
wake_up(&lock_queue);
\
 
 \ /
  Last update: 2005-03-22 13:43    [W:0.055 / U:0.300 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site