Re: [RFC] scsi: generate uevent for SCSI sense code

2017-05-12 Thread Song Liu 

> On May 3, 2017, at 5:50 PM, Song Liu  wrote:
> 
> This patch adds capability for SCSI layer to generate uevent for SCSI
> sense code. The feature is gated by CONFIG_SCSI_SENSE_UEVENT.
> 
> We can configure which sense keys generate uevent for each device
> through sysfs entry sense_event_filter. For example, the following
> enables uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04)
> on scsi drive sdc:
> 
>echo 0x000c > /sys/block/sdc/device/sense_event_filter
> 
> Here is an example output captured by udevadm:
> 
> KERNEL[1214.945358] change   /devices/pci:00/XXX
> ACTION=change
> DEVPATH=/devices/pci:00/:00:01.0/:01:00.0/host6/XXX
> DEVTYPE=scsi_device
> DRIVER=sd
> LBA=0
> MODALIAS=scsi:t-0x00
> SDEV_UA=SCSI_SENSE
> SENSE_CODE=3/11/14
> SEQNUM=4536
> SIZE=4096
> SUBSYSTEM=scsi
> 
> Signed-off-by: Song Liu 
> ---
> drivers/scsi/Kconfig   | 14 +++
> drivers/scsi/scsi_error.c  | 26 
> drivers/scsi/scsi_lib.c| 27 +++--
> drivers/scsi/scsi_sysfs.c  | 60 ++
> include/scsi/scsi_device.h | 26 +++-
> 5 files changed, 150 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index 3c52867..4f7f211 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -237,6 +237,20 @@ config SCSI_LOGGING
> there should be no noticeable performance impact as long as you have
> logging turned off.
> 
> +config SCSI_SENSE_UEVENT
> + bool "SCSI sense code logging"
> + depends on SCSI
> + default n
> + ---help---
> +   This turns on uevent for SCSI sense code.
> +
> +   You can configure which sense keys generate uevent for each device
> +   through sysfs entry sense_event_filter. For example, the following
> +   enables uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04)
> +   on scsi drive sdc:
> +
> +   echo 0x000c > /sys/block/sdc/device/sense_event_filter
> +
> config SCSI_SCAN_ASYNC
>   bool "Asynchronous SCSI scanning"
>   depends on SCSI
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index d70c67c..eda150e 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -426,6 +426,31 @@ static void scsi_report_sense(struct scsi_device *sdev,
>   }
> }
> 
> +/*
> + * generate uevent when receiving sense code from device
> + */
> +static void scsi_send_sense_uevent(struct scsi_device *sdev,
> +struct scsi_cmnd *scmd,
> +struct scsi_sense_hdr *sshdr)
> +{
> +#ifdef CONFIG_SCSI_SENSE_UEVENT
> + struct scsi_event *evt;
> +
> + if (!test_bit(sshdr->sense_key & 0xf,
> +   >sense_event_filter))
> + return;
> + evt = sdev_evt_alloc(SDEV_EVT_SCSI_SENSE, GFP_ATOMIC);
> + if (!evt)
> + return;
> +
> + evt->sense_evt_data.lba = scsi_get_lba(scmd);
> + evt->sense_evt_data.size = blk_rq_bytes(scmd->request);
> + memcpy(>sense_evt_data.sshdr, sshdr,
> +sizeof(struct scsi_sense_hdr));
> + sdev_evt_send(sdev, evt);
> +#endif
> +}
> +
> /**
>  * scsi_check_sense - Examine scsi cmd sense
>  * @scmd: Cmd to have sense checked.
> @@ -446,6 +471,7 @@ int scsi_check_sense(struct scsi_cmnd *scmd)
>   return FAILED;  /* no valid sense data */
> 
>   scsi_report_sense(sdev, );
> + scsi_send_sense_uevent(sdev, scmd, );
> 
>   if (scsi_sense_is_deferred())
>   return NEEDS_RETRY;
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 95f963b..1095f27 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -2656,8 +2656,9 @@ EXPORT_SYMBOL(scsi_device_set_state);
>  */
> static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt)
> {
> - int idx = 0;
> - char *envp[3];
> + int idx = 0, i;
> + char *envp[5];  /* SDEV_EVT_SCSI_SENSE needs most entries (4) */
> + int free_envp = -1;
> 
>   switch (evt->evt_type) {
>   case SDEV_EVT_MEDIA_CHANGE:
> @@ -2682,6 +2683,23 @@ static void scsi_evt_emit(struct scsi_device *sdev, 
> struct scsi_event *evt)
>   case SDEV_EVT_ALUA_STATE_CHANGE_REPORTED:
>   envp[idx++] = "SDEV_UA=ASYMMETRIC_ACCESS_STATE_CHANGED";
>   break;
> +#ifdef CONFIG_SCSI_SENSE_UEVENT
> + case SDEV_EVT_SCSI_SENSE:
> + envp[idx++] = "SDEV_UA=SCSI_SENSE";
> + for (i = idx; i < idx + 3; ++i) {
> + envp[i] = kzalloc(32, GFP_ATOMIC);
> + if (!envp[i])
> + break;
> + free_envp = i;
> + }
> + snprintf(envp[idx++], 32, "LBA=%lu", evt->sense_evt_data.lba);
> + snprintf(envp[idx++], 32, "SIZE=%d", evt->sense_evt_data.size);
> +

Re: [PATCH v2 2/4] scsi_dh_alua: Do not retry for unmapped device

2017-05-12 Thread Bart Van Assche 
On Fri, 2017-05-12 at 15:15 +0200, Martin Wilck wrote:
> From: Hannes Reinecke 
> 
> If a device becomes unmapped on the target we should be returning
> SCSI_DH_DEV_OFFLINED.

Reviewed-by: Bart Van Assche

Re: [PATCH v2 1/4] scsi_dh_alua: Do not modify the interval value for retries

2017-05-12 Thread Bart Van Assche 
On Fri, 2017-05-12 at 15:15 +0200, Martin Wilck wrote:
> From: Hannes Reinecke 
> 
> We shouldn't set the interval value to 0 in alua_rtpg_work(), as the struct
> is accessed from different devices and hence we might end up scheduling
> too early.
> 
> With this change, pg->interval is now effectively constant, thus we might
> as well replace it with a contant expression.

Reviewed-by: Bart Van Assche

Re: [PATCH v2 3/4] scsi_dh_alua: do not call BUG_ON when updating port group

2017-05-12 Thread Bart Van Assche 
On Fri, 2017-05-12 at 15:15 +0200, Martin Wilck wrote:
> alua_rtpg() can race with alua_bus_detach(). The assertion that
> alua_dh_data *h->sdev must be non-NULL is not guaranteed because
> alua_bus_detach sets this field to NULL before removing the entry
> from the port group's dh_list.
> 
> This happens when a device is about to be removed, so don't BUG out
> but continue silently.
> 
> Signed-off-by: Martin Wilck 
> Reviewed-by: Hannes Reinecke 
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 20 ++--
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c 
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 2b60f493f90e..a59783020c66 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -652,9 +652,15 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
> alua_port_group *pg)
>   rcu_read_lock();
>   list_for_each_entry_rcu(h,
>   _pg->dh_list, node) {
> - /* h->sdev should always be 
> valid */
> - BUG_ON(!h->sdev);
> - h->sdev->access_state = desc[0];
> + /*
> +  * We might be racing with
> +  * alua_bus_detach here
> +  */
> + struct scsi_device *sdev =
> + h->sdev;
> + if (sdev)
> + sdev->access_state =
> + desc[0];
>   }
>   rcu_read_unlock();
>   }
> @@ -694,11 +700,13 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
> alua_port_group *pg)
>   pg->expiry = 0;
>   rcu_read_lock();
>   list_for_each_entry_rcu(h, >dh_list, node) {
> - BUG_ON(!h->sdev);
> - h->sdev->access_state =
> + struct scsi_device *sdev = h->sdev;
> + if (!sdev)
> + continue;
> + sdev->access_state =
>   (pg->state & SCSI_ACCESS_STATE_MASK);
>   if (pg->pref)
> - h->sdev->access_state |=
> + sdev->access_state |=
>   SCSI_ACCESS_STATE_PREFERRED;
>   }
>   rcu_read_unlock();

