[PATCH] ch: add refcounting

[Hannes Reinecke]

struct scsi_changer needs refcounting as the device
might be removed while the fd is still open.

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/ch.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index dad959f..b14583a 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -105,6 +105,7 @@
 static struct class * ch_sysfs_class;
 
 typedef struct {
+   struct kref ref;
struct list_headlist;
int minor;
charname[8];
@@ -563,13 +564,23 @@ static int ch_gstatus(scsi_changer *ch, int type, 
unsigned char __user *dest)
 
 /*  */
 
+static void ch_destroy(struct kref *ref)
+{
+   scsi_changer *ch = container_of(ref, scsi_changer, ref);
+
+   kfree(ch->dt);
+   kfree(ch);
+}
+
 static int
 ch_release(struct inode *inode, struct file *file)
 {
scsi_changer *ch = file->private_data;
 
scsi_device_put(ch->device);
+   ch->device = NULL;
file->private_data = NULL;
+   kref_put(>ref, ch_destroy);
return 0;
 }
 
@@ -588,6 +599,7 @@ static int ch_gstatus(scsi_changer *ch, int type, unsigned 
char __user *dest)
mutex_unlock(_mutex);
return -ENXIO;
}
+   kref_get(>ref);
spin_unlock(_index_lock);
 
file->private_data = ch;
@@ -935,8 +947,11 @@ static int ch_probe(struct device *dev)
}
 
mutex_init(>lock);
+   kref_init(>ref);
ch->device = sd;
-   ch_readconfig(ch);
+   ret = ch_readconfig(ch);
+   if (ret)
+   goto destroy_dev;
if (init)
ch_init_elem(ch);
 
@@ -944,6 +959,8 @@ static int ch_probe(struct device *dev)
sdev_printk(KERN_INFO, sd, "Attached scsi changer %s\n", ch->name);
 
return 0;
+destroy_dev:
+   device_destroy(ch_sysfs_class, MKDEV(SCSI_CHANGER_MAJOR,ch->minor));
 remove_idr:
idr_remove(_index_idr, ch->minor);
 free_ch:
@@ -960,8 +977,7 @@ static int ch_remove(struct device *dev)
spin_unlock(_index_lock);
 
device_destroy(ch_sysfs_class, MKDEV(SCSI_CHANGER_MAJOR,ch->minor));
-   kfree(ch->dt);
-   kfree(ch);
+   kref_put(>ref, ch_destroy);
return 0;
 }
 
-- 
1.8.5.6

[PATCH 2/6] qedf: Use granted MAC from the FCF for the FCoE source address if it is available.

[Chad Dupuis]

Currently in the driver we've been using the fc_fcoe_set_mac() function to
set the source MAC for FCoE traffic.  This works well in most cases as it
uses the spec. default FCF-MAC.  However, if the administrator changes the
FCF-MAC switch, then any FCoE traffic we send will be dropped by the
switch.

Instead we should check the granted MAC from the FLOGI payload and use that
address if it is present.  Otherwise, fall back to using the the default
FCF-MAC and the fabric ID of the port as the FCoE MAC address.

Once this address is known we need to set it when doing non-offload
traffic, offload traffic and setting the data_src_address libfcoe uses for
FIP keep alive messages.

Signed-off-by: Chad Dupuis 
---
 drivers/scsi/qedf/qedf.h  |  1 -
 drivers/scsi/qedf/qedf_fip.c  | 17 -
 drivers/scsi/qedf/qedf_main.c | 59 +--
 3 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/qedf/qedf.h b/drivers/scsi/qedf/qedf.h
index 351f06dfc5a0..e05ec331557b 100644
--- a/drivers/scsi/qedf/qedf.h
+++ b/drivers/scsi/qedf/qedf.h
@@ -443,7 +443,6 @@ extern void qedf_cmd_mgr_free(struct qedf_cmd_mgr *cmgr);
 extern int qedf_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *sc_cmd);
 extern void qedf_fip_send(struct fcoe_ctlr *fip, struct sk_buff *skb);
-extern void qedf_update_src_mac(struct fc_lport *lport, u8 *addr);
 extern u8 *qedf_get_src_mac(struct fc_lport *lport);
 extern void qedf_fip_recv(struct qedf_ctx *qedf, struct sk_buff *skb);
 extern void qedf_fcoe_send_vlan_req(struct qedf_ctx *qedf);
diff --git a/drivers/scsi/qedf/qedf_fip.c b/drivers/scsi/qedf/qedf_fip.c
index aefd24ca9604..28ce0f7ffc7b 100644
--- a/drivers/scsi/qedf/qedf_fip.c
+++ b/drivers/scsi/qedf/qedf_fip.c
@@ -242,26 +242,9 @@ void qedf_fip_recv(struct qedf_ctx *qedf, struct sk_buff 
*skb)
}
 }
 
-void qedf_update_src_mac(struct fc_lport *lport, u8 *addr)
-{
-   struct qedf_ctx *qedf = lport_priv(lport);
-
-   QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_DISC,
-   "Setting data_src_addr=%pM.\n", addr);
-   ether_addr_copy(qedf->data_src_addr, addr);
-}
-
 u8 *qedf_get_src_mac(struct fc_lport *lport)
 {
-   u8 mac[ETH_ALEN];
-   u8 port_id[3];
struct qedf_ctx *qedf = lport_priv(lport);
 
-   /* We need to use the lport port_id to create the data_src_addr */
-   if (is_zero_ether_addr(qedf->data_src_addr)) {
-   hton24(port_id, lport->port_id);
-   fc_fcoe_set_mac(mac, port_id);
-   qedf->ctlr.update_mac(lport, mac);
-   }
return qedf->data_src_addr;
 }
diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index 9744b6aa2a03..1df80aa03da1 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -187,6 +188,50 @@ static void qedf_handle_link_update(struct work_struct 
*work)
}
 }
 
+#defineQEDF_FCOE_MAC_METHOD_GRANGED_MAC1
+#define QEDF_FCOE_MAC_METHOD_FCF_MAP   2
+#define QEDF_FCOE_MAC_METHOD_FCOE_SET_MAC  3
+static void qedf_set_data_src_addr(struct qedf_ctx *qedf, struct fc_frame *fp)
+{
+   u8 *granted_mac;
+   struct fc_frame_header *fh = fc_frame_header_get(fp);
+   u8 fc_map[3];
+   int method = 0;
+
+   /* Get granted MAC address from FIP FLOGI payload */
+   granted_mac = fr_cb(fp)->granted_mac;
+
+   /*
+* We set the source MAC for FCoE traffic based on the Granted MAC
+* address from the switch.
+*
+* If granted_mac is non-zero, we used that.
+* If the granted_mac is zeroed out, created the FCoE MAC based on
+* the sel_fcf->fc_map and the d_id fo the FLOGI frame.
+* If sel_fcf->fc_map is 0 then we use the default FCF-MAC plus the
+* d_id of the FLOGI frame.
+*/
+   if (!is_zero_ether_addr(granted_mac)) {
+   ether_addr_copy(qedf->data_src_addr, granted_mac);
+   method = QEDF_FCOE_MAC_METHOD_GRANGED_MAC;
+   } else if (qedf->ctlr.sel_fcf->fc_map != 0) {
+   hton24(fc_map, qedf->ctlr.sel_fcf->fc_map);
+   qedf->data_src_addr[0] = fc_map[0];
+   qedf->data_src_addr[1] = fc_map[1];
+   qedf->data_src_addr[2] = fc_map[2];
+   qedf->data_src_addr[3] = fh->fh_d_id[0];
+   qedf->data_src_addr[4] = fh->fh_d_id[1];
+   qedf->data_src_addr[5] = fh->fh_d_id[2];
+   method = QEDF_FCOE_MAC_METHOD_FCF_MAP;
+   } else {
+   fc_fcoe_set_mac(qedf->data_src_addr, fh->fh_d_id);
+   method = QEDF_FCOE_MAC_METHOD_FCOE_SET_MAC;
+   }
+
+   QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_DISC,
+   "QEDF data_src_mac=%pM method=%d.\n", qedf->data_src_addr, method);
+}
+
 static void qedf_flogi_resp(struct 

[PATCH 1/6] qedf: Set WWNN and WWPN based on values from qed.

[Chad Dupuis]

If dev_info.wwpn and dev_info.wwnn are set by qed use these values to set
the WWNs of the port. Otherwise fall back to the old method using
fcoe_wwn_from_mac().

Signed-off-by: Chad Dupuis 
---
 drivers/scsi/qedf/qedf_main.c | 21 ++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index 1d13c9ca517d..9744b6aa2a03 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -3056,9 +3056,24 @@ static int __qedf_probe(struct pci_dev *pdev, int mode)
QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_DISC, "MAC address is %pM.\n",
   qedf->mac);
 
-   /* Set the WWNN and WWPN based on the MAC address */
-   qedf->wwnn = fcoe_wwn_from_mac(qedf->mac, 1, 0);
-   qedf->wwpn = fcoe_wwn_from_mac(qedf->mac, 2, 0);
+   /*
+* Set the WWNN and WWPN in the following way:
+*
+* If the info we get from qed is non-zero then use that to set the
+* WWPN and WWNN. Otherwise fall back to use fcoe_wwn_from_mac() based
+* on the MAC address.
+*/
+   if (qedf->dev_info.wwnn != 0 && qedf->dev_info.wwpn != 0) {
+   QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_DISC,
+   "Setting WWPN and WWNN from qed dev_info.\n");
+   qedf->wwnn = qedf->dev_info.wwnn;
+   qedf->wwpn = qedf->dev_info.wwpn;
+   } else {
+   QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_DISC,
+   "Setting WWPN and WWNN using fcoe_wwn_from_mac().\n");
+   qedf->wwnn = fcoe_wwn_from_mac(qedf->mac, 1, 0);
+   qedf->wwpn = fcoe_wwn_from_mac(qedf->mac, 2, 0);
+   }
QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_DISC,  "WWNN=%016llx "
   "WWPN=%016llx.\n", qedf->wwnn, qedf->wwpn);
 
