[PATCH] MAINTAINERS: zfcp
James we are planning a major rewrite of the zfcp driver, meaning that a lot of patches will hit the mailing-list in the near future. Since I can't support this additional work-load along with my other responsibilities we are shifting the maintainership to Christof Schmitt as the maintainer and Martin Peschke as the co-maintainer. Please support the two in providing us a new and more stable zfcp environment. Thanks Swen --- MAINTAINERS |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) Index: scsi-misc/MAINTAINERS === --- scsi-misc.orig/MAINTAINERS +++ scsi-misc/MAINTAINERS @@ -3246,8 +3246,10 @@ W: http://www.ibm.com/developerworks/lin S: Supported S390 ZFCP DRIVER -P: Swen Schillig -M: [EMAIL PROTECTED] +P: Christof Schmitt +M: [EMAIL PROTECTED] +P: Martin Peschke +M: [EMAIL PROTECTED] M: [EMAIL PROTECTED] L: [EMAIL PROTECTED] W: http://www.ibm.com/developerworks/linux/linux390/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/s390/: Spelling fixes
On Monday 17 December 2007 20:40, Joe Perches wrote: > > Signed-off-by: Joe Perches <[EMAIL PROTECTED]> > --- > drivers/s390/block/dasd_3990_erp.c |2 +- > drivers/s390/block/dasd_eckd.c |2 +- > drivers/s390/char/sclp_rw.c|2 +- > drivers/s390/char/tape_3590.c |2 +- > drivers/s390/cio/airq.c|2 +- > drivers/s390/scsi/zfcp_erp.c |2 +- > drivers/s390/scsi/zfcp_qdio.c |2 +- > 7 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/s390/block/dasd_3990_erp.c > b/drivers/s390/block/dasd_3990_erp.c > index 5b7385e..c02f960 100644 > --- a/drivers/s390/block/dasd_3990_erp.c > +++ b/drivers/s390/block/dasd_3990_erp.c > @@ -2620,7 +2620,7 @@ dasd_3990_erp_handle_match_erp(struct dasd_ccw_req > *erp_head, > * DASD_3990_ERP_ACTION > * > * DESCRIPTION > - * controll routine for 3990 erp actions. > + * control routine for 3990 erp actions. > * Has to be called with the queue lock (namely the s390_irq_lock) > acquired. > * > * PARAMETER > diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c > index 44adf84..6038d91 100644 > --- a/drivers/s390/block/dasd_eckd.c > +++ b/drivers/s390/block/dasd_eckd.c > @@ -1542,7 +1542,7 @@ dasd_eckd_performance(struct dasd_device *device, void > __user *argp) > prssdp = (struct dasd_psf_prssd_data *) cqr->data; > memset(prssdp, 0, sizeof (struct dasd_psf_prssd_data)); > prssdp->order = PSF_ORDER_PRSSD; > - prssdp->suborder = 0x01;/* Perfomance Statistics */ > + prssdp->suborder = 0x01;/* Performance Statistics */ > prssdp->varies[1] = 0x01; /* Perf Statistics for the Subsystem */ > > ccw = cqr->cpaddr; > diff --git a/drivers/s390/char/sclp_rw.c b/drivers/s390/char/sclp_rw.c > index d6b06ab..ad7195d 100644 > --- a/drivers/s390/char/sclp_rw.c > +++ b/drivers/s390/char/sclp_rw.c > @@ -76,7 +76,7 @@ sclp_make_buffer(void *page, unsigned short columns, > unsigned short htab) > } > > /* > - * Return a pointer to the orignal page that has been used to create > + * Return a pointer to the original page that has been used to create > * the buffer. > */ > void * > diff --git a/drivers/s390/char/tape_3590.c b/drivers/s390/char/tape_3590.c > index da25f8e..8246ef3 100644 > --- a/drivers/s390/char/tape_3590.c > +++ b/drivers/s390/char/tape_3590.c > @@ -1495,7 +1495,7 @@ tape_3590_unit_check(struct tape_device *device, struct > tape_request *request, > device->cdev->dev.bus_id); > return tape_3590_erp_basic(device, request, irb, -EPERM); > case 0x8013: > - PRINT_WARN("(%s): Another host has priviliged access to the " > + PRINT_WARN("(%s): Another host has privileged access to the " > "tape device\n", device->cdev->dev.bus_id); > PRINT_WARN("(%s): To solve the problem unload the current " > "cartridge!\n", device->cdev->dev.bus_id); > diff --git a/drivers/s390/cio/airq.c b/drivers/s390/cio/airq.c > index 5287631..5eada73 100644 > --- a/drivers/s390/cio/airq.c > +++ b/drivers/s390/cio/airq.c > @@ -23,7 +23,7 @@ static adapter_int_handler_t adapter_handler; > * register for adapter interrupts > * > * With HiperSockets the zSeries architecture provides for > - * means of adapter interrups, pseudo I/O interrupts that are > + * means of adapter interrupts, pseudo I/O interrupts that are > * not tied to an I/O subchannel, but to an adapter. However, > * it doesn't disclose the info how to enable/disable them, but > * to recognize them only. Perhaps we should consider them > diff --git a/drivers/s390/scsi/zfcp_erp.c b/drivers/s390/scsi/zfcp_erp.c > index 07fa824..8925b3a 100644 > --- a/drivers/s390/scsi/zfcp_erp.c > +++ b/drivers/s390/scsi/zfcp_erp.c > @@ -1285,7 +1285,7 @@ zfcp_erp_strategy_do_action(struct zfcp_erp_action > *erp_action) >* note: no lock in subsequent strategy routines >* (this allows these routine to call schedule, e.g. >* kmalloc with such flags or qdio_initialize & friends) > - * Note: in case of timeout, the seperate strategies will fail > + * Note: in case of timeout, the separate strategies will fail >* anyhow. No need for a special action. Even worse, a nameserver >* failure would not wake up waiting ports without the call. >*/ > diff --git a/drivers/s390/scsi/zfcp_qdio.c b/drivers/s390/scsi/zfcp_qdio.c > index 51d92b1..22fdc17 100644 > --- a/drivers/s390/scsi/zfcp_qdio.c > +++ b/drivers/s390/scsi/zfcp_qdio.c > @@ -529,7 +529,7 @@ zfcp_qdio_sbals_wipe(struct zfcp_fsf_req *fsf_req) > > > /** > - * zfcp_qdio_sbale_fill - set address and lenght in current SBALE > + * zfcp_qdio_sbale_fill - set address and length in current SBALE > * on request_queue > */ > static void ACK for the zfcp bits. Cheers Swen -- To unsubscribe from this list: send the line "unsubscribe li
Re: [PATCH] zfcp: add some internal zfcp adapter statistics
On Monday 26 November 2007 11:23, Swen Schillig wrote: > On Sunday 25 November 2007 12:16, James Bottomley wrote: > > > > On Wed, 2007-10-31 at 11:33 +0100, Swen Schillig wrote: > > > From: Swen Schillig <[EMAIL PROTECTED]> > > > > > > add some statistics provided by the zFCP adapter to the sysfs > > > > > > The new zFCP adapter statistics provide a variety of information > > > about the virtual adapter (subchannel). In order to collect this > > > information > > > the zFCP driver is extended on one side to query the adapter and > > > on the other side summarize certain values which can then be fetched on > > > demand. > > > This information is made available via files(attributes) in the sysfs > > > filesystem. > > > > > > The information provided by the new zFCP adapter statistics can be fetched > > > by reading from the following files in the sysfs filesystem > > > > > > /sys/class/scsi_host/host/seconds_active > > > /sys/class/scsi_host/host/requests > > > /sys/class/scsi_host/host/megabytes > > > /sys/class/scsi_host/host/utilization > > > > This lot all look like they belong in the FC transport class statistics > > (some even already exist there). > > They might look alike but they are not the same. The values provided through > the FC transport > class always refer to the physical port whereas the new values here refer to > a virtual adapter or subchannel. > The attributes provided here are all new and not covered or displayed > anywhere else ! > > > > > > These are the statistics on a virtual adapter (subchannel) level. > > > In addition latency information is provided on a SCSI device level (LUN) > > > which > > > can be found at the following location > > > > > > /sys/class/scsi_device//device/cmd_latency > > > /sys/class/scsi_device//device/read_latency > > > /sys/class/scsi_device//device/write_latency > > > > These look to duplicate to some degree the figures > > in /sys/block//stat. Isn't the block device the best place to > > gather these, if they're useful? Since user latencies should probably > > include elevator times. > > Actually no, the latencies covered here are channel- and fabric-latencies > grouped by scsi-devices > and not device-, scsi- or block-latencies. In contrast to the stats provided > by the block-layer structure, > tape devices will be covered here as well . > > > > > James > > > > > > > > Cheers Swen > James were my answers, comments sufficient enough to apply my patch or is there still something missing required ? Thanks Cheers Swen -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] zfcp: add some internal zfcp adapter statistics
On Sunday 25 November 2007 12:16, James Bottomley wrote: > > On Wed, 2007-10-31 at 11:33 +0100, Swen Schillig wrote: > > From: Swen Schillig <[EMAIL PROTECTED]> > > > > add some statistics provided by the zFCP adapter to the sysfs > > > > The new zFCP adapter statistics provide a variety of information > > about the virtual adapter (subchannel). In order to collect this information > > the zFCP driver is extended on one side to query the adapter and > > on the other side summarize certain values which can then be fetched on > > demand. > > This information is made available via files(attributes) in the sysfs > > filesystem. > > > > The information provided by the new zFCP adapter statistics can be fetched > > by reading from the following files in the sysfs filesystem > > > > /sys/class/scsi_host/host/seconds_active > > /sys/class/scsi_host/host/requests > > /sys/class/scsi_host/host/megabytes > > /sys/class/scsi_host/host/utilization > > This lot all look like they belong in the FC transport class statistics > (some even already exist there). They might look alike but they are not the same. The values provided through the FC transport class always refer to the physical port whereas the new values here refer to a virtual adapter or subchannel. The attributes provided here are all new and not covered or displayed anywhere else ! > > > These are the statistics on a virtual adapter (subchannel) level. > > In addition latency information is provided on a SCSI device level (LUN) > > which > > can be found at the following location > > > > /sys/class/scsi_device//device/cmd_latency > > /sys/class/scsi_device//device/read_latency > > /sys/class/scsi_device//device/write_latency > > These look to duplicate to some degree the figures > in /sys/block//stat. Isn't the block device the best place to > gather these, if they're useful? Since user latencies should probably > include elevator times. Actually no, the latencies covered here are channel- and fabric-latencies grouped by scsi-devices and not device-, scsi- or block-latencies. In contrast to the stats provided by the block-layer structure, tape devices will be covered here as well . > > James > > > Cheers Swen - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] zfcp: add some internal zfcp adapter statistics
On Saturday 03 November 2007 10:17, Heiko Carstens wrote: > > +static void zfcp_fsf_req_latency(struct zfcp_fsf_req *fsf_req) > > +{ > > + struct fsf_qual_latency_info *lat_inf; > > + struct zfcp_unit *unit; > > + > > + lat_inf = &fsf_req->qtcb->prefix.prot_status_qual.latency_info; > > + unit = fsf_req->unit; > > + > > + switch (fsf_req->qtcb->bottom.io.data_direction) { > > + case FSF_DATADIR_READ: > > + unit->latencies.read.channel += lat_inf->channel_lat; > > + unit->latencies.read.fabric += lat_inf->fabric_lat; > > + unit->latencies.read.counter++; > > + break; > > + case FSF_DATADIR_WRITE: > > + unit->latencies.write.channel += lat_inf->channel_lat; > > + unit->latencies.write.fabric += lat_inf->fabric_lat; > > + unit->latencies.write.counter++; > > + break; > > + case FSF_DATADIR_CMND: > > + unit->latencies.cmd.channel += lat_inf->channel_lat; > > + unit->latencies.cmd.fabric += lat_inf->fabric_lat; > > + unit->latencies.cmd.counter++; > > + break; > > + } > > +} > > These statistics are concurrently updated from several cpus without > any locking. That looks like a bug. > > > +zfcp_sysfs_unit_##_name##_latency_show(struct device *dev, \ > > + struct device_attribute *attr, \ > > + char *buf) { \ > > + struct scsi_device *sdev = to_scsi_device(dev); \ > > + struct zfcp_unit *unit = sdev->hostdata;\ > > + struct zfcp_latencies *lat = &unit->latencies; \ > > + struct zfcp_adapter *adapter = unit->port->adapter; \ > > + \ > > + return sprintf(buf, "%u %u %u\n", \ > > + lat->_name.fabric * adapter->timer_ticks / 1000, \ > > + lat->_name.channel * adapter->timer_ticks / 1000,\ > > + lat->_name.counter); \ > > In addition they can be read concurrently from userspace without any > locking... Since you put several values together in the output I assume > this is supposed to be some sort of snapshot, which it currently isn't. > > > +static int > > +zfcp_sysfs_adapter_ex_config(struct class_device *cdev, > > +struct fsf_qtcb_bottom_config **qtcb_config) > > +{ > > + struct Scsi_Host *scsi_host = class_to_shost(cdev); > > + struct zfcp_adapter *adapter = (struct zfcp_adapter *) > > + scsi_host->hostdata[0]; > > + > > + if (!(adapter->adapter_features & FSF_FEATURE_MEASUREMENT_DATA)) { > > + ZFCP_LOG_NORMAL("error: Enhanced measurement feature not " > > + "supported"); > > + return -EOPNOTSUPP; > > + } > > + > > + *qtcb_config = kzalloc(sizeof(struct fsf_qtcb_bottom_config), > > + GFP_KERNEL); > > + if (!*qtcb_config) > > + return -ENOMEM; > > + > > + return zfcp_fsf_exchange_config_data_sync(adapter, *qtcb_config); > > +} > > + > > +static ssize_t > > +zfcp_sysfs_adapter_request_show(struct class_device *cdev, char *buf) > > +{ > > + struct fsf_qtcb_bottom_config *qtcb_config; > > + int retval; > > + > > + retval = zfcp_sysfs_adapter_ex_config(cdev, &qtcb_config); > > + > > + if (!retval) > > + retval = sprintf(buf, "%lu %lu %lu\n", > > +qtcb_config->stat_info.input_req, > > +qtcb_config->stat_info.output_req, > > +qtcb_config->stat_info.control_req); > > + > > + kfree(qtcb_config); > > + return retval; > > +} > > You're going to call kfree with some random value if the adapter doesn't > support the measurement data feature. > Ok, valid points. I changed the patch to meet the above described issues. Before I will post the modified version I want to do some testing (and an internal review). The updated version will follow soon (hopefully this week), thanks for reviewing. Cheers Swen - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] zfcp: add some internal zfcp adapter statistics
On Saturday 03 November 2007 10:40, Heiko Carstens wrote: > > > + if (!(adapter->adapter_features & FSF_FEATURE_MEASUREMENT_DATA)) { > > > + ZFCP_LOG_NORMAL("error: Enhanced measurement feature not " > > > + "supported"); > > > + return -EOPNOTSUPP; > > > + } > > Btw. any user can flood the console with these messages if the adapter > doesn't support the feature. That can be considered a denial of service > attack. Please just return -EOPNOTSUPP and don't print anything on the > console. > I can see your point but I think you're a bit too extreme in your assessment on a simple console message (with almost no processing behind it). If someone really wants to "flood" the console with messages they go other and possibly easier ways to achieve the same thing, how about "logger" ! But since it seems to bother you a lot, I will remove the message. Cheers Swen - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] zfcp: add some internal zfcp adapter statistics
From: Swen Schillig <[EMAIL PROTECTED]> add some statistics provided by the zFCP adapter to the sysfs The new zFCP adapter statistics provide a variety of information about the virtual adapter (subchannel). In order to collect this information the zFCP driver is extended on one side to query the adapter and on the other side summarize certain values which can then be fetched on demand. This information is made available via files(attributes) in the sysfs filesystem. The information provided by the new zFCP adapter statistics can be fetched by reading from the following files in the sysfs filesystem /sys/class/scsi_host/host/seconds_active /sys/class/scsi_host/host/requests /sys/class/scsi_host/host/megabytes /sys/class/scsi_host/host/utilization These are the statistics on a virtual adapter (subchannel) level. In addition latency information is provided on a SCSI device level (LUN) which can be found at the following location /sys/class/scsi_device//device/cmd_latency /sys/class/scsi_device//device/read_latency /sys/class/scsi_device//device/write_latency The information provided is raw and not modified or interpreted by any means. No interpretation or modification of the values is done by the zFCP driver. The individual values are summed up during normal operation of the virtual adapter. An overrun of the variables is neither detected nor treated. The conclusion is that the file has to be read twice to make a meaningful statement, because only the differences of the values between the two reads can be used. The statistics make use of the SCSI mid-layer interface to provide its data to the user. In detail two hooks from the scsi_host_template are used to integrate the zFCP statistics. struct scsi_host_template { ... .shost_attrs = zfcp_a_stats_attrs, .sdev_attrs = zfcp_sysfs_sdev_attrs, ... }; Signed-off-by: Swen Schillig <[EMAIL PROTECTED]> Signed-off-by: Christof Schmitt <[EMAIL PROTECTED]> --- drivers/s390/scsi/zfcp_def.h | 13 +++ drivers/s390/scsi/zfcp_fsf.c | 36 ++ drivers/s390/scsi/zfcp_fsf.h | 29 +++- drivers/s390/scsi/zfcp_scsi.c | 148 ++ 4 files changed, 223 insertions(+), 3 deletions(-) Index: scsi-misc/drivers/s390/scsi/zfcp_def.h === --- scsi-misc.orig/drivers/s390/scsi/zfcp_def.h +++ scsi-misc/drivers/s390/scsi/zfcp_def.h @@ -868,6 +868,17 @@ struct zfcp_erp_action { struct timer_list timer; }; +struct latency_cont { + u32 channel; + u32 fabric; + u32 counter; +}; + +struct zfcp_latencies { + struct latency_cont read; + struct latency_cont write; + struct latency_cont cmd; +}; struct zfcp_adapter { struct list_headlist; /* list of adapters */ @@ -883,6 +894,7 @@ struct zfcp_adapter { u32 adapter_features; /* FCP channel features */ u32 connection_features; /* host connection features */ u32hardware_version; /* of FCP channel */ + u16 timer_ticks; /* time int for a tick */ struct Scsi_Host*scsi_host;/* Pointer to mid-layer */ struct list_headport_list_head;/* remote port list */ struct list_headport_remove_lh;/* head of ports to be @@ -986,6 +998,7 @@ struct zfcp_unit { all scsi_scan_target requests have been completed. */ + struct zfcp_latencies latencies; }; /* FSF request */ Index: scsi-misc/drivers/s390/scsi/zfcp_fsf.c === --- scsi-misc.orig/drivers/s390/scsi/zfcp_fsf.c +++ scsi-misc/drivers/s390/scsi/zfcp_fsf.c @@ -2079,6 +2079,7 @@ zfcp_fsf_exchange_config_evaluate(struct fc_host_supported_classes(shost) = FC_COS_CLASS2 | FC_COS_CLASS3; adapter->hydra_version = bottom->adapter_type; + adapter->timer_ticks = bottom->timer_interval; if (fc_host_permanent_port_name(shost) == -1) fc_host_permanent_port_name(shost) = fc_host_port_name(shost); @@ -3823,6 +3824,33 @@ zfcp_fsf_send_fcp_command_task_managemen return fsf_req; } +static void zfcp_fsf_req_latency(struct zfcp_fsf_req *fsf_req) +{ + struct fsf_qual_latency_info *lat_inf; + struct zfcp_unit *unit; + + lat_inf = &fsf_req->qtcb->prefix.prot_status_qual.latency_info; + unit = fsf_req->unit; + + switch (fsf_req->qtcb->bottom.io.data_direction) { + case FSF_DATADIR_READ: + uni
Re: [RFC] [Patch 2/3] readahead statistics slimmed down, adapt zfcp
ACK On Thursday 03 May 2007 19:56, Martin Peschke wrote: > This patch adapts zfcp to the counter changes in lib/statistics.c. > > Signed-off-by: Martin Peschke <[EMAIL PROTECTED]> > --- > > zfcp_ccw.c |4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > Index: linux/drivers/s390/scsi/zfcp_ccw.c > === > --- linux.orig/drivers/s390/scsi/zfcp_ccw.c > +++ linux/drivers/s390/scsi/zfcp_ccw.c > @@ -137,7 +137,7 @@ static struct statistic_info zfcp_statin > .name = "qdio_outb_full", > .x_unit = "sbals_left", > .y_unit = "", > - .defaults = "type=counter_inc" > + .defaults = "type=counter" > }, > [ZFCP_STAT_A_QO] = { > .name = "qdio_outb", > @@ -155,7 +155,7 @@ static struct statistic_info zfcp_statin > .name = "erp", > .x_unit = "", > .y_unit = "", > - .defaults = "type=counter_inc" > + .defaults = "type=counter" > } > }; > > > > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] s390/scsi/zfcp_erp: Convert to use the kthread API
On Sunday 22 April 2007 22:17, Christoph Hellwig wrote: > On Thu, Apr 19, 2007 at 01:58:42AM -0600, Eric W. Biederman wrote: > > From: Eric W. Biederman <[EMAIL PROTECTED]> > > > > Modify zfcperp%s to be started with kthread_run not > > a combination of kernel_thread, daemonize and siginitsetinv > > making the code slightly simpler and more maintainable. > > This driver would also benefit from a full kthread conversion. > Unfortunately it has a strange dual-use semaphore (->erp_ready_sem) > that hinders a straight conversion. Maybe the maintainer can take > a look whether there's a nice way to get rid of that one? > I know and we have it on our schedule, but it's not as easy as it might look like . - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/