On Fri, 21 Oct 2011 15:33:18 +0200 Jes.Sorensen@xxxxxxxxxx wrote: > From: Jes Sorensen <Jes.Sorensen@xxxxxxxxxx> > > This moves the lock handling out of Incremental_container() and relies > on the caller holding the lock. This prevents conflict with a > follow-on mdadm comment which may try and launch the device in > parallel. > > Signed-off-by: Jes Sorensen <Jes.Sorensen@xxxxxxxxxx> Hi Jes, I have applied the 7 patches you sent. However I have question mark over part of this one: > @@ -451,9 +456,10 @@ int Incremental(char *devname, int verbose, int runstop, > devnum = fd2devnum(mdfd); > close(mdfd); > sysfs_free(sra); > - rv = Incremental(chosen_name, verbose, runstop, > - NULL, homehost, require_homehost, autof, > - freeze_reshape); > + rv = Incremental_container(st, chosen_name, homehost, > + verbose, runstop, autof, > + freeze_reshape); > + map_unlock(&map); > if (rv == 1) > /* Don't fail the whole -I if a subarray didn't > * have enough devices to start yet You have replaced a call to Incremental with a call to Incremental_container. I feel that is significant enough that I would have liked to have seen it discussed in the changelog comment. You seem have have open-coded Incremental and then removed all the bits that you don't need in this case - which is that vast majority of it. But you didn't include the call to ->load_content(). So I have put it back in because I'm sure it must be harmless (Because it was there already) but it may not be needed. If you have thoughts on this, please let me know. Thanks, NeilBrown
Attachment:
signature.asc
Description: PGP signature
