All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] factor __btrfs_open_devices()
@ 2017-11-09 15:45 Anand Jain
  2017-11-09 15:45 ` [PATCH 1/4] btrfs: set fs_devices->seed directly Anand Jain
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Anand Jain @ 2017-11-09 15:45 UTC (permalink / raw)
  To: linux-btrfs

This is in preparation to help bring the missing device back to the
alloc list dynamically. As of now if we run 'btrfs dev scan </dev/missing>'
on a mounted FS, nothing happens, there is no code which handles this
condition. So the idea is to fix it by running through the device open
process for the reappeared missing device.

So this patch separates device open steps into a separate function so that
it can be called for a device, instead of for all the devices in the
dev_list. As this function is in the critical mount section, and to show
the src code changes as clearly as possible, I have created 4 no-functional
changes patches, which can be merged together if needed.

Anand Jain (4):
  btrfs: set fs_devices->seed directly
  btrfs: let variable required be declared inside the loop
  btrfs: move check for device generation to the last
  btrfs: factor __btrfs_open_devices() to create btrfs_open_one_device()

 fs/btrfs/volumes.c | 125 +++++++++++++++++++++++++++++------------------------
 1 file changed, 69 insertions(+), 56 deletions(-)

-- 
2.13.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] btrfs: set fs_devices->seed directly
  2017-11-09 15:45 [PATCH 0/4] factor __btrfs_open_devices() Anand Jain
@ 2017-11-09 15:45 ` Anand Jain
  2017-11-09 15:45 ` [PATCH 2/4] btrfs: let variable required be declared inside the loop Anand Jain
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Anand Jain @ 2017-11-09 15:45 UTC (permalink / raw)
  To: linux-btrfs

This is in preparation to move a section of code in __btrfs_open_devices()
into a new function so that it can be reused. As we set seeding if any of
the device is having SB flag BTRFS_SUPER_FLAG_SEEDING, so do it in the
device list loop itself. No functional changes.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index b39737568c22..ab2f349ee293 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -978,7 +978,6 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 	struct buffer_head *bh;
 	struct btrfs_super_block *disk_super;
 	u64 devid;
-	int seeding = 1;
 	int ret = 0;
 
 	flags |= FMODE_EXCL;
@@ -1010,9 +1009,9 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 
 		if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING) {
 			device->writeable = 0;
+			fs_devices->seeding = 1;
 		} else {
 			device->writeable = !bdev_read_only(bdev);
-			seeding = 0;
 		}
 
 		q = bdev_get_queue(bdev);
@@ -1044,7 +1043,6 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 		ret = -EINVAL;
 		goto out;
 	}
-	fs_devices->seeding = seeding;
 	fs_devices->opened = 1;
 	fs_devices->latest_bdev = latest_dev->bdev;
 	fs_devices->total_rw_bytes = 0;
-- 
2.13.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] btrfs: let variable required be declared inside the loop
  2017-11-09 15:45 [PATCH 0/4] factor __btrfs_open_devices() Anand Jain
  2017-11-09 15:45 ` [PATCH 1/4] btrfs: set fs_devices->seed directly Anand Jain
@ 2017-11-09 15:45 ` Anand Jain
  2017-11-09 15:53   ` Nikolay Borisov
  2017-11-09 15:45 ` [PATCH 3/4] btrfs: move check for device generation to the last Anand Jain
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Anand Jain @ 2017-11-09 15:45 UTC (permalink / raw)
  To: linux-btrfs

A preparation patch to create the actual device open in a new function.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index ab2f349ee293..ea6e542cb09d 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -970,19 +970,20 @@ int btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
 static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 				fmode_t flags, void *holder)
 {
-	struct request_queue *q;
-	struct block_device *bdev;
 	struct list_head *head = &fs_devices->devices;
 	struct btrfs_device *device;
 	struct btrfs_device *latest_dev = NULL;
-	struct buffer_head *bh;
-	struct btrfs_super_block *disk_super;
-	u64 devid;
 	int ret = 0;
 
 	flags |= FMODE_EXCL;
 
 	list_for_each_entry(device, head, dev_list) {
+		struct request_queue *q;
+		struct block_device *bdev;
+		struct buffer_head *bh;
+		struct btrfs_super_block *disk_super;
+		u64 devid;
+
 		if (device->bdev)
 			continue;
 		if (!device->name)
-- 
2.13.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] btrfs: move check for device generation to the last
  2017-11-09 15:45 [PATCH 0/4] factor __btrfs_open_devices() Anand Jain
  2017-11-09 15:45 ` [PATCH 1/4] btrfs: set fs_devices->seed directly Anand Jain
  2017-11-09 15:45 ` [PATCH 2/4] btrfs: let variable required be declared inside the loop Anand Jain
@ 2017-11-09 15:45 ` Anand Jain
  2017-11-09 15:45 ` [PATCH 4/4] btrfs: factor __btrfs_open_devices() to create btrfs_open_one_device() Anand Jain
  2017-11-15 17:03 ` [PATCH 0/4] factor __btrfs_open_devices() David Sterba
  4 siblings, 0 replies; 13+ messages in thread
