make ext2_get_page() and friends work without external serialization
authorAl Viro <viro@zeniv.linux.org.uk>
Fri, 22 Apr 2016 19:06:44 +0000 (15:06 -0400)
committerAl Viro <viro@zeniv.linux.org.uk>
Mon, 2 May 2016 23:47:25 +0000 (19:47 -0400)
Right now ext2_get_page() (and its analogues in a bunch of other filesystems)
relies upon the directory being locked - the way it sets and tests Checked and
Error bits would be racy without that.  Switch to a slightly different scheme,
_not_ setting Checked in case of failure.  That way the logics becomes
if Checked => OK
else if Error => fail
else if !validate => fail
else => OK
with validation setting Checked or Error on success and failure resp. and
returning which one had happened.  Equivalent to the current logics, but unlike
the current logics not sensitive to the order of set_bit, test_bit getting
reordered by CPU, etc.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
fs/afs/dir.c
fs/exofs/dir.c
fs/ext2/dir.c
fs/nilfs2/dir.c
fs/ufs/dir.c

index 5fda2bc53cd7a35a38cefae7d72d9c8a142929f7..cdf8fbbc0b57e3d799dea3c11f04efac69874ecd 100644 (file)
@@ -128,7 +128,7 @@ struct afs_lookup_cookie {
 /*
  * check that a directory page is valid
  */
-static inline void afs_dir_check_page(struct inode *dir, struct page *page)
+static inline bool afs_dir_check_page(struct inode *dir, struct page *page)
 {
        struct afs_dir_page *dbuf;
        loff_t latter;
@@ -168,11 +168,11 @@ static inline void afs_dir_check_page(struct inode *dir, struct page *page)
        }
 
        SetPageChecked(page);
-       return;
+       return true;
 
 error:
-       SetPageChecked(page);
        SetPageError(page);
+       return false;
 }
 
 /*
@@ -196,10 +196,10 @@ static struct page *afs_dir_get_page(struct inode *dir, unsigned long index,
        page = read_cache_page(dir->i_mapping, index, afs_page_filler, key);
        if (!IS_ERR(page)) {
                kmap(page);
-               if (!PageChecked(page))
-                       afs_dir_check_page(dir, page);
-               if (PageError(page))
-                       goto fail;
+               if (unlikely(!PageChecked(page))) {
+                       if (PageError(page) || !afs_dir_check_page(dir, page))
+                               goto fail;
+               }
        }
        return page;
 
index 547b93cbea63b522746c9cc2e6dbf70aa3abdb78..a7661aaead9ac633946b088bf6a2cef9ec8b9611 100644 (file)
@@ -79,7 +79,7 @@ static int exofs_commit_chunk(struct page *page, loff_t pos, unsigned len)
        return err;
 }
 
-static void exofs_check_page(struct page *page)
+static bool exofs_check_page(struct page *page)
 {
        struct inode *dir = page->mapping->host;
        unsigned chunk_size = exofs_chunk_size(dir);
@@ -114,7 +114,7 @@ static void exofs_check_page(struct page *page)
                goto Eend;
 out:
        SetPageChecked(page);
-       return;
+       return true;
 
 Ebadsize:
        EXOFS_ERR("ERROR [exofs_check_page]: "
@@ -150,8 +150,8 @@ Eend:
                dir->i_ino, (page->index<<PAGE_SHIFT)+offs,
                _LLU(le64_to_cpu(p->inode_no)));
 fail:
-       SetPageChecked(page);
        SetPageError(page);
+       return false;
 }
 
 static struct page *exofs_get_page(struct inode *dir, unsigned long n)
@@ -161,10 +161,10 @@ static struct page *exofs_get_page(struct inode *dir, unsigned long n)
 
        if (!IS_ERR(page)) {
                kmap(page);
-               if (!PageChecked(page))
-                       exofs_check_page(page);
-               if (PageError(page))
-                       goto fail;
+               if (unlikely(!PageChecked(page))) {
+                       if (PageError(page) || !exofs_check_page(page))
+                               goto fail;
+               }
        }
        return page;
 
index 7ff6fcfa685d49158455b391bc8b30c8a1a237ef..f0f4363a15b2b00534c27c335380ff7f5ad50bde 100644 (file)
@@ -110,7 +110,7 @@ static int ext2_commit_chunk(struct page *page, loff_t pos, unsigned len)
        return err;
 }
 
-static void ext2_check_page(struct page *page, int quiet)
+static bool ext2_check_page(struct page *page, int quiet)
 {
        struct inode *dir = page->mapping->host;
        struct super_block *sb = dir->i_sb;
@@ -148,7 +148,7 @@ static void ext2_check_page(struct page *page, int quiet)
                goto Eend;
 out:
        SetPageChecked(page);
-       return;
+       return true;
 
        /* Too bad, we had an error */
 
