Mis-Design of Btrfs?

Good morning devs,

I'm wondering whether the raid- and volume-management-builtin of btrfs is actually a sane idea or not.
Currently we do have md/device-mapper support for raid already, btrfs lacks raid5 support and re-implements stuff that has already been done.

I'm aware of the fact that it is very useful to know on which devices we are in a filesystem. But I'm wondering, whether it wouldn't be smarter to generalise the information exposure through the VFS layer instead of replicating functionality:

Physical:   USB-HD   SSD   USB-Flash          | Exposes information to
Raid:       Raid1, Raid5, Raid10, etc.        | higher levels
Crypto:     Luks                              |
LVM:        Groups/Volumes                    |
FS:         xfs/jfs/reiser/ext3               v

Thus a filesystem like ext3 could be aware that it is running on a USB HD, enable -o sync be default or have the filesystem to rewrite blocks when running on crypto or optimise for an SSD, ...

Quoted 19 lines · On Thu, 23 Jun 2011 12:53:37 +0200 Nico Schottelius…

Good morning devs,

I'm wondering whether the raid- and volume-management-builtin of btrfs is actually a sane idea or not.
Currently we do have md/device-mapper support for raid already, btrfs lacks raid5 support and re-implements stuff that has already been done.

I'm aware of the fact that it is very useful to know on which devices we are in a filesystem. But I'm wondering, whether it wouldn't be smarter to generalise the information exposure through the VFS layer instead of replicating functionality:

Physical:   USB-HD   SSD   USB-Flash          | Exposes information to
Raid:       Raid1, Raid5, Raid10, etc.        | higher levels
Crypto:     Luks                              |
LVM:        Groups/Volumes                    |
FS:         xfs/jfs/reiser/ext3               v

Quoted 3 lines

Thus a filesystem like ext3 could be aware that it is running on a USB HD, enable -o sync be default or have the filesystem to rewrite blocks when running on crypto or optimise for an SSD, ...

I would certainly agree that exposing information to higher levels is a good idea. To some extent we do. But it isn't always as easy as it might sound. Choosing exactly what information to expose is the challenge. If you lack sufficient foresight you might expose something which turns out to be very specific to just one device, so all those upper levels which make use of the information find they are really special-casing one specific device, which isn't a good idea.

However it doesn't follow that RAID5 should not be implemented in BTRFS. The levels that you have drawn are just one perspective. While that has value, it may not be universal.
I could easily argue that the LVM layer is a mistake and that filesystems should provide that functionality directly.
I could almost argue the same for crypto.
RAID1 can make a lot of sense to be tightly integrated with the FS. RAID5 ... I'm less convinced, but then I have a vested interest there so that isn't an objective assessment.

Part of "the way Linux works" is that s/he who writes the code gets to make
the design decisions. The BTRFS developers might create something truly
awesome, or might end up having to support a RAID feature that they subsequently think is a bad idea. But it really is their decision to make.

NeilBrown

Quoted 44 lines · NeilBrown

Quoted 19 lines · On Thu, 23 Jun 2011 12:53:37 +0200 Nico Schottelius…

Good morning devs,

I'm wondering whether the raid- and volume-management-builtin of btrfs is actually a sane idea or not.
Currently we do have md/device-mapper support for raid already, btrfs lacks raid5 support and re-implements stuff that has already been done.

I'm aware of the fact that it is very useful to know on which devices we are in a filesystem. But I'm wondering, whether it wouldn't be smarter to generalise the information exposure through the VFS layer instead of replicating functionality:

Physical:   USB-HD   SSD   USB-Flash          | Exposes information to
Raid:       Raid1, Raid5, Raid10, etc.        | higher levels
Crypto:     Luks                              |
LVM:        Groups/Volumes                    |
FS:         xfs/jfs/reiser/ext3               v

Quoted 3 lines

Thus a filesystem like ext3 could be aware that it is running on a USB HD, enable -o sync be default or have the filesystem to rewrite blocks when running on crypto or optimise for an SSD, ...

I would certainly agree that exposing information to higher levels is a good idea. To some extent we do. But it isn't always as easy as it might sound. Choosing exactly what information to expose is the challenge. If you lack sufficient foresight you might expose something which turns out to be very specific to just one device, so all those upper levels which make use of the information find they are really special-casing one specific device, which isn't a good idea.

However it doesn't follow that RAID5 should not be implemented in BTRFS. The levels that you have drawn are just one perspective. While that has value, it may not be universal.
I could easily argue that the LVM layer is a mistake and that filesystems should provide that functionality directly.
I could almost argue the same for crypto.
RAID1 can make a lot of sense to be tightly integrated with the FS. RAID5 ... I'm less convinced, but then I have a vested interest there so that isn't an objective assessment.

Quoted 7 lines

Part of "the way Linux works" is that s/he who writes the code gets to make
the design decisions. The BTRFS developers might create something truly
awesome, or might end up having to support a RAID feature that they subsequently think is a bad idea. But it really is their decision to make.

NeilBrown

One more thing to add here is that I think that we still have a chance to increase the sharing between btrfs and the MD stack if we can get those changes made. No one likes to duplicate code, but we will need a richer interface between the block and file system layer to help close that gap.

Ric

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Is there a possibility that one could have a 3 disk RAID5 array, and then add a 4th disk and then do a balance, growing the RAID5 onto 4 disks and gaining the space still with RAID5? It seems that to be consistent, BTRFS would have to do this.

If this is the case, then I think that the BTRFS implementation of RA=

would have to be quite different to the MD implementation.

James.

Quoted 44 lines · NeilBrown

Quoted 19 lines · On Thu, 23 Jun 2011 12:53:37 +0200 Nico Schottelius…

Good morning devs,

I'm wondering whether the raid- and volume-management-builtin of btrfs is actually a sane idea or not.
Currently we do have md/device-mapper support for raid already, btrfs lacks raid5 support and re-implements stuff that has already been done.

I'm aware of the fact that it is very useful to know on which devices we are in a filesystem. But I'm wondering, whether it wouldn't be smarter to generalise the information exposure through the VFS layer instead of replicating functionality:

Physical:   USB-HD   SSD   USB-Flash          | Exposes information to
Raid:       Raid1, Raid5, Raid10, etc.        | higher levels
Crypto:     Luks                              |
LVM:        Groups/Volumes                    |
FS:         xfs/jfs/reiser/ext3               v

Quoted 3 lines

Thus a filesystem like ext3 could be aware that it is running on a USB HD, enable -o sync be default or have the filesystem to rewrite blocks when running on crypto or optimise for an SSD, ...

I would certainly agree that exposing information to higher levels is a good idea. To some extent we do. But it isn't always as easy as it might sound. Choosing exactly what information to expose is the challenge. If you lack sufficient foresight you might expose something which turns out to be very specific to just one device, so all those upper levels which make use of the information find they are really special-casing one specific device, which isn't a good idea.

However it doesn't follow that RAID5 should not be implemented in BTRFS. The levels that you have drawn are just one perspective. While that has value, it may not be universal.
I could easily argue that the LVM layer is a mistake and that filesystems should provide that functionality directly.
I could almost argue the same for crypto.
RAID1 can make a lot of sense to be tightly integrated with the FS. RAID5 ... I'm less convinced, but then I have a vested interest there so that isn't an objective assessment.

Quoted 7 lines

Part of "the way Linux works" is that s/he who writes the code gets to make
the design decisions. The BTRFS developers might create something truly
awesome, or might end up having to support a RAID feature that they subsequently think is a bad idea. But it really is their decision to make.

NeilBrown

One more thing to add here is that I think that we still have a chance to increase the sharing between btrfs and the MD stack if we can get those changes made. No one likes to duplicate code, but we will need a richer interface between the block and file system layer to help close that gap.

Ric

Quoted 57 lines · On Wed, 29 Jun 2011 10:29:53 +0100 Ric Wheeler<[email protected]> wrote:

Quoted 44 lines · NeilBrown

Quoted 19 lines · On Thu, 23 Jun 2011 12:53:37 +0200 Nico Schottelius…

Good morning devs,

I'm wondering whether the raid- and volume-management-builtin of btrfs is actually a sane idea or not.
Currently we do have md/device-mapper support for raid already, btrfs lacks raid5 support and re-implements stuff that has already been done.

I'm aware of the fact that it is very useful to know on which devices we are in a filesystem. But I'm wondering, whether it wouldn't be smarter to generalise the information exposure through the VFS layer instead of replicating functionality:

Physical:   USB-HD   SSD   USB-Flash          | Exposes information to
Raid:       Raid1, Raid5, Raid10, etc.        | higher levels
Crypto:     Luks                              |
LVM:        Groups/Volumes                    |
FS:         xfs/jfs/reiser/ext3               v

Quoted 3 lines

Thus a filesystem like ext3 could be aware that it is running on a USB HD, enable -o sync be default or have the filesystem to rewrite blocks when running on crypto or optimise for an SSD, ...

I would certainly agree that exposing information to higher levels is a good idea. To some extent we do. But it isn't always as easy as it might sound. Choosing exactly what information to expose is the challenge. If you lack sufficient foresight you might expose something which turns out to be very specific to just one device, so all those upper levels which make use of the information find they are really special-casing one specific device, which isn't a good idea.