From: Anand Jain @ 2017-11-09 15:45 UTC (permalink / raw)
  To: linux-btrfs

No functional changes. This helps to move the entire section into
a new function.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index ea6e542cb09d..0857b580014d 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1004,9 +1004,6 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 			goto error_brelse;
 
 		device->generation = btrfs_super_generation(disk_super);
-		if (!latest_dev ||
-		    device->generation > latest_dev->generation)
-			latest_dev = device;
 
 		if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING) {
 			device->writeable = 0;
@@ -1033,6 +1030,11 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 				 &fs_devices->alloc_list);
 		}
 		brelse(bh);
+
+		if (!latest_dev ||
+		    device->generation > latest_dev->generation)
+			latest_dev = device;
+
 		continue;
 
 error_brelse:
-- 
2.13.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] btrfs: factor __btrfs_open_devices() to create btrfs_open_one_device()
  2017-11-09 15:45 [PATCH 0/4] factor __btrfs_open_devices() Anand Jain
                   ` (2 preceding siblings ...)
  2017-11-09 15:45 ` [PATCH 3/4] btrfs: move check for device generation to the last Anand Jain
@ 2017-11-09 15:45 ` Anand Jain
  2017-11-27  6:47   ` Lakshmipathi.G
  2017-11-15 17:03 ` [PATCH 0/4] factor __btrfs_open_devices() David Sterba
  4 siblings, 1 reply; 13+ messages in thread
From: Anand Jain @ 2017-11-09 15:45 UTC (permalink / raw)
  To: linux-btrfs

No functional changes, create btrfs_open_one_device() from
__btrfs_open_devices(). This is a preparatory work to add dynamic
device scan.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c | 126 +++++++++++++++++++++++++++++------------------------
 1 file changed, 69 insertions(+), 57 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 0857b580014d..d24e966ee29f 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -601,6 +601,73 @@ void btrfs_free_stale_device(struct btrfs_device *cur_dev)
 	}
 }
 
+static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
+			struct btrfs_device *device, fmode_t flags,
+			void *holder)
+{
+	struct request_queue *q;
+	struct block_device *bdev;
+	struct buffer_head *bh;
+	struct btrfs_super_block *disk_super;
+	u64 devid;
+	int ret;
+
+	if (device->bdev)
+		return -EINVAL;
+	if (!device->name)
+		return -EINVAL;
+
+	ret = btrfs_get_bdev_and_sb(device->name->str, flags, holder, 1,
+				    &bdev, &bh);
+	if (ret)
+		return ret;
+
+	disk_super = (struct btrfs_super_block *)bh->b_data;
+	devid = btrfs_stack_device_id(&disk_super->dev_item);
+	if (devid != device->devid)
+		goto error_brelse;
+
+	if (memcmp(device->uuid, disk_super->dev_item.uuid,
+		   BTRFS_UUID_SIZE))
+		goto error_brelse;
+
+	device->generation = btrfs_super_generation(disk_super);
+
+	if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING) {
+		device->writeable = 0;
+		fs_devices->seeding = 1;
+	} else {
+		device->writeable = !bdev_read_only(bdev);
+	}
+
+	q = bdev_get_queue(bdev);
+	if (blk_queue_discard(q))
+		device->can_discard = 1;
+	if (!blk_queue_nonrot(q))
+		fs_devices->rotating = 1;
+
+	device->bdev = bdev;
+	device->in_fs_metadata = 0;
+	device->mode = flags;
+
+	fs_devices->open_devices++;
+	if (device->writeable &&
+	    device->devid != BTRFS_DEV_REPLACE_DEVID) {
+		fs_devices->rw_devices++;
+		list_add(&device->dev_alloc_list,
+			 &fs_devices->alloc_list);
+	}
+	brelse(bh);
+
+	return 0;
+
+error_brelse:
+	brelse(bh);
+	blkdev_put(bdev, flags);
+
+	return -EINVAL;
+}
+
 /*
  * Add new device to list of registered devices
  *
@@ -978,69 +1045,14 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 	flags |= FMODE_EXCL;
 
 	list_for_each_entry(device, head, dev_list) {
-		struct request_queue *q;
-		struct block_device *bdev;
-		struct buffer_head *bh;
-		struct btrfs_super_block *disk_super;
-		u64 devid;
-
-		if (device->bdev)
-			continue;
-		if (!device->name)
-			continue;
-
 		/* Just open everything we can; ignore failures here */
-		if (btrfs_get_bdev_and_sb(device->name->str, flags, holder, 1,
-					    &bdev, &bh))
+		ret = btrfs_open_one_device(fs_devices, device, flags, holder);
+		if (ret)
 			continue;
 
-		disk_super = (struct btrfs_super_block *)bh->b_data;
-		devid = btrfs_stack_device_id(&disk_super->dev_item);
-		if (devid != device->devid)
-			goto error_brelse;
-
-		if (memcmp(device->uuid, disk_super->dev_item.uuid,
-			   BTRFS_UUID_SIZE))
-			goto error_brelse;
-
-		device->generation = btrfs_super_generation(disk_super);
-
-		if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING) {
-			device->writeable = 0;
-			fs_devices->seeding = 1;
-		} else {
-			device->writeable = !bdev_read_only(bdev);
-		}
-
-		q = bdev_get_queue(bdev);
-		if (blk_queue_discard(q))
-			device->can_discard = 1;
-		if (!blk_queue_nonrot(q))
-			fs_devices->rotating = 1;
-
-		device->bdev = bdev;
-		device->in_fs_metadata = 0;
-		device->mode = flags;
-
-		fs_devices->open_devices++;
-		if (device->writeable &&
-		    device->devid != BTRFS_DEV_REPLACE_DEVID) {
-			fs_devices->rw_devices++;
-			list_add(&device->dev_alloc_list,
-				 &fs_devices->alloc_list);
-		}
-		brelse(bh);
-
 		if (!latest_dev ||
 		    device->generation > latest_dev->generation)
 			latest_dev = device;
-
-		continue;
-
-error_brelse:
-		brelse(bh);
-		blkdev_put(bdev, flags);
-		continue;
 	}
 	if (fs_devices->open_devices == 0) {
 		ret = -EINVAL;
-- 
2.13.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] btrfs: let variable required be declared inside the loop
  2017-11-09 15:45 ` [PATCH 2/4] btrfs: let variable required be declared inside the loop Anand Jain
