Skip to contentSkip to conversation

SATA Transport Attributes

PATCH In review Martin Hicks 3 participants 3 messages Mar 30, 2004

Subsystems (2)
SCSI libata
lore
Share & tools
lore

Martin Hicks

author
#0 lore raw

PATCH 1/1 SATA Transport Attributes

Hello,

Here is a patch that introduces a Transport Attribute class for SATA devices. The patch also includes an update to the Vitesse driver to use the transport attribute.

The only problem that I'm having right now is that sometimes I get a NULL pointer dereference in scsi_remove_device() when I rmmod sata_vsc. Although sdev->host->transportt->cleanup is NULL coming into the function, the call to class_device_unregister(&sdev->transport_classdev) somtimes makes ->cleanup non-NULL and bad things happen from there.

I could not reproduce this when calling rmmod on the qla2200 driver.

Any comments on this patch? Any ideas about this rmmod issue?

The patch is against 2.6.5-rc1.

thanks
mh

Signature · 2 lines
Martin Hicks                Wild Open Source Inc.
[email protected]     613-266-2296

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas:
# ChangeSet 1.1664 -> 1.1666
# drivers/scsi/libata-core.c 1.26 -> 1.27
# include/linux/libata.h 1.13 -> 1.14
# drivers/scsi/sata_vsc.c 1.3 -> 1.4
# drivers/scsi/Makefile 1.57 -> 1.58

#	drivers/scsi/Kconfig	1.60    -> 1.61   
#	               (new)	        -> 1.1     include/scsi/scsi_transport_sata.h
#	               (new)	        -> 1.2     drivers/scsi/scsi_transport_sata.c

#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 04/03/29 [email protected] 1.1665

# sata-transport-attributes-2.patch
# --------------------------------------------
# 04/03/29	[email protected]	1.1666
# scsi_transport_sata.c:
#   Remove some debug info
# --------------------------------------------
#
diff -Nru a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
--- a/drivers/scsi/Kconfig	Mon Mar 29 15:47:30 2004
+++ b/drivers/scsi/Kconfig	Mon Mar 29 15:47:30 2004
@@ -214,6 +214,14 @@
 	  each attached FiberChannel device to sysfs, say Y.
 	  Otherwise, say N.

+config SCSI_SATA_ATTRS

+        tristate "Serial ATA Transport Attributes"
+        depends on SCSI
+        help
+          If you wish to export transport-specific information about
+          each attached Serial ATA device to sysfs, say Y. Otherwise,
+          say N.

+
endmenu

menu "SCSI low-level drivers"
@@ -460,6 +468,7 @@
config SCSI_SATA_VITESSE
tristate "VITESSE VSC-7174 SATA support"
depends on SCSI_SATA && PCI && EXPERIMENTAL
+ select SCSI_SATA_ATTRS
help
This option enables support for Vitesse VSC7174 Serial ATA.

drivers/scsi/Makefile +1 -0
diff -Nru a/drivers/scsi/Makefile b/drivers/scsi/Makefile
@@ -28,6 +28,7 @@
 # --------------------------
 obj-$(CONFIG_SCSI_SPI_ATTRS)	+= scsi_transport_spi.o
 obj-$(CONFIG_SCSI_FC_ATTRS) 	+= scsi_transport_fc.o
+obj-$(CONFIG_SCSI_SATA_ATTRS)	+= scsi_transport_sata.o
 
 
 obj-$(CONFIG_SCSI_AMIGA7XX)	+= amiga7xx.o	53c7xx.o
drivers/scsi/libata-core.c +18 -0
diff -Nru a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
@@ -38,6 +38,7 @@
 #include <scsi/scsi.h>
 #include "scsi.h"
 #include "hosts.h"
+#include <scsi/scsi_transport_sata.h>
 #include <linux/libata.h>
 #include <asm/io.h>
 #include <asm/semaphore.h>

@@ -2800,6 +2801,8 @@
 	host->max_channel = 1;
 	host->unique_id = ata_unique_id++;
 	host->max_cmd_len = 12;
+	if (ent->transportt)
+		host->transportt = ent->transportt;
 	scsi_set_device(host, &ent->pdev->dev);
 
 	ap->flags = ATA_FLAG_PORT_DISABLED;

@@ -3334,6 +3337,20 @@
 	return (tmp == bits->val) ? 1 : 0;
 }
 
