[scsi 4/4] scsi: ufs: refactor device descriptor reading

2017-01-04 Thread Tomas Winkler
Pull device descriptor reading out of ufs quirk so it
can be used also for other purposes.

Revamp the fixup setup:
1. Rename ufs_device_info to ufs_dev_desc as very similar
name ufs_dev_info is already in use.
2. Make the handlers static as they are not used out of the
ufshdc.c file.

Signed-off-by: Tomas Winkler 
---
 drivers/scsi/ufs/ufs.h| 12 
 drivers/scsi/ufs/ufs_quirks.h | 28 ++--
 drivers/scsi/ufs/ufshcd.c | 40 +++-
 3 files changed, 37 insertions(+), 43 deletions(-)

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 8e6709a3fb6b..318e4a1f76c9 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -523,4 +523,16 @@ struct ufs_dev_info {
bool is_lu_power_on_wp;
 };
 
+#define MAX_MODEL_LEN 16
+/**
+ * ufs_dev_desc - ufs device details from the device descriptor
+ *
+ * @wmanufacturerid: card details
+ * @model: card model
+ */
+struct ufs_dev_desc {
+   u16 wmanufacturerid;
+   char model[MAX_MODEL_LEN + 1];
+};
+
 #endif /* End of Header */
diff --git a/drivers/scsi/ufs/ufs_quirks.h b/drivers/scsi/ufs/ufs_quirks.h
index 08b799d4efcc..71f73d1d1ad1 100644
--- a/drivers/scsi/ufs/ufs_quirks.h
+++ b/drivers/scsi/ufs/ufs_quirks.h
@@ -21,41 +21,28 @@
 #define UFS_ANY_VENDOR 0x
 #define UFS_ANY_MODEL  "ANY_MODEL"
 
-#define MAX_MODEL_LEN 16
-
 #define UFS_VENDOR_TOSHIBA 0x198
 #define UFS_VENDOR_SAMSUNG 0x1CE
 #define UFS_VENDOR_SKHYNIX 0x1AD
 
 /**
- * ufs_device_info - ufs device details
- * @wmanufacturerid: card details
- * @model: card model
- */
-struct ufs_device_info {
-   u16 wmanufacturerid;
-   char model[MAX_MODEL_LEN + 1];
-};
-
-/**
  * ufs_dev_fix - ufs device quirk info
  * @card: ufs card details
  * @quirk: device quirk
  */
 struct ufs_dev_fix {
-   struct ufs_device_info card;
+   struct ufs_dev_desc card;
unsigned int quirk;
 };
 
 #define END_FIX { { 0 }, 0 }
 
 /* add specific device quirk */
-#define UFS_FIX(_vendor, _model, _quirk) \
-   { \
-   .card.wmanufacturerid = (_vendor),\
-   .card.model = (_model),   \
-   .quirk = (_quirk),\
-   }
+#define UFS_FIX(_vendor, _model, _quirk) { \
+   .card.wmanufacturerid = (_vendor),\
+   .card.model = (_model),\
+   .quirk = (_quirk), \
+}
 
 /*
  * If UFS device is having issue in processing LCC (Line Control
@@ -144,7 +131,4 @@ struct ufs_dev_fix {
  */
 #define UFS_DEVICE_QUIRK_HOST_PA_SAVECONFIGTIME(1 << 8)
 
-struct ufs_hba;
-void ufs_advertise_fixup_device(struct ufs_hba *hba);
-
 #endif /* UFS_QUIRKS_H_ */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index fdea08f79b7d..53b3ec40a7b0 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5008,8 +5008,8 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
return ret;
 }
 