@ 2017-11-09 15:53   ` Nikolay Borisov
  2017-11-15 17:01     ` David Sterba
  0 siblings, 1 reply; 13+ messages in thread
From: Nikolay Borisov @ 2017-11-09 15:53 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs



On  9.11.2017 17:45, Anand Jain wrote:
> A preparation patch to create the actual device open in a new function.
> 

Just say you are reducing the visibility of some variables to the loop.

> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/volumes.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index ab2f349ee293..ea6e542cb09d 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -970,19 +970,20 @@ int btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
>  static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
>  				fmode_t flags, void *holder)
>  {
> -	struct request_queue *q;
> -	struct block_device *bdev;
>  	struct list_head *head = &fs_devices->devices;
>  	struct btrfs_device *device;
>  	struct btrfs_device *latest_dev = NULL;
> -	struct buffer_head *bh;
> -	struct btrfs_super_block *disk_super;
> -	u64 devid;
>  	int ret = 0;
>  
>  	flags |= FMODE_EXCL;
>  
>  	list_for_each_entry(device, head, dev_list) {
> +		struct request_queue *q;
> +		struct block_device *bdev;
> +		struct buffer_head *bh;
> +		struct btrfs_super_block *disk_super;
> +		u64 devid;
> +
>  		if (device->bdev)
>  			continue;
>  		if (!device->name)
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] btrfs: let variable required be declared inside the loop
  2017-11-09 15:53   ` Nikolay Borisov
@ 2017-11-15 17:01     ` David Sterba
  0 siblings, 0 replies; 13+ messages in thread
From: David Sterba @ 2017-11-15 17:01 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: Anand Jain, linux-btrfs

On Thu, Nov 09, 2017 at 05:53:58PM +0200, Nikolay Borisov wrote:
> On  9.11.2017 17:45, Anand Jain wrote:
> > A preparation patch to create the actual device open in a new function.
> Just say you are reducing the visibility of some variables to the loop.

This patch is IMHO too fine-grained and can be folded with the last one.
A preparatory change like this would make sense if there are some
non-obvious consequences, but here it's just moving the declaration
block to a new function, that was easy to review.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/4] factor __btrfs_open_devices()
  2017-11-09 15:45 [PATCH 0/4] factor __btrfs_open_devices() Anand Jain
                   ` (3 preceding siblings ...)
  2017-11-09 15:45 ` [PATCH 4/4] btrfs: factor __btrfs_open_devices() to create btrfs_open_one_device() Anand Jain
@ 2017-11-15 17:03 ` David Sterba
  2017-11-16  4:08   ` Anand Jain
  4 siblings, 1 reply; 13+ messages in thread
From: David Sterba @ 2017-11-15 17:03 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Thu, Nov 09, 2017 at 11:45:22PM +0800, Anand Jain wrote:
> This is in preparation to help bring the missing device back to the
> alloc list dynamically. As of now if we run 'btrfs dev scan </dev/missing>'
> on a mounted FS, nothing happens, there is no code which handles this
> condition. So the idea is to fix it by running through the device open
> process for the reappeared missing device.
> 
> So this patch separates device open steps into a separate function so that
> it can be called for a device, instead of for all the devices in the
> dev_list. As this function is in the critical mount section, and to show
> the src code changes as clearly as possible, I have created 4 no-functional
> changes patches, which can be merged together if needed.

The splitting is fine, I've only merged 2 and 4 to one, but otherwise
it's clear from each patch if there are no functional changes. 1,3,2+4
added to next, thanks.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/4] factor __btrfs_open_devices()
  2017-11-15 17:03 ` [PATCH 0/4] factor __btrfs_open_devices() David Sterba
@ 2017-11-16  4:08   ` Anand Jain
  0 siblings, 0 replies; 13+ messages in thread
