]> git.hungrycats.org Git - linux/commitdiff
[PATCH] combine generic_writepages() and mpage_writepages()
authorAndrew Morton <akpm@zip.com.au>
Thu, 4 Jul 2002 15:31:40 +0000 (08:31 -0700)
committerLinus Torvalds <torvalds@home.transmeta.com>
Thu, 4 Jul 2002 15:31:40 +0000 (08:31 -0700)
generic_writepages and mpage_writepages are basically identical,
except one calls ->writepage() and the other calls mpage_writepage().
This duplication is irritating.

The patch folds generic_writepage() into mpage_writepages().  It does
this rather kludgily: if the get_block argument to mpage_writepages()
is NULL then use ->writepage().

Can't think of a better way, really - we could go for a fully-blown
write_actor_t thing, but that would be overly elaborate and would not
allow mpage_writepage() to be inlined inside mpage_writepages(), which
is rather desirable.

fs/mpage.c
mm/page-writeback.c

index 3dfe1187e877283a23a453bc2edec036689f8dc5..82a9ab89af331346355568248f8df08b3c860259 100644 (file)
@@ -475,9 +475,44 @@ out:
        return bio;
 }
 
-/*
- * This is a cut-n-paste of generic_writepages().  We _could_
- * generalise that function.  It'd get a bit messy.  We'll see.
+/**
+ * mpage_writepages - walk the list of dirty pages of the given
+ * address space and writepage() all of them.
+ * 
+ * @mapping: address space structure to write
+ * @nr_to_write: subtract the number of written pages from *@nr_to_write
+ * @get_block: the filesystem's block mapper function.
+ *             If this is NULL then use a_ops->writepage.  Otherwise, go
+ *             direct-to-BIO.
+ *
+ * This is a library function, which implements the writepages()
+ * address_space_operation.
+ *
+ * (The next two paragraphs refer to code which isn't here yet, but they
+ *  explain the presence of address_space.io_pages)
+ *
+ * Pages can be moved from clean_pages or locked_pages onto dirty_pages
+ * at any time - it's not possible to lock against that.  So pages which
+ * have already been added to a BIO may magically reappear on the dirty_pages
+ * list.  And generic_writepages() will again try to lock those pages.
+ * But I/O has not yet been started against the page.  Thus deadlock.
+ *
+ * To avoid this, the entire contents of the dirty_pages list are moved
+ * onto io_pages up-front.  We then walk io_pages, locking the
+ * pages and submitting them for I/O, moving them to locked_pages.
+ *
+ * This has the added benefit of preventing a livelock which would otherwise
+ * occur if pages are being dirtied faster than we can write them out.
+ *
+ * If a page is already under I/O, generic_writepages() skips it, even
+ * if it's dirty.  This is desirable behaviour for memory-cleaning writeback,
+ * but it is INCORRECT for data-integrity system calls such as fsync().  fsync()
+ * and msync() need to guarentee that all the data which was dirty at the time
+ * the call was made get new I/O started against them.  The way to do this is
+ * to run filemap_fdatawait() before calling filemap_fdatawrite().
+ *
+ * It's fairly rare for PageWriteback pages to be on ->dirty_pages.  It
+ * means that someone redirtied the page while it was under I/O.
  */
 int
 mpage_writepages(struct address_space *mapping,
@@ -487,6 +522,11 @@ mpage_writepages(struct address_space *mapping,
        sector_t last_block_in_bio = 0;
        int ret = 0;
        int done = 0;
+       int (*writepage)(struct page *);
+
+       writepage = NULL;
+       if (get_block == NULL)
+               writepage = mapping->a_ops->writepage;
 
        write_lock(&mapping->page_lock);
 
@@ -526,8 +566,13 @@ mpage_writepages(struct address_space *mapping,
                                }
                                spin_unlock(&pagemap_lru_lock);
                        }
-                       bio = mpage_writepage(bio, page, get_block,
-                                       &last_block_in_bio, &ret);
+
+                       if (writepage) {
+                               ret = (*writepage)(page);
+                       } else {
+                               bio = mpage_writepage(bio, page, get_block,
+                                               &last_block_in_bio, &ret);
+                       }
                        if (ret || (nr_to_write && --(*nr_to_write) <= 0))
                                done = 1;
                } else {
index b6dd75000b021d789890f5573b231a33f8174496..415b57658aa2d6896a4223b0fe2c62581e47e1b4 100644 (file)
@@ -19,8 +19,9 @@
 #include <linux/pagemap.h>
 #include <linux/writeback.h>
 #include <linux/init.h>
-#include <linux/sysrq.h>
+//#include <linux/sysrq.h>
 #include <linux/backing-dev.h>
+#include <linux/mpage.h>
 
 /*
  * The maximum number of pages to writeout in a single bdflush/kupdate
@@ -313,108 +314,9 @@ int generic_vm_writeback(struct page *page, int *nr_to_write)
 }
 EXPORT_SYMBOL(generic_vm_writeback);
 
-/**
- * generic_writepages - walk the list of dirty pages of the given
- * address space and writepage() all of them.
- * 
- * @mapping: address space structure to write
- * @nr_to_write: subtract the number of written pages from *@nr_to_write
- *
- * This is a library function, which implements the writepages()
- * address_space_operation.
- *
- * (The next two paragraphs refer to code which isn't here yet, but they
- *  explain the presence of address_space.io_pages)
- *
- * Pages can be moved from clean_pages or locked_pages onto dirty_pages
- * at any time - it's not possible to lock against that.  So pages which
- * have already been added to a BIO may magically reappear on the dirty_pages
- * list.  And generic_writepages() will again try to lock those pages.
- * But I/O has not yet been started against the page.  Thus deadlock.
- *
- * To avoid this, the entire contents of the dirty_pages list are moved
- * onto io_pages up-front.  We then walk io_pages, locking the
- * pages and submitting them for I/O, moving them to locked_pages.
- *
- * This has the added benefit of preventing a livelock which would otherwise
- * occur if pages are being dirtied faster than we can write them out.
- *
- * If a page is already under I/O, generic_writepages() skips it, even
- * if it's dirty.  This is desirable behaviour for memory-cleaning writeback,
- * but it is INCORRECT for data-integrity system calls such as fsync().  fsync()
- * and msync() need to guarentee that all the data which was dirty at the time
- * the call was made get new I/O started against them.  The way to do this is
- * to run filemap_fdatawait() before calling filemap_fdatawrite().
- *
- * It's fairly rare for PageWriteback pages to be on ->dirty_pages.  It
- * means that someone redirtied the page while it was under I/O.
- */
 int generic_writepages(struct address_space *mapping, int *nr_to_write)
 {
-       int (*writepage)(struct page *) = mapping->a_ops->writepage;
-       int ret = 0;
-       int done = 0;
-       int err;
-
-       write_lock(&mapping->page_lock);
-
-       list_splice(&mapping->dirty_pages, &mapping->io_pages);
-       INIT_LIST_HEAD(&mapping->dirty_pages);
-
-        while (!list_empty(&mapping->io_pages) && !done) {
-               struct page *page = list_entry(mapping->io_pages.prev,
-                                       struct page, list);
-               list_del(&page->list);
-               if (PageWriteback(page)) {
-                       if (PageDirty(page)) {
-                               list_add(&page->list, &mapping->dirty_pages);
-                               continue;
-                       }
-                       list_add(&page->list, &mapping->locked_pages);
-                       continue;
-               }
-               if (!PageDirty(page)) {
-                       list_add(&page->list, &mapping->clean_pages);
-                       continue;
-               }
-               list_add(&page->list, &mapping->locked_pages);
-               page_cache_get(page);
-               write_unlock(&mapping->page_lock);
-               lock_page(page);
-
-               /* It may have been removed from swapcache: check ->mapping */
-               if (page->mapping && !PageWriteback(page) &&
-                                       TestClearPageDirty(page)) {
-                       /* FIXME: batch this up */
-                       if (!PageActive(page) && PageLRU(page)) {
-                               spin_lock(&pagemap_lru_lock);
-                               if (!PageActive(page) && PageLRU(page)) {
-                                       list_del(&page->lru);
-                                       list_add(&page->lru, &inactive_list);
-                               }
-                               spin_unlock(&pagemap_lru_lock);
-                       }
-                       err = writepage(page);
-                       if (!ret)
-                               ret = err;
-                       if (nr_to_write && --(*nr_to_write) <= 0)
-                               done = 1;
-               } else {
-                       unlock_page(page);
-               }
-
-               page_cache_release(page);
-               write_lock(&mapping->page_lock);
-       }
-       if (!list_empty(&mapping->io_pages)) {
-               /*
-                * Put the rest back, in the correct order.
-                */
-               list_splice(&mapping->io_pages, mapping->dirty_pages.prev);
-               INIT_LIST_HEAD(&mapping->io_pages);
-       }
-       write_unlock(&mapping->page_lock);
-       return ret;
+       return mpage_writepages(mapping, nr_to_write, NULL);
 }
 EXPORT_SYMBOL(generic_writepages);