* Re: [PATCH v3 05/14] md: raid1: don't use bio's vec table to manage resync pages
From: NeilBrown @ 2017-07-10 4:38 UTC (permalink / raw)
To: Ming Lei, Ming Lei
Cc: Shaohua Li, Jens Axboe,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT, linux-block,
Christoph Hellwig
In-Reply-To: <20170710041304.GB15321@ming.t460p>
[-- Attachment #1: Type: text/plain, Size: 1618 bytes --]
On Mon, Jul 10 2017, Ming Lei wrote:
> On Mon, Jul 10, 2017 at 11:35:12AM +0800, Ming Lei wrote:
>> On Mon, Jul 10, 2017 at 7:09 AM, NeilBrown <neilb@suse.com> wrote:
...
>> >> +
>> >> + rp->idx = 0;
>> >
>> > This is the only place the ->idx is initialized, in r1buf_pool_alloc().
>> > The mempool alloc function is suppose to allocate memory, not initialize
>> > it.
>> >
>> > If the mempool_alloc() call cannot allocate memory it will use memory
>> > from the pool. If this memory has already been used, then it will no
>> > longer have the initialized value.
>> >
>> > In short: you need to initialise memory *after* calling
>> > mempool_alloc(), unless you ensure it is reset to the init values before
>> > calling mempool_free().
>> >
>> > https://bugzilla.kernel.org/show_bug.cgi?id=196307
>>
>> OK, thanks for posting it out.
>>
>> Another fix might be to reinitialize the variable(rp->idx = 0) in
>> r1buf_pool_free().
>> Or just set it as zero every time when it is used.
>>
>> But I don't understand why mempool_free() calls pool->free() at the end of
>> this function, which may cause to run pool->free() on a new allocated buf,
>> seems a bug in mempool?
>
> Looks I missed the 'return' in mempool_free(), so it is fine.
>
> How about the following fix?
It looks like it would probably work, but it is rather unusual to
initialise something just before freeing it.
Couldn't you just move the initialization to shortly after the
mempool_alloc() call. There looks like a good place that already loops
over all the bios....
Thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH v3 05/14] md: raid1: don't use bio's vec table to manage resync pages
From: Ming Lei @ 2017-07-10 7:25 UTC (permalink / raw)
To: NeilBrown
Cc: Ming Lei, Shaohua Li, Jens Axboe,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT, linux-block,
Christoph Hellwig
In-Reply-To: <87h8yk6h50.fsf@notabene.neil.brown.name>
On Mon, Jul 10, 2017 at 02:38:19PM +1000, NeilBrown wrote:
> On Mon, Jul 10 2017, Ming Lei wrote:
>
> > On Mon, Jul 10, 2017 at 11:35:12AM +0800, Ming Lei wrote:
> >> On Mon, Jul 10, 2017 at 7:09 AM, NeilBrown <neilb@suse.com> wrote:
> ...
> >> >> +
> >> >> + rp->idx = 0;
> >> >
> >> > This is the only place the ->idx is initialized, in r1buf_pool_alloc().
> >> > The mempool alloc function is suppose to allocate memory, not initialize
> >> > it.
> >> >
> >> > If the mempool_alloc() call cannot allocate memory it will use memory
> >> > from the pool. If this memory has already been used, then it will no
> >> > longer have the initialized value.
> >> >
> >> > In short: you need to initialise memory *after* calling
> >> > mempool_alloc(), unless you ensure it is reset to the init values before
> >> > calling mempool_free().
> >> >
> >> > https://bugzilla.kernel.org/show_bug.cgi?id=196307
> >>
> >> OK, thanks for posting it out.
> >>
> >> Another fix might be to reinitialize the variable(rp->idx = 0) in
> >> r1buf_pool_free().
> >> Or just set it as zero every time when it is used.
> >>
> >> But I don't understand why mempool_free() calls pool->free() at the end of
> >> this function, which may cause to run pool->free() on a new allocated buf,
> >> seems a bug in mempool?
> >
> > Looks I missed the 'return' in mempool_free(), so it is fine.
> >
> > How about the following fix?
>
> It looks like it would probably work, but it is rather unusual to
> initialise something just before freeing it.
>
> Couldn't you just move the initialization to shortly after the
> mempool_alloc() call. There looks like a good place that already loops
> over all the bios....
OK, follows the revised patch according to your suggestion.
---
From 68f9936635b3dda13c87a6b6125ac543145bb940 Mon Sep 17 00:00:00 2001
From: Ming Lei <ming.lei@redhat.com>
Date: Mon, 10 Jul 2017 15:16:16 +0800
Subject: [PATCH] MD: move initialization of resync pages' index out of mempool
allocator
mempool_alloc() is only responsible for allocation, not for initialization,
so we need to move the initialization of resync pages's index out of the
allocator function.
Reported-by: NeilBrown <neilb@suse.com>
Fixes: f0250618361d(md: raid10: don't use bio's vec table to manage resync pages)
Fixes: 98d30c5812c3(md: raid1: don't use bio's vec table to manage resync pages)
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/md/raid1.c | 4 +++-
drivers/md/raid10.c | 6 +++++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index e1a7e3d4c5e4..26f5efba0504 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -170,7 +170,6 @@ static void * r1buf_pool_alloc(gfp_t gfp_flags, void *data)
resync_get_all_pages(rp);
}
- rp->idx = 0;
rp->raid_bio = r1_bio;
bio->bi_private = rp;
}
@@ -2698,6 +2697,9 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
struct md_rdev *rdev;
bio = r1_bio->bios[i];
+ /* This initialization should follow mempool_alloc() */
+ get_resync_pages(bio)->idx = 0;
+
rdev = rcu_dereference(conf->mirrors[i].rdev);
if (rdev == NULL ||
test_bit(Faulty, &rdev->flags)) {
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 797ed60abd5e..5ebcb7487284 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -221,7 +221,6 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
resync_get_all_pages(rp);
}
- rp->idx = 0;
rp->raid_bio = r10_bio;
bio->bi_private = rp;
if (rbio) {
@@ -3095,6 +3094,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
bio = r10_bio->devs[0].bio;
bio->bi_next = biolist;
biolist = bio;
+ get_resync_pages(bio)->idx = 0;
bio->bi_end_io = end_sync_read;
bio_set_op_attrs(bio, REQ_OP_READ, 0);
if (test_bit(FailFast, &rdev->flags))
@@ -3120,6 +3120,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
bio = r10_bio->devs[1].bio;
bio->bi_next = biolist;
biolist = bio;
+ get_resync_pages(bio)->idx = 0;
bio->bi_end_io = end_sync_write;
bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
bio->bi_iter.bi_sector = to_addr
@@ -3146,6 +3147,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
break;
bio->bi_next = biolist;
biolist = bio;
+ get_resync_pages(bio)->idx = 0;
bio->bi_end_io = end_sync_write;
bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
bio->bi_iter.bi_sector = to_addr +
@@ -3291,6 +3293,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
atomic_inc(&r10_bio->remaining);
bio->bi_next = biolist;
biolist = bio;
+ get_resync_pages(bio)->idx = 0;
bio->bi_end_io = end_sync_read;
bio_set_op_attrs(bio, REQ_OP_READ, 0);
if (test_bit(FailFast, &conf->mirrors[d].rdev->flags))
@@ -3314,6 +3317,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
sector = r10_bio->devs[i].addr;
bio->bi_next = biolist;
biolist = bio;
+ get_resync_pages(bio)->idx = 0;
bio->bi_end_io = end_sync_write;
bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
if (test_bit(FailFast, &conf->mirrors[d].rdev->flags))
--
2.9.4
--
Ming
^ permalink raw reply related
* Re: Linear device of two arrays
From: Veljko @ 2017-07-10 11:03 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <87van15j9b.fsf@notabene.neil.brown.name>
On 07/10/2017 12:37 AM, NeilBrown wrote:
> I wasn't clear to me that I needed to chime in.. and the complete lack
> of details (not even an "mdadm --examine" output), meant I could only
> answer in vague generalizations.
> However, seeing you asked.
> If you really want to have a 'linear' of 2 RAID10s, then
> 0/ unmount the xfs filesystem
> 1/ backup the last few megabytes of the device
> dd if=/dev/mdXX of=/safe/place/backup bs=1M skip=$BIGNUM
> 2/ create a linear array of the two RAID10s, ensuring the
> metadata is v1.0, and the dataoffset is zero (should be default with
> 1.0)
> mdadm -C /dev/mdZZ -l linear -n 2 -e 1.0 --data-offset=0 /dev/mdXX /dev/mdYY
> 3/ restore the saved data
> dd of=/dev/mdZZ if=/safe/place/backup bs=1M seek=$BIGNUM
> 4/ grow the xfs filesystem
> 5/ be happy.
>
> I cannot comment on the values of "few" and "$BUGNUM" without seeing
> specifics.
>
> NeilBrown
Thanks for your response, Neil!
md0 is boot (raid1), md1 is root (raid10) and md2 is data (raid10) that
I need to expand. Here are details:
# mdadm --detail /dev/md0
/dev/md0:
Version : 1.2
Creation Time : Mon Sep 10 14:45:11 2012
Raid Level : raid1
Array Size : 488128 (476.77 MiB 499.84 MB)
Used Dev Size : 488128 (476.77 MiB 499.84 MB)
Raid Devices : 2
Total Devices : 2
Persistence : Superblock is persistent
Update Time : Mon Jul 3 11:57:24 2017
State : clean
Active Devices : 2
Working Devices : 2
Failed Devices : 0
Spare Devices : 0
Name : backup1:0 (local to host backup1)
UUID : e5a17766:b4df544d:c2770d6e:214113ec
Events : 302
Number Major Minor RaidDevice State
2 8 18 0 active sync /dev/sdb2
3 8 34 1 active sync /dev/sdc2
# mdadm --detail /dev/md1
/dev/md1:
Version : 1.2
Creation Time : Fri Sep 14 12:39:00 2012
Raid Level : raid10
Array Size : 97590272 (93.07 GiB 99.93 GB)
Used Dev Size : 48795136 (46.53 GiB 49.97 GB)
Raid Devices : 4
Total Devices : 4
Persistence : Superblock is persistent
Update Time : Mon Jul 10 12:30:46 2017
State : clean
Active Devices : 4
Working Devices : 4
Failed Devices : 0
Spare Devices : 0
Layout : near=2
Chunk Size : 512K
Name : backup1:1 (local to host backup1)
UUID : 91560d5a:245bbc56:cc08b0ce:9c78fea1
Events : 1003350
Number Major Minor RaidDevice State
4 8 19 0 active sync set-A /dev/sdb3
6 8 35 1 active sync set-B /dev/sdc3
7 8 50 2 active sync set-A /dev/sdd2
5 8 2 3 active sync set-B /dev/sda2
# mdadm --detail /dev/md2
/dev/md2:
Version : 1.2
Creation Time : Fri Sep 14 12:40:13 2012
Raid Level : raid10
Array Size : 5761631232 (5494.72 GiB 5899.91 GB)
Used Dev Size : 2880815616 (2747.36 GiB 2949.96 GB)
Raid Devices : 4
Total Devices : 4
Persistence : Superblock is persistent
Update Time : Mon Jul 10 12:32:51 2017
State : clean
Active Devices : 4
Working Devices : 4
Failed Devices : 0
Spare Devices : 0
Layout : near=2
Chunk Size : 512K
Name : backup1:2 (local to host backup1)
UUID : f6eeaa57:a55f36ff:6980a62a:d4781e44
Events : 2689040
Number Major Minor RaidDevice State
4 8 20 0 active sync set-A /dev/sdb4
6 8 36 1 active sync set-B /dev/sdc4
7 8 51 2 active sync set-A /dev/sdd3
5 8 3 3 active sync set-B /dev/sda3
And here is examine output for md2 partitions:
# mdadm --examine /dev/sda3
/dev/sda3:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : f6eeaa57:a55f36ff:6980a62a:d4781e44
Name : backup1:2 (local to host backup1)
Creation Time : Fri Sep 14 12:40:13 2012
Raid Level : raid10
Raid Devices : 4
Avail Dev Size : 5762609152 (2747.83 GiB 2950.46 GB)
Array Size : 5761631232 (5494.72 GiB 5899.91 GB)
Used Dev Size : 5761631232 (2747.36 GiB 2949.96 GB)
Data Offset : 262144 sectors
Super Offset : 8 sectors
Unused Space : before=262064 sectors, after=977920 sectors
State : clean
Device UUID : 92beeec2:7ff92b1d:473a9641:2a078b16
Update Time : Mon Jul 10 12:35:53 2017
Checksum : d1abfc30 - correct
Events : 2689040
Layout : near=2
Chunk Size : 512K
Device Role : Active device 3
Array State : AAAA ('A' == active, '.' == missing, 'R' == replacing)
# mdadm --examine /dev/sdb4
/dev/sdb4:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : f6eeaa57:a55f36ff:6980a62a:d4781e44
Name : backup1:2 (local to host backup1)
Creation Time : Fri Sep 14 12:40:13 2012
Raid Level : raid10
Raid Devices : 4
Avail Dev Size : 5761632256 (2747.36 GiB 2949.96 GB)
Array Size : 5761631232 (5494.72 GiB 5899.91 GB)
Used Dev Size : 5761631232 (2747.36 GiB 2949.96 GB)
Data Offset : 262144 sectors
Super Offset : 8 sectors
Unused Space : before=262064 sectors, after=1024 sectors
State : clean
Device UUID : 01e1cb21:01a011a9:85761911:9b4d437a
Update Time : Mon Jul 10 12:37:00 2017
Checksum : ef9b6012 - correct
Events : 2689040
Layout : near=2
Chunk Size : 512K
Device Role : Active device 0
Array State : AAAA ('A' == active, '.' == missing, 'R' == replacing)
# mdadm --examine /dev/sdc4
/dev/sdc4:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : f6eeaa57:a55f36ff:6980a62a:d4781e44
Name : backup1:2 (local to host backup1)
Creation Time : Fri Sep 14 12:40:13 2012
Raid Level : raid10
Raid Devices : 4
Avail Dev Size : 5761632256 (2747.36 GiB 2949.96 GB)
Array Size : 5761631232 (5494.72 GiB 5899.91 GB)
Used Dev Size : 5761631232 (2747.36 GiB 2949.96 GB)
Data Offset : 262144 sectors
Super Offset : 8 sectors
Unused Space : before=262064 sectors, after=1024 sectors
State : clean
Device UUID : 1a2c966f:a78ffaf3:83cf37d4:135087b7
Update Time : Mon Jul 10 12:37:53 2017
Checksum : 88b0f680 - correct
Events : 2689040
Layout : near=2
Chunk Size : 512K
Device Role : Active device 1
Array State : AAAA ('A' == active, '.' == missing, 'R' == replacing)
# mdadm --examine /dev/sdd3
/dev/sdd3:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : f6eeaa57:a55f36ff:6980a62a:d4781e44
Name : backup1:2 (local to host backup1)
Creation Time : Fri Sep 14 12:40:13 2012
Raid Level : raid10
Raid Devices : 4
Avail Dev Size : 5762609152 (2747.83 GiB 2950.46 GB)
Array Size : 5761631232 (5494.72 GiB 5899.91 GB)
Used Dev Size : 5761631232 (2747.36 GiB 2949.96 GB)
Data Offset : 262144 sectors
Super Offset : 8 sectors
Unused Space : before=262064 sectors, after=977920 sectors
State : clean
Device UUID : 52f92e76:15228eee:a20c1ee5:8d4a17d2
Update Time : Mon Jul 10 12:38:24 2017
Checksum : b56275df - correct
Events : 2689040
Layout : near=2
Chunk Size : 512K
Device Role : Active device 2
Array State : AAAA ('A' == active, '.' == missing, 'R' == replacing)
Regards,
Veljko
^ permalink raw reply
* Re: [PATCH 1/5] md: add a 'md_numa_node' module parameter
From: Shaohua Li @ 2017-07-10 17:28 UTC (permalink / raw)
To: Zhengyuan Liu; +Cc: linux-raid, neilb, colyli, liuyun01
In-Reply-To: <1498893658-18409-1-git-send-email-liuzhengyuan@kylinos.cn>
On Sat, Jul 01, 2017 at 03:20:54PM +0800, Zhengyuan Liu wrote:
> This module parameter allows user to control which numa node
> the memory for mainly structures (e.g. mddev, md_rdev,
> request_queue, gendisk) is allocated from. The idea came from
> commit 115485e83f49 ("dm: add 'dm_numa_node' module parameter").
> If we don't set this parameter while loading module, it won't
> affect the md default behavior.
>
> numa_node_id field is added so personality such as raid0 could
> inherit the node from mddev, for other personality which has its
> own handling thread we could add such a module parameter too.
>
> This patch includes a bug fix about parameter range check suggested
> by Coly Li <colyli@suse.de>.
>
> Signed-off-by: Zhengyuan Liu <liuzhengyuan@kylinos.cn>
Can you give an example why this is useful?
If we have several raid arrays, we can't control numa node for them separately.
And in the patch 4/5, one array could have two different parameters to control
memory allocation. this is weird. I have no idea how users use this feature.
If we really want to support numa node, would it be better to deduce the numa
node from the combination of all underlayer disks?
Thanks,
Shaohua
> ---
> drivers/md/md.c | 18 ++++++++++++++----
> drivers/md/md.h | 2 ++
> 2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 092b48f..fae93db 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -184,6 +184,8 @@ static int start_readonly;
> */
> static bool create_on_open = true;
>
> +static int md_numa_node = NUMA_NO_NODE;
> +
> /* bio_clone_mddev
> * like bio_clone, but with a local bio set
> */
> @@ -588,7 +590,7 @@ static struct mddev *mddev_find(dev_t unit)
> }
> spin_unlock(&all_mddevs_lock);
>
> - new = kzalloc(sizeof(*new), GFP_KERNEL);
> + new = kzalloc_node(sizeof(*new), GFP_KERNEL, md_numa_node);
> if (!new)
> return NULL;
>
> @@ -598,6 +600,7 @@ static struct mddev *mddev_find(dev_t unit)
> else
> new->md_minor = MINOR(unit) >> MdpMinorShift;
>
> + new->numa_node_id = md_numa_node;
> mddev_init(new);
>
> goto retry;
> @@ -3387,7 +3390,7 @@ static struct md_rdev *md_import_device(dev_t newdev, int super_format, int supe
> struct md_rdev *rdev;
> sector_t size;
>
> - rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
> + rdev = kzalloc_node(sizeof(*rdev), GFP_KERNEL, md_numa_node);
> if (!rdev)
> return ERR_PTR(-ENOMEM);
>
> @@ -5256,7 +5259,7 @@ static int md_alloc(dev_t dev, char *name)
> mddev->hold_active = UNTIL_STOP;
>
> error = -ENOMEM;
> - mddev->queue = blk_alloc_queue(GFP_KERNEL);
> + mddev->queue = blk_alloc_queue_node(GFP_KERNEL, md_numa_node);
> if (!mddev->queue)
> goto abort;
> mddev->queue->queuedata = mddev;
> @@ -5264,7 +5267,7 @@ static int md_alloc(dev_t dev, char *name)
> blk_queue_make_request(mddev->queue, md_make_request);
> blk_set_stacking_limits(&mddev->queue->limits);
>
> - disk = alloc_disk(1 << shift);
> + disk = alloc_disk_node(1 << shift, md_numa_node);
> if (!disk) {
> blk_cleanup_queue(mddev->queue);
> mddev->queue = NULL;
> @@ -8969,6 +8972,11 @@ static int __init md_init(void)
> register_reboot_notifier(&md_notifier);
> raid_table_header = register_sysctl_table(raid_root_table);
>
> + if (md_numa_node < NUMA_NO_NODE)
> + md_numa_node = NUMA_NO_NODE;
> + else if (md_numa_node > (num_online_nodes() - 1))
> + md_numa_node = num_online_nodes() - 1;
> +
> md_geninit();
> return 0;
>
> @@ -9252,6 +9260,8 @@ module_param_call(start_ro, set_ro, get_ro, NULL, S_IRUSR|S_IWUSR);
> module_param(start_dirty_degraded, int, S_IRUGO|S_IWUSR);
> module_param_call(new_array, add_named_array, NULL, NULL, S_IWUSR);
> module_param(create_on_open, bool, S_IRUSR|S_IWUSR);
> +module_param(md_numa_node, int, S_IRUGO);
> +MODULE_PARM_DESC(md_numa_node, "NUMA node for md device memory allocations");
>
> MODULE_LICENSE("GPL");
> MODULE_DESCRIPTION("MD RAID framework");
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 991f0fe..5c91033 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -459,6 +459,8 @@ struct mddev {
> void (*sync_super)(struct mddev *mddev, struct md_rdev *rdev);
> struct md_cluster_info *cluster_info;
> unsigned int good_device_nr; /* good device num within cluster raid */
> +
> + int numa_node_id;
> };
>
> enum recovery_flags {
> --
> 2.7.4
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" 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
* Re: [PATCH v2] md/bitmap: don't read page from device with Bitmap_sync
From: Shaohua Li @ 2017-07-10 17:29 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: neilb, linux-raid
In-Reply-To: <20170704032030.27665-1-gqjiang@suse.com>
On Tue, Jul 04, 2017 at 11:20:30AM +0800, Guoqing Jiang wrote:
> The device owns Bitmap_sync flag needs recovery
> to become in sync, and read page from this type
> device could get stale status.
>
> Also add comments for Bitmap_sync bit per the
> suggestion from Shaohua and Neil.
>
> Previous disscussion can be found here:
> https://marc.info/?t=149760428900004&r=1&w=2
>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
Applied!
> ---
> drivers/md/bitmap.c | 3 ++-
> drivers/md/md.h | 4 +++-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
> index f4eace5..40f3cd7 100644
> --- a/drivers/md/bitmap.c
> +++ b/drivers/md/bitmap.c
> @@ -156,7 +156,8 @@ static int read_sb_page(struct mddev *mddev, loff_t offset,
>
> rdev_for_each(rdev, mddev) {
> if (! test_bit(In_sync, &rdev->flags)
> - || test_bit(Faulty, &rdev->flags))
> + || test_bit(Faulty, &rdev->flags)
> + || test_bit(Bitmap_sync, &rdev->flags))
> continue;
>
> target = offset + index * (PAGE_SIZE/512);
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 991f0fe..b50eb4a 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -134,7 +134,9 @@ enum flag_bits {
> Faulty, /* device is known to have a fault */
> In_sync, /* device is in_sync with rest of array */
> Bitmap_sync, /* ..actually, not quite In_sync. Need a
> - * bitmap-based recovery to get fully in sync
> + * bitmap-based recovery to get fully in sync.
> + * The bit is only meaningful before device
> + * has been passed to pers->hot_add_disk.
> */
> WriteMostly, /* Avoid reading if at all possible */
> AutoDetected, /* added by auto-detect */
> --
> 2.10.0
>
^ permalink raw reply
* Re: [PATCH v2 1/3] mdadm/test: Refactor and revamp 'test' script
From: Jes Sorensen @ 2017-07-10 17:31 UTC (permalink / raw)
To: Zhilong Liu; +Cc: linux-raid
In-Reply-To: <1497445373-27985-2-git-send-email-zlliu@suse.com>
On 06/14/2017 09:02 AM, Zhilong Liu wrote:
> Adding functions:
> die()
> uniform the abnormal situations that have to abort.
> check_env()
> do various basic checking before running test suite.
> save_log()
> collect array infos, include of dmesg, superblock,
> bitmap and /proc/mdstat.
> main()
> the core function of this script.
>
> Improve functions:
> cleanup()
> clear dmesg and remove the /vat/tmp/mdtest* files.
> mdadm()
> clear superblock once creating or building arrays
> every time, because it's always creating arrays
> many times in a test case.
> check()
> just tidy up with die(), didn't change code meanings.
> testdev()
> add checking $1 must be a block device, add 'return 0'
> in final because this function exists in last line of
> test case, such as tests/05r6tor0.
> do_test()
> add checking abnormal dmesg and changing log management.
> do_help()
> just recommend a better way to print Usage.
> parse_args()
> revamp and improve.
>
> Delete function:
> fast_sync()
> It's no longer used, so get rid of it.
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
Applied!
I fixed the s/vat/var/ issue.
Thanks,
Jes
^ permalink raw reply
* Re: [PATCH v2 2/3] mdadm/test: Add '--raidtype=' to run different raidlevel cases
From: Jes Sorensen @ 2017-07-10 17:35 UTC (permalink / raw)
To: Zhilong Liu; +Cc: linux-raid
In-Reply-To: <1497445373-27985-3-git-send-email-zlliu@suse.com>
On 06/14/2017 09:02 AM, Zhilong Liu wrote:
> It supports to specify the argument of "--raidtype"
> to run the different raid level cases. Details refer
> to the do_help() usage.
> For example: ./test --raidtype=raid1
> could execute all the raid1 test cases under tests/.
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
Applied!
Thanks,
Jes
^ permalink raw reply
* Re: [PATCH v2] Monitor: don't assume mdadm parameter is a block device
From: Jes Sorensen @ 2017-07-10 17:40 UTC (permalink / raw)
To: Tomasz Majchrzak, linux-raid; +Cc: zlliu
In-Reply-To: <1497863993-9725-1-git-send-email-tomasz.majchrzak@intel.com>
On 06/19/2017 05:19 AM, Tomasz Majchrzak wrote:
> If symlink (e.g. /dev/md/raid) is passed as a parameter to mdadm --wait,
> it fails as it's not able to find a corresponding entry in /proc/mdstat
> output. Get parameter file major:minor and look for block device name in
> sysfs. This commit is partial revert of commit 9e04ac1c43e6
> ("mdadm/util: unify stat checking blkdev into function").
>
> Signed-off-by: Tomasz Majchrzak <tomasz.majchrzak@intel.com>
> ---
> Monitor.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> v2:
> corrected referenced commit title in commit message
Applied!
Thanks,
Jes
^ permalink raw reply
* Re: [MD PATCH 1/1 -v2] Raid5 should update rdev->sectors after reshape
From: Shaohua Li @ 2017-07-10 17:43 UTC (permalink / raw)
To: Xiao Ni; +Cc: linux-raid, ncroxon, tbskyd, gqjiang
In-Reply-To: <1499247244-4483-1-git-send-email-xni@redhat.com>
On Wed, Jul 05, 2017 at 05:34:04PM +0800, Xiao Ni wrote:
> The raid5 md device is created by the disks which we don't use the total size. For example,
> the size of the device is 5G and it just uses 3G of the devices to create one raid5 device.
> Then change the chunksize and wait reshape to finish. After reshape finishing stop the raid
> and assemble it again. It fails.
> mdadm -CR /dev/md0 -l5 -n3 /dev/loop[0-2] --size=3G --chunk=32 --assume-clean
> mdadm /dev/md0 --grow --chunk=64
> wait reshape to finish
> mdadm -S /dev/md0
> mdadm -As
> The error messages:
> [197519.814302] md: loop1 does not have a valid v1.2 superblock, not importing!
> [197519.821686] md: md_import_device returned -22
>
> After reshape the data offset is changed. It selects backwards direction in this condition.
> In function super_1_load it compares the available space of the underlying device with
> sb->data_size. The new data offset gets bigger after reshape. So super_1_load returns -EINVAL.
> rdev->sectors is updated in md_finish_reshape. Then sb->data_size is set in super_1_sync based
> on rdev->sectors. So add md_finish_reshape in end_reshape.
>
> Signed-off-by: Xiao Ni <xni@redhat.com>
Applied, thanks! Will add to stable too.
Can you also add the test to mdadm test suite?
> ---
> drivers/md/raid5.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index ec0f951..e7f527c 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -7947,12 +7947,10 @@ static void end_reshape(struct r5conf *conf)
> {
>
> if (!test_bit(MD_RECOVERY_INTR, &conf->mddev->recovery)) {
> - struct md_rdev *rdev;
>
> spin_lock_irq(&conf->device_lock);
> conf->previous_raid_disks = conf->raid_disks;
> - rdev_for_each(rdev, conf->mddev)
> - rdev->data_offset = rdev->new_data_offset;
> + md_finish_reshape(conf->mddev);
> smp_wmb();
> conf->reshape_progress = MaxSector;
> conf->mddev->reshape_position = MaxSector;
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v3 05/14] md: raid1: don't use bio's vec table to manage resync pages
From: Shaohua Li @ 2017-07-10 19:05 UTC (permalink / raw)
To: Ming Lei
Cc: NeilBrown, Ming Lei, Jens Axboe,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT, linux-block,
Christoph Hellwig
In-Reply-To: <20170710072538.GA32208@ming.t460p>
On Mon, Jul 10, 2017 at 03:25:41PM +0800, Ming Lei wrote:
> On Mon, Jul 10, 2017 at 02:38:19PM +1000, NeilBrown wrote:
> > On Mon, Jul 10 2017, Ming Lei wrote:
> >
> > > On Mon, Jul 10, 2017 at 11:35:12AM +0800, Ming Lei wrote:
> > >> On Mon, Jul 10, 2017 at 7:09 AM, NeilBrown <neilb@suse.com> wrote:
> > ...
> > >> >> +
> > >> >> + rp->idx = 0;
> > >> >
> > >> > This is the only place the ->idx is initialized, in r1buf_pool_alloc().
> > >> > The mempool alloc function is suppose to allocate memory, not initialize
> > >> > it.
> > >> >
> > >> > If the mempool_alloc() call cannot allocate memory it will use memory
> > >> > from the pool. If this memory has already been used, then it will no
> > >> > longer have the initialized value.
> > >> >
> > >> > In short: you need to initialise memory *after* calling
> > >> > mempool_alloc(), unless you ensure it is reset to the init values before
> > >> > calling mempool_free().
> > >> >
> > >> > https://bugzilla.kernel.org/show_bug.cgi?id=196307
> > >>
> > >> OK, thanks for posting it out.
> > >>
> > >> Another fix might be to reinitialize the variable(rp->idx = 0) in
> > >> r1buf_pool_free().
> > >> Or just set it as zero every time when it is used.
> > >>
> > >> But I don't understand why mempool_free() calls pool->free() at the end of
> > >> this function, which may cause to run pool->free() on a new allocated buf,
> > >> seems a bug in mempool?
> > >
> > > Looks I missed the 'return' in mempool_free(), so it is fine.
> > >
> > > How about the following fix?
> >
> > It looks like it would probably work, but it is rather unusual to
> > initialise something just before freeing it.
> >
> > Couldn't you just move the initialization to shortly after the
> > mempool_alloc() call. There looks like a good place that already loops
> > over all the bios....
>
> OK, follows the revised patch according to your suggestion.
> ---
>
> From 68f9936635b3dda13c87a6b6125ac543145bb940 Mon Sep 17 00:00:00 2001
> From: Ming Lei <ming.lei@redhat.com>
> Date: Mon, 10 Jul 2017 15:16:16 +0800
> Subject: [PATCH] MD: move initialization of resync pages' index out of mempool
> allocator
>
> mempool_alloc() is only responsible for allocation, not for initialization,
> so we need to move the initialization of resync pages's index out of the
> allocator function.
>
> Reported-by: NeilBrown <neilb@suse.com>
> Fixes: f0250618361d(md: raid10: don't use bio's vec table to manage resync pages)
> Fixes: 98d30c5812c3(md: raid1: don't use bio's vec table to manage resync pages)
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
> drivers/md/raid1.c | 4 +++-
> drivers/md/raid10.c | 6 +++++-
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index e1a7e3d4c5e4..26f5efba0504 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -170,7 +170,6 @@ static void * r1buf_pool_alloc(gfp_t gfp_flags, void *data)
> resync_get_all_pages(rp);
> }
>
> - rp->idx = 0;
> rp->raid_bio = r1_bio;
> bio->bi_private = rp;
> }
> @@ -2698,6 +2697,9 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
> struct md_rdev *rdev;
> bio = r1_bio->bios[i];
>
> + /* This initialization should follow mempool_alloc() */
> + get_resync_pages(bio)->idx = 0;
> +
This is fragile and hard to maintain. Can we add a wrap for the
allocation/init?
Thanks,
Shaohua
> rdev = rcu_dereference(conf->mirrors[i].rdev);
> if (rdev == NULL ||
> test_bit(Faulty, &rdev->flags)) {
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 797ed60abd5e..5ebcb7487284 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -221,7 +221,6 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
> resync_get_all_pages(rp);
> }
>
> - rp->idx = 0;
> rp->raid_bio = r10_bio;
> bio->bi_private = rp;
> if (rbio) {
> @@ -3095,6 +3094,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> bio = r10_bio->devs[0].bio;
> bio->bi_next = biolist;
> biolist = bio;
> + get_resync_pages(bio)->idx = 0;
> bio->bi_end_io = end_sync_read;
> bio_set_op_attrs(bio, REQ_OP_READ, 0);
> if (test_bit(FailFast, &rdev->flags))
> @@ -3120,6 +3120,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> bio = r10_bio->devs[1].bio;
> bio->bi_next = biolist;
> biolist = bio;
> + get_resync_pages(bio)->idx = 0;
> bio->bi_end_io = end_sync_write;
> bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
> bio->bi_iter.bi_sector = to_addr
> @@ -3146,6 +3147,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> break;
> bio->bi_next = biolist;
> biolist = bio;
> + get_resync_pages(bio)->idx = 0;
> bio->bi_end_io = end_sync_write;
> bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
> bio->bi_iter.bi_sector = to_addr +
> @@ -3291,6 +3293,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> atomic_inc(&r10_bio->remaining);
> bio->bi_next = biolist;
> biolist = bio;
> + get_resync_pages(bio)->idx = 0;
> bio->bi_end_io = end_sync_read;
> bio_set_op_attrs(bio, REQ_OP_READ, 0);
> if (test_bit(FailFast, &conf->mirrors[d].rdev->flags))
> @@ -3314,6 +3317,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> sector = r10_bio->devs[i].addr;
> bio->bi_next = biolist;
> biolist = bio;
> + get_resync_pages(bio)->idx = 0;
> bio->bi_end_io = end_sync_write;
> bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
> if (test_bit(FailFast, &conf->mirrors[d].rdev->flags))
> --
> 2.9.4
>
>
>
> --
> Ming
^ permalink raw reply
* Re: [PATCH v3 05/14] md: raid1: don't use bio's vec table to manage resync pages
From: Ming Lei @ 2017-07-10 22:54 UTC (permalink / raw)
To: Shaohua Li
Cc: NeilBrown, Ming Lei, Jens Axboe,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT, linux-block,
Christoph Hellwig
In-Reply-To: <20170710190549.luj7zrnq7mo4x36b@kernel.org>
On Mon, Jul 10, 2017 at 12:05:49PM -0700, Shaohua Li wrote:
> On Mon, Jul 10, 2017 at 03:25:41PM +0800, Ming Lei wrote:
> > On Mon, Jul 10, 2017 at 02:38:19PM +1000, NeilBrown wrote:
> > > On Mon, Jul 10 2017, Ming Lei wrote:
> > >
> > > > On Mon, Jul 10, 2017 at 11:35:12AM +0800, Ming Lei wrote:
> > > >> On Mon, Jul 10, 2017 at 7:09 AM, NeilBrown <neilb@suse.com> wrote:
> > > ...
> > > >> >> +
> > > >> >> + rp->idx = 0;
> > > >> >
> > > >> > This is the only place the ->idx is initialized, in r1buf_pool_alloc().
> > > >> > The mempool alloc function is suppose to allocate memory, not initialize
> > > >> > it.
> > > >> >
> > > >> > If the mempool_alloc() call cannot allocate memory it will use memory
> > > >> > from the pool. If this memory has already been used, then it will no
> > > >> > longer have the initialized value.
> > > >> >
> > > >> > In short: you need to initialise memory *after* calling
> > > >> > mempool_alloc(), unless you ensure it is reset to the init values before
> > > >> > calling mempool_free().
> > > >> >
> > > >> > https://bugzilla.kernel.org/show_bug.cgi?id=196307
> > > >>
> > > >> OK, thanks for posting it out.
> > > >>
> > > >> Another fix might be to reinitialize the variable(rp->idx = 0) in
> > > >> r1buf_pool_free().
> > > >> Or just set it as zero every time when it is used.
> > > >>
> > > >> But I don't understand why mempool_free() calls pool->free() at the end of
> > > >> this function, which may cause to run pool->free() on a new allocated buf,
> > > >> seems a bug in mempool?
> > > >
> > > > Looks I missed the 'return' in mempool_free(), so it is fine.
> > > >
> > > > How about the following fix?
> > >
> > > It looks like it would probably work, but it is rather unusual to
> > > initialise something just before freeing it.
> > >
> > > Couldn't you just move the initialization to shortly after the
> > > mempool_alloc() call. There looks like a good place that already loops
> > > over all the bios....
> >
> > OK, follows the revised patch according to your suggestion.
> > ---
> >
> > From 68f9936635b3dda13c87a6b6125ac543145bb940 Mon Sep 17 00:00:00 2001
> > From: Ming Lei <ming.lei@redhat.com>
> > Date: Mon, 10 Jul 2017 15:16:16 +0800
> > Subject: [PATCH] MD: move initialization of resync pages' index out of mempool
> > allocator
> >
> > mempool_alloc() is only responsible for allocation, not for initialization,
> > so we need to move the initialization of resync pages's index out of the
> > allocator function.
> >
> > Reported-by: NeilBrown <neilb@suse.com>
> > Fixes: f0250618361d(md: raid10: don't use bio's vec table to manage resync pages)
> > Fixes: 98d30c5812c3(md: raid1: don't use bio's vec table to manage resync pages)
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> > drivers/md/raid1.c | 4 +++-
> > drivers/md/raid10.c | 6 +++++-
> > 2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> > index e1a7e3d4c5e4..26f5efba0504 100644
> > --- a/drivers/md/raid1.c
> > +++ b/drivers/md/raid1.c
> > @@ -170,7 +170,6 @@ static void * r1buf_pool_alloc(gfp_t gfp_flags, void *data)
> > resync_get_all_pages(rp);
> > }
> >
> > - rp->idx = 0;
> > rp->raid_bio = r1_bio;
> > bio->bi_private = rp;
> > }
> > @@ -2698,6 +2697,9 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
> > struct md_rdev *rdev;
> > bio = r1_bio->bios[i];
> >
> > + /* This initialization should follow mempool_alloc() */
> > + get_resync_pages(bio)->idx = 0;
> > +
>
> This is fragile and hard to maintain. Can we add a wrap for the
> allocation/init?
The resync pages's index init is done in the following loop
after mempool_alloc(), actually the whole loop is still sort of init,
so I think it isn't fragile, and it should be fine.
--
Ming
^ permalink raw reply
* Re: [PATCH v3 05/14] md: raid1: don't use bio's vec table to manage resync pages
From: NeilBrown @ 2017-07-10 23:14 UTC (permalink / raw)
To: Shaohua Li, Ming Lei
Cc: Ming Lei, Jens Axboe,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT, linux-block,
Christoph Hellwig
In-Reply-To: <20170710190549.luj7zrnq7mo4x36b@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 3622 bytes --]
On Mon, Jul 10 2017, Shaohua Li wrote:
> On Mon, Jul 10, 2017 at 03:25:41PM +0800, Ming Lei wrote:
>> On Mon, Jul 10, 2017 at 02:38:19PM +1000, NeilBrown wrote:
>> > On Mon, Jul 10 2017, Ming Lei wrote:
>> >
>> > > On Mon, Jul 10, 2017 at 11:35:12AM +0800, Ming Lei wrote:
>> > >> On Mon, Jul 10, 2017 at 7:09 AM, NeilBrown <neilb@suse.com> wrote:
>> > ...
>> > >> >> +
>> > >> >> + rp->idx = 0;
>> > >> >
>> > >> > This is the only place the ->idx is initialized, in r1buf_pool_alloc().
>> > >> > The mempool alloc function is suppose to allocate memory, not initialize
>> > >> > it.
>> > >> >
>> > >> > If the mempool_alloc() call cannot allocate memory it will use memory
>> > >> > from the pool. If this memory has already been used, then it will no
>> > >> > longer have the initialized value.
>> > >> >
>> > >> > In short: you need to initialise memory *after* calling
>> > >> > mempool_alloc(), unless you ensure it is reset to the init values before
>> > >> > calling mempool_free().
>> > >> >
>> > >> > https://bugzilla.kernel.org/show_bug.cgi?id=196307
>> > >>
>> > >> OK, thanks for posting it out.
>> > >>
>> > >> Another fix might be to reinitialize the variable(rp->idx = 0) in
>> > >> r1buf_pool_free().
>> > >> Or just set it as zero every time when it is used.
>> > >>
>> > >> But I don't understand why mempool_free() calls pool->free() at the end of
>> > >> this function, which may cause to run pool->free() on a new allocated buf,
>> > >> seems a bug in mempool?
>> > >
>> > > Looks I missed the 'return' in mempool_free(), so it is fine.
>> > >
>> > > How about the following fix?
>> >
>> > It looks like it would probably work, but it is rather unusual to
>> > initialise something just before freeing it.
>> >
>> > Couldn't you just move the initialization to shortly after the
>> > mempool_alloc() call. There looks like a good place that already loops
>> > over all the bios....
>>
>> OK, follows the revised patch according to your suggestion.
Thanks.
That isn't as tidy as I hoped. So I went deeper into the code to try to
understand why...
I think that maybe we should just discard the ->idx field completely.
It is only used in this code:
do {
struct page *page;
int len = PAGE_SIZE;
if (sector_nr + (len>>9) > max_sector)
len = (max_sector - sector_nr) << 9;
if (len == 0)
break;
for (bio= biolist ; bio ; bio=bio->bi_next) {
struct resync_pages *rp = get_resync_pages(bio);
page = resync_fetch_page(rp, rp->idx++);
/*
* won't fail because the vec table is big enough
* to hold all these pages
*/
bio_add_page(bio, page, len, 0);
}
nr_sectors += len>>9;
sector_nr += len>>9;
} while (get_resync_pages(biolist)->idx < RESYNC_PAGES);
and all of the different 'rp' always have the same value for 'idx'.
This code is more complex than it needs to be. This is because it used
to be possible for bio_add_page() to fail. That cannot happen any more.
So we can make the code something like:
for (idx = 0; idx < RESYNC_PAGES; idx++) {
struct page *page;
int len = PAGE_SIZE;
if (sector_nr + (len >> 9) > max_sector)
len = (max_sector - sector_nr) << 9
if (len == 0)
break;
for (bio = biolist; bio; bio = bio->bi_next) {
struct resync_pages *rp = get_resync_pages(bio);
page = resync_fetch_page(rp, idx);
bio_add_page(bio, page, len, 0);
}
nr_sectors += len >> 9;
sector_nr += len >> 9;
}
Or did I miss something?
Thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [MD PATCH 1/1 -v2] Raid5 should update rdev->sectors after reshape
From: Xiao Ni @ 2017-07-11 0:52 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, ncroxon, tbskyd, gqjiang
In-Reply-To: <20170710174330.avznqdpakf4guiec@kernel.org>
On 07/11/2017 01:43 AM, Shaohua Li wrote:
> On Wed, Jul 05, 2017 at 05:34:04PM +0800, Xiao Ni wrote:
>> The raid5 md device is created by the disks which we don't use the total size. For example,
>> the size of the device is 5G and it just uses 3G of the devices to create one raid5 device.
>> Then change the chunksize and wait reshape to finish. After reshape finishing stop the raid
>> and assemble it again. It fails.
>> mdadm -CR /dev/md0 -l5 -n3 /dev/loop[0-2] --size=3G --chunk=32 --assume-clean
>> mdadm /dev/md0 --grow --chunk=64
>> wait reshape to finish
>> mdadm -S /dev/md0
>> mdadm -As
>> The error messages:
>> [197519.814302] md: loop1 does not have a valid v1.2 superblock, not importing!
>> [197519.821686] md: md_import_device returned -22
>>
>> After reshape the data offset is changed. It selects backwards direction in this condition.
>> In function super_1_load it compares the available space of the underlying device with
>> sb->data_size. The new data offset gets bigger after reshape. So super_1_load returns -EINVAL.
>> rdev->sectors is updated in md_finish_reshape. Then sb->data_size is set in super_1_sync based
>> on rdev->sectors. So add md_finish_reshape in end_reshape.
>>
>> Signed-off-by: Xiao Ni <xni@redhat.com>
> Applied, thanks! Will add to stable too.
>
> Can you also add the test to mdadm test suite?
Sure, I'll add this test suite.
Regards
Xiao
>
>> ---
>> drivers/md/raid5.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index ec0f951..e7f527c 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -7947,12 +7947,10 @@ static void end_reshape(struct r5conf *conf)
>> {
>>
>> if (!test_bit(MD_RECOVERY_INTR, &conf->mddev->recovery)) {
>> - struct md_rdev *rdev;
>>
>> spin_lock_irq(&conf->device_lock);
>> conf->previous_raid_disks = conf->raid_disks;
>> - rdev_for_each(rdev, conf->mddev)
>> - rdev->data_offset = rdev->new_data_offset;
>> + md_finish_reshape(conf->mddev);
>> smp_wmb();
>> conf->reshape_progress = MaxSector;
>> conf->mddev->reshape_position = MaxSector;
>> --
>> 2.7.4
>>
^ permalink raw reply
* Need to shrink md component disk partitions - what is the best method?
From: Ram Ramesh @ 2017-07-11 2:27 UTC (permalink / raw)
To: Linux Raid
I shrunk md device to suit my needs and now I need to shrink the
partition of each component device so that I can reuse the unused space
on the component disks. Every google search that I encountered only
outlines a method where you fail, remove, re-partition, add, sync each
disk. With bitmap, I guess each sync will be very fast as data is really
not being changed. Since I have raid6, I should be able to work on one
disk at a time without much risk also.
However, is this the recommended method? Why not simply boot on a rescue
system, do not assemble the array, simply change all partitions and
reboot the original system so that the array will get assembled the
normal way? Is this too risky or inferior?
While we are at it, mdadm -E says
> Avail Dev Size : 11720780943 (5588.90 GiB 6001.04 GB)
> Array Size : 12348030976 (11776.00 GiB 12644.38 GB)
> Used Dev Size : 6174015488 (2944.00 GiB 3161.10 GB)
What is Used Dev Size? Is is the last sector used or the number of
sectors used? If my current partition(s) start at 2048, does it mean I
should end the md partitions on or after 6174015488 or (6174015488+2048)?
How do I make mdadm scrub the entire array even though bit map says
there is no need?
Ramesh
^ permalink raw reply
* Re: Need to shrink md component disk partitions - what is the best method?
From: Ram Ramesh @ 2017-07-11 4:56 UTC (permalink / raw)
To: Linux Raid
In-Reply-To: <317ec719-0979-e1ae-b6ad-7192ef074ac4@gmail.com>
On 07/10/2017 09:27 PM, Ram Ramesh wrote:
> I shrunk md device to suit my needs and now I need to shrink the
> partition of each component device so that I can reuse the unused
> space on the component disks. Every google search that I encountered
> only outlines a method where you fail, remove, re-partition, add, sync
> each disk. With bitmap, I guess each sync will be very fast as data is
> really not being changed. Since I have raid6, I should be able to work
> on one disk at a time without much risk also.
>
> However, is this the recommended method? Why not simply boot on a
> rescue system, do not assemble the array, simply change all partitions
> and reboot the original system so that the array will get assembled
> the normal way? Is this too risky or inferior?
>
> While we are at it, mdadm -E says
>> Avail Dev Size : 11720780943 (5588.90 GiB 6001.04 GB)
>> Array Size : 12348030976 (11776.00 GiB 12644.38 GB)
>> Used Dev Size : 6174015488 (2944.00 GiB 3161.10 GB)
> What is Used Dev Size? Is is the last sector used or the number of
> sectors used? If my current partition(s) start at 2048, does it mean I
> should end the md partitions on or after 6174015488 or (6174015488+2048)?
>
> How do I make mdadm scrub the entire array even though bit map says
> there is no need?
>
> Ramesh
>
My attempt to remove a drive repartition and readd did not work. mdadm
refused to readd the device. Any one know why? This is what I did.
sudo mdadm /dev/md0 --fail /dev/sdb1
sudo mdadm /dev/md0 --remove /dev/sdb1
sudo gdisk /dev/sdb
< shrunk the partition size so that /dev/sdb1 is 2048 - 6442452991 and
created /dev/sdb2 6442452992-11721045134>
New partition table:
Number Start (sector) End (sector) Size Code Name
1 2048 6442452991 3.0 TiB FD00 Linux RAID
2 6442452992 11721045134 2.5 TiB FD00 Linux RAID
sudo mdadm /dev/md0 --re-add /dev/sdb1
***mdadm: --re-add for /dev/sdb1 to /dev/md0 is not possible***
Why? I have enabled bit maps and /proc/mdstat shows that.
Any way, I simply added the drive and it is rebuilding. I thought I knew
what will happen. You live and learn.
Now I am happy that I did not redo the partition on more than one drive.
I will wait for expert advice before proceeding.
Ramesh
^ permalink raw reply
* Re: Need to shrink md component disk partitions - what is the best method?
From: Adam Goryachev @ 2017-07-11 5:07 UTC (permalink / raw)
To: Ram Ramesh, Linux Raid
In-Reply-To: <2f8f0538-2a5f-a01a-616f-34b98cc7d67e@gmail.com>
On 11/07/17 14:56, Ram Ramesh wrote:
> On 07/10/2017 09:27 PM, Ram Ramesh wrote:
>> I shrunk md device to suit my needs and now I need to shrink the
>> partition of each component device so that I can reuse the unused
>> space on the component disks. Every google search that I encountered
>> only outlines a method where you fail, remove, re-partition, add,
>> sync each disk. With bitmap, I guess each sync will be very fast as
>> data is really not being changed. Since I have raid6, I should be
>> able to work on one disk at a time without much risk also.
>>
>> However, is this the recommended method? Why not simply boot on a
>> rescue system, do not assemble the array, simply change all
>> partitions and reboot the original system so that the array will get
>> assembled the normal way? Is this too risky or inferior?
>>
>> While we are at it, mdadm -E says
>>> Avail Dev Size : 11720780943 (5588.90 GiB 6001.04 GB)
>>> Array Size : 12348030976 (11776.00 GiB 12644.38 GB)
>>> Used Dev Size : 6174015488 (2944.00 GiB 3161.10 GB)
>> What is Used Dev Size? Is is the last sector used or the number of
>> sectors used? If my current partition(s) start at 2048, does it mean
>> I should end the md partitions on or after 6174015488 or
>> (6174015488+2048)?
>>
>> How do I make mdadm scrub the entire array even though bit map says
>> there is no need?
>>
>> Ramesh
>>
> My attempt to remove a drive repartition and readd did not work. mdadm
> refused to readd the device. Any one know why? This is what I did.
>
> sudo mdadm /dev/md0 --fail /dev/sdb1
> sudo mdadm /dev/md0 --remove /dev/sdb1
> sudo gdisk /dev/sdb
> < shrunk the partition size so that /dev/sdb1 is 2048 - 6442452991
> and created /dev/sdb2 6442452992-11721045134>
>
> New partition table:
> Number Start (sector) End (sector) Size Code Name
> 1 2048 6442452991 3.0 TiB FD00 Linux RAID
> 2 6442452992 11721045134 2.5 TiB FD00 Linux RAID
>
> sudo mdadm /dev/md0 --re-add /dev/sdb1
> ***mdadm: --re-add for /dev/sdb1 to /dev/md0 is not possible***
>
> Why? I have enabled bit maps and /proc/mdstat shows that.
>
> Any way, I simply added the drive and it is rebuilding. I thought I
> knew what will happen. You live and learn.
>
> Now I am happy that I did not redo the partition on more than one
> drive. I will wait for expert advice before proceeding.
>
> Ramesh
I think the md signature is at the end of the device, thus it can't be
found after you shrink the partition. You would need to use a version of
the MD label which is at the beginning of the partition to make this work.
AFAIK...
Regards,
Adam
--
Adam Goryachev Website Managers www.websitemanagers.com.au
--
The information in this e-mail is confidential and may be legally privileged.
It is intended solely for the addressee. Access to this e-mail by anyone else
is unauthorised. If you are not the intended recipient, any disclosure,
copying, distribution or any action taken or omitted to be taken in reliance
on it, is prohibited and may be unlawful. If you have received this message
in error, please notify us immediately. Please also destroy and delete the
message from your computer. Viruses - Any loss/damage incurred by receiving
this email is not the sender's responsibility.
^ permalink raw reply
* [PATCH] raid5-ppl: use BIOSET_NEED_BVECS when creating bioset
From: Artur Paszkiewicz @ 2017-07-11 14:16 UTC (permalink / raw)
To: shli; +Cc: linux-raid, Artur Paszkiewicz
This bioset is used for allocating bios with nr_iovecs > 0 so this flag
must be set.
Fixes: 011067b05668 ("blk: replace bioset_create_nobvec() with a flags arg to bioset_create()")
Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
---
drivers/md/raid5-ppl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/raid5-ppl.c b/drivers/md/raid5-ppl.c
index 77cce3573aa8..44ad5baf3206 100644
--- a/drivers/md/raid5-ppl.c
+++ b/drivers/md/raid5-ppl.c
@@ -1150,7 +1150,7 @@ int ppl_init_log(struct r5conf *conf)
goto err;
}
- ppl_conf->bs = bioset_create(conf->raid_disks, 0, 0);
+ ppl_conf->bs = bioset_create(conf->raid_disks, 0, BIOSET_NEED_BVECS);
if (!ppl_conf->bs) {
ret = -ENOMEM;
goto err;
--
2.13.1
^ permalink raw reply related
* Re: [PATCH v3 05/14] md: raid1: don't use bio's vec table to manage resync pages
From: Ming Lei @ 2017-07-12 1:40 UTC (permalink / raw)
To: NeilBrown
Cc: Shaohua Li, Ming Lei, Jens Axboe,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT, linux-block,
Christoph Hellwig
In-Reply-To: <874luj6g1y.fsf@notabene.neil.brown.name>
On Tue, Jul 11, 2017 at 7:14 AM, NeilBrown <neilb@suse.com> wrote:
> On Mon, Jul 10 2017, Shaohua Li wrote:
>
>> On Mon, Jul 10, 2017 at 03:25:41PM +0800, Ming Lei wrote:
>>> On Mon, Jul 10, 2017 at 02:38:19PM +1000, NeilBrown wrote:
>>> > On Mon, Jul 10 2017, Ming Lei wrote:
>>> >
>>> > > On Mon, Jul 10, 2017 at 11:35:12AM +0800, Ming Lei wrote:
>>> > >> On Mon, Jul 10, 2017 at 7:09 AM, NeilBrown <neilb@suse.com> wrote:
>>> > ...
>>> > >> >> +
>>> > >> >> + rp->idx = 0;
>>> > >> >
>>> > >> > This is the only place the ->idx is initialized, in r1buf_pool_alloc().
>>> > >> > The mempool alloc function is suppose to allocate memory, not initialize
>>> > >> > it.
>>> > >> >
>>> > >> > If the mempool_alloc() call cannot allocate memory it will use memory
>>> > >> > from the pool. If this memory has already been used, then it will no
>>> > >> > longer have the initialized value.
>>> > >> >
>>> > >> > In short: you need to initialise memory *after* calling
>>> > >> > mempool_alloc(), unless you ensure it is reset to the init values before
>>> > >> > calling mempool_free().
>>> > >> >
>>> > >> > https://bugzilla.kernel.org/show_bug.cgi?id=196307
>>> > >>
>>> > >> OK, thanks for posting it out.
>>> > >>
>>> > >> Another fix might be to reinitialize the variable(rp->idx = 0) in
>>> > >> r1buf_pool_free().
>>> > >> Or just set it as zero every time when it is used.
>>> > >>
>>> > >> But I don't understand why mempool_free() calls pool->free() at the end of
>>> > >> this function, which may cause to run pool->free() on a new allocated buf,
>>> > >> seems a bug in mempool?
>>> > >
>>> > > Looks I missed the 'return' in mempool_free(), so it is fine.
>>> > >
>>> > > How about the following fix?
>>> >
>>> > It looks like it would probably work, but it is rather unusual to
>>> > initialise something just before freeing it.
>>> >
>>> > Couldn't you just move the initialization to shortly after the
>>> > mempool_alloc() call. There looks like a good place that already loops
>>> > over all the bios....
>>>
>>> OK, follows the revised patch according to your suggestion.
>
> Thanks.
>
> That isn't as tidy as I hoped. So I went deeper into the code to try to
> understand why...
>
> I think that maybe we should just discard the ->idx field completely.
> It is only used in this code:
>
> do {
> struct page *page;
> int len = PAGE_SIZE;
> if (sector_nr + (len>>9) > max_sector)
> len = (max_sector - sector_nr) << 9;
> if (len == 0)
> break;
> for (bio= biolist ; bio ; bio=bio->bi_next) {
> struct resync_pages *rp = get_resync_pages(bio);
> page = resync_fetch_page(rp, rp->idx++);
> /*
> * won't fail because the vec table is big enough
> * to hold all these pages
> */
> bio_add_page(bio, page, len, 0);
> }
> nr_sectors += len>>9;
> sector_nr += len>>9;
> } while (get_resync_pages(biolist)->idx < RESYNC_PAGES);
>
> and all of the different 'rp' always have the same value for 'idx'.
> This code is more complex than it needs to be. This is because it used
> to be possible for bio_add_page() to fail. That cannot happen any more.
> So we can make the code something like:
>
> for (idx = 0; idx < RESYNC_PAGES; idx++) {
> struct page *page;
> int len = PAGE_SIZE;
> if (sector_nr + (len >> 9) > max_sector)
> len = (max_sector - sector_nr) << 9
> if (len == 0)
> break;
> for (bio = biolist; bio; bio = bio->bi_next) {
> struct resync_pages *rp = get_resync_pages(bio);
> page = resync_fetch_page(rp, idx);
> bio_add_page(bio, page, len, 0);
> }
> nr_sectors += len >> 9;
> sector_nr += len >> 9;
> }
>
> Or did I miss something?
I think this approach is much clean.
--
Ming Lei
^ permalink raw reply
* [PATCH] dm bufio: Fix error code in dm_bufio_write_dirty_buffers()
From: Dan Carpenter @ 2017-07-12 7:26 UTC (permalink / raw)
To: Alasdair Kergon, Christoph Hellwig
Cc: Mike Snitzer, dm-devel, Shaohua Li, linux-raid, kernel-janitors
We should be returning normal negative error codes here. The "a"
variables comes from &c->async_write_error which is a blk_status_t
converted to a regular error code.
In the current code, the blk_status_t gets propogated back to
pool_create() and eventually results in an Oops.
Fixes: 4e4cbee93d56 ("block: switch bios to blk_status_t")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c
index 850ff6c67994..44f4a8ac95bd 100644
--- a/drivers/md/dm-bufio.c
+++ b/drivers/md/dm-bufio.c
@@ -1258,8 +1258,7 @@ EXPORT_SYMBOL_GPL(dm_bufio_write_dirty_buffers_async);
*/
int dm_bufio_write_dirty_buffers(struct dm_bufio_client *c)
{
- blk_status_t a;
- int f;
+ int a, f;
unsigned long buffers_processed = 0;
struct dm_buffer *b, *tmp;
^ permalink raw reply related
* Re: [PATCH] dm bufio: Fix error code in dm_bufio_write_dirty_buffers()
From: Christoph Hellwig @ 2017-07-12 7:30 UTC (permalink / raw)
To: Dan Carpenter
Cc: Alasdair Kergon, Christoph Hellwig, Mike Snitzer, dm-devel,
Shaohua Li, linux-raid, kernel-janitors
In-Reply-To: <20170712072634.qtm4tq7xlnp572hs@mwanda>
On Wed, Jul 12, 2017 at 10:26:34AM +0300, Dan Carpenter wrote:
> We should be returning normal negative error codes here. The "a"
> variables comes from &c->async_write_error which is a blk_status_t
> converted to a regular error code.
>
> In the current code, the blk_status_t gets propogated back to
> pool_create() and eventually results in an Oops.
And sparse correctly warns about this, so no idea how this slipped in..
>
> Fixes: 4e4cbee93d56 ("block: switch bios to blk_status_t")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* [PATCH 0/2] md: two patches
From: Ming Lei @ 2017-07-12 8:28 UTC (permalink / raw)
To: Shaohua Li, linux-raid
Cc: NeilBrown, linux-block, Jens Axboe, Christoph Hellwig, Ming Lei
This 1st patch fixes one issue introduced in the following two
commits:
Fixes: f0250618361d(md: raid10: don't use bio's vec table to manage resync pages)
Fixes: 98d30c5812c3(md: raid1: don't use bio's vec table to manage resync pages)
The 2nd one initializes bvec table of bio via bio_add_page() after bio_reset().
Ming Lei (2):
md: remove 'idx' from 'struct resync_pages'
md: raid1/raid10: initialize bvec table via bio_add_page()
drivers/md/md.c | 21 +++++++++++++++++++++
drivers/md/md.h | 4 +++-
drivers/md/raid1.c | 22 +++++-----------------
drivers/md/raid10.c | 10 +++++-----
4 files changed, 34 insertions(+), 23 deletions(-)
--
2.9.4
^ permalink raw reply
* [PATCH 1/2] md: remove 'idx' from 'struct resync_pages'
From: Ming Lei @ 2017-07-12 8:28 UTC (permalink / raw)
To: Shaohua Li, linux-raid
Cc: NeilBrown, linux-block, Jens Axboe, Christoph Hellwig, Ming Lei
bio_add_page() won't fail for resync bio, and the page index for each
bio is same, so remove it.
More importantly the 'idx' of 'struct resync_pages' is initialized in
mempool allocator function, this way is wrong since mempool is only
responsible for allocation, we can't use that for initialization.
Suggested-by: NeilBrown <neilb@suse.com>
Reported-by: NeilBrown <neilb@suse.com>
Fixes: f0250618361d(md: raid10: don't use bio's vec table to manage resync pages)
Fixes: 98d30c5812c3(md: raid1: don't use bio's vec table to manage resync pages)
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/md/md.h | 1 -
drivers/md/raid1.c | 6 +++---
drivers/md/raid10.c | 6 +++---
3 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 991f0fe2dcc6..2c780aa8d07f 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -736,7 +736,6 @@ static inline void mddev_check_write_zeroes(struct mddev *mddev, struct bio *bio
/* for managing resync I/O pages */
struct resync_pages {
- unsigned idx; /* for get/put page from the pool */
void *raid_bio;
struct page *pages[RESYNC_PAGES];
};
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 3febfc8391fb..7901ddc3362f 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -170,7 +170,6 @@ static void * r1buf_pool_alloc(gfp_t gfp_flags, void *data)
resync_get_all_pages(rp);
}
- rp->idx = 0;
rp->raid_bio = r1_bio;
bio->bi_private = rp;
}
@@ -2619,6 +2618,7 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
int good_sectors = RESYNC_SECTORS;
int min_bad = 0; /* number of sectors that are bad in all devices */
int idx = sector_to_idx(sector_nr);
+ int page_idx = 0;
if (!conf->r1buf_pool)
if (init_resync(conf))
@@ -2846,7 +2846,7 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
bio = r1_bio->bios[i];
rp = get_resync_pages(bio);
if (bio->bi_end_io) {
- page = resync_fetch_page(rp, rp->idx++);
+ page = resync_fetch_page(rp, page_idx);
/*
* won't fail because the vec table is big
@@ -2858,7 +2858,7 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
nr_sectors += len>>9;
sector_nr += len>>9;
sync_blocks -= (len>>9);
- } while (get_resync_pages(r1_bio->bios[disk]->bi_private)->idx < RESYNC_PAGES);
+ } while (page_idx++ < RESYNC_PAGES);
r1_bio->sectors = nr_sectors;
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 5026e7ad51d3..e594ca610f27 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -221,7 +221,6 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
resync_get_all_pages(rp);
}
- rp->idx = 0;
rp->raid_bio = r10_bio;
bio->bi_private = rp;
if (rbio) {
@@ -2853,6 +2852,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
sector_t sectors_skipped = 0;
int chunks_skipped = 0;
sector_t chunk_mask = conf->geo.chunk_mask;
+ int page_idx = 0;
if (!conf->r10buf_pool)
if (init_resync(conf))
@@ -3355,7 +3355,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
break;
for (bio= biolist ; bio ; bio=bio->bi_next) {
struct resync_pages *rp = get_resync_pages(bio);
- page = resync_fetch_page(rp, rp->idx++);
+ page = resync_fetch_page(rp, page_idx);
/*
* won't fail because the vec table is big enough
* to hold all these pages
@@ -3364,7 +3364,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
}
nr_sectors += len>>9;
sector_nr += len>>9;
- } while (get_resync_pages(biolist)->idx < RESYNC_PAGES);
+ } while (page_idx++ < RESYNC_PAGES);
r10_bio->sectors = nr_sectors;
while (biolist) {
--
2.9.4
^ permalink raw reply related
* [PATCH 2/2] md: raid1/raid10: initialize bvec table via bio_add_page()
From: Ming Lei @ 2017-07-12 8:29 UTC (permalink / raw)
To: Shaohua Li, linux-raid
Cc: NeilBrown, linux-block, Jens Axboe, Christoph Hellwig, Ming Lei
We will support multipage bvec soon, so initialize bvec
table using the standardy way instead of writing the
talbe directly. Otherwise it won't work any more once
multipage bvec is enabled.
Acked-by: Guoqing Jiang <gqjiang@suse.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/md/md.c | 21 +++++++++++++++++++++
drivers/md/md.h | 3 +++
drivers/md/raid1.c | 16 ++--------------
drivers/md/raid10.c | 4 ++--
4 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 8cdca0296749..cc8dcd928dde 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9130,6 +9130,27 @@ void md_reload_sb(struct mddev *mddev, int nr)
}
EXPORT_SYMBOL(md_reload_sb);
+/* generally called after bio_reset() for reseting bvec */
+void md_bio_reset_resync_pages(struct bio *bio, struct resync_pages *rp,
+ int size)
+{
+ int idx = 0;
+
+ /* initialize bvec table again */
+ do {
+ struct page *page = resync_fetch_page(rp, idx);
+ int len = min_t(int, size, PAGE_SIZE);
+
+ /*
+ * won't fail because the vec table is big
+ * enough to hold all these pages
+ */
+ bio_add_page(bio, page, len, 0);
+ size -= len;
+ } while (idx++ < RESYNC_PAGES && size > 0);
+}
+EXPORT_SYMBOL(md_bio_reset_resync_pages);
+
#ifndef MODULE
/*
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 2c780aa8d07f..efb32ce7a2f1 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -782,4 +782,7 @@ static inline struct page *resync_fetch_page(struct resync_pages *rp,
return NULL;
return rp->pages[idx];
}
+
+void md_bio_reset_resync_pages(struct bio *bio, struct resync_pages *rp,
+ int size);
#endif /* _MD_MD_H */
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 7901ddc3362f..5dc3fda2fdf7 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2085,10 +2085,7 @@ static void process_checks(struct r1bio *r1_bio)
/* Fix variable parts of all bios */
vcnt = (r1_bio->sectors + PAGE_SIZE / 512 - 1) >> (PAGE_SHIFT - 9);
for (i = 0; i < conf->raid_disks * 2; i++) {
- int j;
- int size;
blk_status_t status;
- struct bio_vec *bi;
struct bio *b = r1_bio->bios[i];
struct resync_pages *rp = get_resync_pages(b);
if (b->bi_end_io != end_sync_read)
@@ -2097,8 +2094,6 @@ static void process_checks(struct r1bio *r1_bio)
status = b->bi_status;
bio_reset(b);
b->bi_status = status;
- b->bi_vcnt = vcnt;
- b->bi_iter.bi_size = r1_bio->sectors << 9;
b->bi_iter.bi_sector = r1_bio->sector +
conf->mirrors[i].rdev->data_offset;
b->bi_bdev = conf->mirrors[i].rdev->bdev;
@@ -2106,15 +2101,8 @@ static void process_checks(struct r1bio *r1_bio)
rp->raid_bio = r1_bio;
b->bi_private = rp;
- size = b->bi_iter.bi_size;
- bio_for_each_segment_all(bi, b, j) {
- bi->bv_offset = 0;
- if (size > PAGE_SIZE)
- bi->bv_len = PAGE_SIZE;
- else
- bi->bv_len = size;
- size -= PAGE_SIZE;
- }
+ /* initialize bvec table again */
+ md_bio_reset_resync_pages(b, rp, r1_bio->sectors << 9);
}
for (primary = 0; primary < conf->raid_disks * 2; primary++)
if (r1_bio->bios[primary]->bi_end_io == end_sync_read &&
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index e594ca610f27..cb8e803cd1c2 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -2086,8 +2086,8 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
rp = get_resync_pages(tbio);
bio_reset(tbio);
- tbio->bi_vcnt = vcnt;
- tbio->bi_iter.bi_size = fbio->bi_iter.bi_size;
+ md_bio_reset_resync_pages(tbio, rp, fbio->bi_iter.bi_size);
+
rp->raid_bio = r10_bio;
tbio->bi_private = rp;
tbio->bi_iter.bi_sector = r10_bio->devs[i].addr;
--
2.9.4
^ permalink raw reply related
* Re: [PATCH 2/2] md: raid1/raid10: initialize bvec table via bio_add_page()
From: Christoph Hellwig @ 2017-07-12 9:16 UTC (permalink / raw)
To: Ming Lei
Cc: Shaohua Li, linux-raid, NeilBrown, linux-block, Jens Axboe,
Christoph Hellwig
In-Reply-To: <20170712082912.491-1-ming.lei@redhat.com>
On Wed, Jul 12, 2017 at 04:29:12PM +0800, Ming Lei wrote:
> We will support multipage bvec soon, so initialize bvec
> table using the standardy way instead of writing the
> talbe directly. Otherwise it won't work any more once
> multipage bvec is enabled.
It seems to me like these callsites also should use bio_init
instead of bio_reset to get a clean slate (and eventually
phase out bio_reset), which should probably got into
the helper as well. E.g. instead of pretending to preserve
things we should simply build a new bio here.
^ permalink raw reply
* Re: Linear device of two arrays
From: Veljko @ 2017-07-12 10:21 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <e394625b-18d7-d141-3957-3c16c9bc6e44@gmail.com>
Hello Neil,
On 07/10/2017 01:03 PM, Veljko wrote:
> On 07/10/2017 12:37 AM, NeilBrown wrote:
>> I wasn't clear to me that I needed to chime in.. and the complete lack
>> of details (not even an "mdadm --examine" output), meant I could only
>> answer in vague generalizations.
>> However, seeing you asked.
>> If you really want to have a 'linear' of 2 RAID10s, then
>> 0/ unmount the xfs filesystem
>> 1/ backup the last few megabytes of the device
>> dd if=/dev/mdXX of=/safe/place/backup bs=1M skip=$BIGNUM
>> 2/ create a linear array of the two RAID10s, ensuring the
>> metadata is v1.0, and the dataoffset is zero (should be default with
>> 1.0)
>> mdadm -C /dev/mdZZ -l linear -n 2 -e 1.0 --data-offset=0 /dev/mdXX
>> /dev/mdYY
>> 3/ restore the saved data
>> dd of=/dev/mdZZ if=/safe/place/backup bs=1M seek=$BIGNUM
>> 4/ grow the xfs filesystem
>> 5/ be happy.
>>
>> I cannot comment on the values of "few" and "$BUGNUM" without seeing
>> specifics.
>>
>> NeilBrown
>
> Thanks for your response, Neil!
>
> md0 is boot (raid1), md1 is root (raid10) and md2 is data (raid10) that
> I need to expand. Here are details:
>
>
> # mdadm --detail /dev/md0
> /dev/md0:
> Version : 1.2
> Creation Time : Mon Sep 10 14:45:11 2012
> Raid Level : raid1
> Array Size : 488128 (476.77 MiB 499.84 MB)
> Used Dev Size : 488128 (476.77 MiB 499.84 MB)
> Raid Devices : 2
> Total Devices : 2
> Persistence : Superblock is persistent
>
> Update Time : Mon Jul 3 11:57:24 2017
> State : clean
> Active Devices : 2
> Working Devices : 2
> Failed Devices : 0
> Spare Devices : 0
>
> Name : backup1:0 (local to host backup1)
> UUID : e5a17766:b4df544d:c2770d6e:214113ec
> Events : 302
>
> Number Major Minor RaidDevice State
> 2 8 18 0 active sync /dev/sdb2
> 3 8 34 1 active sync /dev/sdc2
>
>
> # mdadm --detail /dev/md1
> /dev/md1:
> Version : 1.2
> Creation Time : Fri Sep 14 12:39:00 2012
> Raid Level : raid10
> Array Size : 97590272 (93.07 GiB 99.93 GB)
> Used Dev Size : 48795136 (46.53 GiB 49.97 GB)
> Raid Devices : 4
> Total Devices : 4
> Persistence : Superblock is persistent
>
> Update Time : Mon Jul 10 12:30:46 2017
> State : clean
> Active Devices : 4
> Working Devices : 4
> Failed Devices : 0
> Spare Devices : 0
>
> Layout : near=2
> Chunk Size : 512K
>
> Name : backup1:1 (local to host backup1)
> UUID : 91560d5a:245bbc56:cc08b0ce:9c78fea1
> Events : 1003350
>
> Number Major Minor RaidDevice State
> 4 8 19 0 active sync set-A /dev/sdb3
> 6 8 35 1 active sync set-B /dev/sdc3
> 7 8 50 2 active sync set-A /dev/sdd2
> 5 8 2 3 active sync set-B /dev/sda2
>
>
> # mdadm --detail /dev/md2
> /dev/md2:
> Version : 1.2
> Creation Time : Fri Sep 14 12:40:13 2012
> Raid Level : raid10
> Array Size : 5761631232 (5494.72 GiB 5899.91 GB)
> Used Dev Size : 2880815616 (2747.36 GiB 2949.96 GB)
> Raid Devices : 4
> Total Devices : 4
> Persistence : Superblock is persistent
>
> Update Time : Mon Jul 10 12:32:51 2017
> State : clean
> Active Devices : 4
> Working Devices : 4
> Failed Devices : 0
> Spare Devices : 0
>
> Layout : near=2
> Chunk Size : 512K
>
> Name : backup1:2 (local to host backup1)
> UUID : f6eeaa57:a55f36ff:6980a62a:d4781e44
> Events : 2689040
>
> Number Major Minor RaidDevice State
> 4 8 20 0 active sync set-A /dev/sdb4
> 6 8 36 1 active sync set-B /dev/sdc4
> 7 8 51 2 active sync set-A /dev/sdd3
> 5 8 3 3 active sync set-B /dev/sda3
>
>
> And here is examine output for md2 partitions:
>
> # mdadm --examine /dev/sda3
> /dev/sda3:
> Magic : a92b4efc
> Version : 1.2
> Feature Map : 0x0
> Array UUID : f6eeaa57:a55f36ff:6980a62a:d4781e44
> Name : backup1:2 (local to host backup1)
> Creation Time : Fri Sep 14 12:40:13 2012
> Raid Level : raid10
> Raid Devices : 4
>
> Avail Dev Size : 5762609152 (2747.83 GiB 2950.46 GB)
> Array Size : 5761631232 (5494.72 GiB 5899.91 GB)
> Used Dev Size : 5761631232 (2747.36 GiB 2949.96 GB)
> Data Offset : 262144 sectors
> Super Offset : 8 sectors
> Unused Space : before=262064 sectors, after=977920 sectors
> State : clean
> Device UUID : 92beeec2:7ff92b1d:473a9641:2a078b16
>
> Update Time : Mon Jul 10 12:35:53 2017
> Checksum : d1abfc30 - correct
> Events : 2689040
>
> Layout : near=2
> Chunk Size : 512K
>
> Device Role : Active device 3
> Array State : AAAA ('A' == active, '.' == missing, 'R' == replacing)
>
>
> # mdadm --examine /dev/sdb4
> /dev/sdb4:
> Magic : a92b4efc
> Version : 1.2
> Feature Map : 0x0
> Array UUID : f6eeaa57:a55f36ff:6980a62a:d4781e44
> Name : backup1:2 (local to host backup1)
> Creation Time : Fri Sep 14 12:40:13 2012
> Raid Level : raid10
> Raid Devices : 4
>
> Avail Dev Size : 5761632256 (2747.36 GiB 2949.96 GB)
> Array Size : 5761631232 (5494.72 GiB 5899.91 GB)
> Used Dev Size : 5761631232 (2747.36 GiB 2949.96 GB)
> Data Offset : 262144 sectors
> Super Offset : 8 sectors
> Unused Space : before=262064 sectors, after=1024 sectors
> State : clean
> Device UUID : 01e1cb21:01a011a9:85761911:9b4d437a
>
> Update Time : Mon Jul 10 12:37:00 2017
> Checksum : ef9b6012 - correct
> Events : 2689040
>
> Layout : near=2
> Chunk Size : 512K
>
> Device Role : Active device 0
> Array State : AAAA ('A' == active, '.' == missing, 'R' == replacing)
>
>
>
> # mdadm --examine /dev/sdc4
> /dev/sdc4:
> Magic : a92b4efc
> Version : 1.2
> Feature Map : 0x0
> Array UUID : f6eeaa57:a55f36ff:6980a62a:d4781e44
> Name : backup1:2 (local to host backup1)
> Creation Time : Fri Sep 14 12:40:13 2012
> Raid Level : raid10
> Raid Devices : 4
>
> Avail Dev Size : 5761632256 (2747.36 GiB 2949.96 GB)
> Array Size : 5761631232 (5494.72 GiB 5899.91 GB)
> Used Dev Size : 5761631232 (2747.36 GiB 2949.96 GB)
> Data Offset : 262144 sectors
> Super Offset : 8 sectors
> Unused Space : before=262064 sectors, after=1024 sectors
> State : clean
> Device UUID : 1a2c966f:a78ffaf3:83cf37d4:135087b7
>
> Update Time : Mon Jul 10 12:37:53 2017
> Checksum : 88b0f680 - correct
> Events : 2689040
>
> Layout : near=2
> Chunk Size : 512K
>
> Device Role : Active device 1
> Array State : AAAA ('A' == active, '.' == missing, 'R' == replacing)
>
>
>
>
> # mdadm --examine /dev/sdd3
> /dev/sdd3:
> Magic : a92b4efc
> Version : 1.2
> Feature Map : 0x0
> Array UUID : f6eeaa57:a55f36ff:6980a62a:d4781e44
> Name : backup1:2 (local to host backup1)
> Creation Time : Fri Sep 14 12:40:13 2012
> Raid Level : raid10
> Raid Devices : 4
>
> Avail Dev Size : 5762609152 (2747.83 GiB 2950.46 GB)
> Array Size : 5761631232 (5494.72 GiB 5899.91 GB)
> Used Dev Size : 5761631232 (2747.36 GiB 2949.96 GB)
> Data Offset : 262144 sectors
> Super Offset : 8 sectors
> Unused Space : before=262064 sectors, after=977920 sectors
> State : clean
> Device UUID : 52f92e76:15228eee:a20c1ee5:8d4a17d2
>
> Update Time : Mon Jul 10 12:38:24 2017
> Checksum : b56275df - correct
> Events : 2689040
>
> Layout : near=2
> Chunk Size : 512K
>
> Device Role : Active device 2
> Array State : AAAA ('A' == active, '.' == missing, 'R' == replacing)
Do you know what would be the "few" and "$BIGNUM" values from the output
above?
Since I need to expand md2 device, I guess `that I need to subtract
"few" number of megabytes as ($few x 1024 x 1024) in bytes from array
size of md2 (in my case 5761631232). Is this correct? $BIGNUM is the
size of md2 array? How to know how many megabytes needs to be backed up?
Data offset is not zero on md2 partitions. Is that a dealbreaker?
Would it be than better to reshape the current RAID10 to increase the
number of devices used from 4 to 8 (as advised by Roman)?
Regards,
Veljko
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox