linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] 3ware: use scsi_scan_target()
@ 2005-10-04  0:57 Jeff Garzik
  2005-10-05 16:28 ` Christoph Hellwig
  0 siblings, 1 reply; 15+ messages in thread
From: Jeff Garzik @ 2005-10-04  0:57 UTC (permalink / raw)
  To: SCSI Mailing List, linuxraid

[-- Attachment #1: Type: text/plain, Size: 288 bytes --]

This change updates the 3ware raid drivers to use scsi_scan_target(), 
rather than scsi_scan_host().  This is especially nice for 3w-xxxx, 
which does not support LUNs.  The device scan is a bit quicker and more 
direct, even if it is a tiny bit more code in the driver.

Patch untested.

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1409 bytes --]

diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -1992,7 +1992,7 @@ static int __devinit twa_probe(struct pc
 	struct Scsi_Host *host = NULL;
 	TW_Device_Extension *tw_dev;
 	u32 mem_addr;
-	int retval = -ENODEV;
+	int retval = -ENODEV, i;
 
 	retval = pci_enable_device(pdev);
 	if (retval) {
@@ -2092,7 +2092,8 @@ static int __devinit twa_probe(struct pc
 	TW_ENABLE_AND_CLEAR_INTERRUPTS(tw_dev);
 
 	/* Finally, scan the host */
-	scsi_scan_host(host);
+	for (i = 0; i < TW_MAX_UNITS; i++)
+		scsi_scan_target(&host->shost_gendev, 0, i, ~0, 0);
 
 	if (twa_major == -1) {
 		if ((twa_major = register_chrdev (0, "twa", &twa_fops)) < 0)
diff --git a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c
--- a/drivers/scsi/3w-xxxx.c
+++ b/drivers/scsi/3w-xxxx.c
@@ -2315,7 +2315,7 @@ static int __devinit tw_probe(struct pci
 {
 	struct Scsi_Host *host = NULL;
 	TW_Device_Extension *tw_dev;
-	int retval = -ENODEV;
+	int retval = -ENODEV, i;
 
 	retval = pci_enable_device(pdev);
 	if (retval) {
@@ -2404,7 +2404,8 @@ static int __devinit tw_probe(struct pci
 	TW_ENABLE_AND_CLEAR_INTERRUPTS(tw_dev);
 
 	/* Finally, scan the host */
-	scsi_scan_host(host);
+	for (i = 0; i < TW_MAX_UNITS; i++)
+		scsi_scan_target(&host->shost_gendev, 0, i, 0, 0);
 
 	if (twe_major == -1) {
 		if ((twe_major = register_chrdev (0, "twe", &tw_fops)) < 0)

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

* Re: [PATCH] 3ware: use scsi_scan_target()
  2005-10-04  0:57 Jeff Garzik
@ 2005-10-05 16:28 ` Christoph Hellwig
  2005-10-05 19:01   ` Luben Tuikov
  2005-10-05 19:34   ` Jeff Garzik
  0 siblings, 2 replies; 15+ messages in thread
From: Christoph Hellwig @ 2005-10-05 16:28 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: SCSI Mailing List, linuxraid

On Mon, Oct 03, 2005 at 08:57:46PM -0400, Jeff Garzik wrote:
> This change updates the 3ware raid drivers to use scsi_scan_target(), 
> rather than scsi_scan_host().  This is especially nice for 3w-xxxx, 
> which does not support LUNs.  The device scan is a bit quicker and more 
> direct, even if it is a tiny bit more code in the driver.

if it doesn't support luns scsi_add_device sounds like the better
interface to use.  How does 3x-9xxx support luns?  From reading the
code it doesn't seem to support passthru and faking up LUNs for logical
volumes sounds odd.


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

* Re: [PATCH] 3ware: use scsi_scan_target()
  2005-10-05 16:28 ` Christoph Hellwig
@ 2005-10-05 19:01   ` Luben Tuikov
  2005-10-05 19:20     ` adam radford
  2005-10-05 19:24     ` Jeff Garzik
  2005-10-05 19:34   ` Jeff Garzik
  1 sibling, 2 replies; 15+ messages in thread
From: Luben Tuikov @ 2005-10-05 19:01 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jeff Garzik, SCSI Mailing List, linuxraid

On 10/05/05 12:28, Christoph Hellwig wrote:
> On Mon, Oct 03, 2005 at 08:57:46PM -0400, Jeff Garzik wrote:
> 
>>This change updates the 3ware raid drivers to use scsi_scan_target(), 
>>rather than scsi_scan_host().  This is especially nice for 3w-xxxx, 
>>which does not support LUNs.  The device scan is a bit quicker and more 
>>direct, even if it is a tiny bit more code in the driver.
> 
> 
> if it doesn't support luns scsi_add_device sounds like the better
> interface to use.

I agree.

> How does 3x-9xxx support luns?  From reading the

AFAIR the firmware just exports the LUs.  The FW doesn't
support REPORT LUNS; the LLDD exported LUs directly to SCSI Core.

	Luben

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

* Re: [PATCH] 3ware: use scsi_scan_target()
  2005-10-05 19:01   ` Luben Tuikov
@ 2005-10-05 19:20     ` adam radford
  2005-10-05 19:24     ` Jeff Garzik
  1 sibling, 0 replies; 15+ messages in thread
From: adam radford @ 2005-10-05 19:20 UTC (permalink / raw)
  To: Luben Tuikov; +Cc: Christoph Hellwig, Jeff Garzik, SCSI Mailing List, linuxraid

