Re: [PATCH 6/6] scsi: ufs: just use sizeof() for snprintf()

[Subhash Jadavani]

On 2017-03-28 06:49, kusumi.tomoh...@gmail.com wrote:

From: Tomohiro Kusumi 

Not much reason to use ARRAY_SIZE() when we know it's for a C string.

Signed-off-by: Tomohiro Kusumi 
---
 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 109a762..790c19c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7880,7 +7880,7 @@ int ufshcd_init(struct ufs_hba *hba, void
__iomem *mmio_base, unsigned int irq)
INIT_WORK(>clk_scaling.resume_work,
  ufshcd_clk_scaling_resume_work);

-   snprintf(wq_name, ARRAY_SIZE(wq_name), "ufs_clkscaling_%d",
+   snprintf(wq_name, sizeof(wq_name), "ufs_clkscaling_%d",
 host->host_no);
hba->clk_scaling.workq = create_singlethread_workqueue(wq_name);



Looks good to me.
Reviewed-by: Subhash Jadavani 

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 5/6] scsi: ufs: remove deprecated enum for hw interrupt

[Subhash Jadavani]

On 2017-03-28 06:49, kusumi.tomoh...@gmail.com wrote:

From: Tomohiro Kusumi 

These flags are no longer needed after 2fbd009b in 2013.

Signed-off-by: Tomohiro Kusumi 
---
 drivers/scsi/ufs/ufshcd.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 089b76f..109a762 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -143,13 +143,6 @@ enum {
UFSHCD_UIC_DME_ERROR = (1 << 5), /* DME error */
 };

-/* Interrupt configuration options */
-enum {
-   UFSHCD_INT_DISABLE,
-   UFSHCD_INT_ENABLE,
-   UFSHCD_INT_CLEAR,
-};
-
 #define ufshcd_set_eh_in_progress(h) \
((h)->eh_flags |= UFSHCD_EH_IN_PROGRESS)
 #define ufshcd_eh_in_progress(h) \



Looks good to me.
Reviewed-by: Subhash Jadavani 

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 4/6] scsi: ufs: add missing macros for register bits from UFSHCI spec

[Subhash Jadavani]

On 2017-03-28 06:49, kusumi.tomoh...@gmail.com wrote:

From: Tomohiro Kusumi 

Add macros for register bits that can be found in JESD223C (v2.1).

Not all registers are defined in ufshci.h (i.e. some are unused
whether macros are defined or undefined), but all the bits for
those registers that are already defined should appear here.

Signed-off-by: Tomohiro Kusumi 
---
 drivers/scsi/ufs/ufshci.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index d14e9b9..88acfd3 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -48,6 +48,7 @@ enum {
REG_UFS_VERSION = 0x08,
REG_CONTROLLER_DEV_ID   = 0x10,
REG_CONTROLLER_PROD_ID  = 0x14,
+   REG_AUTO_HIBERNATE_IDLE_TIMER   = 0x18,
REG_INTERRUPT_STATUS= 0x20,
REG_INTERRUPT_ENABLE= 0x24,
REG_CONTROLLER_STATUS   = 0x30,
@@ -171,6 +172,7 @@ enum {
 /* HCE - Host Controller Enable 34h */
 #define CONTROLLER_ENABLE  UFS_BIT(0)
 #define CONTROLLER_DISABLE 0x0
+#define CRYPTO_GENERAL_ENABLE  UFS_BIT(1)

 /* UECPA - Host UIC Error Code PHY Adapter Layer 38h */
 #define UIC_PHY_ADAPTER_LAYER_ERRORUFS_BIT(31)


Looks good to me.
Reviewed-by: Subhash Jadavani 

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 3/6] scsi: ufs: non functional macro fix

[Subhash Jadavani]

On 2017-03-28 06:49, kusumi.tomoh...@gmail.com wrote:

From: Tomohiro Kusumi 

Not having () isn't likely to do any harm in this case, but all the
other macros below do have it. Also add "are" in a comment.

Signed-off-by: Tomohiro Kusumi 
---
 drivers/scsi/ufs/ufshcd.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index dd46259..089b76f 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -151,11 +151,11 @@ enum {
 };

 #define ufshcd_set_eh_in_progress(h) \
-   (h->eh_flags |= UFSHCD_EH_IN_PROGRESS)
+   ((h)->eh_flags |= UFSHCD_EH_IN_PROGRESS)
 #define ufshcd_eh_in_progress(h) \
-   (h->eh_flags & UFSHCD_EH_IN_PROGRESS)
+   ((h)->eh_flags & UFSHCD_EH_IN_PROGRESS)
 #define ufshcd_clear_eh_in_progress(h) \
-   (h->eh_flags &= ~UFSHCD_EH_IN_PROGRESS)
+   ((h)->eh_flags &= ~UFSHCD_EH_IN_PROGRESS)

 #define ufshcd_set_ufs_dev_active(h) \
((h)->curr_dev_pwr_mode = UFS_ACTIVE_PWR_MODE)
@@ -1491,7 +1491,7 @@ int ufshcd_hold(struct ufs_hba *hba, bool async)
break;
}
/*
-* If we here, it means gating work is either done or
+* If we are here, it means gating work is either done or
 * currently running. Hence, fall through to cancel gating
 * work and to enable clocks.
 */


Looks good to me.
Reviewed-by: Subhash Jadavani 

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 2/6] scsi: ufs: use existing macro CONTROLLER_ENABLE to test register bit

[Subhash Jadavani]

On 2017-03-28 06:49, kusumi.tomoh...@gmail.com wrote:

From: Tomohiro Kusumi 

(Note this commit directly goes on top of the previous one)

Signed-off-by: Tomohiro Kusumi 
---
 drivers/scsi/ufs/ufshcd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b006f1e..dd46259 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -836,7 +836,8 @@ static inline void ufshcd_hba_start(struct ufs_hba 
*hba)

  */
 static inline bool ufshcd_is_hba_active(struct ufs_hba *hba)
 {
-	return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? false : 
true;

+   return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & CONTROLLER_ENABLE)
+   ? false : true;
 }

 static const char *ufschd_uic_link_state_to_string(


Looks good to me.
Reviewed-by: Subhash Jadavani 

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/6] scsi: ufs: make ufshcd_is_{device_present,hba_active}() return bool

[Subhash Jadavani]

On 2017-03-28 06:49, kusumi.tomoh...@gmail.com wrote:

From: Tomohiro Kusumi 

ufshcd driver generally uses bool for is_xxx type things instead of 
int,

so conform to its style.

Signed-off-by: Tomohiro Kusumi 
---
 drivers/scsi/ufs/ufshcd.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b7e5128..b006f1e 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -585,12 +585,12 @@ static inline u32 ufshcd_get_ufs_version(struct
ufs_hba *hba)
  *   the host controller
  * @hba: pointer to adapter instance
  *
- * Returns 1 if device present, 0 if no device detected
+ * Returns true if device present, false if no device detected
  */
