Re: [PATCHv7 21/23] scsi_dh_alua: use common definitions for ALUA state

[Hannes Reinecke]

On 02/18/2016 11:08 PM, Bart Van Assche wrote:
> On 02/15/2016 12:24 AM, Hannes Reinecke wrote:
>> @@ -180,7 +172,7 @@ static int submit_stpg(struct scsi_device
>> *sdev, int group_id,
>>
>>   /* Prepare the data buffer */
>>   memset(stpg_data, 0, stpg_len);
>> -stpg_data[4] = TPGS_STATE_OPTIMIZED & 0x0f;
>> +stpg_data[4] = SCSI_ACCESS_STATE_OPTIMAL & 0x0f;
>>   put_unaligned_be16(group_id, _data[6]);
>>
> 
> In the previous patch a symbolic name for the ALUA state mask has
> been introduced (SCSI_ACCESS_STATE_MASK). Hence please either leave
> out "& 0x0f" from the above code or use that constant.
> 
Ok.

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: [PATCHv7 20/23] scsi: Add 'access_state' attribute

[Hannes Reinecke]

On 02/18/2016 11:06 PM, Bart Van Assche wrote:
> On 02/15/2016 12:24 AM, Hannes Reinecke wrote:
>> --- a/include/scsi/scsi_proto.h
>> +++ b/include/scsi/scsi_proto.h
>> @@ -277,5 +277,17 @@ struct scsi_lun {
>>   __u8 scsi_lun[8];
>>   };
>>
>> +/* SPC asymmetric access states */
>> +#define SCSI_ACCESS_STATE_OPTIMAL 0x00
>> +#define SCSI_ACCESS_STATE_ACTIVE  0x01
>> +#define SCSI_ACCESS_STATE_STANDBY 0x02
>> +#define SCSI_ACCESS_STATE_UNAVAILABLE 0x03
>> +#define SCSI_ACCESS_STATE_LBA 0x04
>> +#define SCSI_ACCESS_STATE_OFFLINE 0x0e
>> +#define SCSI_ACCESS_STATE_TRANSITIONING 0x0f
>> +
>> +#define SCSI_ACCESS_STATE_MASK0x0f
>> +#define SCSI_ACCESS_STATE_PREFERRED   0x80
>> +#define SCSI_ACCESS_STATE_UNKNOWN 0x70
> 
> Please mention in the comment above these constants that the ALUA
> state values apply to both the RTPG and STPG commands but that the
> PREFERRED bit only applies to the RTPG command.
> 
> All constants in scsi_proto.h come from a SCSI standard. However,
> this patch adds a constant that does not come from a SCSI standard
> (SCSI_ACCESS_STATE_UNKNOWN). Please remove this constant entirely
> because it causes confusion. sdev_show_access_state() only
> interprets the PREFERRED bit and the lower 4 bits of the
> access_state member variable. This means that the
> SCSI_ACCESS_STATE_UNKNOWN entry in sdev_access_states[] is never
> used and hence should be left out. This also means that the
> "sdev->access_state = SCSI_ACCESS_STATE_UNKNOWN" assignment in
> scsi_scan.c is equivalent to "sdev->access_state =
> SCSI_ACCESS_STATE_OPTIMAL".
> 
Correct. Will be fixing it up.

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: [PATCHv7 20/23] scsi: Add 'access_state' attribute

[Hannes Reinecke]

On 02/18/2016 10:55 PM, Bart Van Assche wrote:
> On 02/15/2016 12:24 AM, Hannes Reinecke wrote:
>> Add an 'access_state' attribute to struct scsi_device to
>> display the asymmetric LUN access state.
> 
> Do we really need to add this attribute to SCSI devices that do not
> support ALUA, e.g. ATA devices ? From a system on which this patch
> series has been installed:
> 
> [root ~]# lsscsi | grep 0:0:0:0
> [0:0:0:0] disk ATA ST1000NM0033-9ZM GA67 /dev/sda
> [root ~]# cat /sys/class/scsi_device/0:0:0:0/device/access_state
> active/optimized
> 
Well.

I've hit the same issue with my patches exposing vpd pages to sysfs.

Thing is, the sysfs entries are created during scsi_alloc_sdev().
But we only know if the have these entries during scsi_add_lun(), ie
way beyond that point. So ATM we don't have a chance of selectively
enabling them.

Any change here would require a modification to the core sysfs
behaviour, which might not that easy.

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: [PATCHv7 07/23] scsi_dh_alua: Use separate alua_port_group structure

[Hannes Reinecke]

On 02/18/2016 08:41 PM, Bart Van Assche wrote:
> On 02/15/2016 12:24 AM, Hannes Reinecke wrote:
>>   if (optimize)
>> -h->flags |= ALUA_OPTIMIZE_STPG;
>> +pg->flags |= ALUA_OPTIMIZE_STPG;
>>   else
>> -h->flags &= ~ALUA_OPTIMIZE_STPG;
>> +pg->flags |= ~ALUA_OPTIMIZE_STPG;
> 
> The above looks weird to me. Why has "&=" been changed into "|=" ?
> 
Bah. Of course.

Will be fixing it up.

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: dm-multipath test scripts

[Junichi Nomura]

Hi Mike,

On 02/19/16 02:17, Mike Snitzer wrote:
> But unfortunately I cannot get either the scsidebug or tcmloop mode to
> run against v4.5-rc4
> 
> For tcmloop, targetcli fails with:
> "Could not create ISCSIFabricModule in configFS."

Hmm, it sounds like there's unnecessary dependency in targetcli.

> (fixed by enabling CONFIG_ISCSI_TARGET under TARGET_CORE)

OK.

> I'm seeing all tests fail due to fio verification failure.  I'll need to
> inspect this further..
> 
> But the most problematic test is ./tests/test_03_dm_failpath -- it seems
> to actively break _any_ v4.5-rc kernel I try (with a never-ending flood
> of messages like "device-mapper: multipath: Failing path 8:192."); I
> haven't tried older kernels.

It seems fail/recover cycle runs too fast for I/O to make any progress.
I hit similar case and had to slow down the stress with attached patch.
Please try this. Sorry for the inconvenience.

> What is the last kernel version that your scripts have worked on?

v4.4 worked fine. I'll check with v4.5-rc4 when I get a machine.

> Taking a step back:
> These scripts don't belong in Documentation/device-mapper/mptest/ (or
> anywhere in the kernel tree for that matter).
> 
> I'd really prefer it if we could port your scripts over to the
> device-mapper-test-suite, see:
> https://github.com/jthornber/device-mapper-test-suite

Yes, I agree such a project is better place for this to live.

-- 
Jun'ichi Nomura, NEC Corporation

diff --git a/lib/failpath_dm_message b/lib/failpath_dm_message
index 1a3bcf8..5b8f28a 100755
--- a/lib/failpath_dm_message
+++ b/lib/failpath_dm_message
@@ -30,9 +30,11 @@ start_failpath_dm_message () {
for m in $majs; do
dmsetup message $MPNAME 0 "fail_path $m"
done
+   sleep 1
for m in $majs; do
dmsetup message $MPNAME 0 "reinstate_path $m"
done
+   sleep 1
done &
 }
 --
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 3/7] scsi: Drop runtime PM usage count after host is added

[Julian Calaby]

Hi Mika,

On Thu, Feb 18, 2016 at 7:54 PM, Mika Westerberg
 wrote:
> Runtime PM of the SCSI host is already handled by calls to
> scsi_autopm_get_host() and scsi_autopm_put_host() from appropriate places
> whenever the host needs to be powered on. This works fine when there is
> device connected to the host as once it runtime suspends the host will too.
>
> However, if there is no device connected the host is never runtime
> suspended (the usage counter is always 0).
>
> Allow runtime suspend of host even if it has no devices connected by
> calling scsi_autopm_put_host() at the end of scsi_add_host_with_dma(). We
> temporarily increase runtime PM usage counter first so call to
> scsi_autopm_put_host() will result idle request to be scheduled for the
> device.
>
> Signed-off-by: Mika Westerberg 
> ---
>  drivers/scsi/hosts.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
> index 82ac1cd818ac..e46bf4d152a0 100644
> --- a/drivers/scsi/hosts.c
> +++ b/drivers/scsi/hosts.c
> @@ -250,6 +250,12 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, 
> struct device *dev,
> if (error)
> goto out_destroy_freelist;
>
> +   /*
> +* Increase usage count temporarily here so that calling
> +* scsi_autopm_put_host() will trigger runtime idle if there is
> +* nothing else preventing suspending the device.
> +*/
> +   pm_runtime_get_noresume(>shost_gendev);
> pm_runtime_set_active(>shost_gendev);
> pm_runtime_enable(>shost_gendev);
> device_enable_async_suspend(>shost_gendev);
> @@ -290,6 +296,7 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, 
> struct device *dev,
> goto out_destroy_host;
>
> scsi_proc_host_add(shost);
> +   scsi_autopm_put_host(shost);

Would it be cleaner to export the code that runs when the usage
counter decrements and call it here?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
--
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: [PATCHv7 22/23] scsi_dh_alua: update 'access_state' field

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

Track attached SCSI devices and update the 'access_state' field
whenever an ALUA state change has been detected.


Reviewed-by: Bart Van Assche 
--
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: [PATCHv7 21/23] scsi_dh_alua: use common definitions for ALUA state

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

