* mdadm --create parameter ordering
From: Steven Reddie @ 2015-07-13 10:26 UTC (permalink / raw)
To: linux-raid
Hi,
Given the following command:
mdadm -Cf /dev/md3 -e1.2 -n5 -l6 /dev/sdga5 /dev/sdgb5 /dev/sdgc5
/dev/sdgd5 /dev/sdge5 -u<uuid>
Is the order that the disks are specified on the command-line significant?
Somebody suggested this command to put the disks back in the correct
order (though I believe he was mistaken about the correct order). It
seems to me that if the order on the command-line is significant that
this would have caused parity to get recalculated (based on the
[incorrect] order of the disks) and the new parity to be written over
actual data blocks. I see no evidence that this has occurred, which
leads me to believe that the actual ordering within the array is
detected and everything proceeds as it should, in which case issuing
the command above likely was a very time-consuming no-op.
Also, it seems that I should be able to issue such a command without
the full scan of the disks. Is that what the "--run" option is for? If
so, does "--readonly" protect me from harm that improper use of
"--run" may cause?
Thanks,
Steven
^ permalink raw reply
* Re: mdadm --create parameter ordering
From: Robin Hill @ 2015-07-13 10:59 UTC (permalink / raw)
To: Steven Reddie; +Cc: linux-raid
In-Reply-To: <CAFcup8E3iKxb8H-Fz76bKJQhjmhq1FuVu11rEPb-k=g2U_nX=w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2960 bytes --]
On Mon Jul 13, 2015 at 08:26:48PM +1000, Steven Reddie wrote:
> Hi,
>
> Given the following command:
>
> mdadm -Cf /dev/md3 -e1.2 -n5 -l6 /dev/sdga5 /dev/sdgb5 /dev/sdgc5
> /dev/sdgd5 /dev/sdge5 -u<uuid>
>
> Is the order that the disks are specified on the command-line significant?
>
Yes, it specifies the disks' position within the array.
> Somebody suggested this command to put the disks back in the correct
> order (though I believe he was mistaken about the correct order). It
> seems to me that if the order on the command-line is significant that
> this would have caused parity to get recalculated (based on the
> [incorrect] order of the disks) and the new parity to be written over
> actual data blocks. I see no evidence that this has occurred, which
> leads me to believe that the actual ordering within the array is
> detected and everything proceeds as it should, in which case issuing
> the command above likely was a very time-consuming no-op.
>
You're creating a new array, so there is no "actual ordering" to detect
- any old array will be ignored. If you are re-creating the array in
order to fix an issue (which you should only do as a very last resort,
and only after seeking help and advice here) then an incorrect order
will definitely cause new parity information to be written over data
blocks (which is why you should always leave "missing" disks, or use
--assume-clean, to prevent parity calculation until after you've
verified the order is correct and the data is readable), If the
filesystem shows as readable (and error free) then the order specified
would appear to be correct.
> Also, it seems that I should be able to issue such a command without
> the full scan of the disks. Is that what the "--run" option is for? If
> so, does "--readonly" protect me from harm that improper use of
> "--run" may cause?
>
The --run option is to start an array which has been assembled but not
started. The udev auto-assemble rule, for example, will add disks into
the array as they're discovered but only start it once the array is
complete (or as complete as it was the last time it was assembled), so
in the case of a missing disk, a manual --run command may be required.
I'm not entirely sure what the --readonly option will do in the case of
a create command. Possibly just the metadata will be written and the
parity calculation will be paused.
The --assume-clean option will prevent parity calculation (as it assumes
that the parity on disk is already correct), which may be what you're
after. It should only be used it you're absolutely certain of the data
though, as it can lead to large-scale corruption otherwise.
Cheers,
Robin
--
___
( ' } | Robin Hill <robin@robinhill.me.uk> |
/ / ) | Little Jim says .... |
// !! | "He fallen in de water !!" |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: mdadm --create parameter ordering
From: Phil Turmel @ 2015-07-13 13:17 UTC (permalink / raw)
To: Steven Reddie, linux-raid
In-Reply-To: <20150713105928.GA19366@cthulhu.home.robinhill.me.uk>
On 07/13/2015 06:59 AM, Robin Hill wrote:
[snip /]
Let me elaborate on one of Robin's comments:
> The --assume-clean option will prevent parity calculation (as it assumes
> that the parity on disk is already correct), which may be what you're
> after. It should only be used it you're absolutely certain of the data
> though, as it can lead to large-scale corruption otherwise.
It is not enough to be certain of the order to use --create
--assume-clean on existing data. You must also be certain of superblock
type, chunk size, data offset, and layout option.
Data loss due to unwise use of --create has a history of bad outcomes.
It destroys the previous metadata on array members, metadata that is
almost always needed to recover damaged arrays. The --create option
should *never* be used on an array with important data without first
asking for advice on this list.
Phil
^ permalink raw reply
* Re: mdadm-grow-continue service crashing (similiar to "raid5 reshape is stuck" thread from May)
From: Phil Turmel @ 2015-07-13 13:54 UTC (permalink / raw)
To: Edward Kuns; +Cc: linux-raid, NeilBrown
In-Reply-To: <CACsGCyT5vgFkQEc1zgKCGgT_rO5V4cmuFmo9K4vwiqN4JsPLJA@mail.gmail.com>
Hi Eddie,
On 07/12/2015 03:24 PM, Edward Kuns wrote:
> On Sun, Jul 12, 2015 at 8:45 AM, Phil Turmel <philip@turmel.org> wrote:
>> Why were you using --grow for these operations only to reverse it? This
>> is dangerous if you have a layer or filesystem on your array that
>> doesn't support shrinking. None of the --grow operations were necessary
>> in this sequence to achieve the end result of replacing disks.
> [snip]
>> At no point should you have changed the number of raid devices.
> [snip]
>> And for the still-running but suspect drive, the --replace operation
>> would have been the right choice, again, after --add of a spare.
>
> I didn't mention the steps I did to replace the failed drive because
> that went flawlessly. I did a fail and remove on it to be sure, but
> got complaints that it was already failed/removed. When I did an add
> for the replacement drive, it came in and synced automatically. I
> only ran into trouble trying to replace the "not yet dead but suspect"
> drive. I was following examples on the Internet. The example I was
> following was a clearly a bad one. The examples I found didn't
> suggest the --replace option. This is ultimately my fault for not
> being familiar enough with this. Now I know better.
Even without the --replace operation, --grow should never have been
used. On older kernels without support for --replace, the correct
operation is --add spare then --fail, --remove.
> FWIW, I had LVM on top of the raid5, with two partitions (/var and an
> extra storage one) on the LVM. (I think there is some spare space
> too.) The goal, of course, is being able to survive any single-drive
> failure, which I did.
>
> You said this is dangerous. I went from 4->5 and then immediately
> 5->4 drives. I didn't expand the LVM on the raid5, and the
> replacement partition was a little bigger than the original. Next
> time, I'll use --replace, obviously. I just want to understand why it
> is dangerous. As long as the replacement partition is as big as the
> one it is replacing, isn't this just extra work, and more chance of
> running into problems like the one I ran into? But other than that,
> it shouldn't risk the actual data stored on the RAID,should it?
In theory, no. But the --grow operation has to move virtually every
data block to a new location, and in your case, then back to its
original location. Lots of unnecessary data movement that has a low but
non-zero error-rate.
Also, the complex operations in --grow have produced somewhat more than
its fair share of mdadm bugs. Stuck reshapes are usually recoverable,
but typically only with assistance from this list. Drive failures
during reshapes can be particularly sticky, especially when the failure
is of the device holding a critical section backup.
>> many modern distros delete /tmp on reboot and/or play
>> games with namespaces to isolate different users' /tmp spaces.
>
> So if the machine crashes during a rebuild, you may lose that backup
> file, depending on the distro. OK. Is there a better solution to
> this? Unfortunately, at the time of the failure to shrink, the
> rebuild that failed to start, stdout and stderr were not going to
> /var/log/messages, so I have no idea what the complaint was at that
> time. Does this service send so much output to stdout/stderr that
> it's useful to suppress it? If I'd seen something in
> /var/log/messages, it would have been more clear that there was a
> service with a complaint that was the cause of the rebuild failing to
> start. I wouldn't have done as much thrashing trying to figure out
> why.
I don't use systemd so can't advise on this. Without systemd, mdadm
just runs mdmon in the background and it all just works.
>> These are the only operations you should have done in the first place.
>> Although I would have put the --add first, so the --fail operation would
>> have triggered a rebuild onto the spare right away.
>
> I did the fail/remove/add at the very end, after replacing the dead
> drive, after finally completing the "don't do it this way again"
> grow-to-5-then-shrink-to-4 process to replace the not-yet-dead drive.
> After the shrink finally completed, the new 4th drive showed as a
> spare and removed at the same time. i.e., this dump from my first
Growing and shrinking didn't do anything to replace your suspect drive.
It just moved the data blocks around on your other drives, all while
not redundant.
> EMail:
>
> Number Major Minor RaidDevice State
> 0 8 2 0 active sync /dev/sda2
> 1 8 17 1 active sync /dev/sdb1
> 5 8 33 2 active sync /dev/sdc1
> 6 0 0 6 removed
>
> 6 8 49 - spare /dev/sdd1
It seems there is a corner case where at completion of shrink where one
device becomes a spare, the new spare doesn't trigger the recovery code
to pull it into service.
Probably never noticed because reshaping a degraded array is *uncommon*.
:-)
This one is for Neil, I think...
Phil
^ permalink raw reply
* CPU requirements......
From: Another Sillyname @ 2015-07-13 16:14 UTC (permalink / raw)
To: linux-raid
I've been testing a test RAID6 setup with a 5970 CPU that has worked fine.
As this box is almost exclusively going to be a file server I'm
thinking of swapping to a G3258 processor, dual core, not overclocked
and no hyperthreading or turbo features.
The box may also be used to do transcodes of media files from HD mpeg
to x264 which is pretty CPU intensive......
Are there any likely issues with the RAID running on a substantially
slower CPU? I can't seem to find an recent/decent docs suggesting
minimum CPU requirements for MDADM raid configs.
Thanks
Tony
^ permalink raw reply
* Re: CPU requirements......
From: Jared Mauch @ 2015-07-13 16:26 UTC (permalink / raw)
To: Another Sillyname; +Cc: linux-raid
In-Reply-To: <CAOS+5GGAL-8R7ZNd563Z-5oXbRw1QAswOKv7DstT5cLFdyR+Sw@mail.gmail.com>
On Mon, Jul 13, 2015 at 05:14:51PM +0100, Another Sillyname wrote:
> I've been testing a test RAID6 setup with a 5970 CPU that has worked fine.
>
> As this box is almost exclusively going to be a file server I'm
> thinking of swapping to a G3258 processor, dual core, not overclocked
> and no hyperthreading or turbo features.
>
> The box may also be used to do transcodes of media files from HD mpeg
> to x264 which is pretty CPU intensive......
>
> Are there any likely issues with the RAID running on a substantially
> slower CPU? I can't seem to find an recent/decent docs suggesting
> minimum CPU requirements for MDADM raid configs.
you are likely to be I/O bound either on the drives
or on the controller card to the bus/cpu.
While imperfect, take a look at doing
hdparm -t -T /dev/device-name
This will give you an idea of what a userland process
will see as the performance for cached and non-cached reads.
simply: the more you can keep in memory or on fast media
the better off you are.
- Jared
--
Jared Mauch | pgp key available via finger from jared@puck.nether.net
clue++; | http://puck.nether.net/~jared/ My statements are only mine.
^ permalink raw reply
* Re: mdadm-grow-continue service crashing (similiar to "raid5 reshape is stuck" thread from May)
From: Wols Lists @ 2015-07-13 18:37 UTC (permalink / raw)
To: Edward Kuns, Phil Turmel; +Cc: linux-raid
In-Reply-To: <CACsGCyT5vgFkQEc1zgKCGgT_rO5V4cmuFmo9K4vwiqN4JsPLJA@mail.gmail.com>
On 12/07/15 20:24, Edward Kuns wrote:
>> > many modern distros delete /tmp on reboot and/or play
>> > games with namespaces to isolate different users' /tmp spaces.
> So if the machine crashes during a rebuild, you may lose that backup
> file, depending on the distro. OK.
Please note that this is the DEFINED behaviour of /tmp, so it has a very
high probability of happening.
If you want temporary data to survive a reboot, put it in /var/tmp.
Oh - and if SeLinux only lets you put it in /tmp, what happens if you
don't have a separate /tmp partition? You can't put the backup file on
the partition you are rebuilding, and SeLinux won't let you put it
anywhere else? That's a big disaster in the making ...
Cheers,
Wol
^ permalink raw reply
* Re: mdadm-grow-continue service crashing (similiar to "raid5 reshape is stuck" thread from May)
From: Edward Kuns @ 2015-07-13 21:38 UTC (permalink / raw)
To: Phil Turmel; +Cc: linux-raid, NeilBrown
In-Reply-To: <55A3C30F.404@turmel.org>
On Mon, Jul 13, 2015 at 8:54 AM, Phil Turmel <philip@turmel.org> wrote:
> Hi Eddie,
> On older kernels without support for --replace, the correct
> operation is --add spare then --fail, --remove.
Makes sense. That was my original plan, since I didn't know about the
replace option. Doing otherwise was a bad decision on my part.
To make sure I understand this: 1) If you start out with a 4-drive
healthy raid5 array and do add / fail / remove, the "fail" step
immediately removes that drive from being an active participant in the
array and causes the new drive to be populated with data recalculated
from parity, right? 2) The new drive will sit in the array as a
"spare" until it is needed, which doesn't happen until the "fail"
step? And, 3) The "replace" option, instead, does the logical
equivalent of moving all the data off one drive onto a spare but
doesn't involve the other drives in a parity recalculation?
>> it shouldn't risk the actual data stored on the RAID,should it?
>
> In theory, no. But the --grow operation has to move virtually every
> data block to a new location, and in your case, then back to its
> original location. Lots of unnecessary data movement that has a
> low but non-zero error-rate.
>
> Also, the complex operations in --grow have produced somewhat
> more than its fair share of mdadm bugs. Stuck reshapes are usually
> recoverable, but typically only with assistance from this list. Drive
> failures during reshapes can be particularly sticky, especially when
> the failure is of the device holding a critical section backup.
That all makes perfect sense, thanks.
> I don't use systemd so can't advise on this. Without systemd, mdadm
> just runs mdmon in the background and it all just works.
I can't exactly say I use it by choice. I'd change distros but that
would only delay the inevitable.
> Growing and shrinking didn't do anything to replace your suspect drive.
> It just moved the data blocks around on your other drives, all while
> not redundant.
I'm confused here. I started the grow 4->5 with a healthy raid5 with
4 drives. One of the four drives was "suspect" in that I expect it to
fail at some point in the near future -- but it hadn't yet failed. I
thought this grow would give me a raid with four data drives + one
parity drive, all working. (And it seemed to.) And then I could fail
the suspect drive and go back down to three data drives + one parity.
The final output of the shrink certainly agrees with what you say, but
I clearly don't understand it. I don't understand how going from 4
healthy drives to 5 healthy drives, and then failing and removing one
of them and shrinking back down to 4 drives, ended up with 3 good and
one spare. But that is what happened.
> It seems there is a corner case where at completion of shrink where one
> device becomes a spare, the new spare doesn't trigger the recovery code
> to pull it into service.
>
> Probably never noticed because reshaping a degraded array is *uncommon*.
> :-)
It would be nice if my error in judgement helps save someone else in
the future! If there is any data I can gather from my server that
will help, I can get it. Although I won't be reproducing this
experiment any time in the future on a server that has any data I care
about. But note that I didn't reshape a degraded array. I reshaped a
healthy array and ended up with a degraded one.
Eddie
^ permalink raw reply
* Re: mdadm-grow-continue service crashing (similiar to "raid5 reshape is stuck" thread from May)
From: Edward Kuns @ 2015-07-13 22:07 UTC (permalink / raw)
To: Wols Lists; +Cc: Phil Turmel, linux-raid
In-Reply-To: <55A4056C.6050004@youngman.org.uk>
On Mon, Jul 13, 2015 at 1:37 PM, Wols Lists <antlists@youngman.org.uk> wrote:
> Please note that this is the DEFINED behaviour of /tmp, so it has a very
> high probability of happening.
>
> If you want temporary data to survive a reboot, put it in /var/tmp.
OK. Looking more carefully, I see my "/tmp" partition is of type
tmpfs. So yes, on reboot it would have been totally clean, exactly as
you say. In my case, my /var partition was on the raid5 being
reshaped, so /var/tmp wasn't an option for me.
> Oh - and if SeLinux only lets you put it in /tmp, what happens if you
> don't have a separate /tmp partition? You can't put the backup file on
> the partition you are rebuilding, and SeLinux won't let you put it
> anywhere else? That's a big disaster in the making ...
Well, SELinux will let you put it anywhere that is labeled to allow
it. So if it needs to go on some folder that isn't labelled properly,
then some labeling needs to be done. I didn't want to deal with the
(minor) hassle of creating a label, of having to understand what label
would be appropriate, so I wanted to find a folder that was already
allowed by the existing labeling. So I tried a bunch of folders in
succession until one worked.
What is the interaction between the backup file I had to specify in
/lib/systemd/system/mdadm-grow-continue@.service and the backup file I
had to specify on the command line to do the --grow to shrink the
array? It kind of looks like the backup file on the "mdadm" command
line doesn't really matter, except that I had to specify one, because
"mdadm --grow --raid-devices=4 /dev/md125" wouldn't *try* to start
without a backup file specified, but then just crashed in the
mdadm-grow-continue service. Specifying a (different) backup file
there and restarting the service got the reshape to complete.
This raises a big question with SELinux. When a backup file is truly
needed, mdadm needs the ability to write the backup file to more than
one partition (not at a time, but in general), depending on which raid
device is being modified. This means that some custom labeling may
need to be done either in advance to prepare for recovery or
on-the-fly in a recovery situation.
Eddie
^ permalink raw reply
* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: NeilBrown @ 2015-07-13 22:22 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, songliubraving, hch, dan.j.williams, Kernel-team
In-Reply-To: <20150710174835.GA1837928@devbig257.prn2.facebook.com>
On Fri, 10 Jul 2015 10:48:45 -0700 Shaohua Li <shli@fb.com> wrote:
> On Fri, Jul 10, 2015 at 04:42:09PM +1000, NeilBrown wrote:
> > On Thu, 9 Jul 2015 22:18:15 -0700 Shaohua Li <shli@fb.com> wrote:
> >
> > > On Fri, Jul 10, 2015 at 03:10:44PM +1000, NeilBrown wrote:
> > > > On Thu, 9 Jul 2015 21:52:43 -0700 Shaohua Li <shli@fb.com> wrote:
> > > >
> > > > > On Fri, Jul 10, 2015 at 02:36:56PM +1000, NeilBrown wrote:
> > > > > > On Thu, 9 Jul 2015 21:08:49 -0700 Shaohua Li <shli@fb.com> wrote:
> > > > > >
> > > >
> > > > > > There is also the issue of what action commits a previous transaction.
> > > > > > I'm not sure what you had. I'm suggesting that each metadata block
> > > > > > commits previous transactions. Is that a close-enough match to what
> > > > > > you had?
> > > > >
> > > > > What did you mean about a transaction? In my implementation, metadata
> > > > > block and followed stripe data/parity consist of an io unit. io units can
> > > > > be finished out of order. but if io unit has flush request (the data has
> > > > > flush/flush bio or metadata is a flush block), the io unit can only
> > > > > start after all previous io units and disk cache flush finish. Such io
> > > > > unit is strictly ordered. The log patch describes this behavior. Does it
> > > > > match?
> > > >
> > > > Yes, a "transaction" is an "io unit". The flushing is the same.
> > > > I just couldn't remember how, when reading the log on restart, you
> > > > determined if a given "io unit" was reliably consistent, or whether it
> > > > should be ignored (having possibly only partially been written).
> > >
> > > The metadata block has a checksum for data of the block. data/parity has
> > > checksum stored in metadata block. This way we can know if metadata and
> > > data is consistent.
> > >
> >
> > OK .. though I'm not totally sold on the value of checksums. When a
> > checksum doesn't match, that means something. When a checksum does
> > match, it could just be a co-incidence.
> > I'd rather have a process that made checksums unnecessary, and only use
> > the checksums as a double-check.
>
> We could do something like: write metadata/data, wait, write another
> metadata. the second metadata indicates the first is in disk. But this
> can impact performance very much.
The performance consideration is why I suggested a double-buffered
approach. Write metadata1, data1, metadata2, data2, then don't write
metdata3 until metdata1 and data1 has been written.
I haven't actually tried that so I don't know for certain it would help.
> I think checksum should be fine. It
> might be just a coninsidence, but the rate should extremely low. jbd2 is
> using checksum too now.
Maybe I'll have a look at jbd2 - do you know what sort of checksum it
uses? I'd be surprised it didn't use something quite a bit stronger
than crc32 for a task like this.
NeilBrown
>
> Thanks,
> Shaohua
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: Shaohua Li @ 2015-07-13 22:35 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, songliubraving, hch, dan.j.williams, Kernel-team
In-Reply-To: <20150714082254.3889ef43@noble>
On Tue, Jul 14, 2015 at 08:22:54AM +1000, NeilBrown wrote:
> On Fri, 10 Jul 2015 10:48:45 -0700 Shaohua Li <shli@fb.com> wrote:
>
> > On Fri, Jul 10, 2015 at 04:42:09PM +1000, NeilBrown wrote:
> > > On Thu, 9 Jul 2015 22:18:15 -0700 Shaohua Li <shli@fb.com> wrote:
> > >
> > > > On Fri, Jul 10, 2015 at 03:10:44PM +1000, NeilBrown wrote:
> > > > > On Thu, 9 Jul 2015 21:52:43 -0700 Shaohua Li <shli@fb.com> wrote:
> > > > >
> > > > > > On Fri, Jul 10, 2015 at 02:36:56PM +1000, NeilBrown wrote:
> > > > > > > On Thu, 9 Jul 2015 21:08:49 -0700 Shaohua Li <shli@fb.com> wrote:
> > > > > > >
> > > > >
> > > > > > > There is also the issue of what action commits a previous transaction.
> > > > > > > I'm not sure what you had. I'm suggesting that each metadata block
> > > > > > > commits previous transactions. Is that a close-enough match to what
> > > > > > > you had?
> > > > > >
> > > > > > What did you mean about a transaction? In my implementation, metadata
> > > > > > block and followed stripe data/parity consist of an io unit. io units can
> > > > > > be finished out of order. but if io unit has flush request (the data has
> > > > > > flush/flush bio or metadata is a flush block), the io unit can only
> > > > > > start after all previous io units and disk cache flush finish. Such io
> > > > > > unit is strictly ordered. The log patch describes this behavior. Does it
> > > > > > match?
> > > > >
> > > > > Yes, a "transaction" is an "io unit". The flushing is the same.
> > > > > I just couldn't remember how, when reading the log on restart, you
> > > > > determined if a given "io unit" was reliably consistent, or whether it
> > > > > should be ignored (having possibly only partially been written).
> > > >
> > > > The metadata block has a checksum for data of the block. data/parity has
> > > > checksum stored in metadata block. This way we can know if metadata and
> > > > data is consistent.
> > > >
> > >
> > > OK .. though I'm not totally sold on the value of checksums. When a
> > > checksum doesn't match, that means something. When a checksum does
> > > match, it could just be a co-incidence.
> > > I'd rather have a process that made checksums unnecessary, and only use
> > > the checksums as a double-check.
> >
> > We could do something like: write metadata/data, wait, write another
> > metadata. the second metadata indicates the first is in disk. But this
> > can impact performance very much.
>
> The performance consideration is why I suggested a double-buffered
> approach. Write metadata1, data1, metadata2, data2, then don't write
> metdata3 until metdata1 and data1 has been written.
> I haven't actually tried that so I don't know for certain it would help.
Not sure if double buffer works, but you can't write metadata1 till
data1 hits to disk, which has big penality. The only possible way is to
origanize data/metadata as a big transaction so wait doesn't hurt too
much like jbd does.
> > I think checksum should be fine. It
> > might be just a coninsidence, but the rate should extremely low. jbd2 is
> > using checksum too now.
>
> Maybe I'll have a look at jbd2 - do you know what sort of checksum it
> uses? I'd be surprised it didn't use something quite a bit stronger
> than crc32 for a task like this.
It uses crc32. 32bits checksum for every 4k as far as I check.
Thanks,
Shaohua
^ permalink raw reply
* Re: CPU requirements......
From: Adam Goryachev @ 2015-07-13 23:10 UTC (permalink / raw)
To: Another Sillyname, linux-raid
In-Reply-To: <CAOS+5GGAL-8R7ZNd563Z-5oXbRw1QAswOKv7DstT5cLFdyR+Sw@mail.gmail.com>
On 14/07/15 02:14, Another Sillyname wrote:
> I've been testing a test RAID6 setup with a 5970 CPU that has worked fine.
>
> As this box is almost exclusively going to be a file server I'm
> thinking of swapping to a G3258 processor, dual core, not overclocked
> and no hyperthreading or turbo features.
>
> The box may also be used to do transcodes of media files from HD mpeg
> to x264 which is pretty CPU intensive......
>
> Are there any likely issues with the RAID running on a substantially
> slower CPU? I can't seem to find an recent/decent docs suggesting
> minimum CPU requirements for MDADM raid configs.
>
I'm not sure about the specific CPU's you are referencing, however, as I
understand it, RAID5 (and RAID6) are still single threaded (only one CPU
is used for calculating the parity) which is done during all writes, and
when degraded during all reads as well.
In addition, transcoding media files will also be very CPU intensive.
Some questions to ask:
1) How many transcodes do you want to do concurrently? Just one at a
time means you maybe only need one core for that.
2) Does transcoding a single file/stream utilise more than one core?
Having more cores will mean a faster result.
3) What is the read throughput you need to keep the transcoding process
CPU busy (ie, you want to be able to read the file from disk faster than
the CPU can do the work to convert the file)
4) What is the write throughput you need to save the resulting file from
the transcoding process (ie, if the transcode can't write the result,
then it will waste CPU time).
5) Don't forget you now have concurrent read/write on the same set of
disks, consider tuning the read ahead (read and cache more of the input
file) and potentially write buffers (assuming you can always recode the
input file if the system crashes during the process).
What is the goal?
a) Make sure it works (should not be a problem, just performance will be
slower)
b) Make the transcode as fast as possible with the given disks (If the
disks are slower than the slow CPU, then no problem, otherwise use the
faster CPU)
c) Make the transcode as fast as possible with the given CPU (either
one) (take your pick...)
You would probably need to do a lot of test and measure of the above.
The advantage you have is a well defined work load, that can be easily
tested. Just remember to record the results clearly showing what you
changed in each scenario. Though it might be difficult to "swap" CPU for
the various tests...
Hope that helps....
Regards,
Adam
--
Adam Goryachev
Website Managers
P: +61 2 8304 0000 adam@websitemanagers.com.au
F: +61 2 8304 0001 www.websitemanagers.com.au
^ permalink raw reply
* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: Shaohua Li @ 2015-07-15 0:45 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, songliubraving, hch, dan.j.williams, Kernel-team
In-Reply-To: <20150710143656.4ee7e647@noble>
On Fri, Jul 10, 2015 at 02:36:56PM +1000, NeilBrown wrote:
> On Thu, 9 Jul 2015 21:08:49 -0700 Shaohua Li <shli@fb.com> wrote:
>
> > On Fri, Jul 10, 2015 at 09:21:19AM +1000, NeilBrown wrote:
> > > On Tue, 7 Jul 2015 22:44:02 -0700 Shaohua Li <shli@fb.com> wrote:
> > >
> > > > On Wed, Jul 08, 2015 at 11:56:36AM +1000, NeilBrown wrote:
> > > > >
> > > > > Hi,
> > > > > I made some time to look at these this morning - sorry for the delay.
> > > > >
> > > > > Having it all broken down with more complete comments helps a lot -
> > > > > thanks.
> > > > >
> > > > > Unfortunately ... it helps confirm that I really don't like this. It
> > > > > seems much more complex that it should be. There certainly are a lot
> > > > > of details that need to be carefully considered, but the result needs
> > > > > to be simpler.
> > > > >
> > > > > A big part of my concern is that you choose to avoid making proper use
> > > > > of the current stripe cache. Your argument is that it is already
> > > > > too complex. That is probably true but I don't think you decrease
> > > > > total complexity by adding extra bits on the side that themselves are
> > > > > complex in completely different ways.
> > > > >
> > > > > I would like to start small and keep it as simple as possible. I
> > > > > think the minimum useful functionality is closing the write-hole.
> > > > > Anything else could be done outside raid5 (e.g. bcache), but the
> > > > > write-hole needs integration. Once that is closed, it may make sense
> > > > > to add more functionality.
> > > > >
> > > > > To do that we need a log, and it was quite sensible for you to put
> > > > > that first in the list of patches. So let's start with that.
> > > > >
> > > > > I would like to propose a single metadata block format. This block
> > > > > includes the general RAID5 parameters (both 'old' and 'new' for when a
> > > > > reshape is happening), it includes linkage to find nearby metadata.
> > > > > It includes a list of the data written after the metadata. And it
> > > > > includes a list of device-addresses of parity blocks which are now safe
> > > > > on the RAID and so any log content is now ignored.
> > > > >
> > > > > The general mechanism for writing to the log is:
> > > > > - collect a list of bios. As each arrives update the metadata
> > > > > block with index information.
> > > > > - when ready (there are various triggers) write the metadata block
> > > > > with FLUSH/FUA and write all the data blocks normally. This
> > > > > metadata block plus data/parity blocks is a 'transaction'.
> > > > >
> > > > > We never trigger a log transaction until all writes for the previous
> > > > > transaction have completed. This means that once the metadata block is
> > > > > safe, all previous data must also be safe. If this imposes too much
> > > > > waiting we could allow a "double-buffer" approach were each metadata
> > > > > block completes the previous-but-one transaction.
> > > > >
> > > > > When all of the previous writes complete we trigger a new log write
> > > > > if there is an outstanding SYNC write, or maybe an outstanding
> > > > > FLUSH/FUA, but otherwise we wait until the metadata block is full, or
> > > > > some amount of time has passed.
> > > > >
> > > > > This one metadata block serves all of the purposes that you
> > > > > identified. It records where data is, it commits previous writes, and
> > > > > records which stripes are being written to RAID and which have been
> > > > > fully committed.
> > > > >
> > > > > With that log in place, we add the code to "hijack ops_run_io" to
> > > > > write all dirty blocks from the stripe_head to the cache. This
> > > > > includes the parity of course. Immediately that data is safe the
> > > > > write requests can be returned and the data is written the the RAID
> > > > > devices.
> > > > >
> > > > > "Recovery" at boot time simply involves:
> > > > > 1. find and load first metadata block
> > > > > 2. load the "next" metadata block. If it doesn't exist, stop.
> > > > > 3. For each parity/data block listed in current block, get the
> > > > > stripe_head and read the block in.
> > > > > 4. For each completed stripe listed, find the stripe_head and
> > > > > invalidate it.
> > > > > 5. Make the "next" metadata block the current block and goto 2
> > > > > 6. Flush out all dirty data in stripe cache.
> > > > >
> > > > >
> > > > > I think that all of this *needs* to use the stripe_cache.
> > > > > Intercepting normal processing between party computation and writing
> > > > > to the RAID devices must involve the stripe_cache.
> > > > >
> > > > > So this is a "simple" start. Once this is written and agreed and
> > > > > tested and debugged, then we can look at the latency-hiding side of
> > > > > the cache. I think this should still use the stripe_cache - just
> > > > > schedule the writes to the log a bit earlier.
> > > > > If a case can be made for a separate cache then I'm not completely
> > > > > closed to the idea but I don't really want to think about it until the
> > > > > write-hole closing code is done.
> > > > >
> > > > > So:
> > > > > [PATCH V4 01/13] MD: add a new disk role to present cache device
> > > > > probably OK
> > > > > [PATCH V4 02/13] raid5: directly use mddev->queue
> > > > > OK
> > > > > [PATCH V4 03/13] raid5: cache log handling
> > > > > strip this down to a bare minimum. It needs:
> > > > > -on-disk metadata block format
> > > > > -two or three active transactions which identify a list of bios
> > > > > that are currently in the stripe cache. As the bio is added,
> > > > > its address info is added to the metadata block.
> > > > > -interface to "add a bio to a transaction" and "record a
> > > > > completed stripe".
> > > > > -timer flush the current transaction if previous one was not
> > > > > empty.
> > > > >
> > > > > I don't think there needs to be a separate superblock in the log.
> > > > > Each device in the array already has an 'md' superblock. Use e.g
> > > > > the 'recovery_offset' field in there (which is per-device and
> > > > > meaningless for a log) to store the address of the most recent
> > > > > metadata blocks at the time the superblock was written. Search
> > > > > forward and backward from there to find whole log. Each metadata
> > > > > block holds everything else that might be useful.
> > > > >
> > > > > [PATCH V4 05/13] raid5: cache reclaim support
> > > > > Just want the "hijack ops_run_io" part of this so that when normal
> > > > > RAID5 processing wants to write a stripe, blocks get diverted to
> > > > > the log first and further processing of the stripe is delayed until
> > > > > those writes complete and the transaction is safe.
> > > > >
> > > > > [PATCH V4 08/13] raid5: cache recovery support
> > > > > Just load all the log into the stripe cache.
> > > > >
> > > > >
> > > > > That should be much more manageable and would be a good start towards
> > > > > getting all the functionality that you want.
> > > > >
> > > > > Feel free to ask questions if I haven't explained things, or if you
> > > > > want me to look over the metadata format or whatever. I tend to answer
> > > > > direct questions more quickly than I answer 100kB patchsets :-)
> > > > Hi,
> > > >
> > > > So we are back to the original point (eg, only fix the write hole issue) after
> > > > 3 months development, which is really disappointed. I don't object just fixing
> > > > the write hole issue as a start, it's simple which is my arguement of first
> > > > post too, and probably a slight change of patch 3 meets the goal. The cache
> > > > does significantly increase complexity, which is well known when we move from
> > > > 'just fix write hole' to 'fix write hole and do cache'. But the code is right
> > > > there, I'm wondering how bad it is. Just because of stripe_cache? I don't see
> > > > any significant technical merit stripe_cache is a must-have here. You said the
> > > > code is complex. Yes, it is, but it's not because of the new data cache. It's a
> > > > simple radix tree and takes less than 20% of the total code. The remaining code
> > > > does what any log device should do (write to log, reclaim when log is full,
> > > > recovery when log crash). Similar things must be done even stripe_cache is
> > > > used. And if we don't use stripe_cache, we don't deeply couple with current
> > > > state machine, the new data cache actually brings flexibility (for example, we
> > > > can choose not using cache, or we can easily put the cache to NVRAM).
> > >
> > > I don't think we are completely back to the start. The product of
> > > recent months isn't just code - it is also your understanding and
> > > expertise. You've explored the problems and understand them. You
> > > could quickly see issues because you have struggled with them already.
> > >
> > > I think the principal of "write one to throw away" is a good one - and
> > > probably one we don't apply often enough. The first time you implement
> > > something you don't really know what you are doing, so the result is
> > > bound to be imperfect. The second time you have a much better idea of
> > > the big picture.
> > >
> > > The real big problem that I have with the current proposal is that I
> > > just don't understand it. Maybe that is my failing. But all the
> > > evidence suggests that I'll end up needing to maintain it, so I *have*
> > > to understand it.
> > >
> > > But let's not assume we are throwing everything away (but equally not
> > > promise that we won't) and start with just the first (non trivial)
> > > patch. The log.
> > >
> > > I think we do want a log. I think you have identified the key
> > > functionality that the log must provide. But I think the current
> > > layout is too complex. This is something that will be persistent on
> > > storage devices, so we want to get it as "right" as we can, early.
> > > It is fairly cheap to throw away a log and start a new one, so fixing
> > > design mistakes isn't too hard. But we do have to support any format
> > > we create indefinitely.
> > > So simple is best. Simple is most extensible.
> > >
> > > As I said: A single metadata block format, with:
> > > - forward/backward linkage - and probably a link to the "first" block
> > > in the log at the time of writing
> > > - array state data - brieifly
> > > - list of data/parity block addresses and maybe checksums
> > > - list of "completed" stripes
> > >
> > > The log is simply these metadata blocks mixed with the data/parity they
> > > they describe.
> > > Each metadata block commits all previous transactions (or maybe all but
> > > the most recent).
> > > The current md superblock contains the address of one of these metadata
> > > blocks.
> > >
> > > Does that seem reasonable? Does that seem simpler than what you had?
> > > Will that meet your perceived need? It meets mine.
> > > If we can do that, I'll apply it and we can move on to the next step.
> >
> > My original layout has 3 types of blocks: super block, metadata block for data/parity,
> > flush block to list completed stripes.
> >
> > Compared to your single metadata block format, your format
> > - delete super block. I'm fine with this. The only useful bits in my
> > super block is log tail and seq. we can overload MD superblock
> > recovery_offset/resync_offset to record seq/log tail.
> > - merge metadata block and flush block to one metadata block. That is
> > the new metadata block can record a mix of data/parity/'completed'
> > stripes. I don't object to this, it's doable, but don't think this
> > makes things simpler.
> >
> > If you insist, we can take this one. The new metadata block can record a
> > mix of data/parity/completed stripes, but it can also just record data
> > or completed stripes. So I'll change the disk format to cater this, but
> > I'll always put 'completed stripes' info into a separate metadata block
> > (that is not mixing with data/parity, but we still just have one
> > metadata type). That way I only need slight change of current code and
> > it works for the new format. We can, if necessary, let the new metadata
> > records any mix of data/parity/completed stripes later. Sounds good?
>
> Yes it does. Having a single sort of metadata block is an important
> part of the goal. How the code actually chooses to use these is a
> separate issue that can change harmlessly.
Taking a close look to reuse MD superblock for caching. It turns out to
be quite hacky. Suppose I use md_update_sb to update superblock when we
checkpoint the log. So I update corresponding fields of mddev
(resync_offset, recovery_offset). In md_update_sb, I must add a bunch of
'if (caching_disk) xxx' as raid disks shouldn't store the
resync_offset/recovery_offset. Or I can add a new cache_update_sb, but I
thought I must add the same hack code if we don't duplicate a lot of
code.
Adding a superblock for caching looks better. A 4k page data doesn't
increase any complexity. If you think we shouldn't duplicate too fileds
of md superblock to the cache superblock, we can store the most
necessary data in cache superblock.
Thanks,
Shaohua
^ permalink raw reply
* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: NeilBrown @ 2015-07-15 2:12 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, songliubraving, hch, dan.j.williams, Kernel-team
In-Reply-To: <20150715004455.GA1024035@devbig257.prn2.facebook.com>
On Tue, 14 Jul 2015 17:45:04 -0700 Shaohua Li <shli@fb.com> wrote:
> On Fri, Jul 10, 2015 at 02:36:56PM +1000, NeilBrown wrote:
> > Yes it does. Having a single sort of metadata block is an important
> > part of the goal. How the code actually chooses to use these is a
> > separate issue that can change harmlessly.
>
> Taking a close look to reuse MD superblock for caching. It turns out to
> be quite hacky. Suppose I use md_update_sb to update superblock when we
> checkpoint the log. So I update corresponding fields of mddev
> (resync_offset, recovery_offset). In md_update_sb, I must add a bunch of
> 'if (caching_disk) xxx' as raid disks shouldn't store the
> resync_offset/recovery_offset. Or I can add a new cache_update_sb, but I
> thought I must add the same hack code if we don't duplicate a lot of
> code.
in md_update_sb, in the loop:
/* First make sure individual recovery_offsets are correct */
rdev_for_each(rdev, mddev) {
if (rdev->raid_disk >= 0 &&
mddev->delta_disks >= 0 &&
!test_bit(In_sync, &rdev->flags) &&
mddev->curr_resync_completed > rdev->recovery_offset)
rdev->recovery_offset = mddev->curr_resync_completed;
}
add something like:
else if (rdev->is_cache)
rdev->recovery_offset =
mddev->cache->latest_checkpoint
In super_1_sync, where the code:
if (rdev->raid_disk >= 0 &&
!test_bit(In_sync, &rdev->flags)) {
sb->feature_map |=
cpu_to_le32(MD_FEATURE_RECOVERY_OFFSET);
sb->recovery_offset =
cpu_to_le64(rdev->recovery_offset);
if (rdev->saved_raid_disk >= 0 && mddev->bitmap)
sb->feature_map |=
cpu_to_le32(MD_FEATURE_RECOVERY_BITMAP);
}
is, add something like
else if (rdev->is_a_cache_disk) {
sb->feature_map |= MD_FEATURE_IMA_CACHE;
sb->recovery_offset = cpu_to_le64(rdev->recovery_Offset);
}
or just make the original code a little more general - I'm not sure
exactly how you flag the cache device.
You don't need to do this every time you checkpoint the log. The
pointer just needs to point to somewhere in the log so that the
start/end can be found (each metadata block points to the next one).
You could leave it until the log wraps completely, though that probably
isn't ideal.
So when you checkpoint the log, if the ->recovery_offset of the cache
device is more than (say) 25% behind the new checkpoint location, just
set MD_CHANGE_PENDING and wake the md thread.
I don't see that as particularly hackish.
Thanks,
NeilBrown
>
> Adding a superblock for caching looks better. A 4k page data doesn't
> increase any complexity. If you think we shouldn't duplicate too fileds
> of md superblock to the cache superblock, we can store the most
> necessary data in cache superblock.
>
> Thanks,
> Shaohua
^ permalink raw reply
* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: Shaohua Li @ 2015-07-15 3:16 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, songliubraving, hch, dan.j.williams, Kernel-team
In-Reply-To: <20150715121234.22803890@noble>
On Wed, Jul 15, 2015 at 12:12:34PM +1000, NeilBrown wrote:
> On Tue, 14 Jul 2015 17:45:04 -0700 Shaohua Li <shli@fb.com> wrote:
>
> > On Fri, Jul 10, 2015 at 02:36:56PM +1000, NeilBrown wrote:
>
> > > Yes it does. Having a single sort of metadata block is an important
> > > part of the goal. How the code actually chooses to use these is a
> > > separate issue that can change harmlessly.
> >
> > Taking a close look to reuse MD superblock for caching. It turns out to
> > be quite hacky. Suppose I use md_update_sb to update superblock when we
> > checkpoint the log. So I update corresponding fields of mddev
> > (resync_offset, recovery_offset). In md_update_sb, I must add a bunch of
> > 'if (caching_disk) xxx' as raid disks shouldn't store the
> > resync_offset/recovery_offset. Or I can add a new cache_update_sb, but I
> > thought I must add the same hack code if we don't duplicate a lot of
> > code.
>
> in md_update_sb, in the loop:
>
>
> /* First make sure individual recovery_offsets are correct */
> rdev_for_each(rdev, mddev) {
> if (rdev->raid_disk >= 0 &&
> mddev->delta_disks >= 0 &&
> !test_bit(In_sync, &rdev->flags) &&
> mddev->curr_resync_completed > rdev->recovery_offset)
> rdev->recovery_offset = mddev->curr_resync_completed;
>
> }
>
> add something like:
> else if (rdev->is_cache)
> rdev->recovery_offset =
> mddev->cache->latest_checkpoint
>
>
> In super_1_sync, where the code:
>
> if (rdev->raid_disk >= 0 &&
> !test_bit(In_sync, &rdev->flags)) {
> sb->feature_map |=
> cpu_to_le32(MD_FEATURE_RECOVERY_OFFSET);
> sb->recovery_offset =
> cpu_to_le64(rdev->recovery_offset);
> if (rdev->saved_raid_disk >= 0 && mddev->bitmap)
> sb->feature_map |=
> cpu_to_le32(MD_FEATURE_RECOVERY_BITMAP);
> }
>
> is, add something like
> else if (rdev->is_a_cache_disk) {
> sb->feature_map |= MD_FEATURE_IMA_CACHE;
> sb->recovery_offset = cpu_to_le64(rdev->recovery_Offset);
> }
>
> or just make the original code a little more general - I'm not sure
> exactly how you flag the cache device.
>
> You don't need to do this every time you checkpoint the log. The
> pointer just needs to point to somewhere in the log so that the
> start/end can be found (each metadata block points to the next one).
> You could leave it until the log wraps completely, though that probably
> isn't ideal.
>
> So when you checkpoint the log, if the ->recovery_offset of the cache
> device is more than (say) 25% behind the new checkpoint location, just
> set MD_CHANGE_PENDING and wake the md thread.
>
> I don't see that as particularly hackish.
The policy above about when superblock should be written is fine with
me, but I'd like to focus on where/how superblock should be written
here. I'd say exporting a structure of cache (the
cache->latest_checkpoint) to generic MD layer is very hackish.
md_update_sb writes all raid disks, that's bad since we just want to
update cache disk. Overloading some fields of MD superblock and using
them with some 'if (cache)' stuff is not natural way too. I don't
understand why you object adding a superblock for cache. The advantage
is it's self contained. And there is nothing about
complexity/maintaince, as we can store the most necessary fields into
the superblock.
Thanks,
Shaohua
^ permalink raw reply
* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: NeilBrown @ 2015-07-15 4:06 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, songliubraving, hch, dan.j.williams, Kernel-team
In-Reply-To: <20150715031615.GA1292235@devbig257.prn2.facebook.com>
On Tue, 14 Jul 2015 20:16:17 -0700 Shaohua Li <shli@fb.com> wrote:
> On Wed, Jul 15, 2015 at 12:12:34PM +1000, NeilBrown wrote:
> > On Tue, 14 Jul 2015 17:45:04 -0700 Shaohua Li <shli@fb.com> wrote:
> >
> > > On Fri, Jul 10, 2015 at 02:36:56PM +1000, NeilBrown wrote:
> >
> > > > Yes it does. Having a single sort of metadata block is an important
> > > > part of the goal. How the code actually chooses to use these is a
> > > > separate issue that can change harmlessly.
> > >
> > > Taking a close look to reuse MD superblock for caching. It turns out to
> > > be quite hacky. Suppose I use md_update_sb to update superblock when we
> > > checkpoint the log. So I update corresponding fields of mddev
> > > (resync_offset, recovery_offset). In md_update_sb, I must add a bunch of
> > > 'if (caching_disk) xxx' as raid disks shouldn't store the
> > > resync_offset/recovery_offset. Or I can add a new cache_update_sb, but I
> > > thought I must add the same hack code if we don't duplicate a lot of
> > > code.
> >
> > in md_update_sb, in the loop:
> >
> >
> > /* First make sure individual recovery_offsets are correct */
> > rdev_for_each(rdev, mddev) {
> > if (rdev->raid_disk >= 0 &&
> > mddev->delta_disks >= 0 &&
> > !test_bit(In_sync, &rdev->flags) &&
> > mddev->curr_resync_completed > rdev->recovery_offset)
> > rdev->recovery_offset = mddev->curr_resync_completed;
> >
> > }
> >
> > add something like:
> > else if (rdev->is_cache)
> > rdev->recovery_offset =
> > mddev->cache->latest_checkpoint
> >
> >
> > In super_1_sync, where the code:
> >
> > if (rdev->raid_disk >= 0 &&
> > !test_bit(In_sync, &rdev->flags)) {
> > sb->feature_map |=
> > cpu_to_le32(MD_FEATURE_RECOVERY_OFFSET);
> > sb->recovery_offset =
> > cpu_to_le64(rdev->recovery_offset);
> > if (rdev->saved_raid_disk >= 0 && mddev->bitmap)
> > sb->feature_map |=
> > cpu_to_le32(MD_FEATURE_RECOVERY_BITMAP);
> > }
> >
> > is, add something like
> > else if (rdev->is_a_cache_disk) {
> > sb->feature_map |= MD_FEATURE_IMA_CACHE;
> > sb->recovery_offset = cpu_to_le64(rdev->recovery_Offset);
> > }
> >
> > or just make the original code a little more general - I'm not sure
> > exactly how you flag the cache device.
> >
> > You don't need to do this every time you checkpoint the log. The
> > pointer just needs to point to somewhere in the log so that the
> > start/end can be found (each metadata block points to the next one).
> > You could leave it until the log wraps completely, though that probably
> > isn't ideal.
> >
> > So when you checkpoint the log, if the ->recovery_offset of the cache
> > device is more than (say) 25% behind the new checkpoint location, just
> > set MD_CHANGE_PENDING and wake the md thread.
> >
> > I don't see that as particularly hackish.
>
> The policy above about when superblock should be written is fine with
> me, but I'd like to focus on where/how superblock should be written
> here. I'd say exporting a structure of cache (the
> cache->latest_checkpoint) to generic MD layer is very hackish.
OK, get the cache code to write the desired value into the
recovery_offset field, so the md code only has to look at that.
The core md code does still need to know there is a cache, and which is
the cache device - it cannot be completely unaware...
> md_update_sb writes all raid disks, that's bad since we just want to
> update cache disk.
How bad? How often? Would you really be able to notice?
And having a per-device "update superblock" flag is not completely out
of the question. RAID10 could benefit from the clean/dirty state being
localized to the device which was actually dirty.
> Overloading some fields of MD superblock and using
> them with some 'if (cache)' stuff is not natural way too.
If we could foresee everything, we could assign everything its own
field. But unfortunately I didn't. That is why we have feature bits.
Different feature bits mean different fields have different meanings.
> I don't
> understand why you object adding a superblock for cache. The advantage
> is it's self contained. And there is nothing about
> complexity/maintaince, as we can store the most necessary fields into
> the superblock.
Because there is precisely 1 number that needs to be stored in the
superblock, and there seems no point having a superblock just to store
one number.
It isn't much extra complexity, but any extra thing is still an extra
thing.
Having the data section of the log device containing just a log is
elegant. Elegant is good.
If we decided that keeping two copies for superblocks was a good idea
(which I think it is, I just haven't created a "v1.3" layout yet), then
re-using the main superblock for the head-of-log pointer would instantly
give us two copies of that as well.
NeilBrown
^ permalink raw reply
* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: Shaohua Li @ 2015-07-15 19:49 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, songliubraving, hch, dan.j.williams, Kernel-team
In-Reply-To: <20150715140641.3c61cf78@noble>
On Wed, Jul 15, 2015 at 02:06:41PM +1000, NeilBrown wrote:
> On Tue, 14 Jul 2015 20:16:17 -0700 Shaohua Li <shli@fb.com> wrote:
>
> > On Wed, Jul 15, 2015 at 12:12:34PM +1000, NeilBrown wrote:
> > > On Tue, 14 Jul 2015 17:45:04 -0700 Shaohua Li <shli@fb.com> wrote:
> > >
> > > > On Fri, Jul 10, 2015 at 02:36:56PM +1000, NeilBrown wrote:
> > >
> > > > > Yes it does. Having a single sort of metadata block is an important
> > > > > part of the goal. How the code actually chooses to use these is a
> > > > > separate issue that can change harmlessly.
> > > >
> > > > Taking a close look to reuse MD superblock for caching. It turns out to
> > > > be quite hacky. Suppose I use md_update_sb to update superblock when we
> > > > checkpoint the log. So I update corresponding fields of mddev
> > > > (resync_offset, recovery_offset). In md_update_sb, I must add a bunch of
> > > > 'if (caching_disk) xxx' as raid disks shouldn't store the
> > > > resync_offset/recovery_offset. Or I can add a new cache_update_sb, but I
> > > > thought I must add the same hack code if we don't duplicate a lot of
> > > > code.
> > >
> > > in md_update_sb, in the loop:
> > >
> > >
> > > /* First make sure individual recovery_offsets are correct */
> > > rdev_for_each(rdev, mddev) {
> > > if (rdev->raid_disk >= 0 &&
> > > mddev->delta_disks >= 0 &&
> > > !test_bit(In_sync, &rdev->flags) &&
> > > mddev->curr_resync_completed > rdev->recovery_offset)
> > > rdev->recovery_offset = mddev->curr_resync_completed;
> > >
> > > }
> > >
> > > add something like:
> > > else if (rdev->is_cache)
> > > rdev->recovery_offset =
> > > mddev->cache->latest_checkpoint
> > >
> > >
> > > In super_1_sync, where the code:
> > >
> > > if (rdev->raid_disk >= 0 &&
> > > !test_bit(In_sync, &rdev->flags)) {
> > > sb->feature_map |=
> > > cpu_to_le32(MD_FEATURE_RECOVERY_OFFSET);
> > > sb->recovery_offset =
> > > cpu_to_le64(rdev->recovery_offset);
> > > if (rdev->saved_raid_disk >= 0 && mddev->bitmap)
> > > sb->feature_map |=
> > > cpu_to_le32(MD_FEATURE_RECOVERY_BITMAP);
> > > }
> > >
> > > is, add something like
> > > else if (rdev->is_a_cache_disk) {
> > > sb->feature_map |= MD_FEATURE_IMA_CACHE;
> > > sb->recovery_offset = cpu_to_le64(rdev->recovery_Offset);
> > > }
> > >
> > > or just make the original code a little more general - I'm not sure
> > > exactly how you flag the cache device.
> > >
> > > You don't need to do this every time you checkpoint the log. The
> > > pointer just needs to point to somewhere in the log so that the
> > > start/end can be found (each metadata block points to the next one).
> > > You could leave it until the log wraps completely, though that probably
> > > isn't ideal.
> > >
> > > So when you checkpoint the log, if the ->recovery_offset of the cache
> > > device is more than (say) 25% behind the new checkpoint location, just
> > > set MD_CHANGE_PENDING and wake the md thread.
> > >
> > > I don't see that as particularly hackish.
> >
> > The policy above about when superblock should be written is fine with
> > me, but I'd like to focus on where/how superblock should be written
> > here. I'd say exporting a structure of cache (the
> > cache->latest_checkpoint) to generic MD layer is very hackish.
>
> OK, get the cache code to write the desired value into the
> recovery_offset field, so the md code only has to look at that.
> The core md code does still need to know there is a cache, and which is
> the cache device - it cannot be completely unaware...
>
> > md_update_sb writes all raid disks, that's bad since we just want to
> > update cache disk.
>
> How bad? How often? Would you really be able to notice?
>
> And having a per-device "update superblock" flag is not completely out
> of the question. RAID10 could benefit from the clean/dirty state being
> localized to the device which was actually dirty.
>
> > Overloading some fields of MD superblock and using
> > them with some 'if (cache)' stuff is not natural way too.
>
> If we could foresee everything, we could assign everything its own
> field. But unfortunately I didn't. That is why we have feature bits.
> Different feature bits mean different fields have different meanings.
>
>
> > I don't
> > understand why you object adding a superblock for cache. The advantage
> > is it's self contained. And there is nothing about
> > complexity/maintaince, as we can store the most necessary fields into
> > the superblock.
>
> Because there is precisely 1 number that needs to be stored in the
> superblock, and there seems no point having a superblock just to store
> one number.
> It isn't much extra complexity, but any extra thing is still an extra
> thing.
> Having the data section of the log device containing just a log is
> elegant. Elegant is good.
> If we decided that keeping two copies for superblocks was a good idea
> (which I think it is, I just haven't created a "v1.3" layout yet), then
> re-using the main superblock for the head-of-log pointer would instantly
> give us two copies of that as well.
I think I need 2 fields to find log head/tail in recovery. Currently
cache superblock records checkpoint disk position (log tail) and
checkpoint sequence number, which can be used to find log head. Just
recording log tail doesn't work well (it might work, for example,
zeroing sectors before log head, so we can identify log head. But it's
really ugly and not efficient). I only found recovery_offset can be
overloaded. Do you have idea other fileds can be overloaded in MD
superblock?
Thanks,
Shaohua
^ permalink raw reply
* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: NeilBrown @ 2015-07-15 23:16 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, songliubraving, hch, dan.j.williams, Kernel-team
In-Reply-To: <20150715194927.GA3502691@devbig257.prn2.facebook.com>
On Wed, 15 Jul 2015 12:49:37 -0700 Shaohua Li <shli@fb.com> wrote:
> On Wed, Jul 15, 2015 at 02:06:41PM +1000, NeilBrown wrote:
> > On Tue, 14 Jul 2015 20:16:17 -0700 Shaohua Li <shli@fb.com> wrote:
> >
> >
> > > I don't
> > > understand why you object adding a superblock for cache. The advantage
> > > is it's self contained. And there is nothing about
> > > complexity/maintaince, as we can store the most necessary fields into
> > > the superblock.
> >
> > Because there is precisely 1 number that needs to be stored in the
> > superblock, and there seems no point having a superblock just to store
> > one number.
> > It isn't much extra complexity, but any extra thing is still an extra
> > thing.
> > Having the data section of the log device containing just a log is
> > elegant. Elegant is good.
> > If we decided that keeping two copies for superblocks was a good idea
> > (which I think it is, I just haven't created a "v1.3" layout yet), then
> > re-using the main superblock for the head-of-log pointer would instantly
> > give us two copies of that as well.
>
> I think I need 2 fields to find log head/tail in recovery. Currently
> cache superblock records checkpoint disk position (log tail) and
> checkpoint sequence number, which can be used to find log head. Just
> recording log tail doesn't work well (it might work, for example,
> zeroing sectors before log head, so we can identify log head. But it's
> really ugly and not efficient). I only found recovery_offset can be
> overloaded. Do you have idea other fileds can be overloaded in MD
> superblock?
If each metadata block contains
- a magic number
- a checksum of the block
- a sequence number
- a pointer to the "next" metadata block (which is equivalent to
the size of all described data)
- a pointer to the tail (oldest active metadata block).
Then given the address of any block in the log you can easily find the
head: walk the "next" pointers forward until you find a block
that has the wrong magic or checksum or sequence or previous pointer.
The last block that was consistent is the head.
You can then find the tail directly, and walk forward processing the
log.
Efficiency is not really an issue. On a clean shutdown (which should
be the norm), the md superblock will contain a pointer to the head, and
the "next" block after that can quickly be determined to be invalid.
On an unclean shutdown it is expected that we need to do a bit more
work, and skipping forward along the chain to find the head of the log
is the least of our worries.
NeilBrown
^ permalink raw reply
* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: Shaohua Li @ 2015-07-16 0:07 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, songliubraving, hch, dan.j.williams, Kernel-team
In-Reply-To: <20150716091653.7b970b32@noble>
On Thu, Jul 16, 2015 at 09:16:53AM +1000, NeilBrown wrote:
> On Wed, 15 Jul 2015 12:49:37 -0700 Shaohua Li <shli@fb.com> wrote:
>
> > On Wed, Jul 15, 2015 at 02:06:41PM +1000, NeilBrown wrote:
> > > On Tue, 14 Jul 2015 20:16:17 -0700 Shaohua Li <shli@fb.com> wrote:
> > >
>
> > >
> > > > I don't
> > > > understand why you object adding a superblock for cache. The advantage
> > > > is it's self contained. And there is nothing about
> > > > complexity/maintaince, as we can store the most necessary fields into
> > > > the superblock.
> > >
> > > Because there is precisely 1 number that needs to be stored in the
> > > superblock, and there seems no point having a superblock just to store
> > > one number.
> > > It isn't much extra complexity, but any extra thing is still an extra
> > > thing.
> > > Having the data section of the log device containing just a log is
> > > elegant. Elegant is good.
> > > If we decided that keeping two copies for superblocks was a good idea
> > > (which I think it is, I just haven't created a "v1.3" layout yet), then
> > > re-using the main superblock for the head-of-log pointer would instantly
> > > give us two copies of that as well.
> >
> > I think I need 2 fields to find log head/tail in recovery. Currently
> > cache superblock records checkpoint disk position (log tail) and
> > checkpoint sequence number, which can be used to find log head. Just
> > recording log tail doesn't work well (it might work, for example,
> > zeroing sectors before log head, so we can identify log head. But it's
> > really ugly and not efficient). I only found recovery_offset can be
> > overloaded. Do you have idea other fileds can be overloaded in MD
> > superblock?
>
> If each metadata block contains
> - a magic number
> - a checksum of the block
> - a sequence number
> - a pointer to the "next" metadata block (which is equivalent to
> the size of all described data)
> - a pointer to the tail (oldest active metadata block).
>
> Then given the address of any block in the log you can easily find the
> head: walk the "next" pointers forward until you find a block
> that has the wrong magic or checksum or sequence or previous pointer.
> The last block that was consistent is the head.
>
> You can then find the tail directly, and walk forward processing the
> log.
>
> Efficiency is not really an issue. On a clean shutdown (which should
> be the norm), the md superblock will contain a pointer to the head, and
> the "next" block after that can quickly be determined to be invalid.
> On an unclean shutdown it is expected that we need to do a bit more
> work, and skipping forward along the chain to find the head of the log
> is the least of our worries.
if superblock records 2 fileds (the log tail and the seq of log tail), metadata
block doesn't need 'a pointer to the tail (oldest active metadata block)'. The
log tail/seq pair can help us find log head easily. Adding a pointer to the
tail in every metadata block is definitionly worse than adding a filed in the
superblock.
Further, how can you handle the case when log winds. For example, initially the log is
................................
^ superblock points to here
then we add meta and wind
|meta n-1|meta n|meta 0|meta 1|....
^ superblock points to here
Next time we reload log, superblock points to a valid meta. recovery will think
this is an unclean shutdown, so we rescan the whole log disk (because all metas
are valid) and apply all the changes to raid array. this is terrible. But if
superblock stores both log tail and seq. we will find the meta 0 sequence
number doesn't match with superblock, recovery stops instantly.
Thanks,
Shaohua
^ permalink raw reply
* Re: [PATCH v2 1/3] PM suspend/hibernate: Call notifier after freezing processes
From: Rafael J. Wysocki @ 2015-07-16 1:02 UTC (permalink / raw)
To: Pali Rohár
Cc: Alasdair Kergon, Mike Snitzer, Neil Brown, Len Brown,
Pavel Machek, dm-devel, linux-raid, linux-kernel, linux-pm
In-Reply-To: <1434885634-19895-2-git-send-email-pali.rohar@gmail.com>
On Sunday, June 21, 2015 01:20:32 PM Pali Rohár wrote:
> To prevent race conditions on userspace processes with I/O some taks must be
> called after processes are freezed. This patch adds new events which are
> delivered by pm_notifier_call_chain() after freezing processes when doing
> suspend or hibernate action.
>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
> include/linux/suspend.h | 2 ++
> kernel/power/hibernate.c | 2 ++
> kernel/power/suspend.c | 4 +++-
> 3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index 5efe743..bc743c8 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -368,6 +368,8 @@ static inline bool hibernation_available(void) { return false; }
> #define PM_POST_SUSPEND 0x0004 /* Suspend finished */
> #define PM_RESTORE_PREPARE 0x0005 /* Going to restore a saved image */
> #define PM_POST_RESTORE 0x0006 /* Restore failed */
> +#define PM_HIBERNATION_AFTER_FREEZE 0x0007 /* After hibernation freeze */
> +#define PM_SUSPEND_AFTER_FREEZE 0x0008 /* After suspend freeze */
>
> extern struct mutex pm_mutex;
>
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 2329daa..184f7ee 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -671,6 +671,8 @@ int hibernate(void)
> if (error)
> goto Exit;
>
> + pm_notifier_call_chain(PM_HIBERNATION_AFTER_FREEZE);
Don't we need to check errors from these?
Also, if you're adding AFTER_FREEZE, it would be good to add BEFORE_THAW too
for symmetry.
> +
> lock_device_hotplug();
> /* Allocate memory management structures */
> error = create_basic_memory_bitmaps();
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index 8d7a1ef..ba2a945 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -277,8 +277,10 @@ static int suspend_prepare(suspend_state_t state)
> trace_suspend_resume(TPS("freeze_processes"), 0, true);
> error = suspend_freeze_processes();
> trace_suspend_resume(TPS("freeze_processes"), 0, false);
> - if (!error)
> + if (!error) {
> + pm_notifier_call_chain(PM_SUSPEND_AFTER_FREEZE);
> return 0;
> + }
>
> suspend_stats.failed_freeze++;
> dpm_save_failed_step(SUSPEND_FREEZE);
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: NeilBrown @ 2015-07-16 1:22 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, songliubraving, hch, dan.j.williams, Kernel-team
In-Reply-To: <20150716000739.GA4041321@devbig257.prn2.facebook.com>
On Wed, 15 Jul 2015 17:07:39 -0700 Shaohua Li <shli@fb.com> wrote:
> On Thu, Jul 16, 2015 at 09:16:53AM +1000, NeilBrown wrote:
> > On Wed, 15 Jul 2015 12:49:37 -0700 Shaohua Li <shli@fb.com> wrote:
> >
> > > On Wed, Jul 15, 2015 at 02:06:41PM +1000, NeilBrown wrote:
> > > > On Tue, 14 Jul 2015 20:16:17 -0700 Shaohua Li <shli@fb.com> wrote:
> > > >
> >
> > > >
> > > > > I don't
> > > > > understand why you object adding a superblock for cache. The advantage
> > > > > is it's self contained. And there is nothing about
> > > > > complexity/maintaince, as we can store the most necessary fields into
> > > > > the superblock.
> > > >
> > > > Because there is precisely 1 number that needs to be stored in the
> > > > superblock, and there seems no point having a superblock just to store
> > > > one number.
> > > > It isn't much extra complexity, but any extra thing is still an extra
> > > > thing.
> > > > Having the data section of the log device containing just a log is
> > > > elegant. Elegant is good.
> > > > If we decided that keeping two copies for superblocks was a good idea
> > > > (which I think it is, I just haven't created a "v1.3" layout yet), then
> > > > re-using the main superblock for the head-of-log pointer would instantly
> > > > give us two copies of that as well.
> > >
> > > I think I need 2 fields to find log head/tail in recovery. Currently
> > > cache superblock records checkpoint disk position (log tail) and
> > > checkpoint sequence number, which can be used to find log head. Just
> > > recording log tail doesn't work well (it might work, for example,
> > > zeroing sectors before log head, so we can identify log head. But it's
> > > really ugly and not efficient). I only found recovery_offset can be
> > > overloaded. Do you have idea other fileds can be overloaded in MD
> > > superblock?
> >
> > If each metadata block contains
> > - a magic number
> > - a checksum of the block
> > - a sequence number
> > - a pointer to the "next" metadata block (which is equivalent to
> > the size of all described data)
> > - a pointer to the tail (oldest active metadata block).
> >
> > Then given the address of any block in the log you can easily find the
> > head: walk the "next" pointers forward until you find a block
> > that has the wrong magic or checksum or sequence or previous pointer.
> > The last block that was consistent is the head.
> >
> > You can then find the tail directly, and walk forward processing the
> > log.
> >
> > Efficiency is not really an issue. On a clean shutdown (which should
> > be the norm), the md superblock will contain a pointer to the head, and
> > the "next" block after that can quickly be determined to be invalid.
> > On an unclean shutdown it is expected that we need to do a bit more
> > work, and skipping forward along the chain to find the head of the log
> > is the least of our worries.
>
> if superblock records 2 fileds (the log tail and the seq of log tail), metadata
> block doesn't need 'a pointer to the tail (oldest active metadata block)'. The
> log tail/seq pair can help us find log head easily. Adding a pointer to the
> tail in every metadata block is definitionly worse than adding a filed in the
> superblock.
I don't really follow... maybe we are confusing terms.
In my mind, the "head" is where new data gets written and the "tail" is
where the oldest data is - though I can see that the reverse could also
make sense.
Previously you said you wanted to record "checkpoint disk position and
checkpoint sequence number" which you also referred to as the "tail".
So maybe you mean the "tail" to be the most recent checkpoint?
In any case, wouldn't the checkpoint metadata records its own sequence
number? So why do you need to start the sequence number in the
superblock as well.
If you record a specific end of the log in the superblock,
then you need to update the superblock any time that end moves (don't
you?). If you just record some starting point for a search, then you
only need to update the superblock when that location might get
over-written soon.
I think it makes perfect sense for all the metadata blocks in the log
to be linked together. Whether they link to the previous or to the
first doesn't make a lot of difference.
By "first" here, I mean the block that has been in the log for the
longest, but still refers to "live" data.
>
> Further, how can you handle the case when log winds. For example, initially the log is
> ................................
> ^ superblock points to here
> then we add meta and wind
> |meta n-1|meta n|meta 0|meta 1|....
> ^ superblock points to here
>
> Next time we reload log, superblock points to a valid meta. recovery will think
> this is an unclean shutdown, so we rescan the whole log disk (because all metas
> are valid) and apply all the changes to raid array. this is terrible. But if
> superblock stores both log tail and seq. we will find the meta 0 sequence
> number doesn't match with superblock, recovery stops instantly.
There should always be at least one valid metadata block in the log,
and the superblock should point to one of those.
On restart, it is loaded and the 'next' pointers are followed until you
get to the end (most recently written) of the log. You confirm you are
still in the log as sequence numbers will be increasing.
After a clean shutdown, the 'next' pointer won't lead any where useful.
From the last metadata block, you work backwards to find the first.
Maybe you walk all the way following backwards links. Maybe each
metadata block has a link to the earliest block worth looking at.
Then you follow the log forward recording every block that is found,
and discarding blocks when you find a metadata block which says
something can be discarded. Once you have walked all the way from the
start to the end (from the earliest block to the most recently written
block) you have a complete list of all the live blocks that are in the
log. Then they can be written to the RAID.
I think you are suggesting that the thing stored in the superblock is
the address of the oldest valid block. I don't exactly object to that,
though I feel it would require updating the superblock more often
than needed. I don't think you need the seq number though.
On a clean shutdown you would write out a metadata with a new seq
number and store a pointer to that.
On restart, that metadata block is loaded and its sequence number
examined. Any other blocks in the log will have a lower sequence number
and so will be ignored.
NeilBrown
^ permalink raw reply
* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: Shaohua Li @ 2015-07-16 4:13 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, songliubraving, hch, dan.j.williams, Kernel-team
In-Reply-To: <20150716112217.3e2e685f@noble>
On Thu, Jul 16, 2015 at 11:22:17AM +1000, NeilBrown wrote:
> On Wed, 15 Jul 2015 17:07:39 -0700 Shaohua Li <shli@fb.com> wrote:
>
> > On Thu, Jul 16, 2015 at 09:16:53AM +1000, NeilBrown wrote:
> > > On Wed, 15 Jul 2015 12:49:37 -0700 Shaohua Li <shli@fb.com> wrote:
> > >
> > > > On Wed, Jul 15, 2015 at 02:06:41PM +1000, NeilBrown wrote:
> > > > > On Tue, 14 Jul 2015 20:16:17 -0700 Shaohua Li <shli@fb.com> wrote:
> > > > >
> > >
> > > > >
> > > > > > I don't
> > > > > > understand why you object adding a superblock for cache. The advantage
> > > > > > is it's self contained. And there is nothing about
> > > > > > complexity/maintaince, as we can store the most necessary fields into
> > > > > > the superblock.
> > > > >
> > > > > Because there is precisely 1 number that needs to be stored in the
> > > > > superblock, and there seems no point having a superblock just to store
> > > > > one number.
> > > > > It isn't much extra complexity, but any extra thing is still an extra
> > > > > thing.
> > > > > Having the data section of the log device containing just a log is
> > > > > elegant. Elegant is good.
> > > > > If we decided that keeping two copies for superblocks was a good idea
> > > > > (which I think it is, I just haven't created a "v1.3" layout yet), then
> > > > > re-using the main superblock for the head-of-log pointer would instantly
> > > > > give us two copies of that as well.
> > > >
> > > > I think I need 2 fields to find log head/tail in recovery. Currently
> > > > cache superblock records checkpoint disk position (log tail) and
> > > > checkpoint sequence number, which can be used to find log head. Just
> > > > recording log tail doesn't work well (it might work, for example,
> > > > zeroing sectors before log head, so we can identify log head. But it's
> > > > really ugly and not efficient). I only found recovery_offset can be
> > > > overloaded. Do you have idea other fileds can be overloaded in MD
> > > > superblock?
> > >
> > > If each metadata block contains
> > > - a magic number
> > > - a checksum of the block
> > > - a sequence number
> > > - a pointer to the "next" metadata block (which is equivalent to
> > > the size of all described data)
> > > - a pointer to the tail (oldest active metadata block).
> > >
> > > Then given the address of any block in the log you can easily find the
> > > head: walk the "next" pointers forward until you find a block
> > > that has the wrong magic or checksum or sequence or previous pointer.
> > > The last block that was consistent is the head.
> > >
> > > You can then find the tail directly, and walk forward processing the
> > > log.
> > >
> > > Efficiency is not really an issue. On a clean shutdown (which should
> > > be the norm), the md superblock will contain a pointer to the head, and
> > > the "next" block after that can quickly be determined to be invalid.
> > > On an unclean shutdown it is expected that we need to do a bit more
> > > work, and skipping forward along the chain to find the head of the log
> > > is the least of our worries.
> >
> > if superblock records 2 fileds (the log tail and the seq of log tail), metadata
> > block doesn't need 'a pointer to the tail (oldest active metadata block)'. The
> > log tail/seq pair can help us find log head easily. Adding a pointer to the
> > tail in every metadata block is definitionly worse than adding a filed in the
> > superblock.
>
> I don't really follow... maybe we are confusing terms.
> In my mind, the "head" is where new data gets written and the "tail" is
> where the oldest data is - though I can see that the reverse could also
> make sense.
we are in the same page about the terms.
> Previously you said you wanted to record "checkpoint disk position and
> checkpoint sequence number" which you also referred to as the "tail".
> So maybe you mean the "tail" to be the most recent checkpoint?
yes
> In any case, wouldn't the checkpoint metadata records its own sequence
> number? So why do you need to start the sequence number in the
> superblock as well.
The problem is you don't know if the metadata is valid/invalid (or
checkpointed) according to the sequence number stored in the metadata
itself. But if we store a sequence number in superblock, comparing the
sequence numebr in superblock and sequence in metadata let us know if
the metadata is valid.
> If you record a specific end of the log in the superblock,
> then you need to update the superblock any time that end moves (don't
> you?).
we record the tail. recovery will search from the tail
> If you just record some starting point for a search, then you
> only need to update the superblock when that location might get
> over-written soon.
that's what I'm doing. we only update the superblock when the location
can be reused.
> I think it makes perfect sense for all the metadata blocks in the log
> to be linked together. Whether they link to the previous or to the
> first doesn't make a lot of difference.
> By "first" here, I mean the block that has been in the log for the
> longest, but still refers to "live" data.
I just don't find how it's useful at all.
> >
> > Further, how can you handle the case when log winds. For example, initially the log is
> > ................................
> > ^ superblock points to here
> > then we add meta and wind
> > |meta n-1|meta n|meta 0|meta 1|....
> > ^ superblock points to here
> >
> > Next time we reload log, superblock points to a valid meta. recovery will think
> > this is an unclean shutdown, so we rescan the whole log disk (because all metas
> > are valid) and apply all the changes to raid array. this is terrible. But if
> > superblock stores both log tail and seq. we will find the meta 0 sequence
> > number doesn't match with superblock, recovery stops instantly.
>
> There should always be at least one valid metadata block in the log,
> and the superblock should point to one of those.
> On restart, it is loaded and the 'next' pointers are followed until you
> get to the end (most recently written) of the log. You confirm you are
> still in the log as sequence numbers will be increasing.
> After a clean shutdown, the 'next' pointer won't lead any where useful.
This is exactly the problem. Just storing tail in superblock can't
handle the log wind issue efficiently, because we can't know if the
metadata superblock pointed to is checkpointed or not. So we need
workaround. Here your workaround is always keeping one valid metadata
block in the log. If we store both checkpoint and sequence number in
superblock, we don't need the workaround.
The log will look like:
initially:
............
^super point here, super seq = 0
we add meta:
|meta 0|meta 1|...
^super point here, super seq = 0
We do a checkpoint, so space of meta 0 can be reused:
|meta 0|meta 1|...
^super point here, super seq = 1
if log rewind, we already checkpoint meta x:
|meta x|meta 1|...
^super point here, super seq = x + 1
it's easy to know meta 1 should be ignored since 1 != x + 1
This is the classic log structure implementation.
> From the last metadata block, you work backwards to find the first.
> Maybe you walk all the way following backwards links. Maybe each
> metadata block has a link to the earliest block worth looking at.
>
> Then you follow the log forward recording every block that is found,
> and discarding blocks when you find a metadata block which says
> something can be discarded. Once you have walked all the way from the
> start to the end (from the earliest block to the most recently written
> block) you have a complete list of all the live blocks that are in the
> log. Then they can be written to the RAID.
>
> I think you are suggesting that the thing stored in the superblock is
> the address of the oldest valid block. I don't exactly object to that,
> though I feel it would require updating the superblock more often
> than needed.
No, we only updating superblock in checkpoint.
> I don't think you need the seq number though.
> On a clean shutdown you would write out a metadata with a new seq
> number and store a pointer to that.
> On restart, that metadata block is loaded and its sequence number
> examined. Any other blocks in the log will have a lower sequence number
> and so will be ignored.
As I said, you can workaround the issue without recording seq number in
superblock, writing an extra metadata as you proposed for example. But
recording seq number makes things much easier and cleaner.
Thanks,
Shaohua
^ permalink raw reply
* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: NeilBrown @ 2015-07-16 6:07 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, songliubraving, hch, dan.j.williams, Kernel-team
In-Reply-To: <20150716041305.GA283306@devbig257.prn2.facebook.com>
On Wed, 15 Jul 2015 21:13:06 -0700 Shaohua Li <shli@fb.com> wrote:
> On Thu, Jul 16, 2015 at 11:22:17AM +1000, NeilBrown wrote:
> > On Wed, 15 Jul 2015 17:07:39 -0700 Shaohua Li <shli@fb.com> wrote:
> >
> > > On Thu, Jul 16, 2015 at 09:16:53AM +1000, NeilBrown wrote:
> > > > On Wed, 15 Jul 2015 12:49:37 -0700 Shaohua Li <shli@fb.com> wrote:
> > > >
> > > > > On Wed, Jul 15, 2015 at 02:06:41PM +1000, NeilBrown wrote:
> > > > > > On Tue, 14 Jul 2015 20:16:17 -0700 Shaohua Li <shli@fb.com> wrote:
> > > > > >
> > > >
> > > > > >
> > > > > > > I don't
> > > > > > > understand why you object adding a superblock for cache. The advantage
> > > > > > > is it's self contained. And there is nothing about
> > > > > > > complexity/maintaince, as we can store the most necessary fields into
> > > > > > > the superblock.
> > > > > >
> > > > > > Because there is precisely 1 number that needs to be stored in the
> > > > > > superblock, and there seems no point having a superblock just to store
> > > > > > one number.
> > > > > > It isn't much extra complexity, but any extra thing is still an extra
> > > > > > thing.
> > > > > > Having the data section of the log device containing just a log is
> > > > > > elegant. Elegant is good.
> > > > > > If we decided that keeping two copies for superblocks was a good idea
> > > > > > (which I think it is, I just haven't created a "v1.3" layout yet), then
> > > > > > re-using the main superblock for the head-of-log pointer would instantly
> > > > > > give us two copies of that as well.
> > > > >
> > > > > I think I need 2 fields to find log head/tail in recovery. Currently
> > > > > cache superblock records checkpoint disk position (log tail) and
> > > > > checkpoint sequence number, which can be used to find log head. Just
> > > > > recording log tail doesn't work well (it might work, for example,
> > > > > zeroing sectors before log head, so we can identify log head. But it's
> > > > > really ugly and not efficient). I only found recovery_offset can be
> > > > > overloaded. Do you have idea other fileds can be overloaded in MD
> > > > > superblock?
> > > >
> > > > If each metadata block contains
> > > > - a magic number
> > > > - a checksum of the block
> > > > - a sequence number
> > > > - a pointer to the "next" metadata block (which is equivalent to
> > > > the size of all described data)
> > > > - a pointer to the tail (oldest active metadata block).
> > > >
> > > > Then given the address of any block in the log you can easily find the
> > > > head: walk the "next" pointers forward until you find a block
> > > > that has the wrong magic or checksum or sequence or previous pointer.
> > > > The last block that was consistent is the head.
> > > >
> > > > You can then find the tail directly, and walk forward processing the
> > > > log.
> > > >
> > > > Efficiency is not really an issue. On a clean shutdown (which should
> > > > be the norm), the md superblock will contain a pointer to the head, and
> > > > the "next" block after that can quickly be determined to be invalid.
> > > > On an unclean shutdown it is expected that we need to do a bit more
> > > > work, and skipping forward along the chain to find the head of the log
> > > > is the least of our worries.
> > >
> > > if superblock records 2 fileds (the log tail and the seq of log tail), metadata
> > > block doesn't need 'a pointer to the tail (oldest active metadata block)'. The
> > > log tail/seq pair can help us find log head easily. Adding a pointer to the
> > > tail in every metadata block is definitionly worse than adding a filed in the
> > > superblock.
> >
> > I don't really follow... maybe we are confusing terms.
> > In my mind, the "head" is where new data gets written and the "tail" is
> > where the oldest data is - though I can see that the reverse could also
> > make sense.
>
> we are in the same page about the terms.
> > Previously you said you wanted to record "checkpoint disk position and
> > checkpoint sequence number" which you also referred to as the "tail".
> > So maybe you mean the "tail" to be the most recent checkpoint?
>
> yes
> > In any case, wouldn't the checkpoint metadata records its own sequence
> > number? So why do you need to start the sequence number in the
> > superblock as well.
>
> The problem is you don't know if the metadata is valid/invalid (or
> checkpointed) according to the sequence number stored in the metadata
> itself. But if we store a sequence number in superblock, comparing the
> sequence numebr in superblock and sequence in metadata let us know if
> the metadata is valid.
So it basically comes down to:
You think we should store extra information in the superblock to
validate the thing that the superblock points to.
I think we should always have the superblock pointing to something
that is valid.
>
> > If you record a specific end of the log in the superblock,
> > then you need to update the superblock any time that end moves (don't
> > you?).
>
> we record the tail. recovery will search from the tail
> > If you just record some starting point for a search, then you
> > only need to update the superblock when that location might get
> > over-written soon.
>
> that's what I'm doing. we only update the superblock when the location
> can be reused.
>
> > I think it makes perfect sense for all the metadata blocks in the log
> > to be linked together. Whether they link to the previous or to the
> > first doesn't make a lot of difference.
> > By "first" here, I mean the block that has been in the log for the
> > longest, but still refers to "live" data.
>
> I just don't find how it's useful at all.
It's not a show-stopper.
However if you keep the backwards links, then the superblock is free to
point to any metadata block anywhere in the log. That means we can let
the pointer in the superblock be updated lazily. The cache module only
needs to force a superblock update if there hasn't been one for any
other reason.
> > >
> > > Further, how can you handle the case when log winds. For example, initially the log is
> > > ................................
> > > ^ superblock points to here
> > > then we add meta and wind
> > > |meta n-1|meta n|meta 0|meta 1|....
> > > ^ superblock points to here
> > >
> > > Next time we reload log, superblock points to a valid meta. recovery will think
> > > this is an unclean shutdown, so we rescan the whole log disk (because all metas
> > > are valid) and apply all the changes to raid array. this is terrible. But if
> > > superblock stores both log tail and seq. we will find the meta 0 sequence
> > > number doesn't match with superblock, recovery stops instantly.
> >
> > There should always be at least one valid metadata block in the log,
> > and the superblock should point to one of those.
> > On restart, it is loaded and the 'next' pointers are followed until you
> > get to the end (most recently written) of the log. You confirm you are
> > still in the log as sequence numbers will be increasing.
> > After a clean shutdown, the 'next' pointer won't lead any where useful.
>
> This is exactly the problem. Just storing tail in superblock can't
> handle the log wind issue efficiently, because we can't know if the
> metadata superblock pointed to is checkpointed or not. So we need
> workaround. Here your workaround is always keeping one valid metadata
> block in the log. If we store both checkpoint and sequence number in
> superblock, we don't need the workaround.
I don't see that as a work-around. But then I don't really trust
checksums and so would rather that we always write a metadata block
(with FUA or FLUSH) after writing data, and using that to know that the
data is safe. If we did that, it would be natural to always have a
valid metadata block.
>
> The log will look like:
> initially:
> ............
> ^super point here, super seq = 0
>
> we add meta:
> |meta 0|meta 1|...
> ^super point here, super seq = 0
>
> We do a checkpoint, so space of meta 0 can be reused:
> |meta 0|meta 1|...
> ^super point here, super seq = 1
>
> if log rewind, we already checkpoint meta x:
> |meta x|meta 1|...
> ^super point here, super seq = x + 1
>
> it's easy to know meta 1 should be ignored since 1 != x + 1
> This is the classic log structure implementation.
>
> > From the last metadata block, you work backwards to find the first.
> > Maybe you walk all the way following backwards links. Maybe each
> > metadata block has a link to the earliest block worth looking at.
> >
> > Then you follow the log forward recording every block that is found,
> > and discarding blocks when you find a metadata block which says
> > something can be discarded. Once you have walked all the way from the
> > start to the end (from the earliest block to the most recently written
> > block) you have a complete list of all the live blocks that are in the
> > log. Then they can be written to the RAID.
> >
> > I think you are suggesting that the thing stored in the superblock is
> > the address of the oldest valid block. I don't exactly object to that,
> > though I feel it would require updating the superblock more often
> > than needed.
> No, we only updating superblock in checkpoint.
hm... what exactly do you mean by "checkpoint" here?
I imagine there would be more checkpoints than times when the
superblock really needs updating.
>
> > I don't think you need the seq number though.
> > On a clean shutdown you would write out a metadata with a new seq
> > number and store a pointer to that.
> > On restart, that metadata block is loaded and its sequence number
> > examined. Any other blocks in the log will have a lower sequence number
> > and so will be ignored.
>
> As I said, you can workaround the issue without recording seq number in
> superblock, writing an extra metadata as you proposed for example. But
> recording seq number makes things much easier and cleaner.
Clearly "cleaner" is in the eye of the beholder. I think it is much
clean for the superblock to point to something that is definitely valid.
NeilBrown
^ permalink raw reply
* Re: [PATCH v2 1/3] PM suspend/hibernate: Call notifier after freezing processes
From: Pali Rohár @ 2015-07-16 7:33 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Alasdair Kergon, Mike Snitzer, Neil Brown, Len Brown,
Pavel Machek, dm-devel, linux-raid, linux-kernel, linux-pm
In-Reply-To: <3729395.1oJCgMXcLU@vostro.rjw.lan>
On Thursday 16 July 2015 03:02:03 Rafael J. Wysocki wrote:
> On Sunday, June 21, 2015 01:20:32 PM Pali Rohár wrote:
> > To prevent race conditions on userspace processes with I/O some taks must be
> > called after processes are freezed. This patch adds new events which are
> > delivered by pm_notifier_call_chain() after freezing processes when doing
> > suspend or hibernate action.
> >
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > ---
> > include/linux/suspend.h | 2 ++
> > kernel/power/hibernate.c | 2 ++
> > kernel/power/suspend.c | 4 +++-
> > 3 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> > index 5efe743..bc743c8 100644
> > --- a/include/linux/suspend.h
> > +++ b/include/linux/suspend.h
> > @@ -368,6 +368,8 @@ static inline bool hibernation_available(void) { return false; }
> > #define PM_POST_SUSPEND 0x0004 /* Suspend finished */
> > #define PM_RESTORE_PREPARE 0x0005 /* Going to restore a saved image */
> > #define PM_POST_RESTORE 0x0006 /* Restore failed */
> > +#define PM_HIBERNATION_AFTER_FREEZE 0x0007 /* After hibernation freeze */
> > +#define PM_SUSPEND_AFTER_FREEZE 0x0008 /* After suspend freeze */
> >
> > extern struct mutex pm_mutex;
> >
> > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> > index 2329daa..184f7ee 100644
> > --- a/kernel/power/hibernate.c
> > +++ b/kernel/power/hibernate.c
> > @@ -671,6 +671,8 @@ int hibernate(void)
> > if (error)
> > goto Exit;
> >
> > + pm_notifier_call_chain(PM_HIBERNATION_AFTER_FREEZE);
>
> Don't we need to check errors from these?
>
If yes, what to do in this case? Fail hibernation and goto Exit?
> Also, if you're adding AFTER_FREEZE, it would be good to add BEFORE_THAW too
> for symmetry.
>
But there is no use case for BEFORE_THAW. At least it is not needed for now.
> > +
> > lock_device_hotplug();
> > /* Allocate memory management structures */
> > error = create_basic_memory_bitmaps();
> > diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> > index 8d7a1ef..ba2a945 100644
> > --- a/kernel/power/suspend.c
> > +++ b/kernel/power/suspend.c
> > @@ -277,8 +277,10 @@ static int suspend_prepare(suspend_state_t state)
> > trace_suspend_resume(TPS("freeze_processes"), 0, true);
> > error = suspend_freeze_processes();
> > trace_suspend_resume(TPS("freeze_processes"), 0, false);
> > - if (!error)
> > + if (!error) {
> > + pm_notifier_call_chain(PM_SUSPEND_AFTER_FREEZE);
> > return 0;
> > + }
> >
> > suspend_stats.failed_freeze++;
> > dpm_save_failed_step(SUSPEND_FREEZE);
> >
>
--
Pali Rohár
pali.rohar@gmail.com
^ permalink raw reply
* Re: [PATCH V4 00/13] MD: a caching layer for raid5/6
From: John Stoffel @ 2015-07-16 15:07 UTC (permalink / raw)
To: NeilBrown
Cc: Shaohua Li, linux-raid, songliubraving, hch, dan.j.williams,
Kernel-team
In-Reply-To: <20150716160711.08c37fc1@noble>
So.... how will wrapping of sequence numbers be handled in the cache
metadata on disk? They can't grow for ever...
John
^ 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