-static inline int ufshcd_is_device_present(struct ufs_hba *hba)
+static inline bool ufshcd_is_device_present(struct ufs_hba *hba)
 {
return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) &
-   DEVICE_PRESENT) ? 1 : 0;
+   DEVICE_PRESENT) ? true : false;
 }

 /**
@@ -832,11 +832,11 @@ static inline void ufshcd_hba_start(struct 
ufs_hba *hba)

  * ufshcd_is_hba_active - Get controller state
  * @hba: per adapter instance
  *
- * Returns zero if controller is active, 1 otherwise
+ * Returns false if controller is active, true otherwise
  */
-static inline int ufshcd_is_hba_active(struct ufs_hba *hba)
+static inline bool ufshcd_is_hba_active(struct ufs_hba *hba)
 {
-   return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? 0 : 1;
+	return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? false : 
true;

 }

 static const char *ufschd_uic_link_state_to_string(


Looks good to me.
Reviewed-by: Subhash Jadavani 

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] Revert "scsi: ufs: add queries retry mechanism"

[Subhash Jadavani]

On 2017-03-28 01:11, szy...@codeaurora.org wrote:

From: Szymon Mielczarek 

This reverts commit 61e073590b82a539654626ecae91b8fab11db3f3.

The patch introduced redundant query retries as we already had such
mechanism provided with _retry functions.
Both ufshcd_read_desc and ufshcd_read_unit_desc_param functions
call ufshcd_query_descriptor_retry wrapper.

Signed-off-by: Szymon Mielczarek 
---
 drivers/scsi/ufs/ufshcd.c | 54 
---

 1 file changed, 9 insertions(+), 45 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 096e95b..313 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3103,18 +3103,7 @@ static inline int ufshcd_read_power_desc(struct
ufs_hba *hba,
 u8 *buf,
 u32 size)
 {
-   int err = 0;
-   int retries;
-
-   for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
-   /* Read descriptor*/
-   err = ufshcd_read_desc(hba, QUERY_DESC_IDN_POWER, 0, buf, size);
-   if (!err)
-   break;
-   dev_dbg(hba->dev, "%s: error %d retrying\n", __func__, err);
-   }
-
-   return err;
+   return ufshcd_read_desc(hba, QUERY_DESC_IDN_POWER, 0, buf, size);
 }

 static int ufshcd_read_device_desc(struct ufs_hba *hba, u8 *buf, u32 
size)

@@ -4272,24 +4261,16 @@ static void ufshcd_set_queue_depth(struct
scsi_device *sdev)
 {
int ret = 0;
u8 lun_qdepth;
-   int retries;
struct ufs_hba *hba;

hba = shost_priv(sdev->host);

lun_qdepth = hba->nutrs;
-   for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
-   /* Read descriptor*/
-   ret = ufshcd_read_unit_desc_param(hba,
- ufshcd_scsi_to_upiu_lun(sdev->lun),
- UNIT_DESC_PARAM_LU_Q_DEPTH,
- _qdepth,
- sizeof(lun_qdepth));
-   if (!ret || ret == -ENOTSUPP)
-   break;
-
-   dev_dbg(hba->dev, "%s: error %d retrying\n", __func__, ret);
-   }
+   ret = ufshcd_read_unit_desc_param(hba,
+ ufshcd_scsi_to_upiu_lun(sdev->lun),
+ UNIT_DESC_PARAM_LU_Q_DEPTH,
+ _qdepth,
+ sizeof(lun_qdepth));

/* Some WLUN doesn't support unit descriptor */
if (ret == -EOPNOTSUPP)
@@ -5960,24 +5941,6 @@ static u32
ufshcd_find_max_sup_active_icc_level(struct ufs_hba *hba,
return icc_level;
 }

-static int ufshcd_set_icc_levels_attr(struct ufs_hba *hba, u32 
icc_level)

-{
-   int ret = 0;
-   int retries;
-
-   for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
-   /* write attribute */
-   ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
-   QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, _level);
-   if (!ret)
-   break;
-
-   dev_dbg(hba->dev, "%s: failed with error %d\n", __func__, ret);
-   }
-
-   return ret;
-}
-
 static void ufshcd_init_icc_levels(struct ufs_hba *hba)
 {
int ret;
@@ -5998,8 +5961,9 @@ static void ufshcd_init_icc_levels(struct ufs_hba 
*hba)

dev_dbg(hba->dev, "%s: setting icc_level 0x%x",
__func__, hba->init_prefetch_data.icc_level);

-   ret = ufshcd_set_icc_levels_attr(hba,
-hba->init_prefetch_data.icc_level);
+   ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
+   QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0,
+   >init_prefetch_data.icc_level);

