RE: [PATCH 4/9] imsm: FIX: After checkpoint mark array have to be clean

-----Original Message-----
From: NeilBrown [mailto:neilb@suse.de]
Sent: Tuesday, March 08, 2011 11:14 PM
To: Kwolek, Adam
Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
Neubauer, Wojciech
Subject: Re: [PATCH 4/9] imsm: FIX: After checkpoint mark array have to
be clean

On Tue, 8 Mar 2011 08:52:40 +0000 "Kwolek, Adam" [off-list ref]
wrote:

quoted

quoted

-----Original Message-----
From: NeilBrown [mailto:neilb@suse.de]
Sent: Tuesday, March 08, 2011 6:08 AM
To: Kwolek, Adam
Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
Neubauer, Wojciech
Subject: Re: [PATCH 4/9] imsm: FIX: After checkpoint mark array have

to

quoted

quoted

be clean

On Wed, 02 Mar 2011 14:29:27 +0100 Adam Kwolek

[off-list ref]

quoted

quoted

wrote:

quoted

When checkpoint is marked set volume as clean.
Reshape on dirty volume cannot be restarted from checkpoint.

Signed-off-by: Adam Kwolek <redacted>
---

 super-intel.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index 11972f3..04e32ae 100644
--- a/super-intel.c
+++ b/super-intel.c

@@ -5218,6 +5218,7 @@ static int imsm_set_array_state(struct

active_array *a, int consistent)

quoted

 				    __le32_to_cpu(dev-

vol.curr_migr_unit)) {

quoted

quoted

 					dev->vol.curr_migr_unit =
 						__cpu_to_le32(unit);
+					dev->vol.dirty = 0;
 					super->updates_pending++;
 				}
 			}

hi Adam,
 You'll need to explain this one a bit more.

 If the array isn't clean, then it is wrong to mark it as clean.
 If it is clean, then 'consistent' should be 'true' and it will
 be marked clean anyway.

This could be true if set_array_state has possibility to execute this

code in general migration case.

quoted

During reshape after updtate curr_migr_unit set array state exits

(super-intel.c: 5225 /return 0/)

quoted

To achieve this it can be changed:
1. Consistent flag should be == 1 during reshape
monitor.c:387
- a->container->ss->set_array_state(a, a->curr_state <= clean);

+ a->container->ss->set_array_state(a, a->curr_state <= active);

This change doesn't make any sense at all.
Whether the array is 'clean' or 'active', it completely independent of
whether the array is being reshaped.
While it is being reshaped it could sometimes be clean, and sometime
be active, and the recorded state should reflect this.
Say it is 'clean' (or 'consistent') when it actually isn't would be a
lie.

During grow from some point array is always dirty, this can be not true also.
I'll put focus on enabling assembly dirty array during reshape.

quoted

Array state is active during reshape

2. use consistent flag during reshape for imsm
Super-intel.c: set_array_state()
	- remove all code for a->curr_action == reshape (around line 5211)
	- instead removed code add 'goto' to calculating blocks_per_unit

at the end of this function

quoted

 	   This will allow for mark clear/dirty volume also

I like this patch - nice cleanup of the code.  I have applied it,
thanks.

quoted

I'll send 2 patch to make this description more clear.
 1. FIX: During reshape array is in active state
 2. imsm: FIX: Mark checkpoint and array state clean during reshape

quoted

 Why cannot a reshape of a dirty volume be restarted from a

checkpoint?

quoted

quoted

 I would think it would continue with the reshape and then when that
 finished, go back and do the resync.

NeilBrown

If array is dirty and during reshape is not assembled due to i.e:
super-intel.c:1792 for dirty volume info->resync_start = 0; Code for

analyzing migration type is not executed.

I see - that code in super-intel.c::getinfo_super_imsm_volume needs to
be
fixed.  Maybe just remove the 'else' there

	if (map_to_analyse->map_state == IMSM_T_STATE_UNINITIALIZED ||
	    dev->vol.dirty) {
		info->resync_start = 0;
	} else if (dev->vol.migr_state) {
          ^^^^^

Not sure though.

I've tried this yesterday at first shot, it is not enough.
I'll fix it.

Thanks
Adam

NeilBrown