[PATCH] [SCSI] libsas: Allow expander T-T attachments

[PATCH] [SCSI] libsas: Allow expander T-T attachments

[Luben Tuikov]

Allow expander table-to-table attachments for
expanders that support it.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 drivers/scsi/libsas/sas_expander.c |   20 ++++++++++++++------
 include/scsi/libsas.h              |    3 +++
 include/scsi/sas.h                 |   14 ++++++++++++--
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index f84084b..e8d0115 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -329,6 +329,7 @@ static void ex_assign_report_general(struct domain_device *dev,
     dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
     dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
     dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
+    dev->ex_dev.t2t_supp = rg->t2t_supp;
     dev->ex_dev.conf_route_table = rg->conf_route_table;
     dev->ex_dev.configuring = rg->configuring;
     memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id, 8);
@@ -1133,15 +1134,17 @@ static void sas_print_parent_topology_bug(struct domain_device *child,
     };
     struct domain_device *parent = child->parent;
 
-    sas_printk("%s ex %016llx phy 0x%x <--> %s ex %016llx phy 0x%x "
-           "has %c:%c routing link!\n",
+    sas_printk("%s ex %016llx (T2T supp:%d) phy 0x%x <--> %s ex %016llx "
+           "(T2T supp:%d) phy 0x%x has %c:%c routing link!\n",
 
            ex_type[parent->dev_type],
            SAS_ADDR(parent->sas_addr),
+           parent->ex_dev.t2t_supp,
            parent_phy->phy_id,
 
            ex_type[child->dev_type],
            SAS_ADDR(child->sas_addr),
+           child->ex_dev.t2t_supp,
            child_phy->phy_id,
 
            ra_char[parent_phy->routing_attr],
@@ -1238,10 +1241,15 @@ static int sas_check_parent_topology(struct domain_device *child)
                     sas_print_parent_topology_bug(child, parent_phy, child_phy);
                     res = -ENODEV;
                 }
-            } else if (parent_phy->routing_attr == TABLE_ROUTING &&
-                   child_phy->routing_attr != SUBTRACTIVE_ROUTING) {
-                sas_print_parent_topology_bug(child, parent_phy, child_phy);
-                res = -ENODEV;
+            } else if (parent_phy->routing_attr == TABLE_ROUTING) {
+                if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||
+                    (child_phy->routing_attr == TABLE_ROUTING &&
+                     child_ex->t2t_supp && parent_ex->t2t_supp)) {
+                    /* All good */;
+                } else {
+                    sas_print_parent_topology_bug(child, parent_phy, child_phy);
+                    res = -ENODEV;
+                }
             }
             break;
         case FANOUT_DEV:
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index ee86606..793f80b 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -142,8 +142,11 @@ struct expander_device {
     u16    ex_change_count;
     u16    max_route_indexes;
     u8     num_phys;
+
+    u8     t2t_supp:1;
     u8     configuring:1;
     u8     conf_route_table:1;
+
     u8     enclosure_logical_id[8];
 
     struct ex_phy *ex_phy;
diff --git a/include/scsi/sas.h b/include/scsi/sas.h
index e9fd022..f59f182 100644
--- a/include/scsi/sas.h
+++ b/include/scsi/sas.h
@@ -341,7 +341,12 @@ struct report_general_resp {
 
     u8      conf_route_table:1;
     u8      configuring:1;
-    u8      _r_b:6;
+    u8    config_others:1;
+    u8    orej_retry_supp:1;
+    u8    stp_cont_awt:1;
+    u8    self_config:1;
+    u8    zone_config:1;
+    u8    t2t_supp:1;
 
     u8      _r_c;
 
@@ -528,7 +533,12 @@ struct report_general_resp {
     u8      _r_a;
     u8      num_phys;
 
-    u8      _r_b:6;
+    u8    t2t_supp:1;
+    u8    zone_config:1;
+    u8    self_config:1;
+    u8    stp_cont_awt:1;
+    u8    orej_retry_supp:1;
+    u8    config_others:1;
     u8      configuring:1;
     u8      conf_route_table:1;
 
-- 
1.7.2.2.165.gbc382

RE: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Jack Wang]

> 
> Allow expander table-to-table attachments for
> expanders that support it.
> 
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---
>  drivers/scsi/libsas/sas_expander.c |   20 ++++++++++++++------
>  include/scsi/libsas.h              |    3 +++
>  include/scsi/sas.h                 |   14 ++++++++++++--
>  3 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c
> b/drivers/scsi/libsas/sas_expander.c
> index f84084b..e8d0115 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -329,6 +329,7 @@ static void ex_assign_report_general(struct
domain_device
> *dev,
>      dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
>      dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
>      dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
> +    dev->ex_dev.t2t_supp = rg->t2t_supp;
>      dev->ex_dev.conf_route_table = rg->conf_route_table;
>      dev->ex_dev.configuring = rg->configuring;
>      memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id,
> 8);
> @@ -1133,15 +1134,17 @@ static void sas_print_parent_topology_bug(struct
> domain_device *child,
>      };
>      struct domain_device *parent = child->parent;
> 
> -    sas_printk("%s ex %016llx phy 0x%x <--> %s ex %016llx phy 0x%x "
> -           "has %c:%c routing link!\n",
> +    sas_printk("%s ex %016llx (T2T supp:%d) phy 0x%x <--> %s ex %016llx "
> +           "(T2T supp:%d) phy 0x%x has %c:%c routing link!\n",
> 
>             ex_type[parent->dev_type],
>             SAS_ADDR(parent->sas_addr),
> +           parent->ex_dev.t2t_supp,
>             parent_phy->phy_id,
> 
>             ex_type[child->dev_type],
>             SAS_ADDR(child->sas_addr),
> +           child->ex_dev.t2t_supp,
>             child_phy->phy_id,
> 
>             ra_char[parent_phy->routing_attr],
> @@ -1238,10 +1241,15 @@ static int sas_check_parent_topology(struct
> domain_device *child)
>                      sas_print_parent_topology_bug(child, parent_phy,
> child_phy);
>                      res = -ENODEV;
>                  }
> -            } else if (parent_phy->routing_attr == TABLE_ROUTING &&
> -                   child_phy->routing_attr != SUBTRACTIVE_ROUTING) {
> -                sas_print_parent_topology_bug(child, parent_phy,
> child_phy);
> -                res = -ENODEV;
> +            } else if (parent_phy->routing_attr == TABLE_ROUTING) {
> +                if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||
> +                    (child_phy->routing_attr == TABLE_ROUTING &&
> +                     child_ex->t2t_supp && parent_ex->t2t_supp)) {
> +                    /* All good */;
> +                } else {
> +                    sas_print_parent_topology_bug(child, parent_phy,
> child_phy);
> +                    res = -ENODEV;
> +                }
>              }
>              break;
>          case FANOUT_DEV:
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index ee86606..793f80b 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -142,8 +142,11 @@ struct expander_device {
>      u16    ex_change_count;
>      u16    max_route_indexes;
>      u8     num_phys;
> +
> +    u8     t2t_supp:1;
>      u8     configuring:1;
>      u8     conf_route_table:1;
> +
>      u8     enclosure_logical_id[8];
> 
>      struct ex_phy *ex_phy;
> diff --git a/include/scsi/sas.h b/include/scsi/sas.h
> index e9fd022..f59f182 100644
> --- a/include/scsi/sas.h
> +++ b/include/scsi/sas.h
> @@ -341,7 +341,12 @@ struct report_general_resp {
> 
>      u8      conf_route_table:1;
>      u8      configuring:1;
> -    u8      _r_b:6;
> +    u8    config_others:1;
> +    u8    orej_retry_supp:1;
> +    u8    stp_cont_awt:1;
> +    u8    self_config:1;
> +    u8    zone_config:1;
> +    u8    t2t_supp:1;
> 
>      u8      _r_c;
> 
> @@ -528,7 +533,12 @@ struct report_general_resp {
>      u8      _r_a;
>      u8      num_phys;
> 
> -    u8      _r_b:6;
> +    u8    t2t_supp:1;
> +    u8    zone_config:1;
> +    u8    self_config:1;
> +    u8    stp_cont_awt:1;
> +    u8    orej_retry_supp:1;
> +    u8    config_others:1;
>      u8      configuring:1;
>      u8      conf_route_table:1;
> 

[Jack Wang] Looks nice to me ,thanks for doing this.
Reviewed-by: Jack Wang <jack_wang@usish.com>

RE: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Jack Wang]

> 
> Allow expander table-to-table attachments for
> expanders that support it.
> 
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---
>  drivers/scsi/libsas/sas_expander.c |   20 ++++++++++++++------
>  include/scsi/libsas.h              |    3 +++
>  include/scsi/sas.h                 |   14 ++++++++++++--
>  3 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c
> b/drivers/scsi/libsas/sas_expander.c
> index f84084b..e8d0115 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -329,6 +329,7 @@ static void ex_assign_report_general(struct
domain_device
> *dev,
>      dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
>      dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
>      dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
> +    dev->ex_dev.t2t_supp = rg->t2t_supp;
>      dev->ex_dev.conf_route_table = rg->conf_route_table;
>      dev->ex_dev.configuring = rg->configuring;
>      memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id,
> 8);
> @@ -1133,15 +1134,17 @@ static void sas_print_parent_topology_bug(struct
> domain_device *child,
>      };
>      struct domain_device *parent = child->parent;
> 
> -    sas_printk("%s ex %016llx phy 0x%x <--> %s ex %016llx phy 0x%x "
> -           "has %c:%c routing link!\n",
> +    sas_printk("%s ex %016llx (T2T supp:%d) phy 0x%x <--> %s ex %016llx "
> +           "(T2T supp:%d) phy 0x%x has %c:%c routing link!\n",
> 
>             ex_type[parent->dev_type],
>             SAS_ADDR(parent->sas_addr),
> +           parent->ex_dev.t2t_supp,
>             parent_phy->phy_id,
> 
>             ex_type[child->dev_type],
>             SAS_ADDR(child->sas_addr),
> +           child->ex_dev.t2t_supp,
>             child_phy->phy_id,
> 
>             ra_char[parent_phy->routing_attr],
> @@ -1238,10 +1241,15 @@ static int sas_check_parent_topology(struct
> domain_device *child)
>                      sas_print_parent_topology_bug(child, parent_phy,
> child_phy);
>                      res = -ENODEV;
>                  }
> -            } else if (parent_phy->routing_attr == TABLE_ROUTING &&
> -                   child_phy->routing_attr != SUBTRACTIVE_ROUTING) {
> -                sas_print_parent_topology_bug(child, parent_phy,
> child_phy);
> -                res = -ENODEV;
> +            } else if (parent_phy->routing_attr == TABLE_ROUTING) {
> +                if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||
> +                    (child_phy->routing_attr == TABLE_ROUTING &&
> +                     child_ex->t2t_supp && parent_ex->t2t_supp)) {
> +                    /* All good */;
> +                } else {
> +                    sas_print_parent_topology_bug(child, parent_phy,
> child_phy);
> +                    res = -ENODEV;
> +                }
>              }
>              break;
>          case FANOUT_DEV:
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index ee86606..793f80b 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -142,8 +142,11 @@ struct expander_device {
>      u16    ex_change_count;
>      u16    max_route_indexes;
>      u8     num_phys;
> +
> +    u8     t2t_supp:1;
>      u8     configuring:1;
>      u8     conf_route_table:1;
> +
>      u8     enclosure_logical_id[8];
> 
>      struct ex_phy *ex_phy;
> diff --git a/include/scsi/sas.h b/include/scsi/sas.h
> index e9fd022..f59f182 100644
> --- a/include/scsi/sas.h
> +++ b/include/scsi/sas.h
> @@ -341,7 +341,12 @@ struct report_general_resp {
> 
>      u8      conf_route_table:1;
>      u8      configuring:1;
> -    u8      _r_b:6;
> +    u8    config_others:1;
> +    u8    orej_retry_supp:1;
> +    u8    stp_cont_awt:1;
> +    u8    self_config:1;
> +    u8    zone_config:1;
> +    u8    t2t_supp:1;
> 
>      u8      _r_c;
> 
> @@ -528,7 +533,12 @@ struct report_general_resp {
>      u8      _r_a;
>      u8      num_phys;
> 
> -    u8      _r_b:6;
> +    u8    t2t_supp:1;
> +    u8    zone_config:1;
> +    u8    self_config:1;
> +    u8    stp_cont_awt:1;
> +    u8    orej_retry_supp:1;
> +    u8    config_others:1;
>      u8      configuring:1;
>      u8      conf_route_table:1;
> 

[Jack Wang] Looks nice to me ,thanks for doing this.
Reviewed-by: Jack Wang <jack_wang@usish.com>

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

Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[James Bottomley]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4779 bytes --]

On Wed, 2011-07-27 at 21:19 -0700, Luben Tuikov wrote:
> Allow expander table-to-table attachments for
> expanders that support it.

This is half the code to do SAS 2.0 expanders.  You seem to have
additions from the other half (self configuring expanders) see below;
does this mean you have a patch for self configuring expanders in the
works?

> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---
>  drivers/scsi/libsas/sas_expander.c |   20 ++++++++++++++------
>  include/scsi/libsas.h              |    3 +++
>  include/scsi/sas.h                 |   14 ++++++++++++--
>  3 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index f84084b..e8d0115 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -329,6 +329,7 @@ static void ex_assign_report_general(struct domain_device *dev,
>      dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
>      dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
>      dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
> +    dev->ex_dev.t2t_supp = rg->t2t_supp;
>      dev->ex_dev.conf_route_table = rg->conf_route_table;
>      dev->ex_dev.configuring = rg->configuring;
>      memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id, 8);
> @@ -1133,15 +1134,17 @@ static void sas_print_parent_topology_bug(struct domain_device *child,
>      };
>      struct domain_device *parent = child->parent;
>  
> -    sas_printk("%s ex %016llx phy 0x%x <--> %s ex %016llx phy 0x%x "
> -           "has %c:%c routing link!\n",
> +    sas_printk("%s ex %016llx (T2T supp:%d) phy 0x%x <--> %s ex %016llx "
> +           "(T2T supp:%d) phy 0x%x has %c:%c routing link!\n",
>  
>             ex_type[parent->dev_type],
>             SAS_ADDR(parent->sas_addr),
> +           parent->ex_dev.t2t_supp,
>             parent_phy->phy_id,
>  
>             ex_type[child->dev_type],
>             SAS_ADDR(child->sas_addr),
> +           child->ex_dev.t2t_supp,
>             child_phy->phy_id,
>  
>             ra_char[parent_phy->routing_attr],
> @@ -1238,10 +1241,15 @@ static int sas_check_parent_topology(struct domain_device *child)
>                      sas_print_parent_topology_bug(child, parent_phy, child_phy);
>                      res = -ENODEV;
>                  }
> -            } else if (parent_phy->routing_attr == TABLE_ROUTING &&
> -                   child_phy->routing_attr != SUBTRACTIVE_ROUTING) {
> -                sas_print_parent_topology_bug(child, parent_phy, child_phy);
> -                res = -ENODEV;
> +            } else if (parent_phy->routing_attr == TABLE_ROUTING) {
> +                if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||
> +                    (child_phy->routing_attr == TABLE_ROUTING &&
> +                     child_ex->t2t_supp && parent_ex->t2t_supp)) {
> +                    /* All good */;
> +                } else {
> +                    sas_print_parent_topology_bug(child, parent_phy, child_phy);
> +                    res = -ENODEV;
> +                }
>              }
>              break;
>          case FANOUT_DEV:
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index ee86606..793f80b 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -142,8 +142,11 @@ struct expander_device {
>      u16    ex_change_count;
>      u16    max_route_indexes;
>      u8     num_phys;
> +
> +    u8     t2t_supp:1;
>      u8     configuring:1;
>      u8     conf_route_table:1;
> +
>      u8     enclosure_logical_id[8];
>  
>      struct ex_phy *ex_phy;
> diff --git a/include/scsi/sas.h b/include/scsi/sas.h
> index e9fd022..f59f182 100644
> --- a/include/scsi/sas.h
> +++ b/include/scsi/sas.h
> @@ -341,7 +341,12 @@ struct report_general_resp {
>  
>      u8      conf_route_table:1;
>      u8      configuring:1;
> -    u8      _r_b:6;
> +    u8    config_others:1;
> +    u8    orej_retry_supp:1;
> +    u8    stp_cont_awt:1;
> +    u8    self_config:1;
> +    u8    zone_config:1;
> +    u8    t2t_supp:1;

>      u8      _r_c;
>  
> @@ -528,7 +533,12 @@ struct report_general_resp {
>      u8      _r_a;
>      u8      num_phys;
>  
> -    u8      _r_b:6;
> +    u8    t2t_supp:1;
> +    u8    zone_config:1;
> +    u8    self_config:1;
> +    u8    stp_cont_awt:1;
> +    u8    orej_retry_supp:1;
> +    u8    config_others:1;
>      u8      configuring:1;
>      u8      conf_route_table:1;
>  

These are all SAS2.0 additions to the reserved fields, but you only make
use of one of them.

James

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[James Bottomley]

On Wed, 2011-07-27 at 21:19 -0700, Luben Tuikov wrote:
> Allow expander table-to-table attachments for
> expanders that support it.

This is half the code to do SAS 2.0 expanders.  You seem to have
additions from the other half (self configuring expanders) see below;
does this mean you have a patch for self configuring expanders in the
works?

> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---
>  drivers/scsi/libsas/sas_expander.c |   20 ++++++++++++++------
>  include/scsi/libsas.h              |    3 +++
>  include/scsi/sas.h                 |   14 ++++++++++++--
>  3 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index f84084b..e8d0115 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -329,6 +329,7 @@ static void ex_assign_report_general(struct domain_device *dev,
>      dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
>      dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
>      dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
> +    dev->ex_dev.t2t_supp = rg->t2t_supp;
>      dev->ex_dev.conf_route_table = rg->conf_route_table;
>      dev->ex_dev.configuring = rg->configuring;
>      memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id, 8);
> @@ -1133,15 +1134,17 @@ static void sas_print_parent_topology_bug(struct domain_device *child,
>      };
>      struct domain_device *parent = child->parent;
>  
> -    sas_printk("%s ex %016llx phy 0x%x <--> %s ex %016llx phy 0x%x "
> -           "has %c:%c routing link!\n",
> +    sas_printk("%s ex %016llx (T2T supp:%d) phy 0x%x <--> %s ex %016llx "
> +           "(T2T supp:%d) phy 0x%x has %c:%c routing link!\n",
>  
>             ex_type[parent->dev_type],
>             SAS_ADDR(parent->sas_addr),
> +           parent->ex_dev.t2t_supp,
>             parent_phy->phy_id,
>  
>             ex_type[child->dev_type],
>             SAS_ADDR(child->sas_addr),
> +           child->ex_dev.t2t_supp,
>             child_phy->phy_id,
>  
>             ra_char[parent_phy->routing_attr],
> @@ -1238,10 +1241,15 @@ static int sas_check_parent_topology(struct domain_device *child)
>                      sas_print_parent_topology_bug(child, parent_phy, child_phy);
>                      res = -ENODEV;
>                  }
> -            } else if (parent_phy->routing_attr == TABLE_ROUTING &&
> -                   child_phy->routing_attr != SUBTRACTIVE_ROUTING) {
> -                sas_print_parent_topology_bug(child, parent_phy, child_phy);
> -                res = -ENODEV;
> +            } else if (parent_phy->routing_attr == TABLE_ROUTING) {
> +                if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||
> +                    (child_phy->routing_attr == TABLE_ROUTING &&
> +                     child_ex->t2t_supp && parent_ex->t2t_supp)) {
> +                    /* All good */;
> +                } else {
> +                    sas_print_parent_topology_bug(child, parent_phy, child_phy);
> +                    res = -ENODEV;
> +                }
>              }
>              break;
>          case FANOUT_DEV:
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index ee86606..793f80b 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -142,8 +142,11 @@ struct expander_device {
>      u16    ex_change_count;
>      u16    max_route_indexes;
>      u8     num_phys;
> +
> +    u8     t2t_supp:1;
>      u8     configuring:1;
>      u8     conf_route_table:1;
> +
>      u8     enclosure_logical_id[8];
>  
>      struct ex_phy *ex_phy;
> diff --git a/include/scsi/sas.h b/include/scsi/sas.h
> index e9fd022..f59f182 100644
> --- a/include/scsi/sas.h
> +++ b/include/scsi/sas.h
> @@ -341,7 +341,12 @@ struct report_general_resp {
>  
>      u8      conf_route_table:1;
>      u8      configuring:1;
> -    u8      _r_b:6;
> +    u8    config_others:1;
> +    u8    orej_retry_supp:1;
> +    u8    stp_cont_awt:1;
> +    u8    self_config:1;
> +    u8    zone_config:1;
> +    u8    t2t_supp:1;

>      u8      _r_c;
>  
> @@ -528,7 +533,12 @@ struct report_general_resp {
>      u8      _r_a;
>      u8      num_phys;
>  
> -    u8      _r_b:6;
> +    u8    t2t_supp:1;
> +    u8    zone_config:1;
> +    u8    self_config:1;
> +    u8    stp_cont_awt:1;
> +    u8    orej_retry_supp:1;
> +    u8    config_others:1;
>      u8      configuring:1;
>      u8      conf_route_table:1;
>  

These are all SAS2.0 additions to the reserved fields, but you only make
use of one of them.

James

Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Dan Williams]

On Thu, Jul 28, 2011 at 12:52 AM, James Bottomley
<jbottomley@parallels.com> wrote:
> On Wed, 2011-07-27 at 21:19 -0700, Luben Tuikov wrote:
>> Allow expander table-to-table attachments for
>> expanders that support it.
>
> This is half the code to do SAS 2.0 expanders.  You seem to have
> additions from the other half (self configuring expanders) see below;
> does this mean you have a patch for self configuring expanders in the
> works?

I'd like to see that as well, but in the meantime support for the
other SAS-2 fields does not block basic operation like the lack of
table-to-table support does.

Can we also tag this for -stable?  It's akin to a pci device id update
in terms of impact.

Acked-by: Dan Williams <dan.j.williams@intel.com>

Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Dan Williams]

On Thu, Jul 28, 2011 at 12:52 AM, James Bottomley
<jbottomley@parallels.com> wrote:
> On Wed, 2011-07-27 at 21:19 -0700, Luben Tuikov wrote:
>> Allow expander table-to-table attachments for
>> expanders that support it.
>
> This is half the code to do SAS 2.0 expanders.  You seem to have
> additions from the other half (self configuring expanders) see below;
> does this mean you have a patch for self configuring expanders in the
> works?

I'd like to see that as well, but in the meantime support for the
other SAS-2 fields does not block basic operation like the lack of
table-to-table support does.

Can we also tag this for -stable?  It's akin to a pci device id update
in terms of impact.

Acked-by: Dan Williams <dan.j.williams@intel.com>
--
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

Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[James Bottomley]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 541 bytes --]

On Wed, 2011-07-27 at 21:19 -0700, Luben Tuikov wrote:
> Allow expander table-to-table attachments for
> expanders that support it.
> 
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>

OK, so now I need an applyable patch, please.  It looks like the mail
server has mapped all tabs to spaces.  If you can't fix the mailer, just
sending the patch as an attachment is fine.

James

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[James Bottomley]

On Wed, 2011-07-27 at 21:19 -0700, Luben Tuikov wrote:
> Allow expander table-to-table attachments for
> expanders that support it.
> 
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>

OK, so now I need an applyable patch, please.  It looks like the mail
server has mapped all tabs to spaces.  If you can't fix the mailer, just
sending the patch as an attachment is fine.

James

RE: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Mark Salyzyn]

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

Enclosed is an updated patch that passes checkpatch.pl (a few 80-character line errors in the original), maintain existing indent mechanics and contains my 'Ack' and the recommended Cc list.

Wait for Luben's Ack or Override (despite the fact I took the liberty of adding his Signed-off-by on the checkpatch required adjustments).

Sincerely -- Mark Salyzyn 

-----Original Message-----
From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of James Bottomley
Sent: Thursday, September 22, 2011 7:30 AM
To: Luben Tuikov
Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments

On Wed, 2011-07-27 at 21:19 -0700, Luben Tuikov wrote:
> Allow expander table-to-table attachments for
> expanders that support it.
> 
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>

OK, so now I need an applyable patch, please.  It looks like the mail
server has mapped all tabs to spaces.  If you can't fix the mailer, just
sending the patch as an attachment is fine.

James

NrybXǧv^)޺{.n+{"{ay\x1dʇڙ,j fhz\x1ew\fj:+vwjm zZ+ݢj"!

______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 


[-- Attachment #2: libsas_t2t.patch --]
[-- Type: application/octet-stream, Size: 4154 bytes --]

Allow expander table-to-table attachments for expanders that support it. [LT]
Adjusted to pass checkpatch.pl. [MS]

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
Cc: Darrick J Wong <djwong@us.ibm.com>
Cc: James Bottomley <jbottomley@parallels.com>
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Cc: Jack Wang <jack_wang@usish.com>
Acked-by: Mark Salyzyn <mark_salyzyn@us.xyratex.com>

 drivers/scsi/libsas/sas_expander.c |   24 ++++++++++++++++++------
 include/scsi/libsas.h              |    3 +++
 include/scsi/sas.h                 |   14 ++++++++++++--
 3 files changed, 33 insertions(+), 8 deletions(-)

diff -rup scsi-misc-2.6/drivers/scsi/libsas/sas_expander.c scsi-misc-2.6.new/drivers/scsi/libsas/sas_expander.c
--- scsi-misc-2.6/drivers/scsi/libsas/sas_expander.c	2011-08-31 08:32:21.000000000 -0400
+++ scsi-misc-2.6.new/drivers/scsi/libsas/sas_expander.c	2011-09-22 10:49:08.000000000 -0400
@@ -329,6 +329,7 @@ static void ex_assign_report_general(str
 	dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
 	dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
 	dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
+	dev->ex_dev.t2t_supp = rg->t2t_supp;
 	dev->ex_dev.conf_route_table = rg->conf_route_table;
 	dev->ex_dev.configuring = rg->configuring;
 	memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id, 8);
@@ -1133,15 +1134,17 @@ static void sas_print_parent_topology_bu
 	};
 	struct domain_device *parent = child->parent;
 
-	sas_printk("%s ex %016llx phy 0x%x <--> %s ex %016llx phy 0x%x "
-		   "has %c:%c routing link!\n",
+	sas_printk("%s ex %016llx (T2T supp:%d) phy 0x%x <--> %s ex %016llx "
+		   "(T2T supp:%d) phy 0x%x has %c:%c routing link!\n",
 
 		   ex_type[parent->dev_type],
 		   SAS_ADDR(parent->sas_addr),
+		   parent->ex_dev.t2t_supp,
 		   parent_phy->phy_id,
 
 		   ex_type[child->dev_type],
 		   SAS_ADDR(child->sas_addr),
+		   child->ex_dev.t2t_supp,
 		   child_phy->phy_id,
 
 		   ra_char[parent_phy->routing_attr],
@@ -1238,10 +1241,19 @@ static int sas_check_parent_topology(str
 					sas_print_parent_topology_bug(child, parent_phy, child_phy);
 					res = -ENODEV;
 				}
-			} else if (parent_phy->routing_attr == TABLE_ROUTING &&
-				   child_phy->routing_attr != SUBTRACTIVE_ROUTING) {
-				sas_print_parent_topology_bug(child, parent_phy, child_phy);
-				res = -ENODEV;
+			} else if (parent_phy->routing_attr == TABLE_ROUTING) {
+				if (child_phy->routing_attr ==
+							SUBTRACTIVE_ROUTING ||
+				    (child_phy->routing_attr ==
+							TABLE_ROUTING &&
+				     child_ex->t2t_supp &&
+				     parent_ex->t2t_supp)) {
+					/* All good */;
+				} else {
+					sas_print_parent_topology_bug(child,
+						parent_phy, child_phy);
+					res = -ENODEV;
+				}
 			}
 			break;
 		case FANOUT_DEV:
diff -rup scsi-misc-2.6/include/scsi/libsas.h scsi-misc-2.6.new/include/scsi/libsas.h
--- scsi-misc-2.6/include/scsi/libsas.h	2011-08-31 08:32:22.000000000 -0400
+++ scsi-misc-2.6.new/include/scsi/libsas.h	2011-09-22 10:38:08.000000000 -0400
@@ -142,8 +142,11 @@ struct expander_device {
 	u16    ex_change_count;
 	u16    max_route_indexes;
 	u8     num_phys;
+
+	u8     t2t_supp:1;
 	u8     configuring:1;
 	u8     conf_route_table:1;
+
 	u8     enclosure_logical_id[8];
 
 	struct ex_phy *ex_phy;
diff -rup scsi-misc-2.6/include/scsi/sas.h scsi-misc-2.6.new/include/scsi/sas.h
--- scsi-misc-2.6/include/scsi/sas.h	2011-08-31 08:32:22.000000000 -0400
+++ scsi-misc-2.6.new/include/scsi/sas.h	2011-09-22 10:36:54.000000000 -0400
@@ -341,7 +341,12 @@ struct report_general_resp {
 
 	u8      conf_route_table:1;
 	u8      configuring:1;
-	u8      _r_b:6;
+	u8      config_others:1;
+	u8      orej_retry_supp:1;
+	u8      stp_count_awt:1;
+	u8      self_config:1;
+	u8      zone_config:1;
+	u8      t2t_supp:1;
 
 	u8      _r_c;
 
@@ -528,7 +533,12 @@ struct report_general_resp {
 	u8      _r_a;
 	u8      num_phys;
 
-	u8      _r_b:6;
+	u8      t2t_supp:1;
+	u8      zone_config:1;
+	u8      self_config:1;
+	u8      stp_count_awt:1;
+	u8      orej_retry_supp:1;
+	u8      config_others:1;
 	u8      configuring:1;
 	u8      conf_route_table:1;
 

RE: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Mark Salyzyn]

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

Enclosed is an updated patch that passes checkpatch.pl (a few 80-character line errors in the original), maintain existing indent mechanics and contains my 'Ack' and the recommended Cc list.

Wait for Luben's Ack or Override (despite the fact I took the liberty of adding his Signed-off-by on the checkpatch required adjustments).

Sincerely -- Mark Salyzyn 

-----Original Message-----
From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of James Bottomley
Sent: Thursday, September 22, 2011 7:30 AM
To: Luben Tuikov
Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments

On Wed, 2011-07-27 at 21:19 -0700, Luben Tuikov wrote:
> Allow expander table-to-table attachments for
> expanders that support it.
> 
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>

OK, so now I need an applyable patch, please.  It looks like the mail
server has mapped all tabs to spaces.  If you can't fix the mailer, just
sending the patch as an attachment is fine.

James

NrybXǧv^)޺{.n+{"{ay\x1dʇڙ,j fhz\x1ew\fj:+vwjm zZ+ݢj"!

______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 


[-- Attachment #2: libsas_t2t.patch --]
[-- Type: application/octet-stream, Size: 4154 bytes --]

Allow expander table-to-table attachments for expanders that support it. [LT]
Adjusted to pass checkpatch.pl. [MS]

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
Cc: Darrick J Wong <djwong@us.ibm.com>
Cc: James Bottomley <jbottomley@parallels.com>
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Cc: Jack Wang <jack_wang@usish.com>
Acked-by: Mark Salyzyn <mark_salyzyn@us.xyratex.com>

 drivers/scsi/libsas/sas_expander.c |   24 ++++++++++++++++++------
 include/scsi/libsas.h              |    3 +++
 include/scsi/sas.h                 |   14 ++++++++++++--
 3 files changed, 33 insertions(+), 8 deletions(-)

diff -rup scsi-misc-2.6/drivers/scsi/libsas/sas_expander.c scsi-misc-2.6.new/drivers/scsi/libsas/sas_expander.c
--- scsi-misc-2.6/drivers/scsi/libsas/sas_expander.c	2011-08-31 08:32:21.000000000 -0400
+++ scsi-misc-2.6.new/drivers/scsi/libsas/sas_expander.c	2011-09-22 10:49:08.000000000 -0400
@@ -329,6 +329,7 @@ static void ex_assign_report_general(str
 	dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
 	dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
 	dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
+	dev->ex_dev.t2t_supp = rg->t2t_supp;
 	dev->ex_dev.conf_route_table = rg->conf_route_table;
 	dev->ex_dev.configuring = rg->configuring;
 	memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id, 8);
@@ -1133,15 +1134,17 @@ static void sas_print_parent_topology_bu
 	};
 	struct domain_device *parent = child->parent;
 
-	sas_printk("%s ex %016llx phy 0x%x <--> %s ex %016llx phy 0x%x "
-		   "has %c:%c routing link!\n",
+	sas_printk("%s ex %016llx (T2T supp:%d) phy 0x%x <--> %s ex %016llx "
+		   "(T2T supp:%d) phy 0x%x has %c:%c routing link!\n",
 
 		   ex_type[parent->dev_type],
 		   SAS_ADDR(parent->sas_addr),
+		   parent->ex_dev.t2t_supp,
 		   parent_phy->phy_id,
 
 		   ex_type[child->dev_type],
 		   SAS_ADDR(child->sas_addr),
+		   child->ex_dev.t2t_supp,
 		   child_phy->phy_id,
 
 		   ra_char[parent_phy->routing_attr],
@@ -1238,10 +1241,19 @@ static int sas_check_parent_topology(str
 					sas_print_parent_topology_bug(child, parent_phy, child_phy);
 					res = -ENODEV;
 				}
-			} else if (parent_phy->routing_attr == TABLE_ROUTING &&
-				   child_phy->routing_attr != SUBTRACTIVE_ROUTING) {
-				sas_print_parent_topology_bug(child, parent_phy, child_phy);
-				res = -ENODEV;
+			} else if (parent_phy->routing_attr == TABLE_ROUTING) {
+				if (child_phy->routing_attr ==
+							SUBTRACTIVE_ROUTING ||
+				    (child_phy->routing_attr ==
+							TABLE_ROUTING &&
+				     child_ex->t2t_supp &&
+				     parent_ex->t2t_supp)) {
+					/* All good */;
+				} else {
+					sas_print_parent_topology_bug(child,
+						parent_phy, child_phy);
+					res = -ENODEV;
+				}
 			}
 			break;
 		case FANOUT_DEV:
diff -rup scsi-misc-2.6/include/scsi/libsas.h scsi-misc-2.6.new/include/scsi/libsas.h
--- scsi-misc-2.6/include/scsi/libsas.h	2011-08-31 08:32:22.000000000 -0400
+++ scsi-misc-2.6.new/include/scsi/libsas.h	2011-09-22 10:38:08.000000000 -0400
@@ -142,8 +142,11 @@ struct expander_device {
 	u16    ex_change_count;
 	u16    max_route_indexes;
 	u8     num_phys;
+
+	u8     t2t_supp:1;
 	u8     configuring:1;
 	u8     conf_route_table:1;
+
 	u8     enclosure_logical_id[8];
 
 	struct ex_phy *ex_phy;
diff -rup scsi-misc-2.6/include/scsi/sas.h scsi-misc-2.6.new/include/scsi/sas.h
--- scsi-misc-2.6/include/scsi/sas.h	2011-08-31 08:32:22.000000000 -0400
+++ scsi-misc-2.6.new/include/scsi/sas.h	2011-09-22 10:36:54.000000000 -0400
@@ -341,7 +341,12 @@ struct report_general_resp {
 
 	u8      conf_route_table:1;
 	u8      configuring:1;
-	u8      _r_b:6;
+	u8      config_others:1;
+	u8      orej_retry_supp:1;
+	u8      stp_count_awt:1;
+	u8      self_config:1;
+	u8      zone_config:1;
+	u8      t2t_supp:1;
 
 	u8      _r_c;
 
@@ -528,7 +533,12 @@ struct report_general_resp {
 	u8      _r_a;
 	u8      num_phys;
 
-	u8      _r_b:6;
+	u8      t2t_supp:1;
+	u8      zone_config:1;
+	u8      self_config:1;
+	u8      stp_count_awt:1;
+	u8      orej_retry_supp:1;
+	u8      config_others:1;
 	u8      configuring:1;
 	u8      conf_route_table:1;
 

[PATCH] [SCSI] libsas panic when single phy disabled on a wide port

[Mark Salyzyn]

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

When a wide port is being utilized to a target, if one disables only one
of the
phys, we get an OS crash:

BUG: unable to handle kernel NULL pointer dereference at
0000000000000238
IP: [<ffffffff814ca9b1>] mutex_lock+0x21/0x50
PGD 4103f5067 PUD 41dba9067 PMD 0
Oops: 0002 [#1] SMP
last sysfs file: /sys/bus/pci/slots/5/address
CPU 0
Modules linked in: pm8001(U) ses enclosure fuse nfsd exportfs autofs4
ipmi_devintf ipmi_si ipmi_msghandler nfs lockd fscache nfs_acl
auth_rpcgss 8021q fcoe libfcoe garp libfc scsi_transport_fc stp scsi_tgt
llc sunrpc cpufreq_ondemand acpi_cpufreq freq_table ipv6 sr_mod cdrom
dm_mirror dm_region_hash dm_log uinput sg i2c_i801 i2c_core iTCO_wdt
iTCO_vendor_support e1000e mlx4_ib ib_mad ib_core mlx4_en mlx4_core ext3
jbd mbcache sd_mod crc_t10dif usb_storage ata_generic pata_acpi ata_piix
libsas(U) scsi_transport_sas dm_mod [last unloaded: pm8001]

Modules linked in: pm8001(U) ses enclosure fuse nfsd exportfs autofs4
ipmi_devintf ipmi_si ipmi_msghandler nfs lockd fscache nfs_acl
auth_rpcgss 8021q fcoe libfcoe garp libfc scsi_transport_fc stp scsi_tgt
llc sunrpc cpufreq_ondemand acpi_cpufreq freq_table ipv6 sr_mod cdrom
dm_mirror dm_region_hash dm_log uinput sg i2c_i801 i2c_core iTCO_wdt
iTCO_vendor_support e1000e mlx4_ib ib_mad ib_core mlx4_en mlx4_core ext3
jbd mbcache sd_mod crc_t10dif usb_storage ata_generic pata_acpi ata_piix
libsas(U) scsi_transport_sas dm_mod [last unloaded: pm8001]
Pid: 5146, comm: scsi_wq_5 Not tainted
2.6.32-71.29.1.el6.lustre.7.x86_64 #1 Storage Server
RIP: 0010:[<ffffffff814ca9b1>]  [<ffffffff814ca9b1>]
mutex_lock+0x21/0x50
RSP: 0018:ffff8803e4e33d30  EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000238 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffff8803e664c800 RDI: 0000000000000238
RBP: ffff8803e4e33d40 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000001 R12: 0000000000000000
R13: 0000000000000238 R14: ffff88041acb7200 R15: ffff88041c51ada0
FS:  0000000000000000(0000) GS:ffff880028200000(0000)
knlGS:0000000000000000
CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
CR2: 0000000000000238 CR3: 0000000410143000 CR4: 00000000000006f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process scsi_wq_5 (pid: 5146, threadinfo ffff8803e4e32000, task
ffff8803e4e294a0)
Stack:
 ffff8803e664c800 0000000000000000 ffff8803e4e33d70 ffffffffa001f06e
<0> ffff8803e4e33d60 ffff88041c51ada0 ffff88041acb7200 ffff88041bc0aa00
<0> ffff8803e4e33d90 ffffffffa0032b6c 0000000000000014 ffff88041acb7200
Call Trace:
 [<ffffffffa001f06e>] sas_port_delete_phy+0x2e/0xa0 [scsi_transport_sas]
 [<ffffffffa0032b6c>] sas_unregister_devs_sas_addr+0xac/0xe0 [libsas]
 [<ffffffffa0034914>] sas_ex_revalidate_domain+0x204/0x330 [libsas]
 [<ffffffffa00307f0>] ? sas_revalidate_domain+0x0/0x90 [libsas]
 [<ffffffffa0030855>] sas_revalidate_domain+0x65/0x90 [libsas]
 [<ffffffff8108c7d0>] worker_thread+0x170/0x2a0
 [<ffffffff81091ea0>] ? autoremove_wake_function+0x0/0x40
 [<ffffffff8108c660>] ? worker_thread+0x0/0x2a0
 [<ffffffff81091b36>] kthread+0x96/0xa0
 [<ffffffff810141ca>] child_rip+0xa/0x20
 [<ffffffff81091aa0>] ? kthread+0x0/0xa0
 [<ffffffff810141c0>] ? child_rip+0x0/0x20
Code: ff ff 85 c0 75 ed eb d6 66 90 55 48 89 e5 48 83 ec 10 48 89 1c 24
4c 89 64 24 08 0f 1f 44 00 00 48 89 fb e8 92 f4 ff ff 48 89 df <f0> ff
0f 79 05 e8 25 00 00 00 65 48 8b 04 25 08 cc 00 00 48 2d
RIP  [<ffffffff814ca9b1>] mutex_lock+0x21/0x50
 RSP <ffff8803e4e33d30>
CR2: 0000000000000238

The following patch is admittedly a band-aid, and does not solve the
root cause, but it still is a good candidate for hardening as a pointer
check before reference.

The real patch is enclosed as an attachment since Outlook is my current
MTA.

Signed-off-by: Mark Salyzyn <mark_salyzyn@xyus.xyratex.com>
Cc: Luben Tuikov <ltuikov@yahoo.com>
Cc: Darrick J Wong <djwong@us.ibm.com>
Cc: James Bottomley <jbottomley@parallels.com>
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Cc: Jack Wang <jack_wang@usish.com>

 sas_expander.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff -rup scsi-misc-2.6/drivers/scsi/libsas/sas_expander.c
scsi-misc-2.6.new/drivers/scsi/libsas/sas_expander.c
--- scsi-misc-2.6/drivers/scsi/libsas/sas_expander.c	2011-08-31
08:32:21.000000000 -0400
+++ scsi-misc-2.6.new/drivers/scsi/libsas/sas_expander.c
2011-09-22 11:20:06.000000000 -0400
@@ -1769,10 +1769,12 @@ static void sas_unregister_devs_sas_addr
 		sas_disable_routing(parent, phy->attached_sas_addr);
 	}
 	memset(phy->attached_sas_addr, 0, SAS_ADDR_SIZE);
-	sas_port_delete_phy(phy->port, phy->phy);
-	if (phy->port->num_phys == 0)
-		sas_port_delete(phy->port);
-	phy->port = NULL;
+	if (phy->port) {
+		sas_port_delete_phy(phy->port, phy->phy);
+		if (phy->port->num_phys == 0)
+			sas_port_delete(phy->port);
+		phy->port = NULL;
+	}
 }
 
 static int sas_discover_bfs_by_root_level(struct domain_device *root,

______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 
\r

[-- Attachment #2: libsas_single_wide.patch --]
[-- Type: application/octet-stream, Size: 4906 bytes --]

When a wide port is being utilized to a target, if one disables only one of the
phys, we get an OS crash:

BUG: unable to handle kernel NULL pointer dereference at 0000000000000238
IP: [<ffffffff814ca9b1>] mutex_lock+0x21/0x50
PGD 4103f5067 PUD 41dba9067 PMD 0
Oops: 0002 [#1] SMP
last sysfs file: /sys/bus/pci/slots/5/address
CPU 0
Modules linked in: pm8001(U) ses enclosure fuse nfsd exportfs autofs4 ipmi_devintf ipmi_si ipmi_msghandler nfs lockd fscache nfs_acl auth_rpcgss 8021q fcoe libfcoe garp libfc scsi_transport_fc stp scsi_tgt llc sunrpc cpufreq_ondemand acpi_cpufreq freq_table ipv6 sr_mod cdrom dm_mirror dm_region_hash dm_log uinput sg i2c_i801 i2c_core iTCO_wdt iTCO_vendor_support e1000e mlx4_ib ib_mad ib_core mlx4_en mlx4_core ext3 jbd mbcache sd_mod crc_t10dif usb_storage ata_generic pata_acpi ata_piix libsas(U) scsi_transport_sas dm_mod [last unloaded: pm8001]

Modules linked in: pm8001(U) ses enclosure fuse nfsd exportfs autofs4 ipmi_devintf ipmi_si ipmi_msghandler nfs lockd fscache nfs_acl auth_rpcgss 8021q fcoe libfcoe garp libfc scsi_transport_fc stp scsi_tgt llc sunrpc cpufreq_ondemand acpi_cpufreq freq_table ipv6 sr_mod cdrom dm_mirror dm_region_hash dm_log uinput sg i2c_i801 i2c_core iTCO_wdt iTCO_vendor_support e1000e mlx4_ib ib_mad ib_core mlx4_en mlx4_core ext3 jbd mbcache sd_mod crc_t10dif usb_storage ata_generic pata_acpi ata_piix libsas(U) scsi_transport_sas dm_mod [last unloaded: pm8001]
Pid: 5146, comm: scsi_wq_5 Not tainted 2.6.32-71.29.1.el6.lustre.7.x86_64 #1 Storage Server
RIP: 0010:[<ffffffff814ca9b1>]  [<ffffffff814ca9b1>] mutex_lock+0x21/0x50
RSP: 0018:ffff8803e4e33d30  EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000238 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffff8803e664c800 RDI: 0000000000000238
RBP: ffff8803e4e33d40 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000001 R12: 0000000000000000
R13: 0000000000000238 R14: ffff88041acb7200 R15: ffff88041c51ada0
FS:  0000000000000000(0000) GS:ffff880028200000(0000) knlGS:0000000000000000
CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
CR2: 0000000000000238 CR3: 0000000410143000 CR4: 00000000000006f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process scsi_wq_5 (pid: 5146, threadinfo ffff8803e4e32000, task ffff8803e4e294a0)
Stack:
 ffff8803e664c800 0000000000000000 ffff8803e4e33d70 ffffffffa001f06e
<0> ffff8803e4e33d60 ffff88041c51ada0 ffff88041acb7200 ffff88041bc0aa00
<0> ffff8803e4e33d90 ffffffffa0032b6c 0000000000000014 ffff88041acb7200
Call Trace:
 [<ffffffffa001f06e>] sas_port_delete_phy+0x2e/0xa0 [scsi_transport_sas]
 [<ffffffffa0032b6c>] sas_unregister_devs_sas_addr+0xac/0xe0 [libsas]
 [<ffffffffa0034914>] sas_ex_revalidate_domain+0x204/0x330 [libsas]
 [<ffffffffa00307f0>] ? sas_revalidate_domain+0x0/0x90 [libsas]
 [<ffffffffa0030855>] sas_revalidate_domain+0x65/0x90 [libsas]
 [<ffffffff8108c7d0>] worker_thread+0x170/0x2a0
 [<ffffffff81091ea0>] ? autoremove_wake_function+0x0/0x40
 [<ffffffff8108c660>] ? worker_thread+0x0/0x2a0
 [<ffffffff81091b36>] kthread+0x96/0xa0
 [<ffffffff810141ca>] child_rip+0xa/0x20
 [<ffffffff81091aa0>] ? kthread+0x0/0xa0
 [<ffffffff810141c0>] ? child_rip+0x0/0x20
Code: ff ff 85 c0 75 ed eb d6 66 90 55 48 89 e5 48 83 ec 10 48 89 1c 24 4c 89 64 24 08 0f 1f 44 00 00 48 89 fb e8 92 f4 ff ff 48 89 df <f0> ff 0f 79 05 e8 25 00 00 00 65 48 8b 04 25 08 cc 00 00 48 2d
RIP  [<ffffffff814ca9b1>] mutex_lock+0x21/0x50
 RSP <ffff8803e4e33d30>
CR2: 0000000000000238

The following patch is admittedly a band-aid, and does not solve the root cause, but it still is a good candidate for hardening as a pointer check before reference.

Signed-off-by: Mark Salyzyn <mark_salyzyn@xyus.xyratex.com>
Cc: Luben Tuikov <ltuikov@yahoo.com>
Cc: Darrick J Wong <djwong@us.ibm.com>
Cc: James Bottomley <jbottomley@parallels.com>
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Cc: Jack Wang <jack_wang@usish.com>

 sas_expander.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff -rup scsi-misc-2.6/drivers/scsi/libsas/sas_expander.c scsi-misc-2.6.new/drivers/scsi/libsas/sas_expander.c
--- scsi-misc-2.6/drivers/scsi/libsas/sas_expander.c	2011-08-31 08:32:21.000000000 -0400
+++ scsi-misc-2.6.new/drivers/scsi/libsas/sas_expander.c	2011-09-22 11:20:06.000000000 -0400
@@ -1769,10 +1769,12 @@ static void sas_unregister_devs_sas_addr
 		sas_disable_routing(parent, phy->attached_sas_addr);
 	}
 	memset(phy->attached_sas_addr, 0, SAS_ADDR_SIZE);
-	sas_port_delete_phy(phy->port, phy->phy);
-	if (phy->port->num_phys == 0)
-		sas_port_delete(phy->port);
-	phy->port = NULL;
+	if (phy->port) {
+		sas_port_delete_phy(phy->port, phy->phy);
+		if (phy->port->num_phys == 0)
+			sas_port_delete(phy->port);
+		phy->port = NULL;
+	}
 }
 
 static int sas_discover_bfs_by_root_level(struct domain_device *root,

[PATCH] [SCSI] pm8001 DEV_IS_GONE infinite retry

[Mark Salyzyn]

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

On the pm8001, when a device is in the process of going away (device
power off or hot plug), depending on the timing, the driver would return
SAS_PHY_DOWN as the return value to the queuecommand DEV_IS_GONE logic.
The net result is an near infinite retry (especially if SAS debugging is
enabled), the logs will fill with:

kernel: mpi_ssp_completion 2119:e21:SSP IO status 0x13 tag 0xcc1c0000
dlen=90 param=0xe
kernel: wwn=5000c50034069e86  cdb=12 00 00 00 5a 00 00 00 00 00 00 00 00
00 00 00
kernel: sas: lldd_execute_task returned: 138
kernel: sas: lldd_execute_task returned: 138
kernel: sas: lldd_execute_task returned: 138
kernel: sas: lldd_execute_task returned: 138
kernel: sas: lldd_execute_task returned: 138
kernel: sas: lldd_execute_task returned: 138
kernel: sas: lldd_execute_task returned: 138
. . .

This patch changes to leverage the port_attached logic to complete the
command with a status of PHY_DOWN so that the disposition can be handled
immediately and correctly.

The real patch is enclosed as an attachment since Outlook is my current
MTA.

Signed-off-by: Mark Salyzyn <mark_salyzyn@xyus.xyratex.com>
Cc: James Bottomley <jbottomley@parallels.com>
Cc: Jack Wang <jack_wang@usish.com>

 pm8001_sas.c |   15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff -ru scsi-misc-2.6/drivers/scsi/pm8001/pm8001_sas.c
scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_sas.c
--- scsi-misc-2.6/drivers/scsi/pm8001/pm8001_sas.c	2011-08-31
08:32:21.000000000 -0400
+++ scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_sas.c	2011-09-22
11:36:24.000000000 -0400
@@ -385,21 +385,8 @@
 	do {
 		dev = t->dev;
 		pm8001_dev = dev->lldd_dev;
-		if (DEV_IS_GONE(pm8001_dev)) {
-			if (pm8001_dev) {
-				PM8001_IO_DBG(pm8001_ha,
-					pm8001_printk("device %d not
ready.\n",
-					pm8001_dev->device_id));
-			} else {
-				PM8001_IO_DBG(pm8001_ha,
-					pm8001_printk("device %016llx
not "
-					"ready.\n",
SAS_ADDR(dev->sas_addr)));
-			}
-			rc = SAS_PHY_DOWN;
-			goto out_done;
-		}
 		port = &pm8001_ha->port[sas_find_local_port_id(dev)];
-		if (!port->port_attached) {
+		if (DEV_IS_GONE(pm8001_dev) || !port->port_attached) {
 			if (sas_protocol_ata(t->task_proto)) {
 				struct task_status_struct *ts =
&t->task_status;
 				ts->resp = SAS_TASK_UNDELIVERED;

______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 
\r

[-- Attachment #2: pm8001_DEV_IS_GONE.patch --]
[-- Type: application/octet-stream, Size: 2199 bytes --]

On the pm8001, when a device is in the process of going away (power off or hot plug), depending on the timing, the driver would return SAS_PHY_DOWN as the return value to the queuecommand DEV_IS_GONE logic. The net result is an near infinite retry (especially if SAS debugging is enabled), the logs will fill with:

kernel: mpi_ssp_completion 2119:e21:SSP IO status 0x13 tag 0xcc1c0000 dlen=90 param=0xe
kernel: wwn=5000c50034069e86  cdb=12 00 00 00 5a 00 00 00 00 00 00 00 00 00 00 00
kernel: sas: lldd_execute_task returned: 138
kernel: sas: lldd_execute_task returned: 138
kernel: sas: lldd_execute_task returned: 138
kernel: sas: lldd_execute_task returned: 138
kernel: sas: lldd_execute_task returned: 138
kernel: sas: lldd_execute_task returned: 138
kernel: sas: lldd_execute_task returned: 138
. . .

This patch changes to leverage the port_attached logic to complete the command with a status of PHY_DOWN so that the disposition can be handled immediately and correctly.

Signed-off-by: Mark Salyzyn <mark_salyzyn@xyus.xyratex.com>
Cc: James Bottomley <jbottomley@parallels.com>
Cc: Jack Wang <jack_wang@usish.com>

 pm8001_sas.c |   15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff -ru scsi-misc-2.6/drivers/scsi/pm8001/pm8001_sas.c scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_sas.c
--- scsi-misc-2.6/drivers/scsi/pm8001/pm8001_sas.c	2011-08-31 08:32:21.000000000 -0400
+++ scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_sas.c	2011-09-22 11:36:24.000000000 -0400
@@ -385,21 +385,8 @@
 	do {
 		dev = t->dev;
 		pm8001_dev = dev->lldd_dev;
-		if (DEV_IS_GONE(pm8001_dev)) {
-			if (pm8001_dev) {
-				PM8001_IO_DBG(pm8001_ha,
-					pm8001_printk("device %d not ready.\n",
-					pm8001_dev->device_id));
-			} else {
-				PM8001_IO_DBG(pm8001_ha,
-					pm8001_printk("device %016llx not "
-					"ready.\n", SAS_ADDR(dev->sas_addr)));
-			}
-			rc = SAS_PHY_DOWN;
-			goto out_done;
-		}
 		port = &pm8001_ha->port[sas_find_local_port_id(dev)];
-		if (!port->port_attached) {
+		if (DEV_IS_GONE(pm8001_dev) || !port->port_attached) {
 			if (sas_protocol_ata(t->task_proto)) {
 				struct task_status_struct *ts = &t->task_status;
 				ts->resp = SAS_TASK_UNDELIVERED;

Re: [PATCH] [SCSI] pm8001 DEV_IS_GONE infinite retry

[Jack Wang]

Hi Mark

Thanks for your fix. You can add my acked.
Acked-by: Jack Wang <jack_wang@usish.com>.

Ps, your signed-off-by mail addr is not same with your sender mail address,
is this a mistake?

Re [PATCH] [SCSI] pm8001 DEV_IS_GONE infinite retry
> 
> On the pm8001, when a device is in the process of going away (device
> power off or hot plug), depending on the timing, the driver would return
> SAS_PHY_DOWN as the return value to the queuecommand DEV_IS_GONE logic.
> The net result is an near infinite retry (especially if SAS debugging is
> enabled), the logs will fill with:
> 
> kernel: mpi_ssp_completion 2119:e21:SSP IO status 0x13 tag 0xcc1c0000
> dlen=90 param=0xe
> kernel: wwn=5000c50034069e86  cdb=12 00 00 00 5a 00 00 00 00 00 00 00 00
> 00 00 00
> kernel: sas: lldd_execute_task returned: 138
> kernel: sas: lldd_execute_task returned: 138
> kernel: sas: lldd_execute_task returned: 138
> kernel: sas: lldd_execute_task returned: 138
> kernel: sas: lldd_execute_task returned: 138
> kernel: sas: lldd_execute_task returned: 138
> kernel: sas: lldd_execute_task returned: 138
> . . .
> 
> This patch changes to leverage the port_attached logic to complete the
> command with a status of PHY_DOWN so that the disposition can be handled
> immediately and correctly.
> 
> The real patch is enclosed as an attachment since Outlook is my current
> MTA.
> 
> Signed-off-by: Mark Salyzyn <mark_salyzyn@xyus.xyratex.com>
> Cc: James Bottomley <jbottomley@parallels.com>
> Cc: Jack Wang <jack_wang@usish.com>
> 
>  pm8001_sas.c |   15 +--------------
>  1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff -ru scsi-misc-2.6/drivers/scsi/pm8001/pm8001_sas.c
> scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_sas.c
> --- scsi-misc-2.6/drivers/scsi/pm8001/pm8001_sas.c	2011-08-31
> 08:32:21.000000000 -0400
> +++ scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_sas.c	2011-09-22
> 11:36:24.000000000 -0400
> @@ -385,21 +385,8 @@
>  	do {
>  		dev = t->dev;
>  		pm8001_dev = dev->lldd_dev;
> -		if (DEV_IS_GONE(pm8001_dev)) {
> -			if (pm8001_dev) {
> -				PM8001_IO_DBG(pm8001_ha,
> -					pm8001_printk("device %d not
> ready.\n",
> -					pm8001_dev->device_id));
> -			} else {
> -				PM8001_IO_DBG(pm8001_ha,
> -					pm8001_printk("device %016llx
> not "
> -					"ready.\n",
> SAS_ADDR(dev->sas_addr)));
> -			}
> -			rc = SAS_PHY_DOWN;
> -			goto out_done;
> -		}
>  		port = &pm8001_ha->port[sas_find_local_port_id(dev)];
> -		if (!port->port_attached) {
> +		if (DEV_IS_GONE(pm8001_dev) || !port->port_attached) {
>  			if (sas_protocol_ata(t->task_proto)) {
>  				struct task_status_struct *ts =
> &t->task_status;
>  				ts->resp = SAS_TASK_UNDELIVERED;
> 
> ______________________________________________________________________
> This email may contain privileged or confidential information, which
should
> only be used for the purpose for which it was sent by Xyratex. No further
rights
> or licenses are granted to use such information. If you are not the
intended
> recipient of this message, please notify the sender by return and delete
it.
> You may not use, copy, disclose or rely on the information contained in
it.
> 
> Internet email is susceptible to data corruption, interception and
> unauthorised amendment for which Xyratex does not accept liability. While
we
> have taken reasonable precautions to ensure that this email is free of
viruses,
> Xyratex does not accept liability for the presence of any computer viruses
in
> this email, nor for any losses caused as a result of viruses.
> 
> Xyratex Technology Limited (03134912), Registered in England & Wales,
> Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
> 
> The Xyratex group of companies also includes, Xyratex Ltd, registered in
> Bermuda, Xyratex International Inc, registered in California, Xyratex
> (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co
Ltd
> registered in The People's Republic of China and Xyratex Japan Limited
> registered in Japan.
> ______________________________________________________________________
> 
> 

RE: [PATCH] [SCSI] pm8001 DEV_IS_GONE infinite retry

[Mark Salyzyn]

Ooops <grin> should be:

	mark_salyzyn@us.xyratex.com

Yes, it is a fumble-finger mistake! (NIS prefix)

James, please take note and correct as you propagate and join the acks
to this patch?

Sincerely -- Mark Salyzyn

	You can call me msalyzyn@xyratex.com, msalyzyn@us.xyratex.com,
mark_salyzyn@xyratex.com or mark_salyzyn@us.xyratex.com; but you can not
call me Al, Sally, Joe or mark_salyzyn@xyus.xyratex.com.

-----Original Message-----
From: Jack Wang [mailto:jack_wang@usish.com] 
Sent: Sunday, September 25, 2011 10:21 PM
To: Mark Salyzyn; linux-scsi@vger.kernel.org
Cc: 'James Bottomley'
Subject: Re: [PATCH] [SCSI] pm8001 DEV_IS_GONE infinite retry

Hi Mark

Thanks for your fix. You can add my acked.
Acked-by: Jack Wang <jack_wang@usish.com>.

Ps, your signed-off-by mail addr is not same with your sender mail
address,
is this a mistake?

Re [PATCH] [SCSI] pm8001 DEV_IS_GONE infinite retry
. . .
______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 

[PATCH] [SCSI] pm8001 missing break statements

[Mark Salyzyn]

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

Code Inspection: found two missing break directives. First one will
result in not retrying an a task that report
IO_OPEN_CNX_ERROR_HW_RESOURCE_BUSY, the second will result in cosmetic
debug printk conflicting statement stutter. Because checkpatch.pl came
up with a warning regarding unnecessary space before a newline on one of
the fragments associated with the diff context, I took the liberty of
fixing all the cases of this issue in the pair of files touched by this
defect. These cosmetic changes hide the break changes :-(

To help focus, break changes are in pm8001_hwi.c fragment line 1649 for
the IO_OPEN_CNX_ERROR_HW_RESOURCE_BUSY case statement and pm8001_sas.c
line 1000 deals with the conflicting debug print stutter.

The real patch is enclosed as an attachment since Outlook is my current
MTA. This code change has not shown any side effect in roughly a year of
random acts of testing.

Signed-off-by: Mark Salyzyn <mark_salyzyn@us.xyratex.com>
Cc: James Bottomley <jbottomley@parallels.com>
Cc: Jack Wang <jack_wang@usish.com>

 pm8001_hwi.c |   73
+++++++++++++++++++++++++++++------------------------------
 pm8001_sas.c |    7 +++--
 2 files changed, 41 insertions(+), 39 deletions(-)

diff -ru scsi-misc-2.6/drivers/scsi/pm8001/pm8001_hwi.c
scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_hwi.c
--- scsi-misc-2.6/drivers/scsi/pm8001/pm8001_hwi.c	2011-09-15
12:52:02.000000000 -0400
+++ scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_hwi.c	2011-09-26
10:41:45.000000000 -0400
@@ -567,11 +567,11 @@
 	value = pm8001_cr32(pm8001_ha, 0, 0x44);
 	offset = value & 0x03FFFFFF;
 	PM8001_INIT_DBG(pm8001_ha,
-		pm8001_printk("Scratchpad 0 Offset: %x \n", offset));
+		pm8001_printk("Scratchpad 0 Offset: %x\n", offset));
 	pcilogic = (value & 0xFC000000) >> 26;
 	pcibar = get_pci_bar_index(pcilogic);
 	PM8001_INIT_DBG(pm8001_ha,
-		pm8001_printk("Scratchpad 0 PCI BAR: %d \n", pcibar));
+		pm8001_printk("Scratchpad 0 PCI BAR: %d\n", pcibar));
 	pm8001_ha->main_cfg_tbl_addr = base_addr =
 		pm8001_ha->io_mem[pcibar].memvirtaddr + offset;
 	pm8001_ha->general_stat_tbl_addr =
@@ -1245,7 +1245,7 @@
 
 	if (mpi_msg_free_get(circularQ, 64, &pMessage) < 0) {
 		PM8001_IO_DBG(pm8001_ha,
-			pm8001_printk("No free mpi buffer \n"));
+			pm8001_printk("No free mpi buffer\n"));
 		return -1;
 	}
 	BUG_ON(!payload);
@@ -1262,7 +1262,7 @@
 	pm8001_cw32(pm8001_ha, circularQ->pi_pci_bar,
 		circularQ->pi_offset, circularQ->producer_idx);
 	PM8001_IO_DBG(pm8001_ha,
-		pm8001_printk("after PI= %d CI= %d \n",
circularQ->producer_idx,
+		pm8001_printk("after PI= %d CI= %d\n",
circularQ->producer_idx,
 		circularQ->consumer_index));
 	return 0;
 }
@@ -1474,7 +1474,7 @@
 	switch (status) {
 	case IO_SUCCESS:
 		PM8001_IO_DBG(pm8001_ha, pm8001_printk("IO_SUCCESS"
-			",param = %d \n", param));
+			",param = %d\n", param));
 		if (param == 0) {
 			ts->resp = SAS_TASK_COMPLETE;
 			ts->stat = SAM_STAT_GOOD;
@@ -1490,14 +1490,14 @@
 		break;
 	case IO_ABORTED:
 		PM8001_IO_DBG(pm8001_ha,
-			pm8001_printk("IO_ABORTED IOMB Tag \n"));
+			pm8001_printk("IO_ABORTED IOMB Tag\n"));
 		ts->resp = SAS_TASK_COMPLETE;
 		ts->stat = SAS_ABORTED_TASK;
 		break;
 	case IO_UNDERFLOW:
 		/* SSP Completion with error */
 		PM8001_IO_DBG(pm8001_ha, pm8001_printk("IO_UNDERFLOW"
-			",param = %d \n", param));
+			",param = %d\n", param));
 		ts->resp = SAS_TASK_COMPLETE;
 		ts->stat = SAS_DATA_UNDERRUN;
 		ts->residual = param;
@@ -1649,6 +1649,7 @@
 		ts->resp = SAS_TASK_COMPLETE;
 		ts->stat = SAS_OPEN_REJECT;
 		ts->open_rej_reason = SAS_OREJ_RSVD_RETRY;
+		break;
 	default:
 		PM8001_IO_DBG(pm8001_ha,
 			pm8001_printk("Unknown status 0x%x\n", status));
@@ -1937,14 +1938,14 @@
 				ts->buf_valid_size = sizeof(*resp);
 			} else
 				PM8001_IO_DBG(pm8001_ha,
-					pm8001_printk("response to large
\n"));
+					pm8001_printk("response to
large\n"));
 		}
 		if (pm8001_dev)
 			pm8001_dev->running_req--;
 		break;
 	case IO_ABORTED:
 		PM8001_IO_DBG(pm8001_ha,
-			pm8001_printk("IO_ABORTED IOMB Tag \n"));
+			pm8001_printk("IO_ABORTED IOMB Tag\n"));
 		ts->resp = SAS_TASK_COMPLETE;
 		ts->stat = SAS_ABORTED_TASK;
 		if (pm8001_dev)
@@ -2728,11 +2729,11 @@
 	u32 phy_op = le32_to_cpu(pPayload->phyop_phyid) & OP_BITS;
 	if (status != 0) {
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("%x phy execute %x phy op failed!
\n",
+			pm8001_printk("%x phy execute %x phy op
failed!\n",
 			phy_id, phy_op));
 	} else
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("%x phy execute %x phy op success!
\n",
+			pm8001_printk("%x phy execute %x phy op
success!\n",
 			phy_id, phy_op));
 	return 0;
 }
@@ -3018,7 +3019,7 @@
 		break;
 	case PORT_INVALID:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk(" PortInvalid portID %d \n",
port_id));
+			pm8001_printk(" PortInvalid portID %d\n",
port_id));
 		PM8001_MSG_DBG(pm8001_ha,
 			pm8001_printk(" Last phy Down and port
invalid\n"));
 		port->port_attached = 0;
@@ -3027,7 +3028,7 @@
 		break;
 	case PORT_IN_RESET:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk(" Port In Reset portID %d \n",
port_id));
+			pm8001_printk(" Port In Reset portID %d\n",
port_id));
 		break;
 	case PORT_NOT_ESTABLISHED:
 		PM8001_MSG_DBG(pm8001_ha,
@@ -3220,7 +3221,7 @@
 		pm8001_printk(" status = 0x%x\n", status));
 	for (i = 0; i < GENERAL_EVENT_PAYLOAD; i++)
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("inb_IOMB_payload[0x%x] 0x%x, \n",
i,
+			pm8001_printk("inb_IOMB_payload[0x%x] 0x%x,\n",
i,
 			pPayload->inb_IOMB_payload[i]));
 	return 0;
 }
@@ -3312,12 +3313,12 @@
 		break;
 	case HW_EVENT_SAS_PHY_UP:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_PHY_START_STATUS \n"));
+			pm8001_printk("HW_EVENT_PHY_START_STATUS\n"));
 		hw_event_sas_phy_up(pm8001_ha, piomb);
 		break;
 	case HW_EVENT_SATA_PHY_UP:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_SATA_PHY_UP \n"));
+			pm8001_printk("HW_EVENT_SATA_PHY_UP\n"));
 		hw_event_sata_phy_up(pm8001_ha, piomb);
 		break;
 	case HW_EVENT_PHY_STOP_STATUS:
@@ -3329,12 +3330,12 @@
 		break;
 	case HW_EVENT_SATA_SPINUP_HOLD:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_SATA_SPINUP_HOLD \n"));
+			pm8001_printk("HW_EVENT_SATA_SPINUP_HOLD\n"));
 		sas_ha->notify_phy_event(&phy->sas_phy,
PHYE_SPINUP_HOLD);
 		break;
 	case HW_EVENT_PHY_DOWN:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_PHY_DOWN \n"));
+			pm8001_printk("HW_EVENT_PHY_DOWN\n"));
 		sas_ha->notify_phy_event(&phy->sas_phy,
PHYE_LOSS_OF_SIGNAL);
 		phy->phy_attached = 0;
 		phy->phy_state = 0;
@@ -3446,7 +3447,7 @@
 		break;
 	case HW_EVENT_LINK_ERR_PHY_RESET_FAILED:
 		PM8001_MSG_DBG(pm8001_ha,
-
pm8001_printk("HW_EVENT_LINK_ERR_PHY_RESET_FAILED \n"));
+
pm8001_printk("HW_EVENT_LINK_ERR_PHY_RESET_FAILED\n"));
 		pm8001_hw_event_ack_req(pm8001_ha, 0,
 			HW_EVENT_LINK_ERR_PHY_RESET_FAILED,
 			port_id, phy_id, 0, 0);
@@ -3456,25 +3457,25 @@
 		break;
 	case HW_EVENT_PORT_RESET_TIMER_TMO:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_PORT_RESET_TIMER_TMO
\n"));
+
pm8001_printk("HW_EVENT_PORT_RESET_TIMER_TMO\n"));
 		sas_phy_disconnected(sas_phy);
 		phy->phy_attached = 0;
 		sas_ha->notify_port_event(sas_phy,
PORTE_LINK_RESET_ERR);
 		break;
 	case HW_EVENT_PORT_RECOVERY_TIMER_TMO:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_PORT_RECOVERY_TIMER_TMO
\n"));
+
pm8001_printk("HW_EVENT_PORT_RECOVERY_TIMER_TMO\n"));
 		sas_phy_disconnected(sas_phy);
 		phy->phy_attached = 0;
 		sas_ha->notify_port_event(sas_phy,
PORTE_LINK_RESET_ERR);
 		break;
 	case HW_EVENT_PORT_RECOVER:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_PORT_RECOVER \n"));
+			pm8001_printk("HW_EVENT_PORT_RECOVER\n"));
 		break;
 	case HW_EVENT_PORT_RESET_COMPLETE:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_PORT_RESET_COMPLETE
\n"));
+
pm8001_printk("HW_EVENT_PORT_RESET_COMPLETE\n"));
 		break;
 	case EVENT_BROADCAST_ASYNCH_EVENT:
 		PM8001_MSG_DBG(pm8001_ha,
@@ -3502,21 +3503,21 @@
 
 	switch (opc) {
 	case OPC_OUB_ECHO:
-		PM8001_MSG_DBG(pm8001_ha, pm8001_printk("OPC_OUB_ECHO
\n"));
+		PM8001_MSG_DBG(pm8001_ha,
pm8001_printk("OPC_OUB_ECHO\n"));
 		break;
 	case OPC_OUB_HW_EVENT:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("OPC_OUB_HW_EVENT \n"));
+			pm8001_printk("OPC_OUB_HW_EVENT\n"));
 		mpi_hw_event(pm8001_ha, piomb);
 		break;
 	case OPC_OUB_SSP_COMP:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("OPC_OUB_SSP_COMP \n"));
+			pm8001_printk("OPC_OUB_SSP_COMP\n"));
 		mpi_ssp_completion(pm8001_ha, piomb);
 		break;
 	case OPC_OUB_SMP_COMP:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("OPC_OUB_SMP_COMP \n"));
+			pm8001_printk("OPC_OUB_SMP_COMP\n"));
 		mpi_smp_completion(pm8001_ha, piomb);
 		break;
 	case OPC_OUB_LOCAL_PHY_CNTRL:
@@ -3526,26 +3527,26 @@
 		break;
 	case OPC_OUB_DEV_REGIST:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("OPC_OUB_DEV_REGIST \n"));
+			pm8001_printk("OPC_OUB_DEV_REGIST\n"));
 		mpi_reg_resp(pm8001_ha, piomb);
 		break;
 	case OPC_OUB_DEREG_DEV:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("unresgister the deviece \n"));
+			pm8001_printk("unresgister the deviece\n"));
 		mpi_dereg_resp(pm8001_ha, piomb);
 		break;
 	case OPC_OUB_GET_DEV_HANDLE:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("OPC_OUB_GET_DEV_HANDLE \n"));
+			pm8001_printk("OPC_OUB_GET_DEV_HANDLE\n"));
 		break;
 	case OPC_OUB_SATA_COMP:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("OPC_OUB_SATA_COMP \n"));
+			pm8001_printk("OPC_OUB_SATA_COMP\n"));
 		mpi_sata_completion(pm8001_ha, piomb);
 		break;
 	case OPC_OUB_SATA_EVENT:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("OPC_OUB_SATA_EVENT \n"));
