RE: [PATCH 3/3] hpsa: add 'ctlr_num' sysfs attribute

2016-11-28 Thread Don Brace 
> -Original Message-
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Friday, November 18, 2016 1:33 AM
> To: Martin K. Petersen
> Cc: Christoph Hellwig; James Bottomley; Don Brace; Martin Wilck; linux-
> s...@vger.kernel.org; Hannes Reinecke; Hannes Reinecke
> Subject: [PATCH 3/3] hpsa: add 'ctlr_num' sysfs attribute
> 
> EXTERNAL EMAIL
> 
> 
> Add a sysfs attribute 'ctlr_num' holding the current HPSA controller
> number. This is required to construct compability 'cciss' links.
> 
> Signed-off-by: Hannes Reinecke 
> ---
>  drivers/scsi/hpsa.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index 3d89f74..5834117 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -867,6 +867,16 @@ static ssize_t path_info_show(struct device *dev,
> return output_len;
>  }
> 
> +static ssize_t host_show_ctlr_num(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->ctlr);
> +}
> +
>  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);
> @@ -890,6 +900,8 @@ static DEVICE_ATTR(resettable, S_IRUGO,
> host_show_resettable, NULL);
>  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 struct device_attribute *hpsa_sdev_attrs[] = {
> _attr_raid_level,
> @@ -910,6 +922,7 @@ static DEVICE_ATTR(lockup_detected, S_IRUGO,
> _attr_hp_ssd_smart_path_status,
> _attr_raid_offload_debug,
> _attr_lockup_detected,
> +   _attr_ctlr_num,
> NULL,
>  };
> 
> --
> 1.8.5.6

Acked-by: Don Brace 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/3] hpsa: use correct DID_NO_CONNECT hostbyte

2016-11-28 Thread Don Brace 
> -Original Message-
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Friday, November 18, 2016 1:33 AM
> To: Martin K. Petersen
> Cc: Christoph Hellwig; James Bottomley; Don Brace; Martin Wilck; linux-
> s...@vger.kernel.org; Hannes Reinecke; Hannes Reinecke
> Subject: [PATCH 1/3] hpsa: use correct DID_NO_CONNECT hostbyte
> 
> EXTERNAL EMAIL
> 
> 
> NOT_READY is a sense key, not a legit scsi hostbyte value.
> Use DID_NO_CONNECT instead.
> 
> Signed-off-by: Hannes Reinecke 
> ---
>  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 ea64c01..488e17c 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -5493,7 +5493,7 @@ static int hpsa_scsi_queue_command(struct
> Scsi_Host *sh, struct scsi_cmnd *cmd)
> 
> dev = cmd->device->hostdata;
> if (!dev) {
> -   cmd->result = NOT_READY << 16; /* host byte */
> +   cmd->result = DID_NO_CONNECT << 16;
> cmd->scsi_done(cmd);
> return 0;
> }
> --
> 1.8.5.6

Acked-by: Don Brace 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] hpsa: cleanup sas_phy structures in sysfs when unloading

2016-11-28 Thread Don Brace 
> -Original Message-
> From: Martin Wilck [mailto:mwi...@suse.de]
> Sent: Monday, November 21, 2016 8:04 AM
> To: Don Brace
> Cc: dl-esc-Team ESD Storage Dev Support; iss_storage...@hp.com; linux-
> s...@vger.kernel.org; jbottom...@odin.com; h...@lst.de; h...@suse.de;
> Martin Wilck
> Subject: [PATCH 1/2] hpsa: cleanup sas_phy structures in sysfs when
> unloading
> 
> EXTERNAL EMAIL
> 
> 
> When the hpsa module is unloaded using rmmod, dangling
> symlinks remain under /sys/class/sas_phy. Fix this by
> calling sas_phy_delete() rather than sas_phy_free (which,
> according to comments, should not be called for PHYs that
> have been set up successfully, anyway).
> 
> References: bsc#1010946.
> Signed-off-by: Martin Wilck 
> ---
>  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 efe2f36..8ec77c3 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -9547,9 +9547,9 @@ static void hpsa_free_sas_phy(struct hpsa_sas_phy
> *hpsa_sas_phy)
> struct sas_phy *phy = hpsa_sas_phy->phy;
> 
> sas_port_delete_phy(hpsa_sas_phy->parent_port->port, phy);
> -   sas_phy_free(phy);
> if (hpsa_sas_phy->added_to_port)
> list_del(_sas_phy->phy_list_entry);
> +   sas_phy_delete(phy);
> kfree(hpsa_sas_phy);
>  }
> 
> --
> 2.10.1

I tried these patches on: 4.9.0-rc7, was this correct?

I got the following stack trace:
[  231.192289] [ cut here ]
[  231.214333] WARNING: CPU: 51 PID: 15876 at fs/sysfs/group.c:237 
sysfs_remove_group+0x8e/0x90
[  231.254371] sysfs group 'power' not found for kobject '4:0:0:0'
[  231.282637] Modules linked in: ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 
nf_conntrack_ipv6 nf_defrag_ipv6 ipt_REJECT nf_reject_ipv4 nf_conntrack_ipv4 
nf_defrag_ipv4 xt_conntrack nf_conntrack cfg80211 rfkill ebtable_nat 
ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_mangle 
ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_mangle 
iptable_security iptable_raw iptable_filter ip_tables sb_edac edac_core 
x86_pkg_temp_thermal coretemp crct10dif_pclmul crc32_pclmul iTCO_wdt 
ghash_clmulni_intel iTCO_vendor_support aesni_intel lrw gf128mul glue_helper 
ablk_helper cryptd osst pcspkr ch st ioatdma lpc_ich hpilo hpwdt mfd_core dca 
ipmi_si ipmi_msghandler pcc_cpufreq wmi acpi_cpufreq acpi_power_meter uinput 
mgag200 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt 
fb_sys_fops
[  231.620046]  ttm drm crc32c_intel tg3 serio_raw ptp usb_storage i2c_core 
hpsa(-) pps_core scsi_transport_sas dm_mirror dm_region_hash dm_log dm_mod
[  231.676364] CPU: 51 PID: 15876 Comm: rmmod Not tainted 4.9.0-rc7+ #22
[  231.706769] Hardware name: HP ProLiant DL580 Gen8, BIOS P79 08/18/2016
[  231.737730]  c9002123bbf0 815909bd c9002123bc40 

[  231.772474]  c9002123bc30 81090901 00ed0246 

[  231.807897]  81f71560 8820529faea8 88205542ea60 
024b0090
[  231.842671] Call Trace:
[  231.854426]  [] dump_stack+0x85/0xc8
[  231.878826]  [] __warn+0xd1/0xf0
[  231.901476]  [] warn_slowpath_fmt+0x5f/0x80
[  231.929515]  [] ? mutex_unlock+0xe/0x10
[  231.955067]  [] ? kernfs_find_and_get_ns+0x4a/0x60
[  231.985095]  [] sysfs_remove_group+0x8e/0x90
[  232.012416]  [] dpm_sysfs_remove+0x57/0x60
[  232.038904]  [] device_del+0x58/0x270
[  232.064056]  [] device_unregister+0x1a/0x60
[  232.091138]  [] bsg_unregister_queue+0x60/0xa0
[  232.119498]  [] __scsi_remove_device+0xaa/0xd0
[  232.147745]  [] scsi_forget_host+0x69/0x70
[  232.174666]  [] scsi_remove_host+0x82/0x130
[  232.201804]  [] hpsa_remove_one+0x93/0x190 [hpsa]
[  232.231329]  [] pci_device_remove+0x39/0xc0
[  232.258089]  [] __device_release_driver+0x9a/0x150
[  232.288005]  [] driver_detach+0xc1/0xd0
[  232.313479]  [] bus_remove_driver+0x58/0xd0
[  232.341280]  [] driver_unregister+0x2c/0x50
[  232.369272]  [] pci_unregister_driver+0x2a/0x80
[  232.398723]  [] hpsa_cleanup+0x10/0x7a7 [hpsa]
[  232.428094]  [] SyS_delete_module+0x1bc/0x220
[  232.456716]  [] do_syscall_64+0x6c/0x200
[  232.483125]  [] entry_SYSCALL64_slow_path+0x25/0x25
[  232.514162] ---[ end trace 3c490662736284eb ]---


Thanks,
Don Brace
ESC - Smart Storage
Microsemi Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 13/14] cxlflash: Migrate IOARRIN specific routines to function pointers

2016-11-28 Thread Uma Krishnan 
From: "Matthew R. Ochs" 

As staging for supporting hardware with a different queuing mechanism,
move the send_cmd() and context_reset() routines to function pointers
that are configured when the AFU is initialized. In addition, rename
the existing routines to better reflect the queue model they support.

Signed-off-by: Matthew R. Ochs 
---
 drivers/scsi/cxlflash/common.h |  3 +++
 drivers/scsi/cxlflash/main.c   | 21 +++--
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index bed8e60..6b8d1d3 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -161,6 +161,9 @@ struct afu {
 * fields after this point
 */
 
+   int (*send_cmd)(struct afu *, struct afu_cmd *);
+   void (*context_reset)(struct afu_cmd *);
+
/* AFU HW */
struct cxl_ioctl_start_work work;
struct cxlflash_afu_map __iomem *afu_map;   /* entire MMIO map */
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 4e70c9a..d2d2d83 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -188,12 +188,10 @@ static void cmd_complete(struct afu_cmd *cmd)
 }
 
 /**
- * context_reset() - timeout handler for AFU commands
+ * context_reset_ioarrin() - reset command owner context via IOARRIN register
  * @cmd:   AFU command that timed out.
- *
- * Sends a reset to the AFU.
  */
