From 04c172b686763be0d42eb4c36532d5795166eb7c Mon Sep 17 00:00:00 2001 From: Bobi Jam Date: Sun, 11 Nov 2018 16:41:21 +0800 Subject: [PATCH] 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 Change-Id: I44ba6bea3dec4f0a110d1ae2a749514ec7dd0d12 Reviewed-on: https://review.whamcloud.com/33639 Tested-by: Jenkins Reviewed-by: Andreas Dilger Reviewed-by: Patrick Farrell Tested-by: Maloo --- lustre/include/lustre_compat.h | 5 ----- lustre/llite/llite_lib.c | 23 ++++++++++++++++++++--- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/lustre/include/lustre_compat.h b/lustre/include/lustre_compat.h index 13ed36b..fdb2010 100644 --- 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 --- 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); -- 1.8.3.1