@@ -180,7 +172,7 @@ static int submit_stpg(struct scsi_device *sdev, int 
group_id,

/* Prepare the data buffer */
memset(stpg_data, 0, stpg_len);
-   stpg_data[4] = TPGS_STATE_OPTIMIZED & 0x0f;
+   stpg_data[4] = SCSI_ACCESS_STATE_OPTIMAL & 0x0f;
put_unaligned_be16(group_id, _data[6]);



In the previous patch a symbolic name for the ALUA state mask has been 
introduced (SCSI_ACCESS_STATE_MASK). Hence please either leave out "& 
0x0f" from the above code or use that constant.


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

Re: [PATCHv7 20/23] scsi: Add 'access_state' attribute

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

--- a/include/scsi/scsi_proto.h
+++ b/include/scsi/scsi_proto.h
@@ -277,5 +277,17 @@ struct scsi_lun {
__u8 scsi_lun[8];
  };

+/* SPC asymmetric access states */
+#define SCSI_ACCESS_STATE_OPTIMAL 0x00
+#define SCSI_ACCESS_STATE_ACTIVE  0x01
+#define SCSI_ACCESS_STATE_STANDBY 0x02
+#define SCSI_ACCESS_STATE_UNAVAILABLE 0x03
+#define SCSI_ACCESS_STATE_LBA 0x04
+#define SCSI_ACCESS_STATE_OFFLINE 0x0e
+#define SCSI_ACCESS_STATE_TRANSITIONING 0x0f
+
+#define SCSI_ACCESS_STATE_MASK0x0f
+#define SCSI_ACCESS_STATE_PREFERRED   0x80
+#define SCSI_ACCESS_STATE_UNKNOWN 0x70


Please mention in the comment above these constants that the ALUA state 
values apply to both the RTPG and STPG commands but that the PREFERRED 
bit only applies to the RTPG command.


All constants in scsi_proto.h come from a SCSI standard. However, this 
patch adds a constant that does not come from a SCSI standard 
(SCSI_ACCESS_STATE_UNKNOWN). Please remove this constant entirely 
because it causes confusion. sdev_show_access_state() only interprets 
the PREFERRED bit and the lower 4 bits of the access_state member 
variable. This means that the SCSI_ACCESS_STATE_UNKNOWN entry in 
sdev_access_states[] is never used and hence should be left out. This 
also means that the "sdev->access_state = SCSI_ACCESS_STATE_UNKNOWN" 
assignment in scsi_scan.c is equivalent to "sdev->access_state = 
SCSI_ACCESS_STATE_OPTIMAL".


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

Re: [PATCHv7 20/23] scsi: Add 'access_state' attribute

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

Add an 'access_state' attribute to struct scsi_device to
display the asymmetric LUN access state.


Do we really need to add this attribute to SCSI devices that do not 
support ALUA, e.g. ATA devices ? From a system on which this patch 
series has been installed:


[root ~]# lsscsi | grep 0:0:0:0
[0:0:0:0] disk ATA ST1000NM0033-9ZM GA67 /dev/sda
[root ~]# cat /sys/class/scsi_device/0:0:0:0/device/access_state
active/optimized

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

Re: [PATCHv7 19/23] scsi_dh: add 'rescan' callback

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

If a device needs to be rescanned the device_handler might need
to be rechecked, too.
So add a 'rescan' callback to the device handler and call it
upon scsi_rescan_device(). The rescan callback will be invoked
from the Unit Attention handling of ASC/ASCQ 3F 03
(INQUIRY DATA HAS CHANGED).


Reviewed-by: Bart Van Assche 
--
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: [PATCHv7 18/23] scsi_dh_alua: Send TEST UNIT READY to poll for transitioning

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

Sending a 'REPORT TARGET PORT GROUP' command is a costly operation,
as the array has to gather information about all ports.
So instead of using RTPG to poll for a status update when a port
is in transitioning we should be sending a TEST UNIT READY, and
wait for the sense code to report success.


Reviewed-by: Bart Van Assche 
--
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: [PATCHv7 17/23] scsi_dh_alua: update all port states

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

When we read in the target port group state we should be
updating all affected port groups, otherwise we risk
running out of sync.


Reviewed-by: Bart Van Assche 
--
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: [PATCHv7 16/23] scsi_dh_alua: Recheck state on unit attention

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

When we receive a unit attention code of 'ALUA state changed'
we should recheck the state, as it might be due to an implicit
ALUA state transition. This allows us to return NEEDS_RETRY
instead of ADD_TO_MLQUEUE, allowing to terminate the retries
after a certain time.
At the same time a workqueue item might already be queued, which
should be started immediately to avoid any delays.


Reviewed-by: Bart Van Assche 
--
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: [PATCHv7 15/23] scsi_dh_alua: Add new blacklist flag 'BLIST_SYNC_ALUA'

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

Add a new blacklist flag BLIST_SYNC_ALUA to instruct the
alua device handler to use synchronous command submission
for ALUA commands.


Reviewed-by: Bart Van Assche 
--
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: [PATCHv7 14/23] scsi_dh_alua: Allow workqueue to run synchronously

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

Some arrays may only capable of handling one STPG at a time,
so this patch adds a singlethreaded workqueue for STPGs to be
submitted synchronously.


Reviewed-by: Bart Van Assche 
--
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: [PATCHv7 13/23] scsi_dh_alua: Use workqueue for RTPG

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

The current ALUA device_handler has two drawbacks:
- We're sending a 'SET TARGET PORT GROUP' command to every LUN,
   disregarding the fact that several LUNs might be in a port group
   and will be automatically switched whenever _any_ LUN within
   that port group receives the command.
- Whenever a LUN is in 'transitioning' mode we cannot block I/O
   to that LUN, instead the controller has to abort the command.
   This leads to increased traffic across the wire and heavy load
   on the controller during switchover.

With this patch the RTPG handling is moved to a per-portgroup
workqueue. This reduces the number of 'REPORT TARGET PORT GROUP'
and 'SET TARGET PORT GROUPS' sent to the controller as we're sending
them now per port group, and not per device as previously.
It also allows us to block I/O to any LUN / port group found to be
in 'transitioning' ALUA mode, as the workqueue item will be requeued
until the controller moves out of transitioning.


Reviewed-by: Bart Van Assche 
--
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: [Announce] sg3_utils-1.42 available

[Douglas Gilbert]

On 16-02-17 11:59 PM, Douglas Gilbert wrote:

sg3_utils is a package of command line utilities for sending
SCSI and some ATA commands to devices. This package targets
the Linux 4, 3, 2.6 and 2.4 kernel series. It has ports to
FreeBSD, Tru64, Solaris, and Windows (cygwin and MinGW).

There are two new utilities (sg_read_attr and sg_timestamp)
and additions to many others, see the ChangeLog below. This
version tracks various changes made by www.t10.org since May
2015 until January 2016.


Missed the links:
For an overview of sg3_utils and downloads see this page:
http://sg.danny.cz/sg/sg3_utils.html
The sg_ses utility (for enclosure devices) is discussed at:
http://sg.danny.cz/sg/sg_ses.html
A full changelog can be found at:
http://sg.danny.cz/sg/p/sg3_utils.ChangeLog


Changelog for sg3_utils-1.42 [20160217] [svn: r663]
   - sg_timestamp: new, to report or set timestamp
   - sg_read_attr: new, supported by tape drives
   - sg_stpg: fix truncation of target port field
   - sg_inq: cope with unicode strings, udev fixes
 - update version descriptor list to 20160125
 - '--export': new entries for UUID descriptor
   - sg_ses: add more field acronyms (ses3r11)
   - sg_logs: add Utilization lpage (sbc4r07)
 - add Background operation lpage
 - add Pending defects lpage
 - add LPS misalignment lpage (sbc4r10)
 - document '--All' ('-A') option
 - rework lto tape vendor lpages
   - sg_vpd: add Block limits extension VPD page
 - add Device constituents VPD page
 - add LB Protection VPD page (ssc 15-296r1)
 - LB provisioning VPD page: expand LBPRZ, add
   Minimum and Threshold percentage fields
 - rework lto tape vendor VPD pages
   - sg_inq+sg_vpd+sg_xcopy: add support for locally
 assigned UUIDs in VPD page 0x83 (spc5r08)
   - sg_sanitize: add --znr option (sbc4r07)
   - sg_rep_zones: add --partial option (zbc-r04)
   - sg_format: add ffmt option (sbc4r10)
 - add support for FORMAT MEDIUM (for tape)
   - sg_raw: document length relationships
   - rescan-scsi-bus.sh: updates from Suse
   - sg_lib_data: sync asc/ascq codes with T10 20151126
   - sg_lib: add 'sense' categories for SCSI statuses:
 condition met, busy, task set full, ACA active and
 task aborted
 - add pr2serr() extern
 - change sg_get_sense_str() and dStrHexStr(), return
   chars written (returned void previously)
 - add sg_get_sense_descriptors_str() function
 - add sg_get_designation_descriptor_str() function
 - sg_get_desig_type_str()+sg_get_desig_assoc_str()
   and sg_get_desig_code_set_str() added
 - sg_get_opcode_sa_name() break out zoning in/out,
   read attribute and read position service actions
   - sg_cmds_extra: add sg_ll_format_unit2() for FFMT
   - sg_pr2serr.h: new, to shorten fprintf(stderr, ...)
   - sg_io_linux, sg_pt_linux: drop SUGGEST_* decoding
   - sg_unaligned.h: add 48 bit support and gets for
 variable length unsigned integers

Changelog for sg3_utils-1.41 [20150511] [svn: r644]
...

Doug Gilbert


--
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: [PATCHv7 12/23] scsi_dh_alua: remove 'rel_port' from alua_dh_data structure

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

The 'relative port' field is not used, and might get stale when
the port group changes. So remove the field altogether.


Reviewed-by: Bart Van Assche 
--
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: [PATCHv7 11/23] scsi_dh_alua: move optimize_stpg evaluation

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

When the optimize_stpg module option is set we should just set it
once during port_group allocation. Doing so allows us to override
it later with device specific settings.


Reviewed-by: Bart Van Assche 
--
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: [PATCHv7 10/23] revert commit a8e5a2d593cb ("[SCSI] scsi_dh_alua: ALUA handler attach should succeed while TPG is transitioning")

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

This reverts commit a8e5a2d593cbfccf530c3382c2c328d2edaa7b66


Reviewed-by: Bart Van Assche 
--
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: [PATCHv7 09/23] scsi_dh_alua: simplify alua_initialize()

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

Rework alua_check_vpd() to use scsi_vpd_get_tpg()
and move the port group selection into the function, too.
With that we can simplify alua_initialize() to just
call alua_check_tpgs() and alua_check_vpd();


Reviewed-by: Bart Van Assche 
--
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: [PATCHv7 08/23] scsi_dh_alua: use unique device id

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

Use scsi_vpd_lun_id() to assign a unique device identification
to the alua port group structure.


Reviewed-by: Bart Van Assche 
--
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: [PATCHv7 07/23] scsi_dh_alua: Use separate alua_port_group structure

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

if (optimize)
-   h->flags |= ALUA_OPTIMIZE_STPG;
+   pg->flags |= ALUA_OPTIMIZE_STPG;
else
-   h->flags &= ~ALUA_OPTIMIZE_STPG;
+   pg->flags |= ~ALUA_OPTIMIZE_STPG;


The above looks weird to me. Why has "&=" been changed into "|=" ?

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

Re: [PATCHv7 05/23] scsi_dh_alua: switch to scsi_execute_req_flags()

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

All commands are issued synchronously, so no need to open-code
scsi_execute_req_flags() anymore. And we can get rid of the
static sense code structure element. scsi_execute_req_flags()
will be setting REQ_QUIET and REQ_PREEMPT, but that is
perfectly fine as we're evaluating and logging any errors
ourselves and we really need to send the command even if
the device is quiesced.


Reviewed-by: Bart Van Assche 
--
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: [PATCHv7 03/23] scsi_dh_alua: Make stpg synchronous

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

default:
+   sdev_printk(KERN_INFO, sdev,
+   "%s: stpg failed, unhandled TPGS state %d",
+   ALUA_DH_NAME, h->state);
+   return SCSI_DH_NOSYS;
break;
}


The "break" above should have been removed. Anyway:

Reviewed-by: Bart Van Assche 
--
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: [PATCHv7 04/23] scsi_dh_alua: call alua_rtpg() if stpg fails

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

If the call to SET TARGET PORT GROUPS fails we have no idea what
state the array is left in, so we need to issue a call to
REPORT TARGET PORT GROUPS in these cases.


Reviewed-by: Bart Van Assche 
--
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: [PATCHv7 02/23] scsi_dh_alua: separate out alua_stpg()

[Bart Van Assche]

On 02/15/2016 12:24 AM, Hannes Reinecke wrote:

Separate out SET TARGET PORT GROUP functionality into a separate
function alua_stpg().


Reviewed-by: Bart Van Assche 
--
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

[GIT PULL] SCSI fixes for 4.5-rc4

[James Bottomley]

Two simple fixes.  One prevents a soft lockup on some target removal
scenarios and the other prevents us trying to probe the marvell console
device, which causes it to time out and need the bus resetting.

The patch is available here:

git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes

The short changelog is:

James Bottomley (1):
  scsi: fix soft lockup in scsi_remove_target() on module removal

Todd Fujinaka (1):
  SCSI: Add Marvell configuration device to VPD blacklist

And the diffstat:

 drivers/scsi/scsi_devinfo.c | 1 +
 drivers/scsi/scsi_sysfs.c   | 6 --
 2 files changed, 5 insertions(+), 2 deletions(-)

With the full diff below.

James