However it doesn't follow that RAID5 should not be implemented in BTRFS. The levels that you have drawn are just one perspective. While that has value, it may not be universal.
I could easily argue that the LVM layer is a mistake and that filesystems should provide that functionality directly.
I could almost argue the same for crypto.
RAID1 can make a lot of sense to be tightly integrated with the FS. RAID5 ... I'm less convinced, but then I have a vested interest there so that isn't an objective assessment.

Quoted 7 lines

Part of "the way Linux works" is that s/he who writes the code gets to make
the design decisions. The BTRFS developers might create something truly
awesome, or might end up having to support a RAID feature that they subsequently think is a bad idea. But it really is their decision to make.

NeilBrown

One more thing to add here is that I think that we still have a chance to increase the sharing between btrfs and the MD stack if we can get those changes made. No one likes to duplicate code, but we will need a richer interface between the block and file system layer to help close that gap.

Quoted 2 lines

Ric

I'm certainly open to suggestions and collaboration. Do you have in mind any particular way to make the interface richer??

NeilBrown

Quoted 4 lines

I'm certainly open to suggestions and collaboration. Do you have in mind any particular way to make the interface richer??

NeilBrown

Hi Neil,

I know that Chris has a very specific set of use cases for btrfs and think that Alasdair and others have started to look at what is doable.

The obvious use case is the following:

If a file system uses checksumming or other data corruption detection bits, it can detect that it got bad data on a write. If that data was protected by RAID, it would like to ask the block layer to try to read from another mirror (for raid1) or try to validate/rebuild from parity.

Today, I think that a retry will basically just give us back a random chance of getting data from a different mirror or the same one that we got data from on the first go.

Chris, Alasdair, was that a good summary of one concern?

Thanks!

Ric

Quoted 20 lines · On Thu, 14 Jul 2011 07:02:22 +0100 Ric Wheeler<[email protected]> wrote:

Quoted 4 lines

I'm certainly open to suggestions and collaboration. Do you have in mind any particular way to make the interface richer??

NeilBrown

Hi Neil,

I know that Chris has a very specific set of use cases for btrfs and think that Alasdair and others have started to look at what is doable.

The obvious use case is the following:

If a file system uses checksumming or other data corruption detection bits, it can detect that it got bad data on a write. If that data was protected by RAID, it would like to ask the block layer to try to read from another mirror (for raid1) or try to validate/rebuild from parity.

Today, I think that a retry will basically just give us back a random chance of getting data from a different mirror or the same one that we got data from on the first go.

Quoted 5 lines

Chris, Alasdair, was that a good summary of one concern?

Thanks!

Ric

I imagine a new field in 'struct bio' which was normally zero but could be some small integer. It is only meaningful for read.
When 0 it means "get this data way you like".
When non-zero it means "get this data using method N", where the different methods are up to the device.

For a mirrored RAID, method N means read from device N-1. For stripe/parity RAID, method 1 means "use other data blocks and parity blocks to reconstruct data.

The default for non RAID devices is to return EINVAL for any N> 0. A remapping device (dm-linear, dm-stripe etc) would just pass the number down. I'm not sure how RAID1 over RAID5 would handle it... that might need some thought.

So if btrfs reads a block and the checksum looks wrong, it reads again with a larger N. It continues incrementing N and retrying until it gets a block that it likes or it gets EINVAL. There should probably be an error code (EAGAIN?) which means "I cannot work with that number, but try the next one".

It would be trivial for me to implement this for RAID1 and RAID10, and relatively easy for RAID5.
I'd need to give a bit of thought to RAID6 as there are possibly multiple ways to reconstruct from different combinations of parity and data. I'm not sure if there would be much point in doing that though.

It might make sense for a device to be able to report what the maximum 'N' supported is... that might make stacked raid easier to manage...

NeilBrown

Quoted 53 lines · On Thu, 14 Jul 2011 07:02:22 +0100 Ric Wheeler<[email protected]> wrote:

Quoted 21 lines

Quoted 5 lines

I'm certainly open to suggestions and collaboration. Do you have in mind any particular way to make the interface richer??

NeilBrown

Hi Neil,

I know that Chris has a very specific set of use cases for btrfs and think that Alasdair and others have started to look at what is doable.

The obvious use case is the following:

If a file system uses checksumming or other data corruption detection bits, it can detect that it got bad data on a write. If that data was protected by RAID, it would like to ask the block layer to try to read from another mirror (for raid1) or try to validate/rebuild from parity.

Today, I think that a retry will basically just give us back a random chance of getting data from a different mirror or the same one that we got data from on the first go.

Quoted 5 lines

Chris, Alasdair, was that a good summary of one concern?

Thanks!

Ric

I imagine a new field in 'struct bio' which was normally zero but could be some small integer. It is only meaningful for read.
When 0 it means "get this data way you like".
When non-zero it means "get this data using method N", where the different methods are up to the device.

For a mirrored RAID, method N means read from device N-1. For stripe/parity RAID, method 1 means "use other data blocks and parity blocks to reconstruct data.

The default for non RAID devices is to return EINVAL for any N> 0. A remapping device (dm-linear, dm-stripe etc) would just pass the number down. I'm not sure how RAID1 over RAID5 would handle it... that might need some thought.

So if btrfs reads a block and the checksum looks wrong, it reads again with a larger N. It continues incrementing N and retrying until it gets a block that it likes or it gets EINVAL. There should probably be an error code (EAGAIN?) which means "I cannot work with that number, but try the next one".

It would be trivial for me to implement this for RAID1 and RAID10, and relatively easy for RAID5.
I'd need to give a bit of thought to RAID6 as there are possibly multiple ways to reconstruct from different combinations of parity and data. I'm not sure if there would be much point in doing that though.

Quoted 5 lines

It might make sense for a device to be able to report what the maximum 'N' supported is... that might make stacked raid easier to manage...

NeilBrown

I think that the above makes sense. Not sure what your "0" definition is, but I would assume that for non-raided devices (i.e., a single s-ata disk), "0" would be an indication that there is nothing more that can be tried. The data you got is all you are ever going to get :)

For multiple mirrors, you might want to have a scheme where you would be able to cycle through the mirrors. You could retry, cycling through the various mirrors until you have tried and returned them all at which point you would get a "no more" error back or some such thing.

Excerpts from Ric Wheeler's message of 2011-07-14 02:57:54 -0400:

Quoted 65 lines · NeilBrown

Quoted 53 lines · On Thu, 14 Jul 2011 07:02:22 +0100 Ric Wheeler<[email protected]> wrote:

Quoted 21 lines

Quoted 5 lines

I'm certainly open to suggestions and collaboration. Do you have in mind any particular way to make the interface richer??

NeilBrown

Hi Neil,

I know that Chris has a very specific set of use cases for btrfs and think that Alasdair and others have started to look at what is doable.

The obvious use case is the following:

If a file system uses checksumming or other data corruption detection bits, it can detect that it got bad data on a write. If that data was protected by RAID, it would like to ask the block layer to try to read from another mirror (for raid1) or try to validate/rebuild from parity.

Today, I think that a retry will basically just give us back a random chance of getting data from a different mirror or the same one that we got data from on the first go.

Quoted 5 lines

Chris, Alasdair, was that a good summary of one concern?

Thanks!

Ric

I imagine a new field in 'struct bio' which was normally zero but could be some small integer. It is only meaningful for read.
When 0 it means "get this data way you like".
When non-zero it means "get this data using method N", where the different methods are up to the device.

For a mirrored RAID, method N means read from device N-1. For stripe/parity RAID, method 1 means "use other data blocks and parity blocks to reconstruct data.

The default for non RAID devices is to return EINVAL for any N> 0. A remapping device (dm-linear, dm-stripe etc) would just pass the number down. I'm not sure how RAID1 over RAID5 would handle it... that might need some thought.

So if btrfs reads a block and the checksum looks wrong, it reads again with a larger N. It continues incrementing N and retrying until it gets a block that it likes or it gets EINVAL. There should probably be an error code (EAGAIN?) which means "I cannot work with that number, but try the next one".

It would be trivial for me to implement this for RAID1 and RAID10, and relatively easy for RAID5.
I'd need to give a bit of thought to RAID6 as there are possibly multiple ways to reconstruct from different combinations of parity and data. I'm not sure if there would be much point in doing that though.

Quoted 5 lines

It might make sense for a device to be able to report what the maximum 'N' supported is... that might make stacked raid easier to manage...

NeilBrown

I think that the above makes sense. Not sure what your "0" definition is, but I would assume that for non-raided devices (i.e., a single s-ata disk), "0" would be an indication that there is nothing more that can be tried. The data you got is all you are ever going to get :)

Quoted 4 lines

For multiple mirrors, you might want to have a scheme where you would be able to cycle through the mirrors. You could retry, cycling through the various mirrors until you have tried and returned them all at which point you would get a "no more" error back or some such thing.

Hi everyone,

The mirror number idea is basically what btrfs does today, and I think it fits in with Neil's idea to have different policies for different blocks.

