LU-11582 llite: protect reading inode->i_data.nrpages

truncate_inode_pages() looks up pages in the radix tree without
lock, and could miss finding pages removed from the radix tree
by __remove_mapping(), so that after calling truncate_inode_pages()
we need to read the nrpages of the inode->i_data with the protection
of tree_lock.

Since it could still be in the race window of __remove_mapping()->
__delete_from_page_cache()->page_cache_tree_delte(), before the
nrpages being decreased.

Signed-off-by: Bobi Jam <bobijam@whamcloud.com>
Change-Id: I44ba6bea3dec4f0a110d1ae2a749514ec7dd0d12
Reviewed-on: https://review.whamcloud.com/33639
Tested-by: Jenkins
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Patrick Farrell <paf@cray.com>
Tested-by: Maloo <maloo@whamcloud.com>

diff --git a/lustre/include/lustre_compat.h b/lustre/include/lustre_compat.h
index 13ed36b..fdb2010 100644 (file)
--- a/lustre/include/lustre_compat.h
+++ b/lustre/include/lustre_compat.h
@@ -386,11 +386,6 @@ static inline int radix_tree_exceptional_entry(void *arg)
 static inline void truncate_inode_pages_final(struct address_space *map)
 {
        truncate_inode_pages(map, 0);
-               /* Workaround for LU-118 */
-       if (map->nrpages) {
-               spin_lock_irq(&map->tree_lock);
-               spin_unlock_irq(&map->tree_lock);
-       }       /* Workaround end */
 }
 #endif
 

diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c
index f8db6a1..ad377fe 100644 (file)
--- a/lustre/llite/llite_lib.c
+++ b/lustre/llite/llite_lib.c
@@ -2087,6 +2087,8 @@ int ll_read_inode2(struct inode *inode, void *opaque)
 void ll_delete_inode(struct inode *inode)
 {
        struct ll_inode_info *lli = ll_i2info(inode);
+       struct address_space *mapping = &inode->i_data;
+       unsigned long nrpages;
        ENTRY;
 
        if (S_ISREG(inode->i_mode) && lli->lli_clob != NULL)
@@ -2094,11 +2096,26 @@ void ll_delete_inode(struct inode *inode)
                 * otherwise we may lose data while umount */
                cl_sync_file_range(inode, 0, OBD_OBJECT_EOF, CL_FSYNC_LOCAL, 1);
 
-       truncate_inode_pages_final(&inode->i_data);
+       truncate_inode_pages_final(mapping);
 
-       LASSERTF(inode->i_data.nrpages == 0, "inode="DFID"(%p) nrpages=%lu, "
+       /* Workaround for LU-118: Note nrpages may not be totally updated when
+        * truncate_inode_pages() returns, as there can be a page in the process
+        * of deletion (inside __delete_from_page_cache()) in the specified
+        * range. Thus mapping->nrpages can be non-zero when this function
+        * returns even after truncation of the whole mapping.  Only do this if
+        * npages isn't already zero.
+        */
+       nrpages = mapping->nrpages;
+       if (nrpages) {
+               spin_lock_irq(&mapping->tree_lock);
+               nrpages = mapping->nrpages;
+               spin_unlock_irq(&mapping->tree_lock);
+       } /* Workaround end */
+
+       LASSERTF(nrpages == 0, "%s: inode="DFID"(%p) nrpages=%lu, "
                 "see https://jira.whamcloud.com/browse/LU-118\n",
-                PFID(ll_inode2fid(inode)), inode, inode->i_data.nrpages);
+                ll_get_fsname(inode->i_sb, NULL, 0),
+                PFID(ll_inode2fid(inode)), inode, nrpages);
 
 #ifdef HAVE_SBOPS_EVICT_INODE
        ll_clear_inode(inode);