The firmware for the 9000 series 3ware controllers definately does support
the REPORT LUNS scsi opcode.  The only reason the LUN support is there
is for older 2.4 customers who have carved up > 2TB arrays into several
luns who then upgrade to kernel 2.6.

-Adam

On 10/5/05, Luben Tuikov <luben_tuikov@adaptec.com> wrote:
> On 10/05/05 12:28, Christoph Hellwig wrote:
> > On Mon, Oct 03, 2005 at 08:57:46PM -0400, Jeff Garzik wrote:
> >
> >>This change updates the 3ware raid drivers to use scsi_scan_target(),
> >>rather than scsi_scan_host().  This is especially nice for 3w-xxxx,
> >>which does not support LUNs.  The device scan is a bit quicker and more
> >>direct, even if it is a tiny bit more code in the driver.
> >
> >
> > if it doesn't support luns scsi_add_device sounds like the better
> > interface to use.
>
> I agree.
>
> > How does 3x-9xxx support luns?  From reading the
>
> AFAIR the firmware just exports the LUs.  The FW doesn't
> support REPORT LUNS; the LLDD exported LUs directly to SCSI Core.
>
>         Luben
> -
> 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] 15+ messages in thread

* Re: [PATCH] 3ware: use scsi_scan_target()
  2005-10-05 19:01   ` Luben Tuikov
  2005-10-05 19:20     ` adam radford
@ 2005-10-05 19:24     ` Jeff Garzik
  1 sibling, 0 replies; 15+ messages in thread
From: Jeff Garzik @ 2005-10-05 19:24 UTC (permalink / raw)
  To: Luben Tuikov, Christoph Hellwig; +Cc: SCSI Mailing List, linuxraid

Luben Tuikov wrote:
> On 10/05/05 12:28, Christoph Hellwig wrote:
>>How does 3x-9xxx support luns?  From reading the

> AFAIR the firmware just exports the LUs.  The FW doesn't
> support REPORT LUNS; the LLDD exported LUs directly to SCSI Core.

Some 9xxx firmwares do support LUNs:

>         /* Check if this FW supports luns */
>         if ((SCpnt->device->lun != 0) && (tw_dev->working_srl < TW_FW_SRL_LUNS_SUPPORTED)) {

and the newer 9xxx are quite different from 3w-xxxx, because 9xxx 
firmware interface is largely just a execute-scsi-command message, 
rather than the translation stuff the goes on in 3w-xxxx.

As to Christoph's question, _how_ does the firmware support LUNs?  No 
idea.  It clearly does support it, though.

	Jeff



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

* Re: [PATCH] 3ware: use scsi_scan_target()
  2005-10-05 16:28 ` Christoph Hellwig
  2005-10-05 19:01   ` Luben Tuikov
@ 2005-10-05 19:34   ` Jeff Garzik
  2005-10-05 23:19     ` Luben Tuikov
  1 sibling, 1 reply; 15+ messages in thread
From: Jeff Garzik @ 2005-10-05 19:34 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: SCSI Mailing List, linuxraid

Christoph Hellwig wrote:
> On Mon, Oct 03, 2005 at 08:57:46PM -0400, Jeff Garzik wrote:
> 
>>This change updates the 3ware raid drivers to use scsi_scan_target(), 
>>rather than scsi_scan_host().  This is especially nice for 3w-xxxx, 
>>which does not support LUNs.  The device scan is a bit quicker and more 
>>direct, even if it is a tiny bit more code in the driver.
> 
> 
> if it doesn't support luns scsi_add_device sounds like the better
> interface to use.

That applies to 3w-xxxx, certainly.

I don't see a strong argument for scsi_add_device() over 
scsi_scan_target(), though.  scsi_scan_target() directly supports the 
generic struct device/class_device stuff, and supports both wildcard LUN 
scans as well as no-LUN targets.  My vague preference would be to use 
scsi_scan_target() unless there is a strong reason not to...


> How does 3x-9xxx support luns?  From reading the
> code it doesn't seem to support passthru and faking up LUNs for logical
> volumes sounds odd.

[other emails appear to have addressed this]

	Jeff



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

* Re: [PATCH] 3ware: use scsi_scan_target()
  2005-10-05 19:34   ` Jeff Garzik
@ 2005-10-05 23:19     ` Luben Tuikov
  2005-10-07  1:07       ` James Bottomley
  0 siblings, 1 reply; 15+ messages in thread
From: Luben Tuikov @ 2005-10-05 23:19 UTC (permalink / raw)
  To: Jeff Garzik, Christoph Hellwig; +Cc: SCSI Mailing List, linuxraid

--- Jeff Garzik <jgarzik@pobox.com> wrote:
> scans as well as no-LUN targets.  My vague preference would be to use 
> scsi_scan_target() unless there is a strong reason not to...

This sounds good too.

I'd like to move sas_do_lu_discovery(struct domain_device *dev)
into SCSI Core (as the comment therein says), for _new_ (non-legacy)
devices, i.e. with newer FW.

It also handles devices who do not even respond
to REPORT LUNS on either LU 0 or RL WLUN (like early prototype
SES FW), in which case it would register LU 0 for them since
this is where tasks are sent.

Example of those is seen in the domain sysfs tree here:
http://marc.theaimsgroup.com/?l=linux-scsi&m=112629509826900&w=2

The code is at drivers/scsi/sas/sas_discover.c
http://linux.adaptec.com/sas/ .

     Luben
P.S. REPORT LUNS is Mandatory as per SPC, so newer devices
(SAS) support it.  Furthermore if their LUs are sparse they
(really) support REPORT LUNS.


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