Basically what btrfs does:

read_block(block_num, mirror = 0)
if (no io error and not csum error)
horray()

num_mirrors = get_num_copies(block number)

for (i = 1; i < num_mirrors; i++) {
	read_block(block_num, mirror = i);
}

In a stacked configuration, the get_num_copies function can be smarter, basically adding up all the copies of the lower levels and finding a way to combine them. We could just send down a fake bio that is responsible for adding up the storage combinations into a bitmask or whatever works.

We could also just keep retrying until the lower layers reported no more mirror were available.

In btrfs at least, we don't set the data blocks up to date until the crc has passed, so replacing bogus blocks is easy. Metadata is a little more complex, but that's not really related to this topic.

mirror number 0 just means "no mirror preference/pick the fastest mirror" to the btrfs block mapping code.

Btrfs has the concept of different raid levels for different logical block numbers, so you get_num_copies might return one answer for block A and a different answer for block B.

Either way, we could easily make use of a bio field here if it were exported out.

Quoted 103 lines · Chris Mason

Excerpts from Ric Wheeler's message of 2011-07-14 02:57:54 -0400:

Quoted 65 lines · NeilBrown

Quoted 53 lines · On Thu, 14 Jul 2011 07:02:22 +0100 Ric Wheeler<[email protected]> wrote:

Quoted 21 lines

Quoted 5 lines

I'm certainly open to suggestions and collaboration. Do you have in mind any particular way to make the interface richer??

NeilBrown

Hi Neil,

I know that Chris has a very specific set of use cases for btrfs and think that Alasdair and others have started to look at what is doable.

The obvious use case is the following:

If a file system uses checksumming or other data corruption detection bits, it can detect that it got bad data on a write. If that data was protected by RAID, it would like to ask the block layer to try to read from another mirror (for raid1) or try to validate/rebuild from parity.

Today, I think that a retry will basically just give us back a random chance of getting data from a different mirror or the same one that we got data from on the first go.

Quoted 5 lines

Chris, Alasdair, was that a good summary of one concern?

Thanks!

Ric

I imagine a new field in 'struct bio' which was normally zero but could be some small integer. It is only meaningful for read.
When 0 it means "get this data way you like".
When non-zero it means "get this data using method N", where the different methods are up to the device.

For a mirrored RAID, method N means read from device N-1. For stripe/parity RAID, method 1 means "use other data blocks and parity blocks to reconstruct data.

The default for non RAID devices is to return EINVAL for any N> 0. A remapping device (dm-linear, dm-stripe etc) would just pass the number down. I'm not sure how RAID1 over RAID5 would handle it... that might need some thought.

So if btrfs reads a block and the checksum looks wrong, it reads again with a larger N. It continues incrementing N and retrying until it gets a block that it likes or it gets EINVAL. There should probably be an error code (EAGAIN?) which means "I cannot work with that number, but try the next one".

It would be trivial for me to implement this for RAID1 and RAID10, and relatively easy for RAID5.
I'd need to give a bit of thought to RAID6 as there are possibly multiple ways to reconstruct from different combinations of parity and data. I'm not sure if there would be much point in doing that though.

Quoted 5 lines

It might make sense for a device to be able to report what the maximum 'N' supported is... that might make stacked raid easier to manage...

NeilBrown

I think that the above makes sense. Not sure what your "0" definition is, but I would assume that for non-raided devices (i.e., a single s-ata disk), "0" would be an indication that there is nothing more that can be tried. The data you got is all you are ever going to get :)

Quoted 4 lines

For multiple mirrors, you might want to have a scheme where you would be able to cycle through the mirrors. You could retry, cycling through the various mirrors until you have tried and returned them all at which point you would get a "no more" error back or some such thing.

Hi everyone,

The mirror number idea is basically what btrfs does today, and I think it fits in with Neil's idea to have different policies for different blocks.

Basically what btrfs does:

read_block(block_num, mirror = 0)
if (no io error and not csum error)
horray()

num_mirrors = get_num_copies(block number)

for (i = 1; i < num_mirrors; i++) {
	read_block(block_num, mirror = i);
}

In a stacked configuration, the get_num_copies function can be smarter, basically adding up all the copies of the lower levels and finding a way to combine them. We could just send down a fake bio that is responsible for adding up the storage combinations into a bitmask or whatever works.

We could also just keep retrying until the lower layers reported no more mirror were available.

In btrfs at least, we don't set the data blocks up to date until the crc has passed, so replacing bogus blocks is easy. Metadata is a little more complex, but that's not really related to this topic.

mirror number 0 just means "no mirror preference/pick the fastest mirror" to the btrfs block mapping code.

Quoted 6 lines

Btrfs has the concept of different raid levels for different logical block numbers, so you get_num_copies might return one answer for block A and a different answer for block B.

Either way, we could easily make use of a bio field here if it were exported out.

you don't want to just pass the value down as that will cause problems with layering (especially if the lower layer supports more values than a higher layer)

I would suggest that each layer take the value it's give, do an integer division by the number of values that layer supports, using the modulo value for that layer and pass the rest of the result down to the next layer.

this works for just trying values until you get the error that tells you there are no more options.

things can get very 'intersesting' if the different options below you have different number of options (say a raid1 across a raid5 and a single disk) but I can't think of any good way to figure this out and assemble a sane order without doing an exaustive search down to find the number of options for each layer (and since this can be different for different blocks, you would have to do this each time you invoked this option)

David Lang

Quoted 113 lines · Chris Mason

Quoted 104 lines

Excerpts from Ric Wheeler's message of 2011-07-14 02:57:54 -0400:

Quoted 65 lines · NeilBrown

Quoted 53 lines · On Thu, 14 Jul 2011 07:02:22 +0100 Ric Wheeler<[email protected]> wrote:

Quoted 21 lines

Quoted 5 lines

I'm certainly open to suggestions and collaboration. Do you have in mind any particular way to make the interface richer??

NeilBrown

Hi Neil,

I know that Chris has a very specific set of use cases for btrfs and think that Alasdair and others have started to look at what is doable.

The obvious use case is the following:

If a file system uses checksumming or other data corruption detection bits, it can detect that it got bad data on a write. If that data was protected by RAID, it would like to ask the block layer to try to read from another mirror (for raid1) or try to validate/rebuild from parity.

Today, I think that a retry will basically just give us back a random chance of getting data from a different mirror or the same one that we got data from on the first go.

Quoted 5 lines

Chris, Alasdair, was that a good summary of one concern?

Thanks!

Ric

I imagine a new field in 'struct bio' which was normally zero but could be some small integer. It is only meaningful for read.
When 0 it means "get this data way you like".
When non-zero it means "get this data using method N", where the different methods are up to the device.

For a mirrored RAID, method N means read from device N-1. For stripe/parity RAID, method 1 means "use other data blocks and parity blocks to reconstruct data.

The default for non RAID devices is to return EINVAL for any N> 0. A remapping device (dm-linear, dm-stripe etc) would just pass the number down. I'm not sure how RAID1 over RAID5 would handle it... that might need some thought.

So if btrfs reads a block and the checksum looks wrong, it reads again with a larger N. It continues incrementing N and retrying until it gets a block that it likes or it gets EINVAL. There should probably be an error code (EAGAIN?) which means "I cannot work with that number, but try the next one".

It would be trivial for me to implement this for RAID1 and RAID10, and relatively easy for RAID5.
I'd need to give a bit of thought to RAID6 as there are possibly multiple ways to reconstruct from different combinations of parity and data. I'm not sure if there would be much point in doing that though.

Quoted 5 lines

It might make sense for a device to be able to report what the maximum 'N' supported is... that might make stacked raid easier to manage...

NeilBrown

I think that the above makes sense. Not sure what your "0" definition is, but I would assume that for non-raided devices (i.e., a single s-ata disk), "0" would be an indication that there is nothing more that can be tried. The data you got is all you are ever going to get :)

Quoted 4 lines

For multiple mirrors, you might want to have a scheme where you would be able to cycle through the mirrors. You could retry, cycling through the various mirrors until you have tried and returned them all at which point you would get a "no more" error back or some such thing.

Hi everyone,

The mirror number idea is basically what btrfs does today, and I think it fits in with Neil's idea to have different policies for different blocks.

Basically what btrfs does:

read_block(block_num, mirror = 0)
if (no io error and not csum error)
horray()

num_mirrors = get_num_copies(block number)

for (i = 1; i < num_mirrors; i++) {
    read_block(block_num, mirror = i);
}

In a stacked configuration, the get_num_copies function can be smarter, basically adding up all the copies of the lower levels and finding a way to combine them. We could just send down a fake bio that is responsible for adding up the storage combinations into a bitmask or whatever works.

We could also just keep retrying until the lower layers reported no more mirror were available.

In btrfs at least, we don't set the data blocks up to date until the crc has passed, so replacing bogus blocks is easy. Metadata is a little more complex, but that's not really related to this topic.

mirror number 0 just means "no mirror preference/pick the fastest mirror" to the btrfs block mapping code.

Quoted 6 lines

