linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2] scsi: disable automatic target scan
@ 2016-03-15  7:19 Hannes Reinecke
  2016-03-15  8:31 ` Johannes Thumshirn
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Hannes Reinecke @ 2016-03-15  7:19 UTC (permalink / raw)
  To: Martin K. Petersen
  Cc: Christoph Hellwig, James Bottomley, linux-scsi, Hannes Reinecke

On larger installations it is useful to disable automatic LUN
scanning, and only add the required LUNs via udev rules.
This can speed up bootup dramatically.

This patch introduces a new scan module parameter value 'manual',
which works like 'none', but can be overriden by setting the 'rescan'
value from scsi_scan_target to 'SCSI_SCAN_MANUAL'.
And it updates all relevant callers to set the 'rescan' value
to 'SCSI_SCAN_MANUAL' if invoked via the 'scan' option in sysfs.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_scan.c            | 25 ++++++++++++++++++-------
 drivers/scsi/scsi_sysfs.c           |  3 ++-
 drivers/scsi/scsi_transport_fc.c    |  3 ++-
 drivers/scsi/scsi_transport_iscsi.c |  2 +-
 drivers/scsi/scsi_transport_sas.c   |  4 ++--
 include/scsi/scsi_device.h          |  6 ++++++
 6 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 97074c9..54dc517 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -96,10 +96,13 @@ MODULE_PARM_DESC(max_luns,
 #define SCSI_SCAN_TYPE_DEFAULT "sync"
 #endif
 
-char scsi_scan_type[6] = SCSI_SCAN_TYPE_DEFAULT;
+char scsi_scan_type[7] = SCSI_SCAN_TYPE_DEFAULT;
 
-module_param_string(scan, scsi_scan_type, sizeof(scsi_scan_type), S_IRUGO);
-MODULE_PARM_DESC(scan, "sync, async or none");
+module_param_string(scan, scsi_scan_type, sizeof(scsi_scan_type),
+		    S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(scan, "sync, async, manual, or none. "
+		 "Setting to 'manual' disables automatic scanning, but allows "
+		 "for manual device scan via the 'scan' sysfs attribute.");
 
 static unsigned int scsi_inq_timeout = SCSI_TIMEOUT/HZ + 18;
 
@@ -1040,7 +1043,8 @@ static unsigned char *scsi_inq_str(unsigned char *buf, unsigned char *inq,
  * @lun:	LUN of target device
  * @bflagsp:	store bflags here if not NULL
  * @sdevp:	probe the LUN corresponding to this scsi_device
- * @rescan:     if nonzero skip some code only needed on first scan
+ * @rescan:     if greater than SCSI_SCAN_NONE skip some code only
+ *              needed on first scan
  * @hostdata:	passed to scsi_alloc_sdev()
  *
  * Description:
@@ -1069,7 +1073,7 @@ static int scsi_probe_and_add_lun(struct scsi_target *starget,
 	 */
 	sdev = scsi_device_lookup_by_target(starget, lun);
 	if (sdev) {
-		if (rescan || !scsi_device_created(sdev)) {
+		if (rescan > SCSI_SCAN_NONE || !scsi_device_created(sdev)) {
 			SCSI_LOG_SCAN_BUS(3, sdev_printk(KERN_INFO, sdev,
 				"scsi scan: device exists on %s\n",
 				dev_name(&sdev->sdev_gendev)));
@@ -1604,7 +1608,9 @@ static void __scsi_scan_target(struct device *parent, unsigned int channel,
  * @channel:	channel to scan
  * @id:		target id to scan
  * @lun:	Specific LUN to scan or SCAN_WILD_CARD
- * @rescan:	passed to LUN scanning routines
+ * @rescan:	passed to LUN scanning routines; SCSI_SCAN_NONE for no rescan,
+ *              SCSI_SCAN_RESCAN to rescan existing LUNs, and SCSI_SCAN_MANUAL
+ *              to force scanning even if 'scan=manual' is set.
  *
  * Description:
  *     Scan the target id on @parent, @channel, and @id. Scan at least LUN 0,
@@ -1621,6 +1627,10 @@ void scsi_scan_target(struct device *parent, unsigned int channel,
 	if (strncmp(scsi_scan_type, "none", 4) == 0)
 		return;
 
+	if (rescan < SCSI_SCAN_MANUAL &&
+	    strncmp(scsi_scan_type, "manual", 6) == 0)
+		return;
+
 	mutex_lock(&shost->scan_mutex);
 	if (!shost->async_scan)
 		scsi_complete_async_scans();
@@ -1844,7 +1854,8 @@ void scsi_scan_host(struct Scsi_Host *shost)
 {
 	struct async_scan_data *data;
 
-	if (strncmp(scsi_scan_type, "none", 4) == 0)
+	if (strncmp(scsi_scan_type, "none", 4) == 0 ||
+	    strncmp(scsi_scan_type, "manual", 6) == 0)
 		return;
 	if (scsi_autopm_get_host(shost) < 0)
 		return;
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 92ffd24..3b2045f 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -143,7 +143,8 @@ static int scsi_scan(struct Scsi_Host *shost, const char *str)
 	if (shost->transportt->user_scan)
 		res = shost->transportt->user_scan(shost, channel, id, lun);
 	else
-		res = scsi_scan_host_selected(shost, channel, id, lun, 1);
+		res = scsi_scan_host_selected(shost, channel, id, lun,
+					      SCSI_SCAN_MANUAL);
 	return res;
 }
 
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 8a88226..3482a55 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -2110,7 +2110,8 @@ fc_user_scan_tgt(struct Scsi_Host *shost, uint channel, uint id, u64 lun)
 		if ((channel == rport->channel) &&
 		    (id == rport->scsi_target_id)) {
 			spin_unlock_irqrestore(shost->host_lock, flags);
-			scsi_scan_target(&rport->dev, channel, id, lun, 1);
+			scsi_scan_target(&rport->dev, channel, id, lun,
+					 SCSI_SCAN_MANUAL);
 			return;
 		}
 	}
diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index 4414816..f4c16dc 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -1819,7 +1819,7 @@ static int iscsi_user_scan_session(struct device *dev, void *data)
 		    (scan_data->id == SCAN_WILD_CARD ||
 		     scan_data->id == id))
 			scsi_scan_target(&session->dev, 0, id,
-					 scan_data->lun, 1);
+					 scan_data->lun, SCSI_SCAN_MANUAL);
 	}
 
 user_scan_exit:
diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index b6f958193..d8d38e6 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -1739,8 +1739,8 @@ static int sas_user_scan(struct Scsi_Host *shost, uint channel,
 
 		if ((channel == SCAN_WILD_CARD || channel == 0) &&
 		    (id == SCAN_WILD_CARD || id == rphy->scsi_target_id)) {
-			scsi_scan_target(&rphy->dev, 0,
-					 rphy->scsi_target_id, lun, 1);
+			scsi_scan_target(&rphy->dev, 0, rphy->scsi_target_id,
+					 lun, SCSI_SCAN_MANUAL);
 		}
 	}
 	mutex_unlock(&sas_host->lock);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index c067019..f954272 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -50,6 +50,12 @@ enum scsi_device_state {
 	SDEV_CREATED_BLOCK,	/* same as above but for created devices */
 };
 
+enum scsi_scan_mode {
+	SCSI_SCAN_NONE = 0,
+	SCSI_SCAN_RESCAN,
+	SCSI_SCAN_MANUAL,
+};
+
 enum scsi_device_event {
 	SDEV_EVT_MEDIA_CHANGE	= 1,	/* media has changed */
 	SDEV_EVT_INQUIRY_CHANGE_REPORTED,		/* 3F 03  UA reported */
-- 
1.8.5.6


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCHv2] scsi: disable automatic target scan
  2016-03-15  7:19 [PATCHv2] scsi: disable automatic target scan Hannes Reinecke
@ 2016-03-15  8:31 ` Johannes Thumshirn
  2016-03-15 11:03 ` Steffen Maier
  2016-03-15 13:36 ` Christoph Hellwig
  2 siblings, 0 replies; 7+ messages in thread