---

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index da2e068..bbfbfd9 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -206,6 +206,7 @@ static struct {
{"iRiver", "iFP Mass Driver", NULL, BLIST_NOT_LOCKABLE | 
BLIST_INQUIRY_36},
{"LASOUND", "CDX7405", "3.10", BLIST_MAX5LUN | BLIST_SINGLELUN},
{"Marvell", "Console", NULL, BLIST_SKIP_VPD_PAGES},
+   {"Marvell", "91xx Config", "1.01", BLIST_SKIP_VPD_PAGES},
{"MATSHITA", "PD-1", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
{"MATSHITA", "DMC-LC5", NULL, BLIST_NOT_LOCKABLE | BLIST_INQUIRY_36},
{"MATSHITA", "DMC-LC40", NULL, BLIST_NOT_LOCKABLE | BLIST_INQUIRY_36},
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 4f18a85..00bc721 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1272,16 +1272,18 @@ static void __scsi_remove_target(struct scsi_target 
*starget)
 void scsi_remove_target(struct device *dev)
 {
struct Scsi_Host *shost = dev_to_shost(dev->parent);
-   struct scsi_target *starget;
+   struct scsi_target *starget, *last_target = NULL;
unsigned long flags;
 
 restart:
spin_lock_irqsave(shost->host_lock, flags);
list_for_each_entry(starget, >__targets, siblings) {
-   if (starget->state == STARGET_DEL)
+   if (starget->state == STARGET_DEL ||
+   starget == last_target)
continue;
if (starget->dev.parent == dev || >dev == dev) {
kref_get(>reap_ref);
+   last_target = starget;
spin_unlock_irqrestore(shost->host_lock, flags);
__scsi_remove_target(starget);
scsi_target_reap(starget);
--
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: dm-multipath test scripts

[Mike Snitzer]

On Wed, Oct 07 2015 at  1:39am -0400,
Junichi Nomura  wrote:

> This is a set of scripts for kernel-side dm-multipath testing. Current
> set of scripts are stress testing of extreme situation and its coverage
> is limited. But recently found dm-mpath regressions should be detectable
> with this. I hope this helps people working on dm-multipath related code
> as a regression test set.
> 
> Though this is created as a patch for linux kernel source, it's just for
> convenience of kernel developers (I don't have a place to host this kind
> of scripts). If there are better place to contribute this, please let me know.
> 
> Please check README for how-to-run and possible improvements you could
> contribute in future.

I finally got around to trying these scripts.  Thanks for making them
available.

But unfortunately I cannot get either the scsidebug or tcmloop mode to
run against v4.5-rc4

For tcmloop, targetcli fails with:
"Could not create ISCSIFabricModule in configFS."
(fixed by enabling CONFIG_ISCSI_TARGET under TARGET_CORE)

I'm seeing all tests fail due to fio verification failure.  I'll need to
inspect this further..

But the most problematic test is ./tests/test_03_dm_failpath -- it seems
to actively break _any_ v4.5-rc kernel I try (with a never-ending flood
of messages like "device-mapper: multipath: Failing path 8:192."); I
haven't tried older kernels.

What is the last kernel version that your scripts have worked on?

Taking a step back:
These scripts don't belong in Documentation/device-mapper/mptest/ (or
anywhere in the kernel tree for that matter).

I'd really prefer it if we could port your scripts over to the
device-mapper-test-suite, see:
https://github.com/jthornber/device-mapper-test-suite

Mike
--
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] fusion-mptbase: handle failed allocation for workqueue

[Ewan Milne]

On Thu, 2016-02-18 at 11:40 -0500, Ewan Milne wrote:
> It also appears to me upon further inspection that the existing code has
> other problems.  In particular, once mpt_mapresources() has returned
> with a nonzero error code, it looks like iounmap() should be called, but
> it is not in the case of a failed allocation of reset_work_q.  I'm also
> not sure why pci_release_selected_regions() is only called for the case
> of mpt_do_ioc_recovery() returning != -5 when it is called whenever
> there is a failed allocation of reset_work_q.
> 
> Consider the attached patch (untested, because I don't have hardware):
> It shows what I would do for labels & error handling.  If the rc != -5
> case of return from mpt_do_ioc_recovery() could be eliminated, then
> another label "out_free_fw_event_q:" could be added prior to the other
> error cases at the end, and all the code after the printk() in that path
> could be replaced by "goto out_free_fw_event_q;"
> 
>   if ((r = mpt_do_ioc_recovery(ioc, MPT_HOSTEVENT_IOC_BRINGUP,
>   CAN_SLEEP)) != 0){
>   printk(MYIOC_s_ERR_FMT "didn't initialize properly! (%d)\n",
>   ioc->name, r);
>   goto out_free_fw_event_q;
>   }
> ...
> 
> out_free_fw_event_q:
>   destroy_workqueue(ioc->fw_event_q);
>   ioc->fw_event_q = NULL;
> 
> out_remove_ioc:
> ...
> 
> However I do not know if that change is legitimate.
> 
> -Ewan

There is an error in the patch attached in my previous mail.
Please refer to the attached PATCH v2 RFC.

-Ewan

>From 79fd59c66c3afb339cf9fdd1fda65e78e32831a4 Mon Sep 17 00:00:00 2001
From: "Ewan D. Milne" 
Date: Thu, 18 Feb 2016 11:49:37 -0500
Subject: [PATCH v2 RFC] mptbase: fixup error handling paths in mpt_attach()

mpt_attach() was not checking for the failure to create fw_event_q.
Also, iounmap() was not being called in all error cases after ioremap()
had been called by mpt_mapresources().
---
 drivers/message/fusion/mptbase.c | 37 +++--
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 5dcc031..a4f362b 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -1801,8 +1801,7 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	ioc->pcidev = pdev;
 	if (mpt_mapresources(ioc)) {
-		kfree(ioc);
-		return r;
+		goto out_free_ioc;
 	}
 
 	/*
@@ -1871,9 +1870,8 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (!ioc->reset_work_q) {
 		printk(MYIOC_s_ERR_FMT "Insufficient memory to add adapter!\n",
 		ioc->name);
-		pci_release_selected_regions(pdev, ioc->bars);
-		kfree(ioc);
-		return -ENOMEM;
+		r = -ENOMEM;
+		goto out_unmap_resources;
 	}
 
 	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "facts @ %p, pfacts[0] @ %p\n",
@@ -1995,12 +1993,21 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
 	spin_lock_init(>fw_event_lock);
 	snprintf(ioc->fw_event_q_name, MPT_KOBJ_NAME_LEN, "mpt/%d", ioc->id);
 	ioc->fw_event_q = create_singlethread_workqueue(ioc->fw_event_q_name);
+	if (!ioc->fw_event_q) {
+		printk(MYIOC_s_ERR_FMT "Insufficient memory to add adapter!\n",
+		ioc->name);
+		r = -ENOMEM;
+		goto out_remove_ioc;
+	}
 
 	if ((r = mpt_do_ioc_recovery(ioc, MPT_HOSTEVENT_IOC_BRINGUP,
 	CAN_SLEEP)) != 0){
 		printk(MYIOC_s_ERR_FMT "didn't initialize properly! (%d)\n",
 		ioc->name, r);
 
+		destroy_workqueue(ioc->fw_event_q);
+		ioc->fw_event_q = NULL;
+
 		list_del(>list);
 		if (ioc->alt_ioc)
 			ioc->alt_ioc->alt_ioc = NULL;
@@ -2040,6 +2047,24 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
 			msecs_to_jiffies(MPT_POLLING_INTERVAL));
 
 	return 0;
+
+out_remove_ioc:
+	list_del(>list);
+	if (ioc->alt_ioc)
+		ioc->alt_ioc->alt_ioc = NULL;
+	pci_set_drvdata(ioc->pcidev, NULL);
+
+	destroy_workqueue(ioc->reset_work_q);
+	ioc->reset_work_q = NULL;
+
+out_unmap_resources:
+	iounmap(ioc->memmap);
+	pci_release_selected_regions(pdev, ioc->bars);
+
+out_free_ioc:
+	kfree(ioc);
+
+	return r;
 }
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
@@ -6229,7 +6254,7 @@ mpt_get_manufacturing_pg_0(MPT_ADAPTER *ioc)
 	memcpy(ioc->board_assembly, pbuf->BoardAssembly, sizeof(ioc->board_assembly));
 	memcpy(ioc->board_tracer, pbuf->BoardTracerNumber, sizeof(ioc->board_tracer));
 
-	out:
+out:
 
 	if (pbuf)
 		pci_free_consistent(ioc->pcidev, hdr.PageLength * 4, pbuf, buf_dma);
-- 
2.1.0

RE: [PATCH] target: Fix incorrect unmap_zeroes_data_store return

[Pocas, Jamie]

Sorry about this. I am working on an older 3.10 codebase (RHEL7.x) where the 
equivalent function int se_dev_set_unmap_zeroes_data(struct se_device *dev, int 
flag) does and SHOULD return 0. There are apparently more changes between 
configfs in 3.10 and latest than I found. That one slipped by when I ported it 
up to latest. Good catch!

-Original Message-
From: Nicholas A. Bellinger [mailto:n...@daterainc.com] 
Sent: Thursday, February 11, 2016 2:02 AM
To: target-devel; linux-scsi
Cc: Christoph Hellwig; Pocas, Jamie; Nicholas Bellinger
Subject: [PATCH] target: Fix incorrect unmap_zeroes_data_store return

From: Nicholas Bellinger 

This patch fixes an incorrect return of zero from the new
unmap_zeroes_data_store() configfs store attribute handler introduced in 
v4.5-rc1, to use the correct 'count' bytes return value.

Signed-off-by: Nicholas Bellinger 
---
 drivers/target/target_core_configfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/target_core_configfs.c 
b/drivers/target/target_core_configfs.c
index 3327c49..713c63d9 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -898,7 +898,7 @@ static ssize_t unmap_zeroes_data_store(struct config_item 
*item,
da->unmap_zeroes_data = flag;
pr_debug("dev[%p]: SE Device Thin Provisioning LBPRZ bit: %d\n",
 da->da_dev, flag);
-   return 0;
+   return count;
 }
 
 /*
--
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: [PATCH v2] fusion-mptbase: handle failed allocation for workqueue

[Ewan Milne]

On Thu, 2016-02-18 at 10:00 +0100, Johannes Thumshirn wrote:
> On Wed, Feb 17, 2016 at 11:40:59PM -0500, Insu Yun wrote:
> > the failure of ioc->reset_work_q is checked,
> > but not ioc->fw_event_q.
> > 
> > Signed-off-by: Insu Yun 
> > ---
> >  drivers/message/fusion/mptbase.c | 44 
> > 
> >  1 file changed, 27 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/message/fusion/mptbase.c 
> > b/drivers/message/fusion/mptbase.c
> > index 5dcc031..53a5015 100644
> > --- a/drivers/message/fusion/mptbase.c
> > +++ b/drivers/message/fusion/mptbase.c
> > @@ -1871,9 +1871,8 @@ mpt_attach(struct pci_dev *pdev, const struct 
> > pci_device_id *id)
> > if (!ioc->reset_work_q) {
> > printk(MYIOC_s_ERR_FMT "Insufficient memory to add adapter!\n",
> > ioc->name);
> > -   pci_release_selected_regions(pdev, ioc->bars);
> > -   kfree(ioc);
> > -   return -ENOMEM;
> > +   r = -ENOMEM;
> > +   goto err3;
> > }
> >  
> > dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "facts @ %p, pfacts[0] @ %p\n",
> > @@ -1996,24 +1995,16 @@ mpt_attach(struct pci_dev *pdev, const struct 
> > pci_device_id *id)
> > snprintf(ioc->fw_event_q_name, MPT_KOBJ_NAME_LEN, "mpt/%d", ioc->id);
> > ioc->fw_event_q = create_singlethread_workqueue(ioc->fw_event_q_name);
> >  
> > +   if (!ioc->fw_event_q) {
> > +   r = -ENOMEM;
> > +   goto err2;
> > +   }
> > +
> > if ((r = mpt_do_ioc_recovery(ioc, MPT_HOSTEVENT_IOC_BRINGUP,
> > CAN_SLEEP)) != 0){
> > printk(MYIOC_s_ERR_FMT "didn't initialize properly! (%d)\n",
> > ioc->name, r);
> > -
> > -   list_del(>list);
> > -   if (ioc->alt_ioc)
> > -   ioc->alt_ioc->alt_ioc = NULL;
> > -   iounmap(ioc->memmap);
> > -   if (r != -5)
> > -   pci_release_selected_regions(pdev, ioc->bars);
> > -
> > -   destroy_workqueue(ioc->reset_work_q);
> > -   ioc->reset_work_q = NULL;
> > -
> > -   kfree(ioc);
> > -   pci_set_drvdata(pdev, NULL);
> > -   return r;
> > +   goto err1;
> > }
> >  
> > /* call per device driver probe entry point */
> > @@ -2040,6 +2031,25 @@ mpt_attach(struct pci_dev *pdev, const struct 
> > pci_device_id *id)
> > msecs_to_jiffies(MPT_POLLING_INTERVAL));
> >  
> > return 0;
> > +
> > +err1:
> > +   destroy_workqueue(ioc->fw_event_q);
> > +   ioc->fw_event_q = NULL;
> > +err2::
> > +   destroy_workqueue(ioc->reset_work_q);
> > +   ioc->reset_work_q = NULL;
> > +
> > +   list_del(>list);
> > +   if (ioc->alt_ioc)
> > +   ioc->alt_ioc->alt_ioc = NULL;
> > +   iounmap(ioc->memmap);
> > +   pci_set_drvdata(pdev, NULL);
> > +err3:
> > +   if (r != -5)
> > +   pci_release_selected_regions(pdev, ioc->bars);
> > +   kfree(ioc);
> > +   return r;
> > +
> >  }
> 
> Please no. Not err1, err2 and err3.
> 
> err1 could be "goto destroy_fw_event_q", err2 "destroy_reset_workq", err3
> goto "free_ioc".

It also appears to me upon further inspection that the existing code has
other problems.  In particular, once mpt_mapresources() has returned
with a nonzero error code, it looks like iounmap() should be called, but
it is not in the case of a failed allocation of reset_work_q.  I'm also
not sure why pci_release_selected_regions() is only called for the case
of mpt_do_ioc_recovery() returning != -5 when it is called whenever
there is a failed allocation of reset_work_q.

Consider the attached patch (untested, because I don't have hardware):
It shows what I would do for labels & error handling.  If the rc != -5
case of return from mpt_do_ioc_recovery() could be eliminated, then
another label "out_free_fw_event_q:" could be added prior to the other
error cases at the end, and all the code after the printk() in that path
could be replaced by "goto out_free_fw_event_q;"

if ((r = mpt_do_ioc_recovery(ioc, MPT_HOSTEVENT_IOC_BRINGUP,
CAN_SLEEP)) != 0){
printk(MYIOC_s_ERR_FMT "didn't initialize properly! (%d)\n",
ioc->name, r);
goto out_free_fw_event_q;
}
...

out_free_fw_event_q:
destroy_workqueue(ioc->fw_event_q);
ioc->fw_event_q = NULL;

out_remove_ioc:
...

However I do not know if that change is legitimate.

-Ewan

> >  
> >  
> > /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
> > -- 
> > 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
> 

>From 83f117b81d58d01bfe7e01e04e4f2f2a602e2c9f Mon Sep 17 00:00:00 2001
From: "Ewan D. Milne" 
Date: Thu, 

Re: [PATCH 0/7] Runtime PM support for AHCI host controller driver

[Tejun Heo]

Hello,

Patchset looks good to me.  Once Jens acks the first patch, I'll apply
the whole series to libata/for-4.6 w/ the field initialization
updated.

Thanks.

-- 
tejun
--
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: NULL pointer dereference: IP: [] sr_runtime_suspend+0xc/0x20 [sr_mod]

[Alan Stern]

On Tue, 9 Feb 2016, Alexandre Rossi wrote:

> Hi,
> 
> netconsole does not seem to work so early in the boot process this time.
> 
> > As this is Linux 4.3 and not 4.4, I guess this is a different problem
> > though. Alexandre, where you able to capture the stack trace? I’d submit
> > a new bug report with this.
> 
> Here is a photo. Please ping me if you need to test some debugging patches.

It looks like the problem occurs in blk_post_runtime_resume().  Since 
there have been recent changes to this routine, it's hard to tell 
whether you're using the most up-to-date code.

In particular, the first few lines of blk_post_runtime_resume() in 
block/blk-core.c should look like this:

void blk_post_runtime_resume(struct request_queue *q, int err)
{
if (!q->dev)
return;

The test was introduced by commit 4fd41a8552af ("SCSI: Fix NULL pointer
dereference in runtime PM"), which was added to the mainline kernel
between 4.3 and 4.4.  I don't know what the commit ID would be for a
.stable kernel.

Alan Stern

--
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 5/7] ahci: Convert driver to use modern PM hooks

[Christoph Hellwig]

On Thu, Feb 18, 2016 at 08:12:46AM -0500, Tejun Heo wrote:
> On Thu, Feb 18, 2016 at 12:45:05PM +0200, Andy Shevchenko wrote:
> > > +   .driver.pm  = _pci_pm_ops,
> > 
> > Please, do this in several assignments, since to support older compilers.
> 
> Can you please elaborate?  That's a single assignment.  Are there
> compilers which can't do that?

Old gcc (I think 4.4 is the last one) needs:

.driver = {
.pm = _pci_pm_ops,
},
--
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 v8 3/3] add support for DWC UFS Host Controller

[Rob Herring]

On Mon, Feb 15, 2016 at 03:25:13PM +, Joao Pinto wrote:
> This patch has the goal to add support for DesignWare UFS Controller
> specific operations and to add specific platform and pci drivers.
> 
> Signed-off-by: Joao Pinto 
> ---
> Changes v7->v8 (Akinobu Mita):
> - DME sets were simplified for easier reading
> - CLK DIV default values definitions names were changed to match the
>  register's name
> - New line added to dev_err and dev_info statements
> Changes v6->v7 (Arnd Bergmann):
> - Changed DT node name (to ufs only) and the memory address (to 0xd00)
> - Removed CONFIG_PM from the PCI glue driver (pm.h already does this)
> - No other changes are necessary in ufshcd.c because of the link up notify
>  function usage (it is simpler now)
> - Removed the PHY mentioning since the Test Chip is not a real PHY for real
>  world usage, since it is a test chip for prototyping with a very specific
>  usage
> - Added again the Test Chip 20-bit option
> Changes v5->v6:
> - Patch bad format fixed
> Changes v4->v5 (Akinobu Mita):
> - All functions used only locally in ufshcd-dwc are now declared as static
> - ufshcd_dwc_configuration() was removed in ufshcd-dwc and a notify
>  function (ufshcd_dwc_link_startup_notify) was created to deal with the
>  DWC specific init routines
> - 20-bit RMMI option was removed from Kconfig. Now if MPHY TC is selected
>  and 40-bit is not then it assumes a 20-bit config
> Changes v3->v4 (Arnd Bergmann and Mark Rutland):
> - SCSI_UFS_DWC_HOOKS is now silent and selected by the SCSI_UFS_DWC_PLAT
>  or SCSI_UFS_DWC_PCI
> - Compatibility string has the ufs core version for info purposes since
>  the driver is capable of getting the controller version from its 
>  registers
> - Created ufs-dwc-pci glue driver with specific DWC data
> - MPHY configuration remains in the ufshcd-dwc since it is unipro
>  attribute writting only not following the a linux phy framework logic
> Changes v2->v3 (Julian Calaby):
> - Implement a common DWC code to be used by the platform and pci glue
>  drivers
> - Synopsys ID & Class added to the existing pci driver and specific DWC
>  was also added to the pci driver
> Changes v1->v2 (Akinobu Mita):
> - Implement a platform driver that uses the existing UFS core driver
> - Add DWC specific code to the existing UFS core driver
> 
>  Documentation/devicetree/bindings/ufs/ufs-dwc.txt |  19 +
>  MAINTAINERS   |   6 +
>  drivers/scsi/ufs/Kconfig  |  51 +++
>  drivers/scsi/ufs/Makefile |   3 +
>  drivers/scsi/ufs/ufs-dwc-pci.c| 172 +
>  drivers/scsi/ufs/ufs-dwc.c|  96 +
>  drivers/scsi/ufs/ufshcd-dwc.c | 431 
> ++
>  drivers/scsi/ufs/ufshcd-dwc.h |  18 +
>  drivers/scsi/ufs/ufshci-dwc.h |  42 +++
>  drivers/scsi/ufs/unipro.h |  39 ++
>  10 files changed, 877 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ufs/ufs-dwc.txt
>  create mode 100644 drivers/scsi/ufs/ufs-dwc-pci.c
>  create mode 100644 drivers/scsi/ufs/ufs-dwc.c
>  create mode 100644 drivers/scsi/ufs/ufshcd-dwc.c
>  create mode 100644 drivers/scsi/ufs/ufshcd-dwc.h
>  create mode 100644 drivers/scsi/ufs/ufshci-dwc.h
> 
> diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt 
> b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
> new file mode 100644
> index 000..59e9822
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
> @@ -0,0 +1,19 @@
> +* Universal Flash Storage (UFS) DesignWare Host Controller
> +
> +DWC_UFSHC nodes are defined to describe on-chip UFS host controllers.
> +Each UFS controller instance should have its own node.
> +
> +Required properties:
> +- compatible: compatible list must contain "snps,ufshcd-dwc" and 
> should
> +   also contain the JEDEC version of the controller:
> + "jedec,ufs-1.1"
> + "jedec,ufs-2.0"

As I said before, this needs to mention also requiring an SoC specific 
compatible.

> +- reg   : 
> +- interrupts: 
> +
> +Example:
> + ufs@0xd000 {

Drop '0x'

> + compatible = "jedec,ufs-2.0", "snps,dwc-ufshcd";

This should be reversed. Most specific first.

> + reg = < 0xd000 0x1 >;
> + interrupts = < 24 >;
> + };

--
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 v8 2/3] added UFS 2.0 capabilities

[Rob Herring]

On Mon, Feb 15, 2016 at 03:25:12PM +, Joao Pinto wrote:
> Adding UFS 2.0 support to the UFS core driver.
> 
> Signed-off-by: Joao Pinto 
> ---
> Changes v7->v8:
> - Added "jedec, ufs-2.0" to the ufschd-platform compatibility strings
> Changes v0->v7:
> - Nothing changed (just to keep up with patch set version).
> 
>  .../devicetree/bindings/ufs/ufshcd-pltfrm.txt  |  4 +--

Acked-by: Rob Herring 

>  drivers/scsi/ufs/ufshcd.c  | 29 
> +++---
>  drivers/scsi/ufs/ufshci.h  |  1 +
>  3 files changed, 28 insertions(+), 6 deletions(-)
> 
--
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 5/7] ahci: Convert driver to use modern PM hooks

[Tejun Heo]

On Thu, Feb 18, 2016 at 12:45:05PM +0200, Andy Shevchenko wrote:
> > +   .driver.pm  = _pci_pm_ops,
> 
> Please, do this in several assignments, since to support older compilers.

Can you please elaborate?  That's a single assignment.  Are there
compilers which can't do that?

Thanks.

-- 
tejun
--
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 1/2] mpt3sas: Free memory pools before retrying to allocate with different value.

