kernel BUG at drivers/block/ub.c:820!

kernel BUG at drivers/block/ub.c:820!

[Martin Schwidefsky]

Greetings,
I have an 'evil' USB stick that crashes my T43 with the yesterdays git.
All I have to do is to insert the USB stick and watch the kernel die:

kernel BUG at drivers/block/ub.c:820!
invalid opcode: 0000 [#1] PREEMPT
Modules linked in: usb_storage ub ipt_LOG xt_limit ipt_REJECT xt_tcpudp
xt_state iptable_nat nf_nat nf_conntrack_ipv4 nf_conntrack
iptable_filter ip_tables x_tables deflate zlib_deflate ctr twofish
twofish_common camellia serpent blowfish des_generic cbc aes_i586
aes_generic xcbc sha256_generic sha1_generic crypto_null af_key
nls_iso8859_1 nsl_cp437 vfat fat fuse dm_crypt crypto_blkcipher
dm_snapshot dm_mirror dm_mod freq_table ide_generic ide_disk irtty_sir
mousedev sir_dev snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm_oss
snd_mixer_oss snd_pcm snd_timer pcmcia nsc_ircc snd_ipw2200 psmouse
sdhci irda ieee80211 ieee80211_crypt serio_raw soundcore i2c_i801 rtc
crc_ccitt parport_pc parport 8250_pnp pcspkr snd_page_alloc i2c_core
8250_pci yenta_socket rsrc_nonstatic pcmcia_core mmc_core firmware_class
video output intel_agp agpgart evdev thinkpad_acpi sg sr_mod sd_mod
ide_pci_generic ata_piix floppy pata_acpi libata scsi_mod ehci_hcd
uhci_hcd usbcore tg3 unix ide_cd_mod cdrom

Pid: 3893, comm: udevd Not tainted (2.6.25-rc8 #4)
EIP: 0060:[<f8ca6b99>] EFLAGS: 00010002 CPU: 0
EIP is at ub_end_rq+0x2b/0x33 [ub]
EAX: 00000001 EBX: f78f296c ECX: 00000024 EDX: 00000001
ESI: 00000000 EDI: f7bb5ad4 EBP: f79fde0c ESP: f79fde04
 DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
Process udevd (pid: 3893, ti=f79fc000 task=f64e01b0 task.ti=f79fc000)
Stack: f7bb5ad4 00000000 f79fde34 f8ca6fa1 f7b6f400 f7bb5a00 f7bb5a34 f78f296c
       00000086 f7b6f400 f7b6f400 f7bb5ad4 f79fde48 f8ca73b6 f7b6f400 c03ec2ec
       0000000a f79fde6c f8ca7c06 f7b6f5c0 f7b6f550 00000286 f7bb5ad4 00000000
Call Trace:
 [<f8ca6fa1>] ? ub_rw_cmd_done+0x1c2/0x1d8 [ub]
 [<f8ca73b6>] ? ub_scsi_urb_compl+0x31c/0x35b [ub]
 [<f8ca7c06>] ? ub_scsi_action+0x18f/0x1c4 [ub]
 [<c011e0bf>] ? tasklet_action+0x35/0x56
 [<c011e01e>] ? __do_softirq+0x38/0x7a
 [<c011e085>] ? do_softirq+0x25/0x2a
 [<c011e1db>] ? irq_exit+0x28/0x61
 [<c010630f>] ? do_IRQ+0x58/0x6d
 [<c01052c3>] ? common_interrupt+0x23/0x28
 [<c024cf52>] ? skb_dequeue+0x3d/0x50
 [<c024d2a0>] ? skb_queue_purge+0x14/0x1b
 [<f8843544>] ? unix_sock_destructor+0xe/0xb4 [unix]
 [<f8841e65>] ? unix_release+0x1a/0x20 [unix]
 [<c0248ee6>] ? sock_release+0x14/0x6f
 [<c02492fe>] ? sock_close+0x29/0x30
 [<c015afa5>] ? __fput+0x93/0x14a
 [<c015b1e4>] ? fput+0x16/0x18
 [<c0158aad>] ? filp_close+0x50/0x5a
 [<c0159d1e>] ? sys_close+0x6f/0xb9
 [<c01048b2>] ? sysenter_past_esp+0x5f/0x85
 [<c02a0000>] ? __xfrm_state_bump_genids+0x22/0xfd
 =======================
Code: 55 89 e5 56 31 f6 85 d2 53 89 c3 74 08 89 50 7c be fb ff ff ff 89 d8 e8 75
 b5 51 c7 89 f2 89 c1 89 d8 e8 30 c0 51 c7 85 c0 74 04 <0f> 0b eb fe 5b 5e 5d c3
 55 89 e5 57 56 53 83 ec 14 89 45 e8 8b
EIP: [<f8ca6b99>] ub_end_rq+0x2b/0x33 [ub] SS:ESP 0068:f79fde04
Kernel panic - not syncing: Fatal exception in interrupt

Looking at the code I find:

static void ub_end_rq(struct request *rq, unsigned int scsi_status)
{
        int error;

	if (scsi_status == 0) {
		error = 0;
	} else {
		error = -EIO;
		rq->errors = scsi_status;
	}
	if (__blk_end_request(rq, error, blk_rq_bytes(rq)))
		BUG();  <-- kernel crashes here.
	}
}

With git bisect I could came up with:
610d8b0c972e3b75493efef8e96175518fd736d3 good
3bcddeac1c4c7e6fb90531b80f236b1a05dfe514 compile error
5450d3e1d68f10be087f0855d8bad5458b50ecbe compile error
b8286239ddaf2632cec65c01e68a403ac4c3d079 compile error
7d699bafe258ebd8f9b4ec182c554200b369a504 bad

If I replace the BUG() with a printk my kernel survives the usb plug but
udev doesn't find the stick.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.

Re: kernel BUG at drivers/block/ub.c:820!

[Boaz Harrosh]

On Thu, Apr 03 2008 at 11:42 +0300, Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
> Greetings,
> I have an 'evil' USB stick that crashes my T43 with the yesterdays git.
> All I have to do is to insert the USB stick and watch the kernel die:
> 
> kernel BUG at drivers/block/ub.c:820!
> invalid opcode: 0000 [#1] PREEMPT
> Modules linked in: usb_storage ub ipt_LOG xt_limit ipt_REJECT xt_tcpudp
> xt_state iptable_nat nf_nat nf_conntrack_ipv4 nf_conntrack
> iptable_filter ip_tables x_tables deflate zlib_deflate ctr twofish
> twofish_common camellia serpent blowfish des_generic cbc aes_i586
> aes_generic xcbc sha256_generic sha1_generic crypto_null af_key
> nls_iso8859_1 nsl_cp437 vfat fat fuse dm_crypt crypto_blkcipher
> dm_snapshot dm_mirror dm_mod freq_table ide_generic ide_disk irtty_sir
> mousedev sir_dev snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm_oss
> snd_mixer_oss snd_pcm snd_timer pcmcia nsc_ircc snd_ipw2200 psmouse
> sdhci irda ieee80211 ieee80211_crypt serio_raw soundcore i2c_i801 rtc
> crc_ccitt parport_pc parport 8250_pnp pcspkr snd_page_alloc i2c_core
> 8250_pci yenta_socket rsrc_nonstatic pcmcia_core mmc_core firmware_class
> video output intel_agp agpgart evdev thinkpad_acpi sg sr_mod sd_mod
> ide_pci_generic ata_piix floppy pata_acpi libata scsi_mod ehci_hcd
> uhci_hcd usbcore tg3 unix ide_cd_mod cdrom
> 
> Pid: 3893, comm: udevd Not tainted (2.6.25-rc8 #4)
> EIP: 0060:[<f8ca6b99>] EFLAGS: 00010002 CPU: 0
> EIP is at ub_end_rq+0x2b/0x33 [ub]
> EAX: 00000001 EBX: f78f296c ECX: 00000024 EDX: 00000001
> ESI: 00000000 EDI: f7bb5ad4 EBP: f79fde0c ESP: f79fde04
>  DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
> Process udevd (pid: 3893, ti=f79fc000 task=f64e01b0 task.ti=f79fc000)
> Stack: f7bb5ad4 00000000 f79fde34 f8ca6fa1 f7b6f400 f7bb5a00 f7bb5a34 f78f296c
>        00000086 f7b6f400 f7b6f400 f7bb5ad4 f79fde48 f8ca73b6 f7b6f400 c03ec2ec
>        0000000a f79fde6c f8ca7c06 f7b6f5c0 f7b6f550 00000286 f7bb5ad4 00000000
> Call Trace:
>  [<f8ca6fa1>] ? ub_rw_cmd_done+0x1c2/0x1d8 [ub]
>  [<f8ca73b6>] ? ub_scsi_urb_compl+0x31c/0x35b [ub]
>  [<f8ca7c06>] ? ub_scsi_action+0x18f/0x1c4 [ub]
>  [<c011e0bf>] ? tasklet_action+0x35/0x56
>  [<c011e01e>] ? __do_softirq+0x38/0x7a
>  [<c011e085>] ? do_softirq+0x25/0x2a
>  [<c011e1db>] ? irq_exit+0x28/0x61
>  [<c010630f>] ? do_IRQ+0x58/0x6d
>  [<c01052c3>] ? common_interrupt+0x23/0x28
>  [<c024cf52>] ? skb_dequeue+0x3d/0x50
>  [<c024d2a0>] ? skb_queue_purge+0x14/0x1b
>  [<f8843544>] ? unix_sock_destructor+0xe/0xb4 [unix]
>  [<f8841e65>] ? unix_release+0x1a/0x20 [unix]
>  [<c0248ee6>] ? sock_release+0x14/0x6f
>  [<c02492fe>] ? sock_close+0x29/0x30
>  [<c015afa5>] ? __fput+0x93/0x14a
>  [<c015b1e4>] ? fput+0x16/0x18
>  [<c0158aad>] ? filp_close+0x50/0x5a
>  [<c0159d1e>] ? sys_close+0x6f/0xb9
>  [<c01048b2>] ? sysenter_past_esp+0x5f/0x85
>  [<c02a0000>] ? __xfrm_state_bump_genids+0x22/0xfd
>  =======================
> Code: 55 89 e5 56 31 f6 85 d2 53 89 c3 74 08 89 50 7c be fb ff ff ff 89 d8 e8 75
>  b5 51 c7 89 f2 89 c1 89 d8 e8 30 c0 51 c7 85 c0 74 04 <0f> 0b eb fe 5b 5e 5d c3
>  55 89 e5 57 56 53 83 ec 14 89 45 e8 8b
> EIP: [<f8ca6b99>] ub_end_rq+0x2b/0x33 [ub] SS:ESP 0068:f79fde04
> Kernel panic - not syncing: Fatal exception in interrupt
> 
> Looking at the code I find:
> 
> static void ub_end_rq(struct request *rq, unsigned int scsi_status)
> {
>         int error;
> 
> 	if (scsi_status == 0) {
> 		error = 0;
> 	} else {
> 		error = -EIO;
> 		rq->errors = scsi_status;
> 	}
> 	if (__blk_end_request(rq, error, blk_rq_bytes(rq)))
> 		BUG();  <-- kernel crashes here.
> 	}
> }
> 
> With git bisect I could came up with:
> 610d8b0c972e3b75493efef8e96175518fd736d3 good
> 3bcddeac1c4c7e6fb90531b80f236b1a05dfe514 compile error
> 5450d3e1d68f10be087f0855d8bad5458b50ecbe compile error
> b8286239ddaf2632cec65c01e68a403ac4c3d079 compile error
> 7d699bafe258ebd8f9b4ec182c554200b369a504 bad
> 
> If I replace the BUG() with a printk my kernel survives the usb plug but
> udev doesn't find the stick.
> 

It is not the right fix but what happens if you change above code to this:

static void ub_end_rq(struct request *rq, unsigned int scsi_status)
{
        int error;

	if (scsi_status == 0) {
		error = 0;
	} else {
		error = -EIO;
		rq->errors = scsi_status;
	}
	if (__blk_end_request(rq, error, blk_rq_bytes(rq)))
		__blk_end_request(rq, error, ~0);  /* <-- added line BUG removed. */
	}
}

Boaz

Re: kernel BUG at drivers/block/ub.c:820!

[Martin Schwidefsky]

On Thu, 2008-04-03 at 14:32 +0300, Boaz Harrosh wrote:
> > With git bisect I could came up with:
> > 610d8b0c972e3b75493efef8e96175518fd736d3 good
> > 3bcddeac1c4c7e6fb90531b80f236b1a05dfe514 compile error
> > 5450d3e1d68f10be087f0855d8bad5458b50ecbe compile error
> > b8286239ddaf2632cec65c01e68a403ac4c3d079 compile error
> > 7d699bafe258ebd8f9b4ec182c554200b369a504 bad
> > 
> > If I replace the BUG() with a printk my kernel survives the usb plug but
> > udev doesn't find the stick.
> > 
> 
> It is not the right fix but what happens if you change above code to this:
> 
> static void ub_end_rq(struct request *rq, unsigned int scsi_status)
> {
>         int error;
> 
>         if (scsi_status == 0) {
>                 error = 0;
>         } else {
>                 error = -EIO;
>                 rq->errors = scsi_status;
>         }
>         if (__blk_end_request(rq, error, blk_rq_bytes(rq)))
>                 __blk_end_request(rq, error, ~0);  /* <-- added line BUG removed. */
>         }
> }