* RE: [PATCH] 3ware: use scsi_scan_target()
@ 2005-10-06 13:13 James.Smart
  2005-10-06 18:09 ` Luben Tuikov
  0 siblings, 1 reply; 15+ messages in thread
From: James.Smart @ 2005-10-06 13:13 UTC (permalink / raw)
  To: ltuikov, jgarzik, hch; +Cc: linux-scsi, linuxraid


> I'd like to move sas_do_lu_discovery(struct domain_device *dev)
> into SCSI Core (as the comment therein says), for _new_ (non-legacy)
> devices, i.e. with newer FW.

My vote, for what it's worth, is to *not* have multiple discovery threads.
I don't care about "old" or "new", and in general, even things with "new"
interfaces behave in "old" manners, as they typically are existing
subsystems with new interface chips placed on them. It also makes life
hell for transport bridging devices, which want to map at the transport
endpoint level, and leave luns up to the actual SCSI task manager in the
device. Remember - devices are made to work with many OS's, and the level
of SCSI/SAM support in each os differs, so the devices choose a median
that allows them to function with the OS's they care about. It's no longer
a SAM thing. Also - SAM/SPC/SBC/etc has many many grey areas, and SAM (the
original) was written such that SCSI-2 devices were still SAM compliant.
And if you say SAM - what rev of SAM (SAM, SAM-2, SAM-3) ?  There are
things allowed in SAM that are disallowed in SAM-3. Same with SPC and so on.

I've lived with a driver with it's own scan subsystem (which equates to
your own SAS scan functions) and all it did was create confusion for the
end users. If a device is mis-behaving, do you tweak driver/sas_transport
tunables, or do you tweak black/white list entries ? How do you deal with
the same SAS target device that behaves differently behind 2 different
adapters - one using the midlayers standard scanning functions (which the
device may already be supported by) vs the other using its own custom scan
logic. I know you'll come back at this from the pristine view that SAS is
new - well true, the transport is new, but the devices aren't necessarily.
Go back and look at the exceptions I mentioned above.

Bottomline, this kind of choice just makes life confusing for the end user.

As has been said before on this list - replicating scan functions is silly.
Let's fix or extend the current scan behavior.

-- james

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

* Re: [PATCH] 3ware: use scsi_scan_target()
  2005-10-06 13:13 [PATCH] 3ware: use scsi_scan_target() James.Smart
@ 2005-10-06 18:09 ` Luben Tuikov
  0 siblings, 0 replies; 15+ messages in thread
From: Luben Tuikov @ 2005-10-06 18:09 UTC (permalink / raw)
  To: James.Smart; +Cc: ltuikov, jgarzik, hch, linux-scsi, linuxraid

On 10/06/05 09:13, James.Smart@Emulex.Com wrote:
>>I'd like to move sas_do_lu_discovery(struct domain_device *dev)
>>into SCSI Core (as the comment therein says), for _new_ (non-legacy)
>>devices, i.e. with newer FW.
> 

Hi James, how are you?

> My vote, for what it's worth, is to *not* have multiple discovery threads.
> I don't care about "old" or "new", and in general, even things with "new"
> interfaces behave in "old" manners, as they typically are existing
> subsystems with new interface chips placed on them. It also makes life
> hell for transport bridging devices, which want to map at the transport
> endpoint level, and leave luns up to the actual SCSI task manager in the
> device.

This is perfectly fine -- SAS FW supports REPORT LUNS.

> Remember - devices are made to work with many OS's, and the level
> of SCSI/SAM support in each os differs, so the devices choose a median
> that allows them to function with the OS's they care about. It's no longer
> a SAM thing.

Would you be presenting SAS devices as something else (FC, etc)?
In which case the old, broken, semantically fat scanning technique
would apply.  This is ok also.

> Also - SAM/SPC/SBC/etc has many many grey areas, and SAM (the
> original) was written such that SCSI-2 devices were still SAM compliant.
> And if you say SAM - what rev of SAM (SAM, SAM-2, SAM-3) ?  There are
> things allowed in SAM that are disallowed in SAM-3. Same with SPC and so on.

SAM-3 and/or SAM-4.  (What you'd normally see is SAM-4 behaviour advertized
as SAM-3.)

> I've lived with a driver with it's own scan subsystem (which equates to
> your own SAS scan functions) and all it did was create confusion for the
> end users. If a device is mis-behaving, do you tweak driver/sas_transport
> tunables, or do you tweak black/white list entries ? How do you deal with

Yes, this is a good hypotethical _SAS_ situation.  Let's get there
first, who knows, we may never do.

SAS FW writers are aware that REPORT LUNS is mandatory.

Can you imagine a SAS device supporting more than one LUN and _not_
supporting REPORT LUNS?

As to transport bridging -- at least one thing the transport bridge
would do is implement the _first_ command that would be sent to a
SCSI device: REPORT LUNS.  Imagine a transport bridge device _not_
supporting REPORT LUNS.

> the same SAS target device that behaves differently behind 2 different
> adapters - one using the midlayers standard scanning functions (which the
> device may already be supported by) vs the other using its own custom scan
> logic.

Then the bug is in the "adapters", since the task router hasn't changed.

> I know you'll come back at this from the pristine view that SAS is
> new - well true, the transport is new, but the devices aren't necessarily.

Yes, as an engineer I'm an optimist.

You have a good point, but if the underlying devices are _old_ and there's
a SAS-to-whatever bridge, then _that_ bridge would have to properly implement
REPORT LUNS (somehow).

It would be quite unbelievable (to me) to find out a SAS device which
does not support REPORT LUNS and we have to black listed it.

