| Message ID | 1459516223-32106-2-git-send-email-chad.dupuis@qlogic.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On 2016-04-01 15:10, Chad Dupuis wrote: > From: Joe Carnuccio <joe.carnuccio@qlogic.com> > > Per customer request, add the following driver tunables: > > o devloss_tmo > o max_luns > o queue_depth > o tm_timeout > > Signed-off-by: Joe Carnuccio <joe.carnuccio@qlogic.com> > Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com> > --- > drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 38 > +++++++++++++++++++++++++++++++++++++- > drivers/scsi/bnx2fc/bnx2fc_io.c | 4 +++- > 2 files changed, 40 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c > b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c > index d7029ea..600c29d 100644 > --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c > +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c > @@ -107,6 +107,26 @@ MODULE_PARM_DESC(debug_logging, > "\t\t0x10 - fcoe L2 fame related logs.\n" > "\t\t0xff - LOG all messages."); > > +uint bnx2fc_devloss_tmo; > +module_param_named(devloss_tmo, bnx2fc_devloss_tmo, uint, S_IRUGO); > +MODULE_PARM_DESC(devloss_tmo, " Change devloss_tmo for the remote > ports " > + "attached via bnx2fc."); > + > +uint bnx2fc_max_luns = BNX2FC_MAX_LUN; > +module_param_named(max_luns, bnx2fc_max_luns, uint, S_IRUGO); > +MODULE_PARM_DESC(max_luns, " Change the default max_lun per SCSI > host. Default " > + "0xffff."); > + > +uint bnx2fc_queue_depth; > +module_param_named(queue_depth, bnx2fc_queue_depth, uint, S_IRUGO); > +MODULE_PARM_DESC(queue_depth, " Change the default queue depth of > SCSI devices " > + "attached via bnx2fc."); > + > +uint bnx2fc_tm_timeout = BNX2FC_TM_TIMEOUT; > +module_param_named(tm_timeout, bnx2fc_tm_timeout, uint, > S_IRUGO|S_IWUSR); > +MODULE_PARM_DESC(tm_timeout, " Change the default timeout for " > + "task management commands. Default 60 seconds."); > + Just a question, can't this be made dynamically adjustable via sysfs instead of a module parameter? -- 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
On Fri, 1 Apr 2016, Johannes Thumshirn wrote: > On 2016-04-01 15:10, Chad Dupuis wrote: >> From: Joe Carnuccio <joe.carnuccio@qlogic.com> >> >> Per customer request, add the following driver tunables: >> >> o devloss_tmo >> o max_luns >> o queue_depth >> o tm_timeout >> >> Signed-off-by: Joe Carnuccio <joe.carnuccio@qlogic.com> >> Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com> >> --- >> drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 38 >> +++++++++++++++++++++++++++++++++++++- >> drivers/scsi/bnx2fc/bnx2fc_io.c | 4 +++- >> 2 files changed, 40 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c >> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c >> index d7029ea..600c29d 100644 >> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c >> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c >> @@ -107,6 +107,26 @@ MODULE_PARM_DESC(debug_logging, >> "\t\t0x10 - fcoe L2 fame related logs.\n" >> "\t\t0xff - LOG all messages."); >> >> +uint bnx2fc_devloss_tmo; >> +module_param_named(devloss_tmo, bnx2fc_devloss_tmo, uint, S_IRUGO); >> +MODULE_PARM_DESC(devloss_tmo, " Change devloss_tmo for the remote ports " >> + "attached via bnx2fc."); >> + >> +uint bnx2fc_max_luns = BNX2FC_MAX_LUN; >> +module_param_named(max_luns, bnx2fc_max_luns, uint, S_IRUGO); >> +MODULE_PARM_DESC(max_luns, " Change the default max_lun per SCSI >> host. Default " >> + "0xffff."); >> + >> +uint bnx2fc_queue_depth; >> +module_param_named(queue_depth, bnx2fc_queue_depth, uint, S_IRUGO); >> +MODULE_PARM_DESC(queue_depth, " Change the default queue depth of >> SCSI devices " >> + "attached via bnx2fc."); >> + >> +uint bnx2fc_tm_timeout = BNX2FC_TM_TIMEOUT; >> +module_param_named(tm_timeout, bnx2fc_tm_timeout, uint, S_IRUGO|S_IWUSR); >> +MODULE_PARM_DESC(tm_timeout, " Change the default timeout for " >> + "task management commands. Default 60 seconds."); >> + > > Just a question, can't this be made dynamically adjustable via sysfs instead > of a module parameter? > I presume you're talking about something like a /sys/class/scsi_host/hostX/tm_timeout sysfs node? -- 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
On Fri, 2016-04-01 at 10:06 -0400, Chad Dupuis wrote: > On Fri, 1 Apr 2016, Johannes Thumshirn wrote: > > > On 2016-04-01 15:10, Chad Dupuis wrote: > > > From: Joe Carnuccio <joe.carnuccio@qlogic.com> > > > > > > Per customer request, add the following driver tunables: > > > > > > o devloss_tmo > > > o max_luns > > > o queue_depth > > > o tm_timeout > > > > > > Signed-off-by: Joe Carnuccio <joe.carnuccio@qlogic.com> > > > Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com> > > > --- > > > drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 38 > > > +++++++++++++++++++++++++++++++++++++- > > > drivers/scsi/bnx2fc/bnx2fc_io.c | 4 +++- > > > 2 files changed, 40 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c > > > b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c > > > index d7029ea..600c29d 100644 > > > --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c > > > +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c > > > @@ -107,6 +107,26 @@ MODULE_PARM_DESC(debug_logging, > > > "\t\t0x10 - fcoe L2 fame related logs.\n" > > > "\t\t0xff - LOG all messages."); > > > > > > +uint bnx2fc_devloss_tmo; > > > +module_param_named(devloss_tmo, bnx2fc_devloss_tmo, uint, > > > S_IRUGO); > > > +MODULE_PARM_DESC(devloss_tmo, " Change devloss_tmo for the > > > remote ports " > > > + "attached via bnx2fc."); > > > + > > > +uint bnx2fc_max_luns = BNX2FC_MAX_LUN; > > > +module_param_named(max_luns, bnx2fc_max_luns, uint, S_IRUGO); > > > +MODULE_PARM_DESC(max_luns, " Change the default max_lun per SCSI > > > host. Default " > > > + "0xffff."); > > > + > > > +uint bnx2fc_queue_depth; > > > +module_param_named(queue_depth, bnx2fc_queue_depth, uint, > > > S_IRUGO); > > > +MODULE_PARM_DESC(queue_depth, " Change the default queue depth > > > of > > > SCSI devices " > > > + "attached via bnx2fc."); > > > + > > > +uint bnx2fc_tm_timeout = BNX2FC_TM_TIMEOUT; > > > +module_param_named(tm_timeout, bnx2fc_tm_timeout, uint, > > > S_IRUGO|S_IWUSR); > > > +MODULE_PARM_DESC(tm_timeout, " Change the default timeout for " > > > + "task management commands. Default 60 seconds."); > > > + > > > > Just a question, can't this be made dynamically adjustable via > > sysfs instead > > of a module parameter? > > > > I presume you're talking about something like a > /sys/class/scsi_host/hostX/tm_timeout sysfs node? Yes, but there's also the question of whether they should be generic rather than bnx2fc specific. At least queue_depth, max_luns and possibly tm_timeout would seem to belong to the SCSI host itself. devloss_tmo looks like it should be a host parameter within the fc transport class. James -- 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
On 2016-04-01 16:06, Chad Dupuis wrote: > On Fri, 1 Apr 2016, Johannes Thumshirn wrote: > >> On 2016-04-01 15:10, Chad Dupuis wrote: >>> From: Joe Carnuccio <joe.carnuccio@qlogic.com> >>> >>> Per customer request, add the following driver tunables: >>> >>> o devloss_tmo >>> o max_luns >>> o queue_depth >>> o tm_timeout >>> >>> Signed-off-by: Joe Carnuccio <joe.carnuccio@qlogic.com> >>> Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com> >>> --- >>> drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 38 >>> +++++++++++++++++++++++++++++++++++++- >>> drivers/scsi/bnx2fc/bnx2fc_io.c | 4 +++- >>> 2 files changed, 40 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c >>> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c >>> index d7029ea..600c29d 100644 >>> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c >>> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c >>> @@ -107,6 +107,26 @@ MODULE_PARM_DESC(debug_logging, >>> "\t\t0x10 - fcoe L2 fame related logs.\n" >>> "\t\t0xff - LOG all messages."); >>> >>> +uint bnx2fc_devloss_tmo; >>> +module_param_named(devloss_tmo, bnx2fc_devloss_tmo, uint, S_IRUGO); >>> +MODULE_PARM_DESC(devloss_tmo, " Change devloss_tmo for the remote >>> ports " >>> + "attached via bnx2fc."); >>> + >>> +uint bnx2fc_max_luns = BNX2FC_MAX_LUN; >>> +module_param_named(max_luns, bnx2fc_max_luns, uint, S_IRUGO); >>> +MODULE_PARM_DESC(max_luns, " Change the default max_lun per SCSI >>> host. Default " >>> + "0xffff."); >>> + >>> +uint bnx2fc_queue_depth; >>> +module_param_named(queue_depth, bnx2fc_queue_depth, uint, S_IRUGO); >>> +MODULE_PARM_DESC(queue_depth, " Change the default queue depth of >>> SCSI devices " >>> + "attached via bnx2fc."); >>> + >>> +uint bnx2fc_tm_timeout = BNX2FC_TM_TIMEOUT; >>> +module_param_named(tm_timeout, bnx2fc_tm_timeout, uint, >>> S_IRUGO|S_IWUSR); >>> +MODULE_PARM_DESC(tm_timeout, " Change the default timeout for " >>> + "task management commands. Default 60 seconds."); >>> + >> >> Just a question, can't this be made dynamically adjustable via sysfs >> instead of a module parameter? >> > > I presume you're talking about something like a > /sys/class/scsi_host/hostX/tm_timeout sysfs node? Yeah, something like that -- 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
On Fri, 1 Apr 2016, James Bottomley wrote: > On Fri, 2016-04-01 at 10:06 -0400, Chad Dupuis wrote: >> On Fri, 1 Apr 2016, Johannes Thumshirn wrote: >> >>> On 2016-04-01 15:10, Chad Dupuis wrote: >>>> From: Joe Carnuccio <joe.carnuccio@qlogic.com> >>>> >>>> Per customer request, add the following driver tunables: >>>> >>>> o devloss_tmo >>>> o max_luns >>>> o queue_depth >>>> o tm_timeout >>>> >>>> Signed-off-by: Joe Carnuccio <joe.carnuccio@qlogic.com> >>>> Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com> >>>> --- >>>> drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 38 >>>> +++++++++++++++++++++++++++++++++++++- >>>> drivers/scsi/bnx2fc/bnx2fc_io.c | 4 +++- >>>> 2 files changed, 40 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c >>>> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c >>>> index d7029ea..600c29d 100644 >>>> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c >>>> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c >>>> @@ -107,6 +107,26 @@ MODULE_PARM_DESC(debug_logging, >>>> "\t\t0x10 - fcoe L2 fame related logs.\n" >>>> "\t\t0xff - LOG all messages."); >>>> >>>> +uint bnx2fc_devloss_tmo; >>>> +module_param_named(devloss_tmo, bnx2fc_devloss_tmo, uint, >>>> S_IRUGO); >>>> +MODULE_PARM_DESC(devloss_tmo, " Change devloss_tmo for the >>>> remote ports " >>>> + "attached via bnx2fc."); >>>> + >>>> +uint bnx2fc_max_luns = BNX2FC_MAX_LUN; >>>> +module_param_named(max_luns, bnx2fc_max_luns, uint, S_IRUGO); >>>> +MODULE_PARM_DESC(max_luns, " Change the default max_lun per SCSI >>>> host. Default " >>>> + "0xffff."); >>>> + >>>> +uint bnx2fc_queue_depth; >>>> +module_param_named(queue_depth, bnx2fc_queue_depth, uint, >>>> S_IRUGO); >>>> +MODULE_PARM_DESC(queue_depth, " Change the default queue depth >>>> of >>>> SCSI devices " >>>> + "attached via bnx2fc."); >>>> + >>>> +uint bnx2fc_tm_timeout = BNX2FC_TM_TIMEOUT; >>>> +module_param_named(tm_timeout, bnx2fc_tm_timeout, uint, >>>> S_IRUGO|S_IWUSR); >>>> +MODULE_PARM_DESC(tm_timeout, " Change the default timeout for " >>>> + "task management commands. Default 60 seconds."); >>>> + >>> >>> Just a question, can't this be made dynamically adjustable via >>> sysfs instead >>> of a module parameter? >>> >> >> I presume you're talking about something like a >> /sys/class/scsi_host/hostX/tm_timeout sysfs node? > > Yes, but there's also the question of whether they should be generic > rather than bnx2fc specific. At least queue_depth, max_luns and > possibly tm_timeout would seem to belong to the SCSI host itself. > devloss_tmo looks like it should be a host parameter within the fc > transport class. > > James > The use case for this is so that someone can administratively set these parameters on driver load so that all devices discovered via our driver will get these parameters set automatically usually via the slave_configure callback. So functionally that's what is trying to be accomplished with this patch and what we would be looking for in a generic solution. Would the scsi_host_template and/or fc_host_template be the appropriate place to set this then so the values could be initialized during scsi_host/fc_host discovery? -- 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
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c index d7029ea..600c29d 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c @@ -107,6 +107,26 @@ MODULE_PARM_DESC(debug_logging, "\t\t0x10 - fcoe L2 fame related logs.\n" "\t\t0xff - LOG all messages."); +uint bnx2fc_devloss_tmo; +module_param_named(devloss_tmo, bnx2fc_devloss_tmo, uint, S_IRUGO); +MODULE_PARM_DESC(devloss_tmo, " Change devloss_tmo for the remote ports " + "attached via bnx2fc."); + +uint bnx2fc_max_luns = BNX2FC_MAX_LUN; +module_param_named(max_luns, bnx2fc_max_luns, uint, S_IRUGO); +MODULE_PARM_DESC(max_luns, " Change the default max_lun per SCSI host. Default " + "0xffff."); + +uint bnx2fc_queue_depth; +module_param_named(queue_depth, bnx2fc_queue_depth, uint, S_IRUGO); +MODULE_PARM_DESC(queue_depth, " Change the default queue depth of SCSI devices " + "attached via bnx2fc."); + +uint bnx2fc_tm_timeout = BNX2FC_TM_TIMEOUT; +module_param_named(tm_timeout, bnx2fc_tm_timeout, uint, S_IRUGO|S_IWUSR); +MODULE_PARM_DESC(tm_timeout, " Change the default timeout for " + "task management commands. Default 60 seconds."); + static int bnx2fc_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu); /* notification function for CPU hotplug events */ @@ -692,7 +712,7 @@ static int bnx2fc_shost_config(struct fc_lport *lport, struct device *dev) int rc = 0; shost->max_cmd_len = BNX2FC_MAX_CMD_LEN; - shost->max_lun = BNX2FC_MAX_LUN; + shost->max_lun = bnx2fc_max_luns; shost->max_id = BNX2FC_MAX_FCP_TGT; shost->max_channel = 0; if (lport->vport) @@ -1102,6 +1122,9 @@ static int bnx2fc_vport_create(struct fc_vport *vport, bool disabled) return -EIO; } + if (bnx2fc_devloss_tmo) + fc_host_dev_loss_tmo(vn_port->host) = bnx2fc_devloss_tmo; + if (disabled) { fc_vport_set_state(vport, FC_VPORT_DISABLED); } else { @@ -1495,6 +1518,9 @@ static struct fc_lport *bnx2fc_if_create(struct bnx2fc_interface *interface, } fc_host_port_type(lport->host) = FC_PORTTYPE_UNKNOWN; + if (bnx2fc_devloss_tmo) + fc_host_dev_loss_tmo(shost) = bnx2fc_devloss_tmo; + /* Allocate exchange manager */ if (!npiv) rc = bnx2fc_em_config(lport, hba); @@ -2612,6 +2638,15 @@ static int bnx2fc_cpu_callback(struct notifier_block *nfb, return NOTIFY_OK; } +static int bnx2fc_slave_configure(struct scsi_device *sdev) +{ + if (!bnx2fc_queue_depth) + return 0; + + scsi_change_queue_depth(sdev, bnx2fc_queue_depth); + return 0; +} + /** * bnx2fc_mod_init - module init entry point * @@ -2877,6 +2912,7 @@ static struct scsi_host_template bnx2fc_shost_template = { .sg_tablesize = BNX2FC_MAX_BDS_PER_CMD, .max_sectors = 1024, .track_queue_depth = 1, + .slave_configure = bnx2fc_slave_configure, }; static struct libfc_function_template bnx2fc_libfc_fcn_templ = { diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c index 0002caf..0f60e22 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_io.c +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c @@ -15,6 +15,8 @@ #define RESERVE_FREE_LIST_INDEX num_possible_cpus() +extern uint bnx2fc_tm_timeout; + static int bnx2fc_split_bd(struct bnx2fc_cmd *io_req, u64 addr, int sg_len, int bd_index); static int bnx2fc_map_sg(struct bnx2fc_cmd *io_req); @@ -770,7 +772,7 @@ retry_tmf: spin_unlock_bh(&tgt->tgt_lock); rc = wait_for_completion_timeout(&io_req->tm_done, - BNX2FC_TM_TIMEOUT * HZ); + bnx2fc_tm_timeout * HZ); spin_lock_bh(&tgt->tgt_lock); io_req->wait_for_comp = 0;