RE: [PATCH 3/8] imsm: FIX: Size is already set in metadata

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: linux-raid-owner@xxxxxxxxxxxxxxx [mailto:linux-raid-
> owner@xxxxxxxxxxxxxxx] On Behalf Of NeilBrown
> Sent: Thursday, February 03, 2011 7:45 AM
> To: Kwolek, Adam
> Cc: linux-raid@xxxxxxxxxxxxxxx; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH 3/8] imsm: FIX: Size is already set in metadata
> 
> On Tue, 01 Feb 2011 14:49:20 +0100 Adam Kwolek <adam.kwolek@xxxxxxxxx>
> wrote:
> 
> > In metadata size is set already during migration initialization.
> > There is no reason for second /the same/ calculation.
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@xxxxxxxxx>
> > ---
> >
> >  super-intel.c |    5 +----
> >  1 files changed, 1 insertions(+), 4 deletions(-)
> >
> > diff --git a/super-intel.c b/super-intel.c
> > index 42f7065..62b13b0 100644
> > --- a/super-intel.c
> > +++ b/super-intel.c
> > @@ -5185,15 +5185,12 @@ static int imsm_set_array_state(struct
> active_array *a, int consistent)
> >  				/* round array size down to closest MB */
> >  				array_blocks = (array_blocks >>
> SECT_PER_MB_SHIFT)
> >  					<< SECT_PER_MB_SHIFT;
> > -				dev->size_low = __cpu_to_le32((__u32)
> array_blocks);
> > -				dev->size_high = __cpu_to_le32((__u32)
> (array_blocks >> 32));
> >  				a->info.custom_array_size = array_blocks/2;
> >  				a->check_reshape = 1; /* encourage manager to
> update
> >  						       * array size
> >  						       */
> > -				super->updates_pending++;
> >  				imsm_progress_container_reshape(super);
> > -			}
> > +			}
> >  		}
> >  	}
> >
> >
> 
> Doing the same calculation in two placed does seem wrong, but are you
> sure this is the right one to remove?
> 
> The available space in the array does not change until the reshape
> completes.  So changing dev->size_{low,high} during initialisation
> seems wrong.
> 
> Are you able to confirm what the windows driver does?

Yes (look below).

> 
> If we do set the size early, then we have to be careful to reduce to
> the
> original size when assembling an array that is in the middle of
> reshape.

Yes (as you describes in your roadmap, it will be part of 3.2.1 code changes/investigation).
Size for array with reshape in progress process, should be calculated based on second map information.

> 
> So I won't apply this patch now, and will wait for you to confirm when
> the
> windows driver updates dev->size_{low,high}
> 
> Thanks,
> NeilBrown

I've (double) checked Windows driver behavior. Array size is set during expansion initialization (first metadata update).
This means, that code for size changes on reshape end has to be removed as it was proposed.
For volume expansion, size in imsm metadata has to be set in 2 places/cases:
1. in metadata update during reshape update processing for first array(it is implemented)
2. In initialization reshape by set_array_state on second array (it is implemented)

BR
Adam

> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid"
> in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux