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 <viro@zeniv.linux.org.uk>
Signed-off-by: Mike Marshall <hubcap@omnibond.com>

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));