@@ -190,8 +190,8 @@ Eend:
                        (unsigned long) le32_to_cpu(p->inode));
        }
 fail:
-       SetPageChecked(page);
        SetPageError(page);
+       return false;
 }
 
 static struct page * ext2_get_page(struct inode *dir, unsigned long n,
@@ -201,10 +201,10 @@ static struct page * ext2_get_page(struct inode *dir, unsigned long n,
        struct page *page = read_mapping_page(mapping, n, NULL);
        if (!IS_ERR(page)) {
                kmap(page);
-               if (!PageChecked(page))
-                       ext2_check_page(page, quiet);
-               if (PageError(page))
-                       goto fail;
+               if (unlikely(!PageChecked(page))) {
+                       if (PageError(page) || !ext2_check_page(page, quiet))
+                               goto fail;
+               }
        }
        return page;
 
index e08f064e4bd7b1e46e39319413c2c03e4df1d789..2eaed9254eb2c1376bc09db49d3876c28facec9b 100644 (file)
@@ -102,7 +102,7 @@ static void nilfs_commit_chunk(struct page *page,
        unlock_page(page);
 }
 
-static void nilfs_check_page(struct page *page)
+static bool nilfs_check_page(struct page *page)
 {
        struct inode *dir = page->mapping->host;
        struct super_block *sb = dir->i_sb;
@@ -137,7 +137,7 @@ static void nilfs_check_page(struct page *page)
                goto Eend;
 out:
        SetPageChecked(page);
-       return;
+       return true;
 
        /* Too bad, we had an error */
 
@@ -173,8 +173,8 @@ Eend:
                    dir->i_ino, (page->index<<PAGE_SHIFT)+offs,
                    (unsigned long) le64_to_cpu(p->inode));
 fail:
-       SetPageChecked(page);
        SetPageError(page);
+       return false;
 }
 
 static struct page *nilfs_get_page(struct inode *dir, unsigned long n)
@@ -184,10 +184,10 @@ static struct page *nilfs_get_page(struct inode *dir, unsigned long n)
 
        if (!IS_ERR(page)) {
                kmap(page);
-               if (!PageChecked(page))
-                       nilfs_check_page(page);
-               if (PageError(page))
-                       goto fail;
+               if (unlikely(!PageChecked(page))) {
+                       if (PageError(page) || !nilfs_check_page(page))
+                               goto fail;
+               }
        }
        return page;
 
index 0b1457292734c8f02c1747b27624bc1851b1aa53..4c07421453af4dc74cece3868ae1c04db2d2ba05 100644 (file)
@@ -105,7 +105,7 @@ void ufs_set_link(struct inode *dir, struct ufs_dir_entry *de,
 }
 
 
-static void ufs_check_page(struct page *page)
+static bool ufs_check_page(struct page *page)
 {
        struct inode *dir = page->mapping->host;
        struct super_block *sb = dir->i_sb;
@@ -143,7 +143,7 @@ static void ufs_check_page(struct page *page)
                goto Eend;
 out:
        SetPageChecked(page);
-       return;
+       return true;
 
        /* Too bad, we had an error */
 
@@ -180,8 +180,8 @@ Eend:
                   "offset=%lu",
                   dir->i_ino, (page->index<<PAGE_SHIFT)+offs);
 fail:
-       SetPageChecked(page);
        SetPageError(page);
+       return false;
 }
 
 static struct page *ufs_get_page(struct inode *dir, unsigned long n)
@@ -190,10 +190,10 @@ static struct page *ufs_get_page(struct inode *dir, unsigned long n)
        struct page *page = read_mapping_page(mapping, n, NULL);
        if (!IS_ERR(page)) {
                kmap(page);
-               if (!PageChecked(page))
-                       ufs_check_page(page);
-               if (PageError(page))
-                       goto fail;
+               if (unlikely(!PageChecked(page))) {
+                       if (PageError(page) || !ufs_check_page(page))
+                               goto fail;
+               }
        }
        return page;