if (ret)
dev_err(hba->dev,


Looks good to me.
Reviewed-by: Subhash Jadavani 

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[REGRESSION] v4.11-rc3: lpfc: panic during module removal / shutdown

[Junichi Nomura]

Since commit 895427bd012c ("scsi: lpfc: NVME Initiator: Base modifications"),
"rmmod lpfc" starting to cause panic or corruption due to double free.

The double-free occurs as followings:
  - During initialization, lpfc_create_wq_cq() binds cq and wq to
the same ring in the way that both cq->pring and wq->pring point
to the same object.
  - Upon removal, lpfc_sli4_queue_destroy() ends up calling
lpfc_sli4_queue_free() for both wqs and cqs
and kfree(queue->pring) is done twice.

The problem became more visible in v4.11-rc3 because commit 85e8a23936ab
("scsi: lpfc: Add shutdown method for kexec") made lpfc_pci_remove_one()
called during driver shutdown.

A sample of slub_debug output is attached below.

=
BUG kmalloc-512 (Not tainted): Object already free
-

Disabling lock debugging due to kernel taint
INFO: Allocated in lpfc_wq_create+0x31c/0x4f0 [lpfc] age=259902 cpu=0 pid=314
___slab_alloc+0x47f/0x4b0
__slab_alloc+0x40/0x5c
kmem_cache_alloc_trace+0x16c/0x1b0
lpfc_wq_create+0x31c/0x4f0 [lpfc]
lpfc_create_wq_cq+0xb6/0x370 [lpfc]
lpfc_sli4_queue_setup+0x331/0xd70 [lpfc]
lpfc_sli4_hba_setup+0x12ce/0x1e90 [lpfc]
lpfc_pci_probe_one_s4.isra.43+0x7c2/0x8f0 [lpfc]
lpfc_pci_probe_one+0xbd/0xc30 [lpfc]
local_pci_probe+0x45/0xa0
work_for_cpu_fn+0x14/0x20
process_one_work+0x165/0x410
worker_thread+0x27f/0x4c0
kthread+0x101/0x140
ret_from_fork+0x2c/0x40
INFO: Freed in lpfc_sli4_queue_free+0x11b/0x160 [lpfc] age=100 cpu=3 pid=11802
__slab_free+0x1ba/0x2c0
kfree+0x122/0x170
lpfc_sli4_queue_free+0x11b/0x160 [lpfc]
lpfc_sli4_queue_destroy+0xba/0x470 [lpfc]
lpfc_pci_remove_one+0x6b4/0x880 [lpfc]
pci_device_remove+0x39/0xc0
device_release_driver_internal+0x141/0x1f0
driver_detach+0x3f/0x80
bus_remove_driver+0x55/0xd0
driver_unregister+0x2c/0x50
pci_unregister_driver+0x2a/0xa0
lpfc_exit+0x1c/0xe84 [lpfc]
SyS_delete_module+0x1ba/0x220
do_syscall_64+0x67/0x180
return_from_SYSCALL_64+0x0/0x6a
INFO: Slab 0xea0040c9ce00 objects=38 used=34 fp=0x881032739a88 
flags=0x17c0008101
INFO: Object 0x881032739098 @offset=4248 fp=0x  (null)

Redzone 881032739090: bb bb bb bb bb bb bb bb  

Object 881032739098: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 8810327390a8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 8810327390b8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 8810327390c8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 8810327390d8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 8810327390e8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 8810327390f8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 881032739108: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 881032739118: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 881032739128: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 881032739138: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 881032739148: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 881032739158: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 881032739168: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 881032739178: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 881032739188: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 881032739198: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 8810327391a8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 8810327391b8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 8810327391c8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 8810327391d8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 8810327391e8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 8810327391f8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 881032739208: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 881032739218: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 881032739228: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  

Object 881032739238: 6b 6b 6b 6b 6b 6b 6b 

Re: [PATCH v2] libsas: fix "sysfs group not found" warnings at port teardown time

[Tejun Heo]

Hello,

On Fri, Mar 24, 2017 at 05:53:54PM +0100, Johannes Thumshirn wrote:
> [ +Cc Tejun ]
> 
> On Fri, Mar 24, 2017 at 11:44:55AM +, John Garry wrote:
> > To be clear, was this the same test with isci which you initially reported?
> 
> Yes, just echo into the PCI device's sysfs remove file and it'll trigger the
> problem.
> 
> I did some archeology and it seems as if commit bcdde7e ("sysfs: make
> __sysfs_remove_dir() recursive") introduced/uncovered this behavior.

I couldn't reproduce it with other devices (don't have a sas controller).

> For reference, here's one of my calltraces (the first of 40!):
> [ cut here ]
> WARNING: CPU: 2 PID: 5 at fs/sysfs/group.c:241 sysfs_remove_group+0xc3/0xd0
> sysfs group 'power' not found for kobject 'end_device-6:0'
> CPU: 16 PID: 5884 Comm: repro.sh Not tainted 4.11.0-rc3-libsas+ #504
> Call Trace:
>  dump_stack+0x85/0xc2
>  __warn+0xc6/0xe0
>  warn_slowpath_fmt+0x4a/0x50
>  sysfs_remove_group+0xc3/0xd0
>  dpm_sysfs_remove+0x52/0x60
>  device_del+0x13c/0x360
>  ? device_remove_file+0x14/0x20
>  attribute_container_class_device_del+0x15/0x20
>  transport_remove_classdev+0x4c/0x60
>  ? transport_add_class_device+0x40/0x40
>  attribute_container_device_trigger+0xb3/0xc0
>  transport_remove_device+0x10/0x20
>  sas_port_delete+0x12d/0x160 [scsi_transport_sas]
>  sas_deform_port+0x1bf/0x1d0 [libsas]
>  sas_unregister_ports+0x36/0x50 [libsas]
>  sas_unregister_ha+0x1b/0x40 [libsas]
>  isci_unregister+0x2a/0x40 [isci]
>  isci_pci_remove+0x52/0xb0 [isci]
>  ? __pm_runtime_resume+0x56/0x80
>  pci_device_remove+0x34/0xb0
>  device_release_driver_internal+0x158/0x210
>  device_release_driver+0xd/0x10
>  pci_stop_bus_device+0x85/0x90
>  pci_stop_and_remove_bus_device_locked+0x15/0x30
>  remove_store+0x59/0x70
>  dev_attr_store+0x13/0x20
>  sysfs_kf_write+0x40/0x50
>  kernfs_fop_write+0x130/0x1b0
>  __vfs_write+0x23/0x130
>  ? rcu_read_lock_sched_held+0x6d/0x80
>  ? rcu_sync_lockdep_assert+0x2a/0x50
>  ? __sb_start_write+0xd7/0x1e0
>  ? vfs_write+0x1a4/0x1f0
>  vfs_write+0xc6/0x1f0
>  SyS_write+0x44/0xa0
>  entry_SYSCALL_64_fastpath+0x23/0xc6
> 
> But as I said, I don't belive this is a problem in the SAS transport or the
> SAS drivers, but a device core or transport class.

So, what's most likely happening is that the parent device or kobject
which contains the attribute group has already been removed earlier
and because the removal is recursive, the later explicit removal is
trying to remove already removed files.  It can be fixed either by
reordering so that the parent node is removed after the children or
simply dropping the explicit removal of children.

Thanks.

-- 
tejun

[PATCH 1/2] hpsa: update pci ids

[Don Brace]

Reviewed-by: Gerry Morong 
Reviewed-by: Scott Teel 
Signed-off-by: Don Brace 
---
 drivers/scsi/hpsa.c |4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 0d0be77..668cf47 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -108,10 +108,12 @@ static const struct pci_device_id hpsa_pci_device_id[] = {
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C, 0x3354},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C, 0x3355},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSF, 0x103C, 0x3356},
+   {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103c, 0x1920},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1921},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1922},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1923},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1924},
+   {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103c, 0x1925},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1926},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1928},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1929},
@@ -171,10 +173,12 @@ static struct board_type products[] = {
{0x3354103C, "Smart Array P420i", _access},
{0x3355103C, "Smart Array P220i", _access},
{0x3356103C, "Smart Array P721m", _access},
+   {0x1920103C, "Smart Array P430i", _access},
{0x1921103C, "Smart Array P830i", _access},
{0x1922103C, "Smart Array P430", _access},
{0x1923103C, "Smart Array P431", _access},
{0x1924103C, "Smart Array P830", _access},
+   {0x1925103C, "Smart Array P831", _access},
{0x1926103C, "Smart Array P731m", _access},
{0x1928103C, "Smart Array P230i", _access},
{0x1929103C, "Smart Array P530", _access},

[PATCH 0/2] hpsa updates

[Don Brace]

These patches are based on Linus's tree

- add some PCI IDs
- update the driver version

---

Don Brace (2):
  hpsa: update pci ids
  hpsa: change driver version


 drivers/scsi/hpsa.c |6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

--
Signature

[PATCH 2/2] hpsa: change driver version

[Don Brace]

Reviewed-by: Gerry Morong 
Reviewed-by: Scott Teel 
Signed-off-by: Don Brace 
---
 drivers/scsi/hpsa.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 668cf47..4a6fb95 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -60,7 +60,7 @@
  * HPSA_DRIVER_VERSION must be 3 byte values (0-255) separated by '.'
  * with an optional trailing '-' followed by a byte value (0-255).
  */
-#define HPSA_DRIVER_VERSION "3.4.16-0"
+#define HPSA_DRIVER_VERSION "3.4.18-0"
 #define DRIVER_NAME "HP HPSA Driver (v " HPSA_DRIVER_VERSION ")"
 #define HPSA "hpsa"
 

Re: [PATCH 03/23] sd: implement REQ_OP_WRITE_ZEROES

[Mike Snitzer]

On Tue, Mar 28 2017 at  2:50pm -0400,
Bart Van Assche  wrote:

> On Thu, 2017-03-23 at 10:33 -0400, Christoph Hellwig wrote:
> > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> > index af632e350ab4..b6f70a09a301 100644
> > --- a/drivers/scsi/sd.c
> > +++ b/drivers/scsi/sd.c
> > @@ -748,7 +748,7 @@ static int sd_setup_unmap_cmnd(struct scsi_cmnd *cmd)
> > return scsi_init_io(cmd);
> >  }
> >  
> > -static int sd_setup_write_same16_cmnd(struct scsi_cmnd *cmd)
> > +static int sd_setup_write_same16_cmnd(struct scsi_cmnd *cmd, bool unmap)
> >  {
> > struct scsi_device *sdp = cmd->device;
> > struct request *rq = cmd->request;
> > @@ -765,13 +765,14 @@ static int sd_setup_write_same16_cmnd(struct 
> > scsi_cmnd *cmd)
> >  
> > cmd->cmd_len = 16;
> > cmd->cmnd[0] = WRITE_SAME_16;
> > -   cmd->cmnd[1] = 0x8; /* UNMAP */
> > +   if (unmap)
> > +   cmd->cmnd[1] = 0x8; /* UNMAP */
> > put_unaligned_be64(sector, >cmnd[2]);
> > put_unaligned_be32(nr_sectors, >cmnd[10]);
> 
> Hello Christoph,
> 
> A quote from SBC: "An OPTIMAL UNMAP GRANULARITY field set to a non-zero value
> indicates the optimal granularity in logical blocks for unmap requests (e.g.,
> an UNMAP command or a WRITE SAME (16) command with the UNMAP bit set to one).
> An unmap request with a number of logical blocks that is not a multiple of
> this value may result in unmap operations on fewer LBAs than requested."
> 
> This means that just like the start and end of a discard must be aligned on a
> discard_granularity boundary, WRITE SAME commands with the UNMAP bit set must
> also respect that granularity. I think this means that either
> __blkdev_issue_zeroout() has to be modified such that it rejects unaligned
> REQ_OP_WRITE_ZEROES operations or that blk_bio_write_same_split() has to be
> modified such that it generates REQ_OP_WRITEs for the unaligned start and 
> tail.

That'd get DM thinp off the hook from having to zero the unaligned start
and tail...

Re: [PATCH 03/23] sd: implement REQ_OP_WRITE_ZEROES

[Bart Van Assche]

On Thu, 2017-03-23 at 10:33 -0400, Christoph Hellwig wrote:
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index af632e350ab4..b6f70a09a301 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -748,7 +748,7 @@ static int sd_setup_unmap_cmnd(struct scsi_cmnd *cmd)
>   return scsi_init_io(cmd);
>  }
>  
> -static int sd_setup_write_same16_cmnd(struct scsi_cmnd *cmd)
> +static int sd_setup_write_same16_cmnd(struct scsi_cmnd *cmd, bool unmap)
>  {
>   struct scsi_device *sdp = cmd->device;
>   struct request *rq = cmd->request;
> @@ -765,13 +765,14 @@ static int sd_setup_write_same16_cmnd(struct scsi_cmnd 
> *cmd)
>  
>   cmd->cmd_len = 16;
>   cmd->cmnd[0] = WRITE_SAME_16;
> - cmd->cmnd[1] = 0x8; /* UNMAP */
> + if (unmap)
> + cmd->cmnd[1] = 0x8; /* UNMAP */
>   put_unaligned_be64(sector, >cmnd[2]);
>   put_unaligned_be32(nr_sectors, >cmnd[10]);

Hello Christoph,

A quote from SBC: "An OPTIMAL UNMAP GRANULARITY field set to a non-zero value
indicates the optimal granularity in logical blocks for unmap requests (e.g.,
an UNMAP command or a WRITE SAME (16) command with the UNMAP bit set to one).
An unmap request with a number of logical blocks that is not a multiple of
this value may result in unmap operations on fewer LBAs than requested."

This means that just like the start and end of a discard must be aligned on a
discard_granularity boundary, WRITE SAME commands with the UNMAP bit set must
also respect that granularity. I think this means that either
__blkdev_issue_zeroout() has to be modified such that it rejects unaligned
REQ_OP_WRITE_ZEROES operations or that blk_bio_write_same_split() has to be
modified such that it generates REQ_OP_WRITEs for the unaligned start and tail.

Bart.

Re: [PATCHv5 2/2] tcmu: Fix wrongly calculating of the base_command_size

[Mike Christie]

On 03/27/2017 04:07 AM, lixi...@cmss.chinamobile.com wrote:
> From: Xiubo Li 
> 
> The t_data_nents and t_bidi_data_nents are the numbers of the
> segments, but it couldn't be sure the block size equals to size
> of the segment.
> 
> For the worst case, all the blocks are discontiguous and there
> will need the same number of iovecs, that's to say: blocks == iovs.
> So here just set the number of iovs to block count needed by tcmu
> cmd.
> 
> Signed-off-by: Xiubo Li 
> Tested-by: Ilias Tsitsimpis 
> ---
>  drivers/target/target_core_user.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/target/target_core_user.c 
> b/drivers/target/target_core_user.c
> index 65d475f..ede815c 100644
> --- a/drivers/target/target_core_user.c
> +++ b/drivers/target/target_core_user.c
> @@ -408,6 +408,13 @@ static inline size_t tcmu_cmd_get_data_length(struct 
> tcmu_cmd *tcmu_cmd)
>   return data_length;
>  }
>  
> +static inline uint32_t tcmu_cmd_get_block_cnt(struct tcmu_cmd *tcmu_cmd)
> +{
> + size_t data_length = tcmu_cmd_get_data_length(tcmu_cmd);
> +
> + return data_length / DATA_BLOCK_SIZE;
> +}
> +
>  static sense_reason_t
>  tcmu_queue_cmd_ring(struct tcmu_cmd *tcmu_cmd)
>  {
> @@ -435,8 +442,7 @@ static inline size_t tcmu_cmd_get_data_length(struct 
> tcmu_cmd *tcmu_cmd)
>* expensive to tell how many regions are freed in the bitmap
>   */
>   base_command_size = max(offsetof(struct tcmu_cmd_entry,
> - req.iov[se_cmd->t_bidi_data_nents +
> - se_cmd->t_data_nents]),
> + req.iov[tcmu_cmd_get_block_cnt(tcmu_cmd)]),
>   sizeof(struct tcmu_cmd_entry));
>   command_size = base_command_size
>   + round_up(scsi_command_size(se_cmd->t_task_cdb), 
> TCMU_OP_ALIGN_SIZE);
> 