-static void context_reset(struct afu_cmd *cmd)
+static void context_reset_ioarrin(struct afu_cmd *cmd)
 {
int nretry = 0;
u64 rrin = 0x1;
@@ -217,14 +215,14 @@ static void context_reset(struct afu_cmd *cmd)
 }
 
 /**
- * send_cmd() - sends an AFU command
+ * send_cmd_ioarrin() - sends an AFU command via IOARRIN register
  * @afu:   AFU associated with the host.
  * @cmd:   AFU command to send.
  *
  * Return:
  * 0 on success, SCSI_MLQUEUE_HOST_BUSY on failure
  */
-static int send_cmd(struct afu *afu, struct afu_cmd *cmd)
+static int send_cmd_ioarrin(struct afu *afu, struct afu_cmd *cmd)
 {
struct cxlflash_cfg *cfg = afu->parent;
struct device *dev = >dev->dev;
@@ -273,7 +271,7 @@ static int wait_resp(struct afu *afu, struct afu_cmd *cmd)
 
timeout = wait_for_completion_timeout(>cevent, timeout);
if (!timeout) {
-   context_reset(cmd);
+   afu->context_reset(cmd);
rc = -1;
}
 
@@ -330,7 +328,7 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, 
u64 tmfcmd)
  SISL_REQ_FLAGS_TMF_CMD);
memcpy(cmd->rcb.cdb, , sizeof(tmfcmd));
 
-   rc = send_cmd(afu, cmd);
+   rc = afu->send_cmd(afu, cmd);
if (unlikely(rc)) {
spin_lock_irqsave(>tmf_slock, lock_flags);
cfg->tmf_active = false;
@@ -461,7 +459,7 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *scp)
cmd->rcb.req_flags = req_flags;
memcpy(cmd->rcb.cdb, scp->cmnd, sizeof(cmd->rcb.cdb));
 
-   rc = send_cmd(afu, cmd);
+   rc = afu->send_cmd(afu, cmd);
if (unlikely(rc))
scsi_dma_unmap(scp);
 out:
@@ -1631,6 +1629,9 @@ static int init_afu(struct cxlflash_cfg *cfg)
goto err2;
}
 
+   afu->send_cmd = send_cmd_ioarrin;
+   afu->context_reset = context_reset_ioarrin;
+
pr_debug("%s: afu version %s, interface version 0x%llX\n", __func__,
 afu->version, afu->interface_version);
 
@@ -1723,7 +1724,7 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t 
ctx_hndl_u,
*((__be16 *)>rcb.cdb[2]) = cpu_to_be16(ctx_hndl_u);
*((__be32 *)>rcb.cdb[4]) = cpu_to_be32(res_hndl_u);
 
-   rc = send_cmd(afu, cmd);
+   rc = afu->send_cmd(afu, cmd);
if (unlikely(rc))
goto out;
 
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 12/14] cxlflash: Cleanup queuecommand()

2016-11-28 Thread Uma Krishnan 
From: "Matthew R. Ochs" 

The queuecommand routine is disorganized where it populates the
private command and also contains some logic/statements that are
not needed given that cxlflash devices do not (and likely never
will) support scatter-gather.

Restructure the code to remove the unnecessary logic and create an
organized flow:

handle state -> DMA map -> populate command -> send command

Signed-off-by: Matthew R. Ochs 
---
 drivers/scsi/cxlflash/main.c | 50 ++--
 1 file changed, 20 insertions(+), 30 deletions(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index b763699..4e70c9a 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -388,11 +388,11 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *scp)
struct afu *afu = cfg->afu;
struct device *dev = >dev->dev;
struct afu_cmd *cmd = sc_to_afucz(scp);
+   struct scatterlist *sg = scsi_sglist(scp);
u32 port_sel = scp->device->channel + 1;
-   int nseg, i, ncount;
-   struct scatterlist *sg;
+   u16 req_flags = SISL_REQ_FLAGS_SUP_UNDERRUN;
ulong lock_flags;
-   short lflag = 0;
+   int nseg = 0;
int rc = 0;
int kref_got = 0;
 
@@ -435,45 +435,35 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *scp)
kref_get(>afu->mapcount);
kref_got = 1;
 
-   cmd->rcb.ctx_id = afu->ctx_hndl;
-   cmd->rcb.msi = SISL_MSI_RRQ_UPDATED;
-   cmd->rcb.port_sel = port_sel;
-   cmd->rcb.lun_id = lun_to_lunid(scp->device->lun);
-
-   if (scp->sc_data_direction == DMA_TO_DEVICE)
-   lflag = SISL_REQ_FLAGS_HOST_WRITE;
-   else
-   lflag = SISL_REQ_FLAGS_HOST_READ;
+   if (likely(sg)) {
+   nseg = scsi_dma_map(scp);
+   if (unlikely(nseg < 0)) {
+   dev_err(dev, "%s: Fail DMA map!\n", __func__);
+   rc = SCSI_MLQUEUE_HOST_BUSY;
+   goto out;
+   }
 
-   cmd->rcb.req_flags = (SISL_REQ_FLAGS_PORT_LUN_ID |
- SISL_REQ_FLAGS_SUP_UNDERRUN | lflag);
+   cmd->rcb.data_len = sg_dma_len(sg);
+   cmd->rcb.data_ea = sg_dma_address(sg);
+   }
 
-   /* Stash the scp in the reserved field, for reuse during interrupt */
cmd->rcb.scp = scp;
cmd->parent = afu;
 
-   nseg = scsi_dma_map(scp);
-   if (unlikely(nseg < 0)) {
-   dev_err(dev, "%s: Fail DMA map! nseg=%d\n",
-   __func__, nseg);
-   rc = SCSI_MLQUEUE_HOST_BUSY;
-   goto out;
-   }
+   cmd->rcb.ctx_id = afu->ctx_hndl;
+   cmd->rcb.msi = SISL_MSI_RRQ_UPDATED;
+   cmd->rcb.port_sel = port_sel;
+   cmd->rcb.lun_id = lun_to_lunid(scp->device->lun);
 
-   ncount = scsi_sg_count(scp);
-   scsi_for_each_sg(scp, sg, ncount, i) {
-   cmd->rcb.data_len = sg_dma_len(sg);
-   cmd->rcb.data_ea = sg_dma_address(sg);
-   }
+   if (scp->sc_data_direction == DMA_TO_DEVICE)
+   req_flags |= SISL_REQ_FLAGS_HOST_WRITE;
 
-   /* Copy the CDB from the scsi_cmnd passed in */
+   cmd->rcb.req_flags = req_flags;
memcpy(cmd->rcb.cdb, scp->cmnd, sizeof(cmd->rcb.cdb));
 
-   /* Send the command */
rc = send_cmd(afu, cmd);
if (unlikely(rc))
scsi_dma_unmap(scp);
-
 out:
if (kref_got)
kref_put(>mapcount, afu_unmap);
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 10/14] cxlflash: Remove AFU command lock

2016-11-28 Thread Uma Krishnan 
From: "Matthew R. Ochs" 

The original design of the cxlflash driver required AFU commands
to convey state information across multiple threads. The IOASA
"host use" byte was used to track if a command was done, errored,
or timed out. A per-command spin lock was used to serialize access
to this byte. As this is no longer required with the introduction
of completions and various refactoring over time, the spin lock,
state tracking, and associated code can be removed. To support the
simplification, the wait_resp() routine is refactored to return a
success or failure. Additionally, as the simplification to the
AFU internal command routine, explicit assignments of AFU command
fields to zero are removed as the memory is zeroed upon allocation.

Signed-off-by: Matthew R. Ochs 
---
 drivers/scsi/cxlflash/common.h |  8 +---
 drivers/scsi/cxlflash/main.c   | 46 ++
 2 files changed, 16 insertions(+), 38 deletions(-)

diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 6211677..bed8e60 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -63,11 +63,6 @@ static inline void check_sizes(void)
 /* AFU defines a fixed size of 4K for command buffers (borrow 4K page define) 
*/
 #define CMD_BUFSIZE SIZE_4K
 