> Go back and look at the exceptions I mentioned above.
> 
> Bottomline, this kind of choice just makes life confusing for the end user.
> 
> As has been said before on this list - replicating scan functions is silly.
> Let's fix or extend the current scan behavior.

That's fine James, but there is only so much crud the current code can
take.  After a while, it starts breaking at the seams.

So why don't we take it one step at a time.  If you have a specific
SAS device which would not work with the proper LUN scan as shown
in sas_discover.c::sas_do_lu_discovery(struct domain_device *dev),
please let lsml know about it.

	Luben

P.S. How much crud can we add to older code?

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

* Re: [PATCH] 3ware: use scsi_scan_target()
  2005-10-05 23:19     ` Luben Tuikov
@ 2005-10-07  1:07       ` James Bottomley
  2005-10-07 21:36         ` Luben Tuikov
  0 siblings, 1 reply; 15+ messages in thread
From: James Bottomley @ 2005-10-07  1:07 UTC (permalink / raw)
  To: ltuikov; +Cc: Jeff Garzik, Christoph Hellwig, SCSI Mailing List, linuxraid

On Wed, 2005-10-05 at 16:19 -0700, Luben Tuikov wrote:
> I'd like to move sas_do_lu_discovery(struct domain_device *dev)
> into SCSI Core (as the comment therein says), for _new_ (non-legacy)
> devices, i.e. with newer FW.

Initially my reaction is not for the time being, for two reasons

1. The domain device as you have implemented it requires a lot of
infrastructure support.  I'd like to see this refined in the transport
classes to hone it before considering pulling it into the mid-layer
2. sas_do_lu_discovery() duplicates a lot of existing functionality, but
also lacks a lot of quirk processing.  I know the argument is that SAS
won't have any quirks, but I'd like to have this proven in the field
before I take it as read.

> It also handles devices who do not even respond
> to REPORT LUNS on either LU 0 or RL WLUN (like early prototype
> SES FW), in which case it would register LU 0 for them since
> this is where tasks are sent.

I'm sure you're aware that not responding on either LUN 0 or the report
luns WLUN is a violation of SAM  ... however, if we really already have
broken SAS devices with this problem, then you're welcome to expand
scsi_scan_target() to cope.

WLUN support is really the only piece that scsi_scan_target() doesn't
currently possess and, as has been discussed before, that only really
matters if we get a target that's not going to respond to an INQUIRY on
LUN0 (I expect most of them, even if they have no LUN0 will respond with
PQ 3 to the inquiry and thus be caught by scsi_scan_target() anyway).

However, to add WLUN support to scsi_scan_target() looks pretty simple:
If the transport supports > 256 LUNs and the initial INQUIRY comes back
SCSI_SCAN_NO_RESPONSE then fire off a WLUN report lun scan anyway.  By
all means, submit a patch to do this.

> P.S. REPORT LUNS is Mandatory as per SPC, so newer devices
> (SAS) support it.  Furthermore if their LUs are sparse they
> (really) support REPORT LUNS.

Actually, it's optional per SPC; it's mandatory in SPC-2 but *only* if
your device is multi-LUN; and it finally became mandatory for everything
in SPC-3 (or I should say "becomes" since SPC-3 isn't ratified yet).

James



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

* Re: [PATCH] 3ware: use scsi_scan_target()
  2005-10-07  1:07       ` James Bottomley
@ 2005-10-07 21:36         ` Luben Tuikov
  2005-10-08 14:30           ` James Bottomley
  0 siblings, 1 reply; 15+ messages in thread
From: Luben Tuikov @ 2005-10-07 21:36 UTC (permalink / raw)
  To: James Bottomley
  Cc: ltuikov, Jeff Garzik, Christoph Hellwig, SCSI Mailing List,
	linuxraid

On 10/06/05 21:07, James Bottomley wrote:
> 1. The domain device as you have implemented it requires a lot of

The domain device as I've implemented it, is exactly a SAS domain
device, supporting expanders, SAS, SATA, SATA PM, etc. devices.  It
doesn't add any cruft, it is the minimum of what it should be.

struct domain_device {
	struct kobject    dev_obj;
	enum sas_dev_type dev_type;

	enum sas_phy_linkrate linkrate;
	enum sas_phy_linkrate min_linkrate;
	enum sas_phy_linkrate max_linkrate;

	int  pathways;

	struct domain_device *parent;
	struct list_head siblings; /* devices on the same level */
	struct sas_port *port;	  /* shortcut to root of the tree */

	struct list_head dev_list_node;

	enum sas_proto    iproto;
	enum sas_proto    tproto;

	u8  sas_addr[SAS_ADDR_SIZE];
	u8  hashed_sas_addr[HASHED_SAS_ADDR_SIZE];

	u8  frame_rcvd[32];

	union {
		struct expander_device ex_dev;
		struct end_device      end_dev;
		struct sata_device     sata_dev; /* STP & directly attached */
	};

	void *lldd_dev;
};

> infrastructure support.  I'd like to see this refined in the transport

I'm not sure that you can "refine" (take stuff out) from
struct domain_device.

You're better off creating struct scsi_domain_device { ... }; and starting
from there.

In effect struct scsi_domain_device { ... }; (non existant yet)
would be a superclass (in OO sense) of the SAS struct domain_device, shown
above.

In contrast to "templates", the OO sense can be kept _by design_,
by _SCSI_ design, not by code enforcement (i.e. struct template_abcd...).

> 2. sas_do_lu_discovery() duplicates a lot of existing functionality, but
> also lacks a lot of quirk processing.  I know the argument is that SAS