Btrfs has the concept of different raid levels for different logical block numbers, so you get_num_copies might return one answer for block A and a different answer for block B.

Either way, we could easily make use of a bio field here if it were exported out.

Quoted 8 lines

you don't want to just pass the value down as that will cause problems with layering (especially if the lower layer supports more values than a higher layer)

I would suggest that each layer take the value it's give, do an integer division by the number of values that layer supports, using the modulo value for that layer and pass the rest of the result down to the next layer.

I, on the other hand, would suggest that each layer be given the freedom, and the responsibility, to do whatever it thinks is most appropriate.

This would probably involved checking the lower levels to see how many strategies each has, and doing some appropriate arithmetic depending on how it combines those devices.

One problem here is the assumption that the lower levels don't change, and we know that not to be the case.
However this is already a problem. It is entirely possible that the request size parameters of devices can change at any moment, even while a request is in-flight ... though we try to avoid that or work around it.

The sort of dependencies that we see forming here would probably just make the problem worse.

Not sure what to do about it though.... maybe just hope it isn't a problem.

Excerpts from NeilBrown's message of 2011-07-15 02:33:54 -0400:

Quoted 132 lines · On Thu, 14 Jul 2011 21:58:46 -0700 (PDT) [email protected] wrote:

Quoted 110 lines · Chris Mason

Quoted 102 lines

Excerpts from Ric Wheeler's message of 2011-07-14 02:57:54 -0400:

Quoted 64 lines · NeilBrown

Quoted 53 lines · On Thu, 14 Jul 2011 07:02:22 +0100 Ric Wheeler<[email protected]> wrote:

Quoted 21 lines

Quoted 5 lines

I'm certainly open to suggestions and collaboration. Do you have in mind any particular way to make the interface richer??

NeilBrown

Hi Neil,

I know that Chris has a very specific set of use cases for btrfs and think that Alasdair and others have started to look at what is doable.

The obvious use case is the following:

If a file system uses checksumming or other data corruption detection bits, it can detect that it got bad data on a write. If that data was protected by RAID, it would like to ask the block layer to try to read from another mirror (for raid1) or try to validate/rebuild from parity.

Today, I think that a retry will basically just give us back a random chance of getting data from a different mirror or the same one that we got data from on the first go.

Quoted 5 lines

Chris, Alasdair, was that a good summary of one concern?

Thanks!

Ric

I imagine a new field in 'struct bio' which was normally zero but could be some small integer. It is only meaningful for read.
When 0 it means "get this data way you like".
When non-zero it means "get this data using method N", where the different methods are up to the device.

For a mirrored RAID, method N means read from device N-1. For stripe/parity RAID, method 1 means "use other data blocks and parity blocks to reconstruct data.

The default for non RAID devices is to return EINVAL for any N> 0.
A remapping device (dm-linear, dm-stripe etc) would just pass the number down. I'm not sure how RAID1 over RAID5 would handle it... that might need some thought.

So if btrfs reads a block and the checksum looks wrong, it reads again with a larger N. It continues incrementing N and retrying until it gets a block that it likes or it gets EINVAL. There should probably be an error code (EAGAIN?) which means "I cannot work with that number, but try the next one".

It would be trivial for me to implement this for RAID1 and RAID10, and relatively easy for RAID5.
I'd need to give a bit of thought to RAID6 as there are possibly multiple ways to reconstruct from different combinations of parity and data. I'm not sure if there would be much point in doing that though.

Quoted 5 lines

It might make sense for a device to be able to report what the maximum 'N' supported is... that might make stacked raid easier to manage...

NeilBrown

I think that the above makes sense. Not sure what your "0" definition is, but I would assume that for non-raided devices (i.e., a single s-ata disk), "0" would be an indication that there is nothing more that can be tried. The data you got is all you are ever going to get :)

Quoted 4 lines

For multiple mirrors, you might want to have a scheme where you would be able to cycle through the mirrors. You could retry, cycling through the various mirrors until you have tried and returned them all at which point you would get a "no more" error back or some such thing.

Hi everyone,

The mirror number idea is basically what btrfs does today, and I think it fits in with Neil's idea to have different policies for different blocks.

Basically what btrfs does:

read_block(block_num, mirror = 0)
if (no io error and not csum error)
horray()

num_mirrors = get_num_copies(block number)

for (i = 1; i<  num_mirrors; i++) {
     read_block(block_num, mirror = i);
}

In a stacked configuration, the get_num_copies function can be smarter, basically adding up all the copies of the lower levels and finding a way to combine them. We could just send down a fake bio that is responsible for adding up the storage combinations into a bitmask or whatever works.

We could also just keep retrying until the lower layers reported no more mirror were available.

In btrfs at least, we don't set the data blocks up to date until the crc has passed, so replacing bogus blocks is easy. Metadata is a little more complex, but that's not really related to this topic.

mirror number 0 just means "no mirror preference/pick the fastest mirror" to the btrfs block mapping code.

Quoted 6 lines

Btrfs has the concept of different raid levels for different logical block numbers, so you get_num_copies might return one answer for block A and a different answer for block B.

Either way, we could easily make use of a bio field here if it were exported out.

Quoted 8 lines

you don't want to just pass the value down as that will cause problems with layering (especially if the lower layer supports more values than a higher layer)

I would suggest that each layer take the value it's give, do an integer division by the number of values that layer supports, using the modulo value for that layer and pass the rest of the result down to the next layer.

I, on the other hand, would suggest that each layer be given the freedom, and the responsibility, to do whatever it thinks is most appropriate.

This would probably involved checking the lower levels to see how many strategies each has, and doing some appropriate arithmetic depending on how it combines those devices.

One problem here is the assumption that the lower levels don't change, and we know that not to be the case.
However this is already a problem. It is entirely possible that the request size parameters of devices can change at any moment, even while a request is in-flight ... though we try to avoid that or work around it.

Quoted 4 lines

The sort of dependencies that we see forming here would probably just make the problem worse.

Not sure what to do about it though.... maybe just hope it isn't a problem.

Agreed, if we want to go beyond best effort for a stacking config, we'll need to put some state struct in the bio that each layer can play with. That way each layer knows which mirrors have already been tried.

But, maybe the whole btrfs model is backwards for a generic layer. Instead of sending down ios and testing when they come back, we could just set a verification function (or stack of them?).

For metadata, btrfs compares the crc and a few other fields of the metadata block, so we can easily add a compare function pointer and a void * to pass in.

The problem is the crc can take a lot of CPU, so btrfs kicks it off to threading pools so saturate all the cpus on the box. But there's no reason we can't make that available lower down.

If we pushed the verification down, the retries could bubble up the stack instead of the other way around.

-chris

Quoted 16 lines

But, maybe the whole btrfs model is backwards for a generic layer. Instead of sending down ios and testing when they come back, we could just set a verification function (or stack of them?).

For metadata, btrfs compares the crc and a few other fields of the metadata block, so we can easily add a compare function pointer and a void * to pass in.

The problem is the crc can take a lot of CPU, so btrfs kicks it off to threading pools so saturate all the cpus on the box. But there's no reason we can't make that available lower down.

If we pushed the verification down, the retries could bubble up the stack instead of the other way around.

Quoted 1 line

-chris

I do like the idea of having the ability to do the verification and retries down the stack where you actually have the most context to figure out what is possible...

Why would you need to bubble back up anything other than an error when all retries have failed?

Excerpts from Ric Wheeler's message of 2011-07-15 08:58:04 -0400:

Quoted 1 line · Chris Mason

[ triggering IO retries on failed crc or other checks ]

Quoted 19 lines

Quoted 8 lines

But, maybe the whole btrfs model is backwards for a generic layer. Instead of sending down ios and testing when they come back, we could just set a verification function (or stack of them?).

For metadata, btrfs compares the crc and a few other fields of the metadata block, so we can easily add a compare function pointer and a void * to pass in.

Quoted 8 lines

The problem is the crc can take a lot of CPU, so btrfs kicks it off to threading pools so saturate all the cpus on the box. But there's no reason we can't make that available lower down.

If we pushed the verification down, the retries could bubble up the stack instead of the other way around.

-chris

I do like the idea of having the ability to do the verification and retries down the stack where you actually have the most context to figure out what is possible...

Quoted 2 lines

Why would you need to bubble back up anything other than an error when all retries have failed?

By bubble up I mean that if you have multiple layers capable of doing retries, the lowest levels would retry first. Basically by the time we get an -EIO_ALREADY_RETRIED we know there's nothing that lower level can do to help.

-chris

Quoted 25 lines

Excerpts from Ric Wheeler's message of 2011-07-15 08:58:04 -0400:

Quoted 1 line · Chris Mason

[ triggering IO retries on failed crc or other checks ]

Quoted 21 lines

Quoted 8 lines

But, maybe the whole btrfs model is backwards for a generic layer. Instead of sending down ios and testing when they come back, we could just set a verification function (or stack of them?).

For metadata, btrfs compares the crc and a few other fields of the metadata block, so we can easily add a compare function pointer and a void * to pass in.

Quoted 8 lines

The problem is the crc can take a lot of CPU, so btrfs kicks it off to threading pools so saturate all the cpus on the box. But there's no reason we can't make that available lower down.

