3ware: use scsi_scan_target()

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.

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

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

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.

I agree.

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

Luben

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

Some 9xxx firmwares do support LUNs:

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

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

[other emails appear to have addressed this]

Jeff

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.

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.

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.

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

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

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

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

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

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

What do you suggest?

-------------------------------------------------------------------------

-------------------------------------------------------------------------

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.

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

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.

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

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.

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

[...]

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.

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

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

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

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

That's true.

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.

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.

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.

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? ;-) )

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.

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: [email protected] or [email protected].

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
[email protected]

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 [email protected].