Re: [PATCH 4/9] imsm: FIX: Do not clean checkpoint for active reshape

[NeilBrown <neilb@suse.de>]

On Wed, 09 Mar 2011 14:46:02 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> Smaller checkpoint can be stored when metadata is not in migration state
> (this means when there is no second map).
> Without this additional condition it can occur that during array start/stop
> sequence checkpoint was roll back to 0.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  super-intel.c |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/super-intel.c b/super-intel.c
> index cd409e9..4e3de8a 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -5335,9 +5335,15 @@ mark_checkpoint:
>  		 */
>  		if (units32 == units &&
>  		    __le32_to_cpu(dev->vol.curr_migr_unit) != units32) {
> -			dprintf("imsm: mark checkpoint (%u)\n", units32);
> -			dev->vol.curr_migr_unit = __cpu_to_le32(units32);
> -			super->updates_pending++;
> +			struct imsm_map *map2 = get_imsm_map(dev, 1);
> +			if ((__le32_to_cpu(dev->vol.curr_migr_unit) < units32)
> +			    || (map2 == NULL)) {
> +				dprintf("imsm: mark checkpoint (%u)\n",
> +					units32);
> +				dev->vol.curr_migr_unit =
> +					__cpu_to_le32(units32);
> +				super->updates_pending++;
> +			}
>  		}
>  	}
>  
> 

Not applied.

I don't really understand the change you are making here.

As it seems to me that:
  - before the patch, we only update the 'curr_migr_unit' when
    units32 is different
  - after the patch, we only update it if units32 is bigger, or if
    map2==NULL (meaning this isn't a migration?).

So when would units32 be smaller??  What is the important case that this
fixes?

From the comment, it seems to be to avoid 'roll back to 0'.   I don't see
how it does that, and I don't see why that is a problem.

If some reshape has happened but curr_migr_unit is 0, that just means
that we need to replay the backed-up data and set curr_migr_unit
accordingly (or reflect that the backed-up data has been migrated).
So after Grow_restart has run, curr_migr_unit should never be zero.
You will need to get update_super_imsm to understand '_reshape_progress'
for this to work properly.

NeilBrown