Re: [PATCH] scsi_transport_sas: make minimum and maximum linkrate settable quantities

Re: [PATCH] scsi_transport_sas: make minimum and maximum linkrate settable quantities

[Douglas Gilbert]

I meant to cc this to the list ...


James Bottomley wrote:
> On Wed, 2006-09-06 at 23:38 -0400, Douglas Gilbert wrote:
>> James Bottomley wrote:
>>> According to SPEC, the minimum_linkrate and maximum_linkrate should be
>>                         ^                    ^
>>                     programmable       programmable
>>> settable by the user.  This patch introduces a callback that allows the
>>> sas class to pass these settings on to the driver.
>> There are also "hardware" variants of min/max_linkrate.
> 
> hardware rates represent the max and min capabilities of the hw ...
> they're not settable.
> 
>> Hmmm. I get a bit confused reading the above. Are you talking
>> about a SAS initiator that this machine controls (i.e. a "host"),
>> an expander phy in a visible SAS domain, a visible SSP target
>> (e.g. a SAS disk) and/or a visible SATA device? All of them?
>> Will all levels (i.e. initiator, expander and targets) have
>> these fields (if they exist) represented by an attribute
>> in sysfs?
> 
> I mean for all of the phys that are visible in the sysfs representation
> of the domain. Non expander end device phys aren't visible in our
> representation. 

I'm still confused. An attached phy either identifies itself
as an expander phy or an end device phy. So what is
the difference between a "non expander end device phy"
and a "end device phy"? [There can be SAS (virtual) phys
within an expander (e.g. associated with an enclosure
service target) but each SAS virtual phy is paired with
an expander virtual phy.]

Dropping the double negative I read your last sentence as
"Only expander phys are visible in our representation."

> However, this is irrelevant really, since it takes two
> phys to make a link, so if you force one side to a specific rate, the
> other must follow or fail the link negotiation.

True, as long as a phy at one end of the link allows
its programmable linkrates to be changed and a
link or hard reset to be instigated.

