Boot failure with block/for-next

DISCUSSION James Bottomley 3 participants 11 messages Dec 22, 2010 – Dec 24, 2010 SCSI

Share & tools

James Bottomley

author maintainer

2010-12-22 17:27 UTC #0 lore raw

Trying to test out the SCSI post merge tree, I've found it won't boot on my SCSI test system. The reason is a failure to read the partition table of the root disc. It just gives

sda: unable to read partition table

The other four discs in the system seem to read their partition tables OK.

The obvious candidate is the partition code rework in block/for-next. Simply reverting

commit d2bf1b6723ed0eab378363649d15b7893bf14e91

Author: Tejun Heo <[email protected]>
Date:   Wed Dec 8 20:57:36 2010 +0100

block: move register_disk() and del_gendisk() to block/genhd.c

Doesn't fix the boot failure, so it's obviously something deeper.

James

Tejun Heo

2010-12-22 17:53 UTC #1 lore raw

Hello,

Quoted 8 lines · James Bottomley

Trying to test out the SCSI post merge tree, I've found it won't boot on my SCSI test system. The reason is a failure to read the partition table of the root disc. It just gives
sda: unable to read partition table
The other four discs in the system seem to read their partition tables OK.

That's really weird.

Quoted 10 lines

The obvious candidate is the partition code rework in block/for-next. Simply reverting
commit d2bf1b6723ed0eab378363649d15b7893bf14e91
Author: Tejun Heo <[email protected]>
Date:   Wed Dec 8 20:57:36 2010 +0100
block: move register_disk() and del_gendisk() to block/genhd.c
Doesn't fix the boot failure, so it's obviously something deeper.

Can you please apply the following patch and see what the kernel says?

Thanks.

fs/partitions/check.c +6 -1

index bdf8d3c..dd0b0f3 100644
@@ -186,6 +186,8 @@ check_partition(struct gendisk *hd, struct block_device *bdev)
 			/* We have hit an I/O error which we don't report now.
 		 	* But record it, and let the others do their job.
 		 	*/
+			printk(KERN_WARNING "XXX %s: check_part %pf failed with %d\n",
+			       state->name, check_part[i-1], res);
 			err = res;
 			res = 0;
 		}

@@ -197,8 +199,11 @@ check_partition(struct gendisk *hd, struct block_device *bdev)
 		free_page((unsigned long)state->pp_buf);
 		return state;
 	}
-	if (state->access_beyond_eod)
+	if (state->access_beyond_eod) {
+		printk(KERN_WARNING "XXX %s: access_beyond_eod, err=-ENOSPC\n",
+		       state->name);
 		err = -ENOSPC;
+	}
 	if (err)
 	/* The partition is unrecognized. So report I/O errors if there were any */
 		res = err;

fs/partitions/check.h +2 -0