-/* flags in IOA status area for host use */
-#define B_DONE   0x01
-#define B_ERROR  0x02  /* set with B_DONE */
-#define B_TIMEOUT0x04  /* set with B_DONE & B_ERROR */
-
 enum cxlflash_lr_state {
LINK_RESET_INVALID,
LINK_RESET_REQUIRED,
@@ -133,9 +128,8 @@ struct cxlflash_cfg {
 struct afu_cmd {
struct sisl_ioarcb rcb; /* IOARCB (cache line aligned) */
struct sisl_ioasa sa;   /* IOASA must follow IOARCB */
-   spinlock_t slock;
-   struct completion cevent;
struct afu *parent;
+   struct completion cevent;
 
u8 cmd_tmf:1;
 
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 839eca4..db77030 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -161,10 +161,6 @@ static void cmd_complete(struct afu_cmd *cmd)
struct cxlflash_cfg *cfg = afu->parent;
bool cmd_is_tmf;
 
-   spin_lock_irqsave(>slock, lock_flags);
-   cmd->sa.host_use_b[0] |= B_DONE;
-   spin_unlock_irqrestore(>slock, lock_flags);
-
if (cmd->rcb.scp) {
scp = cmd->rcb.scp;
if (unlikely(cmd->sa.ioasc))
@@ -204,21 +200,9 @@ static void context_reset(struct afu_cmd *cmd)
struct afu *afu = cmd->parent;
struct cxlflash_cfg *cfg = afu->parent;
struct device *dev = >dev->dev;
-   ulong lock_flags;
 
pr_debug("%s: cmd=%p\n", __func__, cmd);
 
-   spin_lock_irqsave(>slock, lock_flags);
-
-   /* Already completed? */
-   if (cmd->sa.host_use_b[0] & B_DONE) {
-   spin_unlock_irqrestore(>slock, lock_flags);
-   return;
-   }
-
-   cmd->sa.host_use_b[0] |= (B_DONE | B_ERROR | B_TIMEOUT);
-   spin_unlock_irqrestore(>slock, lock_flags);
-
writeq_be(rrin, >host_map->ioarrin);
do {
rrin = readq_be(>host_map->ioarrin);
@@ -278,20 +262,30 @@ static int send_cmd(struct afu *afu, struct afu_cmd *cmd)
  * wait_resp() - polls for a response or timeout to a sent AFU command
  * @afu:   AFU associated with the host.
  * @cmd:   AFU command that was sent.
+ *
+ * Return:
+ * 0 on success, -1 on timeout/error
  */
-static void wait_resp(struct afu *afu, struct afu_cmd *cmd)
+static int wait_resp(struct afu *afu, struct afu_cmd *cmd)
 {
+   int rc = 0;
ulong timeout = msecs_to_jiffies(cmd->rcb.timeout * 2 * 1000);
 
timeout = wait_for_completion_timeout(>cevent, timeout);
-   if (!timeout)
+   if (!timeout) {
context_reset(cmd);
+   rc = -1;
+   }
 
-   if (unlikely(cmd->sa.ioasc != 0))
+   if (unlikely(cmd->sa.ioasc != 0)) {
pr_err("%s: CMD 0x%X failed, IOASC: flags 0x%X, afu_rc 0x%X, "
   "scsi_rc 0x%X, fc_rc 0x%X\n", __func__, cmd->rcb.cdb[0],
   cmd->sa.rc.flags, cmd->sa.rc.afu_rc, cmd->sa.rc.scsi_rc,
   cmd->sa.rc.fc_rc);
+   rc = -1;
+   }
+
+   return rc;
 }
 
 /**
@@ -339,7 +333,6 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, 
u64 tmfcmd)
/* Stash the scp in the command, for reuse during interrupt */
cmd->rcb.scp = scp;
cmd->parent = afu;
-   spin_lock_init(>slock);
 
/* Copy the CDB from the cmd passed in */
memcpy(cmd->rcb.cdb, , sizeof(tmfcmd));
@@ -466,7 +459,6 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *scp)
/* Stash the scp in the reserved field, for reuse during interrupt */
cmd->rcb.scp = scp;
cmd->parent =

[PATCH v2 14/14] cxlflash: Migrate scsi command pointer to AFU command

2016-11-28 Thread Uma Krishnan 
From: "Matthew R. Ochs" 

Currently, when sending a SCSI command, the pointer is stored in a
reserved field of the AFU command descriptor for retrieval once the
SCSI command has completed. In order to support new descriptor formats
that make use of the reserved field, the pointer is migrated to outside
the descriptor where it can still be found during completion processing.

Signed-off-by: Matthew R. Ochs 
---
 drivers/scsi/cxlflash/common.h  |  1 +
 drivers/scsi/cxlflash/main.c| 10 +-
 drivers/scsi/cxlflash/sislite.h |  2 +-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 6b8d1d3..0e9de5d 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -129,6 +129,7 @@ struct afu_cmd {
struct sisl_ioarcb rcb; /* IOARCB (cache line aligned) */
struct sisl_ioasa sa;   /* IOASA must follow IOARCB */
struct afu *parent;
+   struct scsi_cmnd *scp;
struct completion cevent;
 
u8 cmd_tmf:1;
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index d2d2d83..b17ebf6 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -151,7 +151,7 @@ static void process_cmd_err(struct afu_cmd *cmd, struct 
scsi_cmnd *scp)
  *
  * Prepares and submits command that has either completed or timed out to
  * the SCSI stack. Checks AFU command back into command pool for non-internal
- * (rcb.scp populated) commands.
+ * (cmd->scp populated) commands.
  */
 static void cmd_complete(struct afu_cmd *cmd)
 {
@@ -161,8 +161,8 @@ static void cmd_complete(struct afu_cmd *cmd)
struct cxlflash_cfg *cfg = afu->parent;
bool cmd_is_tmf;
 
-   if (cmd->rcb.scp) {
-   scp = cmd->rcb.scp;
+   if (cmd->scp) {
+   scp = cmd->scp;
if (unlikely(cmd->sa.ioasc))
process_cmd_err(cmd, scp);
else
@@ -315,7 +315,7 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, 
u64 tmfcmd)
cfg->tmf_active = true;
spin_unlock_irqrestore(>tmf_slock, lock_flags);
 
-   cmd->rcb.scp = scp;
+   cmd->scp = scp;
cmd->parent = afu;
cmd->cmd_tmf = true;
 
@@ -445,7 +445,7 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *scp)
cmd->rcb.data_ea = sg_dma_address(sg);
}
 
-   cmd->rcb.scp = scp;
+   cmd->scp = scp;
cmd->parent = afu;
 
cmd->rcb.ctx_id = afu->ctx_hndl;
diff --git a/drivers/scsi/cxlflash/sislite.h b/drivers/scsi/cxlflash/sislite.h
index 347fc16..1a2d09c 100644
--- a/drivers/scsi/cxlflash/sislite.h
+++ b/drivers/scsi/cxlflash/sislite.h
@@ -72,7 +72,7 @@ struct sisl_ioarcb {
u16 timeout;/* in units specified by req_flags */
u32 rsvd1;
u8 cdb[16]; /* must be in big endian */
-   struct scsi_cmnd *scp;
+   u64 reserved;   /* Reserved area */
 } __packed;
 
 struct sisl_rc {
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 11/14] cxlflash: Cleanup send_tmf()

2016-11-28 Thread Uma Krishnan 
From: "Matthew R. Ochs" 

The send_tmf() routine includes some copy/paste cruft that can be
removed as well as the setting of an AFU command-specific while
holding the tmf_slock. While not a bug, it is out of place and
should be shifted down alongside the other command initialization
statements for clarity.

Signed-off-by: Matthew R. Ochs 
---
 drivers/scsi/cxlflash/main.c | 22 +++---
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index db77030..b763699 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -299,12 +299,10 @@ static int wait_resp(struct afu *afu, struct afu_cmd *cmd)
  */
 static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd)
 {
-   struct afu_cmd *cmd = sc_to_afucz(scp);
-
u32 port_sel = scp->device->channel + 1;
-   short lflag = 0;
struct Scsi_Host *host = scp->device->host;
struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)host->hostdata;
+   struct afu_cmd *cmd = sc_to_afucz(scp);
struct device *dev = >dev->dev;
ulong lock_flags;
int rc = 0;
@@ -317,27 +315,21 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd 
*scp, u64 tmfcmd)
  !cfg->tmf_active,
  cfg->tmf_slock);
cfg->tmf_active = true;
-   cmd->cmd_tmf = true;
spin_unlock_irqrestore(>tmf_slock, lock_flags);
 
+   cmd->rcb.scp = scp;
+   cmd->parent = afu;
+   cmd->cmd_tmf = true;
+
cmd->rcb.ctx_id = afu->ctx_hndl;
cmd->rcb.msi = SISL_MSI_RRQ_UPDATED;
cmd->rcb.port_sel = port_sel;
cmd->rcb.lun_id = lun_to_lunid(scp->device->lun);
-
-   lflag = SISL_REQ_FLAGS_TMF_CMD;
-
cmd->rcb.req_flags = (SISL_REQ_FLAGS_PORT_LUN_ID |
- SISL_REQ_FLAGS_SUP_UNDERRUN | lflag);
-
-   /* Stash the scp in the command, for reuse during interrupt */
-   cmd->rcb.scp = scp;
-   cmd->parent = afu;
-
-   /* Copy the CDB from the cmd passed in */
+ SISL_REQ_FLAGS_SUP_UNDERRUN |
+ SISL_REQ_FLAGS_TMF_CMD);
memcpy(cmd->rcb.cdb, , sizeof(tmfcmd));
 
-   /* Send the command */
rc = send_cmd(afu, cmd);
if (unlikely(rc)) {
spin_lock_irqsave(>tmf_slock, lock_flags);
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 07/14] cxlflash: Use cmd_size for private commands

2016-11-28 Thread Uma Krishnan 
From: "Matthew R. Ochs" 

Instead of using a private pool of AFU commands, use cmd_size to prime
the private pool of SCSI commands such that they are allocated with a
size large enough to contain an aligned AFU command. Use scsi_cmd_priv()
to derive the aligned/zeroed private command on queuecommand and TMF
paths. Remove cmd_checkout() as it is no longer required. The remaining
AFU private command infrastructure will be removed in a cleanup commit.

Signed-off-by: Matthew R. Ochs 
---
 drivers/scsi/cxlflash/common.h | 14 +
 drivers/scsi/cxlflash/main.c   | 65 +++---
 2 files changed, 25 insertions(+), 54 deletions(-)

diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 4525514..539908f 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 extern const struct file_operations cxlflash_cxl_fops;
@@ -146,6 +147,19 @@ struct afu_cmd {
 */
 } __aligned(cache_line_size());
 
+static inline struct afu_cmd *sc_to_afuc(struct scsi_cmnd *sc)
+{
+   return PTR_ALIGN(scsi_cmd_priv(sc), __alignof__(struct afu_cmd));
+}
+
+static inline struct afu_cmd *sc_to_afucz(struct scsi_cmnd *sc)
+{
+   struct afu_cmd *afuc = sc_to_afuc(sc);
+
+   memset(afuc, 0, sizeof(*afuc));
+   return afuc;
+}
+
 struct afu {
/* Stuff requiring alignment go first. */
 
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 0f13b4d..43140ce 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -35,38 +35,6 @@ MODULE_AUTHOR("Matthew R. Ochs ");
 MODULE_LICENSE("GPL");
 
 /**
- * cmd_checkout() - checks out an AFU command
- * @afu:   AFU to checkout from.
- *
- * Commands are checked out in a round-robin fashion. Note that since
- * the command pool is larger than the hardware queue, the majority of
- * times we will only loop once or twice before getting a command. The
- * CDB within the command is initialized (zeroed) prior to returning.
- *
- * Return: The checked out command or NULL when command pool is empty.
- */
-static struct afu_cmd *cmd_checkout(struct afu *afu)
-{
-   int k, dec = CXLFLASH_NUM_CMDS;
-   struct afu_cmd *cmd;
-
-   while (dec--) {
-   k = (afu->cmd_couts++ & (CXLFLASH_NUM_CMDS - 1));
-
-   cmd = >cmd[k];
-
-   if (!atomic_dec_if_positive(>free)) {
-   pr_devel("%s: returning found index=%d cmd=%p\n",
-__func__, cmd->slot, cmd);
-   memset(cmd->rcb.cdb, 0, sizeof(cmd->rcb.cdb));
-   return cmd;
-   }
-   }
-
-   return NULL;
-}
-
-/**
  * cmd_checkin() - checks in an AFU command
  * @cmd:   AFU command to checkin.
  *
@@ -232,7 +200,6 @@ static void cmd_complete(struct afu_cmd *cmd)
scp->result = (DID_OK << 16);
 
cmd_is_tmf = cmd->cmd_tmf;
-   cmd_checkin(cmd); /* Don't use cmd after here */
 
pr_debug_ratelimited("%s: calling scsi_done scp=%p result=%X "
 "ioasc=%d\n", __func__, scp, scp->result,
@@ -365,7 +332,7 @@ static void wait_resp(struct afu *afu, struct afu_cmd *cmd)
  */
 static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd)
 {
-   struct afu_cmd *cmd;
+   struct afu_cmd *cmd = sc_to_afucz(scp);
 
u32 port_sel = scp->device->channel + 1;
short lflag = 0;
@@ -376,13 +343,6 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd 
*scp, u64 tmfcmd)
int rc = 0;
ulong to;
 
-   cmd = cmd_checkout(afu);
-   if (unlikely(!cmd)) {
-   dev_err(dev, "%s: could not get a free command\n", __func__);
-   rc = SCSI_MLQUEUE_HOST_BUSY;
-   goto out;
-   }
-
/* When Task Management Function is active do not send another */
spin_lock_irqsave(>tmf_slock, lock_flags);
if (cfg->tmf_active)
@@ -394,6 +354,7 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, 
u64 tmfcmd)
spin_unlock_irqrestore(>tmf_slock, lock_flags);
 
cmd->rcb.ctx_id = afu->ctx_hndl;
+   cmd->rcb.msi = SISL_MSI_RRQ_UPDATED;
cmd->rcb.port_sel = port_sel;
cmd->rcb.lun_id = lun_to_lunid(scp->device->lun);
 
@@ -402,8 +363,10 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd 
*scp, u64 tmfcmd)
cmd->rcb.req_flags = (SISL_REQ_FLAGS_PORT_LUN_ID |
  SISL_REQ_FLAGS_SUP_UNDERRUN | lflag);
 