From: Anand Jain @ 2017-11-16  4:08 UTC (permalink / raw)
  To: dsterba, linux-btrfs



On 11/16/2017 01:03 AM, David Sterba wrote:
> On Thu, Nov 09, 2017 at 11:45:22PM +0800, Anand Jain wrote:
>> This is in preparation to help bring the missing device back to the
>> alloc list dynamically. As of now if we run 'btrfs dev scan </dev/missing>'
>> on a mounted FS, nothing happens, there is no code which handles this
>> condition. So the idea is to fix it by running through the device open
>> process for the reappeared missing device.
>>
>> So this patch separates device open steps into a separate function so that
>> it can be called for a device, instead of for all the devices in the
>> dev_list. As this function is in the critical mount section, and to show
>> the src code changes as clearly as possible, I have created 4 no-functional
>> changes patches, which can be merged together if needed.
> 
> The splitting is fine, I've only merged 2 and 4 to one, but otherwise
> it's clear from each patch if there are no functional changes. 1,3,2+4
> added to next, thanks.

Thanks indeed. (git-diff showed changes which didn't make sense,
I had to split, merge is fine).

Thanks, Anand

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] btrfs: factor __btrfs_open_devices() to create btrfs_open_one_device()
  2017-11-09 15:45 ` [PATCH 4/4] btrfs: factor __btrfs_open_devices() to create btrfs_open_one_device() Anand Jain
@ 2017-11-27  6:47   ` Lakshmipathi.G
  2017-11-27 11:47     ` Anand Jain
  2017-11-27 14:00     ` [PATCH] btrfs: ignore return from btrfs_open_one_device() Anand Jain
  0 siblings, 2 replies; 13+ messages in thread
From: Lakshmipathi.G @ 2017-11-27  6:47 UTC (permalink / raw)
  To: Anand Jain; +Cc: btrfs

Hi Anand,

With this patch applied, btrfs-progs/misc-test/021 error out. Is this
same for you?

Without this patch: https://asciinema.org/a/RJmE5469mHlL3S1BIOCifWVn6
With this patch:    https://asciinema.org/a/1h5UX6DIFNsvvMXgLo4GiEgdE

thanks!
----
Cheers,
Lakshmipathi.G
http://www.giis.co.in http://www.webminal.org