[Johannes Thumshirn]

On Thu, Feb 18, 2016 at 01:54:02PM +0100, Tomas Henzl wrote:
> On 18.2.2016 09:39, Suganath Prabu Subramani wrote:
> > From: Suganath prabu Subramani 
> >
> > Deallocate resources before reallocating of the same in retry_allocation
> > path of _base_allocate_memory_pools()
> >
> > Signed-off-by: Suganath prabu Subramani 
> > 
> > Signed-off-by: Chaitra P B 
> > ---
> >  drivers/scsi/mpt3sas/mpt3sas_base.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
> > b/drivers/scsi/mpt3sas/mpt3sas_base.c
> > index afdb13a..5f2 100644
> > --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> > +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> > @@ -3414,6 +3414,7 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER 
> > *ioc,  int sleep_flag)
> > goto out;
> > retry_sz = 64;
> > ioc->hba_queue_depth -= retry_sz;
> > +   _base_release_memory_pools(ioc);
> > goto retry_allocation;
> > }
> >  
> 
> Reviewed-by: Tomas Henzl 
> 
> --
> 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

Reviewed-by: Johannes Thumshirn 
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
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 1/2] mpt3sas: Free memory pools before retrying to allocate with different value.

[Tomas Henzl]

On 18.2.2016 09:39, Suganath Prabu Subramani wrote:
> From: Suganath prabu Subramani 
>
> Deallocate resources before reallocating of the same in retry_allocation
> path of _base_allocate_memory_pools()
>
> Signed-off-by: Suganath prabu Subramani 
> 
> Signed-off-by: Chaitra P B 
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
> b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index afdb13a..5f2 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -3414,6 +3414,7 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER 
> *ioc,  int sleep_flag)
>   goto out;
>   retry_sz = 64;
>   ioc->hba_queue_depth -= retry_sz;
> + _base_release_memory_pools(ioc);
>   goto retry_allocation;
>   }
>  

Reviewed-by: Tomas Henzl 

--
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 20/34] iscsi-target: update struct iscsit_transport definition

[Varun Prakash]

On Mon, Feb 15, 2016 at 10:39:55PM +0530, Sagi Grimberg wrote:
> 
> > 1. void (*iscsit_rx_pdu)(struct iscsi_conn *);
> > Rx thread uses this for receiving and processing
> > iSCSI PDU in full feature phase.
> 
> Is iscsit_rx_pdu the best name for this? it sounds like
> a function that would handle A pdu, but it's actually the
> thread function dequeuing pdus correct?

iscsit_rx_pdu is for both dequeuing and processing iscsi pdus,
I will rename it to iscsit_get_rx_pdu in the next revision.
--
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 5/6] hisi_sas: add hisi_sas_slave_configure()

[John Garry]

On 18/02/2016 10:30, Hannes Reinecke wrote:

On 02/18/2016 11:12 AM, John Garry wrote:

On 18/02/2016 07:40, Hannes Reinecke wrote:

[ .. ]

Well, the classical thing would be to associate each request tag
with a SAS task; or, in your case, associate each slot index with a
request tag.
You probably would need to reserve some slots for TMFs, ie you'd
need to decrease the resulting ->can_queue variable by that.
But once you've done that you shouldn't hit any QUEUE_FULL issues,
as the block layer will ensure that no tags will be reused while the
command is in flight.
Plus this is something you really need to be doing if you ever
consider moving to scsi-mq ...

Cheers,

Hannes


Hi,

So would you recommend this method under the assumption that the
can_queue value for the host is similar to the queue depth for the
device?


That depends.
Typically the can_queue setting reflects the number of commands the
_host_ can queue internally (due to hardware limitations etc).
They do not necessarily reflect the queue depth for the device
(unless you have a single device, of course).
So if the host has a hardware limit on the number of commands it can
queue, it should set the 'can_queue' variable to the appropriate
number; a host-wide shared tag map is always assumed with recent
kernels.

The queue_depth of an individual device is controlled by the
'cmd_per_lun' setting, and of course capped by can_queue.

But yes, I definitely recommend this method.
Is saves one _so much_ time trying to figure out which command slot
to use. Drawback is that you have to have some sort of fixed order
on them slots to do an efficient lookup.

Cheers,

Hannes



I would like to make a point on cmd_per_lun before considering tagging 
slots: For our host the can_queue is considerably greater than 
cmd_per_lun (even though we initially set the same in the host template, 
which would be incorrect). Regardless I find the host cmd_per_lun is 
effectively ignored for the slave device queue depth as it is reset in 
sas_slave_configure() to 256 [if this function is used and tagging 
enabled]. So if we we choose a reasonable cmd_per_lun for our host, it 
is ignored, right? Or am I missing something?


Thanks,
John


--
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 5/7] ahci: Convert driver to use modern PM hooks

[Andy Shevchenko]

On Thu, Feb 18, 2016 at 10:54 AM, Mika Westerberg
 wrote:
> In order to add support for runtime PM to the ahci driver we first need to
> convert the driver to use modern non-legacy system suspend hooks. There
> should be no functional changes.
>

One comment below, otherwise:
Reviewed-by: Andy Shevchenko 

> Signed-off-by: Mika Westerberg 
> ---
>  drivers/ata/ahci.c | 49 ++---
>  1 file changed, 22 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 546a3692774f..44f9d1c09bbe 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -93,9 +93,9 @@ static void ahci_mcp89_apple_enable(struct pci_dev *pdev);
>  static bool is_mcp89_apple(struct pci_dev *pdev);
>  static int ahci_p5wdh_hardreset(struct ata_link *link, unsigned int *class,
> unsigned long deadline);
> -#ifdef CONFIG_PM
> -static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
> -static int ahci_pci_device_resume(struct pci_dev *pdev);
> +#ifdef CONFIG_PM_SLEEP
> +static int ahci_pci_device_suspend(struct device *dev);
> +static int ahci_pci_device_resume(struct device *dev);
>  #endif
>
>  static struct scsi_host_template ahci_sht = {
> @@ -557,16 +557,16 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { } /* terminate list */
>  };
>
> +static const struct dev_pm_ops ahci_pci_pm_ops = {
> +   SET_SYSTEM_SLEEP_PM_OPS(ahci_pci_device_suspend, 
> ahci_pci_device_resume)
> +};
>
>  static struct pci_driver ahci_pci_driver = {
> .name   = DRV_NAME,
> .id_table   = ahci_pci_tbl,
> .probe  = ahci_init_one,
> .remove = ata_pci_remove_one,
> -#ifdef CONFIG_PM
> -   .suspend= ahci_pci_device_suspend,
> -   .resume = ahci_pci_device_resume,
> -#endif
> +   .driver.pm  = _pci_pm_ops,

Please, do this in several assignments, since to support older compilers.

>  };
>
>  #if defined(CONFIG_PATA_MARVELL) || defined(CONFIG_PATA_MARVELL_MODULE)
> @@ -794,44 +794,39 @@ static int ahci_avn_hardreset(struct ata_link *link, 
> unsigned int *class,
>  }
>
>
> -#ifdef CONFIG_PM
> -static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
> +#ifdef CONFIG_PM_SLEEP
> +static int ahci_pci_device_suspend(struct device *dev)
>  {
> +   struct pci_dev *pdev = to_pci_dev(dev);
> struct ata_host *host = pci_get_drvdata(pdev);
> struct ahci_host_priv *hpriv = host->private_data;
> void __iomem *mmio = hpriv->mmio;
> u32 ctl;
>
> -   if (mesg.event & PM_EVENT_SUSPEND &&
> -   hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
> +   if (hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
> dev_err(>dev,
> "BIOS update required for suspend/resume\n");
> return -EIO;
> }
>
> -   if (mesg.event & PM_EVENT_SLEEP) {
> -   /* AHCI spec rev1.1 section 8.3.3:
> -* Software must disable interrupts prior to requesting a
> -* transition of the HBA to D3 state.
> -*/
> -   ctl = readl(mmio + HOST_CTL);
> -   ctl &= ~HOST_IRQ_EN;
> -   writel(ctl, mmio + HOST_CTL);
> -   readl(mmio + HOST_CTL); /* flush */
> -   }
> +   /* AHCI spec rev1.1 section 8.3.3:
> +* Software must disable interrupts prior to requesting a
> +* transition of the HBA to D3 state.
> +*/
> +   ctl = readl(mmio + HOST_CTL);
> +   ctl &= ~HOST_IRQ_EN;
> +   writel(ctl, mmio + HOST_CTL);
> +   readl(mmio + HOST_CTL); /* flush */
>
> -   return ata_pci_device_suspend(pdev, mesg);
> +   return ata_host_suspend(host, PMSG_SUSPEND);
>  }
>
> -static int ahci_pci_device_resume(struct pci_dev *pdev)
> +static int ahci_pci_device_resume(struct device *dev)
>  {
> +   struct pci_dev *pdev = to_pci_dev(dev);
> struct ata_host *host = pci_get_drvdata(pdev);
> int rc;
>
> -   rc = ata_pci_device_do_resume(pdev);
> -   if (rc)
> -   return rc;
> -
> /* Apple BIOS helpfully mangles the registers on resume */
> if (is_mcp89_apple(pdev))
> ahci_mcp89_apple_enable(pdev);
> --
> 2.7.0
>
> --
> 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



-- 
With Best Regards,
Andy Shevchenko
--
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 5/6] hisi_sas: add hisi_sas_slave_configure()

[Hannes Reinecke]

On 02/18/2016 11:12 AM, John Garry wrote:
> On 18/02/2016 07:40, Hannes Reinecke wrote:
[ .. ]
>> Well, the classical thing would be to associate each request tag
>> with a SAS task; or, in your case, associate each slot index with a
>> request tag.
>> You probably would need to reserve some slots for TMFs, ie you'd
>> need to decrease the resulting ->can_queue variable by that.
>> But once you've done that you shouldn't hit any QUEUE_FULL issues,
>> as the block layer will ensure that no tags will be reused while the
>> command is in flight.
>> Plus this is something you really need to be doing if you ever
>> consider moving to scsi-mq ...
>>
>> Cheers,
>>
>> Hannes
>>
> Hi,
> 
> So would you recommend this method under the assumption that the
> can_queue value for the host is similar to the queue depth for the
> device?
> 
That depends.
Typically the can_queue setting reflects the number of commands the
_host_ can queue internally (due to hardware limitations etc).
They do not necessarily reflect the queue depth for the device
(unless you have a single device, of course).
So if the host has a hardware limit on the number of commands it can
queue, it should set the 'can_queue' variable to the appropriate
number; a host-wide shared tag map is always assumed with recent
kernels.

The queue_depth of an individual device is controlled by the
'cmd_per_lun' setting, and of course capped by can_queue.

But yes, I definitely recommend this method.
Is saves one _so much_ time trying to figure out which command slot
to use. Drawback is that you have to have some sort of fixed order
on them slots to do an efficient lookup.

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 5/6] hisi_sas: add hisi_sas_slave_configure()

[John Garry]

On 18/02/2016 07:40, Hannes Reinecke wrote:

On 02/16/2016 05:56 PM, John Garry wrote:

On 16/02/2016 15:33, Hannes Reinecke wrote:

On 02/16/2016 01:22 PM, John Garry wrote:

In high-datarate aging tests, it is found that
the SCSI framework can periodically
issue lu resets to the device. This is because scsi
commands begin to timeout. It is found that TASK SET
FULL may be returned many times for the same command,
causing the timeouts.
To overcome this, the queue depth for the device needs
to be reduced to 64 (from 256, set in
sas_slave_configure()).


Hmm. TASK SET FULL should cause the queue depth to be reduced
automatically, no?

Cheers,

Hannes



I need to double-check if Task set full reduces the depth, I don't
think it does.

Regardless I found we were getting a combination of commands being
retried due to Task Set Full and also SAS_QUEUE_FULL errors. For
sure the SAS_QUEUE_FULL task errors reduce the queue depth in
scsi_track_queue_full(). However I found it to be very slow in
tracking, and we were getting commands timing out before the queue
depth fell enough.

It would be nice to change default queue depth in
sas_slave_configure() to a lower value so we can avoid this patch,
but I am not sure if other vendor's HBA performance would be
affected. From looking at the history of sas_slave_configure(), it
would seem the value of 256 was inherited from mpt2sas driver, so
I'm not sure.


Well, the classical thing would be to associate each request tag
with a SAS task; or, in your case, associate each slot index with a
request tag.
You probably would need to reserve some slots for TMFs, ie you'd
need to decrease the resulting ->can_queue variable by that.
But once you've done that you shouldn't hit any QUEUE_FULL issues,
as the block layer will ensure that no tags will be reused while the
command is in flight.
Plus this is something you really need to be doing if you ever
consider moving to scsi-mq ...

Cheers,

Hannes


Hi,

So would you recommend this method under the assumption that the 
can_queue value for the host is similar to the queue depth for the device?


Regards,
John


--
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 3/6] hisi_sas: use slot abort in v1 hw

[John Garry]

   /* by default, task resp is complete */