-   /* Stash the scp in the reserved field, for reuse during interrupt */
+   /* Stash the scp in the command, for reuse during interrupt */
cmd->rcb.scp = scp;
+   cmd->parent = afu;
+   spin_lock_init(>slock);
 
/* Copy the CDB from

[PATCH v2 08/14] cxlflash: Remove private command pool

2016-11-28 Thread Uma Krishnan 
From: "Matthew R. Ochs" 

Clean up and remove the remaining private command pool infrastructure
that is no longer required.

Signed-off-by: Matthew R. Ochs 
---
 drivers/scsi/cxlflash/common.h |  7 -
 drivers/scsi/cxlflash/main.c   | 68 --
 2 files changed, 75 deletions(-)

diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 539908f..7e4ba31 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -136,8 +136,6 @@ struct afu_cmd {
spinlock_t slock;
struct completion cevent;
struct afu *parent;
-   int slot;
-   atomic_t free;
 
u8 cmd_tmf:1;
 
@@ -164,10 +162,6 @@ struct afu {
/* Stuff requiring alignment go first. */
 
u64 rrq_entry[NUM_RRQ_ENTRY];   /* 2K RRQ */
-   /*
-* Command & data for AFU commands.
-*/
-   struct afu_cmd cmd[CXLFLASH_NUM_CMDS];
 
/* Beware of alignment till here. Preferably introduce new
 * fields after this point
@@ -189,7 +183,6 @@ struct afu {
s64 room;
spinlock_t rrin_slock; /* Lock to rrin queuing and cmd_room updates */
u64 hb;
-   u32 cmd_couts;  /* Number of command checkouts */
u32 internal_lun;   /* User-desired LUN mode for this AFU */
 
char version[16];
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 43140ce..19156ad 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -35,33 +35,6 @@ MODULE_AUTHOR("Matthew R. Ochs ");
 MODULE_LICENSE("GPL");
 
 /**
- * cmd_checkin() - checks in an AFU command
- * @cmd:   AFU command to checkin.
- *
- * Safe to pass commands that have already been checked in. Several
- * internal tracking fields are reset as part of the checkin. Note
- * that these are intentionally reset prior to toggling the free bit
- * to avoid clobbering values in the event that the command is checked
- * out right away.
- */
-static void cmd_checkin(struct afu_cmd *cmd)
-{
-   cmd->rcb.scp = NULL;
-   cmd->rcb.timeout = 0;
-   cmd->sa.ioasc = 0;
-   cmd->cmd_tmf = false;
-   cmd->sa.host_use[0] = 0; /* clears both completion and retry bytes */
-
-   if (unlikely(atomic_inc_return(>free) != 1)) {
-   pr_err("%s: Freeing cmd (%d) that is not in use!\n",
-  __func__, cmd->slot);
-   return;
-   }
-
-   pr_devel("%s: released cmd %p index=%d\n", __func__, cmd, cmd->slot);
-}
-
-/**
  * process_cmd_err() - command error handler
  * @cmd:   AFU command that experienced the error.
  * @scp:   SCSI command associated with the AFU command in error.
@@ -560,28 +533,12 @@ static void free_mem(struct cxlflash_cfg *cfg)
  *
  * Cleans up all state associated with the command queue, and unmaps
  * the MMIO space.
- *
- *  - complete() will take care of commands we initiated (they'll be checked
- *  in as part of the cleanup that occurs after the completion)
- *
- *  - cmd_checkin() will take care of entries that we did not initiate and that
- *  have not (and will not) complete because they are sitting on a [now stale]
- *  hardware queue
  */
 static void stop_afu(struct cxlflash_cfg *cfg)
 {
-   int i;
struct afu *afu = cfg->afu;
-   struct afu_cmd *cmd;
 
if (likely(afu)) {
-   for (i = 0; i < CXLFLASH_NUM_CMDS; i++) {
-   cmd = >cmd[i];
-   complete(>cevent);
-   if (!atomic_read(>free))
-   cmd_checkin(cmd);
-   }
-
if (likely(afu->afu_map)) {
cxl_psa_unmap((void __iomem *)afu->afu_map);
afu->afu_map = NULL;
@@ -794,7 +751,6 @@ static void cxlflash_remove(struct pci_dev *pdev)
 static int alloc_mem(struct cxlflash_cfg *cfg)
 {
int rc = 0;
-   int i;
struct device *dev = >dev->dev;
 
/* AFU is ~12k, i.e. only one 64k page or up to four 4k pages */
@@ -808,12 +764,6 @@ static int alloc_mem(struct cxlflash_cfg *cfg)
}
cfg->afu->parent = cfg;
cfg->afu->afu_map = NULL;
-
-   for (i = 0; i < CXLFLASH_NUM_CMDS; i++) {
-   atomic_set(>afu->cmd[i].free, 1);
-   cfg->afu->cmd[i].slot = i;
-   }
-
 out:
return rc;
 }
@@ -1443,13 +1393,6 @@ static void init_pcr(struct cxlflash_cfg *cfg)
 
/* Program the Endian Control for the master context */
writeq_be(SISL_ENDIAN_CTRL, >host_map->endian_ctrl);
-
-   /* Initialize cmd fields that never change */
-   for (i = 0; i < CXLFLASH_NUM_CMDS; i++) {
-   afu->cmd[i].rcb.ctx_id = afu->ctx_hndl;
-   afu->cmd[i].rcb.msi = SISL_MSI_RRQ_UPDATED;
-   afu->cmd[i].rcb.rrq = 0x0;
-   }
 }
 
 /**
@@ -1538,19 +1481,8 @@ static int

[PATCH v2 09/14] cxlflash: Wait for active AFU commands to timeout upon tear down

2016-11-28 Thread Uma Krishnan 
From: "Matthew R. Ochs" 

With the removal of the static private command pool, the ability to
'complete' outstanding commands was lost. While not an issue for the
commands originating outside the driver, internal AFU commands are
synchronous and therefore have a timeout associated with them. To
avoid a stale memory access, the tear down sequence needs to ensure
that there are not any active commands before proceeding. As these
internal AFU commands are rare events, the simplest way to accomplish
this is detecting the activity and waiting for it to timeout.

Signed-off-by: Matthew R. Ochs 
---
 drivers/scsi/cxlflash/common.h | 1 +
 drivers/scsi/cxlflash/main.c   | 6 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 7e4ba31..6211677 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -180,6 +180,7 @@ struct afu {
u64 *hrrq_end;
u64 *hrrq_curr;
bool toggle;
+   atomic_t cmds_active;   /* Number of currently active AFU commands */
s64 room;
spinlock_t rrin_slock; /* Lock to rrin queuing and cmd_room updates */
u64 hb;
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 19156ad..839eca4 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -531,7 +531,7 @@ static void free_mem(struct cxlflash_cfg *cfg)
  *
  * Safe to call with AFU in a partially allocated/initialized state.
  *
- * Cleans up all state associated with the command queue, and unmaps
+ * Waits for any active internal AFU commands to timeout and then unmaps
  * the MMIO space.
  */
 static void stop_afu(struct cxlflash_cfg *cfg)
@@ -539,6 +539,8 @@ static void stop_afu(struct cxlflash_cfg *cfg)
struct afu *afu = cfg->afu;
 
if (likely(afu)) {
+   while (atomic_read(>cmds_active))
+   ssleep(1);
if (likely(afu->afu_map)) {
cxl_psa_unmap((void __iomem *)afu->afu_map);
afu->afu_map = NULL;
@@ -1721,6 +1723,7 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t 
ctx_hndl_u,
}
 
mutex_lock(_active);
+   atomic_inc(>cmds_active);
buf = kzalloc(sizeof(*cmd) + __alignof__(*cmd) - 1, GFP_KERNEL);
if (unlikely(!buf)) {
dev_err(dev, "%s: no memory for command\n", __func__);
@@ -1762,6 +1765,7 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t 
ctx_hndl_u,
 (cmd->sa.host_use_b[0] & B_ERROR)))
rc = -1;
 out:
+   atomic_dec(>cmds_active);
mutex_unlock(_active);
kfree(buf);
pr_debug("%s: returning rc=%d\n", __func__, rc);
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 06/14] cxlflash: Allocate memory instead of using command pool for AFU sync