Looks ok to me. Thanks.
Reviewed-by: Mike Christie 

Re: [PATCH 23/23] block: remove the discard_zeroes_data flag

[Bart Van Assche]

On Thu, 2017-03-23 at 10:33 -0400, Christoph Hellwig wrote:
> Now that we use the proper REQ_OP_WRITE_ZEROES operation everywhere we can
> kill this hack.
> 
> [ ... ]
>
> diff --git a/Documentation/ABI/testing/sysfs-block 
> b/Documentation/ABI/testing/sysfs-block
> index 2da04ce6aeef..dea212db9df3 100644
> --- a/Documentation/ABI/testing/sysfs-block
> +++ b/Documentation/ABI/testing/sysfs-block
> @@ -213,14 +213,8 @@ What:
> /sys/block//queue/discard_zeroes_data
>  Date:May 2011
>  Contact: Martin K. Petersen 
>  Description:
> - Devices that support discard functionality may return
> - stale or random data when a previously discarded block
> - is read back. This can cause problems if the filesystem
> - expects discarded blocks to be explicitly cleared. If a
> - device reports that it deterministically returns zeroes
> - when a discarded area is read the discard_zeroes_data
> - parameter will be set to one. Otherwise it will be 0 and
> - the result of reading a discarded area is undefined.
> + Will always return 0.  Don't rely on any specific behavior
> + for discards, and don't read this file.
>  
>  What:/sys/block//queue/write_same_max_bytes
>  Date:January 2012
>
> [ ... ]
>
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -208,7 +208,7 @@ static ssize_t queue_discard_max_store(struct 
> request_queue *q,
>  
>  static ssize_t queue_discard_zeroes_data_show(struct request_queue *q, char 
> *page)
>  {
> - return queue_var_show(queue_discard_zeroes_data(q), page);
> + return 0;
>  }

Hello Christoph,

It seems to me like the documentation in Documentation/ABI/testing/sysfs-block
and the above code are not in sync. I think the above code will cause reading
from the discard_zeroes_data attribute to return an empty string ("") instead
of "0\n".

BTW, my personal preference is to remove this attribute entirely because keeping
it will cause confusion, no matter how well we document the behavior of this
attribute.

Thanks,

Bart.

Re: [PATCH 11/23] block_dev: use blkdev_issue_zerout for hole punches

[Bart Van Assche]

On Thu, 2017-03-23 at 10:33 -0400, Christoph Hellwig wrote:
> This gets us support for non-discard efficient write of zeroes (e.g. NVMe)
> and preparse for removing the discard_zeroes_data flag.

Hello Christoph,

"preparse" probably should have been "prepare"?

Thanks,

Bart.

Re: [PATCH] scsi: hisi_sas: fix SATA dependency

[John Garry]

On 28/03/2017 15:22, Arnd Bergmann wrote:

Removing the 'select SCSI_SAS_LIBSAS' statement in Kconfig resulted
in a link failure in configurations that have hisi_sas built-in
but libsas as a loadable module:

drivers/scsi/built-in.o: In function `hisi_sas_scan_finished':
hisi_sas_main.c:(.text+0x37ce9): undefined reference to `sas_drain_work'
drivers/scsi/built-in.o: In function `hisi_sas_slave_configure':
hisi_sas_main.c:(.text+0x37d17): undefined reference to `sas_slave_configure'
hisi_sas_main.c:(.text+0x37d40): undefined reference to `sas_change_queue_depth'
drivers/scsi/built-in.o: In function `hisi_sas_remove':

All other libsas users have the 'select' statement, so we should do the
same here for consistency. For all I can tell, the patch that added
the sata softreset does not actually introduce a dependency on SCSI_SAS_ATA
but instead adds calls into libata itself, so we can express that with
a more specific dependency.

We cannot have 'select SCSI_SAS_LIBSAS; depends on SCSI_SAS_ATA' as that
would cause a dependency loop.



Hi Arnd,

Thanks for the fix. So we missed that SCSI_SAS_ATA is a bool and not a 
tristate, like SCSI_SAS_LIBSAS. Just adding the dependency on ATA is better.




Fixes: 7c594f0407de ("scsi: hisi_sas: add softreset function for SATA disk")
Signed-off-by: Arnd Bergmann 


Signed-off-by: John Garry 


---
 drivers/scsi/hisi_sas/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/hisi_sas/Kconfig b/drivers/scsi/hisi_sas/Kconfig
index ded2c201071d..374a329b91fc 100644
--- a/drivers/scsi/hisi_sas/Kconfig
+++ b/drivers/scsi/hisi_sas/Kconfig
@@ -2,7 +2,8 @@ config SCSI_HISI_SAS
tristate "HiSilicon SAS"
depends on HAS_DMA && HAS_IOMEM
depends on ARM64 || COMPILE_TEST
-   depends on SCSI_SAS_ATA
+   select SCSI_SAS_LIBSAS
select BLK_DEV_INTEGRITY
+   depends on ATA
help
This driver supports HiSilicon's SAS HBA

Re: [PATCH 12/23] sd: handle REQ_UNMAP

[Bart Van Assche]

On Thu, 2017-03-23 at 10:33 -0400, Christoph Hellwig wrote:
> Try to use a write same with unmap bit variant if the device supports it
> and the caller asks for it.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/scsi/sd.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index b6f70a09a301..ca96bb33471b 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -871,6 +871,16 @@ static int sd_setup_write_zeroes_cmnd(struct scsi_cmnd 
> *cmd)
>   return BLKPREP_INVALID;
>   return sd_setup_ata_trim_cmnd(cmd);
>   }
> +
> + if (rq->cmd_flags & REQ_UNMAP) {
> + switch (sdkp->provisioning_mode) {
> + case SD_LBP_WS16:
> + return sd_setup_write_same16_cmnd(cmd, true);
> + case SD_LBP_WS10:
> + return sd_setup_write_same10_cmnd(cmd, true);
> + }
> + }
> +
>   if (sdp->no_write_same)
>   return BLKPREP_INVALID;
>   if (sdkp->ws16 || sector > 0x || nr_sectors > 0x)

