From b5ce1d83a62fc109d8e815b1fc71dcdb0d26bc49 Mon Sep 17 00:00:00 2001 From: Yoshihisa Abe Date: Mon, 25 Oct 2010 02:03:44 -0400 Subject: [PATCH] Coda: add spin lock to protect accesses to struct coda_inode_info. We mostly need it to protect cached user permissions. The c_flags field is advisory, reading the wrong value is harmless and in the worst case we hit a slow path where we have to make an extra upcall to the userspace cache manager when revalidating a dentry or inode. Signed-off-by: Yoshihisa Abe Signed-off-by: Jan Harkes Signed-off-by: Linus Torvalds --- fs/coda/cache.c | 17 ++++++++++++----- fs/coda/cnode.c | 19 ++++++++++++++----- fs/coda/dir.c | 6 ++++++ fs/coda/file.c | 12 ++++++++++-- fs/coda/inode.c | 2 ++ include/linux/coda_fs_i.h | 13 +++++++++---- include/linux/coda_linux.h | 6 +++++- 7 files changed, 58 insertions(+), 17 deletions(-) diff --git a/fs/coda/cache.c b/fs/coda/cache.c index a5bf5771a22a..9060f08e70cf 100644 --- a/fs/coda/cache.c +++ b/fs/coda/cache.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -31,19 +32,23 @@ void coda_cache_enter(struct inode *inode, int mask) { struct coda_inode_info *cii = ITOC(inode); + spin_lock(&cii->c_lock); cii->c_cached_epoch = atomic_read(&permission_epoch); if (cii->c_uid != current_fsuid()) { cii->c_uid = current_fsuid(); cii->c_cached_perm = mask; } else cii->c_cached_perm |= mask; + spin_unlock(&cii->c_lock); } /* remove cached acl from an inode */ void coda_cache_clear_inode(struct inode *inode) { struct coda_inode_info *cii = ITOC(inode); + spin_lock(&cii->c_lock); cii->c_cached_epoch = atomic_read(&permission_epoch) - 1; + spin_unlock(&cii->c_lock); } /* remove all acl caches */ @@ -57,13 +62,15 @@ void coda_cache_clear_all(struct super_block *sb) int coda_cache_check(struct inode *inode, int mask) { struct coda_inode_info *cii = ITOC(inode); - int hit; + int hit; - hit = (mask & cii->c_cached_perm) == mask && - cii->c_uid == current_fsuid() && - cii->c_cached_epoch == atomic_read(&permission_epoch); + spin_lock(&cii->c_lock); + hit = (mask & cii->c_cached_perm) == mask && + cii->c_uid == current_fsuid() && + cii->c_cached_epoch == atomic_read(&permission_epoch); + spin_unlock(&cii->c_lock); - return hit; + return hit; } diff --git a/fs/coda/cnode.c b/fs/coda/cnode.c index a7a780929eec..602240569c89 100644 --- a/fs/coda/cnode.c +++ b/fs/coda/cnode.c @@ -45,13 +45,15 @@ static void coda_fill_inode(struct inode *inode, struct coda_vattr *attr) static int coda_test_inode(struct inode *inode, void *data) { struct CodaFid *fid = (struct CodaFid *)data; - return coda_fideq(&(ITOC(inode)->c_fid), fid); + struct coda_inode_info *cii = ITOC(inode); + return coda_fideq(&cii->c_fid, fid); } static int coda_set_inode(struct inode *inode, void *data) { struct CodaFid *fid = (struct CodaFid *)data; - ITOC(inode)->c_fid = *fid; + struct coda_inode_info *cii = ITOC(inode); + cii->c_fid = *fid; return 0; } @@ -71,6 +73,7 @@ struct inode * coda_iget(struct super_block * sb, struct CodaFid * fid, cii = ITOC(inode); /* we still need to set i_ino for things like stat(2) */ inode->i_ino = hash; + /* inode is locked and unique, no need to grab cii->c_lock */ cii->c_mapcount = 0; unlock_new_inode(inode); } @@ -107,14 +110,20 @@ int coda_cnode_make(struct inode **inode, struct CodaFid *fid, struct super_bloc } +/* Although we treat Coda file identifiers as immutable, there is one + * special case for files created during a disconnection where they may + * not be globally unique. When an identifier collision is detected we + * first try to flush the cached inode from the kernel and finally + * resort to renaming/rehashing in-place. Userspace remembers both old + * and new values of the identifier to handle any in-flight upcalls. + * The real solution is to use globally unique UUIDs as identifiers, but + * retrofitting the existing userspace code for this is non-trivial. */ void coda_replace_fid(struct inode *inode, struct CodaFid *oldfid, struct CodaFid *newfid) { - struct coda_inode_info *cii; + struct coda_inode_info *cii = ITOC(inode); unsigned long hash = coda_f2i(newfid); - cii = ITOC(inode); - BUG_ON(!coda_fideq(&cii->c_fid, oldfid)); /* replace fid and rehash inode */ diff --git a/fs/coda/dir.c b/fs/coda/dir.c index ccd98b0f2b0b..69fbbea75f1b 100644 --- a/fs/coda/dir.c +++ b/fs/coda/dir.c @@ -18,6 +18,7 @@ #include #include #include +#include #include @@ -617,7 +618,9 @@ static int coda_dentry_revalidate(struct dentry *de, struct nameidata *nd) goto out; /* clear the flags. */ + spin_lock(&cii->c_lock); cii->c_flags &= ~(C_VATTR | C_PURGE | C_FLUSH); + spin_unlock(&cii->c_lock); bad: unlock_kernel(); @@ -691,7 +694,10 @@ int coda_revalidate_inode(struct dentry *dentry) goto return_bad; coda_flag_inode_children(inode, C_FLUSH); + + spin_lock(&cii->c_lock); cii->c_flags &= ~(C_VATTR | C_PURGE | C_FLUSH); + spin_unlock(&cii->c_lock); } ok: diff --git a/fs/coda/file.c b/fs/coda/file.c index ad3cd2abeeb4..c4e395781d41 100644 --- a/fs/coda/file.c +++ b/fs/coda/file.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -109,19 +110,24 @@ coda_file_mmap(struct file *coda_file, struct vm_area_struct *vma) coda_inode = coda_file->f_path.dentry->d_inode; host_inode = host_file->f_path.dentry->d_inode; + + cii = ITOC(coda_inode); + spin_lock(&cii->c_lock); coda_file->f_mapping = host_file->f_mapping; if (coda_inode->i_mapping == &coda_inode->i_data) coda_inode->i_mapping = host_inode->i_mapping; /* only allow additional mmaps as long as userspace isn't changing * the container file on us! */ - else if (coda_inode->i_mapping != host_inode->i_mapping) + else if (coda_inode->i_mapping != host_inode->i_mapping) { + spin_unlock(&cii->c_lock); return -EBUSY; + } /* keep track of how often the coda_inode/host_file has been mmapped */ - cii = ITOC(coda_inode); cii->c_mapcount++; cfi->cfi_mapcount++; + spin_unlock(&cii->c_lock); return host_file->f_op->mmap(host_file, vma); } @@ -185,11 +191,13 @@ int coda_release(struct inode *coda_inode, struct file *coda_file) cii = ITOC(coda_inode); /* did we mmap this file? */ + spin_lock(&cii->c_lock); if (coda_inode->i_mapping == &host_inode->i_data) { cii->c_mapcount -= cfi->cfi_mapcount; if (!cii->c_mapcount) coda_inode->i_mapping = &coda_inode->i_data; } + spin_unlock(&cii->c_lock); fput(cfi->cfi_container); kfree(coda_file->private_data); diff --git a/fs/coda/inode.c b/fs/coda/inode.c index bfe8179b1295..0553f3bd7b1b 100644 --- a/fs/coda/inode.c +++ b/fs/coda/inode.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -51,6 +52,7 @@ static struct inode *coda_alloc_inode(struct super_block *sb) ei->c_flags = 0; ei->c_uid = 0; ei->c_cached_perm = 0; + spin_lock_init(&ei->c_lock); return &ei->vfs_inode; } diff --git a/include/linux/coda_fs_i.h b/include/linux/coda_fs_i.h index b3ef0c461578..e35071b1de0e 100644 --- a/include/linux/coda_fs_i.h +++ b/include/linux/coda_fs_i.h @@ -10,19 +10,24 @@ #include #include +#include #include /* * coda fs inode data + * c_lock protects accesses to c_flags, c_mapcount, c_cached_epoch, c_uid and + * c_cached_perm. + * vfs_inode is set only when the inode is created and never changes. + * c_fid is set when the inode is created and should be considered immutable. */ struct coda_inode_info { - struct CodaFid c_fid; /* Coda identifier */ - u_short c_flags; /* flags (see below) */ - struct list_head c_cilist; /* list of all coda inodes */ + struct CodaFid c_fid; /* Coda identifier */ + u_short c_flags; /* flags (see below) */ unsigned int c_mapcount; /* nr of times this inode is mapped */ unsigned int c_cached_epoch; /* epoch for cached permissions */ vuid_t c_uid; /* fsuid for cached permissions */ - unsigned int c_cached_perm; /* cached access permissions */ + unsigned int c_cached_perm; /* cached access permissions */ + spinlock_t c_lock; struct inode vfs_inode; }; diff --git a/include/linux/coda_linux.h b/include/linux/coda_linux.h index dcc228aa335a..2e914d0771b9 100644 --- a/include/linux/coda_linux.h +++ b/include/linux/coda_linux.h @@ -89,7 +89,11 @@ static __inline__ char *coda_i2s(struct inode *inode) /* this will not zap the inode away */ static __inline__ void coda_flag_inode(struct inode *inode, int flag) { - ITOC(inode)->c_flags |= flag; + struct coda_inode_info *cii = ITOC(inode); + + spin_lock(&cii->c_lock); + cii->c_flags |= flag; + spin_unlock(&cii->c_lock); } #endif -- 2.20.1