2016-11-28 Thread Uma Krishnan 
From: "Matthew R. Ochs" 

As staging for the removal of the AFU command pool, remove the reliance
upon the pool for the internal AFU sync command. Instead of obtaining an
AFU command from the pool, dynamically allocate memory with the appropriate
alignment requirements. Since the AFU sync service is only executed from
the process environment, blocking is acceptable.

Signed-off-by: Matthew R. Ochs 
---
 drivers/scsi/cxlflash/main.c | 25 -
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 9f2821d..0f13b4d 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1823,8 +1823,8 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t 
ctx_hndl_u,
struct cxlflash_cfg *cfg = afu->parent;
struct device *dev = >dev->dev;
struct afu_cmd *cmd = NULL;
+   char *buf = NULL;
int rc = 0;
-   int retry_cnt = 0;
static DEFINE_MUTEX(sync_active);
 
if (cfg->state != STATE_NORMAL) {
@@ -1833,23 +1833,23 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t 
ctx_hndl_u,
}
 
mutex_lock(_active);
-retry:
-   cmd = cmd_checkout(afu);
-   if (unlikely(!cmd)) {
-   retry_cnt++;
-   udelay(1000 * retry_cnt);
-   if (retry_cnt < MC_RETRY_CNT)
-   goto retry;
-   dev_err(dev, "%s: could not get a free command\n", __func__);
+   buf = kzalloc(sizeof(*cmd) + __alignof__(*cmd) - 1, GFP_KERNEL);
+   if (unlikely(!buf)) {
+   dev_err(dev, "%s: no memory for command\n", __func__);
rc = -1;
goto out;
}
 
-   pr_debug("%s: afu=%p cmd=%p %d\n", __func__, afu, cmd, ctx_hndl_u);
+   cmd = (struct afu_cmd *)PTR_ALIGN(buf, __alignof__(*cmd));
+   init_completion(>cevent);
+   spin_lock_init(>slock);
+   cmd->parent = afu;
 
-   memset(cmd->rcb.cdb, 0, sizeof(cmd->rcb.cdb));
+   pr_debug("%s: afu=%p cmd=%p %d\n", __func__, afu, cmd, ctx_hndl_u);
 
cmd->rcb.req_flags = SISL_REQ_FLAGS_AFU_CMD;
+   cmd->rcb.ctx_id = afu->ctx_hndl;
+   cmd->rcb.msi = SISL_MSI_RRQ_UPDATED;
cmd->rcb.port_sel = 0x0;/* NA */
cmd->rcb.lun_id = 0x0;  /* NA */
cmd->rcb.data_len = 0x0;
@@ -1875,8 +1875,7 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t 
ctx_hndl_u,
rc = -1;
 out:
mutex_unlock(_active);
-   if (cmd)
-   cmd_checkin(cmd);
+   kfree(buf);
pr_debug("%s: returning rc=%d\n", __func__, rc);
return rc;
 }
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 04/14] cxlflash: Avoid command room violation

2016-11-28 Thread Uma Krishnan 
During test, a command room violation interrupt is occasionally seen
for the master context when the CXL flash devices are stressed.

After studying the code, there could be gaps in the way command room
value is being cached in cxlflash. When the cached command room is zero
the thread attempting to send becomes burdened with updating the cached
value with the actual value from the AFU. Today, this is handled with an
atomic set operation of the raw value read. Following the atomic update,
the thread proceeds to send.

This behavior is incorrect on two counts:

   - The update fails to take into account the current thread and its
 consumption of one of the hardware commands.

   - The update does not take into account other threads also atomically
 updating. Per design, a worker thread updates the cached value when a
 send thread times out. By not protecting the update with a lock, the
 cached value can be incorrectly clobbered.

To correct these issues, the update of the cached command room has been
simplified and also protected using a spin lock which is held until the
MMIO is complete. This ensures the command room is properly consumed by
the same thread. Update of cached value also takes into account the
current thread consuming a hardware command.

Signed-off-by: Uma Krishnan 
---
 drivers/scsi/cxlflash/common.h |  4 +--
 drivers/scsi/cxlflash/main.c   | 66 --
 2 files changed, 20 insertions(+), 50 deletions(-)

diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 6e68155..ef2943d 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -173,8 +173,8 @@ struct afu {
u64 *hrrq_end;
u64 *hrrq_curr;
bool toggle;
-   bool read_room;
-   atomic64_t room;
+   s64 room;
+   spinlock_t rrin_slock; /* Lock to rrin queuing and cmd_room updates */
u64 hb;
u32 cmd_couts;  /* Number of command checkouts */
u32 internal_lun;   /* User-desired LUN mode for this AFU */
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 6d33d8c..1292a74 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -306,59 +306,34 @@ static int send_cmd(struct afu *afu, struct afu_cmd *cmd)
 {
struct cxlflash_cfg *cfg = afu->parent;
struct device *dev = >dev->dev;
-   int nretry = 0;
int rc = 0;
-   u64 room;
-   long newval;
+   s64 room;
+   ulong lock_flags;
 
/*
-* This routine is used by critical users such an AFU sync and to
-* send a task management function (TMF). Thus we want to retry a
-* bit before returning an error. To avoid the performance penalty
-* of MMIO, we spread the update of 'room' over multiple commands.
+* To avoid the performance penalty of MMIO, spread the update of
+* 'room' over multiple commands.
 */
-retry:
-   newval = atomic64_dec_if_positive(>room);
-   if (!newval) {
-   do {
-   room = readq_be(>host_map->cmd_room);
-   atomic64_set(>room, room);
-   if (room)
-   goto write_ioarrin;
-   udelay(1 << nretry);
-   } while (nretry++ < MC_ROOM_RETRY_CNT);
-
-   dev_err(dev, "%s: no cmd_room to send 0x%X\n",
-  __func__, cmd->rcb.cdb[0]);
-
-   goto no_room;
-   } else if (unlikely(newval < 0)) {
-   /* This should be rare. i.e. Only if two threads race and
-* decrement before the MMIO read is done. In this case
-* just benefit from the other thread having updated
-* afu->room.
-*/
-   if (nretry++ < MC_ROOM_RETRY_CNT) {
-   udelay(1 << nretry);
-   goto retry;
+   spin_lock_irqsave(>rrin_slock, lock_flags);
+   if (--afu->room < 0) {
+   room = readq_be(>host_map->cmd_room);
+   if (room <= 0) {
+   dev_dbg_ratelimited(dev, "%s: no cmd_room to send "
+   "0x%02X, room=0x%016llX\n",
+   __func__, cmd->rcb.cdb[0], room);
+   afu->room = 0;
+   rc = SCSI_MLQUEUE_HOST_BUSY;
+   goto out;
}
-
-   goto no_room;
+   afu->room = room - 1;
}
 
-write_ioarrin:
writeq_be((u64)>rcb, >host_map->ioarrin);
 out:
+   spin_unlock_irqrestore(>rrin_slock, lock_flags);
pr_devel("%s: cmd=%p len=%d ea=%p rc=%d\n", __func__, cmd,
 cmd->rcb.data_len, (void *)cmd->rcb.data_ea, rc);
return rc;
-
-no_room:
-   afu->read_room = true;
-   kref_get(>afu->mapcount);
-

[PATCH v2 05/14] cxlflash: Remove unused buffer from AFU command

2016-11-28 Thread Uma Krishnan 
From: "Matthew R. Ochs" 

The cxlflash driver originally required a per-command 4K buffer that
hosted data passed to the AFU. When the routines that initiate AFU
and internal SCSI commands were refactored to use scsi_execute(), the
need for this buffer became obsolete. As it is no longer necessary,
the buffer is removed.

Signed-off-by: Matthew R. Ochs 
---
 drivers/scsi/cxlflash/common.h |  1 -
 drivers/scsi/cxlflash/main.c   | 28 ++--
 2 files changed, 2 insertions(+), 27 deletions(-)

diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index ef2943d..4525514 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -134,7 +134,6 @@ struct afu_cmd {
struct sisl_ioasa sa;   /* IOASA must follow IOARCB */
spinlock_t slock;
struct completion cevent;
-   char *buf;  /* per command buffer */
struct afu *parent;
int slot;
atomic_t free;
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 1292a74..9f2821d 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -41,8 +41,7 @@ MODULE_LICENSE("GPL");
  * Commands are checked out in a round-robin fashion. Note that since
  * the command pool is larger than the hardware queue, the majority of
  * times we will only loop once or twice before getting a command. The
- * buffer and CDB within the command are initialized (zeroed) prior to
- * returning.
+ * CDB within the command is initialized (zeroed) prior to returning.
  *
  * Return: The checked out command or NULL when command pool is empty.
  */
@@ -59,7 +58,6 @@ static struct afu_cmd *cmd_checkout(struct afu *afu)
if (!atomic_dec_if_positive(>free)) {
pr_devel("%s: returning found index=%d cmd=%p\n",
 __func__, cmd->slot, cmd);
-   memset(cmd->buf, 0, CMD_BUFSIZE);
memset(cmd->rcb.cdb, 0, sizeof(cmd->rcb.cdb));
return cmd;
}
@@ -590,17 +588,9 @@ static void cxlflash_wait_for_pci_err_recovery(struct 
cxlflash_cfg *cfg)
  */
 static void free_mem(struct cxlflash_cfg *cfg)
 {
-   int i;
-   char *buf = NULL;
struct afu *afu = cfg->afu;
 
if (cfg->afu) {
-   for (i = 0; i < CXLFLASH_NUM_CMDS; i++) {
-   buf = afu->cmd[i].buf;
-   if (!((u64)buf & (PAGE_SIZE - 1)))
-   free_page((ulong)buf);
-   }
-
free_pages((ulong)afu, get_order(sizeof(struct afu)));
cfg->afu = NULL;
}
@@ -849,7 +839,6 @@ static int alloc_mem(struct cxlflash_cfg *cfg)
 {
int rc = 0;
int i;
-   char *buf = NULL;
struct device *dev = >dev->dev;
 
/* AFU is ~12k, i.e. only one 64k page or up to four 4k pages */
@@ -864,20 +853,7 @@ static int alloc_mem(struct cxlflash_cfg *cfg)
cfg->afu->parent = cfg;
cfg->afu->afu_map = NULL;
 
-   for (i = 0; i < CXLFLASH_NUM_CMDS; buf += CMD_BUFSIZE, i++) {
-   if (!((u64)buf & (PAGE_SIZE - 1))) {
-   buf = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
-   if (unlikely(!buf)) {
-   dev_err(dev,
-   "%s: Allocate command buffers fail!\n",
-  __func__);
-   rc = -ENOMEM;
-   free_mem(cfg);
-   goto out;
-   }
-   }
-
-   cfg->afu->cmd[i].buf = buf;
+   for (i = 0; i < CXLFLASH_NUM_CMDS; i++) {
atomic_set(>afu->cmd[i].free, 1);
cfg->afu->cmd[i].slot = i;
}
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 03/14] cxlflash: Improve context_reset() logic

2016-11-28 Thread Uma Krishnan 
Currently, the context reset routine waits for command room to
be available before sending the reset request. Per review of the
SISLite specification and clarifications from the CXL Flash AFU
designers, this wait is unnecessary. The reset request can be
sent anytime regardless of command room, so long as only a single
reset request is active at any one point in time.

This commit simplifies the reset routine by removing the wait for
command room. Additionally it adds a debug trace to help pinpoint
hardware errors when a context reset does not complete.

Signed-off-by: Uma Krishnan 
Acked-by: Matthew R. Ochs 
---
 drivers/scsi/cxlflash/main.c | 23 +--
 1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 6004860..6d33d8c 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -263,8 +263,9 @@ static void context_reset(struct afu_cmd *cmd)
 {
int nretry = 0;
u64 rrin = 0x1;
-   u64 room = 0;
struct afu *afu = cmd->parent;
+   struct cxlflash_cfg *cfg = afu->parent;
+   struct device *dev = >dev->dev;
ulong lock_flags;
 
pr_debug("%s: cmd=%p\n", __func__, cmd);
@@ -280,23 +281,6 @@ static void context_reset(struct afu_cmd *cmd)
cmd->sa.host_use_b[0] |= (B_DONE | B_ERROR | B_TIMEOUT);
spin_unlock_irqrestore(>slock, lock_flags);
 
-   /*
-* We really want to send this reset at all costs, so spread
-* out wait time on successive retries for available room.
-*/
-   do {
-   room = readq_be(>host_map->cmd_room);
-   atomic64_set(>room, room);
-   if (room)
-   goto write_rrin;
-   udelay(1 << nretry);
-   } while (nretry++ < MC_ROOM_RETRY_CNT);
-
-   pr_err("%s: no cmd_room to send reset\n", __func__);
-   return;
-
-write_rrin:
-   nretry = 0;
writeq_be(rrin, >host_map->ioarrin);
do {
rrin = readq_be(>host_map->ioarrin);
@@ -305,6 +289,9 @@ static void context_reset(struct afu_cmd *cmd)
/* Double delay each time */
udelay(1 << nretry);
} while (nretry++ < MC_ROOM_RETRY_CNT);
+
+   dev_dbg(dev, "%s: returning rrin=0x%016llX nretry=%d\n",
+   __func__, rrin, nretry);
 }
 
 /**
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 01/14] cxlflash: Set sg_tablesize to 1 instead of SG_NONE

2016-11-28 Thread Uma Krishnan 
The following Oops is encountered when blk_mq is enabled with the
cxlflash driver:

[ 2960.817172] Oops: Kernel access of bad area, sig: 11 [#5]
[ 2960.817309] NIP  __blk_mq_run_hw_queue+0x278/0x4c0
[ 2960.817313] LR __blk_mq_run_hw_queue+0x2bc/0x4c0
[ 2960.817314] Call Trace:
[ 2960.817320] __blk_mq_run_hw_queue+0x2bc/0x4c0 (unreliable)
[ 2960.817324] blk_mq_run_hw_queue+0xd8/0x100
[ 2960.817329] blk_mq_insert_requests+0x14c/0x1f0
[ 2960.817333] blk_mq_flush_plug_list+0x150/0x190
[ 2960.817338] blk_flush_plug_list+0x11c/0x2b0
[ 2960.817344] blk_finish_plug+0x58/0x80
[ 2960.817348] __do_page_cache_readahead+0x1c0/0x2e0
[ 2960.817352] force_page_cache_readahead+0x68/0xd0
[ 2960.817356] generic_file_read_iter+0x43c/0x6a0
[ 2960.817359] blkdev_read_iter+0x68/0xa0
[ 2960.817361] __vfs_read+0x11c/0x180
[ 2960.817364] vfs_read+0xa4/0x1c0
[ 2960.817366] SyS_read+0x6c/0x110
[ 2960.817369] system_call+0x38/0xb4

The SCSI blk_mq stack assumes that sg_tablesize is always a non-zero
value with scsi_mq_setup_tags() allocating tags using sg_tablesize.
The cxlflash driver currently uses SG_NONE (0) for the sg_tablesize
as the devices it supports are not capable of scatter gather. This
mismatch of values results in the Oops above.

To resolve this issue, sg_tablesize for cxlflash can simply be set
to 1, a value which satisfies the constraints in cxlflash and the
lack of support of SG_NONE in SCSI blk_mq.

Signed-off-by: Uma Krishnan 
Acked-by: Matthew R. Ochs 
---
 drivers/scsi/cxlflash/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index b301655..6004860 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -2377,7 +2377,7 @@ static struct scsi_host_template driver_template = {
.cmd_per_lun = CXLFLASH_MAX_CMDS_PER_LUN,
.can_queue = CXLFLASH_MAX_CMDS,
.this_id = -1,
-   .sg_tablesize = SG_NONE,/* No scatter gather support */
+   .sg_tablesize = 1,  /* No scatter gather support */
.max_sectors = CXLFLASH_MAX_SECTORS,
.use_clustering = ENABLE_CLUSTERING,
.shost_attrs = cxlflash_host_attrs,
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 02/14] cxlflash: Fix crash in cxlflash_restore_luntable()

2016-11-28 Thread Uma Krishnan 
During test, the following crash was observed:

[34538.981505] Faulting instruction address: 0xd7c9c870
cpu 0x9: Vector: 300 (Data Access) at [c007f1e8f590]
pc: d7c9c870: cxlflash_restore_luntable+0x70/0x1d0 [cxlflash]
lr: d7c9c84c: cxlflash_restore_luntable+0x4c/0x1d0 [cxlflash]
sp: c007f1e8f810
   msr: 90019033
   dar: c0171d637438
 dsisr: 4000
  current = 0xc007f1e43f90
  paca= 0xc7b25100   softe: 0irq_happened: 0x01
pid   = 493, comm = eehd
enter ? for help
[c007f1e8f8a0] d7c940b0 init_afu+0xd60/0x1200 [cxlflash]
[c007f1e8f9a0] d7c945a8 cxlflash_pci_slot_reset+0x58/0xe0 [cxlflash]
[c007f1e8fa20] d715f790 cxl_pci_slot_reset+0x230/0x340 [cxl]
[c007f1e8fae0] c0040dd4 eeh_report_reset+0x144/0x180
[c007f1e8fb20] c003f708 eeh_pe_dev_traverse+0x98/0x170
[c007f1e8fbb0] c0041618 eeh_handle_normal_event+0x328/0x410
[c007f1e8fc30] c0041db8 eeh_handle_event+0x178/0x330
[c007f1e8fce0] c0042118 eeh_event_handler+0x1a8/0x1b0
[c007f1e8fd80] c011420c kthread+0xec/0x100
[c007f1e8fe30] c000a47c ret_from_kernel_thread+0x5c/0xe0

When superpipe mode is disabled for a LUN, the references for the
local lun are deleted but the LUN is still identified as being present
in the LUN table. This mismatched state can result in the above crash
when the LUN table is restored during an error recovery operation.

To fix this issue, the local LUN information structure is updated to
reflect the LUN is no longer in the LUN table once all references to
the LUN are gone.

Signed-off-by: Uma Krishnan 
Acked-by: Matthew R. Ochs 
---
 drivers/scsi/cxlflash/lunmgt.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/scsi/cxlflash/lunmgt.c b/drivers/scsi/cxlflash/lunmgt.c
index a0923ca..6c318db9 100644
--- a/drivers/scsi/cxlflash/lunmgt.c
+++ b/drivers/scsi/cxlflash/lunmgt.c
@@ -254,8 +254,14 @@ int cxlflash_manage_lun(struct scsi_device *sdev,
if (lli->parent->mode != MODE_NONE)
rc = -EBUSY;
else {
+   /*
+* Clean up local LUN for this port and reset table
+* tracking when no more references exist.
+*/
sdev->hostdata = NULL;
lli->port_sel &= ~CHAN2PORT(chan);
+   if (lli->port_sel == 0U)
+   lli->in_table = false;
}
}
 
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 00/14] cxlflash: Fixes, enhancements, cleanup and staging

2016-11-28 Thread Uma Krishnan 
The first four patches in this patch series include fixes for command
room violation and lun table management.

The remaining patches remove the reliance upon an internally maintained
private command pool in favor of private commands being allocated
alongside the SCSI commands. Several cleanup opportunities were noticed
while removing the private command pool infrastructure and have been
included as well. Lastly, the final two patches provide staging for
supporting hardware with a different queuing model.

The series is based upon 4.9-rc5, intended for 4.10 and is bisectable.

v2 Changes:
- Incorporate comments from Matthew R. Ochs
- Updated command room violation patch to use a spin lock

Matthew R. Ochs (10):
  cxlflash: Remove unused buffer from AFU command
  cxlflash: Allocate memory instead of using command pool for AFU sync
  cxlflash: Use cmd_size for private commands
  cxlflash: Remove private command pool
  cxlflash: Wait for active AFU commands to timeout upon tear down
  cxlflash: Remove AFU command lock
  cxlflash: Cleanup send_tmf()
  cxlflash: Cleanup queuecommand()
  cxlflash: Migrate IOARRIN specific routines to function pointers
  cxlflash: Migrate scsi command pointer to AFU command

Uma Krishnan (4):
  cxlflash: Set sg_tablesize to 1 instead of SG_NONE
  cxlflash: Fix crash in cxlflash_restore_luntable()
  cxlflash: Improve context_reset() logic
  cxlflash: Avoid command room violation

 drivers/scsi/cxlflash/common.h  |  39 ++--
 drivers/scsi/cxlflash/lunmgt.c  |   6 +
 drivers/scsi/cxlflash/main.c| 410 ++--
 drivers/scsi/cxlflash/sislite.h |   2 +-
 4 files changed, 130 insertions(+), 327 deletions(-)

-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] qlogicpti: Fix compiler warnings

2016-11-28 Thread David Miller 
From: Tushar Dave 
Date: Wed, 23 Nov 2016 18:28:04 -0800

> qlogicpti uses '__u32' for dma handle while invoking kernel DMA APIs,
> instead of using dma_addr_t. This hasn't caused any 'incompatible
> pointer type' warning on SPARC because until now dma_addr_t is of
> type u32. However, recent changes in SPARC ATU (iommu) enabled 64bit
> DMA and therefore dma_addr_t became of type u64. This makes
> 'incompatible pointer type' warnings inevitable.
> 
> e.g.
> drivers/scsi/qlogicpti.c: In function ‘qpti_map_queues’:
> drivers/scsi/qlogicpti.c:813: warning: passing argument 3 of 
> ‘dma_alloc_coherent’ from incompatible pointer type
> ./include/linux/dma-mapping.h:445: note: expected ‘dma_addr_t *’ but argument 
> is of type ‘__u32 *’
> drivers/scsi/qlogicpti.c:822: warning: passing argument 3 of 
> ‘dma_alloc_coherent’ from incompatible pointer type
> ./include/linux/dma-mapping.h:445: note: expected ‘dma_addr_t *’ but argument 
> is of type ‘__u32 *’
> 
> For the record, qlogicpti never executes on sun4v. Therefore even
> though 64bit DMA is enabled on SPARC, qlogicpti continues to use
> legacy iommu that guarantees DMA address is always in 32bit range.
> 
> This patch resolves aforementioned compiler warnings.
> 
> Signed-off-by: Tushar Dave 
> Reviewed-by: thomas tai 

Applied.
N‹§²ζμrΈ›yϊθšΨb²X¬ΆΗ§vΨ^–)ήΊ{.nΗ+‰·₯Š{±±Λ"Š{ayΊΚ‡Ϊ™λ,j­’f£’·hš‹ΰzΉ�w₯’Έ
’·¦j:+v‰¨ŠwθjΨmΆŸ�Ύ«‘κηzZ+ƒωšŽŠέ’j"ϊ!Άi

Re: [PATCH] bnx2fc: shift wrapping bug in bnx2fc_process_unsol_compl()

2016-11-28 Thread Laurence Oberman 


- Original Message -
> From: "Christophe JAILLET" 
> To: qlogic-storage-upstr...@qlogic.com, j...@linux.vnet.ibm.com, "martin 
> petersen" 
> Cc: linux-scsi@vger.kernel.org, linux-ker...@vger.kernel.org, 
> kernel-janit...@vger.kernel.org, "Christophe JAILLET"
> 
> Sent: Saturday, November 26, 2016 1:36:29 PM
> Subject: [PATCH] bnx2fc: shift wrapping bug in bnx2fc_process_unsol_compl()
> 
> BNX2FC_NUM_ERR_BITS is 63. err_warn_bit_map is a u64. So, to make sure that
> no shift wrapping will occur, we need need additionnal casting.
> 
> The same test is already done a few lines above and '(u64)1' is already
> used there. So just do the same here.
> 
> Signed-off-by: Christophe JAILLET 
> ---
> I guess that this could also be written with a '1ULL << i' which would be
> cleaner and less verbose IMHO, but apparently this driver does not use
> such things yet. So keep the current style with casting.
> ---
>  drivers/scsi/bnx2fc/bnx2fc_hwi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_hwi.c
> b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
> index 5ff9f89c17c7..a9dccb3b49cc 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_hwi.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
> @@ -829,7 +829,7 @@ static void bnx2fc_process_unsol_compl(struct
> bnx2fc_rport *tgt, u16 wqe)
>   ((u64)err_entry->data.err_warn_bitmap_hi << 32) |
>   (u64)err_entry->data.err_warn_bitmap_lo;
>   for (i = 0; i < BNX2FC_NUM_ERR_BITS; i++) {
> - if (err_warn_bit_map & (u64) (1 << i)) {
> + if (err_warn_bit_map & (u64)((u64)1 << i)) {
>   err_warn = i;
>   break;
>   }
> --
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Looks fine to me.
Reviewed-by: Laurence Oberman 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] scsi: aic94xx: Add a missing call to kfree

2016-11-28 Thread Tomas Henzl 
On 25.11.2016 13:23, Quentin Lambert wrote:
> Most error branches following the call to kzalloc contain
> a call to kfree. This patch add these calls where they are
> missing and set the relevant pointers to NULL.
>
> This issue was found with Hector.
>
> Signed-off-by: Quentin Lambert 

Looks good,
Reviewed-by: Tomas Henzl 
Tomas

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bnx2fc: shift wrapping bug in bnx2fc_process_unsol_compl()

2016-11-28 Thread Dan Carpenter 
On Sat, Nov 26, 2016 at 07:36:29PM +0100, Christophe JAILLET wrote:
> BNX2FC_NUM_ERR_BITS is 63. err_warn_bit_map is a u64. So, to make sure that
> no shift wrapping will occur, we need need additionnal casting.
> 
> The same test is already done a few lines above and '(u64)1' is already
> used there. So just do the same here.
> 
> Signed-off-by: Christophe JAILLET 
> ---
> I guess that this could also be written with a '1ULL << i' which would be
> cleaner and less verbose IMHO, but apparently this driver does not use
> such things yet. So keep the current style with casting.

Ugh...  No.  This is not code to emulate.  Use 1ULL << i.  Even if we
did the cast, you would only need one:

if (err_warn_bit_map & ((u64)1 << i)) {


regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] fs: configfs: don't return anything from drop_link

2016-11-28 Thread Andrzej Pietrasiewicz 
Documentation/filesystems/configfs/configfs.txt says:

"When unlink(2) is called on the symbolic link, the source item is
notified via the ->drop_link() method.  Like the ->drop_item() method,
this is a void function and cannot return failure."

The ->drop_item() is indeed a void function, the ->drop_link() is
actually not. This, together with the fact that the value of ->drop_link()
is silently ignored suggests, that it is the ->drop_link() return
type that should be corrected and changed to void.

This patch changes drop_link() signature and all its users.

Compile-tested only! It needs Tested-by from respective subsystems.

Signed-off-by: Andrzej Pietrasiewicz 
---
 Documentation/filesystems/configfs/configfs.txt |  2 +-
 drivers/nvme/target/configfs.c  | 46 ++---
 drivers/target/target_core_fabric_configfs.c|  7 ++--
 drivers/usb/gadget/configfs.c   |  8 ++---
 drivers/usb/gadget/function/uvc_configfs.c  | 25 +++---
 include/linux/configfs.h|  2 +-
 6 files changed, 31 insertions(+), 59 deletions(-)

diff --git a/Documentation/filesystems/configfs/configfs.txt 
b/Documentation/filesystems/configfs/configfs.txt
index 8ec9136..3828e85 100644
--- a/Documentation/filesystems/configfs/configfs.txt
+++ b/Documentation/filesystems/configfs/configfs.txt
@@ -174,7 +174,7 @@ among other things.  For that, it needs a type.
void (*release)(struct config_item *);
int (*allow_link)(struct config_item *src,
  struct config_item *target);
-   int (*drop_link)(struct config_item *src,
+   void (*drop_link)(struct config_item *src,
 struct config_item *target);
};
 
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index af5e2dc..9ee1490 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -466,7 +466,7 @@ static int nvmet_port_subsys_allow_link(struct config_item 
*parent,
return ret;
 }
 
-static int nvmet_port_subsys_drop_link(struct config_item *parent,
+static void nvmet_port_subsys_drop_link(struct config_item *parent,
struct config_item *target)
 {
struct nvmet_port *port = to_nvmet_port(parent->ci_parent);
@@ -474,21 +474,16 @@ static int nvmet_port_subsys_drop_link(struct config_item 
*parent,
struct nvmet_subsys_link *p;
 
down_write(_config_sem);
-   list_for_each_entry(p, >subsystems, entry) {
-   if (p->subsys == subsys)
-   goto found;
-   }
-   up_write(_config_sem);
-   return -EINVAL;
-
-found:
-   list_del(>entry);
-   nvmet_genctr++;
-   if (list_empty(>subsystems))
-   nvmet_disable_port(port);
+   list_for_each_entry(p, >subsystems, entry)
+   if (p->subsys == subsys) {
+   list_del(>entry);
+   nvmet_genctr++;
+   if (list_empty(>subsystems))
+   nvmet_disable_port(port);
+   kfree(p);
+   break;
+   }
up_write(_config_sem);
-   kfree(p);
-   return 0;
 }
 
 static struct configfs_item_operations nvmet_port_subsys_item_ops = {
@@ -542,7 +537,7 @@ static int nvmet_allowed_hosts_allow_link(struct 
config_item *parent,
return ret;
 }
 
-static int nvmet_allowed_hosts_drop_link(struct config_item *parent,
+static void nvmet_allowed_hosts_drop_link(struct config_item *parent,
struct config_item *target)
 {
struct nvmet_subsys *subsys = to_subsys(parent->ci_parent);
@@ -550,19 +545,14 @@ static int nvmet_allowed_hosts_drop_link(struct 
config_item *parent,
struct nvmet_host_link *p;
 
down_write(_config_sem);
-   list_for_each_entry(p, >hosts, entry) {
-   if (!strcmp(nvmet_host_name(p->host), nvmet_host_name(host)))
-   goto found;
-   }
-   up_write(_config_sem);
-   return -EINVAL;
-
-found:
-   list_del(>entry);
-   nvmet_genctr++;
+   list_for_each_entry(p, >hosts, entry)
+   if (!strcmp(nvmet_host_name(p->host), nvmet_host_name(host))) {
+   list_del(>entry);
+   nvmet_genctr++;
+   kfree(p);
+   break;
+   }
up_write(_config_sem);
-   kfree(p);
-   return 0;
 }
 
 static struct configfs_item_operations nvmet_allowed_hosts_item_ops = {
diff --git a/drivers/target/target_core_fabric_configfs.c 
b/drivers/target/target_core_fabric_configfs.c
index 31a096a..d8a16ca 100644
--- a/drivers/target/target_core_fabric_configfs.c
+++ b/drivers/target/target_core_fabric_configfs.c
@@ -137,7 +137,7 @@ static int target_fabric_mappedlun_link(
return core_dev_add_initiator_node_lun_acl(se_tpg, lacl,

[PATCH] scsi: mvsas: Replace pci_pool_alloc by pci_pool_zalloc

2016-11-28 Thread Souptick Joarder 
Inside mvs_task_prep(), pci_pool_alloc() followed by memset will be
replaced by pci_pool_zalloc()

Signed-off-by: Souptick joarder 
---
 drivers/scsi/mvsas/mv_sas.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index 86eb199..681e5f7 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -790,10 +790,9 @@ static int mvs_task_prep(struct sas_task *task, struct 
mvs_info *mvi, int is_tmf
slot->n_elem = n_elem;
slot->slot_tag = tag;
 
-   slot->buf = pci_pool_alloc(mvi->dma_pool, GFP_ATOMIC, >buf_dma);
+   slot->buf = pci_pool_zalloc(mvi->dma_pool, GFP_ATOMIC, >buf_dma);
if (!slot->buf)
goto err_out_tag;
-   memset(slot->buf, 0, MVS_SLOT_BUF_SZ);
 
tei.task = task;
tei.hdr = >slot[tag];
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: SG does not ignore dxferp (direct io + mmap)

2016-11-28 Thread Johannes Thumshirn 
On Fri, Nov 25, 2016 at 09:46:05AM -0500, Laurence Oberman wrote:

[...]

> 
> Johannes, you are reproducing another race in your test I think.
> 

This might actually be true. Disclaimer I'm hauntiung that one for quite some
time and now have a reproducer for it not just a downstream bug report.

I was tracing it down to the exact opposite of this KASAN report [1] which came
in over the weekend, so I'll be looking into this one as well now.

[1] http://www.spinics.net/lists/linux-scsi/msg102232.html

Byte,
Johannes

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: lpfc: Replace pci_pool_alloc by pci_pool_zalloc

2016-11-28 Thread Johannes Thumshirn 
On Mon, Nov 28, 2016 at 03:22:37PM +0530, Souptick Joarder wrote:
> In lpfc_new_scsi_buf_s3() and lpfc_new_scsi_buf_s4() pci_pool_alloc
> followed by memset will be replaced by pci_pool_zalloc()
> 
> Signed-off-by: Souptick joarder 
> ---

Looks good,
Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi: lpfc: Replace pci_pool_alloc by pci_pool_zalloc

2016-11-28 Thread Souptick Joarder 
In lpfc_new_scsi_buf_s3() and lpfc_new_scsi_buf_s4() pci_pool_alloc
followed by memset will be replaced by pci_pool_zalloc()

Signed-off-by: Souptick joarder 
---
 drivers/scsi/lpfc/lpfc_scsi.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
index d197aa1..58851c7 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.c
+++ b/drivers/scsi/lpfc/lpfc_scsi.c
@@ -413,15 +413,13 @@ struct scsi_dif_tuple {
 * struct fcp_cmnd, struct fcp_rsp and the number of bde's
 * necessary to support the sg_tablesize.
 */
-   psb->data = pci_pool_alloc(phba->lpfc_scsi_dma_buf_pool,
+   psb->data = pci_pool_zalloc(phba->lpfc_scsi_dma_buf_pool,
GFP_KERNEL, >dma_handle);
if (!psb->data) {
kfree(psb);
break;
}
 
-   /* Initialize virtual ptrs to dma_buf region. */
-   memset(psb->data, 0, phba->cfg_sg_dma_buf_size);
 
/* Allocate iotag for psb->cur_iocbq. */
iotag = lpfc_sli_next_iotag(phba, >cur_iocbq);
@@ -821,13 +819,12 @@ struct scsi_dif_tuple {
 * for the struct fcp_cmnd, struct fcp_rsp and the number
 * of bde's necessary to support the sg_tablesize.
 */
-   psb->data = pci_pool_alloc(phba->lpfc_scsi_dma_buf_pool,
+   psb->data = pci_pool_zalloc(phba->lpfc_scsi_dma_buf_pool,
GFP_KERNEL, >dma_handle);
if (!psb->data) {
kfree(psb);
break;
}
-   memset(psb->data, 0, phba->cfg_sg_dma_buf_size);
 
/*
 * 4K Page alignment is CRITICAL to BlockGuard, double check
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html