On Thu, Nov 9, 2017 at 9:15 PM, Anand Jain <anand.jain@oracle.com> wrote:
> No functional changes, create btrfs_open_one_device() from
> __btrfs_open_devices(). This is a preparatory work to add dynamic
> device scan.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/volumes.c | 126 +++++++++++++++++++++++++++++------------------------
>  1 file changed, 69 insertions(+), 57 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 0857b580014d..d24e966ee29f 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -601,6 +601,73 @@ void btrfs_free_stale_device(struct btrfs_device *cur_dev)
>         }
>  }
>
> +static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
> +                       struct btrfs_device *device, fmode_t flags,
> +                       void *holder)
> +{
> +       struct request_queue *q;
> +       struct block_device *bdev;
> +       struct buffer_head *bh;
> +       struct btrfs_super_block *disk_super;
> +       u64 devid;
> +       int ret;
> +
> +       if (device->bdev)
> +               return -EINVAL;
> +       if (!device->name)
> +               return -EINVAL;
> +
> +       ret = btrfs_get_bdev_and_sb(device->name->str, flags, holder, 1,
> +                                   &bdev, &bh);
> +       if (ret)
> +               return ret;
> +
> +       disk_super = (struct btrfs_super_block *)bh->b_data;
> +       devid = btrfs_stack_device_id(&disk_super->dev_item);
> +       if (devid != device->devid)
> +               goto error_brelse;
> +
> +       if (memcmp(device->uuid, disk_super->dev_item.uuid,
> +                  BTRFS_UUID_SIZE))
> +               goto error_brelse;
> +
> +       device->generation = btrfs_super_generation(disk_super);
> +
> +       if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING) {
> +               device->writeable = 0;
> +               fs_devices->seeding = 1;
> +       } else {
> +               device->writeable = !bdev_read_only(bdev);
> +       }
> +
> +       q = bdev_get_queue(bdev);
> +       if (blk_queue_discard(q))
> +               device->can_discard = 1;
> +       if (!blk_queue_nonrot(q))
> +               fs_devices->rotating = 1;
> +
> +       device->bdev = bdev;
> +       device->in_fs_metadata = 0;
> +       device->mode = flags;
> +
> +       fs_devices->open_devices++;
> +       if (device->writeable &&
> +           device->devid != BTRFS_DEV_REPLACE_DEVID) {
> +               fs_devices->rw_devices++;
> +               list_add(&device->dev_alloc_list,
> +                        &fs_devices->alloc_list);
> +       }
> +       brelse(bh);
> +
> +       return 0;
> +
> +error_brelse:
> +       brelse(bh);
> +       blkdev_put(bdev, flags);
> +
> +       return -EINVAL;
> +}
> +
>  /*
>   * Add new device to list of registered devices
>   *
> @@ -978,69 +1045,14 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
>         flags |= FMODE_EXCL;
>
>         list_for_each_entry(device, head, dev_list) {
> -               struct request_queue *q;
> -               struct block_device *bdev;
> -               struct buffer_head *bh;
> -               struct btrfs_super_block *disk_super;
> -               u64 devid;
> -
> -               if (device->bdev)
> -                       continue;
> -               if (!device->name)
> -                       continue;
> -
>                 /* Just open everything we can; ignore failures here */
> -               if (btrfs_get_bdev_and_sb(device->name->str, flags, holder, 1,
> -                                           &bdev, &bh))
> +               ret = btrfs_open_one_device(fs_devices, device, flags, holder);
> +               if (ret)
>                         continue;
>
> -               disk_super = (struct btrfs_super_block *)bh->b_data;
> -               devid = btrfs_stack_device_id(&disk_super->dev_item);
> -               if (devid != device->devid)
> -                       goto error_brelse;
> -
> -               if (memcmp(device->uuid, disk_super->dev_item.uuid,
> -                          BTRFS_UUID_SIZE))
> -                       goto error_brelse;
> -
> -               device->generation = btrfs_super_generation(disk_super);
> -
> -               if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING) {
> -                       device->writeable = 0;
> -                       fs_devices->seeding = 1;
> -               } else {
> -                       device->writeable = !bdev_read_only(bdev);
> -               }
> -
> -               q = bdev_get_queue(bdev);
> -               if (blk_queue_discard(q))
> -                       device->can_discard = 1;
> -               if (!blk_queue_nonrot(q))
> -                       fs_devices->rotating = 1;
> -
> -               device->bdev = bdev;
> -               device->in_fs_metadata = 0;
> -               device->mode = flags;
> -
> -               fs_devices->open_devices++;
> -               if (device->writeable &&
> -                   device->devid != BTRFS_DEV_REPLACE_DEVID) {
> -                       fs_devices->rw_devices++;
> -                       list_add(&device->dev_alloc_list,
> -                                &fs_devices->alloc_list);
> -               }
> -               brelse(bh);
> -
>                 if (!latest_dev ||
>                     device->generation > latest_dev->generation)
>                         latest_dev = device;
> -
> -               continue;
> -
> -error_brelse:
> -               brelse(bh);
> -               blkdev_put(bdev, flags);
> -               continue;
>         }
>         if (fs_devices->open_devices == 0) {
>                 ret = -EINVAL;
> --
> 2.13.1
>
> --
> 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] btrfs: factor __btrfs_open_devices() to create btrfs_open_one_device()
  2017-11-27  6:47   ` Lakshmipathi.G
@ 2017-11-27 11:47     ` Anand Jain
  2017-11-29 16:02       ` Lakshmipathi.G
  2017-11-27 14:00     ` [PATCH] btrfs: ignore return from btrfs_open_one_device() Anand Jain
  1 sibling, 1 reply; 13+ messages in thread
From: Anand Jain @ 2017-11-27 11:47 UTC (permalink / raw)
  To: Lakshmipathi.G; +Cc: btrfs


Hi Lakshmipathi,

  Oops I can see the same. I am trying to narrow down.

Thanks, Anand

