Re: [Patch] Fix oops on rmmod usb-storage

[James Bottomley <James.Bottomley@SteelEye.com>]

[-- Attachment #1: Type: text/plain, Size: 269 bytes --]

On Wed, 2004-09-29 at 10:44, Hannes Reinecke wrote:
> Oh, that can be fixed. Attached is the full trace (including USB 
> debugging output).
> It does crash. Hard.

OK, looks like a refcounting problem again.

Try the attached and see if it goes away.

Thanks,

James


[-- Attachment #2: Type: message/rfc822, Size: 4917 bytes --]

From: James Bottomley <James.Bottomley@steeleye.com>
To: SCSI Mailing List <linux-scsi@vger.kernel.org>
Cc: Alan Stern <stern@rowland.harvard.edu>, Andrew Morton <akpm@osdl.org>, USB development list <linux-usb-devel@lists.sourceforge.net>
Subject: Re: [linux-usb-devel] Fw: [Bug 3466] New: Bug while connecting USB-HDD (fwd)
Date: 28 Sep 2004 12:40:37 -0400
Message-ID: <1096389644.1717.45.camel@mulgrave>

On Sun, 2004-09-26 at 18:05, James Bottomley wrote:
> There's not enough information to say why it happened.  However, all the
> SCSI code checks out (it's dated ... open coded reference counting
> instead of kref, but it looks sound).  The scenario described could be
> seen if there's a problem in the host reference counting.
> 
> In that case, there should have been a slab error earlier on in the logs
> at the point the error occurred saying something like "slab error in
> kmem_cache_destory(): can't free all objects"
> 
> It's possible this could be caused by a refcounting race on the
> commands.

OK, I have a definite theory about this, but it hinges on finding the
above message in the logs.

I think we tried to destroy the command slab while some commands were
still active.  The refcounting only applies to in-flight commands, but
commands can also be allocated and queued in the block layer.

If I'm right, the attached will close this refcounting hole.

James

===== drivers/scsi/scsi.c 1.146 vs edited =====
--- 1.146/drivers/scsi/scsi.c	2004-08-09 12:55:05 -05:00
+++ edited/drivers/scsi/scsi.c	2004-09-28 11:23:31 -05:00
@@ -244,7 +244,13 @@
  */
 struct scsi_cmnd *scsi_get_command(struct scsi_device *dev, int gfp_mask)
 {
-	struct scsi_cmnd *cmd = __scsi_get_command(dev->host, gfp_mask);
+	struct scsi_cmnd *cmd;
+
+	/* Bail if we can't get a reference to the device */
+	if (!get_device(&dev->sdev_gendev))
+		return NULL;
+
+	cmd = __scsi_get_command(dev->host, gfp_mask);
 
 	if (likely(cmd != NULL)) {
 		unsigned long flags;
@@ -258,7 +264,8 @@
 		spin_lock_irqsave(&dev->list_lock, flags);
 		list_add_tail(&cmd->list, &dev->cmd_list);
 		spin_unlock_irqrestore(&dev->list_lock, flags);
-	}
+	} else
+		put_device(&dev->sdev_gendev);
 
 	return cmd;
 }				
@@ -276,7 +283,8 @@
  */
 void scsi_put_command(struct scsi_cmnd *cmd)
 {
-	struct Scsi_Host *shost = cmd->device->host;
+	struct scsi_device *sdev = cmd->device;
+	struct Scsi_Host *shost = sdev->host;
 	unsigned long flags;
 	
 	/* serious error if the command hasn't come from a device list */
@@ -294,6 +302,8 @@
 
 	if (likely(cmd != NULL))
 		kmem_cache_free(shost->cmd_pool->slab, cmd);
+
+	put_device(&sdev->sdev_gendev);
 }
 
 /*

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html