>> The SATA device is easy to rule out (as is a SATA port selector).
>> There may be some way of doing linkrate changes via the
>> SATA command set.
>>
>> SSP targets (e.g. SAS disks) have the "phy control and discover"
>> mode subpage. The SAS disks that I have examined (all from
>> one manufacturer) do not allow the programmable min/max_linkrate
>> fields to be changed by the user (i.e. the changeable
>> mode page bits are all zero). Worse still, their product
>> manuals say that both ports (phys) will negotiate the
>> same linkrate which will be the linkrate first attained.
>> Looking at the spec (sas2r05b section 10.2.7.2.3), it just
>> says all _other_ fields in that mode subpage shall not be
>> changeable. So your "should be settable" statement above
>> is a bit strong. How about "might be settable for SSP
>> targets".
> 
> The same one end follows the other should pertain even to SSP targets.
> I can't validate this, though, since I don't have any SATA-2 disks
> (SATA-1 can't negotiate speed, being fixed at 1.5GB).

I can connect a SATA disk that claims to do 3 Gbps
(Seagate 7200.9 series) directly to a LSIL HBA phy
and I can see:
# cat /sys/class/sas_phy/phy-4:0/negotiated_linkrate
3.0 Gbit

My 48300 (aic94xx based) HBA says in its BIOS that it
negotiates with all SATA disks at 1.5 Gbps. I believe
the SASx12 expander I have has the same restriction.

<snip>

>> Also folks playing with these fields should realize that
>> link reset is the appropriate type of reset to issue to
>> cause a linkrate change. A hard reset will do it as well
>> but may cause additional collateral damage. For example,
>> if any phy on a SAS target device does a hard reset
>> sequence, then all LUs within that target device receive
>> a LU reset: clearing out all SCSI commands in progress and
>> those pending. The idea with link reset is that only the link
>> layers and below are impacted, as longer of the renegotiation
>> succeeds in a timely fashion. Setting the prog_max_linkrate
>> to 1.5 Gbps at one end of physical link while setting the
>> prog_min_linkrate to 3 Gbps at the other end will cause
>> the renegotiation following a link reset to fail.
> 
> Yes, the setting algorithms do a link reset, if you look at the code ...
> and there's logic to prevent silly things like min going above max.

However the user may not want a link reset sent immediately.
For example, in SAS-2 with 6 Gbps available, to force 3 Gbps
the min_linkrate needs to be increased to 3 Gbps and the
max_linkrate needs to be decreased to 3 Gbps, then a reset
sent. It is also possible that the user may want a
(expander) phy's next negotiated link_rate changed but
the phy left disabled.


Building SAS discovery into the kernel SAS transport layer
might be a necessity now but it could cause problems down the
track. Consideration should be given to having a switch to
turn it off. SAS discovery in SAS-1 and SAS-1.1 does not
scale well with every SAS management client (i.e. the SAS
kernel transport layer) thinking that it has to do a full
or partial discovery of a potentially large SAS domain
every time a BROADCAST(CHANGE) event is received.
[BTW a link reset causes a BROADCAST(CHANGE) event so sending
more link resets than necessary will impact performance.]

In SAS-2 there are several improvements:
  - higher level SMP commands that move more data
    in one function (e.g. DISCOVER LIST)
  - much smarter expanders that can
     - self config and configure other (older) expanders
     - indicate to management clients when the are finished
     - more efficient routing tables (expander based rather
       than phy based)
     - support expander based zoning which will control the
       accessibility of end devices to one another.

Together these changes could relieve the burden of the SAS
discovery process on the SAS kernel transport layer. However
there seems nothing to stop such a kernel layer ploughing
ahead and doing discovery in the old inefficient way
making the worst case assumptions.

As ever finding the boot disk and root file system could be
a problem, but hopefully folks will soon be using flash
cards for that!

Doug Gilbert

Re: [PATCH] scsi_transport_sas: make minimum and maximum linkrate settable quantities

[James Bottomley]

On Thu, 2006-09-07 at 11:56 -0400, Douglas Gilbert wrote:
> Dropping the double negative I read your last sentence as
> "Only expander phys are visible in our representation."

Only expander and host phys are visible in the sysfs representation.

> However the user may not want a link reset sent immediately.
> For example, in SAS-2 with 6 Gbps available, to force 3 Gbps
> the min_linkrate needs to be increased to 3 Gbps and the
> max_linkrate needs to be decreased to 3 Gbps, then a reset
> sent. It is also possible that the user may want a
> (expander) phy's next negotiated link_rate changed but
> the phy left disabled.

A link reset is merely supposed to renegotiate ... it's not supposed to
be disruptive


> Building SAS discovery into the kernel SAS transport layer
> might be a necessity now but it could cause problems down the
> track. Consideration should be given to having a switch to
> turn it off. SAS discovery in SAS-1 and SAS-1.1 does not
> scale well with every SAS management client (i.e. the SAS
> kernel transport layer) thinking that it has to do a full
> or partial discovery of a potentially large SAS domain

We already have this, but it's optional: aic94xx uses it, mptsas
doesn't.

> every time a BROADCAST(CHANGE) event is received.
> [BTW a link reset causes a BROADCAST(CHANGE) event so sending
> more link resets than necessary will impact performance.]

Yes, these are actually used to update the phy parameters (they trigger
SMP probes to find out what changed).


> In SAS-2 there are several improvements:
>   - higher level SMP commands that move more data
>     in one function (e.g. DISCOVER LIST)
>   - much smarter expanders that can
>      - self config and configure other (older) expanders
>      - indicate to management clients when the are finished
>      - more efficient routing tables (expander based rather
>        than phy based)
>      - support expander based zoning which will control the
>        accessibility of end devices to one another.
> 
> Together these changes could relieve the burden of the SAS
> discovery process on the SAS kernel transport layer. However
> there seems nothing to stop such a kernel layer ploughing
> ahead and doing discovery in the old inefficient way
> making the worst case assumptions.

I'm not really planning to do anything about the new SAS-2 features
until I see expanders which support them.  The current discovery doesn't
reprogram an expander, it queries the route table to see if it's usable
as is and only modifies it if it isn't.  It can also support table to
table routing, so it should support a large chunk of the new features
anyway.

James

[PATCH] scsi_transport_sas: make minimum and maximum linkrate settable quantities

[James Bottomley]

According to SPEC, the minimum_linkrate and maximum_linkrate should be
settable by the user.  This patch introduces a callback that allows the
sas class to pass these settings on to the driver.

James

Index: BUILD-2.6/drivers/scsi/scsi_transport_sas.c
===================================================================
--- BUILD-2.6.orig/drivers/scsi/scsi_transport_sas.c	2006-09-06 16:32:11.000000000 -0500
+++ BUILD-2.6/drivers/scsi/scsi_transport_sas.c	2006-09-06 17:51:15.000000000 -0500
@@ -77,6 +77,24 @@ get_sas_##title##_names(u32 table_key, c
 	return len;						\
 }
 
