Transport Attributes Export API
Subsystems (2)
PATCH 1/1 Transport Attributes Export API
Hello,
I've run into a situation where I need to export driver-hidden information to sysfs. This information is specific to a transport (e.g., Parallel SCSI or Fiber Channel) and, in general, the kernel doesn't use this information so it is not available outside the HBA driver.
Attached is a patch that attempts to provide a means to export this information to sysfs. It introduces a new class for the scsi_device's transport and provides facilities to export class_device_attributes.
A few suggestions that James Bottomley has already given are below, but I thought that it was time that others saw this and could give their suggestions too. Thanks.
mh.
James' suggestions:
- Separate the transports out into two separate .c files (so they can be loaded as modules). This also plays in to future work I'm thinking
about: make the error handler more transport specific.
- Then, the class register/unregister can be done in the module_init/exit routines
- Probably move all the show/store routines in .h files so they can be used equally for fc and parallel
- This switch on ttype (and the enum transport_type) in scsi_add_transport_attributes is not scaleable. I think a better solution might be to have a struct scsi_transport which has the class device and a pointer to the overridden attribute. Then, this is the thing pointed to in the scsi host structure (and it can contain the attributes, the class and a pointer to the default attributes, so most of the trying to find the defaults code goes away).
Signature · 2 lines
Martin Hicks Wild Open Source Inc. [email protected] 613-266-2296
===== drivers/scsi/qla1280.c 1.51 vs edited =====
drivers/scsi/qla1280.c +64 -0
@@ -324,6 +324,7 @@#if LINUX_VERSION_CODE >=0x020545 #include <scsi/scsi_host.h> +#include <scsi/scsi_sysfs.h> #include "scsi.h" #else #include <linux/blk.h>
@@ -440,7 +441,9 @@*/ static void qla1280_done (struct scsi_qla_host *,struct srb **,struct srb **);static void qla1280_done_q_put (struct srb *,struct srb **,struct srb **); +static int qla1280_slave_alloc (Scsi_Device *);static int qla1280_slave_configure (Scsi_Device *); +static void qla1280_slave_destroy (Scsi_Device *);#if LINUX_VERSION_CODE <0x020545 static void qla1280_select_queue_depth (struct Scsi_Host *,Scsi_Device *);static void qla1280_get_target_options (struct scsi_cmnd *,struct scsi_qla_host *);
@@ -499,6 +502,27 @@static struct qla_driver_setup driver_setup__initdata ; +static ssize_t +qla1280_show_someval (struct class_device *dev,char *buf) +{ +int i =5 ; +return snprintf (buf,20 ,"%d\n" , i); +} + +static struct class_device_attribute qla1280_someval_attr = { + .attr = { + .name ="someval" , + .mode =S_IRUGO , + }, + .show = qla1280_show_someval, +}; + +static struct class_device_attribute *qla1280_transport_attributes[] = { + &qla1280_someval_attr, +NULL , +}; + +/* * convert scsi data direction to request_t control flags */
@@ -949,6 +973,8 @@ ha->instance = num_hosts; host->unique_id = ha->instance; + host->hostt->trans_attrs = qla1280_transport_attributes; +if (qla1280_pci_config (ha)) {printk (KERN_INFO "qla1x160: Unable to configure PCI\n" ); goto error_mem_alloced;
@@ -1671,6 +1697,31 @@ } +/* +* qla1280_slave_alloc +*/ +static int +qla1280_slave_alloc (Scsi_Device *device) +{ +struct scsi_qla_host *ha; +struct nvram *nv; +struct scsi_transport_attrs *sattrs; +int bus = device->channel; +int target = device->id; + +if (scsi_sysfs_setup_scsi_transport (device)) +return -1 ; + + ha = (struct scsi_qla_host *)device->host->hostdata; + nv = &ha->nvram; + sattrs = (struct scsi_transport_attrs *)device->transport_attrs; + + sattrs->period = nv->bus[bus].target[target].sync_period; + sattrs->offset = nv->bus[bus].target[target].flags.flags_43 &0xff ; + +return 0 ; +} +/************************************************************************** * qla1280_slave_configure *
@@ -1739,6 +1790,17 @@return status; } + +/* +* qla1280_slave_destroy +*/ +static void +qla1280_slave_destroy (Scsi_Device *device) +{ +scsi_sysfs_cleanup_scsi_transport (device); +} + +#if LINUX_VERSION_CODE <0x020545 /************************************************************************** * qla1280_select_queue_depth
@@ -5145,7 +5207,9 @@ .info = qla1280_info, .queuecommand = qla1280_queuecommand, #if LINUX_VERSION_CODE >= 0x020545 + .slave_alloc = qla1280_slave_alloc, .slave_configure = qla1280_slave_configure, + .slave_destroy = qla1280_slave_destroy, #endif .eh_abort_handler = qla1280_eh_abort, .eh_device_reset_handler= qla1280_eh_device_reset,
qla_os.c renamed from qla_os.c.orig +43 -1
77 lines not on this page · raw message
drivers/scsi/scsi_syms.c +8 -0
@@ -21,6 +21,7 @@#include <scsi/scsi_driver.h> #include <scsi/scsi_host.h> #include <scsi/scsi_ioctl.h> +#include <scsi/scsi_sysfs.h> #include <scsi/scsicam.h> #include "scsi.h"
@@ -86,6 +87,13 @@EXPORT_SYMBOL (scsi_add_device);EXPORT_SYMBOL (scsi_remove_device);EXPORT_SYMBOL (scsi_device_cancel); + +EXPORT_SYMBOL (scsi_sysfs_setup_scsi_transport); +EXPORT_SYMBOL (scsi_sysfs_cleanup_scsi_transport); +EXPORT_SYMBOL (scsi_sysfs_setup_fc_transport); +EXPORT_SYMBOL (scsi_sysfs_cleanup_fc_transport); +EXPORT_SYMBOL (scsi_sysfs_scsitransport_attrs); +EXPORT_SYMBOL (scsi_sysfs_fctransport_attrs);EXPORT_SYMBOL (__scsi_mode_sense);EXPORT_SYMBOL (scsi_mode_sense);
drivers/scsi/scsi_sysfs.c +270 -7
338 lines not on this page · raw message
include/scsi/scsi_device.h +5 -0
@@ -103,12 +103,17 @@struct device sdev_gendev;struct class_device sdev_classdev; +void * transport_attrs; +struct class_device transport_classdev; +enum scsi_device_state sdev_state; };#define to_scsi_device (d) \ container_of(d, struct scsi_device, sdev_gendev)#define class_to_sdev (d) \ container_of(d, struct scsi_device, sdev_classdev) +#define transport_class_to_sdev (d) \ + container_of(d, struct scsi_device, transport_classdev)extern struct scsi_device *scsi_add_device (struct Scsi_Host *,uint ,uint ,uint );
include/scsi/scsi_host.h +5 -0
@@ -337,6 +337,11 @@struct device_attribute **sdev_attrs;/* +* Pointer to the device transport attributes for this host, NULL terminated. +*/ +struct class_device_attribute **trans_attrs; + +/* * List of hosts per template. * * This is only for use by scsi_module.c for legacy templates.
include/scsi/scsi_sysfs.h new file +42 -0
42 lines not on this page · raw message
Quoted 9 lines · Martin Hicks
Hello,
I've run into a situation where I need to export driver-hidden information to sysfs. This information is specific to a transport (e.g., Parallel SCSI or Fiber Channel) and, in general, the kernel doesn't use this information so it is not available outside the HBA driver.
Quoted 7 lines
Attached is a patch that attempts to provide a means to export this information to sysfs. It introduces a new class for the scsi_device's transport and provides facilities to export
class_device_attributes.A few suggestions that James Bottomley has already given are below, but I thought that it was time that others saw this and could give their suggestions too. Thanks.
You could get rid of most code in the drivers by placing a pointer to the transport attributes and the desired transport class into the host template and letting the common code do all setup for you.
That's also consistant to how we handle all the other attributes.
Quoted 1 line
- Separate the transports out into two separate .c files
probably a good idea.
Quoted 1 line
(so they can be loaded as modules).
for the sysfs code alone that's total overkill.
Quoted 2 lines
This also plays in to future work I'm thinking
about: make the error handler more transport specific.
Yupp, that's a good idea, but not really much related to this patch.
Quoted 6 lines
- Then, the class register/unregister can be done in the module_init/exit routines
- Probably move all the show/store routines in .h files so they can be used equally for fc and parallel
- This switch on ttype (and the enumtransport_type) in
Quoted 6 lines
scsi_add_transport_attributesis not scaleable. I think a better solution might be to have a structscsi_transportwhich has the class device and a pointer to the overridden attribute. Then, this is the thing pointed to in the scsi host structure (and it can contain the attributes, the class and a pointer to the default attributes, so most of the trying to find the defaults code goes away).
Or no extra structure and put the two pointers directly in the host template as I suggested above. Yes, that means drivers that support multiple transports need a template for each of them, but that's not a big overhead anyway.
Quoted 3 lines · Christoph Hellwig
You could get rid of most code in the drivers by placing a pointer to the transport attributes and the desired transport class into the host template and letting the common code do all setup for you.
Yes, that's what I was thinking: just make the transport class an object that embeds the device class. it could then contain the useful parameters (which drivers would use directly). We could then use a transport specific API for updating their values (which the driver could choose to implement, but the attribute would be exported ro if it didn't).
Quoted 9 lines
That's also consistant to how we handle all the other attributes.
Quoted 1 line
- Separate the transports out into two separate .c files
probably a good idea.
Quoted 1 line
(so they can be loaded as modules).
for the sysfs code alone that's total overkill.
But if the the error handler (and domain validation for SPI) are bound up in here, it won't be...when someone offers to do work for you you don't say no.
Quoted 18 lines
Quoted 2 lines
This also plays in to future work I'm thinking
about: make the error handler more transport specific.Yupp, that's a good idea, but not really much related to this patch.
Quoted 6 lines
- Then, the class register/unregister can be done in the module_init/exit routines
- Probably move all the show/store routines in .h files so they can be used equally for fc and parallel
- This switch on ttype (and the enumtransport_type) inQuoted 6 lines
scsi_add_transport_attributesis not scaleable. I think a better solution might be to have a structscsi_transportwhich has the class device and a pointer to the overridden attribute. Then, this is the thing pointed to in the scsi host structure (and it can contain the attributes, the class and a pointer to the default attributes, so most of the trying to find the defaults code goes away).
Quoted 4 lines
Or no extra structure and put the two pointers directly in the host template as I suggested above. Yes, that means drivers that support multiple transports need a template for each of them, but that's not a big overhead anyway.
Actually, I think we really only need one set of overrideable attributes, which we have in the sdev class. All the other classes we add (for transport or anything else) should be API driven---that is, after all, the job of a class: to embody an interface, so I think the transport classes will only ever have a fixed set of attributes.
Of course, the mechanism will exist to allow arbitrary transport classes to be registered for drivers that wish to create them...
James