+/**
+ *	ata_get_port - sysfs transport attributes function
+ *
+ *	LOCKING:
+ *
+ *	RETURNS:
+ */
+
+void ata_get_port(struct scsi_device *sdev)
+{
+	struct ata_port *ap = (struct ata_port *) &sdev->host->hostdata[0];
+
+	sata_port(sdev) = ap->port_no;
+}
 
 /**
  *	ata_init -

@@ -3391,3 +3408,4 @@
 EXPORT_SYMBOL_GPL(ata_scsi_slave_config);
 EXPORT_SYMBOL_GPL(ata_scsi_release);
 EXPORT_SYMBOL_GPL(ata_host_intr);
+EXPORT_SYMBOL_GPL(ata_get_port);
drivers/scsi/sata_vsc.c +16 -0
diff -Nru a/drivers/scsi/sata_vsc.c b/drivers/scsi/sata_vsc.c
@@ -19,6 +19,8 @@
 #include <linux/interrupt.h>
 #include "scsi.h"
 #include "hosts.h"
+#include <scsi/scsi_transport.h>
+#include <scsi/scsi_transport_sata.h>
 #include <linux/libata.h>
 
 #define DRV_NAME	"sata_vsc"

@@ -237,6 +239,7 @@
 	port->scr_addr		= base + VSC_SATA_SCR_STATUS_OFFSET;
 }
 
+static struct scsi_transport_template *vsc_transport_template = NULL;
 
 static int __devinit vsc_sata_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {

@@ -305,6 +308,8 @@
 	probe_ent->irq = pdev->irq;
 	probe_ent->irq_flags = SA_SHIRQ;
 	probe_ent->mmio_base = mmio_base;
+	BUG_ON(vsc_transport_template == NULL);
+	probe_ent->transportt = vsc_transport_template;
 
 	/* We don't care much about the PIO/UDMA masks, but the core won't like us
 	 * if we don't fill these

@@ -355,15 +360,26 @@
 	.remove			= ata_pci_remove_one,
 };
 
+/* Need to actually export some attributes */
+static struct sata_function_template vsc_transport_functions = {
+	.get_port	= ata_get_port,
+	.show_port	= 1,
+};
 
 static int __init vsc_sata_init(void)
 {
+	vsc_transport_template = sata_attach_transport(&vsc_transport_functions);
+	if (!vsc_transport_template)
+		return -ENODEV;
+
 	return pci_module_init(&vsc_sata_pci_driver);
 }
 
 
 static void __exit vsc_sata_exit(void)
 {
+	sata_release_transport(vsc_transport_template);
+
 	pci_unregister_driver(&vsc_sata_pci_driver);
 }
drivers/scsi/scsi_transport_sata.c new file +187 -0

187 lines not on this page · raw message

include/linux/libata.h +3 -0
diff -Nru a/include/linux/libata.h b/include/linux/libata.h
@@ -201,6 +201,7 @@
 	struct pci_dev		*pdev;
 	struct ata_port_operations	*port_ops;
 	Scsi_Host_Template	*sht;
+	struct scsi_transport_template *transportt;
 	struct ata_ioports	port[ATA_MAX_PORTS];
 	unsigned int		n_ports;
 	unsigned int		pio_mask;

@@ -435,6 +436,8 @@
 			      struct block_device *bdev,
 			      sector_t capacity, int geom[]);
 
+/* SATA Transport attributes functions */
+extern void ata_get_port(struct scsi_device *sdev);
 
 static inline unsigned long msecs_to_jiffies(unsigned long msecs)
 {
include/scsi/scsi_transport_sata.h new file +48 -0

48 lines not on this page · raw message

#1 lore raw
Quoted 9 lines · Martin Hicks

Hello,

Here is a patch that introduces a Transport Attribute class for SATA devices. The patch also includes an update to the Vitesse driver to use the transport attribute.

The only problem that I'm having right now is that sometimes I get a NULL pointer dereference in scsi_remove_device() when I rmmod sata_vsc.

There's a nasty sysfs "issue" right now that can cause this to happen. See the thread about it on lkml for more details. This might be causing your oops.

thanks,

greg k-h

#2 lore raw
In reply to Martin Hicks
Quoted 12 lines · Martin Hicks

Hello,

Here is a patch that introduces a Transport Attribute class for SATA devices. The patch also includes an update to the Vitesse driver to use the transport attribute.

The only problem that I'm having right now is that sometimes I get a NULL pointer dereference in scsi_remove_device() when I rmmod sata_vsc. Although sdev->host->transportt->cleanup is NULL coming into the function, the call to class_device_unregister(&sdev->transport_classdev) somtimes makes ->cleanup non-NULL and bad things happen from there.

Quoted 5 lines

I could not reproduce this when calling rmmod on the qla2200 driver.

Any comments on this patch? Any ideas about this rmmod issue?

The patch is against 2.6.5-rc1.

hmmm... since libata is moving out of drivers/scsi in 2.7, I would rather leave it as-is. The port number is easily found from userspace, assuming you know the PCI device, with zero code. This adds ~300 lines of code to do that which is already doable... :)

Jeff