linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: NeilBrown <neilb@suse.de>
Cc: "Kwolek, Adam" <adam.kwolek@intel.com>,
	"linux-raid@vger.kernel.org" <linux-raid@vger.kernel.org>,
	"Ciechanowski, Ed" <ed.ciechanowski@intel.com>,
	"Neubauer, Wojciech" <Wojciech.Neubauer@intel.com>,
	"Wojcik, Krzysztof" <krzysztof.wojcik@intel.com>
Subject: Re: Something wrong with __prep_thunderdome in super-intel.c
Date: Mon, 28 Mar 2011 09:56:49 -0700	[thread overview]
Message-ID: <1301331409.5888.8.camel@dwillia2-linux> (raw)
In-Reply-To: <20110328123509.043555e7@notabene.brown>

On Sun, 2011-03-27 at 18:35 -0700, NeilBrown wrote:
> On Thu, 24 Mar 2011 19:40:46 -0700 Dan Williams <dan.j.williams@intel.com>
> wrote:
> 
> > <context switch out of isci driver review mode>
> 
> :-)
> 
[..]
> > -	disk = get_imsm_disk(super, ord_to_idx(ord));
> > +	dl = get_imsm_dl_disk(super, ord_to_idx(ord));
> 
> This sometimes return NULL, leading to bad stuff and mdmon crashing....
> 
> So there is more to this than meets the eye...

Yes, (and I chalk this up to context switch latency), setting the index
to -2 is not correct as other paths need to be able to reference a valid
disk index until the failed device is removed via a rebuild.

> I'll stop trying this patch.

Ok, here is a proposed v2 on top of the latest devel-3.2, but I need to
play with it a bit more, and figure out what the spare migration test is
complaining about.

diff --git a/super-intel.c b/super-intel.c
index 6e12af2..e2f66aa 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -3993,7 +3993,7 @@ static int write_super_imsm(struct supertype *st, int doclose)
 
 	/* write the mpb for disks that compose raid devices */
 	for (d = super->disks; d ; d = d->next) {
-		if (d->index < 0)
+		if (d->index < 0 || is_failed(&d->disk))
 			continue;
 		if (store_imsm_mpb(d->fd, mpb))
 			fprintf(stderr, "%s: failed for device %d:%d %s\n",
@@ -5218,6 +5218,8 @@ static int mark_failure(struct imsm_dev *dev, struct imsm_disk *disk, int idx)
 	__u32 ord;
 	int slot;
 	struct imsm_map *map;
+	char buf[MAX_RAID_SERIAL_LEN+3];
+	unsigned int len, shift = 0;
 
 	/* new failures are always set in map[0] */
 	map = get_imsm_map(dev, 0);
@@ -5230,6 +5232,11 @@ static int mark_failure(struct imsm_dev *dev, struct imsm_disk *disk, int idx)
 	if (is_failed(disk) && (ord & IMSM_ORD_REBUILD))
 		return 0;
 
+	sprintf(buf, "%s:0", disk->serial);
+	if ((len = strlen(buf)) >= MAX_RAID_SERIAL_LEN)
+		shift = len - MAX_RAID_SERIAL_LEN + 1;
+	strncpy((char *)disk->serial, &buf[shift], MAX_RAID_SERIAL_LEN);
+
 	disk->status |= FAILED_DISK;
 	set_imsm_ord_tbl_ent(map, slot, idx | IMSM_ORD_REBUILD);
 	if (map->failed_disk_num == 0xff)




      reply	other threads:[~2011-03-28 16:56 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-14 14:09 [PATCH 0/3] UT and error case changes Adam Kwolek
2011-03-14 14:09 ` [PATCH 1/3] imsm: FIX: existing backup file fails unit tests Adam Kwolek
2011-03-14 14:09 ` [PATCH 2/3] External metadata has to be restored to initial state in error case Adam Kwolek
2011-03-14 14:09 ` [PATCH 3/3] imsm: Add metadata abort changes handler template Adam Kwolek
2011-03-14 21:53 ` [PATCH 0/3] UT and error case changes NeilBrown
2011-03-15  7:28   ` Kwolek, Adam
2011-03-18  2:07     ` NeilBrown
2011-03-22  2:23     ` Something wrong with __prep_thunderdome in super-intel.c NeilBrown
2011-03-25  2:40       ` Dan Williams
2011-03-25  8:43         ` Kwolek, Adam
2011-03-25 18:50           ` Dan Williams
2011-03-28  2:28           ` NeilBrown
2011-03-28  1:35         ` NeilBrown
2011-03-28 16:56           ` Dan Williams [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=1301331409.5888.8.camel@dwillia2-linux \
    --to=dan.j.williams@intel.com \
    --cc=Wojciech.Neubauer@intel.com \
    --cc=adam.kwolek@intel.com \
    --cc=ed.ciechanowski@intel.com \
    --cc=krzysztof.wojcik@intel.com \
    --cc=linux-raid@vger.kernel.org \
    --cc=neilb@suse.de \
    /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).