index d68bf4d..f836fb2 100644
@@ -26,6 +26,8 @@ static inline void *read_part_sector(struct parsed_partitions *state,
 				     sector_t n, Sector *p)
 {
 	if (n >= get_capacity(state->bdev->bd_disk)) {
+		printk(KERN_WARNING "XXX %s: setting access_beyond_eod for sector %llu\n",
+		       state->name, (unsigned long long)n);
 		state->access_beyond_eod = true;
 		return NULL;
 	}

James Bottomley

2010-12-23 04:31 UTC #2 lore raw

In reply to James Bottomley

Quoted 11 lines · James Bottomley

Trying to test out the SCSI post merge tree, I've found it won't boot on my SCSI test system. The reason is a failure to read the partition table of the root disc. It just gives
sda: unable to read partition table
The other four discs in the system seem to read their partition tables OK.
The obvious candidate is the partition code rework in block/for-next. Simply reverting

Actually, surprisingly, bisection confirms it's this patch:

Author: Tejun Heo <[email protected]>
Date:   Wed Dec 8 20:57:42 2010 +0100

sd: implement sd_check_events()

I can't quite see how yet.

James

Tejun Heo

2010-12-23 10:09 UTC #3 lore raw

Hello, James.

Quoted 8 lines · James Bottomley

Actually, surprisingly, bisection confirms it's this patch:
Author: Tejun Heo <[email protected]>
Date:   Wed Dec 8 20:57:42 2010 +0100
sd: implement sd_check_events()
I can't quite see how yet.

Does the following patch fix the problem? Thanks.

Subject: sd: don't clear media presence unless it's removable

set_media_not_present() can be called for a non-removable device if it raises UA for whatever reason; however, block layer calls into sd_check_events() only if the device is removable. As the function is responsible for setting media presence, sd continues to think that media is missing on the non-removable device making it inaccessible.

This patch makes set_media_not_present() clear media presence iff the device is removable.

Signed-off-by Tejun Heo <[email protected]>

---
 drivers/scsi/sd.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/scsi/sd.c +5 -2

Index: work/drivers/scsi/sd.c
===================================================================
@@ -993,8 +993,11 @@ static void set_media_not_present(struct
 {
 	if (sdkp->media_present)
 		sdkp->device->changed = 1;
-	sdkp->media_present = 0;
-	sdkp->capacity = 0;
+
+	if (sdkp->device->removable) {
+		sdkp->media_present = 0;
+		sdkp->capacity = 0;
+	}
 }
 
 static int media_not_present(struct scsi_disk *sdkp,

James Bottomley

2010-12-23 15:27 UTC #4 lore raw

Quoted 8 lines · Tejun Heo

Hello, James.
Quoted 5 lines · James Bottomley
Actually, surprisingly, bisection confirms it's this patch:
Author: Tejun Heo <[email protected]>
Date:   Wed Dec 8 20:57:42 2010 +0100

Quoted 7 lines

Quoted 3 lines
sd: implement sd_check_events()
I can't quite see how yet.
Does the following patch fix the problem? Thanks.
Subject: sd: don't clear media presence unless it's removable

No, still the same failure. Here's an excised log of an unsuccessful and successful disk probe:

/bin/sh: can't access tty; job control turned off
(initramfs) dmesg|grep sda
sd 2:0:1:0: [sda] 71132960 512-byte logical blocks: (36.4 GB/33.9 GiB) sd 2:0:1:0: [sda] Write Protect is off
sd 2:0:1:0: [sda] Mode Sense: d3 00 10 08
sd 2:0:1:0: [sda] Write cache: disabled, read cache: enabled, supports DPO and FUA
device: 'sda': device_add
PM: Adding info for No Bus:sda
sda: unable to read partition table
sd 2:0:1:0: [sda] Attached SCSI disk
(initramfs) dmesg|grep sdb
sd 2:0:2:0: [sdb] 17942584 512-byte logical blocks: (9.18 GB/8.55 GiB) sd 2:0:2:0: [sdb] Write Protect is off
sd 2:0:2:0: [sdb] Mode Sense: e3 00 10 08
sd 2:0:2:0: [sdb] Write cache: disabled, read cache: enabled, supports DPO and FUA
device: 'sdb': device_add
PM: Adding info for No Bus:sdb
sdb: sdb1 sdb2
device: 'sdb1': device_add
PM: Adding info for No Bus:sdb1
device: 'sdb2': device_add
PM: Adding info for No Bus:sdb2
sd 2:0:2:0: [sdb] Attached SCSI disk

James

Tejun Heo

2010-12-23 15:52 UTC #5 lore raw

Hey,

Quoted 4 lines · James Bottomley

Quoted 1 line
Subject: sd: don't clear media presence unless it's removable
No, still the same failure. Here's an excised log of an unsuccessful and successful disk probe:

Hmmm... That's unfortunate.

Quoted 15 lines

/bin/sh: can't access tty; job control turned off
(initramfs) dmesg|grep sda
sd 2:0:1:0: [sda] 71132960 512-byte logical blocks: (36.4 GB/33.9 GiB) sd 2:0:1:0: [sda] Write Protect is off
sd 2:0:1:0: [sda] Mode Sense: d3 00 10 08
sd 2:0:1:0: [sda] Write cache: disabled, read cache: enabled, supports DPO and FUA
device: 'sda': device_add
PM: Adding info for No Bus:sda
sda: unable to read partition table
sd 2:0:1:0: [sda] Attached SCSI disk
(initramfs) dmesg|grep sdb
sd 2:0:2:0: [sdb] 17942584 512-byte logical blocks: (9.18 GB/8.55 GiB) sd 2:0:2:0: [sdb] Write Protect is off
sd 2:0:2:0: [sdb] Mode Sense: e3 00 10 08
sd 2:0:2:0: [sdb] Write cache: disabled, read cache: enabled, supports DPO and FUA

Quoted 8 lines

device: 'sdb': device_add
PM: Adding info for No Bus:sdb
sdb: sdb1 sdb2
device: 'sdb1': device_add
PM: Adding info for No Bus:sdb1
device: 'sdb2': device_add
PM: Adding info for No Bus:sdb2
sd 2:0:2:0: [sdb] Attached SCSI disk

Can you please apply the debug patch I posted in the other message and post the boot log? Let's see how the partition read is failing.

Thanks.

Signature · 1 line

tejun

James Bottomley

2010-12-23 16:10 UTC #6 lore raw

Quoted 34 lines · Tejun Heo

Hey,
Quoted 4 lines · James Bottomley
Quoted 1 line
Subject: sd: don't clear media presence unless it's removable
No, still the same failure. Here's an excised log of an unsuccessful and successful disk probe:
Hmmm... That's unfortunate.
Quoted 15 lines
/bin/sh: can't access tty; job control turned off
(initramfs) dmesg|grep sda
sd 2:0:1:0: [sda] 71132960 512-byte logical blocks: (36.4 GB/33.9 GiB) sd 2:0:1:0: [sda] Write Protect is off
sd 2:0:1:0: [sda] Mode Sense: d3 00 10 08
sd 2:0:1:0: [sda] Write cache: disabled, read cache: enabled, supports DPO and FUA
device: 'sda': device_add
PM: Adding info for No Bus:sda
sda: unable to read partition table
sd 2:0:1:0: [sda] Attached SCSI disk
(initramfs) dmesg|grep sdb
sd 2:0:2:0: [sdb] 17942584 512-byte logical blocks: (9.18 GB/8.55 GiB) sd 2:0:2:0: [sdb] Write Protect is off
sd 2:0:2:0: [sdb] Mode Sense: e3 00 10 08
sd 2:0:2:0: [sdb] Write cache: disabled, read cache: enabled, supports DPO and FUA
Quoted 8 lines
device: 'sdb': device_add
PM: Adding info for No Bus:sdb
sdb: sdb1 sdb2
device: 'sdb1': device_add
PM: Adding info for No Bus:sdb1
device: 'sdb2': device_add
PM: Adding info for No Bus:sdb2
sd 2:0:2:0: [sdb] Attached SCSI disk

Quoted 2 lines

Can you please apply the debug patch I posted in the other message and post the boot log? Let's see how the partition read is failing.

That's actually a red herring ... the disk isn't spinning up, so the partition read is getting a not ready.

James

Tejun Heo

2010-12-23 16:13 UTC #7 lore raw

Quoted 5 lines · James Bottomley

Quoted 2 lines
Can you please apply the debug patch I posted in the other message and post the boot log? Let's see how the partition read is failing.
That's actually a red herring ... the disk isn't spinning up, so the partition read is getting a not ready.

Oh, yay, but at any rate I think the don't-clear-media-presence patch is probably a good idea just in case UA gets reported somehow.

Thanks.

Signature · 1 line

tejun

James Bottomley

2010-12-23 18:25 UTC #8 lore raw

Quoted 9 lines · Tejun Heo

Quoted 5 lines · James Bottomley
Quoted 2 lines
Can you please apply the debug patch I posted in the other message and post the boot log? Let's see how the partition read is failing.
That's actually a red herring ... the disk isn't spinning up, so the partition read is getting a not ready.
Oh, yay, but at any rate I think the don't-clear-media-presence patch is probably a good idea just in case UA gets reported somehow.

Well, it wasn't this either. It turns out that this disk alone reports UNIT_ATTENTION RESET_OCCURRED on the first TEST UNIT READY of spin up. Ordinarily this is harmless, but the new medium change code wrongly interprets any UNIT_ATTENTION as medium changed (and then refuses to talk to the disk). This is actually a change from the previous code, so the fix is to put it back the way it was.

James

---

drivers/scsi/sd.c +0 -2

index 6a88fc1..7d25746 100644
@@ -1008,8 +1008,6 @@ static int media_not_present(struct scsi_disk *sdkp,
 	/* not invoked for commands that could return deferred errors */
 	switch (sshdr->sense_key) {
 	case UNIT_ATTENTION:
-		sdkp->device->changed = 1;
-		/* fall through */
 	case NOT_READY:
 		/* medium not present */
 		if (sshdr->asc == 0x3A) {

Tejun Heo

2010-12-24 11:03 UTC #9 lore raw

Hello, James.

Quoted 11 lines · James Bottomley

Quoted 9 lines · Tejun Heo
Quoted 5 lines · James Bottomley
Quoted 2 lines
Can you please apply the debug patch I posted in the other message and post the boot log? Let's see how the partition read is failing.
That's actually a red herring ... the disk isn't spinning up, so the partition read is getting a not ready.
Oh, yay, but at any rate I think the don't-clear-media-presence patch is probably a good idea just in case UA gets reported somehow.

Quoted 6 lines

Well, it wasn't this either. It turns out that this disk alone reports UNIT_ATTENTION RESET_OCCURRED on the first TEST UNIT READY of spin up. Ordinarily this is harmless, but the new medium change code wrongly interprets any UNIT_ATTENTION as medium changed (and then refuses to talk to the disk). This is actually a change from the previous code, so the fix is to put it back the way it was.

I see. I wonder why the previous patch didn't work. It should have had about the same effect. I think the UA change went in there while trying to bring sr and sd behaviors closer to each other, but yes it seems the original code didn't have that. That said, now there are paths where UA would be consumed without setting ->changed and thus sd may miss media change events. This has been like this for quite some time and there aren't many removable sd devices these days, so maybe this doesn't matter too much.

Anyways, for now, the change looks good to me too. Thanks.

Signature · 1 line

tejun

James Bottomley

2010-12-24 15:47 UTC #10 lore raw

Quoted 14 lines · Tejun Heo

Hello, James.
Quoted 11 lines · James Bottomley
Quoted 9 lines · Tejun Heo
Quoted 5 lines · James Bottomley
Quoted 2 lines
Can you please apply the debug patch I posted in the other message and post the boot log? Let's see how the partition read is failing.
That's actually a red herring ... the disk isn't spinning up, so the partition read is getting a not ready.
Oh, yay, but at any rate I think the don't-clear-media-presence patch is probably a good idea just in case UA gets reported somehow.

Quoted 8 lines

Quoted 6 lines
Well, it wasn't this either. It turns out that this disk alone reports UNIT_ATTENTION RESET_OCCURRED on the first TEST UNIT READY of spin up. Ordinarily this is harmless, but the new medium change code wrongly interprets any UNIT_ATTENTION as medium changed (and then refuses to talk to the disk). This is actually a change from the previous code, so the fix is to put it back the way it was.
I see. I wonder why the previous patch didn't work.

Oh, you didn't put a ->removable check in enough paths. The setting on UA was still unconditional, as was the check in sd_prep_fn().

Quoted 8 lines

It should have
had about the same effect. I think the UA change went in there while trying to bring sr and sd behaviors closer to each other, but yes it seems the original code didn't have that. That said, now there are paths where UA would be consumed without setting ->changed and thus sd may miss media change events. This has been like this for quite some time and there aren't many removable sd devices these days, so maybe this doesn't matter too much.

The code can never really be merged. for CD/DVD, UA pretty much does mean medium removal. Discs and arrays emit a panoply of UA events (it's the SCSI asynchronous event mechanism) if you assume media change on all of them, there'll be terrible confusion. We can narrow to 28/00 that means medium may have changed. It could really do with checking by someone who has a removable disc device, though ... it looks like some of the 3B/xx might be applicable.

Quoted 1 line

Anyways, for now, the change looks good to me too. Thanks.

Thanks,

James

---

drivers/scsi/sd.c +4 -1

index 7d25746..1995533 100644
@@ -578,7 +578,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
 		goto out;
 	}
 
-	if (sdp->changed) {
+	if (sdp->removable && sdp->changed) {
 		/*
 		 * quietly refuse to do anything to a changed disc until 
 		 * the changed bit has been reset

@@ -1008,6 +1008,9 @@ static int media_not_present(struct scsi_disk *sdkp,
 	/* not invoked for commands that could return deferred errors */
 	switch (sshdr->sense_key) {
 	case UNIT_ATTENTION:
+		if (sdkp->device->removable && sshdr->asc == 0x28 &&
+		    sshdr->ascq == 0x00)
+			sdkp->device->changed = 1;
 	case NOT_READY:
 		/* medium not present */
 		if (sshdr->asc == 0x3A) {