Re: [PATCH] scsi: libfc: fix ELS request handling

2017-11-27 Thread Johannes Thumshirn
Thanks Martin,
Reviewed-by: Johannes Thumshirn 
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


UFS utilities

2017-11-27 Thread Bean Huo (beanhuo)
Hi, all 
Is there someone knows if exists one utilis dedicated to UFS device, rather 
than SCSI utils?
I have tried sg3-utils, but it is not convenient for the embedded ARM-based 
system.
And also it doesn't support several UFS special command. 
If we don't have this kind of tool for UFS, is it necessary for us to develop a 
ufs-utils?
I need your every expert's inputs and suggestions.
Thanks in advance.

//Bean Huo


[PATCH] scsi: debug: remove jiffies_to_timespec

2017-11-27 Thread Arnd Bergmann
There is no need to go through an intermediate timespec to convert
to ktime_t when we just want a simple multiplication. This gets
rid of one of the few users of jiffies_to_timespec, which I
hope to remove as part of the y2038 cleanup.

Signed-off-by: Arnd Bergmann 
---
 drivers/scsi/scsi_debug.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index e4f037f0f38b..df7e9db44d44 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -4085,10 +4085,7 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct 
sdebug_dev_info *devip,
ktime_t kt;
 
if (delta_jiff > 0) {
-   struct timespec ts;
-
-   jiffies_to_timespec(delta_jiff, &ts);
-   kt = ktime_set(ts.tv_sec, ts.tv_nsec);
+   kt = ns_to_ktime((u64)delta_jiff * (NSEC_PER_SEC / HZ));
} else
kt = sdebug_ndelay;
if (NULL == sd_dp) {
-- 
2.9.0



Re: UFS utilities

2017-11-27 Thread Greg KH
On Mon, Nov 27, 2017 at 11:25:47AM +, Bean Huo (beanhuo) wrote:
> Hi, all 
> Is there someone knows if exists one utilis dedicated to UFS device, rather 
> than SCSI utils?
> I have tried sg3-utils, but it is not convenient for the embedded ARM-based 
> system.
> And also it doesn't support several UFS special command. 

What specific UFS commands do you need to make to the device that the
current driver does not support?

And yes, this is a trick question as there are about 4 different major
forks that I know of of the UFS driver in different vendor trees, all of
which support different types of UFS commands :(

> If we don't have this kind of tool for UFS, is it necessary for us to develop 
> a ufs-utils?

I doubt it, what neds to happen is getting all of the functionality that
lives in these different forks all merged upstream into the in-kernel
driver.  Then I bet all of the needed functionality you are looking for
will be there.

good luck!

greg k-h


Re: [PATCH] scsi: debug: remove jiffies_to_timespec

2017-11-27 Thread Johannes Thumshirn
Looks good,
Reviewed-by: Johannes Thumshirn 
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [PATCH] scsi: debug: remove jiffies_to_timespec

2017-11-27 Thread Douglas Gilbert

On 2017-11-27 12:36 PM, Arnd Bergmann wrote:

There is no need to go through an intermediate timespec to convert
to ktime_t when we just want a simple multiplication. This gets
rid of one of the few users of jiffies_to_timespec, which I
hope to remove as part of the y2038 cleanup.

Signed-off-by: Arnd Bergmann 

Acked-by: Douglas Gilbert 


---
  drivers/scsi/scsi_debug.c | 5 +
  1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index e4f037f0f38b..df7e9db44d44 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -4085,10 +4085,7 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct 
sdebug_dev_info *devip,
ktime_t kt;
  
  		if (delta_jiff > 0) {

-   struct timespec ts;
-
-   jiffies_to_timespec(delta_jiff, &ts);
-   kt = ktime_set(ts.tv_sec, ts.tv_nsec);
+   kt = ns_to_ktime((u64)delta_jiff * (NSEC_PER_SEC / HZ));
} else
kt = sdebug_ndelay;
if (NULL == sd_dp) {





[PATCH] scsi: lpfc: Fix potential NULL pointer dereference in lpfc_nvme_fcp_io_submit

2017-11-27 Thread Gustavo A. R. Silva
pnvme_lport is being dereferenced before it is null checked, hence there
is a potential null pointer dereference.

Fix this by null checking pnvme_lport before it is dereferenced.

Addresses-Coverity-ID: 1423709 ("Dereference before null check")
Fixes: b7672ae681f8 ("scsi: lpfc: Fix crash in lpfc_nvme_fcp_io_submit during 
LIP")
Signed-off-by: Gustavo A. R. Silva 
---
Also, I wonder if the right pointer to check at line:

if (!pnvme_rport || !freqpriv) {

is pnvme_fcreq instead of freqpriv

 drivers/scsi/lpfc/lpfc_nvme.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index 517ae57..68cba7d 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -1251,6 +1251,11 @@ lpfc_nvme_fcp_io_submit(struct nvme_fc_local_port 
*pnvme_lport,
uint64_t start = 0;
 #endif
 
+   if (!pnvme_lport) {
+   ret = -ENODEV;
+   goto out_fail;
+   }
+
lport = (struct lpfc_nvme_lport *)pnvme_lport->private;
vport = lport->vport;
phba = vport->phba;
@@ -1261,7 +1266,7 @@ lpfc_nvme_fcp_io_submit(struct nvme_fc_local_port 
*pnvme_lport,
}
 
/* Validate pointers. */
-   if (!pnvme_lport || !pnvme_rport || !freqpriv) {
+   if (!pnvme_rport || !freqpriv) {
lpfc_printf_vlog(vport, KERN_INFO, LOG_NVME_IOERR | LOG_NODE,
 "6117 No Send:IO submit ptrs NULL, lport %p, "
 "rport %p fcreq_priv %p\n",
-- 
2.7.4



4.13.12: frequent AACraid crash with a drive

2017-11-27 Thread Marc MERLIN
Howdy,

https://www.kernel.org/doc/Documentation/scsi/aacraid.txt
mentions that I should Email here.

This is not a new problem with a new kernel, I just switched to an
adaptec 16 port SATA card, and one of the drives (apparently always the
same) is causing the card to crash apparently due to not handling a
communication fault.

Adaptec aacraid driver 1.2.1[50834]-custom
aacraid :02:00.0: can't disable ASPM; OS doesn't have ASPM control
aacraid: Comm Interface type2 enabled
aacraid :02:00.0: 64 Bit DAC enabled
scsi host10: aacraid

I'm assuming it doesn't like my green drive:
Model Family: Western Digital Caviar Green
Device Model: WDC WD20EADS-00S2B0
Serial Number:WD-WCAVY1362941
LU WWN Device Id: 5 0014ee 258fc61f7
Firmware Version: 01.00A01
User Capacity:2,000,398,934,016 bytes [2.00 TB]

I have 5 of them (although they're not exactly the same) and only this
one seems to be causing problems.

when the kernel crashes, the drive does not get kicked out of the array, and 
things seem to work aftere reboot.
Time between reboots are nconsistent. Since last night:
Sun Nov 26 21:07:18 PST 2017
Sun Nov 26 21:25:38 PST 2017
Sun Nov 26 23:27:12 PST 2017
Mon Nov 27 02:20:47 PST 2017
Mon Nov 27 07:12:31 PST 2017
Mon Nov 27 07:12:31 PST 2017

Crash log:
aacraid: Host adapter abort request.
aacraid: Outstanding commands on (6,1,0,0):
aacraid: Host adapter reset request. SCSI hang ?
aacraid :02:00.0: outstanding cmd: midlevel-0
aacraid :02:00.0: outstanding cmd: lowlevel-0
aacraid :02:00.0: outstanding cmd: error handler-1
aacraid :02:00.0: outstanding cmd: firmware-0
aacraid :02:00.0: outstanding cmd: kernel-0
aacraid :02:00.0: Controller reset type is 3
aacraid :02:00.0: Issuing IOP reset
aacraid :02:00.0: IOP reset failed
aacraid :02:00.0: ARC Reset attempt failed
aacraid: Host adapter abort request.
aacraid: Outstanding commands on (6,1,0,0):
aacraid: Host adapter reset request. SCSI hang ?
aacraid :02:00.0: Adapter health - -3
aacraid :02:00.0: outstanding cmd: midlevel-0
aacraid :02:00.0: outstanding cmd: lowlevel-0
aacraid :02:00.0: outstanding cmd: error handler-1
aacraid :02:00.0: outstanding cmd: firmware-0
aacraid :02:00.0: outstanding cmd: kernel-3
[ cut here ]
WARNING: CPU: 6 PID: 366 at kernel/kthread.c:71 to_kthread+0xa/0x15
Modules linked in: veth ip6table_filter ip6_tables ebtable_nat ebtables ppdev 
lp xt_addrtype br_netfilter bridge stp llc tun autofs4 softdog binfmt_misc 
ftdi_sio nfsd auth_rpcgss nfs_acl nfs lockd grace fscache sunrpc ipt_REJECT 
nf_reject_ipv4 xt_conntrack xt_mark xt_nat xt_tcpudp nf_log_ipv4 nf_log_common 
xt_LOG iptable_mangle iptable_filter lm85 hwmon_vid pl2303 dm_snapshot dm_bufio 
iptable_nat ip_tables nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 
nf_conntrack_ftp ipt_MASQUERADE nf_nat_masquerade_ipv4 nf_nat nf_conntrack 
x_tables sg st snd_pcm_oss snd_mixer_oss bcache kvm_intel kvm irqbypass 
eeepc_wmi snd_cmipci asus_wmi snd_mpu401_uart sparse_keymap snd_opl3_lib rfkill 
snd_rawmidi asix snd_hda_codec_realtek snd_hda_codec_generic tpm_infineon 
tpm_tis usbnet wmi_bmof hwmon tpm_tis_core
 snd_seq_device usbserial libphy wmi tpm lpc_ich battery i915 rc_ati_x10 
snd_hda_intel snd_hda_codec ati_remote snd_hda_core i2c_i801 snd_hwdep pcspkr 
snd_pcm input_leds mei_me rc_core snd_timer parport_pc parport evdev snd 
soundcore e1000e ptp pps_core fuse raid456 multipath mmc_block mmc_core lrw 
ablk_helper dm_crypt dm_mod async_raid6_recov async_pq async_xor async_memcpy 
async_tx crc32c_intel blowfish_x86_64 blowfish_common pcbc aesni_intel 
aes_x86_64 crypto_simd glue_helper cryptd xhci_pci ehci_pci xhci_hcd ehci_hcd 
r8169 aacraid sata_sil24 usbcore mii thermal fan [last unloaded: ftdi_sio]
CPU: 6 PID: 366 Comm: scsi_eh_6 Tainted: G U  
4.13.12-amd64-stkreg-sysrq-20171018 #2
Hardware name: System manufacturer System Product Name/P8H67-M PRO, BIOS 3904 
04/27/2013
task: 9830b57da000 task.stack: a804c3cbc000
RIP: 0010:to_kthread+0xa/0x15
RSP: 0018:a804c3cbfb90 EFLAGS: 00210246
RAX: 016e RBX: 9828dbc8c180 RCX: 
RDX: 9828dbc8c180 RSI: 00200286 RDI: 9828dbc8c180
RBP: a804c3cbfb90 R08:  R09: 
R10: a804c3cbfbd8 R11: 94899a20 R12: 9830b9f88000
R13: 0002 R14: 9830b9f88000 R15: 0003
FS:  () GS:9830de38() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: df76a000 CR3: 000143c09000 CR4: 001406e0
Call Trace:
 kthread_stop+0x53/0xf4
 aac_reset_adapter+0x186/0x700 [aacraid]
 aac_eh_reset+0x396/0x3aa [aacraid]
 scsi_try_host_reset+0x5d/0xb1
 scsi_send_eh_cmnd+0x296/0x2dc
 ? __call_rcu.constprop.44+0x10d/0x188
 ? schedule_timeout+0xca/0x101
 scsi_eh_try_stu+0x53/0x7a
 scsi_eh_test_devices+0xcd/0x16e
 scsi_eh_ready_devs+0x824/0x8c4
 scsi_error_handler+0x291/0x523

Re: 4.13.12: frequent AACraid crash with a drive

2017-11-27 Thread Marc MERLIN
On Mon, Nov 27, 2017 at 07:21:47AM -0800, Marc MERLIN wrote:
> Crash log:

I should add that sometimes the reset works ok:
aacraid: Host adapter abort request.
aacraid: Outstanding commands on (10,1,0,0):
aacraid: Host adapter reset request. SCSI hang ?
aacraid :02:00.0: outstanding cmd: midlevel-0
aacraid :02:00.0: outstanding cmd: lowlevel-0
aacraid :02:00.0: outstanding cmd: error handler-1
aacraid :02:00.0: outstanding cmd: firmware-0
aacraid :02:00.0: outstanding cmd: kernel-0
aacraid :02:00.0: Controller reset type is 3
aacraid :02:00.0: Issuing IOP reset
aacraid :02:00.0: IOP reset succeded
aacraid: Comm Interface type2 enabled
aacraid :02:00.0: Issuing bus rescan
sd 10:1:0:0: [sdd] tag#0 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_TIMEOUT
sd 10:1:0:0: [sdd] tag#0 CDB: ATA command pass through(16) 85 06 20 00 d8 00 00 
00 00 00 4f 00 c2 00 b0 00

The drive in question seems ok when I test it directly, but it's hard to tell 
for sure.

Marc
-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems 
   what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/  


News UBSAN warnings in aacraid

2017-11-27 Thread Meelis Roos
Tried 4.15-rc1 on an old 32-bit HP Netserver with aacraid card. Compared 
to 4.14, there are new UBSAN warnings with timer related backtraces, so 
the timespec64 change seems suspicious:

[   12.228058] 

[   12.228155] UBSAN: Undefined behaviour in 
drivers/scsi/aacraid/commsup.c:2514:49
[   12.228229] signed integer overflow:
[   12.228283] 964297611 * 250 cannot be represented in type 'long int'
[   12.228347] CPU: 1 PID: 276 Comm: aacraid Not tainted 4.15.0-rc1 #80
[   12.228404] Hardware name: Hewlett Packard HP NetServer/HP System Board, 
BIOS 4.06.46 PW 06/25/2003
[   12.228477] Call Trace:
[   12.228560]  dump_stack+0x48/0x65
[   12.228620]  ubsan_epilogue+0xe/0x40
[   12.228677]  handle_overflow+0xad/0xc0
[   12.228754]  ? del_timer_sync+0x39/0x50
[   12.228818]  ? __getnstimeofday64+0x4d/0x200
[   12.228877]  __ubsan_handle_mul_overflow+0x12/0x20
[   12.229037]  aac_command_thread+0x1243/0x1290 [aacraid]
[   12.229109]  ? pick_next_task_fair+0x27f/0x760
[   12.229175]  ? __schedule+0x1b1/0x8e0
[   12.229245]  ? wake_up_q+0xa0/0xa0
[   12.229311]  kthread+0x13d/0x1f0
[   12.229390]  ? aac_send_hosttime+0xf0/0xf0 [aacraid]
[   12.229449]  ? __kthread_create_worker+0x110/0x110
[   12.229516]  ret_from_fork+0x19/0x24
[   12.229571] 

[   12.292055] 

[   12.292130] UBSAN: Undefined behaviour in 
drivers/scsi/aacraid/commsup.c:2515:7
[   12.292200] signed integer overflow:
[   12.292252] 1734571608 + 5 cannot be represented in type 'long int'
[   12.292312] CPU: 1 PID: 276 Comm: aacraid Not tainted 4.15.0-rc1 #80
[   12.292368] Hardware name: Hewlett Packard HP NetServer/HP System Board, 
BIOS 4.06.46 PW 06/25/2003
[   12.292439] Call Trace:
[   12.292494]  dump_stack+0x48/0x65
[   12.292550]  ubsan_epilogue+0xe/0x40
[   12.292606]  handle_overflow+0xad/0xc0
[   12.292665]  ? del_timer_sync+0x39/0x50
[   12.292722]  ? __getnstimeofday64+0x4d/0x200
[   12.292780]  __ubsan_handle_add_overflow+0x12/0x20
[   12.292861]  aac_command_thread+0x1259/0x1290 [aacraid]
[   12.292922]  ? pick_next_task_fair+0x27f/0x760
[   12.292980]  ? __schedule+0x1b1/0x8e0
[   12.293037]  ? wake_up_q+0xa0/0xa0
[   12.293094]  kthread+0x13d/0x1f0
[   12.293172]  ? aac_send_hosttime+0xf0/0xf0 [aacraid]
[   12.293231]  ? __kthread_create_worker+0x110/0x110
[   12.293289]  ret_from_fork+0x19/0x24
[   12.293345] 



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


Re: UFS utilities

2017-11-27 Thread Bart Van Assche
On Mon, 2017-11-27 at 11:25 +, Bean Huo (beanhuo) wrote:
> Is there someone knows if exists one utilis dedicated to UFS device,
> rather than SCSI utils? I have tried sg3-utils, but it is not convenient
> for the embedded ARM-based system.

Hello Bean,

Please be more specific. What is inconvenient about sg3_utils on embedded
ARM systems?

> And also it doesn't support several UFS special command. 

Are you referring to SCSI commands or rather to UFS commands that fall
outside the SCSI spec? Anyway, an approach that is used by many SCSI drivers
to export information to user space that falls outside the SCSI spec is to
create additional sysfs attributes. See also the sdev_attrs and shost_attrs
members of struct scsi_host_template.

Bart.

Re: News UBSAN warnings in aacraid

2017-11-27 Thread Arnd Bergmann
On Mon, Nov 27, 2017 at 8:17 PM, Meelis Roos  wrote:
> Tried 4.15-rc1 on an old 32-bit HP Netserver with aacraid card. Compared
> to 4.14, there are new UBSAN warnings with timer related backtraces, so
> the timespec64 change seems suspicious:

> [   12.228155] UBSAN: Undefined behaviour in 
> drivers/scsi/aacraid/commsup.c:2514:49
> [   12.228229] signed integer overflow:
> [   12.228283] 964297611 * 250 cannot be represented in type 'long int'

Thanks for reporting it! For reference, this is my change that got applied to
aac_command_thread:

@@ -2496,7 +2496,7 @@ int aac_command_thread(void *data)
}
if (!time_before(next_check_jiffies,next_jiffies)
 && ((difference = next_jiffies - jiffies) <= 0)) {
-   struct timeval now;
+   struct timespec64 now;
int ret;

/* Don't even try to talk to adapter if its sick */
@@ -2506,15 +2506,15 @@ int aac_command_thread(void *data)
next_check_jiffies = jiffies
   + ((long)(unsigned)check_interval)
   * HZ;
-   do_gettimeofday(&now);
+   ktime_get_real_ts64(&now);

/* Synchronize our watches */
-   if (((100 - (100 / HZ)) > now.tv_usec)
-&& (now.tv_usec > (100 / HZ)))
-   difference = (((100 - now.tv_usec) * HZ)
- + 50) / 100;
+   if (((NSEC_PER_SEC - (NSEC_PER_SEC / HZ)) > now.tv_nsec)
+&& (now.tv_nsec > (NSEC_PER_SEC / HZ)))
+   difference = (((NSEC_PER_SEC -
now.tv_nsec) * HZ)
+ + NSEC_PER_SEC / 2) / NSEC_PER_SEC;
else {
-   if (now.tv_usec > 50)
+   if (now.tv_nsec > NSEC_PER_SEC / 2)
++now.tv_sec;

if (dev->sa_firmware)

The problem is that a microsecond number (0 to 99) multiplied by
HZ (100 to 1024) always fits in a 32-bit integer, but the nanosecond
number doesn't.

We could make that a 64-bit division, but that would be fairly expensive.

I'm trying to understand the bigger picture now, rather than simply
attempting to do a simple conversion, but I don't see what we are
actually trying to compute in 'difference' here.

I think this chunk would solve the problem and result in the
same behavior as before:

--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -2511,8 +2511,8 @@ int aac_command_thread(void *data)
/* Synchronize our watches */
if (((NSEC_PER_SEC - (NSEC_PER_SEC / HZ)) > now.tv_nsec)
 && (now.tv_nsec > (NSEC_PER_SEC / HZ)))
-   difference = (((NSEC_PER_SEC -
now.tv_nsec) * HZ)
- + NSEC_PER_SEC / 2) / NSEC_PER_SEC;
+   difference = HZ + HZ / 2 -
+now.tv_nsec / (NSEC_PER_SEC / HZ);
else {
if (now.tv_nsec > NSEC_PER_SEC / 2)
++now.tv_sec;

but I don't see why we add in half a second here. Any ideas?

   Arnd


Re: [PATCH] Ensure that the SCSI error handler gets woken up

2017-11-27 Thread Bart Van Assche
On Thu, 2017-11-23 at 09:18 +0100, Christoph Hellwig wrote:
> > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> > index 5e89049e9b4e..f7f014c755d7 100644
> > --- a/drivers/scsi/scsi_error.c
> > +++ b/drivers/scsi/scsi_error.c
> > @@ -61,9 +61,10 @@ static int scsi_eh_try_stu(struct scsi_cmnd *scmd);
> >  static int scsi_try_to_abort_cmd(struct scsi_host_template *,
> >  struct scsi_cmnd *);
> >  
> > -/* called with shost->host_lock held */
> >  void scsi_eh_wakeup(struct Scsi_Host *shost)
> >  {
> > +   lockdep_assert_held(shost->host_lock);
> > +
> > if (atomic_read(&shost->host_busy) == shost->host_failed) {
> > trace_scsi_eh_wakeup(shost);
> > wake_up_process(shost->ehandler);
> 
> Can you split this comment to assert change into a separate patch, please?

Hello Christoph.

Thanks for the review. I will make the requested change.

Bart.

Re: [PATCH] Ensure that the SCSI error handler gets woken up

2017-11-27 Thread Stuart Hayes
For what it's worth, this does not fix the problem that both Pavel's original 
patch (https://patchwork.kernel.org/patch/9938919/) and the patch I submitted 
(https://patchwork.kernel.org/patch/10067059/) would fix.  I verified that 
this patch still fails on my system.

The only problem I am able to reproduce where the error handler doesn't get 
woken up is when scsi_eh_scmd_add() and scsi_device_unbusy() are running at 
the same time on different CPUs... scsi_eh_scmd_add() increments host_failed 
and checks host_busy, while scsi_device_unbusy() decrements host_busy and 
checks host_failed, and scsi_device_unbusy() does that when it does not hold 
the spin lock, and there's no smp_mb(), so they can each see stale values 
and neither will actually wake the error handler.

Could you modify this patch to make scsi_dec_host_busy() get the spin lock 
right before checking host_failed instead of right after, like Pavel's patch, 
to protect against this?

Thanks
Stuart

On 11/22/2017 7:05 PM, Bart Van Assche wrote:
> If scsi_eh_scmd_add() is called concurrently with scsi_host_queue_ready()
> while shost->host_blocked > 0 then it can happen that neither function
> wakes up the SCSI error handler. Fix this by making every function that
> decreases the host_busy counter to wake up the error handler if necessary.
> 
> Reported-by: Pavel Tikhomirov 
> Fixes: commit 746650160866 ("scsi: convert host_busy to atomic_t")
> Signed-off-by: Bart Van Assche 
> Cc: Konstantin Khorenko 
> Cc: Stuart Hayes 
> Cc: Christoph Hellwig 
> Cc: Hannes Reinecke 
> Cc: Johannes Thumshirn 
> Cc: 
> ---
>  drivers/scsi/scsi_error.c |  3 ++-
>  drivers/scsi/scsi_lib.c   | 22 ++
>  2 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 5e89049e9b4e..f7f014c755d7 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -61,9 +61,10 @@ static int scsi_eh_try_stu(struct scsi_cmnd *scmd);
>  static int scsi_try_to_abort_cmd(struct scsi_host_template *,
>struct scsi_cmnd *);
>  
> -/* called with shost->host_lock held */
>  void scsi_eh_wakeup(struct Scsi_Host *shost)
>  {
> + lockdep_assert_held(shost->host_lock);
> +
>   if (atomic_read(&shost->host_busy) == shost->host_failed) {
>   trace_scsi_eh_wakeup(shost);
>   wake_up_process(shost->ehandler);
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 1e05e1885ac8..abd37d77af2d 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -318,22 +318,28 @@ static void scsi_init_cmd_errh(struct scsi_cmnd *cmd)
>   cmd->cmd_len = scsi_command_size(cmd->cmnd);
>  }
>  
> -void scsi_device_unbusy(struct scsi_device *sdev)
> +static void scsi_dec_host_busy(struct Scsi_Host *shost)
>  {
> - struct Scsi_Host *shost = sdev->host;
> - struct scsi_target *starget = scsi_target(sdev);
>   unsigned long flags;
>  
>   atomic_dec(&shost->host_busy);
> - if (starget->can_queue > 0)
> - atomic_dec(&starget->target_busy);
> -
>   if (unlikely(scsi_host_in_recovery(shost) &&
>(shost->host_failed || shost->host_eh_scheduled))) {
>   spin_lock_irqsave(shost->host_lock, flags);
>   scsi_eh_wakeup(shost);
>   spin_unlock_irqrestore(shost->host_lock, flags);
>   }
> +}
> +
> +void scsi_device_unbusy(struct scsi_device *sdev)
> +{
> + struct Scsi_Host *shost = sdev->host;
> + struct scsi_target *starget = scsi_target(sdev);
> +
> + scsi_dec_host_busy(shost);
> +
> + if (starget->can_queue > 0)
> + atomic_dec(&starget->target_busy);
>  
>   atomic_dec(&sdev->device_busy);
>  }
> @@ -1532,7 +1538,7 @@ static inline int scsi_host_queue_ready(struct 
> request_queue *q,
>   list_add_tail(&sdev->starved_entry, &shost->starved_list);
>   spin_unlock_irq(shost->host_lock);
>  out_dec:
> - atomic_dec(&shost->host_busy);
> + scsi_dec_host_busy(shost);
>   return 0;
>  }
>  
> @@ -2020,7 +2026,7 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx 
> *hctx,
>   return BLK_STS_OK;
>  
>  out_dec_host_busy:
> -   atomic_dec(&shost->host_busy);
> + scsi_dec_host_busy(shost);
>  out_dec_target_busy:
>   if (scsi_target(sdev)->can_queue > 0)
>   atomic_dec(&scsi_target(sdev)->target_busy);
> 

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



[PATCH] scsi: scsi_devinfo: handle non-terminated strings

2017-11-27 Thread Martin Wilck
devinfo->vendor and devinfo->model aren't necessarily
zero-terminated.

Fixes: b8018b973c7c "scsi_devinfo: fixup string compare"
Signed-off-by: Martin Wilck 
---
 drivers/scsi/scsi_devinfo.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index fe5a9ea27b5e..d6db5697472e 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -458,7 +458,8 @@ static struct scsi_dev_info_list 
*scsi_dev_info_list_find(const char *vendor,
/*
 * vendor strings must be an exact match
 */
-   if (vmax != strlen(devinfo->vendor) ||
+   if (vmax != strnlen(devinfo->vendor,
+   sizeof(devinfo->vendor)) ||
memcmp(devinfo->vendor, vskip, vmax))
continue;
 
@@ -466,7 +467,7 @@ static struct scsi_dev_info_list 
*scsi_dev_info_list_find(const char *vendor,
 * @model specifies the full string, and
 * must be larger or equal to devinfo->model
 */
-   mlen = strlen(devinfo->model);
+   mlen = strnlen(devinfo->model, sizeof(devinfo->model));
if (mmax < mlen || memcmp(devinfo->model, mskip, mlen))
continue;
return devinfo;
-- 
2.15.0



[PATCH] scsi_devinfo: cleanly zero-pad devinfo strings

2017-11-27 Thread Martin Wilck
Cleanly fill memory for "vendor" and "model" with 0-bytes for
the "compatible" case rather than adding only a single 0 byte.
This simplifies the devinfo code a a bit, and avoids mistakes
in other places of the code (not in current upstream, but we
had one such mistake in the SUSE kernel).

Signed-off-by: Martin Wilck 
---
 drivers/scsi/scsi_devinfo.c | 23 +++
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index d6db5697472e..6e784a09b21a 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -34,7 +34,6 @@ struct scsi_dev_info_list_table {
 };
 
 
-static const char spaces[] = ""; /* 16 of them */
 static unsigned scsi_default_dev_flags;
 static LIST_HEAD(scsi_dev_info_list);
 static char scsi_dev_flags[256];
@@ -298,21 +297,13 @@ static void scsi_strcpy_devinfo(char *name, char *to, 
size_t to_length,
size_t from_length;
 
from_length = strlen(from);
-   strncpy(to, from, min(to_length, from_length));
-   if (from_length < to_length) {
-   if (compatible) {
-   /*
-* NUL terminate the string if it is short.
-*/
-   to[from_length] = '\0';
-   } else {
-   /*
-* space pad the string if it is short.
-*/
-   strncpy(&to[from_length], spaces,
-   to_length - from_length);
-   }
-   }
+   /* This zero-pads the destination */
+   strncpy(to, from, to_length);
+   if (from_length < to_length && !compatible)
+   /*
+* space pad the string if it is short.
+*/
+   memset(&to[from_length], ' ', to_length - from_length);
if (from_length > to_length)
 printk(KERN_WARNING "%s: %s string '%s' is too long\n",
__func__, name, from);
-- 
2.15.0



Re: [PATCH] Ensure that the SCSI error handler gets woken up

2017-11-27 Thread Bart Van Assche
On Mon, 2017-11-27 at 15:53 -0600, Stuart Hayes wrote:
> Could you modify this patch to make scsi_dec_host_busy() get the spin lock 
> right before checking host_failed instead of right after, like Pavel's patch, 
> to protect against this?

Hello Stuart,

Thanks for the feedback. I will look into this.

Bart.

[PATCH] libsas: flush pending destruct work in sas_unregister_domain_devices()

2017-11-27 Thread Cong Wang
We saw dozens of the following kernel waring:

 WARNING: CPU: 0 PID: 705 at fs/sysfs/group.c:224 sysfs_remove_group+0x54/0x88()
 sysfs group 81ab7670 not found for kobject '6:0:3:0'
 Modules linked in: cpufreq_ondemand x86_pkg_temp_thermal coretemp kvm_intel 
kvm microcode raid0 iTCO_wdt iTCO_vendor_support sb_edac edac_core lpc_ich 
mfd_core ioatdma i2c_i801 shpchp wmi hed acpi_cpufreq lp parport tcp_diag 
inet_diag ipmi_si ipmi_devintf ipmi_msghandler sch_fq_codel igb ptp pps_core 
i2c_algo_bit i2c_core crc32c_intel isci libsas scsi_transport_sas dca ipv6
 CPU: 0 PID: 705 Comm: kworker/u240:0 Not tainted 4.1.35.el7.x86_64 #1
 Hardware name: WIWYNN Lyra/JD/S2600GZ, BIOS 
SE5C600.86B.02.03.2004.030620151456 03/06/2015
 Workqueue: scsi_wq_6 sas_destruct_devices [libsas]
   88056c393ba8 81544a6d 88056c393bf8
  0009 88056c393be8 81069b4c 88081790d078
  811dad37  81ab7670 88081b29dc10
 Call Trace:
  [] dump_stack+0x4d/0x63
  [] warn_slowpath_common+0xa1/0xbb
  [] ? sysfs_remove_group+0x54/0x88
  [] warn_slowpath_fmt+0x46/0x48
  [] ? kernfs_find_and_get_ns+0x4d/0x58
  [] sysfs_remove_group+0x54/0x88
  [] dpm_sysfs_remove+0x50/0x55
  [] device_del+0x47/0x1ec
  [] ? mutex_unlock+0x16/0x18
  [] device_unregister+0x48/0x54
  [] bsg_unregister_queue+0x5f/0x86
  [] __scsi_remove_device+0x3a/0xc3
  [] scsi_remove_device+0x26/0x33
  [] scsi_remove_target+0x134/0x19b
  [] sas_rphy_remove+0x2c/0x72 [scsi_transport_sas]
  [] sas_rphy_delete+0x13/0x1f [scsi_transport_sas]
  [] sas_destruct_devices+0x58/0x79 [libsas]
  [] process_one_work+0x19b/0x2d1
  [] worker_thread+0x1dd/0x2bb
  [] ? cancel_delayed_work+0x72/0x72
  [] kthread+0xa5/0xad
  [] ? task_work_add+0xd/0x53
  [] ? __kthread_parkme+0x61/0x61
  [] ret_from_fork+0x42/0x70
  [] ? __kthread_parkme+0x61/0x61

It looks like we don't wait for sas destruct work properly
on tear down path, at least sas_deform_port() calls
sas_unregister_domain_devices() to schedule destruct work
to a workqueue and then calls sas_port_delete() to remove
the related sysfs files concurrently.

Dan tried to fix this with a different way:

 https://patchwork.kernel.org/patch/6450921/

but that patch is never applied. I take a better approach
as suggested by Johannes, that is waiting for pending destruct
work to remove child sysfs files and then removing the parent
sysfs files.

Cc: Dan Williams 
Cc: Johannes Thumshirn 
Cc: Praveen Murali 
Cc: "James E.J. Bottomley" 
Cc: "Martin K. Petersen" 
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Cong Wang 
---
 drivers/scsi/libsas/sas_discover.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/libsas/sas_discover.c 
b/drivers/scsi/libsas/sas_discover.c
index 60de66252fa2..27c11fc7aa2b 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -388,6 +388,11 @@ void sas_unregister_dev(struct asd_sas_port *port, struct 
domain_device *dev)
}
 }
 
+static void sas_flush_work(struct asd_sas_port *port)
+{
+   scsi_flush_work(port->ha->core.shost);
+}
+
 void sas_unregister_domain_devices(struct asd_sas_port *port, int gone)
 {
struct domain_device *dev, *n;
@@ -401,8 +406,8 @@ void sas_unregister_domain_devices(struct asd_sas_port 
*port, int gone)
list_for_each_entry_safe(dev, n, &port->disco_list, disco_list_node)
sas_unregister_dev(port, dev);
 
+   sas_flush_work(port);
port->port->rphy = NULL;
-
 }
 
 void sas_device_set_phy(struct domain_device *dev, struct sas_port *port)
-- 
2.13.0



[PATCH 1/2] scsi: ufs: add some definition included in UFS HCI specifications

2017-11-27 Thread 김기웅
These would be used in the future in some specific drivers.

Signed-off-by: Kiwoong Kim 
---
 drivers/scsi/ufs/ufshci.h | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index 277752b0fc6f..1a1b5d9fe514 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -157,6 +157,8 @@ enum {
 #define UTP_TRANSFER_REQ_LIST_READY0x2
 #define UTP_TASK_REQ_LIST_READY0x4
 #define UIC_COMMAND_READY  0x8
+#define HOST_ERROR_INDICATOR   0x10
+#define DEVICE_ERROR_INDICATOR 0x20
 #define UIC_POWER_MODE_CHANGE_REQ_STATUS_MASK  UFS_MASK(0x7, 8)
 
 #define UFSHCD_STATUS_READY(UTP_TRANSFER_REQ_LIST_READY |\
@@ -185,6 +187,10 @@ enum {
 /* UECDL - Host UIC Error Code Data Link Layer 3Ch */
 #define UIC_DATA_LINK_LAYER_ERROR  0x8000
 #define UIC_DATA_LINK_LAYER_ERROR_CODE_MASK0x7FFF
+#define UIC_DATA_LINK_LAYER_ERROR_TCX_REP_TIMER_EXP0x2
+#define UIC_DATA_LINK_LAYER_ERROR_AFCX_REQ_TIMER_EXP   0x4
+#define UIC_DATA_LINK_LAYER_ERROR_FCX_PRO_TIMER_EXP0x8
+#define UIC_DATA_LINK_LAYER_ERROR_RX_BUF_OF0x20
 #define UIC_DATA_LINK_LAYER_ERROR_PA_INIT  0x2000
 #define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED 0x0001
 #define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT 0x0002
@@ -192,10 +198,20 @@ enum {
 /* UECN - Host UIC Error Code Network Layer 40h */
 #define UIC_NETWORK_LAYER_ERROR0x8000
 #define UIC_NETWORK_LAYER_ERROR_CODE_MASK  0x7
+#define UIC_NETWORK_UNSUPPORTED_HEADER_TYPE0x1
+#define UIC_NETWORK_BAD_DEVICEID_ENC   0x2
+#define UIC_NETWORK_LHDR_TRAP_PACKET_DROPPING  0x4
 
 /* UECT - Host UIC Error Code Transport Layer 44h */
 #define UIC_TRANSPORT_LAYER_ERROR  0x8000
 #define UIC_TRANSPORT_LAYER_ERROR_CODE_MASK0x7F
+#define UIC_TRANSPORT_UNSUPPORTED_HEADER_TYPE  0x1
+#define UIC_TRANSPORT_UNKNOWN_CPORTID  0x2
+#define UIC_TRANSPORT_NO_CONNECTION_RX 0x4
+#define UIC_TRANSPORT_CONTROLLED_SEGMENT_DROPPING  0x8
+#define UIC_TRANSPORT_BAD_TC   0x10
+#define UIC_TRANSPORT_E2E_CREDIT_OVERFOW   0x20
+#define UIC_TRANSPORT_SAFETY_VALUE_DROPPING0x40
 
 /* UECDME - Host UIC Error Code DME 48h */
 #define UIC_DME_ERROR  0x8000
-- 
2.11.0



[PATCH 2/2] scsi: ufs: add Exynos-specific driver

2017-11-27 Thread 김기웅
This driver is to use UFS devices on Exynos SoC and
has been already used for many years for commercial products.

Signed-off-by: Kiwoong Kim 
---
 drivers/scsi/ufs/Kconfig  |  10 +
 drivers/scsi/ufs/Makefile |   1 +
 drivers/scsi/ufs/ufs-exynos.c | 962 ++
 drivers/scsi/ufs/ufs-exynos.h | 351 +++
 drivers/scsi/ufs/ufshcd.h |   1 +
 5 files changed, 1325 insertions(+)
 create mode 100644 drivers/scsi/ufs/ufs-exynos.c
 create mode 100644 drivers/scsi/ufs/ufs-exynos.h

diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
index e27b4d4e6ae2..7d71ad8768c3 100644
--- a/drivers/scsi/ufs/Kconfig
+++ b/drivers/scsi/ufs/Kconfig
@@ -100,3 +100,13 @@ config SCSI_UFS_QCOM
 
  Select this if you have UFS controller on QCOM chipset.
  If unsure, say N.
+
+config SCSI_UFS_EXYNOS
+   tristate "EXYNOS UFS Host Controller Driver"
+   depends on SCSI_UFSHCD && SCSI_UFSHCD_PLATFORM
+   ---help---
+ This selects the EXYNOS UFS host controller driver.
+
+ If you have a controller with this interface, say Y or M here.
+
+ If unsure, say N.
diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
index 9310c6c83041..3312b052dcff 100644
--- a/drivers/scsi/ufs/Makefile
+++ b/drivers/scsi/ufs/Makefile
@@ -3,6 +3,7 @@
 obj-$(CONFIG_SCSI_UFS_DWC_TC_PCI) += tc-dwc-g210-pci.o ufshcd-dwc.o 
tc-dwc-g210.o
 obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o 
tc-dwc-g210.o
 obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
+obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o
 obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
 obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
 obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
diff --git a/drivers/scsi/ufs/ufs-exynos.c b/drivers/scsi/ufs/ufs-exynos.c
new file mode 100644
index ..98e5aeb80b06
--- /dev/null
+++ b/drivers/scsi/ufs/ufs-exynos.c
@@ -0,0 +1,962 @@
+/*
+ * UFS Host Controller driver for Exynos specific extensions
+ *
+ * Copyright (C) 2013-2014 Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "ufshcd.h"
+#include "ufshcd-pltfrm.h"
+#include "ufs-exynos.h"
+#include 
+#include 
+#include 
+
+/*
+ * Debugging information, SFR/attributes/misc
+ */
+static struct exynos_ufs *ufs_host_backup[1];
+static int ufs_host_index = 0;
+
+static struct exynos_ufs_sfr_log ufs_log_std_sfr[] = {
+   {"CAPABILITIES" ,   REG_CONTROLLER_CAPABILITIES,
0},
+   {"UFS VERSION"  ,   REG_UFS_VERSION,
0},
+   {"PRODUCT ID"   ,   REG_CONTROLLER_DEV_ID,  
0},
+   {"MANUFACTURE ID"   ,   REG_CONTROLLER_PROD_ID, 
0},
+   {"INTERRUPT STATUS" ,   REG_INTERRUPT_STATUS,   
0},
+   {"INTERRUPT ENABLE" ,   REG_INTERRUPT_ENABLE,   
0},
+   {"CONTROLLER STATUS",   REG_CONTROLLER_STATUS,  
0},
+   {"CONTROLLER ENABLE",   REG_CONTROLLER_ENABLE,  
0},
+   {"UIC ERR PHY ADAPTER LAYER",   
REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER,   0},
+   {"UIC ERR DATA LINK LAYER"  ,   
REG_UIC_ERROR_CODE_DATA_LINK_LAYER, 0},
+   {"UIC ERR NETWORK LATER",   
REG_UIC_ERROR_CODE_NETWORK_LAYER,   0},
+   {"UIC ERR TRANSPORT LAYER"  ,   
REG_UIC_ERROR_CODE_TRANSPORT_LAYER, 0},
+   {"UIC ERR DME"  ,   REG_UIC_ERROR_CODE_DME, 
0},
+   {"UTP TRANSF REQ INT AGG CNTRL" ,   
REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL,   0},
+   {"UTP TRANSF REQ LIST BASE L"   ,   
REG_UTP_TRANSFER_REQ_LIST_BASE_L,   0},
+   {"UTP TRANSF REQ LIST BASE H"   ,   
REG_UTP_TRANSFER_REQ_LIST_BASE_H,   0},
+   {"UTP TRANSF REQ DOOR BELL" ,   REG_UTP_TRANSFER_REQ_DOOR_BELL, 
0},
+   {"UTP TRANSF REQ LIST CLEAR",   
REG_UTP_TRANSFER_REQ_LIST_CLEAR,0},
+   {"UTP TRANSF REQ LIST RUN STOP" ,   
REG_UTP_TRANSFER_REQ_LIST_RUN_STOP, 0},
+   {"UTP TASK REQ LIST BASE L" ,   REG_UTP_TASK_REQ_LIST_BASE_L,   
0},
+   {"UTP TASK REQ LIST BASE H" ,   REG_UTP_TASK_REQ_LIST_BASE_H,   
0},
+   {"UTP TASK REQ DOOR BELL"   ,   REG_UTP_TASK_REQ_DOOR_BELL, 
0},
+   {"UTP TASK REQ LIST CLEAR"  ,   REG_UTP_TASK_REQ_LIST_CLEAR,
0},
+   {"UTP TASK REQ LIST RUN STOP"   ,   REG_UTP_TASK_REQ_LIST_RUN_STOP, 
0},
+   {"UIC COMMAND"  ,   REG_UIC_COMMAND,