+			pm8001_printk("OPC_OUB_SATA_EVENT\n"));
 		mpi_sata_event(pm8001_ha, piomb);
 		break;
 	case OPC_OUB_SSP_EVENT:
@@ -3858,19 +3859,19 @@
 	circularQ = &pm8001_ha->inbnd_q_tbl[0];
 	if (task->data_dir == PCI_DMA_NONE) {
 		ATAP = 0x04;  /* no data*/
-		PM8001_IO_DBG(pm8001_ha, pm8001_printk("no data \n"));
+		PM8001_IO_DBG(pm8001_ha, pm8001_printk("no data\n"));
 	} else if (likely(!task->ata_task.device_control_reg_update)) {
 		if (task->ata_task.dma_xfer) {
 			ATAP = 0x06; /* DMA */
-			PM8001_IO_DBG(pm8001_ha, pm8001_printk("DMA
\n"));
+			PM8001_IO_DBG(pm8001_ha,
pm8001_printk("DMA\n"));
 		} else {
 			ATAP = 0x05; /* PIO*/
-			PM8001_IO_DBG(pm8001_ha, pm8001_printk("PIO
\n"));
+			PM8001_IO_DBG(pm8001_ha,
pm8001_printk("PIO\n"));
 		}
 		if (task->ata_task.use_ncq &&
 			dev->sata_dev.command_set != ATAPI_COMMAND_SET)
{
 			ATAP = 0x07; /* FPDMA */
-			PM8001_IO_DBG(pm8001_ha, pm8001_printk("FPDMA
\n"));
+			PM8001_IO_DBG(pm8001_ha,
pm8001_printk("FPDMA\n"));
 		}
 	}
 	if (task->ata_task.use_ncq && pm8001_get_ncq_tag(task,
&hdr_tag))
diff -ru scsi-misc-2.6/drivers/scsi/pm8001/pm8001_sas.c
scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_sas.c
--- scsi-misc-2.6/drivers/scsi/pm8001/pm8001_sas.c	2011-09-15
12:52:02.000000000 -0400
+++ scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_sas.c	2011-09-26
10:40:37.000000000 -0400
@@ -651,7 +651,7 @@
 				flag = 1; /* directly sata*/
 		}
 	} /*register this device to HBA*/
-	PM8001_DISC_DBG(pm8001_ha, pm8001_printk("Found device \n"));
+	PM8001_DISC_DBG(pm8001_ha, pm8001_printk("Found device\n"));
 	PM8001_CHIP_DISP->reg_dev_req(pm8001_ha, pm8001_device, flag);
 	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 	wait_for_completion(&completion);
@@ -1000,13 +1000,14 @@
 		/* The task is still in Lun, release it then */
 		case TMF_RESP_FUNC_SUCC:
 			PM8001_EH_DBG(pm8001_ha,
-				pm8001_printk("The task is still in Lun
\n"));
+				pm8001_printk("The task is still in
Lun\n"));
+			break;
 		/* The task is not in Lun or failed, reset the phy */
 		case TMF_RESP_FUNC_FAILED:
 		case TMF_RESP_FUNC_COMPLETE:
 			PM8001_EH_DBG(pm8001_ha,
 			pm8001_printk("The task is not in Lun or
failed,"
-			" reset the phy \n"));
+			" reset the phy\n"));
 			break;
 		}
 	}

______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 
\r

[-- Attachment #2: pm8001_missing_breaks.patch --]
[-- Type: application/octet-stream, Size: 11641 bytes --]

Code Inspection: found two missing break directives. First one will result in not retrying an a task that report IO_OPEN_CNX_ERROR_HW_RESOURCE_BUSY, the second will result in cosmetic debug printk conflicting statement stutter. Because checkpatch.pl came up with a warning regarding unnecessary space before a newline on one of the fragments associated with the diff context, I took the liberty of fixing all the cases of this issue in the pair of files. These cosmetic changes hide the break changes.

To help focus, break changes are in pm8001_hwi.c fragment line 1649 for the IO_OPEN_CNX_ERROR_HW_RESOURCE_BUSY case statement and pm8001_sas.c line 1000 deals with the conflicting debug print stutter.

Signed-off-by: Mark Salyzyn <mark_salyzyn@us.xyratex.com>
Cc: James Bottomley <jbottomley@parallels.com>
Cc: Jack Wang <jack_wang@usish.com>

 pm8001_hwi.c |   73 +++++++++++++++++++++++++++++------------------------------
 pm8001_sas.c |    7 +++--
 2 files changed, 41 insertions(+), 39 deletions(-)

diff -ru scsi-misc-2.6/drivers/scsi/pm8001/pm8001_hwi.c scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_hwi.c
--- scsi-misc-2.6/drivers/scsi/pm8001/pm8001_hwi.c	2011-09-15 12:52:02.000000000 -0400
+++ scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_hwi.c	2011-09-26 10:41:45.000000000 -0400
@@ -567,11 +567,11 @@
 	value = pm8001_cr32(pm8001_ha, 0, 0x44);
 	offset = value & 0x03FFFFFF;
 	PM8001_INIT_DBG(pm8001_ha,
-		pm8001_printk("Scratchpad 0 Offset: %x \n", offset));
+		pm8001_printk("Scratchpad 0 Offset: %x\n", offset));
 	pcilogic = (value & 0xFC000000) >> 26;
 	pcibar = get_pci_bar_index(pcilogic);
 	PM8001_INIT_DBG(pm8001_ha,
-		pm8001_printk("Scratchpad 0 PCI BAR: %d \n", pcibar));
+		pm8001_printk("Scratchpad 0 PCI BAR: %d\n", pcibar));
 	pm8001_ha->main_cfg_tbl_addr = base_addr =
 		pm8001_ha->io_mem[pcibar].memvirtaddr + offset;
 	pm8001_ha->general_stat_tbl_addr =
@@ -1245,7 +1245,7 @@
 
 	if (mpi_msg_free_get(circularQ, 64, &pMessage) < 0) {
 		PM8001_IO_DBG(pm8001_ha,
-			pm8001_printk("No free mpi buffer \n"));
+			pm8001_printk("No free mpi buffer\n"));
 		return -1;
 	}
 	BUG_ON(!payload);
@@ -1262,7 +1262,7 @@
 	pm8001_cw32(pm8001_ha, circularQ->pi_pci_bar,
 		circularQ->pi_offset, circularQ->producer_idx);
 	PM8001_IO_DBG(pm8001_ha,
-		pm8001_printk("after PI= %d CI= %d \n", circularQ->producer_idx,
+		pm8001_printk("after PI= %d CI= %d\n", circularQ->producer_idx,
 		circularQ->consumer_index));
 	return 0;
 }
@@ -1474,7 +1474,7 @@
 	switch (status) {
 	case IO_SUCCESS:
 		PM8001_IO_DBG(pm8001_ha, pm8001_printk("IO_SUCCESS"
-			",param = %d \n", param));
+			",param = %d\n", param));
 		if (param == 0) {
 			ts->resp = SAS_TASK_COMPLETE;
 			ts->stat = SAM_STAT_GOOD;
@@ -1490,14 +1490,14 @@
 		break;
 	case IO_ABORTED:
 		PM8001_IO_DBG(pm8001_ha,
-			pm8001_printk("IO_ABORTED IOMB Tag \n"));
+			pm8001_printk("IO_ABORTED IOMB Tag\n"));
 		ts->resp = SAS_TASK_COMPLETE;
 		ts->stat = SAS_ABORTED_TASK;
 		break;
 	case IO_UNDERFLOW:
 		/* SSP Completion with error */
 		PM8001_IO_DBG(pm8001_ha, pm8001_printk("IO_UNDERFLOW"
-			",param = %d \n", param));
+			",param = %d\n", param));
 		ts->resp = SAS_TASK_COMPLETE;
 		ts->stat = SAS_DATA_UNDERRUN;
 		ts->residual = param;
