On Wed, 30 Nov 2011 08:17:32 +0000 "Kwolek, Adam" <adam.kwolek@xxxxxxxxx> wrote: > > > > -----Original Message----- > > From: NeilBrown [mailto:neilb@xxxxxxx] > > Sent: Wednesday, November 30, 2011 1:14 AM > > To: Kwolek, Adam > > Cc: linux-raid@xxxxxxxxxxxxxxx; Ciechanowski, Ed; Labun, Marcin; Williams, > > Dan J > > Subject: Re: [PATCH] md: Add ability for disable bad block management > > > > On Thu, 24 Nov 2011 13:19:53 +0100 Adam Kwolek > > <adam.kwolek@xxxxxxxxx> wrote: > > > > > When external metadata doesn't support BBM, mdadm cannot answer > > > correctly for BBM requests. It causes reshape process being stopped. > > > > > > Add ability for external metadata /mdadm/ to disable BBM via sysfs. > > > md will ignore bad blocks as it is for metadata v0.90. > > > > This should not be necessary. > > > > The intention is that a device with a bad block looks exactly like a device with > > a failed device. i.e. 'faulty' and 'blocked' appear in the 'state' > > file. > > > > If the metadata doesn't support a bad-block list, it will record that the device > > has failed and will unblock the device. At that point the failure is forced. > > If the metadata does support a bad block list it will just record the bad blocks > > and acknowledge them, and the unblock the device. At that point the device > > won't be failed, the 'faulty' state will disappear, and it will continue to be > > used with the known bad blocks. > > > > What exactly is going wrong that makes you think you need this patch? > > > When degradation occurs during migration BBM is signaled to mdmon and mdmon /monitor.c/ tries to mark disk '-blocked' > This operation fails. Momon goes in to loop, and nothing can be done /I cannot make it using sysfs/ to signal or remove device. > In sysfs device is present in /sys/block/mdXXX/md but entry /sys/block/mdXXX/md/dev-sdX/~block is missing /disk was pulled out/. I've found a couple of issues. I'm not sure if they completely explain what you are seeing. Could you please test with these two fixes and tell me the results? Firstly, I find that writing "-blocked" succeeds (no error returned) but the "blocked" flag does not get cleared, which is certainly confusing. This is fixed by: diff --git a/drivers/md/md.c b/drivers/md/md.c index 4adcbb4..7258dc1 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -2562,7 +2562,8 @@ state_show(struct md_rdev *rdev, char *page) sep = ","; } if (test_bit(Blocked, &rdev->flags) || - rdev->badblocks.unacked_exist) { + (rdev->badblocks.unacked_exist + && !test_bit(Faulty, &rdev->flags))) { len += sprintf(page+len, "%sblocked", sep); sep = ","; } Secondly mdmon writes "-blocked" even when the "blocked" flag is not set. This succeeds so state_store() calls sysfs_notify_dirent_safe(rdev->sysfs_state); so mdmon/monitor.c is woken up to go around the loop again and it writes "-blocked" again and so it continues in a loop. This is fixed by: diff --git a/monitor.c b/monitor.c index b002e90..29bde18 100644 --- a/monitor.c +++ b/monitor.c @@ -339,7 +339,8 @@ static int read_and_act(struct active_array *a) a->container->ss->set_disk(a, mdi->disk.raid_disk, mdi->curr_state); check_degraded = 1; - mdi->next_state |= DS_UNBLOCK; + if (mdi->curr_state & DS_BLOCKED) + mdi->next_state |= DS_UNBLOCK; if (a->curr_state == read_auto) { a->container->ss->set_array_state(a, 0); a->next_state = active; Finally, when a badblock is added to the list we don't currently notify rdev->sysfs_state so mdmon doesn't notice straight away and so is delayed in taking action. It will only notice when a write blocks. This is fixed by: diff --git a/drivers/md/md.c b/drivers/md/md.c index 4adcbb4..9cc7983 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -7940,6 +7941,7 @@ int rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors, s + rdev->data_offset, sectors, acknowledged); if (rv) { /* Make sure they get written out promptly */ + sysfs_notify_dirent_safe(rdev->sysfs_state); set_bit(MD_CHANGE_CLEAN, &rdev->mddev->flags); md_wakeup_thread(rdev->mddev->thread); } With these 3 changes in place I get substantially improved behaviour on my simple test (just doing resync, not reshape). Thanks, NeilBrown
Attachment:
signature.asc
Description: PGP signature