It doesn't duplicate a lot of functionality.  While the code
in SCSI Core does do LU discovery, the infrastructure is very broken
in that
  1) there is no native "target" (i.e. SCSI Device with Target port)
     support (struct domain_device), and
  2) that code uses HCIL, by scsi_scan_target definition and from
     seeing things like:

	if (shost->this_id == id)
		/*
		 * Don't scan the host adapter
		 */
		return;

As to SAS, the comment is about the size of the function:

/**
 * sas_do_lu_discovery -- Discover LUs of a SCSI device
 * @dev: pointer to a domain device of interest
 *
 * Discover logical units present in the SCSI device.  I'd like this
 * to be moved to SCSI Core, but SCSI Core has no concept of a "SCSI
 * device with a SCSI Target port".  A SCSI device with a SCSI Target
 * port is a device which the _transport_ found, but other than that,
 * the transport has little or _no_ knowledge about the device.
 * Ideally, a LLDD would register a "SCSI device with a SCSI Target
 * port" with SCSI Core and then SCSI Core would do LU discovery of
 * that device.
 *
 * REPORT LUNS is mandatory.  If a device doesn't support it,
 * it is broken and you should return it.  Nevertheless, we
 * assume (optimistically) that the link hasn't been severed and
 * that maybe we can get to the device anyhow.
 */
static int sas_do_lu_discovery(struct domain_device *dev)
{
	int  res;
	u8  *buffer;
	int  size;

	res = sas_get_report_luns(dev, &buffer, &size);
	if (res) {
		SAS_DPRINTK("dev %llx didn't reply to REPORT LUNS, trying "
			    "LUN 0 anyway\n",
			    SAS_ADDR(dev->sas_addr));
		size = 16;
		buffer = kzalloc(size, GFP_KERNEL);
	}

	res = sas_register_lu(dev, buffer, size);
	if (res) {
		SAS_DPRINTK("dev %llx didn't report any LUs\n",
			    SAS_ADDR(dev->sas_addr));
		res = 0;
	}

	kfree(buffer);
	return res;
}

> won't have any quirks, but I'd like to have this proven in the field
> before I take it as read.

Indeed, it doesn't have quirks (because there are none yet out there).

Let's include it _without_ quirks, and add them as new devices come
to the market, instead of crudding the code unnecessarily.  This gives
more freedom of future "patching" and future "design".

The reason is that if all you have is the SAS/SATA chip, then you do not
need all the legacy quirks, code bloat, bigger binary.

> I'm sure you're aware that not responding on either LUN 0 or the report
> luns WLUN is a violation of SAM  ... however, if we really already have

I am. I've repeated this about 100 million times on this list,
and it is in the SAS code comments at sas_do_lu_discovery().

> broken SAS devices with this problem, then you're welcome to expand
> scsi_scan_target() to cope.

I don't have a problem working on scsi_scan_target().
But I have a problem with it using _HCIL_.

What do you suggest?

> WLUN support is really the only piece that scsi_scan_target() doesn't
> currently possess and, as has been discussed before, that only really

-------------------------------------------------------------------------
> matters if we get a target that's not going to respond to an INQUIRY on
> LUN0 (I expect most of them, even if they have no LUN0 will respond with
> PQ 3 to the inquiry and thus be caught by scsi_scan_target() anyway).
-------------------------------------------------------------------------

I think you clarify your point below but as far as SAM-4 and
thus SAS devices are concerned:

REPORT LUNS is sent first, to figure out what LUs are contained
in the SCSI device, and then for each LU, send an INQUIRY to the LU.
>From the code:

static int sas_register_lu(struct domain_device *dev, u8 *buf, int size)
{
	int res;

	for (buf = buf+8, size -= 8; size > 0; size -= 8, buf += 8) {
		struct LU *lu = sas_alloc_lu();

		SAS_DPRINTK("%016llx probing LUN:%016llx\n",
			    SAS_ADDR(dev->sas_addr),
			    be64_to_cpu(*(__be64 *)buf));
		if (lu) {
			lu->parent = dev;
			memcpy(lu->LUN, buf, 8);
			res = sas_get_inquiry(lu);
			if (res) {
				SAS_DPRINTK("dev %llx LUN %016llx didn't reply"
					    " to INQUIRY, forgotten\n",
					    SAS_ADDR(dev->sas_addr),
					    SAS_ADDR(lu->LUN));
				kfree(lu);
				continue;
			}
			lu->lu_obj.kset = &dev->end_dev.LU_kset;
			kobject_set_name(&lu->lu_obj, "%016llx",
					 SAS_ADDR(lu->LUN));
			lu->lu_obj.ktype = dev->end_dev.LU_kset.ktype;
			list_add_tail(&lu->list, &dev->end_dev.LU_list);
		}
	}

	return list_empty(&dev->end_dev.LU_list) ? -ENODEV : 0;
}

> However, to add WLUN support to scsi_scan_target() looks pretty simple:
> If the transport supports > 256 LUNs and the initial INQUIRY comes back
> SCSI_SCAN_NO_RESPONSE then fire off a WLUN report lun scan anyway.  By
> all means, submit a patch to do this.

I did take a look and can see "transport" calls right in SCSI Core.
Firthermore the "transport" calls are HCIL, for example:
	shost->transportt->target_parent(shost, channel, id);

"If the transport supports > 256 LUNs" -- as James S hints,
it maybe the case that the transport does, but the device doesn't
or vice versa.  Add to this the shaky REPORT LUN support for legacy
devices.

Also the order of operations... "initial INQUIRY comes back
SCSI_SCAN_NO_RESPONSE then fire off a WLUN report lun scan anyway".

