[patch 10/13] remove add_to_page_cache_unique()

From: Andrew Morton (akpm@zip.com.au)
Date: Wed Jul 17 2002 - 00:30:10 EST


A tasty patch from Hugh Dickens. radix_tree_insert() fails if something
was already present at the target index, so that error can be
propagated back through add_to_page_cache(). Hence
add_to_page_cache_unique() is obsolete.

Hugh's patch removes add_to_page_cache_unique() and cleans up a bunch of
stuff.

 fs/mpage.c | 2
 include/linux/pagemap.h | 2
 mm/filemap.c | 182 +++++++++++++-----------------------------------
 mm/readahead.c | 2
 mm/swap_state.c | 3
 5 files changed, 53 insertions(+), 138 deletions(-)

--- 2.5.26/fs/mpage.c~hugh-add_to_page_cache Tue Jul 16 21:47:15 2002
+++ 2.5.26-akpm/fs/mpage.c Tue Jul 16 21:47:15 2002
@@ -268,7 +268,7 @@ mpage_readpages(struct address_space *ma
 
                 prefetchw(&page->flags);
                 list_del(&page->list);
- if (!add_to_page_cache_unique(page, mapping, page->index))
+ if (!add_to_page_cache(page, mapping, page->index))
                         bio = do_mpage_readpage(bio, page,
                                         nr_pages - page_idx,
                                         &last_block_in_bio, get_block);
--- 2.5.26/include/linux/pagemap.h~hugh-add_to_page_cache Tue Jul 16 21:47:15 2002
+++ 2.5.26-akpm/include/linux/pagemap.h Tue Jul 16 21:47:15 2002
@@ -52,8 +52,6 @@ extern struct page * read_cache_page(str
 
 extern int add_to_page_cache(struct page *page,
                 struct address_space *mapping, unsigned long index);
-extern int add_to_page_cache_unique(struct page *page,
- struct address_space *mapping, unsigned long index);
 
 static inline void ___add_to_page_cache(struct page *page,
                 struct address_space *mapping, unsigned long index)
--- 2.5.26/mm/filemap.c~hugh-add_to_page_cache Tue Jul 16 21:47:15 2002
+++ 2.5.26-akpm/mm/filemap.c Tue Jul 16 21:59:35 2002
@@ -520,8 +520,6 @@ int filemap_fdatawait(struct address_spa
  * This adds a page to the page cache, starting out as locked, unreferenced,
  * not uptodate and with no errors.
  *
- * The caller must hold a write_lock on mapping->page_lock.
- *
  * This function is used for two things: adding newly allocated pagecache
  * pages and for moving existing anon pages into swapcache.
  *
@@ -533,44 +531,20 @@ int filemap_fdatawait(struct address_spa
  * SetPageLocked() is ugly-but-OK there too. The required page state has been
  * set up by swap_out_add_to_swap_cache().
  */
-static int __add_to_page_cache(struct page *page,
+int add_to_page_cache(struct page *page,
                 struct address_space *mapping, unsigned long offset)
 {
- if (radix_tree_insert(&mapping->page_tree, offset, page) == 0) {
+ int error;
+
+ write_lock(&mapping->page_lock);
+ error = radix_tree_insert(&mapping->page_tree, offset, page);
+ if (!error) {
                 SetPageLocked(page);
                 ClearPageDirty(page);
                 ___add_to_page_cache(page, mapping, offset);
                 page_cache_get(page);
- return 0;
         }
- return -ENOMEM;
-}
-
-int add_to_page_cache(struct page *page,
- struct address_space *mapping, unsigned long offset)
-{
- write_lock(&mapping->page_lock);
- if (__add_to_page_cache(page, mapping, offset) < 0)
- goto nomem;
         write_unlock(&mapping->page_lock);
- lru_cache_add(page);
- return 0;
-nomem:
- write_unlock(&mapping->page_lock);
- return -ENOMEM;
-}
-
-int add_to_page_cache_unique(struct page *page,
- struct address_space *mapping, unsigned long offset)
-{
- struct page *alias;
- int error = -EEXIST;
-
- write_lock(&mapping->page_lock);
- if (!(alias = radix_tree_lookup(&mapping->page_tree, offset)))
- error = __add_to_page_cache(page, mapping, offset);
- write_unlock(&mapping->page_lock);
-
         if (!error)
                 lru_cache_add(page);
         return error;
@@ -587,17 +561,11 @@ static int page_cache_read(struct file *
         struct page *page;
         int error;
 
- read_lock(&mapping->page_lock);
- page = radix_tree_lookup(&mapping->page_tree, offset);
- read_unlock(&mapping->page_lock);
- if (page)
- return 0;
-
         page = page_cache_alloc(mapping);
         if (!page)
                 return -ENOMEM;
 
- error = add_to_page_cache_unique(page, mapping, offset);
+ error = add_to_page_cache(page, mapping, offset);
         if (!error) {
                 error = mapping->a_ops->readpage(file, page);
                 page_cache_release(page);
@@ -759,28 +727,31 @@ struct page *find_trylock_page(struct ad
         return page;
 }
 
-/*
- * Must be called with the mapping lock held for writing.
- * Will return with it held for writing, but it may be dropped
- * while locking the page.
+/**
+ * find_lock_page - locate, pin and lock a pagecache page
+ *
+ * @mapping - the address_space to search
+ * @offset - the page index
+ *
+ * Locates the desired pagecache page, locks it, increments its reference
+ * count and returns its address.
+ *
+ * Returns zero if the page was not present. find_lock_page() may sleep.
  */
-static struct page *__find_lock_page(struct address_space *mapping,
- unsigned long offset)
+struct page *find_lock_page(struct address_space *mapping,
+ unsigned long offset)
 {
         struct page *page;
 
- /*
- * We scan the hash list read-only. Addition to and removal from
- * the hash-list needs a held write-lock.
- */
+ read_lock(&mapping->page_lock);
 repeat:
         page = radix_tree_lookup(&mapping->page_tree, offset);
         if (page) {
                 page_cache_get(page);
                 if (TestSetPageLocked(page)) {
- write_unlock(&mapping->page_lock);
+ read_unlock(&mapping->page_lock);
                         lock_page(page);
- write_lock(&mapping->page_lock);
+ read_lock(&mapping->page_lock);
 
                         /* Has the page been truncated while we slept? */
                         if (page->mapping != mapping || page->index != offset) {
@@ -790,34 +761,7 @@ repeat:
                         }
                 }
         }
- return page;
-}
-
-/**
- * find_lock_page - locate, pin and lock a pagecache page
- *
- * @mapping - the address_space to search
- * @offset - the page index
- *
- * Locates the desired pagecache page, locks it, increments its reference
- * count and returns its address.
- *
- * Returns zero if the page was not present. find_lock_page() may sleep.
- */
-
-/*
- * The write_lock is unfortunate, but __find_lock_page() requires that on
- * behalf of find_or_create_page(). We could just clone __find_lock_page() -
- * one for find_lock_page(), one for find_or_create_page()...
- */
-struct page *find_lock_page(struct address_space *mapping,
- unsigned long offset)
-{
- struct page *page;
-
- write_lock(&mapping->page_lock);
- page = __find_lock_page(mapping, offset);
- write_unlock(&mapping->page_lock);
+ read_unlock(&mapping->page_lock);
         return page;
 }
 
@@ -842,32 +786,25 @@ struct page *find_lock_page(struct addre
 struct page *find_or_create_page(struct address_space *mapping,
                 unsigned long index, unsigned int gfp_mask)
 {
- struct page *page;
-
+ struct page *page, *cached_page = NULL;
+ int err;
+repeat:
         page = find_lock_page(mapping, index);
         if (!page) {
- struct page *newpage = alloc_page(gfp_mask);
- if (newpage) {
- write_lock(&mapping->page_lock);
- page = __find_lock_page(mapping, index);
- if (likely(!page)) {
- page = newpage;
- if (__add_to_page_cache(page, mapping, index)) {
- write_unlock(&mapping->page_lock);
- page_cache_release(page);
- page = NULL;
- goto out;
- }
- newpage = NULL;
- }
- write_unlock(&mapping->page_lock);
- if (newpage == NULL)
- lru_cache_add(page);
- else
- page_cache_release(newpage);
+ if (!cached_page) {
+ cached_page = alloc_page(gfp_mask);
+ if (!cached_page)
+ return NULL;
                 }
+ err = add_to_page_cache(cached_page, mapping, index);
+ if (!err) {
+ page = cached_page;
+ cached_page = NULL;
+ } else if (err == -EEXIST)
+ goto repeat;
         }
-out:
+ if (cached_page)
+ page_cache_release(cached_page);
         return page;
 }
 
@@ -901,7 +838,7 @@ grab_cache_page_nowait(struct address_sp
                 return NULL;
         }
         page = alloc_pages(mapping->gfp_mask & ~__GFP_FS, 0);
- if (page && add_to_page_cache_unique(page, mapping, index)) {
+ if (page && add_to_page_cache(page, mapping, index)) {
                 page_cache_release(page);
                 page = NULL;
         }
@@ -968,18 +905,16 @@ void do_generic_file_read(struct file *
                 /*
                  * Try to find the data in the page cache..
                  */
-
- write_lock(&mapping->page_lock);
+find_page:
+ read_lock(&mapping->page_lock);
                 page = radix_tree_lookup(&mapping->page_tree, index);
                 if (!page) {
- write_unlock(&mapping->page_lock);
+ read_unlock(&mapping->page_lock);
                         handle_ra_thrashing(filp);
- write_lock(&mapping->page_lock);
                         goto no_cached_page;
                 }
-found_page:
                 page_cache_get(page);
- write_unlock(&mapping->page_lock);
+ read_unlock(&mapping->page_lock);
 
                 if (!PageUptodate(page))
                         goto page_not_up_to_date;
@@ -1059,40 +994,23 @@ no_cached_page:
                 /*
                  * Ok, it wasn't cached, so we need to create a new
                  * page..
- *
- * We get here with the page cache lock held.
                  */
                 if (!cached_page) {
- write_unlock(&mapping->page_lock);
                         cached_page = page_cache_alloc(mapping);
                         if (!cached_page) {
                                 desc->error = -ENOMEM;
                                 break;
                         }
-
- /*
- * Somebody may have added the page while we
- * dropped the page cache lock. Check for that.
- */
- write_lock(&mapping->page_lock);
- page = radix_tree_lookup(&mapping->page_tree, index);
- if (page)
- goto found_page;
                 }
-
- /*
- * Ok, add the new page to the hash-queues...
- */
- if (__add_to_page_cache(cached_page, mapping, index) < 0) {
- write_unlock(&mapping->page_lock);
- desc->error = -ENOMEM;
+ error = add_to_page_cache(cached_page, mapping, index);
+ if (error) {
+ if (error == -EEXIST)
+ goto find_page;
+ desc->error = error;
                         break;
                 }
                 page = cached_page;
- write_unlock(&mapping->page_lock);
- lru_cache_add(page);
                 cached_page = NULL;
-
                 goto readpage;
         }
 
@@ -1875,7 +1793,7 @@ repeat:
                         if (!cached_page)
                                 return ERR_PTR(-ENOMEM);
                 }
- err = add_to_page_cache_unique(cached_page, mapping, index);
+ err = add_to_page_cache(cached_page, mapping, index);
                 if (err == -EEXIST)
                         goto repeat;
                 if (err < 0) {
@@ -1948,7 +1866,7 @@ repeat:
                         if (!*cached_page)
                                 return NULL;
                 }
- err = add_to_page_cache_unique(*cached_page, mapping, index);
+ err = add_to_page_cache(*cached_page, mapping, index);
                 if (err == -EEXIST)
                         goto repeat;
                 if (err == 0) {
--- 2.5.26/mm/readahead.c~hugh-add_to_page_cache Tue Jul 16 21:47:15 2002
+++ 2.5.26-akpm/mm/readahead.c Tue Jul 16 21:59:35 2002
@@ -43,7 +43,7 @@ read_pages(struct file *file, struct add
         for (page_idx = 0; page_idx < nr_pages; page_idx++) {
                 struct page *page = list_entry(pages->prev, struct page, list);
                 list_del(&page->list);
- if (!add_to_page_cache_unique(page, mapping, page->index))
+ if (!add_to_page_cache(page, mapping, page->index))
                         mapping->a_ops->readpage(file, page);
                 page_cache_release(page);
         }
--- 2.5.26/mm/swap_state.c~hugh-add_to_page_cache Tue Jul 16 21:47:15 2002
+++ 2.5.26-akpm/mm/swap_state.c Tue Jul 16 21:47:15 2002
@@ -75,8 +75,7 @@ int add_to_swap_cache(struct page *page,
                 INC_CACHE_INFO(noent_race);
                 return -ENOENT;
         }
-
- error = add_to_page_cache_unique(page, &swapper_space, entry.val);
+ error = add_to_page_cache(page, &swapper_space, entry.val);
         if (error != 0) {
                 swap_free(entry);
                 if (error == -EEXIST)

.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Tue Jul 23 2002 - 22:00:22 EST