* Re: A sector-of-mismatch warning patch (was Re: Fault tolerance with badblocks)
From: NeilBrown @ 2017-05-21 22:00 UTC (permalink / raw)
To: Shaohua Li, Nix
Cc: Chris Murphy, David Brown, Anthony Youngman, Phil Turmel,
Ravi (Tom) Hale, Linux-RAID
In-Reply-To: <20170519165517.257ny67pxkcbtpkq@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 3081 bytes --]
On Fri, May 19 2017, Shaohua Li wrote:
> On Fri, May 19, 2017 at 11:32:43AM +0100, Nix wrote:
>> On 19 May 2017, NeilBrown said:
>>
>> > On Tue, May 16 2017, Nix wrote:
>> >
>> >> On 16 May 2017, NeilBrown spake thusly:
>> >>
>> >>> Actually, I have another caveat. I don't think we want these messages
>> >>> during initial resync, or any resync. Only during a 'check' or
>> >>> 'repair'.
>> >>> So add a check for MD_RECOVERY_REQUESTED or maybe for
>> >>> sh->sectors >= conf->mddev->recovery_cp
>> >>
>> >> I completely agree, but it's already inside MD_RECOVERY_CHECK:
>> >>
>> >> if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery)) {
>> >> /* don't try to repair!! */
>> >> set_bit(STRIPE_INSYNC, &sh->state);
>> >> pr_warn_ratelimited("%s: mismatch sector in range "
>> >> "%llu-%llu\n", mdname(conf->mddev),
>> >> (unsigned long long) sh->sector,
>> >> (unsigned long long) sh->sector +
>> >> STRIPE_SECTORS);
>> >> } else {
>> >>
>> >> Doesn't that already mean that someone has explicitly triggered a check
>> >> action?
>> >
>> > Uhmm... yeah. I lose track of which flags me what exactly.
>> > You log messages aren't generated when 'repair' is used, only when
>> > 'check' is.
>> > I can see why you might have chosen that, but I wonder if it is best.
>>
>> I'm not sure what the point is of being told when repair is used: hey,
>> there was an inconsistency here but there isn't any more! I suppose you
>> could still use it to see if the repair did the right thing. My problem
>> on that front was that I'm not sure what flag should be used to catch
>> repair but not resync etc: everywhere else in the code, repair is in an
>> unadorned else branch... is it the *lack* of MD_RECOVERY_CHECK and the
>> presence of, uh, something else?
> MD_RECOVERY_SYNC && MD_RECOVERY_REQUESTED && MD_RECOVERY_CHECK == check
> MD_RECOVERY_SYNC && MD_RECOVERY_REQUESTED == repair
> MD_RECOVERY_SYNC && !MD_RECOVERY_REQUESTED == resync
>
> Don't see the poin to print the info for 'repair'. 'repair' already changes the
> data, how could we use the info?
Surprising data is can be potentially valuable.
I don't think you should *ever* get an inconsistency in a RAID6 unless
you have faulty hardware.
If you do, then any information about the nature of the inconsistency
might be valuable in understanding the hardware fault.
I don't know in advance how I would interpret the data, but I do
know that if I didn't have the data, then I wouldn't be able to
interpret it.
However .... running "repair" when you don't know exactly what has
happened and why, is probably a bad idea. So logging probably won't
provide value.
I wouldn't go out of my way to add extra logging for the 'repair' case,
but I certainly wouldn't go out of my way to avoid logging in that case.
It seems inconsistent to log for 'check' but not 'repair', but it isn't
a big deal for me.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: How to backup of large md raid volumes?
From: Ram Ramesh @ 2017-05-21 19:48 UTC (permalink / raw)
To: Linux Raid; +Cc: Chris Schanzle
In-Reply-To: <95546cb8-e195-a693-f4c8-e8b250cec30e@cas.homelinux.org>
On 05/21/2017 12:04 PM, Chris Schanzle wrote:
> On 05/18/2017 11:39 PM, Ram Ramesh wrote:
>> On 05/18/2017 08:34 PM, Adam Goryachev wrote:
>>> On 19/05/17 07:37, Ram Ramesh wrote:
>>>> Any one have a method to backup large volumes like md raid6 (16TB)?
>>>> Since the backup will not fit in one disk (in many cases and mine
>>>> too) I am wondering, if there is a known/easy technique to backup
>>>> using multiple usb hard drives. I googed and found a few fancy
>>>> backup utils/systems like Amanda etc. They are overkill for me. I
>>>> am choosing not to back up simply because of the complexity of
>>>> setup as the data in my RAID volume is NOT precious and can be
>>>> replaced with a week of effort.
>>>>
>>>> If any one can think of some thing simple, please point me to it
>>>> and I will do the reading to figure out.
>>>>
>>> I suppose it depends on what you want to achieve. There are various
>>> options such as mirroring to another raid6 array (ie, RAID60) but
>>> really that isn't a backup, it's another replica.
>>> I use backuppc for my backups, it works well under linux with rsync,
>>> I'm sure there are many various options (including amanda which I've
>>> used in the past). Ultimately, it depends on your requirements,
>>> backups vary significantly depending on needs/etc.
>>>
>>> Regards,
>>> Adam
>>>
>>>
>> Here is a summary of what I like to do. I want to backup files on to
>> (multiple) disks that will be loaded on to a USB dock. Simple one to
>> one copy is all I am looking for. I am not interested in full vs.
>> incremental or keeping versions of files for restore. My data is just
>> movies and songs. All I want is a SW that understands links (to avoid
>> duplicates) and copy files in batch on to multiple disks. I want
>> content of each (backup) disk to be independent. This way if one
>> backup disk dies, I have all other files unaffected by this failure.
>> The only reason I did not try multi-volume tar is the lack of
>> independence across disks.
>>
>> I suppose this is a backup question rather than RAID question. I
>> asked here because the size of RAID volumes make it impossible to
>> back up to a single drive and this is a more common problem RAID
>> world rather than general user forum in a backup mailing list.
>>
>> Ramesh
>
> Thanks for the additional info on your requirements.
>
> Rsync will be your best/fastest tool, as writing 16 TB of data over
> USB will likely take days for a full backup...and that seems silly
> when you probably already have 90% of the data already.
>
> If one drive failures are a concern, why not use RAID5 for your group
> of USB backup disks? Might take you a bit more effort
> starting/stopping the array, but I'm sure you'll enjoy the large
> single volume/filesystem in the long run. Use a fairly large
> --chunk= size for your large files. I have had very positive
> experience with XFS for large multimedia filesystems (mythtv user and
> 20TB backup servers).
> If the drives you choose to use are desktop grade, be sure to increase
> the timeouts. Search this list archives for Chris Murphy's subject:
> "Re: URE, link resets, user hostile defaults".
>
The only problem with raid is that all disks need to be present to
create md. I only have the ability to load two disks and the disks are
fairly small compared to RAID l (3TB vs. 16TB) So RAID5 won't work.
Besides, I do not want to use RAID to backup RAID as my worry is md
going bad and preventing access to my files. It is not disk that I am
worried about mdadm layout and requirement that many disk to be alive to
get even one file.
^ permalink raw reply
* Re: [PATCH 03/23] uuid: remove uuid_be defintions from the uapi header
From: Christoph Hellwig @ 2017-05-21 6:55 UTC (permalink / raw)
To: David Howells
Cc: linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Mimi Zohar, Amir Goldstein,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Steven Whitehouse,
linux-xfs-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Joe Perches, Andy Shevchenko,
Shaohua Li, Christoph Hellwig
In-Reply-To: <13041.1495191521-S6HVgzuS8uM4Awkfq6JHfwNdhmdF6hFW@public.gmane.org>
On Fri, May 19, 2017 at 11:58:41AM +0100, David Howells wrote:
> Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org> wrote:
>
> > It's not a userspace ABI, so by defintion it does not break an
> > existing user program.
>
> That's an invalid assumption. It is a de facto userspace ABI as it has been
> exposed in /usr/include/linux/uuid.h for some time.
It is never passed through any kernel interface, which by defintion does
not make it an ABI. And even if it was we are not going to change the
ABI for a uuid_t ever. We're just changing the internal name, and stop
exposing the old name which was never used in an ABI to userspace.
^ permalink raw reply
* (unknown),
From: citydesk @ 2017-05-20 20:00 UTC (permalink / raw)
To: linux-raid
[-- Attachment #1: 39874.zip --]
[-- Type: application/zip, Size: 2821 bytes --]
^ permalink raw reply
* Re: How to backup of large md raid volumes?
From: Reindl Harald @ 2017-05-20 18:14 UTC (permalink / raw)
To: Wols Lists, Ram Ramesh, Linux Raid
In-Reply-To: <59208443.7080404@youngman.org.uk>
Am 20.05.2017 um 20:00 schrieb Wols Lists:
> I'd be inclined to try and get an e-SATA or USB-3 dock, and just back up
> on to independent 8TB drives. Mind you, at £70 for a 3TB Barracuda, $500
> for 16TB of disk doesn't sound pricey at all ...
yes but it don't make much fun if you need to spread your backups to
several disks when you can have just one with all
> One thing to watch out for, it sounds like the OP has mostly music and
> videos, so compression is likely to INCREASE the disk space required...
yo are confusing "compress-force=zlib" with "compress=zlib" - read the
BTRFS manuals, it's practically the same as comp-lzo for openvpn -
uncompressable data are not compressed
^ permalink raw reply
* Re: How to backup of large md raid volumes?
From: Reindl Harald @ 2017-05-20 18:11 UTC (permalink / raw)
To: Wols Lists, Ram Ramesh, Linux Raid
In-Reply-To: <59208443.7080404@youngman.org.uk>
Am 20.05.2017 um 20:00 schrieb Wols Lists:
> On 19/05/17 06:49, Reindl Harald wrote:
>>
>>
>> Am 18.05.2017 um 23:37 schrieb Ram Ramesh:
>>> Any one have a method to backup large volumes like md raid6 (16TB)?
>>> Since the backup will not fit in one disk (in many cases and mine too)
>>> I am wondering, if there is a known/easy technique to backup using
>>> multiple usb hard drives. I googed and found a few fancy backup
>>> utils/systems like Amanda etc. They are overkill for me. I am choosing
>>> not to back up simply because of the complexity of setup as the data
>>> in my RAID volume is NOT precious and can be replaced with a week of
>>> effort.
>>>
>>> If any one can think of some thing simple, please point me to it and I
>>> will do the reading to figure out
>>
>> https://www.amazon.com/16TB-Desktop-External-Drive-WDBLWE0160JCH-NESN/dp/B01B6BN1CU
>>
>>
>> 499$ is not that much for a 16 TB external backup disk and whn you
>> format it with BTRFS and enable compression you most likely have a lot
>> fo space and your backup finally is just a single rsync command
>
> My only reaction to that is it's not a 16TB disk. It's a 16TB raid-0
> array. We had a similar setup ages ago (when 500GB was huge) with a
> 500GB external USB drive. One of the 250GB drives failed and the entire
> array was lost
well, it's a *backup* drive - when it's gone it's gone
we are using a bunch of them with 4 TB since 2011 for offsite-backups of
the whole company cluster and they are a RAID0 too
^ permalink raw reply
* Re: How to backup of large md raid volumes?
From: Wols Lists @ 2017-05-20 18:00 UTC (permalink / raw)
To: Reindl Harald, Ram Ramesh, Linux Raid
In-Reply-To: <28a4540b-c5ac-bd8d-c030-6d718a02fdfc@thelounge.net>
On 19/05/17 06:49, Reindl Harald wrote:
>
>
> Am 18.05.2017 um 23:37 schrieb Ram Ramesh:
>> Any one have a method to backup large volumes like md raid6 (16TB)?
>> Since the backup will not fit in one disk (in many cases and mine too)
>> I am wondering, if there is a known/easy technique to backup using
>> multiple usb hard drives. I googed and found a few fancy backup
>> utils/systems like Amanda etc. They are overkill for me. I am choosing
>> not to back up simply because of the complexity of setup as the data
>> in my RAID volume is NOT precious and can be replaced with a week of
>> effort.
>>
>> If any one can think of some thing simple, please point me to it and I
>> will do the reading to figure out
>
> https://www.amazon.com/16TB-Desktop-External-Drive-WDBLWE0160JCH-NESN/dp/B01B6BN1CU
>
>
> 499$ is not that much for a 16 TB external backup disk and whn you
> format it with BTRFS and enable compression you most likely have a lot
> fo space and your backup finally is just a single rsync command
My only reaction to that is it's not a 16TB disk. It's a 16TB raid-0
array. We had a similar setup ages ago (when 500GB was huge) with a
500GB external USB drive. One of the 250GB drives failed and the entire
array was lost.
I'd be inclined to try and get an e-SATA or USB-3 dock, and just back up
on to independent 8TB drives. Mind you, at £70 for a 3TB Barracuda, $500
for 16TB of disk doesn't sound pricey at all ...
One thing to watch out for, it sounds like the OP has mostly music and
videos, so compression is likely to INCREASE the disk space required...
Cheers,
Wol
^ permalink raw reply
* Re: [PATCH v2] manpage: bad block support for IMSM
From: Jes Sorensen @ 2017-05-19 19:44 UTC (permalink / raw)
To: Tomasz Majchrzak, linux-raid
In-Reply-To: <1495113289-20167-1-git-send-email-tomasz.majchrzak@intel.com>
On 05/18/2017 09:14 AM, Tomasz Majchrzak wrote:
> Signed-off-by: Tomasz Majchrzak <tomasz.majchrzak@intel.com>
> ---
> mdadm.8.in | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> v2:
> signed-off-by added
Applied!
Thanks,
Jes
^ permalink raw reply
* Re: [PATCH] Get failed disk count from array state
From: Jes Sorensen @ 2017-05-19 19:43 UTC (permalink / raw)
To: Tomasz Majchrzak, linux-raid
In-Reply-To: <1494850636-28438-1-git-send-email-tomasz.majchrzak@intel.com>
On 05/15/2017 08:17 AM, Tomasz Majchrzak wrote:
> Recent commit has changed the way failed disks are counted. It breaks
> recovery for external metadata arrays as failed disks are not part of
> the array and have no corresponding entries is sysfs (they are only
> reported for containers) so degraded arrays show no failed disks.
>
> Recent commit overwrites GET_DEGRADED result prior to GET_STATE and it
> is not set again if GET_STATE has not been requested. As GET_STATE
> provides the same information as GET_DEGRADED, the latter is not needed
> anymore. Remove GET_DEGRADED option and replace it with GET_STATE
> option.
>
> Don't count number of failed disks looking at sysfs entries but
> calculate it at the end. Do it only for arrays as containers report
> no disks, just spares.
Hi Tomasz,
I have been sitting on a stack of patches in this area as I wanted to
test them before pushing them out. I finally got a change to run the
testsuite and pushed them, but your patch no longer applied.
Would you mind having a look at the current trunk and see how you want
to address it?
Thanks,
Jes
^ permalink raw reply
* Re: A sector-of-mismatch warning patch (was Re: Fault tolerance with badblocks)
From: Shaohua Li @ 2017-05-19 16:55 UTC (permalink / raw)
To: Nix
Cc: NeilBrown, Chris Murphy, David Brown, Anthony Youngman,
Phil Turmel, Ravi (Tom) Hale, Linux-RAID
In-Reply-To: <8737c13zn8.fsf@esperi.org.uk>
On Fri, May 19, 2017 at 11:32:43AM +0100, Nix wrote:
> On 19 May 2017, NeilBrown said:
>
> > On Tue, May 16 2017, Nix wrote:
> >
> >> On 16 May 2017, NeilBrown spake thusly:
> >>
> >>> Actually, I have another caveat. I don't think we want these messages
> >>> during initial resync, or any resync. Only during a 'check' or
> >>> 'repair'.
> >>> So add a check for MD_RECOVERY_REQUESTED or maybe for
> >>> sh->sectors >= conf->mddev->recovery_cp
> >>
> >> I completely agree, but it's already inside MD_RECOVERY_CHECK:
> >>
> >> if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery)) {
> >> /* don't try to repair!! */
> >> set_bit(STRIPE_INSYNC, &sh->state);
> >> pr_warn_ratelimited("%s: mismatch sector in range "
> >> "%llu-%llu\n", mdname(conf->mddev),
> >> (unsigned long long) sh->sector,
> >> (unsigned long long) sh->sector +
> >> STRIPE_SECTORS);
> >> } else {
> >>
> >> Doesn't that already mean that someone has explicitly triggered a check
> >> action?
> >
> > Uhmm... yeah. I lose track of which flags me what exactly.
> > You log messages aren't generated when 'repair' is used, only when
> > 'check' is.
> > I can see why you might have chosen that, but I wonder if it is best.
>
> I'm not sure what the point is of being told when repair is used: hey,
> there was an inconsistency here but there isn't any more! I suppose you
> could still use it to see if the repair did the right thing. My problem
> on that front was that I'm not sure what flag should be used to catch
> repair but not resync etc: everywhere else in the code, repair is in an
> unadorned else branch... is it the *lack* of MD_RECOVERY_CHECK and the
> presence of, uh, something else?
MD_RECOVERY_SYNC && MD_RECOVERY_REQUESTED && MD_RECOVERY_CHECK == check
MD_RECOVERY_SYNC && MD_RECOVERY_REQUESTED == repair
MD_RECOVERY_SYNC && !MD_RECOVERY_REQUESTED == resync
Don't see the poin to print the info for 'repair'. 'repair' already changes the
data, how could we use the info?
Thanks,
Shaohua
^ permalink raw reply
* Re: A sector-of-mismatch warning patch (was Re: Fault tolerance with badblocks)
From: Shaohua Li @ 2017-05-19 16:48 UTC (permalink / raw)
To: Nix
Cc: NeilBrown, Chris Murphy, David Brown, Anthony Youngman,
Phil Turmel, Ravi (Tom) Hale, Linux-RAID
In-Reply-To: <877f1d3zpg.fsf@esperi.org.uk>
On Fri, May 19, 2017 at 11:31:23AM +0100, Nix wrote:
> On 19 May 2017, NeilBrown verbalised:
>
> > On Wed, May 17 2017, Shaohua Li wrote:
> >
> >> On Tue, May 16, 2017 at 10:46:13PM +0100, Nix wrote:
> >>> Doesn't that already mean that someone has explicitly triggered a check
> >>> action?
> >>
> >> So the idea is: run 'check' and report mismatch, userspace (raid6check for
> >> example) uses the reported info to fix the mismatch. The pr_warn_ratelimited
> >> isn't a good way to communicate the info to userspace. I'm wondering why we
> >> don't just run raid6check solely, it can do the job like what kernel does and
> >> we avoid the crappy pr_warn_ratelimited.
>
> It'll do when there are a few inconsistencies but you don't want to
> spend days recovering a huge array to fix a small but nonzero
> mismatch_cnt, or to reassure you that yes, these mismatch_cnts are in
> swap, ignore them. When there are a lot, enough that a ratelimited
> warning hits its rate limit, Neil's right: the array is probably toast.
> The limit is then important to stop log flooding.
>
> > If we really wanted a seamless "fix the raid6 thing" (which I don't
> > think we do),
>
> Oh, I want seamless everything -- the seamlessness and flexibility of md
> are its killer features over hardware RAID in my eyes -- but I'm
> convinced that this is probably too hard to test and simply too
> disruptive to bother with for a likely vanishingly rare failure mode all
> entangled with fairly hot paths.
>
> > we'd probably make the list of inconsistencies appear in a
> > sysfs file. That would be less 'crappy'. But as I say, I don't think
> > we really want to do that.
>
> Aren't sysfs files in effect length-limited to one page (or at least
> length-limited by virtue of being stored in memory?) It seems to me this
> would just bring the same problem ratelimit is solving right back again,
> except a sysfs file doesn't have a logging daemon sucking the contents
> out constantly so you can overwrite your old output without worrying.
> (And there is no other daemon running to do that, except mdadm in
> monitor mode, which might not be running and really this job feels out
> of scope for it anyway.)
No, my question is not the print is ratelimited. The problem is dmesg isn't a
good way to communicate info to userspace. You can easily lose all dmesg info
with a simple 'dmesg -c'. sysfs file is more reliable. Length-limited isn't a
problem, as you said, if there are a lot of mismatch, the array is toast.
Alright, I'll accept Neil's suggestion. Unless your guys really need a seamless
fix (which I'm still thinking about doing it in usespace by optimizing
raid6check) and we'd take this simple warning patch.
Thanks,
Shaohua
^ permalink raw reply
* (unknown),
From: citydesk @ 2017-05-19 14:51 UTC (permalink / raw)
To: linux-raid
[-- Attachment #1: 128734285588468.zip --]
[-- Type: application/zip, Size: 2883 bytes --]
^ permalink raw reply
* Re: [PATCH 03/23] uuid: remove uuid_be defintions from the uapi header
From: David Howells @ 2017-05-19 10:58 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Mimi Zohar, Amir Goldstein,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dhowells-H+wXaHxf7aLQT0dZR+AlfA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Joe Perches, Andy Shevchenko,
Shaohua Li, Steven Whitehouse
In-Reply-To: <20170518071352.GA515-jcswGhMUV9g@public.gmane.org>
Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org> wrote:
> It's not a userspace ABI, so by defintion it does not break an
> existing user program.
That's an invalid assumption. It is a de facto userspace ABI as it has been
exposed in /usr/include/linux/uuid.h for some time.
> If someone was using it they should be using uuid_t from libuuid instead, as
> that gives them the routines to deal with it.
Yes, they should - but that doesn't mean they do.
David
^ permalink raw reply
* Re: A sector-of-mismatch warning patch (was Re: Fault tolerance with badblocks)
From: Nix @ 2017-05-19 10:32 UTC (permalink / raw)
To: NeilBrown
Cc: Chris Murphy, David Brown, Anthony Youngman, Phil Turmel,
Ravi (Tom) Hale, Linux-RAID
In-Reply-To: <87h90hlac0.fsf@notabene.neil.brown.name>
On 19 May 2017, NeilBrown said:
> On Tue, May 16 2017, Nix wrote:
>
>> On 16 May 2017, NeilBrown spake thusly:
>>
>>> Actually, I have another caveat. I don't think we want these messages
>>> during initial resync, or any resync. Only during a 'check' or
>>> 'repair'.
>>> So add a check for MD_RECOVERY_REQUESTED or maybe for
>>> sh->sectors >= conf->mddev->recovery_cp
>>
>> I completely agree, but it's already inside MD_RECOVERY_CHECK:
>>
>> if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery)) {
>> /* don't try to repair!! */
>> set_bit(STRIPE_INSYNC, &sh->state);
>> pr_warn_ratelimited("%s: mismatch sector in range "
>> "%llu-%llu\n", mdname(conf->mddev),
>> (unsigned long long) sh->sector,
>> (unsigned long long) sh->sector +
>> STRIPE_SECTORS);
>> } else {
>>
>> Doesn't that already mean that someone has explicitly triggered a check
>> action?
>
> Uhmm... yeah. I lose track of which flags me what exactly.
> You log messages aren't generated when 'repair' is used, only when
> 'check' is.
> I can see why you might have chosen that, but I wonder if it is best.
I'm not sure what the point is of being told when repair is used: hey,
there was an inconsistency here but there isn't any more! I suppose you
could still use it to see if the repair did the right thing. My problem
on that front was that I'm not sure what flag should be used to catch
repair but not resync etc: everywhere else in the code, repair is in an
unadorned else branch... is it the *lack* of MD_RECOVERY_CHECK and the
presence of, uh, something else?
--
NULL && (void)
^ permalink raw reply
* Re: A sector-of-mismatch warning patch (was Re: Fault tolerance with badblocks)
From: Nix @ 2017-05-19 10:31 UTC (permalink / raw)
To: NeilBrown
Cc: Shaohua Li, Chris Murphy, David Brown, Anthony Youngman,
Phil Turmel, Ravi (Tom) Hale, Linux-RAID
In-Reply-To: <87efvlla69.fsf@notabene.neil.brown.name>
On 19 May 2017, NeilBrown verbalised:
> On Wed, May 17 2017, Shaohua Li wrote:
>
>> On Tue, May 16, 2017 at 10:46:13PM +0100, Nix wrote:
>>> Doesn't that already mean that someone has explicitly triggered a check
>>> action?
>>
>> So the idea is: run 'check' and report mismatch, userspace (raid6check for
>> example) uses the reported info to fix the mismatch. The pr_warn_ratelimited
>> isn't a good way to communicate the info to userspace. I'm wondering why we
>> don't just run raid6check solely, it can do the job like what kernel does and
>> we avoid the crappy pr_warn_ratelimited.
It'll do when there are a few inconsistencies but you don't want to
spend days recovering a huge array to fix a small but nonzero
mismatch_cnt, or to reassure you that yes, these mismatch_cnts are in
swap, ignore them. When there are a lot, enough that a ratelimited
warning hits its rate limit, Neil's right: the array is probably toast.
The limit is then important to stop log flooding.
> If we really wanted a seamless "fix the raid6 thing" (which I don't
> think we do),
Oh, I want seamless everything -- the seamlessness and flexibility of md
are its killer features over hardware RAID in my eyes -- but I'm
convinced that this is probably too hard to test and simply too
disruptive to bother with for a likely vanishingly rare failure mode all
entangled with fairly hot paths.
> we'd probably make the list of inconsistencies appear in a
> sysfs file. That would be less 'crappy'. But as I say, I don't think
> we really want to do that.
Aren't sysfs files in effect length-limited to one page (or at least
length-limited by virtue of being stored in memory?) It seems to me this
would just bring the same problem ratelimit is solving right back again,
except a sysfs file doesn't have a logging daemon sucking the contents
out constantly so you can overwrite your old output without worrying.
(And there is no other daemon running to do that, except mdadm in
monitor mode, which might not be running and really this job feels out
of scope for it anyway.)
^ permalink raw reply
* Re: [PATCH v7 0/2] dm: boot a mapped device without an initramfs
From: Enric Balletbo Serra @ 2017-05-19 7:06 UTC (permalink / raw)
To: dm-devel
Cc: Kees Cook, Mike Snitzer, linux-doc, linux-kernel, linux-raid,
linux-lvm, Alasdair Kergon
In-Reply-To: <20170518162911.15538-1-enric.balletbo@collabora.com>
Hi,
2017-05-18 18:29 GMT+02:00 Enric Balletbo i Serra
<enric.balletbo@collabora.com>:
> Dear all,
>
> So here is a new version of the patches to be reviewed, this time as
> suggested by Alasdair the patches are reworked to match with the new
> dmsetup bootformat feature [1]. These patches are not reviewed yet but
> the format was discussed in the IRC and was suggested to send the
> kernel patches in parallel.
>
> Changes since v6:
> - Add a new function to issue the equivalent of a DM ioctl programatically.
> - Use the new ioctl interface to create the devices.
> - Use a comma-delimited and semi-colon delimited dmsetup-like commands.
>
> Changes since v5:
> - https://www.redhat.com/archives/dm-devel/2016-February/msg00112.html
>
> [1] https://www.redhat.com/archives/linux-lvm/2017-May/msg00047.html
>
> Wating for your feedback,
>
> Enric Balletbo i Serra (1):
> dm ioctl: add a device mapper ioctl function.
>
> Will Drewry (1):
> init: add support to directly boot to a mapped device
>
> Documentation/admin-guide/kernel-parameters.rst | 1 +
> Documentation/admin-guide/kernel-parameters.txt | 3 +
> Documentation/device-mapper/dm-boot.txt | 65 ++++
> drivers/md/dm-ioctl.c | 45 +++
> include/linux/device-mapper.h | 6 +
> init/Makefile | 1 +
> init/do_mounts.c | 1 +
> init/do_mounts.h | 10 +
> init/do_mounts_dm.c | 459 ++++++++++++++++++++++++
> 9 files changed, 591 insertions(+)
> create mode 100644 Documentation/device-mapper/dm-boot.txt
> create mode 100644 init/do_mounts_dm.c
>
> --
> 2.9.3
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
Rebasing the patches against current linux-next I just noticied that
there is a build error due commit
commit e516db4f676ac88c7c7f698f8047178e8accc3b8
Author: Mikulas Patocka <mpatocka@redhat.com>
Date: Fri May 5 11:12:52 2017 -0700
dm ioctl: add a new DM_DEV_ARM_POLL ioctl
So I'll send v8 to fix this.
Best regards,
Enric
^ permalink raw reply
* [PATCH v3] super1: fix sb->max_dev when adding a new disk in linear array
From: Lidong Zhong @ 2017-05-19 6:06 UTC (permalink / raw)
To: linux-raid, neilb; +Cc: colyli, Jes.Sorensen
The value of sb->max_dev will always be increased by 1 when adding
a new disk in linear array. It causes an inconsistence between each
disk in the array and the "Array State" value of "mdadm --examine DISK"
is wrong. For example, when adding the first new disk into linear array
it will be:
Array State : RAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
('A' == active, '.' == missing, 'R' == replacing)
Adding the second disk into linear array it will be
Array State : .AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
('A' == active, '.' == missing, 'R' == replacing)
Signed-off-by: Lidong Zhong <lzhong@suse.com>
---
super1.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/super1.c b/super1.c
index 2fcb814..811923f 100644
--- a/super1.c
+++ b/super1.c
@@ -1267,8 +1267,13 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
break;
sb->dev_number = __cpu_to_le32(i);
info->disk.number = i;
- if (max >= __le32_to_cpu(sb->max_dev))
- sb->max_dev = __cpu_to_le32(max+1);
+ if (i >= max) {
+ while (max <= i) {
+ sb->dev_roles[max] = __cpu_to_le16(MD_DISK_ROLE_SPARE);
+ max += 1;
+ }
+ sb->max_dev = __cpu_to_le32(max);
+ }
random_uuid(sb->device_uuid);
@@ -1296,6 +1301,10 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
sb->raid_disks = __cpu_to_le32(info->array.raid_disks);
sb->dev_roles[info->disk.number] =
__cpu_to_le16(info->disk.raid_disk);
+ if (sb->raid_disks+1 >= __le32_to_cpu(sb->max_dev)) {
+ sb->dev_roles[sb->raid_disks] = __cpu_to_le16(MD_DISK_ROLE_SPARE);
+ sb->max_dev = __cpu_to_le32(sb->raid_disks+1);
+ }
} else if (strcmp(update, "resync") == 0) {
/* make sure resync happens */
sb->resync_offset = 0ULL;
--
2.12.0
^ permalink raw reply related
* Re: How to backup of large md raid volumes?
From: Reindl Harald @ 2017-05-19 5:49 UTC (permalink / raw)
To: Ram Ramesh, Linux Raid
In-Reply-To: <442a4af0-00d2-853d-5294-84ae0353262b@gmail.com>
Am 18.05.2017 um 23:37 schrieb Ram Ramesh:
> Any one have a method to backup large volumes like md raid6 (16TB)?
> Since the backup will not fit in one disk (in many cases and mine too) I
> am wondering, if there is a known/easy technique to backup using
> multiple usb hard drives. I googed and found a few fancy backup
> utils/systems like Amanda etc. They are overkill for me. I am choosing
> not to back up simply because of the complexity of setup as the data in
> my RAID volume is NOT precious and can be replaced with a week of effort.
>
> If any one can think of some thing simple, please point me to it and I
> will do the reading to figure out
https://www.amazon.com/16TB-Desktop-External-Drive-WDBLWE0160JCH-NESN/dp/B01B6BN1CU
499$ is not that much for a 16 TB external backup disk and whn you
format it with BTRFS and enable compression you most likely have a lot
fo space and your backup finally is just a single rsync command
^ permalink raw reply
* Re: [PATCH] super1: fix sb->max_dev when adding a new disk in linear array
From: NeilBrown @ 2017-05-19 5:37 UTC (permalink / raw)
To: Lidong Zhong, linux-raid; +Cc: colyli, Jes.Sorensen
In-Reply-To: <4b01d20c-be52-eaa1-6809-909fb375b274@suse.com>
[-- Attachment #1: Type: text/plain, Size: 4018 bytes --]
On Fri, May 19 2017, Lidong Zhong wrote:
> On 05/19/2017 12:36 PM, NeilBrown wrote:
>> On Tue, May 16 2017, Lidong Zhong wrote:
>>
>>> The value of sb->max_dev will always be increased by 1 when adding
>>> a new disk in linear array. It causes an inconsistence between each
>>> disk in the array and the "Array State" value of "mdadm --examine DISK"
>>> is wrong. For example, when adding the first new disk into linear array
>>> it will be:
>>>
>>> Array State : RAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
>>> ('A' == active, '.' == missing, 'R' == replacing)
>>>
>>> Adding the second disk into linear array it will be
>>>
>>> Array State : .AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
>>> ('A' == active, '.' == missing, 'R' == replacing)
>>>
>>> Signed-off-by: Lidong Zhong <lzhong@suse.com>
>>> ---
>>> super1.c | 8 +++++++-
>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/super1.c b/super1.c
>>> index 87a74cb..3d49bee 100644
>>> --- a/super1.c
>>> +++ b/super1.c
>>> @@ -1184,8 +1184,10 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
>>> break;
>>> sb->dev_number = __cpu_to_le32(i);
>>> info->disk.number = i;
>>> - if (max >= __le32_to_cpu(sb->max_dev))
>>> + if (i >= __le32_to_cpu(sb->max_dev)) {
>>
>> This change is correct - thanks. Though
>> if (i >= max) {
>>
>> might be clearer and simpler.
>>
>>
>>> sb->max_dev = __cpu_to_le32(max+1);
>>> + sb->dev_roles[sb->max_dev] = __cpu_to_le16(MD_DISK_ROLE_SPARE);
>>
>> This change is wrong.
>> At the very least, the dev_roles[] array needs to be indexed by a
>> host-order number, not a little-endian number.
>> But the change is not needed because dev_roles[max_dev] is never used.
>> See role_from_sb().
>> dev_rols[max_dev - 1] does need to be set, but the line
>>
>> sb->dev_roles[i] = __cpu_to_le16(info->disk.raid_disk);
>>
>> almost certainly does that.
> Hi Neil,
>
> The reason I set all the dev_roles[0~max_dev-1] is because
> the following code
>
> 552 printf(" Array State : ");
> 553 for (d = 0; d < __le32_to_cpu(sb->raid_disks) + delta_extra;
> d++) {
> 554 int cnt = 0;
> 555 unsigned int i;
> 556 for (i = 0; i < __le32_to_cpu(sb->max_dev); i++) {
> 557 unsigned int role = __le16_to_cpu(sb->dev_roles[i]);
>
> 558 if (role == d)
>
> 559 cnt++;
>
> 560 }
This code does not access dev_roles[max_dev], only up to
dev_roles[max_dev-1].
You changed dev_roles[max_dev], which will never be accessed.
>
>
>> It might be better to do
>> if (i >= max) {
>> while (max <= i) {
>> sb->dev_roles[max] = __cpu_to_le16(MD_DISK_ROLE_SPARE);
>> max += 1;
>> }
>> sb->max_dev = __cpu_to_le32(max);
>> }
>>
>
>
> Thanks for the advice.
>
>>> + }
>>>
>>> random_uuid(sb->device_uuid);
>>>
>>> @@ -1214,6 +1216,10 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
>>> sb->raid_disks = __cpu_to_le32(info->array.raid_disks);
>>> sb->dev_roles[info->disk.number] =
>>> __cpu_to_le16(info->disk.raid_disk);
>>> + if (sb->raid_disks+1 >= __le32_to_cpu(sb->max_dev)) {
>>> + sb->max_dev = __cpu_to_le32(sb->raid_disks+1);
>>> + sb->dev_roles[sb->max_dev] = __cpu_to_le16(MD_DISK_ROLE_SPARE);
>>
>> Again, max_dev is little-endian, so cannot be used as an index.
>> And I think you are updating the wrong element in the dev_roles array.
>
> Yes, I didn't realized the valude is conversed to little-endian and the
> index is wrong too. Thank you for pointing this out. I will submit
> another version patch.
I look forward to it,
Thanks,
NeilBrown
>
> Thanks,
> Lidong
>>
>> Thanks,
>> NeilBrown
>>
>>
>>> + }
>>> } else if (strcmp(update, "resync") == 0) {
>>> /* make sure resync happens */
>>> sb->resync_offset = 0ULL;
>>> --
>>> 2.12.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH] super1: fix sb->max_dev when adding a new disk in linear array
From: Lidong Zhong @ 2017-05-19 5:31 UTC (permalink / raw)
To: NeilBrown, linux-raid; +Cc: colyli, Jes.Sorensen
In-Reply-To: <87pof5laxm.fsf@notabene.neil.brown.name>
On 05/19/2017 12:36 PM, NeilBrown wrote:
> On Tue, May 16 2017, Lidong Zhong wrote:
>
>> The value of sb->max_dev will always be increased by 1 when adding
>> a new disk in linear array. It causes an inconsistence between each
>> disk in the array and the "Array State" value of "mdadm --examine DISK"
>> is wrong. For example, when adding the first new disk into linear array
>> it will be:
>>
>> Array State : RAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
>> ('A' == active, '.' == missing, 'R' == replacing)
>>
>> Adding the second disk into linear array it will be
>>
>> Array State : .AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
>> ('A' == active, '.' == missing, 'R' == replacing)
>>
>> Signed-off-by: Lidong Zhong <lzhong@suse.com>
>> ---
>> super1.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/super1.c b/super1.c
>> index 87a74cb..3d49bee 100644
>> --- a/super1.c
>> +++ b/super1.c
>> @@ -1184,8 +1184,10 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
>> break;
>> sb->dev_number = __cpu_to_le32(i);
>> info->disk.number = i;
>> - if (max >= __le32_to_cpu(sb->max_dev))
>> + if (i >= __le32_to_cpu(sb->max_dev)) {
>
> This change is correct - thanks. Though
> if (i >= max) {
>
> might be clearer and simpler.
>
>
>> sb->max_dev = __cpu_to_le32(max+1);
>> + sb->dev_roles[sb->max_dev] = __cpu_to_le16(MD_DISK_ROLE_SPARE);
>
> This change is wrong.
> At the very least, the dev_roles[] array needs to be indexed by a
> host-order number, not a little-endian number.
> But the change is not needed because dev_roles[max_dev] is never used.
> See role_from_sb().
> dev_rols[max_dev - 1] does need to be set, but the line
>
> sb->dev_roles[i] = __cpu_to_le16(info->disk.raid_disk);
>
> almost certainly does that.
Hi Neil,
The reason I set all the dev_roles[0~max_dev-1] is because
the following code
552 printf(" Array State : ");
553 for (d = 0; d < __le32_to_cpu(sb->raid_disks) + delta_extra;
d++) {
554 int cnt = 0;
555 unsigned int i;
556 for (i = 0; i < __le32_to_cpu(sb->max_dev); i++) {
557 unsigned int role = __le16_to_cpu(sb->dev_roles[i]);
558 if (role == d)
559 cnt++;
560 }
> It might be better to do
> if (i >= max) {
> while (max <= i) {
> sb->dev_roles[max] = __cpu_to_le16(MD_DISK_ROLE_SPARE);
> max += 1;
> }
> sb->max_dev = __cpu_to_le32(max);
> }
>
Thanks for the advice.
>> + }
>>
>> random_uuid(sb->device_uuid);
>>
>> @@ -1214,6 +1216,10 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
>> sb->raid_disks = __cpu_to_le32(info->array.raid_disks);
>> sb->dev_roles[info->disk.number] =
>> __cpu_to_le16(info->disk.raid_disk);
>> + if (sb->raid_disks+1 >= __le32_to_cpu(sb->max_dev)) {
>> + sb->max_dev = __cpu_to_le32(sb->raid_disks+1);
>> + sb->dev_roles[sb->max_dev] = __cpu_to_le16(MD_DISK_ROLE_SPARE);
>
> Again, max_dev is little-endian, so cannot be used as an index.
> And I think you are updating the wrong element in the dev_roles array.
Yes, I didn't realized the valude is conversed to little-endian and the
index is wrong too. Thank you for pointing this out. I will submit
another version patch.
Thanks,
Lidong
>
> Thanks,
> NeilBrown
>
>
>> + }
>> } else if (strcmp(update, "resync") == 0) {
>> /* make sure resync happens */
>> sb->resync_offset = 0ULL;
>> --
>> 2.12.0
^ permalink raw reply
* Re: How to backup of large md raid volumes?
From: Roman Mamedov @ 2017-05-19 5:11 UTC (permalink / raw)
To: Ram Ramesh; +Cc: Adam Goryachev, Linux Raid
In-Reply-To: <8b840e53-1be2-b1b5-632f-e75f0c6c53b7@gmail.com>
On Thu, 18 May 2017 22:39:59 -0500
Ram Ramesh <rramesh2400@gmail.com> wrote:
> Here is a summary of what I like to do. I want to backup files on to
> (multiple) disks that will be loaded on to a USB dock. Simple one to one
> copy is all I am looking for. I am not interested in full vs.
> incremental or keeping versions of files for restore. My data is just
> movies and songs. All I want is a SW that understands links (to avoid
> duplicates) and copy files in batch on to multiple disks. I want content
> of each (backup) disk to be independent. This way if one backup disk
> dies, I have all other files unaffected by this failure. The only reason
> I did not try multi-volume tar is the lack of independence across disks.
What I'm doing is storing files on my primary storage in folders according to
where they are backed up to.
E.g. "[back up of this is to be stored] On USB disk A", "B", etc.
In your case that would be:
/storage/OnDiskA/Movies/......
/storage/OnDiskA/Music/......
/storage/OnDiskB/Movies/......
/storage/OnDiskB/Software/......
(here I hit some wicked hotkey which causes the message to be saved and sent
prematurely :D)
/storage/OnDiskC/Photos/......
etc.
After that you can set up simple rsync to synchronize each folder to its
corresponding disk (though mine are network locations of various kind, not
transient USB devices, so I can set up rsync by crontab).
And for convenience in day-to-day use, you can merge all those folders into
one using one of the Union FS tools, such as mhddfs: https://romanrm.net/mhddfs
Running it with /storage/DiskA/,/storage/DiskB/,/storage/DiskC/ and mounting
them into /storage/all/, you would have
/storage/all/Movies/
/storage/all/Music/
/storage/all/Software/
/storage/all/Photos/
conveniently within one directory, i.e. directory trees from all the "Disk"
dirs overlaid onto each other. You do not have to use mhddfs to write through
it (since with one large storage it doesn't help you manage space), can just
keep using the individual dirs for precise allocation of what you want backed
up where.
Perhaps there are backup tools which would let you split backups without also
separating them in primary storage, but this is the way I found to be the
simplest (regular rsync), most transparent (can be inspected with any file
manager) and easy to work with (just drag files and dirs around).
--
With respect,
Roman
^ permalink raw reply
* Re: How to backup of large md raid volumes?
From: Roman Mamedov @ 2017-05-19 5:01 UTC (permalink / raw)
To: Ram Ramesh; +Cc: Adam Goryachev, Linux Raid
In-Reply-To: <8b840e53-1be2-b1b5-632f-e75f0c6c53b7@gmail.com>
On Thu, 18 May 2017 22:39:59 -0500
Ram Ramesh <rramesh2400@gmail.com> wrote:
> Here is a summary of what I like to do. I want to backup files on to
> (multiple) disks that will be loaded on to a USB dock. Simple one to one
> copy is all I am looking for. I am not interested in full vs.
> incremental or keeping versions of files for restore. My data is just
> movies and songs. All I want is a SW that understands links (to avoid
> duplicates) and copy files in batch on to multiple disks. I want content
> of each (backup) disk to be independent. This way if one backup disk
> dies, I have all other files unaffected by this failure. The only reason
> I did not try multi-volume tar is the lack of independence across disks.
What I'm doing is storing files on my primary storage in folders according to
where they are backed up to.
E.g. "[back up of this is to be stored] On USB disk A", "B", etc.
In your case that would be:
/storage/OnDiskA/Movies/......
/storage/OnDiskA/Music/......
/storage/OnDiskB/Movies/......
--
With respect,
Roman
^ permalink raw reply
* Re: A sector-of-mismatch warning patch (was Re: Fault tolerance with badblocks)
From: NeilBrown @ 2017-05-19 4:53 UTC (permalink / raw)
To: Shaohua Li, Nix
Cc: Chris Murphy, David Brown, Anthony Youngman, Phil Turmel,
Ravi (Tom) Hale, Linux-RAID
In-Reply-To: <20170518000624.xmvttuyio6llu25r@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1989 bytes --]
On Wed, May 17 2017, Shaohua Li wrote:
> On Tue, May 16, 2017 at 10:46:13PM +0100, Nix wrote:
>> On 16 May 2017, NeilBrown spake thusly:
>>
>> > Actually, I have another caveat. I don't think we want these messages
>> > during initial resync, or any resync. Only during a 'check' or
>> > 'repair'.
>> > So add a check for MD_RECOVERY_REQUESTED or maybe for
>> > sh->sectors >= conf->mddev->recovery_cp
>>
>> I completely agree, but it's already inside MD_RECOVERY_CHECK:
>>
>> if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery)) {
>> /* don't try to repair!! */
>> set_bit(STRIPE_INSYNC, &sh->state);
>> pr_warn_ratelimited("%s: mismatch sector in range "
>> "%llu-%llu\n", mdname(conf->mddev),
>> (unsigned long long) sh->sector,
>> (unsigned long long) sh->sector +
>> STRIPE_SECTORS);
>> } else {
>>
>> Doesn't that already mean that someone has explicitly triggered a check
>> action?
>
>
> Hi,
> So the idea is: run 'check' and report mismatch, userspace (raid6check for
> example) uses the reported info to fix the mismatch. The pr_warn_ratelimited
> isn't a good way to communicate the info to userspace. I'm wondering why we
> don't just run raid6check solely, it can do the job like what kernel does and
> we avoid the crappy pr_warn_ratelimited.
>
raid6check is *much* slower than doing it in the kernel, as the
interlocking to avoid checking a stripe that is being written are
clumsy.... and async IO is harder in user space.
I think the warnings are useful as warnings quite apart from the
possibility of raid6check using them.
If we really wanted a seamless "fix the raid6 thing" (which I don't
think we do), we'd probably make the list of inconsistencies appear in a
sysfs file. That would be less 'crappy'. But as I say, I don't think
we really want to do that.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: A sector-of-mismatch warning patch (was Re: Fault tolerance with badblocks)
From: NeilBrown @ 2017-05-19 4:49 UTC (permalink / raw)
To: Nix
Cc: Chris Murphy, David Brown, Anthony Youngman, Phil Turmel,
Ravi (Tom) Hale, Linux-RAID
In-Reply-To: <871sroscey.fsf@esperi.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1209 bytes --]
On Tue, May 16 2017, Nix wrote:
> On 16 May 2017, NeilBrown spake thusly:
>
>> Actually, I have another caveat. I don't think we want these messages
>> during initial resync, or any resync. Only during a 'check' or
>> 'repair'.
>> So add a check for MD_RECOVERY_REQUESTED or maybe for
>> sh->sectors >= conf->mddev->recovery_cp
>
> I completely agree, but it's already inside MD_RECOVERY_CHECK:
>
> if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery)) {
> /* don't try to repair!! */
> set_bit(STRIPE_INSYNC, &sh->state);
> pr_warn_ratelimited("%s: mismatch sector in range "
> "%llu-%llu\n", mdname(conf->mddev),
> (unsigned long long) sh->sector,
> (unsigned long long) sh->sector +
> STRIPE_SECTORS);
> } else {
>
> Doesn't that already mean that someone has explicitly triggered a check
> action?
Uhmm... yeah. I lose track of which flags me what exactly.
You log messages aren't generated when 'repair' is used, only when
'check' is.
I can see why you might have chosen that, but I wonder if it is best.
But I'm OK with this patch as it stands.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH] super1: fix sb->max_dev when adding a new disk in linear array
From: NeilBrown @ 2017-05-19 4:36 UTC (permalink / raw)
To: Lidong Zhong, linux-raid; +Cc: colyli, Jes.Sorensen
In-Reply-To: <20170516045129.21815-1-lzhong@suse.com>
[-- Attachment #1: Type: text/plain, Size: 2774 bytes --]
On Tue, May 16 2017, Lidong Zhong wrote:
> The value of sb->max_dev will always be increased by 1 when adding
> a new disk in linear array. It causes an inconsistence between each
> disk in the array and the "Array State" value of "mdadm --examine DISK"
> is wrong. For example, when adding the first new disk into linear array
> it will be:
>
> Array State : RAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
> ('A' == active, '.' == missing, 'R' == replacing)
>
> Adding the second disk into linear array it will be
>
> Array State : .AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
> ('A' == active, '.' == missing, 'R' == replacing)
>
> Signed-off-by: Lidong Zhong <lzhong@suse.com>
> ---
> super1.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/super1.c b/super1.c
> index 87a74cb..3d49bee 100644
> --- a/super1.c
> +++ b/super1.c
> @@ -1184,8 +1184,10 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
> break;
> sb->dev_number = __cpu_to_le32(i);
> info->disk.number = i;
> - if (max >= __le32_to_cpu(sb->max_dev))
> + if (i >= __le32_to_cpu(sb->max_dev)) {
This change is correct - thanks. Though
if (i >= max) {
might be clearer and simpler.
> sb->max_dev = __cpu_to_le32(max+1);
> + sb->dev_roles[sb->max_dev] = __cpu_to_le16(MD_DISK_ROLE_SPARE);
This change is wrong.
At the very least, the dev_roles[] array needs to be indexed by a
host-order number, not a little-endian number.
But the change is not needed because dev_roles[max_dev] is never used.
See role_from_sb().
dev_rols[max_dev - 1] does need to be set, but the line
sb->dev_roles[i] = __cpu_to_le16(info->disk.raid_disk);
almost certainly does that.
It might be better to do
if (i >= max) {
while (max <= i) {
sb->dev_roles[max] = __cpu_to_le16(MD_DISK_ROLE_SPARE);
max += 1;
}
sb->max_dev = __cpu_to_le32(max);
}
> + }
>
> random_uuid(sb->device_uuid);
>
> @@ -1214,6 +1216,10 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
> sb->raid_disks = __cpu_to_le32(info->array.raid_disks);
> sb->dev_roles[info->disk.number] =
> __cpu_to_le16(info->disk.raid_disk);
> + if (sb->raid_disks+1 >= __le32_to_cpu(sb->max_dev)) {
> + sb->max_dev = __cpu_to_le32(sb->raid_disks+1);
> + sb->dev_roles[sb->max_dev] = __cpu_to_le16(MD_DISK_ROLE_SPARE);
Again, max_dev is little-endian, so cannot be used as an index.
And I think you are updating the wrong element in the dev_roles array.
Thanks,
NeilBrown
> + }
> } else if (strcmp(update, "resync") == 0) {
> /* make sure resync happens */
> sb->resync_offset = 0ULL;
> --
> 2.12.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox