[PATCH] sd_zbc: Force use of READ16/WRITE16

[Damien Le Moal]

Normally, sd_read_capacity sets sdp->use_16_for_rw to 1 based on the
disk capacity so that READ16/WRITE16 are used for large drives.
However, for a zoned disk with RC_BASIS set to 0, the capacity reported
through READ_CAPACITY may be very small, leading to use_16_for_rw not being
set and READ10/WRITE10 commands being used, even after the actual zoned disk
capacity is corrected in sd_zbc_read_zones. This causes LBA offset overflow for
accesses beyond 2TB.

As the ZBC standard makes it mandatory for ZBC drives to support
the READ16/WRITE16 commands anyway, make sure that use_16_for_rw is set.

Signed-off-by: Damien Le Moal 
---
 drivers/scsi/sd_zbc.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 394ab49..92620c8 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -612,6 +612,10 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp,
if (ret)
goto err;
 
+   /* READ16/WRITE16 is mandatory for ZBC disks */
+   sdkp->device->use_16_for_rw = 1;
+   sdkp->device->use_10_for_rw = 0;
+
return 0;
 
 err:
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: iscsi: make mutex for target scanning and unbinding per-session

[Mike Christie]

On 11/10/2016 07:13 PM, Chris Leech wrote:
> On Thu, Nov 10, 2016 at 05:22:44PM -0600, Mike Christie wrote:
>> > On 11/07/2016 12:22 PM, Chris Leech wrote:
>>> > > Currently the iSCSI transport class synchronises target scanning and
>>> > > unbinding with a host level mutex.  For multi-session hosts (offloading
>>> > > iSCSI HBAs) connecting to storage arrays that may implement one
>>> > > target-per-lun, this can result in the target scan work for hundreds of
>>> > > sessions being serialized behind a single mutex.  With slow enough
>> > 
>> > Does this patch alone help or is there a scsi piece too?
>  
> I had this tested when working a hung task timeout issue at boot, and
> was told that it fixed the issue.  The exact situation may be more
> complex, I think it was only 128 sessions which is surprising that it
> would hit a 2 minute timeout.  But every backtrace was at this mutex.
> 

I think you are also hitting a issue where the normal scan time is
higher than usual and that might be a userspace bug. I am not sure how
the mutex patch helps, but I have not thought about it.

I think iscsid will scan the entire host during login. We only want
iscsi_sysfs_scan_host to scan the specific target for the session we
just logged into.

In the kernel we are setting SCSI_SCAN_RESCAN for the scan and userspace
did echo - - - > /scan for the entire host, so iscsi_user_scan is
going to rescan every target that is setup. So once you get to 127
sessions/targets it could be a loonngg scan of all 127 of them, and
target 128 is going to have to wait a lnnggg time for that mutex and
then also execute a long scan.

If you have userspace do the single target scan, it should execute
faster. I know that does not solve the serialization problem. You will
still have lots of targets waiting to be scanned.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] Fix: scsi: megaraid: reduce the scope of pending-list lock to avoid double lock

[Kashyap Desai]