-static int ufs_get_device_info(struct ufs_hba *hba,
-   struct ufs_device_info *card_data)
+static int ufs_get_device_desc(struct ufs_hba *hba,
+  struct ufs_dev_desc *dev_desc)
 {
int err;
u8 model_index;
@@ -5028,7 +5028,7 @@ static int ufs_get_device_info(struct ufs_hba *hba,
 * getting vendor (manufacturerID) and Bank Index in big endian
 * format
 */
-   card_data->wmanufacturerid = desc_buf[DEVICE_DESC_PARAM_MANF_ID] << 8 |
+   dev_desc->wmanufacturerid = desc_buf[DEVICE_DESC_PARAM_MANF_ID] << 8 |
 desc_buf[DEVICE_DESC_PARAM_MANF_ID + 1];
 
model_index = desc_buf[DEVICE_DESC_PARAM_PRDCT_NAME];
@@ -5042,36 +5042,26 @@ static int ufs_get_device_info(struct ufs_hba *hba,
}
 
str_desc_buf[QUERY_DESC_STRING_MAX_SIZE] = '\0';
-   strlcpy(card_data->model, (str_desc_buf + QUERY_DESC_HDR_SIZE),
+   strlcpy(dev_desc->model, (str_desc_buf + QUERY_DESC_HDR_SIZE),
min_t(u8, str_desc_buf[QUERY_DESC_LENGTH_OFFSET],
  MAX_MODEL_LEN));
 
/* Null terminate the model string */
-   card_data->model[MAX_MODEL_LEN] = '\0';
+   dev_desc->model[MAX_MODEL_LEN] = '\0';
 
 out:
return err;
 }
 
-void ufs_advertise_fixup_device(struct ufs_hba *hba)
+static void ufs_fixup_device_setup(struct ufs_hba *hba,
+  struct ufs_dev_desc *dev_desc)
 {
-   int err;
struct ufs_dev_fix *f;
-   struct ufs_device_info card_data;
-
-   card_data.wmanufacturerid = 0;
-
-   err = ufs_get_device_info(hba, &card_data);
-   if (err) {
-   dev_err(hba->dev, "%s: Failed getting device info. err = %d\n",
-   __func__, err);
-   return;
-   }
 
for (f = ufs_fixups; f->quirk; f++) {
-   if (((f->car

[scsi 1/4] scsi: ufs: ufshcd_query_descriptor_retry should be static

2017-01-04 Thread Tomas Winkler
Fix the following compilation warning:

drivers/scsi/ufs/ufshcd.c:2076:5: warning: no previous prototype for
‘ufshcd_query_descriptor_retry’ [-Wmissing-prototypes]

Also do not export the function, it should not be used out of ufs
context.

Signed-off-by: Tomas Winkler 
---
 drivers/scsi/ufs/ufshcd.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 20e5e5fb048c..be3c2900b6bb 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2073,9 +2073,11 @@ static int __ufshcd_query_descriptor(struct ufs_hba *hba,
  * The buf_len parameter will contain, on return, the length parameter
  * received on the response.
  */
-int ufshcd_query_descriptor_retry(struct ufs_hba *hba,
-   enum query_opcode opcode, enum desc_idn idn, u8 index,
-   u8 selector, u8 *desc_buf, int *buf_len)
+static int ufshcd_query_descriptor_retry(struct ufs_hba *hba,
+enum query_opcode opcode,
+enum desc_idn idn, u8 index,
+u8 selector,
+u8 *desc_buf, int *buf_len)
 {
int err;
int retries;
@@ -2089,7 +2091,6 @@ int ufshcd_query_descriptor_retry(struct ufs_hba *hba,
 
return err;
 }
-EXPORT_SYMBOL(ufshcd_query_descriptor_retry);
 
 /**
  * ufshcd_read_desc_param - read the specified descriptor parameter
-- 
2.7.4

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


[scsi 2/4] scsi: ufs: unexport descritpor reading functions

2017-01-04 Thread Tomas Winkler
Unexport ufshcd_read_device_desc and ufshcd_read_string_desc
there is no really possibility to calling them directly
outside of UFS context.

Signed-off-by: Tomas Winkler 
---
 drivers/scsi/ufs/ufshcd.c | 9 -
 drivers/scsi/ufs/ufshcd.h | 7 ---
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index be3c2900b6bb..63d7ae2c3be9 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2208,11 +2208,10 @@ static inline int ufshcd_read_power_desc(struct ufs_hba 
*hba,
return err;
 }
 
-int ufshcd_read_device_desc(struct ufs_hba *hba, u8 *buf, u32 size)
+static int ufshcd_read_device_desc(struct ufs_hba *hba, u8 *buf, u32 size)
 {
return ufshcd_read_desc(hba, QUERY_DESC_IDN_DEVICE, 0, buf, size);
 }
-EXPORT_SYMBOL(ufshcd_read_device_desc);
 
 /**
  * ufshcd_read_string_desc - read string descriptor
@@ -2224,8 +2223,9 @@ EXPORT_SYMBOL(ufshcd_read_device_desc);
  *
  * Return 0 in case of success, non-zero otherwise
  */
-int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index, u8 *buf,
-   u32 size, bool ascii)
+#define ASCII_STD true
+static int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index,
+  u8 *buf, u32 size, bool ascii)
 {
int err = 0;
 
@@ -2281,7 +2281,6 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, int 
desc_index, u8 *buf,
 out:
return err;
 }
-EXPORT_SYMBOL(ufshcd_read_string_desc);
 
 /**
  * ufshcd_read_unit_desc_param - read the specified unit descriptor parameter
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 08cd26ed2382..00fb82589895 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -713,8 +713,6 @@ static inline int ufshcd_dme_peer_get(struct ufs_hba *hba,
return ufshcd_dme_get_attr(hba, attr_sel, mib_val, DME_PEER);
 }
 
-int ufshcd_read_device_desc(struct ufs_hba *hba, u8 *buf, u32 size);
-
 static inline bool ufshcd_is_hs_mode(struct ufs_pa_layer_attr *pwr_info)
 {
return (pwr_info->pwr_rx == FAST_MODE ||
@@ -723,11 +721,6 @@ static inline bool ufshcd_is_hs_mode(struct 
ufs_pa_layer_attr *pwr_info)
pwr_info->pwr_tx == FASTAUTO_MODE);
 }
 
-#define ASCII_STD true
-
-int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index, u8 *buf,
-   u32 size, bool ascii);
-
 /* Expose Query-Request API */
 int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
enum flag_idn idn, bool *flag_res);
-- 
2.7.4

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


[scsi 3/4] scsi: ufs: ufshcd_get_max_icc_level fix endianity handling

2017-01-04 Thread Tomas Winkler
Reading big endian value from a buffer requires explicit cast.
Fix sparse warning:
drivers/scsi/ufs/ufshcd.c:4825:24: warning: cast to restricted __be16

Signed-off-by: Tomas Winkler 
---
 drivers/scsi/ufs/ufshcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 63d7ae2c3be9..fdea08f79b7d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4822,7 +4822,7 @@ static u32 ufshcd_get_max_icc_level(int sup_curr_uA, u32 
start_scan, char *buff)
u16 unit;
 
for (i = start_scan; i >= 0; i--) {
-   data = be16_to_cpu(*((u16 *)(buff + 2*i)));
+   data = be16_to_cpup((__be16 *)&buff[2 * i]);
unit = (data & ATTR_ICC_LVL_UNIT_MASK) >>
ATTR_ICC_LVL_UNIT_OFFSET;
curr_uA = data & ATTR_ICC_LVL_VALUE_MASK;
-- 
2.7.4

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


[v1] scsi: qla2xxx: qla_mr:- Release and Unmap memory regions when an error occurred.

2017-01-04 Thread Arvind Yadav
Free memory regions, if qlafx00_iospace_config is not successful.

Signed-off-by: Arvind Yadav 
---
 drivers/scsi/qla2xxx/qla_mr.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
index 02f1de1..2861df8 100644
--- a/drivers/scsi/qla2xxx/qla_mr.c
+++ b/drivers/scsi/qla2xxx/qla_mr.c
@@ -770,7 +770,7 @@
ql_log_pci(ql_log_fatal, ha->pdev, 0x014e,
"Failed to reserve PIO/MMIO regions (%s), aborting.\n",
pci_name(ha->pdev));
-   goto iospace_error_exit;
+   return -ENOMEM;
}
 
/* Use MMIO operations for all accesses. */
@@ -799,13 +799,13 @@
ql_log_pci(ql_log_warn, ha->pdev, 0x0129,
"region #2 not an MMIO resource (%s), aborting\n",
pci_name(ha->pdev));
-   goto iospace_error_exit;
+   goto iounmap_error_exit;
}
if (pci_resource_len(ha->pdev, 2) < BAR2_LEN_FX00) {
ql_log_pci(ql_log_warn, ha->pdev, 0x012a,
"Invalid PCI mem BAR2 region size (%s), aborting\n",
pci_name(ha->pdev));
-   goto iospace_error_exit;
+   goto iounmap_error_exit;
}
 
ha->iobase =
@@ -813,7 +813,7 @@
if (!ha->iobase) {
ql_log_pci(ql_log_fatal, ha->pdev, 0x012b,
"cannot remap MMIO (%s), aborting\n", pci_name(ha->pdev));
-   goto iospace_error_exit;
+   goto iounmap_error_exit;
}
 
/* Determine queue resources */
@@ -825,7 +825,10 @@
 
return 0;
 
+iounmap_error_exit:
+   iounmap(ha->cregbase);
 iospace_error_exit:
+   pci_release_selected_regions(ha->pdev, ha->bars);
return -ENOMEM;
 }
 
-- 
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: [PATCH] scsi/qla4: comments correction

2017-01-04 Thread Martin K. Petersen
> "Cao" == Cao jin  writes:

Cao>  drivers/scsi/qla4xxx/ql4_os.c | 6 +++--- 1 file changed, 3
Cao>  insertions(+), 3 deletions(-)

Applied to 4.11/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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: qedi: return via va_end to match corresponding va_start

2017-01-04 Thread Martin K. Petersen
> "Colin" == Colin King  writes:

Colin> Although on most systems va_end is a no-op, it is good practice
Colin> to use va_end on the function return path, especially since the
Colin> va_start documenation states:

Colin>   "Each invocation of va_start() must be matched by a
Colin>   corresponding
Colin>invocation of va_end() in the same function."

Applied to 4.11/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 00/13] be2iscsi: driver update 11.2.1.0

2017-01-04 Thread Martin K. Petersen
> "Jitendra" == Jitendra Bhivare  writes:

Jitendra> This patch is generated against for-next branch.  v2 changes:
Jitendra>   +be2iscsi: Reinit SGL handle, CID tables after TPE

Jitendra> Jitendra Bhivare (12):
Jitendra>   be2iscsi: Fix use of invalidate command table req be2iscsi:
Jitendra>   Fix for crash in beiscsi_eh_device_reset be2iscsi: Take
Jitendra>   iscsi_task ref in abort handler be2iscsi: Set WRB invalid
Jitendra>   bit for SkyHawk be2iscsi: Add checks to validate completions
Jitendra>   be2iscsi: Fix iSCSI cmd cleanup IOCTL be2iscsi: Remove
Jitendra>   redundant receive buffers posting be2iscsi: Remove unused
Jitendra>   struct members be2iscsi: Remove wq_name from beiscsi_hba
Jitendra>   be2iscsi: Add checks to validate CID alloc/free be2iscsi:
Jitendra>   Reinit SGL handle, CID tables after TPE be2iscsi: Update
Jitendra>   driver version

Jitendra> Ketan Mukadam (1):
Jitendra>   be2iscsi: Add warning message for unsupported adapter

Applied to 4.11/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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: [LSF/MM TOPIC] blk-trace update vs API stability

2017-01-04 Thread Jeff Moyer
Hannes Reinecke  writes:

> At LSF I'd like to discuss
> - Do we consider blktrace (and any other tracepoint in eg SCSI) as a
> stable API?

I don't have a strong opinion on this.

> - How do we go about modifying blktrace?

Blktrace has a version number associated with trace events.  Bump the
version for non-backward-compatible changes.

> - Is this even desired?

Yes.

Cheers,
Jeff
--
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: [4.10, panic, regression] iscsi: null pointer deref at iscsi_tcp_segment_done+0x20d/0x2e0

2017-01-04 Thread Laurence Oberman


- Original Message -
> From: "Laurence Oberman" 
> To: "Jan Kara" 
> Cc: "Johannes Weiner" , "Hugh Dickins" 
> , "Linus Torvalds"
> , "Dave Chinner" , "Chris 
> Leech" , "Linux
> Kernel Mailing List" , "Lee Duncan" 
> , open-is...@googlegroups.com,
> "Linux SCSI List" , linux-bl...@vger.kernel.org, 
> "Christoph Hellwig" ,
> "Andrea Arcangeli" 
> Sent: Wednesday, January 4, 2017 10:26:09 AM
> Subject: Re: [4.10, panic, regression] iscsi: null pointer deref at 
> iscsi_tcp_segment_done+0x20d/0x2e0
> 
> 
> 
> - Original Message -
> > From: "Jan Kara" 
> > To: "Johannes Weiner" 
> > Cc: "Hugh Dickins" , "Linus Torvalds"
> > , "Dave Chinner"
> > , "Chris Leech" , "Linux Kernel
> > Mailing List"
> > , "Lee Duncan" ,
> > open-is...@googlegroups.com, "Linux SCSI List"
> > , linux-bl...@vger.kernel.org, "Christoph
> > Hellwig" , "Jan Kara"
> > , "Andrea Arcangeli" 
> > Sent: Tuesday, January 3, 2017 7:28:25 AM
> > Subject: Re: [4.10, panic, regression] iscsi: null pointer deref at
> > iscsi_tcp_segment_done+0x20d/0x2e0
> > 
> > On Mon 02-01-17 16:11:36, Johannes Weiner wrote:
> > > On Fri, Dec 23, 2016 at 03:33:29AM -0500, Johannes Weiner wrote:
> > > > On Fri, Dec 23, 2016 at 02:32:41AM -0500, Johannes Weiner wrote:
> > > > > On Thu, Dec 22, 2016 at 12:22:27PM -0800, Hugh Dickins wrote:
> > > > > > On Wed, 21 Dec 2016, Linus Torvalds wrote:
> > > > > > > On Wed, Dec 21, 2016 at 9:13 PM, Dave Chinner
> > > > > > > 
> > > > > > > wrote:
> > > > > > > > I unmounted the fs, mkfs'd it again, ran the
> > > > > > > > workload again and about a minute in this fired:
> > > > > > > >
> > > > > > > > [628867.607417] [ cut here ]
> > > > > > > > [628867.608603] WARNING: CPU: 2 PID: 16925 at
> > > > > > > > mm/workingset.c:461
> > > > > > > > shadow_lru_isolate+0x171/0x220
> > > > > > > 
> > > > > > > Well, part of the changes during the merge window were the shadow
> > > > > > > entry tracking changes that came in through Andrew's tree. Adding
> > > > > > > Johannes Weiner to the participants.
> > > > > > > 
> > > > > > > > Now, this workload does not touch the page cache at all - it's
> > > > > > > > entirely an XFS metadata workload, so it should not really be
> > > > > > > > affecting the working set code.
> > > > > > > 
> > > > > > > Well, I suspect that anything that creates memory pressure will
> > > > > > > end
> > > > > > > up
> > > > > > > triggering the working set code, so ..
> > > > > > > 
> > > > > > > That said, obviously memory corruption could be involved and
> > > > > > > result
> > > > > > > in
> > > > > > > random issues too, but I wouldn't really expect that in this
> > > > > > > code.
> > > > > > > 
> > > > > > > It would probably be really useful to get more data points - is
> > > > > > > the
> > > > > > > problem reliably in this area, or is it going to be random and
> > > > > > > all
> > > > > > > over the place.
> > > > > > 
> > > > > > Data point: kswapd got WARNING on mm/workingset.c:457 in
> > > > > > shadow_lru_isolate,
> > > > > > soon followed by NULL pointer deref in list_lru_isolate, one time
> > > > > > when
> > > > > > I tried out Sunday's git tree.  Not seen since, I haven't had time
> > > > > > to
> > > > > > investigate, just set it aside as something to worry about if it
> > > > > > happens
> > > > > > again.  But it looks like shadow_lru_isolate() has issues beyond
> > > > > > Dave's
> > > > > > case (I've no XFS and no iscsi), suspect unrelated to his other
> > > > > > problems.
> > > > > 
> > > > > This seems consistent with what Dave observed: we encounter regular
> > > > > pages in radix tree nodes on the shadow LRU that should only contain
> > > > > nodes full of exceptional shadow entries. It could be an issue in the
> > > > > new slot replacement code and the node tracking callback.
> > > > 
> > > > Both encounters seem to indicate use-after-free. Dave's node didn't
> > > > warn about an unexpected node->count / node->exceptional state, but
> > > > had entries that were inconsistent with that. Hugh got the counter
> > > > warning but crashed on a list_head that's not NULLed in a live node.
> > > > 
> > > > workingset_update_node() should be called on page cache radix tree
> > > > leaf nodes that go empty. I must be missing an update_node callback
> > > > where a leaf node gets freed somewhere.
> > > 
> > > Sorry for dropping silent on this. I'm traveling over the holidays
> > > with sporadic access to my emails and no access to real equipment.
> > > 
> > > The times I managed to sneak away to look at the code didn't turn up
> > > anything useful yet.
> > > 
> > > Andrea encountered the warning as well and I gave him a debugging
> > > patch (attached below), but he hasn't been able to reproduce this
> > > condition. I've personally never seen the warning trigger, even though
> > > the patches have been running on my main development machine for quite
> > > a while now. Albeit against an older base; I've updated to Linus's
> > > master bra

[RFC PATCH] scsi: scsi_transport_fc: Create a lightweight option for Virtual FC Hosts.

2017-01-04 Thread Cathy Avery
This patch represents an attempt to resurrect the conversation
based on the submission of patch:

[PATCH 1/1] scsi: storvsc: Support manual scan of FC hosts on Hyper-V
K. Y. Srinivasan kys at microsoft.com
Sat Mar 12 21:52:48 UTC 2016

http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2016-March/087116.html

That patch attempted to address the problem of not being able to scan FC
hosts on a Hyper-V guest via sysfs due to the fact that they did not
contain the complete characteristic set associated with a normal FC host
( missing rports, vports, etc ). These new lightweight hosts as they
were subsequently referred to are not consistent with the current FC
transport model.

The patch below provides a method to reconcile the issue by offering
a lightweight option to the current FC transport class. The new option
is selected by a driver when it indicates it wants the lightweight
transport in fc_function_template. I have included the changes for
storvsc_drv.c in this patch as an example of a driver making use of the
lightweight transport option.

Signed-off-by: Cathy Avery 
---
 drivers/scsi/scsi_transport_fc.c | 125 +--
 drivers/scsi/storvsc_drv.c   |   6 +-
 include/scsi/scsi_transport_fc.h |   1 +
 3 files changed, 123 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 03577bd..4adc669 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -50,6 +50,15 @@ static int fc_bsg_hostadd(struct Scsi_Host *, struct 
fc_host_attrs *);
 static int fc_bsg_rportadd(struct Scsi_Host *, struct fc_rport *);
 static void fc_bsg_remove(struct request_queue *);
 static void fc_bsg_goose_queue(struct fc_rport *);
+static int fc_host_lw_setup(struct Scsi_Host *, struct fc_host_attrs *);
+static int fc_host_hw_setup(struct Scsi_Host *, struct fc_host_attrs *);
+static int fc_host_hw_remove(struct fc_host_attrs *);
+static struct scsi_transport_template *
+   fc_attach_lw_transport(struct fc_function_template *);
+static struct scsi_transport_template *
+   fc_attach_hw_transport(struct fc_function_template *);
+static void fc_remove_lw_host(struct Scsi_Host *);
+static void fc_remove_hw_host(struct Scsi_Host *, struct fc_host_attrs *);
 
 /*
  * Module Parameters
@@ -352,6 +361,10 @@ struct fc_internal {
 
 #define to_fc_internal(tmpl)   container_of(tmpl, struct fc_internal, t)
 
+
+static void fc_release_lw_transport(struct fc_internal *);
+static void fc_release_hw_transport(struct fc_internal *);
+
 static int fc_target_setup(struct transport_container *tc, struct device *dev,
   struct device *cdev)
 {
@@ -387,7 +400,26 @@ static int fc_host_setup(struct transport_container *tc, 
struct device *dev,
 {
struct Scsi_Host *shost = dev_to_shost(dev);
struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
+   struct fc_internal *i = to_fc_internal(shost->transportt);
+
+   if (i->f->lightweight_transport)
+   return fc_host_lw_setup(shost, fc_host);
+
+   return fc_host_hw_setup(shost, fc_host);
+}
+
+static int fc_host_lw_setup(struct Scsi_Host *shost,
+   struct fc_host_attrs *fc_host)
+{
+   fc_host->node_name = -1;
+   fc_host->port_name = -1;
+
+   return 0;
+}
 
+static int fc_host_hw_setup(struct Scsi_Host *shost,
+   struct fc_host_attrs *fc_host)
+{
/*
 * Set default values easily detected by the midlayer as
 * failure cases.  The scsi lldd is responsible for initializing
@@ -468,7 +500,16 @@ static int fc_host_remove(struct transport_container *tc, 
struct device *dev,
 {
struct Scsi_Host *shost = dev_to_shost(dev);
struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
+   struct fc_internal *i = to_fc_internal(shost->transportt);
+
+   if (i->f->lightweight_transport)
+   return 0;
 
+   return fc_host_hw_remove(fc_host);
+}
+
+static int fc_host_hw_remove(struct fc_host_attrs *fc_host)
+{
fc_bsg_remove(fc_host->rqst_q);
return 0;
 }
@@ -2175,6 +2216,49 @@ static int fc_it_nexus_response(struct Scsi_Host *shost, 
u64 nexus, int result)
 struct scsi_transport_template *
 fc_attach_transport(struct fc_function_template *ft)
 {
+   if (ft->lightweight_transport)
+   return fc_attach_lw_transport(ft);
+
+   return fc_attach_hw_transport(ft);
+}
+EXPORT_SYMBOL(fc_attach_transport);
+
+
+struct scsi_transport_template *
+fc_attach_lw_transport(struct fc_function_template *ft)
+{
+   int count;
+   struct fc_internal *i;
+
+   i = kzalloc(sizeof(struct fc_internal),
+   GFP_KERNEL);
+
+   if (unlikely(!i))
+   return NULL;
+
+   i->t.host_attrs.ac.attrs = &i->host_attrs[0];
+   i->t.host_attrs.ac.class = &fc_host_class.class;
+   i->t.host_attrs.ac.match = fc_host_match;
+   i

Re: [PATCH v3] scsi: avoid a permanent stop of the scsi device's request queue

2017-01-04 Thread Ewan D. Milne
On Mon, 2016-12-12 at 11:23 -0500, Ewan D. Milne wrote:
> ...I have not heard back yet
> from the site that reported this problem to me on their reproducer.

Not that it matters much now, but..

The response from my original reporter is that indeed the state was
being changed from SDEV_BLOCK -> SDEV_RUNNING in scsi_sysfs_add_sdev()
so the fix is correct.

-Ewan


--
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: [4.10, panic, regression] iscsi: null pointer deref at iscsi_tcp_segment_done+0x20d/0x2e0

2017-01-04 Thread Laurence Oberman


- Original Message -
> From: "Jan Kara" 
> To: "Johannes Weiner" 
> Cc: "Hugh Dickins" , "Linus Torvalds" 
> , "Dave Chinner"
> , "Chris Leech" , "Linux Kernel 
> Mailing List"
> , "Lee Duncan" , 
> open-is...@googlegroups.com, "Linux SCSI List"
> , linux-bl...@vger.kernel.org, "Christoph 
> Hellwig" , "Jan Kara"
> , "Andrea Arcangeli" 
> Sent: Tuesday, January 3, 2017 7:28:25 AM
> Subject: Re: [4.10, panic, regression] iscsi: null pointer deref at 
> iscsi_tcp_segment_done+0x20d/0x2e0
> 
> On Mon 02-01-17 16:11:36, Johannes Weiner wrote:
> > On Fri, Dec 23, 2016 at 03:33:29AM -0500, Johannes Weiner wrote:
> > > On Fri, Dec 23, 2016 at 02:32:41AM -0500, Johannes Weiner wrote:
> > > > On Thu, Dec 22, 2016 at 12:22:27PM -0800, Hugh Dickins wrote:
> > > > > On Wed, 21 Dec 2016, Linus Torvalds wrote:
> > > > > > On Wed, Dec 21, 2016 at 9:13 PM, Dave Chinner 
> > > > > > wrote:
> > > > > > > I unmounted the fs, mkfs'd it again, ran the
> > > > > > > workload again and about a minute in this fired:
> > > > > > >
> > > > > > > [628867.607417] [ cut here ]
> > > > > > > [628867.608603] WARNING: CPU: 2 PID: 16925 at mm/workingset.c:461
> > > > > > > shadow_lru_isolate+0x171/0x220
> > > > > > 
> > > > > > Well, part of the changes during the merge window were the shadow
> > > > > > entry tracking changes that came in through Andrew's tree. Adding
> > > > > > Johannes Weiner to the participants.
> > > > > > 
> > > > > > > Now, this workload does not touch the page cache at all - it's
> > > > > > > entirely an XFS metadata workload, so it should not really be
> > > > > > > affecting the working set code.
> > > > > > 
> > > > > > Well, I suspect that anything that creates memory pressure will end
> > > > > > up
> > > > > > triggering the working set code, so ..
> > > > > > 
> > > > > > That said, obviously memory corruption could be involved and result
> > > > > > in
> > > > > > random issues too, but I wouldn't really expect that in this code.
> > > > > > 
> > > > > > It would probably be really useful to get more data points - is the
> > > > > > problem reliably in this area, or is it going to be random and all
> > > > > > over the place.
> > > > > 
> > > > > Data point: kswapd got WARNING on mm/workingset.c:457 in
> > > > > shadow_lru_isolate,
> > > > > soon followed by NULL pointer deref in list_lru_isolate, one time
> > > > > when
> > > > > I tried out Sunday's git tree.  Not seen since, I haven't had time to
> > > > > investigate, just set it aside as something to worry about if it
> > > > > happens
> > > > > again.  But it looks like shadow_lru_isolate() has issues beyond
> > > > > Dave's
> > > > > case (I've no XFS and no iscsi), suspect unrelated to his other
> > > > > problems.
> > > > 
> > > > This seems consistent with what Dave observed: we encounter regular
> > > > pages in radix tree nodes on the shadow LRU that should only contain
> > > > nodes full of exceptional shadow entries. It could be an issue in the
> > > > new slot replacement code and the node tracking callback.
> > > 
> > > Both encounters seem to indicate use-after-free. Dave's node didn't
> > > warn about an unexpected node->count / node->exceptional state, but
> > > had entries that were inconsistent with that. Hugh got the counter
> > > warning but crashed on a list_head that's not NULLed in a live node.
> > > 
> > > workingset_update_node() should be called on page cache radix tree
> > > leaf nodes that go empty. I must be missing an update_node callback
> > > where a leaf node gets freed somewhere.
> > 
> > Sorry for dropping silent on this. I'm traveling over the holidays
> > with sporadic access to my emails and no access to real equipment.
> > 
> > The times I managed to sneak away to look at the code didn't turn up
> > anything useful yet.
> > 
> > Andrea encountered the warning as well and I gave him a debugging
> > patch (attached below), but he hasn't been able to reproduce this
> > condition. I've personally never seen the warning trigger, even though
> > the patches have been running on my main development machine for quite
> > a while now. Albeit against an older base; I've updated to Linus's
> > master branch now in case it's an interaction with other new code.
> > 
> > If anybody manages to reproduce this, that would be helpful. Any extra
> > eyes on this would be much appreciated too until I'm back at my desk.
> 
> I was looking into this but I didn't find a way how we could possibly leave
> radix tree node on LRU. So your debug patch looks like a good way forward.
> 
>   Honza
> --
> Jan Kara 
> SUSE Labs, CR
> --
> 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
> 

Running on 4.10.0-rc2+ and days of ISCSI served XFS load have not reproduced 
this yet for me with an 4-core (hyper 8) sy

Re: [4.10, panic, regression] iscsi: null pointer deref at iscsi_tcp_segment_done+0x20d/0x2e0

2017-01-04 Thread Christoph Hellwig
On Sat, Dec 24, 2016 at 02:17:26PM +0100, Hannes Reinecke wrote:
> Christoph, do you have a pointer to your patchset?

Here is a pointer to the current one after splitting it into properly
bisectable chunks.  Besides proper changelogs the biggest item left
is fixing up dm-mpath to not allocate its own request structures.

http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/block-pc-refactor
--
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: [bug report] [SCSI] mpt3sas: add new driver supporting 12GB SAS

2017-01-04 Thread Dan Carpenter
Oops...  This one is really old.  I didn't look carefully at it before I
hit send.  Sorry about that.

regards,
dan carpenter


On Wed, Jan 04, 2017 at 04:33:16PM +0300, Dan Carpenter wrote:
> Hello Sreekanth Reddy,
> 
> The patch f92363d12359: "[SCSI] mpt3sas: add new driver supporting
> 12GB SAS" from Nov 30, 2012, leads to the following static checker
> warning:
> 
>   drivers/scsi/mpt3sas/mpt3sas_base.c:620 _base_display_event_data()
>   warn: curly braces intended?
> 
> drivers/scsi/mpt3sas/mpt3sas_base.c
>610  case MPI2_EVENT_SAS_DISCOVERY:
>611  {
>612  Mpi2EventDataSasDiscovery_t *event_data =
>613  (Mpi2EventDataSasDiscovery_t 
> *)mpi_reply->EventData;
>614  pr_info(MPT3SAS_FMT "Discovery: (%s)", ioc->name,
>615  (event_data->ReasonCode == 
> MPI2_EVENT_SAS_DISC_RC_STARTED) ?
>616  "start" : "stop");
>617  if (event_data->DiscoveryStatus)
>618  pr_info("discovery_status(0x%08x)",
>619  le32_to_cpu(event_data->DiscoveryStatus));
>620  pr_info("\n");
> 
> The indenting is messed up here and also this should be using pr_cont()
> because pr_info() will put a bunch of crap in the middle of the line.
> 
>621  return;
>622  }
> 
> 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


[bug report] [SCSI] mpt3sas: add new driver supporting 12GB SAS

2017-01-04 Thread Dan Carpenter
Hello Sreekanth Reddy,

The patch f92363d12359: "[SCSI] mpt3sas: add new driver supporting
12GB SAS" from Nov 30, 2012, leads to the following static checker
warning:

drivers/scsi/mpt3sas/mpt3sas_base.c:620 _base_display_event_data()
warn: curly braces intended?

drivers/scsi/mpt3sas/mpt3sas_base.c
   610  case MPI2_EVENT_SAS_DISCOVERY:
   611  {
   612  Mpi2EventDataSasDiscovery_t *event_data =
   613  (Mpi2EventDataSasDiscovery_t *)mpi_reply->EventData;
   614  pr_info(MPT3SAS_FMT "Discovery: (%s)", ioc->name,
   615  (event_data->ReasonCode == 
MPI2_EVENT_SAS_DISC_RC_STARTED) ?
   616  "start" : "stop");
   617  if (event_data->DiscoveryStatus)
   618  pr_info("discovery_status(0x%08x)",
   619  le32_to_cpu(event_data->DiscoveryStatus));
   620  pr_info("\n");

The indenting is messed up here and also this should be using pr_cont()
because pr_info() will put a bunch of crap in the middle of the line.

   621  return;
   622  }

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


Re: [PATCH] mpt3sas: Force request partial completion alignment

2017-01-04 Thread Sreekanth Reddy
Hi Guilherme,

Can please share us the driver logs (with driver logging_level set to
0x3f8) for the original issue (i.e. without this patch changes) and
the HBA firmware version you have used.

Also can you please share us the steps you have followed to reproduce
this issue, so that I can try on my setup.

Thanks,
Sreekanth

On Wed, Dec 28, 2016 at 6:21 PM, Guilherme G. Piccoli
 wrote:
> From: Ram Pai 
>
> The firmware or device, possibly under a heavy I/O load, can return
> on a partial unaligned boundary. Scsi-ml expects these requests to be
> completed on an alignment boundary. Scsi-ml blindly requeues the I/O
> without checking the alignment boundary of the I/O request for the
> remaining bytes. This leads to errors, since devices cannot perform
> non-aligned read/write operations.
>
> This patch fixes the issue in the driver. It aligns unaligned
> completions of FS requests, by truncating them to the nearest
> alignment boundary.
>
> Reported-by: Mauricio Faria De Oliveira 
> Signed-off-by: Guilherme G. Piccoli 
> Signed-off-by: Ram Pai 
> ---
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 16 
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
> b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index b5c966e..55332a3 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -4644,6 +4644,8 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, 
> u8 msix_index, u32 reply)
> struct MPT3SAS_DEVICE *sas_device_priv_data;
> u32 response_code = 0;
> unsigned long flags;
> +   unsigned int sector_sz;
> +   struct request *req;
>
> mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
> scmd = _scsih_scsi_lookup_get_clear(ioc, smid);
> @@ -4703,6 +4705,20 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, 
> u8 msix_index, u32 reply)
> }
>
> xfer_cnt = le32_to_cpu(mpi_reply->TransferCount);
> +
> +   /* In case of bogus fw or device, we could end up having
> +* unaligned partial completion. We can force alignment here,
> +* then scsi-ml does not need to handle this misbehavior.
> +*/
> +   sector_sz = scmd->device->sector_size;
> +   req = scmd->request;
> +   if (unlikely(sector_sz && req && (req->cmd_type == REQ_TYPE_FS) &&
> +   (xfer_cnt % sector_sz))) {
> +   sdev_printk(KERN_INFO, scmd->device,
> +   "unaligned partial completion avoided\n");
> +   xfer_cnt = (xfer_cnt / sector_sz) * sector_sz;
> +   }
> +
> scsi_set_resid(scmd, scsi_bufflen(scmd) - xfer_cnt);
> if (ioc_status & MPI2_IOCSTATUS_FLAG_LOG_INFO_AVAILABLE)
> log_info =  le32_to_cpu(mpi_reply->IOCLogInfo);
> --
> 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] target/user: Fix use-after-free cmd->se_cmd if the cmd isexpired

2017-01-04 Thread Xiubo Li


Hi Mike

Thanks very much for your analysis.


diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index 2e33100..6396581 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -684,7 +684,6 @@ static int tcmu_check_expired_cmd(int id, void *p, void 
*data)
  
  	set_bit(TCMU_CMD_BIT_EXPIRED, &cmd->flags);

target_complete_cmd(cmd->se_cmd, SAM_STAT_CHECK_CONDITION);
-   cmd->se_cmd = NULL;
  

How did tcmu_handle_completion get to a point it was accessing the
se_cmd if the TCMU_CMD_BIT_EXPIRED bit was set?
Were memory accesses out
of order?

No, even using the -O3, becuase has there memory dependency ?


CPU1 set the TCMU_CMD_BIT_EXPIRED bit then cleared
cmd->se_cmd, but CPU2 copied cmd->se_cmd to se_cmd and saw it was NULL
but did not yet see the TCMU_CMD_BIT_EXPIRED bit set?


Because the debug rpms for my kernel version were lost, and the crash
tools couldn't be used to have a more accurate analysis.


It looks like, if you do the above patch, the above function will call
target_complete_cmd and tcmu_handle_completion will call it again, so we
will have a double free issue.

Maybe the best resolution is to move tcmu_handle_completion() between
spin_lock(&udev->commands_lock) and spin_unlock(&udev->commands_lock)?

Thanks.

BRs
Xiubo Li


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


[LSF/MM TOPIC] blk-trace update vs API stability

2017-01-04 Thread Hannes Reinecke
Hi all,

during investigating performance issues I frequently have the need to
include the request tag in the scsi tracepoints. While this can be
easily done for scsi, including it in block trace output is not that
easy due to the interdependency with the blktrace utilities.

Also there had been some discussion on KS last year, but with no
conclusive result IIRC.

At LSF I'd like to discuss
- Do we consider blktrace (and any other tracepoint in eg SCSI) as a
stable API?
- How do we go about modifying blktrace? Is this even desired?
  Or should any trace event modification be opaque to blktrace output?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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