When in fact REPORT LUNS should be the first thing sent, then if that
fails we assume that LUN 0 is present anyway as you can see I've done
in sas_do_lu_discovery() above.

So it gets really messy if you can see what I mean.

>>P.S. REPORT LUNS is Mandatory as per SPC, so newer devices
>>(SAS) support it.  Furthermore if their LUs are sparse they
>>(really) support REPORT LUNS.
> 
> 
> Actually, it's optional per SPC; it's mandatory in SPC-2 but *only* if
> your device is multi-LUN; and it finally became mandatory for everything
> in SPC-3 (or I should say "becomes" since SPC-3 isn't ratified yet).

"Actually" when I say SPC, I mean SPC-latest.  When I say SAM, I mean
SAM-latest, when I say SCSI, I mean SCSI-3 which T10 says means SCSI.

I don't see your point in "correcting" me, while SCSI Core uses
HCIL extensively.

	Luben
http://linux.adaptec.com/sas/
Disclaimer: Opinions stated in this email are my own, not of my employer.


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

* Re: [PATCH] 3ware: use scsi_scan_target()
  2005-10-07 21:36         ` Luben Tuikov
@ 2005-10-08 14:30           ` James Bottomley
  2005-10-09 16:25             ` Luben Tuikov
  0 siblings, 1 reply; 15+ messages in thread
From: James Bottomley @ 2005-10-08 14:30 UTC (permalink / raw)
  To: Luben Tuikov
  Cc: ltuikov, Jeff Garzik, Christoph Hellwig, SCSI Mailing List,
	linuxraid

On Fri, 2005-10-07 at 17:36 -0400, Luben Tuikov wrote: 
> On 10/06/05 21:07, James Bottomley wrote:
> > 1. The domain device as you have implemented it requires a lot of
> 
> The domain device as I've implemented it, is exactly a SAS domain
> device, supporting expanders, SAS, SATA, SATA PM, etc. devices.  It
> doesn't add any cruft, it is the minimum of what it should be.

But it doesn't represent a SCSI domain; it represents a particular type
of SCSI domain (as you say yourself, SAS or SATA).  I'm trying to eject
transport specific knowledge from the mid-layer, so a domain device that
would be used in the mid-layer should be capable of representing any
SCSI domain (FC/SPI/SBP etc ..).  I suppose in the worst case, anything
that comes back to the mid-layer from the transports should be relevant
to at least two separate transports.

> I'm not sure that you can "refine" (take stuff out) from
> struct domain_device.
> 
> You're better off creating struct scsi_domain_device { ... }; and starting
> from there.
> 
> In effect struct scsi_domain_device { ... }; (non existant yet)
> would be a superclass (in OO sense) of the SAS struct domain_device, shown
> above.

Exactly ... the transport independent abstraction of a domain device.

> In contrast to "templates", the OO sense can be kept _by design_,
> by _SCSI_ design, not by code enforcement (i.e. struct template_abcd...).
> 
> > 2. sas_do_lu_discovery() duplicates a lot of existing functionality, but
> > also lacks a lot of quirk processing.  I know the argument is that SAS
> 
> It doesn't duplicate a lot of functionality.  While the code
> in SCSI Core does do LU discovery, the infrastructure is very broken
> in that
>   1) there is no native "target" (i.e. SCSI Device with Target port)
>      support (struct domain_device), and
>   2) that code uses HCIL, by scsi_scan_target definition and from
>      seeing things like:
> 
> 	if (shost->this_id == id)
> 		/*
> 		 * Don't scan the host adapter
> 		 */
> 		return;
> 
> As to SAS, the comment is about the size of the function:
[...]

> > won't have any quirks, but I'd like to have this proven in the field
> > before I take it as read.
> 
> Indeed, it doesn't have quirks (because there are none yet out there).
> 
> Let's include it _without_ quirks, and add them as new devices come
> to the market, instead of crudding the code unnecessarily.  This gives
> more freedom of future "patching" and future "design".
> 
> The reason is that if all you have is the SAS/SATA chip, then you do not
> need all the legacy quirks, code bloat, bigger binary.

That's why I want SAS to prove itself to be quirk free.  If we have to
add them, then all we get is a parallel version of scsi_scan_target with
a parallel quirk table ... which is highly undesirable.

> > I'm sure you're aware that not responding on either LUN 0 or the report
> > luns WLUN is a violation of SAM  ... however, if we really already have
> 
> I am. I've repeated this about 100 million times on this list,
> and it is in the SAS code comments at sas_do_lu_discovery().
> 
> > broken SAS devices with this problem, then you're welcome to expand
> > scsi_scan_target() to cope.
> 
> I don't have a problem working on scsi_scan_target().
> But I have a problem with it using _HCIL_.
> 
> What do you suggest?

Let Jeff come up with the incorporation scheme and see how it looks.

> > WLUN support is really the only piece that scsi_scan_target() doesn't
> > currently possess and, as has been discussed before, that only really
> 
> -------------------------------------------------------------------------
> > matters if we get a target that's not going to respond to an INQUIRY on
> > LUN0 (I expect most of them, even if they have no LUN0 will respond with
> > PQ 3 to the inquiry and thus be caught by scsi_scan_target() anyway).
> -------------------------------------------------------------------------
> 
> I think you clarify your point below but as far as SAM-4 and
> thus SAS devices are concerned:
> 
> REPORT LUNS is sent first, to figure out what LUs are contained
> in the SCSI device, and then for each LU, send an INQUIRY to the LU.
> >From the code:

I know.  But, in the presence of quirks, this is wrong.  We need an
inquiry first so we have something to look up in the quirk capabilities
table.  report luns doesn't return enough information ... and since one
of the quirks is that report luns returns incorrect information ...

> Also the order of operations... "initial INQUIRY comes back
> SCSI_SCAN_NO_RESPONSE then fire off a WLUN report lun scan anyway".
> 
> When in fact REPORT LUNS should be the first thing sent, then if that
> fails we assume that LUN 0 is present anyway as you can see I've done
> in sas_do_lu_discovery() above.

Nothing in the standard mandates exactly how discovery be done.  But for
the quirk reason outlined above, inquiry has to be first.

> So it gets really messy if you can see what I mean.
> 
> >>P.S. REPORT LUNS is Mandatory as per SPC, so newer devices
> >>(SAS) support it.  Furthermore if their LUs are sparse they
> >>(really) support REPORT LUNS.
> > 
> > 
> > Actually, it's optional per SPC; it's mandatory in SPC-2 but *only* if
> > your device is multi-LUN; and it finally became mandatory for everything
> > in SPC-3 (or I should say "becomes" since SPC-3 isn't ratified yet).
> 
> "Actually" when I say SPC, I mean SPC-latest.  When I say SAM, I mean
> SAM-latest, when I say SCSI, I mean SCSI-3 which T10 says means SCSI.

It takes time for standards to bubble into the field.  There are still
manufacturers today producing devices claiming scsi-2 compliance (I
suspect they tried scsi-3 found a problem in testing and "corrected" it
by dropping back to scsi-2, but they're still rolling off the production
lines).  Of all the arrays I see certified in my day job, one claims
SCSI-2, one claims SPC-3 and the rest seem to have a 50/50 split between
SPC and SPC-2.  So, fully 50% of these devices are not required to
support report LUNs; if they were single LUN devices, then the vast
majority wouldn't be required to support it.

The point to all this is that the version of the standard matters a lot,
especially where the actual standards differ from version to version
because we have to provide support for what the devices were built to,
not what the latest standard says.

James



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

* Re: [PATCH] 3ware: use scsi_scan_target()
  2005-10-08 14:30           ` James Bottomley
@ 2005-10-09 16:25             ` Luben Tuikov
  2005-10-10  5:05               ` Mike Anderson
  0 siblings, 1 reply; 15+ messages in thread
From: Luben Tuikov @ 2005-10-09 16:25 UTC (permalink / raw)
  To: James Bottomley
  Cc: ltuikov, Jeff Garzik, Christoph Hellwig, SCSI Mailing List,
	linuxraid

On 10/08/05 10:30, James Bottomley wrote:
> But it doesn't represent a SCSI domain; it represents a particular type
> of SCSI domain (as you say yourself, SAS or SATA).  I'm trying to eject

That's true.

> transport specific knowledge from the mid-layer, so a domain device that
> would be used in the mid-layer should be capable of representing any
> SCSI domain (FC/SPI/SBP etc ..).  I suppose in the worst case, anything
> that comes back to the mid-layer from the transports should be relevant
> to at least two separate transports.

struct scsi_domain_device { ... }; (to be created) is your friend.

The only way that that design
	"should be capable of representing any 
	 SCSI domain (FC/SPI/SBP etc ..)"

Is if it _does not_ have any knowlege about the underlying
physical domain -- just as it is shown in SAM (and that is the whole point).
Else you get in this neverending cat-and-mouse game.  If you have the
abstraction right, then whatever new transport comes along, it would
be properly represented.

> That's why I want SAS to prove itself to be quirk free.  If we have to
> add them, then all we get is a parallel version of scsi_scan_target with
> a parallel quirk table ... which is highly undesirable.

Well, this way we get into the chicken and the egg problem.

Pessimistically: "Yeah, SAS will have all the same problems, so might
as well crud it up good, let's kill it from the get go."

Optimistically: "No, SAS will not have the same problems.  FW writers
have gotten smarter, they've read SAS and SAM at least once.  I'll
give them the benefit of the doubt."

Naturally (as an engineer), I'm on optimist, plus this is what
I've observed with beta and release FW.

> Let Jeff come up with the incorporation scheme and see how it looks.

Hmm, I haven't seen or heard anything from Jeff.  Have you?

I have no idea what his plans are.  If the idea is to create
struct scsi_domain_device { ... }; and start from there,
then I'd like to be involved since this was what I'd wanted
for SCSI since 2002.

> I know.  But, in the presence of quirks, this is wrong.  We need an
> inquiry first so we have something to look up in the quirk capabilities
> table.  report luns doesn't return enough information ... and since one
> of the quirks is that report luns returns incorrect information ...

Yes, it gets very messy.

On a different note: I'd love to _buy_ a device which returns
_incorrect_ REPORT LUNS.  Who hires those people?  (Or do they
get the job because of politics? ;-) )

