* [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
* 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
* [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 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
* 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
* [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 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
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.