Users can change the provisioning mode from user space from SD_LBP_WS16 into
SD_LBP_WS10 so I'm not sure it's safe to skip the (sdkp->ws16 || sector >
0x || nr_sectors > 0x) check if REQ_UNMAP is set.

Bart.

Re: [PATCH 01/23] block: renumber REQ_OP_WRITE_ZEROES

[Bart Van Assche]

On Thu, 2017-03-23 at 10:33 -0400, Christoph Hellwig wrote:
> Make life easy for implementations that needs to send a data buffer
> to the device (e.g. SCSI) by numbering it as a data out command.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  include/linux/blk_types.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index d703acb55d0f..6393c13a6498 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -160,7 +160,7 @@ enum req_opf {
>   /* write the same sector many times */
>   REQ_OP_WRITE_SAME   = 7,
>   /* write the zero filled sector many times */
> - REQ_OP_WRITE_ZEROES = 8,
> + REQ_OP_WRITE_ZEROES = 9,
>  
>   /* SCSI passthrough using struct scsi_request */
>   REQ_OP_SCSI_IN  = 32,

Hello Christoph,

Since REQ_OP_WRITE_ZEROES was introduced in kernel v4.10, do we need
"Cc: stable" and "Fixes: a6f0788ec2881" tags for this patch?

Thanks,

Bart.

Re: [REGRESSION][Stable][v3.12.y][v4.4.y][v4.9.y][v4.10.y][v4.11-rc1] scsi: storvsc: properly set residual data length on errors

[Joseph Salisbury]

On 03/27/2017 06:14 PM, Stephen Hemminger wrote:
> Are you sure the real problem is not the one fixed by this commit?
>
> commit f1c635b439a5c01776fe3a25b1e2dc546ea82e6f
> Author: Stephen Hemminger 
> Date:   Tue Mar 7 09:15:53 2017 -0800
>
> scsi: storvsc: Workaround for virtual DVD SCSI version
> 
> Hyper-V host emulation of SCSI for virtual DVD device reports SCSI
> version 0 (UNKNOWN) but is still capable of supporting REPORTLUN.
> 
> Without this patch, a GEN2 Linux guest on Hyper-V will not boot 4.11
> successfully with virtual DVD ROM device. What happens is that the SCSI
> scan process falls back to doing sequential probing by INQUIRY.  But the
> storvsc driver has a previous workaround that masks/blocks all errors
> reports from INQUIRY (or MODE_SENSE) commands.  This workaround causes
> the scan to then populate a full set of bogus LUN's on the target and
> then sends kernel spinning off into a death spiral doing block reads on
> the non-existent LUNs.
> 
> By setting the correct blacklist flags, the target with the DVD device
> is scanned with REPORTLUN and that works correctly.
> 
> Patch needs to go in current 4.11, it is safe but not necessary in older
> kernels.
> 
> Signed-off-by: Stephen Hemminger 
> Reviewed-by: K. Y. Srinivasan 
> Reviewed-by: Christoph Hellwig 
> Signed-off-by: Martin K. Petersen 
>
> -Original Message-
> From: Joseph Salisbury [mailto:joseph.salisb...@canonical.com] 
> Sent: Monday, March 27, 2017 1:22 PM
> To: Long Li 
> Cc: KY Srinivasan ; Martin K. Petersen 
> ; Haiyang Zhang ; Stephen 
> Hemminger ; j...@linux.vnet.ibm.com; 
> de...@linuxdriverproject.org; linux-scsi ; LKML 
> ; sta...@vger.kernel.org; Greg KH 
> 
> Subject: [REGRESSION][Stable][v3.12.y][v4.4.y][v4.9.y][v4.10.y][v4.11-rc1] 
> scsi: storvsc: properly set residual data length on errors
>
> Hi Long Li,
>
> A kernel bug report was opened against Ubuntu [0].  After a kernel
> bisect, it was found that reverting the following commit resolved this bug:
>
> commit 40630f462824ee24bc00d692865c86c3828094e0
> Author: Long Li 
> Date:   Wed Dec 14 18:46:03 2016 -0800
>
> scsi: storvsc: properly set residual data length on errors
>
>
> The regression was introduced in mainline as of v4.11-rc1.  It was also
> cc'd to stable and has landed in v3.12.y, v4.4.y, v4.9.y and v4.10.y.
>
>
> This regression seems pretty severe since it's preventing virtual
> machines from booting.  It's affecting a couple of users so far.  I was
> hoping to get your feedback, since you are the patch author.  Do you
> think gathering any additional data will help diagnose this issue, or
> would it be best to submit a revert request?
>
>
> Thanks,
>
> Joe
>
>
> [0] http://pad.lv/1674635
>
>
Hi Stephen,


Thanks again for pointing out commit
f1c635b439a5c01776fe3a25b1e2dc546ea82e6f.  It does indeed fix the bug. 
I noticed the commit was not cc'd to stable.  Would it be possible to do
that?


Thanks,


Joe

[PATCH] scsi: hisi_sas: fix SATA dependency

[Arnd Bergmann]

Removing the 'select SCSI_SAS_LIBSAS' statement in Kconfig resulted
in a link failure in configurations that have hisi_sas built-in
but libsas as a loadable module:

drivers/scsi/built-in.o: In function `hisi_sas_scan_finished':
hisi_sas_main.c:(.text+0x37ce9): undefined reference to `sas_drain_work'
drivers/scsi/built-in.o: In function `hisi_sas_slave_configure':
hisi_sas_main.c:(.text+0x37d17): undefined reference to `sas_slave_configure'
hisi_sas_main.c:(.text+0x37d40): undefined reference to `sas_change_queue_depth'
drivers/scsi/built-in.o: In function `hisi_sas_remove':

All other libsas users have the 'select' statement, so we should do the
same here for consistency. For all I can tell, the patch that added
the sata softreset does not actually introduce a dependency on SCSI_SAS_ATA
but instead adds calls into libata itself, so we can express that with
a more specific dependency.

We cannot have 'select SCSI_SAS_LIBSAS; depends on SCSI_SAS_ATA' as that
would cause a dependency loop.

Fixes: 7c594f0407de ("scsi: hisi_sas: add softreset function for SATA disk")
Signed-off-by: Arnd Bergmann 
---
 drivers/scsi/hisi_sas/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/hisi_sas/Kconfig b/drivers/scsi/hisi_sas/Kconfig
index ded2c201071d..374a329b91fc 100644
--- a/drivers/scsi/hisi_sas/Kconfig
+++ b/drivers/scsi/hisi_sas/Kconfig
@@ -2,7 +2,8 @@ config SCSI_HISI_SAS
tristate "HiSilicon SAS"
depends on HAS_DMA && HAS_IOMEM
depends on ARM64 || COMPILE_TEST
-   depends on SCSI_SAS_ATA
+   select SCSI_SAS_LIBSAS
select BLK_DEV_INTEGRITY
+   depends on ATA
help
This driver supports HiSilicon's SAS HBA
-- 
2.9.0

Re: [PATCH 1/7] ѕd: split sd_setup_discard_cmnd

[ax...@kernel.dk]

On Mon, Mar 27 2017, Bart Van Assche wrote:
> On Mon, 2017-03-20 at 16:43 -0400, Christoph Hellwig wrote:
> > +   u64 sector = blk_rq_pos(rq) >> (ilog2(sdp->sector_size) - 9);
> > +   u32 nr_sectors = blk_rq_sectors(rq) >> (ilog2(sdp->sector_size) - 9);
> 
> Although I know this is an issue in the existing code and not something
> introduced by you: please consider using logical_to_sectors() instead of
> open-coding this function. Otherwise this patch looks fine to me.

The downside of doing that is that we still call ilog2() twice, which
sucks. Would be faster to cache ilog2(sector_size) and use that in the
shift calculation.

-- 
Jens Axboe

[PATCH 1/6] scsi: ufs: make ufshcd_is_{device_present,hba_active}() return bool

[kusumi . tomohiro]

From: Tomohiro Kusumi 

ufshcd driver generally uses bool for is_xxx type things instead of int,
so conform to its style.

Signed-off-by: Tomohiro Kusumi 
---
 drivers/scsi/ufs/ufshcd.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b7e5128..b006f1e 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -585,12 +585,12 @@ static inline u32 ufshcd_get_ufs_version(struct ufs_hba 
*hba)
  *   the host controller
  * @hba: pointer to adapter instance
  *
- * Returns 1 if device present, 0 if no device detected
+ * Returns true if device present, false if no device detected
  */
-static inline int ufshcd_is_device_present(struct ufs_hba *hba)
+static inline bool ufshcd_is_device_present(struct ufs_hba *hba)
 {
return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) &
-   DEVICE_PRESENT) ? 1 : 0;
+   DEVICE_PRESENT) ? true : false;
 }
 
 /**
@@ -832,11 +832,11 @@ static inline void ufshcd_hba_start(struct ufs_hba *hba)
  * ufshcd_is_hba_active - Get controller state
  * @hba: per adapter instance
  *
- * Returns zero if controller is active, 1 otherwise
+ * Returns false if controller is active, true otherwise
  */
