Re: [PATCH 4/5] target: user: Fix sense data handling

[Damien Le Moal]

Mike,

On 7/11/17 02:26, Mike Christie wrote:
> On 07/10/2017 12:36 AM, Damien Le Moal wrote:
>> Nicholas, Mike,
>>
>> On 7/7/17 15:05, Nicholas A. Bellinger wrote:
>>> Everything including MNC's #1-6 and your #1-2 be pushed to
>>> target-pending/for-next shortly.
>>>
>>> Please use this as your base for testing.  :)
>>
>> I ran tests this morning with the latest target-pending/for-next branch.
>> I ran libzbc test suite on top of 4 different configurations:
>>
>> 1) ZBC drive + pscsi + loopback -> OK, no problems.
>> 2) ZBC drive + pscsi + iscsi -> OK, no problems.
>> 3) ZBC emulation tcmu-runner handler + loopback -> OK, no problems.
>> 4) ZBC emulation tcmu-runner handler + iscsi -> Crash !
>>
>> Here is the oops for case (4):
>>
>> [  169.545459] scsi host7: iSCSI Initiator over TCP/IP
>> [  169.559013] scsi 7:0:0:0: Direct-Access-ZBC LIO-ORG  TCMU ZBC device
>> 0002 PQ: 0 ANSI: 5
>> [  169.576920] sd 7:0:0:0: Attached scsi generic sg9 type 20
>> [  169.577209] sd 7:0:0:0: [sdi] Host-managed zoned block device
>> [  169.577794] sd 7:0:0:0: [sdi] 20971520 512-byte logical blocks: (10.7
>> GB/10.0 GiB)
>> [  169.577796] sd 7:0:0:0: [sdi] 40 zones of 524288 logical blocks
>> [  169.577980] sd 7:0:0:0: [sdi] Write Protect is off
>> [  169.578329] sd 7:0:0:0: [sdi] Write cache: enabled, read cache:
>> enabled, doesn't support DPO or FUA
>> [  169.590379] sd 7:0:0:0: [sdi] Attached SCSI disk
>> [  240.071464] BUG: unable to handle kernel paging request at
>> c9065db85540
>> [  240.078460] IP: memcpy_erms+0x6/0x10
>> [  240.082044] PGD 7ff0ba067
>> [  240.082045] P4D 7ff0ba067
>> [  240.084766] PUD 0
>> [  240.087486]
>> [  240.091006] Oops: 0002 [#1] PREEMPT SMP
>> [  240.094855] Modules linked in: ip6table_filter ip6_tables
>> rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache
>> iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi sunrpc
>> snd_hda_codec_hdmi snd_hda_codec_generic snd_hda_intel snd_hda_codec
>> snd_hwdep snd_hda_core snd_seq snd_seq_device x86_pkg_temp_thermal
>> coretemp snd_pcm crc32_pclmul snd_timer iTCO_wdt snd i2c_i801
>> iTCO_vendor_support soundcore i915 iosf_mbi i2c_algo_bit drm_kms_helper
>> syscopyarea sysfillrect sysimgblt fb_sys_fops drm e1000e r8169 mpt3sas
>> mii i2c_core raid_class video
>> [  240.143969] CPU: 0 PID: 1285 Comm: iscsi_trx Not tainted 4.12.0-rc1+ #3
>> [  240.150607] Hardware name: ASUS All Series/H87-PRO, BIOS 2104 10/28/2014
>> [  240.157331] task: 8807de4f5800 task.stack: c900047dc000
>> [  240.163270] RIP: 0010:memcpy_erms+0x6/0x10
>> [  240.167377] RSP: 0018:c900047dfc68 EFLAGS: 00010202
>> [  240.172621] RAX: c9065db85540 RBX: 8807f798 RCX:
>> 0010
>> [  240.179771] RDX: 0010 RSI: 8807de574fe0 RDI:
>> c9065db85540
>> [  240.186930] RBP: c900047dfd30 R08: 8807de41b000 R09:
>> 
>> [  240.194088] R10: 0040 R11: 8807e9b726f0 R12:
>> 0006565726b0
>> [  240.201246] R13: c90007612ea0 R14: 00065657d540 R15:
>> 
>> [  240.208397] FS:  () GS:88081fa0()
>> knlGS:
>> [  240.216510] CS:  0010 DS:  ES:  CR0: 80050033
>> [  240.80] CR2: c9065db85540 CR3: 01c0f000 CR4:
>> 001406f0
>> [  240.229430] Call Trace:
>> [  240.231887]  ? tcmu_queue_cmd+0x83c/0xa80
>> [  240.235916]  ? target_check_reservation+0xcd/0x6f0
>> [  240.240725]  __target_execute_cmd+0x27/0xa0
>> [  240.244918]  target_execute_cmd+0x232/0x2c0
>> [  240.249124]  ? __local_bh_enable_ip+0x64/0xa0
>> [  240.253499]  iscsit_execute_cmd+0x20d/0x270
>> [  240.257693]  iscsit_sequence_cmd+0x110/0x190
>> [  240.261985]  iscsit_get_rx_pdu+0x360/0xc80
>> [  240.267565]  ? iscsi_target_rx_thread+0x54/0xd0
>> [  240.273571]  iscsi_target_rx_thread+0x9a/0xd0
>> [  240.279413]  kthread+0x113/0x150
>> [  240.284120]  ? iscsi_target_tx_thread+0x1e0/0x1e0
>> [  240.290297]  ? kthread_create_on_node+0x40/0x40
>> [  240.296297]  ret_from_fork+0x2e/0x40
>> [  240.301332] Code: 90 90 90 90 90 eb 1e 0f 1f 00 48 89 f8 48 89 d1 48
>> c1 e9 03 83 e2 07 f3 48 a5 89 d1 f3 a4 c3 66 0f 1f 44 00 00 48 89 f8 48
>> 89 d1  a4 c3 0f 1f 80 00 00 00 00 48 89 f8 48 83 fa 20 72 7e 40 38
>> [  240.321751] RIP: memcpy_erms+0x6/0x10 RSP: c900047dfc68
>> [  240.328838] CR2: c9065db85540
>> [  240.333667] ---[ end trace b7e5354cfb54d08b ]---
>>
>> I went back to running my initial 5 patch series on top of the current
>> 4.12 kernel and everything is fine, including case (4).
>>
>> A diff of the 2 versions of drivers/target/target_core_user.c did not
>> reveal anything obvious that could result in this... It does look like a
>> race condition on the session command or some memory corruption/bad
>> pointer. Any idea ?
>>
> 
> I have not seen this crash before. You are running these tests:
> 
> https://github.com/hgst/libzbc/tree/master/test
> 
> right?

Yes.

> What test was it? If you need a device that 

Re: [PATCH v2 0/4] scsi_dh_alua: fix stuck I/O after unavailable/standby states

[Mauricio Faria de Oliveira]

On 07/10/2017 07:47 PM, Mauricio Faria de Oliveira wrote:

This patchset addresses that problem, and adds a few improvements
to the logging of PG state changes.


Here are some kernel log snippets with the patchset, if that helps.

The 2 port groups temporarily gone into unavailable state, and
recovered to active states later on, when I/O has been restored.

The devices, port groups, and relative port IDs:
2 LUNs, 2 targets per LUN, 2 hosts (8 paths total).

[  294.482107] scsi 4:0:0:0: alua: device 
naa.60050764008100dac104 port group 1 rel port 881
[  294.642063] scsi 4:0:0:1: alua: device 
naa.60050764008100dac105 port group 0 rel port 881
[  294.812140] scsi 4:0:1:0: alua: device 
naa.60050764008100dac104 port group 0 rel port 81
[  295.002027] scsi 4:0:1:1: alua: device 
naa.60050764008100dac105 port group 1 rel port 81
[  296.952092] scsi 5:0:0:0: alua: device 
naa.60050764008100dac104 port group 1 rel port 881
[  297.162043] scsi 5:0:0:1: alua: device 
naa.60050764008100dac105 port group 0 rel port 881
[  297.312130] scsi 5:0:1:0: alua: device 
naa.60050764008100dac104 port group 0 rel port 81
[  297.512006] scsi 5:0:1:1: alua: device 
naa.60050764008100dac105 port group 1 rel port 81


First, mpatha goes down:

mpatha (360050764008100dac104) dm-6 IBM ,2145
size=40G features='1 queue_if_no_path' hwhandler='1 alua' wp=rw
|-+- policy='round-robin 0' prio=0 status=active
| |- 4:0:1:0 sde 8:64  active undef unknown
| `- 5:0:1:0 sdi 8:128 active undef unknown
`-+- policy='round-robin 0' prio=0 status=enabled
  |- 4:0:0:0 sdc 8:32  active undef unknown
  `- 5:0:0:0 sdg 8:96  active undef unknown

[258929.940734] device-mapper: multipath: Failing path 8:64.
[258929.940751] device-mapper: multipath: Failing path 8:128.

[258929.940777] sd 4:0:0:0: alua: alua_activate(): port group 01, fn: 
pg_init_done [dm_multipath]()
[258929.940784] sd 5:0:0:0: alua: alua_activate(): port group 01, fn: 
pg_init_done [dm_multipath]()


[258929.941575] sd 5:0:1:0: alua: alua_check_sense(): Sense Key: 0x2, 
ASC: 0x4, ASCQ: 0xc
[258929.941578] sd 5:0:1:0: [sdi] tag#1 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE

[258929.941874] sd 5:0:1:0: [sdi] tag#1 Sense Key : Not Ready [current]
[258929.941928] sd 5:0:1:0: [sdi] tag#1 Add. Sense: Logical unit not 
accessible, target port in unavailable state


[258929.942096] sd 4:0:1:0: alua: alua_check_sense(): Sense Key: 0x2, 
ASC: 0x4, ASCQ: 0xc
[258929.942104] sd 4:0:1:0: [sde] tag#2 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE

[258929.942300] sd 4:0:1:0: [sde] tag#2 Sense Key : Not Ready [current]
[258929.942354] sd 4:0:1:0: [sde] tag#2 Add. Sense: Logical unit not 
accessible, target port in unavailable state


[258929.956347] sd 4:0:1:0: alua: port group 00 state U non-preferred 
supports tolusna

[258929.956389] sd 4:0:1:0: alua: port group 01 state U non-preferred

[258930.036355] sd 5:0:0:0: alua: alua_check_sense(): Sense Key: 0x2, 
ASC: 0x4, ASCQ: 0xc

[258930.036360] sd 5:0:0:0: alua: alua_rtpg_queue(): port group 01
[258930.036365] device-mapper: multipath: Failing path 8:96.

[258930.036444] sd 4:0:0:0: alua: alua_check_sense(): Sense Key: 0x2, 
ASC: 0x4, ASCQ: 0xc

[258930.036506] device-mapper: multipath: Failing path 8:32.

Then, mpathb goes down:

mpathb (360050764008100dac105) dm-7 IBM ,2145
size=40G features='1 queue_if_no_path' hwhandler='1 alua' wp=rw
|-+- policy='round-robin 0' prio=0 status=active
| |- 4:0:0:1 sdd 8:48  active undef unknown
| `- 5:0:0:1 sdh 8:112 active undef unknown
`-+- policy='round-robin 0' prio=0 status=enabled
  |- 4:0:1:1 sdf 8:80  active undef unknown
  `- 5:0:1:1 sdj 8:144 active undef unknown


[258930.162558] sd 4:0:0:1: alua: alua_check_sense(): Sense Key: 0x2, 
ASC: 0x4, ASCQ: 0xc

[258930.162568] sd 4:0:0:1: alua: alua_rtpg_queue(): port group 00

[258930.162572] sd 4:0:0:1: [sdd] tag#9 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE

[258930.162731] sd 4:0:0:1: [sdd] tag#9 Sense Key : Not Ready [current]
[258930.162762] sd 4:0:0:1: [sdd] tag#9 Add. Sense: Logical unit not 
accessible, target port in unavailable state

[258930.162860] device-mapper: multipath: Failing path 8:48.

[258930.162979] sd 5:0:0:1: [sdh] tag#1 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE

[258930.162980] sd 5:0:0:1: [sdh] tag#1 Sense Key : Not Ready [current]
[258930.162982] sd 5:0:0:1: [sdh] tag#1 Add. Sense: Logical unit not 
accessible, target port in unavailable state

[258930.162992] device-mapper: multipath: Failing path 8:112.

[258930.163025] sd 4:0:1:1: alua: alua_activate(): port group 01, fn: 
pg_init_done [dm_multipath]()

[258930.163029] sd 4:0:1:1: alua: alua_rtpg_queue(): port group 01
[258930.163048] sd 5:0:1:1: alua: alua_activate(): port group 01, fn: 
pg_init_done [dm_multipath]()


[258930.176323] sd 4:0:0:1: alua: port group 00 state U non-preferred 
supports tolusna

[PATCH v2 2/4] scsi: scsi_dh_alua: print changes to RTPG state of all PGs

[Mauricio Faria de Oliveira]

Currently, alua_rtpg() can change the 'state' and 'preferred'
values for the current port group _and_ of other port groups.

However, it reports that _only_ for the current port group.

This might cause uncertainty and confusion when going through
the kernel logs for analyzing/debugging scsi_dh_alua behavior,
which is not helpful during support and development scenarios.

So, print of such changes for all port groups, when it occurs.

It's possible to distinguish between the messages printed for
the current and other PGs by the 'supports' (supported states)
part, which is only printed for the current PG.

Signed-off-by: Mauricio Faria de Oliveira 

---
v2:
- use lockdep_assert_held() instead of documenting locking conventions
  (Bart Van Assche )
- define two functions (with/without supported states information)
  (Bart Van Assche )
- simplify which device is used for printing the messages
  (use the evaluated scsi device and tell which PG the info is for)
- call the print functions from nearby places
  (right after modifying the PG data)

 drivers/scsi/device_handler/scsi_dh_alua.c | 63 +++---
 1 file changed, 49 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c 
b/drivers/scsi/device_handler/scsi_dh_alua.c
index a1cf3d6aa853..937341ddb767 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -529,6 +529,49 @@ static int alua_tur(struct scsi_device *sdev)
 }
 
 /*
+ * alua_rtpg_print - Print REPORT TARGET GROUP STATES information
+ *  (without supported states)
+ * @sdev: the device evaluated (source of information).
+ * @pg: the port group associated with the information.
+ */
+static void alua_rtpg_print(struct scsi_device *sdev,
+   struct alua_port_group *pg)
+{
+   lockdep_assert_held(>lock);
+
+   sdev_printk(KERN_INFO, sdev,
+   "%s: port group %02x state %c %s\n",
+   ALUA_DH_NAME, pg->group_id, print_alua_state(pg->state),
+   pg->pref ? "preferred" : "non-preferred");
+}
+
+/*
+ * alua_rtpg_print_supported - Print REPORT TARGET GROUP STATES information
+ *(with supported states)
+ * @sdev: the device evaluated (source of information).
+ * @pg: the port group associated with the information.
+ * @supported_states: the supported states information.
+ */
+static void alua_rtpg_print_supported(struct scsi_device *sdev,
+ struct alua_port_group *pg,
+ int supported_states)
+{
+   lockdep_assert_held(>lock);
+
+   sdev_printk(KERN_INFO, sdev,
+   "%s: port group %02x state %c %s supports %c%c%c%c%c%c%c\n",
+   ALUA_DH_NAME, pg->group_id, print_alua_state(pg->state),
+   pg->pref ? "preferred" : "non-preferred",
+   supported_states & TPGS_SUPPORT_TRANSITION? 'T' : 't',
+   supported_states & TPGS_SUPPORT_OFFLINE   ? 'O' : 'o',
+   supported_states & TPGS_SUPPORT_LBA_DEPENDENT ? 'L' : 'l',
+   supported_states & TPGS_SUPPORT_UNAVAILABLE   ? 'U' : 'u',
+   supported_states & TPGS_SUPPORT_STANDBY   ? 'S' : 's',
+   supported_states & TPGS_SUPPORT_NONOPTIMIZED  ? 'N' : 'n',
+   supported_states & TPGS_SUPPORT_OPTIMIZED ? 'A' : 'a');
+}
+
+/*
  * alua_rtpg - Evaluate REPORT TARGET GROUP STATES
  * @sdev: the device to be evaluated.
  *
@@ -540,7 +583,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
 {
struct scsi_sense_hdr sense_hdr;
struct alua_port_group *tmp_pg;
-   int len, k, off, valid_states = 0, bufflen = ALUA_RTPG_SIZE;
+   int len, k, off, bufflen = ALUA_RTPG_SIZE;
unsigned char *desc, *buff;
unsigned err, retval;
unsigned int tpg_desc_tbl_off;
@@ -674,9 +717,12 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
h->sdev->access_state = desc[0];
}
rcu_read_unlock();
+
+   if (tmp_pg == pg)
+   alua_rtpg_print_supported(sdev, 
tmp_pg, desc[1]);
+   else
+   alua_rtpg_print(sdev, tmp_pg);
}
-   if (tmp_pg == pg)
-   valid_states = desc[1];
spin_unlock_irqrestore(_pg->lock, flags);
}
kref_put(_pg->kref, release_port_group);
@@ -685,17 +731,6 @@ static int 

[PATCH v2 1/4] scsi: scsi_dh_alua: allow I/O in target port unavailable and standby states

[Mauricio Faria de Oliveira]

According to SPC-4 (5.15.2.4.5 Unavailable state), the unavailable
state may (or may not) transition to other states (e.g., microcode
downloading or hardware error, which may be temporary or permanent).

But, scsi_dh_alua currently fails I/O requests early on once that
state occurs (in alua_prep_fn()) preventing path checkers in such
function path to actually check if I/O still fails or now works.

And that prevents a path activation (alua_activate()) which could
update the PG state if it eventually recovered to an active state,
thus resume I/O. (This is also the case with the standby state.)

This might cause device-mapper multipath to fail all paths to some
storage system that moves the controllers to the unavailable state
for firmware upgrades, and never recover regardless of the storage
system doing upgrades one controller at a time and get them online.

Then I/O requests are blocked indefinitely due to queue_if_no_path
but the underlying individual paths are fully operational, and can
be verified as such through other function paths (e.g., SG_IO):

# multipath -l
mpatha (360050764008100dac100) dm-0 IBM,2145
size=40G features='2 queue_if_no_path retain_attached_hw_handler'
hwhandler='1 alua' wp=rw
|-+- policy='service-time 0' prio=0 status=enabled
| |- 1:0:1:0 sdf 8:80  failed undef running
| `- 2:0:1:0 sdn 8:208 failed undef running
`-+- policy='service-time 0' prio=0 status=enabled
  |- 1:0:0:0 sdb 8:16  failed undef running
  `- 2:0:0:0 sdj 8:144 failed undef running

# strace -e read \
sg_dd blk_sgio=0 \
if=/dev/sdj of=/dev/null bs=512 count=1 iflag=direct \
2>&1 | grep 512
read(3, 0x3fff7ba8, 512) = -1 EIO (Input/output error)

# strace -e ioctl \
sg_dd blk_sgio=1 \
if=/dev/sdj of=/dev/null bs=512 count=1 iflag=direct \
2>&1 | grep 512
ioctl(3, SG_IO, {'S', SG_DXFER_FROM_DEV, cmd[10]=[28, 00, 00, 00,
00, 00, 00, 00, 01, 00], <...>) = 0

So, allow I/O to paths of PGs in unavailable/standby state, so path
checkers can actually check them.

Also, schedule a recheck when unavailable/standby state is detected
(in alua_check_sense()) to update pg->state, and quiet further SCSI
error messages (in alua_prep_fn()).

Once a path checker eventually detects a working/active state again,
the PG state is normally updated on path activation (alua_activate(),
as it schedules a recheck), thus I/O requests are no longer failed.

Signed-off-by: Mauricio Faria de Oliveira 
Reported-by: Naresh Bannoth 

---
v2:
- also add support for standby state to alua_check_sense(), alua_prep_fn()
  (Bart Van Assche )

 drivers/scsi/device_handler/scsi_dh_alua.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c 
b/drivers/scsi/device_handler/scsi_dh_alua.c
index c01b47e5b55a..a1cf3d6aa853 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -431,6 +431,26 @@ static int alua_check_sense(struct scsi_device *sdev,
alua_check(sdev, false);
return NEEDS_RETRY;
}
+   if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0b) {
+   /*
+* LUN Not Accessible - target port in standby state.
+*
+* Do not retry, so failover to another target port 
occur.
+* Schedule a recheck to update state for other 
functions.
+*/
+   alua_check(sdev, true);
+   return SUCCESS;
+   }
+   if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0c) {
+   /*
+* LUN Not Accessible - target port in unavailable 
state.
+*
+* Do not retry, so failover to another target port 
occur.
+* Schedule a recheck to update state for other 
functions.
+*/
+   alua_check(sdev, true);
+   return SUCCESS;
+   }
break;
case UNIT_ATTENTION:
if (sense_hdr->asc == 0x29 && sense_hdr->ascq == 0x00) {
@@ -1057,6 +1077,8 @@ static void alua_check(struct scsi_device *sdev, bool 
force)
  *
  * Fail I/O to all paths not in state
  * active/optimized or active/non-optimized.
+ * Allow I/O to paths in state unavailable/standby
+ * so path checkers can actually check them.
  */
 static int alua_prep_fn(struct scsi_device *sdev, struct request *req)
 {
@@ -1072,6 +1094,9 @@ static int alua_prep_fn(struct scsi_device *sdev, struct 
request *req)
rcu_read_unlock();
if (state == SCSI_ACCESS_STATE_TRANSITIONING)
ret = BLKPREP_DEFER;
+   else 

[PATCH v2 3/4] scsi: scsi_dh_alua: do not print RTPG state if it remains unavailable/standby

[Mauricio Faria de Oliveira]

Path checkers will check paths of a port group in unavailable/standby
state more frequently (as they are 'failed') - possibly for a long or
indefinite period of time, and/or for a large number of paths.

That might flood the kernel log with scsi_dh_alua RTPG state messages,
due to the recheck scheduled in alua_check_sense() to update PG state.

So, do not to print such message if unavailable/standby state remains
(i.e., the PG did not transition to/from such states). All other cases
continue to be printed.

Signed-off-by: Mauricio Faria de Oliveira 

---
v2:
- changed v1's alua_state_remains() into alua_rtpg_print_check(),
  which includes in the states (not) to print for (deduplicates code).
- added support for Standby state too (requested in patch 1 of this set)
  (Bart Van Assche )
- remove superfluous parentheses in return statement.
  (Bart Van Assche )
- remove likely() hint
  (Bart Van Assche )

 drivers/scsi/device_handler/scsi_dh_alua.c | 35 ++
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c 
b/drivers/scsi/device_handler/scsi_dh_alua.c
index 937341ddb767..e0bf0827a980 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -572,6 +572,30 @@ static void alua_rtpg_print_supported(struct scsi_device 
*sdev,
 }
 
 /*
+ * alua_rtpg_print_check - Whether to print RTPG state information
+ *on particular state transitions.
+ * @old_state: the old state value.
+ * @new_state: the new state value.
+ */
+static bool alua_rtpg_print_check(int old_state, int new_state)
+{
+   /*
+* Do not print RTPG state information in case
+* state remains either unavailable or standby.
+*
+* Print it for everything else.
+*/
+
+   switch (old_state) {
+   case SCSI_ACCESS_STATE_UNAVAILABLE:
+   case SCSI_ACCESS_STATE_STANDBY:
+   return old_state != new_state;
+   default:
+   return 1;
+   }
+}
+
+/*
  * alua_rtpg - Evaluate REPORT TARGET GROUP STATES
  * @sdev: the device to be evaluated.
  *
@@ -706,6 +730,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
if ((tmp_pg == pg) ||
!(tmp_pg->flags & ALUA_PG_RUNNING)) {
struct alua_dh_data *h;
+   int tmp_pg_state_orig = tmp_pg->state;
 
tmp_pg->state = desc[0] & 0x0f;
tmp_pg->pref = desc[0] >> 7;
@@ -718,10 +743,12 @@ static int alua_rtpg(struct scsi_device *sdev, struct 
alua_port_group *pg)
}
rcu_read_unlock();
 
-   if (tmp_pg == pg)
-   alua_rtpg_print_supported(sdev, 
tmp_pg, desc[1]);
-   else
-   alua_rtpg_print(sdev, tmp_pg);
+   if 
(alua_rtpg_print_check(tmp_pg_state_orig, tmp_pg->state)) {
+   if (tmp_pg == pg)
+   
alua_rtpg_print_supported(sdev, tmp_pg, desc[1]);
+   else
+   alua_rtpg_print(sdev, 
tmp_pg);
+   }
}
spin_unlock_irqrestore(_pg->lock, flags);
}
-- 
1.8.3.1

[PATCH v2 4/4] scsi: scsi_dh_alua: add sdev_dbg() to track alua_rtpg_work()

[Mauricio Faria de Oliveira]

Insert sdev_dbg() calls in the function path which may queue
alua_rtpg_work() past initialization, for debugging purposes:
- alua_activate()
- alua_check_sense()
- alua_rtpg_queue()

Signed-off-by: Mauricio Faria de Oliveira 
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c 
b/drivers/scsi/device_handler/scsi_dh_alua.c
index e0bf0827a980..5bf0d55f6eb1 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -422,6 +422,10 @@ static char print_alua_state(unsigned char state)
 static int alua_check_sense(struct scsi_device *sdev,
struct scsi_sense_hdr *sense_hdr)
 {
+   sdev_dbg(sdev, "%s: %s(): Sense Key: 0x%x, ASC: 0x%x, ASCQ: 0x%x\n",
+ALUA_DH_NAME, __func__, sense_hdr->sense_key,
+sense_hdr->asc, sense_hdr->ascq);
+
switch (sense_hdr->sense_key) {
case NOT_READY:
if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0a) {
@@ -987,10 +991,13 @@ static bool alua_rtpg_queue(struct alua_port_group *pg,
spin_unlock_irqrestore(>lock, flags);
 
if (start_queue) {
+
if (queue_delayed_work(alua_wq, >rtpg_work,
-   msecs_to_jiffies(ALUA_RTPG_DELAY_MSECS)))
+   msecs_to_jiffies(ALUA_RTPG_DELAY_MSECS))) {
+   sdev_dbg(sdev, "%s: %s(): port group %02x\n",
+ALUA_DH_NAME, __func__, pg->group_id);
sdev = NULL;
-   else
+   } else
kref_put(>kref, release_port_group);
}
if (sdev)
@@ -1100,6 +1107,9 @@ static int alua_activate(struct scsi_device *sdev,
rcu_read_unlock();
mutex_unlock(>init_mutex);
 
+   sdev_dbg(sdev, "%s: %s(): port group %02x, fn: %pf()\n",
+ALUA_DH_NAME, __func__, pg->group_id, fn);
+
if (alua_rtpg_queue(pg, sdev, qdata, true))
fn = NULL;
else
-- 
1.8.3.1

[PATCH v2 0/4] scsi_dh_alua: fix stuck I/O after unavailable/standby states

[Mauricio Faria de Oliveira]

Currently, scsi_dh_alua fails I/O requests early on once ALUA state
unavailable/standby occur, which prevents path checkers to actually
check if I/O still fails or now works.

Then I/O requests are blocked indefinitely due to queue_if_no_path
but the underlying individual paths are fully operational, and can
be verified as such otherways (e.g., SG_IO).

This patchset addresses that problem, and adds a few improvements
to the logging of PG state changes.

Patch 1 fixes the problem.
Patch 2 makes sure that state changes for all PGs are logged.
Patch 3 makes sure that state no-changes for PGs in unavailable/standby
are not logged - only changes are.
Patch 4 adds few sdev_dbg() calls to track the path to alua_rtpg_work()

Tested on v4.12+ (commit b4b8cbf679c4).

Mauricio Faria de Oliveira (4):
  scsi: scsi_dh_alua: allow I/O in target port unavailable and standby
states
  scsi: scsi_dh_alua: print changes to RTPG state of all PGs
  scsi: scsi_dh_alua: do not print RTPG state if it remains
unavailable/standby
  scsi: scsi_dh_alua: add sdev_dbg() to track alua_rtpg_work()

 drivers/scsi/device_handler/scsi_dh_alua.c | 129 +
 1 file changed, 113 insertions(+), 16 deletions(-)

-- 
1.8.3.1

RE: device support in hpsa, was: Re: OOPS from cciss_ioctl in 4.12+git

[Don Brace]

So, adding adding  hpsa_allow_any=1 did not work...

When you added the 0x40800e11, did you add it to both tables?

/* define the PCI info for the cards we can control */
static const struct pci_device_id hpsa_pci_device_id[] = {
   {PCI_VENDOR_ID_COMPAQ, PCI_DEVICE_ID_COMPAQ_CISSB, 0x0E11, 0x4080},
...
{0,}
};

/*  board_id = Subsystem Device ID & Vendor ID
 *  product = Marketing Name for the board
 *  access = Address of the struct of function pointers
 */
static struct board_type products[] = {
{0x40800E11, "Smart Array 5i", _access},


...
};

I added it at the very first entry to make it easier.


---
However, there is not a SA5B_access table in hpsa.h.

/*
 *  This card is the opposite of the other cards.
 *   0 turns interrupts on...
 *   0x04 turns them off...
 */
static void SA5B_intr_mask(ctlr_info_t *h, unsigned long val)
{
if (val)
{ /* Turn interrupts on */
h->interrupts_enabled = 1;
writel(0, h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
(void) readl(h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
} else /* Turn them off */
{
h->interrupts_enabled = 0;
writel( SA5B_INTR_OFF,
h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
(void) readl(h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
}
}


/*
 *  Returns true if an interrupt is pending..
 */
static bool SA5B_intr_pending(ctlr_info_t *h)
{
unsigned long register_value  =
readl(h->vaddr + SA5_INTR_STATUS);
#ifdef CCISS_DEBUG
printk("cciss: intr_pending %lx\n", register_value);
#endif  /* CCISS_DEBUG */
if( register_value &  SA5B_INTR_PENDING)
return  1;
return 0 ;
}


static struct access_method SA5B_access = {
.submit_command = SA5_submit_command,
.set_intr_mask = SA5B_intr_mask,
.fifo_full = SA5_fifo_full,
.intr_pending = SA5B_intr_pending,
.command_completed = SA5_completed,
};



Can you try adding the two table entries and the SA5B definitions in hpsa.h?



> -Original Message-
> From: mr...@math.ut.ee [mailto:mr...@math.ut.ee] On Behalf Of Meelis
> Roos
> Sent: Monday, July 10, 2017 9:08 AM
> To: Christoph Hellwig 
> Cc: Laurence Oberman ; Jens Axboe
> ; Linux Kernel list ; linux-
> bl...@vger.kernel.org; Don Brace ; Scott
> Benesh ; Scott Teel
> ; Kevin Barnett
> ; linux-scsi@vger.kernel.org; Hannes
> Reinecke 
> Subject: Re: device support in hpsa, was: Re: OOPS from cciss_ioctl in 
> 4.12+git
> 
> EXTERNAL EMAIL
> 
> 
> > On Fri, Jul 07, 2017 at 11:42:38AM -0400, Laurence Oberman wrote:
> > > What happens when  hpsa_allow_any=1 with the Smart Array 64xx
> > > It should probe.
> >
> > But only if it has a HP vendor ID as far as I can tell.  We'd
> > still need to add the compaq ids so that these controllers get
> > probed.  But maybe it's time to add them and flip the hpsa_allow_any
> > default (maybe conditionally on a config option?) and mark cciss
> > deprecated.
> 
> I added hpsa_allow_any=1, did not help.
> 
> Added a wildcard Compaq entry with RAID class, like the one for HP,
> still no go:
> 
> [5.199125] hpsa :00:04.0: unrecognized board ID: 0x40800e11, ignoring.
> [5.282517] hpsa :00:04.0: Board ID not found
> 
> Added specific PCI ID and subdevice ID quad and I still get the same
> messages and the adapter is ignored.
> 
> What am I doing wrong?
> 
> --
> Meelis Roos (mr...@linux.ee)

Re: [PATCH 4/5] target: user: Fix sense data handling

[Mike Christie]

On 07/10/2017 12:36 AM, Damien Le Moal wrote:
> Nicholas, Mike,
> 
> On 7/7/17 15:05, Nicholas A. Bellinger wrote:
>> Everything including MNC's #1-6 and your #1-2 be pushed to
>> target-pending/for-next shortly.
>>
>> Please use this as your base for testing.  :)
> 
> I ran tests this morning with the latest target-pending/for-next branch.
> I ran libzbc test suite on top of 4 different configurations:
> 
> 1) ZBC drive + pscsi + loopback -> OK, no problems.
> 2) ZBC drive + pscsi + iscsi -> OK, no problems.
> 3) ZBC emulation tcmu-runner handler + loopback -> OK, no problems.
> 4) ZBC emulation tcmu-runner handler + iscsi -> Crash !
> 
> Here is the oops for case (4):
> 
> [  169.545459] scsi host7: iSCSI Initiator over TCP/IP
> [  169.559013] scsi 7:0:0:0: Direct-Access-ZBC LIO-ORG  TCMU ZBC device
> 0002 PQ: 0 ANSI: 5
> [  169.576920] sd 7:0:0:0: Attached scsi generic sg9 type 20
> [  169.577209] sd 7:0:0:0: [sdi] Host-managed zoned block device
> [  169.577794] sd 7:0:0:0: [sdi] 20971520 512-byte logical blocks: (10.7
> GB/10.0 GiB)
> [  169.577796] sd 7:0:0:0: [sdi] 40 zones of 524288 logical blocks
> [  169.577980] sd 7:0:0:0: [sdi] Write Protect is off
> [  169.578329] sd 7:0:0:0: [sdi] Write cache: enabled, read cache:
> enabled, doesn't support DPO or FUA
> [  169.590379] sd 7:0:0:0: [sdi] Attached SCSI disk
> [  240.071464] BUG: unable to handle kernel paging request at
> c9065db85540
> [  240.078460] IP: memcpy_erms+0x6/0x10
> [  240.082044] PGD 7ff0ba067
> [  240.082045] P4D 7ff0ba067
> [  240.084766] PUD 0
> [  240.087486]
> [  240.091006] Oops: 0002 [#1] PREEMPT SMP
> [  240.094855] Modules linked in: ip6table_filter ip6_tables
> rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache
> iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi sunrpc
> snd_hda_codec_hdmi snd_hda_codec_generic snd_hda_intel snd_hda_codec
> snd_hwdep snd_hda_core snd_seq snd_seq_device x86_pkg_temp_thermal
> coretemp snd_pcm crc32_pclmul snd_timer iTCO_wdt snd i2c_i801
> iTCO_vendor_support soundcore i915 iosf_mbi i2c_algo_bit drm_kms_helper
> syscopyarea sysfillrect sysimgblt fb_sys_fops drm e1000e r8169 mpt3sas
> mii i2c_core raid_class video
> [  240.143969] CPU: 0 PID: 1285 Comm: iscsi_trx Not tainted 4.12.0-rc1+ #3
> [  240.150607] Hardware name: ASUS All Series/H87-PRO, BIOS 2104 10/28/2014
> [  240.157331] task: 8807de4f5800 task.stack: c900047dc000
> [  240.163270] RIP: 0010:memcpy_erms+0x6/0x10
> [  240.167377] RSP: 0018:c900047dfc68 EFLAGS: 00010202
> [  240.172621] RAX: c9065db85540 RBX: 8807f798 RCX:
> 0010
> [  240.179771] RDX: 0010 RSI: 8807de574fe0 RDI:
> c9065db85540
> [  240.186930] RBP: c900047dfd30 R08: 8807de41b000 R09:
> 
> [  240.194088] R10: 0040 R11: 8807e9b726f0 R12:
> 0006565726b0
> [  240.201246] R13: c90007612ea0 R14: 00065657d540 R15:
> 
> [  240.208397] FS:  () GS:88081fa0()
> knlGS:
> [  240.216510] CS:  0010 DS:  ES:  CR0: 80050033
> [  240.80] CR2: c9065db85540 CR3: 01c0f000 CR4:
> 001406f0
> [  240.229430] Call Trace:
> [  240.231887]  ? tcmu_queue_cmd+0x83c/0xa80
> [  240.235916]  ? target_check_reservation+0xcd/0x6f0
> [  240.240725]  __target_execute_cmd+0x27/0xa0
> [  240.244918]  target_execute_cmd+0x232/0x2c0
> [  240.249124]  ? __local_bh_enable_ip+0x64/0xa0
> [  240.253499]  iscsit_execute_cmd+0x20d/0x270
> [  240.257693]  iscsit_sequence_cmd+0x110/0x190
> [  240.261985]  iscsit_get_rx_pdu+0x360/0xc80
> [  240.267565]  ? iscsi_target_rx_thread+0x54/0xd0
> [  240.273571]  iscsi_target_rx_thread+0x9a/0xd0
> [  240.279413]  kthread+0x113/0x150
> [  240.284120]  ? iscsi_target_tx_thread+0x1e0/0x1e0
> [  240.290297]  ? kthread_create_on_node+0x40/0x40
> [  240.296297]  ret_from_fork+0x2e/0x40
> [  240.301332] Code: 90 90 90 90 90 eb 1e 0f 1f 00 48 89 f8 48 89 d1 48
> c1 e9 03 83 e2 07 f3 48 a5 89 d1 f3 a4 c3 66 0f 1f 44 00 00 48 89 f8 48
> 89 d1  a4 c3 0f 1f 80 00 00 00 00 48 89 f8 48 83 fa 20 72 7e 40 38
> [  240.321751] RIP: memcpy_erms+0x6/0x10 RSP: c900047dfc68
> [  240.328838] CR2: c9065db85540
> [  240.333667] ---[ end trace b7e5354cfb54d08b ]---
> 
> I went back to running my initial 5 patch series on top of the current
> 4.12 kernel and everything is fine, including case (4).
> 
> A diff of the 2 versions of drivers/target/target_core_user.c did not
> reveal anything obvious that could result in this... It does look like a
> race condition on the session command or some memory corruption/bad
> pointer. Any idea ?
> 

I have not seen this crash before. You are running these tests:

https://github.com/hgst/libzbc/tree/master/test

right?

What test was it? If you need a device that supports zone to run the
test, do you know what scsi command it crashed on? If not can you send a
tcmpdump trace and/or enable lio kernel debugging?

Re: [PATCH] scsi: default to scsi-mq

[Bart Van Assche]

On Fri, 2017-06-16 at 10:27 +0200, Christoph Hellwig wrote:
> Remove the SCSI_MQ_DEFAULT config option and default to the blk-mq I/O
> path now that we had plenty of testing, and have I/O schedulers for
> blk-mq.  The module option to disable the blk-mq path is kept around
> for now.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/scsi/Kconfig | 11 ---
>  drivers/scsi/scsi.c  |  4 
>  2 files changed, 15 deletions(-)
> 
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index 3c52867dfe28..d384f4f86c26 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -47,17 +47,6 @@ config SCSI_NETLINK
>   default n
>   depends on NET
>  
> -config SCSI_MQ_DEFAULT
> - bool "SCSI: use blk-mq I/O path by default"
> - depends on SCSI
> - ---help---
> -   This option enables the new blk-mq based I/O path for SCSI
> -   devices by default.  With the option the scsi_mod.use_blk_mq
> -   module/boot option defaults to Y, without it to N, but it can
> -   still be overridden either way.
> -
> -   If unsure say N.
> -
>  config SCSI_PROC_FS
>   bool "legacy /proc/scsi/ support"
>   depends on SCSI && PROC_FS
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 1bf274e3b2b6..3d38c6d463b8 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -800,11 +800,7 @@ MODULE_LICENSE("GPL");
>  module_param(scsi_logging_level, int, S_IRUGO|S_IWUSR);
>  MODULE_PARM_DESC(scsi_logging_level, "a bit mask of logging levels");
>  
> -#ifdef CONFIG_SCSI_MQ_DEFAULT
>  bool scsi_use_blk_mq = true;
> -#else
> -bool scsi_use_blk_mq = false;
> -#endif
>  module_param_named(use_blk_mq, scsi_use_blk_mq, bool, S_IWUSR | S_IRUGO);
>  
>  static int __init init_scsi(void)

Since a fix for the performance regression triggered by this patch will be 
upstream
soon (see also 
https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=for-linus=32825c45ff8f4cce937ab85b030dc693ceb1aa0a):

Acked-by: Bart Van Assche 

Re: device support in hpsa, was: Re: OOPS from cciss_ioctl in 4.12+git

[Meelis Roos]

> On Fri, Jul 07, 2017 at 11:42:38AM -0400, Laurence Oberman wrote:
> > What happens when  hpsa_allow_any=1 with the Smart Array 64xx
> > It should probe.
> 
> But only if it has a HP vendor ID as far as I can tell.  We'd
> still need to add the compaq ids so that these controllers get
> probed.  But maybe it's time to add them and flip the hpsa_allow_any
> default (maybe conditionally on a config option?) and mark cciss
> deprecated.

I added hpsa_allow_any=1, did not help.

Added a wildcard Compaq entry with RAID class, like the one for HP, 
still no go:

[5.199125] hpsa :00:04.0: unrecognized board ID: 0x40800e11, ignoring.
[5.282517] hpsa :00:04.0: Board ID not found

Added specific PCI ID and subdevice ID quad and I still get the same 
messages and the adapter is ignored.

What am I doing wrong?

-- 
Meelis Roos (mr...@linux.ee)

Re: [PATCH v2 07/15] megaraid_sas: Check valid aen class range to avoid kernel panic

[Tomas Henzl]

On 5.7.2017 14:00, Shivasharan S wrote:
> Signed-off-by: Kashyap Desai 
> Signed-off-by: Shivasharan S 
> Reviewed-by: Hannes Reinecke 

Reviewed-by: Tomas Henzl 

tomash

Re: [PATCH v2 06/15] megaraid_sas: Fix endianness issues in DCMD handling

[Tomas Henzl]

On 5.7.2017 14:00, Shivasharan S wrote:
> Signed-off-by: Kashyap Desai 
> Signed-off-by: Shivasharan S 
> Reviewed-by: Hannes Reinecke 

Reviewed-by: Tomas Henzl 

tomash

Re: [PATCH v2 05/15] megaraid_sas: Do not re-fire shutdown DCMD after OCR

[Tomas Henzl]

On 5.7.2017 14:00, Shivasharan S wrote:
> Signed-off-by: Kashyap Desai 
> Signed-off-by: Shivasharan S 
> Reviewed-by: Hannes Reinecke 

Reviewed-by: Tomas Henzl 

tomash

Re: [PATCH v2 04/15] megaraid_sas: Call megasas_complete_cmd_dpc_fusion every 1 second while there are pending commands

[Tomas Henzl]

On 5.7.2017 14:00, Shivasharan S wrote:
> Fix - megasas_wait_for_outstanding_fusion checks for pending commands every
> 1second. But megasas_complete_cmd_dpc_fusion is only called every 5seconds.
> If the commands are already completed by firmware, there is an additional
> delay of 5seconds before driver will process completion for these commands.
>
> Signed-off-by: Kashyap Desai 
> Signed-off-by: Shivasharan S 

Reviewed-by: Tomas Henzl 

tomash

Re: [PATCH v2 03/15] megaraid_sas: Use synchronize_irq in target reset case

[Tomas Henzl]

On 5.7.2017 14:00, Shivasharan S wrote:
> Similar to task abort case, use synchronize_irq API in target reset case.
> Also, remove redundant call to megasas_complete_cmd_dpc_fusion
> after calling megasas_sync_irqs in task abort case.
>
> Signed-off-by: Kashyap Desai 
> Signed-off-by: Shivasharan S 
> Reviewed-by: Hannes Reinecke 

Reviewed-by: Tomas Henzl 

tomash

Re: [PATCH v2 02/15] megaraid_sas: set minimum value of resetwaittime to be 1 secs

[Tomas Henzl]

On 5.7.2017 14:00, Shivasharan S wrote:
> Setting resetwaittime to 0 during a FW fault will result in driver
> not calling the OCR.
>
> Signed-off-by: Kashyap Desai 
> Signed-off-by: Shivasharan S 
> Reviewed-by: Hannes Reinecke 

Reviewed-by: Tomas Henzl 

tomash

Re: [PATCH v2 01/15] megaraid_sas: mismatch of allocated MFI frame size and length exposed in MFI MPT pass through command

[Tomas Henzl]

On 5.7.2017 14:00, Shivasharan S wrote:
>  Fix - Driver allocated 256 byte MFI frames bytes but while sending MFI
>  frame (embedded inside chain frame of MPT frame) to firmware, driver
>  sets the length as 4k. This results in DMA read error messages during
>  boot.
>
> Signed-off-by: Kashyap Desai 
> Signed-off-by: Shivasharan S 
> Reviewed-by: Hannes Reinecke 

Reviewed-by: Tomas Henzl 

tomash

> ---
>  drivers/scsi/megaraid/megaraid_sas_fusion.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c 
> b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> index f990ab4d..f717fbc 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> @@ -3283,7 +3283,7 @@ irqreturn_t megasas_isr_fusion(int irq, void *devp)
>   mpi25_ieee_chain->Flags = IEEE_SGE_FLAGS_CHAIN_ELEMENT |
>   MPI2_IEEE_SGE_FLAGS_IOCPLBNTA_ADDR;
>  
> - mpi25_ieee_chain->Length = cpu_to_le32(instance->max_chain_frame_sz);
> + mpi25_ieee_chain->Length = cpu_to_le32(instance->mfi_frame_size);
>  }
>  
>  /**

Re: [PATCH] ses: do not add a device to an enclosure if enclosure_add_links() fails.

[Maurizio Lombardi]

Douglas,

> Has there been any progress with getting this patch accepted?
> 

It has been merged already.
It's in linux-next, commit 62e62ffd95539b9220894a7900a619e0f3ef4756

https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next/+/62e62ffd95539b9220894a7900a619e0f3ef4756

Maurizio.

Re: [PATCH v2 09/15] megaraid_sas: use vmalloc for crash dump buffers and driver's local RAID map

[Tomas Henzl]

On 5.7.2017 14:00, Shivasharan S wrote:
> Signed-off-by: Kashyap Desai 
> Signed-off-by: Shivasharan S 
> Reviewed-by: Hannes Reinecke 
> ---
>  drivers/scsi/megaraid/megaraid_sas.h|   1 -
>  drivers/scsi/megaraid/megaraid_sas_base.c   |  12 ++-
>  drivers/scsi/megaraid/megaraid_sas_fusion.c | 121 
> ++--
>  3 files changed, 88 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/scsi/megaraid/megaraid_sas.h 
> b/drivers/scsi/megaraid/megaraid_sas.h
> index 2b209bb..6d9f111 100644
> --- a/drivers/scsi/megaraid/megaraid_sas.h
> +++ b/drivers/scsi/megaraid/megaraid_sas.h
> @@ -2115,7 +2115,6 @@ struct megasas_instance {
>   u32 *crash_dump_buf;
>   dma_addr_t crash_dump_h;
>   void *crash_buf[MAX_CRASH_DUMP_SIZE];
> - u32 crash_buf_pages;
>   unsigned intfw_crash_buffer_size;
>   unsigned intfw_crash_state;
>   unsigned intfw_crash_buffer_offset;
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c 
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> index e490272..c63ef88 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -49,6 +49,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -6672,9 +6673,14 @@ static void megasas_detach_one(struct pci_dev *pdev)
> fusion->max_map_sz,
> fusion->ld_map[i],
> fusion->ld_map_phys[i]);
> - if (fusion->ld_drv_map[i])
> - free_pages((ulong)fusion->ld_drv_map[i],
> - fusion->drv_map_pages);
> + if (fusion->ld_drv_map[i]) {
> + if (is_vmalloc_addr(fusion->ld_drv_map[i]))
> + vfree(fusion->ld_drv_map[i]);
> + else
> + free_pages((ulong)fusion->ld_drv_map[i],
> +fusion->drv_map_pages);
> + }
> +
>   if (fusion->pd_seq_sync[i])
>   dma_free_coherent(>pdev->dev,
>   pd_seq_map_sz,
> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c 
> b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> index c239762..ff4a3a8 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> @@ -1257,6 +1257,80 @@ static int megasas_create_sg_sense_fusion(struct 
> megasas_instance *instance)
>  }
>  
>  /**
> + * megasas_allocate_raid_maps -  Allocate memory for RAID maps
> + * @instance:Adapter soft state
> + *
> + * return:   if success: return 0
> + *   failed:  return -ENOMEM
> + */
> +static inline int megasas_allocate_raid_maps(struct megasas_instance 
> *instance)
> +{
> + struct fusion_context *fusion;
> + int i = 0;
> +
> + fusion = instance->ctrl_context;
> +
> + fusion->drv_map_pages = get_order(fusion->drv_map_sz);
> +
> + for (i = 0; i < 2; i++) {
> + fusion->ld_map[i] = NULL;

Hi, does this assignment^ mean, that you need a fusion->ld_drv_map[0;1] = NULL 
setting
before this for cycle as well or is it just superfluos ?
 

> +
> + fusion->ld_drv_map[i] = (void *)
> + __get_free_pages(__GFP_ZERO | GFP_KERNEL,
> +  fusion->drv_map_pages);

The subject says - 'use vmalloc for ... and driver's local RAID map'
in the code here you use vmalloc only if __get_free_pages fails
is this intended ? (maybe an explanation in the mail body would be nice) 

tomash

> +
> + if (!fusion->ld_drv_map[i]) {
> + fusion->ld_drv_map[i] = vzalloc(fusion->drv_map_sz);
> +
> + if (!fusion->ld_drv_map[i]) {
> + dev_err(>pdev->dev,
> + "Could not allocate memory for local 
> map"
> + " size requested: %d\n",
> + fusion->drv_map_sz);
> + goto ld_drv_map_alloc_fail;
> + }
> + }
> + }
> +
> + for (i = 0; i < 2; i++) {
> + fusion->ld_map[i] = dma_alloc_coherent(>pdev->dev,
> +fusion->max_map_sz,
> +>ld_map_phys[i],
> +GFP_KERNEL);
> + if (!fusion->ld_map[i]) {
> + dev_err(>pdev->dev,
> + "Could not allocate memory for map info 
> 

Re: [PATCH] ses: do not add a device to an enclosure if enclosure_add_links() fails.

[Douglas Miller]

On 06/27/2017 07:50 AM, Douglas Miller wrote:

On 06/27/2017 04:53 AM, Maurizio Lombardi wrote:

The enclosure_add_device() function should fail if it can't
create the relevant sysfs links.

Signed-off-by: Maurizio Lombardi 
---
  drivers/misc/enclosure.c | 14 ++
  1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index d3fe3ea..eb29113 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -375,6 +375,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 +385,17 @@ 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);


Tested-by: Douglas Miller 

This fixes a problem where udevd (insmod ses) races with/overtakes 
do_scan_async(), which creates the directory target of the symlink, 
resulting in missing enclosure symlinks. This patch relaxes the 
symlink creation allowing for delayed addition to enclosure and 
creation of symlinks after do_scan_async() has created the target 
directory.



Has there been any progress with getting this patch accepted?

[PATCH] scsi: qla2xxx: Off by one in qlt_ctio_to_cmd()

[Dan Carpenter]

There are "req->num_outstanding_cmds" elements in the
req->outstanding_cmds[] array so the > here should be >=.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/scsi/qla2xxx/qla_target.c 
b/drivers/scsi/qla2xxx/qla_target.c
index 6e4794367e0b..ecd1a95511f9 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -3728,7 +3728,7 @@ static struct qla_tgt_cmd *qlt_ctio_to_cmd(struct 
scsi_qla_host *vha,
h &= QLA_CMD_HANDLE_MASK;
 
if (h != QLA_TGT_NULL_HANDLE) {
-   if (unlikely(h > req->num_outstanding_cmds)) {
+   if (unlikely(h >= req->num_outstanding_cmds)) {
ql_dbg(ql_dbg_tgt, vha, 0xe052,
"qla_target(%d): Wrong handle %x received\n",
vha->vp_idx, handle);

[PATCH v3 1/7] libsas: Use static sas event pool to appease sas event lost

[Yijing Wang]

Now libsas hotplug work is static, every sas event type has its own
static work, LLDD driver queue the hotplug work into shost->work_q.
If LLDD driver burst post lots hotplug events to libsas, the hotplug
events may pending in the workqueue like

shost->work_q
new work[PORTE_BYTES_DMAED] --> |[PHYE_LOSS_OF_SIGNAL][PORTE_BYTES_DMAED] -> 
processing
|<---wait worker to process>|
In this case, a new PORTE_BYTES_DMAED event coming, libsas try to queue it
to shost->work_q, but this work is already pending, so it would be lost.
Finally, libsas delete the related sas port and sas devices, but LLDD driver
expect libsas add the sas port and devices(last sas event).

This patch and use static sas event work pool to appease this issue, since
it's static work pool, it won't make memory exhaust.

Signed-off-by: Yijing Wang 
CC: John Garry 
CC: Johannes Thumshirn 
CC: Ewan Milne 
CC: Christoph Hellwig 
CC: Tomas Henzl 
CC: Dan Williams 
---
 drivers/scsi/libsas/sas_event.c| 208 -
 drivers/scsi/libsas/sas_init.c |   6 --
 drivers/scsi/libsas/sas_internal.h |   3 +
 drivers/scsi/libsas/sas_phy.c  |  48 +++--
 drivers/scsi/libsas/sas_port.c |  18 ++--
 include/scsi/libsas.h  |  16 +--
 6 files changed, 216 insertions(+), 83 deletions(-)

diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
index c0d0d97..a1370bd 100644
--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c
@@ -27,13 +27,20 @@
 #include "sas_internal.h"
 #include "sas_dump.h"
 
+static DEFINE_SPINLOCK(sas_event_lock);
+
+static const work_func_t sas_ha_event_fns[HA_NUM_EVENTS] = {
+  [HAE_RESET] = sas_hae_reset,
+};
+
 int sas_queue_work(struct sas_ha_struct *ha, struct sas_work *sw)
 {
int rc = 0;
 
if (!test_bit(SAS_HA_REGISTERED, >state))
-   return 0;
+   return rc;
 
+   rc = 1;
if (test_bit(SAS_HA_DRAINING, >state)) {
/* add it to the defer list, if not already pending */
if (list_empty(>drain_node))
@@ -44,19 +51,15 @@ int sas_queue_work(struct sas_ha_struct *ha, struct 
sas_work *sw)
return rc;
 }
 
-static int sas_queue_event(int event, unsigned long *pending,
-   struct sas_work *work,
+static int sas_queue_event(int event, struct sas_work *work,
struct sas_ha_struct *ha)
 {
int rc = 0;
+   unsigned long flags;
 
-   if (!test_and_set_bit(event, pending)) {
-   unsigned long flags;
-
-   spin_lock_irqsave(>lock, flags);
-   rc = sas_queue_work(ha, work);
-   spin_unlock_irqrestore(>lock, flags);
-   }
+   spin_lock_irqsave(>lock, flags);
+   rc = sas_queue_work(ha, work);
+   spin_unlock_irqrestore(>lock, flags);
 
return rc;
 }
@@ -64,6 +67,8 @@ static int sas_queue_event(int event, unsigned long *pending,
 
 void __sas_drain_work(struct sas_ha_struct *ha)
 {
+   int ret;
+   unsigned long flags;
struct workqueue_struct *wq = ha->core.shost->work_q;
struct sas_work *sw, *_sw;
 
@@ -78,7 +83,12 @@ void __sas_drain_work(struct sas_ha_struct *ha)
clear_bit(SAS_HA_DRAINING, >state);
list_for_each_entry_safe(sw, _sw, >defer_q, drain_node) {
list_del_init(>drain_node);
-   sas_queue_work(ha, sw);
+   ret = sas_queue_work(ha, sw);
+   if (ret != 1) {
+   spin_lock_irqsave(_event_lock, flags);
+   sw->used = false;
+   spin_unlock_irqrestore(_event_lock, flags);
+   }
}
spin_unlock_irq(>lock);
 }
@@ -119,51 +129,197 @@ void sas_enable_revalidation(struct sas_ha_struct *ha)
if (!test_and_clear_bit(ev, >pending))
continue;
 
-   sas_queue_event(ev, >pending, >disc_work[ev].work, ha);
+   sas_queue_event(ev, >disc_work[ev].work, ha);
}
mutex_unlock(>disco_mutex);
 }
 
+static void sas_free_ha_event(struct sas_ha_event *event)
+{
+   unsigned long flags;
+   spin_lock_irqsave(_event_lock, flags);
+   event->work.used = false;
+   spin_unlock_irqrestore(_event_lock, flags);
+}
+
+static void sas_free_port_event(struct asd_sas_event *event)
+{
+   unsigned long flags;
+   spin_lock_irqsave(_event_lock, flags);
+   event->work.used = false;
+   spin_unlock_irqrestore(_event_lock, flags);
+}
+
+static void sas_free_phy_event(struct asd_sas_event *event)
+{
+   unsigned long flags;
+   spin_lock_irqsave(_event_lock, flags);
+   event->work.used = false;
+   spin_unlock_irqrestore(_event_lock, flags);
+}
+
+static void 

[PATCH v3 6/7] libsas: add wait-complete support to sync discovery event

[Yijing Wang]

Introduce a sync flag to tag discovery event whether need to
sync execute, per-event wait-complete ensure sync.

Signed-off-by: Yijing Wang 
CC: John Garry 
CC: Johannes Thumshirn 
CC: Ewan Milne 
CC: Christoph Hellwig 
CC: Tomas Henzl 
CC: Dan Williams 
---
 drivers/scsi/libsas/sas_discover.c |  8 ++--
 drivers/scsi/libsas/sas_expander.c | 12 +++-
 drivers/scsi/libsas/sas_internal.h | 27 +++
 include/scsi/libsas.h  |  2 ++
 4 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libsas/sas_discover.c 
b/drivers/scsi/libsas/sas_discover.c
index a25d648..d68f8dd 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -378,6 +378,7 @@ void sas_unregister_dev(struct asd_sas_port *port, struct 
domain_device *dev)
list_del_init(>disco_list_node);
sas_rphy_free(dev->rphy);
sas_unregister_common_dev(port, dev);
+   sas_disc_cancel_sync(>disc.disc_work[DISCE_DESTRUCT]);
return;
}
 
@@ -541,6 +542,7 @@ static void sas_discover_common_fn(struct work_struct *work)
struct asd_sas_port *port = ev->port;
 
sas_event_fns[ev->type](work);
+   sas_disc_wakeup(ev);
sas_port_put(port);
 }
 
@@ -571,8 +573,10 @@ static void sas_chain_work(struct sas_ha_struct *ha, 
struct sas_work *sw)
else
ret = scsi_queue_work(ha->core.shost, >work);
 
-   if (ret != 1)
+   if (ret != 1) {
sas_port_put(port);
+   sas_disc_cancel_sync(ev);
+   }
 }
 
 static void sas_chain_event(int event, unsigned long *pending,
@@ -592,9 +596,9 @@ int sas_discover_event(struct asd_sas_port *port, enum 
discover_event ev)
 {
struct sas_discovery *disc;
 
+   disc = >disc;
if (!port)
return 0;
-   disc = >disc;
 
BUG_ON(ev >= DISC_NUM_EVENTS);
 
diff --git a/drivers/scsi/libsas/sas_expander.c 
b/drivers/scsi/libsas/sas_expander.c
index 570b2cb..9d26c28 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -822,14 +822,18 @@ static struct domain_device *sas_ex_discover_end_dev(
 
list_add_tail(>disco_list_node, 
>port->disco_list);
 
+   sas_disc_wait_init(child->port, DISCE_PROBE);
res = sas_discover_sata(child);
if (res) {
+   
sas_disc_cancel_sync(>port->disc.disc_work[DISCE_PROBE]);
SAS_DPRINTK("sas_discover_sata() for device %16llx at "
"%016llx:0x%x returned 0x%x\n",
SAS_ADDR(child->sas_addr),
SAS_ADDR(parent->sas_addr), phy_id, res);
goto out_list_del;
}
+   sas_disc_wait_completion(child->port, DISCE_PROBE);
+
} else
 #endif
  if (phy->attached_tproto & SAS_PROTOCOL_SSP) {
@@ -847,14 +851,17 @@ static struct domain_device *sas_ex_discover_end_dev(
 
list_add_tail(>disco_list_node, 
>port->disco_list);
 
+   sas_disc_wait_init(child->port, DISCE_PROBE);
res = sas_discover_end_dev(child);
if (res) {
+   
sas_disc_cancel_sync(>port->disc.disc_work[DISCE_PROBE]);
SAS_DPRINTK("sas_discover_end_dev() for device %16llx "
"at %016llx:0x%x returned 0x%x\n",
SAS_ADDR(child->sas_addr),
SAS_ADDR(parent->sas_addr), phy_id, res);
goto out_list_del;
}
+   sas_disc_wait_completion(child->port, DISCE_PROBE);
} else {
SAS_DPRINTK("target proto 0x%x at %016llx:0x%x not handled\n",
phy->attached_tproto, SAS_ADDR(parent->sas_addr),
@@ -1890,8 +1897,11 @@ static void sas_unregister_devs_sas_addr(struct 
domain_device *parent,
if (child->dev_type == SAS_EDGE_EXPANDER_DEVICE 
||
child->dev_type == 
SAS_FANOUT_EXPANDER_DEVICE)
sas_unregister_ex_tree(parent->port, 
child);
-   else
+   else {
+   sas_disc_wait_init(parent->port, 
DISCE_DESTRUCT);
sas_unregister_dev(parent->port, child);
+   sas_disc_wait_completion(parent->port, 
DISCE_DESTRUCT);
+   }
found = child;
break;
}
diff --git 

[PATCH v3 5/7] libsas: add a new workqueue to run probe/destruct discovery event

[Yijing Wang]

Sometimes, we want sync libsas probe or destruct in sas discovery work,
like when libsas revalidate domain. We need to split probe and destruct
work from the scsi host workqueue.

Signed-off-by: Yijing Wang 
CC: John Garry 
CC: Johannes Thumshirn 
CC: Ewan Milne 
CC: Christoph Hellwig 
CC: Tomas Henzl 
CC: Dan Williams 
---
 drivers/scsi/libsas/sas_discover.c | 13 -
 drivers/scsi/libsas/sas_init.c |  8 
 include/scsi/libsas.h  |  1 +
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/libsas/sas_discover.c 
b/drivers/scsi/libsas/sas_discover.c
index 5d4a3a8..a25d648 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -559,7 +559,18 @@ static void sas_chain_work(struct sas_ha_struct *ha, 
struct sas_work *sw)
 * not racing against draining
 */
sas_port_get(port);
-   ret = scsi_queue_work(ha->core.shost, >work);
+   /*
+* discovery event probe and destruct would be called in other
+* discovery event like discover domain and revalidate domain
+* events, in some cases, we need to sync execute probe and destruct
+* events, so run probe and destruct discover events except in a new
+* workqueue.
+*/
+   if (ev->type == DISCE_PROBE || ev->type == DISCE_DESTRUCT)
+   ret = queue_work(ha->disc_q, >work);
+   else
+   ret = scsi_queue_work(ha->core.shost, >work);
+
if (ret != 1)
sas_port_put(port);
 }
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index 2f3b736..df1d78b 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -152,6 +152,13 @@ int sas_register_ha(struct sas_ha_struct *sas_ha)
if (!sas_ha->event_q)
goto Undo_ports;
 
+   snprintf(name, 64, "%s_disc_q", dev_name(sas_ha->dev));
+   sas_ha->disc_q = create_singlethread_workqueue(name);
+   if(!sas_ha->disc_q) {
+   destroy_workqueue(sas_ha->event_q);
+   goto Undo_ports;
+   }
+
INIT_LIST_HEAD(_ha->eh_done_q);
INIT_LIST_HEAD(_ha->eh_ata_q);
 
@@ -187,6 +194,7 @@ int sas_unregister_ha(struct sas_ha_struct *sas_ha)
__sas_drain_work(sas_ha);
mutex_unlock(_ha->drain_mutex);
destroy_workqueue(sas_ha->event_q);
+   destroy_workqueue(sas_ha->disc_q);
 
return 0;
 }
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index c2ef05e..4bcb9fe 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -406,6 +406,7 @@ struct sas_ha_struct {
struct device *dev;   /* should be set */
struct module *lldd_module; /* should be set */
struct workqueue_struct *event_q;
+   struct workqueue_struct *disc_q;
 
u8 *sas_addr; /* must be set */
u8 hashed_sas_addr[HASHED_SAS_ADDR_SIZE];
-- 
2.5.0

[PATCH v3 4/7] libsas: add sas event wait-complete support

[Yijing Wang]

Introduce wait-complete for libsas sas event processing,
execute sas port create/destruct in sync.

Signed-off-by: Yijing Wang 
CC: John Garry 
CC: Johannes Thumshirn 
CC: Ewan Milne 
CC: Christoph Hellwig 
CC: Tomas Henzl 
CC: Dan Williams 
---
 drivers/scsi/libsas/sas_discover.c | 41 --
 drivers/scsi/libsas/sas_internal.h | 34 +++
 drivers/scsi/libsas/sas_port.c |  4 
 include/scsi/libsas.h  |  5 -
 4 files changed, 72 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/libsas/sas_discover.c 
b/drivers/scsi/libsas/sas_discover.c
index 60de662..5d4a3a8 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -525,16 +525,43 @@ static void sas_revalidate_domain(struct work_struct 
*work)
mutex_unlock(>disco_mutex);
 }
 
+static const work_func_t sas_event_fns[DISC_NUM_EVENTS] = {
+   [DISCE_DISCOVER_DOMAIN] = sas_discover_domain,
+   [DISCE_REVALIDATE_DOMAIN] = sas_revalidate_domain,
+   [DISCE_PROBE] = sas_probe_devices,
+   [DISCE_SUSPEND] = sas_suspend_devices,
+   [DISCE_RESUME] = sas_resume_devices,
+   [DISCE_DESTRUCT] = sas_destruct_devices,
+};
+
+/* a simple wrapper for sas discover event funtions */
+static void sas_discover_common_fn(struct work_struct *work)
+{
+   struct sas_discovery_event *ev = to_sas_discovery_event(work);
+   struct asd_sas_port *port = ev->port;
+
+   sas_event_fns[ev->type](work);
+   sas_port_put(port);
+}
+
+
 /* -- Events -- */
 
 static void sas_chain_work(struct sas_ha_struct *ha, struct sas_work *sw)
 {
+   int ret;
+   struct sas_discovery_event *ev = to_sas_discovery_event(>work);
+   struct asd_sas_port *port = ev->port;
+
/* chained work is not subject to SA_HA_DRAINING or
 * SAS_HA_REGISTERED, because it is either submitted in the
 * workqueue, or known to be submitted from a context that is
 * not racing against draining
 */
-   scsi_queue_work(ha->core.shost, >work);
+   sas_port_get(port);
+   ret = scsi_queue_work(ha->core.shost, >work);
+   if (ret != 1)
+   sas_port_put(port);
 }
 
 static void sas_chain_event(int event, unsigned long *pending,
@@ -575,18 +602,10 @@ void sas_init_disc(struct sas_discovery *disc, struct 
asd_sas_port *port)
 {
int i;
 
-   static const work_func_t sas_event_fns[DISC_NUM_EVENTS] = {
-   [DISCE_DISCOVER_DOMAIN] = sas_discover_domain,
-   [DISCE_REVALIDATE_DOMAIN] = sas_revalidate_domain,
-   [DISCE_PROBE] = sas_probe_devices,
-   [DISCE_SUSPEND] = sas_suspend_devices,
-   [DISCE_RESUME] = sas_resume_devices,
-   [DISCE_DESTRUCT] = sas_destruct_devices,
-   };
-
disc->pending = 0;
for (i = 0; i < DISC_NUM_EVENTS; i++) {
-   INIT_SAS_WORK(>disc_work[i].work, sas_event_fns[i]);
+   INIT_SAS_WORK(>disc_work[i].work, sas_discover_common_fn);
disc->disc_work[i].port = port;
+   disc->disc_work[i].type = i;
}
 }
diff --git a/drivers/scsi/libsas/sas_internal.h 
b/drivers/scsi/libsas/sas_internal.h
index f03ce64..890b5d26 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -100,6 +100,40 @@ void sas_free_device(struct kref *kref);
 extern const work_func_t sas_phy_event_fns[PHY_NUM_EVENTS];
 extern const work_func_t sas_port_event_fns[PORT_NUM_EVENTS];
 
+static void sas_complete_event(struct kref *kref)
+{
+   struct asd_sas_port *port = container_of(kref, struct asd_sas_port, 
ref);
+
+   complete_all(>completion);
+}
+
+static inline void sas_port_put(struct asd_sas_port *port)
+{
+   if (port->is_sync)
+   kref_put(>ref, sas_complete_event);
+}
+
+static inline void sas_port_wait_init(struct asd_sas_port *port)
+{
+   init_completion(>completion);
+   kref_init(>ref);
+   port->is_sync = true;
+}
+
+static inline void sas_port_wait_completion(
+   struct asd_sas_port *port)
+{
+   sas_port_put(port);
+   wait_for_completion(>completion);
+   port->is_sync = false;
+}
+
+static inline void sas_port_get(struct asd_sas_port *port)
+{
+   if (port && port->is_sync)
+   kref_get(>ref);
+}
+
 #ifdef CONFIG_SCSI_SAS_HOST_SMP
 extern int sas_smp_host_handler(struct Scsi_Host *shost, struct request *req,
struct request *rsp);
diff --git a/drivers/scsi/libsas/sas_port.c b/drivers/scsi/libsas/sas_port.c
index 9326628..d589adb 100644
--- a/drivers/scsi/libsas/sas_port.c
+++ b/drivers/scsi/libsas/sas_port.c
@@ -191,7 +191,9 @@ static void sas_form_port(struct asd_sas_phy *phy)
if (si->dft->lldd_port_formed)

[PATCH v3 3/7] libsas: Use new workqueue to run sas event

[Yijing Wang]

Now all libsas works are queued to scsi host workqueue,
include sas event work post by LLDD and sas discovery
work, and a sas hotplug flow may be divided into several
works, e.g libsas receive a PORTE_BYTES_DMAED event,
now we process it as following steps:
sas_form_port  --- run in work in shot workq
sas_discover_domain  --- run in another work in shost workq
...
sas_probe_devices  --- run in new work in shost workq
We found during hot-add a device, libsas may need run several
works in same workqueue to add device in system, the process is
not atomic, it may interrupt by other sas event works, like
PHYE_LOSS_OF_SIGNAL. Finally, we would found lots unexpected
errors. This patch is preparation of execute libsas sas event
in sync.

Signed-off-by: Yijing Wang 
CC: John Garry 
CC: Johannes Thumshirn 
CC: Ewan Milne 
CC: Christoph Hellwig 
CC: Tomas Henzl 
CC: Dan Williams 
---
 drivers/scsi/libsas/sas_event.c | 4 ++--
 drivers/scsi/libsas/sas_init.c  | 7 +++
 include/scsi/libsas.h   | 1 +
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
index a1370bd..a72a089 100644
--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c
@@ -46,7 +46,7 @@ int sas_queue_work(struct sas_ha_struct *ha, struct sas_work 
*sw)
if (list_empty(>drain_node))
list_add(>drain_node, >defer_q);
} else
-   rc = scsi_queue_work(ha->core.shost, >work);
+   rc = queue_work(ha->event_q, >work);
 
return rc;
 }
@@ -69,7 +69,7 @@ void __sas_drain_work(struct sas_ha_struct *ha)
 {
int ret;
unsigned long flags;
-   struct workqueue_struct *wq = ha->core.shost->work_q;
+   struct workqueue_struct *wq = ha->event_q;
struct sas_work *sw, *_sw;
 
set_bit(SAS_HA_DRAINING, >state);
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index c227a8b..2f3b736 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -115,6 +115,7 @@ void sas_hae_reset(struct work_struct *work)
 
 int sas_register_ha(struct sas_ha_struct *sas_ha)
 {
+   char name[64];
int error = 0;
 
mutex_init(_ha->disco_mutex);
@@ -146,6 +147,11 @@ int sas_register_ha(struct sas_ha_struct *sas_ha)
goto Undo_ports;
}
 
+   snprintf(name, 64, "%s_event_q", dev_name(sas_ha->dev));
+   sas_ha->event_q = create_singlethread_workqueue(name);
+   if (!sas_ha->event_q)
+   goto Undo_ports;
+
INIT_LIST_HEAD(_ha->eh_done_q);
INIT_LIST_HEAD(_ha->eh_ata_q);
 
@@ -180,6 +186,7 @@ int sas_unregister_ha(struct sas_ha_struct *sas_ha)
mutex_lock(_ha->drain_mutex);
__sas_drain_work(sas_ha);
mutex_unlock(_ha->drain_mutex);
+   destroy_workqueue(sas_ha->event_q);
 
return 0;
 }
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 628f48b..a01ca42 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -402,6 +402,7 @@ struct sas_ha_struct {
char *sas_ha_name;
struct device *dev;   /* should be set */
struct module *lldd_module; /* should be set */
+   struct workqueue_struct *event_q;
 
u8 *sas_addr; /* must be set */
u8 hashed_sas_addr[HASHED_SAS_ADDR_SIZE];
-- 
2.5.0

[PATCH v3 2/7] libsas: remove unused port_gone_completion

[Yijing Wang]

No one uses the port_gone_completion in struct asd_sas_port,
clean it out.

Signed-off-by: Yijing Wang 
---
 include/scsi/libsas.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index c41328d..628f48b 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -263,8 +263,6 @@ struct sas_discovery {
 /* The port struct is Class:RW, driver:RO */
 struct asd_sas_port {
 /* private: */
-   struct completion port_gone_completion;
-
struct sas_discovery disc;
struct domain_device *port_dev;
spinlock_t dev_list_lock;
-- 
2.5.0

[PATCH v3 7/7] libsas: release disco mutex during waiting in sas_ex_discover_end_dev

[Yijing Wang]

Disco mutex was introudced to prevent domain rediscovery competing
with ata error handling(87c8331). If we have already hold the lock
in sas_revalidate_domain and sync executing probe, deadlock caused,
because, sas_probe_sata() also need hold disco_mutex. Since disco mutex
use to prevent revalidata domain happen during ata error handler,
it should be safe to release disco mutex when sync probe, because
no new revalidate domain event would be process until the sync return,
and the current sas revalidate domain finish.

Signed-off-by: Yijing Wang 
CC: John Garry 
CC: Johannes Thumshirn 
CC: Ewan Milne 
CC: Christoph Hellwig 
CC: Tomas Henzl 
CC: Dan Williams 
---
 drivers/scsi/libsas/sas_expander.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/scsi/libsas/sas_expander.c 
b/drivers/scsi/libsas/sas_expander.c
index 9d26c28..077024e 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -776,6 +776,7 @@ static struct domain_device *sas_ex_discover_end_dev(
struct ex_phy *phy = _ex->ex_phy[phy_id];
struct domain_device *child = NULL;
struct sas_rphy *rphy;
+   bool prev_lock;
int res;
 
if (phy->attached_sata_host || phy->attached_sata_ps)
@@ -803,6 +804,7 @@ static struct domain_device *sas_ex_discover_end_dev(
sas_ex_get_linkrate(parent, child, phy);
sas_device_set_phy(child, phy->port);
 
+   prev_lock = mutex_is_locked(>port->ha->disco_mutex);
 #ifdef CONFIG_SCSI_SAS_ATA
if ((phy->attached_tproto & SAS_PROTOCOL_STP) || 
phy->attached_sata_dev) {
res = sas_get_ata_info(child, phy);
@@ -832,7 +834,11 @@ static struct domain_device *sas_ex_discover_end_dev(
SAS_ADDR(parent->sas_addr), phy_id, res);
goto out_list_del;
}
+   if (prev_lock)
+   mutex_unlock(>port->ha->disco_mutex);
sas_disc_wait_completion(child->port, DISCE_PROBE);
+   if (prev_lock)
+   mutex_lock(>port->ha->disco_mutex);
 
} else
 #endif
@@ -861,7 +867,11 @@ static struct domain_device *sas_ex_discover_end_dev(
SAS_ADDR(parent->sas_addr), phy_id, res);
goto out_list_del;
}
+   if (prev_lock)
+   mutex_unlock(>port->ha->disco_mutex);
sas_disc_wait_completion(child->port, DISCE_PROBE);
+   if (prev_lock)
+   mutex_lock(>port->ha->disco_mutex);
} else {
SAS_DPRINTK("target proto 0x%x at %016llx:0x%x not handled\n",
phy->attached_tproto, SAS_ADDR(parent->sas_addr),
-- 
2.5.0

[PATCH v3 0/7] Enhance libsas hotplug feature

[Yijing Wang]

This patchset is based Johannes's patch
"scsi: sas: scsi_queue_work can fail, so make callers aware"

Now the libsas hotplug has some issues, Dan Williams report
a similar bug here before
https://www.mail-archive.com/linux-scsi@vger.kernel.org/msg39187.html

The issues we have found
1. if LLDD burst reports lots of phy-up/phy-down sas events, some events
   may lost because a same sas events is pending now, finally libsas topo
   may different the hardware.
2. receive a phy down sas event, libsas call sas_deform_port to remove
   devices, it would first delete the sas port, then put a destruction
   discovery event in a new work, and queue it at the tail of workqueue,
   once the sas port be deleted, its children device will be deleted too,
   when the destruction work start, it will found the target device has
   been removed, and report a sysfs warnning.
3. since a hotplug process will be devided into several works, if a phy up
   sas event insert into phydown works, like
   destruction work  ---> PORTE_BYTES_DMAED (sas_form_port) 
>PHYE_LOSS_OF_SIGNAL
   the hot remove flow would broken by PORTE_BYTES_DMAED event, it's not
   we expected, and issues would occur.

The first patch fix the sas events lost, and the second one introudce 
wait-complete
to fix the hotplug order issues.

v2->v3: some code improvements suggested by Johannes and John,
split v2 patch 2 into several small pathes.
v1->v2: some code improvements suggested by John Garry

Yijing Wang (7):
  libsas: Use static sas event pool to appease sas event lost
  libsas: remove unused port_gone_completion
  libsas: Use new workqueue to run sas event
  libsas: add sas event wait-complete support
  libsas: add a new workqueue to run probe/destruct discovery event
  libsas: add wait-complete support to sync discovery event
  libsas: release disco mutex during waiting in sas_ex_discover_end_dev

 drivers/scsi/libsas/sas_discover.c |  58 +++---
 drivers/scsi/libsas/sas_event.c| 212 -
 drivers/scsi/libsas/sas_expander.c |  22 +++-
 drivers/scsi/libsas/sas_init.c |  21 ++--
 drivers/scsi/libsas/sas_internal.h |  64 +++
 drivers/scsi/libsas/sas_phy.c  |  48 +++--
 drivers/scsi/libsas/sas_port.c |  22 ++--
 include/scsi/libsas.h  |  27 +++--
 8 files changed, 373 insertions(+), 101 deletions(-)

-- 
2.5.0