From 8b82aa7f18950cfa328538be77236270fab1c47d Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Mon, 19 Dec 2016 13:38:24 -0800 Subject: [PATCH] Consistently initialize inode fields Inode info struct initialization spread out over three places: - once for the memory of a slab obect - when reading an existing inode from items - when initializing a newly allocated inode Over time field initializtion got out of sync with these rules. This makes it more clear which fields get initialized where. In the inode info struct we group fields by where there initialized. We order the fields by size and location in the inode struct. Then we make sure that all the initialization sites have everything covered. Doing everything in consistent struct order makes it easier to audit that we haven't missed anything. What lead to this was realizing that we missed initializing the seqcount when reading existing inodes. It should have been initialized in the slab object constructor. The 'staging' boolean has the same problem. Signed-off-by: Zach Brown Reviewed-by: Mark Fasheh --- kmod/src/inode.c | 9 +++++++-- kmod/src/inode.h | 9 ++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/kmod/src/inode.c b/kmod/src/inode.c index 046de8e5..7f61f552 100644 --- a/kmod/src/inode.c +++ b/kmod/src/inode.c @@ -49,10 +49,17 @@ struct free_ino_pool { static struct kmem_cache *scoutfs_inode_cachep; +/* + * This is called once before all the allocations and frees of a inode + * object within a slab. It's for inode fields that don't need to be + * initialized for a given instance of an inode. + */ static void scoutfs_inode_ctor(void *obj) { struct scoutfs_inode_info *ci = obj; + seqcount_init(&ci->seqcount); + ci->staging = false; init_rwsem(&ci->xattr_rwsem); inode_init_once(&ci->inode); @@ -501,10 +508,8 @@ struct inode *scoutfs_new_inode(struct super_block *sb, struct inode *dir, ci = SCOUTFS_I(inode); ci->ino = ino; - seqcount_init(&ci->seqcount); ci->data_version = 0; ci->next_readdir_pos = SCOUTFS_DIRENT_FIRST_POS; - ci->staging = false; inode->i_ino = ino; /* XXX overflow */ inode_init_owner(inode, dir, mode); diff --git a/kmod/src/inode.h b/kmod/src/inode.h index b282960a..da24e9af 100644 --- a/kmod/src/inode.h +++ b/kmod/src/inode.h @@ -4,15 +4,14 @@ #include "key.h" struct scoutfs_inode_info { + /* read or initialized for each inode instance */ u64 ino; - - seqcount_t seqcount; u64 data_version; u64 next_readdir_pos; - /* holder of i_mutex is staging */ - bool staging; - + /* initialized once for slab object */ + seqcount_t seqcount; + bool staging; /* holder of i_mutex is staging */ struct rw_semaphore xattr_rwsem; struct inode inode;