@@ -1649,6 +1649,7 @@
 		ts->resp = SAS_TASK_COMPLETE;
 		ts->stat = SAS_OPEN_REJECT;
 		ts->open_rej_reason = SAS_OREJ_RSVD_RETRY;
+		break;
 	default:
 		PM8001_IO_DBG(pm8001_ha,
 			pm8001_printk("Unknown status 0x%x\n", status));
@@ -1937,14 +1938,14 @@
 				ts->buf_valid_size = sizeof(*resp);
 			} else
 				PM8001_IO_DBG(pm8001_ha,
-					pm8001_printk("response to large \n"));
+					pm8001_printk("response to large\n"));
 		}
 		if (pm8001_dev)
 			pm8001_dev->running_req--;
 		break;
 	case IO_ABORTED:
 		PM8001_IO_DBG(pm8001_ha,
-			pm8001_printk("IO_ABORTED IOMB Tag \n"));
+			pm8001_printk("IO_ABORTED IOMB Tag\n"));
 		ts->resp = SAS_TASK_COMPLETE;
 		ts->stat = SAS_ABORTED_TASK;
 		if (pm8001_dev)
@@ -2728,11 +2729,11 @@
 	u32 phy_op = le32_to_cpu(pPayload->phyop_phyid) & OP_BITS;
 	if (status != 0) {
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("%x phy execute %x phy op failed! \n",
+			pm8001_printk("%x phy execute %x phy op failed!\n",
 			phy_id, phy_op));
 	} else
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("%x phy execute %x phy op success! \n",
+			pm8001_printk("%x phy execute %x phy op success!\n",
 			phy_id, phy_op));
 	return 0;
 }
