lkml.org 
[lkml]   [1998]   [May]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subjectpatch for 2.1.102 swap code
Hi Stephen,

I've been reviewing the swap code and spotted a couple of prolems, which
the attached patch should take care of.

In read_swap_cache there was a race condition that could lead to pages
being left locked if the swap entry becomes unused while a process is
trying to read it. The low level rw_read_swap() routine bails out if the
entry isn't in use at the time, which would leave the page locked. The
patch avoids this problem by calling swap_duplicate() before checking
for the page.

In try_to_unuse_page there were some problems with swap counts still
non-zero after replacing all of the process references to a page,
apparently due to the swap map count being elevated while swapping is in
progress. (It shows up if a swapoff command is run while the system is
swapping heavily.) I've modified the code to make multiple passes in the
event that pages are still in use, and to report EBUSY if the counts
can't all be cleared.

In rw_swap_page there was an atomic_dec() of the page count after a sync
read or write, and I think it's possible that the page could have become
unused while waiting for the I/O operation. I've changed the atomic_dec
into a free_page() and added a printk to show whether it actually occurs
in practice.

The patch also includes a few minor cleanups for unused code and
prototypes in swap.h. I've tested it here and it seems to work OK, but
would like some further testing. Also, it probably won't help with the
"found a writable swap page" message reported recently; I'm continuing
to look for that problem.

Regards,
Bill--- linux-2.1.102/include/linux/swap.h.old Tue May 5 11:22:03 1998
+++ linux-2.1.102/include/linux/swap.h Thu May 21 09:30:24 1998
@@ -68,6 +68,12 @@
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;
@@ -75,6 +81,8 @@
void si_swapinfo(struct sysinfo *);
unsigned long get_swap_page(void);
extern void FASTCALL(swap_free(unsigned long));
+int sys_swapoff(const char *);
+int sys_swapon(const char *, int);

/*
* vm_ops not present page codes for shared memory.
@@ -137,14 +145,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.102/mm/swap_state.c.old Sun Mar 22 11:30:40 1998
+++ linux-2.1.102/mm/swap_state.c Thu May 21 09:16:13 1998
@@ -165,23 +165,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
@@ -254,40 +237,47 @@
* 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.
+ *
+ * The swap map count is duplicated in advance so that the entry can't
+ * be unused if we block for memory. This is necessary to avoid false
+ * failure returns from this routine and other unpleasantries.
*/

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:
+ /*
+ * Lock down the entry so it can't disappear ...
+ */
+ swap_duplicate(entry); /* Account for 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;

/* 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 */
- }
+ 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 ... we might have stalled.
+ */
+ found_page = lookup_swap_cache(entry);
+ if (found_page)
+ goto out_free;

- if (!add_to_swap_cache(new_page, entry)) {
- free_page(new_page_addr);
- return 0;
- }
- swap_duplicate(entry); /* Account for the swap cache */
+ if (!add_to_swap_cache(new_page, entry))
+ goto out_free;
+
set_bit(PG_locked, &new_page->flags);
rw_swap_page(READ, entry, (char *) new_page_addr, wait);
#ifdef DEBUG_SWAP
@@ -296,5 +286,12 @@
entry, (char *) page_address(new_page));
#endif
return new_page;
+
+out_free:
+ __free_page(new_page);
+out:
+ /* We didn't use the entry ... */
+ swap_free(entry);
+ return found_page;
}

--- linux-2.1.102/mm/swapfile.c.old Sun May 17 12:19:41 1998
+++ linux-2.1.102/mm/swapfile.c Thu May 21 12:51:07 1998
@@ -302,23 +302,21 @@
{
struct swap_info_struct * si = &swap_info[type];
struct task_struct *p;
- unsigned long page = 0;
struct page *page_map;
+ unsigned long page;
unsigned long entry;
- int i;
+ int i, repeat, tries = 5, error = 0;

- while (1) {
+repeat:
+ repeat = 0;
+ for (i = 1; i < si->max ; i++) {
+ int count = si->swap_map[i];
/*
- * Find a swap page in use and read it in.
+ * Look for a swap page that's still in use.
*/
- 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;
- }
- }
- if (!entry)
- break;
+ if (count == 0 || count == 0x80)
+ continue;
+ 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
@@ -335,16 +333,33 @@
page we've been using. */
if (PageSwapCache(page_map))
delete_from_swap_cache(page_map);
- free_page(page);
+ __free_page(page_map);
if (si->swap_map[i] != 0) {
- if (si->swap_map[i] != 127)
- printk("try_to_unuse: entry %08lx "
- "not in use\n", entry);
- si->swap_map[i] = 0;
- nr_swap_pages++;
+ repeat = 1;
+ /*
+ * On the last pass, clear any overflowed counts,
+ * but return EBUSY if swap is still in use.
+ */
+ if (!tries) {
+ if (si->swap_map[i] == 127) {
+ printk(KERN_WARNING
+ "VM: cleared overflow for "
+ "entry=%08lx\n", entry);
+ si->swap_map[i] = 0;
+ nr_swap_pages++;
+ } else {
+ printk(KERN_ERR
+ "try_to_unuse: entry=%08lx, "
+ "count=%d??\n",
+ entry, si->swap_map[i]);
+ error = -EBUSY;
+ }
+ }
}
}
- return 0;
+ if (repeat && tries--)
+ goto repeat;
+ return error;
}

asmlinkage int sys_swapoff(const char * specialfile)
@@ -380,10 +395,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 {
@@ -396,7 +410,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)
@@ -407,7 +420,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));
@@ -422,9 +435,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;
@@ -432,6 +445,9 @@
p->swap_lockmap = NULL;
p->flags = 0;
err = 0;
+
+out_dput:
+ dput(dentry);
out:
unlock_kernel();
return err;
--- linux-2.1.102/mm/page_io.c.old Tue May 5 11:22:03 1998
+++ linux-2.1.102/mm/page_io.c Thu May 21 10:38:39 1998
@@ -74,18 +74,18 @@
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;
}

if (!PageLocked(page)) {
- printk("VM: swap page is unlocked\n");
+ printk("rw_swap_page: swap page is unlocked\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:42    [from the cache]
©2003-2011 Jasper Spaans