-static void slot_err_v1_hw(struct hisi_hba *hisi_hba,
-   struct sas_task *task,
-   struct hisi_sas_slot *slot)
+static void slot_err_v1_hw(struct hisi_hba *hisi_hba, struct
sas_task *task,
+   struct hisi_sas_slot *slot, int *abort_slot)
   {
   struct task_status_struct *ts = >task_status;
   struct hisi_sas_err_record_v1 *err_record =
slot->status_buffer;
@@ -1212,6 +1211,14 @@ static void slot_err_v1_hw(struct hisi_hba
*hisi_hba,
   ts->stat = SAS_NAK_R_ERR;
   break;
   }
+case TRANS_TX_CREDIT_TIMEOUT_ERR:
+case TRANS_TX_CLOSE_NORMAL_ERR:
+{
+/* This will request a retry */
+ts->stat = SAS_QUEUE_FULL;
+++(*abort_slot);
+break;
+}
   default:
   {
   ts->stat = SAM_STAT_CHECK_CONDITION;
@@ -1317,8 +1324,14 @@ static int slot_complete_v1_hw(struct
hisi_hba *hisi_hba,

   if (cmplt_hdr_data & CMPLT_HDR_ERR_RCRD_XFRD_MSK &&
   !(cmplt_hdr_data & CMPLT_HDR_RSPNS_XFRD_MSK)) {
+int abort_slot = 0;

-slot_err_v1_hw(hisi_hba, task, slot);
+slot_err_v1_hw(hisi_hba, task, slot,  _slot);
+if (unlikely(abort_slot)) {
+queue_work(hisi_hba->wq, >abort_slot);
+sts = ts->stat;
+goto out_1;
+}
   goto out;
   }



static int slot_complete_v1_hw(struct hisi_hba *hisi_hba,
struct hisi_sas_slot *slot, int abort)
{
...

 if (cmplt_hdr_data & CMPLT_HDR_ERR_RCRD_XFRD_MSK &&
 !(cmplt_hdr_data & CMPLT_HDR_RSPNS_XFRD_MSK)) {
 int abort_slot = 0;

 slot_err_v1_hw(hisi_hba, task, slot,  _slot);
 if (unlikely(abort_slot)) { /* check if we need to abort the
task */
 queue_work(hisi_hba->wq, >abort_slot);
 sts = ts->stat;
 goto out_1;
 }
 goto out;
 }




Variable abort_slot is really a boolean flag which can be set in
slot_err_v1_hw(). When error TRANS_TX_CREDIT_TIMEOUT_ERR or
TRANS_TX_CLOSE_NORMAL_ERR occurs in the slot, abort_slot is set. In
this case we don't immediately complete the task (goto out and call
hisi_sas_slot_task_free() and task->task_done()), but instead queue
the task to be aborted in the device before completing (call
queue_work() and then goto out_1).

So why not make slot_err_vi_hw() a boolean and have abort_slot as
the return value?



I am not happy that this function should return anything, more 
specifically only whether the task should be aborted. I would be 
concerned that if it did return this value then it may have to be 
changed later on if the code needs to be changed. However it would make 
the code a bit tighter now.


Alternatively I could pass a pointer to a boolean (sounds bad), or even 
inline slot_err_v1_hw() in slot_complete_v1_hw(), as this is the only 
place it is called from.


Cheers,
John



--
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 2/6] hisi_sas: add hisi_sas_slot_abort()

[John Garry]

On 16/02/2016 15:41, John Garry wrote:

On 16/02/2016 15:22, Hannes Reinecke wrote:

On 02/16/2016 01:22 PM, John Garry wrote:

Add a function to abort a slot (task) in the device
(if it is in the task set) and then cleanup and
complete the task.
The function is called from work queue context as
it cannot be called from the context where it is
triggered (interrupt).

Signed-off-by: John Garry 
---
  drivers/scsi/hisi_sas/hisi_sas.h  |  1 +
  drivers/scsi/hisi_sas/hisi_sas_main.c | 43
+++
  2 files changed, 44 insertions(+)

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h
b/drivers/scsi/hisi_sas/hisi_sas.h
index 02da7e4..a05ce71 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -120,6 +120,7 @@ struct hisi_sas_slot {
  dma_addr_t command_table_dma;
  struct hisi_sas_sge_page *sge_page;
  dma_addr_t sge_page_dma;
+struct work_struct abort_slot;
  };

  struct hisi_sas_tmf_task {
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
b/drivers/scsi/hisi_sas/hisi_sas_main.c
index c600f5e..65509eb 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -15,6 +15,9 @@
  #define DEV_IS_GONE(dev) \
  ((!dev) || (dev->dev_type == SAS_PHY_UNUSED))

+static int hisi_sas_debug_issue_ssp_tmf(struct domain_device *device,
+u8 *lun, struct hisi_sas_tmf_task *tmf);
+
  static struct hisi_hba *dev_to_hisi_hba(struct domain_device *device)
  {
  return device->port->ha->lldd_ha;
@@ -113,6 +116,45 @@ static int hisi_sas_task_prep_ata(struct
hisi_hba *hisi_hba,
  return hisi_hba->hw->prep_stp(hisi_hba, slot);
  }

+/*
+ * This function will issue an abort TMF if a task is still in
+ * the target. Then it will do the task complete cleanup and
+ * callbacks.
+ */
+static void hisi_sas_slot_abort(struct work_struct *work)
+{
+struct hisi_sas_slot *abort_slot =
+container_of(work, struct hisi_sas_slot, abort_slot);
+struct sas_task *task = abort_slot->task;
+struct hisi_hba *hisi_hba = dev_to_hisi_hba(task->dev);
+struct scsi_cmnd *cmnd = task->uldd_task;
+struct hisi_sas_tmf_task tmf_task;
+struct domain_device *device = task->dev;
+struct hisi_sas_device *sas_dev = device->lldd_dev;
+struct scsi_lun lun;
+int tag = abort_slot->idx, rc;
+
+int_to_scsilun(cmnd->device->lun, );
+tmf_task.tmf = TMF_QUERY_TASK;
+tmf_task.tag_of_task_to_be_managed = cpu_to_le16(tag);
+
+rc = hisi_sas_debug_issue_ssp_tmf(task->dev, lun.scsi_lun,
_task);
+
+/* TMF_RESP_FUNC_SUCC means task is present in the task set */
+if (rc != TMF_RESP_FUNC_SUCC)
+goto out;
+tmf_task.tmf = TMF_ABORT_TASK;
+tmf_task.tag_of_task_to_be_managed = cpu_to_le16(tag);
+
+rc = hisi_sas_debug_issue_ssp_tmf(task->dev, lun.scsi_lun,
_task);
+out:
+hisi_sas_slot_task_free(hisi_hba, task, abort_slot);
+if (task->task_done)
+task->task_done(task);
+if (sas_dev && sas_dev->running_req)
+sas_dev->running_req--;
+}
+
  static int hisi_sas_task_prep(struct sas_task *task, struct
hisi_hba *hisi_hba,
int is_tmf, struct hisi_sas_tmf_task *tmf,
int *pass)

Do you really need to query the task first?
As per SAM a successful return from an ABORT TASK TMF has this meaning:

A response of FUNCTION COMPLETE shall indicate that the task was
aborted or was not in the task set.

Ie it doesn't matter if the task was present or not.

Cheers,

Hannes



I think that it should be ok to the abort without first querying. I'll
just test this to double-check. (This would make the first patch in the
series superflous)

Cheers,
John


___
linuxarm mailing list
linux...@huawei.com
http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm


As suggested, it is ok to remove the query tmf.

Thanks,
John

--
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] fusion-mptbase: handle failed allocation for workqueue

[Johannes Thumshirn]

On Wed, Feb 17, 2016 at 11:40:59PM -0500, Insu Yun wrote:
> the failure of ioc->reset_work_q is checked,
> but not ioc->fw_event_q.
> 
> Signed-off-by: Insu Yun 
> ---
>  drivers/message/fusion/mptbase.c | 44 
> 
>  1 file changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/message/fusion/mptbase.c 
> b/drivers/message/fusion/mptbase.c
> index 5dcc031..53a5015 100644
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c
> @@ -1871,9 +1871,8 @@ mpt_attach(struct pci_dev *pdev, const struct 
> pci_device_id *id)
>   if (!ioc->reset_work_q) {
>   printk(MYIOC_s_ERR_FMT "Insufficient memory to add adapter!\n",
>   ioc->name);
> - pci_release_selected_regions(pdev, ioc->bars);
> - kfree(ioc);
> - return -ENOMEM;
> + r = -ENOMEM;
> + goto err3;
>   }
>  
>   dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "facts @ %p, pfacts[0] @ %p\n",
> @@ -1996,24 +1995,16 @@ mpt_attach(struct pci_dev *pdev, const struct 
> pci_device_id *id)
>   snprintf(ioc->fw_event_q_name, MPT_KOBJ_NAME_LEN, "mpt/%d", ioc->id);
>   ioc->fw_event_q = create_singlethread_workqueue(ioc->fw_event_q_name);
>  
> + if (!ioc->fw_event_q) {
> + r = -ENOMEM;
> + goto err2;
> + }
> +
>   if ((r = mpt_do_ioc_recovery(ioc, MPT_HOSTEVENT_IOC_BRINGUP,
>   CAN_SLEEP)) != 0){
>   printk(MYIOC_s_ERR_FMT "didn't initialize properly! (%d)\n",
>   ioc->name, r);
> -
> - list_del(>list);
> - if (ioc->alt_ioc)
> - ioc->alt_ioc->alt_ioc = NULL;
> - iounmap(ioc->memmap);
> - if (r != -5)
> - pci_release_selected_regions(pdev, ioc->bars);
> -
> - destroy_workqueue(ioc->reset_work_q);
> - ioc->reset_work_q = NULL;
> -
> - kfree(ioc);
> - pci_set_drvdata(pdev, NULL);
> - return r;
> + goto err1;
>   }
>  
>   /* call per device driver probe entry point */
> @@ -2040,6 +2031,25 @@ mpt_attach(struct pci_dev *pdev, const struct 
> pci_device_id *id)
>   msecs_to_jiffies(MPT_POLLING_INTERVAL));
>  
>   return 0;
> +
> +err1:
> + destroy_workqueue(ioc->fw_event_q);
> + ioc->fw_event_q = NULL;
> +err2::
> + destroy_workqueue(ioc->reset_work_q);
> + ioc->reset_work_q = NULL;
> +
> + list_del(>list);
> + if (ioc->alt_ioc)
> + ioc->alt_ioc->alt_ioc = NULL;
> + iounmap(ioc->memmap);
> + pci_set_drvdata(pdev, NULL);
> +err3:
> + if (r != -5)
> + pci_release_selected_regions(pdev, ioc->bars);
> + kfree(ioc);
> + return r;
> +
>  }

Please no. Not err1, err2 and err3.

err1 could be "goto destroy_fw_event_q", err2 "destroy_reset_workq", err3
goto "free_ioc".

>  
>  
> /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
> -- 
> 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

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
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 0/7] Runtime PM support for AHCI host controller driver

[Mika Westerberg]

Hi,

Linux already supports runtime PM of disks (drivers/scsi/sd.c) so that
after certain amount of idle time the disk is suspended automatically. This
series extends the support to AHCI host controllers. Whenever SATA ports
are determined to be idle (all children are runtime suspended) the host
controller is also suspended.

On recent Intel CPUs like Broxton this allows the CPU to go low power idle
states like S0ix runtime (given that all necessary blocks are also in their
correesponding low power states).

Patches [1-2/7] fix a lockup where disk is runtime suspended and the system
is put to sleep. They are independent of the rest of the series.

Patch [3/7] makes it possible for SATA ports to be runtime suspended when
there is not disk connected. For example on Lenovo Yoga 900 there are two
SATA ports which only one of them has disk connected. This patch allows the
host controller to runtime suspend whenever the disk is idle.

Rest of the patches bring runtime PM support for the AHCI driver. By
default runtime PM is blocked and needs to be unblocked from userspace
(following what other PCI drivers do). I've used following script to
unblock runtime PM for the whole stack (with 15 seconds of idle time):

--8<--8<--8<--8<--8<--8<--8<--
#!/bin/sh

TIMEOUT=${1:-15}
HOST=$(lspci -D | grep "SATA controller" | cut -f 1 -d ' ')
DISK=sda

# Enable runtime PM for all SATA ports
for port in /sys/bus/pci/devices/$HOST/ata*; do
echo auto > $port/power/control
done
# Then for the host controller
echo auto > /sys/bus/pci/devices/$HOST/power/control

# And last for the disk
echo auto > /sys/block/$DISK/device/power/control
echo $(($TIMEOUT * 1000)) > /sys/block/$DISK/device/power/autosuspend_delay_ms
--8<--8<--8<--8<--8<--8<--8<--

Mika Westerberg (7):
  block: Add blk_set_runtime_active()
  scsi: Set request queue runtime PM status back to active on resume
  scsi: Drop runtime PM usage count after host is added
  ahci: Cache host controller version
  ahci: Convert driver to use modern PM hooks
  ahci: Add functions to manage runtime PM of AHCI ports
  ahci: Add runtime PM support for the host controller

 block/blk-core.c   |  24 
 drivers/ata/ahci.c | 102 +++--
 drivers/ata/ahci.h |   1 +
 drivers/ata/libahci.c  |  55 +++---
 drivers/scsi/hosts.c   |   7 
 drivers/scsi/scsi_pm.c |  10 +
 include/linux/blkdev.h |   2 +
 7 files changed, 167 insertions(+), 34 deletions(-)

-- 
2.7.0

--
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 3/7] scsi: Drop runtime PM usage count after host is added

[Mika Westerberg]

Runtime PM of the SCSI host is already handled by calls to
scsi_autopm_get_host() and scsi_autopm_put_host() from appropriate places
whenever the host needs to be powered on. This works fine when there is
device connected to the host as once it runtime suspends the host will too.

However, if there is no device connected the host is never runtime
suspended (the usage counter is always 0).

Allow runtime suspend of host even if it has no devices connected by
calling scsi_autopm_put_host() at the end of scsi_add_host_with_dma(). We
temporarily increase runtime PM usage counter first so call to
scsi_autopm_put_host() will result idle request to be scheduled for the
device.

Signed-off-by: Mika Westerberg 
---
 drivers/scsi/hosts.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 82ac1cd818ac..e46bf4d152a0 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -250,6 +250,12 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct 
device *dev,
if (error)
goto out_destroy_freelist;
 
+   /*
+* Increase usage count temporarily here so that calling
+* scsi_autopm_put_host() will trigger runtime idle if there is
+* nothing else preventing suspending the device.
+*/
+   pm_runtime_get_noresume(>shost_gendev);
pm_runtime_set_active(>shost_gendev);
pm_runtime_enable(>shost_gendev);
device_enable_async_suspend(>shost_gendev);
@@ -290,6 +296,7 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct 
device *dev,
goto out_destroy_host;
 
scsi_proc_host_add(shost);
+   scsi_autopm_put_host(shost);
return error;
 
  out_destroy_host:
-- 
2.7.0

--
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 1/7] block: Add blk_set_runtime_active()

[Mika Westerberg]

If block device is left runtime suspended during system suspend, resume
hook of the driver typically corrects runtime PM status of the device back
to "active" after it is resumed. However, this is not enough as queue's
runtime PM status is still "suspended". As long as it is in this state
blk_pm_peek_request() returns NULL and thus prevents new requests to be
processed.

Add new function blk_set_runtime_active() that can be used to force the
queue status back to "active" as needed.

Signed-off-by: Mika Westerberg 
---
 block/blk-core.c   | 24 
 include/linux/blkdev.h |  2 ++
 2 files changed, 26 insertions(+)

diff --git a/block/blk-core.c b/block/blk-core.c
index b83d29755b5a..d0c6f4c92cb6 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -3529,6 +3529,30 @@ void blk_post_runtime_resume(struct request_queue *q, 
int err)
spin_unlock_irq(q->queue_lock);
 }
 EXPORT_SYMBOL(blk_post_runtime_resume);