On 11/27/2017 02:47 PM, Lakshmipathi.G wrote:
> Hi Anand,
> 
> With this patch applied, btrfs-progs/misc-test/021 error out. Is this
> same for you?
> 
> Without this patch: https://asciinema.org/a/RJmE5469mHlL3S1BIOCifWVn6
> With this patch:    https://asciinema.org/a/1h5UX6DIFNsvvMXgLo4GiEgdE
> 
> thanks!
> ----
> Cheers,
> Lakshmipathi.G
> http://www.giis.co.in http://www.webminal.org
> 
> 
> On Thu, Nov 9, 2017 at 9:15 PM, Anand Jain <anand.jain@oracle.com> wrote:
>> No functional changes, create btrfs_open_one_device() from
>> __btrfs_open_devices(). This is a preparatory work to add dynamic
>> device scan.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   fs/btrfs/volumes.c | 126 +++++++++++++++++++++++++++++------------------------
>>   1 file changed, 69 insertions(+), 57 deletions(-)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 0857b580014d..d24e966ee29f 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -601,6 +601,73 @@ void btrfs_free_stale_device(struct btrfs_device *cur_dev)
>>          }
>>   }
>>
>> +static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
>> +                       struct btrfs_device *device, fmode_t flags,
>> +                       void *holder)
>> +{
>> +       struct request_queue *q;
>> +       struct block_device *bdev;
>> +       struct buffer_head *bh;
>> +       struct btrfs_super_block *disk_super;
>> +       u64 devid;
>> +       int ret;
>> +
>> +       if (device->bdev)
>> +               return -EINVAL;
>> +       if (!device->name)
>> +               return -EINVAL;
>> +
>> +       ret = btrfs_get_bdev_and_sb(device->name->str, flags, holder, 1,
>> +                                   &bdev, &bh);
>> +       if (ret)
>> +               return ret;
>> +
>> +       disk_super = (struct btrfs_super_block *)bh->b_data;
>> +       devid = btrfs_stack_device_id(&disk_super->dev_item);
>> +       if (devid != device->devid)
>> +               goto error_brelse;
>> +
>> +       if (memcmp(device->uuid, disk_super->dev_item.uuid,
>> +                  BTRFS_UUID_SIZE))
>> +               goto error_brelse;
>> +
>> +       device->generation = btrfs_super_generation(disk_super);
>> +
>> +       if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING) {
>> +               device->writeable = 0;
>> +               fs_devices->seeding = 1;
>> +       } else {
>> +               device->writeable = !bdev_read_only(bdev);
>> +       }
>> +
>> +       q = bdev_get_queue(bdev);
>> +       if (blk_queue_discard(q))
>> +               device->can_discard = 1;
>> +       if (!blk_queue_nonrot(q))
>> +               fs_devices->rotating = 1;
>> +
>> +       device->bdev = bdev;
>> +       device->in_fs_metadata = 0;
>> +       device->mode = flags;
>> +
>> +       fs_devices->open_devices++;
>> +       if (device->writeable &&
>> +           device->devid != BTRFS_DEV_REPLACE_DEVID) {
>> +               fs_devices->rw_devices++;
>> +               list_add(&device->dev_alloc_list,
>> +                        &fs_devices->alloc_list);
>> +       }
>> +       brelse(bh);
>> +
>> +       return 0;
>> +
>> +error_brelse:
>> +       brelse(bh);
>> +       blkdev_put(bdev, flags);
>> +
>> +       return -EINVAL;
>> +}
>> +
>>   /*
>>    * Add new device to list of registered devices
>>    *
>> @@ -978,69 +1045,14 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
>>          flags |= FMODE_EXCL;
>>
>>          list_for_each_entry(device, head, dev_list) {
>> -               struct request_queue *q;
>> -               struct block_device *bdev;
>> -               struct buffer_head *bh;
>> -               struct btrfs_super_block *disk_super;
>> -               u64 devid;
>> -
>> -               if (device->bdev)
>> -                       continue;
>> -               if (!device->name)
>> -                       continue;
>> -
>>                  /* Just open everything we can; ignore failures here */
>> -               if (btrfs_get_bdev_and_sb(device->name->str, flags, holder, 1,
>> -                                           &bdev, &bh))
>> +               ret = btrfs_open_one_device(fs_devices, device, flags, holder);
>> +               if (ret)
>>                          continue;
>>
>> -               disk_super = (struct btrfs_super_block *)bh->b_data;
>> -               devid = btrfs_stack_device_id(&disk_super->dev_item);
>> -               if (devid != device->devid)
>> -                       goto error_brelse;
>> -
>> -               if (memcmp(device->uuid, disk_super->dev_item.uuid,
>> -                          BTRFS_UUID_SIZE))
>> -                       goto error_brelse;
>> -
>> -               device->generation = btrfs_super_generation(disk_super);
>> -
>> -               if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING) {
>> -                       device->writeable = 0;
>> -                       fs_devices->seeding = 1;
>> -               } else {
>> -                       device->writeable = !bdev_read_only(bdev);
>> -               }
>> -
>> -               q = bdev_get_queue(bdev);
>> -               if (blk_queue_discard(q))
>> -                       device->can_discard = 1;
>> -               if (!blk_queue_nonrot(q))
>> -                       fs_devices->rotating = 1;
>> -
>> -               device->bdev = bdev;
>> -               device->in_fs_metadata = 0;
>> -               device->mode = flags;
>> -
>> -               fs_devices->open_devices++;
>> -               if (device->writeable &&
>> -                   device->devid != BTRFS_DEV_REPLACE_DEVID) {
>> -                       fs_devices->rw_devices++;
>> -                       list_add(&device->dev_alloc_list,
>> -                                &fs_devices->alloc_list);
>> -               }
>> -               brelse(bh);
>> -
>>                  if (!latest_dev ||
>>                      device->generation > latest_dev->generation)
>>                          latest_dev = device;
>> -
>> -               continue;
>> -
>> -error_brelse:
>> -               brelse(bh);
>> -               blkdev_put(bdev, flags);
>> -               continue;
>>          }
>>          if (fs_devices->open_devices == 0) {
>>                  ret = -EINVAL;
>> --
>> 2.13.1
>>
>> --
>> 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
> --
> 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
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH] btrfs: ignore return from btrfs_open_one_device()
  2017-11-27  6:47   ` Lakshmipathi.G
  2017-11-27 11:47     ` Anand Jain