If we pushed the verification down, the retries could bubble up the stack instead of the other way around.

-chris

I do like the idea of having the ability to do the verification and retries down the stack where you actually have the most context to figure out what is possible...

Why would you need to bubble back up anything other than an error when all retries have failed?

Quoted 6 lines

By bubble up I mean that if you have multiple layers capable of doing retries, the lowest levels would retry first. Basically by the time we get an -EIO_ALREADY_RETRIED we know there's nothing that lower level can do to help.

-chris

Absolutely sounds like the most sane way to go to me, thanks!

Excerpts from Ric Wheeler's message of 2011-07-15 09:31:37 -0400:

Quoted 26 lines · Chris Mason

Quoted 25 lines

Excerpts from Ric Wheeler's message of 2011-07-15 08:58:04 -0400:

Quoted 1 line · Chris Mason

[ triggering IO retries on failed crc or other checks ]

Quoted 21 lines

Quoted 8 lines

But, maybe the whole btrfs model is backwards for a generic layer. Instead of sending down ios and testing when they come back, we could just set a verification function (or stack of them?).

For metadata, btrfs compares the crc and a few other fields of the metadata block, so we can easily add a compare function pointer and a void * to pass in.

Quoted 8 lines

The problem is the crc can take a lot of CPU, so btrfs kicks it off to threading pools so saturate all the cpus on the box. But there's no reason we can't make that available lower down.

If we pushed the verification down, the retries could bubble up the stack instead of the other way around.

-chris

I do like the idea of having the ability to do the verification and retries down the stack where you actually have the most context to figure out what is possible...

Why would you need to bubble back up anything other than an error when all retries have failed?

Quoted 9 lines

Quoted 6 lines

By bubble up I mean that if you have multiple layers capable of doing retries, the lowest levels would retry first. Basically by the time we get an -EIO_ALREADY_RETRIED we know there's nothing that lower level can do to help.

-chris

Absolutely sounds like the most sane way to go to me, thanks!

It really seemed like a good idea, but I just realized it doesn't work well when parts of the stack transform the data.

Picture dm-crypt on top of raid1. If raid1 is responsible for the crc retries, there's no way to crc the data because it needs to be decrypted first.

I think the raided dm-crypt config is much more common (and interesting) than multiple layers that can retry for other reasons (raid1 on top of raid10?)

In other words, do we really want to do a lot of design work for multiple layers where each one maintains multiple copies of the data blocks? Are there configs where this really makes sense?

Quoted 40 lines

Excerpts from Ric Wheeler's message of 2011-07-15 09:31:37 -0400:

Quoted 26 lines · Chris Mason

Quoted 25 lines

Excerpts from Ric Wheeler's message of 2011-07-15 08:58:04 -0400:

Quoted 1 line · Chris Mason

[ triggering IO retries on failed crc or other checks ]

Quoted 21 lines

Quoted 8 lines

But, maybe the whole btrfs model is backwards for a generic layer. Instead of sending down ios and testing when they come back, we could just set a verification function (or stack of them?).

For metadata, btrfs compares the crc and a few other fields of the metadata block, so we can easily add a compare function pointer and a void * to pass in.

Quoted 8 lines

The problem is the crc can take a lot of CPU, so btrfs kicks it off to threading pools so saturate all the cpus on the box. But there's no reason we can't make that available lower down.

If we pushed the verification down, the retries could bubble up the stack instead of the other way around.

-chris

I do like the idea of having the ability to do the verification and retries down the stack where you actually have the most context to figure out what is possible...

Why would you need to bubble back up anything other than an error when all retries have failed?

Quoted 9 lines

Quoted 6 lines

By bubble up I mean that if you have multiple layers capable of doing retries, the lowest levels would retry first. Basically by the time we get an -EIO_ALREADY_RETRIED we know there's nothing that lower level can do to help.

-chris

Absolutely sounds like the most sane way to go to me, thanks!

It really seemed like a good idea, but I just realized it doesn't work well when parts of the stack transform the data.

Quoted 7 lines

Picture dm-crypt on top of raid1. If raid1 is responsible for the crc retries, there's no way to crc the data because it needs to be decrypted first.

I think the raided dm-crypt config is much more common (and interesting) than multiple layers that can retry for other reasons (raid1 on top of raid10?)

Isn't this a case where the transformative mid-layer would replace
the validation function before passing it down the stack? So btrfs hands dm-crypt a checksum function; dm-crypt then stores that function for its own purposes and hands off a new function to the DM layer below that which decrypts the data and calls the btrfs checksum function it stored earlier.

Excerpts from Hugo Mills's message of 2011-07-15 10:07:24 -0400:

Quoted 4 lines · Chris Mason

Quoted 3 lines

Excerpts from Ric Wheeler's message of 2011-07-15 09:31:37 -0400:

Quoted 2 lines · Chris Mason

Quoted 1 line

Excerpts from Ric Wheeler's message of 2011-07-15 08:58:04 -0=

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 1 line · Chris Mason

[ triggering IO retries on failed crc or other checks ]

Quoted 1 line

Quoted 1 line

But, maybe the whole btrfs model is backwards for a generic=

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Instead of sending down ios and testing when they come back=

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

just set a verification function (or stack of them?).

For metadata, btrfs compares the crc and a few other fields=

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

metadata block, so we can easily add a compare function poi=

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

void * to pass in.

The problem is the crc can take a lot of CPU, so btrfs kick=

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

threading pools so saturate all the cpus on the box. But t=

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

reason we can't make that available lower down.

If we pushed the verification down, the retries could bubbl=

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

stack instead of the other way around.

-chris

I do like the idea of having the ability to do the verificat=

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

the stack where you actually have the most context to figure=

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Why would you need to bubble back up anything other than an =

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 1 line

retries have failed?

By bubble up I mean that if you have multiple layers capable =

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

retries, the lowest levels would retry first. Basically by t=

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

get an -EIO_ALREADY_RETRIED we know there's nothing that lowe=

Quoted 8 lines

Quoted 8 lines

Quoted 6 lines

Quoted 3 lines

do to help.

-chris

=20
Absolutely sounds like the most sane way to go to me, thanks! =20

=20
It really seemed like a good idea, but I just realized it doesn't=

Quoted 3 lines

Quoted 3 lines

well when parts of the stack transform the data.
=20
Picture dm-crypt on top of raid1. If raid1 is responsible for th=

Quoted 1 line

Quoted 1 line

crc retries, there's no way to crc the data because it needs to b=

Quoted 3 lines

Quoted 3 lines