+
+/**
+ * blk_set_runtime_active - Force runtime status of the queue to be active
+ * @q: the queue of the device
+ *
+ * If the device is left runtime suspended during system suspend the resume
+ * hook typically resumes the device and corrects runtime status
+ * accordingly. However, that does not affect the queue runtime PM status
+ * which is still "suspended". This prevents processing requests from the
+ * queue.
+ *
+ * This function can be used in driver's resume hook to correct queue
+ * runtime PM status and re-enable peeking requests from the queue. It
+ * should be called before first request is added to the queue.
+ */
+void blk_set_runtime_active(struct request_queue *q)
+{
+   spin_lock_irq(q->queue_lock);
+   q->rpm_status = RPM_ACTIVE;
+   pm_runtime_mark_last_busy(q->dev);
+   pm_request_autosuspend(q->dev);
+   spin_unlock_irq(q->queue_lock);
+}
+EXPORT_SYMBOL(blk_set_runtime_active);
 #endif
 
 int __init blk_dev_init(void)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 4571ef1a12a9..40c0241a6f28 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1029,6 +1029,7 @@ extern int blk_pre_runtime_suspend(struct request_queue 
*q);
 extern void blk_post_runtime_suspend(struct request_queue *q, int err);
 extern void blk_pre_runtime_resume(struct request_queue *q);
 extern void blk_post_runtime_resume(struct request_queue *q, int err);
+extern void blk_set_runtime_active(struct request_queue *q);
 #else
 static inline void blk_pm_runtime_init(struct request_queue *q,
struct device *dev) {}
@@ -1039,6 +1040,7 @@ static inline int blk_pre_runtime_suspend(struct 
request_queue *q)
 static inline void blk_post_runtime_suspend(struct request_queue *q, int err) 
{}
 static inline void blk_pre_runtime_resume(struct request_queue *q) {}
 static inline void blk_post_runtime_resume(struct request_queue *q, int err) {}