From: Johannes Thumshirn @ 2016-03-15  8:31 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
	linux-scsi

On Tue, Mar 15, 2016 at 08:19:42AM +0100, Hannes Reinecke wrote:
> On larger installations it is useful to disable automatic LUN
> scanning, and only add the required LUNs via udev rules.
> This can speed up bootup dramatically.
> 
> This patch introduces a new scan module parameter value 'manual',
> which works like 'none', but can be overriden by setting the 'rescan'
> value from scsi_scan_target to 'SCSI_SCAN_MANUAL'.
> And it updates all relevant callers to set the 'rescan' value
> to 'SCSI_SCAN_MANUAL' if invoked via the 'scan' option in sysfs.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2] scsi: disable automatic target scan
  2016-03-15  7:19 [PATCHv2] scsi: disable automatic target scan Hannes Reinecke
  2016-03-15  8:31 ` Johannes Thumshirn
@ 2016-03-15 11:03 ` Steffen Maier
  2016-03-15 11:28   ` Hannes Reinecke
  2016-03-15 13:36 ` Christoph Hellwig
  2 siblings, 1 reply; 7+ messages in thread
From: Steffen Maier @ 2016-03-15 11:03 UTC (permalink / raw)
  To: Hannes Reinecke, Martin K. Petersen
  Cc: Christoph Hellwig, James Bottomley, linux-scsi

