linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
Cc: linux-raid@vger.kernel.org, wojciech.neubauer@intel.com,
	adam.kwolek@intel.com, dan.j.williams@intel.com,
	ed.ciechanowski@intel.com
Subject: Re: [PATCH] FIX: Cleanup after raid45->raid0 takeover
Date: Thu, 7 Apr 2011 11:18:38 +1000	[thread overview]
Message-ID: <20110407111838.2480b14c@notabene.brown> (raw)
In-Reply-To: <20110328133156.10923.98647.stgit@gklab-128-111.igk.intel.com>

On Mon, 28 Mar 2011 15:31:57 +0200 Krzysztof Wojcik
<krzysztof.wojcik@intel.com> wrote:

> Problem:
> After raid4->raid0 takeover operation, another takeover operation
> (e.g raid0->raid10) results "kernel oops".
> Root cause:
> Variables 'degraded' and 'plug' in mddev structure are not cleared
> after raid45->raid0 takeover.
> 
> This patch reset these variables.
> 
> Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
> ---
>  drivers/md/raid0.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> index c0ac457..5d47113 100644
> --- a/drivers/md/raid0.c
> +++ b/drivers/md/raid0.c
> @@ -603,6 +603,8 @@ static void *raid0_takeover_raid45(mddev_t *mddev)
>  	mddev->new_chunk_sectors = mddev->chunk_sectors;
>  	mddev->raid_disks--;
>  	mddev->delta_disks = -1;
> +	mddev->degraded = 0;
> +	mddev->plug = NULL;
>  	/* make sure it will be not marked as dirty */
>  	mddev->recovery_cp = MaxSector;
>  

This is wrong.

The raid0 'takeover' method is called while the raid5 array is still active,
and mddev->plug and mddev->degraded are used by the active array.  So you
cannot just clear them like this.  The new_* and delta_disks values are not
used by an active array - also raid_disks isn't used - each personality keeps
a copy of the effective raid_disks in it's private structure.

So the ->plug must be cleared in the raid5 stop routine, and degraded
should be cleared by level_store after calling ->stop on the raid5 and before
calling ->run on the raid0.

I have changed the patch to do that instead.

Thanks,
NeilBrown


      reply	other threads:[~2011-04-07  1:18 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-28 13:31 [PATCH] FIX: Cleanup after raid45->raid0 takeover Krzysztof Wojcik
2011-04-07  1:18 ` NeilBrown [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110407111838.2480b14c@notabene.brown \
    --to=neilb@suse.de \
    --cc=adam.kwolek@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=ed.ciechanowski@intel.com \
    --cc=krzysztof.wojcik@intel.com \
    --cc=linux-raid@vger.kernel.org \
    --cc=wojciech.neubauer@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).