* (unknown),
From: Vikas Aggarwal @ 2016-06-05 12:28 UTC (permalink / raw)
To: linux-raid
Hello All,
Will appreciate if someone can clear my doubts regarding XOR/GF
offload for raid5/6.
1) What is the purpose of device_prep_dma_interrupt callback ?
2) My driver currently polls for checking the xor completions
and does'nt implement device_prep_dma_interrupt
callback at all. What performance variation I can expect by
implementing this callback
in my async_tx driver.
3) Purpose of DMA_ACK as I read - it is for higher layers to inform
dma driver that descriptors can now be freed. Can someone explain this
with an example as applicable with raid5/6 clients.
4) With example - why dma_run_dependencies(tx) needed after the
hardware engine posts completion for a descriptor.
5) Purpose of tx->callback(cb_arg) - again with an example from a
raid5/6 offload perspective.
Goal: I want to use offload engine efficiently with recent
multithreaded raid5/6.
I tried to dig through code and linux/Documentation etc but not
thoroughly clear of functionality.
Thanks & Regards
Vikas Aggarwal
^ permalink raw reply
* Re: raid 5 crashed
From: bobzer @ 2016-06-04 2:56 UTC (permalink / raw)
To: Sarah Newman; +Cc: linux-raid
In-Reply-To: <5751B0E9.8040008@prgmr.com>
> The --name is for if you want to call the device something other than just md0. If you use /dev/md0 instead of --name=/dev/md0 it might work.
thanks in fact it was just that ...
so i did :
mdadm --assemble --force /dev/md0 /dev/sdb1 /dev/sdc1 /dev/loop1 /dev/sdd1
and now i'm just hoping that it will work... every finger crossed :-)
^ permalink raw reply
* Re: [PATCH 1/2] md-cluster: fix deadlock issue when add disk to an recoverying array
From: Shaohua Li @ 2016-06-03 23:21 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: linux-raid
In-Reply-To: <1464924725-18096-1-git-send-email-gqjiang@suse.com>
On Thu, Jun 02, 2016 at 11:32:04PM -0400, Guoqing Jiang wrote:
> Add a disk to an array which is performing recovery
> is a little complicated, we need to do both reap the
> sync thread and perform add disk for the case, then
> it caused deadlock as follows.
>
> linux44:~ # ps aux|grep md|grep D
> root 1822 0.0 0.0 0 0 ? D 16:50 0:00 [md127_resync]
> root 1848 0.0 0.0 19860 952 pts/0 D+ 16:50 0:00 mdadm --manage /dev/md127 --re-add /dev/vdb
> linux44:~ # cat /proc/1848/stack
> [<ffffffff8107afde>] kthread_stop+0x6e/0x120
> [<ffffffffa051ddb0>] md_unregister_thread+0x40/0x80 [md_mod]
> [<ffffffffa0526e45>] md_reap_sync_thread+0x15/0x150 [md_mod]
> [<ffffffffa05271e0>] action_store+0x260/0x270 [md_mod]
> [<ffffffffa05206b4>] md_attr_store+0xb4/0x100 [md_mod]
> [<ffffffff81214a7e>] sysfs_write_file+0xbe/0x140
> [<ffffffff811a6b98>] vfs_write+0xb8/0x1e0
> [<ffffffff811a75b8>] SyS_write+0x48/0xa0
> [<ffffffff8152a5c9>] system_call_fastpath+0x16/0x1b
> [<00007f068ea1ed30>] 0x7f068ea1ed30
> linux44:~ # cat /proc/1822/stack
> [<ffffffffa05251a6>] md_do_sync+0x846/0xf40 [md_mod]
> [<ffffffffa052402d>] md_thread+0x16d/0x180 [md_mod]
> [<ffffffff8107ad94>] kthread+0xb4/0xc0
> [<ffffffff8152a518>] ret_from_fork+0x58/0x90
>
> Task1848 Task1822
> md_attr_store (held reconfig_mutex by call mddev_lock())
> action_store
> md_reap_sync_thread
> md_unregister_thread
> kthread_stop md_wakeup_thread(mddev->thread);
> wait_event(mddev->sb_wait, !test_bit(MD_CHANGE_PENDING))
>
> md_check_recovery is triggered by wakeup mddev->thread,
> but it can't clear MD_CHANGE_PENDING flag since it can't
> get lock which was held by md_attr_store already.
>
> To solve the deadlock problem, we move "->resync_finish()"
> from md_do_sync to md_reap_sync_thread (after md_update_sb),
> also MD_HELD_RESYNC_LOCK is introduced since it is possible
> that node can't get resync lock in md_do_sync.
>
> Then we do not need to wait for MD_CHANGE_PENDING is cleared
> or not since metadata should be updated after md_update_sb,
> so just call resync_finish if MD_HELD_RESYNC_LOCK is set.
>
> We also unified the code after skip label, since set PENDING
> for non-clustered case should be harmless.
applied the two, thanks!
^ permalink raw reply
* Re: [md PATCH 09/18] md/raid10: stop print_conf from being too verbose.
From: Shaohua Li @ 2016-06-03 22:39 UTC (permalink / raw)
To: NeilBrown; +Cc: John Stoffel, linux-raid
In-Reply-To: <87d1nzp5q6.fsf@notabene.neil.brown.name>
On Fri, Jun 03, 2016 at 08:48:33AM +1000, Neil Brown wrote:
> On Fri, Jun 03 2016, John Stoffel wrote:
>
> > NeilBrown> raid10 arrays can usefully be very large. When they are,
> > NeilBrown> the noise generated by print_conf can over-whelm logs. So
> > NeilBrown> truncate the listing at 16 devices.
> >
> > Why is this too noisy and how often does this print_conf() happen?
> > And why 16 devices? I guess I don't like the magic number of 16 here,
> > I'd prefer it to be a define, and possibly even something that can by
> > dynamically changed.
>
> print_conf happens whenever a device becomes active in the array, or a
> device is removed from the array (usually because it has failed).
>
> I got 16 by choosing a random number and multiplying by 4 (or maybe by
> raising 2 to that power) :-)
>
> More seriously, I guessed that most arrays were 16 devices or less, so
> this would not affect most arrays.
>
> I definitely don't think it needs a define. I'm very tempted to remove
> print_conf() completely, but it is sometimes useful. So having it
> present as long as it isn't a burden seems reasonable.
>
> If configuration was important I would change it to use pr_debug(), but
> then the default would be to not see the messages at all, and they can
> be useful in diagnosing problems reported on mailing lists.
>
> >
> > But how big a problem is this really? And what about for big RAID5/6
> > arrays as well?
>
> When you have 2000 devices in your RAID10 and half of them are removed
> at once, it currently reports on 2,000,000 devices. With the patch it
> is only 32000. Still possibly too many.
>
> If you have 2000 devices in your RAID5/6 and half of them are removed,
> you have other problems.
>
> >
> > Or would it be also good to condence the output of print_conf()
> > itself?
>
> Probably a very good idea. Maybe the default could print a fixed-size
> summary and the rest goes in pr_debug()??
>
> >
> > Of if it's noise, why not just remove it completely? Can this
> > information be found in /proc/mdstat instead?
>
> Its value is historical - trying to understand a past sequence of
> events. For that, something in the logs beats something in /proc.
>
> >
> > Sorry I havent' looked in the code deeply, but this just struck me as
> > a change that might not be ideal.
>
> Fair enough - your comments are very valid. I'm not really sure what is
> best. I only know what is worst :-) and want to avoid that.
I would prefer pr_debug. pr_debug can be enabled dynamically. If the info is
helpful for diagnosing, enabling it is simple.
Thanks,
Shaohua
^ permalink raw reply
* Re: [md PATCH 05/18] md/raid10: add rcu protection to rdev access in raid10_sync_request.
From: Shaohua Li @ 2016-06-03 22:33 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
In-Reply-To: <20160602061952.2939.98809.stgit@noble>
On Thu, Jun 02, 2016 at 04:19:52PM +1000, Neil Brown wrote:
> mirrors[].rdev can become NULL at any point unless:
> - a counted reference is held
> - ->reconfig_mutex is held, or
> - rcu_read_lock() is held
>
> Previously they could not become NULL during a resync/recovery/reshape either.
> However when remove_and_add_spares() was added to hot_remove_disk(), that
> changed.
>
> So raid10_sync_request didn't previously need to protect rdev access,
> but now it does.
>
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
> drivers/md/raid10.c | 120 +++++++++++++++++++++++++++++++--------------------
> 1 file changed, 72 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index e6f3a11f8f70..f6f21a253926 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -2871,11 +2871,14 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> /* Completed a full sync so the replacements
> * are now fully recovered.
> */
> - for (i = 0; i < conf->geo.raid_disks; i++)
> - if (conf->mirrors[i].replacement)
> - conf->mirrors[i].replacement
> - ->recovery_offset
> - = MaxSector;
> + rcu_read_lock();
> + for (i = 0; i < conf->geo.raid_disks; i++) {
> + struct md_rdev *rdev =
> + rcu_dereference(conf->mirrors[i].replacement);
> + if (rdev)
> + rdev->recovery_offset = MaxSector;
> + }
> + rcu_read_unlock();
> }
> conf->fullsync = 0;
> }
> @@ -2934,14 +2937,17 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> int must_sync;
> int any_working;
> struct raid10_info *mirror = &conf->mirrors[i];
> + struct md_rdev *mrdev, *mreplace;
>
> - if ((mirror->rdev == NULL ||
> - test_bit(In_sync, &mirror->rdev->flags))
> - &&
> - (mirror->replacement == NULL ||
> - test_bit(Faulty,
> - &mirror->replacement->flags)))
> + rcu_read_lock();
> + mrdev = rcu_dereference(mirror->rdev);
> + mreplace = rcu_dereference(mirror->replacement);
> +
> + if ((mrdev == NULL ||
> + test_bit(In_sync, &mrdev->flags))) {
> + rcu_read_unlock();
> continue;
> + }
We don't check mreplace here.
Thanks,
Shaohua
^ permalink raw reply
* Re: [md PATCH 01/18] md: disconnect device from personality before trying to remove it.
From: Shaohua Li @ 2016-06-03 22:31 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
In-Reply-To: <20160602061952.2939.77544.stgit@noble>
On Thu, Jun 02, 2016 at 04:19:52PM +1000, Neil Brown wrote:
> When the HOT_REMOVE_DISK ioctl is used to remove a device, we
> call remove_and_add_spares() which will remove it from the personality
> if possible. This improves the chances that the removal will succeed.
>
> When writing "remove" to dev-XX/state, we don't. So that can fail more easily.
>
> So add the remove_and_add_spares() into "remove" handling.
>
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
> drivers/md/md.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 866825f10b4c..2d26099e1160 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -2596,6 +2596,8 @@ state_store(struct md_rdev *rdev, const char *buf, size_t len)
> else
> err = -EBUSY;
> } else if (cmd_match(buf, "remove")) {
> + clear_bit(Blocked, &rdev->flags);
> + remove_and_add_spares(rdev->mddev, rdev);
> if (rdev->raid_disk >= 0)
> err = -EBUSY;
> else {
Do we need wakeup rdev->blocked_wait here? I noticed some times we do the
wakeup but sometimes we not. This makes me worry about
md_wait_for_blocked_rdev. Will timeout cause anything bad?
Thanks,
Shaohua
^ permalink raw reply
* Re: [md PATCH 00/18] Assorted minor fixes, particularly RCU protection.
From: Shaohua Li @ 2016-06-03 22:28 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
In-Reply-To: <20160602061319.2939.72280.stgit@noble>
On Thu, Jun 02, 2016 at 04:19:52PM +1000, Neil Brown wrote:
> The 'rdev' fields in each personality's config data are often accessed
> under rcu_read_lock() protection to avoid races with
> ->hot_remove_disk() removing the rdev.
>
> Originally this was not necessary during resync/recovery etc because
> ->hot_remove_disk() was only called from md_check_recovery(), and it
> would only make the call if there was no resync etc happening.
>
> However we now call ->hot_remove_disk() (from
> remove_and_add_spares()) from other contexts, so there could be a race
> in the resync code.
>
> So this patch set adds a lot of extra rcu_read_lock protection and
> clean up some other bits and pieces on the way.
>
> My goal was the final patch. If you have a large raid10 array and
> fail half of the devices at once (e.g. unplug a rack with half of the
> mirrors) then synchronize_rcu() will be called once for each device,
> which can add up to a big delay. A single call should suffice.
> The final patch makes that change.
Thanks, make a lot of sense. Actually there is bug report related to RCU.
https://bugzilla.kernel.org/show_bug.cgi?id=116021
I had the same patch of your patch 3 and the bug submitter is trying.
I only had some minor issues, will reply separated. Otherwise this patch set is
great.
Thanks,
Shaohua
^ permalink raw reply
* Re: [PATCH 0/2] Fixes for lots of arrays
From: Jes Sorensen @ 2016-06-03 19:37 UTC (permalink / raw)
To: Mike Lovell; +Cc: linux-raid
In-Reply-To: <1463595794-17045-1-git-send-email-mlovell@bluehost.com>
Mike Lovell <mlovell@bluehost.com> writes:
> This patch series fixes two issues around having more than 127 arrays on a
> system. The first one fixes an issue with using a dev_t as int and the
> number going negative when the array number would be larger than 2<<19. This
> would happen when more than 128 arrays were created on the system without
> creating the arrays by name. Manually specifying the large number would also
> fail.
>
> The second patch changes find_free_devnm in mdopen.c to use go to (2<<9)-1
> after 128 arrays have been created. Newer versions of the kernel don't allow
> the user to specify an array number than 511 so mdadm shouldn't automatically
> choose a bigger number. There was discussion about checking for new_array
> in /sys/module/md_mod/parameters on the list but that parameter has been in
> the kernel since 2.6.29. Any kernel from the last 7 years would still be
> limited by the check so it probably isn't worth a special case.
>
> Mike Lovell (2):
> Use dev_t for devnm2devid and devid2devnm
> Change behavior in find_free_devnm when wrapping around.
>
> Detail.c | 4 ++--
> Grow.c | 2 +-
> lib.c | 2 +-
> mapfile.c | 2 +-
> mdadm.h | 4 ++--
> mdopen.c | 6 +++---
> util.c | 6 +++---
> 7 files changed, 13 insertions(+), 13 deletions(-)
Applied!
Thanks,
Jes
^ permalink raw reply
* Re: raid 5 crashed
From: Sarah Newman @ 2016-06-03 16:31 UTC (permalink / raw)
To: bobzer; +Cc: linux-raid
In-Reply-To: <CADzS=aowHsk5LhoFMBXaVy5=nBBF5iMn6Axoq8z=okHTo=b9hQ@mail.gmail.com>
On 06/03/2016 08:27 AM, bobzer wrote:
> i use losetup to make my dd image a dev/loop device and took the risk
> of start the raid but it doesn't work:
> i did :
> mdadm --assemble --force --name=/dev/md0 /dev/sdb1 /dev/sdd1
> /dev/loop2 /dev/sdc1
> it answer :
> mdadm: device /dev/sdb1 exist but is not an md array
>
> except that is not true, sdb1 and sdd1 are the correct drive. sdc1 is
> the one saw as a spare
The --name is for if you want to call the device something other than just md0. If you use /dev/md0 instead of --name=/dev/md0 it might work.
^ permalink raw reply
* Re: raid 5 crashed
From: bobzer @ 2016-06-03 15:27 UTC (permalink / raw)
To: Mikael Abrahamsson; +Cc: Brad Campbell, linux-raid
In-Reply-To: <alpine.DEB.2.02.1606030947180.28955@uplift.swm.pp.se>
I'm still trying to recover my data, i did a dd_rescue of the
partition only (i regret that, i don't know why i didn't took the
whole disk :-( )
unluckily me i got problem to be able to see all the disk i need in my
serveur (it's a vm and i miss sata port on the host)
i got a sata controller card which is not a recognize by esxi but
perfectly working with linux so i'm using gparted live
in gparted live the version of mdadm is 3.4 28th january 2016 , on my
serveur is mdadm - v3.3-78-gf43f5b3 - 02nd avril 2014
so i am asking myself is it better for me to try to rebuild my the
array from my original serveur or from gparted ?
i use losetup to make my dd image a dev/loop device and took the risk
of start the raid but it doesn't work:
i did :
mdadm --assemble --force --name=/dev/md0 /dev/sdb1 /dev/sdd1
/dev/loop2 /dev/sdc1
it answer :
mdadm: device /dev/sdb1 exist but is not an md array
except that is not true, sdb1 and sdd1 are the correct drive. sdc1 is
the one saw as a spare
so i don't know what to do :-(
thank you for your advice
PS: sorry for my english it's not my first language
^ permalink raw reply
* Re: [RFC v2 1/3] block: Introduce blk_bio_map_sg() to map one bio
From: Jens Axboe @ 2016-06-03 14:38 UTC (permalink / raw)
To: Baolin Wang, agk, snitzer, dm-devel, herbert, davem
Cc: ebiggers3, js1304, tadeusz.struk, smueller, standby24x7, shli,
dan.j.williams, martin.petersen, sagig, kent.overstreet,
keith.busch, tj, ming.lei, broonie, arnd, linux-crypto,
linux-block, linux-raid, linux-kernel
In-Reply-To: <2f0f6811938dafd14c6c7b1dbe8fbabfdcbb19c1.1464346333.git.baolin.wang@linaro.org>
On 05/27/2016 05:11 AM, Baolin Wang wrote:
> +/*
> + * Map a bio to scatterlist, return number of sg entries setup. Caller must
> + * make sure sg can hold bio segments entries.
> + */
> +int blk_bio_map_sg(struct request_queue *q, struct bio *bio,
> + struct scatterlist *sglist)
> +{
> + struct scatterlist *sg = NULL;
> + int nsegs = 0;
> +
> + if (bio)
> + nsegs = __blk_bios_map_sg(q, bio, sglist, &sg, true);
> +
> + if (sg)
> + sg_mark_end(sg);
Put that if (sg) inside the if (bio) section, 'sg' isn't going to be
non-NULL outside of that.
Additionally, who would call this with a NULL bio? That seems odd, I'd
get rid of that check completely.
--
Jens Axboe
^ permalink raw reply
* Re: [RFC v2 1/3] block: Introduce blk_bio_map_sg() to map one bio
From: Jens Axboe @ 2016-06-03 14:35 UTC (permalink / raw)
To: Baolin Wang, agk, snitzer, dm-devel, herbert, davem
Cc: ebiggers3, js1304, tadeusz.struk, smueller, standby24x7, shli,
dan.j.williams, martin.petersen, sagig, kent.overstreet,
keith.busch, tj, ming.lei, broonie, arnd, linux-crypto,
linux-block, linux-raid, linux-kernel
In-Reply-To: <2f0f6811938dafd14c6c7b1dbe8fbabfdcbb19c1.1464346333.git.baolin.wang@linaro.org>
On 05/27/2016 05:11 AM, Baolin Wang wrote:
> In dm-crypt, it need to map one bio to scatterlist for improving the
> hardware engine encryption efficiency. Thus this patch introduces the
> blk_bio_map_sg() function to map one bio with scatterlists.
>
> For avoiding the duplicated code in __blk_bios_map_sg() function, add
> one parameter to distinguish bio map or request map.
Just detach the bio in blk_bio_map_sg() instead of adding a separate
case (and argument) for it in __blk_bios_map_sg().
--
Jens Axboe
^ permalink raw reply
* Re: [RFC v2 2/3] crypto: Introduce CRYPTO_ALG_BULK flag
From: Baolin Wang @ 2016-06-03 10:47 UTC (permalink / raw)
To: Herbert Xu
Cc: Jens Axboe, Alasdair G Kergon, Mike Snitzer,
open list:DEVICE-MAPPER (LVM), David Miller, Eric Biggers,
Joonsoo Kim, tadeusz.struk, smueller, Masanari Iida, Shaohua Li,
Dan Williams, Martin K. Petersen, Sagi Grimberg, Kent Overstreet,
Keith Busch, Tejun Heo, Ming Lei, Mark Brown, Arnd Bergmann,
linux-crypto, linux-block,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT, LKML
In-Reply-To: <20160603100951.GA31382@gondor.apana.org.au>
On 3 June 2016 at 18:09, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Fri, Jun 03, 2016 at 05:23:59PM +0800, Baolin Wang wrote:
>>
>> Assuming one 64K size bio coming, we can map the whole bio with one sg
>> table in crypt_convert_bulk_block() function. But if we send this bulk
>> request to crypto layer, we should divide the bulk request into small
>> requests, and each small request should be one sector size (512 bytes)
>> with assuming the correct IV, but we need to allocate small requests
>> memory for the division, which will not good for IO mapping, and how
>> each small request connect to dm-crypt (how to notify the request is
>> done?)?
>
> Why won't it be good? The actual AES block size is 16 and yet we
Like I said, we should avoid memory allocation to improve efficiency
in the IO path. Another hand is how the divided small requests
(allocate request memory at crypt layer) connect with dm-crypt? Since
dm-crypt just send one bulk request to crypt layer, but it will be
divided into small requests at crypt layer.
> have no trouble when you feed it a block of 512 bytes.
That's right.
--
Baolin.wang
Best Regards
^ permalink raw reply
* Re: [RFC v2 2/3] crypto: Introduce CRYPTO_ALG_BULK flag
From: Herbert Xu @ 2016-06-03 10:09 UTC (permalink / raw)
To: Baolin Wang
Cc: Jens Axboe, Alasdair G Kergon, Mike Snitzer,
open list:DEVICE-MAPPER (LVM), David Miller, Eric Biggers,
Joonsoo Kim, tadeusz.struk, smueller, Masanari Iida, Shaohua Li,
Dan Williams, Martin K. Petersen, Sagi Grimberg, Kent Overstreet,
Keith Busch, Tejun Heo, Ming Lei, Mark Brown, Arnd Bergmann,
linux-crypto, linux-block,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT, LKML
In-Reply-To: <CAMz4kuL4zAg8dRRM7Lv4tp-c_GHq=0mYzvb88gDEd5YSOtvpRA@mail.gmail.com>
On Fri, Jun 03, 2016 at 05:23:59PM +0800, Baolin Wang wrote:
>
> Assuming one 64K size bio coming, we can map the whole bio with one sg
> table in crypt_convert_bulk_block() function. But if we send this bulk
> request to crypto layer, we should divide the bulk request into small
> requests, and each small request should be one sector size (512 bytes)
> with assuming the correct IV, but we need to allocate small requests
> memory for the division, which will not good for IO mapping, and how
> each small request connect to dm-crypt (how to notify the request is
> done?)?
Why won't it be good? The actual AES block size is 16 and yet we
have no trouble when you feed it a block of 512 bytes.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [RFC v2 2/3] crypto: Introduce CRYPTO_ALG_BULK flag
From: Baolin Wang @ 2016-06-03 9:23 UTC (permalink / raw)
To: Herbert Xu
Cc: Jens Axboe, Alasdair G Kergon, Mike Snitzer,
open list:DEVICE-MAPPER (LVM), David Miller, Eric Biggers,
Joonsoo Kim, tadeusz.struk, smueller, Masanari Iida, Shaohua Li,
Dan Williams, Martin K. Petersen, Sagi Grimberg, Kent Overstreet,
Keith Busch, Tejun Heo, Ming Lei, Mark Brown, Arnd Bergmann,
linux-crypto, linux-block,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT, LKML
In-Reply-To: <20160603082152.GA30551@gondor.apana.org.au>
On 3 June 2016 at 16:21, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Fri, Jun 03, 2016 at 04:15:28PM +0800, Baolin Wang wrote:
>>
>> Suppose the cbc(aes) algorithm, which can not be handled through bulk
>> interface, it need to map the data sector by sector.
>> If we also handle the cbc(aes) algorithm with bulk interface, we need
>> to divide the sg table into sectors and need to allocate request
>> memory for each divided sectors (As Mike pointed out this is in the
>> IO mapping
>> path and we try to avoid memory allocations at all costs -- due to the
>> risk of deadlock when issuing IO to stacked block devices (dm-crypt
>> could be part of a much more elaborate IO stack). ), that will
>> introduce more messy things I think.
>
> Perhaps I'm not making myself very clear. If you move the IV
> generation into the crypto API, those crypto API algorithms will
> be operating at the sector level.
Yeah, IV generation is OK. But it is not only related to IV thing. For example:
(1) For ecb(aes) algorithm which don't need to handle IV generation,
so it can support bulk mode:
Assuming one 64K size bio coming , we can map the whole bio with one
sg table in dm-crypt (assume it used 16 scatterlists from sg table),
then issue the 'skcipher_request_set_crypt()' function to set one
request with the mapped sg table, which will be sent to crypto driver
to be handled.
(2) For cbc(aes) algorithm which need to handle IV generation sector
by sector, so it can not support bulk mode and can not use bulk
interface:
Assuming one 64K size bio coming , we should map the bio sector by
sector with one scatterlist at one time. Each time we will issue the
'skcipher_request_set_crypt()' function to set one request with only
one mapped scatterlist, until it handled done the whole bio.
(3) As your suggestion, if we also use bulk interface for cbc(aes)
algorithm assuming it did all the requisite magic to generate the
correct IV:
Assuming one 64K size bio coming, we can map the whole bio with one sg
table in crypt_convert_bulk_block() function. But if we send this bulk
request to crypto layer, we should divide the bulk request into small
requests, and each small request should be one sector size (512 bytes)
with assuming the correct IV, but we need to allocate small requests
memory for the division, which will not good for IO mapping, and how
each small request connect to dm-crypt (how to notify the request is
done?)?
Thus we should not handle every algorithm with bulk interface.
>
> For example, assuming you're doing lmk, then the algorithm would
> be called lmk(cbc(aes)) and it will take as its input one or more
> sectors and for each sector it should generate an IV and operate
> on it.
>
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
Baolin.wang
Best Regards
^ permalink raw reply
* Re: [RFC v2 2/3] crypto: Introduce CRYPTO_ALG_BULK flag
From: Herbert Xu @ 2016-06-03 8:21 UTC (permalink / raw)
To: Baolin Wang
Cc: Jens Axboe, Alasdair G Kergon, Mike Snitzer,
open list:DEVICE-MAPPER (LVM), David Miller, Eric Biggers,
Joonsoo Kim, tadeusz.struk, smueller, Masanari Iida, Shaohua Li,
Dan Williams, Martin K. Petersen, Sagi Grimberg, Kent Overstreet,
Keith Busch, Tejun Heo, Ming Lei, Mark Brown, Arnd Bergmann,
linux-crypto, linux-block,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT, LKML
In-Reply-To: <CAMz4kuJA7ii=sjGq9APNSDMZQmh4yj46mmm44noYQegQo+=cAw@mail.gmail.com>
On Fri, Jun 03, 2016 at 04:15:28PM +0800, Baolin Wang wrote:
>
> Suppose the cbc(aes) algorithm, which can not be handled through bulk
> interface, it need to map the data sector by sector.
> If we also handle the cbc(aes) algorithm with bulk interface, we need
> to divide the sg table into sectors and need to allocate request
> memory for each divided sectors (As Mike pointed out this is in the
> IO mapping
> path and we try to avoid memory allocations at all costs -- due to the
> risk of deadlock when issuing IO to stacked block devices (dm-crypt
> could be part of a much more elaborate IO stack). ), that will
> introduce more messy things I think.
Perhaps I'm not making myself very clear. If you move the IV
generation into the crypto API, those crypto API algorithms will
be operating at the sector level.
For example, assuming you're doing lmk, then the algorithm would
be called lmk(cbc(aes)) and it will take as its input one or more
sectors and for each sector it should generate an IV and operate
on it.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [RFC v2 2/3] crypto: Introduce CRYPTO_ALG_BULK flag
From: Baolin Wang @ 2016-06-03 8:15 UTC (permalink / raw)
To: Herbert Xu
Cc: Jens Axboe, Alasdair G Kergon, Mike Snitzer,
open list:DEVICE-MAPPER (LVM), David Miller, Eric Biggers,
Joonsoo Kim, tadeusz.struk, smueller, Masanari Iida, Shaohua Li,
Dan Williams, Martin K. Petersen, Sagi Grimberg, Kent Overstreet,
Keith Busch, Tejun Heo, Ming Lei, Mark Brown, Arnd Bergmann,
linux-crypto, linux-block,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT, LKML
In-Reply-To: <20160603075435.GA30149@gondor.apana.org.au>
On 3 June 2016 at 15:54, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Fri, Jun 03, 2016 at 03:10:31PM +0800, Baolin Wang wrote:
>> On 3 June 2016 at 14:51, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>> > On Fri, Jun 03, 2016 at 02:48:34PM +0800, Baolin Wang wrote:
>> >>
>> >> If we move the IV generation into the crypto API, we also can not
>> >> handle every algorithm with the bulk interface. Cause we also need to
>> >> use different methods to map one whole bio or map one sector according
>> >> to the algorithm whether can support bulk mode or not. Please correct
>> >> me if I misunderstand your points. Thanks.
>> >
>> > Which ones can't be handled this way?
>>
>> What I mean is bulk mode and sector mode's difference is not only the
>> IV handling method, but also the method to map the data with
>> scatterlists.
>> Then we have two processes in dm-crypt ( crypt_convert_block() and
>> crypt_convert_bulk_block() ) to handle the data, so we can not handle
>> every algorithm with the bulk interface.
>
> As I asked, which algorithm can't you handle through the bulk
> interface, assuming it did all the requisite magic to generate
> the correct IV?
Suppose the cbc(aes) algorithm, which can not be handled through bulk
interface, it need to map the data sector by sector.
If we also handle the cbc(aes) algorithm with bulk interface, we need
to divide the sg table into sectors and need to allocate request
memory for each divided sectors (As Mike pointed out this is in the
IO mapping
path and we try to avoid memory allocations at all costs -- due to the
risk of deadlock when issuing IO to stacked block devices (dm-crypt
could be part of a much more elaborate IO stack). ), that will
introduce more messy things I think.
>
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
Baolin.wang
Best Regards
^ permalink raw reply
* Re: [RFC v2 2/3] crypto: Introduce CRYPTO_ALG_BULK flag
From: Herbert Xu @ 2016-06-03 7:54 UTC (permalink / raw)
To: Baolin Wang
Cc: Jens Axboe, Alasdair G Kergon, Mike Snitzer,
open list:DEVICE-MAPPER (LVM), David Miller, Eric Biggers,
Joonsoo Kim, tadeusz.struk, smueller, Masanari Iida, Shaohua Li,
Dan Williams, Martin K. Petersen, Sagi Grimberg, Kent Overstreet,
Keith Busch, Tejun Heo, Ming Lei, Mark Brown, Arnd Bergmann,
linux-crypto, linux-block,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT, LKML
In-Reply-To: <CAMz4ku+C75VxiO41dBZDm9+PZ_BuUWqY_bmhRDbRwKPVCp+O-g@mail.gmail.com>
On Fri, Jun 03, 2016 at 03:10:31PM +0800, Baolin Wang wrote:
> On 3 June 2016 at 14:51, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > On Fri, Jun 03, 2016 at 02:48:34PM +0800, Baolin Wang wrote:
> >>
> >> If we move the IV generation into the crypto API, we also can not
> >> handle every algorithm with the bulk interface. Cause we also need to
> >> use different methods to map one whole bio or map one sector according
> >> to the algorithm whether can support bulk mode or not. Please correct
> >> me if I misunderstand your points. Thanks.
> >
> > Which ones can't be handled this way?
>
> What I mean is bulk mode and sector mode's difference is not only the
> IV handling method, but also the method to map the data with
> scatterlists.
> Then we have two processes in dm-crypt ( crypt_convert_block() and
> crypt_convert_bulk_block() ) to handle the data, so we can not handle
> every algorithm with the bulk interface.
As I asked, which algorithm can't you handle through the bulk
interface, assuming it did all the requisite magic to generate
the correct IV?
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: raid 5 crashed
From: Mikael Abrahamsson @ 2016-06-03 7:52 UTC (permalink / raw)
To: Brad Campbell; +Cc: linux-raid
In-Reply-To: <40b1ed1e-b0bd-a496-b5c8-6ad1f370b3ee@fnarfbargle.com>
On Fri, 3 Jun 2016, Brad Campbell wrote:
> If we took the bad block list from the dd_rescue, and fed it to hdparm
> to create bad sectors in all those locations on the cloned disk, md
> would get a bad sector on read and attempt a recovery rather than
> returning zero, This would "in theory" cause a re-write of good data
> back to that disk and minimise the chance of data loss.
Why would you use dd_rescue on a drive unless you're doing it because
you're getting UREs on your remaining component drives on your degraded
RAID5 so it won't complete resync? That's at least the most common
use-case for ddrescue I've seen in md-raid scenarios.
> This might be a useful "last ditch" recovery method to allow you to
> bring up an array with a cloned disk and minimise data loss. On the
> other hand, lets say you are using it to bring up a RAID 5 with 2 failed
> disks. One completely dead and one that you managed to clone most of.
> When you extract the data from the running and degraded array, md will
> pass the read error up the stack when it encounters the bad sectors,
> allowing your copy or rsync session to log which files are affected as
> you backup the remaining contents rather than just return silently
> corrupted files.
This use case makes a lot more sense to me than the first one. Knowing
what files are now bad would be very useful.
But wouldn't it be better to put the known errors in the new bad_blocks
list in md that I believe is a fairly recent feature?
--
Mikael Abrahamsson email: swmike@swm.pp.se
^ permalink raw reply
* Re: [RFC v2 2/3] crypto: Introduce CRYPTO_ALG_BULK flag
From: Baolin Wang @ 2016-06-03 7:10 UTC (permalink / raw)
To: Herbert Xu
Cc: Jens Axboe, Alasdair G Kergon, Mike Snitzer,
open list:DEVICE-MAPPER (LVM), David Miller, Eric Biggers,
Joonsoo Kim, tadeusz.struk, smueller, Masanari Iida, Shaohua Li,
Dan Williams, Martin K. Petersen, Sagi Grimberg, Kent Overstreet,
Keith Busch, Tejun Heo, Ming Lei, Mark Brown, Arnd Bergmann,
linux-crypto, linux-block,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT, LKML
In-Reply-To: <20160603065146.GA29953@gondor.apana.org.au>
On 3 June 2016 at 14:51, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Fri, Jun 03, 2016 at 02:48:34PM +0800, Baolin Wang wrote:
>>
>> If we move the IV generation into the crypto API, we also can not
>> handle every algorithm with the bulk interface. Cause we also need to
>> use different methods to map one whole bio or map one sector according
>> to the algorithm whether can support bulk mode or not. Please correct
>> me if I misunderstand your points. Thanks.
>
> Which ones can't be handled this way?
What I mean is bulk mode and sector mode's difference is not only the
IV handling method, but also the method to map the data with
scatterlists.
Then we have two processes in dm-crypt ( crypt_convert_block() and
crypt_convert_bulk_block() ) to handle the data, so we can not handle
every algorithm with the bulk interface.
>
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
Baolin.wang
Best Regards
^ permalink raw reply
* Re: [RFC v2 2/3] crypto: Introduce CRYPTO_ALG_BULK flag
From: Herbert Xu @ 2016-06-03 6:51 UTC (permalink / raw)
To: Baolin Wang
Cc: Jens Axboe, Alasdair G Kergon, Mike Snitzer,
open list:DEVICE-MAPPER (LVM), David Miller, Eric Biggers,
Joonsoo Kim, tadeusz.struk, smueller, Masanari Iida, Shaohua Li,
Dan Williams, Martin K. Petersen, Sagi Grimberg, Kent Overstreet,
Keith Busch, Tejun Heo, Ming Lei, Mark Brown, Arnd Bergmann,
linux-crypto, linux-block,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT, LKML
In-Reply-To: <CAMz4kuJsR0OUJFq68B6znmoTfSeaT_Xhmq14Y4yGSWDJcNE6Hw@mail.gmail.com>
On Fri, Jun 03, 2016 at 02:48:34PM +0800, Baolin Wang wrote:
>
> If we move the IV generation into the crypto API, we also can not
> handle every algorithm with the bulk interface. Cause we also need to
> use different methods to map one whole bio or map one sector according
> to the algorithm whether can support bulk mode or not. Please correct
> me if I misunderstand your points. Thanks.
Which ones can't be handled this way?
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [RFC v2 2/3] crypto: Introduce CRYPTO_ALG_BULK flag
From: Baolin Wang @ 2016-06-03 6:48 UTC (permalink / raw)
To: Herbert Xu
Cc: Jens Axboe, Alasdair G Kergon, Mike Snitzer,
open list:DEVICE-MAPPER (LVM), David Miller, Eric Biggers,
Joonsoo Kim, tadeusz.struk, smueller, Masanari Iida, Shaohua Li,
Dan Williams, Martin K. Petersen, Sagi Grimberg, Kent Overstreet,
Keith Busch, Tejun Heo, Ming Lei, Mark Brown, Arnd Bergmann,
linux-crypto, linux-block,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT, LKML
In-Reply-To: <20160602082606.GA15226@gondor.apana.org.au>
Hi Herbet,
On 2 June 2016 at 16:26, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Fri, May 27, 2016 at 07:11:23PM +0800, Baolin Wang wrote:
>> Now some cipher hardware engines prefer to handle bulk block rather than one
>> sector (512 bytes) created by dm-crypt, cause these cipher engines can handle
>> the intermediate values (IV) by themselves in one bulk block. This means we
>> can increase the size of the request by merging request rather than always 512
>> bytes and thus increase the hardware engine processing speed.
>>
>> So introduce 'CRYPTO_ALG_BULK' flag to indicate this cipher can support bulk
>> mode.
>>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>
> I think a better aproach would be to explicitly move the IV generation
> into the crypto API, similar to how we handle IPsec. Once you do
> that then every algorithm can be handled through the bulk interface.
>
Sorry for late reply.
If we move the IV generation into the crypto API, we also can not
handle every algorithm with the bulk interface. Cause we also need to
use different methods to map one whole bio or map one sector according
to the algorithm whether can support bulk mode or not. Please correct
me if I misunderstand your points. Thanks.
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
Baolin.wang
Best Regards
^ permalink raw reply
* [PATCH 2/2] md: simplify the code with md_kick_rdev_from_array
From: Guoqing Jiang @ 2016-06-03 3:32 UTC (permalink / raw)
To: shli; +Cc: linux-raid, Guoqing Jiang
In-Reply-To: <1464924725-18096-1-git-send-email-gqjiang@suse.com>
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
drivers/md/md.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 25d4542..459f189 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2478,8 +2478,7 @@ static int add_bound_rdev(struct md_rdev *rdev)
if (add_journal)
mddev_resume(mddev);
if (err) {
- unbind_rdev_from_array(rdev);
- export_rdev(rdev);
+ md_kick_rdev_from_array(rdev);
return err;
}
}
--
2.6.6
^ permalink raw reply related
* [PATCH 1/2] md-cluster: fix deadlock issue when add disk to an recoverying array
From: Guoqing Jiang @ 2016-06-03 3:32 UTC (permalink / raw)
To: shli; +Cc: linux-raid, Guoqing Jiang
Add a disk to an array which is performing recovery
is a little complicated, we need to do both reap the
sync thread and perform add disk for the case, then
it caused deadlock as follows.
linux44:~ # ps aux|grep md|grep D
root 1822 0.0 0.0 0 0 ? D 16:50 0:00 [md127_resync]
root 1848 0.0 0.0 19860 952 pts/0 D+ 16:50 0:00 mdadm --manage /dev/md127 --re-add /dev/vdb
linux44:~ # cat /proc/1848/stack
[<ffffffff8107afde>] kthread_stop+0x6e/0x120
[<ffffffffa051ddb0>] md_unregister_thread+0x40/0x80 [md_mod]
[<ffffffffa0526e45>] md_reap_sync_thread+0x15/0x150 [md_mod]
[<ffffffffa05271e0>] action_store+0x260/0x270 [md_mod]
[<ffffffffa05206b4>] md_attr_store+0xb4/0x100 [md_mod]
[<ffffffff81214a7e>] sysfs_write_file+0xbe/0x140
[<ffffffff811a6b98>] vfs_write+0xb8/0x1e0
[<ffffffff811a75b8>] SyS_write+0x48/0xa0
[<ffffffff8152a5c9>] system_call_fastpath+0x16/0x1b
[<00007f068ea1ed30>] 0x7f068ea1ed30
linux44:~ # cat /proc/1822/stack
[<ffffffffa05251a6>] md_do_sync+0x846/0xf40 [md_mod]
[<ffffffffa052402d>] md_thread+0x16d/0x180 [md_mod]
[<ffffffff8107ad94>] kthread+0xb4/0xc0
[<ffffffff8152a518>] ret_from_fork+0x58/0x90
Task1848 Task1822
md_attr_store (held reconfig_mutex by call mddev_lock())
action_store
md_reap_sync_thread
md_unregister_thread
kthread_stop md_wakeup_thread(mddev->thread);
wait_event(mddev->sb_wait, !test_bit(MD_CHANGE_PENDING))
md_check_recovery is triggered by wakeup mddev->thread,
but it can't clear MD_CHANGE_PENDING flag since it can't
get lock which was held by md_attr_store already.
To solve the deadlock problem, we move "->resync_finish()"
from md_do_sync to md_reap_sync_thread (after md_update_sb),
also MD_HELD_RESYNC_LOCK is introduced since it is possible
that node can't get resync lock in md_do_sync.
Then we do not need to wait for MD_CHANGE_PENDING is cleared
or not since metadata should be updated after md_update_sb,
so just call resync_finish if MD_HELD_RESYNC_LOCK is set.
We also unified the code after skip label, since set PENDING
for non-clustered case should be harmless.
Reviewed-by: NeilBrown <neilb@suse.com>
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
drivers/md/md.c | 23 +++++++++++------------
drivers/md/md.h | 3 +++
2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 866825f..25d4542 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -7809,6 +7809,7 @@ void md_do_sync(struct md_thread *thread)
if (ret)
goto skip;
+ set_bit(MD_CLUSTER_RESYNC_LOCKED, &mddev->flags);
if (!(test_bit(MD_RECOVERY_SYNC, &mddev->recovery) ||
test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) ||
test_bit(MD_RECOVERY_RECOVER, &mddev->recovery))
@@ -8147,18 +8148,11 @@ void md_do_sync(struct md_thread *thread)
}
}
skip:
- if (mddev_is_clustered(mddev) &&
- ret == 0) {
- /* set CHANGE_PENDING here since maybe another
- * update is needed, so other nodes are informed */
- set_mask_bits(&mddev->flags, 0,
- BIT(MD_CHANGE_PENDING) | BIT(MD_CHANGE_DEVS));
- md_wakeup_thread(mddev->thread);
- wait_event(mddev->sb_wait,
- !test_bit(MD_CHANGE_PENDING, &mddev->flags));
- md_cluster_ops->resync_finish(mddev);
- } else
- set_bit(MD_CHANGE_DEVS, &mddev->flags);
+ /* set CHANGE_PENDING here since maybe another update is needed,
+ * so other nodes are informed. It should be harmless for normal
+ * raid */
+ set_mask_bits(&mddev->flags, 0,
+ BIT(MD_CHANGE_PENDING) | BIT(MD_CHANGE_DEVS));
spin_lock(&mddev->lock);
if (!test_bit(MD_RECOVERY_INTR, &mddev->recovery)) {
@@ -8502,6 +8496,11 @@ void md_reap_sync_thread(struct mddev *mddev)
rdev->saved_raid_disk = -1;
md_update_sb(mddev, 1);
+ /* MD_CHANGE_PENDING should be cleared by md_update_sb, so we can
+ * call resync_finish here if MD_CLUSTER_RESYNC_LOCKED is set by
+ * clustered raid */
+ if (test_and_clear_bit(MD_CLUSTER_RESYNC_LOCKED, &mddev->flags))
+ md_cluster_ops->resync_finish(mddev);
clear_bit(MD_RECOVERY_RUNNING, &mddev->recovery);
clear_bit(MD_RECOVERY_DONE, &mddev->recovery);
clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
diff --git a/drivers/md/md.h b/drivers/md/md.h
index b5c4be7..03b19aa 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -204,6 +204,9 @@ struct mddev {
#define MD_RELOAD_SB 7 /* Reload the superblock because another node
* updated it.
*/
+#define MD_CLUSTER_RESYNC_LOCKED 8 /* cluster raid only, which means node
+ * already took resync lock, need to
+ * release the lock */
int suspended;
atomic_t active_io;
--
2.6.6
^ permalink raw reply related
* Re: raid 5 crashed
From: Brad Campbell @ 2016-06-03 1:05 UTC (permalink / raw)
To: Wols Lists, Edward Kuns
Cc: Phil Turmel, bobzer, linux-raid, Mikael Abrahamsson
In-Reply-To: <57503C3F.6020005@youngman.org.uk>
On 02/06/16 22:01, Wols Lists wrote:
> On 02/06/16 00:15, Brad Campbell wrote:
>> People keep saying that. I've never encountered it. I suspect it's just
>> not the problem that the hysterical ranting makes it out to be (either
>> that or the pile of cheap and nasty drives I have here are model citizens).
>> I've *never* seen a read error unless the drive was in trouble, and that
>> includes running dd reads in a loop over multiple days continuously.
>> If it were that bad I'd see drives failing SMART long tests routinely
>> also, and that does not happen either.
>
> Note I didn't say you *will* see an error. BUT. If I recall correctly,
> the specs say that one read error per 10TB read is acceptable for a
> desktop drive that is designated healthy. In other words, if a 4TB drive
> throws an error every third pass, then according to the spec it's a
> perfectly healthy drive.
>
> Yes. We know that most drives are far better than spec, and if it
> degrades to spec then it's probably heading for failure, but the fact
> remains. If you have 3 x 4TB desktop drives in an array, then the spec
> says you should expect, and be able to deal with, an error EVERY time
> you scan the array.
No, it really doesn't. Those URE figures say <' 1 in' 10^14, not '= 1'
in 10^14. So that's a statistical worst case rather than a "this is what
you should expect". In addition, it's not a linear extrapolation, it's a
probability.
By that logic I should "expect" to roll a 6 at least once every 6 dice
rolls.
You can't extrapolate statistical figures like that. Just the same as
you can't calculate drive failures from MTBF figures.
Just perform regular read tests on all drives and periodic array scrubs
and you'll be much better off.
I've never had a reported URE on any of my arrays with SAS drvies, most
have reallocated sectors. They perform background reads periodically and
auto-reallocate anything that is looking dodgy.
SATA drives don't do that, but we can manage that externally with long
SMART tests and array scrubs to force rewrite/reallocation.
Just don't go trying to extrapolate from manufacturers probability data.
There are plenty of garbage web pages littered around the net where
"experts" do that, leading to 'hysterical ranting' about how the world
is ending and RAID5 is the devil. Sure RAID5 can be an issue when
dealing with a catastrophic drive failure requiring a rebuild if you
don't look after your drives, and I use and prefer RAID6 to mitigate
that, but it's not the end of the world.
Now, on an interesting, related and completely different note. To get
back to the concept of using dd or dd_rescue, I had a thought last night
and I've never seen it mentioned anywhere.
When you clone a dud drive using dd_rescue, it creates a bad block log.
The reason we don't like doing this is because when you put the
replacement drive back into the array, md does not see the errors and
will happily return zero data when it reads any sector that was bad on
the old drive.
hdparm has a neat feature called --make-bad-sector. It uses a feature of
the ATA protocol to write a sector that contains an invalid CRC, so the
drive returns an error when you try and read it. The sector is restored
by a normal re-write, so no reallocation or permanent damage takes place.
If we took the bad block list from the dd_rescue, and fed it to hdparm
to create bad sectors in all those locations on the cloned disk, md
would get a bad sector on read and attempt a recovery rather than
returning zero, This would "in theory" cause a re-write of good data
back to that disk and minimise the chance of data loss.
This might be a useful "last ditch" recovery method to allow you to
bring up an array with a cloned disk and minimise data loss. On the
other hand, lets say you are using it to bring up a RAID 5 with 2 failed
disks. One completely dead and one that you managed to clone most of.
When you extract the data from the running and degraded array, md will
pass the read error up the stack when it encounters the bad sectors,
allowing your copy or rsync session to log which files are affected as
you backup the remaining contents rather than just return silently
corrupted files.
^ 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