+#define sas_bitfield_name_set(title, table)			\
+static ssize_t							\
+set_sas_##title##_names(u32 *table_key, const char *buf)	\
+{								\
+	ssize_t len = 0;					\
+	int i;							\
+								\
+	for (i = 0; i < ARRAY_SIZE(table); i++) {		\
+		len = strlen(table[i].name);			\
+		if (strncmp(buf, table[i].name, len) == 0 &&	\
+		    (buf[len] == '\n' || buf[len] == '\0')) {	\
+			*table_key = table[i].value;		\
+			return 0;				\
+		}						\
+	}							\
+	return -EINVAL;						\
+}
+
 #define sas_bitfield_name_search(title, table)			\
 static ssize_t							\
 get_sas_##title##_names(u32 table_key, char *buf)		\
@@ -131,7 +149,7 @@ static struct {
 	{ SAS_LINK_RATE_6_0_GBPS,	"6.0 Gbit" },
 };
 sas_bitfield_name_search(linkspeed, sas_linkspeed_names)
-
+sas_bitfield_name_set(linkspeed, sas_linkspeed_names)
 
 /*
  * SAS host attributes
@@ -253,10 +271,39 @@ show_sas_phy_##field(struct class_device
 	return get_sas_linkspeed_names(phy->field, buf);		\
 }
 
+/* Fudge to tell if we're minimum or maximum */
+#define sas_phy_store_linkspeed(field)					\
+static ssize_t								\
+store_sas_phy_##field(struct class_device *cdev, const char *buf,	\
+		      size_t count)					\
+{									\
+	struct sas_phy *phy = transport_class_to_phy(cdev);		\
+	struct Scsi_Host *shost = dev_to_shost(phy->dev.parent);	\
+	struct sas_internal *i = to_sas_internal(shost->transportt);	\
+	u32 value;							\
+	struct sas_phy_linkrates rates = {0};				\
+	int error;							\
+									\
+	error = set_sas_linkspeed_names(&value, buf);			\
+	if (error)							\
+		return error;						\
+	rates.field = value;						\
+	error = i->f->set_phy_speed(phy, &rates);			\
+									\
+	return error ? error : count;					\
+}
+
+#define sas_phy_linkspeed_rw_attr(field)				\
+	sas_phy_show_linkspeed(field)					\
+	sas_phy_store_linkspeed(field)					\
+static CLASS_DEVICE_ATTR(field, S_IRUGO, show_sas_phy_##field,		\
+	store_sas_phy_##field)
+
 #define sas_phy_linkspeed_attr(field)					\
 	sas_phy_show_linkspeed(field)					\
 static CLASS_DEVICE_ATTR(field, S_IRUGO, show_sas_phy_##field, NULL)
 
+
 #define sas_phy_show_linkerror(field)					\
 static ssize_t								\
 show_sas_phy_##field(struct class_device *cdev, char *buf)		\
@@ -326,9 +373,9 @@ sas_phy_simple_attr(identify.phy_identif
 //sas_phy_simple_attr(port_identifier, port_identifier, "%d\n", int);
 sas_phy_linkspeed_attr(negotiated_linkrate);
 sas_phy_linkspeed_attr(minimum_linkrate_hw);
-sas_phy_linkspeed_attr(minimum_linkrate);
+sas_phy_linkspeed_rw_attr(minimum_linkrate);
 sas_phy_linkspeed_attr(maximum_linkrate_hw);
-sas_phy_linkspeed_attr(maximum_linkrate);
+sas_phy_linkspeed_rw_attr(maximum_linkrate);
 sas_phy_linkerror_attr(invalid_dword_count);
 sas_phy_linkerror_attr(running_disparity_error_count);
 sas_phy_linkerror_attr(loss_of_dword_sync_count);
@@ -1310,13 +1357,23 @@ static int sas_user_scan(struct Scsi_Hos
  * Setup / Teardown code
  */
 
-#define SETUP_TEMPLATE(attrb, field, perm, test)				\
+#define SETUP_TEMPLATE(attrb, field, perm, test)			\
 	i->private_##attrb[count] = class_device_attr_##field;		\
 	i->private_##attrb[count].attr.mode = perm;			\
 	i->attrb[count] = &i->private_##attrb[count];			\
 	if (test)							\
 		count++
 
+#define SETUP_TEMPLATE_RW(attrb, field, perm, test, ro_test, ro_perm)	\
+	i->private_##attrb[count] = class_device_attr_##field;		\
+	i->private_##attrb[count].attr.mode = perm;			\
+	if (ro_test) {							\
+		i->private_##attrb[count].attr.mode = ro_perm;		\
+		i->private_##attrb[count].store = NULL;			\
+	}								\
+	i->attrb[count] = &i->private_##attrb[count];			\
+	if (test)							\
+		count++
 
 #define SETUP_RPORT_ATTRIBUTE(field) 					\
 	SETUP_TEMPLATE(rphy_attrs, field, S_IRUGO, 1)
@@ -1327,6 +1384,10 @@ static int sas_user_scan(struct Scsi_Hos
 #define SETUP_PHY_ATTRIBUTE(field)					\
 	SETUP_TEMPLATE(phy_attrs, field, S_IRUGO, 1)
 
+#define SETUP_PHY_ATTRIBUTE_RW(field)					\
+	SETUP_TEMPLATE_RW(phy_attrs, field, S_IRUGO | S_IWUSR, 1,	\
+			!i->f->set_phy_speed, S_IRUGO)
+
 #define SETUP_PORT_ATTRIBUTE(field)					\
 	SETUP_TEMPLATE(port_attrs, field, S_IRUGO, 1)
 
@@ -1407,9 +1468,9 @@ sas_attach_transport(struct sas_function
 	//SETUP_PHY_ATTRIBUTE(port_identifier);
 	SETUP_PHY_ATTRIBUTE(negotiated_linkrate);
 	SETUP_PHY_ATTRIBUTE(minimum_linkrate_hw);
-	SETUP_PHY_ATTRIBUTE(minimum_linkrate);
+	SETUP_PHY_ATTRIBUTE_RW(minimum_linkrate);
 	SETUP_PHY_ATTRIBUTE(maximum_linkrate_hw);
-	SETUP_PHY_ATTRIBUTE(maximum_linkrate);
+	SETUP_PHY_ATTRIBUTE_RW(maximum_linkrate);
 
 	SETUP_PHY_ATTRIBUTE(invalid_dword_count);
 	SETUP_PHY_ATTRIBUTE(running_disparity_error_count);
Index: BUILD-2.6/include/scsi/scsi_transport_sas.h
===================================================================
--- BUILD-2.6.orig/include/scsi/scsi_transport_sas.h	2006-09-06 16:32:11.000000000 -0500
+++ BUILD-2.6/include/scsi/scsi_transport_sas.h	2006-09-06 17:48:02.000000000 -0500
@@ -150,12 +150,18 @@ struct sas_port {
 #define transport_class_to_sas_port(cdev) \
 	dev_to_sas_port((cdev)->dev)
 
+struct sas_phy_linkrates {
+	enum sas_linkrate maximum_linkrate;
+	enum sas_linkrate minimum_linkrate;
+};
+
 /* The functions by which the transport class and the driver communicate */
 struct sas_function_template {
 	int (*get_linkerrors)(struct sas_phy *);
 	int (*get_enclosure_identifier)(struct sas_rphy *, u64 *);
 	int (*get_bay_identifier)(struct sas_rphy *);
 	int (*phy_reset)(struct sas_phy *, int);
+	int (*set_phy_speed)(struct sas_phy *, struct sas_phy_linkrates *);
 };
 
 

Re: [PATCH] scsi_transport_sas: make minimum and maximum linkrate settable quantities

[Douglas Gilbert]

James Bottomley wrote:
> According to SPEC, the minimum_linkrate and maximum_linkrate should be
                        ^                    ^
                    programmable       programmable
> settable by the user.  This patch introduces a callback that allows the
> sas class to pass these settings on to the driver.

There are also "hardware" variants of min/max_linkrate.

Hmmm. I get a bit confused reading the above. Are you talking
about a SAS initiator that this machine controls (i.e. a "host"),
an expander phy in a visible SAS domain, a visible SSP target
(e.g. a SAS disk) and/or a visible SATA device? All of them?
Will all levels (i.e. initiator, expander and targets) have
these fields (if they exist) represented by an attribute
in sysfs?

The SATA device is easy to rule out (as is a SATA port selector).
There may be some way of doing linkrate changes via the
SATA command set.

SSP targets (e.g. SAS disks) have the "phy control and discover"
mode subpage. The SAS disks that I have examined (all from
one manufacturer) do not allow the programmable min/max_linkrate
fields to be changed by the user (i.e. the changeable
mode page bits are all zero). Worse still, their product
manuals say that both ports (phys) will negotiate the
same linkrate which will be the linkrate first attained.
Looking at the spec (sas2r05b section 10.2.7.2.3), it just
says all _other_ fields in that mode subpage shall not be
changeable. So your "should be settable" statement above
is a bit strong. How about "might be settable for SSP
targets".

As for expanders, the PHY CONTROL SMP function allows
those fields to be changed on a phy and the spec implies
those changes shall be obeyed, if they are still in place
when the next link/bus reset occurs. However the spec says
that function "may be implemented" so it is not required.

The SAS HBAs (initiators) that I have fought with allow
those fields to be changed. Being able to make a HBA
initiator phy do a link or hard reset required correspondence
with the LLD maintainers, in my experience :-)


Another thing worth pointing out is that the negotiated
physical link rate only applies to one point to point
physical link. The link rate can be changed by altering
these fields associated with the phy at either end of the
physical link, and then doing a link (or hard) reset from
either end to force a renegotiation.

Also folks playing with these fields should realize that
link reset is the appropriate type of reset to issue to
cause a linkrate change. A hard reset will do it as well
but may cause additional collateral damage. For example,
if any phy on a SAS target device does a hard reset
sequence, then all LUs within that target device receive
a LU reset: clearing out all SCSI commands in progress and
those pending. The idea with link reset is that only the link
layers and below are impacted, as longer of the renegotiation
succeeds in a timely fashion. Setting the prog_max_linkrate
to 1.5 Gbps at one end of physical link while setting the
prog_min_linkrate to 3 Gbps at the other end will cause
the renegotiation following a link reset to fail.

Doug Gilbert

Re: [PATCH] scsi_transport_sas: make minimum and maximum linkrate settable quantities

[James Bottomley]

On Wed, 2006-09-06 at 23:38 -0400, Douglas Gilbert wrote:
> James Bottomley wrote:
> > According to SPEC, the minimum_linkrate and maximum_linkrate should be
>                         ^                    ^
>                     programmable       programmable
> > settable by the user.  This patch introduces a callback that allows the
> > sas class to pass these settings on to the driver.
> 
> There are also "hardware" variants of min/max_linkrate.

hardware rates represent the max and min capabilities of the hw ...
they're not settable.

> Hmmm. I get a bit confused reading the above. Are you talking
> about a SAS initiator that this machine controls (i.e. a "host"),
> an expander phy in a visible SAS domain, a visible SSP target
> (e.g. a SAS disk) and/or a visible SATA device? All of them?
> Will all levels (i.e. initiator, expander and targets) have
> these fields (if they exist) represented by an attribute
> in sysfs?

I mean for all of the phys that are visible in the sysfs representation
of the domain. Non expander end device phys aren't visible in our
representation.  However, this is irrelevant really, since it takes two
phys to make a link, so if you force one side to a specific rate, the
other must follow or fail the link negotiation.

> The SATA device is easy to rule out (as is a SATA port selector).
> There may be some way of doing linkrate changes via the
> SATA command set.
> 
> SSP targets (e.g. SAS disks) have the "phy control and discover"
> mode subpage. The SAS disks that I have examined (all from
> one manufacturer) do not allow the programmable min/max_linkrate
> fields to be changed by the user (i.e. the changeable
> mode page bits are all zero). Worse still, their product
> manuals say that both ports (phys) will negotiate the
> same linkrate which will be the linkrate first attained.
> Looking at the spec (sas2r05b section 10.2.7.2.3), it just
> says all _other_ fields in that mode subpage shall not be
> changeable. So your "should be settable" statement above
> is a bit strong. How about "might be settable for SSP
> targets".

The same one end follows the other should pertain even to SSP targets.
I can't validate this, though, since I don't have any SATA-2 disks
(SATA-1 can't negotiate speed, being fixed at 1.5GB).

> As for expanders, the PHY CONTROL SMP function allows
> those fields to be changed on a phy and the spec implies
> those changes shall be obeyed, if they are still in place
> when the next link/bus reset occurs. However the spec says
> that function "may be implemented" so it is not required.
> 
> The SAS HBAs (initiators) that I have fought with allow
> those fields to be changed. Being able to make a HBA
> initiator phy do a link or hard reset required correspondence
> with the LLD maintainers, in my experience :-)

That would be why this is implemented with a callback into the HBA.  At
the moment, the expander functions are done with the SMP functions of
libsas.  Hopefully, when the mptsas gets an SMP execution function, we
can move this up into the transport class.

> Another thing worth pointing out is that the negotiated
> physical link rate only applies to one point to point
> physical link. The link rate can be changed by altering
> these fields associated with the phy at either end of the
> physical link, and then doing a link (or hard) reset from
> either end to force a renegotiation.
> 
> Also folks playing with these fields should realize that
> link reset is the appropriate type of reset to issue to
> cause a linkrate change. A hard reset will do it as well
> but may cause additional collateral damage. For example,
> if any phy on a SAS target device does a hard reset
> sequence, then all LUs within that target device receive
> a LU reset: clearing out all SCSI commands in progress and
> those pending. The idea with link reset is that only the link
> layers and below are impacted, as longer of the renegotiation
> succeeds in a timely fashion. Setting the prog_max_linkrate
> to 1.5 Gbps at one end of physical link while setting the
> prog_min_linkrate to 3 Gbps at the other end will cause
> the renegotiation following a link reset to fail.

Yes, the setting algorithms do a link reset, if you look at the code ...
and there's logic to prevent silly things like min going above max.

James