@ 2017-11-27 14:00     ` Anand Jain
  1 sibling, 0 replies; 13+ messages in thread
From: Anand Jain @ 2017-11-27 14:00 UTC (permalink / raw)
  To: linux-btrfs, dsterba; +Cc: lakshmipathi.g

Test case btrfs-progs test-misc/012 can recreate the same fsid with
different number of struct btrfs_fs_devices::total_devices. And the
previous device which is in the kernel device list is stale now. But
as we don't clean the kernel device list (unless same device is scanned
with different fsid), so in the mount context it really ended up
reading the device to find zeroed SB. And thus return the fail to mount.

The long term fix for this should be to refresh the kernel device list.

Though the patch
  btrfs: factor __btrfs_open_devices() to create btrfs_open_one_device()
did tried to skip the failed open, but it forgot to reset the ret value
or not to assign, thus error went up the stack in the mount context.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Fixes:
  btrfs: factor __btrfs_open_devices() to create btrfs_open_one_device()
---
Hi David,

 My bad. The above patch introduced a regression. Can you kindly squash
 this patch to it.

Thanks, Anand

 fs/btrfs/volumes.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 5bd73edc2602..a3fa2dc39881 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1047,8 +1047,7 @@ static int __btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 
 	list_for_each_entry(device, head, dev_list) {
 		/* Just open everything we can; ignore failures here */
-		ret = btrfs_open_one_device(fs_devices, device, flags, holder);
-		if (ret)
+		if (btrfs_open_one_device(fs_devices, device, flags, holder))
 			continue;
 
 		if (!latest_dev ||
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] btrfs: factor __btrfs_open_devices() to create btrfs_open_one_device()
  2017-11-27 11:47     ` Anand Jain