@@ -3018,7 +3019,7 @@
 		break;
 	case PORT_INVALID:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk(" PortInvalid portID %d \n", port_id));
+			pm8001_printk(" PortInvalid portID %d\n", port_id));
 		PM8001_MSG_DBG(pm8001_ha,
 			pm8001_printk(" Last phy Down and port invalid\n"));
 		port->port_attached = 0;
@@ -3027,7 +3028,7 @@
 		break;
 	case PORT_IN_RESET:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk(" Port In Reset portID %d \n", port_id));
+			pm8001_printk(" Port In Reset portID %d\n", port_id));
 		break;
 	case PORT_NOT_ESTABLISHED:
 		PM8001_MSG_DBG(pm8001_ha,
@@ -3220,7 +3221,7 @@
 		pm8001_printk(" status = 0x%x\n", status));
 	for (i = 0; i < GENERAL_EVENT_PAYLOAD; i++)
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("inb_IOMB_payload[0x%x] 0x%x, \n", i,
+			pm8001_printk("inb_IOMB_payload[0x%x] 0x%x,\n", i,
 			pPayload->inb_IOMB_payload[i]));
 	return 0;
 }
@@ -3312,12 +3313,12 @@
 		break;
 	case HW_EVENT_SAS_PHY_UP:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_PHY_START_STATUS \n"));
+			pm8001_printk("HW_EVENT_PHY_START_STATUS\n"));
 		hw_event_sas_phy_up(pm8001_ha, piomb);
 		break;
 	case HW_EVENT_SATA_PHY_UP:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_SATA_PHY_UP \n"));
+			pm8001_printk("HW_EVENT_SATA_PHY_UP\n"));
 		hw_event_sata_phy_up(pm8001_ha, piomb);
 		break;
 	case HW_EVENT_PHY_STOP_STATUS:
@@ -3329,12 +3330,12 @@
 		break;
 	case HW_EVENT_SATA_SPINUP_HOLD:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_SATA_SPINUP_HOLD \n"));
+			pm8001_printk("HW_EVENT_SATA_SPINUP_HOLD\n"));
 		sas_ha->notify_phy_event(&phy->sas_phy, PHYE_SPINUP_HOLD);
 		break;
 	case HW_EVENT_PHY_DOWN:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_PHY_DOWN \n"));
+			pm8001_printk("HW_EVENT_PHY_DOWN\n"));
 		sas_ha->notify_phy_event(&phy->sas_phy, PHYE_LOSS_OF_SIGNAL);
 		phy->phy_attached = 0;
 		phy->phy_state = 0;
@@ -3446,7 +3447,7 @@
 		break;
 	case HW_EVENT_LINK_ERR_PHY_RESET_FAILED:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_LINK_ERR_PHY_RESET_FAILED \n"));
+			pm8001_printk("HW_EVENT_LINK_ERR_PHY_RESET_FAILED\n"));
 		pm8001_hw_event_ack_req(pm8001_ha, 0,
 			HW_EVENT_LINK_ERR_PHY_RESET_FAILED,
 			port_id, phy_id, 0, 0);
@@ -3456,25 +3457,25 @@
 		break;
 	case HW_EVENT_PORT_RESET_TIMER_TMO:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_PORT_RESET_TIMER_TMO \n"));
+			pm8001_printk("HW_EVENT_PORT_RESET_TIMER_TMO\n"));
 		sas_phy_disconnected(sas_phy);
 		phy->phy_attached = 0;
 		sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
 		break;
 	case HW_EVENT_PORT_RECOVERY_TIMER_TMO:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_PORT_RECOVERY_TIMER_TMO \n"));
+			pm8001_printk("HW_EVENT_PORT_RECOVERY_TIMER_TMO\n"));
 		sas_phy_disconnected(sas_phy);
 		phy->phy_attached = 0;
 		sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
 		break;
 	case HW_EVENT_PORT_RECOVER:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_PORT_RECOVER \n"));
+			pm8001_printk("HW_EVENT_PORT_RECOVER\n"));
 		break;
 	case HW_EVENT_PORT_RESET_COMPLETE:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("HW_EVENT_PORT_RESET_COMPLETE \n"));
+			pm8001_printk("HW_EVENT_PORT_RESET_COMPLETE\n"));
 		break;
 	case EVENT_BROADCAST_ASYNCH_EVENT:
 		PM8001_MSG_DBG(pm8001_ha,
@@ -3502,21 +3503,21 @@
 
 	switch (opc) {
 	case OPC_OUB_ECHO:
-		PM8001_MSG_DBG(pm8001_ha, pm8001_printk("OPC_OUB_ECHO \n"));
+		PM8001_MSG_DBG(pm8001_ha, pm8001_printk("OPC_OUB_ECHO\n"));
 		break;
 	case OPC_OUB_HW_EVENT:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("OPC_OUB_HW_EVENT \n"));
+			pm8001_printk("OPC_OUB_HW_EVENT\n"));
 		mpi_hw_event(pm8001_ha, piomb);
 		break;
 	case OPC_OUB_SSP_COMP:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("OPC_OUB_SSP_COMP \n"));
+			pm8001_printk("OPC_OUB_SSP_COMP\n"));
 		mpi_ssp_completion(pm8001_ha, piomb);
 		break;
 	case OPC_OUB_SMP_COMP:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("OPC_OUB_SMP_COMP \n"));
+			pm8001_printk("OPC_OUB_SMP_COMP\n"));
 		mpi_smp_completion(pm8001_ha, piomb);
 		break;
 	case OPC_OUB_LOCAL_PHY_CNTRL:
@@ -3526,26 +3527,26 @@
 		break;
 	case OPC_OUB_DEV_REGIST:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("OPC_OUB_DEV_REGIST \n"));
+			pm8001_printk("OPC_OUB_DEV_REGIST\n"));
 		mpi_reg_resp(pm8001_ha, piomb);
 		break;
 	case OPC_OUB_DEREG_DEV:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("unresgister the deviece \n"));
+			pm8001_printk("unresgister the deviece\n"));
 		mpi_dereg_resp(pm8001_ha, piomb);
 		break;
 	case OPC_OUB_GET_DEV_HANDLE:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("OPC_OUB_GET_DEV_HANDLE \n"));
+			pm8001_printk("OPC_OUB_GET_DEV_HANDLE\n"));
 		break;
 	case OPC_OUB_SATA_COMP:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("OPC_OUB_SATA_COMP \n"));
+			pm8001_printk("OPC_OUB_SATA_COMP\n"));
 		mpi_sata_completion(pm8001_ha, piomb);
 		break;
 	case OPC_OUB_SATA_EVENT:
 		PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk("OPC_OUB_SATA_EVENT \n"));
+			pm8001_printk("OPC_OUB_SATA_EVENT\n"));
 		mpi_sata_event(pm8001_ha, piomb);
 		break;
 	case OPC_OUB_SSP_EVENT:
@@ -3858,19 +3859,19 @@
 	circularQ = &pm8001_ha->inbnd_q_tbl[0];
 	if (task->data_dir == PCI_DMA_NONE) {
 		ATAP = 0x04;  /* no data*/
-		PM8001_IO_DBG(pm8001_ha, pm8001_printk("no data \n"));
+		PM8001_IO_DBG(pm8001_ha, pm8001_printk("no data\n"));
 	} else if (likely(!task->ata_task.device_control_reg_update)) {
 		if (task->ata_task.dma_xfer) {
 			ATAP = 0x06; /* DMA */
-			PM8001_IO_DBG(pm8001_ha, pm8001_printk("DMA \n"));
+			PM8001_IO_DBG(pm8001_ha, pm8001_printk("DMA\n"));
 		} else {
 			ATAP = 0x05; /* PIO*/
-			PM8001_IO_DBG(pm8001_ha, pm8001_printk("PIO \n"));
+			PM8001_IO_DBG(pm8001_ha, pm8001_printk("PIO\n"));
 		}
 		if (task->ata_task.use_ncq &&
 			dev->sata_dev.command_set != ATAPI_COMMAND_SET) {
 			ATAP = 0x07; /* FPDMA */
-			PM8001_IO_DBG(pm8001_ha, pm8001_printk("FPDMA \n"));
+			PM8001_IO_DBG(pm8001_ha, pm8001_printk("FPDMA\n"));
 		}
 	}
 	if (task->ata_task.use_ncq && pm8001_get_ncq_tag(task, &hdr_tag))
diff -ru scsi-misc-2.6/drivers/scsi/pm8001/pm8001_sas.c scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_sas.c
--- scsi-misc-2.6/drivers/scsi/pm8001/pm8001_sas.c	2011-09-15 12:52:02.000000000 -0400
+++ scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_sas.c	2011-09-26 10:40:37.000000000 -0400
@@ -651,7 +651,7 @@
 				flag = 1; /* directly sata*/
 		}
 	} /*register this device to HBA*/
-	PM8001_DISC_DBG(pm8001_ha, pm8001_printk("Found device \n"));
+	PM8001_DISC_DBG(pm8001_ha, pm8001_printk("Found device\n"));
 	PM8001_CHIP_DISP->reg_dev_req(pm8001_ha, pm8001_device, flag);
 	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
 	wait_for_completion(&completion);
@@ -1000,13 +1000,14 @@
 		/* The task is still in Lun, release it then */
 		case TMF_RESP_FUNC_SUCC:
 			PM8001_EH_DBG(pm8001_ha,
-				pm8001_printk("The task is still in Lun \n"));
+				pm8001_printk("The task is still in Lun\n"));
+			break;
 		/* The task is not in Lun or failed, reset the phy */
 		case TMF_RESP_FUNC_FAILED:
 		case TMF_RESP_FUNC_COMPLETE:
 			PM8001_EH_DBG(pm8001_ha,
 			pm8001_printk("The task is not in Lun or failed,"
-			" reset the phy \n"));
+			" reset the phy\n"));
 			break;
 		}
 	}

RE: [PATCH] [SCSI] pm8001 missing break statements

[Jack Wang]

Hi Mark,

Thanks for fix.
Acked-by: Jack Wang <jack_wang@usish.com>