I've tried this patch:

--- snip
diff --git a/drivers/block/ub.c b/drivers/block/ub.c
index c452e2d..0031e72 100644
--- a/drivers/block/ub.c
+++ b/drivers/block/ub.c
@@ -808,7 +808,7 @@ static void ub_rw_cmd_done(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
 
 static void ub_end_rq(struct request *rq, unsigned int scsi_status)
 {
-	int error;
+	int error, size;
 
 	if (scsi_status == 0) {
 		error = 0;
@@ -816,8 +816,12 @@ static void ub_end_rq(struct request *rq, unsigned int scsi_status)
 		error = -EIO;
 		rq->errors = scsi_status;
 	}
-	if (__blk_end_request(rq, error, blk_rq_bytes(rq)))
-		BUG();
+	size = blk_rq_bytes(rq);
+	if (__blk_end_request(rq, error, size)) {
+		  printk(KERN_WARNING "ub_end_rq: __blk_end_request failed "
+			 "with size %i\n", size);
+		__blk_end_request(rq, error, ~0);
+	}
 }
 
 static int ub_rw_cmd_retry(struct ub_dev *sc, struct ub_lun *lun,

--- snip

The output on the console:

# usb 5-4: new high speed USB device using ehci_hcd and address 3
# usb 5-4: configuration #1 chosen from 1 choice
#  uba: uba1
# usbcore: registered new interface driver ub
# Initializing USB Mass Storage driver...
# usbcore: registered new interface driver usb-storage
# USB Mass Storage support registered.
# ub_end_rq: __blk_end_request failed with size 218
# ub_end_rq: __blk_end_request failed with size 218
# ub_end_rq: __blk_end_request failed with size 218
# ub_end_rq: __blk_end_request failed with size 218

I can mount the stick and read files from it without any additional
warnings. Just the initial detection seems to create the odd sized
requests.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.

Re: kernel BUG at drivers/block/ub.c:820!

[Boaz Harrosh]

On Thu, Apr 03 2008 at 16:57 +0300, Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
> On Thu, 2008-04-03 at 14:32 +0300, Boaz Harrosh wrote:
>>> With git bisect I could came up with:
>>> 610d8b0c972e3b75493efef8e96175518fd736d3 good
>>> 3bcddeac1c4c7e6fb90531b80f236b1a05dfe514 compile error
>>> 5450d3e1d68f10be087f0855d8bad5458b50ecbe compile error
>>> b8286239ddaf2632cec65c01e68a403ac4c3d079 compile error
>>> 7d699bafe258ebd8f9b4ec182c554200b369a504 bad
>>>
>>> If I replace the BUG() with a printk my kernel survives the usb plug but
>>> udev doesn't find the stick.
>>>
>> It is not the right fix but what happens if you change above code to this:
>>
>> static void ub_end_rq(struct request *rq, unsigned int scsi_status)
>> {
>>         int error;
>>
>>         if (scsi_status == 0) {
>>                 error = 0;
>>         } else {
>>                 error = -EIO;
>>                 rq->errors = scsi_status;
>>         }
>>         if (__blk_end_request(rq, error, blk_rq_bytes(rq)))
>>                 __blk_end_request(rq, error, ~0);  /* <-- added line BUG removed. */
>>         }
>> }
> 
> I've tried this patch:
> 
> --- snip
> diff --git a/drivers/block/ub.c b/drivers/block/ub.c
> index c452e2d..0031e72 100644
> --- a/drivers/block/ub.c
> +++ b/drivers/block/ub.c
> @@ -808,7 +808,7 @@ static void ub_rw_cmd_done(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
>  
>  static void ub_end_rq(struct request *rq, unsigned int scsi_status)
>  {
> -	int error;
> +	int error, size;
>  
>  	if (scsi_status == 0) {
>  		error = 0;
> @@ -816,8 +816,12 @@ static void ub_end_rq(struct request *rq, unsigned int scsi_status)
>  		error = -EIO;
>  		rq->errors = scsi_status;
>  	}
> -	if (__blk_end_request(rq, error, blk_rq_bytes(rq)))
> -		BUG();
> +	size = blk_rq_bytes(rq);
> +	if (__blk_end_request(rq, error, size)) {
> +		  printk(KERN_WARNING "ub_end_rq: __blk_end_request failed "
> +			 "with size %i\n", size);
> +		__blk_end_request(rq, error, ~0);
> +	}
>  }
>  
>  static int ub_rw_cmd_retry(struct ub_dev *sc, struct ub_lun *lun,
> 
> --- snip
> 
> The output on the console:
> 
> # usb 5-4: new high speed USB device using ehci_hcd and address 3
> # usb 5-4: configuration #1 chosen from 1 choice
> #  uba: uba1
> # usbcore: registered new interface driver ub
> # Initializing USB Mass Storage driver...
> # usbcore: registered new interface driver usb-storage
> # USB Mass Storage support registered.
> # ub_end_rq: __blk_end_request failed with size 218
> # ub_end_rq: __blk_end_request failed with size 218
> # ub_end_rq: __blk_end_request failed with size 218
> # ub_end_rq: __blk_end_request failed with size 218
> 
> I can mount the stick and read files from it without any additional
> warnings. Just the initial detection seems to create the odd sized
> requests.
> 

OK So first this confirms that for ages the ub.c driver was leaking BIO's
on first connection. Just that it was never noticed before.

or that we have a BLOCK_PC at hand but before the 7d699baf patch
we completed with - rq->hard_nr_sectors << 9 - where now blk_rq_bytes(rq)
will return 218 which is less. Could you also put rq->hard_nr_sectors
in the print above?

Did you mange to find what is that 218 bytes command. Put a WARN_ON(1)
in ub_request_fn_1 for any command that has a blk_rq_bytes(rq) of 218
so we'll see who issues these commands. And what is the real bug.

Boaz

Re: kernel BUG at drivers/block/ub.c:820!

[Martin Schwidefsky]

On Thu, 2008-04-03 at 17:15 +0300, Boaz Harrosh wrote:
> OK So first this confirms that for ages the ub.c driver was leaking
> BIO's
> on first connection. Just that it was never noticed before.
> 
> or that we have a BLOCK_PC at hand but before the 7d699baf patch
> we completed with - rq->hard_nr_sectors << 9 - where now
> blk_rq_bytes(rq)
> will return 218 which is less. Could you also put rq->hard_nr_sectors
> in the print above?
> 
> Did you mange to find what is that 218 bytes command. Put a WARN_ON(1)
> in ub_request_fn_1 for any command that has a blk_rq_bytes(rq) of 218
> so we'll see who issues these commands. And what is the real bug.

I've added a WARN_ON(blk_rq_bytes(rq) == 218) right before the
ub_submit_scsi in ub_request_fn_1. ub_end_rq still reports the odd size
but I get nothing from ub_request_fn_1. Interesting.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.

Re: kernel BUG at drivers/block/ub.c:820!

[Martin Schwidefsky]

Hi Kiyoshi,

On Thu, 2008-04-03 at 11:08 -0500, Kiyoshi Ueda wrote:
> I believe the patch from Pete fixes this BUG():
>     http://marc.info/?l=linux-usb&m=120613888005382&w=2
> 
> Could you try that?

Just tested the patch and it indeed fixes the bug.

> Original ub driver was probably ignoring bio completion on BLOCK_PC,
> and I missed the conversion to blk_end_request for BLOCK_PC.
> Sorry about that.

Could we push this upstream for 2.6.25? It is a regression, I would
prefer to have a 2.6.25 that doesn't panic my machine when I attach a
USB stick..

Anyway, thanks for the patch. For now my machine is happy again :-)

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.

Re: kernel BUG at drivers/block/ub.c:820!

[Kiyoshi Ueda]

Hi Martin,

On Thu, 03 Apr 2008 10:42:36 +0200, Martin Schwidefsky wrote:
> kernel BUG at drivers/block/ub.c:820!
> invalid opcode: 0000 [#1] PREEMPT
> Modules linked in: usb_storage ub ipt_LOG xt_limit ipt_REJECT xt_tcpudp
> xt_state iptable_nat nf_nat nf_conntrack_ipv4 nf_conntrack
> iptable_filter ip_tables x_tables deflate zlib_deflate ctr twofish
> twofish_common camellia serpent blowfish des_generic cbc aes_i586
> aes_generic xcbc sha256_generic sha1_generic crypto_null af_key
> nls_iso8859_1 nsl_cp437 vfat fat fuse dm_crypt crypto_blkcipher
> dm_snapshot dm_mirror dm_mod freq_table ide_generic ide_disk irtty_sir
> mousedev sir_dev snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm_oss
> snd_mixer_oss snd_pcm snd_timer pcmcia nsc_ircc snd_ipw2200 psmouse
> sdhci irda ieee80211 ieee80211_crypt serio_raw soundcore i2c_i801 rtc
> crc_ccitt parport_pc parport 8250_pnp pcspkr snd_page_alloc i2c_core
> 8250_pci yenta_socket rsrc_nonstatic pcmcia_core mmc_core firmware_class
> video output intel_agp agpgart evdev thinkpad_acpi sg sr_mod sd_mod
> ide_pci_generic ata_piix floppy pata_acpi libata scsi_mod ehci_hcd
> uhci_hcd usbcore tg3 unix ide_cd_mod cdrom
> 
> Pid: 3893, comm: udevd Not tainted (2.6.25-rc8 #4)
> EIP: 0060:[<f8ca6b99>] EFLAGS: 00010002 CPU: 0
> EIP is at ub_end_rq+0x2b/0x33 [ub]
> EAX: 00000001 EBX: f78f296c ECX: 00000024 EDX: 00000001
> ESI: 00000000 EDI: f7bb5ad4 EBP: f79fde0c ESP: f79fde04
>  DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
> Process udevd (pid: 3893, ti=f79fc000 task=f64e01b0 task.ti=f79fc000)
> Stack: f7bb5ad4 00000000 f79fde34 f8ca6fa1 f7b6f400 f7bb5a00 f7bb5a34 f78f296c
>        00000086 f7b6f400 f7b6f400 f7bb5ad4 f79fde48 f8ca73b6 f7b6f400 c03ec2ec
>        0000000a f79fde6c f8ca7c06 f7b6f5c0 f7b6f550 00000286 f7bb5ad4 00000000
> Call Trace:
>  [<f8ca6fa1>] ? ub_rw_cmd_done+0x1c2/0x1d8 [ub]
>  [<f8ca73b6>] ? ub_scsi_urb_compl+0x31c/0x35b [ub]
>  [<f8ca7c06>] ? ub_scsi_action+0x18f/0x1c4 [ub]
>  [<c011e0bf>] ? tasklet_action+0x35/0x56
>  [<c011e01e>] ? __do_softirq+0x38/0x7a
>  [<c011e085>] ? do_softirq+0x25/0x2a
>  [<c011e1db>] ? irq_exit+0x28/0x61
>  [<c010630f>] ? do_IRQ+0x58/0x6d
>  [<c01052c3>] ? common_interrupt+0x23/0x28
>  [<c024cf52>] ? skb_dequeue+0x3d/0x50
>  [<c024d2a0>] ? skb_queue_purge+0x14/0x1b
>  [<f8843544>] ? unix_sock_destructor+0xe/0xb4 [unix]
>  [<f8841e65>] ? unix_release+0x1a/0x20 [unix]
>  [<c0248ee6>] ? sock_release+0x14/0x6f
>  [<c02492fe>] ? sock_close+0x29/0x30
>  [<c015afa5>] ? __fput+0x93/0x14a
>  [<c015b1e4>] ? fput+0x16/0x18
>  [<c0158aad>] ? filp_close+0x50/0x5a
>  [<c0159d1e>] ? sys_close+0x6f/0xb9
>  [<c01048b2>] ? sysenter_past_esp+0x5f/0x85
>  [<c02a0000>] ? __xfrm_state_bump_genids+0x22/0xfd
>  =======================
> Code: 55 89 e5 56 31 f6 85 d2 53 89 c3 74 08 89 50 7c be fb ff ff ff 89 d8 e8 75
>  b5 51 c7 89 f2 89 c1 89 d8 e8 30 c0 51 c7 85 c0 74 04 <0f> 0b eb fe 5b 5e 5d c3
>  55 89 e5 57 56 53 83 ec 14 89 45 e8 8b
> EIP: [<f8ca6b99>] ub_end_rq+0x2b/0x33 [ub] SS:ESP 0068:f79fde04
> Kernel panic - not syncing: Fatal exception in interrupt
> 
> Looking at the code I find:
> 
> static void ub_end_rq(struct request *rq, unsigned int scsi_status)
> {
>         int error;
> 
> 	if (scsi_status == 0) {
> 		error = 0;
> 	} else {
> 		error = -EIO;
> 		rq->errors = scsi_status;
> 	}
> 	if (__blk_end_request(rq, error, blk_rq_bytes(rq)))
> 		BUG();  <-- kernel crashes here.
> 	}
> }
> 
> With git bisect I could came up with:
> 610d8b0c972e3b75493efef8e96175518fd736d3 good
> 3bcddeac1c4c7e6fb90531b80f236b1a05dfe514 compile error
> 5450d3e1d68f10be087f0855d8bad5458b50ecbe compile error
> b8286239ddaf2632cec65c01e68a403ac4c3d079 compile error
> 7d699bafe258ebd8f9b4ec182c554200b369a504 bad

I believe the patch from Pete fixes this BUG():
    http://marc.info/?l=linux-usb&m=120613888005382&w=2

Could you try that?

Original ub driver was probably ignoring bio completion on BLOCK_PC,
and I missed the conversion to blk_end_request for BLOCK_PC.
Sorry about that.

Thanks,
Kiyoshi Ueda

Re: kernel BUG at drivers/block/ub.c:820!

[Pete Zaitcev]

On Thu, 03 Apr 2008 17:18:31 +0200, Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
> On Thu, 2008-04-03 at 11:08 -0500, Kiyoshi Ueda wrote:

> > I believe the patch from Pete fixes this BUG():
> >     http://marc.info/?l=linux-usb&m=120613888005382&w=2
> > 
> > Could you try that?
> 
> Just tested the patch and it indeed fixes the bug.

I'm glad.

> Could we push this upstream for 2.6.25? It is a regression, I would
> prefer to have a 2.6.25 that doesn't panic my machine when I attach a
> USB stick..

I was going to work through Greg as usual and then push it to
the stable tree. I'm not Linus' "leutenant", so I cannot just send
him random patches.

-- Pete

Re: kernel BUG at drivers/block/ub.c:820!

[Boaz Harrosh]

On Thu, Apr 03 2008 at 19:30 +0300, Pete Zaitcev <zaitcev@redhat.com> wrote:
> On Thu, 03 Apr 2008 17:18:31 +0200, Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
>> On Thu, 2008-04-03 at 11:08 -0500, Kiyoshi Ueda wrote:
> 
>>> I believe the patch from Pete fixes this BUG():
>>>     http://marc.info/?l=linux-usb&m=120613888005382&w=2
>>>
>>> Could you try that?
>> Just tested the patch and it indeed fixes the bug.
> 
> I'm glad.
> 
>> Could we push this upstream for 2.6.25? It is a regression, I would
>> prefer to have a 2.6.25 that doesn't panic my machine when I attach a
>> USB stick..
> 
> I was going to work through Greg as usual and then push it to
> the stable tree. I'm not Linus' "leutenant", so I cannot just send
> him random patches.
> 
> -- Pete

This is a block device siting in block/ folder. It is certainly Jens's
linux-block territory. Best put it through the tree that introduced the
bug, no?

Best would be to resend to Jens ASAP, it is a 2.6.25 regression.

Boaz

Re: kernel BUG at drivers/block/ub.c:820!

[Andrew Morton]

On Thu, 03 Apr 2008 20:36:38 +0300 Boaz Harrosh <bharrosh@panasas.com> wrote:

> On Thu, Apr 03 2008 at 19:30 +0300, Pete Zaitcev <zaitcev@redhat.com> wrote:
> > On Thu, 03 Apr 2008 17:18:31 +0200, Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
> >> On Thu, 2008-04-03 at 11:08 -0500, Kiyoshi Ueda wrote:
> > 
> >>> I believe the patch from Pete fixes this BUG():
> >>>     http://marc.info/?l=linux-usb&m=120613888005382&w=2
> >>>
> >>> Could you try that?
> >> Just tested the patch and it indeed fixes the bug.
> > 
> > I'm glad.
> > 
> >> Could we push this upstream for 2.6.25? It is a regression, I would
> >> prefer to have a 2.6.25 that doesn't panic my machine when I attach a
> >> USB stick..
> > 
> > I was going to work through Greg as usual and then push it to
> > the stable tree. I'm not Linus' "leutenant", so I cannot just send
> > him random patches.
> > 
> > -- Pete
> 
> This is a block device siting in block/ folder. It is certainly Jens's
> linux-block territory. Best put it through the tree that introduced the
> bug, no?
> 
> Best would be to resend to Jens ASAP, it is a 2.6.25 regression.

I stole it out of Greg's tree - I'll merge it Saturdayish unless someone
stops me.

Re: kernel BUG at drivers/block/ub.c:820!

[Kiyoshi Ueda]

Hi Andrew, Greg, Pete,

On Thu, 3 Apr 2008 21:30:10 -0700, Andrew Morton wrote:
> On Thu, 03 Apr 2008 20:36:38 +0300 Boaz Harrosh <bharrosh@panasas.com> wrote:
> 
> > On Thu, Apr 03 2008 at 19:30 +0300, Pete Zaitcev <zaitcev@redhat.com> wrote:
> > > On Thu, 03 Apr 2008 17:18:31 +0200, Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
> > >> On Thu, 2008-04-03 at 11:08 -0500, Kiyoshi Ueda wrote:
> > > 
> > >>> I believe the patch from Pete fixes this BUG():
> > >>>     http://marc.info/?l=linux-usb&m=120613888005382&w=2
> > >>>
> > >>> Could you try that?
> > >> Just tested the patch and it indeed fixes the bug.
> > > 
> > > I'm glad.
> > > 
> > >> Could we push this upstream for 2.6.25? It is a regression, I would
> > >> prefer to have a 2.6.25 that doesn't panic my machine when I attach a
> > >> USB stick..
> > > 
> > > I was going to work through Greg as usual and then push it to
> > > the stable tree. I'm not Linus' "leutenant", so I cannot just send
> > > him random patches.
> > > 
> > > -- Pete
> > 
> > This is a block device siting in block/ folder. It is certainly Jens's
> > linux-block territory. Best put it through the tree that introduced the
> > bug, no?
> > 
> > Best would be to resend to Jens ASAP, it is a 2.6.25 regression.
> 
> I stole it out of Greg's tree - I'll merge it Saturdayish unless someone
> stops me.

I think that it is a broken patch and Pete wants you to take
the patch below:
    http://marc.info/?l=linux-usb&m=120613888005382&w=2

The patch, which Greg and Andrew is taking now, was tested by Chris
Wedgwood and he detected a bug is still in there:
    http://marc.info/?l=linux-usb&m=120603584207543&w=2
Pete revised it and posted the new patch as above, but the new patch
seems not to be included in the Greg's tree yet.


Pete,
The new patch you posted seems not having your sign-off.
Could you follow it so that the new patch can be included instead of
the old patch?

Thanks,
Kiyoshi Ueda

Re: kernel BUG at drivers/block/ub.c:820!

[Pete Zaitcev]

Remove BUG() after __blk_end_request and fix the condition causing it.

When __blk_end_request returns nonzero, it means that the request was
not completely processed and some BIOs are still attached. Since we
have dequeued it by that time, it means leaking requests and hanging
processes, which is why BUG() was in there. In ub this happens if
a packet request ends normally, but with residue (e.g. when scsi_id
issues INQUIRY).

The fix is to make sure that arguments passed to __blk_end_request
are correct: the full request length and not just transferred length.
The transferred length is indicated to applications by adjusting
rq->data_len with old, unchanged code outside of this patch.

Signed-Off-By: Pete Zaitcev <zaitcev@redhat.com>

---

> Date: Fri, 04 Apr 2008 10:29:02 -0500 (EST)
> From: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>

> The new patch you posted seems not having your sign-off.
> Could you follow it so that the new patch can be included instead of
> the old patch?

Done. Sorry for the delay, I was really sick yesterday and unable
to sit at the computer.

diff --git a/drivers/block/ub.c b/drivers/block/ub.c
index c452e2d..12961f7 100644
--- a/drivers/block/ub.c
+++ b/drivers/block/ub.c
@@ -8,6 +8,7 @@
  * and is not licensed separately. See file COPYING for details.
  *
  * TODO (sorted by decreasing priority)
+ *  -- Return sense now that rq allows it (we always auto-sense anyway).
  *  -- set readonly flag for CDs, set removable flag for CF readers
  *  -- do inquiry and verify we got a disk and not a tape (for LUN mismatch)
  *  -- verify the 13 conditions and do bulk resets
@@ -359,7 +361,8 @@ static void ub_cmd_build_block(struct ub_dev *sc, struct ub_lun *lun,
 static void ub_cmd_build_packet(struct ub_dev *sc, struct ub_lun *lun,
     struct ub_scsi_cmd *cmd, struct ub_request *urq);
 static void ub_rw_cmd_done(struct ub_dev *sc, struct ub_scsi_cmd *cmd);
-static void ub_end_rq(struct request *rq, unsigned int status);
+static void ub_end_rq(struct request *rq, unsigned int status,
+    unsigned int cmd_len);
 static int ub_rw_cmd_retry(struct ub_dev *sc, struct ub_lun *lun,
     struct ub_request *urq, struct ub_scsi_cmd *cmd);
 static int ub_submit_scsi(struct ub_dev *sc, struct ub_scsi_cmd *cmd);
@@ -642,13 +645,13 @@ static int ub_request_fn_1(struct ub_lun *lun, struct request *rq)
 
 	if (atomic_read(&sc->poison)) {
 		blkdev_dequeue_request(rq);
-		ub_end_rq(rq, DID_NO_CONNECT << 16);
+		ub_end_rq(rq, DID_NO_CONNECT << 16, blk_rq_bytes(rq));
 		return 0;
 	}
 
 	if (lun->changed && !blk_pc_request(rq)) {
 		blkdev_dequeue_request(rq);
-		ub_end_rq(rq, SAM_STAT_CHECK_CONDITION);
+		ub_end_rq(rq, SAM_STAT_CHECK_CONDITION, blk_rq_bytes(rq));
 		return 0;
 	}
 
@@ -701,7 +704,7 @@ static int ub_request_fn_1(struct ub_lun *lun, struct request *rq)
 
 drop:
 	ub_put_cmd(lun, cmd);
-	ub_end_rq(rq, DID_ERROR << 16);
+	ub_end_rq(rq, DID_ERROR << 16, blk_rq_bytes(rq));
 	return 0;
 }
 
@@ -770,6 +773,7 @@ static void ub_rw_cmd_done(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
 	struct ub_request *urq = cmd->back;
 	struct request *rq;
 	unsigned int scsi_status;
+	unsigned int cmd_len;
 
 	rq = urq->rq;
 
@@ -779,8 +783,18 @@ static void ub_rw_cmd_done(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
 				rq->data_len = 0;
 			else
 				rq->data_len -= cmd->act_len;
+			scsi_status = 0;
+		} else {
+			if (cmd->act_len != cmd->len) {
+				if ((cmd->key == MEDIUM_ERROR ||
+				     cmd->key == UNIT_ATTENTION) &&
+				    ub_rw_cmd_retry(sc, lun, urq, cmd) == 0)
+					return;
+				scsi_status = SAM_STAT_CHECK_CONDITION;
+			} else {
+				scsi_status = 0;
+			}
 		}
-		scsi_status = 0;
 	} else {
 		if (blk_pc_request(rq)) {
 			/* UB_SENSE_SIZE is smaller than SCSI_SENSE_BUFFERSIZE */
@@ -801,14 +815,17 @@ static void ub_rw_cmd_done(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
 
 	urq->rq = NULL;
 
+	cmd_len = cmd->len;
 	ub_put_cmd(lun, cmd);
-	ub_end_rq(rq, scsi_status);
+	ub_end_rq(rq, scsi_status, cmd_len);
 	blk_start_queue(lun->disk->queue);
 }
 
-static void ub_end_rq(struct request *rq, unsigned int scsi_status)
+static void ub_end_rq(struct request *rq, unsigned int scsi_status,
+    unsigned int cmd_len)
 {
 	int error;
+	long rqlen;
 
 	if (scsi_status == 0) {
 		error = 0;
@@ -816,8 +833,12 @@ static void ub_end_rq(struct request *rq, unsigned int scsi_status)
 		error = -EIO;
 		rq->errors = scsi_status;
 	}
-	if (__blk_end_request(rq, error, blk_rq_bytes(rq)))
-		BUG();
+	rqlen = blk_rq_bytes(rq);    /* Oddly enough, this is the residue. */
+	if (__blk_end_request(rq, error, cmd_len)) {
+		printk(KERN_WARNING DRV_NAME
+		    ": __blk_end_request blew, %s-cmd total %u rqlen %ld\n",
+		    blk_pc_request(rq)? "pc": "fs", cmd_len, rqlen);
+	}
 }
 
 static int ub_rw_cmd_retry(struct ub_dev *sc, struct ub_lun *lun,