* Re: Recovery after accidental raid5 superblock rewrite
From: Andreas Klauer @ 2017-06-05 23:56 UTC (permalink / raw)
To: Paul Tonelli; +Cc: linux-raid
In-Reply-To: <dfa4dd0d-c68a-4e1b-aac7-649f7f1a72f6@tonel.li>
On Tue, Jun 06, 2017 at 01:24:41AM +0200, Paul Tonelli wrote:
> mdadm --create /dev/md0 --level=5 --assume-clean --raid-devices=3 missing /dev/mapper/sdd /dev/mapper/sdb
You did not specify the --data-offset here?
Check mdadm --examine to make sure which offset it's using.
> xxd -u /dev/mapper/sdc | grep -C 3 'LABELONE'
> >7f001e0: 0000 0000 0000 0000 0000 0000 0000 0000 ................
> >7f001f0: 0000 0000 0000 0000 0000 0000 0000 0000 ................
> >7f00200: 4C41 4245 4C4F 4E45 0100 0000 0000 0000 LABELONE........
If correct this should appear at the start of /dev/md0 (even w/o adding sdc).
LABELONE should appear on the first drive so this should not be wrong,
however the sdd sdb could still have switched order, and of course
the chunk size could be different (although unlikely according to your log).
> I believe you are right, the issue is still the raid: I have tried
> photorec and most files I have opened look like they have been
> truncated.
Well, files could be fragmented, there's a certain sweet spot
(like - megapixel JPEGs of few megs size) where it's sufficiently
unlikely to be a problem.
I don't know what files you have, if it's movies it would be okay
too if the first few megs of the file were playable.
> - apart from the raid superblock, the disks I use (sdd and sdb) have not
> been erased (as sdc is rebuilt from xor)
It only rebuilds starting from offset. So it should not have covered that
offset if you did not specify it. Check it's not there before you --add.
If it's there then this is after all not the drive you overwrite with dd?
I am confused now.
Regards
Andreas Klauer
^ permalink raw reply
* Re: [PATCH] md: fix deadlock between mddev_suspend() and md_write_start()
From: Shaohua Li @ 2017-06-06 0:01 UTC (permalink / raw)
To: NeilBrown; +Cc: Nix, linux-raid
In-Reply-To: <87wp8rc4jg.fsf@notabene.neil.brown.name>
On Mon, Jun 05, 2017 at 04:49:39PM +1000, Neil Brown wrote:
>
> If mddev_suspend() races with md_write_start() we can deadlock
> with mddev_suspend() waiting for the request that is currently
> in md_write_start() to complete the ->make_request() call,
> and md_write_start() waiting for the metadata to be updated
> to mark the array as 'dirty'.
> As metadata updates done by md_check_recovery() only happen then
> the mddev_lock() can be claimed, and as mddev_suspend() is often
> called with the lock held, these threads wait indefinitely for each
> other.
>
> We fix this by having md_write_start() abort if mddev_suspend()
> is happening, and ->make_request() aborts if md_write_start()
> aborted.
> md_make_request() can detect this abort, decrease the ->active_io
> count, and wait for mddev_suspend().
Applied the two patches, thanks! For this one, I added md_start_write symbol back. Will
push these to Linus in the 4.12 cycle.
> Reported-by: Nix <nix@esperi.org.uk>
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
> drivers/md/faulty.c | 5 +++--
> drivers/md/linear.c | 7 ++++---
> drivers/md/md.c | 23 +++++++++++++++++------
> drivers/md/md.h | 4 ++--
> drivers/md/multipath.c | 8 ++++----
> drivers/md/raid0.c | 7 ++++---
> drivers/md/raid1.c | 11 +++++++----
> drivers/md/raid10.c | 10 ++++++----
> drivers/md/raid5.c | 17 +++++++++--------
> 9 files changed, 56 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/md/faulty.c b/drivers/md/faulty.c
> index b0536cfd8e17..06a64d5d8c6c 100644
> --- a/drivers/md/faulty.c
> +++ b/drivers/md/faulty.c
> @@ -170,7 +170,7 @@ static void add_sector(struct faulty_conf *conf, sector_t start, int mode)
> conf->nfaults = n+1;
> }
>
> -static void faulty_make_request(struct mddev *mddev, struct bio *bio)
> +static bool faulty_make_request(struct mddev *mddev, struct bio *bio)
> {
> struct faulty_conf *conf = mddev->private;
> int failit = 0;
> @@ -182,7 +182,7 @@ static void faulty_make_request(struct mddev *mddev, struct bio *bio)
> * just fail immediately
> */
> bio_io_error(bio);
> - return;
> + return true;
> }
>
> if (check_sector(conf, bio->bi_iter.bi_sector,
> @@ -224,6 +224,7 @@ static void faulty_make_request(struct mddev *mddev, struct bio *bio)
> bio->bi_bdev = conf->rdev->bdev;
>
> generic_make_request(bio);
> + return true;
> }
>
> static void faulty_status(struct seq_file *seq, struct mddev *mddev)
> diff --git a/drivers/md/linear.c b/drivers/md/linear.c
> index df6f2c98eca7..5f1eb9189542 100644
> --- a/drivers/md/linear.c
> +++ b/drivers/md/linear.c
> @@ -245,7 +245,7 @@ static void linear_free(struct mddev *mddev, void *priv)
> kfree(conf);
> }
>
> -static void linear_make_request(struct mddev *mddev, struct bio *bio)
> +static bool linear_make_request(struct mddev *mddev, struct bio *bio)
> {
> char b[BDEVNAME_SIZE];
> struct dev_info *tmp_dev;
> @@ -254,7 +254,7 @@ static void linear_make_request(struct mddev *mddev, struct bio *bio)
>
> if (unlikely(bio->bi_opf & REQ_PREFLUSH)) {
> md_flush_request(mddev, bio);
> - return;
> + return true;
> }
>
> tmp_dev = which_dev(mddev, bio_sector);
> @@ -292,7 +292,7 @@ static void linear_make_request(struct mddev *mddev, struct bio *bio)
> mddev_check_write_zeroes(mddev, bio);
> generic_make_request(bio);
> }
> - return;
> + return true;
>
> out_of_bounds:
> pr_err("md/linear:%s: make_request: Sector %llu out of bounds on dev %s: %llu sectors, offset %llu\n",
> @@ -302,6 +302,7 @@ static void linear_make_request(struct mddev *mddev, struct bio *bio)
> (unsigned long long)tmp_dev->rdev->sectors,
> (unsigned long long)start_sector);
> bio_io_error(bio);
> + return true;
> }
>
> static void linear_status (struct seq_file *seq, struct mddev *mddev)
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 23f4adf3a8cc..da59051545a2 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -277,7 +277,7 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
> bio_endio(bio);
> return BLK_QC_T_NONE;
> }
> - smp_rmb(); /* Ensure implications of 'active' are visible */
> +check_suspended:
> rcu_read_lock();
> if (mddev->suspended) {
> DEFINE_WAIT(__wait);
> @@ -302,7 +302,11 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
> sectors = bio_sectors(bio);
> /* bio could be mergeable after passing to underlayer */
> bio->bi_opf &= ~REQ_NOMERGE;
> - mddev->pers->make_request(mddev, bio);
> + if (!mddev->pers->make_request(mddev, bio)) {
> + atomic_dec(&mddev->active_io);
> + wake_up(&mddev->sb_wait);
> + goto check_suspended;
> + }
>
> cpu = part_stat_lock();
> part_stat_inc(cpu, &mddev->gendisk->part0, ios[rw]);
> @@ -327,6 +331,7 @@ void mddev_suspend(struct mddev *mddev)
> if (mddev->suspended++)
> return;
> synchronize_rcu();
> + wake_up(&mddev->sb_wait);
> wait_event(mddev->sb_wait, atomic_read(&mddev->active_io) == 0);
> mddev->pers->quiesce(mddev, 1);
>
> @@ -7950,12 +7955,14 @@ EXPORT_SYMBOL(md_done_sync);
> * If we need to update some array metadata (e.g. 'active' flag
> * in superblock) before writing, schedule a superblock update
> * and wait for it to complete.
> + * A return value of 'false' means that the write wasn't recorded
> + * and cannot proceed as the array is being suspend.
> */
> -void md_write_start(struct mddev *mddev, struct bio *bi)
> +bool md_write_start(struct mddev *mddev, struct bio *bi)
> {
> int did_change = 0;
> if (bio_data_dir(bi) != WRITE)
> - return;
> + return true;
>
> BUG_ON(mddev->ro == 1);
> if (mddev->ro == 2) {
> @@ -7987,9 +7994,13 @@ void md_write_start(struct mddev *mddev, struct bio *bi)
> if (did_change)
> sysfs_notify_dirent_safe(mddev->sysfs_state);
> wait_event(mddev->sb_wait,
> - !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags));
> + !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags) && !mddev->suspended);
> + if (test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags)) {
> + percpu_ref_put(&mddev->writes_pending);
> + return false;
> + }
> + return true;
> }
> -EXPORT_SYMBOL(md_write_start);
>
> /* md_write_inc can only be called when md_write_start() has
> * already been called at least once of the current request.
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 0fa1de42c42b..63d342d560b8 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -510,7 +510,7 @@ struct md_personality
> int level;
> struct list_head list;
> struct module *owner;
> - void (*make_request)(struct mddev *mddev, struct bio *bio);
> + bool (*make_request)(struct mddev *mddev, struct bio *bio);
> int (*run)(struct mddev *mddev);
> void (*free)(struct mddev *mddev, void *priv);
> void (*status)(struct seq_file *seq, struct mddev *mddev);
> @@ -649,7 +649,7 @@ extern void md_wakeup_thread(struct md_thread *thread);
> extern void md_check_recovery(struct mddev *mddev);
> extern void md_reap_sync_thread(struct mddev *mddev);
> extern int mddev_init_writes_pending(struct mddev *mddev);
> -extern void md_write_start(struct mddev *mddev, struct bio *bi);
> +extern bool md_write_start(struct mddev *mddev, struct bio *bi);
> extern void md_write_inc(struct mddev *mddev, struct bio *bi);
> extern void md_write_end(struct mddev *mddev);
> extern void md_done_sync(struct mddev *mddev, int blocks, int ok);
> diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c
> index e95d521d93e9..c8d985ba008d 100644
> --- a/drivers/md/multipath.c
> +++ b/drivers/md/multipath.c
> @@ -106,7 +106,7 @@ static void multipath_end_request(struct bio *bio)
> rdev_dec_pending(rdev, conf->mddev);
> }
>
> -static void multipath_make_request(struct mddev *mddev, struct bio * bio)
> +static bool multipath_make_request(struct mddev *mddev, struct bio * bio)
> {
> struct mpconf *conf = mddev->private;
> struct multipath_bh * mp_bh;
> @@ -114,7 +114,7 @@ static void multipath_make_request(struct mddev *mddev, struct bio * bio)
>
> if (unlikely(bio->bi_opf & REQ_PREFLUSH)) {
> md_flush_request(mddev, bio);
> - return;
> + return true;
> }
>
> mp_bh = mempool_alloc(conf->pool, GFP_NOIO);
> @@ -126,7 +126,7 @@ static void multipath_make_request(struct mddev *mddev, struct bio * bio)
> if (mp_bh->path < 0) {
> bio_io_error(bio);
> mempool_free(mp_bh, conf->pool);
> - return;
> + return true;
> }
> multipath = conf->multipaths + mp_bh->path;
>
> @@ -141,7 +141,7 @@ static void multipath_make_request(struct mddev *mddev, struct bio * bio)
> mddev_check_writesame(mddev, &mp_bh->bio);
> mddev_check_write_zeroes(mddev, &mp_bh->bio);
> generic_make_request(&mp_bh->bio);
> - return;
> + return true;
> }
>
> static void multipath_status(struct seq_file *seq, struct mddev *mddev)
> diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> index d6c0bc76e837..94d9ae9b0fd0 100644
> --- a/drivers/md/raid0.c
> +++ b/drivers/md/raid0.c
> @@ -548,7 +548,7 @@ static void raid0_handle_discard(struct mddev *mddev, struct bio *bio)
> bio_endio(bio);
> }
>
> -static void raid0_make_request(struct mddev *mddev, struct bio *bio)
> +static bool raid0_make_request(struct mddev *mddev, struct bio *bio)
> {
> struct strip_zone *zone;
> struct md_rdev *tmp_dev;
> @@ -559,12 +559,12 @@ static void raid0_make_request(struct mddev *mddev, struct bio *bio)
>
> if (unlikely(bio->bi_opf & REQ_PREFLUSH)) {
> md_flush_request(mddev, bio);
> - return;
> + return true;
> }
>
> if (unlikely((bio_op(bio) == REQ_OP_DISCARD))) {
> raid0_handle_discard(mddev, bio);
> - return;
> + return true;
> }
>
> bio_sector = bio->bi_iter.bi_sector;
> @@ -599,6 +599,7 @@ static void raid0_make_request(struct mddev *mddev, struct bio *bio)
> mddev_check_writesame(mddev, bio);
> mddev_check_write_zeroes(mddev, bio);
> generic_make_request(bio);
> + return true;
> }
>
> static void raid0_status(struct seq_file *seq, struct mddev *mddev)
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index e1a7e3d4c5e4..c71739b87ab7 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1321,7 +1321,6 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
> * Continue immediately if no resync is active currently.
> */
>
> - md_write_start(mddev, bio); /* wait on superblock update early */
>
> if ((bio_end_sector(bio) > mddev->suspend_lo &&
> bio->bi_iter.bi_sector < mddev->suspend_hi) ||
> @@ -1550,13 +1549,13 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
> wake_up(&conf->wait_barrier);
> }
>
> -static void raid1_make_request(struct mddev *mddev, struct bio *bio)
> +static bool raid1_make_request(struct mddev *mddev, struct bio *bio)
> {
> sector_t sectors;
>
> if (unlikely(bio->bi_opf & REQ_PREFLUSH)) {
> md_flush_request(mddev, bio);
> - return;
> + return true;
> }
>
> /*
> @@ -1571,8 +1570,12 @@ static void raid1_make_request(struct mddev *mddev, struct bio *bio)
>
> if (bio_data_dir(bio) == READ)
> raid1_read_request(mddev, bio, sectors, NULL);
> - else
> + else {
> + if (!md_write_start(mddev,bio))
> + return false;
> raid1_write_request(mddev, bio, sectors);
> + }
> + return true;
> }
>
> static void raid1_status(struct seq_file *seq, struct mddev *mddev)
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 797ed60abd5e..52acffa7a06a 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1303,8 +1303,6 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
> sector_t sectors;
> int max_sectors;
>
> - md_write_start(mddev, bio);
> -
> /*
> * Register the new request and wait if the reconstruction
> * thread has put up a bar for new requests.
> @@ -1525,7 +1523,7 @@ static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
> raid10_write_request(mddev, bio, r10_bio);
> }
>
> -static void raid10_make_request(struct mddev *mddev, struct bio *bio)
> +static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
> {
> struct r10conf *conf = mddev->private;
> sector_t chunk_mask = (conf->geo.chunk_mask & conf->prev.chunk_mask);
> @@ -1534,9 +1532,12 @@ static void raid10_make_request(struct mddev *mddev, struct bio *bio)
>
> if (unlikely(bio->bi_opf & REQ_PREFLUSH)) {
> md_flush_request(mddev, bio);
> - return;
> + return true;
> }
>
> + if (!md_write_start(mddev, bio))
> + return false;
> +
> /*
> * If this request crosses a chunk boundary, we need to split
> * it.
> @@ -1553,6 +1554,7 @@ static void raid10_make_request(struct mddev *mddev, struct bio *bio)
>
> /* In case raid10d snuck in to freeze_array */
> wake_up(&conf->wait_barrier);
> + return true;
> }
>
> static void raid10_status(struct seq_file *seq, struct mddev *mddev)
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 861fc9a8d1be..ad1a644a17bc 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -5469,7 +5469,6 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
> last_sector = bi->bi_iter.bi_sector + (bi->bi_iter.bi_size>>9);
>
> bi->bi_next = NULL;
> - md_write_start(mddev, bi);
>
> stripe_sectors = conf->chunk_sectors *
> (conf->raid_disks - conf->max_degraded);
> @@ -5539,11 +5538,10 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
> release_stripe_plug(mddev, sh);
> }
>
> - md_write_end(mddev);
> bio_endio(bi);
> }
>
> -static void raid5_make_request(struct mddev *mddev, struct bio * bi)
> +static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
> {
> struct r5conf *conf = mddev->private;
> int dd_idx;
> @@ -5559,10 +5557,10 @@ static void raid5_make_request(struct mddev *mddev, struct bio * bi)
> int ret = r5l_handle_flush_request(conf->log, bi);
>
> if (ret == 0)
> - return;
> + return true;
> if (ret == -ENODEV) {
> md_flush_request(mddev, bi);
> - return;
> + return true;
> }
> /* ret == -EAGAIN, fallback */
> /*
> @@ -5572,6 +5570,8 @@ static void raid5_make_request(struct mddev *mddev, struct bio * bi)
> do_flush = bi->bi_opf & REQ_PREFLUSH;
> }
>
> + if (!md_write_start(mddev, bi))
> + return false;
> /*
> * If array is degraded, better not do chunk aligned read because
> * later we might have to read it again in order to reconstruct
> @@ -5581,18 +5581,18 @@ static void raid5_make_request(struct mddev *mddev, struct bio * bi)
> mddev->reshape_position == MaxSector) {
> bi = chunk_aligned_read(mddev, bi);
> if (!bi)
> - return;
> + return true;
> }
>
> if (unlikely(bio_op(bi) == REQ_OP_DISCARD)) {
> make_discard_request(mddev, bi);
> - return;
> + md_write_end(mddev);
> + return true;
> }
>
> logical_sector = bi->bi_iter.bi_sector & ~((sector_t)STRIPE_SECTORS-1);
> last_sector = bio_end_sector(bi);
> bi->bi_next = NULL;
> - md_write_start(mddev, bi);
>
> prepare_to_wait(&conf->wait_for_overlap, &w, TASK_UNINTERRUPTIBLE);
> for (;logical_sector < last_sector; logical_sector += STRIPE_SECTORS) {
> @@ -5730,6 +5730,7 @@ static void raid5_make_request(struct mddev *mddev, struct bio * bi)
> if (rw == WRITE)
> md_write_end(mddev);
> bio_endio(bi);
> + return true;
> }
>
> static sector_t raid5_size(struct mddev *mddev, sector_t sectors, int raid_disks);
> --
> 2.12.2
>
^ permalink raw reply
* Re: 9 second recovery when re-adding a drive that got kicked out?
From: Phil Turmel @ 2017-06-06 2:58 UTC (permalink / raw)
To: Marc MERLIN, linux-raid@vger.kernel.org
In-Reply-To: <20170604223817.ntxlvafgfa2umww2@merlins.org>
On 06/04/2017 06:38 PM, Marc MERLIN wrote:
> Howdy,
>
> Can you confirm that I understand how the write intent bitmap works, and
> that it doesn't cover the entire array, but only a part of it, and once
> you overflow it, syncing reverts to syncing the entire array?
There's no overflow. The size correlation between bits in the bitmap
and areas in the array is adjusted so the whole array can be
represented. One bit can be many multiples of a page, iirc.
> I had a raid5 array with 5 6TB drives.
>
> /dev/sdl1 got kicked out due to a bus disk error of some kind.
> The drive is fine, it was a cabling issue, so I fixed the cabling,
> re-added it, and did
>
> gargamel:~# mdadm -a /dev/md6 /dev/sdl1
>
> Then I saw this:
> [ 1001.728134] md: recovery of RAID array md6
> [ 1010.975255] md: md6: recovery done.
> So let's say I have 64MB chuncks, each take 16 bits.
> The whole array is 22,892,144MiB
> That's 357,689 chunks, or about 700KB (16 bits per chunk) to keep all the
> state, but there is 44 pages of 4KB, or 176KB of write intent
> state.
So each bit in the bitmap represents two 4k pages.
Nine seconds to re-add after a short disconnect is perfectly normal.
For lightly loaded arrays, it can be virtually instant.
Phil
^ permalink raw reply
* Re: internal write-intent bitmap is horribly slow with RAID10 over 20 drives
From: NeilBrown @ 2017-06-06 3:40 UTC (permalink / raw)
To: CoolCold, Linux RAID
In-Reply-To: <CAGqmV7qRh+_BXAr-qf6vUkiZyzi69E8CiX_vA92tZV2LfDxq_A@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2205 bytes --]
On Mon, Jun 05 2017, CoolCold wrote:
> Hello!
> Keep testing the new box and while having not the best sync speed,
> it's not the worst thing I found.
>
> Doing FIO testing, for RAID10 over 20 10k RPM drives, I have very bad
> performance, like _45_ iops only.
...
>
>
> Output from fio with internal write-intent bitmap:
> Jobs: 1 (f=1): [w(1)] [28.3% done] [0KB/183KB/0KB /s] [0/45/0 iops]
> [eta 07m:11s]
>
> array definition:
> [root@spare-a17484327407661 rovchinnikov]# cat /proc/mdstat
> Personalities : [raid1] [raid10] [raid6] [raid5] [raid4]
> md1 : active raid10 sdx[19] sdw[18] sdv[17] sdu[16] sdt[15] sds[14]
> sdr[13] sdq[12] sdp[11] sdo[10] sdn[9] sdm[8] sdl[7] sdk[6] sdj[5]
> sdi[4] sdh[3] sdg[2] sdf[1] sde[0]
> 17580330880 blocks super 1.2 64K chunks 2 near-copies [20/20]
> [UUUUUUUUUUUUUUUUUUUU]
> bitmap: 0/66 pages [0KB], 131072KB chunk
>
> Setting journal to be
> 1) on SSD (separate drives), shows
> Jobs: 1 (f=1): [w(1)] [5.0% done] [0KB/18783KB/0KB /s] [0/4695/0 iops]
> [eta 09m:31s]
> 2) to 'none' (disabling) shows
> Jobs: 1 (f=1): [w(1)] [14.0% done] [0KB/18504KB/0KB /s] [0/4626/0
> iops] [eta 08m:36s]
These numbers suggest that the write intent bitmap causes a 100-fold slow
down.
i.e. 45 iops instead of 4500 iops (roughly).
That is certainly more than I would expect, so maybe there is a bug.
Large RAID10 is a worst-base for bitmap updates as the bitmap is written
to all devices instead of just those devices that contain the data which
the bit corresponds to. So every bitmap update goes to 10 device.
Your bitmap chunk size of 128M is nice and large, but making it larger
might help - maybe 1GB.
Still 100-fold ... that's a lot..
A potentially useful exercise would be to run a series of tests,
changing the number of devices in the array from 2 to 10, changing the
RAID chunk size from 64K to 64M, and changing the bitmap chunk size from
64M to 4G.
In each configuration, run the same test and record the iops.
(You don't need to wait for a resync each time, just use
--assume-clean).
Then graph all this data (or just provide the table and I'll graph it).
That might provide an insight into where to start looking for the
slowdown.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: 9 second recovery when re-adding a drive that got kicked out?
From: NeilBrown @ 2017-06-06 3:57 UTC (permalink / raw)
To: Marc MERLIN, linux-raid@vger.kernel.org
In-Reply-To: <20170604223817.ntxlvafgfa2umww2@merlins.org>
[-- Attachment #1: Type: text/plain, Size: 4975 bytes --]
On Sun, Jun 04 2017, Marc MERLIN wrote:
> Howdy,
>
> Can you confirm that I understand how the write intent bitmap works, and
> that it doesn't cover the entire array, but only a part of it, and once
> you overflow it, syncing reverts to syncing the entire array?
>
> I had a raid5 array with 5 6TB drives.
>
> /dev/sdl1 got kicked out due to a bus disk error of some kind.
> The drive is fine, it was a cabling issue, so I fixed the cabling,
> re-added it, and did
>
> gargamel:~# mdadm -a /dev/md6 /dev/sdl1
>
> Then I saw this:
> [ 1001.728134] md: recovery of RAID array md6
> [ 1010.975255] md: md6: recovery done.
>
> Before the re-add:
> md6 : active raid5 sdk1[5] sdb1[3] sdm1[2] sdj1[1]
> 23441555456 blocks super 1.2 level 5, 512k chunk, algorithm 2 [5/4] [_UUUU]
> bitmap: 3/44 pages [12KB], 65536KB chunk
>
> After the re-add (syncing now just to be safe):
> md6 : active raid5 sdl1[0] sdj1[1] sdk1[5] sdf1[3] sdm1[2]
> 23441555456 blocks super 1.2 level 5, 512k chunk, algorithm 2 [5/5] [UUUUU]
> [>....................] check = 0.8% (49258960/5860388864) finish=569.3min speed=170093K/sec
> bitmap: 0/44 pages [0KB], 65536KB chunk
>
> https://raid.wiki.kernel.org/index.php/Mdstat
> Explains a bit, I don't think it says how big a page is, but it seems to
> be 4KB.
>
> So let's say I have 64MB chuncks, each take 16 bits.
> The whole array is 22,892,144MiB
> That's 357,689 chunks, or about 700KB (16 bits per chunk) to keep all the
> state, but there is 44 pages of 4KB, or 176KB of write intent
> state.
>
> The first bitmap line shows 3 pages totallying 12KB, so each page
> contains 4KB, or 2048 chunks per page.
> Did the above say that I had 6144 chunks that needed to be synced?
No. It said that of the 44 pages of space that might be needed to store
16-bit counters that each represent 1 bitmap-chunk, only 3 of those
pages would contain non-zero counters, so only 3 had been allocated.
There could be as few as 3 chunks that need to be recovered, or there
could be as many a 3*2048 chunks, or any number in between.
Had you run "mdadm --examine-bitmap /dev/sdk1" before the re-add, it
would have told you how many bits were set at that time.
That "x/y pages" information never should have appeared in /proc/mdstat
- it is really just of interest to developers. But it is there now, so
removing it is awkward.
>
> If so it would be 6144 * 65536KB = 393,216 MB to write
> They were written in 9 seconds, so the sync happened at 43MB/s, which is
> believeable.
>
> The part I'm not too clear about is 44 pages of intent isn't enough to
> cover all my data.
44 pages means 90112 16 bit counters, one for each 64M on each device.
90112 * 64M = 5632 GiB or 5905GB.
That is the size of each device.
One bit in the bitmap (one counter in the internal bitmap) corresponds
to "a set of data the might be out of sync" which, in your case, is a
64MB wide stripe across all devices.
So the numbers do add up.
NeilBrown
> Is the idea that once I overflow that write intent bitmap, then it
> reverts to resyncing the entire array?
>
> I looked at https://raid.wiki.kernel.org/index.php/Write-intent_bitmap
> but didn't see anything about that specific bit.
>
>
> Array details if that helps:
> gargamel:~# mdadm --examine /dev/sdl1
> /dev/sdl1:
> Magic : a92b4efc
> Version : 1.2
> Feature Map : 0x1
> Array UUID : 66bccdfb:afbf9683:fcf1f12e:f2af2dcb
> Name : gargamel.svh.merlins.org:6 (local to host gargamel.svh.merlins.org)
> Creation Time : Thu Jan 28 14:38:40 2016
> Raid Level : raid5
> Raid Devices : 5
>
> Avail Dev Size : 11720777728 (5588.90 GiB 6001.04 GB)
> Array Size : 23441555456 (22355.61 GiB 24004.15 GB)
> Data Offset : 262144 sectors
> Super Offset : 8 sectors
> Unused Space : before=262056 sectors, after=0 sectors
> State : clean
> Device UUID : ca4598ba:de585baa:b9935222:e06ac97d
>
> Internal Bitmap : 8 sectors from superblock
> Update Time : Sun Jun 4 15:08:45 2017
> Bad Block Log : 512 entries available at offset 72 sectors
> Checksum : d645f600 - correct
> Events : 84917
>
> Layout : left-symmetric
> Chunk Size : 512K
>
> Device Role : Active device 0
> Array State : AAAAA ('A' == active, '.' == missing, 'R' == replacing)
>
> Thanks,
> Marc
> --
> "A mouse is a device used to point at the xterm you want to type in" - A.S.R.
> Microsoft is to operating systems ....
> .... what McDonalds is to gourmet cooking
> Home page: http://marc.merlins.org/ | PGP 1024R/763BE901
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* [PATCH] mdadm: Send version text to stdout rather than stderr
From: Zhilong Liu @ 2017-06-06 7:59 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
This helps with piping to a paper.
Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
mdadm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mdadm.c b/mdadm.c
index 70b16f2..0f736d7 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -139,7 +139,7 @@ int main(int argc, char *argv[])
continue;
case 'V':
- fputs(Version, stderr);
+ fputs(Version, stdout);
exit(0);
case 'v': c.verbose++;
--
2.6.6
^ permalink raw reply related
* mdadm: Apply to revert the commit of a4dcdb23
From: Zhilong Liu @ 2017-06-06 9:25 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid@vger.kernel.org
Hi, Jes;
Excuse me, there is one problematic commit for Detail.c,
a4dcdb23(Detail: determine array state from sysfs).
This commit does change the codes meaning and it didn't
run to the right path. This commit was result in a bug that
Detail() cannot work normally.
For example:
# ./mdadm -CR /dev/md0 -l1 -n2 /dev/loop[0-1] --assume-clean
# ./mdadm -D /dev/md0
mdadm: cannot get array detail for /dev/md0: No such file or directory
Thanks very much,
-Zhilong
^ permalink raw reply
* Re: internal write-intent bitmap is horribly slow with RAID10 over 20 drives
From: CoolCold @ 2017-06-06 11:31 UTC (permalink / raw)
To: NeilBrown; +Cc: Linux RAID
In-Reply-To: <87tw3tdbs1.fsf@notabene.neil.brown.name>
Hello!
Neil, thanks for reply, further inline
On Tue, Jun 6, 2017 at 10:40 AM, NeilBrown <neilb@suse.com> wrote:
> On Mon, Jun 05 2017, CoolCold wrote:
>
>> Hello!
>> Keep testing the new box and while having not the best sync speed,
>> it's not the worst thing I found.
>>
>> Doing FIO testing, for RAID10 over 20 10k RPM drives, I have very bad
>> performance, like _45_ iops only.
>
> ...
>>
>>
>> Output from fio with internal write-intent bitmap:
>> Jobs: 1 (f=1): [w(1)] [28.3% done] [0KB/183KB/0KB /s] [0/45/0 iops]
>> [eta 07m:11s]
>>
>> array definition:
>> [root@spare-a17484327407661 rovchinnikov]# cat /proc/mdstat
>> Personalities : [raid1] [raid10] [raid6] [raid5] [raid4]
>> md1 : active raid10 sdx[19] sdw[18] sdv[17] sdu[16] sdt[15] sds[14]
>> sdr[13] sdq[12] sdp[11] sdo[10] sdn[9] sdm[8] sdl[7] sdk[6] sdj[5]
>> sdi[4] sdh[3] sdg[2] sdf[1] sde[0]
>> 17580330880 blocks super 1.2 64K chunks 2 near-copies [20/20]
>> [UUUUUUUUUUUUUUUUUUUU]
>> bitmap: 0/66 pages [0KB], 131072KB chunk
>>
>> Setting journal to be
>> 1) on SSD (separate drives), shows
>> Jobs: 1 (f=1): [w(1)] [5.0% done] [0KB/18783KB/0KB /s] [0/4695/0 iops]
>> [eta 09m:31s]
>> 2) to 'none' (disabling) shows
>> Jobs: 1 (f=1): [w(1)] [14.0% done] [0KB/18504KB/0KB /s] [0/4626/0
>> iops] [eta 08m:36s]
>
> These numbers suggest that the write intent bitmap causes a 100-fold slow
> down.
> i.e. 45 iops instead of 4500 iops (roughly).
>
> That is certainly more than I would expect, so maybe there is a bug.
I suppose noone is using raid10 over more than 4 drives then, i can't
believe i'm the one who hit this problem.
>
> Large RAID10 is a worst-base for bitmap updates as the bitmap is written
> to all devices instead of just those devices that contain the data which
> the bit corresponds to. So every bitmap update goes to 10 device.
>
> Your bitmap chunk size of 128M is nice and large, but making it larger
> might help - maybe 1GB.
Tried that already, wasn't any much difference, but will gather more statistics.
>
> Still 100-fold ... that's a lot..
>
> A potentially useful exercise would be to run a series of tests,
> changing the number of devices in the array from 2 to 10, changing the
> RAID chunk size from 64K to 64M, and changing the bitmap chunk size from
> 64M to 4G.
Changing chunk size to up to 64M just to gather statistics or you
suppose it may be some practical usage for this?
> In each configuration, run the same test and record the iops.
> (You don't need to wait for a resync each time, just use
> --assume-clean).
This helps, thanks
> Then graph all this data (or just provide the table and I'll graph it).
> That might provide an insight into where to start looking for the
> slowdown.
>
> NeilBrown
--
Best regards,
[COOLCOLD-RIPN]
^ permalink raw reply
* Re: [PATCH] Grow: set component size prior to array size
From: Tomasz Majchrzak @ 2017-06-06 14:47 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid
In-Reply-To: <611d0b8e-f42f-48de-c881-bd55339d434c@gmail.com>
On Mon, Jun 05, 2017 at 11:06:48AM -0400, Jes Sorensen wrote:
> On 06/05/2017 10:09 AM, Tomasz Majchrzak wrote:
> >It is a partial revert of commit 758b327cf5a7 ("Grow: Remove unnecessary
> >optimization"). For native metadata component size is set in kernel for
> >entire disk space. As external metadata supports multiple arrays within
> >one disk, the component size is set to array size. If component size is
> >not updated prior to array size update, the grow operation fails.
> >
> >Signed-off-by: Tomasz Majchrzak <tomasz.majchrzak@intel.com>
> >---
> > Grow.c | 2 ++
> > 1 file changed, 2 insertions(+)
>
> Applied!
>
> Thanks,
> Jes
>
Would you mind pushing it to the repository? I'd like to have it ported into
certain Linux distribution.
Tomek
^ permalink raw reply
* Re: [PATCH] Grow: set component size prior to array size
From: Jes Sorensen @ 2017-06-06 16:00 UTC (permalink / raw)
To: Tomasz Majchrzak; +Cc: linux-raid
In-Reply-To: <20170606144732.GA1070@proton.igk.intel.com>
On 06/06/2017 10:47 AM, Tomasz Majchrzak wrote:
> On Mon, Jun 05, 2017 at 11:06:48AM -0400, Jes Sorensen wrote:
>> On 06/05/2017 10:09 AM, Tomasz Majchrzak wrote:
>>> It is a partial revert of commit 758b327cf5a7 ("Grow: Remove unnecessary
>>> optimization"). For native metadata component size is set in kernel for
>>> entire disk space. As external metadata supports multiple arrays within
>>> one disk, the component size is set to array size. If component size is
>>> not updated prior to array size update, the grow operation fails.
>>>
>>> Signed-off-by: Tomasz Majchrzak <tomasz.majchrzak@intel.com>
>>> ---
>>> Grow.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>
>> Applied!
>>
>> Thanks,
>> Jes
>>
>
> Would you mind pushing it to the repository? I'd like to have it ported into
> certain Linux distribution.
Only because it's you :)
Done!
Jes
^ permalink raw reply
* Re: mdadm: Apply to revert the commit of a4dcdb23
From: Jes Sorensen @ 2017-06-06 16:02 UTC (permalink / raw)
To: Zhilong Liu; +Cc: linux-raid@vger.kernel.org
In-Reply-To: <0010fa6f-f227-cdd8-31ef-7073ec46546a@suse.com>
On 06/06/2017 05:25 AM, Zhilong Liu wrote:
> Hi, Jes;
> Excuse me, there is one problematic commit for Detail.c,
> a4dcdb23(Detail: determine array state from sysfs).
> This commit does change the codes meaning and it didn't
> run to the right path. This commit was result in a bug that
> Detail() cannot work normally.
>
> For example:
> # ./mdadm -CR /dev/md0 -l1 -n2 /dev/loop[0-1] --assume-clean
> # ./mdadm -D /dev/md0
> mdadm: cannot get array detail for /dev/md0: No such file or directory
>
> Thanks very much,
> -Zhilong
Please be more specific and explain what you believe is wrong. I prefer
a patch fixing the issue to just reverting things.
Jes
^ permalink raw reply
* Re: internal write-intent bitmap is horribly slow with RAID10 over 20 drives
From: NeilBrown @ 2017-06-06 22:02 UTC (permalink / raw)
To: CoolCold; +Cc: Linux RAID
In-Reply-To: <CAGqmV7qVq8nBmop5h7OZKr9ZjCjGLcoPqMF5xa4d2iHzf2uQdw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3630 bytes --]
On Tue, Jun 06 2017, CoolCold wrote:
> Hello!
> Neil, thanks for reply, further inline
>
> On Tue, Jun 6, 2017 at 10:40 AM, NeilBrown <neilb@suse.com> wrote:
>> On Mon, Jun 05 2017, CoolCold wrote:
>>
>>> Hello!
>>> Keep testing the new box and while having not the best sync speed,
>>> it's not the worst thing I found.
>>>
>>> Doing FIO testing, for RAID10 over 20 10k RPM drives, I have very bad
>>> performance, like _45_ iops only.
>>
>> ...
>>>
>>>
>>> Output from fio with internal write-intent bitmap:
>>> Jobs: 1 (f=1): [w(1)] [28.3% done] [0KB/183KB/0KB /s] [0/45/0 iops]
>>> [eta 07m:11s]
>>>
>>> array definition:
>>> [root@spare-a17484327407661 rovchinnikov]# cat /proc/mdstat
>>> Personalities : [raid1] [raid10] [raid6] [raid5] [raid4]
>>> md1 : active raid10 sdx[19] sdw[18] sdv[17] sdu[16] sdt[15] sds[14]
>>> sdr[13] sdq[12] sdp[11] sdo[10] sdn[9] sdm[8] sdl[7] sdk[6] sdj[5]
>>> sdi[4] sdh[3] sdg[2] sdf[1] sde[0]
>>> 17580330880 blocks super 1.2 64K chunks 2 near-copies [20/20]
>>> [UUUUUUUUUUUUUUUUUUUU]
>>> bitmap: 0/66 pages [0KB], 131072KB chunk
>>>
>>> Setting journal to be
>>> 1) on SSD (separate drives), shows
>>> Jobs: 1 (f=1): [w(1)] [5.0% done] [0KB/18783KB/0KB /s] [0/4695/0 iops]
>>> [eta 09m:31s]
>>> 2) to 'none' (disabling) shows
>>> Jobs: 1 (f=1): [w(1)] [14.0% done] [0KB/18504KB/0KB /s] [0/4626/0
>>> iops] [eta 08m:36s]
>>
>> These numbers suggest that the write intent bitmap causes a 100-fold slow
>> down.
>> i.e. 45 iops instead of 4500 iops (roughly).
>>
>> That is certainly more than I would expect, so maybe there is a bug.
> I suppose noone is using raid10 over more than 4 drives then, i can't
> believe i'm the one who hit this problem.
We have customers who use RAID10 will many more than 4 drives, but I
haven't had reports like this. Presumably whatever problem is affecting
you is not affecting them. We cannot know until we drill down to
understand the problem.
>
>>
>> Large RAID10 is a worst-base for bitmap updates as the bitmap is written
>> to all devices instead of just those devices that contain the data which
>> the bit corresponds to. So every bitmap update goes to 10 device.
>>
>> Your bitmap chunk size of 128M is nice and large, but making it larger
>> might help - maybe 1GB.
> Tried that already, wasn't any much difference, but will gather more statistics.
>
>>
>> Still 100-fold ... that's a lot..
>>
>> A potentially useful exercise would be to run a series of tests,
>> changing the number of devices in the array from 2 to 10, changing the
>> RAID chunk size from 64K to 64M, and changing the bitmap chunk size from
>> 64M to 4G.
> Changing chunk size to up to 64M just to gather statistics or you
> suppose it may be some practical usage for this?
I don't have any particular reason to expect this to have an effect.
But it is easy to change, and changing it might show provide some hints.
So probably "just to gather statistics".
NeilBrown
>> In each configuration, run the same test and record the iops.
>> (You don't need to wait for a resync each time, just use
>> --assume-clean).
> This helps, thanks
>> Then graph all this data (or just provide the table and I'll graph it).
>> That might provide an insight into where to start looking for the
>> slowdown.
>>
>> NeilBrown
>
>
>
> --
> Best regards,
> [COOLCOLD-RIPN]
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* (unknown),
From: mdavis @ 2017-06-06 23:46 UTC (permalink / raw)
To: linux-raid
[-- Attachment #1: 37913653393087.zip --]
[-- Type: application/zip, Size: 4706 bytes --]
^ permalink raw reply
* Re: [PATCH] mdadm: Send version text to stdout rather than stderr
From: NeilBrown @ 2017-06-07 1:44 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
In-Reply-To: <1496735987-10653-1-git-send-email-zlliu@suse.com>
[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]
On Tue, Jun 06 2017, Zhilong Liu wrote:
> This helps with piping to a paper.
It might help you, but it is an API change and could cause problems for
others.
Some programs, such as "mount -V", send version info to stdout.
Others, like "gcc -v", send it to stderr.
There is no uniform standard, it is best to leave the behavior
unchanged.
If you need to capture the output use
mdadm -V |& ...
or
mdadm -V 2>&1
or similar.
NeilBrtown
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
> ---
> mdadm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mdadm.c b/mdadm.c
> index 70b16f2..0f736d7 100644
> --- a/mdadm.c
> +++ b/mdadm.c
> @@ -139,7 +139,7 @@ int main(int argc, char *argv[])
> continue;
>
> case 'V':
> - fputs(Version, stderr);
> + fputs(Version, stdout);
> exit(0);
>
> case 'v': c.verbose++;
> --
> 2.6.6
>
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH] md: fix deadlock between mddev_suspend() and md_write_start()
From: NeilBrown @ 2017-06-07 1:45 UTC (permalink / raw)
To: Shaohua Li; +Cc: Nix, linux-raid
In-Reply-To: <20170606000115.jijxe6xvkclm4tsb@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1201 bytes --]
On Mon, Jun 05 2017, Shaohua Li wrote:
> On Mon, Jun 05, 2017 at 04:49:39PM +1000, Neil Brown wrote:
>>
>> If mddev_suspend() races with md_write_start() we can deadlock
>> with mddev_suspend() waiting for the request that is currently
>> in md_write_start() to complete the ->make_request() call,
>> and md_write_start() waiting for the metadata to be updated
>> to mark the array as 'dirty'.
>> As metadata updates done by md_check_recovery() only happen then
>> the mddev_lock() can be claimed, and as mddev_suspend() is often
>> called with the lock held, these threads wait indefinitely for each
>> other.
>>
>> We fix this by having md_write_start() abort if mddev_suspend()
>> is happening, and ->make_request() aborts if md_write_start()
>> aborted.
>> md_make_request() can detect this abort, decrease the ->active_io
>> count, and wait for mddev_suspend().
>
> Applied the two patches, thanks! For this one, I added md_start_write symbol back. Will
> push these to Linus in the 4.12 cycle.
>
Thanks for fixing the md_start_write export! At one stage in
development I made it "static", but then forgot the export when I put it
back :-(
Thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: mdadm: Apply to revert the commit of a4dcdb23
From: Zhilong Liu @ 2017-06-07 2:03 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid@vger.kernel.org
In-Reply-To: <0d1a79b0-a7fd-656d-5cfd-a85b4398c284@gmail.com>
On 06/07/2017 12:02 AM, Jes Sorensen wrote:
> On 06/06/2017 05:25 AM, Zhilong Liu wrote:
>> Hi, Jes;
>> Excuse me, there is one problematic commit for Detail.c,
>> a4dcdb23(Detail: determine array state from sysfs).
>> This commit does change the codes meaning and it didn't
>> run to the right path. This commit was result in a bug that
>> Detail() cannot work normally.
>>
>> For example:
>> # ./mdadm -CR /dev/md0 -l1 -n2 /dev/loop[0-1] --assume-clean
>> # ./mdadm -D /dev/md0
>> mdadm: cannot get array detail for /dev/md0: No such file or directory
>>
>> Thanks very much,
>> -Zhilong
>
> Please be more specific and explain what you believe is wrong. I
> prefer a patch fixing the issue to just reverting things.
>
I just re-checked the mailing-list, found the email that post the commit
of 9b8fea914f82
from Tomasz in May.24th, it has already fixed this issue. please ignore
this mail. Sorry
for that.
Thanks,
-Zhilong
> Jes
>
> --
> 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] mdadm: Send version text to stdout rather than stderr
From: Zhilong Liu @ 2017-06-07 2:11 UTC (permalink / raw)
To: NeilBrown, Jes.Sorensen; +Cc: linux-raid
In-Reply-To: <877f0od124.fsf@notabene.neil.brown.name>
On 06/07/2017 09:44 AM, NeilBrown wrote:
> On Tue, Jun 06 2017, Zhilong Liu wrote:
>
>> This helps with piping to a paper.
> It might help you, but it is an API change and could cause problems for
> others.
>
> Some programs, such as "mount -V", send version info to stdout.
> Others, like "gcc -v", send it to stderr.
> There is no uniform standard, it is best to leave the behavior
> unchanged.
>
> If you need to capture the output use
> mdadm -V |& ...
> or
> mdadm -V 2>&1
>
> or similar.
Thanks for your detail explanations. Yes, I just encountered this issue
in my progress of improving ./mdadm/test script,
the ./mdadm --version always prints to stderr, thus I post this email,
and I have already used "2>&1" in my test script.
Thanks very much,
-Zhilong
> NeilBrtown
>
>> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>> ---
>> mdadm.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mdadm.c b/mdadm.c
>> index 70b16f2..0f736d7 100644
>> --- a/mdadm.c
>> +++ b/mdadm.c
>> @@ -139,7 +139,7 @@ int main(int argc, char *argv[])
>> continue;
>>
>> case 'V':
>> - fputs(Version, stderr);
>> + fputs(Version, stdout);
>> exit(0);
>>
>> case 'v': c.verbose++;
>> --
>> 2.6.6
>>
>> --
>> 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: 9 second recovery when re-adding a drive that got kicked out?
From: Marc MERLIN @ 2017-06-07 3:03 UTC (permalink / raw)
To: NeilBrown, Phil Turmel; +Cc: linux-raid@vger.kernel.org
In-Reply-To: <87r2yxdazc.fsf@notabene.neil.brown.name>
On Tue, Jun 06, 2017 at 01:57:27PM +1000, NeilBrown wrote:
> > The first bitmap line shows 3 pages totallying 12KB, so each page
> > contains 4KB, or 2048 chunks per page.
> > Did the above say that I had 6144 chunks that needed to be synced?
>
> No. It said that of the 44 pages of space that might be needed to store
> 16-bit counters that each represent 1 bitmap-chunk, only 3 of those
> pages would contain non-zero counters, so only 3 had been allocated.
>
> There could be as few as 3 chunks that need to be recovered, or there
> could be as many a 3*2048 chunks, or any number in between.
Ah, I see. I wasn't clear about that part, thanks.
> Had you run "mdadm --examine-bitmap /dev/sdk1" before the re-add, it
> would have told you how many bits were set at that time.
Noted for next time, thanks.
> > The part I'm not too clear about is 44 pages of intent isn't enough to
> > cover all my data.
>
> 44 pages means 90112 16 bit counters, one for each 64M on each device.
> 90112 * 64M = 5632 GiB or 5905GB.
> That is the size of each device.
Ah, so it's not based on the 512k chunk size, I see.
> One bit in the bitmap (one counter in the internal bitmap) corresponds
> to "a set of data the might be out of sync" which, in your case, is a
> 64MB wide stripe across all devices.
I got that part now, just now where the 64MB came from, or how I ended
up with 44 pages of intent maps if it's not based on my chunk size.
Thanks,
Marc
--
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
.... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/ | PGP 1024R/763BE901
^ permalink raw reply
* [PATCH 0/3] mdadm/test: Refactor the codes of 'test' script
From: Zhilong Liu @ 2017-06-07 9:31 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
Hi, Jes;
This patchset mainly focus on improving the 'test' script, rip it
apart and put it together in a more coherent form.
Over 3 months passed, the RFC patch of improving 'test' hadn't received
any ideas, thus I post this sample version that I have tested many times.
It's just the beginning to improve the test part of mdadm, any questions
is very welcome.
Sorry for that, I haven't added the function suggested from Guoqing, cut
the words from the email tree of
"ANNOUNCE: mdadm 4.0 - A tool for managing md Soft RAID under Linux":
Guoqing asked:
...
Is current test can run the test for different raid level, say,
"./test --raidtype=raid1"
could execute all the *r1* tests, does it make sense to do it if we don't
support it now.
...
Just list this suggestion here, waiting for ideas.
RFC patch mail-tree:
[PATCH RFC] test: revise 'test' and make it easier to understand
Acked-by: Neil Brown
And suggest to split those changing into smaller patches from Guoqing.
Thanks very much,
Zhilong
Zhilong Liu (3):
mdadm/test: Convert code format to use Tab
mdadm/test: Refactor the codes of 'test' script
mdadm/test: Get rid of fast_sync()
test | 803 +++++++++++++++++++++++++++++++++++++------------------------------
1 file changed, 446 insertions(+), 357 deletions(-)
--
2.6.6
^ permalink raw reply
* [PATCH 1/3] mdadm/test: Convert code format to use Tab
From: Zhilong Liu @ 2017-06-07 9:31 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
In-Reply-To: <1496827888-17118-1-git-send-email-zlliu@suse.com>
In case to make codes neat, this commit didn't change
any codes, just tidy up and use Tab as code format.
Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
test | 702 ++++++++++++++++++++++++++++++++++++-------------------------------
1 file changed, 378 insertions(+), 324 deletions(-)
diff --git a/test b/test
index 13f1bda..7942d6e 100755
--- a/test
+++ b/test
@@ -3,8 +3,9 @@
# run test suite for mdadm
user=`id -un`
if [ " $user" != " root" ]
-then echo >&2 "test: testing can only be done as 'root'."
- exit 1;
+then
+ echo >&2 "test: testing can only be done as 'root'."
+ exit 1
fi
prefix='[0-9][0-9]'
@@ -13,7 +14,7 @@ dir=`pwd`
mdadm=$dir/mdadm
if [ \! -x $mdadm ]
then
- echo >&2 "test: $mdadm isn't usable."
+ echo >&2 "test: $mdadm isn't usable."
fi
testdir="tests"
@@ -25,8 +26,9 @@ echo "Testing on linux-$(uname -r) kernel"
# Check whether to run multipath tests
modprobe multipath 2> /dev/null
-if grep -s 'Personalities : .*multipath' > /dev/null /proc/mdstat ; then
- MULTIPATH="yes"
+if grep -s 'Personalities : .*multipath' > /dev/null /proc/mdstat
+then
+ MULTIPATH="yes"
fi
INTEGRITY=yes
DEVTYPE=loop
@@ -36,7 +38,9 @@ LVM_VOLGROUP=mdtest
export MDADM_NO_SYSTEMCTL=1
# assume md0, md1, md2 exist in /dev
-md0=/dev/md0 md1=/dev/md1 md2=/dev/md2
+md0=/dev/md0
+md1=/dev/md1
+md2=/dev/md2
mdp0=/dev/md_d0
mdp1=/dev/md_d1
@@ -69,21 +73,21 @@ config=/tmp/mdadm.conf
cleanup() {
udevadm settle
$mdadm -Ssq 2> /dev/null
- case $DEVTYPE in
- loop)
- for d in 0 1 2 3 4 5 6 7 8 9 10 11 12 13
- do
- losetup -d /dev/loop$d ; # rm -f $targetdir/mdtest$d
- rm -f /dev/disk/by-path/loop*
- done
- ;;
- lvm)
- for d in 0 1 2 3 4 5 6 7 8 9 10 11 12 13
- do
- eval "lvremove --quiet -f \$dev$d"
- done
- ;;
- esac
+ case $DEVTYPE in
+ loop)
+ for d in 0 1 2 3 4 5 6 7 8 9 10 11 12 13
+ do
+ losetup -d /dev/loop$d # rm -f $targetdir/mdtest$d
+ rm -f /dev/disk/by-path/loop*
+ done
+ ;;
+ lvm)
+ for d in 0 1 2 3 4 5 6 7 8 9 10 11 12 13
+ do
+ eval "lvremove --quiet -f \$dev$d"
+ done
+ ;;
+ esac
}
ctrl_c() {
@@ -91,329 +95,377 @@ ctrl_c() {
}
do_setup() {
- trap cleanup 0 1 3 15
- trap ctrl_c 2
+ trap cleanup 0 1 3 15
+ trap ctrl_c 2
- # make sure there are no loop devices remaining.
- # udev started things can sometimes prevent them being stopped
- # immediately
- while grep loop /proc/partitions > /dev/null 2>&1
- do
- mdadm -Ss
- losetup -d /dev/loop[0-9]* 2> /dev/null
- sleep 1
- done
- devlist=
- for d in 0 1 2 3 4 5 6 7 8 9 10 11 12 13
- do
- sz=$size
- if [ $d -gt 7 ]; then sz=$ddfsize ; fi
- case $DEVTYPE in
- loop)
- [ -f $targetdir/mdtest$d ] || dd if=/dev/zero of=$targetdir/mdtest$d count=$sz bs=1K > /dev/null 2>&1
- # make sure udev doesn't touch
- mdadm --zero $targetdir/mdtest$d 2> /dev/null
- [ -b /dev/loop$d ] || mknod /dev/loop$d b 7 $d
- if [ $d -eq 7 ]
- then
- losetup /dev/loop$d $targetdir/mdtest6 # for multipath use
- else
- losetup /dev/loop$d $targetdir/mdtest$d
- fi
- eval dev$d=/dev/loop$d
- eval file$d=$targetdir/mdtest$d
- ;;
- lvm)
- unset MULTIPATH
- eval dev$d=/dev/mapper/${LVM_VOLGROUP}-mdtest$d
- if ! lvcreate --quiet -L ${sz}K -n mdtest$d $LVM_VOLGROUP; then
- trap '' 0 # make sure lvremove is not called
- eval echo error creating \$dev$d
- exit 129
- fi
- ;;
- ram)
- unset MULTIPATH
- eval dev$d=/dev/ram$d
- ;;
- esac
- eval devlist=\"\$devlist \$dev$d\"
- eval devlist$d=\"\$devlist\"
- #" <-- add this quote to un-confuse vim syntax highlighting
- done
- path0=$dev6
- path1=$dev7
-
- ulimit -c unlimited
- [ -f /proc/mdstat ] || modprobe md_mod
- echo 2000 > /proc/sys/dev/raid/speed_limit_max
- echo 0 > /sys/module/md_mod/parameters/start_ro
+ # make sure there are no loop devices remaining.
+ # udev started things can sometimes prevent them being stopped
+ # immediately
+ while grep loop /proc/partitions > /dev/null 2>&1
+ do
+ mdadm -Ss
+ losetup -d /dev/loop[0-9]* 2> /dev/null
+ sleep 1
+ done
+ devlist=
+ for d in 0 1 2 3 4 5 6 7 8 9 10 11 12 13
+ do
+ sz=$size
+ if [ $d -gt 7 ]
+ then
+ sz=$ddfsize
+ fi
+ case $DEVTYPE in
+ loop)
+ [ -f $targetdir/mdtest$d ] ||
+ dd if=/dev/zero of=$targetdir/mdtest$d count=$sz bs=1K > /dev/null 2>&1
+ # make sure udev doesn't touch
+ mdadm --zero $targetdir/mdtest$d 2> /dev/null
+ [ -b /dev/loop$d ] || mknod /dev/loop$d b 7 $d
+ if [ $d -eq 7 ]
+ then
+ losetup /dev/loop$d $targetdir/mdtest6 # for multipath use
+ else
+ losetup /dev/loop$d $targetdir/mdtest$d
+ fi
+ eval dev$d=/dev/loop$d
+ eval file$d=$targetdir/mdtest$d
+ ;;
+ lvm)
+ unset MULTIPATH
+ eval dev$d=/dev/mapper/${LVM_VOLGROUP}-mdtest$d
+ if ! lvcreate --quiet -L ${sz}K -n mdtest$d $LVM_VOLGROUP
+ then
+ trap '' 0 # make sure lvremove is not called
+ eval echo error creating \$dev$d
+ exit 129
+ fi
+ ;;
+ ram)
+ unset MULTIPATH
+ eval dev$d=/dev/ram$d
+ ;;
+ esac
+ eval devlist=\"\$devlist \$dev$d\"
+ eval devlist$d=\"\$devlist\"
+ #" <-- add this quote to un-confuse vim syntax highlighting
+ done
+ path0=$dev6
+ path1=$dev7
+ ulimit -c unlimited
+ [ -f /proc/mdstat ] || modprobe md_mod
+ echo 2000 > /proc/sys/dev/raid/speed_limit_max
+ echo 0 > /sys/module/md_mod/parameters/start_ro
}
# mdadm always adds --quiet, and we want to see any unexpected messages
mdadm() {
- rm -f $targetdir/stderr
- case $* in
- *-S* ) udevadm settle
- p=`cat /proc/sys/dev/raid/speed_limit_max`
- echo 20000 > /proc/sys/dev/raid/speed_limit_max
- esac
- case $* in
- *-C* ) $mdadm 2> $targetdir/stderr --quiet "$@" --auto=yes;;
- * ) $mdadm 2> $targetdir/stderr --quiet "$@"
- esac
- rv=$?
- case $* in
- *-S* ) udevadm settle
- echo $p > /proc/sys/dev/raid/speed_limit_max
- esac
- cat >&2 $targetdir/stderr
- return $rv
+ rm -f $targetdir/stderr
+ case $* in
+ *-S* )
+ udevadm settle
+ p=`cat /proc/sys/dev/raid/speed_limit_max`
+ echo 20000 > /proc/sys/dev/raid/speed_limit_max
+ ;;
+ esac
+ case $* in
+ *-C* )
+ $mdadm 2> $targetdir/stderr --quiet "$@" --auto=yes
+ ;;
+ * )
+ $mdadm 2> $targetdir/stderr --quiet "$@"
+ ;;
+ esac
+ rv=$?
+ case $* in
+ *-S* )
+ udevadm settle
+ echo $p > /proc/sys/dev/raid/speed_limit_max
+ ;;
+ esac
+ cat >&2 $targetdir/stderr
+ return $rv
}
# check various things
check() {
- case $1 in
- spares )
- spares=`tr '] ' '\012\012' < /proc/mdstat | grep -c '(S)' || exit 0`
- if [ $spares -ne $2 ]
- then
- echo >&2 "ERROR expected $2 spares, found $spares"; exit 1;
- fi
- ;;
- raid* | linear )
- grep -s "active $1 " /proc/mdstat > /dev/null || {
- echo >&2 "ERROR active $1 not found" ; cat /proc/mdstat ; exit 1;}
- ;;
- algorithm )
- grep -s " algorithm $2 " /proc/mdstat > /dev/null || {
- echo >&2 "ERROR algorithm $2 not found"; cat /proc/mdstat; exit 1;}
- ;;
- resync | recovery | reshape)
- cnt=5
- while ! grep -s $1 /proc/mdstat > /dev/null
- do
- if [ $cnt -gt 0 ] && grep -v idle /sys/block/md*/md/sync_action > /dev/null
- then # Something isn't idle - wait a bit
+ case $1 in
+ spares )
+ spares=`tr '] ' '\012\012' < /proc/mdstat | grep -c '(S)' || exit 0`
+ if [ $spares -ne $2 ]
+ then
+ echo >&2 "ERROR expected $2 spares, found $spares"
+ exit 1
+ fi
+ ;;
+ raid* | linear )
+ grep -s "active $1 " /proc/mdstat > /dev/null || {
+ echo >&2 "ERROR active $1 not found"
+ cat /proc/mdstat
+ exit 1
+ }
+ ;;
+ algorithm )
+ grep -s " algorithm $2 " /proc/mdstat > /dev/null || {
+ echo >&2 "ERROR algorithm $2 not found"
+ cat /proc/mdstat
+ exit 1
+ }
+ ;;
+ resync | recovery | reshape )
+ cnt=5
+ while ! grep -s $1 /proc/mdstat > /dev/null
+ do
+ if [ $cnt -gt 0 ] && grep -v idle /sys/block/md*/md/sync_action > /dev/null
+ then # Something isn't idle - wait a bit
+ sleep 0.5
+ cnt=$[cnt-1]
+ else
+ echo >&2 ERROR no $1 happening
+ cat /proc/mdstat
+ exit 1
+ fi
+ done
+ ;;
+ nosync )
sleep 0.5
- cnt=$[cnt-1]
- else
- echo >&2 ERROR no $1 happening; cat /proc/mdstat; exit 1
- fi
- done
- ;;
-
- nosync )
- sleep 0.5
- # Since 4.2 we delay the close of recovery until there has been a chance for
- # spares to be activated. That means that a recovery that finds nothing
- # to do can still take a little longer than expected.
- # add an extra check: is sync_completed shows the end is reached, assume
- # there is no recovery.
- if grep -s -E '(resync|recovery|reshape) *=' > /dev/null /proc/mdstat ; then
- incomplete=`grep / /sys/block/md*/md/sync_completed 2> /dev/null | sed '/^ *\([0-9]*\) \/ \1/d'`
- if [ -n "$incomplete" ]; then
- echo >&2 "ERROR resync or recovery is happening!"; cat /proc/mdstat ; exit 1;
- fi
- fi
- ;;
-
- wait )
- p=`cat /proc/sys/dev/raid/speed_limit_max`
- echo 2000000 > /proc/sys/dev/raid/speed_limit_max
- sleep 0.1
- while grep -E '(resync|recovery|reshape|check|repair) *=' > /dev/null /proc/mdstat ||
- grep -v idle > /dev/null /sys/block/md*/md/sync_action
- do sleep 0.5;
- done
- echo $p > /proc/sys/dev/raid/speed_limit_max
- ;;
-
- state )
- grep -s "blocks.*\[$2\]\$" /proc/mdstat > /dev/null || {
- echo >&2 "ERROR state $2 not found!"; cat /proc/mdstat ; exit 1; }
- sleep 0.5
- ;;
-
- bitmap )
- grep -s bitmap > /dev/null /proc/mdstat || {
- echo >&2 ERROR no bitmap ; cat /proc/mdstat ; exit 1; }
- ;;
- nobitmap )
- if grep -s "bitmap" > /dev/null /proc/mdstat
- then
- echo >&2 ERROR bitmap present ; cat /proc/mdstat ; exit 1;
- fi
- ;;
-
- readonly )
- grep -s "read-only" > /dev/null /proc/mdstat || {
- echo >&2 "ERROR array is not read-only!"; cat /proc/mdstat ; exit 1; }
- ;;
-
- inactive )
- grep -s "inactive" > /dev/null /proc/mdstat || {
- echo >&2 "ERROR array is not inactive!"; cat /proc/mdstat ; exit 1; }
- ;;
- * ) echo >&2 ERROR unknown check $1 ; exit 1;
- esac
+ # Since 4.2 we delay the close of recovery until there has been a chance for
+ # spares to be activated. That means that a recovery that finds nothing
+ # to do can still take a little longer than expected.
+ # add an extra check: is sync_completed shows the end is reached, assume
+ # there is no recovery.
+ if grep -s -E '(resync|recovery|reshape) *=' > /dev/null /proc/mdstat
+ then
+ incomplete=`grep / /sys/block/md*/md/sync_completed 2> /dev/null | sed '/^ *\([0-9]*\) \/ \1/d'`
+ if [ -n "$incomplete" ]
+ then
+ echo >&2 "ERROR resync or recovery is happening!"
+ cat /proc/mdstat
+ exit 1
+ fi
+ fi
+ ;;
+ wait )
+ p=`cat /proc/sys/dev/raid/speed_limit_max`
+ echo 2000000 > /proc/sys/dev/raid/speed_limit_max
+ sleep 0.1
+ while grep -E '(resync|recovery|reshape|check|repair) *=' > /dev/null /proc/mdstat ||
+ grep -v idle > /dev/null /sys/block/md*/md/sync_action
+ do
+ sleep 0.5
+ done
+ echo $p > /proc/sys/dev/raid/speed_limit_max
+ ;;
+ state )
+ grep -s "blocks.*\[$2\]\$" /proc/mdstat > /dev/null || {
+ echo >&2 "ERROR state $2 not found!"
+ cat /proc/mdstat
+ exit 1
+ }
+ sleep 0.5
+ ;;
+ bitmap )
+ grep -s bitmap > /dev/null /proc/mdstat || {
+ echo >&2 ERROR no bitmap
+ cat /proc/mdstat
+ exit 1
+ }
+ ;;
+ nobitmap )
+ if grep -s "bitmap" > /dev/null /proc/mdstat
+ then
+ echo >&2 ERROR bitmap present
+ cat /proc/mdstat
+ exit 1
+ fi
+ ;;
+ readonly )
+ grep -s "read-only" > /dev/null /proc/mdstat || {
+ echo >&2 "ERROR array is not read-only!"
+ cat /proc/mdstat
+ exit 1
+ }
+ ;;
+ inactive )
+ grep -s "inactive" > /dev/null /proc/mdstat || {
+ echo >&2 "ERROR array is not inactive!"
+ cat /proc/mdstat
+ exit 1
+ }
+ ;;
+ * )
+ echo >&2 ERROR unknown check $1
+ exit 1
+ ;;
+ esac
}
no_errors() {
- if [ -s $targetdir/stderr ]
- then echo Bad errors from mdadm: ; cat $targetdir/stderr; exit 2;
- fi
+ if [ -s $targetdir/stderr ]
+ then
+ echo Bad errors from mdadm:
+ cat $targetdir/stderr
+ exit 2
+ fi
}
-# basic device test
+# basic device test
testdev() {
- udevadm settle
- dev=$1
- cnt=$2
- dvsize=$3
- chunk=$4
- if [ -z "$5" ]; then
- mkfs.ext3 -F -j $dev > /dev/null 2>&1 && fsck -fn $dev >&2
- fi
- dsize=$[dvsize/chunk]
- dsize=$[dsize*chunk]
- rasize=$[dsize*2*cnt]
- # rasize is in sectors
- if [ -n "$DEV_ROUND_K" ]; then
- rasize=$[rasize/DEV_ROUND_K/2]
- rasize=$[rasize*DEV_ROUND_K*2]
- fi
- if [ `/sbin/blockdev --getsize $dev` -eq 0 ]; then sleep 2 ; fi
- _sz=`/sbin/blockdev --getsize $dev`
- if [ $rasize -lt $_sz -o $[rasize*4/5] -gt $_sz ]
- then
- echo "ERROR: size is wrong for $dev: $cnt * $dvsize (chunk=$chunk) = $rasize, not $_sz"
- exit 1
- fi
+ udevadm settle
+ dev=$1
+ cnt=$2
+ dvsize=$3
+ chunk=$4
+ if [ -z "$5" ]
+ then
+ mkfs.ext3 -F -j $dev > /dev/null 2>&1 && fsck -fn $dev >&2
+ fi
+ dsize=$[dvsize/chunk]
+ dsize=$[dsize*chunk]
+ rasize=$[dsize*2*cnt]
+ # rasize is in sectors
+ if [ -n "$DEV_ROUND_K" ]
+ then
+ rasize=$[rasize/DEV_ROUND_K/2]
+ rasize=$[rasize*DEV_ROUND_K*2]
+ fi
+ if [ `/sbin/blockdev --getsize $dev` -eq 0 ]
+ then
+ sleep 2
+ fi
+ _sz=`/sbin/blockdev --getsize $dev`
+ if [ $rasize -lt $_sz -o $[rasize*4/5] -gt $_sz ]
+ then
+ echo "ERROR: size is wrong for $dev: $cnt * $dvsize (chunk=$chunk) = $rasize, not $_sz"
+ exit 1
+ fi
}
fast_sync() {
- echo 200000 > /proc/sys/dev/raid/speed_limit_max
+ echo 200000 > /proc/sys/dev/raid/speed_limit_max
}
rotest() {
- dev=$1
- fsck -fn $dev >&2
+ dev=$1
+ fsck -fn $dev >&2
}
do_test() {
- _script=$1
- _basename=`basename $_script`
- if [ -f "$_script" ]
- then
- rm -f $targetdir/stderr
- # stop all arrays, just incase some script left an array active.
- $mdadm -Ssq 2> /dev/null
- mdadm --zero $devlist 2> /dev/null
- mdadm --zero $devlist 2> /dev/null
- # this might have been reset: restore the default.
- echo 2000 > /proc/sys/dev/raid/speed_limit_max
- # source script in a subshell, so it has access to our
- # namespace, but cannot change it.
- echo -ne "$_script... "
- if ( set -ex ; . $_script ) &> $targetdir/log
- then
- echo "succeeded"
- _fail=0
- else
- log=log
- cat $targetdir/stderr >> $targetdir/log
- echo "=======================dmesg=================" >> $targetdir/log
- dmesg | tail -n 200 >> $targetdir/log
- if [ $exitonerror == 0 ]; then
- log=log-`basename $_script`
- mv $targetdir/log $logdir/$log
- fi
- echo "FAILED - see $logdir/$log for details"
- _fail=1
- fi
- if [ "$savelogs" == "1" ]; then
- cp $targetdir/log $logdir/$_basename.log
- fi
- if [ "$_fail" == "1" -a "$exitonerror" == "1" ]; then
- exit 1
- fi
- fi
+ _script=$1
+ _basename=`basename $_script`
+ if [ -f "$_script" ]
+ then
+ rm -f $targetdir/stderr
+ # stop all arrays, just incase some script left an array active.
+ $mdadm -Ssq 2> /dev/null
+ mdadm --zero $devlist 2> /dev/null
+ mdadm --zero $devlist 2> /dev/null
+ # this might have been reset: restore the default.
+ echo 2000 > /proc/sys/dev/raid/speed_limit_max
+ # source script in a subshell, so it has access to our
+ # namespace, but cannot change it.
+ echo -ne "$_script... "
+ if ( set -ex ; . $_script ) &> $targetdir/log
+ then
+ echo "succeeded"
+ _fail=0
+ else
+ log=log
+ cat $targetdir/stderr >> $targetdir/log
+ echo "=======================dmesg=================" >> $targetdir/log
+ dmesg | tail -n 200 >> $targetdir/log
+ if [ $exitonerror == 0 ]; then
+ log=log-`basename $_script`
+ mv $targetdir/log $logdir/$log
+ fi
+ echo "FAILED - see $logdir/$log for details"
+ _fail=1
+ fi
+ if [ "$savelogs" == "1" ]
+ then
+ cp $targetdir/log $logdir/$_basename.log
+ fi
+ if [ "$_fail" == "1" -a "$exitonerror" == "1" ]
+ then
+ exit 1
+ fi
+ fi
}
do_help() {
- echo "Usage: $0 [options]"
- echo " Options:"
- echo " --tests=<test1,test2,..> Comma separated list of tests to run"
- echo " --disable-multipath Disable any tests involving multipath"
- echo " --disable-integrity Disable slow tests of RAID[56] consistency"
- echo " --logdir=<directory> Directory to save logfiles in"
- echo " --save-logs Save all logs in <logdir>"
- echo " --keep-going Don't stop on error, ie. run all tests"
- echo " --dev=[loop|lvm|ram] Use loop devices (default), LVM, or RAM disk"
- echo " --volgroup=<name> LVM volume group for LVM test"
- echo " setup Setup test environment and exit"
- echo " cleanup Cleanup test environment"
- echo " <prefix> Run tests with <prefix>"
+ echo "Usage: $0 [options]"
+ echo " Options:"
+ echo " --tests=<test1,test2,..> Comma separated list of tests to run"
+ echo " --disable-multipath Disable any tests involving multipath"
+ echo " --disable-integrity Disable slow tests of RAID[56] consistency"
+ echo " --logdir=<directory> Directory to save logfiles in"
+ echo " --save-logs Save all logs in <logdir>"
+ echo " --keep-going Don't stop on error, ie. run all tests"
+ echo " --dev=[loop|lvm|ram] Use loop devices (default), LVM, or RAM disk"
+ echo " --volgroup=<name> LVM volume group for LVM test"
+ echo " setup Setup test environment and exit"
+ echo " cleanup Cleanup test environment"
+ echo " <prefix> Run tests with <prefix>"
}
parse_args() {
- for i in $*
- do
- case $i in
- [0-9]*)
- prefix=$i
- ;;
- setup)
- echo "mdadm test environment setup"
- do_setup
- trap 0; exit 0
- ;;
- cleanup)
- cleanup
- exit 0
- ;;
- --tests=*)
- TESTLIST=`expr "x$i" : 'x[^=]*=\(.*\)' | sed -e 's/,/ /g'`
- ;;
- --logdir=*)
- logdir=`expr "x$i" : 'x[^=]*=\(.*\)'`
- ;;
- --save-logs)
- savelogs=1
- ;;
- --keep-going | --no-error)
- exitonerror=0
- ;;
- --disable-multipath)
- unset MULTIPATH
- ;;
- --disable-integrity)
- unset INTEGRITY
- ;;
- --dev=loop)
- DEVTYPE=loop
- ;;
- --dev=lvm)
- DEVTYPE=lvm
- ;;
- --dev=ram)
- DEVTYPE=ram
- ;;
- --volgroup=*)
- LVM_VOLGROUP=`expr "x$i" : 'x[^=]*=\(.*\)'`
- ;;
- --help)
- do_help
- exit 0;
- ;;
- -*)
- echo " $0: Unknown argument: $i"
- do_help
- exit 0;
- ;;
- esac
-done
+ for i in $*
+ do
+ case $i in
+ [0-9]* )
+ prefix=$i
+ ;;
+ setup )
+ echo "mdadm test environment setup"
+ do_setup
+ trap 0
+ exit 0
+ ;;
+ cleanup )
+ cleanup
+ exit 0
+ ;;
+ --tests=* )
+ TESTLIST=`expr "x$i" : 'x[^=]*=\(.*\)' | sed -e 's/,/ /g'`
+ ;;
+ --logdir=* )
+ logdir=`expr "x$i" : 'x[^=]*=\(.*\)'`
+ ;;
+ --save-logs )
+ savelogs=1
+ ;;
+ --keep-going | --no-error )
+ exitonerror=0
+ ;;
+ --disable-multipath )
+ unset MULTIPATH
+ ;;
+ --disable-integrity )
+ unset INTEGRITY
+ ;;
+ --dev=loop )
+ DEVTYPE=loop
+ ;;
+ --dev=lvm )
+ DEVTYPE=lvm
+ ;;
+ --dev=ram )
+ DEVTYPE=ram
+ ;;
+ --volgroup=* )
+ LVM_VOLGROUP=`expr "x$i" : 'x[^=]*=\(.*\)'`
+ ;;
+ --help )
+ do_help
+ exit 0
+ ;;
+ -* )
+ echo " $0: Unknown argument: $i"
+ do_help
+ exit 0
+ ;;
+ esac
+ done
}
logdir=$targetdir
@@ -422,19 +474,21 @@ parse_args $@
do_setup
mkdir -p $logdir
-if [ "$savelogs" == "1" ]; then
- echo "Saving logs to $logdir"
+if [ "$savelogs" == "1" ]
+then
+ echo "Saving logs to $logdir"
fi
-if [ "x$TESTLIST" != "x" ]; then
- for script in $TESTLIST
- do
- do_test $testdir/$script
- done
+if [ "x$TESTLIST" != "x" ]
+then
+ for script in $TESTLIST
+ do
+ do_test $testdir/$script
+ done
else
- for script in $testdir/$prefix $testdir/$prefix*[^~]
- do
- do_test $script
- done
+ for script in $testdir/$prefix $testdir/$prefix*[^~]
+ do
+ do_test $script
+ done
fi
exit 0
--
2.6.6
^ permalink raw reply related
* [PATCH 2/3] mdadm/test: Refactor the codes of 'test' script
From: Zhilong Liu @ 2017-06-07 9:31 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
In-Reply-To: <1496827888-17118-1-git-send-email-zlliu@suse.com>
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()
just tidy up.
Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
test | 339 +++++++++++++++++++++++++++++++++++++------------------------------
1 file changed, 189 insertions(+), 150 deletions(-)
diff --git a/test b/test
index 7942d6e..8ecfe99 100755
--- a/test
+++ b/test
@@ -1,35 +1,17 @@
#!/bin/bash
#
# run test suite for mdadm
-user=`id -un`
-if [ " $user" != " root" ]
-then
- echo >&2 "test: testing can only be done as 'root'."
- exit 1
-fi
-
-prefix='[0-9][0-9]'
-
-dir=`pwd`
+dir=$(pwd)
mdadm=$dir/mdadm
-if [ \! -x $mdadm ]
-then
- echo >&2 "test: $mdadm isn't usable."
-fi
-
testdir="tests"
-logdir="$testdir/logs"
-logsave=0
-exitonerror=1
+targetdir="/var/tmp"
+logdir="$targetdir"
+config=/tmp/mdadm.conf
-echo "Testing on linux-$(uname -r) kernel"
+savelogs=0
+exitonerror=1
+prefix='[0-9][0-9]'
-# Check whether to run multipath tests
-modprobe multipath 2> /dev/null
-if grep -s 'Personalities : .*multipath' > /dev/null /proc/mdstat
-then
- MULTIPATH="yes"
-fi
INTEGRITY=yes
DEVTYPE=loop
LVM_VOLGROUP=mdtest
@@ -46,7 +28,6 @@ mdp1=/dev/md_d1
# We test mdadm on loop-back block devices.
# dir for storing files should be settable by command line maybe
-targetdir=/var/tmp
size=20000
# super0, round down to multiple of 64 and substract 64
mdsize0=19904
@@ -68,7 +49,47 @@ mdsize12=19988
# ddf needs bigger devices as 32Meg is reserved!
ddfsize=65536
-config=/tmp/mdadm.conf
+# $1 is optional parameter, it shows why to save log
+save_log() {
+ status=$1
+ logfile="$status""$_basename".log
+
+ echo "## $HOSTNAME: saving dmesg." >> $logdir/$logfile
+ dmesg -c >> $logdir/$logfile
+ $mdadm -As 2> /dev/null
+ echo "## $HOSTNAME: saving proc mdstat." >> $logdir/$logfile
+ cat /proc/mdstat >> $logdir/$logfile
+ array=($(mdadm -Ds | cut -d' ' -f2))
+ echo "## $HOSTNAME: mdadm -D ${array[@]}" >> $logdir/$logfile
+ $mdadm -D ${array[@]} >> $logdir/$logfile
+ # ignore saving external(external file, imsm...) bitmap
+ cat /proc/mdstat | grep -q "linear\|external" && return 0
+ if [ $DEVTYPE == 'lvm' ]
+ then
+ # not supported lvm type yet
+ echo
+ elif [ $DEVTYPE == 'loop' ]
+ then
+ if [ ! -z ${array[@]} -a ${#array[@]} -ge 1 ]
+ then
+ md_disks=($($mdadm -D -Y ${array[@]} | grep "/dev/$DEVTYPE" | cut -d'=' -f2))
+ cat /proc/mdstat | grep -q "bitmap"
+ if [ $? -eq 0 ]
+ then
+ echo "## $HOSTNAME: mdadm -X ${md_disks[@]}" >> $logdir/$logfile
+ $mdadm -X ${md_disks[@]} >> $logdir/$logfile
+ fi
+ else
+ echo "## $HOSTNAME: no array assembled!" >> $logdir/$logfile
+ fi
+ fi
+}
+
+die() {
+ echo -e "\n\tERROR: $* \n"
+ save_log fail
+ exit 2
+}
cleanup() {
udevadm settle
@@ -77,8 +98,9 @@ cleanup() {
loop)
for d in 0 1 2 3 4 5 6 7 8 9 10 11 12 13
do
- losetup -d /dev/loop$d # rm -f $targetdir/mdtest$d
+ losetup -d /dev/loop$d
rm -f /dev/disk/by-path/loop*
+ rm -f /var/tmp/mdtest$d
done
;;
lvm)
@@ -88,6 +110,7 @@ cleanup() {
done
;;
esac
+ dmesg -c > /dev/null
}
ctrl_c() {
@@ -98,23 +121,21 @@ do_setup() {
trap cleanup 0 1 3 15
trap ctrl_c 2
+ [ -d $logdir ] || mkdir -p $logdir
# make sure there are no loop devices remaining.
# udev started things can sometimes prevent them being stopped
# immediately
while grep loop /proc/partitions > /dev/null 2>&1
do
- mdadm -Ss
+ $mdadm -Ssq
losetup -d /dev/loop[0-9]* 2> /dev/null
- sleep 1
+ sleep 0.2
done
devlist=
for d in 0 1 2 3 4 5 6 7 8 9 10 11 12 13
do
sz=$size
- if [ $d -gt 7 ]
- then
- sz=$ddfsize
- fi
+ [ $d -gt 7 ] && sz=$ddfsize
case $DEVTYPE in
loop)
[ -f $targetdir/mdtest$d ] ||
@@ -169,7 +190,17 @@ mdadm() {
;;
esac
case $* in
- *-C* )
+ *-C* | *--create* | *-B* | *--build* )
+ # clear superblock every time once creating or
+ # building arrays, because it's always creating
+ # and building array many times in a test case.
+ for args in $*
+ do
+ [[ $args =~ "/dev/" ]] && {
+ [[ $args =~ "md" ]] ||
+ $mdadm --zero $args > /dev/null
+ }
+ done
$mdadm 2> $targetdir/stderr --quiet "$@" --auto=yes
;;
* )
@@ -191,39 +222,28 @@ mdadm() {
check() {
case $1 in
spares )
- spares=`tr '] ' '\012\012' < /proc/mdstat | grep -c '(S)' || exit 0`
- if [ $spares -ne $2 ]
- then
- echo >&2 "ERROR expected $2 spares, found $spares"
- exit 1
- fi
+ spares=$(tr '] ' '\012\012' < /proc/mdstat | grep -c '(S)' || exit 0)
+ [ $spares -ne $2 ] &&
+ die "expected $2 spares, found $spares"
;;
raid* | linear )
- grep -s "active $1 " /proc/mdstat > /dev/null || {
- echo >&2 "ERROR active $1 not found"
- cat /proc/mdstat
- exit 1
- }
+ grep -sq "active $1 " /proc/mdstat ||
+ die "active $1 not found"
;;
algorithm )
- grep -s " algorithm $2 " /proc/mdstat > /dev/null || {
- echo >&2 "ERROR algorithm $2 not found"
- cat /proc/mdstat
- exit 1
- }
+ grep -sq " algorithm $2 " /proc/mdstat ||
+ die "algorithm $2 not found"
;;
resync | recovery | reshape )
cnt=5
- while ! grep -s $1 /proc/mdstat > /dev/null
+ while ! grep -sq $1 /proc/mdstat
do
if [ $cnt -gt 0 ] && grep -v idle /sys/block/md*/md/sync_action > /dev/null
then # Something isn't idle - wait a bit
sleep 0.5
cnt=$[cnt-1]
else
- echo >&2 ERROR no $1 happening
- cat /proc/mdstat
- exit 1
+ die "no $1 happening"
fi
done
;;
@@ -234,22 +254,18 @@ check() {
# to do can still take a little longer than expected.
# add an extra check: is sync_completed shows the end is reached, assume
# there is no recovery.
- if grep -s -E '(resync|recovery|reshape) *=' > /dev/null /proc/mdstat
+ if grep -sq -E '(resync|recovery|reshape) *=' /proc/mdstat
then
incomplete=`grep / /sys/block/md*/md/sync_completed 2> /dev/null | sed '/^ *\([0-9]*\) \/ \1/d'`
- if [ -n "$incomplete" ]
- then
- echo >&2 "ERROR resync or recovery is happening!"
- cat /proc/mdstat
- exit 1
- fi
+ [ -n "$incomplete" ] &&
+ die "resync or recovery is happening!"
fi
;;
wait )
p=`cat /proc/sys/dev/raid/speed_limit_max`
echo 2000000 > /proc/sys/dev/raid/speed_limit_max
sleep 0.1
- while grep -E '(resync|recovery|reshape|check|repair) *=' > /dev/null /proc/mdstat ||
+ while grep -Eq '(resync|recovery|reshape|check|repair) *=' /proc/mdstat ||
grep -v idle > /dev/null /sys/block/md*/md/sync_action
do
sleep 0.5
@@ -257,45 +273,28 @@ check() {
echo $p > /proc/sys/dev/raid/speed_limit_max
;;
state )
- grep -s "blocks.*\[$2\]\$" /proc/mdstat > /dev/null || {
- echo >&2 "ERROR state $2 not found!"
- cat /proc/mdstat
- exit 1
- }
+ grep -sq "blocks.*\[$2\]\$" /proc/mdstat ||
+ die "state $2 not found!"
sleep 0.5
;;
bitmap )
- grep -s bitmap > /dev/null /proc/mdstat || {
- echo >&2 ERROR no bitmap
- cat /proc/mdstat
- exit 1
- }
+ grep -sq bitmap /proc/mdstat ||
+ die "no bitmap"
;;
nobitmap )
- if grep -s "bitmap" > /dev/null /proc/mdstat
- then
- echo >&2 ERROR bitmap present
- cat /proc/mdstat
- exit 1
- fi
+ grep -sq "bitmap" /proc/mdstat &&
+ die "bitmap present"
;;
readonly )
- grep -s "read-only" > /dev/null /proc/mdstat || {
- echo >&2 "ERROR array is not read-only!"
- cat /proc/mdstat
- exit 1
- }
+ grep -sq "read-only" /proc/mdstat ||
+ die "array is not read-only!"
;;
inactive )
- grep -s "inactive" > /dev/null /proc/mdstat || {
- echo >&2 "ERROR array is not inactive!"
- cat /proc/mdstat
- exit 1
- }
+ grep -sq "inactive" /proc/mdstat ||
+ die "array is not inactive!"
;;
* )
- echo >&2 ERROR unknown check $1
- exit 1
+ die "unknown check $1"
;;
esac
}
@@ -311,6 +310,8 @@ no_errors() {
# basic device test
testdev() {
+ # $1 must be a block device
+ [ -b $1 ] || die "$1 isn't a block device."
udevadm settle
dev=$1
cnt=$2
@@ -329,16 +330,11 @@ testdev() {
rasize=$[rasize/DEV_ROUND_K/2]
rasize=$[rasize*DEV_ROUND_K*2]
fi
- if [ `/sbin/blockdev --getsize $dev` -eq 0 ]
- then
- sleep 2
- fi
+ [ `/sbin/blockdev --getsize $dev` -eq 0 ] && sleep 2
_sz=`/sbin/blockdev --getsize $dev`
- if [ $rasize -lt $_sz -o $[rasize*4/5] -gt $_sz ]
- then
- echo "ERROR: size is wrong for $dev: $cnt * $dvsize (chunk=$chunk) = $rasize, not $_sz"
- exit 1
- fi
+ [ $rasize -lt $_sz -o $[rasize*4/5] -gt $_sz ] &&
+ die "size is wrong for $dev: $cnt * $dvsize (chunk=$chunk) = $rasize, not $_sz"
+ return 0
}
fast_sync() {
@@ -359,7 +355,6 @@ do_test() {
# stop all arrays, just incase some script left an array active.
$mdadm -Ssq 2> /dev/null
mdadm --zero $devlist 2> /dev/null
- mdadm --zero $devlist 2> /dev/null
# this might have been reset: restore the default.
echo 2000 > /proc/sys/dev/raid/speed_limit_max
# source script in a subshell, so it has access to our
@@ -367,45 +362,43 @@ do_test() {
echo -ne "$_script... "
if ( set -ex ; . $_script ) &> $targetdir/log
then
+ dmesg | grep -iq "error\|call trace\|segfault" &&
+ die "dmesg prints errors when testing $_basename"
echo "succeeded"
_fail=0
else
- log=log
cat $targetdir/stderr >> $targetdir/log
- echo "=======================dmesg=================" >> $targetdir/log
- dmesg | tail -n 200 >> $targetdir/log
- if [ $exitonerror == 0 ]; then
- log=log-`basename $_script`
- mv $targetdir/log $logdir/$log
- fi
- echo "FAILED - see $logdir/$log for details"
+ save_log fail
+ cp $targetdir/log $logdir/$_basename.log
+ echo "FAILED - see $logdir/$_basename.log and $logdir/$logfile for details"
_fail=1
fi
if [ "$savelogs" == "1" ]
then
- cp $targetdir/log $logdir/$_basename.log
- fi
- if [ "$_fail" == "1" -a "$exitonerror" == "1" ]
- then
- exit 1
+ [ "$targetdir" != "$logdir" ] &&
+ mv $targetdir/log $logdir/$_basename.log
fi
+ [ "$_fail" == "1" -a "$exitonerror" == "1" ] && exit 1
fi
}
do_help() {
- echo "Usage: $0 [options]"
- echo " Options:"
- echo " --tests=<test1,test2,..> Comma separated list of tests to run"
- echo " --disable-multipath Disable any tests involving multipath"
- echo " --disable-integrity Disable slow tests of RAID[56] consistency"
- echo " --logdir=<directory> Directory to save logfiles in"
- echo " --save-logs Save all logs in <logdir>"
- echo " --keep-going Don't stop on error, ie. run all tests"
- echo " --dev=[loop|lvm|ram] Use loop devices (default), LVM, or RAM disk"
- echo " --volgroup=<name> LVM volume group for LVM test"
- echo " setup Setup test environment and exit"
- echo " cleanup Cleanup test environment"
- echo " <prefix> Run tests with <prefix>"
+ cat <<-EOF
+ Usage: $0 [options]
+ Options:
+ --tests=<test1,test2,..> Comma separated list of tests to run
+ --disable-multipath Disable any tests involving multipath
+ --disable-integrity Disable slow tests of RAID[56] consistency
+ --logdir=<directory> Directory to save all logfiles in
+ --save-logs Usually use with --logdir together
+ --keep-going Don't stop on error, ie. run all tests
+ --dev=[loop|lvm|ram] Use loop devices (default), LVM, or RAM disk
+ --volgroup=<name> LVM volume group for LVM test
+ setup Setup test environment and exit
+ cleanup Cleanup test environment
+ <prefix> Run tests with <prefix>
+ EOF
+ exit 0
}
parse_args() {
@@ -457,38 +450,84 @@ parse_args() {
;;
--help )
do_help
- exit 0
;;
-* )
echo " $0: Unknown argument: $i"
do_help
- exit 0
;;
esac
done
}
-logdir=$targetdir
-parse_args $@
+check_env() {
+ user=$(id -un)
+ [ "X$user" != "Xroot" ] && {
+ echo "test: testing can only be done as 'root'."
+ exit 1
+ }
+ [ -x "raid6check" -a -x $mdadm ] || {
+ echo "test: please run 'make everything' before perform testing."
+ exit 1
+ }
+ for cmd in mdadm lsblk df udevadm losetup mkfs.ext3
+ do
+ which $cmd > /dev/null || {
+ echo "$cmd command not found!"
+ exit 1
+ }
+ done
+ mdadm_src_ver="$($mdadm -V 2>&1)"
+ mdadm_sbin_ver="$($(which mdadm) -V 2>&1)"
+ if [ "$mdadm_src_ver" != "$mdadm_sbin_ver" ]
+ then
+ # it's nessesary to 'make install' mdadm to /SBIN/DIR,
+ # such as systemd/mdadm-grow-continue@.service, would
+ # run as an instance by systemd when reshape happens,
+ # thus ensure that the correct mdadm is in testing.
+ echo "test: please run 'make install' before testing."
+ exit 1
+ fi
+ if ! $(df -T . | grep -iq ext)
+ then
+ # 'external file' bitmap only supports with ext[2-4] file system
+ echo "test: please run test suite with ext[2-4] file system."
+ exit 1
+ fi
+ if $(lsblk -a | grep -iq raid)
+ then
+ # For test purpose, it's better to make environment simple,
+ # donot run mdadm -Ss directly if there are RAIDs working.
+ echo "test: please run test suite without running RAIDs environment."
+ exit 1
+ fi
+ # Check whether to run multipath tests
+ modprobe multipath 2> /dev/null
+ grep -sq 'Personalities : .*multipath' /proc/mdstat &&
+ MULTIPATH="yes"
+}
-do_setup
-mkdir -p $logdir
+main() {
+ check_env
+ do_setup
+
+ echo "Testing on linux-$(uname -r) kernel"
+ [ "$savelogs" == "1" ] &&
+ echo "Saving logs to $logdir"
+ if [ "x$TESTLIST" != "x" ]
+ then
+ for script in $TESTLIST
+ do
+ do_test $testdir/$script
+ done
+ else
+ for script in $testdir/$prefix $testdir/$prefix*[^~]
+ do
+ do_test $script
+ done
+ fi
-if [ "$savelogs" == "1" ]
-then
- echo "Saving logs to $logdir"
-fi
+ exit 0
+}
-if [ "x$TESTLIST" != "x" ]
-then
- for script in $TESTLIST
- do
- do_test $testdir/$script
- done
-else
- for script in $testdir/$prefix $testdir/$prefix*[^~]
- do
- do_test $script
- done
-fi
-exit 0
+parse_args $@
+main
--
2.6.6
^ permalink raw reply related
* [PATCH 3/3] mdadm/test: Get rid of fast_sync()
From: Zhilong Liu @ 2017-06-07 9:31 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
In-Reply-To: <1496827888-17118-1-git-send-email-zlliu@suse.com>
fast_sync() is no longer used, so remove it.
Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
test | 4 ----
1 file changed, 4 deletions(-)
diff --git a/test b/test
index 8ecfe99..3cb4bc0 100755
--- a/test
+++ b/test
@@ -337,10 +337,6 @@ testdev() {
return 0
}
-fast_sync() {
- echo 200000 > /proc/sys/dev/raid/speed_limit_max
-}
-
rotest() {
dev=$1
fsck -fn $dev >&2
--
2.6.6
^ permalink raw reply related
* Re: [PATCH] mdadm: Send version text to stdout rather than stderr
From: Jes Sorensen @ 2017-06-07 15:21 UTC (permalink / raw)
To: NeilBrown, Zhilong Liu; +Cc: linux-raid
In-Reply-To: <877f0od124.fsf@notabene.neil.brown.name>
On 06/06/2017 09:44 PM, NeilBrown wrote:
> On Tue, Jun 06 2017, Zhilong Liu wrote:
>
>> This helps with piping to a paper.
>
> It might help you, but it is an API change and could cause problems for
> others.
>
> Some programs, such as "mount -V", send version info to stdout.
> Others, like "gcc -v", send it to stderr.
> There is no uniform standard, it is best to leave the behavior
> unchanged.
>
> If you need to capture the output use
> mdadm -V |& ...
> or
> mdadm -V 2>&1
>
> or similar.
>
> NeilBrtown
I agree with mr Brtown here :)
Changing the API is a big issue. We need to avoid this unless it is
absolutely necessary, so we don't break things for people trying to run
newer mdadm on older distros or those relying on third party scripts.
Jes
^ permalink raw reply
* [PATCH] md: don't use flush_signals in userspace processes
From: Mikulas Patocka @ 2017-06-07 23:05 UTC (permalink / raw)
To: Shaohua Li, NeilBrown
Cc: linux-raid, linux-kernel, Ingo Molnar, Peter Zijlstra
The function flush_signals clears all pending signals for the process. It
may be used by kernel threads when we need to prepare a kernel thread for
responding to signals. However using this function for an userspaces
processes is incorrect - clearing signals without the program expecting it
can cause misbehavior.
The raid1 and raid5 code uses flush_signals in its request routine because
it wants to prepare for an interruptible wait. This patch drops
flush_signals and uses sigprocmask instead to block all signals (including
SIGKILL) around the schedule() call. The signals are not lost, but the
schedule() call won't respond to them.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
---
drivers/md/raid1.c | 5 ++++-
drivers/md/raid5.c | 5 ++++-
2 files changed, 8 insertions(+), 2 deletions(-)
Index: linux-4.12-rc4/drivers/md/raid1.c
===================================================================
--- linux-4.12-rc4.orig/drivers/md/raid1.c
+++ linux-4.12-rc4/drivers/md/raid1.c
@@ -1335,7 +1335,7 @@ static void raid1_write_request(struct m
*/
DEFINE_WAIT(w);
for (;;) {
- flush_signals(current);
+ sigset_t full, old;
prepare_to_wait(&conf->wait_barrier,
&w, TASK_INTERRUPTIBLE);
if (bio_end_sector(bio) <= mddev->suspend_lo ||
@@ -1345,7 +1345,10 @@ static void raid1_write_request(struct m
bio->bi_iter.bi_sector,
bio_end_sector(bio))))
break;
+ sigfillset(&full);
+ sigprocmask(SIG_BLOCK, &full, &old);
schedule();
+ sigprocmask(SIG_SETMASK, &old, NULL);
}
finish_wait(&conf->wait_barrier, &w);
}
Index: linux-4.12-rc4/drivers/md/raid5.c
===================================================================
--- linux-4.12-rc4.orig/drivers/md/raid5.c
+++ linux-4.12-rc4/drivers/md/raid5.c
@@ -5693,12 +5693,15 @@ static void raid5_make_request(struct md
* userspace, we want an interruptible
* wait.
*/
- flush_signals(current);
prepare_to_wait(&conf->wait_for_overlap,
&w, TASK_INTERRUPTIBLE);
if (logical_sector >= mddev->suspend_lo &&
logical_sector < mddev->suspend_hi) {
+ sigset_t full, old;
+ sigfillset(&full);
+ sigprocmask(SIG_BLOCK, &full, &old);
schedule();
+ sigprocmask(SIG_SETMASK, &old, NULL);
do_prepare = true;
}
goto retry;
^ permalink raw reply
* Re: [PATCH] md: don't use flush_signals in userspace processes
From: NeilBrown @ 2017-06-08 6:59 UTC (permalink / raw)
To: Mikulas Patocka, Shaohua Li
Cc: linux-raid, linux-kernel, Ingo Molnar, Peter Zijlstra
In-Reply-To: <alpine.LRH.2.02.1706071834250.22850@file01.intranet.prod.int.rdu2.redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2605 bytes --]
On Wed, Jun 07 2017, Mikulas Patocka wrote:
> The function flush_signals clears all pending signals for the process. It
> may be used by kernel threads when we need to prepare a kernel thread for
> responding to signals. However using this function for an userspaces
> processes is incorrect - clearing signals without the program expecting it
> can cause misbehavior.
>
> The raid1 and raid5 code uses flush_signals in its request routine because
> it wants to prepare for an interruptible wait. This patch drops
> flush_signals and uses sigprocmask instead to block all signals (including
> SIGKILL) around the schedule() call. The signals are not lost, but the
> schedule() call won't respond to them.
>
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Cc: stable@vger.kernel.org
Thanks for catching that!
Acked-by: NeilBrown <neilb@suse.com>
NeilBrown
>
> ---
> drivers/md/raid1.c | 5 ++++-
> drivers/md/raid5.c | 5 ++++-
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> Index: linux-4.12-rc4/drivers/md/raid1.c
> ===================================================================
> --- linux-4.12-rc4.orig/drivers/md/raid1.c
> +++ linux-4.12-rc4/drivers/md/raid1.c
> @@ -1335,7 +1335,7 @@ static void raid1_write_request(struct m
> */
> DEFINE_WAIT(w);
> for (;;) {
> - flush_signals(current);
> + sigset_t full, old;
> prepare_to_wait(&conf->wait_barrier,
> &w, TASK_INTERRUPTIBLE);
> if (bio_end_sector(bio) <= mddev->suspend_lo ||
> @@ -1345,7 +1345,10 @@ static void raid1_write_request(struct m
> bio->bi_iter.bi_sector,
> bio_end_sector(bio))))
> break;
> + sigfillset(&full);
> + sigprocmask(SIG_BLOCK, &full, &old);
> schedule();
> + sigprocmask(SIG_SETMASK, &old, NULL);
> }
> finish_wait(&conf->wait_barrier, &w);
> }
> Index: linux-4.12-rc4/drivers/md/raid5.c
> ===================================================================
> --- linux-4.12-rc4.orig/drivers/md/raid5.c
> +++ linux-4.12-rc4/drivers/md/raid5.c
> @@ -5693,12 +5693,15 @@ static void raid5_make_request(struct md
> * userspace, we want an interruptible
> * wait.
> */
> - flush_signals(current);
> prepare_to_wait(&conf->wait_for_overlap,
> &w, TASK_INTERRUPTIBLE);
> if (logical_sector >= mddev->suspend_lo &&
> logical_sector < mddev->suspend_hi) {
> + sigset_t full, old;
> + sigfillset(&full);
> + sigprocmask(SIG_BLOCK, &full, &old);
> schedule();
> + sigprocmask(SIG_SETMASK, &old, NULL);
> do_prepare = true;
> }
> goto retry;
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ 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