: [PATCH] [SCSI] pm8001 missing break statements
> 
> Code Inspection: found two missing break directives. First one will
> result in not retrying an a task that report
> IO_OPEN_CNX_ERROR_HW_RESOURCE_BUSY, the second will result in cosmetic
> debug printk conflicting statement stutter. Because checkpatch.pl came
> up with a warning regarding unnecessary space before a newline on one of
> the fragments associated with the diff context, I took the liberty of
> fixing all the cases of this issue in the pair of files touched by this
> defect. These cosmetic changes hide the break changes :-(
> 
> To help focus, break changes are in pm8001_hwi.c fragment line 1649 for
> the IO_OPEN_CNX_ERROR_HW_RESOURCE_BUSY case statement and pm8001_sas.c
> line 1000 deals with the conflicting debug print stutter.
> 
> The real patch is enclosed as an attachment since Outlook is my current
> MTA. This code change has not shown any side effect in roughly a year of
> random acts of testing.
> 
> Signed-off-by: Mark Salyzyn <mark_salyzyn@us.xyratex.com>
> Cc: James Bottomley <jbottomley@parallels.com>
> Cc: Jack Wang <jack_wang@usish.com>
> 
>  pm8001_hwi.c |   73
> +++++++++++++++++++++++++++++------------------------------
>  pm8001_sas.c |    7 +++--
>  2 files changed, 41 insertions(+), 39 deletions(-)
> 
> diff -ru scsi-misc-2.6/drivers/scsi/pm8001/pm8001_hwi.c
> scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_hwi.c
> --- scsi-misc-2.6/drivers/scsi/pm8001/pm8001_hwi.c	2011-09-15
> 12:52:02.000000000 -0400
> +++ scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_hwi.c	2011-09-26
> 10:41:45.000000000 -0400
> @@ -567,11 +567,11 @@
>  	value = pm8001_cr32(pm8001_ha, 0, 0x44);
>  	offset = value & 0x03FFFFFF;
>  	PM8001_INIT_DBG(pm8001_ha,
> -		pm8001_printk("Scratchpad 0 Offset: %x \n", offset));
> +		pm8001_printk("Scratchpad 0 Offset: %x\n", offset));
>  	pcilogic = (value & 0xFC000000) >> 26;
>  	pcibar = get_pci_bar_index(pcilogic);
>  	PM8001_INIT_DBG(pm8001_ha,
> -		pm8001_printk("Scratchpad 0 PCI BAR: %d \n", pcibar));
> +		pm8001_printk("Scratchpad 0 PCI BAR: %d\n", pcibar));
>  	pm8001_ha->main_cfg_tbl_addr = base_addr =
>  		pm8001_ha->io_mem[pcibar].memvirtaddr + offset;
>  	pm8001_ha->general_stat_tbl_addr =
> @@ -1245,7 +1245,7 @@
> 
>  	if (mpi_msg_free_get(circularQ, 64, &pMessage) < 0) {
>  		PM8001_IO_DBG(pm8001_ha,
> -			pm8001_printk("No free mpi buffer \n"));
> +			pm8001_printk("No free mpi buffer\n"));
>  		return -1;
>  	}
>  	BUG_ON(!payload);
> @@ -1262,7 +1262,7 @@
>  	pm8001_cw32(pm8001_ha, circularQ->pi_pci_bar,
>  		circularQ->pi_offset, circularQ->producer_idx);
>  	PM8001_IO_DBG(pm8001_ha,
> -		pm8001_printk("after PI= %d CI= %d \n",
> circularQ->producer_idx,
> +		pm8001_printk("after PI= %d CI= %d\n",
> circularQ->producer_idx,
>  		circularQ->consumer_index));
>  	return 0;
>  }
> @@ -1474,7 +1474,7 @@
>  	switch (status) {
>  	case IO_SUCCESS:
>  		PM8001_IO_DBG(pm8001_ha, pm8001_printk("IO_SUCCESS"
> -			",param = %d \n", param));
> +			",param = %d\n", param));
>  		if (param == 0) {
>  			ts->resp = SAS_TASK_COMPLETE;
>  			ts->stat = SAM_STAT_GOOD;
> @@ -1490,14 +1490,14 @@
>  		break;
>  	case IO_ABORTED:
>  		PM8001_IO_DBG(pm8001_ha,
> -			pm8001_printk("IO_ABORTED IOMB Tag \n"));
> +			pm8001_printk("IO_ABORTED IOMB Tag\n"));
>  		ts->resp = SAS_TASK_COMPLETE;
>  		ts->stat = SAS_ABORTED_TASK;
>  		break;
>  	case IO_UNDERFLOW:
>  		/* SSP Completion with error */
>  		PM8001_IO_DBG(pm8001_ha, pm8001_printk("IO_UNDERFLOW"
> -			",param = %d \n", param));
> +			",param = %d\n", param));
>  		ts->resp = SAS_TASK_COMPLETE;
>  		ts->stat = SAS_DATA_UNDERRUN;
>  		ts->residual = param;
> @@ -1649,6 +1649,7 @@
>  		ts->resp = SAS_TASK_COMPLETE;
>  		ts->stat = SAS_OPEN_REJECT;
>  		ts->open_rej_reason = SAS_OREJ_RSVD_RETRY;
> +		break;
>  	default:
>  		PM8001_IO_DBG(pm8001_ha,
>  			pm8001_printk("Unknown status 0x%x\n", status));
> @@ -1937,14 +1938,14 @@
>  				ts->buf_valid_size = sizeof(*resp);
>  			} else
>  				PM8001_IO_DBG(pm8001_ha,
> -					pm8001_printk("response to large
> \n"));
> +					pm8001_printk("response to
> large\n"));
>  		}
>  		if (pm8001_dev)
>  			pm8001_dev->running_req--;
>  		break;
>  	case IO_ABORTED:
>  		PM8001_IO_DBG(pm8001_ha,
> -			pm8001_printk("IO_ABORTED IOMB Tag \n"));
> +			pm8001_printk("IO_ABORTED IOMB Tag\n"));
>  		ts->resp = SAS_TASK_COMPLETE;
>  		ts->stat = SAS_ABORTED_TASK;
>  		if (pm8001_dev)
> @@ -2728,11 +2729,11 @@
>  	u32 phy_op = le32_to_cpu(pPayload->phyop_phyid) & OP_BITS;
>  	if (status != 0) {
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("%x phy execute %x phy op failed!
> \n",
> +			pm8001_printk("%x phy execute %x phy op
> failed!\n",
>  			phy_id, phy_op));
>  	} else
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("%x phy execute %x phy op success!
> \n",
> +			pm8001_printk("%x phy execute %x phy op
> success!\n",
>  			phy_id, phy_op));
>  	return 0;
>  }
> @@ -3018,7 +3019,7 @@
>  		break;
>  	case PORT_INVALID:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk(" PortInvalid portID %d \n",
> port_id));
> +			pm8001_printk(" PortInvalid portID %d\n",
> port_id));
>  		PM8001_MSG_DBG(pm8001_ha,
>  			pm8001_printk(" Last phy Down and port
> invalid\n"));
>  		port->port_attached = 0;
> @@ -3027,7 +3028,7 @@
>  		break;
>  	case PORT_IN_RESET:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk(" Port In Reset portID %d \n",
> port_id));
> +			pm8001_printk(" Port In Reset portID %d\n",
> port_id));
>  		break;
>  	case PORT_NOT_ESTABLISHED:
>  		PM8001_MSG_DBG(pm8001_ha,
> @@ -3220,7 +3221,7 @@
>  		pm8001_printk(" status = 0x%x\n", status));
>  	for (i = 0; i < GENERAL_EVENT_PAYLOAD; i++)
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("inb_IOMB_payload[0x%x] 0x%x, \n",
> i,
> +			pm8001_printk("inb_IOMB_payload[0x%x] 0x%x,\n",
> i,
>  			pPayload->inb_IOMB_payload[i]));
>  	return 0;
>  }
> @@ -3312,12 +3313,12 @@
>  		break;
>  	case HW_EVENT_SAS_PHY_UP:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("HW_EVENT_PHY_START_STATUS \n"));
> +			pm8001_printk("HW_EVENT_PHY_START_STATUS\n"));
>  		hw_event_sas_phy_up(pm8001_ha, piomb);
>  		break;
>  	case HW_EVENT_SATA_PHY_UP:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("HW_EVENT_SATA_PHY_UP \n"));
> +			pm8001_printk("HW_EVENT_SATA_PHY_UP\n"));
>  		hw_event_sata_phy_up(pm8001_ha, piomb);
>  		break;
>  	case HW_EVENT_PHY_STOP_STATUS:
> @@ -3329,12 +3330,12 @@
>  		break;
>  	case HW_EVENT_SATA_SPINUP_HOLD:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("HW_EVENT_SATA_SPINUP_HOLD \n"));
> +			pm8001_printk("HW_EVENT_SATA_SPINUP_HOLD\n"));
>  		sas_ha->notify_phy_event(&phy->sas_phy,
> PHYE_SPINUP_HOLD);
>  		break;
>  	case HW_EVENT_PHY_DOWN:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("HW_EVENT_PHY_DOWN \n"));
> +			pm8001_printk("HW_EVENT_PHY_DOWN\n"));
>  		sas_ha->notify_phy_event(&phy->sas_phy,
> PHYE_LOSS_OF_SIGNAL);
>  		phy->phy_attached = 0;
>  		phy->phy_state = 0;
> @@ -3446,7 +3447,7 @@
>  		break;
>  	case HW_EVENT_LINK_ERR_PHY_RESET_FAILED:
>  		PM8001_MSG_DBG(pm8001_ha,
> -
> pm8001_printk("HW_EVENT_LINK_ERR_PHY_RESET_FAILED \n"));
> +
> pm8001_printk("HW_EVENT_LINK_ERR_PHY_RESET_FAILED\n"));
>  		pm8001_hw_event_ack_req(pm8001_ha, 0,
>  			HW_EVENT_LINK_ERR_PHY_RESET_FAILED,
>  			port_id, phy_id, 0, 0);
> @@ -3456,25 +3457,25 @@
>  		break;
>  	case HW_EVENT_PORT_RESET_TIMER_TMO:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("HW_EVENT_PORT_RESET_TIMER_TMO
> \n"));
> +
> pm8001_printk("HW_EVENT_PORT_RESET_TIMER_TMO\n"));
>  		sas_phy_disconnected(sas_phy);
>  		phy->phy_attached = 0;
>  		sas_ha->notify_port_event(sas_phy,
> PORTE_LINK_RESET_ERR);
>  		break;
>  	case HW_EVENT_PORT_RECOVERY_TIMER_TMO:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("HW_EVENT_PORT_RECOVERY_TIMER_TMO
> \n"));
> +
> pm8001_printk("HW_EVENT_PORT_RECOVERY_TIMER_TMO\n"));
>  		sas_phy_disconnected(sas_phy);
>  		phy->phy_attached = 0;
>  		sas_ha->notify_port_event(sas_phy,
> PORTE_LINK_RESET_ERR);
>  		break;
>  	case HW_EVENT_PORT_RECOVER:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("HW_EVENT_PORT_RECOVER \n"));
> +			pm8001_printk("HW_EVENT_PORT_RECOVER\n"));
>  		break;
>  	case HW_EVENT_PORT_RESET_COMPLETE:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("HW_EVENT_PORT_RESET_COMPLETE
> \n"));
> +
> pm8001_printk("HW_EVENT_PORT_RESET_COMPLETE\n"));
>  		break;
>  	case EVENT_BROADCAST_ASYNCH_EVENT:
>  		PM8001_MSG_DBG(pm8001_ha,
> @@ -3502,21 +3503,21 @@
> 
>  	switch (opc) {
>  	case OPC_OUB_ECHO:
> -		PM8001_MSG_DBG(pm8001_ha, pm8001_printk("OPC_OUB_ECHO
> \n"));
> +		PM8001_MSG_DBG(pm8001_ha,
> pm8001_printk("OPC_OUB_ECHO\n"));
>  		break;
>  	case OPC_OUB_HW_EVENT:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("OPC_OUB_HW_EVENT \n"));
> +			pm8001_printk("OPC_OUB_HW_EVENT\n"));
>  		mpi_hw_event(pm8001_ha, piomb);
>  		break;
>  	case OPC_OUB_SSP_COMP:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("OPC_OUB_SSP_COMP \n"));
> +			pm8001_printk("OPC_OUB_SSP_COMP\n"));
>  		mpi_ssp_completion(pm8001_ha, piomb);
>  		break;
>  	case OPC_OUB_SMP_COMP:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("OPC_OUB_SMP_COMP \n"));
> +			pm8001_printk("OPC_OUB_SMP_COMP\n"));
>  		mpi_smp_completion(pm8001_ha, piomb);
>  		break;
>  	case OPC_OUB_LOCAL_PHY_CNTRL:
> @@ -3526,26 +3527,26 @@
>  		break;
>  	case OPC_OUB_DEV_REGIST:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("OPC_OUB_DEV_REGIST \n"));
> +			pm8001_printk("OPC_OUB_DEV_REGIST\n"));
>  		mpi_reg_resp(pm8001_ha, piomb);
>  		break;
>  	case OPC_OUB_DEREG_DEV:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("unresgister the deviece \n"));
> +			pm8001_printk("unresgister the deviece\n"));
>  		mpi_dereg_resp(pm8001_ha, piomb);
>  		break;
>  	case OPC_OUB_GET_DEV_HANDLE:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("OPC_OUB_GET_DEV_HANDLE \n"));
> +			pm8001_printk("OPC_OUB_GET_DEV_HANDLE\n"));
>  		break;
>  	case OPC_OUB_SATA_COMP:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("OPC_OUB_SATA_COMP \n"));
> +			pm8001_printk("OPC_OUB_SATA_COMP\n"));
>  		mpi_sata_completion(pm8001_ha, piomb);
>  		break;
>  	case OPC_OUB_SATA_EVENT:
>  		PM8001_MSG_DBG(pm8001_ha,
> -			pm8001_printk("OPC_OUB_SATA_EVENT \n"));
> +			pm8001_printk("OPC_OUB_SATA_EVENT\n"));
>  		mpi_sata_event(pm8001_ha, piomb);
>  		break;
>  	case OPC_OUB_SSP_EVENT:
> @@ -3858,19 +3859,19 @@
>  	circularQ = &pm8001_ha->inbnd_q_tbl[0];
>  	if (task->data_dir == PCI_DMA_NONE) {
>  		ATAP = 0x04;  /* no data*/
> -		PM8001_IO_DBG(pm8001_ha, pm8001_printk("no data \n"));
> +		PM8001_IO_DBG(pm8001_ha, pm8001_printk("no data\n"));
>  	} else if (likely(!task->ata_task.device_control_reg_update)) {
>  		if (task->ata_task.dma_xfer) {
>  			ATAP = 0x06; /* DMA */
> -			PM8001_IO_DBG(pm8001_ha, pm8001_printk("DMA
> \n"));
> +			PM8001_IO_DBG(pm8001_ha,
> pm8001_printk("DMA\n"));
>  		} else {
>  			ATAP = 0x05; /* PIO*/
> -			PM8001_IO_DBG(pm8001_ha, pm8001_printk("PIO
> \n"));
> +			PM8001_IO_DBG(pm8001_ha,
> pm8001_printk("PIO\n"));
>  		}
>  		if (task->ata_task.use_ncq &&
>  			dev->sata_dev.command_set != ATAPI_COMMAND_SET)
> {
>  			ATAP = 0x07; /* FPDMA */
> -			PM8001_IO_DBG(pm8001_ha, pm8001_printk("FPDMA
> \n"));
> +			PM8001_IO_DBG(pm8001_ha,
> pm8001_printk("FPDMA\n"));
>  		}
>  	}
>  	if (task->ata_task.use_ncq && pm8001_get_ncq_tag(task,
> &hdr_tag))
> diff -ru scsi-misc-2.6/drivers/scsi/pm8001/pm8001_sas.c
> scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_sas.c
> --- scsi-misc-2.6/drivers/scsi/pm8001/pm8001_sas.c	2011-09-15
> 12:52:02.000000000 -0400
> +++ scsi-misc-2.6.new/drivers/scsi/pm8001/pm8001_sas.c	2011-09-26
> 10:40:37.000000000 -0400
> @@ -651,7 +651,7 @@
>  				flag = 1; /* directly sata*/
>  		}
>  	} /*register this device to HBA*/
> -	PM8001_DISC_DBG(pm8001_ha, pm8001_printk("Found device \n"));
> +	PM8001_DISC_DBG(pm8001_ha, pm8001_printk("Found device\n"));
>  	PM8001_CHIP_DISP->reg_dev_req(pm8001_ha, pm8001_device, flag);
>  	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
>  	wait_for_completion(&completion);
> @@ -1000,13 +1000,14 @@
>  		/* The task is still in Lun, release it then */
>  		case TMF_RESP_FUNC_SUCC:
>  			PM8001_EH_DBG(pm8001_ha,
> -				pm8001_printk("The task is still in Lun
> \n"));
> +				pm8001_printk("The task is still in
> Lun\n"));
> +			break;
>  		/* The task is not in Lun or failed, reset the phy */
>  		case TMF_RESP_FUNC_FAILED:
>  		case TMF_RESP_FUNC_COMPLETE:
>  			PM8001_EH_DBG(pm8001_ha,
>  			pm8001_printk("The task is not in Lun or
> failed,"
> -			" reset the phy \n"));
> +			" reset the phy\n"));
>  			break;
>  		}
>  	}
> 
> ______________________________________________________________________
> This email may contain privileged or confidential information, which
should
> only be used for the purpose for which it was sent by Xyratex. No further
rights
> or licenses are granted to use such information. If you are not the
intended
> recipient of this message, please notify the sender by return and delete
it.
> You may not use, copy, disclose or rely on the information contained in
it.
> 
> Internet email is susceptible to data corruption, interception and
> unauthorised amendment for which Xyratex does not accept liability. While
we
> have taken reasonable precautions to ensure that this email is free of
viruses,
> Xyratex does not accept liability for the presence of any computer viruses
in
> this email, nor for any losses caused as a result of viruses.
> 
> Xyratex Technology Limited (03134912), Registered in England & Wales,
> Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
> 
> The Xyratex group of companies also includes, Xyratex Ltd, registered in
> Bermuda, Xyratex International Inc, registered in California, Xyratex
> (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co
Ltd
> registered in The People's Republic of China and Xyratex Japan Limited
> registered in Japan.
> ______________________________________________________________________
> 
> 

Re: [PATCH] [SCSI] libsas panic when single phy disabled on a wide port

[Jack Wang]

I can reproduce this bug, so I'm ok to get this fix in.

[PATCH] [SCSI] libsas panic when single phy disabled on a wide port
> 
> When a wide port is being utilized to a target, if one disables only one
> of the
> phys, we get an OS crash:
> 
> BUG: unable to handle kernel NULL pointer dereference at
> 0000000000000238
> IP: [<ffffffff814ca9b1>] mutex_lock+0x21/0x50
> PGD 4103f5067 PUD 41dba9067 PMD 0
> Oops: 0002 [#1] SMP
> last sysfs file: /sys/bus/pci/slots/5/address
> CPU 0
> Modules linked in: pm8001(U) ses enclosure fuse nfsd exportfs autofs4
> ipmi_devintf ipmi_si ipmi_msghandler nfs lockd fscache nfs_acl
> auth_rpcgss 8021q fcoe libfcoe garp libfc scsi_transport_fc stp scsi_tgt
> llc sunrpc cpufreq_ondemand acpi_cpufreq freq_table ipv6 sr_mod cdrom
> dm_mirror dm_region_hash dm_log uinput sg i2c_i801 i2c_core iTCO_wdt
> iTCO_vendor_support e1000e mlx4_ib ib_mad ib_core mlx4_en mlx4_core ext3
> jbd mbcache sd_mod crc_t10dif usb_storage ata_generic pata_acpi ata_piix
> libsas(U) scsi_transport_sas dm_mod [last unloaded: pm8001]
> 
> Modules linked in: pm8001(U) ses enclosure fuse nfsd exportfs autofs4
> ipmi_devintf ipmi_si ipmi_msghandler nfs lockd fscache nfs_acl
> auth_rpcgss 8021q fcoe libfcoe garp libfc scsi_transport_fc stp scsi_tgt
> llc sunrpc cpufreq_ondemand acpi_cpufreq freq_table ipv6 sr_mod cdrom
> dm_mirror dm_region_hash dm_log uinput sg i2c_i801 i2c_core iTCO_wdt
> iTCO_vendor_support e1000e mlx4_ib ib_mad ib_core mlx4_en mlx4_core ext3
> jbd mbcache sd_mod crc_t10dif usb_storage ata_generic pata_acpi ata_piix
> libsas(U) scsi_transport_sas dm_mod [last unloaded: pm8001]
> Pid: 5146, comm: scsi_wq_5 Not tainted
> 2.6.32-71.29.1.el6.lustre.7.x86_64 #1 Storage Server
> RIP: 0010:[<ffffffff814ca9b1>]  [<ffffffff814ca9b1>]
> mutex_lock+0x21/0x50
> RSP: 0018:ffff8803e4e33d30  EFLAGS: 00010246
> RAX: 0000000000000000 RBX: 0000000000000238 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: ffff8803e664c800 RDI: 0000000000000238
> RBP: ffff8803e4e33d40 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000001 R12: 0000000000000000
> R13: 0000000000000238 R14: ffff88041acb7200 R15: ffff88041c51ada0
> FS:  0000000000000000(0000) GS:ffff880028200000(0000)
> knlGS:0000000000000000
> CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
> CR2: 0000000000000238 CR3: 0000000410143000 CR4: 00000000000006f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process scsi_wq_5 (pid: 5146, threadinfo ffff8803e4e32000, task
> ffff8803e4e294a0)
> Stack:
>  ffff8803e664c800 0000000000000000 ffff8803e4e33d70 ffffffffa001f06e
> <0> ffff8803e4e33d60 ffff88041c51ada0 ffff88041acb7200 ffff88041bc0aa00
> <0> ffff8803e4e33d90 ffffffffa0032b6c 0000000000000014 ffff88041acb7200
> Call Trace:
>  [<ffffffffa001f06e>] sas_port_delete_phy+0x2e/0xa0 [scsi_transport_sas]
>  [<ffffffffa0032b6c>] sas_unregister_devs_sas_addr+0xac/0xe0 [libsas]
>  [<ffffffffa0034914>] sas_ex_revalidate_domain+0x204/0x330 [libsas]
>  [<ffffffffa00307f0>] ? sas_revalidate_domain+0x0/0x90 [libsas]
>  [<ffffffffa0030855>] sas_revalidate_domain+0x65/0x90 [libsas]
>  [<ffffffff8108c7d0>] worker_thread+0x170/0x2a0
>  [<ffffffff81091ea0>] ? autoremove_wake_function+0x0/0x40
>  [<ffffffff8108c660>] ? worker_thread+0x0/0x2a0
>  [<ffffffff81091b36>] kthread+0x96/0xa0
>  [<ffffffff810141ca>] child_rip+0xa/0x20
>  [<ffffffff81091aa0>] ? kthread+0x0/0xa0
>  [<ffffffff810141c0>] ? child_rip+0x0/0x20
> Code: ff ff 85 c0 75 ed eb d6 66 90 55 48 89 e5 48 83 ec 10 48 89 1c 24
> 4c 89 64 24 08 0f 1f 44 00 00 48 89 fb e8 92 f4 ff ff 48 89 df <f0> ff
> 0f 79 05 e8 25 00 00 00 65 48 8b 04 25 08 cc 00 00 48 2d
> RIP  [<ffffffff814ca9b1>] mutex_lock+0x21/0x50
>  RSP <ffff8803e4e33d30>
> CR2: 0000000000000238
> 
> The following patch is admittedly a band-aid, and does not solve the
> root cause, but it still is a good candidate for hardening as a pointer
> check before reference.
> 
> The real patch is enclosed as an attachment since Outlook is my current
> MTA.
> 
> Signed-off-by: Mark Salyzyn <mark_salyzyn@xyus.xyratex.com>
> Cc: Luben Tuikov <ltuikov@yahoo.com>
> Cc: Darrick J Wong <djwong@us.ibm.com>
> Cc: James Bottomley <jbottomley@parallels.com>
> Cc: Xiangliang Yu <yuxiangl@marvell.com>
> Cc: Jack Wang <jack_wang@usish.com>
> 
>  sas_expander.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff -rup scsi-misc-2.6/drivers/scsi/libsas/sas_expander.c
> scsi-misc-2.6.new/drivers/scsi/libsas/sas_expander.c
> --- scsi-misc-2.6/drivers/scsi/libsas/sas_expander.c	2011-08-31
> 08:32:21.000000000 -0400
> +++ scsi-misc-2.6.new/drivers/scsi/libsas/sas_expander.c
> 2011-09-22 11:20:06.000000000 -0400
> @@ -1769,10 +1769,12 @@ static void sas_unregister_devs_sas_addr
>  		sas_disable_routing(parent, phy->attached_sas_addr);
>  	}
>  	memset(phy->attached_sas_addr, 0, SAS_ADDR_SIZE);
> -	sas_port_delete_phy(phy->port, phy->phy);
> -	if (phy->port->num_phys == 0)
> -		sas_port_delete(phy->port);
> -	phy->port = NULL;
> +	if (phy->port) {
> +		sas_port_delete_phy(phy->port, phy->phy);
> +		if (phy->port->num_phys == 0)
> +			sas_port_delete(phy->port);
> +		phy->port = NULL;
> +	}
>  }
> 
>  static int sas_discover_bfs_by_root_level(struct domain_device *root,
> 
> ______________________________________________________________________
> This email may contain privileged or confidential information, which
should
> only be used for the purpose for which it was sent by Xyratex. No further
rights
> or licenses are granted to use such information. If you are not the
intended
> recipient of this message, please notify the sender by return and delete
it.
> You may not use, copy, disclose or rely on the information contained in
it.
> 
> Internet email is susceptible to data corruption, interception and
> unauthorised amendment for which Xyratex does not accept liability. While
we
> have taken reasonable precautions to ensure that this email is free of
viruses,
> Xyratex does not accept liability for the presence of any computer viruses
in
> this email, nor for any losses caused as a result of viruses.
> 
> Xyratex Technology Limited (03134912), Registered in England & Wales,
> Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
> 
> The Xyratex group of companies also includes, Xyratex Ltd, registered in
> Bermuda, Xyratex International Inc, registered in California, Xyratex
> (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co
Ltd
> registered in The People's Republic of China and Xyratex Japan Limited
> registered in Japan.
> ______________________________________________________________________
> 
> 

Re: [PATCH] [SCSI] libsas panic when single phy disabled on a wide port

[Dan Williams]

On Thu, Sep 29, 2011 at 7:21 PM, Jack Wang <jack_wang@usish.com> wrote:
> I can reproduce this bug, so I'm ok to get this fix in.
>

Mark admits this is a band aid.  I'd like to understand the root cause
of why the port is NULL here.

RE: [PATCH] [SCSI] libsas panic when single phy disabled on a wide port

[Mark Salyzyn]

100% agree with Dan! In fact it is my fault for not digging deeper when
I had the duplication relatively close at hand (but production pressures
had me move on, in my defense). I definitely attacked the symptom. The
patch is a piece of hardening, which can remain as paranoia after the
root cause has properly been discovered (turned into a BUG() likely).

I have lost the resources to duplicate this problem at the moment.

Sincerely -- Mark Salyzyn

-----Original Message-----
From: dan.j.williams@gmail.com [mailto:dan.j.williams@gmail.com] On
Behalf Of Dan Williams
Sent: Friday, September 30, 2011 9:43 PM
To: Jack Wang
Cc: Mark Salyzyn; linux-scsi@vger.kernel.org; Darrick Wong; Xiangliang
Yu; Luben Tuikov; James Bottomley
Subject: Re: [PATCH] [SCSI] libsas panic when single phy disabled on a
wide port

On Thu, Sep 29, 2011 at 7:21 PM, Jack Wang <jack_wang@usish.com> wrote:
> I can reproduce this bug, so I'm ok to get this fix in.
>

Mark admits this is a band aid.  I'd like to understand the root cause
of why the port is NULL here.
______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 

RE: [PATCH] [SCSI] libsas panic when single phy disabled on a wide port

[Mark Salyzyn]

I am talking to myself ;->. Dan (et al), I inspected deeper and I
*think* I see the culprit in sas_deform_port().

Could the problem be that the code should have been refactored to:

+	BUG_ON(!phy->port);
	sas_port_delete_phy(phy->port, phy->phy);
-	if (phy->port->num_phys == 0)
+	if (phy->port->num_phys == 0) {
	        sas_port_delete(phy->port);
-	phy->port = NULL;
+		phy->port = NULL;
+	}

And also fixed up is sas_ex_discover_end_dev() at out_err (this is on
soft territory, need to dig into sas_port_delete):

 out_free:
-       sas_port_delete(phy->port);
+	  if (phy->port->num_phys == 0) {
+	       sas_port_delete(phy->port);
 out_err:
-       phy->port = NULL;
+	       phy->port = NULL;
+	  }

And finally in sas_prot.c at sas_deform_port() (simplified, I still view
this as split-brained though):

        if (port->num_phys == 1) {
                if (dev && gone)
                        dev->gone = 1;
                sas_unregister_domain_devices(port);
                sas_port_delete(port->port);
                port->port = NULL;
-       } else
+	  } else {
                sas_port_delete_phy(port->port, phy->phy);
+		    if (si->dft->lldd_port_deformed)
+				si->dfp->lldd_port_deformed(phy);
+		    return;
+	  }

        if (si->dft->lldd_port_deformed)
                si->dft->lldd_port_deformed(phy);

        spin_lock_irqsave(&sas_ha->phy_port_lock, flags);
        spin_lock(&port->phy_list_lock);

        list_del_init(&phy->port_phy_el);
        phy->port = NULL;
        port->num_phys--;
        port->phy_mask &= ~(1U << phy->id);

        if (port->num_phys == 0) {
                INIT_LIST_HEAD(&port->phy_list);
                memset(port->sas_addr, 0, SAS_ADDR_SIZE);
                memset(port->attached_sas_addr, 0, SAS_ADDR_SIZE);
                port->class = 0;
                port->iproto = 0;
                port->tproto = 0;
                port->oob_mode = 0;
                port->phy_mask = 0;
        }
        spin_unlock(&port->phy_list_lock);
        spin_unlock_irqrestore(&sas_ha->phy_port_lock, flags);

        return;

The way I see it, sas_deform_port() is the culprit that is clearing out
phy->port when the num_phys is equal to two, falling through and
decrementing the count one more time.

Sincerely -- Mark Salyzyn

-----Original Message-----
From: linux-scsi-owner@vger.kernel.org
[mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Mark Salyzyn
Sent: Monday, October 03, 2011 9:08 AM
To: Dan Williams; Jack Wang
Cc: linux-scsi@vger.kernel.org; Darrick Wong; Xiangliang Yu; Luben
Tuikov; James Bottomley
Subject: RE: [PATCH] [SCSI] libsas panic when single phy disabled on a
wide port

100% agree with Dan! In fact it is my fault for not digging deeper when
I had the duplication relatively close at hand (but production pressures
had me move on, in my defense). I definitely attacked the symptom. The
patch is a piece of hardening, which can remain as paranoia after the
root cause has properly been discovered (turned into a BUG() likely).

I have lost the resources to duplicate this problem at the moment.

Sincerely -- Mark Salyzyn

-----Original Message-----
From: dan.j.williams@gmail.com [mailto:dan.j.williams@gmail.com] On
Behalf Of Dan Williams
Sent: Friday, September 30, 2011 9:43 PM
To: Jack Wang
Cc: Mark Salyzyn; linux-scsi@vger.kernel.org; Darrick Wong; Xiangliang
Yu; Luben Tuikov; James Bottomley
Subject: Re: [PATCH] [SCSI] libsas panic when single phy disabled on a
wide port

On Thu, Sep 29, 2011 at 7:21 PM, Jack Wang <jack_wang@usish.com> wrote:
> I can reproduce this bug, so I'm ok to get this fix in.
>

Mark admits this is a band aid.  I'd like to understand the root cause
of why the port is NULL here.
______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 

RE: [PATCH] [SCSI] libsas panic when single phy disabled on a wide port

[Jack Wang]

Hi Mark,

Agree with your analysis, I'll verify when back to office.

Jack
RE: [PATCH] [SCSI] libsas panic when single phy disabled on a wide port
> 
> I am talking to myself ;->. Dan (et al), I inspected deeper and I
> *think* I see the culprit in sas_deform_port().
> 
> Could the problem be that the code should have been refactored to:
> 
> +	BUG_ON(!phy->port);
> 	sas_port_delete_phy(phy->port, phy->phy);
> -	if (phy->port->num_phys == 0)
> +	if (phy->port->num_phys == 0) {
> 	        sas_port_delete(phy->port);
> -	phy->port = NULL;
> +		phy->port = NULL;
> +	}
> 
> And also fixed up is sas_ex_discover_end_dev() at out_err (this is on
> soft territory, need to dig into sas_port_delete):
> 
>  out_free:
> -       sas_port_delete(phy->port);
> +	  if (phy->port->num_phys == 0) {
> +	       sas_port_delete(phy->port);
>  out_err:
> -       phy->port = NULL;
> +	       phy->port = NULL;
> +	  }
> 
> And finally in sas_prot.c at sas_deform_port() (simplified, I still view
> this as split-brained though):
> 
>         if (port->num_phys == 1) {
>                 if (dev && gone)
>                         dev->gone = 1;
>                 sas_unregister_domain_devices(port);
>                 sas_port_delete(port->port);
>                 port->port = NULL;
> -       } else
> +	  } else {
>                 sas_port_delete_phy(port->port, phy->phy);
> +		    if (si->dft->lldd_port_deformed)
> +				si->dfp->lldd_port_deformed(phy);
> +		    return;
> +	  }
> 
>         if (si->dft->lldd_port_deformed)
>                 si->dft->lldd_port_deformed(phy);
> 
>         spin_lock_irqsave(&sas_ha->phy_port_lock, flags);
>         spin_lock(&port->phy_list_lock);
> 
>         list_del_init(&phy->port_phy_el);
>         phy->port = NULL;
>         port->num_phys--;
>         port->phy_mask &= ~(1U << phy->id);
> 
>         if (port->num_phys == 0) {
>                 INIT_LIST_HEAD(&port->phy_list);
>                 memset(port->sas_addr, 0, SAS_ADDR_SIZE);
>                 memset(port->attached_sas_addr, 0, SAS_ADDR_SIZE);
>                 port->class = 0;
>                 port->iproto = 0;
>                 port->tproto = 0;
>                 port->oob_mode = 0;
>                 port->phy_mask = 0;
>         }
>         spin_unlock(&port->phy_list_lock);
>         spin_unlock_irqrestore(&sas_ha->phy_port_lock, flags);
> 
>         return;
> 
> The way I see it, sas_deform_port() is the culprit that is clearing out
> phy->port when the num_phys is equal to two, falling through and
> decrementing the count one more time.
> 
> Sincerely -- Mark Salyzyn
> 
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org
> [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Mark Salyzyn
> Sent: Monday, October 03, 2011 9:08 AM
> To: Dan Williams; Jack Wang
> Cc: linux-scsi@vger.kernel.org; Darrick Wong; Xiangliang Yu; Luben
> Tuikov; James Bottomley
> Subject: RE: [PATCH] [SCSI] libsas panic when single phy disabled on a
> wide port
> 
> 100% agree with Dan! In fact it is my fault for not digging deeper when
> I had the duplication relatively close at hand (but production pressures
> had me move on, in my defense). I definitely attacked the symptom. The
> patch is a piece of hardening, which can remain as paranoia after the
> root cause has properly been discovered (turned into a BUG() likely).
> 
> I have lost the resources to duplicate this problem at the moment.
> 
> Sincerely -- Mark Salyzyn
> 
> -----Original Message-----
> From: dan.j.williams@gmail.com [mailto:dan.j.williams@gmail.com] On
> Behalf Of Dan Williams
> Sent: Friday, September 30, 2011 9:43 PM
> To: Jack Wang
> Cc: Mark Salyzyn; linux-scsi@vger.kernel.org; Darrick Wong; Xiangliang
> Yu; Luben Tuikov; James Bottomley
> Subject: Re: [PATCH] [SCSI] libsas panic when single phy disabled on a
> wide port
> 
> On Thu, Sep 29, 2011 at 7:21 PM, Jack Wang <jack_wang@usish.com> wrote:
> > I can reproduce this bug, so I'm ok to get this fix in.
> >
> 
> Mark admits this is a band aid.  I'd like to understand the root cause
> of why the port is NULL here.
> ______________________________________________________________________
> This email may contain privileged or confidential information, which
should
> only be used for the purpose for which it was sent by Xyratex. No further
rights
> or licenses are granted to use such information. If you are not the
intended
> recipient of this message, please notify the sender by return and delete
it.
> You may not use, copy, disclose or rely on the information contained in
it.
> 
> Internet email is susceptible to data corruption, interception and
> unauthorised amendment for which Xyratex does not accept liability. While
we
> have taken reasonable precautions to ensure that this email is free of
viruses,
> Xyratex does not accept liability for the presence of any computer viruses
in
> this email, nor for any losses caused as a result of viruses.
> 
> Xyratex Technology Limited (03134912), Registered in England & Wales,
> Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
> 
> The Xyratex group of companies also includes, Xyratex Ltd, registered in
> Bermuda, Xyratex International Inc, registered in California, Xyratex
> (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co
Ltd
> registered in The People's Republic of China and Xyratex Japan Limited
> registered in Japan.
> ______________________________________________________________________
> 
> 
> --
> 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

Re: [PATCH] [SCSI] libsas panic when single phy disabled on a wide port

[Dan Williams]

On Mon, Oct 3, 2011 at 8:58 AM, Mark Salyzyn
<mark_salyzyn@us.xyratex.com> wrote:
> I am talking to myself ;->. Dan (et al), I inspected deeper and I
> *think* I see the culprit in sas_deform_port().

Thanks Mark, some notes below to help the investigation...

>
> Could the problem be that the code should have been refactored to:
>
> +       BUG_ON(!phy->port);
>        sas_port_delete_phy(phy->port, phy->phy);
> -       if (phy->port->num_phys == 0)
> +       if (phy->port->num_phys == 0) {
>                sas_port_delete(phy->port);
> -       phy->port = NULL;
> +               phy->port = NULL;
> +       }
>
> And also fixed up is sas_ex_discover_end_dev() at out_err (this is on
> soft territory, need to dig into sas_port_delete):
>
>  out_free:
> -       sas_port_delete(phy->port);
> +         if (phy->port->num_phys == 0) {
> +              sas_port_delete(phy->port);
>  out_err:
> -       phy->port = NULL;
> +              phy->port = NULL;
> +         }

I think we are ok in this case because we checked for wide port
membership before entering this routine, so the number of phys could
only ever be 1 at this point, right?

>
> And finally in sas_prot.c at sas_deform_port() (simplified, I still view
> this as split-brained though):
>
>        if (port->num_phys == 1) {
>                if (dev && gone)
>                        dev->gone = 1;
>                sas_unregister_domain_devices(port);
>                sas_port_delete(port->port);
>                port->port = NULL;
> -       } else
> +         } else {
>                sas_port_delete_phy(port->port, phy->phy);
> +                   if (si->dft->lldd_port_deformed)
> +                               si->dfp->lldd_port_deformed(phy);
> +                   return;
> +         }
>
>        if (si->dft->lldd_port_deformed)
>                si->dft->lldd_port_deformed(phy);
>
>        spin_lock_irqsave(&sas_ha->phy_port_lock, flags);
>        spin_lock(&port->phy_list_lock);
>
>        list_del_init(&phy->port_phy_el);
>        phy->port = NULL;
>        port->num_phys--;
>        port->phy_mask &= ~(1U << phy->id);
>
>        if (port->num_phys == 0) {
>                INIT_LIST_HEAD(&port->phy_list);
>                memset(port->sas_addr, 0, SAS_ADDR_SIZE);
>                memset(port->attached_sas_addr, 0, SAS_ADDR_SIZE);
>                port->class = 0;
>                port->iproto = 0;
>                port->tproto = 0;
>                port->oob_mode = 0;
>                port->phy_mask = 0;
>        }
>        spin_unlock(&port->phy_list_lock);
>        spin_unlock_irqrestore(&sas_ha->phy_port_lock, flags);
>
>        return;
>
> The way I see it, sas_deform_port() is the culprit that is clearing out
> phy->port when the num_phys is equal to two, falling through and
> decrementing the count one more time.

That initially looked convincing... but on closer look
sas_port_delete_phy is reducing the number of phys in the transport
class representation of the port (struct sas_port), while the
decrement that sas_deform port is doing is operating the on the
host-local port (struct asd_sas_port).

What is your reproduction scenario for this bug?  We see this in our
larger topology hotplug tests, but I don't think we have a simple
reproducer.

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

Re: [PATCH] [SCSI] libsas panic when single phy disabled on a wide port

[Dan Williams]

On Tue, Oct 4, 2011 at 4:30 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> That initially looked convincing... but on closer look
> sas_port_delete_phy is reducing the number of phys in the transport
> class representation of the port (struct sas_port), while the
> decrement that sas_deform port is doing is operating the on the
> host-local port (struct asd_sas_port).

This confusion happens so often for me as well.  Maybe it is time to
rename these structures and local variable instances, something like:

asd_sas_port --> host_sas_port
asd_sas_phy --> host_sas_phy

...and:

port --> hport
phy --> hphy

...for the local variable instances.

??

RE: [PATCH] [SCSI] libsas panic when single phy disabled on a wide port

[Mark Salyzyn]

Our duplication is to send an out-of-band CLI command to a downstream
expander's Firmware and tell it to disable one of it's phys. Sounds
simple, but not when someone else has all the customized pieces and
serial cables ;-/ and the duplicator has moved on in priority.

Pregnant thought, smp_phy_control --phy=? --op=dis (wrong side, probably
too 'wide' of an aim) has not been tried ...

Sincerely -- Mark Salyzyn

-----Original Message-----
From: dan.j.williams@gmail.com [mailto:dan.j.williams@gmail.com] On
Behalf Of Dan Williams
Sent: Tuesday, October 04, 2011 7:31 PM
To: Mark Salyzyn
Subject: Re: [PATCH] [SCSI] libsas panic when single phy disabled on a
wide port

. . .

What is your reproduction scenario for this bug?  We see this in our
larger topology hotplug tests, but I don't think we have a simple
reproducer.
______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 

Re: [PATCH] [SCSI] libsas panic when single phy disabled on a wide port

[Jack Wang]

I reproduce this when echo 1/0 > /sys/class/sas_phy/phyxxx/enable phyxxx is
one in a wide port.
RE: [PATCH] [SCSI] libsas panic when single phy disabled on a wide port
> 
> Our duplication is to send an out-of-band CLI command to a downstream
> expander's Firmware and tell it to disable one of it's phys. Sounds
> simple, but not when someone else has all the customized pieces and
> serial cables ;-/ and the duplicator has moved on in priority.
> 
> Pregnant thought, smp_phy_control --phy=? --op=dis (wrong side, probably
> too 'wide' of an aim) has not been tried ...
> 
> Sincerely -- Mark Salyzyn
> 
> -----Original Message-----
> From: dan.j.williams@gmail.com [mailto:dan.j.williams@gmail.com] On
> Behalf Of Dan Williams
> Sent: Tuesday, October 04, 2011 7:31 PM
> To: Mark Salyzyn
> Subject: Re: [PATCH] [SCSI] libsas panic when single phy disabled on a
> wide port
> 
> . . .
> 
> What is your reproduction scenario for this bug?  We see this in our
> larger topology hotplug tests, but I don't think we have a simple
> reproducer.
> ______________________________________________________________________
> This email may contain privileged or confidential information, which
should
> only be used for the purpose for which it was sent by Xyratex. No further
rights
> or licenses are granted to use such information. If you are not the
intended
> recipient of this message, please notify the sender by return and delete
it.
> You may not use, copy, disclose or rely on the information contained in
it.
> 
> Internet email is susceptible to data corruption, interception and
> unauthorised amendment for which Xyratex does not accept liability. While
we
> have taken reasonable precautions to ensure that this email is free of
viruses,
> Xyratex does not accept liability for the presence of any computer viruses
in
> this email, nor for any losses caused as a result of viruses.
> 
> Xyratex Technology Limited (03134912), Registered in England & Wales,
> Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
> 
> The Xyratex group of companies also includes, Xyratex Ltd, registered in
> Bermuda, Xyratex International Inc, registered in California, Xyratex
> (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co
Ltd
> registered in The People's Republic of China and Xyratex Japan Limited
> registered in Japan.
> ______________________________________________________________________
> 
> 
> --
> 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

Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Luben Tuikov]

Isn't this my own patch? Shouldn't have I regenerated it and sent of off? Is it common here to have other people generate other people's patches? I think that not very professional. 

James, you should apply the patch I'm about to send. Same one, tabs fixed. 

        Luben


On Sep 22, 2011, at 8:04, "Mark Salyzyn" <mark_salyzyn@us.xyratex.com> wrote:

> Enclosed is an updated patch that passes checkpatch.pl (a few 80-character line errors in the original), maintain existing indent mechanics and contains my 'Ack' and the recommended Cc list.
> 
> Wait for Luben's Ack or Override (despite the fact I took the liberty of adding his Signed-off-by on the checkpatch required adjustments).
> 
> Sincerely -- Mark Salyzyn 
> 
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of James Bottomley
> Sent: Thursday, September 22, 2011 7:30 AM
> To: Luben Tuikov
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> On Wed, 2011-07-27 at 21:19 -0700, Luben Tuikov wrote:
>> Allow expander table-to-table attachments for
>> expanders that support it.
>> 
>> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> 
> OK, so now I need an applyable patch, please.  It looks like the mail
> server has mapped all tabs to spaces.  If you can't fix the mailer, just
> sending the patch as an attachment is fine.
> 
> James
> 
> NrybXǧv^)޺{.n+{"{ay\x1dʇڙ,j fhz\x1ew\fj:+vwjm zZ+ݢj"!
> 
> ______________________________________________________________________
> This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
> 
> Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
> 
> Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
> 
> The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
> ______________________________________________________________________
> 
> 
> <libsas_t2t.patch>

Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Luben Tuikov]

Salyzyn, DON'T do this any more with mine or anybody else's patches. OKAY?



----- Original Message -----
> From: Mark Salyzyn <mark_salyzyn@us.xyratex.com>
> To: James Bottomley <jbottomley@parallels.com>; Luben Tuikov <ltuikov@yahoo.com>
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; Darrick Wong <djwong@us.ibm.com>; Xiangliang Yu <yuxiangl@marvell.com>; Jack Wang <jack_wang@usish.com>
> Sent: Thursday, September 22, 2011 8:04 AM
> Subject: RE: [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> Enclosed is an updated patch that passes checkpatch.pl (a few 80-character line 
> errors in the original), maintain existing indent mechanics and contains my 
> 'Ack' and the recommended Cc list.
> 
> Wait for Luben's Ack or Override (despite the fact I took the liberty of 
> adding his Signed-off-by on the checkpatch required adjustments).
> 
> Sincerely -- Mark Salyzyn 
> 
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] 
> On Behalf Of James Bottomley
> Sent: Thursday, September 22, 2011 7:30 AM
> To: Luben Tuikov
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> On Wed, 2011-07-27 at 21:19 -0700, Luben Tuikov wrote:
>>  Allow expander table-to-table attachments for
>>  expanders that support it.
>> 
>>  Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> 
> OK, so now I need an applyable patch, please.  It looks like the mail
> server has mapped all tabs to spaces.  If you can't fix the mailer, just
> sending the patch as an attachment is fine.
> 
> James
> 
> NrybXǧv^)޺{.n+{"{ayʇڙ,j fhzwj:+vwjm zZ+ݢj"!
> 
> ______________________________________________________________________
> This email may contain privileged or confidential information, which should only 
> be used for the purpose for which it was sent by Xyratex. No further rights or 
> licenses are granted to use such information. If you are not the intended 
> recipient of this message, please notify the sender by return and delete it. You 
> may not use, copy, disclose or rely on the information contained in it.
> 
> Internet email is susceptible to data corruption, interception and unauthorised 
> amendment for which Xyratex does not accept liability. While we have taken 
> reasonable precautions to ensure that this email is free of viruses, Xyratex 
> does not accept liability for the presence of any computer viruses in this 
> email, nor for any losses caused as a result of viruses.
> 
> Xyratex Technology Limited (03134912), Registered in England & Wales, 
> Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
> 
> The Xyratex group of companies also includes, Xyratex Ltd, registered in 
> Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) 
> Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in 
> The People's Republic of China and Xyratex Japan Limited registered in 
> Japan.
> ______________________________________________________________________
>

Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Christoph Hellwig]

On Thu, Sep 22, 2011 at 09:35:19AM -0700, Luben Tuikov wrote:
> Salyzyn, DON'T do this any more with mine or anybody else's patches. OKAY?

Respinning patches to fix whitespace issues is perfectly fine.  What
Marc missed, and what is very important is to preserve the proper
attribution of the original author.  That is usually done by keeping a
copy of the original From: line as the first line in the mail body.

Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Luben Tuikov]

>From the original author yes, (unless he's dead or something like that). Not from someone else, when the original author is available.


When have your had a patch of yours being re-spun and submitted from someone else? Please point to a marc.info thread.



----- Original Message -----
> From: Christoph Hellwig <hch@infradead.org>
> To: Luben Tuikov <ltuikov@yahoo.com>
> Cc: Mark Salyzyn <mark_salyzyn@us.xyratex.com>; James Bottomley <jbottomley@parallels.com>; "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>; "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>; Darrick Wong <djwong@us.ibm.com>; Xiangliang Yu <yuxiangl@marvell.com>; Jack Wang <jack_wang@usish.com>
> Sent: Thursday, September 22, 2011 10:03 AM
> Subject: Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> On Thu, Sep 22, 2011 at 09:35:19AM -0700, Luben Tuikov wrote:
>>  Salyzyn, DON'T do this any more with mine or anybody else's 
> patches. OKAY?
> 
> Respinning patches to fix whitespace issues is perfectly fine.  What
> Marc missed, and what is very important is to preserve the proper
> attribution of the original author.  That is usually done by keeping a
> copy of the original From: line as the first line in the mail body.
>

Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Luben Tuikov]

From the original author yes, (unless he's dead or something like that). Not from someone else, when the original author is available.


When have your had a patch of yours being re-spun and submitted from someone else? Please point to a marc.info thread.



----- Original Message -----
> From: Christoph Hellwig <hch@infradead.org>
> To: Luben Tuikov <ltuikov@yahoo.com>
> Cc: Mark Salyzyn <mark_salyzyn@us.xyratex.com>; James Bottomley <jbottomley@parallels.com>; "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>; "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>; Darrick Wong <djwong@us.ibm.com>; Xiangliang Yu <yuxiangl@marvell.com>; Jack Wang <jack_wang@usish.com>
> Sent: Thursday, September 22, 2011 10:03 AM
> Subject: Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> On Thu, Sep 22, 2011 at 09:35:19AM -0700, Luben Tuikov wrote:
>>  Salyzyn, DON'T do this any more with mine or anybody else's 
> patches. OKAY?
> 
> Respinning patches to fix whitespace issues is perfectly fine.  What
> Marc missed, and what is very important is to preserve the proper
> attribution of the original author.  That is usually done by keeping a
> copy of the original From: line as the first line in the mail body.
>
--
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

Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Christoph Hellwig]

On Thu, Sep 22, 2011 at 10:11:09AM -0700, Luben Tuikov wrote:
> >From the original author yes, (unless he's dead or something like that). Not from someone else, when the original author is available.
> 
> 
> When have your had a patch of yours being re-spun and submitted from someone else? Please point to a marc.info thread.

I'm not going to look them up for you, but if you care enough you'll
find plenty of XFS commits from me that got edited and resent or sent on
by Alex or Dave (or others earlier). 

Re: [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Alan Cox]

On Fri, 23 Sep 2011 14:42:13 -0400
Christoph Hellwig <hch@infradead.org> wrote:

> On Thu, Sep 22, 2011 at 10:11:09AM -0700, Luben Tuikov wrote:
> > >From the original author yes, (unless he's dead or something like that). Not from someone else, when the original author is available.
> > 
> > 
> > When have your had a patch of yours being re-spun and submitted from someone else? Please point to a marc.info thread.
> 
> I'm not going to look them up for you, but if you care enough you'll
> find plenty of XFS commits from me that got edited and resent or sent on
> by Alex or Dave (or others earlier). 

Likewise I've done it for various things passing through me in the past.
Generally the reply I get is "thanks".

Alan

[RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Luben Tuikov]

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

Allow expander table-to-table attachments for
expanders that support it.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 drivers/scsi/libsas/sas_expander.c |   20 ++++++++++++++------
 include/scsi/libsas.h              |    3 +++
 include/scsi/sas.h                 |   14 ++++++++++++--
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index f84084b..e8d0115 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -329,6 +329,7 @@ static void ex_assign_report_general(struct domain_device *dev,
     dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
     dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
     dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
+    dev->ex_dev.t2t_supp = rg->t2t_supp;
     dev->ex_dev.conf_route_table = rg->conf_route_table;
     dev->ex_dev.configuring = rg->configuring;
     memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id, 8);
@@ -1133,15 +1134,17 @@ static void sas_print_parent_topology_bug(struct domain_device *child,
     };
     struct domain_device *parent = child->parent;
 
-    sas_printk("%s ex %016llx phy 0x%x <--> %s ex %016llx phy 0x%x "
-           "has %c:%c routing link!\n",
+    sas_printk("%s ex %016llx (T2T supp:%d) phy 0x%x <--> %s ex %016llx "
+           "(T2T supp:%d) phy 0x%x has %c:%c routing link!\n",
 
            ex_type[parent->dev_type],
            SAS_ADDR(parent->sas_addr),
+           parent->ex_dev.t2t_supp,
            parent_phy->phy_id,
 
            ex_type[child->dev_type],
            SAS_ADDR(child->sas_addr),
+           child->ex_dev.t2t_supp,
            child_phy->phy_id,
 
            ra_char[parent_phy->routing_attr],
@@ -1238,10 +1241,15 @@ static int sas_check_parent_topology(struct domain_device *child)
                     sas_print_parent_topology_bug(child, parent_phy, child_phy);
                     res = -ENODEV;
                 }
-            } else if (parent_phy->routing_attr == TABLE_ROUTING &&
-                   child_phy->routing_attr != SUBTRACTIVE_ROUTING) {
-                sas_print_parent_topology_bug(child, parent_phy, child_phy);
-                res = -ENODEV;
+            } else if (parent_phy->routing_attr == TABLE_ROUTING) {
+                if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||
+                    (child_phy->routing_attr == TABLE_ROUTING &&
+                     child_ex->t2t_supp && parent_ex->t2t_supp)) {
+                    /* All good */;
+                } else {
+                    sas_print_parent_topology_bug(child, parent_phy, child_phy);
+                    res = -ENODEV;
+                }
             }
             break;
         case FANOUT_DEV:
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index ee86606..793f80b 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -142,8 +142,11 @@ struct expander_device {
     u16    ex_change_count;
     u16    max_route_indexes;
     u8     num_phys;
+
+    u8     t2t_supp:1;
     u8     configuring:1;
     u8     conf_route_table:1;
+
     u8     enclosure_logical_id[8];
 
     struct ex_phy *ex_phy;
diff --git a/include/scsi/sas.h b/include/scsi/sas.h
index e9fd022..f59f182 100644
--- a/include/scsi/sas.h
+++ b/include/scsi/sas.h
@@ -341,7 +341,12 @@ struct report_general_resp {
 
     u8      conf_route_table:1;
     u8      configuring:1;
-    u8      _r_b:6;
+    u8    config_others:1;
+    u8    orej_retry_supp:1;
+    u8    stp_cont_awt:1;
+    u8    self_config:1;
+    u8    zone_config:1;
+    u8    t2t_supp:1;
 
     u8      _r_c;
 
@@ -528,7 +533,12 @@ struct report_general_resp {
     u8      _r_a;
     u8      num_phys;
 
-    u8      _r_b:6;
+    u8    t2t_supp:1;
+    u8    zone_config:1;
+    u8    self_config:1;
+    u8    stp_cont_awt:1;
+    u8    orej_retry_supp:1;
+    u8    config_others:1;
     u8      configuring:1;
     u8      conf_route_table:1;
 
-- 
1.7.2.2.165.gbc382

[-- Attachment #2: 0001-SCSI-libsas-Allow-expander-T-T-attachments.patch --]
[-- Type: application/octet-stream, Size: 3823 bytes --]

From 7cd733043176127d58911165369f4a8914ac4edd Mon Sep 17 00:00:00 2001
From: Luben Tuikov <ltuikov@yahoo.com>
Date: Wed, 27 Jul 2011 21:02:12 -0700
Subject: [PATCH] [SCSI] libsas: Allow expander T-T attachments

Allow expander table-to-table attachments for
expanders that support it.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 drivers/scsi/libsas/sas_expander.c |   20 ++++++++++++++------
 include/scsi/libsas.h              |    3 +++
 include/scsi/sas.h                 |   14 ++++++++++++--
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index f84084b..e8d0115 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -329,6 +329,7 @@ static void ex_assign_report_general(struct domain_device *dev,
 	dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
 	dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
 	dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
+	dev->ex_dev.t2t_supp = rg->t2t_supp;
 	dev->ex_dev.conf_route_table = rg->conf_route_table;
 	dev->ex_dev.configuring = rg->configuring;
 	memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id, 8);
@@ -1133,15 +1134,17 @@ static void sas_print_parent_topology_bug(struct domain_device *child,
 	};
 	struct domain_device *parent = child->parent;
 
-	sas_printk("%s ex %016llx phy 0x%x <--> %s ex %016llx phy 0x%x "
-		   "has %c:%c routing link!\n",
+	sas_printk("%s ex %016llx (T2T supp:%d) phy 0x%x <--> %s ex %016llx "
+		   "(T2T supp:%d) phy 0x%x has %c:%c routing link!\n",
 
 		   ex_type[parent->dev_type],
 		   SAS_ADDR(parent->sas_addr),
+		   parent->ex_dev.t2t_supp,
 		   parent_phy->phy_id,
 
 		   ex_type[child->dev_type],
 		   SAS_ADDR(child->sas_addr),
+		   child->ex_dev.t2t_supp,
 		   child_phy->phy_id,
 
 		   ra_char[parent_phy->routing_attr],
@@ -1238,10 +1241,15 @@ static int sas_check_parent_topology(struct domain_device *child)
 					sas_print_parent_topology_bug(child, parent_phy, child_phy);
 					res = -ENODEV;
 				}
-			} else if (parent_phy->routing_attr == TABLE_ROUTING &&
-				   child_phy->routing_attr != SUBTRACTIVE_ROUTING) {
-				sas_print_parent_topology_bug(child, parent_phy, child_phy);
-				res = -ENODEV;
+			} else if (parent_phy->routing_attr == TABLE_ROUTING) {
+				if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||
+				    (child_phy->routing_attr == TABLE_ROUTING &&
+				     child_ex->t2t_supp && parent_ex->t2t_supp)) {
+					/* All good */;
+				} else {
+					sas_print_parent_topology_bug(child, parent_phy, child_phy);
+					res = -ENODEV;
+				}
 			}
 			break;
 		case FANOUT_DEV:
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index ee86606..793f80b 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -142,8 +142,11 @@ struct expander_device {
 	u16    ex_change_count;
 	u16    max_route_indexes;
 	u8     num_phys;
+
+	u8     t2t_supp:1;
 	u8     configuring:1;
 	u8     conf_route_table:1;
+
 	u8     enclosure_logical_id[8];
 
 	struct ex_phy *ex_phy;
diff --git a/include/scsi/sas.h b/include/scsi/sas.h
index e9fd022..f59f182 100644
--- a/include/scsi/sas.h
+++ b/include/scsi/sas.h
@@ -341,7 +341,12 @@ struct report_general_resp {
 
 	u8      conf_route_table:1;
 	u8      configuring:1;
-	u8      _r_b:6;
+	u8	config_others:1;
+	u8	orej_retry_supp:1;
+	u8	stp_cont_awt:1;
+	u8	self_config:1;
+	u8	zone_config:1;
+	u8	t2t_supp:1;
 
 	u8      _r_c;
 
@@ -528,7 +533,12 @@ struct report_general_resp {
 	u8      _r_a;
 	u8      num_phys;
 
-	u8      _r_b:6;
+	u8	t2t_supp:1;
+	u8	zone_config:1;
+	u8	self_config:1;
+	u8	stp_cont_awt:1;
+	u8	orej_retry_supp:1;
+	u8	config_others:1;
 	u8      configuring:1;
 	u8      conf_route_table:1;
 
-- 
1.7.2.2.165.gbc382

Re: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Christoph Hellwig]

On Thu, Sep 22, 2011 at 09:41:36AM -0700, Luben Tuikov wrote:
> Allow expander table-to-table attachments for
> expanders that support it.
> 
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---
> ?drivers/scsi/libsas/sas_expander.c |?? 20 ++++++++++++++------
> ?include/scsi/libsas.h????????????? |??? 3 +++
> ?include/scsi/sas.h???????????????? |?? 14 ++++++++++++--
> ?3 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index f84084b..e8d0115 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -329,6 +329,7 @@ static void ex_assign_report_general(struct domain_device *dev,
> ???? dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
> ???? dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
> ???? dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);

This looks completely garbled to me.

Re: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Luben Tuikov]

I've noticed this recently as well. I'm not sure if it is Yahoo or Firefox, or my cutting and pasting from Emacs as opposed to Xterm.



----- Original Message -----
> From: Christoph Hellwig <hch@infradead.org>
> To: Luben Tuikov <ltuikov@yahoo.com>
> Cc: James Bottomley <jbottomley@parallels.com>; "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>; "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
> Sent: Thursday, September 22, 2011 9:50 AM
> Subject: Re: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> On Thu, Sep 22, 2011 at 09:41:36AM -0700, Luben Tuikov wrote:
>>  Allow expander table-to-table attachments for
>>  expanders that support it.
>> 
>>  Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
>>  ---
>>  ?drivers/scsi/libsas/sas_expander.c |?? 20 ++++++++++++++------
>>  ?include/scsi/libsas.h????????????? |??? 3 +++
>>  ?include/scsi/sas.h???????????????? |?? 14 ++++++++++++--
>>  ?3 files changed, 29 insertions(+), 8 deletions(-)
>> 
>>  diff --git a/drivers/scsi/libsas/sas_expander.c 
> b/drivers/scsi/libsas/sas_expander.c
>>  index f84084b..e8d0115 100644
>>  --- a/drivers/scsi/libsas/sas_expander.c
>>  +++ b/drivers/scsi/libsas/sas_expander.c
>>  @@ -329,6 +329,7 @@ static void ex_assign_report_general(struct 
> domain_device *dev,
>>  ???? dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
>>  ???? dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
>>  ???? dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
> 
> This looks completely garbled to me.
>

Re: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Bart Van Assche]

On Thu, Sep 22, 2011 at 7:24 PM, Luben Tuikov <ltuikov@yahoo.com> wrote:
> I've noticed this recently as well. I'm not sure if it is Yahoo or Firefox, or my cutting and pasting from Emacs as opposed to Xterm.

A quote from commit 5ce9f07bf1bed9a1f9886373ad0b149294f84c25 (March 10, 2010):

<quote>
Gmail web gui does not work for sending patches now even with firefox
"view source with" extension.  It will use windows style line breaks to
wrap lines automatically when sening email.
[ ... ]
-The last problem is that Gmail will base64-encode any message that has a
-non-ASCII character.  That includes things like European names.  Be aware.
</quote>

It's not impossible that the same comments apply to Yahoo mail too.

Bart.

RE: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Mark Salyzyn]

Except for the fact the patch as located in the attachment is still not in compliance, the patch works for me.

Sincerely -- Mark Salyzyn
 
> scsi-misc-2.6/scripts/checkpatch.pl 0001-SCSI-libsas-Allow-expander-T-T-attachments.patch
WARNING: line over 80 characters
#57: FILE: drivers/scsi/libsas/sas_expander.c:1245:
+                               if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||

WARNING: line over 80 characters
#59: FILE: drivers/scsi/libsas/sas_expander.c:1247:
+                                    child_ex->t2t_supp && parent_ex->t2t_supp)) {

WARNING: line over 80 characters
#62: FILE: drivers/scsi/libsas/sas_expander.c:1250:
+                                       sas_print_parent_topology_bug(child, parent_phy, child_phy);

total: 0 errors, 3 warnings, 82 lines checked

0001-SCSI-libsas-Allow-expander-T-T-attachments.patch has style problems, please
 review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

-----Original Message-----
From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Luben Tuikov
Sent: Thursday, September 22, 2011 12:42 PM
To: James Bottomley
Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

Allow expander table-to-table attachments for
expanders that support it.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 drivers/scsi/libsas/sas_expander.c |   20 ++++++++++++++------
 include/scsi/libsas.h              |    3 +++
 include/scsi/sas.h                 |   14 ++++++++++++--
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index f84084b..e8d0115 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -329,6 +329,7 @@ static void ex_assign_report_general(struct domain_device *dev,
     dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
     dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
     dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
+    dev->ex_dev.t2t_supp = rg->t2t_supp;
     dev->ex_dev.conf_route_table = rg->conf_route_table;
     dev->ex_dev.configuring = rg->configuring;
     memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id, 8);
@@ -1133,15 +1134,17 @@ static void sas_print_parent_topology_bug(struct domain_device *child,
     };
     struct domain_device *parent = child->parent;
 
-    sas_printk("%s ex %016llx phy 0x%x <--> %s ex %016llx phy 0x%x "
-           "has %c:%c routing link!\n",
+    sas_printk("%s ex %016llx (T2T supp:%d) phy 0x%x <--> %s ex %016llx "
+           "(T2T supp:%d) phy 0x%x has %c:%c routing link!\n",
 
            ex_type[parent->dev_type],
            SAS_ADDR(parent->sas_addr),
+           parent->ex_dev.t2t_supp,
            parent_phy->phy_id,
 
            ex_type[child->dev_type],
            SAS_ADDR(child->sas_addr),
+           child->ex_dev.t2t_supp,
            child_phy->phy_id,
 
            ra_char[parent_phy->routing_attr],
@@ -1238,10 +1241,15 @@ static int sas_check_parent_topology(struct domain_device *child)
                     sas_print_parent_topology_bug(child, parent_phy, child_phy);
                     res = -ENODEV;
                 }
-            } else if (parent_phy->routing_attr == TABLE_ROUTING &&
-                   child_phy->routing_attr != SUBTRACTIVE_ROUTING) {
-                sas_print_parent_topology_bug(child, parent_phy, child_phy);
-                res = -ENODEV;
+            } else if (parent_phy->routing_attr == TABLE_ROUTING) {
+                if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||
+                    (child_phy->routing_attr == TABLE_ROUTING &&
+                     child_ex->t2t_supp && parent_ex->t2t_supp)) {
+                    /* All good */;
+                } else {
+                    sas_print_parent_topology_bug(child, parent_phy, child_phy);
+                    res = -ENODEV;
+                }
             }
             break;
         case FANOUT_DEV:
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index ee86606..793f80b 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -142,8 +142,11 @@ struct expander_device {
     u16    ex_change_count;
     u16    max_route_indexes;
     u8     num_phys;
+
+    u8     t2t_supp:1;
     u8     configuring:1;
     u8     conf_route_table:1;
+
     u8     enclosure_logical_id[8];
 
     struct ex_phy *ex_phy;
diff --git a/include/scsi/sas.h b/include/scsi/sas.h
index e9fd022..f59f182 100644
--- a/include/scsi/sas.h
+++ b/include/scsi/sas.h
@@ -341,7 +341,12 @@ struct report_general_resp {
 
     u8      conf_route_table:1;
     u8      configuring:1;
-    u8      _r_b:6;
+    u8    config_others:1;
+    u8    orej_retry_supp:1;
+    u8    stp_cont_awt:1;
+    u8    self_config:1;
+    u8    zone_config:1;
+    u8    t2t_supp:1;
 
     u8      _r_c;
 
@@ -528,7 +533,12 @@ struct report_general_resp {
     u8      _r_a;
     u8      num_phys;
 
-    u8      _r_b:6;
+    u8    t2t_supp:1;
+    u8    zone_config:1;
+    u8    self_config:1;
+    u8    stp_cont_awt:1;
+    u8    orej_retry_supp:1;
+    u8    config_others:1;
     u8      configuring:1;
     u8      conf_route_table:1;
 
-- 
1.7.2.2.165.gbc382
______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 

RE: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Mark Salyzyn]

Except for the fact the patch as located in the attachment is still not in compliance, the patch works for me.

Sincerely -- Mark Salyzyn
 
> scsi-misc-2.6/scripts/checkpatch.pl 0001-SCSI-libsas-Allow-expander-T-T-attachments.patch
WARNING: line over 80 characters
#57: FILE: drivers/scsi/libsas/sas_expander.c:1245:
+                               if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||

WARNING: line over 80 characters
#59: FILE: drivers/scsi/libsas/sas_expander.c:1247:
+                                    child_ex->t2t_supp && parent_ex->t2t_supp)) {

WARNING: line over 80 characters
#62: FILE: drivers/scsi/libsas/sas_expander.c:1250:
+                                       sas_print_parent_topology_bug(child, parent_phy, child_phy);

total: 0 errors, 3 warnings, 82 lines checked

0001-SCSI-libsas-Allow-expander-T-T-attachments.patch has style problems, please
 review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

-----Original Message-----
From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Luben Tuikov
Sent: Thursday, September 22, 2011 12:42 PM
To: James Bottomley
Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

Allow expander table-to-table attachments for
expanders that support it.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 drivers/scsi/libsas/sas_expander.c |   20 ++++++++++++++------
 include/scsi/libsas.h              |    3 +++
 include/scsi/sas.h                 |   14 ++++++++++++--
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index f84084b..e8d0115 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -329,6 +329,7 @@ static void ex_assign_report_general(struct domain_device *dev,
     dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
     dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
     dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
+    dev->ex_dev.t2t_supp = rg->t2t_supp;
     dev->ex_dev.conf_route_table = rg->conf_route_table;
     dev->ex_dev.configuring = rg->configuring;
     memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id, 8);
@@ -1133,15 +1134,17 @@ static void sas_print_parent_topology_bug(struct domain_device *child,
     };
     struct domain_device *parent = child->parent;
 
-    sas_printk("%s ex %016llx phy 0x%x <--> %s ex %016llx phy 0x%x "
-           "has %c:%c routing link!\n",
+    sas_printk("%s ex %016llx (T2T supp:%d) phy 0x%x <--> %s ex %016llx "
+           "(T2T supp:%d) phy 0x%x has %c:%c routing link!\n",
 
            ex_type[parent->dev_type],
            SAS_ADDR(parent->sas_addr),
+           parent->ex_dev.t2t_supp,
            parent_phy->phy_id,
 
            ex_type[child->dev_type],
            SAS_ADDR(child->sas_addr),
+           child->ex_dev.t2t_supp,
            child_phy->phy_id,
 
            ra_char[parent_phy->routing_attr],
@@ -1238,10 +1241,15 @@ static int sas_check_parent_topology(struct domain_device *child)
                     sas_print_parent_topology_bug(child, parent_phy, child_phy);
                     res = -ENODEV;
                 }
-            } else if (parent_phy->routing_attr == TABLE_ROUTING &&
-                   child_phy->routing_attr != SUBTRACTIVE_ROUTING) {
-                sas_print_parent_topology_bug(child, parent_phy, child_phy);
-                res = -ENODEV;
+            } else if (parent_phy->routing_attr == TABLE_ROUTING) {
+                if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||
+                    (child_phy->routing_attr == TABLE_ROUTING &&
+                     child_ex->t2t_supp && parent_ex->t2t_supp)) {
+                    /* All good */;
+                } else {
+                    sas_print_parent_topology_bug(child, parent_phy, child_phy);
+                    res = -ENODEV;
+                }
             }
             break;
         case FANOUT_DEV:
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index ee86606..793f80b 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -142,8 +142,11 @@ struct expander_device {
     u16    ex_change_count;
     u16    max_route_indexes;
     u8     num_phys;
+
+    u8     t2t_supp:1;
     u8     configuring:1;
     u8     conf_route_table:1;
+
     u8     enclosure_logical_id[8];
 
     struct ex_phy *ex_phy;
diff --git a/include/scsi/sas.h b/include/scsi/sas.h
index e9fd022..f59f182 100644
--- a/include/scsi/sas.h
+++ b/include/scsi/sas.h
@@ -341,7 +341,12 @@ struct report_general_resp {
 
     u8      conf_route_table:1;
     u8      configuring:1;
-    u8      _r_b:6;
+    u8    config_others:1;
+    u8    orej_retry_supp:1;
+    u8    stp_cont_awt:1;
+    u8    self_config:1;
+    u8    zone_config:1;
+    u8    t2t_supp:1;
 
     u8      _r_c;
 
@@ -528,7 +533,12 @@ struct report_general_resp {
     u8      _r_a;
     u8      num_phys;
 
-    u8      _r_b:6;
+    u8    t2t_supp:1;
+    u8    zone_config:1;
+    u8    self_config:1;
+    u8    stp_cont_awt:1;
+    u8    orej_retry_supp:1;
+    u8    config_others:1;
     u8      configuring:1;
     u8      conf_route_table:1;
 
-- 
1.7.2.2.165.gbc382
______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 

Re: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Luben Tuikov]

LOOK AT THE SOURCE CODE: drivers/scsi/libsas/sas_expander.c:

/* Here we spill over 80 columns.  It is intentional.
 */
static int sas_check_parent_topology(struct domain_device *child)
{
    struct expander_device *child_ex = &child->ex_dev;
    struct expander_device *parent_ex;
    int i;
    int res = 0;

    if (!child->parent)
        return 0;

    if (child->parent->dev_type != EDGE_DEV &&
        child->parent->dev_type != FANOUT_DEV)
        return 0;
...

ARE WE DONE?

----- Original Message -----

> From: Mark Salyzyn <mark_salyzyn@us.xyratex.com>
> To: Luben Tuikov <ltuikov@yahoo.com>; James Bottomley <jbottomley@parallels.com>
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
> Sent: Thursday, September 22, 2011 9:55 AM
> Subject: RE: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> Except for the fact the patch as located in the attachment is still not in 
> compliance, the patch works for me.
> 
> Sincerely -- Mark Salyzyn
> 
>>  scsi-misc-2.6/scripts/checkpatch.pl 
> 0001-SCSI-libsas-Allow-expander-T-T-attachments.patch
> WARNING: line over 80 characters
> #57: FILE: drivers/scsi/libsas/sas_expander.c:1245:
> +                               if (child_phy->routing_attr == 
> SUBTRACTIVE_ROUTING ||
> 
> WARNING: line over 80 characters
> #59: FILE: drivers/scsi/libsas/sas_expander.c:1247:
> +                                    child_ex->t2t_supp && 
> parent_ex->t2t_supp)) {
> 
> WARNING: line over 80 characters
> #62: FILE: drivers/scsi/libsas/sas_expander.c:1250:
> +                                       sas_print_parent_topology_bug(child, 
> parent_phy, child_phy);
> 
> total: 0 errors, 3 warnings, 82 lines checked
> 
> 0001-SCSI-libsas-Allow-expander-T-T-attachments.patch has style problems, please
> review.
> 
> If any of these errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
> 
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] 
> On Behalf Of Luben Tuikov
> Sent: Thursday, September 22, 2011 12:42 PM
> To: James Bottomley
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> Allow expander table-to-table attachments for
> expanders that support it.
> 
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---
>  drivers/scsi/libsas/sas_expander.c |   20 ++++++++++++++------
>  include/scsi/libsas.h              |    3 +++
>  include/scsi/sas.h                 |   14 ++++++++++++--
>  3 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c 
> b/drivers/scsi/libsas/sas_expander.c
> index f84084b..e8d0115 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -329,6 +329,7 @@ static void ex_assign_report_general(struct domain_device 
> *dev,
>      dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
>      dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
>      dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
> +    dev->ex_dev.t2t_supp = rg->t2t_supp;
>      dev->ex_dev.conf_route_table = rg->conf_route_table;
>      dev->ex_dev.configuring = rg->configuring;
>      memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id, 
> 8);
> @@ -1133,15 +1134,17 @@ static void sas_print_parent_topology_bug(struct 
> domain_device *child,
>      };
>      struct domain_device *parent = child->parent;
>  
> -    sas_printk("%s ex %016llx phy 0x%x <--> %s ex %016llx phy 0x%x 
> "
> -           "has %c:%c routing link!\n",
> +    sas_printk("%s ex %016llx (T2T supp:%d) phy 0x%x <--> %s ex 
> %016llx "
> +           "(T2T supp:%d) phy 0x%x has %c:%c routing link!\n",
>  
>             ex_type[parent->dev_type],
>             SAS_ADDR(parent->sas_addr),
> +           parent->ex_dev.t2t_supp,
>             parent_phy->phy_id,
>  
>             ex_type[child->dev_type],
>             SAS_ADDR(child->sas_addr),
> +           child->ex_dev.t2t_supp,
>             child_phy->phy_id,
>  
>             ra_char[parent_phy->routing_attr],
> @@ -1238,10 +1241,15 @@ static int sas_check_parent_topology(struct 
> domain_device *child)
>                      sas_print_parent_topology_bug(child, parent_phy, 
> child_phy);
>                      res = -ENODEV;
>                  }
> -            } else if (parent_phy->routing_attr == TABLE_ROUTING &&
> -                   child_phy->routing_attr != SUBTRACTIVE_ROUTING) {
> -                sas_print_parent_topology_bug(child, parent_phy, child_phy);
> -                res = -ENODEV;
> +            } else if (parent_phy->routing_attr == TABLE_ROUTING) {
> +                if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||
> +                    (child_phy->routing_attr == TABLE_ROUTING &&
> +                     child_ex->t2t_supp && parent_ex->t2t_supp)) 
> {
> +                    /* All good */;
> +                } else {
> +                    sas_print_parent_topology_bug(child, parent_phy, 
> child_phy);
> +                    res = -ENODEV;
> +                }
>              }
>              break;
>          case FANOUT_DEV:
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index ee86606..793f80b 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -142,8 +142,11 @@ struct expander_device {
>      u16    ex_change_count;
>      u16    max_route_indexes;
>      u8     num_phys;
> +
> +    u8     t2t_supp:1;
>      u8     configuring:1;
>      u8     conf_route_table:1;
> +
>      u8     enclosure_logical_id[8];
>  
>      struct ex_phy *ex_phy;
> diff --git a/include/scsi/sas.h b/include/scsi/sas.h
> index e9fd022..f59f182 100644
> --- a/include/scsi/sas.h
> +++ b/include/scsi/sas.h
> @@ -341,7 +341,12 @@ struct report_general_resp {
>  
>      u8      conf_route_table:1;
>      u8      configuring:1;
> -    u8      _r_b:6;
> +    u8    config_others:1;
> +    u8    orej_retry_supp:1;
> +    u8    stp_cont_awt:1;
> +    u8    self_config:1;
> +    u8    zone_config:1;
> +    u8    t2t_supp:1;
>  
>      u8      _r_c;
>  
> @@ -528,7 +533,12 @@ struct report_general_resp {
>      u8      _r_a;
>      u8      num_phys;
>  
> -    u8      _r_b:6;
> +    u8    t2t_supp:1;
> +    u8    zone_config:1;
> +    u8    self_config:1;
> +    u8    stp_cont_awt:1;
> +    u8    orej_retry_supp:1;
> +    u8    config_others:1;
>      u8      configuring:1;
>      u8      conf_route_table:1;
>  
> -- 
> 1.7.2.2.165.gbc382
> ______________________________________________________________________
> This email may contain privileged or confidential information, which should only 
> be used for the purpose for which it was sent by Xyratex. No further rights or 
> licenses are granted to use such information. If you are not the intended 
> recipient of this message, please notify the sender by return and delete it. You 
> may not use, copy, disclose or rely on the information contained in it.
> 
> Internet email is susceptible to data corruption, interception and unauthorised 
> amendment for which Xyratex does not accept liability. While we have taken 
> reasonable precautions to ensure that this email is free of viruses, Xyratex 
> does not accept liability for the presence of any computer viruses in this 
> email, nor for any losses caused as a result of viruses.
> 
> Xyratex Technology Limited (03134912), Registered in England & Wales, 
> Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
> 
> The Xyratex group of companies also includes, Xyratex Ltd, registered in 
> Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) 
> Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in 
> The People's Republic of China and Xyratex Japan Limited registered in 
> Japan.
> ______________________________________________________________________
>

Re: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Luben Tuikov]

LOOK AT THE SOURCE CODE: drivers/scsi/libsas/sas_expander.c:

/* Here we spill over 80 columns.  It is intentional.
 */
static int sas_check_parent_topology(struct domain_device *child)
{
    struct expander_device *child_ex = &child->ex_dev;
    struct expander_device *parent_ex;
    int i;
    int res = 0;

    if (!child->parent)
        return 0;

    if (child->parent->dev_type != EDGE_DEV &&
        child->parent->dev_type != FANOUT_DEV)
        return 0;
...

ARE WE DONE?

----- Original Message -----

> From: Mark Salyzyn <mark_salyzyn@us.xyratex.com>
> To: Luben Tuikov <ltuikov@yahoo.com>; James Bottomley <jbottomley@parallels.com>
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
> Sent: Thursday, September 22, 2011 9:55 AM
> Subject: RE: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> Except for the fact the patch as located in the attachment is still not in 
> compliance, the patch works for me.
> 
> Sincerely -- Mark Salyzyn
> 
>>  scsi-misc-2.6/scripts/checkpatch.pl 
> 0001-SCSI-libsas-Allow-expander-T-T-attachments.patch
> WARNING: line over 80 characters
> #57: FILE: drivers/scsi/libsas/sas_expander.c:1245:
> +                               if (child_phy->routing_attr == 
> SUBTRACTIVE_ROUTING ||
> 
> WARNING: line over 80 characters
> #59: FILE: drivers/scsi/libsas/sas_expander.c:1247:
> +                                    child_ex->t2t_supp && 
> parent_ex->t2t_supp)) {
> 
> WARNING: line over 80 characters
> #62: FILE: drivers/scsi/libsas/sas_expander.c:1250:
> +                                       sas_print_parent_topology_bug(child, 
> parent_phy, child_phy);
> 
> total: 0 errors, 3 warnings, 82 lines checked
> 
> 0001-SCSI-libsas-Allow-expander-T-T-attachments.patch has style problems, please
> review.
> 
> If any of these errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
> 
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] 
> On Behalf Of Luben Tuikov
> Sent: Thursday, September 22, 2011 12:42 PM
> To: James Bottomley
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> Allow expander table-to-table attachments for
> expanders that support it.
> 
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---
>  drivers/scsi/libsas/sas_expander.c |   20 ++++++++++++++------
>  include/scsi/libsas.h              |    3 +++
>  include/scsi/sas.h                 |   14 ++++++++++++--
>  3 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c 
> b/drivers/scsi/libsas/sas_expander.c
> index f84084b..e8d0115 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -329,6 +329,7 @@ static void ex_assign_report_general(struct domain_device 
> *dev,
>      dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
>      dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
>      dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
> +    dev->ex_dev.t2t_supp = rg->t2t_supp;
>      dev->ex_dev.conf_route_table = rg->conf_route_table;
>      dev->ex_dev.configuring = rg->configuring;
>      memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id, 
> 8);
> @@ -1133,15 +1134,17 @@ static void sas_print_parent_topology_bug(struct 
> domain_device *child,
>      };
>      struct domain_device *parent = child->parent;
>  
> -    sas_printk("%s ex %016llx phy 0x%x <--> %s ex %016llx phy 0x%x 
> "
> -           "has %c:%c routing link!\n",
> +    sas_printk("%s ex %016llx (T2T supp:%d) phy 0x%x <--> %s ex 
> %016llx "
> +           "(T2T supp:%d) phy 0x%x has %c:%c routing link!\n",
>  
>             ex_type[parent->dev_type],
>             SAS_ADDR(parent->sas_addr),
> +           parent->ex_dev.t2t_supp,
>             parent_phy->phy_id,
>  
>             ex_type[child->dev_type],
>             SAS_ADDR(child->sas_addr),
> +           child->ex_dev.t2t_supp,
>             child_phy->phy_id,
>  
>             ra_char[parent_phy->routing_attr],
> @@ -1238,10 +1241,15 @@ static int sas_check_parent_topology(struct 
> domain_device *child)
>                      sas_print_parent_topology_bug(child, parent_phy, 
> child_phy);
>                      res = -ENODEV;
>                  }
> -            } else if (parent_phy->routing_attr == TABLE_ROUTING &&
> -                   child_phy->routing_attr != SUBTRACTIVE_ROUTING) {
> -                sas_print_parent_topology_bug(child, parent_phy, child_phy);
> -                res = -ENODEV;
> +            } else if (parent_phy->routing_attr == TABLE_ROUTING) {
> +                if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||
> +                    (child_phy->routing_attr == TABLE_ROUTING &&
> +                     child_ex->t2t_supp && parent_ex->t2t_supp)) 
> {
> +                    /* All good */;
> +                } else {
> +                    sas_print_parent_topology_bug(child, parent_phy, 
> child_phy);
> +                    res = -ENODEV;
> +                }
>              }
>              break;
>          case FANOUT_DEV:
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index ee86606..793f80b 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -142,8 +142,11 @@ struct expander_device {
>      u16    ex_change_count;
>      u16    max_route_indexes;
>      u8     num_phys;
> +
> +    u8     t2t_supp:1;
>      u8     configuring:1;
>      u8     conf_route_table:1;
> +
>      u8     enclosure_logical_id[8];
>  
>      struct ex_phy *ex_phy;
> diff --git a/include/scsi/sas.h b/include/scsi/sas.h
> index e9fd022..f59f182 100644
> --- a/include/scsi/sas.h
> +++ b/include/scsi/sas.h
> @@ -341,7 +341,12 @@ struct report_general_resp {
>  
>      u8      conf_route_table:1;
>      u8      configuring:1;
> -    u8      _r_b:6;
> +    u8    config_others:1;
> +    u8    orej_retry_supp:1;
> +    u8    stp_cont_awt:1;
> +    u8    self_config:1;
> +    u8    zone_config:1;
> +    u8    t2t_supp:1;
>  
>      u8      _r_c;
>  
> @@ -528,7 +533,12 @@ struct report_general_resp {
>      u8      _r_a;
>      u8      num_phys;
>  
> -    u8      _r_b:6;
> +    u8    t2t_supp:1;
> +    u8    zone_config:1;
> +    u8    self_config:1;
> +    u8    stp_cont_awt:1;
> +    u8    orej_retry_supp:1;
> +    u8    config_others:1;
>      u8      configuring:1;
>      u8      conf_route_table:1;
>  
> -- 
> 1.7.2.2.165.gbc382
> ______________________________________________________________________
> This email may contain privileged or confidential information, which should only 
> be used for the purpose for which it was sent by Xyratex. No further rights or 
> licenses are granted to use such information. If you are not the intended 
> recipient of this message, please notify the sender by return and delete it. You 
> may not use, copy, disclose or rely on the information contained in it.
> 
> Internet email is susceptible to data corruption, interception and unauthorised 
> amendment for which Xyratex does not accept liability. While we have taken 
> reasonable precautions to ensure that this email is free of viruses, Xyratex 
> does not accept liability for the presence of any computer viruses in this 
> email, nor for any losses caused as a result of viruses.
> 
> Xyratex Technology Limited (03134912), Registered in England & Wales, 
> Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
> 
> The Xyratex group of companies also includes, Xyratex Ltd, registered in 
> Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) 
> Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in 
> The People's Republic of China and Xyratex Japan Limited registered in 
> Japan.
> ______________________________________________________________________
>
--
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

RE: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Mark Salyzyn]

Yes, we are Done. I thought I made it clear I am willing to Ack your patch as attached as-is.

Acked-by: Mark Salyzyn <mark_salyzyn@us.xyratex.com>

-----Original Message-----
From: Luben Tuikov [mailto:ltuikov@yahoo.com] 
Sent: Thursday, September 22, 2011 1:20 PM
To: Mark Salyzyn; James Bottomley
Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

LOOK AT THE SOURCE CODE: drivers/scsi/libsas/sas_expander.c:

/* Here we spill over 80 columns.  It is intentional.
 */
static int sas_check_parent_topology(struct domain_device *child)
{
    struct expander_device *child_ex = &child->ex_dev;
    struct expander_device *parent_ex;
    int i;
    int res = 0;

    if (!child->parent)
        return 0;

    if (child->parent->dev_type != EDGE_DEV &&
        child->parent->dev_type != FANOUT_DEV)
        return 0;
...

ARE WE DONE?

----- Original Message -----

> From: Mark Salyzyn <mark_salyzyn@us.xyratex.com>
> To: Luben Tuikov <ltuikov@yahoo.com>; James Bottomley <jbottomley@parallels.com>
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
> Sent: Thursday, September 22, 2011 9:55 AM
> Subject: RE: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> Except for the fact the patch as located in the attachment is still not in 
> compliance, the patch works for me.
> 
> Sincerely -- Mark Salyzyn
> 
>>  scsi-misc-2.6/scripts/checkpatch.pl 
> 0001-SCSI-libsas-Allow-expander-T-T-attachments.patch
> WARNING: line over 80 characters
> #57: FILE: drivers/scsi/libsas/sas_expander.c:1245:
> +                               if (child_phy->routing_attr == 
> SUBTRACTIVE_ROUTING ||
> 
> WARNING: line over 80 characters
> #59: FILE: drivers/scsi/libsas/sas_expander.c:1247:
> +                                    child_ex->t2t_supp && 
> parent_ex->t2t_supp)) {
> 
> WARNING: line over 80 characters
> #62: FILE: drivers/scsi/libsas/sas_expander.c:1250:
> +                                       sas_print_parent_topology_bug(child, 
> parent_phy, child_phy);
> 
> total: 0 errors, 3 warnings, 82 lines checked
> 
> 0001-SCSI-libsas-Allow-expander-T-T-attachments.patch has style problems, please
> review.
> 
> If any of these errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
> 
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] 
> On Behalf Of Luben Tuikov
> Sent: Thursday, September 22, 2011 12:42 PM
> To: James Bottomley
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> Allow expander table-to-table attachments for
> expanders that support it.
> 
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---
>  drivers/scsi/libsas/sas_expander.c |   20 ++++++++++++++------
>  include/scsi/libsas.h              |    3 +++
>  include/scsi/sas.h                 |   14 ++++++++++++--
>  3 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c 
> b/drivers/scsi/libsas/sas_expander.c
> index f84084b..e8d0115 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -329,6 +329,7 @@ static void ex_assign_report_general(struct domain_device 
> *dev,
>      dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
>      dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
>      dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
> +    dev->ex_dev.t2t_supp = rg->t2t_supp;
>      dev->ex_dev.conf_route_table = rg->conf_route_table;
>      dev->ex_dev.configuring = rg->configuring;
>      memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id, 
> 8);
> @@ -1133,15 +1134,17 @@ static void sas_print_parent_topology_bug(struct 
> domain_device *child,
>      };
>      struct domain_device *parent = child->parent;
>  
> -    sas_printk("%s ex %016llx phy 0x%x <--> %s ex %016llx phy 0x%x 
> "
> -           "has %c:%c routing link!\n",
> +    sas_printk("%s ex %016llx (T2T supp:%d) phy 0x%x <--> %s ex 
> %016llx "
> +           "(T2T supp:%d) phy 0x%x has %c:%c routing link!\n",
>  
>             ex_type[parent->dev_type],
>             SAS_ADDR(parent->sas_addr),
> +           parent->ex_dev.t2t_supp,
>             parent_phy->phy_id,
>  
>             ex_type[child->dev_type],
>             SAS_ADDR(child->sas_addr),
> +           child->ex_dev.t2t_supp,
>             child_phy->phy_id,
>  
>             ra_char[parent_phy->routing_attr],
> @@ -1238,10 +1241,15 @@ static int sas_check_parent_topology(struct 
> domain_device *child)
>                      sas_print_parent_topology_bug(child, parent_phy, 
> child_phy);
>                      res = -ENODEV;
>                  }
> -            } else if (parent_phy->routing_attr == TABLE_ROUTING &&
> -                   child_phy->routing_attr != SUBTRACTIVE_ROUTING) {
> -                sas_print_parent_topology_bug(child, parent_phy, child_phy);
> -                res = -ENODEV;
> +            } else if (parent_phy->routing_attr == TABLE_ROUTING) {
> +                if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||
> +                    (child_phy->routing_attr == TABLE_ROUTING &&
> +                     child_ex->t2t_supp && parent_ex->t2t_supp)) 
> {
> +                    /* All good */;
> +                } else {
> +                    sas_print_parent_topology_bug(child, parent_phy, 
> child_phy);
> +                    res = -ENODEV;
> +                }
>              }
>              break;
>          case FANOUT_DEV:
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index ee86606..793f80b 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -142,8 +142,11 @@ struct expander_device {
>      u16    ex_change_count;
>      u16    max_route_indexes;
>      u8     num_phys;
> +
> +    u8     t2t_supp:1;
>      u8     configuring:1;
>      u8     conf_route_table:1;
> +
>      u8     enclosure_logical_id[8];
>  
>      struct ex_phy *ex_phy;
> diff --git a/include/scsi/sas.h b/include/scsi/sas.h
> index e9fd022..f59f182 100644
> --- a/include/scsi/sas.h
> +++ b/include/scsi/sas.h
> @@ -341,7 +341,12 @@ struct report_general_resp {
>  
>      u8      conf_route_table:1;
>      u8      configuring:1;
> -    u8      _r_b:6;
> +    u8    config_others:1;
> +    u8    orej_retry_supp:1;
> +    u8    stp_cont_awt:1;
> +    u8    self_config:1;
> +    u8    zone_config:1;
> +    u8    t2t_supp:1;
>  
>      u8      _r_c;
>  
> @@ -528,7 +533,12 @@ struct report_general_resp {
>      u8      _r_a;
>      u8      num_phys;
>  
> -    u8      _r_b:6;
> +    u8    t2t_supp:1;
> +    u8    zone_config:1;
> +    u8    self_config:1;
> +    u8    stp_cont_awt:1;
> +    u8    orej_retry_supp:1;
> +    u8    config_others:1;
>      u8      configuring:1;
>      u8      conf_route_table:1;
>  
> -- 
> 1.7.2.2.165.gbc382
> ______________________________________________________________________
> This email may contain privileged or confidential information, which should only 
> be used for the purpose for which it was sent by Xyratex. No further rights or 
> licenses are granted to use such information. If you are not the intended 
> recipient of this message, please notify the sender by return and delete it. You 
> may not use, copy, disclose or rely on the information contained in it.
> 
> Internet email is susceptible to data corruption, interception and unauthorised 
> amendment for which Xyratex does not accept liability. While we have taken 
> reasonable precautions to ensure that this email is free of viruses, Xyratex 
> does not accept liability for the presence of any computer viruses in this 
> email, nor for any losses caused as a result of viruses.
> 
> Xyratex Technology Limited (03134912), Registered in England & Wales, 
> Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
> 
> The Xyratex group of companies also includes, Xyratex Ltd, registered in 
> Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) 
> Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in 
> The People's Republic of China and Xyratex Japan Limited registered in 
> Japan.
> ______________________________________________________________________
>
______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 

RE: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Mark Salyzyn]

Yes, we are Done. I thought I made it clear I am willing to Ack your patch as attached as-is.

Acked-by: Mark Salyzyn <mark_salyzyn@us.xyratex.com>

-----Original Message-----
From: Luben Tuikov [mailto:ltuikov@yahoo.com] 
Sent: Thursday, September 22, 2011 1:20 PM
To: Mark Salyzyn; James Bottomley
Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

LOOK AT THE SOURCE CODE: drivers/scsi/libsas/sas_expander.c:

/* Here we spill over 80 columns.  It is intentional.
 */
static int sas_check_parent_topology(struct domain_device *child)
{
    struct expander_device *child_ex = &child->ex_dev;
    struct expander_device *parent_ex;
    int i;
    int res = 0;

    if (!child->parent)
        return 0;

    if (child->parent->dev_type != EDGE_DEV &&
        child->parent->dev_type != FANOUT_DEV)
        return 0;
...

ARE WE DONE?

----- Original Message -----

> From: Mark Salyzyn <mark_salyzyn@us.xyratex.com>
> To: Luben Tuikov <ltuikov@yahoo.com>; James Bottomley <jbottomley@parallels.com>
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
> Sent: Thursday, September 22, 2011 9:55 AM
> Subject: RE: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> Except for the fact the patch as located in the attachment is still not in 
> compliance, the patch works for me.
> 
> Sincerely -- Mark Salyzyn
> 
>>  scsi-misc-2.6/scripts/checkpatch.pl 
> 0001-SCSI-libsas-Allow-expander-T-T-attachments.patch
> WARNING: line over 80 characters
> #57: FILE: drivers/scsi/libsas/sas_expander.c:1245:
> +                               if (child_phy->routing_attr == 
> SUBTRACTIVE_ROUTING ||
> 
> WARNING: line over 80 characters
> #59: FILE: drivers/scsi/libsas/sas_expander.c:1247:
> +                                    child_ex->t2t_supp && 
> parent_ex->t2t_supp)) {
> 
> WARNING: line over 80 characters
> #62: FILE: drivers/scsi/libsas/sas_expander.c:1250:
> +                                       sas_print_parent_topology_bug(child, 
> parent_phy, child_phy);
> 
> total: 0 errors, 3 warnings, 82 lines checked
> 
> 0001-SCSI-libsas-Allow-expander-T-T-attachments.patch has style problems, please
> review.
> 
> If any of these errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
> 
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] 
> On Behalf Of Luben Tuikov
> Sent: Thursday, September 22, 2011 12:42 PM
> To: James Bottomley
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> Allow expander table-to-table attachments for
> expanders that support it.
> 
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---
>  drivers/scsi/libsas/sas_expander.c |   20 ++++++++++++++------
>  include/scsi/libsas.h              |    3 +++
>  include/scsi/sas.h                 |   14 ++++++++++++--
>  3 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c 
> b/drivers/scsi/libsas/sas_expander.c
> index f84084b..e8d0115 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -329,6 +329,7 @@ static void ex_assign_report_general(struct domain_device 
> *dev,
>      dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
>      dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
>      dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
> +    dev->ex_dev.t2t_supp = rg->t2t_supp;
>      dev->ex_dev.conf_route_table = rg->conf_route_table;
>      dev->ex_dev.configuring = rg->configuring;
>      memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id, 
> 8);
> @@ -1133,15 +1134,17 @@ static void sas_print_parent_topology_bug(struct 
> domain_device *child,
>      };
>      struct domain_device *parent = child->parent;
>  
> -    sas_printk("%s ex %016llx phy 0x%x <--> %s ex %016llx phy 0x%x 
> "
> -           "has %c:%c routing link!\n",
> +    sas_printk("%s ex %016llx (T2T supp:%d) phy 0x%x <--> %s ex 
> %016llx "
> +           "(T2T supp:%d) phy 0x%x has %c:%c routing link!\n",
>  
>             ex_type[parent->dev_type],
>             SAS_ADDR(parent->sas_addr),
> +           parent->ex_dev.t2t_supp,
>             parent_phy->phy_id,
>  
>             ex_type[child->dev_type],
>             SAS_ADDR(child->sas_addr),
> +           child->ex_dev.t2t_supp,
>             child_phy->phy_id,
>  
>             ra_char[parent_phy->routing_attr],
> @@ -1238,10 +1241,15 @@ static int sas_check_parent_topology(struct 
> domain_device *child)
>                      sas_print_parent_topology_bug(child, parent_phy, 
> child_phy);
>                      res = -ENODEV;
>                  }
> -            } else if (parent_phy->routing_attr == TABLE_ROUTING &&
> -                   child_phy->routing_attr != SUBTRACTIVE_ROUTING) {
> -                sas_print_parent_topology_bug(child, parent_phy, child_phy);
> -                res = -ENODEV;
> +            } else if (parent_phy->routing_attr == TABLE_ROUTING) {
> +                if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||
> +                    (child_phy->routing_attr == TABLE_ROUTING &&
> +                     child_ex->t2t_supp && parent_ex->t2t_supp)) 
> {
> +                    /* All good */;
> +                } else {
> +                    sas_print_parent_topology_bug(child, parent_phy, 
> child_phy);
> +                    res = -ENODEV;
> +                }
>              }
>              break;
>          case FANOUT_DEV:
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index ee86606..793f80b 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -142,8 +142,11 @@ struct expander_device {
>      u16    ex_change_count;
>      u16    max_route_indexes;
>      u8     num_phys;
> +
> +    u8     t2t_supp:1;
>      u8     configuring:1;
>      u8     conf_route_table:1;
> +
>      u8     enclosure_logical_id[8];
>  
>      struct ex_phy *ex_phy;
> diff --git a/include/scsi/sas.h b/include/scsi/sas.h
> index e9fd022..f59f182 100644
> --- a/include/scsi/sas.h
> +++ b/include/scsi/sas.h
> @@ -341,7 +341,12 @@ struct report_general_resp {
>  
>      u8      conf_route_table:1;
>      u8      configuring:1;
> -    u8      _r_b:6;
> +    u8    config_others:1;
> +    u8    orej_retry_supp:1;
> +    u8    stp_cont_awt:1;
> +    u8    self_config:1;
> +    u8    zone_config:1;
> +    u8    t2t_supp:1;
>  
>      u8      _r_c;
>  
> @@ -528,7 +533,12 @@ struct report_general_resp {
>      u8      _r_a;
>      u8      num_phys;
>  
> -    u8      _r_b:6;
> +    u8    t2t_supp:1;
> +    u8    zone_config:1;
> +    u8    self_config:1;
> +    u8    stp_cont_awt:1;
> +    u8    orej_retry_supp:1;
> +    u8    config_others:1;
>      u8      configuring:1;
>      u8      conf_route_table:1;
>  
> -- 
> 1.7.2.2.165.gbc382
> ______________________________________________________________________
> This email may contain privileged or confidential information, which should only 
> be used for the purpose for which it was sent by Xyratex. No further rights or 
> licenses are granted to use such information. If you are not the intended 
> recipient of this message, please notify the sender by return and delete it. You 
> may not use, copy, disclose or rely on the information contained in it.
> 
> Internet email is susceptible to data corruption, interception and unauthorised 
> amendment for which Xyratex does not accept liability. While we have taken 
> reasonable precautions to ensure that this email is free of viruses, Xyratex 
> does not accept liability for the presence of any computer viruses in this 
> email, nor for any losses caused as a result of viruses.
> 
> Xyratex Technology Limited (03134912), Registered in England & Wales, 
> Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
> 
> The Xyratex group of companies also includes, Xyratex Ltd, registered in 
> Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) 
> Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in 
> The People's Republic of China and Xyratex Japan Limited registered in 
> Japan.
> ______________________________________________________________________
>
______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 

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

Re: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Dan Williams]

On Thu, Sep 22, 2011 at 9:55 AM, Mark Salyzyn
<mark_salyzyn@us.xyratex.com> wrote:
> Except for the fact the patch as located in the attachment is still not in compliance, the patch works for me.
>
> Sincerely -- Mark Salyzyn
>
>> scsi-misc-2.6/scripts/checkpatch.pl 0001-SCSI-libsas-Allow-expander-T-T-attachments.patch
> WARNING: line over 80 characters
> #57: FILE: drivers/scsi/libsas/sas_expander.c:1245:
> +                               if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||
>
> WARNING: line over 80 characters
> #59: FILE: drivers/scsi/libsas/sas_expander.c:1247:
> +                                    child_ex->t2t_supp && parent_ex->t2t_supp)) {
>
> WARNING: line over 80 characters
> #62: FILE: drivers/scsi/libsas/sas_expander.c:1250:
> +                                       sas_print_parent_topology_bug(child, parent_phy, child_phy);
>
> total: 0 errors, 3 warnings, 82 lines checked
>

Minor infractions of 80 columns are acceptable when they improve readability.

checkpactch clean to me is no errors.

--
Dan

Re: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Dan Williams]

On Thu, Sep 22, 2011 at 9:55 AM, Mark Salyzyn
<mark_salyzyn@us.xyratex.com> wrote:
> Except for the fact the patch as located in the attachment is still not in compliance, the patch works for me.
>
> Sincerely -- Mark Salyzyn
>
>> scsi-misc-2.6/scripts/checkpatch.pl 0001-SCSI-libsas-Allow-expander-T-T-attachments.patch
> WARNING: line over 80 characters
> #57: FILE: drivers/scsi/libsas/sas_expander.c:1245:
> +                               if (child_phy->routing_attr == SUBTRACTIVE_ROUTING ||
>
> WARNING: line over 80 characters
> #59: FILE: drivers/scsi/libsas/sas_expander.c:1247:
> +                                    child_ex->t2t_supp && parent_ex->t2t_supp)) {
>
> WARNING: line over 80 characters
> #62: FILE: drivers/scsi/libsas/sas_expander.c:1250:
> +                                       sas_print_parent_topology_bug(child, parent_phy, child_phy);
>
> total: 0 errors, 3 warnings, 82 lines checked
>

Minor infractions of 80 columns are acceptable when they improve readability.

checkpactch clean to me is no errors.

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

Re: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Dan Williams]

On Thu, Sep 22, 2011 at 9:41 AM, Luben Tuikov <ltuikov@yahoo.com> wrote:
> Allow expander table-to-table attachments for
> expanders that support it.
>
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>

Acked-by: Dan Williams <dan.j.williams@intel.com>

...and can we tag this for -stable.  It meets the 100 line count and
"New device IDs and quirks are also accepted." constraints.

Re: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Jack Wang]

Re: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> On Thu, Sep 22, 2011 at 9:41 AM, Luben Tuikov <ltuikov@yahoo.com> wrote:
> > Allow expander table-to-table attachments for
> > expanders that support it.
> >
> > Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> 
> Acked-by: Dan Williams <dan.j.williams@intel.com>

[Jack Wang] Acked-by: Jack Wang <jack_wang@usish.com>
> 
> ...and can we tag this for -stable.  It meets the 100 line count and
> "New device IDs and quirks are also accepted." constraints.
> --
> 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

Re: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments

[Jack Wang]

Re: [RESEND] [PATCH] [SCSI] libsas: Allow expander T-T attachments
> 
> On Thu, Sep 22, 2011 at 9:41 AM, Luben Tuikov <ltuikov@yahoo.com> wrote:
> > Allow expander table-to-table attachments for
> > expanders that support it.
> >
> > Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> 
> Acked-by: Dan Williams <dan.j.williams@intel.com>

[Jack Wang] Acked-by: Jack Wang <jack_wang@usish.com>
> 
> ...and can we tag this for -stable.  It meets the 100 line count and
> "New device IDs and quirks are also accepted." constraints.
> --
> 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