+extern inline void blk_set_runtime_active(struct request_queue *q) {}
 #endif
 
 /*
-- 
2.7.0

--
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 7/7] ahci: Add runtime PM support for the host controller

[Mika Westerberg]

This patch adds runtime PM support for the AHCI host controller driver so
that the host controller is powered down when all SATA ports are runtime
suspended. Powering down the AHCI host controller can reduce power
consumption and possibly allow the CPU to enter lower power idle states
(S0ix) during runtime.

Runtime PM is blocked by default and needs to be unblocked from userspace
as needed (via power/* sysfs nodes).

Signed-off-by: Mika Westerberg 
---
 drivers/ata/ahci.c | 73 +-
 1 file changed, 61 insertions(+), 12 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 44f9d1c09bbe..fb0c23ae3ce9 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -85,6 +85,7 @@ enum board_ids {
 };
 
 static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id 
*ent);
+static void ahci_remove_one(struct pci_dev *dev);
 static int ahci_vt8251_hardreset(struct ata_link *link, unsigned int *class,
 unsigned long deadline);
 static int ahci_avn_hardreset(struct ata_link *link, unsigned int *class,
@@ -93,10 +94,14 @@ static void ahci_mcp89_apple_enable(struct pci_dev *pdev);
 static bool is_mcp89_apple(struct pci_dev *pdev);
 static int ahci_p5wdh_hardreset(struct ata_link *link, unsigned int *class,
unsigned long deadline);
+#ifdef CONFIG_PM
+static int ahci_pci_device_runtime_suspend(struct device *dev);
+static int ahci_pci_device_runtime_resume(struct device *dev);
 #ifdef CONFIG_PM_SLEEP
 static int ahci_pci_device_suspend(struct device *dev);
 static int ahci_pci_device_resume(struct device *dev);
 #endif
+#endif /* CONFIG_PM */
 
 static struct scsi_host_template ahci_sht = {
AHCI_SHT("ahci"),
@@ -559,13 +564,15 @@ static const struct pci_device_id ahci_pci_tbl[] = {
 
 static const struct dev_pm_ops ahci_pci_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(ahci_pci_device_suspend, ahci_pci_device_resume)
+   SET_RUNTIME_PM_OPS(ahci_pci_device_runtime_suspend,
+  ahci_pci_device_runtime_resume, NULL)
 };
 
 static struct pci_driver ahci_pci_driver = {
.name   = DRV_NAME,
.id_table   = ahci_pci_tbl,
.probe  = ahci_init_one,
-   .remove = ata_pci_remove_one,
+   .remove = ahci_remove_one,
.driver.pm  = _pci_pm_ops,
 };
 
@@ -794,21 +801,13 @@ static int ahci_avn_hardreset(struct ata_link *link, 
unsigned int *class,
 }
 
 
-#ifdef CONFIG_PM_SLEEP
-static int ahci_pci_device_suspend(struct device *dev)
+#ifdef CONFIG_PM
+static void ahci_pci_disable_interrupts(struct ata_host *host)
 {
-   struct pci_dev *pdev = to_pci_dev(dev);
-   struct ata_host *host = pci_get_drvdata(pdev);
struct ahci_host_priv *hpriv = host->private_data;
void __iomem *mmio = hpriv->mmio;
u32 ctl;
 
-   if (hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
-   dev_err(>dev,
-   "BIOS update required for suspend/resume\n");
-   return -EIO;
-   }
-
/* AHCI spec rev1.1 section 8.3.3:
 * Software must disable interrupts prior to requesting a
 * transition of the HBA to D3 state.
@@ -817,7 +816,44 @@ static int ahci_pci_device_suspend(struct device *dev)
ctl &= ~HOST_IRQ_EN;
writel(ctl, mmio + HOST_CTL);
readl(mmio + HOST_CTL); /* flush */
+}
+
+static int ahci_pci_device_runtime_suspend(struct device *dev)
+{
+   struct pci_dev *pdev = to_pci_dev(dev);
+   struct ata_host *host = pci_get_drvdata(pdev);
 
+   ahci_pci_disable_interrupts(host);
+   return 0;
+}
+
+static int ahci_pci_device_runtime_resume(struct device *dev)
+{
+   struct pci_dev *pdev = to_pci_dev(dev);
+   struct ata_host *host = pci_get_drvdata(pdev);
+   int rc;
+
+   rc = ahci_pci_reset_controller(host);
+   if (rc)
+   return rc;
+   ahci_pci_init_controller(host);
+   return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int ahci_pci_device_suspend(struct device *dev)
+{
+   struct pci_dev *pdev = to_pci_dev(dev);
+   struct ata_host *host = pci_get_drvdata(pdev);
+   struct ahci_host_priv *hpriv = host->private_data;
+
+   if (hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
+   dev_err(>dev,
+   "BIOS update required for suspend/resume\n");
+   return -EIO;
+   }
+
+   ahci_pci_disable_interrupts(host);
return ata_host_suspend(host, PMSG_SUSPEND);
 }
 
@@ -845,6 +881,8 @@ static int ahci_pci_device_resume(struct device *dev)
 }
 #endif
 
+#endif /* CONFIG_PM */
+
 static int ahci_configure_dma_masks(struct pci_dev *pdev, int using_dac)
 {
int rc;
@@ -1664,7 +1702,18 @@ static int ahci_init_one(struct pci_dev *pdev, const 
struct pci_device_id *ent)
 
pci_set_master(pdev);
 
-   

[PATCH 5/7] ahci: Convert driver to use modern PM hooks

[Mika Westerberg]

In order to add support for runtime PM to the ahci driver we first need to
convert the driver to use modern non-legacy system suspend hooks. There
should be no functional changes.

Signed-off-by: Mika Westerberg 
---
 drivers/ata/ahci.c | 49 ++---
 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 546a3692774f..44f9d1c09bbe 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -93,9 +93,9 @@ static void ahci_mcp89_apple_enable(struct pci_dev *pdev);
 static bool is_mcp89_apple(struct pci_dev *pdev);
 static int ahci_p5wdh_hardreset(struct ata_link *link, unsigned int *class,
unsigned long deadline);
-#ifdef CONFIG_PM
-static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
-static int ahci_pci_device_resume(struct pci_dev *pdev);
+#ifdef CONFIG_PM_SLEEP
+static int ahci_pci_device_suspend(struct device *dev);
+static int ahci_pci_device_resume(struct device *dev);
 #endif
 
 static struct scsi_host_template ahci_sht = {
@@ -557,16 +557,16 @@ static const struct pci_device_id ahci_pci_tbl[] = {
{ } /* terminate list */
 };
 
+static const struct dev_pm_ops ahci_pci_pm_ops = {
+   SET_SYSTEM_SLEEP_PM_OPS(ahci_pci_device_suspend, ahci_pci_device_resume)
+};
 
 static struct pci_driver ahci_pci_driver = {
.name   = DRV_NAME,
.id_table   = ahci_pci_tbl,
.probe  = ahci_init_one,
.remove = ata_pci_remove_one,
-#ifdef CONFIG_PM
-   .suspend= ahci_pci_device_suspend,
-   .resume = ahci_pci_device_resume,
-#endif
+   .driver.pm  = _pci_pm_ops,
 };
 
 #if defined(CONFIG_PATA_MARVELL) || defined(CONFIG_PATA_MARVELL_MODULE)
@@ -794,44 +794,39 @@ static int ahci_avn_hardreset(struct ata_link *link, 
unsigned int *class,
 }
 
 
-#ifdef CONFIG_PM
-static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
+#ifdef CONFIG_PM_SLEEP
+static int ahci_pci_device_suspend(struct device *dev)
 {
+   struct pci_dev *pdev = to_pci_dev(dev);
struct ata_host *host = pci_get_drvdata(pdev);
struct ahci_host_priv *hpriv = host->private_data;
void __iomem *mmio = hpriv->mmio;
u32 ctl;
 
-   if (mesg.event & PM_EVENT_SUSPEND &&
-   hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
+   if (hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
dev_err(>dev,
"BIOS update required for suspend/resume\n");
return -EIO;
}
 
-   if (mesg.event & PM_EVENT_SLEEP) {
-   /* AHCI spec rev1.1 section 8.3.3:
-* Software must disable interrupts prior to requesting a
-* transition of the HBA to D3 state.
-*/
-   ctl = readl(mmio + HOST_CTL);
-   ctl &= ~HOST_IRQ_EN;
-   writel(ctl, mmio + HOST_CTL);
-   readl(mmio + HOST_CTL); /* flush */
-   }
+   /* AHCI spec rev1.1 section 8.3.3:
+* Software must disable interrupts prior to requesting a
+* transition of the HBA to D3 state.
+*/
+   ctl = readl(mmio + HOST_CTL);
+   ctl &= ~HOST_IRQ_EN;
+   writel(ctl, mmio + HOST_CTL);
+   readl(mmio + HOST_CTL); /* flush */
 
-   return ata_pci_device_suspend(pdev, mesg);
+   return ata_host_suspend(host, PMSG_SUSPEND);
 }
 
-static int ahci_pci_device_resume(struct pci_dev *pdev)
+static int ahci_pci_device_resume(struct device *dev)
 {
+   struct pci_dev *pdev = to_pci_dev(dev);
struct ata_host *host = pci_get_drvdata(pdev);
int rc;
 
-   rc = ata_pci_device_do_resume(pdev);
-   if (rc)
-   return rc;
-
/* Apple BIOS helpfully mangles the registers on resume */
if (is_mcp89_apple(pdev))
ahci_mcp89_apple_enable(pdev);
-- 
2.7.0

--
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 6/7] ahci: Add functions to manage runtime PM of AHCI ports

[Mika Westerberg]

Add new functions ahci_rpm_get_port()/ahci_rpm_put_port() that change
runtime PM status of AHCI ports. Depending if the AHCI host has runtime PM
enabled or disabled calling these may trigger runtime suspend/resume of the
host controller.

We also call these functions in appropriate places to make sure host
controller registers are available before using them.

Signed-off-by: Mika Westerberg 
---
 drivers/ata/libahci.c | 48 +++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 2ce4c638714e..a5dc80810a5e 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -224,6 +224,31 @@ static void ahci_enable_ahci(void __iomem *mmio)
WARN_ON(1);
 }
 
+/**
+ * ahci_rpm_get_port - Make sure the port is powered on
+ * @ap: Port to power on
+ *
+ * Whenever there is need to access the AHCI host registers outside of
+ * normal execution paths, call this function to make sure the host is
+ * actually powered on.
+ */
+static int ahci_rpm_get_port(struct ata_port *ap)
+{
+   return pm_runtime_get_sync(ap->dev);
+}
+
+/**
+ * ahci_rpm_put_port - Undoes ahci_rpm_get_port()
+ * @ap: Port to power down
+ *
+ * Undoes ahci_rpm_get_port() and possibly powers down the AHCI host
+ * if it has no more active users.
+ */
+static void ahci_rpm_put_port(struct ata_port *ap)
+{
+   pm_runtime_put(ap->dev);
+}
+
 static ssize_t ahci_show_host_caps(struct device *dev,
   struct device_attribute *attr, char *buf)
 {
@@ -260,8 +285,13 @@ static ssize_t ahci_show_port_cmd(struct device *dev,
struct Scsi_Host *shost = class_to_shost(dev);
struct ata_port *ap = ata_shost_to_port(shost);
void __iomem *port_mmio = ahci_port_base(ap);
+   ssize_t ret;
 
-   return sprintf(buf, "%x\n", readl(port_mmio + PORT_CMD));
+   ahci_rpm_get_port(ap);
+   ret = sprintf(buf, "%x\n", readl(port_mmio + PORT_CMD));
+   ahci_rpm_put_port(ap);
+
+   return ret;
 }
 
 static ssize_t ahci_read_em_buffer(struct device *dev,
@@ -277,17 +307,20 @@ static ssize_t ahci_read_em_buffer(struct device *dev,
size_t count;
int i;
 
+   ahci_rpm_get_port(ap);
spin_lock_irqsave(ap->lock, flags);
 
em_ctl = readl(mmio + HOST_EM_CTL);
if (!(ap->flags & ATA_FLAG_EM) || em_ctl & EM_CTL_XMT ||
!(hpriv->em_msg_type & EM_MSG_TYPE_SGPIO)) {
spin_unlock_irqrestore(ap->lock, flags);
+   ahci_rpm_put_port(ap);
return -EINVAL;
}
 
if (!(em_ctl & EM_CTL_MR)) {
spin_unlock_irqrestore(ap->lock, flags);
+   ahci_rpm_put_port(ap);
return -EAGAIN;
}
 
@@ -315,6 +348,7 @@ static ssize_t ahci_read_em_buffer(struct device *dev,
}
 
spin_unlock_irqrestore(ap->lock, flags);
+   ahci_rpm_put_port(ap);
 
return i;
 }
@@ -339,11 +373,13 @@ static ssize_t ahci_store_em_buffer(struct device *dev,
size % 4 || size > hpriv->em_buf_sz)
return -EINVAL;
 
+   ahci_rpm_get_port(ap);
spin_lock_irqsave(ap->lock, flags);
 
em_ctl = readl(mmio + HOST_EM_CTL);
if (em_ctl & EM_CTL_TM) {
spin_unlock_irqrestore(ap->lock, flags);
+   ahci_rpm_put_port(ap);
return -EBUSY;
}
 
@@ -356,6 +392,7 @@ static ssize_t ahci_store_em_buffer(struct device *dev,
writel(em_ctl | EM_CTL_TM, mmio + HOST_EM_CTL);
 
spin_unlock_irqrestore(ap->lock, flags);
+   ahci_rpm_put_port(ap);
 
return size;
 }
@@ -369,7 +406,9 @@ static ssize_t ahci_show_em_supported(struct device *dev,
void __iomem *mmio = hpriv->mmio;
u32 em_ctl;
 
+   ahci_rpm_get_port(ap);
em_ctl = readl(mmio + HOST_EM_CTL);
+   ahci_rpm_put_port(ap);
 
return sprintf(buf, "%s%s%s%s\n",
   em_ctl & EM_CTL_LED ? "led " : "",
@@ -1010,6 +1049,7 @@ static ssize_t ahci_transmit_led_message(struct ata_port 
*ap, u32 state,
else
return -EINVAL;
 
+   ahci_rpm_get_port(ap);
spin_lock_irqsave(ap->lock, flags);
 
/*
@@ -1019,6 +1059,7 @@ static ssize_t ahci_transmit_led_message(struct ata_port 
*ap, u32 state,
em_ctl = readl(mmio + HOST_EM_CTL);
if (em_ctl & EM_CTL_TM) {
spin_unlock_irqrestore(ap->lock, flags);
+   ahci_rpm_put_port(ap);
return -EBUSY;
}
 
@@ -1046,6 +1087,8 @@ static ssize_t ahci_transmit_led_message(struct ata_port 
*ap, u32 state,
emp->led_state = state;
 
spin_unlock_irqrestore(ap->lock, flags);
+   ahci_rpm_put_port(ap);
+
return size;
 }
 
@@ -2248,6 +2291,8 @@ static void ahci_pmp_detach(struct ata_port *ap)
 
 int ahci_port_resume(struct ata_port *ap)
 {
+

[PATCH 4/7] ahci: Cache host controller version

[Mika Westerberg]

This allows sysfs nodes to read the cached value directly instead of
powering up possibly runtime suspended controller.

Signed-off-by: Mika Westerberg 
---
 drivers/ata/ahci.h| 1 +
 drivers/ata/libahci.c | 7 +++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index a44c75d4c284..8138bc967eda 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -336,6 +336,7 @@ struct ahci_host_priv {
void __iomem *  mmio;   /* bus-independent mem map */
u32 cap;/* cap to use */
u32 cap2;   /* cap2 to use */
+   u32 version;/* cached version */
u32 port_map;   /* port map to use */
u32 saved_cap;  /* saved initial cap */
u32 saved_cap2; /* saved initial cap2 */
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 402967902cbe..2ce4c638714e 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -250,9 +250,8 @@ static ssize_t ahci_show_host_version(struct device *dev,
struct Scsi_Host *shost = class_to_shost(dev);
struct ata_port *ap = ata_shost_to_port(shost);
struct ahci_host_priv *hpriv = ap->host->private_data;
-   void __iomem *mmio = hpriv->mmio;
 
-   return sprintf(buf, "%x\n", readl(mmio + HOST_VERSION));
+   return sprintf(buf, "%x\n", hpriv->version);
 }
 
 static ssize_t ahci_show_port_cmd(struct device *dev,
@@ -508,6 +507,7 @@ void ahci_save_initial_config(struct device *dev, struct 
ahci_host_priv *hpriv)
/* record values to use during operation */
hpriv->cap = cap;
hpriv->cap2 = cap2;
+   hpriv->version = readl(mmio + HOST_VERSION);
hpriv->port_map = port_map;
 
if (!hpriv->start_engine)
@@ -2389,11 +2389,10 @@ static void ahci_port_stop(struct ata_port *ap)
 void ahci_print_info(struct ata_host *host, const char *scc_s)
 {
struct ahci_host_priv *hpriv = host->private_data;
-   void __iomem *mmio = hpriv->mmio;
u32 vers, cap, cap2, impl, speed;
const char *speed_s;
 
-   vers = readl(mmio + HOST_VERSION);
+   vers = hpriv->version;
cap = hpriv->cap;
cap2 = hpriv->cap2;
impl = hpriv->port_map;
-- 
2.7.0

--
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 2/7] scsi: Set request queue runtime PM status back to active on resume

[Mika Westerberg]

We treat system suspend of SCSI devices pretty much the same as runtime
suspend. If the device is already runtime suspended we leave it to that
state during system suspend. On resume from system sleep we then resume the
device and correct the runtime PM status back to "active".

There is a problem with this because runtime PM status of the request queue
in question is not changed (it will be in "suspended" state). When SCSI
disk driver (sd.c) resumes the disk it sends START message to the device
and because the request queue is still in "suspended" state
blk_pm_peek_request() returns NULL preventing resume of the disk.

The issue can be reproduced with following commands:

  # echo auto > /sys/block/sda/device/power/control
  # echo 15000 > /sys/block/sda/device/power/autosuspend_delay_ms
  [   57.191706] sd 0:0:0:0: [sda] Synchronizing SCSI cache
  [   57.380015] sd 0:0:0:0: [sda] Stopping disk

Now suspend the machine:

  # rtcwake -s10 -mmem

This ends up in soft lockup because resume is not proceeding accordingly
and userspace is never restarted. Also there is nothing printed to the
console.

Fix this by forcing request queue status to "active" before the disk is
resumed.

Signed-off-by: Mika Westerberg 
---
 drivers/scsi/scsi_pm.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index 459abe1dcc87..b44c1bb687a2 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -139,6 +139,16 @@ static int scsi_bus_resume_common(struct device *dev,
else
fn = NULL;
 
+   /*
+* Forcibly set runtime PM status of request queue to "active" to
+* make sure we can again get requests from the queue (see also
+* blk_pm_peek_request()).
+*
+* The resume hook will correct runtime PM status of the disk.
+*/
+   if (scsi_is_sdev_device(dev) && pm_runtime_suspended(dev))
+   blk_set_runtime_active(to_scsi_device(dev)->request_queue);
+
if (fn) {
async_schedule_domain(fn, dev, _sd_pm_domain);
 
-- 
2.7.0

--
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 1/2] mpt3sas: Free memory pools before retrying to allocate with different value.

[Suganath Prabu Subramani]

From: Suganath prabu Subramani 

Deallocate resources before reallocating of the same in retry_allocation
path of _base_allocate_memory_pools()

Signed-off-by: Suganath prabu Subramani 
Signed-off-by: Chaitra P B 
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index afdb13a..5f2 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -3414,6 +3414,7 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc,  
int sleep_flag)
goto out;
retry_sz = 64;
ioc->hba_queue_depth -= retry_sz;
+   _base_release_memory_pools(ioc);
goto retry_allocation;
}
 
-- 
1.8.3.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

[PATCH v2 2/2] Updating maintainers list for MPT FUSION DRIVERS.

[Suganath Prabu Subramani]

From: Suganath prabu Subramani 

Updating maintainers list for MPT FUSION DRIVERS,
broadcom support link and email id.

Signed-off-by: Suganath prabu Subramani 
Signed-off-by: Chaitra P B 
---
 MAINTAINERS | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 30aca4a..003eef4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6675,13 +6675,12 @@ S:  Maintained
 F: arch/arm/mach-lpc32xx/
 
 LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI)
-M: Nagalakshmi Nandigama 
-M: Praveen Krishnamoorthy 
-M: Sreekanth Reddy 
-M: Abhijit Mahajan 
-L: mpt-fusionlinux@avagotech.com
+M: Sathya Prakash 
+M: Chaitra P B 
+M: Suganath Prabu Subramani 
+L: mpt-fusionlinux@broadcom.com
 L: linux-scsi@vger.kernel.org
-W: http://www.lsilogic.com/support
+W: http://www.avagotech.com/support/
 S: Supported
 F: drivers/message/fusion/
 F: drivers/scsi/mpt2sas/
-- 
1.8.3.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

[PATCH RESEND] dpt_i2o: fix build warning

[Sudip Mukherjee]

We were getting build warning about:
drivers/scsi/dpt_i2o.c:183:29: warning: 'dptids' defined but not used

dptids[] is only used in the MODULE_DEVICE_TABLE so when MODULE is not
defined then dptids[] becomes unused.

Reviewed-by: Johannes Thumshirn 
Signed-off-by: Sudip Mukherjee 
---

Resending v1 as v2 had a NACK from Alan about the spec not maintained
while converting to pci driver.

 drivers/scsi/dpt_i2o.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index d4cda5e..21c8d21 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -180,11 +180,14 @@ static u8 adpt_read_blink_led(adpt_hba* host)
  *
  */
 
+#ifdef MODULE
 static struct pci_device_id dptids[] = {
{ PCI_DPT_VENDOR_ID, PCI_DPT_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID,},
{ PCI_DPT_VENDOR_ID, PCI_DPT_RAPTOR_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID,},
{ 0, }
 };
+#endif
+
 MODULE_DEVICE_TABLE(pci,dptids);
 
 static int adpt_detect(struct scsi_host_template* sht)
-- 
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: [PATCH v2] [SCSI] dpt_i2o: use proper pci driver

[Johannes Thumshirn]

On Wed, Feb 17, 2016 at 09:20:03PM +0530, Sudip Mukherjee wrote:
> On Wednesday 17 February 2016 07:57 PM, One Thousand Gnomes wrote:
> >On Wed, 17 Feb 2016 17:50:14 +0530
> >Sudip Mukherjee  wrote:
> >
> >>This is a pci device but was not done in the usual way a pci driver is
> >>done. Convert the driver into a proper pci driver.
> >
> >This looks completely wrong. Please read the I2O 1.5 specification
> >document before playing with that code. You can't simply convert it to a
> >standard PCI driver as all the IOPs are supposed to be detected and then
> >the correct initialization sequence executed across the set of IOPs. IOPs
> >are allowed to talk to one another and the system table binds it all
> >together.
> 
> Yes, thanks for letting me know about the spec. It is saying
> "The host locates each IOP, adds it
> to the system configuration table, and initializes the IOP’s outbound
> message queue. The host then
> provides each IOP with a list of all IOPs and the physical location of their
> inbound message queue."
> 
> >
> >If you do hot plug then you need to follow the specification and do
> >all the extra notifications. Unless you've got multiple FC909/FC920
> >fibechannel cards or similar to test with I would just leave this well
> >alone.
> 
> point noted.
> 
> >Your original simple patch is *MUCH* safer in this specific case.
> 
> The original patch is already having NACK from Johannes. So i better leave
> this code here.

Nah, wasn't a hard NACK. I'm OK with the original code, just was reluctant to
the #ifdefs. I agree I didn't know the I2O specifics and thought a "normal"
PCI driver fitting nicely in the driver model would be the way to go.

Please resend your v1 with my Reviewed-by

> 
> regards
> sudip

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
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