On 11/13/2014 02:59 PM, Steffen Maier wrote: > From: Martin Peschke <[email protected]> > > This patch improves the Fibre Channel port scan behaviour of the zfcp lldd. > Without it the zfcp device driver may churn up the storage area network by > excessive scanning and scan bursts, particularly in big virtual server > environments, potentially resulting in interference of virtual servers and > reduced availability of storage connectivity. > > The two main issues as to the zfcp device drivers automatic port scan in > virtual server environments are frequency and simultaneity. > On the one hand, there is no point in allowing lots of ports scans > in a row. It makes sense, though, to make sure that a scan is conducted > eventually if there has been any indication for potential SAN changes. > On the other hand, lots of virtual servers receiving the same indication > for a SAN change had better not attempt to conduct a scan instantly, > that is, at the same time. > > Hence this patch has a two-fold approach for better port scanning: > the introduction of a rate limit to amend frequency issues, and the > introduction of a short random backoff to amend simultaneity issues. > Both approaches boil down to deferred port scans, with delays > comprising parts for both approaches. > > The new port scan behaviour is summarised best by: > > NEW: NEW: > no_auto_port_rescan random rate flush > backoff limit =wait > > adapter resume/thaw yes yes no yes* > adapter online (user) no yes no yes* > port rescan (user) no no no yes > adapter recovery (user) yes yes yes no > adapter recovery (other) yes yes yes no > incoming ELS yes yes yes no > incoming ELS lost yes yes yes no > > Implementation is straight-forward by converting an existing worker to > a delayed worker. But care is needed whenever that worker is going to be > flushed (in order to make sure work has been completed), since a flush > operation cancels the timer set up for deferred execution (see * above). > > There is a small race window whenever a port scan work starts > running up to the point in time of storing the time stamp for that port > scan. The impact is negligible. Closing that gap isn't trivial, though, and > would the destroy the beauty of a simple work-to-delayed-work conversion. > > Signed-off-by: Martin Peschke <[email protected]> > Signed-off-by: Steffen Maier <[email protected]> > --- > drivers/s390/scsi/zfcp_aux.c | 6 +++-- > drivers/s390/scsi/zfcp_ccw.c | 32 ++++++++++++++++++++++---- > drivers/s390/scsi/zfcp_def.h | 3 ++- > drivers/s390/scsi/zfcp_ext.h | 1 + > drivers/s390/scsi/zfcp_fc.c | 52 > +++++++++++++++++++++++++++++++++++++++--- > drivers/s390/scsi/zfcp_sysfs.c | 10 +++++--- > 6 files changed, 90 insertions(+), 14 deletions(-) > > diff --git a/drivers/s390/scsi/zfcp_aux.c b/drivers/s390/scsi/zfcp_aux.c > index 8004b07..01a7339 100644 > --- a/drivers/s390/scsi/zfcp_aux.c > +++ b/drivers/s390/scsi/zfcp_aux.c > @@ -353,9 +353,11 @@ struct zfcp_adapter *zfcp_adapter_enqueue(struct > ccw_device *ccw_device) > adapter->ccw_device = ccw_device; > > INIT_WORK(&adapter->stat_work, _zfcp_status_read_scheduler); > - INIT_WORK(&adapter->scan_work, zfcp_fc_scan_ports); > + INIT_DELAYED_WORK(&adapter->scan_work, zfcp_fc_scan_ports); > INIT_WORK(&adapter->ns_up_work, zfcp_fc_sym_name_update); > > + adapter->next_port_scan = jiffies; > + > if (zfcp_qdio_setup(adapter)) > goto failed; > > @@ -420,7 +422,7 @@ void zfcp_adapter_unregister(struct zfcp_adapter *adapter) > { > struct ccw_device *cdev = adapter->ccw_device; > > - cancel_work_sync(&adapter->scan_work); > + cancel_delayed_work_sync(&adapter->scan_work); > cancel_work_sync(&adapter->stat_work); > cancel_work_sync(&adapter->ns_up_work); > zfcp_destroy_adapter_work_queue(adapter); > diff --git a/drivers/s390/scsi/zfcp_ccw.c b/drivers/s390/scsi/zfcp_ccw.c > index f9879d4..54c7b48 100644 > --- a/drivers/s390/scsi/zfcp_ccw.c > +++ b/drivers/s390/scsi/zfcp_ccw.c > @@ -56,8 +56,22 @@ static int zfcp_ccw_activate(struct ccw_device *cdev, int > clear, char *tag) > zfcp_erp_set_adapter_status(adapter, ZFCP_STATUS_COMMON_RUNNING); > zfcp_erp_adapter_reopen(adapter, ZFCP_STATUS_COMMON_ERP_FAILED, > tag); > + > + /* > + * We want to scan ports here, with some random backoff and without > + * rate limit. Recovery has already scheduled a port scan for us, > + * but with both random delay and rate limit. Nevertheless we get > + * what we want here by flushing the scheduled work after sleeping > + * an equivalent random time. > + * Let the port scan random delay elapse first. If recovery finishes > + * up to that point in time, that would be perfect for both recovery > + * and port scan. If not, i.e. recovery takes ages, there was no > + * point in waiting a random delay on top of the time consumed by > + * recovery. > + */ > + msleep(zfcp_fc_port_scan_backoff()); > zfcp_erp_wait(adapter); > - flush_work(&adapter->scan_work); /* ok to call even if nothing queued */ > + flush_delayed_work(&adapter->scan_work); > > zfcp_ccw_adapter_put(adapter); > > @@ -162,11 +176,19 @@ static int zfcp_ccw_set_online(struct ccw_device *cdev) > adapter->req_no = 0; > > zfcp_ccw_activate(cdev, 0, "ccsonl1"); > - /* scan for remote ports > - either at the end of any successful adapter recovery > - or only after the adapter recovery for setting a device online */ > + > + /* > + * We want to scan ports here, always, with some random delay and > + * without rate limit - basically what zfcp_ccw_activate() has > + * achieved for us. Not quite! That port scan depended on > + * !no_auto_port_rescan. So let's cover the no_auto_port_rescan > + * case here to make sure a port scan is done unconditionally. > + * Since zfcp_ccw_activate() has waited the desired random time, > + * we can immediately schedule and flush a port scan for the > + * remaining cases. > + */ > zfcp_fc_inverse_conditional_port_scan(adapter); > - flush_work(&adapter->scan_work); /* ok to call even if nothing queued */ > + flush_delayed_work(&adapter->scan_work); > zfcp_ccw_adapter_put(adapter); > return 0; > } > diff --git a/drivers/s390/scsi/zfcp_def.h b/drivers/s390/scsi/zfcp_def.h > index d91173f..b8e853e 100644 > --- a/drivers/s390/scsi/zfcp_def.h > +++ b/drivers/s390/scsi/zfcp_def.h > @@ -186,12 +186,13 @@ struct zfcp_adapter { > struct fc_host_statistics *fc_stats; > struct fsf_qtcb_bottom_port *stats_reset_data; > unsigned long stats_reset; > - struct work_struct scan_work; > + struct delayed_work scan_work; > struct work_struct ns_up_work; > struct service_level service_level; > struct workqueue_struct *work_queue; > struct device_dma_parameters dma_parms; > struct zfcp_fc_events events; > + unsigned long next_port_scan; > }; > > struct zfcp_port { > diff --git a/drivers/s390/scsi/zfcp_ext.h b/drivers/s390/scsi/zfcp_ext.h > index a9c570a..5b50065 100644 > --- a/drivers/s390/scsi/zfcp_ext.h > +++ b/drivers/s390/scsi/zfcp_ext.h > @@ -85,6 +85,7 @@ extern void zfcp_fc_gs_destroy(struct zfcp_adapter *); > extern int zfcp_fc_exec_bsg_job(struct fc_bsg_job *); > extern int zfcp_fc_timeout_bsg_job(struct fc_bsg_job *); > extern void zfcp_fc_sym_name_update(struct work_struct *); > +extern unsigned int zfcp_fc_port_scan_backoff(void); > extern void zfcp_fc_conditional_port_scan(struct zfcp_adapter *); > extern void zfcp_fc_inverse_conditional_port_scan(struct zfcp_adapter *); > > diff --git a/drivers/s390/scsi/zfcp_fc.c b/drivers/s390/scsi/zfcp_fc.c > index ca28e1c..25d49f3 100644 > --- a/drivers/s390/scsi/zfcp_fc.c > +++ b/drivers/s390/scsi/zfcp_fc.c > @@ -12,6 +12,7 @@ > #include <linux/types.h> > #include <linux/slab.h> > #include <linux/utsname.h> > +#include <linux/random.h> > #include <scsi/fc/fc_els.h> > #include <scsi/libfc.h> > #include "zfcp_ext.h" > @@ -31,12 +32,54 @@ module_param_named(no_auto_port_rescan, > no_auto_port_rescan, bool, 0600); > MODULE_PARM_DESC(no_auto_port_rescan, > "no automatic port_rescan (default off)"); > > +static unsigned int port_scan_backoff = 500; > +module_param(port_scan_backoff, uint, 0600); > +MODULE_PARM_DESC(port_scan_backoff, > + "upper limit of port scan random backoff in msecs (default 500)"); > + > +static unsigned int port_scan_ratelimit = 60000; > +module_param(port_scan_ratelimit, uint, 0600); > +MODULE_PARM_DESC(port_scan_ratelimit, > + "minimum interval between port scans in msecs (default 60000)"); > + > +unsigned int zfcp_fc_port_scan_backoff(void) > +{ > + if (!port_scan_backoff) > + return 0; > + return get_random_int() % port_scan_backoff; > +} > + > +static void zfcp_fc_port_scan_time(struct zfcp_adapter *adapter) > +{ > + unsigned long interval = msecs_to_jiffies(port_scan_ratelimit); > + unsigned long backoff = msecs_to_jiffies(zfcp_fc_port_scan_backoff()); > + > + adapter->next_port_scan = jiffies + interval + backoff; > +} > + > +static void zfcp_fc_port_scan(struct zfcp_adapter *adapter) > +{ > + unsigned long now = jiffies; > + unsigned long next = adapter->next_port_scan; > + unsigned long delay = 0, max; > + > + /* delay only needed within waiting period */ > + if (time_before(now, next)) { > + delay = next - now; > + /* paranoia: never ever delay scans longer than specified */ > + max = msecs_to_jiffies(port_scan_ratelimit + port_scan_backoff); > + delay = min(delay, max); > + } > + > + queue_delayed_work(adapter->work_queue, &adapter->scan_work, delay); > +} > + > void zfcp_fc_conditional_port_scan(struct zfcp_adapter *adapter) > { > if (no_auto_port_rescan) > return; > > - queue_work(adapter->work_queue, &adapter->scan_work); > + zfcp_fc_port_scan(adapter); > } > > void zfcp_fc_inverse_conditional_port_scan(struct zfcp_adapter *adapter) > @@ -44,7 +87,7 @@ void zfcp_fc_inverse_conditional_port_scan(struct > zfcp_adapter *adapter) > if (!no_auto_port_rescan) > return; > > - queue_work(adapter->work_queue, &adapter->scan_work); > + zfcp_fc_port_scan(adapter); > } > > /** > @@ -680,12 +723,15 @@ static int zfcp_fc_eval_gpn_ft(struct zfcp_fc_req > *fc_req, > */ > void zfcp_fc_scan_ports(struct work_struct *work) > { > - struct zfcp_adapter *adapter = container_of(work, struct zfcp_adapter, > + struct delayed_work *dw = to_delayed_work(work); > + struct zfcp_adapter *adapter = container_of(dw, struct zfcp_adapter, > scan_work); > int ret, i; > struct zfcp_fc_req *fc_req; > int chain, max_entries, buf_num, max_bytes; > > + zfcp_fc_port_scan_time(adapter); > + > chain = adapter->adapter_features & FSF_FEATURE_ELS_CT_CHAINED_SBALS; > buf_num = chain ? ZFCP_FC_GPN_FT_NUM_BUFS : 1; > max_entries = chain ? ZFCP_FC_GPN_FT_MAX_ENT : ZFCP_FC_GPN_FT_ENT_PAGE; > diff --git a/drivers/s390/scsi/zfcp_sysfs.c b/drivers/s390/scsi/zfcp_sysfs.c > index a488c09..96a0be1 100644 > --- a/drivers/s390/scsi/zfcp_sysfs.c > +++ b/drivers/s390/scsi/zfcp_sysfs.c > @@ -221,9 +221,13 @@ static ssize_t zfcp_sysfs_port_rescan_store(struct > device *dev, > if (!adapter) > return -ENODEV; > > - /* sync the user-space- with the kernel-invocation of scan_work */ > - queue_work(adapter->work_queue, &adapter->scan_work); > - flush_work(&adapter->scan_work); > + /* > + * Users wish is our command: immediately schedule and flush a > + * worker to conduct a synchronous port scan, that is, neither > + * a random delay nor a rate limit is applied here. > + */ > + queue_delayed_work(adapter->work_queue, &adapter->scan_work, 0); > + flush_delayed_work(&adapter->scan_work); > zfcp_ccw_adapter_put(adapter); > > return (ssize_t) count; > Reviewed-by: Hannes Reinecke <[email protected]>
Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