decrypted first.
=20
I think the raided dm-crypt config is much more common (and inter=

Quoted 1 line

Quoted 1 line

than multiple layers that can retry for other reasons (raid1 on t=

Quoted 3 lines

Quoted 1 line

raid10?)

=20
Isn't this a case where the transformative mid-layer would repla=

Quoted 2 lines

the validation function before passing it down the stack? So btrfs hands dm-crypt a checksum function; dm-crypt then stores that funct=

Quoted 3 lines

for its own purposes and hands off a new function to the DM layer below that which decrypts the data and calls the btrfs checksum function it stored earlier.

=20
Then we're requiring each transformation layer to have their own crcs=

and if the higher layers have a stronger crc (or other checks), there=

no path to ask the lower layers for other copies.
=20
Here's a concrete example. In each metadata block, btrfs stores the fsid and the transid of the transaction that created it. In the case=

a missed write, we'll read a perfect block from the lower layers. An=

crcs will be correct and it'll pass through dm-crypt with flying colo=

=20
But, it won't be the right block. Btrfs will notice this and EIO. I=

the current ask-for-another-mirror config we'll go down and grab the other copy.
=20
In the stacked validation function model, dm-crypt replaces our verification functions with something that operates on the encrypted data, and it won't be able to detect the error or kick down to the underlying raid1 for another copy.
=20
-chris
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs=

the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Excerpts from Hugo Mills's message of 2011-07-15 10:07:24 -0400:

Quoted 49 lines · Chris Mason

Quoted 40 lines

Excerpts from Ric Wheeler's message of 2011-07-15 09:31:37 -0400:

Quoted 26 lines · Chris Mason

Quoted 25 lines

Excerpts from Ric Wheeler's message of 2011-07-15 08:58:04 -0400:

Quoted 1 line · Chris Mason

[ triggering IO retries on failed crc or other checks ]

Quoted 21 lines

Quoted 8 lines

But, maybe the whole btrfs model is backwards for a generic layer. Instead of sending down ios and testing when they come back, we could just set a verification function (or stack of them?).

For metadata, btrfs compares the crc and a few other fields of the metadata block, so we can easily add a compare function pointer and a void * to pass in.

Quoted 8 lines

The problem is the crc can take a lot of CPU, so btrfs kicks it off to threading pools so saturate all the cpus on the box. But there's no reason we can't make that available lower down.

If we pushed the verification down, the retries could bubble up the stack instead of the other way around.

-chris

I do like the idea of having the ability to do the verification and retries down the stack where you actually have the most context to figure out what is possible...

Why would you need to bubble back up anything other than an error when all retries have failed?

Quoted 9 lines

Quoted 6 lines

By bubble up I mean that if you have multiple layers capable of doing retries, the lowest levels would retry first. Basically by the time we get an -EIO_ALREADY_RETRIED we know there's nothing that lower level can do to help.

-chris

Absolutely sounds like the most sane way to go to me, thanks!

It really seemed like a good idea, but I just realized it doesn't work well when parts of the stack transform the data.

Quoted 7 lines

Picture dm-crypt on top of raid1. If raid1 is responsible for the crc retries, there's no way to crc the data because it needs to be decrypted first.

I think the raided dm-crypt config is much more common (and interesting) than multiple layers that can retry for other reasons (raid1 on top of raid10?)

Quoted 6 lines

Isn't this a case where the transformative mid-layer would replace
the validation function before passing it down the stack? So btrfs hands dm-crypt a checksum function; dm-crypt then stores that function for its own purposes and hands off a new function to the DM layer below that which decrypts the data and calls the btrfs checksum function it stored earlier.

Then we're requiring each transformation layer to have their own crcs, and if the higher layers have a stronger crc (or other checks), there's no path to ask the lower layers for other copies.

Here's a concrete example. In each metadata block, btrfs stores the fsid and the transid of the transaction that created it. In the case of a missed write, we'll read a perfect block from the lower layers. Any crcs will be correct and it'll pass through dm-crypt with flying colors.

But, it won't be the right block. Btrfs will notice this and EIO. In the current ask-for-another-mirror config we'll go down and grab the other copy.

In the stacked validation function model, dm-crypt replaces our verification functions with something that operates on the encrypted data, and it won't be able to detect the error or kick down to the underlying raid1 for another copy.

Quoted 66 lines

Excerpts from Hugo Mills's message of 2011-07-15 10:07:24 -0400:

Quoted 49 lines · Chris Mason

Quoted 40 lines

Excerpts from Ric Wheeler's message of 2011-07-15 09:31:37 -0400:

Quoted 26 lines · Chris Mason

Quoted 25 lines

Excerpts from Ric Wheeler's message of 2011-07-15 08:58:04 -0400:

Quoted 1 line · Chris Mason

[ triggering IO retries on failed crc or other checks ]

Quoted 21 lines

Quoted 8 lines

But, maybe the whole btrfs model is backwards for a generic layer. Instead of sending down ios and testing when they come back, we could just set a verification function (or stack of them?).

For metadata, btrfs compares the crc and a few other fields of the metadata block, so we can easily add a compare function pointer and a void * to pass in.

Quoted 8 lines

The problem is the crc can take a lot of CPU, so btrfs kicks it off to threading pools so saturate all the cpus on the box. But there's no reason we can't make that available lower down.

If we pushed the verification down, the retries could bubble up the stack instead of the other way around.

-chris

I do like the idea of having the ability to do the verification and retries down the stack where you actually have the most context to figure out what is possible...

Why would you need to bubble back up anything other than an error when all retries have failed?

Quoted 9 lines

Quoted 6 lines

By bubble up I mean that if you have multiple layers capable of doing retries, the lowest levels would retry first. Basically by the time we get an -EIO_ALREADY_RETRIED we know there's nothing that lower level can do to help.

-chris

Absolutely sounds like the most sane way to go to me, thanks!

It really seemed like a good idea, but I just realized it doesn't work well when parts of the stack transform the data.

Quoted 7 lines

Picture dm-crypt on top of raid1. If raid1 is responsible for the crc retries, there's no way to crc the data because it needs to be decrypted first.

I think the raided dm-crypt config is much more common (and interesting) than multiple layers that can retry for other reasons (raid1 on top of raid10?)

Quoted 6 lines

Isn't this a case where the transformative mid-layer would replace
the validation function before passing it down the stack? So btrfs hands dm-crypt a checksum function; dm-crypt then stores that function for its own purposes and hands off a new function to the DM layer below that which decrypts the data and calls the btrfs checksum function it stored earlier.

Then we're requiring each transformation layer to have their own crcs, and if the higher layers have a stronger crc (or other checks), there's no path to ask the lower layers for other copies.

Here's a concrete example. In each metadata block, btrfs stores the fsid and the transid of the transaction that created it. In the case of a missed write, we'll read a perfect block from the lower layers. Any crcs will be correct and it'll pass through dm-crypt with flying colors.

Quoted 8 lines

But, it won't be the right block. Btrfs will notice this and EIO. In the current ask-for-another-mirror config we'll go down and grab the other copy.

In the stacked validation function model, dm-crypt replaces our verification functions with something that operates on the encrypted data, and it won't be able to detect the error or kick down to the underlying raid1 for another copy.

What I'm suggesting is easiest to describe with a functional
language, or mathematical notation, but I'll try without those anyway...

A generic validation function is a two-parameter function, taking a
block of data and some layer-dependent state, and returning a boolean.

So, at the btrfs layer, we have something like:

struct btrfs_validate_state {
    u32 cs;
};

bool btrfs_validate(void *state, char *data) {
    return crc32(data) == (btrfs_validate_state*)state->cs;
}

When reading a specific block, we look up the checksum for that
block, put it in state->cs and pass both our state and the btrfs_validate function to the lower layer.

The crypto layer beneath will look something like:

struct crypto_validate_state {
    void *old_state;
    bool (*old_validator)(void *state, char* data);
};

bool crypto_validate(void *state, char *data) {
    char plaintext[4096];
    decrypt(plaintext, data);
    
    return state->old_validator(state->old_state, plaintext);
}

Then, when a request is received from the upper layer, we can put
the (state, validator) pair into a crypto_validate_state structure, call that our new state, and pass on the new (state, crypto_validate) to the layer underneath.

So, where a transformative layer exists in the stack, it replaces
the validation function with a helper function that does the necessary transformation, and performs the check.

The code I've described above would clearly need to be hooked into
the existing data-transformation paths so that we don't replicate the effort of decrypting or uncompressing good data once we've decided that it _is_ good.

Oh, and if there's a layer with its own validation, we can put in a
"firebreak" implementation of the above code, which basically drops the validator it's passed, and replaces it completely with its own. (This would look exactly like the btrfs version at the top of my reply here).

The standard case for most block-layer implementations will simply
be to pass the (state, validator) pair unchanged to the lower layer, because the block being checked won't be any different. It's only where we have layers that change things, or definitely know better than the top layer that we need to play tricks.

Excerpts from Ric Wheeler's message of 2011-07-15 08:58:04 -0400:

Quoted 1 line · Chris Mason

By bubble up I mean that if you have multiple layers capable of doing retries, the lowest levels would retry first. Basically by the time we get an -EIO_ALREADY_RETRIED we know there's nothing that lower level can do to help.

Quoted 7 lines · Chris Mason

Excerpts from Ric Wheeler's message of 2011-07-15 08:58:04 -0400:

Quoted 1 line · Chris Mason

By bubble up I mean that if you have multiple layers capable of doing retries, the lowest levels would retry first. Basically by the time we get an -EIO_ALREADY_RETRIED we know there's nothing that lower level can do to help.

the problem with doing this is that it can end up stalling the box for significant amounts of time while all the retries happen.

we already see this happening today where a disk read failure is retried multiple times by the disk, multiple times by the raid controller, and then multiple times by Linux, resulting is multi-minute stalls when you hit a disk error in some cases.

having the lower layers do the retries automatically runs the risk of making this even worse.

This needs to be able to be throttled by some layer that can see the entire picture (either by cutting off the retries after a number, after some time, or by spacing out the retries to allow other queries to get in and let the box do useful work in the meantime)

David Lang

Quoted 16 lines · On 07/15/2011 05:23 PM, [email protected] wrote:

Quoted 7 lines · Chris Mason

Excerpts from Ric Wheeler's message of 2011-07-15 08:58:04 -0400:

Quoted 1 line · Chris Mason

By bubble up I mean that if you have multiple layers capable of doing retries, the lowest levels would retry first. Basically by the time we get an -EIO_ALREADY_RETRIED we know there's nothing that lower level can do to help.

the problem with doing this is that it can end up stalling the box for significant amounts of time while all the retries happen.

we already see this happening today where a disk read failure is retried multiple times by the disk, multiple times by the raid controller, and then multiple times by Linux, resulting is multi-minute stalls when you hit a disk error in some cases.

having the lower layers do the retries automatically runs the risk of making this even worse.

Quoted 4 lines

This needs to be able to be throttled by some layer that can see the entire picture (either by cutting off the retries after a number, after some time, or by spacing out the retries to allow other queries to get in and let the box do useful work in the meantime)

David Lang

That should not be an issue - we have a "fast fail" path for IO that should avoid retrying just for those reasons (i.e., for multi-path or when recovering a flaky drive).

This is not a scheme for unbounded retries. If you have a 3 disk mirror in RAID1, you would read the data no more than 2 extra times and almost never more than once. That should be *much* faster than the multiple-second long timeout you see when waiting for SCSI timeout to fire, etc.

Quoted 17 lines · Ric Wheeler

Quoted 16 lines · On 07/15/2011 05:23 PM, [email protected] wrote:

Quoted 7 lines · Chris Mason

Excerpts from Ric Wheeler's message of 2011-07-15 08:58:04 -0400:

Quoted 1 line · Chris Mason

By bubble up I mean that if you have multiple layers capable of doing retries, the lowest levels would retry first. Basically by the time we get an -EIO_ALREADY_RETRIED we know there's nothing that lower level can do to help.

the problem with doing this is that it can end up stalling the box for significant amounts of time while all the retries happen.

we already see this happening today where a disk read failure is retried multiple times by the disk, multiple times by the raid controller, and then multiple times by Linux, resulting is multi-minute stalls when you hit a disk error in some cases.

having the lower layers do the retries automatically runs the risk of making this even worse.

Quoted 8 lines

Quoted 4 lines

This needs to be able to be throttled by some layer that can see the entire picture (either by cutting off the retries after a number, after some time, or by spacing out the retries to allow other queries to get in and let the box do useful work in the meantime)

David Lang

That should not be an issue - we have a "fast fail" path for IO that should avoid retrying just for those reasons (i.e., for multi-path or when recovering a flaky drive).

This is not a scheme for unbounded retries. If you have a 3 disk mirror in RAID1, you would read the data no more than 2 extra times and almost never more than once. That should be *much* faster than the multiple-second long timeout you see when waiting for SCSI timeout to fire, etc.

this issue is when you stack things.

what if you have a 3 piece raid 1 on top of a 4 piece raid 6?

so you have 3 raid1 retries * N raid 6 retries. depending on the order that you do the retries in, and how long it takes that try to fail, this could start to take significant amounts of time.

if you do a retry on the lower level first, the raid 6 could try several different ways to combine the drives to get the valid data (disks 1,2 2,3 3,4 1,3 1,4 2,4 1,2,3 1,2,4 1,3,4 2,3,4) add more disks and it gets worse fast.

add more layers and you multiple the number of possible retries.

my guess is that changing to a different method at the upper level is going to avoid the problem area faster then doing so at a lower level (because there is less hardware in common with the method that just gave the wrong answer)

David Lang

Quoted 5 lines

Excerpts from NeilBrown's message of 2011-07-15 02:33:54 -0400:

Quoted 3 lines · On Thu, 14 Jul 2011 21:58:46 -0700 (PDT) [email protected] wrote:

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines · Chris Mason

Quoted 6 lines

Excerpts from Ric Wheeler's message of 2011-07-14 02:57:54 -0400:

Quoted 4 lines · NeilBrown

Quoted 2 lines

On Thu, 14 Jul 2011 07:02:22 +0100 Ric Wheeler<rwheeler@redhat.=

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 1 line · wrote:

Quoted 1 line

I'm certainly open to suggestions and collaboration. =A0Do yo=

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 4 lines

in mind any
particular way to make the interface richer??

NeilBrown

Hi Neil,

I know that Chris has a very specific set of use cases for btr=

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

think that
Alasdair and others have started to look at what is doable.

The obvious use case is the following:

If a file system uses checksumming or other data corruption detection bits, it
can detect that it got bad data on a write. If that data was protected by RAID,
it would like to ask the block layer to try to read from anoth=

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

mirror (for
raid1) or try to validate/rebuild from parity.

Today, I think that a retry will basically just give us back a random chance of
getting data from a different mirror or the same one that we g=

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 8 lines

data from on
the first go.

Chris, Alasdair, was that a good summary of one concern?

Thanks!

Ric

I imagine a new field in 'struct bio' which was normally zero b=

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

could be
some small integer. =A0It is only meaningful for read. When 0 it means "get this data way you like".
When non-zero it means "get this data using method N", where th=

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

different
methods are up to the device.

For a mirrored RAID, method N means read from device N-1. For stripe/parity RAID, method 1 means "use other data blocks a=

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

parity
blocks to reconstruct data.

The default for non RAID devices is to return EINVAL for any N>=

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

A remapping device (dm-linear, dm-stripe etc) would just pass t=

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

number
down. =A0I'm not sure how RAID1 over RAID5 would handle it... t=

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

might need
some thought.

So if btrfs reads a block and the checksum looks wrong, it read=

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

again with
a larger N. =A0It continues incrementing N and retrying until i=

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

block
that it likes or it gets EINVAL. =A0There should probably be an=

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

code
(EAGAIN?) which means "I cannot work with that number, but try =

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

next one".

It would be trivial for me to implement this for RAID1 and RAID=

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

and
relatively easy for RAID5.
I'd need to give a bit of thought to RAID6 as there are possibl=

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

multiple
ways to reconstruct from different combinations of parity and d=

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

=A0I'm not
sure if there would be much point in doing that though.

It might make sense for a device to be able to report what the maximum
'N' supported is... that might make stacked raid easier to mana=

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

NeilBrown

I think that the above makes sense. Not sure what your "0" defin=

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

is, but I
would assume that for non-raided devices (i.e., a single s-ata d=

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

"0" would
be an indication that there is nothing more that can be tried. T=

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

data you got
is all you are ever going to get :)

For multiple mirrors, you might want to have a scheme where you =

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

be able to
cycle through the mirrors. You could retry, cycling through the various mirrors
until you have tried and returned them all at which point you wo=

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 2 lines

get a "no
more" error back or some such thing.

Hi everyone,

The mirror number idea is basically what btrfs does today, and I =

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

it fits in with Neil's idea to have different policies for differ=

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

blocks.

Basically what btrfs does:

read_block(block_num, mirror =3D 0)
if (no io error and not csum error)
=A0 =A0 horray()

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

num_mirrors =3D get_num_copies(block number)
for (i =3D 1; i< =A0num_mirrors; i++) {
=A0 =A0 read_block(block_num, mirror =3D i);
}

In a stacked configuration, the get_num_copies function can be sm=

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

basically adding up all the copies of the lower levels and findin=

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

way
to combine them. =A0We could just send down a fake bio that is responsible
for adding up the storage combinations into a bitmask or whatever works.

We could also just keep retrying until the lower layers reported =

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

more
mirror were available.

In btrfs at least, we don't set the data blocks up to date until =

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

crc
has passed, so replacing bogus blocks is easy. =A0Metadata is a l=

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

more complex, but that's not really related to this topic.

mirror number 0 just means "no mirror preference/pick the fastest mirror" to the btrfs block mapping code.

Btrfs has the concept of different raid levels for different logi=

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

block numbers, so you get_num_copies might return one answer for =

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

A
and a different answer for block B.

Either way, we could easily make use of a bio field here if it we=

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

exported out.

you don't want to just pass the value down as that will cause prob=

Quoted 1 line

Quoted 1 line

Quoted 1 line

with layering (especially if the lower layer supports more values =

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

higher layer)

I would suggest that each layer take the value it's give, do an in=

Quoted 1 line

Quoted 1 line

Quoted 1 line

division by the number of values that layer supports, using the mo=

Quoted 1 line

Quoted 1 line

Quoted 1 line

value for that layer and pass the rest of the result down to the n=

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

layer.

I, on the other hand, would suggest that each layer be given the fr=

Quoted 4 lines

Quoted 4 lines

and
the responsibility, to do whatever it thinks is most appropriate.

This would probably involved checking the lower levels to see how m=

Quoted 1 line

Quoted 1 line

strategies each has, and doing some appropriate arithmetic dependin=

Quoted 4 lines

Quoted 4 lines

how
it combines those devices.

One problem here is the assumption that the lower levels don't chan=

Quoted 3 lines

Quoted 3 lines

and we
know that not to be the case.
However this is already a problem. =A0It is entirely possible that =

Quoted 2 lines

Quoted 2 lines

request
size parameters of devices can change at any moment, even while a r=

Quoted 4 lines

Quoted 4 lines

is
in-flight ... though we try to avoid that or work around it.

The sort of dependencies that we see forming here would probably ju=

Quoted 7 lines

Quoted 5 lines

make
the problem worse.

Not sure what to do about it though.... maybe just hope it isn't a problem.

Agreed, if we want to go beyond best effort for a stacking config, w=

Quoted 1 line

need to put some state struct in the bio that each layer can play wi=

Quoted 4 lines

That way each layer knows which mirrors have already been tried.

But, maybe the whole btrfs model is backwards for a generic layer. Instead of sending down ios and testing when they come back, we coul=

Quoted 4 lines

just set a verification function (or stack of them?).

For metadata, btrfs compares the crc and a few other fields of the metadata block, so we can easily add a compare function pointer and =

Quoted 3 lines

void * to pass in.

The problem is the crc can take a lot of CPU, so btrfs kicks it off =

Quoted 1 line

threading pools so saturate all the cpus on the box. =A0But there's =

Quoted 6 lines

reason we can't make that available lower down.

If we pushed the verification down, the retries could bubble up the stack instead of the other way around.

-chris

I do like the idea of having the ability to do the verification and r=

down the stack where you actually have the most context to figure out=

is possible...

Why would you need to bubble back up anything other than an error whe=

retries have failed?

Quoted 11 lines · On Thu, 14 Jul 2011 21:58:46 -0700 (PDT) [email protected] wrote:

Quoted 4 lines · Chris Mason

Excerpts from Ric Wheeler's message of 2011-07-14 02:57:54 -0400:

Quoted 3 lines · NeilBrown

Quoted 2 lines · On Thu, 14 Jul 2011 07:02:22 +0100 Ric Wheeler<[email protected]> wrote:

I would suggest that each layer take the value it's give, do an integer division by the number of values that layer supports, using the modulo value for that layer and pass the rest of the result down to the next layer.

I, on the other hand, would suggest that each layer be given the freedom, and the responsibility, to do whatever it thinks is most appropriate.

This would probably involved checking the lower levels to see how many strategies each has, and doing some appropriate arithmetic depending on how it combines those devices.

One problem here is the assumption that the lower levels don't change, and we know that not to be the case.
However this is already a problem. It is entirely possible that the request size parameters of devices can change at any moment, even while a request is in-flight ... though we try to avoid that or work around it.

The sort of dependencies that we see forming here would probably just make the problem worse.

Not sure what to do about it though.... maybe just hope it isn't a problem.

NeilBrown

Quoted 13 lines

this works for just trying values until you get the error that tells you there are no more options.

things can get very 'intersesting' if the different options below you have different number of options (say a raid1 across a raid5 and a single disk) but I can't think of any good way to figure this out and assemble a sane order without doing an exaustive search down to find the number of options for each layer (and since this can be different for different blocks, you would have to do this each time you invoked this option)

David Lang

I imagine a new field in 'struct bio' which was normally zero but could be some small integer. It is only meaningful for read.
When 0 it means "get this data way you like".
When non-zero it means "get this data using method N", where the different methods are up to the device.

For a mirrored RAID, method N means read from device N-1. For stripe/parity RAID, method 1 means "use other data blocks and parity blocks to reconstruct data.

The default for non RAID devices is to return EINVAL for any N > 0. A remapping device (dm-linear, dm-stripe etc) would just pass the number down. I'm not sure how RAID1 over RAID5 would handle it... that might need some thought.

So if btrfs reads a block and the checksum looks wrong, it reads again with a larger N. It continues incrementing N and retrying until it gets a block that it likes or it gets EINVAL. There should probably be an error code (EAGAIN?) which means "I cannot work with that number, but try the next one".

Hi Neil,

Quoted 15 lines · NeilBrown

I imagine a new field in 'struct bio' which was normally zero but could be some small integer. It is only meaningful for read.
When 0 it means "get this data way you like".
When non-zero it means "get this data using method N", where the different methods are up to the device.

For a mirrored RAID, method N means read from device N-1. For stripe/parity RAID, method 1 means "use other data blocks and parity blocks to reconstruct data.

The default for non RAID devices is to return EINVAL for any N > 0. A remapping device (dm-linear, dm-stripe etc) would just pass the number down. I'm not sure how RAID1 over RAID5 would handle it... that might need some thought.

Quoted 4 lines

So if btrfs reads a block and the checksum looks wrong, it reads again with a larger N. It continues incrementing N and retrying until it gets a block that it likes or it gets EINVAL. There should probably be an error code (EAGAIN?) which means "I cannot work with that number, but try the next one".

I like this idea. It comes pretty close to what btrfs is currently doing (what was the reason for this thread being started, wasn't it?), only not within struct bio.

The way you describe the extra parameter is input only. I think it would be a nice add on if we knew which "N" was used when "0" passed for the request. At least for the RAID1 case, I'd like to see that when I submit a bio with method (or whatever we call it) "0", it comes back with the method field set to the value that reflects the method the device actually used. Obviously, when passing non-zero values, the bio would have to come back with that value unmodified.

That way, we'll get more control on the retry algorithms and are free to decide not to use the failed method again, if we like.

Setting "method" on the return path might be valuable not only for RAID1, but I haven't thought that trough.

-Jan

Quoted 25 lines · On Thu, 14 Jul 2011 07:02:22 +0100 Ric Wheeler <[email protected]> wrote:

Quoted 4 lines

I'm certainly open to suggestions and collaboration. Do you have in mind any particular way to make the interface richer??

NeilBrown

Hi Neil,

I know that Chris has a very specific set of use cases for btrfs and think that Alasdair and others have started to look at what is doable.

The obvious use case is the following:

If a file system uses checksumming or other data corruption detection bits, it can detect that it got bad data on a write. If that data was protected by RAID, it would like to ask the block layer to try to read from another mirror (for raid1) or try to validate/rebuild from parity.

Today, I think that a retry will basically just give us back a random chance of getting data from a different mirror or the same one that we got data from on the first go.

Chris, Alasdair, was that a good summary of one concern?

Thanks!

Quoted 1 line

Ric

I imagine a new field in 'struct bio' which was normally zero but could be some small integer. It is only meaningful for read.
When 0 it means "get this data way you like".
When non-zero it means "get this data using method N", where the different methods are up to the device.

For a mirrored RAID, method N means read from device N-1. For stripe/parity RAID, method 1 means "use other data blocks and parity blocks to reconstruct data.

The default for non RAID devices is to return EINVAL for any N > 0. A remapping device (dm-linear, dm-stripe etc) would just pass the number down. I'm not sure how RAID1 over RAID5 would handle it... that might need some thought.

So if btrfs reads a block and the checksum looks wrong, it reads again with a larger N. It continues incrementing N and retrying until it gets a block that it likes or it gets EINVAL. There should probably be an error code (EAGAIN?) which means "I cannot work with that number, but try the next one".

It would be trivial for me to implement this for RAID1 and RAID10, and relatively easy for RAID5.
I'd need to give a bit of thought to RAID6 as there are possibly multiple ways to reconstruct from different combinations of parity and data. I'm not sure if there would be much point in doing that though.

It might make sense for a device to be able to report what the maximum 'N' supported is... that might make stacked raid easier to manage...

NeilBrown

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
.

It might make sense for a device to be able to report what the maximum 'N' supported is... that might make stacked raid easier to manage...

It might make sense for a device to be able to report what the maximum 'N' supported is... that might make stacked raid easier to manage...

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Alasdair" == Alasdair G Kergon <[email protected]> writes:

Quoted 2 lines

It might make sense for a device to be able to report what the maximum 'N' supported is... that might make stacked raid easier to manage...

Alasdair> I'll just say that any solution ought to be stackable.

I've been mulling this over too and wondering how you'd handle this, because upper layers really can't peak down into lower layers easily. As far as I understand things.

So if you have btrfs -> luks -> raid1 -> raid6 -> nbd -> remote disks

How does btrfs handle errors (or does it even see them!) from the raid6 level when a single nbd device goes away? Or taking the original example, when btrfs notices a checksum isn't correct, how would it push down multiple levels to try and find the correct data?

Alasdair> This means understanding both that the number of data access Alasdair> routes may vary as you move through the stack, and that this Alasdair> number may depend on the offset within the device.

It almost seems to me that the retry needs to be done at each level on it's own, without pushing down or up the stack. But this doesn't handle the wrong file checksum issue.

Hmm... maybe instead of just one number, we need another to count the levels down you go (or just split 16bit integer in half, bottom half being count of tries, the upper half being levels down to try that read?)

It seems to defeat the purpose of layers if you can go down and find out how many layers there are underneath you....

So if you have btrfs -> luks -> raid1 -> raid6 -> nbd -> remote disks

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Alasdair" == Alasdair G Kergon <[email protected]> writes:

Quoted 2 lines · Neil Brown

Quoted 2 lines

It might make sense for a device to be able to report what the maximum 'N' supported is... that might make stacked raid easier to manage...

Alasdair> I'll just say that any solution ought to be stackable.

I've been mulling this over too and wondering how you'd handle this, because upper layers really can't peak down into lower layers easily. As far as I understand things.

So if you have btrfs -> luks -> raid1 -> raid6 -> nbd -> remote disks

How does btrfs handle errors (or does it even see them!) from the raid6 level when a single nbd device goes away? Or taking the original example, when btrfs notices a checksum isn't correct, how would it push down multiple levels to try and find the correct data?

Alasdair> This means understanding both that the number of data access Alasdair> routes may vary as you move through the stack, and that this Alasdair> number may depend on the offset within the device.

It almost seems to me that the retry needs to be done at each level on it's own, without pushing down or up the stack. But this doesn't handle the wrong file checksum issue.

Hmm... maybe instead of just one number, we need another to count the levels down you go (or just split 16bit integer in half, bottom half being count of tries, the upper half being levels down to try that read?)

It seems to defeat the purpose of layers if you can go down and find out how many layers there are underneath you....

John

Quoted 3 lines · NeilBrown

I'm certainly open to suggestions and collaboration. Do you have in mind any
particular way to make the interface richer??

If a file system uses checksumming or other data corruption detection bits, it can detect that it got bad data on a write. If that data was protected by RAID, it would like to ask the block layer to try to read from another mirror (for raid1) or try to validate/rebuild from parity.

Today, I think that a retry will basically just give us back a random chance of getting data from a different mirror or the same one that we got data from on the first go.

Chris, Alasdair, was that a good summary of one concern?

Thanks!

Ric