diff mbox

Revert "Btrfs: device_list_add() should not update list when

Message ID 541AF37D.5050505@fb.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Mason Sept. 18, 2014, 3 p.m. UTC
Johannes and Sam, could you please confirm this patch fixes your mount
regression for now?  Anand, please make sure I kept the generation check
properly.

This reverts commit b96de000bc8bc9688b3a2abea4332bd57648a49f.

This commit is triggering failures to mount by subvolume id in some
configurations.  The main problem is how many different ways this
scanning function is used, both for scanning while mounted and
unmounted.  A proper cleanup is too big for late rcs.

For now, just revert the commit and we'll put a better fix into a later
merge window.

Signed-off-by: Chris Mason <clm@fb.com>
---
 fs/btrfs/volumes.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Comments

Anand Jain Sept. 19, 2014, 10:21 a.m. UTC | #1
Looks good to me Chris. Thank you.


Reviewed-by: Anand Jain <anand.jain@oracle.com>


On 09/18/2014 11:00 PM, Chris Mason wrote:
>
> Johannes and Sam, could you please confirm this patch fixes your mount
> regression for now?  Anand, please make sure I kept the generation check
> properly.
>
> This reverts commit b96de000bc8bc9688b3a2abea4332bd57648a49f.
>
> This commit is triggering failures to mount by subvolume id in some
> configurations.  The main problem is how many different ways this
> scanning function is used, both for scanning while mounted and
> unmounted.  A proper cleanup is too big for late rcs.
>
> For now, just revert the commit and we'll put a better fix into a later
> merge window.
>
> Signed-off-by: Chris Mason <clm@fb.com>
> ---
>   fs/btrfs/volumes.c | 13 ++++++-------
>   1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 340a92d..2c2d6d1 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -529,12 +529,12 @@ static noinline int device_list_add(const char *path,
>   		 */
>
>   		/*
> -		 * As of now don't allow update to btrfs_fs_device through
> -		 * the btrfs dev scan cli, after FS has been mounted.
> +		 * For now, we do allow update to btrfs_fs_device through the
> +		 * btrfs dev scan cli after FS has been mounted.  We're still
> +		 * tracking a problem where systems fail mount by subvolume id
> +		 * when we reject replacement on a mounted FS.
>   		 */
> -		if (fs_devices->opened) {
> -			return -EBUSY;
> -		} else {
> +		if (!fs_devices->opened && found_transid < device->generation) {
>   			/*
>   			 * That is if the FS is _not_ mounted and if you
>   			 * are here, that means there is more than one
> @@ -542,8 +542,7 @@ static noinline int device_list_add(const char *path,
>   			 * with larger generation number or the last-in if
>   			 * generation are equal.
>   			 */
> -			if (found_transid < device->generation)
> -				return -EEXIST;
> +			return -EEXIST;
>   		}
>
>   		name = rcu_string_strdup(path, GFP_NOFS);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sam Thursfield Sept. 19, 2014, 1:47 p.m. UTC | #2
On 18/09/14 16:00, Chris Mason wrote:
>
> Johannes and Sam, could you please confirm this patch fixes your mount
> regression for now?  Anand, please make sure I kept the generation check
> properly.

I've just tested this patch on top of 3.17-rc5 and it fixes the issue 
for me.

Thanks!
Sam
Chris Mason Sept. 19, 2014, 1:59 p.m. UTC | #3
On 09/19/2014 09:47 AM, Sam Thursfield wrote:
> On 18/09/14 16:00, Chris Mason wrote:
>>
>> Johannes and Sam, could you please confirm this patch fixes your mount
>> regression for now?  Anand, please make sure I kept the generation check
>> properly.
> 
> I've just tested this patch on top of 3.17-rc5 and it fixes the issue
> for me.
> 

Perfect, thanks.

-chris

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 340a92d..2c2d6d1 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -529,12 +529,12 @@  static noinline int device_list_add(const char *path,
 		 */
 
 		/*
-		 * As of now don't allow update to btrfs_fs_device through
-		 * the btrfs dev scan cli, after FS has been mounted.
+		 * For now, we do allow update to btrfs_fs_device through the
+		 * btrfs dev scan cli after FS has been mounted.  We're still
+		 * tracking a problem where systems fail mount by subvolume id
+		 * when we reject replacement on a mounted FS.
 		 */
-		if (fs_devices->opened) {
-			return -EBUSY;
-		} else {
+		if (!fs_devices->opened && found_transid < device->generation) {
 			/*
 			 * That is if the FS is _not_ mounted and if you
 			 * are here, that means there is more than one
@@ -542,8 +542,7 @@  static noinline int device_list_add(const char *path,
 			 * with larger generation number or the last-in if
 			 * generation are equal.
 			 */
-			if (found_transid < device->generation)
-				return -EEXIST;
+			return -EEXIST;
 		}
 
 		name = rcu_string_strdup(path, GFP_NOFS);