> -Original Message-
> From: i...@itu.dk [mailto:i...@itu.dk]
> Sent: Monday, October 17, 2016 1:00 PM
> To: Jiri Kosina
> Cc: Kashyap Desai; Sumit Saxena; Uday Lingala; James E.J. Bottomley;
Martin K.
> Petersen; megaraidlinux@avagotech.com; linux-scsi@vger.kernel.org;
Iago
> Abal
> Subject: [PATCH] Fix: scsi: megaraid: reduce the scope of pending-list
lock to
> avoid double lock
>
> From: Iago Abal 
>
> The EBA code analyzer (https://github.com/models-team/eba) reported the
> following double lock:
>
> 1. In function `megaraid_reset_handler' at 2571;
> 2. take `>pend_list_lock' for the first time at 2602:
>
>// FIRST
>spin_lock_irqsave(PENDING_LIST_LOCK(adapter), flags);
>
> 3. enter the `list_for_each_entry_safe' loop at 2603;
> 4. call `megaraid_mbox_mm_done' at 2616;
> 5. call `megaraid_mbox_runpendq' at 3782;
> 6. take `>pend_list_lock' for the second time at 1892:
>
>// SECOND: DOUBLE LOCK !!!
>spin_lock_irqsave(PENDING_LIST_LOCK(adapter), flags);
>
> From my shallow understanding of the code (so please review carefully),
I think
> that it is not necessary to hold `PENDING_LIST_LOCK(adapter)' while
executing
> the body of the `list_for_each_entry_safe' loop. I assume this because
both
> `megaraid_mbox_mm_done' and `megaraid_dealloc_scb' are called from
> several places where, as far as I can tell, this lock is not hold. In
fact, as reported
> by EBA, at some point `megaraid_mbox_mm_done' will acquire this lock
again.
>
> Fixes: c005fb4fb2d2 ("[SCSI] megaraid_{mm,mbox}: fix a bug in reset
handler")
> Signed-off-by: Iago Abal 
> ---
>  drivers/scsi/megaraid/megaraid_mbox.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/scsi/megaraid/megaraid_mbox.c
> b/drivers/scsi/megaraid/megaraid_mbox.c
> index f0987f2..7f11898 100644
> --- a/drivers/scsi/megaraid/megaraid_mbox.c
> +++ b/drivers/scsi/megaraid/megaraid_mbox.c
> @@ -2603,6 +2603,7 @@ static DEF_SCSI_QCMD(megaraid_queue_command)
>   list_for_each_entry_safe(scb, tmp, >pend_list, list) {
>   list_del_init(>list);  // from pending list
>
> + spin_unlock_irqrestore(PENDING_LIST_LOCK(adapter), flags);
>   if (scb->sno >= MBOX_MAX_SCSI_CMDS) {
>   con_log(CL_ANN, (KERN_WARNING
>   "megaraid: IOCTL packet with %d[%d:%d] being
> reset\n", @@ -2630,6 +2631,7 @@ static
> DEF_SCSI_QCMD(megaraid_queue_command)
>
>   megaraid_dealloc_scb(adapter, scb);
>   }
> + spin_lock_irqsave(PENDING_LIST_LOCK(adapter), flags);
>   }
>   spin_unlock_irqrestore(PENDING_LIST_LOCK(adapter), flags);

Sorry for delay. We had internal discussion and confirm that it is safe to
remove mbox driver from mainline as this product is discontinued and we
are planning to post patch to remove megaraid mbox driver from mainline.

>
> --
> 1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Delivery Status Notification for linuxr...@lsi.com

[Kashyap Desai]

> -Original Message-
> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
> ow...@vger.kernel.org] On Behalf Of Paul Menzel
> Sent: Thursday, November 10, 2016 7:40 PM
> To: Martin K. Petersen
> Cc: linux-scsi@vger.kernel.org; James E.J. Bottomley
> Subject: Re: Delivery Status Notification for linuxr...@lsi.com
>
> Dear Martin,
>
>
> On 11/10/16 15:07, Martin K. Petersen wrote:
> >> "Paul" == Paul Menzel  writes:
>
> > Paul> Probably you know it already, but the listed email address of
> > Paul> the 3WARE SCSI drivers maintainer linuxr...@lsi.com doesn’t work
> > Paul> (for me).
> >
> > Ownership of these products is now with Broadcom. To my knowledge the
> > 3ware product lines have been discontinued.
>
> Indeed. I forgot to actually formulate the intend of my message.
>
> What should happen to the entry in the file `MAINTAINERS`?
3Ware product is discontinue from support point of view.  We are discussing
with LSI/Broadcom management team to clean up this part and provide more
clarity.
Please wait for reply as we are waiting for internal communication.
>
>
> Kind regards,
>
> Paul
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of
> a message to majord...@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [SCSI] mpt3sas: Fix secure erase premature termination (v4)

[Sreekanth Reddy]

On Thu, Nov 10, 2016 at 8:05 PM, Andrey Grodzovsky  wrote:
> Problem:
> This is a work around for a bug with LSI Fusion MPT SAS2 when
> pefroming secure erase. Due to the very long time the operation
> takes commands issued during the erase will time out and will trigger
> execution of abort hook. Even though the abort hook is called for
> the specific command which timed out this leads to entire device halt
> (scsi_state terminated) and premature termination of the secured erase.
>
> Fix:
> Set device state to busy while erase in progress to reject any incoming
> commands until the erase is done. The device is blocked any way during
> this time and cannot execute any other command.
> More data and logs can be found here -
> https://drive.google.com/file/d/0B9ocOHYHbbS1Q3VMdkkzeWFkTjg/view
>
> v2: Update according to example patch by Hannes Reinecke to apply
> the blocking logic to any ATA 12/16 command.
>
> v3: Use SCSI commands opcodes definitions instead of value and
> correct identation.
>
> v4: Fix checkpath errors and warning.
>
> Signed-off-by: Andrey Grodzovsky 
> Cc: 
> Cc: Sathya Prakash 
> Cc: Chaitra P B 
> Cc: Suganath Prabu Subramani 
> Cc: Sreekanth Reddy 
> Cc: Hannes Reinecke 
> Cc: 

Acked-by: Sreekanth Reddy 

> ---
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 16 
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
> b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 5a97e32..c032319 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -3500,6 +3500,10 @@ _scsih_eedp_error_handling(struct scsi_cmnd *scmd, u16 
> ioc_status)
> SAM_STAT_CHECK_CONDITION;
>  }
>
> +static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
> +{
> +   return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16);
> +}
>
>  /**
>   * _scsih_qcmd - main scsi request entry point
> @@ -3528,6 +3532,14 @@ _scsih_qcmd(struct Scsi_Host *shost, struct scsi_cmnd 
> *scmd)
> scsi_print_command(scmd);
>  #endif
>
> +   /**
> +   * Lock the device for any subsequent command until
> +   * command is done.
> +   */
> +   if (ata_12_16_cmd(scmd))
> +   scsi_internal_device_block(scmd->device);
> +
> +
> sas_device_priv_data = scmd->device->hostdata;
> if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
> scmd->result = DID_NO_CONNECT << 16;
> @@ -4062,6 +4074,10 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, 
> u8 msix_index, u32 reply)
> if (scmd == NULL)
> return 1;
>
> +   if (ata_12_16_cmd(scmd))
> +   scsi_internal_device_unblock(scmd->device, SDEV_RUNNING);
> +
> +
> mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
>
> if (mpi_reply == NULL) {
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Avoid that SCSI device removal through sysfs triggers a deadlock

[Eric W. Biederman]

James Bottomley  writes:

> On Tue, 2016-11-08 at 20:10 -0600, Eric W. Biederman wrote:
>> James Bottomley  writes:
>> 
>> > On Tue, 2016-11-08 at 18:57 -0600, Eric W. Biederman wrote:
> [...]
>> > > I am pretty certain that if you are going to make 
>> > > kernfs_remove_self and kernfs_remove_by_name_ns to be safe to 
>> > > race against each other, not just the KERNFS_SUICIDAL check, but 
>> > > the wait when KERNFS_SUICIDAL is set needs to be added ot
>> > > kernfs_remove_by_name_ns.
>> > 
>> > I don't think you can do that: waiting for SUICIDED would introduce
>> > another potential lock entanglement.  I'm reasonably happy that the
>> > deactivation offset coupled with kernfs_drain in the non self 
>> > remove path means that the necessary cleanup is done when the 
>> > directory itself is removed.  That seems to be a common pattern in 
>> > all non-self removes.
>> 
>> But if we don't I am pretty certain there will be asynchrounous 
>> behavior in some cases that could potentially confuse userspace.
>
> But the original behaviour kernfs_remove_self() eliminated was the
> asynchronous callback.  If we go back to that, we're definitely going
> to introduce far more asynchronous behaviour.

Not from a userspace perspective if we use task_work_add(current,...).

In that case we simply get to do an asynchrnous looking thing before we
return to userspace.  So an asynchronous form, but not actually
asynchronous actions.

>> Which is partly why I would like to kill kernfs_remove_self.
>
> I took a look at it.  It's definitely not cleanly revertible given
> what's gone on since.  Even just trying to excise it is going to be
> hard given all the tentacles it has.

Well changing the users and removing the code from kernfs and sysfs
doesn't look hard.  There are only 5-7 users of this insane and broken
remove self thing.

I think even if we do the naive thing and don't use any helpers in
kernfs, sysfs, or the device subsystem we could easily wind up with less
code in the kernel.  Certainly it will be code that is simpler and
easier to get right.

>> Using task_work_add(current, ...) as I posted earlier let's us retain
>> the synchronous property of the current API.
>> 
>> While we debate the details I am happy to look at scsi as a special 
>> case and solve for scsi.  Then when we have the details worked out we 
>> can go fix the other cases.  Given my preliminary patch in my last 
>> reply it looks very straight forward to fix this sanely.
>
> I don't think there's any urgency to fix SCSI.  You can only really
> trigger this by hammering the device and host remove paths, which isn't
> what users normally do ... as the fact it's been in the field for 2.5
> years with no apparent problems shows.

Sure little urgency.  But scsi makes a good concrete example for which
a reproducer is known.  So it is a good test ground.

> I'd like Greg and Tejun to weigh in on this before we start doing
> something, since they created the initial problem.

Fair.  Although I wouldn't be surprised if we don't hear anything short
of a concrete patch that changes everything.

Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] SCSI fixes for 4.9-rc3

[Gabriel C]

On 11.11.2016 04:30, Gabriel C wrote:
> 
> On 05.11.2016 14:29, James Bottomley wrote:
> 
> 
> ...
> 
>> Kashyap Desai (1):
>>   scsi: megaraid_sas: Fix data integrity failure for JBOD (passthrough) 
>> devices
>>
>> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c 
>> b/drivers/scsi/megaraid/megaraid_sas_base.c
>> index 9ff57de..d8b1fbd 100644
>> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
>> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
>> @@ -1700,16 +1700,13 @@ megasas_queue_command(struct Scsi_Host *shost, 
>> struct scsi_cmnd *scmd)
>>  goto out_done;
>>  }
>>  
>> -switch (scmd->cmnd[0]) {
>> -case SYNCHRONIZE_CACHE:
>> -/*
>> - * FW takes care of flush cache on its own
>> - * No need to send it down
>> - */
>> +/*
>> + * FW takes care of flush cache on its own for Virtual Disk.
>> + * No need to send it down for VD. For JBOD send SYNCHRONIZE_CACHE to 
>> FW.
>> + */
>> +if ((scmd->cmnd[0] == SYNCHRONIZE_CACHE) && MEGASAS_IS_LOGICAL(scmd)) {
>>  scmd->result = DID_OK << 16;
>>  goto out_done;
>> -default:
>> -break;
>>  }
>>  
>>  return instance->instancet->build_and_issue_cmd(instance, scmd);
> 
> This patch breaks my box.. I'm not able to boot it anymore.
> It seems with this patch I have /dev/sda[a-z] to /dev/sdz[a-z] ?!?
> 
> I'm not sure how to get an log since dracut times out and I'm dropped , after 
> a very long time
> of probing 'ghost devices', in a emercency shell, journalctl doesn't work 
> also..
> 
> After reverting this one I can boot normal.
> 
> Box is a FUJITSU PRIMERGY TX200 S5..
> 
> This is from an working kernel..
> 
> [5.119371] megaraid_sas :01:00.0: FW now in Ready state
> [5.119418] megaraid_sas :01:00.0: firmware supports msix: (0)
> [5.119420] megaraid_sas :01:00.0: current msix/online cpus  : 
> (1/16)
> [5.119422] megaraid_sas :01:00.0: RDPQ mode : (disabled)
> [5.123100] ehci-pci :00:1a.7: cache line size of 32 is not supported
> [5.123113] ehci-pci :00:1a.7: irq 18, io mem 0xb002
> 
> ...
> 
> [5.208063] megaraid_sas :01:00.0: controller type   : MR(256MB)
> [5.208065] megaraid_sas :01:00.0: Online Controller Reset(OCR)  : 
> Enabled
> [5.208067] megaraid_sas :01:00.0: Secure JBOD support   : No
> [5.208070] megaraid_sas :01:00.0: megasas_init_mfi: fw_support_ieee=0
> [5.208073] megaraid_sas :01:00.0: INIT adapter done
> [5.208075] megaraid_sas :01:00.0: Jbod map is not supported 
> megasas_setup_jbod_map 4967
> [5.230163] megaraid_sas :01:00.0: MR_DCMD_PD_LIST_QUERY failed/not 
> supported by firmware
> [5.252080] megaraid_sas :01:00.0: DCMD not supported by firmware - 
> megasas_ld_list_query 4369
> [5.274086] megaraid_sas :01:00.0: pci id: 
> (0x1000)/(0x0060)/(0x1734)/(0x10f9)
> [5.274089] megaraid_sas :01:00.0: unevenspan support: no
> [5.274090] megaraid_sas :01:00.0: firmware crash dump   : no
> [5.274092] megaraid_sas :01:00.0: jbod sync map : no
> [5.274094] scsi host0: Avago SAS based MegaRAID driver
> [5.280022] scsi 0:0:6:0: Direct-Access ATA  WDC WD5002ABYS-5 3B06 
> PQ: 0 ANSI: 5
> [5.282153] scsi 0:0:7:0: Direct-Access ATA  WDC WD5002ABYS-5 3B06 
> PQ: 0 ANSI: 5
> [5.285180] scsi 0:0:10:0: Direct-Access ATA  ST500NM0011  
> FTM6 PQ: 0 ANSI: 5
> [5.369885] scsi 0:2:0:0: Direct-Access LSI  MegaRAID SAS RMB 1.40 
> PQ: 0 ANSI: 5
> 
> ..
> 
> Please let me know if you need more infos and/or want me to test patches.
> 
> 

I managed to get some parts of the broken dmesg. There it is :

http://ftp.frugalware.org/pub/other/people/crazy/kernel/broken-dmesg


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] SCSI fixes for 4.9-rc3

[Gabriel C]

On 05.11.2016 14:29, James Bottomley wrote:


...

> Kashyap Desai (1):
>   scsi: megaraid_sas: Fix data integrity failure for JBOD (passthrough) 
> devices
> 
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c 
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> index 9ff57de..d8b1fbd 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -1700,16 +1700,13 @@ megasas_queue_command(struct Scsi_Host *shost, struct 
> scsi_cmnd *scmd)
>   goto out_done;
>   }
>  
> - switch (scmd->cmnd[0]) {
> - case SYNCHRONIZE_CACHE:
> - /*
> -  * FW takes care of flush cache on its own
> -  * No need to send it down
> -  */
> + /*
> +  * FW takes care of flush cache on its own for Virtual Disk.
> +  * No need to send it down for VD. For JBOD send SYNCHRONIZE_CACHE to 
> FW.
> +  */
> + if ((scmd->cmnd[0] == SYNCHRONIZE_CACHE) && MEGASAS_IS_LOGICAL(scmd)) {
>   scmd->result = DID_OK << 16;
>   goto out_done;
> - default:
> - break;
>   }
>  
>   return instance->instancet->build_and_issue_cmd(instance, scmd);

This patch breaks my box.. I'm not able to boot it anymore.
It seems with this patch I have /dev/sda[a-z] to /dev/sdz[a-z] ?!?

I'm not sure how to get an log since dracut times out and I'm dropped , after a 
very long time
of probing 'ghost devices', in a emercency shell, journalctl doesn't work also..

After reverting this one I can boot normal.

Box is a FUJITSU PRIMERGY TX200 S5..

This is from an working kernel..

[5.119371] megaraid_sas :01:00.0: FW now in Ready state
[5.119418] megaraid_sas :01:00.0: firmware supports msix: (0)
[5.119420] megaraid_sas :01:00.0: current msix/online cpus  : (1/16)
[5.119422] megaraid_sas :01:00.0: RDPQ mode : (disabled)
[5.123100] ehci-pci :00:1a.7: cache line size of 32 is not supported
[5.123113] ehci-pci :00:1a.7: irq 18, io mem 0xb002

...

[5.208063] megaraid_sas :01:00.0: controller type   : MR(256MB)
[5.208065] megaraid_sas :01:00.0: Online Controller Reset(OCR)  : 
Enabled
[5.208067] megaraid_sas :01:00.0: Secure JBOD support   : No
[5.208070] megaraid_sas :01:00.0: megasas_init_mfi: fw_support_ieee=0
[5.208073] megaraid_sas :01:00.0: INIT adapter done
[5.208075] megaraid_sas :01:00.0: Jbod map is not supported 
megasas_setup_jbod_map 4967
[5.230163] megaraid_sas :01:00.0: MR_DCMD_PD_LIST_QUERY failed/not 
supported by firmware
[5.252080] megaraid_sas :01:00.0: DCMD not supported by firmware - 
megasas_ld_list_query 4369
[5.274086] megaraid_sas :01:00.0: pci id: 
(0x1000)/(0x0060)/(0x1734)/(0x10f9)
[5.274089] megaraid_sas :01:00.0: unevenspan support: no
[5.274090] megaraid_sas :01:00.0: firmware crash dump   : no
[5.274092] megaraid_sas :01:00.0: jbod sync map : no
[5.274094] scsi host0: Avago SAS based MegaRAID driver
[5.280022] scsi 0:0:6:0: Direct-Access ATA  WDC WD5002ABYS-5 3B06 
PQ: 0 ANSI: 5
[5.282153] scsi 0:0:7:0: Direct-Access ATA  WDC WD5002ABYS-5 3B06 
PQ: 0 ANSI: 5
[5.285180] scsi 0:0:10:0: Direct-Access ATA  ST500NM0011  FTM6 
PQ: 0 ANSI: 5
[5.369885] scsi 0:2:0:0: Direct-Access LSI  MegaRAID SAS RMB 1.40 
PQ: 0 ANSI: 5

..

Please let me know if you need more infos and/or want me to test patches.


Best Regards,

Gabriel C
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Avoid that SCSI device removal through sysfs triggers a deadlock

[James Bottomley]

On Tue, 2016-11-08 at 20:10 -0600, Eric W. Biederman wrote:
> James Bottomley  writes:
> 
> > On Tue, 2016-11-08 at 18:57 -0600, Eric W. Biederman wrote:
[...]
> > > I am pretty certain that if you are going to make 
> > > kernfs_remove_self and kernfs_remove_by_name_ns to be safe to 
> > > race against each other, not just the KERNFS_SUICIDAL check, but 
> > > the wait when KERNFS_SUICIDAL is set needs to be added ot
> > > kernfs_remove_by_name_ns.
> > 
> > I don't think you can do that: waiting for SUICIDED would introduce
> > another potential lock entanglement.  I'm reasonably happy that the
> > deactivation offset coupled with kernfs_drain in the non self 
> > remove path means that the necessary cleanup is done when the 
> > directory itself is removed.  That seems to be a common pattern in 
> > all non-self removes.
> 
> But if we don't I am pretty certain there will be asynchrounous 
> behavior in some cases that could potentially confuse userspace.

But the original behaviour kernfs_remove_self() eliminated was the
asynchronous callback.  If we go back to that, we're definitely going
to introduce far more asynchronous behaviour.

> Which is partly why I would like to kill kernfs_remove_self.

I took a look at it.  It's definitely not cleanly revertible given
what's gone on since.  Even just trying to excise it is going to be
hard given all the tentacles it has.

> > > And I suspect if you add the appropriate lockdep annotations to 
> > > that mess you will find yourself in a similar but related ABBA
> > > deadlock.
> > 
> > I can't prove the negative, but as long as there's no waiting on 
> > the SUICIDED/AL flags in the non-self remove path, I believe we're 
> > safe with the current patch.
> > 
> > > Which is why I would like a simpler easier to understand 
> > > mechanism if we can.
> > 
> > I don't disagree: If you want to clean out the Augean Stables, I 
> > can lend you the thigh length rubber boots and the gas mask. 
> >  However, I think that what we're currently proposing: a simple 
> > patch to make device_remove_file_self() actually work for everyone, 
> > along with stringent testing is the better approach.
> > 
> > After all, if you look at
> > 
> > commit ac0ece9174aca9aa895ce0accc54f1f8ff12d117
> > Author: Tejun Heo 
> > Date:   Mon Feb 3 14:03:03 2014 -0500
> > 
> > scsi: use device_remove_file_self() instead of
> > device_schedule_callback()
> > 
> > You'll see Tejun added all this stuff just to remove the async 
> > callback we originally had.  Simply restoring the async callback 
> > back makes us quite considerably worse off because the 
> > device_remove_file_self() mechanism is in use elsewhere.  We need 
> > either to fix it and move on or junk it and go back to the
> > original.
> 
> I vote we junk it and go back to something that resembles the 
> original.  there are only about 5 other callers so this isn't that
> bad to do.
> 
> Tejun's work clearly added this deadlock 2 and a half years ago, and
> it was nasty enough no one noticed until recently.
> 
> Using task_work_add(current, ...) as I posted earlier let's us retain
> the synchronous property of the current API.
> 
> While we debate the details I am happy to look at scsi as a special 
> case and solve for scsi.  Then when we have the details worked out we 
> can go fix the other cases.  Given my preliminary patch in my last 
> reply it looks very straight forward to fix this sanely.

I don't think there's any urgency to fix SCSI.  You can only really
trigger this by hammering the device and host remove paths, which isn't
what users normally do ... as the fact it's been in the field for 2.5
years with no apparent problems shows.

I'd like Greg and Tejun to weigh in on this before we start doing
something, since they created the initial problem.

James

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: iscsi: make mutex for target scanning and unbinding per-session

[Chris Leech]

On Thu, Nov 10, 2016 at 05:22:44PM -0600, Mike Christie wrote:
> On 11/07/2016 12:22 PM, Chris Leech wrote:
> > Currently the iSCSI transport class synchronises target scanning and
> > unbinding with a host level mutex.  For multi-session hosts (offloading
> > iSCSI HBAs) connecting to storage arrays that may implement one
> > target-per-lun, this can result in the target scan work for hundreds of
> > sessions being serialized behind a single mutex.  With slow enough
> 
> Does this patch alone help or is there a scsi piece too?
 
I had this tested when working a hung task timeout issue at boot, and
was told that it fixed the issue.  The exact situation may be more
complex, I think it was only 128 sessions which is surprising that it
would hit a 2 minute timeout.  But every backtrace was at this mutex.

> There is also scsi_host->scan_mutex taken in the scsi layer during
> scsi_scan_target, so it is serialized there too. It seems like this
> patch would move that problem one layer down.

I'll take a closer look at that, thanks.
 
> >  
> > @@ -1801,10 +1798,7 @@ static int iscsi_user_scan_session(struct device 
> > *dev, void *data)
> >  
> > ISCSI_DBG_TRANS_SESSION(session, "Scanning session\n");
> >  
> > -   shost = iscsi_session_to_shost(session);
> > -   ihost = shost->shost_data;
> > -
> > -   mutex_lock(>mutex);
> > +   mutex_lock(>mutex);
> > spin_lock_irqsave(>lock, flags);
> > if (session->state != ISCSI_SESSION_LOGGED_IN) {
> > spin_unlock_irqrestore(>lock, flags);
> > @@ -1823,7 +1817,7 @@ static int iscsi_user_scan_session(struct device 
> > *dev, void *data)
> > }
> 
> The patch will allow you to remove other sessions while scanning is
> running, so it could still be a good idea.
> 
> I think I originally added the mutex because we did our own loop over a
> list of the host's sessions. If a unbind were to occur at the same time
> then it would be freed while scanning. We changed the user scan to use
> device_for_each_child so that will grab a reference to the session so
> the memory will not be freed now. It now just makes sure that scsi
> target removal and iscsi_remove_session wait until the scan is done.
> 
> On a related note, you can remove all the iscsi_scan_session code. We do
> not use it anymore. qla4xxx used to do async scans in the kernel with
> that code but does not anymore. In the future someone will also not ask
> why we grab the mutex around one scan and not the other.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Update 3ware driver email addresses

[adam radford]

This change updates the 3ware drivers (3w-, 3w-9xxx, 3w-sas) email
addresses from linuxr...@lsi.com to aradf...@gmail.com, since the old
email address doesn't exist.

Signed-off-by: Adam Radford 
---
 MAINTAINERS| 2 +-
 drivers/scsi/3w-9xxx.c | 6 +++---
 drivers/scsi/3w-9xxx.h | 6 +++---
 drivers/scsi/3w-sas.c  | 4 ++--
 drivers/scsi/3w-sas.h  | 4 ++--
 drivers/scsi/3w-.c | 4 ++--
 drivers/scsi/3w-.h | 4 ++--
 7 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index b3a7774..1b5ddd0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -138,7 +138,7 @@ S:  Maintained
 F: drivers/net/ethernet/3com/typhoon*

 3WARE SAS/SATA-RAID SCSI DRIVERS (3W-, 3W-9XXX, 3W-SAS)
-M: Adam Radford 
+M: Adam Radford 
 L: linux-scsi@vger.kernel.org
 W: http://www.lsi.com
 S: Supported
diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
index a56a7b2..6de3cab 100644
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -1,8 +1,8 @@
 /*
3w-9xxx.c -- 3ware 9000 Storage Controller device driver for Linux.

-   Written By: Adam Radford 
-   Modifications By: Tom Couch 
+   Written By: Adam Radford 
+   Modifications By: Tom Couch

Copyright (C) 2004-2009 Applied Micro Circuits Corporation.
Copyright (C) 2010 LSI Corporation.
@@ -41,7 +41,7 @@
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

Bugs/Comments/Suggestions should be mailed to:
-   linuxr...@lsi.com
+   aradf...@gmail.com

For more information, goto:
http://www.lsi.com
diff --git a/drivers/scsi/3w-9xxx.h b/drivers/scsi/3w-9xxx.h
index 0fdc83c..5380ce4 100644
--- a/drivers/scsi/3w-9xxx.h
+++ b/drivers/scsi/3w-9xxx.h
@@ -1,8 +1,8 @@
 /*
3w-9xxx.h -- 3ware 9000 Storage Controller device driver for Linux.

-   Written By: Adam Radford 
-   Modifications By: Tom Couch 
+   Written By: Adam Radford 
+   Modifications By: Tom Couch

Copyright (C) 2004-2009 Applied Micro Circuits Corporation.
Copyright (C) 2010 LSI Corporation.
@@ -41,7 +41,7 @@
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

Bugs/Comments/Suggestions should be mailed to:
-   linuxr...@lsi.com
+   aradf...@gmail.com

For more information, goto:
http://www.lsi.com
diff --git a/drivers/scsi/3w-sas.c b/drivers/scsi/3w-sas.c
index f837485..6840217 100644
--- a/drivers/scsi/3w-sas.c
+++ b/drivers/scsi/3w-sas.c
@@ -1,7 +1,7 @@
 /*
3w-sas.c -- LSI 3ware SAS/SATA-RAID Controller device driver for Linux.

-   Written By: Adam Radford 
+   Written By: Adam Radford 

Copyright (C) 2009 LSI Corporation.

@@ -43,7 +43,7 @@
LSI 3ware 9750 6Gb/s SAS/SATA-RAID

Bugs/Comments/Suggestions should be mailed to:
-   linuxr...@lsi.com
+   aradf...@gmail.com

For more information, goto:
http://www.lsi.com
diff --git a/drivers/scsi/3w-sas.h b/drivers/scsi/3w-sas.h
index fec6449..e7b7aec 100644
--- a/drivers/scsi/3w-sas.h
+++ b/drivers/scsi/3w-sas.h
@@ -1,7 +1,7 @@
 /*
3w-sas.h -- LSI 3ware SAS/SATA-RAID Controller device driver for Linux.

-   Written By: Adam Radford 
+   Written By: Adam Radford 

Copyright (C) 2009 LSI Corporation.

@@ -39,7 +39,7 @@
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

Bugs/Comments/Suggestions should be mailed to:
-   linuxr...@lsi.com
+   aradf...@gmail.com

For more information, goto:
http://www.lsi.com
diff --git a/drivers/scsi/3w-.c b/drivers/scsi/3w-.c
index 25aba16..374648d 100644
--- a/drivers/scsi/3w-.c
+++ b/drivers/scsi/3w-.c
@@ -1,7 +1,7 @@
 /*
3w-.c -- 3ware Storage Controller device driver for Linux.

-   Written By: Adam Radford 
+   Written By: Adam Radford 
Modifications By: Joel Jacobson 
 Arnaldo Carvalho de Melo 
  Brad Strand 
@@ -47,7 +47,7 @@
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

Bugs/Comments/Suggestions should be mailed to:
-   linuxr...@lsi.com
+   aradf...@gmail.com

For more information, goto:
http://www.lsi.com
diff --git a/drivers/scsi/3w-.h b/drivers/scsi/3w-.h
index 6f65e66..5a5a5d2 100644
--- a/drivers/scsi/3w-.h
+++ b/drivers/scsi/3w-.h
@@ -1,7 +1,7 @@
 /*
3w-.h -- 3ware Storage Controller device driver for Linux.

-   Written By: Adam Radford 
+   Written By: Adam Radford 
Modifications By: Joel Jacobson 
 Arnaldo Carvalho de Melo 
  Brad Strand 
@@ -45,7 +45,7 @@

Re: iscsi: make mutex for target scanning and unbinding per-session

[Mike Christie]

On 11/07/2016 12:22 PM, Chris Leech wrote:
> Currently the iSCSI transport class synchronises target scanning and
> unbinding with a host level mutex.  For multi-session hosts (offloading
> iSCSI HBAs) connecting to storage arrays that may implement one
> target-per-lun, this can result in the target scan work for hundreds of
> sessions being serialized behind a single mutex.  With slow enough

Does this patch alone help or is there a scsi piece too?

There is also scsi_host->scan_mutex taken in the scsi layer during
scsi_scan_target, so it is serialized there too. It seems like this
patch would move that problem one layer down.

>  
> @@ -1801,10 +1798,7 @@ static int iscsi_user_scan_session(struct device *dev, 
> void *data)
>  
>   ISCSI_DBG_TRANS_SESSION(session, "Scanning session\n");
>  
> - shost = iscsi_session_to_shost(session);
> - ihost = shost->shost_data;
> -
> - mutex_lock(>mutex);
> + mutex_lock(>mutex);
>   spin_lock_irqsave(>lock, flags);
>   if (session->state != ISCSI_SESSION_LOGGED_IN) {
>   spin_unlock_irqrestore(>lock, flags);
> @@ -1823,7 +1817,7 @@ static int iscsi_user_scan_session(struct device *dev, 
> void *data)
>   }

The patch will allow you to remove other sessions while scanning is
running, so it could still be a good idea.

I think I originally added the mutex because we did our own loop over a
list of the host's sessions. If a unbind were to occur at the same time
then it would be freed while scanning. We changed the user scan to use
device_for_each_child so that will grab a reference to the session so
the memory will not be freed now. It now just makes sure that scsi
target removal and iscsi_remove_session wait until the scan is done.

On a related note, you can remove all the iscsi_scan_session code. We do
not use it anymore. qla4xxx used to do async scans in the kernel with
that code but does not anymore. In the future someone will also not ask
why we grab the mutex around one scan and not the other.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] g_NCR5380: Fix release_region in error handling

[Finn Thain]

From: Ondrej Zary 

When a SW-configurable card is specified but not found, the driver
releases wrong region, causing the following message in kernel log:
Trying to free nonexistent resource <-000f>

Fix it by assigning base earlier.

Signed-off-by: Ondrej Zary 
Fixes: a8cfbcaec0c1 ("scsi: g_NCR5380: Stop using scsi_module.c")
Signed-off-by: Finn Thain 

---
 drivers/scsi/g_NCR5380.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
index 6a08d3e..d33e71f 100644
--- a/drivers/scsi/g_NCR5380.c
+++ b/drivers/scsi/g_NCR5380.c
@@ -258,12 +258,12 @@ static int generic_NCR5380_init_one(struct 
scsi_host_template *tpnt,
if (ports[i]) {
/* At this point we have our region reserved */
magic_configure(i, 0, magic); /* no IRQ yet */
-   outb(0xc0, ports[i] + 9);
-   if (inb(ports[i] + 9) != 0x80) {
+   base = ports[i];
+   outb(0xc0, base + 9);
+   if (inb(base + 9) != 0x80) {
ret = -ENODEV;
goto out_release;
}
-   base = ports[i];
port_idx = i;
} else
return -EINVAL;
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] hpsa: correct logical resets

[Don Brace]

- driver was not calling done in some cases which causes
  the volume to be offlined.
- avoid doing rescan during a reset.

Reviewed-by: Scott Teel 
Reviewed-by: Kevin Barnett 
Signed-off-by: Don Brace 
---
 drivers/scsi/hpsa.c |   24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 0b6eb5a..a296537 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -300,6 +300,10 @@ static bool hpsa_cmd_dev_match(struct ctlr_info *h, struct 
CommandList *c,
   struct hpsa_scsi_dev_t *dev,
   unsigned char *scsi3addr);
 
+static int wait_for_device_to_become_ready(struct ctlr_info *h,
+  unsigned char lunaddr[],
+  int reply_queue);
+
 static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev)
 {
unsigned long *priv = shost_priv(sdev->host);
@@ -2540,7 +2544,7 @@ static void complete_scsi_command(struct CommandList *cp)
 
if ((unlikely(hpsa_is_pending_event(cp {
if (cp->reset_pending)
-   return hpsa_cmd_resolve_and_free(h, cp);
+   return hpsa_cmd_free_and_done(h, cp, cmd);
if (cp->abort_pending)
return hpsa_cmd_abort_and_free(h, cp, cmd);
}
@@ -3079,6 +3083,8 @@ static int hpsa_do_reset(struct ctlr_info *h, struct 
hpsa_scsi_dev_t *dev,
 
if (unlikely(rc))
atomic_set(>reset_cmds_out, 0);
+   else
+   wait_for_device_to_become_ready(h, scsi3addr, 0);
 
mutex_unlock(>reset_mutex);
return rc;
@@ -5563,6 +5569,14 @@ static void hpsa_scan_start(struct Scsi_Host *sh)
if (unlikely(lockup_detected(h)))
return hpsa_scan_complete(h);
 
+   /*
+* Do the scan after a reset completion
+*/
+   if (h->reset_in_progress) {
+   h->drv_req_rescan = 1;
+   return;
+   }
+
hpsa_update_scsi_devices(h);
 
hpsa_scan_complete(h);
@@ -8641,6 +8655,14 @@ static void hpsa_rescan_ctlr_worker(struct work_struct 
*work)
if (h->remove_in_progress)
return;
 
+   /*
+* Do the scan after the reset
+*/
+   if (h->reset_in_progress) {
+   h->drv_req_rescan = 1;
+   return;
+   }
+
if (hpsa_ctlr_needs_rescan(h) || hpsa_offline_devices_ready(h)) {
scsi_host_get(h->scsi_host);
hpsa_ack_ctlr_events(h);

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: iscsi: make mutex for target scanning and unbinding per-session

[Chris Leech]

On Thu, Nov 10, 2016 at 10:00:54AM -0800, The Lee-Man wrote:
> On Monday, November 7, 2016 at 11:22:23 AM UTC-7, Chris Leech wrote:
> >
> > Currently the iSCSI transport class synchronises target scanning and 
> > unbinding with a host level mutex.  For multi-session hosts (offloading 
> > iSCSI HBAs) connecting to storage arrays that may implement one 
> > target-per-lun, this can result in the target scan work for hundreds of 
> > sessions being serialized behind a single mutex.  With slow enough 
> > response times, this can cause scan requests initiated from userspace to 
> > block on the mutex long enough to trigger 120 sec hung task warnings. 
> >
> > I can't see any reason not to move this to a session level mutex and let 
> > the target scans run in parallel, speeding up connecting to a large 
> > number of targets.  Note that as iscsi_tcp creates a virtual host for 
> > each session, software iSCSI is effectively doing this already. 
> >
> 
> I understood the reason for this mutex was to protect against the case
> where there are multiple paths to a target. In such cases, you can get
> simultaneous access to sysfs attributes (files), which can cause errors,
> i.e. two threads trying to write an attribute at the same time, or one
> changing an attribute while another reads or removes it.

This particular mutex is only serializing scanning targets for devices,
and used in __iscsi_unbind_session to ensure that no scans are in
progress adding new scsi devices while we're trying to remove a target.
 
> I worry that changing it will not address those issues.
> 
> [Side note: we *really* need a test suite that somehow includes
>  cases like this.]
> 
> >
> > Signed-off-by: Chris Leech  
> > --- 
> >  drivers/scsi/scsi_transport_iscsi.c | 19 ++- 
> >  include/scsi/scsi_transport_iscsi.h |  2 +- 
> >  2 files changed, 7 insertions(+), 14 deletions(-) 
> >
> > diff --git a/drivers/scsi/scsi_transport_iscsi.c 
> > b/drivers/scsi/scsi_transport_iscsi.c 
> > index 42bca61..83c90fa 100644 
> > --- a/drivers/scsi/scsi_transport_iscsi.c 
> > +++ b/drivers/scsi/scsi_transport_iscsi.c 
> > @@ -1568,7 +1568,6 @@ static int iscsi_setup_host(struct 
> > transport_container *tc, struct device *dev, 
> >   
> >  memset(ihost, 0, sizeof(*ihost)); 
> >  atomic_set(>nr_scans, 0); 
> > -mutex_init(>mutex); 
> >   
> >  iscsi_bsg_host_add(shost, ihost); 
> >  /* ignore any bsg add error - we just can't do sgio */ 
> > @@ -1789,8 +1788,6 @@ static int iscsi_user_scan_session(struct device 
> > *dev, void *data) 
> >  { 
> >  struct iscsi_scan_data *scan_data = data; 
> >  struct iscsi_cls_session *session; 
> > -struct Scsi_Host *shost; 
> > -struct iscsi_cls_host *ihost; 
> >  unsigned long flags; 
> >  unsigned int id; 
> >   
> > @@ -1801,10 +1798,7 @@ static int iscsi_user_scan_session(struct device 
> > *dev, void *data) 
> >   
> >  ISCSI_DBG_TRANS_SESSION(session, "Scanning session\n"); 
> >   
> > -shost = iscsi_session_to_shost(session); 
> > -ihost = shost->shost_data; 
> > - 
> > -mutex_lock(>mutex); 
> > +mutex_lock(>mutex); 
> >  spin_lock_irqsave(>lock, flags); 
> >  if (session->state != ISCSI_SESSION_LOGGED_IN) { 
> >  spin_unlock_irqrestore(>lock, flags); 
> > @@ -1823,7 +1817,7 @@ static int iscsi_user_scan_session(struct device 
> > *dev, void *data) 
> >  } 
> >   
> >  user_scan_exit: 
> > -mutex_unlock(>mutex); 
> > +mutex_unlock(>mutex); 
> >  ISCSI_DBG_TRANS_SESSION(session, "Completed session scan\n"); 
> >  return 0; 
> >  } 
> > @@ -2001,26 +1995,24 @@ static void __iscsi_unbind_session(struct 
> > work_struct *work) 
> >  struct iscsi_cls_session *session = 
> >  container_of(work, struct iscsi_cls_session, 
> >   unbind_work); 
> > -struct Scsi_Host *shost = iscsi_session_to_shost(session); 
> > -struct iscsi_cls_host *ihost = shost->shost_data; 
> >  unsigned long flags; 
> >  unsigned int target_id; 
> >   
> >  ISCSI_DBG_TRANS_SESSION(session, "Unbinding session\n"); 
> >   
> >  /* Prevent new scans and make sure scanning is not in progress */ 
> > -mutex_lock(>mutex); 
> > +mutex_lock(>mutex); 
> >  spin_lock_irqsave(>lock, flags); 
> >  if (session->target_id == ISCSI_MAX_TARGET) { 
> >  spin_unlock_irqrestore(>lock, flags); 
> > -mutex_unlock(>mutex); 
> > +mutex_unlock(>mutex); 
> >  return; 
> >  } 
> >   
> >  target_id = session->target_id; 
> >  session->target_id = ISCSI_MAX_TARGET; 
> >  spin_unlock_irqrestore(>lock, flags); 
> > -mutex_unlock(>mutex); 
> > +mutex_unlock(>mutex); 
> >   
> >   

Re: Delivery Status Notification for linuxr...@lsi.com

[adam radford]

On Thu, Nov 10, 2016 at 10:42 AM, James Bottomley
 wrote:
> It would look a lot better if you submitted the patch to change this
> rather than having git history show someone else dumping it on you ...

Sure, I will do that.

-Aadm
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Delivery Status Notification for linuxr...@lsi.com

[James Bottomley]

On Thu, 2016-11-10 at 10:14 -0800, adam radford wrote:
> On Thu, Nov 10, 2016 at 6:07 AM, Martin K. Petersen
>  wrote:
> > Ownership of these products is now with Broadcom. To my knowledge 
> > the 3ware product lines have been discontinued.
> 
> That is true.  I am still lurking on this list, if you want to update
> the email address to:  aradf...@gmail.com, I'm fine with that.  I'll
> probably reply if there is something I can help with.

It would look a lot better if you submitted the patch to change this
rather than having git history show someone else dumping it on you ...

Thanks,

James


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ufs: introduce hibern8_notify callback

[Subhash Jadavani]

On 2016-11-10 04:17, Kiwoong Kim wrote:

Some UFS host controller may need to configure some things
around hibern8 enter/exit

v2: change the callback name and data type of 2nd argument

Signed-off-by: Kiwoong Kim 
---
 drivers/scsi/ufs/ufshcd.c | 12 ++--
 drivers/scsi/ufs/ufshcd.h | 12 
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 301b592..e75fbb3 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2697,6 +2697,8 @@ static int __ufshcd_uic_hibern8_enter(struct 
ufs_hba *hba)

int ret;
struct uic_command uic_cmd = {0};

+   ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_ENTER, PRE_CHANGE);
+
uic_cmd.command = UIC_CMD_DME_HIBER_ENTER;
ret = ufshcd_uic_pwr_ctrl(hba, _cmd);

@@ -2710,7 +2712,9 @@ static int __ufshcd_uic_hibern8_enter(struct 
ufs_hba *hba)

 */
if (ufshcd_link_recovery(hba))
ret = -ENOLINK;
-   }
+   } else
+   ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_ENTER,
+   POST_CHANGE);

return ret;
 }
@@ -2733,13 +2737,17 @@ static int ufshcd_uic_hibern8_exit(struct 
ufs_hba *hba)

struct uic_command uic_cmd = {0};
int ret;

+   ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_EXIT, PRE_CHANGE);
+
uic_cmd.command = UIC_CMD_DME_HIBER_EXIT;
ret = ufshcd_uic_pwr_ctrl(hba, _cmd);
if (ret) {
dev_err(hba->dev, "%s: hibern8 exit failed. ret = %d\n",
__func__, ret);
ret = ufshcd_link_recovery(hba);
-   }
+   } else
+   ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_EXIT,
+   POST_CHANGE);

return ret;
 }
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 05d8384..8e76501 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -265,6 +265,8 @@ struct ufs_pwr_mode_info {
  *  to set some things
  * @setup_task_mgmt: called before any task management request is 
issued

  *  to set some things
+ * @hibern8_notify: called around hibern8 enter/exit
+ * to configure some things
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
@@ -290,6 +292,8 @@ struct ufs_hba_variant_ops {
struct ufs_pa_layer_attr *);
void(*setup_xfer_req)(struct ufs_hba *, int, bool);
void(*setup_task_mgmt)(struct ufs_hba *, int, u8);
+   void(*hibern8_notify)(struct ufs_hba *, enum uic_cmd_dme,
+  enum ufs_notify_change_status);
int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
@@ -821,6 +825,14 @@ static inline void
ufshcd_vops_setup_task_mgmt(struct ufs_hba *hba,
return hba->vops->setup_task_mgmt(hba, tag, tm_function);
 }

+static inline void ufshcd_vops_hibern8_notify(struct ufs_hba *hba,
+   enum uic_cmd_dme cmd,
+   enum ufs_notify_change_status status)
+{
+   if (hba->vops && hba->vops->hibern8_notify)
+   return hba->vops->hibern8_notify(hba, cmd, status);
+}
+
 static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum 
ufs_pm_op op)

 {
if (hba->vops && hba->vops->suspend)



LGTM.
Reviewed-by: Subhash Jadavani 


--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ufs: introduce setup_task_mgmt

[Subhash Jadavani]

On 2016-11-10 04:16, Kiwoong Kim wrote:

Some UFS host controller may need to configure some things
before any task management request is issued

v2: move a comment to another place

Signed-off-by: Kiwoong Kim 
---
 drivers/scsi/ufs/ufshcd.c |  2 ++
 drivers/scsi/ufs/ufshcd.h | 10 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index bf78321..301b592 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4357,6 +4357,8 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba
*hba, int lun_id, int task_id,
task_req_upiup->input_param1 = cpu_to_be32(lun_id);
task_req_upiup->input_param2 = cpu_to_be32(task_id);

+   ufshcd_vops_setup_task_mgmt(hba, free_slot, tm_function);
+
/* send command to the controller */
__set_bit(free_slot, >outstanding_tasks);

diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index e935fd1..05d8384 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -263,6 +263,8 @@ struct ufs_pwr_mode_info {
  * to be set.
  * @setup_xfer_req: called before any transfer request is issued
  *  to set some things
+ * @setup_task_mgmt: called before any task management request is 
issued

+ *  to set some things
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
@@ -287,6 +289,7 @@ struct ufs_hba_variant_ops {
struct ufs_pa_layer_attr *,
struct ufs_pa_layer_attr *);
void(*setup_xfer_req)(struct ufs_hba *, int, bool);
+   void(*setup_task_mgmt)(struct ufs_hba *, int, u8);
int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
@@ -811,6 +814,13 @@ static inline void
ufshcd_vops_setup_xfer_req(struct ufs_hba *hba, int tag,
return hba->vops->setup_xfer_req(hba, tag, is_scsi_cmd);
 }

+static inline void ufshcd_vops_setup_task_mgmt(struct ufs_hba *hba,
+   int tag, u8 tm_function)
+{
+   if (hba->vops && hba->vops->setup_task_mgmt)
+   return hba->vops->setup_task_mgmt(hba, tag, tm_function);
+}
+
 static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum 
ufs_pm_op op)

 {
if (hba->vops && hba->vops->suspend)



LGTM.
Reviewed-by: Subhash Jadavani 

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Delivery Status Notification for linuxr...@lsi.com

[adam radford]

On Thu, Nov 10, 2016 at 6:07 AM, Martin K. Petersen
 wrote:
> Ownership of these products is now with Broadcom. To my knowledge the
> 3ware product lines have been discontinued.

That is true.  I am still lurking on this list, if you want to update
the email address to:  aradf...@gmail.com, I'm fine with that.  I'll
probably reply if there is something I can help with.

-Adam
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ufs: introduce setup_xfer_req callback

[Subhash Jadavani]

On 2016-11-10 04:14, Kiwoong Kim wrote:

Some UFS host controller may need to configure some things
before any transfer request is issued.

V2: change data type of 2nd argument

Signed-off-by: Kiwoong Kim 
---
 drivers/scsi/ufs/ufshcd.c |  2 ++
 drivers/scsi/ufs/ufshcd.h | 10 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8cf5d8f..bf78321 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1516,6 +1516,7 @@ static int ufshcd_queuecommand(struct Scsi_Host
*host, struct scsi_cmnd *cmd)

/* issue command to the controller */
spin_lock_irqsave(hba->host->host_lock, flags);
+   ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false));
ufshcd_send_command(hba, tag);
 out_unlock:
spin_unlock_irqrestore(hba->host->host_lock, flags);
@@ -1727,6 +1728,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba 
*hba,

/* Make sure descriptors are ready before ringing the doorbell */
wmb();
spin_lock_irqsave(hba->host->host_lock, flags);
+   ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false));
ufshcd_send_command(hba, tag);
spin_unlock_irqrestore(hba->host->host_lock, flags);

diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index afff7f4..e935fd1 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -261,6 +261,8 @@ struct ufs_pwr_mode_info {
  * @pwr_change_notify: called before and after a power mode change
  * is carried out to allow vendor spesific capabilities
  * to be set.
+ * @setup_xfer_req: called before any transfer request is issued
+ *  to set some things
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
@@ -284,6 +286,7 @@ struct ufs_hba_variant_ops {
enum ufs_notify_change_status status,
struct ufs_pa_layer_attr *,
struct ufs_pa_layer_attr *);
+   void(*setup_xfer_req)(struct ufs_hba *, int, bool);
int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
@@ -801,6 +804,13 @@ static inline int
ufshcd_vops_pwr_change_notify(struct ufs_hba *hba,
return -ENOTSUPP;
 }

+static inline void ufshcd_vops_setup_xfer_req(struct ufs_hba *hba, int 
tag,

+   bool is_scsi_cmd)
+{
+   if (hba->vops && hba->vops->setup_xfer_req)
+   return hba->vops->setup_xfer_req(hba, tag, is_scsi_cmd);
+}
+
 static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum 
ufs_pm_op op)

 {
if (hba->vops && hba->vops->suspend)



LGTM.
Reviewed-by: Subhash Jadavani 


--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Reduced latency is killing performance

[Jens Axboe]

On 11/10/2016 09:04 AM, Hannes Reinecke wrote:

Hi all,

this really feels like a follow-up to the discussion we've had in Santa
Fe, but finally I'm able to substantiate it with some numbers.

I've made a patch to enable the megaraid_sas driver for multiqueue.
While this is pretty straightforward (I'll be sending the patchset later
on), the results are ... interesting.

I've run the 'ssd-test.fio' script from Jens' repository, and these
results for MQ/SQ (- is mq, + is sq):

 Run status group 0 (all jobs):
-   READ: io=10641MB, aggrb=181503KB/s, minb=181503KB/s,
maxb=181503KB/s, mint=60033msec, maxt=60033msec
+   READ: io=18370MB, aggrb=312572KB/s, minb=312572KB/s,
maxb=312572KB/s, mint=60181msec, maxt=60181msec

 Run status group 1 (all jobs):
-   READ: io=441444KB, aggrb=7303KB/s, minb=7303KB/s, maxb=7303KB/s,
mint=60443msec, maxt=60443msec
+   READ: io=223108KB, aggrb=3707KB/s, minb=3707KB/s, maxb=3707KB/s,
mint=60182msec, maxt=60182msec

 Run status group 2 (all jobs):
-  WRITE: io=22485MB, aggrb=383729KB/s, minb=383729KB/s,
maxb=383729KB/s, mint=60001msec, maxt=60001msec
+  WRITE: io=47421MB, aggrb=807581KB/s, minb=807581KB/s,
maxb=807581KB/s, mint=60129msec, maxt=60129msec

 Run status group 3 (all jobs):
-  WRITE: io=489852KB, aggrb=8110KB/s, minb=8110KB/s, maxb=8110KB/s,
mint=60399msec, maxt=60399msec
+  WRITE: io=489748KB, aggrb=8134KB/s, minb=8134KB/s, maxb=8134KB/s,
mint=60207msec, maxt=60207msec

 Disk stats (read/write):
-  sda: ios=2834412/5878578, merge=0/0, ticks=86269292/48364836,
in_queue=135345876, util=99.20%
+  sda: ios=205278/2680329, merge=4552593/9580622,
ticks=12539912/12965228, in_queue=25512312, util=99.59%

As you can see, we're really losing performance in the multiqueue case.
And the main reason for that is that we submit about _10 times_ as much
I/O as we do for the single-queue case.


What's the setup like? I'm going to need more details.

The baseline test is using the legacy path, single queue. The new test
is multiqueue, scsi-mq. What's sda?


So I guess having an I/O scheduler is critical, even for the scsi-mq case.


Each of these sections is a single job. For some reason we are not
merging as well as we should, that's the reason for the performance
loss. In fact, we're not merging at all. That's not IO scheduling.

--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Reduced latency is killing performance

[Jens Axboe]

On 11/10/2016 09:36 AM, Bart Van Assche wrote:

On 11/10/2016 08:04 AM, Hannes Reinecke wrote:

this really feels like a follow-up to the discussion we've had in Santa
Fe, but finally I'm able to substantiate it with some numbers.


Hi Jens,

Should I send you the notes I took on Thursday morning November 3 at 10
AM such that you can publish these?


I don't think it's related to this thread, but yes, that would be great.

--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Reduced latency is killing performance

[Bart Van Assche]

On 11/10/2016 08:04 AM, Hannes Reinecke wrote:

this really feels like a follow-up to the discussion we've had in Santa
Fe, but finally I'm able to substantiate it with some numbers.


Hi Jens,

Should I send you the notes I took on Thursday morning November 3 at 10 
AM such that you can publish these?


Thanks,

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reduced latency is killing performance

[Hannes Reinecke]

Hi all,

this really feels like a follow-up to the discussion we've had in Santa
Fe, but finally I'm able to substantiate it with some numbers.

I've made a patch to enable the megaraid_sas driver for multiqueue.
While this is pretty straightforward (I'll be sending the patchset later
on), the results are ... interesting.

I've run the 'ssd-test.fio' script from Jens' repository, and these
results for MQ/SQ (- is mq, + is sq):

 Run status group 0 (all jobs):
-   READ: io=10641MB, aggrb=181503KB/s, minb=181503KB/s,
maxb=181503KB/s, mint=60033msec, maxt=60033msec
+   READ: io=18370MB, aggrb=312572KB/s, minb=312572KB/s,
maxb=312572KB/s, mint=60181msec, maxt=60181msec

 Run status group 1 (all jobs):
-   READ: io=441444KB, aggrb=7303KB/s, minb=7303KB/s, maxb=7303KB/s,
mint=60443msec, maxt=60443msec
+   READ: io=223108KB, aggrb=3707KB/s, minb=3707KB/s, maxb=3707KB/s,
mint=60182msec, maxt=60182msec

 Run status group 2 (all jobs):
-  WRITE: io=22485MB, aggrb=383729KB/s, minb=383729KB/s,
maxb=383729KB/s, mint=60001msec, maxt=60001msec
+  WRITE: io=47421MB, aggrb=807581KB/s, minb=807581KB/s,
maxb=807581KB/s, mint=60129msec, maxt=60129msec

 Run status group 3 (all jobs):
-  WRITE: io=489852KB, aggrb=8110KB/s, minb=8110KB/s, maxb=8110KB/s,
mint=60399msec, maxt=60399msec
+  WRITE: io=489748KB, aggrb=8134KB/s, minb=8134KB/s, maxb=8134KB/s,
mint=60207msec, maxt=60207msec

 Disk stats (read/write):
-  sda: ios=2834412/5878578, merge=0/0, ticks=86269292/48364836,
in_queue=135345876, util=99.20%
+  sda: ios=205278/2680329, merge=4552593/9580622,
ticks=12539912/12965228, in_queue=25512312, util=99.59%

As you can see, we're really losing performance in the multiqueue case.
And the main reason for that is that we submit about _10 times_ as much
I/O as we do for the single-queue case.

So I guess having an I/O scheduler is critical, even for the scsi-mq case.

Full fio output attached.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
# Do some important numbers on SSD drives, to gauge what kind of
# performance you might get out of them.
#
# Sequential read and write speeds are tested, these are expected to be
# high. Random reads should also be fast, random writes are where crap
# drives are usually separated from the good drives.
#
# This uses a queue depth of 4. New SATA SSD's will support up to 32
# in flight commands, so it may also be interesting to increase the queue
# depth and compare. Note that most real-life usage will not see that
# large of a queue depth, so 4 is more representative of normal use.
#
[global]
ioengine=libaio
iodepth=256
size=10g
direct=1
runtime=60
numjobs=16
directory=/home
filename=ssd.test.file
group_reporting

[seq-read]
rw=read
stonewall

[rand-read]
rw=randread
stonewall

[seq-write]
rw=write
stonewall

[rand-write]
rw=randwrite
stonewall

seq-read: (g=0): rw=read, bs=4K-4K/4K-4K/4K-4K, ioengine=libaio, iodepth=256
...
rand-read: (g=1): rw=randread, bs=4K-4K/4K-4K/4K-4K, ioengine=libaio, iodepth=256
...
seq-write: (g=2): rw=write, bs=4K-4K/4K-4K/4K-4K, ioengine=libaio, iodepth=256
...
rand-write: (g=3): rw=randwrite, bs=4K-4K/4K-4K/4K-4K, ioengine=libaio, iodepth=256
...
fio-2.15
Starting 64 processes

seq-read: (groupid=0, jobs=16): err= 0: pid=3715: Thu Nov 10 16:48:49 2016
  read : io=18370MB, bw=312573KB/s, iops=78143, runt= 60181msec
slat (usec): min=1, max=5664, avg= 2.33, stdev= 5.74
clat (usec): min=30, max=1283.3K, avg=52320.04, stdev=114862.40
 lat (usec): min=37, max=1283.3K, avg=52322.47, stdev=114862.35
clat percentiles (usec):
 |  1.00th=[  620],  5.00th=[ 1432], 10.00th=[ 2096], 20.00th=[ 2544],
 | 30.00th=[ 2608], 40.00th=[ 2640], 50.00th=[ 2672], 60.00th=[ 2736],
 | 70.00th=[ 2864], 80.00th=[14912], 90.00th=[284672], 95.00th=[342016],
 | 99.00th=[419840], 99.50th=[468992], 99.90th=[585728], 99.95th=[634880],
 | 99.99th=[749568]
lat (usec) : 50=0.01%, 100=0.01%, 250=0.20%, 500=0.47%, 750=1.07%
lat (usec) : 1000=0.90%
lat (msec) : 2=5.92%, 4=63.81%, 10=5.22%, 20=3.28%, 50=1.84%
lat (msec) : 100=1.19%, 250=4.25%, 500=11.54%, 750=0.28%, 1000=0.01%
lat (msec) : 2000=0.01%
  cpu  : usr=0.65%, sys=1.42%, ctx=78762, majf=0, minf=9372
  IO depths: 1=0.1%, 2=0.1%, 4=0.1%, 8=0.1%, 16=0.1%, 32=0.1%, >=64=100.0%
 submit: 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, 64=0.0%, >=64=0.0%
 complete  : 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, 64=0.0%, >=64=0.1%
 issued: total=r=4702738/w=0/d=0, short=r=0/w=0/d=0, drop=r=0/w=0/d=0
 latency   : target=0, window=0, percentile=100.00%, depth=256
rand-read: (groupid=1, jobs=16): err= 0: pid=3772: Thu Nov 10 16:48:49 2016
  read : io=223108KB, bw=3707.3KB/s, iops=926, runt= 60182msec
slat (usec): min=2, max=1131.3K, 

Re: [PATCH] hpsa: switch to pci_alloc_irq_vectors

[Hannes Reinecke]

On 11/10/2016 03:48 PM, Christoph Hellwig wrote:
> On Thu, Nov 10, 2016 at 07:48:20AM +0100, Hannes Reinecke wrote:
>> What I find quite irritating is that we still have to call
>> irq_set_affinity_hint(irq, NULL) when freeing up interrupts.
>> Can't we roll that into the call to free_irq() ?
> 
> If you do call it that's irritation, because you should not call
> irq_set_affinity_hint ever if you are using PCI_IRQ_AFFINITY, and
> the above branch doesn't call it.
> 
Ah. Sorry to have misread that.

>> And you _do_ have to setup a cpumap within the driver :-)
> 
> For the non-mq case, yes - but only to keep the functionality
> as-is.  We shouldn't add anything like this to a new driver.
> 
>> Anyway, remainder looks pretty close to what I've written up.
>> Feel free to add my 'Reviewed-by:' to it.
> 
> I'm mostly looking for someone who has the hardware to actually
> test it, though.
> 
Ah. Right.

Will give it a spin tomorrow.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hpsa: switch to pci_alloc_irq_vectors

[Christoph Hellwig]

On Thu, Nov 10, 2016 at 07:48:20AM +0100, Hannes Reinecke wrote:
> What I find quite irritating is that we still have to call
> irq_set_affinity_hint(irq, NULL) when freeing up interrupts.
> Can't we roll that into the call to free_irq() ?

If you do call it that's irritation, because you should not call
irq_set_affinity_hint ever if you are using PCI_IRQ_AFFINITY, and
the above branch doesn't call it.

> And you _do_ have to setup a cpumap within the driver :-)

For the non-mq case, yes - but only to keep the functionality
as-is.  We shouldn't add anything like this to a new driver.

> Anyway, remainder looks pretty close to what I've written up.
> Feel free to add my 'Reviewed-by:' to it.

I'm mostly looking for someone who has the hardware to actually
test it, though.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] [SCSI] mpt3sas: Fix secure erase premature termination (v4)

[Andrey Grodzovsky]

Problem:
This is a work around for a bug with LSI Fusion MPT SAS2 when
pefroming secure erase. Due to the very long time the operation
takes commands issued during the erase will time out and will trigger
execution of abort hook. Even though the abort hook is called for
the specific command which timed out this leads to entire device halt
(scsi_state terminated) and premature termination of the secured erase.

Fix:
Set device state to busy while erase in progress to reject any incoming
commands until the erase is done. The device is blocked any way during
this time and cannot execute any other command.
More data and logs can be found here -
https://drive.google.com/file/d/0B9ocOHYHbbS1Q3VMdkkzeWFkTjg/view

v2: Update according to example patch by Hannes Reinecke to apply
the blocking logic to any ATA 12/16 command.

v3: Use SCSI commands opcodes definitions instead of value and
correct identation.

v4: Fix checkpath errors and warning.

Signed-off-by: Andrey Grodzovsky 
Cc: 
Cc: Sathya Prakash 
Cc: Chaitra P B 
Cc: Suganath Prabu Subramani 
Cc: Sreekanth Reddy 
Cc: Hannes Reinecke 
Cc: 
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 5a97e32..c032319 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -3500,6 +3500,10 @@ _scsih_eedp_error_handling(struct scsi_cmnd *scmd, u16 
ioc_status)
SAM_STAT_CHECK_CONDITION;
 }
 
+static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
+{
+   return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16);
+}
 
 /**
  * _scsih_qcmd - main scsi request entry point
@@ -3528,6 +3532,14 @@ _scsih_qcmd(struct Scsi_Host *shost, struct scsi_cmnd 
*scmd)
scsi_print_command(scmd);
 #endif
 
+   /**
+   * Lock the device for any subsequent command until
+   * command is done.
+   */
+   if (ata_12_16_cmd(scmd))
+   scsi_internal_device_block(scmd->device);
+
+
sas_device_priv_data = scmd->device->hostdata;
if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
scmd->result = DID_NO_CONNECT << 16;
@@ -4062,6 +4074,10 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 
msix_index, u32 reply)
if (scmd == NULL)
return 1;
 
+   if (ata_12_16_cmd(scmd))
+   scsi_internal_device_unblock(scmd->device, SDEV_RUNNING);
+
+
mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
 
if (mpi_reply == NULL) {
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Delivery Status Notification for linuxr...@lsi.com

[Paul Menzel]

Dear Martin,


On 11/10/16 15:07, Martin K. Petersen wrote:

"Paul" == Paul Menzel  writes:



Paul> Probably you know it already, but the listed email address of the
Paul> 3WARE SCSI drivers maintainer linuxr...@lsi.com doesn’t work (for
Paul> me).

Ownership of these products is now with Broadcom. To my knowledge the
3ware product lines have been discontinued.


Indeed. I forgot to actually formulate the intend of my message.

What should happen to the entry in the file `MAINTAINERS`?


Kind regards,

Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Delivery Status Notification for linuxr...@lsi.com

[Martin K. Petersen]

> "Paul" == Paul Menzel  writes:

Paul,

Paul> Probably you know it already, but the listed email address of the
Paul> 3WARE SCSI drivers maintainer linuxr...@lsi.com doesn’t work (for
Paul> me).

Ownership of these products is now with Broadcom. To my knowledge the
3ware product lines have been discontinued.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Ordering problems with 3ware controller

[Martin K. Petersen]

> "Paul" == Paul Menzel  writes:

Paul,

>> Linux does not provide device discovery ordering guarantees. You need
>> to fix your scripts to use UUIDs, filesystem labels, or DM devices to
>> get stable naming.

Paul> Indeed. But it worked for several years, so that *something* must
Paul> have changed that the ordering of the result of `getdents64` is
Paul> different now.

Could be changes in the PCI or platform code that causes things to be
enumerated differently. Whatever it is, it has nothing to do with the
3ware drivers themselves since they have been dormant for a long time.

Paul> Fixing the scripts is unfortunately not that easy, as `tw_cli` is
Paul> a proprietary tool [1], and we do not have the sources. It does a
Paul> `readdir()`.

That's beside the point. Whatever you are doing that needs to address a
specific physical controller needs to use a different scheme than
discovery order to do so. Don't know if these cards provide a device
identification VPD or something with a serial number that you could key
off of?

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [SCSI] mpt3sas: Fix secure erase premature termination (v3)

[Greg KH]

On Thu, Nov 10, 2016 at 08:42:52AM -0500, Andrey Grodzovsky wrote:
> Problem:
> This is a work around for a bug with LSI Fusion MPT SAS2 when
> pefroming secure erase. Due to the very long time the operation
> takes commands issued during the erase will time out and will trigger
> execution of abort hook. Even though the abort hook is called for
> the specifc command which timed out this leads to entire device halt
> (scsi_state terminated) and premature termination of the secured erase.
> 
> Fix:
> Set device state to busy while erase in progress to reject any incoming
> commands until the erase is done. The device is blocked any way during
> this time and cannot execute any other command.
> More data and logs can be found here -
> https://drive.google.com/file/d/0B9ocOHYHbbS1Q3VMdkkzeWFkTjg/view
> 
> v2: Update according to example patch by Hannes Reinecke to apply
> the blocking logic to any ATA 12/16 command.
> 
> v3: Use SCSI commands opcodes definitions instead of value and
> correct identation.
> 
> Signed-off-by: Andrey Grodzovsky 
> Cc: 
> Cc: Sathya Prakash 
> Cc: Chaitra P B 
> Cc: Suganath Prabu Subramani 
> Cc: Sreekanth Reddy 
> Cc: Hannes Reinecke 
> Cc: 
> ---
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
> b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 5a97e32..320f16c 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -3500,6 +3500,10 @@ _scsih_eedp_error_handling(struct scsi_cmnd *scmd, u16 
> ioc_status)
>   SAM_STAT_CHECK_CONDITION;
>  }
>  
> +static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
> +{
> +   return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16);
> +}

Please always run your patches through checkpatch.pl so you don't get a
grumpy maintainer emailing you and telling you to run your patches
through checkpatch.pl...

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] [SCSI] mpt3sas: Fix secure erase premature termination (v3)

[Andrey Grodzovsky]

Problem:
This is a work around for a bug with LSI Fusion MPT SAS2 when
pefroming secure erase. Due to the very long time the operation
takes commands issued during the erase will time out and will trigger
execution of abort hook. Even though the abort hook is called for
the specifc command which timed out this leads to entire device halt
(scsi_state terminated) and premature termination of the secured erase.

Fix:
Set device state to busy while erase in progress to reject any incoming
commands until the erase is done. The device is blocked any way during
this time and cannot execute any other command.
More data and logs can be found here -
https://drive.google.com/file/d/0B9ocOHYHbbS1Q3VMdkkzeWFkTjg/view

v2: Update according to example patch by Hannes Reinecke to apply
the blocking logic to any ATA 12/16 command.

v3: Use SCSI commands opcodes definitions instead of value and
correct identation.

Signed-off-by: Andrey Grodzovsky 
Cc: 
Cc: Sathya Prakash 
Cc: Chaitra P B 
Cc: Suganath Prabu Subramani 
Cc: Sreekanth Reddy 
Cc: Hannes Reinecke 
Cc: 
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 5a97e32..320f16c 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -3500,6 +3500,10 @@ _scsih_eedp_error_handling(struct scsi_cmnd *scmd, u16 
ioc_status)
SAM_STAT_CHECK_CONDITION;
 }
 
+static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
+{
+   return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16);
+}
 
 /**
  * _scsih_qcmd - main scsi request entry point
@@ -3528,6 +3532,14 @@ _scsih_qcmd(struct Scsi_Host *shost, struct scsi_cmnd 
*scmd)
scsi_print_command(scmd);
 #endif
 
+   /**
+   * Lock the device for any subsequent command until
+   * command is done.
+   */
+   if (ata_12_16_cmd(scmd))
+   scsi_internal_device_block(scmd->device);
+
+
sas_device_priv_data = scmd->device->hostdata;
if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
scmd->result = DID_NO_CONNECT << 16;
@@ -4062,6 +4074,10 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 
msix_index, u32 reply)
if (scmd == NULL)
return 1;
 
+   if (ata_12_16_cmd(scmd))
+   scsi_internal_device_unblock(scmd->device, SDEV_RUNNING);
+
+
mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
 
if (mpi_reply == NULL) {
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Delivery Status Notification for linuxr...@lsi.com

[Paul Menzel]

Dear Linux folks,


Probably you know it already, but the listed email address of the 3WARE 
SCSI drivers maintainer linuxr...@lsi.com doesn’t work (for me).


Please see the attached message.


Kind regards,

Paul
--- Begin Message ---
This is an automatically generated Delivery Status Notification

THIS IS A WARNING MESSAGE ONLY.

YOU DO NOT NEED TO RESEND YOUR MESSAGE.

Delivery to the following recipient has been delayed:

 linuxr...@lsi.com

Message will be retried for 5 more day(s)

Technical details of temporary failure: 
The recipient server did not accept our requests to connect. Learn more at 
https://support.google.com/mail/answer/7720 
[192.19.192.224 192.19.192.224: timed out]

- Original message -

X-Gm-Message-State: 
ABUngveFBksx92G0BZX5qMdBuCDHDG4xuI0c1GPn8OQmdZmNS3ZMR9/TFIpcevk2OOorMUNMld3vQugDvWxMGFOcWZveSLRhDyWLWqRReAKzrCIwHLeIr+9x3z44bqKAnr2A3oQ=
X-Received: by 10.28.209.67 with SMTP id i64mr12975034wmg.48.1478599659285;
Tue, 08 Nov 2016 02:07:39 -0800 (PST)
X-Received: by 10.28.209.67 with SMTP id i64mr12975009wmg.48.1478599659097;
Tue, 08 Nov 2016 02:07:39 -0800 (PST)
Return-Path: 
Received: from mx1.molgen.mpg.de (mx1.molgen.mpg.de. [141.14.17.9])
by mx.google.com with ESMTPS id w203si15677261wmg.41.2016.11.08.02.07.38
for 
(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
Tue, 08 Nov 2016 02:07:38 -0800 (PST)
Received-SPF: pass (google.com: domain of pmen...@molgen.mpg.de designates 
141.14.17.9 as permitted sender) client-ip=141.14.17.9;
Authentication-Results: mx.google.com;
   spf=pass (google.com: domain of pmen...@molgen.mpg.de designates 
141.14.17.9 as permitted sender) smtp.mailfrom=pmen...@molgen.mpg.de
Received: from keineahnung.molgen.mpg.de (keineahnung.molgen.mpg.de 
[141.14.17.193])
(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))
(No client certificate requested)
(Authenticated sender: pmenzel)
by mx.molgen.mpg.de (Postfix) with ESMTPSA id 08BAC20128247D;
Tue,  8 Nov 2016 11:07:38 +0100 (CET)
To: linux-scsi@vger.kernel.org
Cc: Adam Radford 
From: Paul Menzel 
Subject: Ordering problems with 3ware controller
Message-ID: 
Date: Tue, 8 Nov 2016 11:07:37 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.4.0
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
X-Gm-Spam: 0
X-Gm-Phishy: 0

Dear Linux SCSI folks,


Updating from Linux 4.4.X to Linux 4.8.4, we noticed that the 3ware 
devices under `/dev` – `/dev/twa0`, `/dev/twa1`, … – map to the 
controllers differently.

This unfortunately breaks quite a lot of our scripts, as we depend on 
the fact that the first controller is also in the front.

> $ dmesg | grep 3ware
> [   14.509238] 3ware 9000 Storage Controller device driver for Linux 
> v2.26.02.014.
> [   14.824274] scsi host8: 3ware 9000 Storage Controller
> [   14.824537] 3w-9xxx: scsi8: Found a 3ware 9000 Storage Controller at 
> 0xd020, IRQ: 17.
> [   15.508310] scsi host9: 3ware 9000 Storage Controller
> [   15.508569] 3w-9xxx: scsi9: Found a 3ware 9000 Storage Controller at 
> 0xda10, IRQ: 17.

Tracing `twi_cli` it looks like the ordering of the devices in 
`/sys/class/scsi_host` might have changed, as `getdents64` seems to be 
used for the ordering of creating `/dev/twaX`.

> $ find /sys/class/scsi_host/ -ls
>  6033  0 drwxr-xr-x   2  root system  0 Nov  8 10:58 
> /sys/class/scsi_host/
> 23125  0 lrwxrwxrwx   1  root system  0 Oct 27 17:41 
> /sys/class/scsi_host/host0 -> 
> ../../devices/pci:00/:00:07.0/ata1/host0/scsi_host/host0
> 29893  0 lrwxrwxrwx   1  root system  0 Oct 27 18:03 
> /sys/class/scsi_host/host9 -> 
> ../../devices/pci:80/:80:0e.0/:90:00.0/host9/scsi_host/host9
> 23878  0 lrwxrwxrwx   1  root system  0 Oct 27 17:41 
> /sys/class/scsi_host/host7 -> 
> ../../devices/pci:80/:80:08.0/ata8/host7/scsi_host/host7
> 23640  0 lrwxrwxrwx   1  root system  0 Oct 27 17:41 
> /sys/class/scsi_host/host5 -> 
> ../../devices/pci:80/:80:07.0/ata6/host5/scsi_host/host5
> 23402  0 lrwxrwxrwx   1  root system  0 Oct 27 17:41 
> /sys/class/scsi_host/host3 -> 
> ../../devices/pci:00/:00:08.0/ata4/host3/scsi_host/host3
> 23164  0 lrwxrwxrwx   1  root system  0 Oct 27 17:41 
> /sys/class/scsi_host/host1 -> ../.

- Message truncated -

--- End Message ---

[Bug 187381] 4.9.-rc4 produces hundreds of unusable scsi devices.

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=187381

Tommy Wu  changed:

   What|Removed |Added

 CC||wu.to...@gmail.com

--- Comment #4 from Tommy Wu  ---
I got same issue today.
After apply the patch, everything work fine now.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ufs: introduce hibern8_notify callback

[Kiwoong Kim]

Some UFS host controller may need to configure some things
around hibern8 enter/exit

v2: change the callback name and data type of 2nd argument

Signed-off-by: Kiwoong Kim 
---
 drivers/scsi/ufs/ufshcd.c | 12 ++--
 drivers/scsi/ufs/ufshcd.h | 12 
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 301b592..e75fbb3 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2697,6 +2697,8 @@ static int __ufshcd_uic_hibern8_enter(struct ufs_hba *hba)
int ret;
struct uic_command uic_cmd = {0};
 
+   ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_ENTER, PRE_CHANGE);
+
uic_cmd.command = UIC_CMD_DME_HIBER_ENTER;
ret = ufshcd_uic_pwr_ctrl(hba, _cmd);
 
@@ -2710,7 +2712,9 @@ static int __ufshcd_uic_hibern8_enter(struct ufs_hba *hba)
 */
if (ufshcd_link_recovery(hba))
ret = -ENOLINK;
-   }
+   } else
+   ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_ENTER,
+   POST_CHANGE);
 
return ret;
 }
@@ -2733,13 +2737,17 @@ static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba)
struct uic_command uic_cmd = {0};
int ret;
 
+   ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_EXIT, PRE_CHANGE);
+
uic_cmd.command = UIC_CMD_DME_HIBER_EXIT;
ret = ufshcd_uic_pwr_ctrl(hba, _cmd);
if (ret) {
dev_err(hba->dev, "%s: hibern8 exit failed. ret = %d\n",
__func__, ret);
ret = ufshcd_link_recovery(hba);
-   }
+   } else
+   ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_EXIT,
+   POST_CHANGE);
 
return ret;
 }
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 05d8384..8e76501 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -265,6 +265,8 @@ struct ufs_pwr_mode_info {
  *  to set some things
  * @setup_task_mgmt: called before any task management request is issued
  *  to set some things
+ * @hibern8_notify: called around hibern8 enter/exit
+ * to configure some things
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
@@ -290,6 +292,8 @@ struct ufs_hba_variant_ops {
struct ufs_pa_layer_attr *);
void(*setup_xfer_req)(struct ufs_hba *, int, bool);
void(*setup_task_mgmt)(struct ufs_hba *, int, u8);
+   void(*hibern8_notify)(struct ufs_hba *, enum uic_cmd_dme,
+  enum ufs_notify_change_status);
int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
@@ -821,6 +825,14 @@ static inline void ufshcd_vops_setup_task_mgmt(struct 
ufs_hba *hba,
return hba->vops->setup_task_mgmt(hba, tag, tm_function);
 }
 
+static inline void ufshcd_vops_hibern8_notify(struct ufs_hba *hba,
+   enum uic_cmd_dme cmd,
+   enum ufs_notify_change_status status)
+{
+   if (hba->vops && hba->vops->hibern8_notify)
+   return hba->vops->hibern8_notify(hba, cmd, status);
+}
+
 static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op)
 {
if (hba->vops && hba->vops->suspend)
-- 
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ufs: introduce setup_task_mgmt

[Kiwoong Kim]

Some UFS host controller may need to configure some things
before any task management request is issued

v2: move a comment to another place

Signed-off-by: Kiwoong Kim 
---
 drivers/scsi/ufs/ufshcd.c |  2 ++
 drivers/scsi/ufs/ufshcd.h | 10 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index bf78321..301b592 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4357,6 +4357,8 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int 
lun_id, int task_id,
task_req_upiup->input_param1 = cpu_to_be32(lun_id);
task_req_upiup->input_param2 = cpu_to_be32(task_id);
 
+   ufshcd_vops_setup_task_mgmt(hba, free_slot, tm_function);
+
/* send command to the controller */
__set_bit(free_slot, >outstanding_tasks);
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index e935fd1..05d8384 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -263,6 +263,8 @@ struct ufs_pwr_mode_info {
  * to be set.
  * @setup_xfer_req: called before any transfer request is issued
  *  to set some things
+ * @setup_task_mgmt: called before any task management request is issued
+ *  to set some things
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
@@ -287,6 +289,7 @@ struct ufs_hba_variant_ops {
struct ufs_pa_layer_attr *,
struct ufs_pa_layer_attr *);
void(*setup_xfer_req)(struct ufs_hba *, int, bool);
+   void(*setup_task_mgmt)(struct ufs_hba *, int, u8);
int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
@@ -811,6 +814,13 @@ static inline void ufshcd_vops_setup_xfer_req(struct 
ufs_hba *hba, int tag,
return hba->vops->setup_xfer_req(hba, tag, is_scsi_cmd);
 }
 
+static inline void ufshcd_vops_setup_task_mgmt(struct ufs_hba *hba,
+   int tag, u8 tm_function)
+{
+   if (hba->vops && hba->vops->setup_task_mgmt)
+   return hba->vops->setup_task_mgmt(hba, tag, tm_function);
+}
+
 static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op)
 {
if (hba->vops && hba->vops->suspend)
-- 
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ufs: introduce setup_xfer_req callback

[Kiwoong Kim]

Some UFS host controller may need to configure some things
before any transfer request is issued.

V2: change data type of 2nd argument

Signed-off-by: Kiwoong Kim 
---
 drivers/scsi/ufs/ufshcd.c |  2 ++
 drivers/scsi/ufs/ufshcd.h | 10 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8cf5d8f..bf78321 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1516,6 +1516,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *cmd)
 
/* issue command to the controller */
spin_lock_irqsave(hba->host->host_lock, flags);
+   ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false));
ufshcd_send_command(hba, tag);
 out_unlock:
spin_unlock_irqrestore(hba->host->host_lock, flags);
@@ -1727,6 +1728,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
/* Make sure descriptors are ready before ringing the doorbell */
wmb();
spin_lock_irqsave(hba->host->host_lock, flags);
+   ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false));
ufshcd_send_command(hba, tag);
spin_unlock_irqrestore(hba->host->host_lock, flags);
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index afff7f4..e935fd1 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -261,6 +261,8 @@ struct ufs_pwr_mode_info {
  * @pwr_change_notify: called before and after a power mode change
  * is carried out to allow vendor spesific capabilities
  * to be set.
+ * @setup_xfer_req: called before any transfer request is issued
+ *  to set some things
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
@@ -284,6 +286,7 @@ struct ufs_hba_variant_ops {
enum ufs_notify_change_status status,
struct ufs_pa_layer_attr *,
struct ufs_pa_layer_attr *);
+   void(*setup_xfer_req)(struct ufs_hba *, int, bool);
int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
@@ -801,6 +804,13 @@ static inline int ufshcd_vops_pwr_change_notify(struct 
ufs_hba *hba,
return -ENOTSUPP;
 }
 
+static inline void ufshcd_vops_setup_xfer_req(struct ufs_hba *hba, int tag,
+   bool is_scsi_cmd)
+{
+   if (hba->vops && hba->vops->setup_xfer_req)
+   return hba->vops->setup_xfer_req(hba, tag, is_scsi_cmd);
+}
+
 static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op)
 {
if (hba->vops && hba->vops->suspend)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [SCSI] mpt3sas: Fix secure erase premature termination (v2)

[Sreekanth Reddy]

On Sat, Nov 5, 2016 at 6:47 PM, Andrey Grodzovsky  wrote:
> On Fri, Nov 4, 2016 at 10:51 AM, Hannes Reinecke  wrote:
>> On 11/04/2016 01:45 PM, Sreekanth Reddy wrote:
>>>
>>> Hi All,
>>>
>>> From last two days, I was working with my firmware team to get the
>>> required info over this issue. Here is my firmware team response
>>>
>>> "For ATA PASSTHROUGH commands, the IOC SATL will not check for the
>>> opcode and will direct it to the drive. So even though ATA PASSTHOUGH
>>> has ATA erase to the drive, IOC SATL FW will not know that and as a
>>> general logic for all ATA PASSTHOGH commands, IOC FW will pend the
>>> upcoming IOs untill the previous ATA PASSTHORUGH completes. This is as
>>> per the SAT specification for SAS controllers and we can't compare it
>>> with the SATA controllers in the on board that have full fledge SATA
>>> implementation".
>>>
>>> So this is an expected behavior from our HBA firmware. i.e. it will
>>> pend the subsequent commands if any ATA PASSTHROUGH command is going
>>> on. So their is no issue with the FW.
>>>
>> But is there a way to figure out if the firmware / SATL layer is busy
>> processing requests?
>>
>> With 'real' ATA HBAs these issue doesn't occur, as the ATA erase command is
>> a non-queued command, and hence the next command automatically has to wait
>> for the erase command to complete.
>> But this wait happens as the ATA HBA returns 'BUSY', and the linux I/O stack
>> will then reset the timeout for all consecutive commands.
>>
>> With mpt3sas _all_ commands are queued, so if there is a long-running I/O
>> command all other commands already in the queue will time out.
>>
>> Which is at least a very awkward behaviour.
>>
>> Checking with SAT-3 (section 6.2.4: Commands the SATL queues internally) the
>> implemented behaviour is standards conformant, although the standard also
>> allows for returning 'TASK SET FULL' or 'BUSY' in these cases.
>> Doing so would nicely solve this issue.
>>
>>> Today I have tried the same test case on my local setup. i.e. I have
>>> issued a secure erase command using hdparm utility and observed the
>>> same issue on 4.2.3-300.fc23.x86_64 kernel.
>>>
>>> Then after browsing over this issue, I found that some people are
>>> recommending to enable 'CONFIG_IDE_TASK_IOCTL' Kconfig flag. I had a
>>> compiled 4.4.0 kernel, so I have enabled this CONFIG_IDE_TASK_IOCTL
>>> and recompiled this 4.4.0 kernel and booted in to this kernel. Then I
>>> tried same test case and I haven't observed this issue and secure
>>> erase operation was completed successfully.
>>>
>>> So, can you please try once with CONFIG_IDE_TASK_IOCTL enabled.
>>>
>> Errm.
>> CONFIG_IDE_TASK_IOCTL is for the old IDE subsystem, which isn't in use here.
>> So this option does not make a difference when using mpt3sas, as this is a
>> 'real' SCSI driver which never calls out into any of these subsystems.
>>
>> I would be _VERY_ much surprised if that would make a difference.
>>
>> The reason why this behaviour did go unnoticed with older kernels was that a
>> command timeout would trigger SCSI EH to engage, and that in turn required
>> all outstanding commands to complete.
>> So by the time SCSI EH started the ERASE command was complete, and a retry
>> of the timed-out commands would work.
>
> Indeed, when retesting with CONFIG_IDE_TASK_IOCTL=y and. reverting the
> fix the bug is back.
>
> Thanks,
> Andrey

Hi Andrey,

We are fine with this patch with below few changes,

1. Please remove below comment. it not a bug in firmware, it is
designed like that,

/* This is a work around for a bug with LSI Fusion MPT SAS2 when
* pefroming secure erase. Due to the verly long time the operation
* takes commands issued during the erase will time out and will trigger
* execution of abort hook. This leads to device reset and premature
* termination of the secured erase.
*/

2. Use SCSI commands opcodes definitions instead of value, so replace below line

return (scmd->cmnd[0] == 0xa1 || scmd->cmnd[0] == 0x85);
as
return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16);

3.  Please correct alignment for the below comment,

 /**
 * Lock the device for any subsequent command until
 * command is done.
 */

Thanks,
Sreekanth

>>
>>
>> Cheers,
>>
>> Hannes
>> --
>> Dr. Hannes Reinecke   zSeries & Storage
>> h...@suse.de  +49 911 74053 688
>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
>> GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] scsi: libsas: fix WARN on device removal

[John Garry]

On 09/11/2016 20:35, Dan Williams wrote:

On Wed, Nov 9, 2016 at 11:09 AM, Dan Williams  wrote:

On Wed, Nov 9, 2016 at 9:36 AM, John Garry  wrote:

On 09/11/2016 12:28, John Garry wrote:


On 03/11/2016 14:58, John Garry wrote:


The following patch introduces an annoying WARN
when a device is removed from the SAS topology:
[SCSI] libsas: prevent domain rediscovery competing with ata error
handling



Are there any views on this patch? I would have thought that the parties
who use the drivers based on libsas would be interested in fixing this
bug.



I should have added the before and after logs earlier, so the issue is
illustrated. Now attached. When a 24-port expander is unplugged we get >6k
lines of WARN on the console, lasting >30 seconds. Not nice.



I might be mistaken, but this patch seems functionally identical to
this attempt:

http://marc.info/?l=linux-scsi=143459794823595=2


Hi Dan,

They're not the same. I don't see how your solution properly deals with 
remote sas_port deletion.


When we unplug a device connected to an expander, can't the sas_port be 
deleted twice, in sas_unregister_devs_sas_addr() from domain 
revalidation and also now in sas_destruct_devices()? I think that this 
gives a NULL dereference.
And we still get the WARN as the sas_port has still been deleted before 
the device.


In my solution, we should always delete the sas_port after the attached 
device.




i.e. it moves the port destruction to the workqueue and still suffers
from the flutter problem:

http://marc.info/?l=linux-scsi=143801026028006=2
http://marc.info/?l=linux-scsi=143801971131073=2

Perhaps we instead need to quiet this warning?

http://marc.info/?l=linux-scsi=143802229932175=2


I have not seen the flutter issue. I am just trying to solve the 
horrible WARN dump.
However I do understand that there may be a issue related to how we 
queue the events; there was a recent attempt to fix this, but it came to 
nothing:

https://www.spinics.net/lists/linux-scsi/msg1.html

Cheers,
John



Alternatively we need a mechanism to cancel in-flight port shutdown
requests when we start re-attaching devices before queued port
destruction events have run.

.




--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html