[PATCH] mdadm: do not read the 'clean' flag from potentially non-up-to-date drive

[PATCH] mdadm: do not read the 'clean' flag from potentially non-up-to-date drive

[Alexander Lyakas]

Hi Neil,

I noticed that mdadm may decide that array is dirty, based on the
non-freshest drive. This happens, e.g., when there is a drive failure
during initial resync, and then array is stopped and re-assembled. The
failed drive still has a valid sb->resync_offset, so if this drive is
visible during assembly, array will be considered dirty, even though
the rest of the drives have sb->resync_offset==MaxSector.

Do you think array should be considered as dirty in this case? Because
if the failed drive was not visible during assembly, array would have
been considered clean.

The below patch is only to demonstrate the possible fix. The idea is
that we decide that array is dirty only according to up-to-date
drives. Currently the "clean" flag is initialized from the first
superblock, which might not be the freshest one.

I haven't tested it at all. If you think the direction is reasonable,
I will test.

Thanks,
  Alex.


diff --git a/Assemble.c b/Assemble.c
index 23695e7..42cbbd5 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -929,7 +929,6 @@ int Assemble(struct supertype *st, char *mddev,
                st->minor_version = 90;

        st->ss->getinfo_super(st, content, NULL);
-       clean = content->array.state & 1;

        /* now we have some devices that might be suitable.
         * I wonder how many
@@ -1122,6 +1121,7 @@ int Assemble(struct supertype *st, char *mddev,
 #ifndef MDASSEMBLE
        sysfs_init(content, mdfd, 0);
 #endif
+       clean = 1; /* Assume that array is clean, until we see otherwise */
        for (i=0; i<bestcnt; i++) {
                int j = best[i];
                unsigned int desired_state;

Re: [PATCH] mdadm: do not read the 'clean' flag from potentially non-up-to-date drive

[Alexander Lyakas]

Hi Neil,
any comment on this?

Alex.

On Fri, Apr 6, 2012 at 3:24 PM, Alexander Lyakas <alex.bolshoy@gmail.com> wrote:
> Hi Neil,
>
> I noticed that mdadm may decide that array is dirty, based on the
> non-freshest drive. This happens, e.g., when there is a drive failure
> during initial resync, and then array is stopped and re-assembled. The
> failed drive still has a valid sb->resync_offset, so if this drive is
> visible during assembly, array will be considered dirty, even though
> the rest of the drives have sb->resync_offset==MaxSector.
>
> Do you think array should be considered as dirty in this case? Because
> if the failed drive was not visible during assembly, array would have
> been considered clean.
>
> The below patch is only to demonstrate the possible fix. The idea is
> that we decide that array is dirty only according to up-to-date
> drives. Currently the "clean" flag is initialized from the first
> superblock, which might not be the freshest one.
>
> I haven't tested it at all. If you think the direction is reasonable,
> I will test.
>
> Thanks,
>  Alex.
>
>
> diff --git a/Assemble.c b/Assemble.c
> index 23695e7..42cbbd5 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -929,7 +929,6 @@ int Assemble(struct supertype *st, char *mddev,
>                st->minor_version = 90;
>
>        st->ss->getinfo_super(st, content, NULL);
> -       clean = content->array.state & 1;
>
>        /* now we have some devices that might be suitable.
>         * I wonder how many
> @@ -1122,6 +1121,7 @@ int Assemble(struct supertype *st, char *mddev,
>  #ifndef MDASSEMBLE
>        sysfs_init(content, mdfd, 0);
>  #endif
> +       clean = 1; /* Assume that array is clean, until we see otherwise */
>        for (i=0; i<bestcnt; i++) {
>                int j = best[i];
>                unsigned int desired_state;
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mdadm: do not read the 'clean' flag from potentially non-up-to-date drive

[NeilBrown]

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

On Fri, 6 Apr 2012 15:24:22 +0300 Alexander Lyakas <alex.bolshoy@gmail.com>
wrote:

> Hi Neil,
> 
> I noticed that mdadm may decide that array is dirty, based on the
> non-freshest drive. This happens, e.g., when there is a drive failure
> during initial resync, and then array is stopped and re-assembled. The
> failed drive still has a valid sb->resync_offset, so if this drive is
> visible during assembly, array will be considered dirty, even though
> the rest of the drives have sb->resync_offset==MaxSector.
> 
> Do you think array should be considered as dirty in this case? Because
> if the failed drive was not visible during assembly, array would have
> been considered clean.
> 
> The below patch is only to demonstrate the possible fix. The idea is
> that we decide that array is dirty only according to up-to-date
> drives. Currently the "clean" flag is initialized from the first
> superblock, which might not be the freshest one.
> 
> I haven't tested it at all. If you think the direction is reasonable,
> I will test.

Yes, that does seem reasonable.
If nothing surprising shows up in testing I'll be happy to apply it.

Thanks,
NeilBrown


> 
> Thanks,
>   Alex.
> 
> 
> diff --git a/Assemble.c b/Assemble.c
> index 23695e7..42cbbd5 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -929,7 +929,6 @@ int Assemble(struct supertype *st, char *mddev,
>                 st->minor_version = 90;
> 
>         st->ss->getinfo_super(st, content, NULL);
> -       clean = content->array.state & 1;
> 
>         /* now we have some devices that might be suitable.
>          * I wonder how many
> @@ -1122,6 +1121,7 @@ int Assemble(struct supertype *st, char *mddev,
>  #ifndef MDASSEMBLE
>         sysfs_init(content, mdfd, 0);
>  #endif
> +       clean = 1; /* Assume that array is clean, until we see otherwise */
>         for (i=0; i<bestcnt; i++) {
>                 int j = best[i];
>                 unsigned int desired_state;


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