-static inline int ufshcd_is_hba_active(struct ufs_hba *hba)
+static inline bool ufshcd_is_hba_active(struct ufs_hba *hba)
 {
-   return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? 0 : 1;
+   return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? false : true;
 }
 
 static const char *ufschd_uic_link_state_to_string(
-- 
2.9.3

[PATCH 3/6] scsi: ufs: non functional macro fix

[kusumi . tomohiro]

From: Tomohiro Kusumi 

Not having () isn't likely to do any harm in this case, but all the
other macros below do have it. Also add "are" in a comment.

Signed-off-by: Tomohiro Kusumi 
---
 drivers/scsi/ufs/ufshcd.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index dd46259..089b76f 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -151,11 +151,11 @@ enum {
 };
 
 #define ufshcd_set_eh_in_progress(h) \
-   (h->eh_flags |= UFSHCD_EH_IN_PROGRESS)
+   ((h)->eh_flags |= UFSHCD_EH_IN_PROGRESS)
 #define ufshcd_eh_in_progress(h) \
-   (h->eh_flags & UFSHCD_EH_IN_PROGRESS)
+   ((h)->eh_flags & UFSHCD_EH_IN_PROGRESS)
 #define ufshcd_clear_eh_in_progress(h) \
-   (h->eh_flags &= ~UFSHCD_EH_IN_PROGRESS)
+   ((h)->eh_flags &= ~UFSHCD_EH_IN_PROGRESS)
 
 #define ufshcd_set_ufs_dev_active(h) \
((h)->curr_dev_pwr_mode = UFS_ACTIVE_PWR_MODE)
@@ -1491,7 +1491,7 @@ int ufshcd_hold(struct ufs_hba *hba, bool async)
break;
}
/*
-* If we here, it means gating work is either done or
+* If we are here, it means gating work is either done or
 * currently running. Hence, fall through to cancel gating
 * work and to enable clocks.
 */