On 03/15/2016 08:19 AM, Hannes Reinecke wrote:
> On larger installations it is useful to disable automatic LUN
> scanning, and only add the required LUNs via udev rules.
> This can speed up bootup dramatically.
>
> This patch introduces a new scan module parameter value 'manual',
> which works like 'none', but can be overriden by setting the 'rescan'
> value from scsi_scan_target to 'SCSI_SCAN_MANUAL'.
> And it updates all relevant callers to set the 'rescan' value
> to 'SCSI_SCAN_MANUAL' if invoked via the 'scan' option in sysfs.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---

> +MODULE_PARM_DESC(scan, "sync, async, manual, or none. "
> +		 "Setting to 'manual' disables automatic scanning, but allows "
> +		 "for manual device scan via the 'scan' sysfs attribute.");

nice solution

> + * @rescan:	passed to LUN scanning routines; SCSI_SCAN_NONE for no rescan,
> + *              SCSI_SCAN_RESCAN to rescan existing LUNs, and SCSI_SCAN_MANUAL
> + *              to force scanning even if 'scan=manual' is set.

> @@ -1621,6 +1627,10 @@ void scsi_scan_target(struct device *parent, unsigned int channel,
>   	if (strncmp(scsi_scan_type, "none", 4) == 0)
>   		return;
>
> +	if (rescan < SCSI_SCAN_MANUAL &&
> +	    strncmp(scsi_scan_type, "manual", 6) == 0)
> +		return;
> +

I see that the ordering of identifiers in enum scsi_scan_mode allows a 
nice inequation here.

However, it also seems that we need to touch all callers (which intend 
an unconditional manual scan) of scsi_scan_target() that used to call it 
with rescan==1 which would now be rescan==SCSI_SCAN_MANUAL==2 ?

Or could we define the ordering of identifiers in enum scsi_scan_mode 
such that SCSI_SCAN_MANUAL happens to remain 1 so we don't have to touch 
many callers (probably there would still remain some callers we would 
have to touch because 1 does not mean MANUAL but RESCAN there)?

In particular, the following might be necessary in order not to break 
zfcp manual LUN addition (as long as zfcp still has its own unit_add 
sysfs interface; completely replacing it with the solution here is not 
easy because it breaks the zfcp sysfs user interface):

> void zfcp_unit_scsi_scan(struct zfcp_unit *unit)
> {
> 	struct fc_rport *rport = unit->port->rport;
> 	u64 lun;
>
> 	lun = scsilun_to_int((struct scsi_lun *) &unit->fcp_lun);
>
> 	if (rport && rport->port_state == FC_PORTSTATE_ONLINE)
>- 		scsi_scan_target(&rport->dev, 0, rport->scsi_target_id, lun, 1);
>+ 		scsi_scan_target(&rport->dev, 0, rport->scsi_target_id, lun,
>+ 				 SCSI_SCAN_MANUAL);
> }

Not sure about: mpt_work_wrapper(), snic_scsi_scan_tgt().

> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -143,7 +143,8 @@ static int scsi_scan(struct Scsi_Host *shost, const char *str)

> +		res = scsi_scan_host_selected(shost, channel, id, lun,
> +					      SCSI_SCAN_MANUAL);

> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -2110,7 +2110,8 @@ fc_user_scan_tgt(struct Scsi_Host *shost, uint channel, uint id, u64 lun)

> +			scsi_scan_target(&rport->dev, channel, id, lun,
> +					 SCSI_SCAN_MANUAL);

> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -1819,7 +1819,7 @@ static int iscsi_user_scan_session(struct device *dev, void *data)

>   			scsi_scan_target(&session->dev, 0, id,
> +					 scan_data->lun, SCSI_SCAN_MANUAL);

> +++ b/drivers/scsi/scsi_transport_sas.c
> @@ -1739,8 +1739,8 @@ static int sas_user_scan(struct Scsi_Host *shost, uint channel,

> +			scsi_scan_target(&rphy->dev, 0, rphy->scsi_target_id,
> +					 lun, SCSI_SCAN_MANUAL);

> +++ b/include/scsi/scsi_device.h

> +enum scsi_scan_mode {
> +	SCSI_SCAN_NONE = 0,
> +	SCSI_SCAN_RESCAN,
> +	SCSI_SCAN_MANUAL,
> +};

-- 
Mit freundlichen Grüßen / Kind regards
Steffen Maier

Linux on z Systems Development

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

--
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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2] scsi: disable automatic target scan
  2016-03-15 11:03 ` Steffen Maier
@ 2016-03-15 11:28   ` Hannes Reinecke
  2016-03-15 11:48     ` Steffen Maier
  0 siblings, 1 reply; 7+ messages in thread
From: Hannes Reinecke @ 2016-03-15 11:28 UTC (permalink / raw)
  To: Steffen Maier, Martin K. Petersen
  Cc: Christoph Hellwig, James Bottomley, linux-scsi

On 03/15/2016 12:03 PM, Steffen Maier wrote:
> On 03/15/2016 08:19 AM, Hannes Reinecke wrote:
>> On larger installations it is useful to disable automatic LUN
>> scanning, and only add the required LUNs via udev rules.
>> This can speed up bootup dramatically.
>>
>> This patch introduces a new scan module parameter value 'manual',
>> which works like 'none', but can be overriden by setting the 'rescan'
>> value from scsi_scan_target to 'SCSI_SCAN_MANUAL'.
>> And it updates all relevant callers to set the 'rescan' value
>> to 'SCSI_SCAN_MANUAL' if invoked via the 'scan' option in sysfs.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
> 
>> +MODULE_PARM_DESC(scan, "sync, async, manual, or none. "
>> +         "Setting to 'manual' disables automatic scanning, but
>> allows "
>> +         "for manual device scan via the 'scan' sysfs attribute.");
> 
> nice solution
> 
>> + * @rescan:    passed to LUN scanning routines; SCSI_SCAN_NONE
>> for no rescan,
>> + *              SCSI_SCAN_RESCAN to rescan existing LUNs, and
>> SCSI_SCAN_MANUAL
>> + *              to force scanning even if 'scan=manual' is set.
> 
>> @@ -1621,6 +1627,10 @@ void scsi_scan_target(struct device
>> *parent, unsigned int channel,
>>       if (strncmp(scsi_scan_type, "none", 4) == 0)
>>           return;
>>
>> +    if (rescan < SCSI_SCAN_MANUAL &&
>> +        strncmp(scsi_scan_type, "manual", 6) == 0)
>> +        return;
>> +
> 
> I see that the ordering of identifiers in enum scsi_scan_mode allows
> a nice inequation here.
> 
> However, it also seems that we need to touch all callers (which
> intend an unconditional manual scan) of scsi_scan_target() that used
> to call it with rescan==1 which would now be
> rescan==SCSI_SCAN_MANUAL==2 ?
> 
Yes, sort of.

However, not _all_ callers need to be touched (as this would defeat
the purpose). We just need to touch those callers which will be
invoked by a _manual_ scan via

echo "X X X" > /sys/class/scsi_host/hostX/scan

Other callers should _not_ be modified, as the automatic scan should
be disabled.

> Or could we define the ordering of identifiers in enum
> scsi_scan_mode such that SCSI_SCAN_MANUAL happens to remain 1 so we
> don't have to touch many callers (probably there would still remain
> some callers we would have to touch because 1 does not mean MANUAL
> but RESCAN there)?
> 
> In particular, the following might be necessary in order not to
> break zfcp manual LUN addition (as long as zfcp still has its own
> unit_add sysfs interface; completely replacing it with the solution
> here is not easy because it breaks the zfcp sysfs user interface):
> 
>> void zfcp_unit_scsi_scan(struct zfcp_unit *unit)
>> {
>>     struct fc_rport *rport = unit->port->rport;
>>     u64 lun;
>>
>>     lun = scsilun_to_int((struct scsi_lun *) &unit->fcp_lun);
>>
>>     if (rport && rport->port_state == FC_PORTSTATE_ONLINE)
>> -         scsi_scan_target(&rport->dev, 0, rport->scsi_target_id,
>> lun, 1);
>> +         scsi_scan_target(&rport->dev, 0, rport->scsi_target_id,
>> lun,
>> +                  SCSI_SCAN_MANUAL);
>> }
> 
> Not sure about: mpt_work_wrapper(), snic_scsi_scan_tgt().
> 
See above. We only need to modify the call sites triggered from
drivers/scsi/scsi_sysfs.c:scsi_scan(); all other functions need to
remain as-is to allow the mechanism to work correctly.

As for zfcp: it also relies on the 'user_scan' callback from
scsi_transport_fc, so it will fall under the same rules as any
'normal' FC HBA.
But I do think we can tie the 'scan' module parameter with the
'allow_lun_scan' parameter from zfcp; both are now doing essentially
the same. Eg the 'allow_lun_scan' parameter could take it's default
value from the 'scan' parameter.

In the long run we might want to check if and how the global 'scan'
parameter can be moved down to a host parameter, which then would
eliminate the need for a zfcp-specific parameter.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2] scsi: disable automatic target scan
  2016-03-15 11:28   ` Hannes Reinecke
@ 2016-03-15 11:48     ` Steffen Maier
  2016-03-15 11:54       ` Hannes Reinecke
  0 siblings, 1 reply; 7+ messages in thread
From: Steffen Maier @ 2016-03-15 11:48 UTC (permalink / raw)
  To: Hannes Reinecke, Martin K. Petersen
  Cc: Christoph Hellwig, James Bottomley, linux-scsi

On 03/15/2016 12:28 PM, Hannes Reinecke wrote:
> On 03/15/2016 12:03 PM, Steffen Maier wrote:
>> On 03/15/2016 08:19 AM, Hannes Reinecke wrote:
>>> On larger installations it is useful to disable automatic LUN
>>> scanning, and only add the required LUNs via udev rules.
>>> This can speed up bootup dramatically.
>>>
>>> This patch introduces a new scan module parameter value 'manual',
>>> which works like 'none', but can be overriden by setting the 'rescan'
>>> value from scsi_scan_target to 'SCSI_SCAN_MANUAL'.
>>> And it updates all relevant callers to set the 'rescan' value
>>> to 'SCSI_SCAN_MANUAL' if invoked via the 'scan' option in sysfs.
>>>
>>> Signed-off-by: Hannes Reinecke <hare@suse.de>

>> In particular, the following might be necessary in order not to
>> break zfcp manual LUN addition (as long as zfcp still has its own
>> unit_add sysfs interface; completely replacing it with the solution
>> here is not easy because it breaks the zfcp sysfs user interface):
>>
>>> void zfcp_unit_scsi_scan(struct zfcp_unit *unit)
>>> {
>>>      struct fc_rport *rport = unit->port->rport;
>>>      u64 lun;
>>>
>>>      lun = scsilun_to_int((struct scsi_lun *) &unit->fcp_lun);
>>>
>>>      if (rport && rport->port_state == FC_PORTSTATE_ONLINE)
>>> -         scsi_scan_target(&rport->dev, 0, rport->scsi_target_id,
>>> lun, 1);
>>> +         scsi_scan_target(&rport->dev, 0, rport->scsi_target_id,
>>> lun,
>>> +                  SCSI_SCAN_MANUAL);
>>> }

> As for zfcp: it also relies on the 'user_scan' callback from
> scsi_transport_fc, so it will fall under the same rules as any
> 'normal' FC HBA.

I'm not sure, since zfcp needs to continue to support HBA hardware 
virtualization without NPIV.

The zfcp unit_add sysfs interface is kind of a peer of your common code 
scsi scan interface interface. I think, for zfcp users, a unit_add needs 
to be sufficient. It would be strange to additionally have them also 
write the same LUN into the Scsi_Host's scan sysfs attribute, i.e. 
having to double configure things.

> But I do think we can tie the 'scan' module parameter with the
> 'allow_lun_scan' parameter from zfcp; both are now doing essentially
> the same. Eg the 'allow_lun_scan' parameter could take it's default
> value from the 'scan' parameter.

> In the long run we might want to check if and how the global 'scan'
> parameter can be moved down to a host parameter, which then would
> eliminate the need for a zfcp-specific parameter.

I need to think about this.

-- 
Mit freundlichen Grüßen / Kind regards
Steffen Maier

Linux on z Systems Development

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

--
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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2] scsi: disable automatic target scan
  2016-03-15 11:48     ` Steffen Maier
@ 2016-03-15 11:54       ` Hannes Reinecke
  0 siblings, 0 replies; 7+ messages in thread