> Nothing in the standard mandates exactly how discovery be done.  But for
> the quirk reason outlined above, inquiry has to be first.

That's true.  But think about it: you've just discovered
a device on the SCSI Domain (using protocol) and you don't know
anything about it (as stated in my code's comments).  The first
thing you want to do is REPORT LUNS to figure out how many
LUs are there.  LUs in a SCSI Device do not necessarily have to
be homogeneous.

As to quirks, I guess you're right.

But SCSI Core has come to a breaking point: being so SPI-centric
and outdated, full of quirks and legacy stuff.

SCSI Core has a chance now with SAS to become what it had
needed to start becoming in 2002: native target support,
64 bit LUNs, etc.

Note: when I say "native" target support, I do _not_ mean
"struct scsi_target", but "struct scsi_domain_device" (to be
created).  That is, the "native" indicates that the "target",
"SCSI Device", can only be addressed by the transport --
SCSI Core doesn't not know how to address the device (only
LUs).  All the while, struct scsi_target has HCIL in it.

> It takes time for standards to bubble into the field.  There are still
> manufacturers today producing devices claiming scsi-2 compliance (I
> suspect they tried scsi-3 found a problem in testing and "corrected" it
> by dropping back to scsi-2, but they're still rolling off the production
> lines).  Of all the arrays I see certified in my day job, one claims
> SCSI-2, one claims SPC-3 and the rest seem to have a 50/50 split between
> SPC and SPC-2.  So, fully 50% of these devices are not required to
> support report LUNs; if they were single LUN devices, then the vast
> majority wouldn't be required to support it.
> 
> The point to all this is that the version of the standard matters a lot,
> especially where the actual standards differ from version to version
> because we have to provide support for what the devices were built to,
> not what the latest standard says.

