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 <[email protected]>
Signed-off-by: Mike Marshall <[email protected]>

Author: Al Viro

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

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

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