From: Hannes Reinecke @ 2016-03-15 11:54 UTC (permalink / raw)
  To: Steffen Maier, Martin K. Petersen
  Cc: Christoph Hellwig, James Bottomley, linux-scsi

On 03/15/2016 12:48 PM, Steffen Maier wrote:
> On 03/15/2016 12:28 PM, Hannes Reinecke wrote:
>> On 03/15/2016 12:03 PM, Steffen Maier wrote:
>>> On 03/15/2016 08:19 AM, Hannes Reinecke wrote:
>>>> On larger installations it is useful to disable automatic LUN
>>>> scanning, and only add the required LUNs via udev rules.
>>>> This can speed up bootup dramatically.
>>>>
>>>> This patch introduces a new scan module parameter value 'manual',
>>>> which works like 'none', but can be overriden by setting the
>>>> 'rescan'
>>>> value from scsi_scan_target to 'SCSI_SCAN_MANUAL'.
>>>> And it updates all relevant callers to set the 'rescan' value
>>>> to 'SCSI_SCAN_MANUAL' if invoked via the 'scan' option in sysfs.
>>>>
>>>> Signed-off-by: Hannes Reinecke <hare@suse.de>
> 
>>> In particular, the following might be necessary in order not to
>>> break zfcp manual LUN addition (as long as zfcp still has its own
>>> unit_add sysfs interface; completely replacing it with the solution
>>> here is not easy because it breaks the zfcp sysfs user interface):
>>>
>>>> void zfcp_unit_scsi_scan(struct zfcp_unit *unit)
>>>> {
>>>>      struct fc_rport *rport = unit->port->rport;
>>>>      u64 lun;
>>>>
>>>>      lun = scsilun_to_int((struct scsi_lun *) &unit->fcp_lun);
>>>>
>>>>      if (rport && rport->port_state == FC_PORTSTATE_ONLINE)
>>>> -         scsi_scan_target(&rport->dev, 0, rport->scsi_target_id,
>>>> lun, 1);
>>>> +         scsi_scan_target(&rport->dev, 0, rport->scsi_target_id,
>>>> lun,
>>>> +                  SCSI_SCAN_MANUAL);
>>>> }
> 
>> As for zfcp: it also relies on the 'user_scan' callback from
>> scsi_transport_fc, so it will fall under the same rules as any
>> 'normal' FC HBA.
> 
> I'm not sure, since zfcp needs to continue to support HBA hardware
> virtualization without NPIV.
> 
> The zfcp unit_add sysfs interface is kind of a peer of your common
> code scsi scan interface interface. I think, for zfcp users, a
> unit_add needs to be sufficient. It would be strange to additionally
> have them also write the same LUN into the Scsi_Host's scan sysfs
> attribute, i.e. having to double configure things.
> 
Hmm. Good point. So yeah, if auto LUN scan is disabled we should be
overriding any setting from the 'scan' parameter.