-- 
2.9.3

[PATCH 5/6] scsi: ufs: remove deprecated enum for hw interrupt

[kusumi . tomohiro]

From: Tomohiro Kusumi 

These flags are no longer needed after 2fbd009b in 2013.

Signed-off-by: Tomohiro Kusumi 
---
 drivers/scsi/ufs/ufshcd.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 089b76f..109a762 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -143,13 +143,6 @@ enum {
UFSHCD_UIC_DME_ERROR = (1 << 5), /* DME error */
 };
 
-/* Interrupt configuration options */
-enum {
-   UFSHCD_INT_DISABLE,
-   UFSHCD_INT_ENABLE,
-   UFSHCD_INT_CLEAR,
-};
-
 #define ufshcd_set_eh_in_progress(h) \
((h)->eh_flags |= UFSHCD_EH_IN_PROGRESS)
 #define ufshcd_eh_in_progress(h) \
-- 
2.9.3

[PATCH 6/6] scsi: ufs: just use sizeof() for snprintf()

[kusumi . tomohiro]

From: Tomohiro Kusumi 

Not much reason to use ARRAY_SIZE() when we know it's for a C string.

Signed-off-by: Tomohiro Kusumi 
---
 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 109a762..790c19c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7880,7 +7880,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem 
*mmio_base, unsigned int irq)
INIT_WORK(>clk_scaling.resume_work,
  ufshcd_clk_scaling_resume_work);
 