Well, this sounds good, but I'd say give SAS a chance, let's be optimistic.

	Luben
-- 
http://linux.adaptec.com/sas/
Disclaimer: Opinions stated in this email are my own, not of my employer.
For inquiries write to: luben_tuikov@adaptec.com or ltuikov@yahoo.com.

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

* Re: [PATCH] 3ware: use scsi_scan_target()
  2005-10-09 16:25             ` Luben Tuikov
@ 2005-10-10  5:05               ` Mike Anderson
  2005-10-14 16:19                 ` Luben Tuikov
  0 siblings, 1 reply; 15+ messages in thread
From: Mike Anderson @ 2005-10-10  5:05 UTC (permalink / raw)
  To: Luben Tuikov
  Cc: James Bottomley, ltuikov, Jeff Garzik, Christoph Hellwig,
	SCSI Mailing List, linuxraid

Luben Tuikov <luben_tuikov@adaptec.com> wrote:
> On 10/08/05 10:30, James Bottomley wrote:
> > But it doesn't represent a SCSI domain; it represents a particular type
> > of SCSI domain (as you say yourself, SAS or SATA).  I'm trying to eject
> 
> That's true.
> 
> > transport specific knowledge from the mid-layer, so a domain device that
> > would be used in the mid-layer should be capable of representing any
> > SCSI domain (FC/SPI/SBP etc ..).  I suppose in the worst case, anything
> > that comes back to the mid-layer from the transports should be relevant
> > to at least two separate transports.
> 
> struct scsi_domain_device { ... }; (to be created) is your friend.
> 
> The only way that that design
> 	"should be capable of representing any 
> 	 SCSI domain (FC/SPI/SBP etc ..)"
> 
> Is if it _does not_ have any knowlege about the underlying
> physical domain -- just as it is shown in SAM (and that is the whole point).
> Else you get in this neverending cat-and-mouse game.  If you have the
> abstraction right, then whatever new transport comes along, it would
> be properly represented.
> 
> > Let Jeff come up with the incorporation scheme and see how it looks.
> 
> Hmm, I haven't seen or heard anything from Jeff.  Have you?
> 
> I have no idea what his plans are.  If the idea is to create
> struct scsi_domain_device { ... }; and start from there,
> then I'd like to be involved since this was what I'd wanted
> for SCSI since 2002.
> 

I would also be interested in the incorporation work. I have been running
Luben's patches on a couple of x460s. I was also reading, hacking, and
experimenting on the two sas classes to understand them better and would
like to help if I can.

-andmike
--
Michael Anderson
andmike@us.ibm.com

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

* Re: [PATCH] 3ware: use scsi_scan_target()
  2005-10-10  5:05               ` Mike Anderson
@ 2005-10-14 16:19                 ` Luben Tuikov
  0 siblings, 0 replies; 15+ messages in thread
From: Luben Tuikov @ 2005-10-14 16:19 UTC (permalink / raw)
  To: Mike Anderson, Luben Tuikov
  Cc: James Bottomley, ltuikov, Jeff Garzik, Christoph Hellwig,
	SCSI Mailing List, linuxraid

--- Mike Anderson <andmike@us.ibm.com> wrote:

> I would also be interested in the incorporation work. I have been running
> Luben's patches on a couple of x460s. I was also reading, hacking, and
> experimenting on the two sas classes to understand them better and would
> like to help if I can.

Sounds good.

I'll update the code when I come back.

Luben
-- 
http://linux.adaptec.com/sas/
Opinion expressed are my own, not my empoyer's or otherwise.
For inquiries write to ltuikov@yahoo.com.




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

end of thread, other threads:[~2005-10-14 16:19 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-10-06 13:13 [PATCH] 3ware: use scsi_scan_target() James.Smart
2005-10-06 18:09 ` Luben Tuikov
  -- strict thread matches above, loose matches on Subject: below --
2005-10-04  0:57 Jeff Garzik
2005-10-05 16:28 ` Christoph Hellwig
2005-10-05 19:01   ` Luben Tuikov
2005-10-05 19:20     ` adam radford
2005-10-05 19:24     ` Jeff Garzik
2005-10-05 19:34   ` Jeff Garzik
2005-10-05 23:19     ` Luben Tuikov
2005-10-07  1:07       ` James Bottomley
2005-10-07 21:36         ` Luben Tuikov
2005-10-08 14:30           ` James Bottomley
2005-10-09 16:25             ` Luben Tuikov
2005-10-10  5:05               ` Mike Anderson
2005-10-14 16:19                 ` Luben Tuikov

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