[PATCH 0/1] Avoid OOPS when reshaping raid1 to raid0

[PATCH 0/1] Avoid OOPS when reshaping raid1 to raid0

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Hi,

If one tries to reshape a raid1 into a raid0 the kernel will OOPS
because it tries to align the size of the new array to the original
chunk size. However a raid1 doesn't have the notion of chunk size, so
we end up dividing by zero and OOPSing.

Fix it by setting the existing chunk size of the raid1 being reshaped
to the new suggested chunk size.

Cheers,
Jes


Jes Sorensen (1):
  Avoid OOPS when reshaping raid1 to raid0

 drivers/md/raid0.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

-- 
1.7.7.6

[PATCH 1/1] Avoid OOPS when reshaping raid1 to raid0

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

raid1 arrays do not have the notion of chunk size. Set the existing
chunk size of the new raid0 to the same as the proposed new chunk size
to avoid a divide by zero OOPS when aligning the size of the new array
to that of the chunk size.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 drivers/md/raid0.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index 6f31f55..dff8d6b 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -639,6 +639,14 @@ static void *raid0_takeover_raid1(struct mddev *mddev)
 	mddev->new_level = 0;
 	mddev->new_layout = 0;
 	mddev->new_chunk_sectors = 128; /* by default set chunk size to 64k */
+	mddev->chunk_sectors = 128;	/*
+					 * a raid1 one doesn't have the
+					 * notion of chunk size, so set
+					 * existing chunk_sector to match the
+					 * new size to avoid divide by zero
+					 * when aligning the size of the new
+					 * raid0 to the existing chunk size.
+					 */
 	mddev->delta_disks = 1 - mddev->raid_disks;
 	mddev->raid_disks = 1;
 	/* make sure it will be not marked as dirty */
-- 
1.7.7.6

Re: [PATCH 1/1] Avoid OOPS when reshaping raid1 to raid0

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1492 bytes --]

On 03/22/2012 12:15 PM, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> raid1 arrays do not have the notion of chunk size. Set the existing
> chunk size of the new raid0 to the same as the proposed new chunk size
> to avoid a divide by zero OOPS when aligning the size of the new array
> to that of the chunk size.
> 
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  drivers/md/raid0.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> index 6f31f55..dff8d6b 100644
> --- a/drivers/md/raid0.c
> +++ b/drivers/md/raid0.c
> @@ -639,6 +639,14 @@ static void *raid0_takeover_raid1(struct mddev *mddev)
>  	mddev->new_level = 0;
>  	mddev->new_layout = 0;
>  	mddev->new_chunk_sectors = 128; /* by default set chunk size to 64k */
> +	mddev->chunk_sectors = 128;	/*
> +					 * a raid1 one doesn't have the
> +					 * notion of chunk size, so set
> +					 * existing chunk_sector to match the
> +					 * new size to avoid divide by zero
> +					 * when aligning the size of the new
> +					 * raid0 to the existing chunk size.
> +					 */
>  	mddev->delta_disks = 1 - mddev->raid_disks;
>  	mddev->raid_disks = 1;
>  	/* make sure it will be not marked as dirty */

Acked-by: Doug Ledford <dledford@redhat.com>

-- 
Doug Ledford <dledford@redhat.com>
              GPG KeyID: 0E572FDD
	      http://people.redhat.com/dledford



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 900 bytes --]

Re: [PATCH 1/1] Avoid OOPS when reshaping raid1 to raid0

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1962 bytes --]

On Thu, 22 Mar 2012 17:15:53 +0100 Jes.Sorensen@redhat.com wrote:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> raid1 arrays do not have the notion of chunk size. Set the existing
> chunk size of the new raid0 to the same as the proposed new chunk size
> to avoid a divide by zero OOPS when aligning the size of the new array
> to that of the chunk size.

This oops happens in create_strip_zones at

		sector_div(sectors, mddev->chunk_sectors);

correct?

> 
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  drivers/md/raid0.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> index 6f31f55..dff8d6b 100644
> --- a/drivers/md/raid0.c
> +++ b/drivers/md/raid0.c
> @@ -639,6 +639,14 @@ static void *raid0_takeover_raid1(struct mddev *mddev)
>  	mddev->new_level = 0;
>  	mddev->new_layout = 0;
>  	mddev->new_chunk_sectors = 128; /* by default set chunk size to 64k */
> +	mddev->chunk_sectors = 128;	/*
> +					 * a raid1 one doesn't have the
> +					 * notion of chunk size, so set
> +					 * existing chunk_sector to match the
> +					 * new size to avoid divide by zero
> +					 * when aligning the size of the new
> +					 * raid0 to the existing chunk size.
> +					 */

But what if the RAID1 is not a multiple of 64K ?  Then you will lose
data.

We probably want the same thing we have in RAID5:



	chunksect = 64*2; /* 64K by default */

	/* The array must be an exact multiple of chunksize */
	while (chunksect && (mddev->array_sectors & (chunksect-1)))
		chunksect >>= 1;

	if ((chunksect<<9) < STRIPE_SIZE)
		/* array size does not allow a suitable chunk size */
		return ERR_PTR(-EINVAL);


??

Do you want to make a patch, or shall I?

Ofcouse, RAID0 have have non-power-of-2 chunk sizes, so we could just hunt
for a small factor... probably not worth it though.


Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 1/1] Avoid OOPS when reshaping raid1 to raid0

[Jes Sorensen]

On 03/26/12 00:29, NeilBrown wrote:
> On Thu, 22 Mar 2012 17:15:53 +0100 Jes.Sorensen@redhat.com wrote:
> 
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>
>> raid1 arrays do not have the notion of chunk size. Set the existing
>> chunk size of the new raid0 to the same as the proposed new chunk size
>> to avoid a divide by zero OOPS when aligning the size of the new array
>> to that of the chunk size.
> 
> This oops happens in create_strip_zones at
> 
> 		sector_div(sectors, mddev->chunk_sectors);
> 
> correct?

Correct

>>  	mddev->new_chunk_sectors = 128; /* by default set chunk size to 64k */
>> +	mddev->chunk_sectors = 128;	/*
>> +					 * a raid1 one doesn't have the
>> +					 * notion of chunk size, so set
>> +					 * existing chunk_sector to match the
>> +					 * new size to avoid divide by zero
>> +					 * when aligning the size of the new
>> +					 * raid0 to the existing chunk size.
>> +					 */
> 
> But what if the RAID1 is not a multiple of 64K ?  Then you will lose
> data.

Good point - I wasn't sure what the actual sizes would be in practice.

> We probably want the same thing we have in RAID5:
> 
> 
> 
> 	chunksect = 64*2; /* 64K by default */
> 
> 	/* The array must be an exact multiple of chunksize */
> 	while (chunksect && (mddev->array_sectors & (chunksect-1)))
> 		chunksect >>= 1;
> 
> 	if ((chunksect<<9) < STRIPE_SIZE)
> 		/* array size does not allow a suitable chunk size */
> 		return ERR_PTR(-EINVAL);
> 
> 
> ??
> 
> Do you want to make a patch, or shall I?

I have brewed up a patch based on this. I used PAGE_SIZE instead of
STRIPE_SIZE for the limit since we don't have stripes in raid0. I'll
post it in a minute.

Cheers,
Jes