@ 2017-11-29 16:02       ` Lakshmipathi.G
  0 siblings, 0 replies; 13+ messages in thread
From: Lakshmipathi.G @ 2017-11-29 16:02 UTC (permalink / raw)
  To: Anand Jain; +Cc: btrfs

Hi Anand,

Applied your fix, now the issue resolved. thanks!

----
Cheers,
Lakshmipathi.G
http://www.giis.co.in http://www.webminal.org


On Mon, Nov 27, 2017 at 5:17 PM, Anand Jain <anand.jain@oracle.com> wrote:
>
> Hi Lakshmipathi,
>
>  Oops I can see the same. I am trying to narrow down.
>
> Thanks, Anand
>
>
> On 11/27/2017 02:47 PM, Lakshmipathi.G wrote:
>>
>> Hi Anand,
>>
>> With this patch applied, btrfs-progs/misc-test/021 error out. Is this
>> same for you?
>>
>> Without this patch: https://asciinema.org/a/RJmE5469mHlL3S1BIOCifWVn6
>> With this patch:    https://asciinema.org/a/1h5UX6DIFNsvvMXgLo4GiEgdE
>>
>> thanks!
>> ----
>> Cheers,
>> Lakshmipathi.G
>> http://www.giis.co.in http://www.webminal.org
>>
>>
>> On Thu, Nov 9, 2017 at 9:15 PM, Anand Jain <anand.jain@oracle.com> wrote:
>>>
>>> No functional changes, create btrfs_open_one_device() from
>>> __btrfs_open_devices(). This is a preparatory work to add dynamic
>>> device scan.
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>>   fs/btrfs/volumes.c | 126
>>> +++++++++++++++++++++++++++++------------------------
>>>   1 file changed, 69 insertions(+), 57 deletions(-)
>>>
>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>> index 0857b580014d..d24e966ee29f 100644
>>> --- a/fs/btrfs/volumes.c
>>> +++ b/fs/btrfs/volumes.c
>>> @@ -601,6 +601,73 @@ void btrfs_free_stale_device(struct btrfs_device
>>> *cur_dev)
>>>          }
>>>   }
>>>
>>> +static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
>>> +                       struct btrfs_device *device, fmode_t flags,
>>> +                       void *holder)
>>> +{
>>> +       struct request_queue *q;
>>> +       struct block_device *bdev;
>>> +       struct buffer_head *bh;
>>> +       struct btrfs_super_block *disk_super;
>>> +       u64 devid;
>>> +       int ret;
>>> +
>>> +       if (device->bdev)
>>> +               return -EINVAL;
>>> +       if (!device->name)
>>> +               return -EINVAL;
>>> +
>>> +       ret = btrfs_get_bdev_and_sb(device->name->str, flags, holder, 1,
>>> +                                   &bdev, &bh);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       disk_super = (struct btrfs_super_block *)bh->b_data;
>>> +       devid = btrfs_stack_device_id(&disk_super->dev_item);
>>> +       if (devid != device->devid)
>>> +               goto error_brelse;
>>> +
>>> +       if (memcmp(device->uuid, disk_super->dev_item.uuid,
>>> +                  BTRFS_UUID_SIZE))
>>> +               goto error_brelse;
>>> +
>>> +       device->generation = btrfs_super_generation(disk_super);
>>> +
>>> +       if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING) {
>>> +               device->writeable = 0;
>>> +               fs_devices->seeding = 1;
>>> +       } else {
>>> +               device->writeable = !bdev_read_only(bdev);
>>> +       }
>>> +
>>> +       q = bdev_get_queue(bdev);
>>> +       if (blk_queue_discard(q))
>>> +               device->can_discard = 1;
>>> +       if (!blk_queue_nonrot(q))
>>> +               fs_devices->rotating = 1;
>>> +
>>> +       device->bdev = bdev;
>>> +       device->in_fs_metadata = 0;
>>> +       device->mode = flags;
>>> +
>>> +       fs_devices->open_devices++;
>>> +       if (device->writeable &&
>>> +           device->devid != BTRFS_DEV_REPLACE_DEVID) {
>>> +               fs_devices->rw_devices++;
>>> +               list_add(&device->dev_alloc_list,
>>> +                        &fs_devices->alloc_list);
>>> +       }
>>> +       brelse(bh);
>>> +
>>> +       return 0;
>>> +
>>> +error_brelse:
>>> +       brelse(bh);
>>> +       blkdev_put(bdev, flags);
>>> +
>>> +       return -EINVAL;
>>> +}
>>> +
>>>   /*
>>>    * Add new device to list of registered devices
>>>    *
>>> @@ -978,69 +1045,14 @@ static int __btrfs_open_devices(struct
>>> btrfs_fs_devices *fs_devices,
>>>          flags |= FMODE_EXCL;
>>>
>>>          list_for_each_entry(device, head, dev_list) {
>>> -               struct request_queue *q;
>>> -               struct block_device *bdev;
>>> -               struct buffer_head *bh;
>>> -               struct btrfs_super_block *disk_super;
>>> -               u64 devid;
>>> -
>>> -               if (device->bdev)
>>> -                       continue;
>>> -               if (!device->name)
>>> -                       continue;
>>> -
>>>                  /* Just open everything we can; ignore failures here */
>>> -               if (btrfs_get_bdev_and_sb(device->name->str, flags,
>>> holder, 1,
>>> -                                           &bdev, &bh))
>>> +               ret = btrfs_open_one_device(fs_devices, device, flags,
>>> holder);
>>> +               if (ret)
>>>                          continue;
>>>
>>> -               disk_super = (struct btrfs_super_block *)bh->b_data;
>>> -               devid = btrfs_stack_device_id(&disk_super->dev_item);
>>> -               if (devid != device->devid)
>>> -                       goto error_brelse;
>>> -
>>> -               if (memcmp(device->uuid, disk_super->dev_item.uuid,
>>> -                          BTRFS_UUID_SIZE))
>>> -                       goto error_brelse;
>>> -
>>> -               device->generation = btrfs_super_generation(disk_super);
>>> -
>>> -               if (btrfs_super_flags(disk_super) &
>>> BTRFS_SUPER_FLAG_SEEDING) {
>>> -                       device->writeable = 0;
>>> -                       fs_devices->seeding = 1;
>>> -               } else {
>>> -                       device->writeable = !bdev_read_only(bdev);
>>> -               }
>>> -
>>> -               q = bdev_get_queue(bdev);
>>> -               if (blk_queue_discard(q))
>>> -                       device->can_discard = 1;
>>> -               if (!blk_queue_nonrot(q))
>>> -                       fs_devices->rotating = 1;
>>> -
>>> -               device->bdev = bdev;
>>> -               device->in_fs_metadata = 0;
>>> -               device->mode = flags;
>>> -
>>> -               fs_devices->open_devices++;
>>> -               if (device->writeable &&
>>> -                   device->devid != BTRFS_DEV_REPLACE_DEVID) {
>>> -                       fs_devices->rw_devices++;
>>> -                       list_add(&device->dev_alloc_list,
>>> -                                &fs_devices->alloc_list);
>>> -               }
>>> -               brelse(bh);
>>> -
>>>                  if (!latest_dev ||
>>>                      device->generation > latest_dev->generation)
>>>                          latest_dev = device;
>>> -
>>> -               continue;
>>> -
>>> -error_brelse:
>>> -               brelse(bh);
>>> -               blkdev_put(bdev, flags);
>>> -               continue;
>>>          }
>>>          if (fs_devices->open_devices == 0) {
>>>                  ret = -EINVAL;
>>> --
>>> 2.13.1
>>>
>>> --
>>> 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
>>
>> --
>> 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
>>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-11-29 16:03 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-09 15:45 [PATCH 0/4] factor __btrfs_open_devices() Anand Jain
2017-11-09 15:45 ` [PATCH 1/4] btrfs: set fs_devices->seed directly Anand Jain
2017-11-09 15:45 ` [PATCH 2/4] btrfs: let variable required be declared inside the loop Anand Jain
2017-11-09 15:53   ` Nikolay Borisov
2017-11-15 17:01     ` David Sterba
2017-11-09 15:45 ` [PATCH 3/4] btrfs: move check for device generation to the last Anand Jain
2017-11-09 15:45 ` [PATCH 4/4] btrfs: factor __btrfs_open_devices() to create btrfs_open_one_device() Anand Jain
2017-11-27  6:47   ` Lakshmipathi.G
2017-11-27 11:47     ` Anand Jain
2017-11-29 16:02       ` Lakshmipathi.G
2017-11-27 14:00     ` [PATCH] btrfs: ignore return from btrfs_open_one_device() Anand Jain
2017-11-15 17:03 ` [PATCH 0/4] factor __btrfs_open_devices() David Sterba
2017-11-16  4:08   ` Anand Jain

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.