Hello Martin,

Allowing races like the one this patch tries to address to exist makes
the ALUA code harder to maintain than necessary. Have you considered to
make alua_bus_detach() wait until ALUA work has finished by using e.g.
cancel_work_sync() or rcu_synchronize()?

Bart.

Re: [PATCH v2 4/4] scsi_dh_alua: take sdev reference in alua_bus_attach

2017-05-12 Thread Bart Van Assche 
On Fri, 2017-05-12 at 15:15 +0200, Martin Wilck wrote:
> Modification of the access_state field in struct scsi_device
> in alua_rtpg() may race with alua_bus_detach(). Avoid
> the scsi_device struct to be freed while it's being processed
> in the alua code by taking a reference.

Hello Martin,

The approach of this patch seems weird to me. I don't think that it is a
good idea to let any ALUA work continue while alua_bus_detach() is in progress.
Have you considered to stop the ALUA work from inside alua_bus_detach() by
using cancel_work_sync()?

Bart.

Re: [REEEEPOST] bnx2i + bnx2fc: convert to generic workqueue (#3)

2017-05-12 Thread Chad Dupuis 

On Tue, 9 May 2017, 11:18am, James Bottomley wrote:

> On Tue, 2017-05-09 at 10:17 -0400, Chad Dupuis wrote:
> > On Mon, 8 May 2017, 10:04pm, Martin K. Petersen wrote:
> > 
> > > 
> > > Sebastian,
> > > 
> > > > Martin, do you see any chance to get this merged? Chad replied to
> > the
> > > > list that he is going to test it on 2017-04-10, didn't respond to
> > the
> > > > ping 10 days later. The series stalled last time in the same way.
> > > 
> > > I am very reluctant to merge something when a driver has an active
> > > maintainer and that person has not acked the change.
> > > 
> > > That said, Chad: You have been sitting on this for quite a while.
> > Please
> > > make it a priority. In exchange for veto rights you do have to
> > provide
> > > timely feedback on anything that touches your driver.
> > > 
> > > Thanks!
> > > 
> > 
> > We did do some testing and hit a calltrace during device discovery:
> > 
> > [ 1332.551799] INFO: task scsi_eh_15:1970 blocked for more than 120 
> > seconds.
> > [ 1332.551804] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
> > disables 
> > this message.
> > [ 1332.551807] scsi_eh_15  D 880823488c14 0  1970  2 
> > 0x0080
> > [ 1332.551813]  881053a17cb0 0046 88084d693ec0 
> > 881053a17fd8
> > [ 1332.551817]  881053a17fd8 881053a17fd8 88084d693ec0 
> > 880823488d48
> > [ 1332.551821]  880823488d50 7fff 88084d693ec0 
> > 880823488c14
> > [ 1332.551825] Call Trace:
> > [ 1332.551838]  [] schedule+0x29/0x70
> > [ 1332.551844]  [] schedule_timeout+0x239/0x2d0
> > [ 1332.551850]  [] ? console_unlock+0x208/0x400
> > [ 1332.551855]  [] ? vprintk_emit+0x3c4/0x510
> > [ 1332.551861]  [] ?
> > lock_timer_base.isra.33+0x2b/0x50
> > [ 1332.551866]  [] wait_for_completion+0x116/0x170
> > [ 1332.551874]  [] ? wake_up_state+0x20/0x20
> > [ 1332.551885]  [] bnx2fc_abts_cleanup+0x3d/0x62 
> > [bnx2fc]
> > [ 1332.551892]  [] bnx2fc_eh_abort+0x470/0x580
> > [bnx2fc]
> > [ 1332.551900]  [] scsi_error_handler+0x59f/0x8b0
> > [ 1332.551904]  [] ? scsi_eh_get_sense+0x250/0x250
> > [ 1332.551911]  [] kthread+0xcf/0xe0
> > [ 1332.551916]  [] ?
> > kthread_create_on_node+0x140/0x140
> > [ 1332.551923]  [] ret_from_fork+0x58/0x90
> > [ 1332.551928]  [] ?
> > kthread_create_on_node+0x140/0x140 
> 
> Reporting this when you found it would have been helpful ...
> 
> That said, it does look like a genuine hang in the workqueues, so it
> rather invalidates the current patch set.
> 
> > To be honest, I'm reluctant to merge these patches on bnx2fc as the
> > I/O path on this driver has been stable for quite some time and given
> > that it's an older driver I'm not looking to make changes there.
> 
> OK, so find a way to achieve both sets of goals because there's a limit
> to how long we allow "stable" drivers to hold up infrastructure changes
> within the kernel.  The main goal of the current patch set is to remove
> the cpu hotplug calls from the drivers because they want to remove them
> from the kernel.  This is rather complex because you're using per cpu
> work queues so you currently have to manage starting and stopping them
> as the CPUs come up or go down ... getting rid of that for standard
> kernel infrastructure will make the driver easier to keep in
> maintenance mode for longer.
> 
> James
> 

Ok, I believe I've found the issue here.  The machine that the test has 
performed on had many more possible CPUs than active CPUs.  We calculate 
which CPU to the work time on in bnx2fc_process_new_cqes() like this:

unsigned int cpu = wqe % num_possible_cpus();

Since not all CPUs are active, we were trying to schedule work on 
non-active CPUs which meant that the upper layers were never notified of 
the completion.  With this change:

diff --git a/drivers/scsi/bnx2fc/bnx2fc_hwi.c 
b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
index c2288d6..6f08e43 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_hwi.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
@@ -1042,7 +1042,12 @@ static int bnx2fc_process_new_cqes(struct 
bnx2fc_rport *tgt)
/* Pending work request completion */
struct bnx2fc_work *work = NULL;
struct bnx2fc_percpu_s *fps = NULL;
-   unsigned int cpu = wqe % num_possible_cpus();
+   unsigned int cpu = wqe % num_active_cpus();
+
+   /* Sanity check cpu to make sure it's online */
+   if (!cpu_active(cpu))
+   /* Default to CPU 0 */
+   cpu = 0;
 
work = bnx2fc_alloc_work(tgt, wqe);
if (work) {

The issue is fixed.

Sebastian, can you add this change to your patch set?

[PATCH 34/36] scsi: fix some kernel-doc markups

2017-05-12 Thread Mauro Carvalho Chehab 
Sphinx is very pedantic with regards to ident/spacing.
Fix some kernel-doc markups in order to solve those
errors/warnings:

./drivers/scsi/scsicam.c:121: WARNING: Inline emphasis start-string without 
end-string.
./drivers/scsi/scsicam.c:121: WARNING: Inline emphasis start-string without 
end-string.
./drivers/scsi/scsicam.c:121: WARNING: Inline emphasis start-string without 
end-string.
./drivers/scsi/scsi_scan.c:1056: ERROR: Unexpected indentation.
./drivers/scsi/scsi_scan.c:1057: WARNING: Block quote ends without a blank 
line; unexpected unindent.
./drivers/scsi/scsi_transport_fc.c:2918: ERROR: Unexpected indentation.
./drivers/scsi/scsi_transport_fc.c:2921: WARNING: Block quote ends without a 
blank line; unexpected unindent.
./drivers/scsi/scsi_transport_fc.c:2922: WARNING: Enumerated list ends without 
a blank line; unexpected unindent.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/scsi/scsi_scan.c |  7 ---
 drivers/scsi/scsi_transport_fc.c | 18 ++
 drivers/scsi/scsicam.c   |  4 ++--
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 6f7128f49c30..69979574004f 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1051,10 +1051,11 @@ static unsigned char *scsi_inq_str(unsigned char *buf, 
unsigned char *inq,
  * allocate and set it up by calling scsi_add_lun.
  *
  * Return:
- * SCSI_SCAN_NO_RESPONSE: could not allocate or setup a scsi_device
- * SCSI_SCAN_TARGET_PRESENT: target responded, but no device is
+ *
+ *   - SCSI_SCAN_NO_RESPONSE: could not allocate or setup a scsi_device
+ *   - SCSI_SCAN_TARGET_PRESENT: target responded, but no device is
  * attached at the LUN
- * SCSI_SCAN_LUN_PRESENT: a new scsi_device was allocated and initialized
+ *   - SCSI_SCAN_LUN_PRESENT: a new scsi_device was allocated and initialized
  **/
 static int scsi_probe_and_add_lun(struct scsi_target *starget,
  u64 lun, int *bflagsp,
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index d4cf32d55546..1df77453f6b6 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -2914,16 +2914,18 @@ EXPORT_SYMBOL(fc_remote_port_add);
  * port is no longer part of the topology. Note: Although a port
  * may no longer be part of the topology, it may persist in the remote
  * ports displayed by the fc_host. We do this under 2 conditions:
+ *
  * 1) If the port was a scsi target, we delay its deletion by "blocking" it.
- *   This allows the port to temporarily disappear, then reappear without
- *   disrupting the SCSI device tree attached to it. During the "blocked"
- *   period the port will still exist.
+ *This allows the port to temporarily disappear, then reappear without
+ *disrupting the SCSI device tree attached to it. During the "blocked"
+ *period the port will still exist.
+ *
  * 2) If the port was a scsi target and disappears for longer than we
- *   expect, we'll delete the port and the tear down the SCSI device tree
- *   attached to it. However, we want to semi-persist the target id assigned
- *   to that port if it eventually does exist. The port structure will
- *   remain (although with minimal information) so that the target id
- *   bindings remails.
+ *expect, we'll delete the port and the tear down the SCSI device tree
+ *attached to it. However, we want to semi-persist the target id assigned
+ *to that port if it eventually does exist. The port structure will
+ *remain (although with minimal information) so that the target id
+ *bindings remails.
  *
  * If the remote port is not an FCP Target, it will be fully torn down
  * and deallocated, including the fc_remote_port class device.
diff --git a/drivers/scsi/scsicam.c b/drivers/scsi/scsicam.c
index 910f4a7a3924..31273468589c 100644
--- a/drivers/scsi/scsicam.c
+++ b/drivers/scsi/scsicam.c
@@ -116,8 +116,8 @@ EXPORT_SYMBOL(scsicam_bios_param);
  * @hds: put heads here
  * @secs: put sectors here
  *
- * Description: determine the BIOS mapping/geometry used to create the 
partition
- *  table, storing the results in *cyls, *hds, and *secs 
+ * Determine the BIOS mapping/geometry used to create the partition
+ * table, storing the results in @cyls, @hds, and @secs
  *
  * Returns: -1 on failure, 0 on success.
  */
-- 
2.9.3

[PATCH v2 3/4] scsi_dh_alua: do not call BUG_ON when updating port group

2017-05-12 Thread Martin Wilck 
alua_rtpg() can race with alua_bus_detach(). The assertion that
alua_dh_data *h->sdev must be non-NULL is not guaranteed because
alua_bus_detach sets this field to NULL before removing the entry
from the port group's dh_list.

This happens when a device is about to be removed, so don't BUG out
but continue silently.

Signed-off-by: Martin Wilck 
Reviewed-by: Hannes Reinecke 
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c 
b/drivers/scsi/device_handler/scsi_dh_alua.c
index 2b60f493f90e..a59783020c66 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -652,9 +652,15 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
rcu_read_lock();
list_for_each_entry_rcu(h,
_pg->dh_list, node) {
-   /* h->sdev should always be 
valid */
-   BUG_ON(!h->sdev);
-   h->sdev->access_state = desc[0];
+   /*
+* We might be racing with
+* alua_bus_detach here
+*/
+   struct scsi_device *sdev =
+   h->sdev;
+   if (sdev)
+   sdev->access_state =
+   desc[0];
}
rcu_read_unlock();
}
@@ -694,11 +700,13 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
pg->expiry = 0;
rcu_read_lock();
list_for_each_entry_rcu(h, >dh_list, node) {
-   BUG_ON(!h->sdev);
-   h->sdev->access_state =
+   struct scsi_device *sdev = h->sdev;
+   if (!sdev)
+   continue;
+   sdev->access_state =
(pg->state & SCSI_ACCESS_STATE_MASK);
if (pg->pref)
-   h->sdev->access_state |=
+   sdev->access_state |=
SCSI_ACCESS_STATE_PREFERRED;
}
rcu_read_unlock();
-- 
2.12.2

[PATCH v2 4/4] scsi_dh_alua: take sdev reference in alua_bus_attach

2017-05-12 Thread Martin Wilck 
Modification of the access_state field in struct scsi_device
in alua_rtpg() may race with alua_bus_detach(). Avoid
the scsi_device struct to be freed while it's being processed
in the alua code by taking a reference.

Signed-off-by: Martin Wilck 
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c 
b/drivers/scsi/device_handler/scsi_dh_alua.c
index a59783020c66..4b33e076d4f7 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -1108,6 +1108,10 @@ static int alua_bus_attach(struct scsi_device *sdev)
h = kzalloc(sizeof(*h) , GFP_KERNEL);
if (!h)
return -ENOMEM;
+   if (scsi_device_get(sdev)) {
+   ret = -ENXIO;
+   goto failed;
+   }
spin_lock_init(>pg_lock);
rcu_assign_pointer(h->pg, NULL);
h->init_error = SCSI_DH_OK;
@@ -1119,10 +1123,13 @@ static int alua_bus_attach(struct scsi_device *sdev)
if (err == SCSI_DH_NOMEM)
ret = -ENOMEM;
if (err != SCSI_DH_OK && err != SCSI_DH_DEV_OFFLINED)
-   goto failed;
+   goto failed_put;
 
sdev->handler_data = h;
return 0;
+
+failed_put:
+   scsi_device_put(sdev);
 failed:
kfree(h);
return ret;
@@ -1140,6 +1147,7 @@ static void alua_bus_detach(struct scsi_device *sdev)
spin_lock(>pg_lock);
pg = rcu_dereference_protected(h->pg, lockdep_is_held(>pg_lock));
rcu_assign_pointer(h->pg, NULL);
+   scsi_device_put(h->sdev);
h->sdev = NULL;
spin_unlock(>pg_lock);
if (pg) {
-- 
2.12.2

[PATCH v2 1/4] scsi_dh_alua: Do not modify the interval value for retries

2017-05-12 Thread Martin Wilck 
From: Hannes Reinecke 

We shouldn't set the interval value to 0 in alua_rtpg_work(), as the struct
is accessed from different devices and hence we might end up scheduling
too early.

With this change, pg->interval is now effectively constant, thus we might
as well replace it with a contant expression.

Signed-off-by: Hannes Reinecke 
Signed-off-by: Martin Wilck 
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c 
b/drivers/scsi/device_handler/scsi_dh_alua.c
index c01b47e5b55a..f17dccb2f72b 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -53,6 +53,7 @@
 #define ALUA_FAILOVER_TIMEOUT  60
 #define ALUA_FAILOVER_RETRIES  5
 #define ALUA_RTPG_DELAY_MSECS  5
+#define ALUA_RTPG_RETRY_INTERVAL   (2*HZ)
 
 /* device handler flags */
 #define ALUA_OPTIMIZE_STPG 0x01
@@ -86,7 +87,6 @@ struct alua_port_group {
unsignedflags; /* used for optimizing STPG */
unsigned char   transition_tmo;
unsigned long   expiry;
-   unsigned long   interval;
struct delayed_work rtpg_work;
spinlock_t  lock;
struct list_headrtpg_list;
@@ -681,7 +681,6 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
case SCSI_ACCESS_STATE_TRANSITIONING:
if (time_before(jiffies, pg->expiry)) {
/* State transition, retry */
-   pg->interval = 2;
err = SCSI_DH_RETRY;
} else {
struct alua_dh_data *h;
@@ -811,7 +810,7 @@ static void alua_rtpg_work(struct work_struct *work)
pg->flags |= ALUA_PG_RUN_RTPG;
spin_unlock_irqrestore(>lock, flags);
queue_delayed_work(alua_wq, >rtpg_work,
-  pg->interval * HZ);
+  ALUA_RTPG_RETRY_INTERVAL);
return;
}
/* Send RTPG on failure or if TUR indicates SUCCESS */
@@ -823,7 +822,7 @@ static void alua_rtpg_work(struct work_struct *work)
pg->flags |= ALUA_PG_RUN_RTPG;
spin_unlock_irqrestore(>lock, flags);
queue_delayed_work(alua_wq, >rtpg_work,
-  pg->interval * HZ);
+  ALUA_RTPG_RETRY_INTERVAL);
return;
}
if (err != SCSI_DH_OK)
@@ -836,11 +835,9 @@ static void alua_rtpg_work(struct work_struct *work)
spin_lock_irqsave(>lock, flags);
if (err == SCSI_DH_RETRY || pg->flags & ALUA_PG_RUN_RTPG) {
pg->flags |= ALUA_PG_RUN_RTPG;
-   pg->interval = 0;
pg->flags &= ~ALUA_PG_RUNNING;
spin_unlock_irqrestore(>lock, flags);
-   queue_delayed_work(alua_wq, >rtpg_work,
-  pg->interval * HZ);
+   queue_delayed_work(alua_wq, >rtpg_work, 0);
return;
}
}
@@ -886,7 +883,6 @@ static bool alua_rtpg_queue(struct alua_port_group *pg,
force = true;
}
if (pg->rtpg_sdev == NULL) {
-   pg->interval = 0;
pg->flags |= ALUA_PG_RUN_RTPG;
kref_get(>kref);
pg->rtpg_sdev = sdev;
-- 
2.12.2

[PATCH v2 0/4] failover fixes for scsi_dh_alua

2017-05-12 Thread Martin Wilck 
Here are 4 fixes we came up with at SUSE to fix problems we encountered
in multipath failover tests. Feedback welcome.

Best regards,
Martin

Changes wrt v1:

- Use a constant value for the retry interval in patch 1/4
- Style fix in patch 2/4
- Safer coding in patch 3/4 as suggested by Bart
- Protect against sdev being released, as suggested by Bart (added patch 4/4)

Hannes Reinecke (2):
  scsi_dh_alua: Do not modify the interval value for retries
  scsi_dh_alua: Do not retry for unmapped device

Martin Wilck (2):
  scsi_dh_alua: do not call BUG_ON when updating port group
  scsi_dh_alua: take sdev reference in alua_bus_attach

 drivers/scsi/device_handler/scsi_dh_alua.c | 51 +++---
 1 file changed, 33 insertions(+), 18 deletions(-)

-- 
2.12.2

[PATCH v2 2/4] scsi_dh_alua: Do not retry for unmapped device

2017-05-12 Thread Martin Wilck 
From: Hannes Reinecke 

If a device becomes unmapped on the target we should be returning
SCSI_DH_DEV_OFFLINED.

Signed-off-by: Hannes Reinecke 
Reviewed-by: Martin Wilck 
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c 
b/drivers/scsi/device_handler/scsi_dh_alua.c
index f17dccb2f72b..2b60f493f90e 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -522,7 +522,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
struct alua_port_group *tmp_pg;
int len, k, off, valid_states = 0, bufflen = ALUA_RTPG_SIZE;
unsigned char *desc, *buff;
-   unsigned err, retval;
+   unsigned err = SCSI_DH_OK, retval;
unsigned int tpg_desc_tbl_off;
unsigned char orig_transition_tmo;
unsigned long flags;
@@ -541,7 +541,6 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
return SCSI_DH_DEV_TEMP_BUSY;
 
  retry:
-   err = 0;
retval = submit_rtpg(sdev, buff, bufflen, _hdr, pg->flags);
 
if (retval) {
@@ -569,6 +568,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
pg->flags |= ALUA_RTPG_EXT_HDR_UNSUPP;
goto retry;
}
+   err = SCSI_DH_IO;
/*
 * Retry on ALUA state transition or if any
 * UNIT ATTENTION occurred.
@@ -576,6 +576,9 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
if (sense_hdr.sense_key == NOT_READY &&
sense_hdr.asc == 0x04 && sense_hdr.ascq == 0x0a)
err = SCSI_DH_RETRY;
+   else if (sense_hdr.sense_key == ILLEGAL_REQUEST &&
+   sense_hdr.asc == 0x25 && sense_hdr.ascq == 0x00)
+   err = SCSI_DH_DEV_OFFLINED;
else if (sense_hdr.sense_key == UNIT_ATTENTION)
err = SCSI_DH_RETRY;
if (err == SCSI_DH_RETRY &&
@@ -591,7 +594,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
scsi_print_sense_hdr(sdev, ALUA_DH_NAME, _hdr);
kfree(buff);
pg->expiry = 0;
-   return SCSI_DH_IO;
+   return err;
}
 
len = get_unaligned_be32([0]) + 4;
-- 
2.12.2

Re: [PATCH] scsi: libfc: fix incorrect variable assingment

2017-05-12 Thread Ewan D. Milne 
On Thu, 2017-05-11 at 17:24 -0500, Gustavo A. R. Silva wrote:
> Previous assignment was causing the use of the uninitialized variable
> _explan_ inside fc_seq_ls_rjt() function, which in this particular
> case is being called by fc_seq_els_rsp_send().
> 
> Addresses-Coverity-ID: 1398125
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/scsi/libfc/fc_rport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
> index b44c313..5203258 100644
> --- a/drivers/scsi/libfc/fc_rport.c
> +++ b/drivers/scsi/libfc/fc_rport.c
> @@ -1422,7 +1422,7 @@ static void fc_rport_recv_rtv_req(struct fc_rport_priv 
> *rdata,
>   fp = fc_frame_alloc(lport, sizeof(*rtv));
>   if (!fp) {
>   rjt_data.reason = ELS_RJT_UNAB;
> - rjt_data.reason = ELS_EXPL_INSUF_RES;
> + rjt_data.explan = ELS_EXPL_INSUF_RES;
>   fc_seq_els_rsp_send(in_fp, ELS_LS_RJT, _data);
>   goto drop;
>   }

s/assingment/assignment/

Reviewed-by: Ewan D. Milne

APPLY FOR YOUR URGENT LOAN AT 2%

2017-05-12 Thread Thomas Credit Firm 
* Do you need an urgent loan? We offer all kinds of loan
* Very fast and urgent transfer to your bank account.
* Payment begins (6) months after you get the money in your bank account
* Low interest rates 2%
* Payment of long-term (1-30 years) duration
For more information and loan application form through the Contact now

E-mail:  thomascreditf...@gmail.com
Thomas B. Dawson
THOMAS CREDIT FIRM

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

[PATCH] scsi: ufs: Clean up some rpm/spm level SysFS nodes upon remove

2017-05-12 Thread Michal Potomski 
From: Michał Potomski 

When reloading module these two attributes aren't
cleaned up properly and they persist causing warnings
when trying to load module again. Additionally they are
not recreated properly due to that.

Signed-off-by: Michał Potomski 
---
 drivers/scsi/ufs/ufshcd.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index abc7e87..ffe8d86 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7698,6 +7698,12 @@ static inline void ufshcd_add_sysfs_nodes(struct ufs_hba 
*hba)
ufshcd_add_spm_lvl_sysfs_nodes(hba);
 }
 
+static inline void ufshcd_remove_sysfs_nodes(struct ufs_hba *hba)
+{
+   device_remove_file(hba->dev, >rpm_lvl_attr);
+   device_remove_file(hba->dev, >spm_lvl_attr);
+}
+
 /**
  * ufshcd_shutdown - shutdown routine
  * @hba: per adapter instance
@@ -7735,6 +7741,7 @@ int ufshcd_shutdown(struct ufs_hba *hba)
  */
 void ufshcd_remove(struct ufs_hba *hba)
 {
+   ufshcd_remove_sysfs_nodes(hba);
scsi_remove_host(hba->host);
/* disable interrupts */
ufshcd_disable_intr(hba, hba->intr_mask);
-- 
1.9.1



Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial 
Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | 
Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i 
moze zawierac informacje poufne. W razie przypadkowego otrzymania tej 
wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; 
jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole 
use of the intended recipient(s). If you are not the intended recipient, please 
contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.