-- 
2.12.3

[PATCH 0/6] Update qedf to version 8.20.5.0.

[Chad Dupuis]

From: "Chad Dupuis" 

Hi Martin,

Please apply the following patches at your earliest convenience.

Thanks,
Chad

Chad Dupuis (6):
  qedf: Set WWNN and WWPN based on values from qed.
  qedf: Use granted MAC from the FCF for the FCoE source address if it
is available.
  qedf: Corrent VLAN tag insertion in fallback VLAN case.
  qedf: Covert single-threaded workqueues to regular workqueues.
  qedf: Fix up modinfo parameter name for 'debug' in modinfo output.
  qedf: Update driver version to 8.20.5.0.

 drivers/scsi/qedf/qedf.h |   2 -
 drivers/scsi/qedf/qedf_fip.c |  35 +++---
 drivers/scsi/qedf/qedf_main.c| 102 +++
 drivers/scsi/qedf/qedf_version.h |   6 +--
 4 files changed, 92 insertions(+), 53 deletions(-)

-- 
2.12.3

[PATCH 3/6] qedf: Corrent VLAN tag insertion in fallback VLAN case.

[Chad Dupuis]

Currently in the driver the qedf_ctx attribute vlan_hw_insert is used to
which whether to insert a VLAN tag in FIP frames (except for FIP VLAN
request which is explicitly sent out untagged at least from the driver's
point of view).

When we receive a FIP VLAN response, we set qedf->vlan_hw_insert to 0 which
makes the qedf_fip_send function insert the VLAN.  However when we exhaust
our FIP VLAN retries, we do not set qedf->vlan_hw_insert to 0 which means
that the driver will not tag the FIP frame with the correct VLAN ID.  The
result that was observed on the wire is that some entity either int he LL2
or L2 firmware is adding a NULL VLAN tag which can cause FIP solicitation
to fail.

The offload FCoE frame function, qedf_xmit, does not use the vlan_hw_insert
attribute to decide whether to tag frames with the FIP/FCoE VLAN.  Instead
it unilaterially tags the offload frames with the VLAN ID stored in
qedf->vlan_id. This is the correct behavior so the driver can guarantee
that non-offload FIP frames go out with the correct VLAN ID.

Also use the Linux network layer helpers instead of doing the VLAN insert
manually.

Also fix setting the fallback VLAN so that it used the module parameter and
is not hardcoded to 1002 (though 1002 is the default).

Signed-off-by: Chad Dupuis 
---
 drivers/scsi/qedf/qedf.h  |  1 -
 drivers/scsi/qedf/qedf_fip.c  | 18 +++---
 drivers/scsi/qedf/qedf_main.c | 12 
 3 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/qedf/qedf.h b/drivers/scsi/qedf/qedf.h
index e05ec331557b..9bf7b227e69a 100644
--- a/drivers/scsi/qedf/qedf.h
+++ b/drivers/scsi/qedf/qedf.h
@@ -300,7 +300,6 @@ struct qedf_ctx {
 #define QEDF_FALLBACK_VLAN 1002
 #define QEDF_DEFAULT_PRIO  3
int vlan_id;
-   uint vlan_hw_insert:1;
struct qed_dev *cdev;
struct qed_dev_fcoe_info dev_info;
struct qed_int_info int_info;
diff --git a/drivers/scsi/qedf/qedf_fip.c b/drivers/scsi/qedf/qedf_fip.c
index 28ce0f7ffc7b..773558fc0697 100644
--- a/drivers/scsi/qedf/qedf_fip.c
+++ b/drivers/scsi/qedf/qedf_fip.c
@@ -108,7 +108,6 @@ void qedf_fip_send(struct fcoe_ctlr *fip, struct sk_buff 
*skb)
 {
struct qedf_ctx *qedf = container_of(fip, struct qedf_ctx, ctlr);
struct ethhdr *eth_hdr;
-   struct vlan_ethhdr *vlan_hdr;
struct fip_header *fiph;
u16 op, vlan_tci = 0;
u8 sub;
@@ -124,16 +123,14 @@ void qedf_fip_send(struct fcoe_ctlr *fip, struct sk_buff 
*skb)
op = ntohs(fiph->fip_op);
sub = fiph->fip_subcode;
 
-   if (!qedf->vlan_hw_insert) {
-   vlan_hdr = skb_push(skb, sizeof(*vlan_hdr) - sizeof(*eth_hdr));
-   memcpy(vlan_hdr, eth_hdr, 2 * ETH_ALEN);
-   vlan_hdr->h_vlan_proto = htons(ETH_P_8021Q);
-   vlan_hdr->h_vlan_encapsulated_proto = eth_hdr->h_proto;
-   vlan_hdr->h_vlan_TCI = vlan_tci =  htons(qedf->vlan_id);
-   }
+   /*
+* Add VLAN tag to non-offload FIP frame based on current stored VLAN
+* for FIP/FCoE traffic.
+*/
+   __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), qedf->vlan_id);
 
-   /* Update eth_hdr since we added a VLAN tag */
-   eth_hdr = (struct ethhdr *)skb_mac_header(skb);
+   /* Get VLAN ID from skb for printing purposes */
+   __vlan_hwaccel_get_tag(skb, _tci);
 
QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_LL2, "FIP frame send: "
"dest=%pM op=%x sub=%x vlan=%04x.", eth_hdr->h_dest, op, sub,
@@ -174,7 +171,6 @@ void qedf_fip_recv(struct qedf_ctx *qedf, struct sk_buff 
*skb)
/* Handle FIP VLAN resp in the driver */
if (op == FIP_OP_VLAN && sub == FIP_SC_VL_NOTE) {
qedf_fcoe_process_vlan_resp(qedf, skb);
-   qedf->vlan_hw_insert = 0;
kfree_skb(skb);
} else if (op == FIP_OP_CTRL && sub == FIP_SC_CLR_VLINK) {
QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_DISC, "Clear virtual "
diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index 1df80aa03da1..0520dd1be749 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -164,7 +164,7 @@ static void qedf_handle_link_update(struct work_struct 
*work)
QEDF_WARN(&(qedf->dbg_ctx), "Did not receive FIP VLAN "
   "response, falling back to default VLAN %d.\n",
   qedf_fallback_vlan);
-   qedf_set_vlan_id(qedf, QEDF_FALLBACK_VLAN);
+   qedf_set_vlan_id(qedf, qedf_fallback_vlan);
 
/*
 * Zero out data_src_addr so we'll update it with the new
@@ -361,8 +361,9 @@ static void qedf_link_recovery(struct work_struct *work)
/* Since the link when down and up to verify which vlan we're on */
qedf->fipvlan_retries = qedf_fipvlan_retries;
rc = qedf_initiate_fipvlan_req(qedf);
+   /* If getting the VLAN fails, set the 

[PATCH 4/6] qedf: Covert single-threaded workqueues to regular workqueues.

[Chad Dupuis]

There is no ordering required for the various workqueues the driver uses
so they can be converted to regular workqueues.

Signed-off-by: Chad Dupuis 
---
 drivers/scsi/qedf/qedf_main.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index 0520dd1be749..24e8d2ab099c 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -2987,7 +2987,7 @@ static int __qedf_probe(struct pci_dev *pdev, int mode)
 
sprintf(host_buf, "qedf_%u_link",
qedf->lport->host->host_no);
-   qedf->link_update_wq = create_singlethread_workqueue(host_buf);
+   qedf->link_update_wq = create_workqueue(host_buf);
INIT_DELAYED_WORK(>link_update, qedf_handle_link_update);
INIT_DELAYED_WORK(>link_recovery, qedf_link_recovery);
 
@@ -3157,7 +3157,7 @@ static int __qedf_probe(struct pci_dev *pdev, int mode)
/* Start LL2 processing thread */
snprintf(host_buf, 20, "qedf_%d_ll2", host->host_no);
qedf->ll2_recv_wq =
-   create_singlethread_workqueue(host_buf);
+   create_workqueue(host_buf);
if (!qedf->ll2_recv_wq) {
QEDF_ERR(&(qedf->dbg_ctx), "Failed to LL2 workqueue.\n");
goto err7;
@@ -3199,7 +3199,7 @@ static int __qedf_probe(struct pci_dev *pdev, int mode)
 
sprintf(host_buf, "qedf_%u_timer", qedf->lport->host->host_no);
qedf->timer_work_queue =
-   create_singlethread_workqueue(host_buf);
+   create_workqueue(host_buf);
if (!qedf->timer_work_queue) {
QEDF_ERR(&(qedf->dbg_ctx), "Failed to start timer "
  "workqueue.\n");
@@ -3210,7 +3210,7 @@ static int __qedf_probe(struct pci_dev *pdev, int mode)
if (mode != QEDF_MODE_RECOVERY) {
sprintf(host_buf, "qedf_%u_dpc",
qedf->lport->host->host_no);
-   qedf->dpc_wq = create_singlethread_workqueue(host_buf);
+   qedf->dpc_wq = create_workqueue(host_buf);
}
 
/*
-- 
2.12.3

[PATCH 5/6] qedf: Fix up modinfo parameter name for 'debug' in modinfo output.

[Chad Dupuis]

Because we were passing 'qedf_debug' instead of 'debug' to the
MODULE_PARM_DESC() macro, modinfo listed the parameter name as 'qedf_debug'
instead of it's proper name 'debug'.  Correct the parameter name.

Signed-off-by: Chad Dupuis 
---
 drivers/scsi/qedf/qedf_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index 24e8d2ab099c..e192bde73147 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -43,7 +43,7 @@ MODULE_PARM_DESC(dev_loss_tmo,  " dev_loss_tmo setting for 
attached "
 
 uint qedf_debug = QEDF_LOG_INFO;
 module_param_named(debug, qedf_debug, uint, S_IRUGO);
-MODULE_PARM_DESC(qedf_debug, " Debug mask. Pass '1' to enable default 
debugging"
+MODULE_PARM_DESC(debug, " Debug mask. Pass '1' to enable default debugging"
" mask");
 
 static uint qedf_fipvlan_retries = 30;
-- 
2.12.3

[PATCH 6/6] qedf: Update driver version to 8.20.5.0.

[Chad Dupuis]

Signed-off-by: Chad Dupuis 
---
 drivers/scsi/qedf/qedf_version.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/qedf/qedf_version.h b/drivers/scsi/qedf/qedf_version.h
index 6fa442061c32..397b3b8ee51a 100644
--- a/drivers/scsi/qedf/qedf_version.h
+++ b/drivers/scsi/qedf/qedf_version.h
@@ -7,9 +7,9 @@
  *  this source tree.
  */
 
-#define QEDF_VERSION   "8.18.22.0"
+#define QEDF_VERSION   "8.20.5.0"
 #define QEDF_DRIVER_MAJOR_VER  8
-#define QEDF_DRIVER_MINOR_VER  18
-#define QEDF_DRIVER_REV_VER22
+#define QEDF_DRIVER_MINOR_VER  20
+#define QEDF_DRIVER_REV_VER5
 #define QEDF_DRIVER_ENG_VER0
 
-- 
2.12.3

[PATCH] scsi: cxlflash: Fix an error handling path in 'cxlflash_disk_attach()'

[Christophe JAILLET]

'rc' is known to be 0 at this point.
If 'create_context()' fails, returns -ENOMEM instead of 0 which means
success.

Signed-off-by: Christophe JAILLET 
---
 drivers/scsi/cxlflash/superpipe.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/cxlflash/superpipe.c 
b/drivers/scsi/cxlflash/superpipe.c
index ad0f9968ccfb..08da593cb2f6 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -1390,6 +1390,7 @@ static int cxlflash_disk_attach(struct scsi_device *sdev,
if (unlikely(!ctxi)) {
dev_err(dev, "%s: Failed to create context ctxid=%d\n",
__func__, ctxid);
+   rc = -ENOMEM;
goto err;
}
 
-- 
2.11.0

Re: [PATCHv3 2/5] scsi: Export blacklist flags to sysfs

[Bart Van Assche]

On Tue, 2017-08-15 at 10:03 +0200, Hannes Reinecke wrote:
> On 08/15/2017 05:18 AM, Martin K. Petersen wrote:
> > 
> > Hannes,
> > 
> > > + name = sdev_bflags_name(bflags);
> > > + if (name)
> > > + blen = snprintf(ptr, strlen(name) + 1,
> > > + "%s", name);
> > > + else
> > > + blen = snprintf(ptr, 67, "0x%X", bflags);
> > 
> > It seems this else statement facilitates papering over the fact that
> > scsi_sysfs.c and scsi_devinfo.h can get out of sync.
> > 
> 
> But there is no good way of avoiding that, is there?

Hello Hannes,

How about running the following shell code from a makefile, storing the
result in a file and #including that file from drivers/scsi/scsi_sysfs.c?

sed -n 's/^#define BLIST_\([^[:blank:]]*\).*/\t{ BLIST_\1, "\1" },/p' 
include/scsi/scsi_devinfo.h

Bart.

Re: 答复: [iscsi] Deadlock occurred when network is in error

[Bart Van Assche]

On Tue, 2017-08-15 at 02:16 +, Tangchen (UVP) wrote:
> But I'm not using mq, and I run into these two problems in a non-mq system.
> The patch you pointed out is fix for mq, so I don't think it can resolve this 
> problem.
> 
> IIUC, mq is for SSD ?  I'm not using ssd, so mq is disabled.

Hello Tangchen,

Please post replies below the original e-mail instead of above - that is the
reply style used on all Linux-related mailing lists I know of. From
https://en.wikipedia.org/wiki/Posting_style:

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

Regarding your question: sorry but I quoted the wrong commit in my previous
e-mail. The commit I should have referred to is 255ee9320e5d ("scsi: Make
__scsi_remove_device go straight from BLOCKED to DEL"). That patch not only
affects scsi-mq but also the single-queue code in the SCSI core.

blk-mq/scsi-mq was introduced for SSDs but is not only intended for SSDs.
The plan is to remove the blk-sq/scsi-sq code once the blk-mq/scsi-mq code
works at least as fast as the single queue code for all supported devices.
That includes hard disks.

Bart.

Re: [PATCHv3 1/5] scsi_debug: allow to specify inquiry vendor and model

[Bart Van Assche]

On Fri, 2017-08-11 at 16:23 +0200, Hannes Reinecke wrote:
> For testing purposes we need to be able to pass in the inquiry
> vendor and model.

Reviewed-by: Bart Van Assche 

Re: [PATCH] scsi: cxlflash: Fix an error handling path in 'cxlflash_disk_attach()'

[Andrew Donnellan]

On 16/08/17 06:18, Christophe JAILLET wrote:

'rc' is known to be 0 at this point.
If 'create_context()' fails, returns -ENOMEM instead of 0 which means
success.

Signed-off-by: Christophe JAILLET 


ENOMEM seems right here.

Reviewed-by: Andrew Donnellan 


---
  drivers/scsi/cxlflash/superpipe.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/cxlflash/superpipe.c 
b/drivers/scsi/cxlflash/superpipe.c
index ad0f9968ccfb..08da593cb2f6 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -1390,6 +1390,7 @@ static int cxlflash_disk_attach(struct scsi_device *sdev,
if (unlikely(!ctxi)) {
dev_err(dev, "%s: Failed to create context ctxid=%d\n",
__func__, ctxid);
+   rc = -ENOMEM;
goto err;
}
  



--
Andrew Donnellan  OzLabs, ADL Canberra
andrew.donnel...@au1.ibm.com  IBM Australia Limited

[PATCH] sg: protect against races between mmap() and SG_SET_RESERVED_SIZE

[Todd Poynor]

Take f_mutex around mmap() processing to protect against races with
the SG_SET_RESERVED_SIZE ioctl.  Ensure the reserve buffer length
remains consistent during the mapping operation, and set the
"mmap called" flag to prevent further changes to the reserved buffer
size as an atomic operation with the mapping.

Signed-off-by: Todd Poynor 
---
 drivers/scsi/sg.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 3a44b4bc872b..a20718e9f1f4 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1233,6 +1233,7 @@ sg_mmap(struct file *filp, struct vm_area_struct *vma)
unsigned long req_sz, len, sa;
Sg_scatter_hold *rsv_schp;
int k, length;
+int ret = 0;
 
if ((!filp) || (!vma) || (!(sfp = (Sg_fd *) filp->private_data)))
return -ENXIO;
@@ -1243,8 +1244,11 @@ sg_mmap(struct file *filp, struct vm_area_struct *vma)
if (vma->vm_pgoff)
return -EINVAL; /* want no offset */
rsv_schp = >reserve;
-   if (req_sz > rsv_schp->bufflen)
-   return -ENOMEM; /* cannot map more than reserved buffer */
+   mutex_lock(>f_mutex);
+   if (req_sz > rsv_schp->bufflen) {
+   ret = -ENOMEM;  /* cannot map more than reserved buffer */
+   goto out;
+   }
 
sa = vma->vm_start;
length = 1 << (PAGE_SHIFT + rsv_schp->page_order);
@@ -1258,7 +1262,9 @@ sg_mmap(struct file *filp, struct vm_area_struct *vma)
vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
vma->vm_private_data = sfp;
vma->vm_ops = _mmap_vm_ops;
-   return 0;
+out:
+   mutex_unlock(>f_mutex);
+   return ret;
 }
 
 static void
-- 
2.14.1.480.gb18f417b89-goog

Re: [PATCHv3 2/5] scsi: Export blacklist flags to sysfs

[Hannes Reinecke]

On 08/16/2017 12:47 AM, Bart Van Assche wrote:
> On Tue, 2017-08-15 at 10:03 +0200, Hannes Reinecke wrote:
>> On 08/15/2017 05:18 AM, Martin K. Petersen wrote:
>>>
>>> Hannes,
>>>
 +  name = sdev_bflags_name(bflags);
 +  if (name)
 +  blen = snprintf(ptr, strlen(name) + 1,
 +  "%s", name);
 +  else
 +  blen = snprintf(ptr, 67, "0x%X", bflags);
>>>
>>> It seems this else statement facilitates papering over the fact that
>>> scsi_sysfs.c and scsi_devinfo.h can get out of sync.
>>>
>>
>> But there is no good way of avoiding that, is there?
> 
> Hello Hannes,
> 
> How about running the following shell code from a makefile, storing the
> result in a file and #including that file from drivers/scsi/scsi_sysfs.c?
> 
> sed -n 's/^#define BLIST_\([^[:blank:]]*\).*/\t{ BLIST_\1, "\1" },/p' 
> include/scsi/scsi_devinfo.h
> 
Hmm. Yeah, should be possible.
I'll see if I'm able to massage this into the Makefile.

I still would like to keep the above, as the admin can feed blacklist
flags via the kernel commandline, and we don't do any validity checks on
that. So we might end up with invalid flags after all.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCHv3 3/5] scsi_devinfo: Reformat blacklist flags

[Hannes Reinecke]

On 08/16/2017 12:52 AM, Bart Van Assche wrote:
> On Fri, 2017-08-11 at 16:23 +0200, Hannes Reinecke wrote:
>> +typedef __u32 __bitwise sdev_bflags_t;
>> +
>> +/* Only scan LUN 0 */
>> +#define BLIST_NOLUN ((__force sdev_bflags_t)(1 << 0))
> 
> Hello Hannes,
> 
> Is the new typedef used anywhere outside the BLIST flag definitions?
> If not, how about skipping the introduction of a typedef and using
> u32 __bitwise directly?
> 
Yeah, can do.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH] scsi: ncr5380: constify pnp_device_id

[Arvind Yadav]

pnp_device_id are not supposed to change at runtime. All functions
working with pnp_device_id provided by  work with
const pnp_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/scsi/g_NCR5380.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
index c34fc91..1968d81 100644
--- a/drivers/scsi/g_NCR5380.c
+++ b/drivers/scsi/g_NCR5380.c
@@ -703,7 +703,7 @@ static struct isa_driver generic_NCR5380_isa_driver = {
 };
 
 #ifdef CONFIG_PNP
-static struct pnp_device_id generic_NCR5380_pnp_ids[] = {
+static const struct pnp_device_id generic_NCR5380_pnp_ids[] = {
{ .id = "DTC436e", .driver_data = BOARD_DTC3181E },
{ .id = "" }
 };
-- 
2.7.4

[PATCH] scsi: aha1542: constify pnp_device_id

[Arvind Yadav]

pnp_device_id are not supposed to change at runtime. All functions
working with pnp_device_id provided by  work with
const pnp_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/scsi/aha1542.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/aha1542.c b/drivers/scsi/aha1542.c
index a23cc9a..1242179 100644
--- a/drivers/scsi/aha1542.c
+++ b/drivers/scsi/aha1542.c
@@ -986,7 +986,7 @@ static struct isa_driver aha1542_isa_driver = {
 static int isa_registered;
 
 #ifdef CONFIG_PNP
-static struct pnp_device_id aha1542_pnp_ids[] = {
+static const struct pnp_device_id aha1542_pnp_ids[] = {
{ .id = "ADP1542" },
{ .id = "" }
 };
-- 
2.7.4

Re: [PATCHv3 5/5] scsi_devinfo: fixup string compare

[Hannes Reinecke]

On 08/16/2017 01:03 AM, Bart Van Assche wrote:
> On Fri, 2017-08-11 at 16:23 +0200, Hannes Reinecke wrote:
>> When checking the model and vendor string we need to use the
>> minimum value of either string, otherwise we'll miss out on
>> wildcard matches.
>> And we should take card when matching with zero size strings;
> 
> "card"? Did you perhaps mean "care"?
> 
Yes, indeed.

>> results might be unpredictable.
>> With this patch the rules for matching devinfo strings are
>> as follows:
>> - Vendor strings must match exactly
>> - Empty Model strings will only match if the devinfo model
>>   is also empty
>> - Model strings shorter than the devinfo model string will
>>   not match
> 
> The above sentence contradicts the first sentence.
> 
Ah. Will be rewriting this.

>> +/*
>> + * @model specifies the full string, and
>> + * must be larger or equal to devinfo->model
>> + */
> 
> What does "larger than or equal to" mean for strings? Did you perhaps mean 
> that
> devinfo->model must be a prefix of @model?
> 
Yes.

>>  if (!memcmp(devinfo->vendor, vendor,
>> - sizeof(devinfo->vendor)) &&
>> - !memcmp(devinfo->model, model,
>> -  sizeof(devinfo->model)))
>> +sizeof(devinfo->vendor)) &&
>> +!memcmp(devinfo->model, model,
>> +sizeof(devinfo->model)))
>>  return devinfo;
> 
> Is this a whitespace-only change? If so, please fold it in the previous patch.
> 
Ok.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH] sg: recheck MMAP_IO request length with lock held

[Todd Poynor]

Commit 1bc0eb044615 ("scsi: sg: protect accesses to 'reserved' page
array") adds needed concurrency protection for the "reserve" buffer.
Some checks that are initially made outside the lock are replicated once
the lock is taken to ensure the checks and resulting decisions are made
using consistent state.

The check that a request with flag SG_FLAG_MMAP_IO set fits in the
reserve buffer also needs to be performed again under the lock to
ensure the reserve buffer length compared against matches the value in
effect when the request is linked to the reserve buffer.  An -ENOMEM
should be returned in this case, instead of switching over to an
indirect buffer as for non-MMAP_IO requests.

Signed-off-by: Todd Poynor 
---
 drivers/scsi/sg.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index d7ff71e0c85c..3a44b4bc872b 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1735,9 +1735,12 @@ sg_start_req(Sg_request *srp, unsigned char *cmd)
!sfp->res_in_use) {
sfp->res_in_use = 1;
sg_link_reserve(sfp, srp, dxfer_len);
-   } else if ((hp->flags & SG_FLAG_MMAP_IO) && sfp->res_in_use) {
+   } else if (hp->flags & SG_FLAG_MMAP_IO) {
+   res = -EBUSY; /* sfp->res_in_use == 1 */
+   if (dxfer_len > rsv_schp->bufflen)
+   res = -ENOMEM;
mutex_unlock(>f_mutex);
-   return -EBUSY;
+   return res;
} else {
res = sg_build_indirect(req_schp, sfp, dxfer_len);
if (res) {
-- 
2.14.1.480.gb18f417b89-goog

Re: [PATCH] scsi: cxlflash: Fix an error handling path in 'cxlflash_disk_attach()'

[Matthew R. Ochs]

> On Aug 15, 2017, at 3:18 PM, Christophe JAILLET 
>  wrote:
> 
> 'rc' is known to be 0 at this point.
> If 'create_context()' fails, returns -ENOMEM instead of 0 which means
> success.
> 
> Signed-off-by: Christophe JAILLET 

Yep, that's a bug.

Acked-by: Matthew R. Ochs 

Re: [PATCHv3 2/5] scsi: Export blacklist flags to sysfs

[Martin K. Petersen]

Hannes,

>> It seems this else statement facilitates papering over the fact that
>> scsi_sysfs.c and scsi_devinfo.h can get out of sync.

> But there is no good way of avoiding that, is there?
> Out of necessity the definition of the bits and the decoding are two
> distinct steps, and as such are always prone to differences.
>
> So what is your suggestion here?

I was merely fishing for comments being added to both files stating that
it was important to keep them in sync.

However, if you can automate it using Bart's approach, stringify magic
or something else then that's cool. We have several places where
something like that would be useful.

-- 
Martin K. Petersen  Oracle Linux Engineering

RE: 答复: [iscsi] Deadlock occurred when network is in error

[Tangchen (UVP)]

> On Tue, 2017-08-15 at 02:16 +, Tangchen (UVP) wrote:
> > But I'm not using mq, and I run into these two problems in a non-mq system.
> > The patch you pointed out is fix for mq, so I don't think it can resolve 
> > this
> problem.
> >
> > IIUC, mq is for SSD ?  I'm not using ssd, so mq is disabled.
> 
> Hello Tangchen,
> 
> Please post replies below the original e-mail instead of above - that is the 
> reply
> style used on all Linux-related mailing lists I know of. From
> https://en.wikipedia.org/wiki/Posting_style:
> 
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?

Hi Bart,

Thanks for the reply. Will post the reply in e-mail. :)

> 
> Regarding your question: sorry but I quoted the wrong commit in my previous
> e-mail. The commit I should have referred to is 255ee9320e5d ("scsi: Make
> __scsi_remove_device go straight from BLOCKED to DEL"). That patch not only
> affects scsi-mq but also the single-queue code in the SCSI core.

OK, I'll try this one. Thx.

> 
> blk-mq/scsi-mq was introduced for SSDs but is not only intended for SSDs.
> The plan is to remove the blk-sq/scsi-sq code once the blk-mq/scsi-mq code
> works at least as fast as the single queue code for all supported devices.
> That includes hard disks.

OK, thanks for tell me this.

> 
> Bart.

Re: [PATCH 2/2] tests/scsi/0001: Regression test for SCSI device blacklisting

[Bart Van Assche]

On Wed, 2017-08-09 at 12:50 +0200, Hannes Reinecke wrote:
> +requires() {
> +if modinfo scsi_debug | grep -q inq_vendor ; then
> + return 0
> +fi
> +return 1
> +}

How about changing the above four statements into the following, which is
shorter and more robust?

modinfo scsi_debug | grep -q '^parm:[[:blank:]]*inq_vendor:'

> + modprobe scsi_debug inq_vendor="$vendor" inq_product="$model"
> + host=$(lsscsi -H | sed -n 's/.\([0-9]*\).*scsi_debug/\1/p')

Not all systems have lsscsi. How about using 
/sys/bus/pseudo/drivers/scsi_debug/adapter0/host*
instead?

> + rmmod scsi_debug

Please check the rmmod return value to catch unload failures due to a
user space process having opened the SCSI device created by scsi_debug.

Thanks,

Bart.

Re: [PATCHv3 2/5] scsi: Export blacklist flags to sysfs

[Martin K. Petersen]

Hannes,

>> Wouldn't debugfs be more appropriate for this attribute than sysfs?
>> 
> I so don't mind.
> I just need it somewhere in sysfs/debugfs so that I can validate the
> selection logic.
>
> Same goes for hexvalue vs decoded output; I just need to attribute;
> contents can be discussed.

I don't have a problem having it in sysfs. We have several users that
tweak the device blacklist in their boot scripts

Given that we already have an interface for setting the flags, we might
as well be able to print them.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: cxlflash: Fix an error handling path in 'cxlflash_disk_attach()'

[Martin K. Petersen]

Christophe,

> 'rc' is known to be 0 at this point.
> If 'create_context()' fails, returns -ENOMEM instead of 0 which means
> success.

Applied to 4.14/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCHv3 5/5] scsi_devinfo: fixup string compare

[Bart Van Assche]

On Fri, 2017-08-11 at 16:23 +0200, Hannes Reinecke wrote:
> When checking the model and vendor string we need to use the
> minimum value of either string, otherwise we'll miss out on
> wildcard matches.
> And we should take card when matching with zero size strings;

"card"? Did you perhaps mean "care"?

> results might be unpredictable.
> With this patch the rules for matching devinfo strings are
> as follows:
> - Vendor strings must match exactly
> - Empty Model strings will only match if the devinfo model
>   is also empty
> - Model strings shorter than the devinfo model string will
>   not match

The above sentence contradicts the first sentence.

> + /*
> +  * @model specifies the full string, and
> +  * must be larger or equal to devinfo->model
> +  */

What does "larger than or equal to" mean for strings? Did you perhaps mean that
devinfo->model must be a prefix of @model?

>   if (!memcmp(devinfo->vendor, vendor,
> -  sizeof(devinfo->vendor)) &&
> -  !memcmp(devinfo->model, model,
> -   sizeof(devinfo->model)))
> + sizeof(devinfo->vendor)) &&
> + !memcmp(devinfo->model, model,
> + sizeof(devinfo->model)))
>   return devinfo;

Is this a whitespace-only change? If so, please fold it in the previous patch.

Thanks,

Bart.

Re: [PATCHv3 3/5] scsi_devinfo: Reformat blacklist flags

[Bart Van Assche]

On Fri, 2017-08-11 at 16:23 +0200, Hannes Reinecke wrote:
> +typedef __u32 __bitwise sdev_bflags_t;
> +
> +/* Only scan LUN 0 */
> +#define BLIST_NOLUN  ((__force sdev_bflags_t)(1 << 0))

Hello Hannes,

Is the new typedef used anywhere outside the BLIST flag definitions?
If not, how about skipping the introduction of a typedef and using
u32 __bitwise directly?

Thanks,

Bart.

Re: [PATCH 1/2] tests/scsi: add SCSI midlayer test group

[Omar Sandoval]

On Wed, Aug 09, 2017 at 12:50:05PM +0200, Hannes Reinecke wrote:
> Add a test group for tests of the SCSI midlayer.
> 
> Signed-off-by: Hannes Reinecke 
> ---
>  tests/scsi/group | 26 ++
>  1 file changed, 26 insertions(+)
>  create mode 100644 tests/scsi/group
> 
> diff --git a/tests/scsi/group b/tests/scsi/group
> new file mode 100644
> index 000..73459a6
> --- /dev/null
> +++ b/tests/scsi/group
> @@ -0,0 +1,26 @@
> +#!/bin/bash
> +#
> +# SCSI regression tests
> +#
> +# Copyright (C) 2017 Hannes Reinecke, SUSE Linux GmbH
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see .
> +
> +#
> +# Usually, group_requires() just needs to check that any necessary programs 
> and
> +# kernel features are available using the _have_foo helpers. If
> +# group_requires() returns non-zero, all tests in this group will be skipped.

This comment should have been removed, looks fine otherwise.

> +group_requires() {
> + _have_root
> +}
> -- 
> 1.8.5.6
> 

Re: [PATCHv3 2/5] scsi: Export blacklist flags to sysfs

[Bart Van Assche]

On Fri, 2017-08-11 at 16:23 +0200, Hannes Reinecke wrote:
> @@ -1138,6 +1220,7 @@ static umode_t scsi_sdev_bin_attr_is_visible(struct 
> kobject *kobj,
>   _attr_queue_depth.attr,
>   _attr_queue_type.attr,
>   _attr_wwid.attr,
> + _attr_blacklist.attr,
>  #ifdef CONFIG_SCSI_DH
>   _attr_dh_state.attr,
>   _attr_access_state.attr,

Hello Hannes,

Wouldn't debugfs be more appropriate for this attribute than sysfs?

Thanks,

Bart.

Re: [PATCHv3 2/5] scsi: Export blacklist flags to sysfs

[Hannes Reinecke]

On 08/15/2017 04:24 PM, Bart Van Assche wrote:
> On Fri, 2017-08-11 at 16:23 +0200, Hannes Reinecke wrote:
>> @@ -1138,6 +1220,7 @@ static umode_t scsi_sdev_bin_attr_is_visible(struct 
>> kobject *kobj,
>>  _attr_queue_depth.attr,
>>  _attr_queue_type.attr,
>>  _attr_wwid.attr,
>> +_attr_blacklist.attr,
>>  #ifdef CONFIG_SCSI_DH
>>  _attr_dh_state.attr,
>>  _attr_access_state.attr,
> 
> Hello Hannes,
> 
> Wouldn't debugfs be more appropriate for this attribute than sysfs?
> 
I so don't mind.
I just need it somewhere in sysfs/debugfs so that I can validate the
selection logic.

Same goes for hexvalue vs decoded output; I just need to attribute;
contents can be discussed.

Martin?

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCHv2 0/6] hpsa legacy support

[Hannes Reinecke]

On 08/15/2017 05:39 AM, Martin K. Petersen wrote:
> 
> Hannes,
> 
>> here's now the second attempt to add support for legacy boards, ie for
>> boards previously supported by cciss only.  With this patchset the
>> hpsa driver should work with all Smart Array boards if the
>> 'hpsa_allow_any' module option is set, rendering the cciss driver
>> obsolete.  Consequently I've added a patch to remove the cciss driver
>> and make 'hpsa' an alias to 'cciss'.
> 
> How does the module alias solve the problem that hpsa_allow_any=1 needs
> to be added for users currently running cciss?
> 
> I also fail to see what the point is of having an opt-in parameter when
> there is no alternative after cciss has been removed. It's a
> don't-be-broken switch.
> 
Hmm ... considering this (and the fact that we now have the
->legacy_board switch) we can indeed remove that option.
Will be redoing the patchset.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCHv3 1/6] hpsa: add support for legacy boards

[Hannes Reinecke]

Add support for legacy boards, ensuring to enable the driver for
those boards only when 'hpsa_allow_any' is set.
The attribute 'legacy_board' is set to '1' if the device is
a legacy board, and '0' otherwise.

Signed-off-by: Hannes Reinecke 
Acked-by: Don Brace 
---
 drivers/scsi/hpsa.c | 67 +++-
 drivers/scsi/hpsa.h | 81 -
 2 files changed, 121 insertions(+), 27 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 4f7cdb2..fbe7fbc 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -148,6 +148,8 @@
{PCI_VENDOR_ID_HP, 0x333f, 0x103c, 0x333f},
{PCI_VENDOR_ID_HP, PCI_ANY_ID,  PCI_ANY_ID, PCI_ANY_ID,
PCI_CLASS_STORAGE_RAID << 8, 0x << 8, 0},
+   {PCI_VENDOR_ID_COMPAQ, PCI_ANY_ID,  PCI_ANY_ID, PCI_ANY_ID,
+   PCI_CLASS_STORAGE_RAID << 8, 0x << 8, 0},
{0,}
 };
 
@@ -158,6 +160,26 @@
  *  access = Address of the struct of function pointers
  */
 static struct board_type products[] = {
+   {0x40700E11, "Smart Array 5300", _access},
+   {0x40800E11, "Smart Array 5i", _access},
+   {0x40820E11, "Smart Array 532", _access},
+   {0x40830E11, "Smart Array 5312", _access},
+   {0x409A0E11, "Smart Array 641", _access},
+   {0x409B0E11, "Smart Array 642", _access},
+   {0x409C0E11, "Smart Array 6400", _access},
+   {0x409D0E11, "Smart Array 6400 EM", _access},
+   {0x40910E11, "Smart Array 6i", _access},
+   {0x3225103C, "Smart Array P600", _access},
+   {0x3223103C, "Smart Array P800", _access},
+   {0x3234103C, "Smart Array P400", _access},
+   {0x3235103C, "Smart Array P400i", _access},
+   {0x3211103C, "Smart Array E200i", _access},
+   {0x3212103C, "Smart Array E200", _access},
+   {0x3213103C, "Smart Array E200i", _access},
+   {0x3214103C, "Smart Array E200i", _access},
+   {0x3215103C, "Smart Array E200i", _access},
+   {0x3237103C, "Smart Array E500", _access},
+   {0x323D103C, "Smart Array P700m", _access},
{0x3241103C, "Smart Array P212", _access},
{0x3243103C, "Smart Array P410", _access},
{0x3245103C, "Smart Array P410i", _access},
@@ -278,7 +300,8 @@ static int hpsa_find_cfg_addrs(struct pci_dev *pdev, void 
__iomem *vaddr,
   u64 *cfg_offset);
 static int hpsa_pci_find_memory_BAR(struct pci_dev *pdev,
unsigned long *memory_bar);
-static int hpsa_lookup_board_id(struct pci_dev *pdev, u32 *board_id);
+static int hpsa_lookup_board_id(struct pci_dev *pdev, u32 *board_id,
+   bool *legacy_board);
 static int wait_for_device_to_become_ready(struct ctlr_info *h,
   unsigned char lunaddr[],
   int reply_queue);
@@ -866,6 +889,16 @@ static ssize_t host_show_ctlr_num(struct device *dev,
return snprintf(buf, 20, "%d\n", h->ctlr);
 }
 
+static ssize_t host_show_legacy_board(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct ctlr_info *h;
+   struct Scsi_Host *shost = class_to_shost(dev);
+
+   h = shost_to_hba(shost);
+   return snprintf(buf, 20, "%d\n", h->legacy_board ? 1 : 0);
+}
+
 static DEVICE_ATTR(raid_level, S_IRUGO, raid_level_show, NULL);
 static DEVICE_ATTR(lunid, S_IRUGO, lunid_show, NULL);
 static DEVICE_ATTR(unique_id, S_IRUGO, unique_id_show, NULL);
@@ -891,6 +924,8 @@ static DEVICE_ATTR(lockup_detected, S_IRUGO,
host_show_lockup_detected, NULL);
 static DEVICE_ATTR(ctlr_num, S_IRUGO,
host_show_ctlr_num, NULL);
+static DEVICE_ATTR(legacy_board, S_IRUGO,
+   host_show_legacy_board, NULL);
 
 static struct device_attribute *hpsa_sdev_attrs[] = {
_attr_raid_level,
@@ -912,6 +947,7 @@ static DEVICE_ATTR(ctlr_num, S_IRUGO,
_attr_raid_offload_debug,
_attr_lockup_detected,
_attr_ctlr_num,
+   _attr_legacy_board,
NULL,
 };
 
@@ -7232,7 +7268,8 @@ static int hpsa_interrupt_mode(struct ctlr_info *h)
return 0;
 }
 
-static int hpsa_lookup_board_id(struct pci_dev *pdev, u32 *board_id)
+static int hpsa_lookup_board_id(struct pci_dev *pdev, u32 *board_id,
+   bool *legacy_board)
 {
int i;
u32 subsystem_vendor_id, subsystem_device_id;
@@ -7242,9 +7279,22 @@ static int hpsa_lookup_board_id(struct pci_dev *pdev, 
u32 *board_id)
*board_id = ((subsystem_device_id << 16) & 0x) |
subsystem_vendor_id;
 
+   if (legacy_board)
+   *legacy_board = false;
for (i = 0; i < ARRAY_SIZE(products); i++)
-   if (*board_id == products[i].board_id)
-   return i;
+   if (*board_id == products[i].board_id) {
+   if (products[i].access != 

[PATCHv3 2/6] hpsa: disable volume status check for legacy boards

[Hannes Reinecke]

Legacy boards might not support volume status, so assume
the volume is online here.

Signed-off-by: Hannes Reinecke 
Acked-by: Don Brace 
---
 drivers/scsi/hpsa.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index fbe7fbc..0b0b6dc 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -3845,6 +3845,16 @@ static int hpsa_update_device_info(struct ctlr_info *h,
if (h->fw_support & MISC_FW_RAID_OFFLOAD_BASIC)
hpsa_get_ioaccel_status(h, scsi3addr, this_device);
volume_offline = hpsa_volume_offline(h, scsi3addr);
+   if (volume_offline == HPSA_VPD_LV_STATUS_UNSUPPORTED &&
+   h->legacy_board) {
+   /*
+* Legacy boards might not support volume status
+*/
+   dev_info(>pdev->dev,
+"C0:T%d:L%d Volume status not available, 
assuming online.\n",
+this_device->target, this_device->lun);
+   volume_offline = 0;
+   }
this_device->volume_offline = volume_offline;
if (volume_offline == HPSA_LV_FAILED) {
rc = HPSA_LV_FAILED;
-- 
1.8.5.6

[PATCHv3 6/6] hpsa: Remove 'hpsa_allow_any' module option

[Hannes Reinecke]

As the cciss driver has been removed there are no overlapping
PCI IDs anymore, and the 'hpsa_allow_any' flag can be removed.

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/hpsa.c | 26 +++---
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index c633b35..2773dd7 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -83,10 +83,6 @@
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("cciss");
 
-static int hpsa_allow_any;
-module_param(hpsa_allow_any, int, S_IRUGO|S_IWUSR);
-MODULE_PARM_DESC(hpsa_allow_any,
-   "Allow hpsa driver to access unknown HP Smart Array hardware");
 static int hpsa_simple_mode;
 module_param(hpsa_simple_mode, int, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(hpsa_simple_mode,
@@ -7299,23 +7295,15 @@ static int hpsa_lookup_board_id(struct pci_dev *pdev, 
u32 *board_id,
if (products[i].access != _access &&
products[i].access != _access)
return i;
-   if (hpsa_allow_any) {
-   dev_warn(>dev,
-"legacy board ID: 0x%08x\n",
-*board_id);
-   if (legacy_board)
-   *legacy_board = true;
-   return i;
-   }
+   dev_warn(>dev,
+"legacy board ID: 0x%08x\n",
+*board_id);
+   if (legacy_board)
+   *legacy_board = true;
+   return i;
}
 
-   if ((subsystem_vendor_id != PCI_VENDOR_ID_HP &&
-   subsystem_vendor_id != PCI_VENDOR_ID_COMPAQ) ||
-   !hpsa_allow_any) {
-   dev_warn(>dev, "unrecognized board ID: "
-   "0x%08x, ignoring.\n", *board_id);
-   return -ENODEV;
-   }
+   dev_warn(>dev, "unrecognized board ID: 0x%08x\n", *board_id);
if (legacy_board)
*legacy_board = true;
return ARRAY_SIZE(products) - 1; /* generic unknown smart array */
-- 
1.8.5.6

[PATCHv3 0/6] hpsa legacy support

[Hannes Reinecke]

Hi all,

here's now the third attempt to add support for legacy boards, ie
for boards previously supported by cciss only.
With this patchset the hpsa driver should work with all Smart Array
boards if the 'hpsa_allow_any' module option is set, rendering the
cciss driver obsolete.
Consequently I've added a patch to remove the cciss driver and make
'hpsa' an alias to 'cciss' and removed the 'hpsa_allow_any' module
option.

Hannes Reinecke (6):
  hpsa: add support for legacy boards
  hpsa: disable volume status check for legacy boards
  hpsa: Ignore errors for unsupported LV_DEVICE_ID VPD page
  hpsa: do not print errors for unsupported report luns format
  cciss: Drop obsolete driver
  hpsa: Remove 'hpsa_allow_any' module option

 Documentation/blockdev/cciss.txt |  194 --
 MAINTAINERS  |   10 -
 drivers/block/Kconfig|   27 -
 drivers/block/Makefile   |1 -
 drivers/block/cciss.c| 5415 --
 drivers/block/cciss.h|  433 ---
 drivers/block/cciss_cmd.h|  269 --
 drivers/block/cciss_scsi.c   | 1653 
 drivers/block/cciss_scsi.h   |   79 -
 drivers/scsi/hpsa.c  |  108 +-
 drivers/scsi/hpsa.h  |   81 +-
 11 files changed, 142 insertions(+), 8128 deletions(-)
 delete mode 100644 Documentation/blockdev/cciss.txt
 delete mode 100644 drivers/block/cciss.c
 delete mode 100644 drivers/block/cciss.h
 delete mode 100644 drivers/block/cciss_cmd.h
 delete mode 100644 drivers/block/cciss_scsi.c
 delete mode 100644 drivers/block/cciss_scsi.h

-- 
1.8.5.6

[PATCHv3 3/6] hpsa: Ignore errors for unsupported LV_DEVICE_ID VPD page

[Hannes Reinecke]

Legacy boards might not support the LV_DEVICE_ID VPD page, so
we shouldn't print out an error message here.

Signed-off-by: Hannes Reinecke 
Reviewed-by: Christoph Hellwig 
Acked-by: Don Brace 
---
 drivers/scsi/hpsa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 0b0b6dc..b34ec42 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -3827,7 +3827,7 @@ static int hpsa_update_device_info(struct ctlr_info *h,
memset(this_device->device_id, 0,
sizeof(this_device->device_id));
if (hpsa_get_device_id(h, scsi3addr, this_device->device_id, 8,
-   sizeof(this_device->device_id)))
+   sizeof(this_device->device_id) < 0))
dev_err(>pdev->dev,
"hpsa%d: %s: can't get device id for host 
%d:C0:T%d:L%d\t%s\t%.16s\n",
h->ctlr, __func__,
-- 
1.8.5.6

[PATCHv3 4/6] hpsa: do not print errors for unsupported report luns format

[Hannes Reinecke]

Legacy boards might not support the 'extended' report luns format,
but as this is to be expected we don't need to print out an error here.

Signed-off-by: Hannes Reinecke 
Acked-by: Don Brace 
---
 drivers/scsi/hpsa.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index b34ec42..2da8f6f 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -3601,7 +3601,7 @@ static int hpsa_scsi_do_report_luns(struct ctlr_info *h, 
int logical,
memset(scsi3addr, 0, sizeof(scsi3addr));
if (fill_cmd(c, logical ? HPSA_REPORT_LOG : HPSA_REPORT_PHYS, h,
buf, bufsize, 0, scsi3addr, TYPE_CMD)) {
-   rc = -1;
+   rc = -EAGAIN;
goto out;
}
if (extended_response)
@@ -3614,16 +3614,19 @@ static int hpsa_scsi_do_report_luns(struct ctlr_info 
*h, int logical,
if (ei->CommandStatus != 0 &&
ei->CommandStatus != CMD_DATA_UNDERRUN) {
hpsa_scsi_interpret_error(h, c);
-   rc = -1;
+   rc = -EIO;
} else {
struct ReportLUNdata *rld = buf;
 
if (rld->extended_response_flag != extended_response) {
-   dev_err(>pdev->dev,
-   "report luns requested format %u, got %u\n",
-   extended_response,
-   rld->extended_response_flag);
-   rc = -1;
+   if (!h->legacy_board) {
+   dev_err(>pdev->dev,
+   "report luns requested format %u, got 
%u\n",
+   extended_response,
+   rld->extended_response_flag);
+   rc = -EINVAL;
+   } else
+   rc = -EOPNOTSUPP;
}
}
 out:
@@ -3639,7 +3642,7 @@ static inline int hpsa_scsi_do_report_phys_luns(struct 
ctlr_info *h,
 
rc = hpsa_scsi_do_report_luns(h, 0, buf, bufsize,
  HPSA_REPORT_PHYS_EXTENDED);
-   if (!rc || !hpsa_allow_any)
+   if (!rc || rc != -EOPNOTSUPP)
return rc;
 
/* REPORT PHYS EXTENDED is not supported */
@@ -6617,7 +6620,6 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct 
ctlr_info *h,
default:
dev_warn(>pdev->dev, "unknown command 0x%c\n", cmd);
BUG();
-   return -1;
}
} else if (cmd_type == TYPE_MSG) {
switch (cmd) {
-- 
1.8.5.6

Re: [PATCHv3 2/5] scsi: Export blacklist flags to sysfs

[Hannes Reinecke]

On 08/15/2017 05:18 AM, Martin K. Petersen wrote:
> 
> Hannes,
> 
>> +name = sdev_bflags_name(bflags);
>> +if (name)
>> +blen = snprintf(ptr, strlen(name) + 1,
>> +"%s", name);
>> +else
>> +blen = snprintf(ptr, 67, "0x%X", bflags);
> 
> It seems this else statement facilitates papering over the fact that
> scsi_sysfs.c and scsi_devinfo.h can get out of sync.
> 
But there is no good way of avoiding that, is there?
Out of necessity the definition of the bits and the decoding are two
distinct steps, and as such are always prone to differences.

So what is your suggestion here?

Having the 'blacklist' sysfs entry gives us a nice way of figuring out
if the selection algorithm in scsi_devinfo.c works as designed.
Without it it's quite hard to validate this...

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 4/4] ses: make page2 support optional

[Hannes Reinecke]

Simple subenclosures do not need to support SES page 2, so make
it optional.

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/ses.c | 39 ++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 46984cd..c52d110 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -51,6 +51,13 @@ struct ses_component {
u64 addr;
 };
 
+static bool ses_page2_supported(struct enclosure_device *edev)
+{
+   struct ses_device *ses_dev = edev->scratch;
+
+   return (ses_dev->page2 != NULL);
+}
+
 static int ses_probe(struct device *dev)
 {
struct scsi_device *sdev = to_scsi_device(dev);
@@ -204,6 +211,10 @@ static void ses_get_fault(struct enclosure_device *edev,
 {
unsigned char *desc;
 
+   if (!ses_page2_supported(edev)) {
+   ecomp->fault = 0;
+   return;
+   }
desc = ses_get_page2_descriptor(edev, ecomp);
if (desc)
ecomp->fault = (desc[3] & 0x60) >> 4;
@@ -216,6 +227,9 @@ static int ses_set_fault(struct enclosure_device *edev,
unsigned char desc[4];
unsigned char *desc_ptr;
 
+   if (!ses_page2_supported(edev))
+   return -EINVAL;
+
desc_ptr = ses_get_page2_descriptor(edev, ecomp);
 
if (!desc_ptr)
@@ -243,6 +257,10 @@ static void ses_get_status(struct enclosure_device *edev,
 {
unsigned char *desc;
 
+   if (!ses_page2_supported(edev)) {
+   ecomp->status = 0;
+   return;
+   }
desc = ses_get_page2_descriptor(edev, ecomp);
if (desc)
ecomp->status = (desc[0] & 0x0f);
@@ -253,6 +271,10 @@ static void ses_get_locate(struct enclosure_device *edev,
 {
unsigned char *desc;
 
+   if (!ses_page2_supported(edev)) {
+   ecomp->locate = 0;
+   return;
+   }
desc = ses_get_page2_descriptor(edev, ecomp);
if (desc)
ecomp->locate = (desc[2] & 0x02) ? 1 : 0;
@@ -265,6 +287,9 @@ static int ses_set_locate(struct enclosure_device *edev,
unsigned char desc[4];
unsigned char *desc_ptr;
 
+   if (!ses_page2_supported(edev))
+   return -EINVAL;
+
desc_ptr = ses_get_page2_descriptor(edev, ecomp);
 
if (!desc_ptr)
@@ -293,6 +318,9 @@ static int ses_set_active(struct enclosure_device *edev,
unsigned char desc[4];
unsigned char *desc_ptr;
 
+   if (!ses_page2_supported(edev))
+   return -EINVAL;
+
desc_ptr = ses_get_page2_descriptor(edev, ecomp);
 
if (!desc_ptr)
@@ -329,6 +357,11 @@ static void ses_get_power_status(struct enclosure_device 
*edev,
 {
unsigned char *desc;
 
+   if (!ses_page2_supported(edev)) {
+   ecomp->power_status = 0;
+   return;
+   }
+
desc = ses_get_page2_descriptor(edev, ecomp);
if (desc)
ecomp->power_status = (desc[3] & 0x10) ? 0 : 1;
@@ -341,6 +374,9 @@ static int ses_set_power_status(struct enclosure_device 
*edev,
unsigned char desc[4];
unsigned char *desc_ptr;
 
+   if (!ses_page2_supported(edev))
+   return -EINVAL;
+
desc_ptr = ses_get_page2_descriptor(edev, ecomp);
 
if (!desc_ptr)
@@ -674,7 +710,7 @@ static int ses_intf_add(struct device *cdev,
page = 2;
result = ses_recv_diag(sdev, page, hdr_buf, INIT_ALLOC_SIZE);
if (result)
-   goto recv_failed;
+   goto page2_not_supported;
 
len = (hdr_buf[2] << 8) + hdr_buf[3] + 4;
buf = kzalloc(len, GFP_KERNEL);
@@ -707,6 +743,7 @@ static int ses_intf_add(struct device *cdev,
ses_dev->page10_len = len;
buf = NULL;
}
+page2_not_supported:
scomp = kzalloc(sizeof(struct ses_component) * components, GFP_KERNEL);
if (!scomp)
goto err_free;
-- 
1.8.5.6

[PATCH 2/4] ses: check return code from ses_recv_diag()

[Hannes Reinecke]

We should be checking the return code from ses_recv_diag() to
avoid accessing invalid data.

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/ses.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index a37aec8..fc70c00 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -179,7 +179,8 @@ static unsigned char *ses_get_page2_descriptor(struct 
enclosure_device *edev,
unsigned char *type_ptr = ses_dev->page1_types;
unsigned char *desc_ptr = ses_dev->page2 + 8;
 
-   ses_recv_diag(sdev, 2, ses_dev->page2, ses_dev->page2_len);
+   if (ses_recv_diag(sdev, 2, ses_dev->page2, ses_dev->page2_len) < 0)
+   return NULL;
 
for (i = 0; i < ses_dev->page1_num_types; i++, type_ptr += 4) {
for (j = 0; j < type_ptr[1]; j++) {
-- 
1.8.5.6

[PATCH 1/4] scsi: Fixup ses page check

[Hannes Reinecke]

The error code from a scsi_execute_req() is a SCSI status, not
a normal errno. So whenever it returns a value here an error
occurred and there's no point in looking at the page number.

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/ses.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index f1cdf32..a37aec8 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -99,8 +99,8 @@ static int ses_recv_diag(struct scsi_device *sdev, int 
page_code,
 
ret =  scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf, bufflen,
NULL, SES_TIMEOUT, SES_RETRIES, NULL);
-   if (unlikely(!ret))
-   return ret;
+   if (unlikely(ret))
+   return -EIO;
 
recv_page_code = ((unsigned char *)buf)[0];
 
-- 
1.8.5.6

[PATCH 3/4] ses: Fixup error message 'failed to get diagnostic page 0xffffffea'

[Hannes Reinecke]

The printk was using the result as argument, leading to a slightly
confusing log message.

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/ses.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index fc70c00..46984cd 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -602,7 +602,7 @@ static int ses_intf_add(struct device *cdev,
 {
struct scsi_device *sdev = to_scsi_device(cdev->parent);
struct scsi_device *tmp_sdev;
-   unsigned char *buf = NULL, *hdr_buf, *type_ptr;
+   unsigned char *buf = NULL, *hdr_buf, *type_ptr, page;
struct ses_device *ses_dev;
u32 result;
int i, types, len, components = 0;
@@ -631,7 +631,8 @@ static int ses_intf_add(struct device *cdev,
if (!hdr_buf || !ses_dev)
goto err_init_free;
 
-   result = ses_recv_diag(sdev, 1, hdr_buf, INIT_ALLOC_SIZE);
+   page = 1;
+   result = ses_recv_diag(sdev, page, hdr_buf, INIT_ALLOC_SIZE);
if (result)
goto recv_failed;
 
@@ -640,7 +641,7 @@ static int ses_intf_add(struct device *cdev,
if (!buf)
goto err_free;
 
-   result = ses_recv_diag(sdev, 1, buf, len);
+   result = ses_recv_diag(sdev, page, buf, len);
if (result)
goto recv_failed;
 
@@ -670,7 +671,8 @@ static int ses_intf_add(struct device *cdev,
ses_dev->page1_len = len;
buf = NULL;
 
-   result = ses_recv_diag(sdev, 2, hdr_buf, INIT_ALLOC_SIZE);
+   page = 2;
+   result = ses_recv_diag(sdev, page, hdr_buf, INIT_ALLOC_SIZE);
if (result)
goto recv_failed;
 
@@ -689,7 +691,8 @@ static int ses_intf_add(struct device *cdev,
 
/* The additional information page --- allows us
 * to match up the devices */
-   result = ses_recv_diag(sdev, 10, hdr_buf, INIT_ALLOC_SIZE);
+   page = 10;
+   result = ses_recv_diag(sdev, page, hdr_buf, INIT_ALLOC_SIZE);
if (!result) {
 
len = (hdr_buf[2] << 8) + hdr_buf[3] + 4;
@@ -697,7 +700,7 @@ static int ses_intf_add(struct device *cdev,
if (!buf)
goto err_free;
 
-   result = ses_recv_diag(sdev, 10, buf, len);
+   result = ses_recv_diag(sdev, page, buf, len);
if (result)
goto recv_failed;
ses_dev->page10 = buf;
@@ -735,7 +738,7 @@ static int ses_intf_add(struct device *cdev,
 
  recv_failed:
sdev_printk(KERN_ERR, sdev, "Failed to get diagnostic page 0x%x\n",
-   result);
+   page);
err = -ENODEV;
  err_free:
kfree(buf);
-- 
1.8.5.6

[PATCH 0/4] ses: simple subenclosure support

[Hannes Reinecke]

Hi all,

some arrays (most notably 3Par) only support simple subenclosures.
Sadly our ses implementation doesn't handle this properly, so we're
greeted with error messages like:

scsi 1:0:0:254: Wrong diagnostic page; asked for 2 got 0
scsi 1:0:0:254: Failed to get diagnostic page 0xffea
scsi 1:0:0:254: Failed to bind enclosure -19
ses 1:0:0:254: Attached Enclosure device

This patchset fixes up our ses implementation to work properly
with simple subenclosures.

As usual, comments and reviews are welcome.

Hannes Reinecke (4):
  scsi: Fixup ses page check
  ses: check return code from ses_recv_diag()
  ses: Fixup error message 'failed to get diagnostic page 0xffea'
  ses: make page2 support optional

 drivers/scsi/ses.c | 63 --
 1 file changed, 52 insertions(+), 11 deletions(-)

-- 
1.8.5.6

[PATCH] megaraid_sas: Fallback to older scanning if no disks are found

[Hannes Reinecke]

commit 21c9e160a51383d4cb0b882398534b0c95c0cc3b implemented a
new driver lookup using the MR_DCMD_LD_LIST_QUERY firmware command.
However, this command might not work properly on older firmware,
causing the command to return no drives instead of an error.
This causes a regression on older firmware as the driver will
no longer detect any drives.
This patch checks if MR_DCMD_LD_LIST_QUERY return no drives,
and falls back to the original method if so.

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c 
b/drivers/scsi/megaraid/megaraid_sas_base.c
index 39b08fc..a1cf2c3 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -4502,7 +4502,6 @@ int megasas_alloc_cmds(struct megasas_instance *instance)
dev_info(>pdev->dev,
"DCMD not supported by firmware - %s %d\n",
__func__, __LINE__);
-   ret = megasas_get_ld_list(instance);
break;
case DCMD_TIMEOUT:
switch (dcmd_timeout_ocr_possible(instance)) {
@@ -4530,6 +4529,14 @@ int megasas_alloc_cmds(struct megasas_instance *instance)
break;
case DCMD_SUCCESS:
tgtid_count = le32_to_cpu(ci->count);
+   /*
+* Some older firmware return '0' if the LD LIST QUERY
+* command is not supported.
+*/
+   if (tgtid_count == 0) {
+   ret = DCMD_FAILED;
+   break;
+   }
 
if ((tgtid_count > (instance->fw_supported_vd_count)))
break;
@@ -5146,7 +5153,7 @@ static int megasas_init_fw(struct megasas_instance 
*instance)
struct megasas_register_set __iomem *reg_set;
struct megasas_ctrl_info *ctrl_info = NULL;
unsigned long bar_list;
-   int i, j, loop, fw_msix_count = 0;
+   int i, j, loop, fw_msix_count = 0, ret;
struct IOV_111 *iovPtr;
struct fusion_context *fusion;
 
@@ -5384,8 +5391,11 @@ static int megasas_init_fw(struct megasas_instance 
*instance)
}
}
 
-   if (megasas_ld_list_query(instance,
- MR_LD_QUERY_TYPE_EXPOSED_TO_HOST))
+   ret = megasas_ld_list_query(instance,
+   MR_LD_QUERY_TYPE_EXPOSED_TO_HOST);
+   if (ret == DCMD_FAILED)
+   ret = megasas_get_ld_list(instance);
+   if (ret)
goto fail_get_ld_pd_list;
 
/*
@@ -7426,8 +7436,12 @@ static inline void megasas_remove_scsi_device(struct 
scsi_device *sdev)
case MR_EVT_LD_DELETED:
case MR_EVT_LD_CREATED:
if (!instance->requestorId ||
-   (instance->requestorId && 
megasas_get_ld_vf_affiliation(instance, 0)))
+   (instance->requestorId &&
+megasas_get_ld_vf_affiliation(instance, 0))) {
dcmd_ret = megasas_ld_list_query(instance, 
MR_LD_QUERY_TYPE_EXPOSED_TO_HOST);
+   if (dcmd_ret == DCMD_FAILED)
+   dcmd_ret = 
megasas_get_ld_list(instance);
+   }
 
if (dcmd_ret == DCMD_SUCCESS)
doscan = SCAN_VD_CHANNEL;
@@ -7443,8 +7457,11 @@ static inline void megasas_remove_scsi_device(struct 
scsi_device *sdev)
break;
 
if (!instance->requestorId ||
-   (instance->requestorId && 
megasas_get_ld_vf_affiliation(instance, 0)))
+   (instance->requestorId && 
megasas_get_ld_vf_affiliation(instance, 0))) {
dcmd_ret = megasas_ld_list_query(instance, 
MR_LD_QUERY_TYPE_EXPOSED_TO_HOST);
+   if (dcmd_ret == DCMD_FAILED)
+   dcmd_ret = 
megasas_get_ld_list(instance);
+   }
 
if (dcmd_ret != DCMD_SUCCESS)
break;
-- 
1.8.5.6

[PATCH] megaraid_sas: boot hangs while LD is offline

[Hannes Reinecke]

Offline Logical drives (LDs) should not allowed to be visible
to the OS, as the OS will hang trying to send commands to it.
This patch skips offline LDs like it already does for
non-system physical drives (PDs).

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c 
b/drivers/scsi/megaraid/megaraid_sas_base.c
index 5202c2f..39b08fc 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -1897,14 +1897,19 @@ static void megasas_set_static_target_properties(struct 
scsi_device *sdev,
 
 static int megasas_slave_configure(struct scsi_device *sdev)
 {
-   u16 pd_index = 0;
+   u16 pd_index = 0, ld_index;
struct megasas_instance *instance;
int ret_target_prop = DCMD_FAILED;
bool is_target_prop = false;
 
instance = megasas_lookup_instance(sdev->host->host_no);
if (instance->pd_list_not_supported) {
-   if (!MEGASAS_IS_LOGICAL(sdev) && sdev->type == TYPE_DISK) {
+   if (MEGASAS_IS_LOGICAL(sdev)) {
+   ld_index = ((sdev->channel - MEGASAS_MAX_PD_CHANNELS) *
+   MEGASAS_MAX_DEV_PER_CHANNEL) + sdev->id;
+   if (instance->ld_ids[ld_index] == 0xff)
+   return -ENXIO;
+   } else if (sdev->type == TYPE_DISK) {
pd_index = (sdev->channel * 
MEGASAS_MAX_DEV_PER_CHANNEL) +
sdev->id;
if (instance->pd_list[pd_index].driveState !=
-- 
1.8.5.6

[PATCH] scsi: mpt3sas: fix pr_info message continuation

[Colin King]

From: Colin Ian King 

An optional discovery status should be printed with a pr_cont
and needs a leading space to make it more readable. The final
new line should also be a pr_cont and the indentation is out
by one, so fix that too.

Signed-off-by: Colin Ian King 
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 8a44636ab0b5..8705bca3 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -615,9 +615,9 @@ _base_display_event_data(struct MPT3SAS_ADAPTER *ioc,
(event_data->ReasonCode == MPI2_EVENT_SAS_DISC_RC_STARTED) ?
"start" : "stop");
if (event_data->DiscoveryStatus)
-   pr_info("discovery_status(0x%08x)",
+   pr_cont(" discovery_status(0x%08x)",
le32_to_cpu(event_data->DiscoveryStatus));
-   pr_info("\n");
+   pr_cont("\n");
return;
}
case MPI2_EVENT_SAS_BROADCAST_PRIMITIVE:
-- 
2.11.0

[PATCH] scsi: add missing indent on a for loop statement

[Colin King]

From: Colin Ian King 

The for loop is statement is missing an indent, add it.

Signed-off-by: Colin Ian King 
---
 drivers/scsi/osst.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
index 97ab5f160bc6..241908aca468 100644
--- a/drivers/scsi/osst.c
+++ b/drivers/scsi/osst.c
@@ -5434,7 +5434,7 @@ static int append_to_buffer(const char __user *ubp, 
struct osst_buffer *st_bp, i
 
for (i=0, offset=st_bp->buffer_bytes;
 i < st_bp->sg_segs && offset >= st_bp->sg[i].length; i++)
-   offset -= st_bp->sg[i].length;
+   offset -= st_bp->sg[i].length;
if (i == st_bp->sg_segs) {  /* Should never happen */
printk(KERN_WARNING "osst :A: Append_to_buffer offset 
overflow.\n");
return (-EIO);
-- 
2.11.0