From 45996492e5c85aa0ac93a95d1b2d1ed56851c865 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 25 Mar 2016 19:56:34 -0400 Subject: [PATCH] orangefs: fix orangefs_superblock locking * switch orangefs_remount() to taking ORANGEFS_SB(sb) instead of sb * remove from the list _before_ orangefs_unmount() - request_mutex in the latter will make sure that nothing observed in the loop in ORANGEFS_DEV_REMOUNT_ALL handling will get freed until the end of loop * on removal, keep the forward pointer and zero the back one. That way we can drop and regain the spinlock in the loop body (again, ORANGEFS_DEV_REMOUNT_ALL one) and still be able to get to the rest of the list. Signed-off-by: Al Viro Signed-off-by: Mike Marshall --- fs/orangefs/devorangefs-req.c | 41 ++++++++++++++++++++--------------- fs/orangefs/orangefs-kernel.h | 34 +---------------------------- fs/orangefs/super.c | 30 +++++++++++++++++++------ 3 files changed, 47 insertions(+), 58 deletions(-) diff --git a/fs/orangefs/devorangefs-req.c b/fs/orangefs/devorangefs-req.c index 35418d0b77bf..db170beba797 100644 --- a/fs/orangefs/devorangefs-req.c +++ b/fs/orangefs/devorangefs-req.c @@ -572,8 +572,7 @@ static long dispatch_ioctl_command(unsigned int command, unsigned long arg) struct dev_mask_info_s mask_info = { 0 }; struct dev_mask2_info_s mask2_info = { 0, 0 }; int upstream_kmod = 1; - struct list_head *tmp = NULL; - struct orangefs_sb_info_s *orangefs_sb = NULL; + struct orangefs_sb_info_s *orangefs_sb; /* mtmoore: add locking here */ @@ -619,26 +618,32 @@ static long dispatch_ioctl_command(unsigned int command, unsigned long arg) gossip_debug(GOSSIP_DEV_DEBUG, "%s: priority remount in progress\n", __func__); - list_for_each(tmp, &orangefs_superblocks) { - orangefs_sb = - list_entry(tmp, - struct orangefs_sb_info_s, - list); - if (orangefs_sb && (orangefs_sb->sb)) { + spin_lock(&orangefs_superblocks_lock); + list_for_each_entry(orangefs_sb, &orangefs_superblocks, list) { + /* + * We have to drop the spinlock, so entries can be + * removed. They can't be freed, though, so we just + * keep the forward pointers and zero the back ones - + * that way we can get to the rest of the list. + */ + if (!orangefs_sb->list.prev) + continue; + gossip_debug(GOSSIP_DEV_DEBUG, + "%s: Remounting SB %p\n", + __func__, + orangefs_sb); + + spin_unlock(&orangefs_superblocks_lock); + ret = orangefs_remount(orangefs_sb); + spin_lock(&orangefs_superblocks_lock); + if (ret) { gossip_debug(GOSSIP_DEV_DEBUG, - "%s: Remounting SB %p\n", - __func__, + "SB %p remount failed\n", orangefs_sb); - - ret = orangefs_remount(orangefs_sb->sb); - if (ret) { - gossip_debug(GOSSIP_DEV_DEBUG, - "SB %p remount failed\n", - orangefs_sb); - break; - } + break; } } + spin_unlock(&orangefs_superblocks_lock); gossip_debug(GOSSIP_DEV_DEBUG, "%s: priority remount complete\n", __func__); diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h index db258d2ccc6a..a9925e296ceb 100644 --- a/fs/orangefs/orangefs-kernel.h +++ b/fs/orangefs/orangefs-kernel.h @@ -462,7 +462,7 @@ struct dentry *orangefs_mount(struct file_system_type *fst, void *data); void orangefs_kill_sb(struct super_block *sb); -int orangefs_remount(struct super_block *sb); +int orangefs_remount(struct orangefs_sb_info_s *); int fsid_key_table_initialize(void); void fsid_key_table_finalize(void); @@ -598,38 +598,6 @@ int service_operation(struct orangefs_kernel_op_s *op, ((ORANGEFS_SB(inode->i_sb)->flags & ORANGEFS_OPT_INTR) ? \ ORANGEFS_OP_INTERRUPTIBLE : 0) -#define add_orangefs_sb(sb) \ -do { \ - gossip_debug(GOSSIP_SUPER_DEBUG, \ - "Adding SB %p to orangefs superblocks\n", \ - ORANGEFS_SB(sb)); \ - spin_lock(&orangefs_superblocks_lock); \ - list_add_tail(&ORANGEFS_SB(sb)->list, &orangefs_superblocks); \ - spin_unlock(&orangefs_superblocks_lock); \ -} while (0) - -#define remove_orangefs_sb(sb) \ -do { \ - struct list_head *tmp = NULL; \ - struct list_head *tmp_safe = NULL; \ - struct orangefs_sb_info_s *orangefs_sb = NULL; \ - \ - spin_lock(&orangefs_superblocks_lock); \ - list_for_each_safe(tmp, tmp_safe, &orangefs_superblocks) { \ - orangefs_sb = list_entry(tmp, \ - struct orangefs_sb_info_s, \ - list); \ - if (orangefs_sb && (orangefs_sb->sb == sb)) { \ - gossip_debug(GOSSIP_SUPER_DEBUG, \ - "Removing SB %p from orangefs superblocks\n", \ - orangefs_sb); \ - list_del(&orangefs_sb->list); \ - break; \ - } \ - } \ - spin_unlock(&orangefs_superblocks_lock); \ -} while (0) - #define fill_default_sys_attrs(sys_attr, type, mode) \ do { \ sys_attr.owner = from_kuid(current_user_ns(), current_fsuid()); \ diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c index 5a89b8083966..b9da9a0281c9 100644 --- a/fs/orangefs/super.c +++ b/fs/orangefs/super.c @@ -210,7 +210,7 @@ static int orangefs_remount_fs(struct super_block *sb, int *flags, char *data) * the client regains all of the mount information from us. * NOTE: this function assumes that the request_mutex is already acquired! */ -int orangefs_remount(struct super_block *sb) +int orangefs_remount(struct orangefs_sb_info_s *orangefs_sb) { struct orangefs_kernel_op_s *new_op; int ret = -EINVAL; @@ -221,7 +221,7 @@ int orangefs_remount(struct super_block *sb) if (!new_op) return -ENOMEM; strncpy(new_op->upcall.req.fs_mount.orangefs_config_server, - ORANGEFS_SB(sb)->devname, + orangefs_sb->devname, ORANGEFS_MAX_SERVER_ADDR_LEN); gossip_debug(GOSSIP_SUPER_DEBUG, @@ -244,8 +244,8 @@ int orangefs_remount(struct super_block *sb) * short-lived mapping that the system interface uses * to map this superblock to a particular mount entry */ - ORANGEFS_SB(sb)->id = new_op->downcall.resp.fs_mount.id; - ORANGEFS_SB(sb)->mount_pending = 0; + orangefs_sb->id = new_op->downcall.resp.fs_mount.id; + orangefs_sb->mount_pending = 0; } op_release(new_op); @@ -485,7 +485,12 @@ struct dentry *orangefs_mount(struct file_system_type *fst, * finally, add this sb to our list of known orangefs * sb's */ - add_orangefs_sb(sb); + gossip_debug(GOSSIP_SUPER_DEBUG, + "Adding SB %p to orangefs superblocks\n", + ORANGEFS_SB(sb)); + spin_lock(&orangefs_superblocks_lock); + list_add_tail(&ORANGEFS_SB(sb)->list, &orangefs_superblocks); + spin_unlock(&orangefs_superblocks_lock); op_release(new_op); return dget(sb->s_root); @@ -512,10 +517,21 @@ void orangefs_kill_sb(struct super_block *sb) * issue the unmount to userspace to tell it to remove the * dynamic mount info it has for this superblock */ - orangefs_unmount_sb(sb); + orangefs_unmount_sb(sb); /* remove the sb from our list of orangefs specific sb's */ - remove_orangefs_sb(sb); + + spin_lock(&orangefs_superblocks_lock); + __list_del_entry(&ORANGEFS_SB(sb)->list); /* not list_del_init */ + ORANGEFS_SB(sb)->list.prev = NULL; + spin_unlock(&orangefs_superblocks_lock); + + /* + * make sure that ORANGEFS_DEV_REMOUNT_ALL loop that might've seen us + * gets completed before we free the dang thing. + */ + mutex_lock(&request_mutex); + mutex_unlock(&request_mutex); /* free the orangefs superblock private data */ kfree(ORANGEFS_SB(sb)); -- 2.20.1