-   snprintf(wq_name, ARRAY_SIZE(wq_name), "ufs_clkscaling_%d",
+   snprintf(wq_name, sizeof(wq_name), "ufs_clkscaling_%d",
 host->host_no);
hba->clk_scaling.workq = create_singlethread_workqueue(wq_name);
 
-- 
2.9.3

[PATCH 4/6] scsi: ufs: add missing macros for register bits from UFSHCI spec

[kusumi . tomohiro]

From: Tomohiro Kusumi 

Add macros for register bits that can be found in JESD223C (v2.1).

Not all registers are defined in ufshci.h (i.e. some are unused
whether macros are defined or undefined), but all the bits for
those registers that are already defined should appear here.

Signed-off-by: Tomohiro Kusumi 
---
 drivers/scsi/ufs/ufshci.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index d14e9b9..88acfd3 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -48,6 +48,7 @@ enum {
REG_UFS_VERSION = 0x08,
REG_CONTROLLER_DEV_ID   = 0x10,
REG_CONTROLLER_PROD_ID  = 0x14,
+   REG_AUTO_HIBERNATE_IDLE_TIMER   = 0x18,
REG_INTERRUPT_STATUS= 0x20,
REG_INTERRUPT_ENABLE= 0x24,
REG_CONTROLLER_STATUS   = 0x30,
@@ -171,6 +172,7 @@ enum {
 /* HCE - Host Controller Enable 34h */
 #define CONTROLLER_ENABLE  UFS_BIT(0)
 #define CONTROLLER_DISABLE 0x0
+#define CRYPTO_GENERAL_ENABLE  UFS_BIT(1)
 
 /* UECPA - Host UIC Error Code PHY Adapter Layer 38h */
 #define UIC_PHY_ADAPTER_LAYER_ERRORUFS_BIT(31)
-- 
2.9.3

[PATCH 2/6] scsi: ufs: use existing macro CONTROLLER_ENABLE to test register bit

[kusumi . tomohiro]

From: Tomohiro Kusumi 

(Note this commit directly goes on top of the previous one)

Signed-off-by: Tomohiro Kusumi 
---
 drivers/scsi/ufs/ufshcd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b006f1e..dd46259 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -836,7 +836,8 @@ static inline void ufshcd_hba_start(struct ufs_hba *hba)
  */
 static inline bool ufshcd_is_hba_active(struct ufs_hba *hba)
 {
-   return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? false : true;
+   return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & CONTROLLER_ENABLE)
+   ? false : true;
 }
 
 static const char *ufschd_uic_link_state_to_string(
-- 
2.9.3

Re: [PATCH] scsi: hisi_sas: add missing break in switch statement

[John Garry]

On 28/03/2017 12:12, Colin King wrote:

From: Colin Ian King 

It appears that a break in the TRANS_TX_OPEN_CNX_ERR_NO_DESTINATION
case got accidentally removed in an earlier commit, as it stands,
the ts->stat and ts->open_rej_reason are being updated twice for this
case which looks incorrect.  Fix this by adding in the missing break
statement.

Detected by CoverityScan, CID#1422110 ("Missing break in switch")

Fixes: 634a9585f49c7 ("scsi: hisi_sas: process error codes according to their 
priority")
Signed-off-by: Colin Ian King 


Thanks, checkpatch.pl would normally catch this but it was hidden in the 
formatted patch


Signed-off-by: John Garry 

[PATCH] scsi: hisi_sas: add missing break in switch statement

[Colin King]

From: Colin Ian King 

It appears that a break in the TRANS_TX_OPEN_CNX_ERR_NO_DESTINATION
case got accidentally removed in an earlier commit, as it stands,
the ts->stat and ts->open_rej_reason are being updated twice for this
case which looks incorrect.  Fix this by adding in the missing break
statement.

Detected by CoverityScan, CID#1422110 ("Missing break in switch")

Fixes: 634a9585f49c7 ("scsi: hisi_sas: process error codes according to their 
priority")
Signed-off-by: Colin Ian King 
---
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c 
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index a3af380dde9e..66c2de8b7b64 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -1709,6 +1709,7 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba,
{
ts->stat = SAS_OPEN_REJECT;
ts->open_rej_reason = SAS_OREJ_NO_DEST;
+   break;
}
case TRANS_TX_OPEN_CNX_ERR_PROTOCOL_NOT_SUPPORTED:
{
-- 
2.11.0

Re: [PATCH] enclosure: fix sysfs symlinks creation when using multipath

[Maurizio Lombardi]

Dne 21.3.2017 v 10:58 Maurizio Lombardi napsal(a):

> I will ask our customer to test your patch,
> there is only a small problem: you can't set cdev->dev = NULL
> and then call enclosure_add_links(cdev) because you will end up dereferencing
> a NULL pointer.
> I suggest a slightly different patch:
> 
> @@ -375,6 +379,7 @@ int enclosure_add_device(struct enclosure_device *edev, 
> int component,
>struct device *dev)
>  {
>   struct enclosure_component *cdev;
> + int err;
>  
>   if (!edev || component >= edev->components)
>   return -EINVAL;
> @@ -384,12 +389,18 @@ int enclosure_add_device(struct enclosure_device *edev, 
> int component,
>   if (cdev->dev == dev)
>   return -EEXIST;
>  
> - if (cdev->dev)
> + if (cdev->dev) {
>   enclosure_remove_links(cdev);
> -
> - put_device(cdev->dev);
> + put_device(cdev->dev);
> + }
>   cdev->dev = get_device(dev);
> - return enclosure_add_links(cdev);
> +
> + err = enclosure_add_links(cdev);
> + if (err) {
> + put_device(cdev->dev);
> + cdev->dev = NULL;
> + }
> + return err;
>  }
>  EXPORT_SYMBOL_GPL(enclosure_add_device);


Our customer confirms that this patch solves his issues with enclosures' 
symlinks.

Re: [PATCH v2] sd: Consider max_xfer_blocks if opt_xfer_blocks is unusable

[kbuild test robot]

Hi Fam,

[auto build test WARNING on scsi/for-next]
[also build test WARNING on v4.11-rc4 next-20170327]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Fam-Zheng/sd-Consider-max_xfer_blocks-if-opt_xfer_blocks-is-unusable/20170328-141853
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: x86_64-randconfig-x016-201713 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/list.h:8:0,
from include/linux/module.h:9,
from drivers//scsi/sd.c:35:
   drivers//scsi/sd.c: In function 'sd_revalidate_disk':
   include/linux/kernel.h:755:16: warning: comparison of distinct pointer types 
lacks a cast
 (void) ( == );   \
   ^
   include/linux/kernel.h:758:2: note: in expansion of macro '__min'
 __min(typeof(x), typeof(y),   \
 ^
>> include/linux/kernel.h:783:39: note: in expansion of macro 'min'
 __x == 0 ? __y : ((__y == 0) ? __x : min(__x, __y)); })
  ^~~
>> drivers//scsi/sd.c:2960:11: note: in expansion of macro 'min_not_zero'
 rw_max = min_not_zero(rw_max, logical_to_sectors(sdp, dev_max));
  ^~~~
--
   In file included from include/linux/list.h:8:0,
from include/linux/module.h:9,
from drivers/scsi/sd.c:35:
   drivers/scsi/sd.c: In function 'sd_revalidate_disk':
   include/linux/kernel.h:755:16: warning: comparison of distinct pointer types 
lacks a cast
 (void) ( == );   \
   ^
   include/linux/kernel.h:758:2: note: in expansion of macro '__min'
 __min(typeof(x), typeof(y),   \
 ^
>> include/linux/kernel.h:783:39: note: in expansion of macro 'min'
 __x == 0 ? __y : ((__y == 0) ? __x : min(__x, __y)); })
  ^~~
   drivers/scsi/sd.c:2960:11: note: in expansion of macro 'min_not_zero'
 rw_max = min_not_zero(rw_max, logical_to_sectors(sdp, dev_max));
  ^~~~

vim +/min_not_zero +2960 drivers//scsi/sd.c

  2944  dev_max = min_not_zero(dev_max, sdkp->max_xfer_blocks);
  2945  q->limits.max_dev_sectors = logical_to_sectors(sdp, dev_max);
  2946  
  2947  /*
  2948   * Use the device's preferred I/O size for reads and writes
  2949   * unless the reported value is unreasonably small, large, or
  2950   * garbage.
  2951   */
  2952  if (sdkp->opt_xfer_blocks &&
  2953  sdkp->opt_xfer_blocks <= dev_max &&
  2954  sdkp->opt_xfer_blocks <= SD_DEF_XFER_BLOCKS &&
  2955  logical_to_bytes(sdp, sdkp->opt_xfer_blocks) >= PAGE_SIZE) {
  2956  q->limits.io_opt = logical_to_bytes(sdp, 
sdkp->opt_xfer_blocks);
  2957  rw_max = logical_to_sectors(sdp, sdkp->opt_xfer_blocks);
  2958  } else
  2959  rw_max = BLK_DEF_MAX_SECTORS;
> 2960  rw_max = min_not_zero(rw_max, logical_to_sectors(sdp, dev_max));
  2961  
  2962  /* Combine with controller limits */
  2963  q->limits.max_sectors = min(rw_max, queue_max_hw_sectors(q));
  2964  
  2965  set_capacity(disk, logical_to_sectors(sdp, sdkp->capacity));
  2966  sd_config_write_same(sdkp);
  2967  kfree(buffer);
  2968  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip