* Re: [PATCH] IMSM: Correct --examine output for 4k disks
From: Jes Sorensen @ 2017-05-09 16:04 UTC (permalink / raw)
To: Mariusz Dabrowski, linux-raid; +Cc: Maksymilian Kunt
In-Reply-To: <1494331407-2049-1-git-send-email-mariusz.dabrowski@intel.com>
On 05/09/2017 08:03 AM, Mariusz Dabrowski wrote:
> From: Maksymilian Kunt <maksymilian.kunt@intel.com>
>
> "Array Size" and "Per Dev Size" are incorrect for disks with sector size
> different than 512B.
>
> Calculate "Array Size" and "Per Dev Size" based on sector size. Additionally
> print "Sector Size".
>
> Signed-off-by: Maksymilian Kunt <maksymilian.kunt@intel.com>
> Signed-off-by: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
> ---
> super-intel.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
I am a little wary of this change as it changes the order of the output.
Does anyone have tools parsing this information which could break from this?
Jes
> diff --git a/super-intel.c b/super-intel.c
> index e13c940..147a70c 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -1473,13 +1473,16 @@ static void print_imsm_dev(struct intel_super *super,
> ord & IMSM_ORD_REBUILD ? " (out-of-sync)" : "");
> } else
> printf(" This Slot : ?\n");
> + printf(" Sector Size : %u\n", super->sector_size);
> sz = __le32_to_cpu(dev->size_high);
> sz <<= 32;
> sz += __le32_to_cpu(dev->size_low);
> - printf(" Array Size : %llu%s\n", (unsigned long long)sz,
> + printf(" Array Size : %llu%s\n",
> + (unsigned long long)sz * 512 / super->sector_size,
> human_size(sz * 512));
> sz = blocks_per_member(map);
> - printf(" Per Dev Size : %llu%s\n", (unsigned long long)sz,
> + printf(" Per Dev Size : %llu%s\n",
> + (unsigned long long)sz * 512 / super->sector_size,
> human_size(sz * 512));
> printf(" Sector Offset : %llu\n",
> pba_of_lba0(map));
>
^ permalink raw reply
* Re: Fault tolerance with badblocks
From: Nix @ 2017-05-09 15:30 UTC (permalink / raw)
To: Tim Small; +Cc: linux-raid
In-Reply-To: <7dfa3eff-9194-002d-918b-42fbae865df3@buttersideup.com>
On 9 May 2017, Tim Small spake thusly:
> On 09/05/17 11:40, Nix wrote:
>> I've had disk failures without warning, and
>> non-failed disks with both read and write errors that would not go away,
>> but that SMART reallocation value just stayed stuck at zero through all
>> of it.
>
> Really? I see them pretty frequently... Let's see
>
> server1, RAID6 (4 disks), reallocated_sector_ct: 0 9 1 0
> server2, RAID5 (4 disks), reallocated_sector_ct: 0 0 0 0
> server3, RAID6 (5 disks), reallocated_sector_ct: 34 754 15 115 1
> server4, RAID5 (4 disks), reallocated_sector_ct: 0 0 0 0
> server5, RAID5 (4 disks), reallocated_sector_ct: 0 0 0 0
>
> Disk 2 in server3 (which has drives which are a bit long in the tooth)
> is scheduled to be replaced next time I visit that site.
>
> Are you looking at the 'raw' column in the smartctl output?
No, but since they all read all zero:
5 Reallocated_Sector_Ct PO--CK 100 100 010 - 0
this is pretty redundant.
I do see, on all my disks (regardless of hardware versus software RAID
or indeed age, and some of these disks are seven years old):
196 Reallocated_Event_Count 0x0032 200 200 000 Old_age Always - 0
197 Current_Pending_Sector 0x0032 200 200 000 Old_age Always - 0
198 Offline_Uncorrectable 0x0030 200 200 000 Old_age Offline - 0
199 UDMA_CRC_Error_Count 0x0032 200 200 000 Old_age Always - 0
200 Multi_Zone_Error_Rate 0x0008 200 200 051 Old_age Offline - 0
One figure is much higher:
195 Hardware_ECC_Recovered -O-RC- 100 064 000 - 2067212
195 Hardware_ECC_Recovered -O-RC- 100 064 000 - 2088928
195 Hardware_ECC_Recovered -O-RC- 082 064 000 - 156528817
195 Hardware_ECC_Recovered -O-RC- 082 065 000 - 156513792
but this is on a bunch of three-month-old Seagate enterprise disks, and
as with the seek error rate Seagate use a deeply bizarre encoding for
this value, and none of the SeaChest programs seem to be able to decode
it.
It appears that the lower the decoded value, the worse things are -- I
have no idea why two of my drives are doing so much worse than two
others on this score. I guess I should keep an eye on them. In any case,
it's going up fast on those two even when the drives are totally idle
and even when I forcibly spin them down... I don't trust this figure to
tell me anything useful at all. SMART, borderline useless as ever.
Aside: in hex these are
001f8b0c
001fdfe0
095470b1
09543600
which rather suggests that the drives have two distinct encodings to me,
with two drives using one encoding and the other two another one,
probably split at the four-hex-digit mark -- but the drives have
identical firmware and the same model number...
^ permalink raw reply
* Re: [PATCH] mdadm/md.4: set page-length as 1000 to avoid warnings from grotty
From: Jes Sorensen @ 2017-05-09 14:53 UTC (permalink / raw)
To: Zhilong Liu; +Cc: linux-raid
In-Reply-To: <20170509062517.20755-1-zlliu@suse.com>
On 05/09/2017 02:25 AM, Zhilong Liu wrote:
> "nroff" ("man") or ("tbl") needs a long page to avoid warnings
> from "grotty", set 'pl' as 1000 to fix the following issue.
>
> This command invokes by Makefile rule.
> linux-tjrh:~/mdadm # man -l md.4 > test
> grotty:<standard input> (<standard input>):5967:
> character above first line discarded
> ... ...
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
I am not an nroff expert, so I'll take your word for it.
Applied!
Thanks,
Jes
^ permalink raw reply
* Re: Fault tolerance with badblocks
From: Tim Small @ 2017-05-09 12:15 UTC (permalink / raw)
To: Nix; +Cc: linux-raid
In-Reply-To: <87wp9q5n49.fsf@esperi.org.uk>
On 09/05/17 11:40, Nix wrote:
> I've had disk failures without warning, and
> non-failed disks with both read and write errors that would not go away,
> but that SMART reallocation value just stayed stuck at zero through all
> of it.
Really? I see them pretty frequently... Let's see
server1, RAID6 (4 disks), reallocated_sector_ct: 0 9 1 0
server2, RAID5 (4 disks), reallocated_sector_ct: 0 0 0 0
server3, RAID6 (5 disks), reallocated_sector_ct: 34 754 15 115 1
server4, RAID5 (4 disks), reallocated_sector_ct: 0 0 0 0
server5, RAID5 (4 disks), reallocated_sector_ct: 0 0 0 0
Disk 2 in server3 (which has drives which are a bit long in the tooth)
is scheduled to be replaced next time I visit that site.
Are you looking at the 'raw' column in the smartctl output?
Tim
^ permalink raw reply
* Re: [mdadm PATCH] Create: tell udev md device is not ready when first created.
From: Peter Rajnoha @ 2017-05-09 12:14 UTC (permalink / raw)
To: Wols Lists, Jes Sorensen, NeilBrown; +Cc: linux-raid, dm-devel
In-Reply-To: <590DF8FA.4090600@youngman.org.uk>
On 05/06/2017 06:25 PM, Wols Lists wrote:
> On 03/05/17 15:13, Peter Rajnoha wrote:
>> There's a difference though - when you're *creating* a completely new
>> device that is an abstraction over existing devices, you (most of the
>> time) expect that new device to be initialized. For those corner cases
>> where people do need to keep the old data, there can be an option to do
>> that.
>
> That's not a corner case. If there's old data that's the NORM. I get
> what you're after, I'm inclined to agree with you, but the default
> should be to DO NOTHING.
>
> If you want mdadm to mess about with the content of the drives you
> should either (a) explicitly tell it to (yes I would like that option
> :-), or (b) do it yourself beforehand - dd if=/dev/zero etc etc.
As for (b), the "dd" with zeroing all the future mdadm components I'm
going to use for an MD device is quite time-consuming operation, mainly
for large devices.
And we don't need to do that full zeroing at all, what I'm after is to
have the wipefs-like functionality directly integrated into mdadm (that
functionality is already a part of libbblkid and it's very easy to use
it as a matter of fact). With this, we can detect and wipe all
signatures that blkid detects. The blkid is used as a primary source of
information when processing uevents to decide about further processing.
Once we wipe all that blkid can see, we make the device clear for udev
processing too and we don't end up automatically activating some old
garbage that we don't intend to actually based on these uevent hooks.
--
Peter
^ permalink raw reply
* [PATCH] IMSM: Correct --examine output for 4k disks
From: Mariusz Dabrowski @ 2017-05-09 12:03 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, Maksymilian Kunt, Mariusz Dabrowski
From: Maksymilian Kunt <maksymilian.kunt@intel.com>
"Array Size" and "Per Dev Size" are incorrect for disks with sector size
different than 512B.
Calculate "Array Size" and "Per Dev Size" based on sector size. Additionally
print "Sector Size".
Signed-off-by: Maksymilian Kunt <maksymilian.kunt@intel.com>
Signed-off-by: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
---
super-intel.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index e13c940..147a70c 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1473,13 +1473,16 @@ static void print_imsm_dev(struct intel_super *super,
ord & IMSM_ORD_REBUILD ? " (out-of-sync)" : "");
} else
printf(" This Slot : ?\n");
+ printf(" Sector Size : %u\n", super->sector_size);
sz = __le32_to_cpu(dev->size_high);
sz <<= 32;
sz += __le32_to_cpu(dev->size_low);
- printf(" Array Size : %llu%s\n", (unsigned long long)sz,
+ printf(" Array Size : %llu%s\n",
+ (unsigned long long)sz * 512 / super->sector_size,
human_size(sz * 512));
sz = blocks_per_member(map);
- printf(" Per Dev Size : %llu%s\n", (unsigned long long)sz,
+ printf(" Per Dev Size : %llu%s\n",
+ (unsigned long long)sz * 512 / super->sector_size,
human_size(sz * 512));
printf(" Sector Offset : %llu\n",
pba_of_lba0(map));
--
1.8.3.1
^ permalink raw reply related
* Re: Fault tolerance with badblocks
From: David Brown @ 2017-05-09 11:58 UTC (permalink / raw)
To: Nix; +Cc: Anthony Youngman, Phil Turmel, Ravi (Tom) Hale, linux-raid
In-Reply-To: <878tm65kyx.fsf@esperi.org.uk>
On 09/05/17 13:27, Nix wrote:
> On 9 May 2017, David Brown uttered the following:
> (I'm not suggesting repairing RAID-5 mismatches. That's clearly
> impossible. You can't even tell what disk is affected. But in the RAID-6
> case none of this is impossible, or so it seems to me. You have at least
> three and probably four or more drives with consistent syndromes, and
> one that is out of whack. You know which one must be wrong -- the
> "minority vote" -- and you know what has to be done to make it
> consistent with the others again. Why not do it? It's no more risky than
> that aspect of a RAID rebuild from a failed disk would be.)
>
>> RAID will /not/ let you reliably detect or correct other sorts of
>> errors.
>
> ... only it clearly can. What stops it from handling the RAID-6-and-
> one-disk-is-wrong case where it cannot handle the RAID-6-and-one-disk-
> has-failed case, given that you can unambiguously determine which disk
> is wrong using the data on the surviving drives, with an undetected-
> failure probability of something way below 2^128? (I could work out the
> actual value but I haven't had any coffee yet and it seems pointless
> when it's that low.)
>
>> What does /not/ work, however, is trying to squeeze magic capabilities
>> out of existing layers in the system, or expecting more out of them that
>> they can give.
>
> I don't see that these capabilities are any more magic than what RAID-6
> does already. It can recover from two failed drives: why can't it
> recover from one wrong one? (Or, rather, from one drive with very
> occasionally wrong sectors on it. Obviously if it was always getting
> things wrong its presence is not a benefit and you have essentially
> fallen back to nothing better than RAID-5, only with worse performance.
> But that's what error thresholds are for, which md already employs in
> similar situations.)
>
I thought you said that you had read Neil's article. Please go back and
read it again. If you don't agree with what is written there, then
there is little more I can say to convince you.
One thing I can try, is to note that you are /not/ the first person to
think "Surely with RAID-6 we can correct mismatches - it should be
easy?". You are /not/ the first person to think "Correcting RAID-6
mismatches would be a marvellous feature that would make it /far/
better". Linux md raid does not correct RAID-6 mismatches found on a
scrub. To my (admittedly limited) knowledge, hardware RAID-6 systems do
not correct mismatches found on a scrub. If correcting RAID-6
mismatches were as simple, reliably, and useful as you seem to believe,
than I think Linux md raid would already do it - either as part of the
scrub, or as an extra utility to run on mismatched stripes.
^ permalink raw reply
* Re: [mdadm PATCH] Create: tell udev md device is not ready when first created.
From: Peter Rajnoha @ 2017-05-09 11:57 UTC (permalink / raw)
To: Wols Lists, Jes Sorensen, NeilBrown; +Cc: linux-raid, dm-devel
In-Reply-To: <590DF8FA.4090600@youngman.org.uk>
On 05/06/2017 06:25 PM, Wols Lists wrote:
> On 03/05/17 15:13, Peter Rajnoha wrote:
>> There's a difference though - when you're *creating* a completely new
>> device that is an abstraction over existing devices, you (most of the
>> time) expect that new device to be initialized. For those corner cases
>> where people do need to keep the old data, there can be an option to do
>> that.
>
> That's not a corner case. If there's old data that's the NORM. I get
> what you're after, I'm inclined to agree with you, but the default
> should be to DO NOTHING.
>
Well, if keeping old data is norm or not (IOW, if wiping is going to be
default or not) - I'm not going to hassle about this. But at least we
should let users to decide on command line directly. If mdadm decides to
do this part somewhere outside by some other external tool, it should at
least coordinate with that tool then - that's an alternative.
I just think it's not that common to take raw disks with some old data
(presumably starting at the beginning of the disk or some offset) and
then using these disks to create a new MD device on top of them and then
for us to expect we see exactly same data content - maybe yes with a
luck, but usually, the MD signature reserves some space at the beginning
(and/or end) of the device to write its signatures (which overwrites
some of the old content anyway).
The use case where keeping the old data would probably make more sense
is when you had those disks already used by an MD device, then you wrote
some data/content to the MD device, then you removed the MD device and
then you recreate it again with exactly the same raid level and with
exactly the same MD signature lengths as before so you end up with the
same data area revealed by the fresh MD device. Now, the question is,
how many users do this? (yes, for recovery, but probably not for fresh
new array)
Anyway, I'm still OK to have the wiping disabled by default, but what I
want to see is that option to do enable the wiping on demand at least
for mdadm.
The patch as it was proposed and written automatically marks device as
not usable (SYSTEMD_READY=0) on new MD device creation directly in udev
database. So it leaves the decision on something external to mdadm.
OK then, let's wipe it ourselves by calling, for example, "wipefs -a" on
that MD device - the wiping is done and the SYSTEMD_READY=0 flag is
dropped due to the WATCH udev rule that fires (and synthesizes CHANGE
uevent that causes the rules and udev db values to be reevaluated). This
works.
And now the other use case - to not wipe anything - user is not going to
call any wipefs-like tool, but just keeps things as they are. In this
case, for the udev database content to be correct we either need to wait
for next CHANGE uevent to happen (whatever it's cause is) OR we need to
synthesize that CHANGE uevent directly on command line by writing "echo
change > /sys/block/md.../uevent". Now, how should we educate users to
do this (quite low-level) extra call? If the extra uevent is not there,
the udev db is out-of-date simply.
If that patch stays, I think the next logical step for distributions is
to take the upstream version of mdadm and then to create a wrapper over
upstream mdadm to provide this option in addition.
So wrapped mdadm would look something like:
call mdadm --create
if (user_passed_option_to_do_wiping)
call wipefs -a /dev/md_name
else
write "change" to /sys/block/<md_name>/uevent
I'm afraid it's going to end this way, uselessly - it could have been
directly a part of mdadm with very little cost. That cost is dependency
on libbblkid (to get the signature offsets/lenghts we need to wipe for
event-based systems to not hook on them by mistake).
Now, what is the environment where libblkid is not present and mdadm is?
libblkid is quite basic library (part of util-linux) so present on
systems widely, even in initrds. Even that systemd uses it - just to
mention that the mdadm patch uses SYSTEMD_READY variable which is on
systemd-based systems only, but anyway... I'm not going to be that picky :)
> If you want mdadm to mess about with the content of the drives you
> should either (a) explicitly tell it to (yes I would like that option
> :-), or (b) do it yourself beforehand - dd if=/dev/zero etc etc.
>
> It does seem weird to me that mdadm spends a lot of effort initialising
> an array and calculating parity blah-di-blah, and you can't tell it to
> just "set everything to zero". But there's no way it should mess about
> with what was there before, without explicitly being told to.
>
> When you're inserting existing drives, you're not creating them -
>> when those device come from factory (they're "created"), they never
>> contain garbage and old data when you buy them.
>
> As Jes says, USB devices rarely come with nothing on them. MS eventually
> learnt their lesson, "doing something" BY DEFAULT with unknown/untrusted
> data was a really stupid idea - it was far too easy to get your system
> "pwned". Here it would be far too easy to trash an array you're trying
> to recover.
Recovery, yes, I agree.
But I suppose that SUCH recovery is less frequent than creating a new md
array. Anyway, let's keep the default to not wipe, if we like it more,
but I'm begging for the mdadm to have an option to have the possibility
to do the wiping on demand so I can choose that when I'm creating a new
fresh MD device and I know for sure I don't care about old stuff that
was written before. And if I choose to not do the wiping, I want mdadm
to generate the extra uevent that's needed to update the udev database
accordingly (e.g. see the wrapper I mentioned above).
--
Peter
^ permalink raw reply
* Re: Fault tolerance with badblocks
From: Reindl Harald @ 2017-05-09 11:48 UTC (permalink / raw)
To: Nix; +Cc: Phil Turmel, Wols Lists, Ravi (Tom) Hale, linux-raid
In-Reply-To: <87fuge5liz.fsf@esperi.org.uk>
Am 09.05.2017 um 13:15 schrieb Nix:
> On 9 May 2017, Reindl Harald said:
>
>> Am 09.05.2017 um 12:28 schrieb Nix:
>>> Honestly, scrubs are looking less and less desirable the more I talk
>>> about them. Massive worry inducers that don't actually spot problems in
>>> any meaningful sense (not even at the level of "there is a problem on
>>> this disk", just "there is a problem on this array")
>>
>> that is your opinion
>>
>> my expierience over years using md-arrays is that *everytime* smartd triggered a alert mail that a drive will fail soon it happened
>> while the scrub was running and so you can replace drives as soon as possible
>
> What, it triggered a SMART warning while a scrub was running which SMART
> long self-tests didn't? That's depressing. You'd think SMART would be
> watching for errors while it's own tests were running!
different time of tests, different access metrics
i guess smarter people like both of us had a reason to develop scrub
instead say "just let the drive do it at it's own
> (Or were you not running any long self-tests? That's at least as risky
> as not scrubbing, IMNSHO.)
no i do both regulary
* smart short self-test daily
* smart long self-test weekly
* raid scrub weekly
and no - doing a long-smart-test daily is not a good solution, the
RAID10 array in my office makes *terrible noises* when the SMART test is
running and after doing this every week the last 6 years (Power_On_Hours
14786, Start_Stop_Count 1597) i would say they are normal but probably
it's not good doing that operations all the time
well, that machine has not lost a single drive, a clone of it acting as
homeserver 365/24/7 has lost a dozen in the same time....
^ permalink raw reply
* Re: Fault tolerance with badblocks
From: Nix @ 2017-05-09 11:27 UTC (permalink / raw)
To: David Brown; +Cc: Anthony Youngman, Phil Turmel, Ravi (Tom) Hale, linux-raid
In-Reply-To: <5911A371.3030008@hesbynett.no>
On 9 May 2017, David Brown uttered the following:
> On 09/05/17 11:53, Nix wrote:
>> This turns out not to be the case. See this ten-year-old paper:
>> <https://indico.cern.ch/event/13797/contributions/1362288/attachments/115080/163419/Data_integrity_v3.pdf>.
>> Five weeks of doing 2GiB writes on 3000 nodes once every two hours
>> found, they estimated, 50 errors possibly attributable to disk problems
>> (sector- or page-size regions of corrupted data) on 1/30th of their
>> nodes. This is *not* rare and it is hard to imagine that 1/30th of disks
>> used by CERN deserve discarding. It is better to assume that drives
>> misdirect writes now and then, and to provide a means of recovering from
>> them that does not take days of panic. RAID-6 gives you that means: md
>> should use it.
>
> RAID-6 does not help here. You have to understand the types of errors
> that can occur, the reasons for them, the possibilities for detection,
> the possibilities for recovery, and what the different layers in the
> system can do about them.
>
> RAID (1/5/6) will let you recover from one or more known failed reads,
> on the assumption that the driver firmware is correct, memories have no
> errors, buses have no errors, block writes are atomic, write ordering
> matches the flush commands, block reads are either correct or marked as
> failed, etc.
I think you're being too pedantic. Many of these things are known not to
be true on real hardware, and at least one of them cannot possibly be
true without a journal (atomic block writes). Nonetheless, the md layer
is quite happy to rebuild after a failed disk even though the write hole
might have torn garbage into your data, on the grounds that it
*probably* did not. If your argument was used everywhere, md would never
have been started because 100% reliability was not guaranteed.
The same, it seems to me, is true of cases in which one drive in a
RAID-6 reports a few mismatched blocks. It is true that you don't know
the cause of the mismatches, but you *do* know which bit of the mismatch
is wrong and what data should be there, subject only to the assumption
that sufficiently few drives have made simultaneous mistakes that
redundancy is preserved. And that's the same assumption RAID >0 makes
all the time anyway!
The only difference in the disk-failure case is that you know that one
drive has failed without needing to ask other drives to be sure. I mean,
yeah, *possibly* in the RAID-6 mismatch case *five* drives have gone
simultaneously wrong in such a way that their syndromes all match and
the one surviving drive is mistakenly misrepaired, but frankly you'd
need to wait for black holes to evaporate of old age before this became
an issue.
(I'm not suggesting repairing RAID-5 mismatches. That's clearly
impossible. You can't even tell what disk is affected. But in the RAID-6
case none of this is impossible, or so it seems to me. You have at least
three and probably four or more drives with consistent syndromes, and
one that is out of whack. You know which one must be wrong -- the
"minority vote" -- and you know what has to be done to make it
consistent with the others again. Why not do it? It's no more risky than
that aspect of a RAID rebuild from a failed disk would be.)
> RAID will /not/ let you reliably detect or correct other sorts of
> errors.
... only it clearly can. What stops it from handling the RAID-6-and-
one-disk-is-wrong case where it cannot handle the RAID-6-and-one-disk-
has-failed case, given that you can unambiguously determine which disk
is wrong using the data on the surviving drives, with an undetected-
failure probability of something way below 2^128? (I could work out the
actual value but I haven't had any coffee yet and it seems pointless
when it's that low.)
> What does /not/ work, however, is trying to squeeze magic capabilities
> out of existing layers in the system, or expecting more out of them that
> they can give.
I don't see that these capabilities are any more magic than what RAID-6
does already. It can recover from two failed drives: why can't it
recover from one wrong one? (Or, rather, from one drive with very
occasionally wrong sectors on it. Obviously if it was always getting
things wrong its presence is not a benefit and you have essentially
fallen back to nothing better than RAID-5, only with worse performance.
But that's what error thresholds are for, which md already employs in
similar situations.)
--
NULL && (void)
^ permalink raw reply
* Re: Fault tolerance with badblocks
From: Nix @ 2017-05-09 11:15 UTC (permalink / raw)
To: Reindl Harald; +Cc: Phil Turmel, Wols Lists, Ravi (Tom) Hale, linux-raid
In-Reply-To: <e03e2dd0-7283-314d-b5d7-b7ffc5a9c039@thelounge.net>
On 9 May 2017, Reindl Harald said:
> Am 09.05.2017 um 12:28 schrieb Nix:
>> Honestly, scrubs are looking less and less desirable the more I talk
>> about them. Massive worry inducers that don't actually spot problems in
>> any meaningful sense (not even at the level of "there is a problem on
>> this disk", just "there is a problem on this array")
>
> that is your opinion
>
> my expierience over years using md-arrays is that *everytime* smartd triggered a alert mail that a drive will fail soon it happened
> while the scrub was running and so you can replace drives as soon as possible
What, it triggered a SMART warning while a scrub was running which SMART
long self-tests didn't? That's depressing. You'd think SMART would be
watching for errors while it's own tests were running!
(Or were you not running any long self-tests? That's at least as risky
as not scrubbing, IMNSHO.)
^ permalink raw reply
* Re: Fault tolerance with badblocks
From: David Brown @ 2017-05-09 11:09 UTC (permalink / raw)
To: Nix, Anthony Youngman; +Cc: Phil Turmel, Ravi (Tom) Hale, linux-raid
In-Reply-To: <87inla73vz.fsf@esperi.org.uk>
On 09/05/17 11:53, Nix wrote:
> On 8 May 2017, Anthony Youngman told this:
>
>> If the scrub finds a mismatch, then the drives are reporting
>> "everything's fine here". Something's gone wrong, but the question is
>> what? If you've got a four-drive raid that reports a mismatch, how do
>> you know which of the four drives is corrupt? Doing an auto-correct
>> here risks doing even more damage. (I think a raid-6 could recover,
>> but raid-5 is toast ...)
>
> With a RAID-5 you are screwed: you can reconstruct the parity but cannot
> tell if it was actually right. You can make things consistent, but not
> correct.
>
> But with a RAID-6 you *do* have enough data to make things correct, with
> precisely the same probability as recovery of a RAID-5 "drive" of length
> a single sector.
No, you don't have enough data to make things correct. You /might/ have
enough data to make a guess what /might/ be right to make things wrong,
but might also be wrong. And you don't have enough data to have the
slightest idea about the probabilities. And you don't have enough data
to know if "fixing" it will help overall, or make things worse if you
accidentally "fix" the wrong block. (See the link I gave on other posts
for details.)
> It seems wrong that not only does md not do this but
> doesn't even tell you which drive made the mistake so you could do the
> millions-of-times-slower process of a manual fail and readdition of the
> drive (or, if you suspect it of being wholly buggered, a manual fail and
> replacement).
>
>> And seeing as drives are pretty much guaranteed (unless something's
>> gone BADLY wrong) to either (a) accurately return the data written, or
>> (b) return a read error, that means a data mismatch indicates
>> something is seriously wrong that is NOTHING to do with the drives.
>
> This turns out not to be the case. See this ten-year-old paper:
> <https://indico.cern.ch/event/13797/contributions/1362288/attachments/115080/163419/Data_integrity_v3.pdf>.
> Five weeks of doing 2GiB writes on 3000 nodes once every two hours
> found, they estimated, 50 errors possibly attributable to disk problems
> (sector- or page-size regions of corrupted data) on 1/30th of their
> nodes. This is *not* rare and it is hard to imagine that 1/30th of disks
> used by CERN deserve discarding. It is better to assume that drives
> misdirect writes now and then, and to provide a means of recovering from
> them that does not take days of panic. RAID-6 gives you that means: md
> should use it.
RAID-6 does not help here. You have to understand the types of errors
that can occur, the reasons for them, the possibilities for detection,
the possibilities for recovery, and what the different layers in the
system can do about them.
RAID (1/5/6) will let you recover from one or more known failed reads,
on the assumption that the driver firmware is correct, memories have no
errors, buses have no errors, block writes are atomic, write ordering
matches the flush commands, block reads are either correct or marked as
failed, etc.
RAID will /not/ let you reliably detect or correct other sorts of
errors. It is designed to cheaply and simply reduce the risk of a
certain class of possible errors - it is not a magic method of stopping
all errors. Similarly, the drive firmware works under certain
assumptions to greatly reduce other sorts of errors (those local to the
block), but not everything. And ECC memory, PCI bus CRCs, and other
such things reduce the risk of other kinds of error.
If you need more error checking or correction, you need different
mechanisms. For example, BTRFS and ZFS will do checksumming on the
filesystem level. They can be combined with raid/duplication to allow
correction on checksum error. And they can usefully build on top of a
normal md raid layer, or use their own raid (with its pros and cons).
Or you can have multiple servers and also track md5 sums of files, with
cross-server scrubbing of the data. There are lots of possibilities,
depending on what you want to get.
What does /not/ work, however, is trying to squeeze magic capabilities
out of existing layers in the system, or expecting more out of them that
they can give.
^ permalink raw reply
* Re: Fault tolerance with badblocks
From: Reindl Harald @ 2017-05-09 10:50 UTC (permalink / raw)
To: Nix, Phil Turmel; +Cc: Wols Lists, Ravi (Tom) Hale, linux-raid
In-Reply-To: <871sry728q.fsf@esperi.org.uk>
Am 09.05.2017 um 12:28 schrieb Nix:
> Honestly, scrubs are looking less and less desirable the more I talk
> about them. Massive worry inducers that don't actually spot problems in
> any meaningful sense (not even at the level of "there is a problem on
> this disk", just "there is a problem on this array")
that is your opinion
my expierience over years using md-arrays is that *everytime* smartd
triggered a alert mail that a drive will fail soon it happened while the
scrub was running and so you can replace drives as soon as possible
^ permalink raw reply
* Re: Fault tolerance with badblocks
From: Nix @ 2017-05-09 10:40 UTC (permalink / raw)
To: Brad Campbell; +Cc: David Brown, Wols Lists, Ravi (Tom) Hale, linux-raid
In-Reply-To: <9306fd68-4eeb-f487-a500-27050050af98@fnarfbargle.com>
On 9 May 2017, Brad Campbell stated:
> On 09/05/17 17:58, Nix wrote:
>> md doesn't repair the corruption, even though on RAID-6 it could.
>
> Patches are *always* welcome.
Oh good. I might well look at that.
>> but one wonders how many disks actually *do* this. It's hard to
>> tell because sector sparing is so quiet: it's not always even reflected
>> in the SMART data, AIUI.
>
> Decent SAS drvies do it routinely *and* they tell you in the SMART
> data how long it has been since the last scrub, how long it is until
> the next scrub and how many errors it has silently corrected over the
> drive life. You get what you pay for.
Enterprise SATA drives appear similar except that they don't do the
scrubbing automatically: you have to trigger a SMART self-test. (I'm
wondering if that's enough, and perhaps I can ignore RAID scrubbing
entirely, except that if something *does* go wrong I won't know.)
Of course I haven't yet owned a drive that has ever deigned to give a
nonzero sector-sparing value in any of its SMART info, and I've been
using allegedly-enterprise drives (first SCSI, then SATA) for about
fifteen years now. I've had disk failures without warning, and
non-failed disks with both read and write errors that would not go away,
but that SMART reallocation value just stayed stuck at zero through all
of it. I'm wondering if smartctl is even reading the right field, but
it's hard to imagine how it couldn't be...
^ permalink raw reply
* Re: Fault tolerance with badblocks
From: Nix @ 2017-05-09 10:28 UTC (permalink / raw)
To: Phil Turmel; +Cc: Wols Lists, Ravi (Tom) Hale, linux-raid
In-Reply-To: <e2196f02-2b94-8afb-06a0-9695d441c890@turmel.org>
On 8 May 2017, Phil Turmel said:
> On 05/08/2017 03:52 PM, Nix wrote:
>> And... then what do you do? On RAID-6, it appears the answer is "live
>> with a high probability of inevitable corruption".
>
> No, you investigate the quality of your data and the integrity of the
> rest of the system, as something *other* than a drive problem caused the
> mismatch. (Swap is a known exception, though.)
Yeah, I'm going to "rely" on the fact that this machine has heaps of
memory and won't be swapping much when it does a RAID scrub. :)
But "you investigate the quality of your data"... so now, on a single
mismatch that won't go away, I have to compare all my data with backups,
taking countless hours and emitting heaps of spurious errors because no
backup is ever quite up to date? Those backups *live* on hard drives, so
it has exactly the same chance of spurious disk-layer errors as the
thing that preceded it (quite possibly higher).
Honestly, scrubs are looking less and less desirable the more I talk
about them. Massive worry inducers that don't actually spot problems in
any meaningful sense (not even at the level of "there is a problem on
this disk", just "there is a problem on this array").
>> That's not very good.
>> (AIUI, if a check scrub finds a URE, it'll rewrite it, and when in the
>> common case the drive spares it out and the write succeeds, this will
>> not be reported as a mismatch: is this right?)
>
> This is also wrong, because you are assuming sparing-out is the common
> case. A read error does not automatically trigger relocation. It
> triggers *verification* of the next *write*. In young drives,
So I guess we only need to worry about mismatches if they don't go away
and are persistently in the same place on the same drive. (Only you
can't tell what place that is, or what drive that is, because md doesn't
tell you. I'm really tempted to fix *that* at least, a printk() or
something.)
> { Drive self tests might do some pre-emptive rewriting of marginal
> sectors -- it's not something drive manufacturers are documenting. But
> a drive self-test cannot fix an unreadable sector -- it doesn't know
> what to write there. }
Agreed.
>>> This is actually counterproductive. Rewriting everything may refresh
>>> the magnetism on weakening sectors, but will also prevent the drive from
>>> *finding* weakening sectors that really do need relocation.
>>
>> If a sector weakens purely because of neighbouring writes or temperature
>> or a vibrating housing or something (i.e. not because of actual damage),
>> so that a rewrite will strengthen it and relocation was never necessary,
>> surely you've just saved a pointless bit of sector sparing? (I don't
>> know: I'm not sure what the relative frequency of these things is. Read
>> and write errors in general are so rare that it's quite possible I'm
>> worrying about nothing at all. I do know I forgot to scrub my old
>> hardware RAID array for about three years and nothing bad happened...)
>
> Drives that are in applications that get *read* pretty often don't need
> much if any scrubbing -- the application itself will expose problem
> sectors. Hobbyists and home media servers can go months with specific
> files unread, so developing problems can hit in clusters. Regular
> scrubbing will catch these problems before they take your array down.
Yeah, and I have plenty of archival data on this array -- it's the first
one I've ever had that's big enough to consider using for that as well
as for frequently-used stuff whose integrity I care about. (But even the
frequently-read stuff is bcached, so even that is in effect archival
much of the time, from the perspective of its read.)
> And you can't compare hardware array behavior to MD -- they have their
> own algorithms to take care of attached disks without OS intervention.
I don't see what the difference is between a hardware array controller
with its own noddy OS, barely-maintained software, creaking processor,
and not very big battery-backed RAM and md with a decent OS, much faster
processor, decent software, and often masses of RAM and a journal on
SSD, except that the md array will be far faster and if anything goes
wrong you have much higher chance of actually getting your data back
with md. :)
The days of saying "hardware arrays are just different/better, md cannot
compete with them" are many years in the past. People are *replacing*
hardware arrays with md these days because the hardware arrays are
*worse* on almost every metric. If hardware arrays have magic recovery
algorithms that md and/or the Linux block layer don't, the question now
is why not? not "oh we cannot compare"
^ permalink raw reply
* Re: Fault tolerance with badblocks
From: Brad Campbell @ 2017-05-09 10:28 UTC (permalink / raw)
To: Nix, David Brown; +Cc: Wols Lists, Ravi (Tom) Hale, linux-raid
In-Reply-To: <87efvy73n9.fsf@esperi.org.uk>
On 09/05/17 17:58, Nix wrote:
> md doesn't repair the corruption, even though on RAID-6 it could.
Patches are *always* welcome.
> but one wonders how many disks actually *do* this. It's hard to
> tell because sector sparing is so quiet: it's not always even reflected
> in the SMART data, AIUI.
Decent SAS drvies do it routinely *and* they tell you in the SMART data
how long it has been since the last scrub, how long it is until the next
scrub and how many errors it has silently corrected over the drive life.
You get what you pay for.
Brad
^ permalink raw reply
* [PATCH 5/5] Add sector size as spare selection criterion
From: Tomasz Majchrzak @ 2017-05-09 10:25 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, Alexey Obitotskiy, Tomasz Majchrzak
In-Reply-To: <1494325547-3028-1-git-send-email-tomasz.majchrzak@intel.com>
From: Alexey Obitotskiy <aleksey.obitotskiy@intel.com>
Add sector size as new spare selection criterion. Assume that 0 means
there is no requirement for the sector size in the array. Skip disks
with unsuitable sector size when looking for a spare to move across
containers.
Signed-off-by: Alexey Obitotskiy <aleksey.obitotskiy@intel.com>
Signed-off-by: Tomasz Majchrzak <tomasz.majchrzak@intel.com>
---
Incremental.c | 4 ++--
Monitor.c | 8 ++++++++
mdadm.h | 3 +++
super-intel.c | 2 ++
util.c | 31 ++++++++++++++++++++++++++++++-
5 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/Incremental.c b/Incremental.c
index fe9d644..30dc7a2 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -867,7 +867,7 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
struct domainlist *dl = NULL;
struct mdinfo *sra;
unsigned long long devsize;
- struct spare_criteria sc = {0};
+ struct spare_criteria sc = {0, 0};
if (is_subarray(mp->metadata))
continue;
@@ -1627,7 +1627,7 @@ static int Incremental_container(struct supertype *st, char *devname,
struct mdinfo *sinfo;
if (!sst->ss->load_container(sst, sfd, NULL)) {
- struct spare_criteria sc = {0};
+ struct spare_criteria sc = {0, 0};
if (st->ss->get_spare_criteria)
st->ss->get_spare_criteria(st, &sc);
diff --git a/Monitor.c b/Monitor.c
index 4a7afd7..288f8cc 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -755,6 +755,7 @@ static int get_required_spare_criteria(struct state *st,
if (!st->metadata ||
!st->metadata->ss->get_spare_criteria) {
sc->min_size = 0;
+ sc->sector_size = 0;
return 0;
}
@@ -811,6 +812,7 @@ static dev_t choose_spare(struct state *from, struct state *to,
from->devstate[d] == 0) {
struct dev_policy *pol;
unsigned long long dev_size;
+ unsigned int dev_sector_size;
if (to->metadata->ss->external &&
test_partition_from_id(from->devid[d]))
@@ -821,6 +823,12 @@ static dev_t choose_spare(struct state *from, struct state *to,
dev_size < sc->min_size)
continue;
+ if (sc->sector_size &&
+ dev_sector_size_from_id(from->devid[d],
+ &dev_sector_size) &&
+ sc->sector_size != dev_sector_size)
+ continue;
+
pol = devid_policy(from->devid[d]);
if (from->spare_group)
pol_add(&pol, pol_domain,
diff --git a/mdadm.h b/mdadm.h
index 8da7fd3..ec0a39e 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -363,6 +363,7 @@ struct createinfo {
struct spare_criteria {
unsigned long long min_size;
+ unsigned int sector_size;
};
enum mode {
@@ -947,6 +948,7 @@ extern struct superswitch {
/*
* Return spare criteria for array:
* - minimum disk size can be used in array;
+ * - sector size can be used in array.
* Return values: 0 - for success and -EINVAL on error.
*/
int (*get_spare_criteria)(struct supertype *st,
@@ -1189,6 +1191,7 @@ extern int get_dev_size(int fd, char *dname, unsigned long long *sizep);
extern int get_dev_sector_size(int fd, char *dname, unsigned int *sectsizep);
extern int must_be_container(int fd);
extern int dev_size_from_id(dev_t id, unsigned long long *size);
+extern int dev_sector_size_from_id(dev_t id, unsigned int *size);
void wait_for(char *dev, int fd);
/*
diff --git a/super-intel.c b/super-intel.c
index be973f8..ba6f810 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1396,6 +1396,7 @@ int get_spare_criteria_imsm(struct supertype *st, struct spare_criteria *c)
unsigned long long size = 0;
c->min_size = 0;
+ c->sector_size = 0;
if (!super)
return -EINVAL;
@@ -1419,6 +1420,7 @@ int get_spare_criteria_imsm(struct supertype *st, struct spare_criteria *c)
size += imsm_min_reserved_sectors(super);
c->min_size = size * 512;
+ c->sector_size = super->sector_size;
return 0;
}
diff --git a/util.c b/util.c
index 8b3c67d..fc9cd3f 100644
--- a/util.c
+++ b/util.c
@@ -1265,6 +1265,23 @@ int dev_size_from_id(dev_t id, unsigned long long *size)
return 0;
}
+int dev_sector_size_from_id(dev_t id, unsigned int *size)
+{
+ char buf[20];
+ int fd;
+
+ sprintf(buf, "%d:%d", major(id), minor(id));
+ fd = dev_open(buf, O_RDONLY);
+ if (fd < 0)
+ return 0;
+ if (get_dev_sector_size(fd, NULL, size)) {
+ close(fd);
+ return 1;
+ }
+ close(fd);
+ return 0;
+}
+
struct supertype *dup_super(struct supertype *orig)
{
struct supertype *st;
@@ -2129,12 +2146,24 @@ struct mdinfo *container_choose_spares(struct supertype *st,
if (d->disk.state == 0) {
/* check if size is acceptable */
unsigned long long dev_size;
+ unsigned int dev_sector_size;
+ int size_valid = 0;
+ int sector_size_valid = 0;
+
dev_t dev = makedev(d->disk.major,d->disk.minor);
if (!criteria->min_size ||
(dev_size_from_id(dev, &dev_size) &&
dev_size >= criteria->min_size))
- found = 1;
+ size_valid = 1;
+
+ if (!criteria->sector_size ||
+ (dev_sector_size_from_id(dev, &dev_sector_size) &&
+ criteria->sector_size == dev_sector_size))
+ sector_size_valid = 1;
+
+ found = size_valid && sector_size_valid;
+
/* check if domain matches */
if (found && domlist) {
struct dev_policy *pol = devid_policy(dev);
--
1.8.3.1
^ permalink raw reply related
* [PATCH 4/5] Allow more spare selection criteria
From: Tomasz Majchrzak @ 2017-05-09 10:25 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, Alexey Obitotskiy, Tomasz Majchrzak
In-Reply-To: <1494325547-3028-1-git-send-email-tomasz.majchrzak@intel.com>
From: Alexey Obitotskiy <aleksey.obitotskiy@intel.com>
Disks can be moved across containers in order to be used as a spare
drive for reubild. At the moment the only requirement checked for such
disk is its size (if it matches donor expectations). In order to
introduce more criteria rename corresponding superswitch method to more
generic name and move function parameter to a structure. This change is
a big edit but it doesn't introduce any changes in code logic, it just
updates function naming and parameters.
Signed-off-by: Alexey Obitotskiy <aleksey.obitotskiy@intel.com>
Signed-off-by: Tomasz Majchrzak <tomasz.majchrzak@intel.com>
---
Incremental.c | 18 +++++++++++-------
Monitor.c | 30 ++++++++++++++++--------------
mdadm.h | 16 +++++++++++-----
super-intel.c | 33 +++++++++++++++++++++------------
util.c | 6 +++---
5 files changed, 62 insertions(+), 41 deletions(-)
diff --git a/Incremental.c b/Incremental.c
index 680d318..fe9d644 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -867,7 +867,7 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
struct domainlist *dl = NULL;
struct mdinfo *sra;
unsigned long long devsize;
- unsigned long long component_size = 0;
+ struct spare_criteria sc = {0};
if (is_subarray(mp->metadata))
continue;
@@ -936,7 +936,8 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
}
if (st3->ss->load_container &&
!st3->ss->load_container(st3, mdfd, mp->path)) {
- component_size = st3->ss->min_acceptable_spare_size(st3);
+ if (st3->ss->get_spare_criteria)
+ st3->ss->get_spare_criteria(st3, &sc);
st3->ss->free_super(st3);
}
free(st3);
@@ -947,7 +948,7 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
sra->devs ? sra->devs->data_offset :
INVALID_SECTORS) <
sra->component_size) ||
- (sra->component_size == 0 && devsize < component_size)) {
+ (sra->component_size == 0 && devsize < sc.min_size)) {
if (verbose > 1)
pr_err("not adding %s to %s as it is too small\n",
devname, mp->path);
@@ -1624,12 +1625,15 @@ static int Incremental_container(struct supertype *st, char *devname,
struct supertype *sst =
super_imsm.match_metadata_desc("imsm");
struct mdinfo *sinfo;
- unsigned long long min_size = 0;
- if (st->ss->min_acceptable_spare_size)
- min_size = st->ss->min_acceptable_spare_size(st);
+
if (!sst->ss->load_container(sst, sfd, NULL)) {
+ struct spare_criteria sc = {0};
+
+ if (st->ss->get_spare_criteria)
+ st->ss->get_spare_criteria(st, &sc);
+
close(sfd);
- sinfo = container_choose_spares(sst, min_size,
+ sinfo = container_choose_spares(sst, &sc,
domains, NULL,
st->ss->name, 0);
sst->ss->free_super(sst);
diff --git a/Monitor.c b/Monitor.c
index e2b36ff..4a7afd7 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -747,13 +747,14 @@ static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist,
return new_found;
}
-static int get_min_spare_size_required(struct state *st, unsigned long long *sizep)
+static int get_required_spare_criteria(struct state *st,
+ struct spare_criteria *sc)
{
int fd;
if (!st->metadata ||
- !st->metadata->ss->min_acceptable_spare_size) {
- *sizep = 0;
+ !st->metadata->ss->get_spare_criteria) {
+ sc->min_size = 0;
return 0;
}
@@ -767,7 +768,8 @@ static int get_min_spare_size_required(struct state *st, unsigned long long *siz
close(fd);
if (!st->metadata->sb)
return 1;
- *sizep = st->metadata->ss->min_acceptable_spare_size(st->metadata);
+
+ st->metadata->ss->get_spare_criteria(st->metadata, sc);
st->metadata->ss->free_super(st->metadata);
return 0;
@@ -799,7 +801,7 @@ static int check_donor(struct state *from, struct state *to)
}
static dev_t choose_spare(struct state *from, struct state *to,
- struct domainlist *domlist, unsigned long long min_size)
+ struct domainlist *domlist, struct spare_criteria *sc)
{
int d;
dev_t dev = 0;
@@ -814,9 +816,9 @@ static dev_t choose_spare(struct state *from, struct state *to,
test_partition_from_id(from->devid[d]))
continue;
- if (min_size &&
+ if (sc->min_size &&
dev_size_from_id(from->devid[d], &dev_size) &&
- dev_size < min_size)
+ dev_size < sc->min_size)
continue;
pol = devid_policy(from->devid[d]);
@@ -833,7 +835,7 @@ static dev_t choose_spare(struct state *from, struct state *to,
static dev_t container_choose_spare(struct state *from, struct state *to,
struct domainlist *domlist,
- unsigned long long min_size, int active)
+ struct spare_criteria *sc, int active)
{
/* This is similar to choose_spare, but we cannot trust devstate,
* so we need to read the metadata instead
@@ -884,7 +886,7 @@ static dev_t container_choose_spare(struct state *from, struct state *to,
}
/* We only need one spare so full list not needed */
- list = container_choose_spares(st, min_size, domlist, from->spare_group,
+ list = container_choose_spares(st, sc, domlist, from->spare_group,
to->metadata->ss->name, 1);
if (list) {
struct mdinfo *disks = list->devs;
@@ -900,6 +902,7 @@ static void try_spare_migration(struct state *statelist, struct alert_info *info
{
struct state *from;
struct state *st;
+ struct spare_criteria sc;
link_containers_with_subarrays(statelist);
for (st = statelist; st; st = st->next)
@@ -908,7 +911,6 @@ static void try_spare_migration(struct state *statelist, struct alert_info *info
struct domainlist *domlist = NULL;
int d;
struct state *to = st;
- unsigned long long min_size;
if (to->parent_devnm[0] && !to->parent)
/* subarray monitored without parent container
@@ -919,14 +921,14 @@ static void try_spare_migration(struct state *statelist, struct alert_info *info
/* member of a container */
to = to->parent;
- if (get_min_spare_size_required(to, &min_size))
+ if (get_required_spare_criteria(to, &sc))
continue;
if (to->metadata->ss->external) {
/* We must make sure there is
* no suitable spare in container already.
* If there is we don't add more */
dev_t devid = container_choose_spare(
- to, to, NULL, min_size, st->active);
+ to, to, NULL, &sc, st->active);
if (devid > 0)
continue;
}
@@ -949,10 +951,10 @@ static void try_spare_migration(struct state *statelist, struct alert_info *info
continue;
if (from->metadata->ss->external)
devid = container_choose_spare(
- from, to, domlist, min_size, 0);
+ from, to, domlist, &sc, 0);
else
devid = choose_spare(from, to, domlist,
- min_size);
+ &sc);
if (devid > 0
&& move_spare(from->devname, to->devname, devid)) {
alert("MoveSpare", to->devname, from->devname, info);
diff --git a/mdadm.h b/mdadm.h
index a92feb2..8da7fd3 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -361,6 +361,10 @@ struct createinfo {
struct supertype *supertype;
};
+struct spare_criteria {
+ unsigned long long min_size;
+};
+
enum mode {
ASSEMBLE=1,
BUILD,
@@ -940,11 +944,13 @@ extern struct superswitch {
*/
__u64 (*avail_size)(struct supertype *st, __u64 size,
unsigned long long data_offset);
- /* This is similar to 'avail_size' in purpose, but is used for
- * containers for which there is no 'component size' to compare.
- * This reports that whole-device size which is a minimum
+ /*
+ * Return spare criteria for array:
+ * - minimum disk size can be used in array;
+ * Return values: 0 - for success and -EINVAL on error.
*/
- unsigned long long (*min_acceptable_spare_size)(struct supertype *st);
+ int (*get_spare_criteria)(struct supertype *st,
+ struct spare_criteria *sc);
/* Find somewhere to put a bitmap - possibly auto-size it - and
* update the metadata to record this. The array may be newly
* created, in which case data_size may be updated, or it might
@@ -1507,7 +1513,7 @@ extern int assemble_container_content(struct supertype *st, int mdfd,
#define INCR_ALREADY 4
#define INCR_YES 8
extern struct mdinfo *container_choose_spares(struct supertype *st,
- unsigned long long min_size,
+ struct spare_criteria *criteria,
struct domainlist *domlist,
char *spare_group,
const char *metadata, int get_one);
diff --git a/super-intel.c b/super-intel.c
index e88fe82..be973f8 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1383,37 +1383,44 @@ static __u32 imsm_min_reserved_sectors(struct intel_super *super)
return (remainder < rv) ? remainder : rv;
}
-/* Return minimum size of a spare that can be used in this array*/
-static unsigned long long min_acceptable_spare_size_imsm(struct supertype *st)
+/*
+ * Return minimum size of a spare and sector size
+ * that can be used in this array
+ */
+int get_spare_criteria_imsm(struct supertype *st, struct spare_criteria *c)
{
struct intel_super *super = st->sb;
struct dl *dl;
struct extent *e;
int i;
- unsigned long long rv = 0;
+ unsigned long long size = 0;
+
+ c->min_size = 0;
if (!super)
- return rv;
+ return -EINVAL;
/* find first active disk in array */
dl = super->disks;
while (dl && (is_failed(&dl->disk) || dl->index == -1))
dl = dl->next;
if (!dl)
- return rv;
+ return -EINVAL;
/* find last lba used by subarrays */
e = get_extents(super, dl);
if (!e)
- return rv;
+ return -EINVAL;
for (i = 0; e[i].size; i++)
continue;
if (i > 0)
- rv = e[i-1].start + e[i-1].size;
+ size = e[i-1].start + e[i-1].size;
free(e);
/* add the amount of space needed for metadata */
- rv = rv + imsm_min_reserved_sectors(super);
+ size += imsm_min_reserved_sectors(super);
+
+ c->min_size = size * 512;
- return rv * 512;
+ return 0;
}
static int is_gen_migration(struct imsm_dev *dev);
@@ -10817,8 +10824,10 @@ static int imsm_reshape_is_allowed_on_container(struct supertype *st,
*/
static struct mdinfo *get_spares_for_grow(struct supertype *st)
{
- unsigned long long min_size = min_acceptable_spare_size_imsm(st);
- return container_choose_spares(st, min_size, NULL, NULL, NULL, 0);
+ struct spare_criteria sc;
+
+ get_spare_criteria_imsm(st, &sc);
+ return container_choose_spares(st, &sc, NULL, NULL, NULL, 0);
}
/******************************************************************************
@@ -11853,7 +11862,7 @@ struct superswitch super_imsm = {
.update_super = update_super_imsm,
.avail_size = avail_size_imsm,
- .min_acceptable_spare_size = min_acceptable_spare_size_imsm,
+ .get_spare_criteria = get_spare_criteria_imsm,
.compare_super = compare_super_imsm,
diff --git a/util.c b/util.c
index 11ff2cc..8b3c67d 100644
--- a/util.c
+++ b/util.c
@@ -2107,7 +2107,7 @@ int experimental(void)
* if spare_group given add it to domains of each spare
* metadata allows to test domains using metadata of destination array */
struct mdinfo *container_choose_spares(struct supertype *st,
- unsigned long long min_size,
+ struct spare_criteria *criteria,
struct domainlist *domlist,
char *spare_group,
const char *metadata, int get_one)
@@ -2131,9 +2131,9 @@ struct mdinfo *container_choose_spares(struct supertype *st,
unsigned long long dev_size;
dev_t dev = makedev(d->disk.major,d->disk.minor);
- if (!min_size ||
+ if (!criteria->min_size ||
(dev_size_from_id(dev, &dev_size) &&
- dev_size >= min_size))
+ dev_size >= criteria->min_size))
found = 1;
/* check if domain matches */
if (found && domlist) {
--
1.8.3.1
^ permalink raw reply related
* [PATCH 3/5] imsm: don't allow disks with different sector size in one array
From: Tomasz Majchrzak @ 2017-05-09 10:25 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, Alexey Obitotskiy, Tomasz Majchrzak
In-Reply-To: <1494325547-3028-1-git-send-email-tomasz.majchrzak@intel.com>
From: Alexey Obitotskiy <aleksey.obitotskiy@intel.com>
As there is no support in IMSM for arrays including disks with different
sector sizes, don't allow to create such configuration. Also skip the
disk with unsuitable sector size when looking for spares in the same
container.
Signed-off-by: Alexey Obitotskiy <aleksey.obitotskiy@intel.com>
Signed-off-by: Tomasz Majchrzak <tomasz.majchrzak@intel.com>
---
super-intel.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/super-intel.c b/super-intel.c
index cfb10d5..e88fe82 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -5468,6 +5468,22 @@ static int init_super_imsm(struct supertype *st, mdu_array_info_t *info,
return 1;
}
+static int drive_validate_sector_size(struct intel_super *super, struct dl *dl)
+{
+ unsigned int member_sector_size;
+
+ if (dl->fd < 0) {
+ pr_err("Invalid file descriptor for %s\n", dl->devname);
+ return 0;
+ }
+
+ if (!get_dev_sector_size(dl->fd, dl->devname, &member_sector_size))
+ return 0;
+ if (member_sector_size != super->sector_size)
+ return 0;
+ return 1;
+}
+
static int add_to_super_imsm_volume(struct supertype *st, mdu_disk_info_t *dk,
int fd, char *devname)
{
@@ -5507,6 +5523,11 @@ static int add_to_super_imsm_volume(struct supertype *st, mdu_disk_info_t *dk,
return 1;
}
+ if (!drive_validate_sector_size(super, dl)) {
+ pr_err("Combining drives of different sector size in one volume is not allowed\n");
+ return 1;
+ }
+
/* add a pristine spare to the metadata */
if (dl->index < 0) {
dl->index = super->anchor->num_disks;
@@ -8440,6 +8461,9 @@ static struct dl *imsm_add_spare(struct intel_super *super, int slot,
if (dl->index == -1 && !activate_new)
continue;
+ if (!drive_validate_sector_size(super, dl))
+ continue;
+
/* Does this unused device have the requisite free space?
* It needs to be able to cover all member volumes
*/
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/5] imsm: allocate buffer to support maximum sector size
From: Tomasz Majchrzak @ 2017-05-09 10:25 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, Alexey Obitotskiy, Tomasz Majchrzak
In-Reply-To: <1494325547-3028-1-git-send-email-tomasz.majchrzak@intel.com>
From: Alexey Obitotskiy <aleksey.obitotskiy@intel.com>
Allocate migration record buffer to support maximum sector size. Disk with
non-matching sector size is not going to be included in the array, however
some preparation/cleanup actions still take place on it and they would
cause a crash. Clear migration record using sector size of the disk (not
array) as they might not match.
Signed-off-by: Alexey Obitotskiy <aleksey.obitotskiy@intel.com>
Signed-off-by: Tomasz Majchrzak <tomasz.majchrzak@intel.com>
---
super-intel.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 2a5d848..cfb10d5 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -4229,8 +4229,8 @@ static int load_imsm_mpb(int fd, struct intel_super *super, char *devname)
sectors = mpb_sectors(anchor, sector_size) - 1;
free(anchor);
- if (posix_memalign(&super->migr_rec_buf, sector_size,
- MIGR_REC_BUF_SECTORS*sector_size) != 0) {
+ if (posix_memalign(&super->migr_rec_buf, MAX_SECTOR_SIZE,
+ MIGR_REC_BUF_SECTORS*MAX_SECTOR_SIZE) != 0) {
pr_err("could not allocate migr_rec buffer\n");
free(super->buf);
return 2;
@@ -5258,8 +5258,9 @@ static int init_super_imsm_volume(struct supertype *st, mdu_array_info_t *info,
pr_err("could not allocate new mpb\n");
return 0;
}
- if (posix_memalign(&super->migr_rec_buf, sector_size,
- MIGR_REC_BUF_SECTORS*sector_size) != 0) {
+ if (posix_memalign(&super->migr_rec_buf, MAX_SECTOR_SIZE,
+ MIGR_REC_BUF_SECTORS*
+ MAX_SECTOR_SIZE) != 0) {
pr_err("could not allocate migr_rec buffer\n");
free(super->buf);
free(super);
@@ -5719,12 +5720,12 @@ static int add_to_super_imsm(struct supertype *st, mdu_disk_info_t *dk,
}
/* clear migr_rec when adding disk to container */
- memset(super->migr_rec_buf, 0, MIGR_REC_BUF_SECTORS*super->sector_size);
- if (lseek64(fd, size - MIGR_REC_SECTOR_POSITION*super->sector_size,
+ memset(super->migr_rec_buf, 0, MIGR_REC_BUF_SECTORS*MAX_SECTOR_SIZE);
+ if (lseek64(fd, size - MIGR_REC_SECTOR_POSITION*member_sector_size,
SEEK_SET) >= 0) {
if ((unsigned int)write(fd, super->migr_rec_buf,
- MIGR_REC_BUF_SECTORS*super->sector_size) !=
- MIGR_REC_BUF_SECTORS*super->sector_size)
+ MIGR_REC_BUF_SECTORS*member_sector_size) !=
+ MIGR_REC_BUF_SECTORS*member_sector_size)
perror("Write migr_rec failed");
}
@@ -5916,7 +5917,7 @@ static int write_super_imsm(struct supertype *st, int doclose)
}
if (clear_migration_record)
memset(super->migr_rec_buf, 0,
- MIGR_REC_BUF_SECTORS*sector_size);
+ MIGR_REC_BUF_SECTORS*MAX_SECTOR_SIZE);
if (sector_size == 4096)
convert_to_4k(super);
@@ -11770,7 +11771,7 @@ static int imsm_manage_reshape(
/* clear migr_rec on disks after successful migration */
struct dl *d;
- memset(super->migr_rec_buf, 0, MIGR_REC_BUF_SECTORS*sector_size);
+ memset(super->migr_rec_buf, 0, MIGR_REC_BUF_SECTORS*MAX_SECTOR_SIZE);
for (d = super->disks; d; d = d->next) {
if (d->index < 0 || is_failed(&d->disk))
continue;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 1/5] imsm: allow drives in a container regardless of sector size
From: Tomasz Majchrzak @ 2017-05-09 10:25 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, Alexey Obitotskiy, Tomasz Majchrzak
In-Reply-To: <1494325547-3028-1-git-send-email-tomasz.majchrzak@intel.com>
From: Alexey Obitotskiy <aleksey.obitotskiy@intel.com>
IMSM doesn't allow to create arrays including drives with different
sector sizes. The initial idea was not to permit to combine drives
with different sector size in the same container. The problem is it
only worked for array creation. On array assemble there are no
calls to metadata handlers to see if drive is suitable for a container
(e.g. as a spare) and it leads to wrong configuration.
Revert the change and allow adding drives with different sector size
to the container.
Signed-off-by: Alexey Obitotskiy <aleksey.obitotskiy@intel.com>
Signed-off-by: Tomasz Majchrzak <tomasz.majchrzak@intel.com>
---
super-intel.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index e13c940..2a5d848 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -5716,12 +5716,6 @@ static int add_to_super_imsm(struct supertype *st, mdu_disk_info_t *dk,
if (super->sector_size == 0) {
/* this a first device, so sector_size is not set yet */
super->sector_size = member_sector_size;
- } else if (member_sector_size != super->sector_size) {
- pr_err("Mixing between different sector size is forbidden, aborting...\n");
- if (dd->devname)
- free(dd->devname);
- free(dd);
- return 1;
}
/* clear migr_rec when adding disk to container */
--
1.8.3.1
^ permalink raw reply related
* [PATCH 0/5] Don't combine disks with different sector size in the array
From: Tomasz Majchrzak @ 2017-05-09 10:25 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, Tomasz Majchrzak
IMSM doesn't allow to create arrays including drives with different sector
sizes.
Following patchset blocks all paths to create such configuration:
* explicit array create
* array rebuild (spare in the same container, spare moved across containers)
* array grow
Alexey Obitotskiy (5):
imsm: allow drives in a container regardless of sector size
imsm: allocate buffer to support maximum sector size
imsm: don't allow disks with different sector size in one array
Allow more spare selection criteria
Add sector size as spare selection criterion
Incremental.c | 18 ++++++++-----
Monitor.c | 38 ++++++++++++++++----------
mdadm.h | 19 +++++++++----
super-intel.c | 86 ++++++++++++++++++++++++++++++++++++++++-------------------
util.c | 37 ++++++++++++++++++++++---
5 files changed, 140 insertions(+), 58 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: Fault tolerance with badblocks
From: Nix @ 2017-05-09 10:18 UTC (permalink / raw)
To: Anthony Youngman; +Cc: Ravi (Tom) Hale, linux-raid
In-Reply-To: <17fe9ff3-1096-8303-a228-e910a77d8146@youngman.org.uk>
On 8 May 2017, Anthony Youngman verbalised:
> On 08/05/17 15:50, Nix wrote:
>> I wonder... scrubbing is not very useful with md, particularly with RAID
>> 6, because it does no writes unless something mismatches, and on failure
>> there is no attempt to determine which of the N disks is bad and rewrite
>> its contents from the other devices (nor, as I understand it, does it
>> clearly say which drive gave the error, so even failing it out and
>> resyncing it is hard).
>
> With redundant raid (and that doesn't include a two-disk, or even
> three-disk mirror), it SHOULD recalculate the failed block. If it
> doesn't bother even though it can, I'd call that a bug in scrub. What
It didn't, once upon a time (in 2010), and as far as I can tell from the
code it still doesn't.
> I thought happened was that it reads a stripe direct from disk, and if
> that failed it read the same stripe via the raid code, to get the raid
> error correction to fire, and then it rewrote the stripe.
There's *failed*, which does trigger a rewrite, and there's 'we got a
mismatch', which on RAID-6 arguably should trigger a rewrite but instead
just tells you there was a mismatch, but not where, nor even on what
disk.
> What would be a nice touch, is that if we have a massive timeout for
> non-SCT drives, if the scrub has to wait more than, say, 10 seconds
> for a read to succeed it then assumes the block is failing and
> rewrites it.
What tends to happen is that the drive gets reset, which from md's
perspective is the drive vanishing and reappearing again. I don't see
any sane way for md to interpret *that* as anything but a possibly
rather major failure that should be reacted to by failing the drive out.
I mean, all it knows is there was a timeout: for all it knows there are
electrical problems there or something. The drive doesn't say (and
doesn't get a chance to say, because we reset it rather than wait five
minutes for it to tell us what's up).
> Actually, scrub that (groan... :-) - if the drive takes
> longer than 1/3 of the timeout to respond, then the scrub assumes it's
> dodgy and rewrites it.
It's hard to rewrite anything on a drive that's too busy failing a read
to do anything else.
--
NULL && (void)
^ permalink raw reply
* Re: Fault tolerance with badblocks
From: David Brown @ 2017-05-09 10:11 UTC (permalink / raw)
To: Anthony Youngman, Nix; +Cc: Ravi (Tom) Hale, linux-raid
In-Reply-To: <17fe9ff3-1096-8303-a228-e910a77d8146@youngman.org.uk>
On 08/05/17 20:00, Anthony Youngman wrote:
> With redundant raid (and that doesn't include a two-disk, or even
> three-disk mirror), it SHOULD recalculate the failed block. If it
> doesn't bother even though it can, I'd call that a bug in scrub.
Please read:
<http://neil.brown.name/blog/20100211050355>
> What I
> thought happened was that it reads a stripe direct from disk, and if
> that failed it read the same stripe via the raid code, to get the raid
> error correction to fire, and then it rewrote the stripe.
That /is/ what happens.
As I mentioned in another reply, /reading/ is enough to trigger a
re-write on the disk if significant /correctable/ errors are discovered
by the disk's firmware. It is extremely rare that the raid level will
see an error (see the linked article by Neil Brown) - usually, the raid
level sees a missing block because the disk firmware could not read the
block correctly. In such cases, the raid software will write the
correct data back to the disk at the same logical block, and the disk
firmware will re-map it to a different block.
>
> What would be a nice touch, is that if we have a massive timeout for
> non-SCT drives, if the scrub has to wait more than, say, 10 seconds for
> a read to succeed it then assumes the block is failing and rewrites it.
I don't think the raid level can do that - it must wait for the drive to
finish handling the read request, or drop the drive entirely.
If the disk takes a long time to read a block, then it will either fail
and mark the block bad, or it will get the data off the disk and then
automatically re-write the data to a re-mapped block. The scrub can
therefore handle it like any other read.
> Actually, scrub that (groan... :-) - if the drive takes longer than 1/3
> of the timeout to respond, then the scrub assumes it's dodgy and
> rewrites it.
>>
>> If there was a way to get md to *rewrite* everything during scrub,
>> rather than just checking, this might help (in addition to letting the
>> drive refresh the magnetization of absolutely everything). "repair" mode
>> appears to do no writes until an error is found, whereupon (on RAID 6)
>> it proceeds to make a "repair" that is more likely than not to overwrite
>> good data with bad. Optionally writing what's already there on non-error
>> seems like it might be a worthwhile (and fairly simple) change.
>>
> Agreed. But without some heuristic, it's actually going to make a scrub
> much slower, and achieve very little apart from adding unnecessary wear
> to the drive.
>
> Cheers,
> Wol
> --
> 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: Fault tolerance with badblocks
From: Nix @ 2017-05-09 9:58 UTC (permalink / raw)
To: David Brown; +Cc: Wols Lists, Ravi (Tom) Hale, linux-raid
In-Reply-To: <591171BD.3060707@hesbynett.no>
On 9 May 2017, David Brown spake thusly:
> On 08/05/17 16:50, Nix wrote:
>
>> I wonder... scrubbing is not very useful with md, particularly with RAID
>> 6, because it does no writes unless something mismatches, and on failure
>> there is no attempt to determine which of the N disks is bad and rewrite
>> its contents from the other devices (nor, as I understand it, does it
>> clearly say which drive gave the error, so even failing it out and
>> resyncing it is hard).
>
> Please read Neil Brown's article on this: "Smart or simple RAID
> recovery?" <http://neil.brown.name/blog/20100211050355>
I have. THe simple recovery is too simple. So you have a 40TiB RAID-6
array, say, and mismatch_cnt is consistently >0, but a low value, on
scrub. What can you do? The drive is probably not faulty or you'd have
many more mismatches from persistent misdirected reads or writes. md
doesn't repair the corruption, even though on RAID-6 it could. It
doesn't tell you which disk disagreed so you can fail it out. It doesn't
even tell you where the disagreement was so you can try to rebuild it by
hand. What on earth are you supposed to do in this case? Wipe the entire
array and restore from backup? For a *single* sector?
Right now I'm doing scrubs and ignoring the mismatch_cnt, because all it
can do is increase my worry level to no gain at all. I could just as
well do a dd over /dev/md*. It would have the same effect (only without
md's progress feedback and bandwidth throttling. You get progress
feedback, but you don't get told where errors are found?!)
> When the disk is asked to read a block, it pulls up the data and the ECC
> bits, and uses this to check and re-construct the 4K of data, and a
> measure of how many errors were corrected. On modern high-capacity
> drives, it is normal that some errors are corrected on a read. But if
> more than a certain level occur, then the firmware will trigger a
> re-write automatically to the same sector. This will then be re-read.
> If the error rate is low, fine. If it is high, then the sector will be
> remapped by the disk.
>
> So simply /reading/ the data, as far as the processor is concerned, will
> cause re-writes as and when needed.
Last time I asked a disk manufacturer about this, they said oh no we
never correct on read, we can't: if we needed to correct on read, the
data would already be unreadable: you have to trigger a write to get
sparing. Nice to see the drive firmware has improved in the last few
years... but one wonders how many disks actually *do* this. It's hard to
tell because sector sparing is so quiet: it's not always even reflected
in the SMART data, AIUI.
^ 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