Let's see if I can come up with a patch here.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2] scsi: disable automatic target scan
  2016-03-15  7:19 [PATCHv2] scsi: disable automatic target scan Hannes Reinecke
  2016-03-15  8:31 ` Johannes Thumshirn
  2016-03-15 11:03 ` Steffen Maier
@ 2016-03-15 13:36 ` Christoph Hellwig
  2 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2016-03-15 13:36 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley,
	linux-scsi

On Tue, Mar 15, 2016 at 08:19:42AM +0100, Hannes Reinecke wrote:
> On larger installations it is useful to disable automatic LUN
> scanning, and only add the required LUNs via udev rules.
> This can speed up bootup dramatically.
> 
> This patch introduces a new scan module parameter value 'manual',
> which works like 'none', but can be overriden by setting the 'rescan'
> value from scsi_scan_target to 'SCSI_SCAN_MANUAL'.
> And it updates all relevant callers to set the 'rescan' value
> to 'SCSI_SCAN_MANUAL' if invoked via the 'scan' option in sysfs.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/scsi_scan.c            | 25 ++++++++++++++++++-------
>  drivers/scsi/scsi_sysfs.c           |  3 ++-
>  drivers/scsi/scsi_transport_fc.c    |  3 ++-
>  drivers/scsi/scsi_transport_iscsi.c |  2 +-
>  drivers/scsi/scsi_transport_sas.c   |  4 ++--
>  include/scsi/scsi_device.h          |  6 ++++++
>  6 files changed, 31 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 97074c9..54dc517 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -96,10 +96,13 @@ MODULE_PARM_DESC(max_luns,
>  #define SCSI_SCAN_TYPE_DEFAULT "sync"
>  #endif
>  
> -char scsi_scan_type[6] = SCSI_SCAN_TYPE_DEFAULT;
> +char scsi_scan_type[7] = SCSI_SCAN_TYPE_DEFAULT;
>  
> -module_param_string(scan, scsi_scan_type, sizeof(scsi_scan_type), S_IRUGO);
> -MODULE_PARM_DESC(scan, "sync, async or none");
> +module_param_string(scan, scsi_scan_type, sizeof(scsi_scan_type),
> +		    S_IRUGO|S_IWUSR);
> +MODULE_PARM_DESC(scan, "sync, async, manual, or none. "
> +		 "Setting to 'manual' disables automatic scanning, but allows "
> +		 "for manual device scan via the 'scan' sysfs attribute.");
>  
>  static unsigned int scsi_inq_timeout = SCSI_TIMEOUT/HZ + 18;
>  
> @@ -1040,7 +1043,8 @@ static unsigned char *scsi_inq_str(unsigned char *buf, unsigned char *inq,
>   * @lun:	LUN of target device
>   * @bflagsp:	store bflags here if not NULL
>   * @sdevp:	probe the LUN corresponding to this scsi_device
> - * @rescan:     if nonzero skip some code only needed on first scan
> + * @rescan:     if greater than SCSI_SCAN_NONE skip some code only
> + *              needed on first scan

please pass the actual enum to all the functions taking this
recan argument so that we can do proper type checking on it.

> +		if (rescan > SCSI_SCAN_NONE || !scsi_device_created(sdev)) {

And always check just check for equality instead of doing BT / LT on
enum values.

>  
> +enum scsi_scan_mode {
> +	SCSI_SCAN_NONE = 0,

and this could use a better name.  SCSI_SCAN_INITAL maybe?

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-03-15 13:36 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-15  7:19 [PATCHv2] scsi: disable automatic target scan Hannes Reinecke
2016-03-15  8:31 ` Johannes Thumshirn
2016-03-15 11:03 ` Steffen Maier
2016-03-15 11:28   ` Hannes Reinecke
2016-03-15 11:48     ` Steffen Maier
2016-03-15 11:54       ` Hannes Reinecke
2016-03-15 13:36 ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).