Re: [PATCH 1/1] iscsi: kill redundant casts

[Mike Christie]

Switching linux-kernel for linux-scsi mailing list.

On 6/9/14, 2:57 PM, 'Nick Black' via open-iscsi wrote:

Remove two redundant casts from char * to char *.

Signed-off-by: Nick Black n...@google.com
---
  drivers/scsi/scsi_transport_iscsi.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_transport_iscsi.c 
b/drivers/scsi/scsi_transport_iscsi.c
index 0102a2d..38a1de3 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -3059,7 +3059,7 @@ iscsi_get_chap(struct iscsi_transport *transport, struct 
nlmsghdr *nlh)
evchap-u.get_chap.host_no = ev-u.get_chap.host_no;
evchap-u.get_chap.chap_tbl_idx = ev-u.get_chap.chap_tbl_idx;
evchap-u.get_chap.num_entries = ev-u.get_chap.num_entries;
-   buf = (char *) ((char *)evchap + sizeof(*evchap));
+   buf = (char *)evchap + sizeof(*evchap);
memset(buf, 0, chap_buf_size);

err = transport-get_chap(shost, ev-u.get_chap.chap_tbl_idx,
@@ -3463,7 +3463,7 @@ iscsi_get_host_stats(struct iscsi_transport *transport, 
struct nlmsghdr *nlh)
evhost_stats-type = nlh-nlmsg_type;
evhost_stats-u.get_host_stats.host_no =
ev-u.get_host_stats.host_no;
-   buf = (char *)((char *)evhost_stats + sizeof(*evhost_stats));
+   buf = (char *)evhost_stats + sizeof(*evhost_stats);
memset(buf, 0, host_stats_size);

err = transport-get_host_stats(shost, buf, host_stats_size);




Looks ok to me.

Acked-by: Mike Christie micha...@cs.wisc.edu

--
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 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[Nicholas A. Bellinger]

On Mon, 2014-06-09 at 16:30 +0300, Michael S. Tsirkin wrote:
 On Thu, May 22, 2014 at 02:26:16AM +, Nicholas A. Bellinger wrote:
  From: Nicholas Bellinger n...@linux-iscsi.org
  
  Hi MST, MKP, Paolo  Co,
  
  Here is the v2 patch series for adding T1O protection information (PI)
  SGL passthrough support between virtio-scsi LLD + vhost-scsi fabric
  endpoints.
  
  Following MST's recommendation, it includes the changes for using
  bytes intead of number of iovecs in virtio_scsi_cmd_req_pi along with
  the associated changes to virtio-scsi + vhost/scsi code.
  
  For vhost/scsi, this includes walking the leading iovec's length(s)
  to determine where protection payload ends, and real data payload 
  starts.  For virtio-scsi LLD code, this includes locating struct
  blk_integrity for using blk_rq_sectors + -tuple_size to calculate
  the total bytes for outgoing cmd_pi-pi_bytes[out,in] values.
  
  The full list of changes from last series include:
  
- Use pi_bytesout + pi_bytesin instead of niovs in virtio-scsi PI
  header (mst + paolo)
- Add prot_pto=1 in tcm_vhost_submission_work() when no PI buffer
  exists (nab)
- Get rid of req + cdb pointer casts in vhost_scsi_handle_vq (mst)
- Ensure that virtio_scsi_cmd_req_pi processing happens regardless
  of data_num in vhost_scsi_handle_vq (nab)
- Pass TARGET_PROT_ALL into transport_init_session_tags() (nab)
- Convert vhost_scsi_handle_vq to use memcpy_fromiovecend() (mst)
- Convert vhost_scsi_handle_vq to use pi_bytesout + pi_bytesin (nab)
- Convert virtio_scsi_init_hdr_pi() to use pi_bytesout + pi_bytesin
  (mst + paolo + nab)
- Use blk_integrity-tuple_size to calculate pi bytes (nab)
  
  Please review for v3.16-rc1 code.
  
  Thanks!
  
  --nab
 
 OK since this conflicts with my vhost locking
 patches, I have rebased this and parked at vhost-review
 branch in my tree.
 
 git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-review
 
 Nicholas could you please take a look at the patches and confirm this is
 OK ASAP?
 
 If yes I will pack it all up and send to Linus.

From my experience, Linus prefers to fix this type of conflict on his
own at merge time, instead of having subsystem maintainers rebase their
for-next branches to include other's commits.

Stephen (CC'ed) has included a fix in today's linux-next for the merge
conflict here:

https://lkml.org/lkml/2014/6/10/3

Please confirm, as it will be a pointer to Linus within the
target-pending/for-next PULL request.

--nab

--
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 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[Nicholas A. Bellinger]

On Sun, 2014-06-08 at 19:05 +0300, Michael S. Tsirkin wrote:
 On Thu, May 22, 2014 at 02:26:16AM +, Nicholas A. Bellinger wrote:
  From: Nicholas Bellinger n...@linux-iscsi.org
  
  Hi MST, MKP, Paolo  Co,
  
  Here is the v2 patch series for adding T1O protection information (PI)
  SGL passthrough support between virtio-scsi LLD + vhost-scsi fabric
  endpoints.
  
  Following MST's recommendation, it includes the changes for using
  bytes intead of number of iovecs in virtio_scsi_cmd_req_pi along with
  the associated changes to virtio-scsi + vhost/scsi code.
  
  For vhost/scsi, this includes walking the leading iovec's length(s)
  to determine where protection payload ends, and real data payload 
  starts.  For virtio-scsi LLD code, this includes locating struct
  blk_integrity for using blk_rq_sectors + -tuple_size to calculate
  the total bytes for outgoing cmd_pi-pi_bytes[out,in] values.
  
  The full list of changes from last series include:
  
- Use pi_bytesout + pi_bytesin instead of niovs in virtio-scsi PI
  header (mst + paolo)
- Add prot_pto=1 in tcm_vhost_submission_work() when no PI buffer
  exists (nab)
- Get rid of req + cdb pointer casts in vhost_scsi_handle_vq (mst)
- Ensure that virtio_scsi_cmd_req_pi processing happens regardless
  of data_num in vhost_scsi_handle_vq (nab)
- Pass TARGET_PROT_ALL into transport_init_session_tags() (nab)
- Convert vhost_scsi_handle_vq to use memcpy_fromiovecend() (mst)
- Convert vhost_scsi_handle_vq to use pi_bytesout + pi_bytesin (nab)
- Convert virtio_scsi_init_hdr_pi() to use pi_bytesout + pi_bytesin
  (mst + paolo + nab)
- Use blk_integrity-tuple_size to calculate pi bytes (nab)
  
  Please review for v3.16-rc1 code.
  
  Thanks!
  
  --nab
 
 
 OK, finally went over this, looks good to me:
 
 Acked-by: Michael S. Tsirkin m...@redhat.com
 
 However, we really should stop making more changes
 before fixing ANY_LAYOUT in this driver.
 
 virtio 1.0 should be out soon and that makes ANY_LAYOUT
 a required feature.
 

What else is required to complete the ANY_LAYOUT conversion..?

--nab

--
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 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[Paolo Bonzini]

Il 10/06/2014 09:07, Nicholas A. Bellinger ha scritto:

 OK, finally went over this, looks good to me:

 Acked-by: Michael S. Tsirkin m...@redhat.com

 However, we really should stop making more changes
 before fixing ANY_LAYOUT in this driver.

 virtio 1.0 should be out soon and that makes ANY_LAYOUT
 a required feature.


What else is required to complete the ANY_LAYOUT conversion..?


Basically, you need to stop expecting the request and response headers 
to be in a separate iov, and also vhost-scsi should not expect pi to be 
in a separate iov than the main data.  A layout that has everything in 
the same iov would be fine, and similarly for a layout that has the CDB 
in a separate iov than the rest of the header.


Paolo
--
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 v1 3/3] TARGET/sbc,loopback: Adjust command data length in case pi exists on the wire

[Nicholas A. Bellinger]

Hi Sagi  Co,

On Sun, 2014-06-08 at 13:27 +0300, Sagi Grimberg wrote:
 In various areas of the code, it is assumed that
 se_cmd-data_length describes pure data. In case
 that protection information exists over the wire
 (protect bits is are on) the target core decrease
 the protection length from the data length (instead
 of each transport peeking in the cdb).
 
 Modify loopback device to include protection information
 in the transferred data length (like other scsi transports).
 
 Signed-off-by: Sagi Grimberg sa...@mellanox.com
 ---
  drivers/target/loopback/tcm_loop.c |   15 ---
  drivers/target/target_core_sbc.c   |   15 +--
  2 files changed, 25 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/target/loopback/tcm_loop.c 
 b/drivers/target/loopback/tcm_loop.c
 index c886ad1..1f4c015 100644
 --- a/drivers/target/loopback/tcm_loop.c
 +++ b/drivers/target/loopback/tcm_loop.c
 @@ -179,7 +179,7 @@ static void tcm_loop_submission_work(struct work_struct 
 *work)
   struct tcm_loop_hba *tl_hba;
   struct tcm_loop_tpg *tl_tpg;
   struct scatterlist *sgl_bidi = NULL;
 - u32 sgl_bidi_count = 0;
 + u32 sgl_bidi_count = 0, transfer_length;
   int rc;
  
   tl_hba = *(struct tcm_loop_hba **)shost_priv(sc-device-host);
 @@ -213,12 +213,21 @@ static void tcm_loop_submission_work(struct work_struct 
 *work)
  
   }
  
 - if (!scsi_prot_sg_count(sc)  scsi_get_prot_op(sc) != SCSI_PROT_NORMAL)
 + transfer_length = scsi_transfer_length(sc);
 + if (!scsi_prot_sg_count(sc) 
 + scsi_get_prot_op(sc) != SCSI_PROT_NORMAL) {
   se_cmd-prot_pto = true;
 + /*
 +  * loopback transport doesn't support
 +  * WRITE_GENERATE, READ_STRIP protection
 +  * information operations, go ahead unprotected.
 +  */
 + transfer_length = scsi_bufflen(sc);
 + }
  
   rc = target_submit_cmd_map_sgls(se_cmd, tl_nexus-se_sess, sc-cmnd,
   tl_cmd-tl_sense_buf[0], tl_cmd-sc-device-lun,
 - scsi_bufflen(sc), tcm_loop_sam_attr(sc),
 + transfer_length, tcm_loop_sam_attr(sc),
   sc-sc_data_direction, 0,
   scsi_sglist(sc), scsi_sg_count(sc),
   sgl_bidi, sgl_bidi_count,
 diff --git a/drivers/target/target_core_sbc.c 
 b/drivers/target/target_core_sbc.c
 index e022959..06f8ecd 100644
 --- a/drivers/target/target_core_sbc.c
 +++ b/drivers/target/target_core_sbc.c
 @@ -665,8 +665,19 @@ sbc_check_prot(struct se_device *dev, struct se_cmd 
 *cmd, unsigned char *cdb,
  
   cmd-prot_type = dev-dev_attrib.pi_prot_type;
   cmd-prot_length = dev-prot_length * sectors;
 - pr_debug(%s: prot_type=%d, prot_length=%d prot_op=%d prot_checks=%d\n,
 -  __func__, cmd-prot_type, cmd-prot_length,
 +
 + /**
 +  * In case protection information exists over the wire
 +  * we modify command data length to describe pure data.
 +  * The actual transfer length is data length + protection
 +  * length
 +  **/
 + if (protect)
 + cmd-data_length -= cmd-prot_length;
 +

This fabric wide assumption breaks the vhost-scsi + tcm_qla2xxx PI code
already queued for v3.16-rc1..

A vhost-scsi side conversion to combine exp_data_len + prot_bytes is
easy enough in target-pending/for-next (see below), given that
vhost-scsi's exp_data_len is coming from virtio buffer iovecs, so
changing virtio-scsi LLD to use scsi_transfer_length() doesn't really
make any sense. (Adding CC's for MST + Paolo)

The qla2xxx target T10 PI patch in scsi/for-next (adding Quinn CC) will
also need a similar change to update qlt_do_work() to include PI bytes
for data_length being passed into tgt_ops-handle_cmd(), following what
qlt_build_ctio_crc2_pkt() is already doing for calculating
transfer_length for HW offload.

That said, there is one other small qla2xxx change to enable per-session
PI that is currently missing in Quinn's patch in scsi/for-next code. 

Sooo, I'll go ahead and include Sagi's patches with the vhost-scsi
change below if there are no objections from MKP and/or driver
maintainers, and post an -rc2 series after the merge window closes to
address the two remaining qla2xxx specific items.

--nab

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 667e72d..03e484f 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1144,7 +1144,8 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct 
vhost_virtqueue *vq)
}
 
cmd = vhost_scsi_get_tag(vq, tpg, cdb, tag, lun, task_attr,
-exp_data_len, data_direction);
+exp_data_len + prot_bytes,
+data_direction);
if (IS_ERR(cmd)) {
vq_err(vq, vhost_scsi_get_tag failed %ld\n,

Re: [PATCH v1 0/3] Include protection information in iscsi header

[Nicholas A. Bellinger]

Hi MKP + Mike + Roland,

On Sun, 2014-06-08 at 13:27 +0300, Sagi Grimberg wrote:
 At the SCSI transport level, there is no distinction between
 user data and protection information. Thus, iscsi header field
 expected data transfer length should include protection
 information.
 
 Patch #1 introduces scsi helpers scsi_transfer_length (compute
 wire transfer byte count) and scsi_prot_len (compute protection
 information byte count).
 
 Patch #2 modifies iscsi initiator to set correct wire transfer length
 in iscsi header data_length field (and modifies iser accordingly).
 
 Patch #3 modifies target core to expect protection information included
 in the wire transfer length (and modifies loopback transport to do so).
 
 I have 2 patches that convert lpfc/qla2xxx drivers to use scsi helpers
 but these are completely untested at the moment. Once we get this set
 to land upstream, I can queue them up as RFCs.
 
 Changes from v0:
 - Introduce scsi helpers to compute correct transfer length in the
   presence of protection information (instead of having each transport
   doing the same computation).
 - Modify iscsi to set correct transfer length using scsi helpers
 - Modify loopback transport to set correct transfer length using
   scsi helpers
 
 Sagi Grimberg (3):
   scsi_cmnd: Introduce scsi_transfer_length helper
   libiscsi, iser: Adjust data_length to include protection information
   TARGET/sbc,loopback: Adjust command data length in case pi exists on
 the wire
 
  drivers/infiniband/ulp/iser/iser_initiator.c |   34 ++
  drivers/scsi/libiscsi.c  |   18 ++--
  drivers/target/loopback/tcm_loop.c   |   15 --
  drivers/target/target_core_sbc.c |   15 -
  include/scsi/scsi_cmnd.h |   39 
 ++
  5 files changed, 83 insertions(+), 38 deletions(-)
 

Please review + ACK this series.

Also, given the nature of the changes, they will need to be included
into v3.15.y code to avoid any potential wire protocol compatibility
issues.

--nab

--
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 v1 3/3] TARGET/sbc,loopback: Adjust command data length in case pi exists on the wire

[Paolo Bonzini]

Il 10/06/2014 10:04, Nicholas A. Bellinger ha scritto:


That said, there is one other small qla2xxx change to enable per-session
PI that is currently missing in Quinn's patch in scsi/for-next code.

Sooo, I'll go ahead and include Sagi's patches with the vhost-scsi
change below if there are no objections from MKP and/or driver
maintainers, and post an -rc2 series after the merge window closes to
address the two remaining qla2xxx specific items.


Indeed the patch looks like a tcm bugfix.  There is no reason to modify 
the LLD.


Paolo
--
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 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[Michael S. Tsirkin]

On Tue, Jun 10, 2014 at 12:05:12AM -0700, Nicholas A. Bellinger wrote:
 On Mon, 2014-06-09 at 16:30 +0300, Michael S. Tsirkin wrote:
  On Thu, May 22, 2014 at 02:26:16AM +, Nicholas A. Bellinger wrote:
   From: Nicholas Bellinger n...@linux-iscsi.org
   
   Hi MST, MKP, Paolo  Co,
   
   Here is the v2 patch series for adding T1O protection information (PI)
   SGL passthrough support between virtio-scsi LLD + vhost-scsi fabric
   endpoints.
   
   Following MST's recommendation, it includes the changes for using
   bytes intead of number of iovecs in virtio_scsi_cmd_req_pi along with
   the associated changes to virtio-scsi + vhost/scsi code.
   
   For vhost/scsi, this includes walking the leading iovec's length(s)
   to determine where protection payload ends, and real data payload 
   starts.  For virtio-scsi LLD code, this includes locating struct
   blk_integrity for using blk_rq_sectors + -tuple_size to calculate
   the total bytes for outgoing cmd_pi-pi_bytes[out,in] values.
   
   The full list of changes from last series include:
   
 - Use pi_bytesout + pi_bytesin instead of niovs in virtio-scsi PI
   header (mst + paolo)
 - Add prot_pto=1 in tcm_vhost_submission_work() when no PI buffer
   exists (nab)
 - Get rid of req + cdb pointer casts in vhost_scsi_handle_vq (mst)
 - Ensure that virtio_scsi_cmd_req_pi processing happens regardless
   of data_num in vhost_scsi_handle_vq (nab)
 - Pass TARGET_PROT_ALL into transport_init_session_tags() (nab)
 - Convert vhost_scsi_handle_vq to use memcpy_fromiovecend() (mst)
 - Convert vhost_scsi_handle_vq to use pi_bytesout + pi_bytesin (nab)
 - Convert virtio_scsi_init_hdr_pi() to use pi_bytesout + pi_bytesin
   (mst + paolo + nab)
 - Use blk_integrity-tuple_size to calculate pi bytes (nab)
   
   Please review for v3.16-rc1 code.
   
   Thanks!
   
   --nab
  
  OK since this conflicts with my vhost locking
  patches, I have rebased this and parked at vhost-review
  branch in my tree.
  
  git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-review
  
  Nicholas could you please take a look at the patches and confirm this is
  OK ASAP?
  
  If yes I will pack it all up and send to Linus.
 
 From my experience, Linus prefers to fix this type of conflict on his
 own at merge time, instead of having subsystem maintainers rebase their
 for-next branches to include other's commits.

Cross-device type changes is exactly what vhost tree is there to allow
so I don't see a problem.

What Linus does not want is same patch in two trees.

So I see two options:
- I go ahead with my changes and you with yours and let Linus resolve
  the conflict.  This means bisect build will be broken since the
  breakage will likely not be noticed until after the merge.
- I pick up vhost scsi PI patches on my tree and you drop them from yours.

I prefer option 2 because it's cleaner wrt bisect.
But if you see a problem, pls let me know.

 Stephen (CC'ed) has included a fix in today's linux-next for the merge
 conflict here:
 
 https://lkml.org/lkml/2014/6/10/3
 
 Please confirm, as it will be a pointer to Linus within the
 target-pending/for-next PULL request.
 
 --nab


Yes but this does mean people trying to bisect will
hit build breakages, not nice.

-- 
MST
--
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 00/32] making inode time stamps y2038 ready

[Arnd Bergmann]

On Wednesday 04 June 2014 17:10:24 H. Peter Anvin wrote:
 On 06/04/2014 12:24 PM, Arnd Bergmann wrote:
  
  For other timekeeping stuff in the kernel, I agree that using some
  64-bit representation (nanoseconds, 32/32 unsigned seconds/nanoseconds,
  ...) has advantages, that's exactly the point I was making earlier
  against simply extending the internal time_t/timespec to 64-bit
  seconds for everything.
  
 
 How much of a performance issue is it to make time_t 64 bits, and for
 the bits there are, how hard are they to fix?

Probably very little overhead for most uses, it's more the regression
potential in the less common parts of the kernel I'm worried about.

There is a significant but not overwhelming number of uses of the
main problematic types in the kernel:

arnd@wuerfel:~/arm-soc$ git grep -wl time_t | wc
188 1885566
arnd@wuerfel:~/arm-soc$ git grep -wl timeval | wc
320 320   10353
arnd@wuerfel:~/arm-soc$ git grep -wl timespec | wc
406 406   10886

I believe we have to audit all of them anyway if we want to change
the kernel to less problematic types and introduce new user
interfaces.

IMHO this work is helped if we change the uses to a new type
as we find the problems. This lets us do the work one subsystem
at a time and avoid accidental ABI changes. I don't care much what
type that will be, and having a 96-bit type will certainly work
well in a lot of cases, but I don't see a strong reason to use
that over other types, especially when they can be more efficient.

Arnd
--
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 linux-scsi-ml] linux-firmware: qla2xxx: Update ql2{4,5}00_fw.bin to version 7.03.00

[Xose Vazquez Perez]

resent to linux-scsi-ml, without the binary blob(244K).

Firmwares were taken from http://ldriver.qlogic.com/firmware/

Cc: Chad Dupuis chad.dup...@qlogic.com
Cc: Andrew Vasquez andrew.vasq...@qlogic.com
Cc: Joe Carnuccio joe.carnuc...@qlogic.com
Cc: Saurav Kashyap saurav.kash...@qlogic.com
Cc: Qlogic internal list linux-dri...@qlogic.com
Cc: Linux Firmware Maintainers linux-firmw...@kernel.org
Signed-off-by: Xose Vazquez Perez xose.vazq...@gmail.com
---
 WHENCE|   4 ++--
 ql2400_fw.bin | Bin 259332 - 260656 bytes
 ql2500_fw.bin | Bin 265420 - 266912 bytes
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/WHENCE b/WHENCE
index 6a64fc2..f257a93 100644
--- a/WHENCE
+++ b/WHENCE
@@ -429,9 +429,9 @@ Version: 3.03.28 IPX
 File: ql2322_fw.bin
 Version: 3.03.28 IPX
 File: ql2400_fw.bin
-Version: 7.01.00 MID
+Version: 7.03.00 MID
 File: ql2500_fw.bin
-Version: 7.01.00 MIDQ
+Version: 7.03.00 MIDQ
  Licence: Redistributable. See LICENCE.qla2xxx for details

diff --git a/ql2400_fw.bin b/ql2400_fw.bin
 [...]

diff --git a/ql2500_fw.bin b/ql2500_fw.bin
 [...]
-- 
1.9.3

--
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

P/s Get back to me on this email: mrs.florenceeva...@gmail.com

[Birgit Kruse]

Hello my beloved in the Lord.

I am MRS. FLORENCE EVANS, from republic of Ireland, I am legally married to MR. 
JOHN EVANS, a South Africa citizen born brought up in Switzerland, I live in 
Switzerland with my husband for 32 years before we move down to south Africa in 
1985 after my husband retirement in 1974, I am 71 years old by the grace of 
god, I am a new Christian convert, suffering from long time cancer of the 
breast. All indication from my doctor that my condition is really deteriorating 
and it is quite obvious that i wouldn't live more than two months. My dear 
husband was involved with the January 2000 Kenya airways plane crashed as you 
can see on the news line web site.
http://news.bbc.co.uk/2/hi/africa/6627485.stm
I have confident in you because i have prayed. I am willing to donate the sum 
of $20.5m U.S dollars, to the less privileged. May the grace of our lord be 
with you, P/s Get back to me on this email: mrs.florenceeva...@gmail.com

Remain blessed,
MRS. FLORENCE EVANS
--
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/11] drivers/scsi/bfa/bfad_bsg.c: Remove useless return variables

[Sudarsana Kalluru]

Thanks for the patch.
Acked-by: Sudarsana Kalluru sudarsana.kall...@qlogic.com

-Original Message-
From: linux-scsi-ow...@vger.kernel.org 
[mailto:linux-scsi-ow...@vger.kernel.org] On Behalf Of Peter Senna Tschudin
Sent: 31 May 2014 18:44
To: Anil Gurumurthy
Cc: kernel-janit...@vger.kernel.org; Sudarsana Kalluru; James E.J. Bottomley; 
linux-scsi; linux-kernel
Subject: [PATCH 5/11] drivers/scsi/bfa/bfad_bsg.c: Remove useless return 
variables

This patch remove variables that are initialized with a constant, are never 
updated, and are only used as parameter of return.
Return the constant instead of using a variable.

Verified by compilation only.

The coccinelle script that find and fixes this issue is:
// smpl
@@
type T;
constant C;
identifier ret;
@@
- T ret = C;
... when != ret
when strict
return
- ret
+ C
;
// /smpl

Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com

---
 drivers/scsi/bfa/bfad_bsg.c |   10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/bfa/bfad_bsg.c b/drivers/scsi/bfa/bfad_bsg.c index 
8994fb8..a6ee1cf 100644
--- a/drivers/scsi/bfa/bfad_bsg.c
+++ b/drivers/scsi/bfa/bfad_bsg.c
@@ -26,7 +26,6 @@ int
 bfad_iocmd_ioc_enable(struct bfad_s *bfad, void *cmd)  {
struct bfa_bsg_gen_s *iocmd = (struct bfa_bsg_gen_s *)cmd;
-   int rc = 0;
unsigned long   flags;

spin_lock_irqsave(bfad-bfad_lock, flags); @@ -34,7 +33,7 @@ 
bfad_iocmd_ioc_enable(struct bfad_s *bfad, void *cmd)
if (!bfa_ioc_is_disabled(bfad-bfa.ioc)) {
spin_unlock_irqrestore(bfad-bfad_lock, flags);
iocmd-status = BFA_STATUS_OK;
-   return rc;
+   return 0;
}

init_completion(bfad-enable_comp);
@@ -43,21 +42,20 @@ bfad_iocmd_ioc_enable(struct bfad_s *bfad, void *cmd)
spin_unlock_irqrestore(bfad-bfad_lock, flags);
wait_for_completion(bfad-enable_comp);

-   return rc;
+   return 0;
 }

 int
 bfad_iocmd_ioc_disable(struct bfad_s *bfad, void *cmd)  {
struct bfa_bsg_gen_s *iocmd = (struct bfa_bsg_gen_s *)cmd;
-   int rc = 0;
unsigned long   flags;

spin_lock_irqsave(bfad-bfad_lock, flags);
if (bfa_ioc_is_disabled(bfad-bfa.ioc)) {
spin_unlock_irqrestore(bfad-bfad_lock, flags);
iocmd-status = BFA_STATUS_OK;
-   return rc;
+   return 0;
}

if (bfad-disable_active) {
@@ -74,7 +72,7 @@ bfad_iocmd_ioc_disable(struct bfad_s *bfad, void *cmd)
bfad-disable_active = BFA_FALSE;
iocmd-status = BFA_STATUS_OK;

-   return rc;
+   return 0;
 }

 static int

--
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



This message and any attached documents contain information from QLogic 
Corporation or its wholly-owned subsidiaries that may be confidential. If you 
are not the intended recipient, you may not read, copy, distribute, or use this 
information. If you have received this transmission in error, please notify the 
sender immediately by reply e-mail and then delete this message.
attachment: winmail.dat

Re: [PATCH 6/6] scsi_scan: Fixup scsilun_to_int()

[Bart Van Assche]

On 06/03/14 10:58, Hannes Reinecke wrote:
 + * Given a struct scsi_lun of: d2 04 0b 03 00 00 00 00, this function
 + * returns the integer: 0x0b03d204
 + *
 + * This encoding will return a standard integer LUN for LUNs smaller
 + * than 256, which typically use a single level LUN structure with
 + * addressing method 0.
   **/
  u64 scsilun_to_int(struct scsi_lun *scsilun)
  {
 @@ -1279,7 +1280,7 @@ u64 scsilun_to_int(struct scsi_lun *scsilun)
  
   lun = 0;
   for (i = 0; i  sizeof(lun); i += 2)
 - lun = lun | (((scsilun-scsi_lun[i]  8) |
 + lun = lun | (((scsilun-scsi_lun[i]  ((i + 1) *8)) |
 scsilun-scsi_lun[i + 1])  (i * 8));
   return lun;
  }

The above code doesn't match the comment header. Parentheses have been
placed such that each byte with an even index is shifted left (2*i+1)*8
bits instead of (i+1)*8. I assume this means the parentheses have been
misplaced ? Another bug in this code is that a cast from
scsilun-scsi_lun[i] from u8 to u64 is missing. I think the shift
operations in the above code trigger what is called undefined behavior
in the C standard if i = 4.

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: [Open-FCoE] [PATCH] fc: ensure scan_work isn't active when freeing fc_rport

[Christoph Hellwig]

On Mon, Jun 09, 2014 at 03:16:37PM -0400, Neil Horman wrote:
  Given fcoe is quite mature now and its patches volume is very low, so
  getting its kernel patches directly to scsi subsystem should work fine
  and should be okay with James or Christophs to pull into scsi subsystem
  directly once I've my non-author signoff ACK there as described in this
  announcement at http://marc.info/?l=linux-scsim=140050839729415w=2 
  
  If no alternate suggestion or objection to this then I'll formally
  announce this on fcoe mailing list.
  
  However for any huges patches series bomb or RFCs, I'll request fcoe
  developers to send patches against scsi tree at fcoe devel list first
  and then if needed I can roll them up.
  
  Thanks,
  Vasu
  
 Copy that Vasu, Christoph, is that ok with you?

That's fine with me.  It would help greatly if you could make sure
all the paches get a review or two very quickly so I can just pick them
up immediately after reviewing them.

--
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 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[Stephen Rothwell]

Hi Michael,

On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin m...@redhat.com wrote:

 So I see two options:
 - I go ahead with my changes and you with yours and let Linus resolve
   the conflict.  This means bisect build will be broken since the
   breakage will likely not be noticed until after the merge.

Well, since the resolution is known, the one who submits their tree
later should tell Linus (as suggested by Nicholas).  That is part of
the point of the linux-next tree ... and therefore there would be no
bisect problem.

  Stephen (CC'ed) has included a fix in today's linux-next for the merge
  conflict here:
  
  https://lkml.org/lkml/2014/6/10/3
  
  Please confirm, as it will be a pointer to Linus within the
  target-pending/for-next PULL request.
 
 Yes but this does mean people trying to bisect will
 hit build breakages, not nice.

Not necessarily.

-- 
Cheers,
Stephen Rothwells...@canb.auug.org.au


signature.asc
Description: PGP signature

Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[Michael S. Tsirkin]

On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote:
 Hi Michael,
 
 On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin m...@redhat.com 
 wrote:
 
  So I see two options:
  - I go ahead with my changes and you with yours and let Linus resolve
the conflict.  This means bisect build will be broken since the
breakage will likely not be noticed until after the merge.
 
 Well, since the resolution is known, the one who submits their tree
 later should tell Linus (as suggested by Nicholas).  That is part of
 the point of the linux-next tree ... and therefore there would be no
 bisect problem.
 
   Stephen (CC'ed) has included a fix in today's linux-next for the merge
   conflict here:
   
   https://lkml.org/lkml/2014/6/10/3
   
   Please confirm, as it will be a pointer to Linus within the
   target-pending/for-next PULL request.
  
  Yes but this does mean people trying to bisect will
  hit build breakages, not nice.
 
 Not necessarily.
 
 -- 
 Cheers,
 Stephen Rothwells...@canb.auug.org.au


I don't see how that's possible.
Here's a point you might have missed.
Nicholas's patch isn't just introducing a merge conflict.
It is also buggy.
Replacing bit access with has_feature silently fixes the bug.

So if we want to avoid bisect breakage target tree will
have to be rebased.

And if doing that anyway, I don't see any reason not
to merge everything through the vhost tree, esp
since I already put the patches there. Less work for
everyone involved.

-- 
MST
--
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 6/6] scsi_scan: Fixup scsilun_to_int()

[Douglas Gilbert]

On 14-06-10 07:37 AM, Bart Van Assche wrote:

On 06/03/14 10:58, Hannes Reinecke wrote:

+ * Given a struct scsi_lun of: d2 04 0b 03 00 00 00 00, this function
+ * returns the integer: 0x0b03d204
+ *
+ * This encoding will return a standard integer LUN for LUNs smaller
+ * than 256, which typically use a single level LUN structure with
+ * addressing method 0.
   **/
  u64 scsilun_to_int(struct scsi_lun *scsilun)
  {
@@ -1279,7 +1280,7 @@ u64 scsilun_to_int(struct scsi_lun *scsilun)

lun = 0;
for (i = 0; i  sizeof(lun); i += 2)
-   lun = lun | (((scsilun-scsi_lun[i]  8) |
+   lun = lun | (((scsilun-scsi_lun[i]  ((i + 1) *8)) |
  scsilun-scsi_lun[i + 1])  (i * 8));
return lun;
  }


The above code doesn't match the comment header. Parentheses have been
placed such that each byte with an even index is shifted left (2*i+1)*8
bits instead of (i+1)*8. I assume this means the parentheses have been
misplaced ? Another bug in this code is that a cast from
scsilun-scsi_lun[i] from u8 to u64 is missing. I think the shift
operations in the above code trigger what is called undefined behavior
in the C standard if i = 4.


Hmm, we have been over this ground before. In sg_luns,
to support translating between the T10 and Linux
representation of 64 bit LUNs, there are these two
versions:


static uint64_t
t10_2linux_lun(const unsigned char t10_lun[])
{
const unsigned char * cp;
uint64_t res;

 res = (t10_lun[6]  8) + t10_lun[7];
 for (cp = t10_lun + 4; cp = t10_lun; cp -= 2) {
res = 16;
res += (*cp  8) + *(cp + 1);
}
return res;
}

/* Copy of t10_lun -- Linux unsigned int (i.e. 32 bit ) present in Linux
 * kernel, up to least lk 3.8.0, extended to 64 bits.
 * BEWARE: for sizeof(int==4) this function is BROKEN and is left here as
 * as example and may soon be removed. */
static uint64_t
t10_2linux_lun64bitBR(const unsigned char t10_lun[])
{
int i;
uint64_t lun;

lun = 0;
for (i = 0; i  (int)sizeof(lun); i += 2)
lun = lun | (((t10_lun[i]  8) | t10_lun[i + 1])  (i * 8));
return lun;
}

The second one (with BR (for broken) appended) is for testing.
And that second one looks very similar to the code you are
objecting to.

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

[PATCH] aic7xxx: Use kstrdup

[Himangi Saraogi]

Use kstrdup when the goal of an allocation is copy a string into the
allocated region.

The Coccinelle semantic patch that makes this change is as follows:

// smpl
@@
expression from,to;
expression flag,E1,E2;
statement S;
@@

-  to = kmalloc(strlen(from) + 1,flag);
+  to = kstrdup(from, flag);
   ... when != \(from = E1 \| to = E1 \)
   if (to==NULL || ...) S
   ... when != \(from = E2 \| to = E2 \)
-  strcpy(to, from);
// /smpl

Signed-off-by: Himangi Saraogi himangi...@gmail.com
Acked-by: Julia Lawall julia.law...@lip6.fr
---
 drivers/scsi/aic7xxx/aic7xxx_osm_pci.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c 
b/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
index ee05e84..0fc14da 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
@@ -225,10 +225,9 @@ ahc_linux_pci_dev_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
ahc_get_pci_bus(pci),
ahc_get_pci_slot(pci),
ahc_get_pci_function(pci));
-   name = kmalloc(strlen(buf) + 1, GFP_ATOMIC);
+   name = kstrdup(buf, GFP_ATOMIC);
if (name == NULL)
return (-ENOMEM);
-   strcpy(name, buf);
ahc = ahc_alloc(NULL, name);
if (ahc == NULL)
return (-ENOMEM);
-- 
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

[PATCH] aic7xxx/aic7770 : Use kstrdup

[Himangi Saraogi]

Use kstrdup when the goal of an allocation is copy a string into the
allocated region.

The semantic patch that makes this change is as follows:

// smpl
@@
expression from,to;
expression flag,E1,E2;
statement S;
@@

-  to = kmalloc(strlen(from) + 1,flag);
+  to = kstrdup(from, flag);
   ... when != \(from = E1 \| to = E1 \)
   if (to==NULL || ...) S
   ... when != \(from = E2 \| to = E2 \)
-  strcpy(to, from);
// /smpl

Signed-off-by: Himangi Saraogi himangi...@gmail.com
Acked-by: Julia Lawall julia.law...@lip6.fr
---
 drivers/scsi/aic7xxx/aic7770_osm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/aic7xxx/aic7770_osm.c 
b/drivers/scsi/aic7xxx/aic7770_osm.c
index 0cb8ef6..3d401d0 100644
--- a/drivers/scsi/aic7xxx/aic7770_osm.c
+++ b/drivers/scsi/aic7xxx/aic7770_osm.c
@@ -85,10 +85,9 @@ aic7770_probe(struct device *dev)
int error;
 
sprintf(buf, ahc_eisa:%d, eisaBase  12);
-   name = kmalloc(strlen(buf) + 1, GFP_ATOMIC);
+   name = kstrdup(buf, GFP_ATOMIC);
if (name == NULL)
return (ENOMEM);
-   strcpy(name, buf);
ahc = ahc_alloc(aic7xxx_driver_template, name);
if (ahc == NULL)
return (ENOMEM);
-- 
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

[PATCH] aic7xxx/aic79xx : Use kstrdup

[Himangi Saraogi]

Use kstrdup when the goal of an allocation is copy a string into the
allocated region.

The semantic patch that makes this change is as follows:
// smpl
@@
expression from,to;
expression flag,E1,E2;
statement S;
@@

-  to = kmalloc(strlen(from) + 1,flag);
+  to = kstrdup(from, flag);
   ... when != \(from = E1 \| to = E1 \)
   if (to==NULL || ...) S
   ... when != \(from = E2 \| to = E2 \)
-  strcpy(to, from);
// /smpl


Signed-off-by: Himangi Saraogi himangi...@gmail.com
Acked-by: Julia Lawall julia.law...@lip6.fr
---
 drivers/scsi/aic7xxx/aic79xx_osm_pci.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/aic7xxx/aic79xx_osm_pci.c 
b/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
index 3c85873..8466aa7 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
+++ b/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
@@ -178,10 +178,9 @@ ahd_linux_pci_dev_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
ahd_get_pci_bus(pci),
ahd_get_pci_slot(pci),
ahd_get_pci_function(pci));
-   name = kmalloc(strlen(buf) + 1, GFP_ATOMIC);
+   name = kstrdup(buf, GFP_ATOMIC);
if (name == NULL)
return (-ENOMEM);
-   strcpy(name, buf);
ahd = ahd_alloc(NULL, name);
if (ahd == NULL)
return (-ENOMEM);
-- 
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 6/6] scsi_scan: Fixup scsilun_to_int()

[Bart Van Assche]

On 06/10/14 16:06, James Bottomley wrote:
 On Tue, 2014-06-10 at 13:37 +0200, Bart Van Assche wrote:
 On 06/03/14 10:58, Hannes Reinecke wrote:
 + * Given a struct scsi_lun of: d2 04 0b 03 00 00 00 00, this function
 + * returns the integer: 0x0b03d204
 + *
 + * This encoding will return a standard integer LUN for LUNs smaller
 + * than 256, which typically use a single level LUN structure with
 + * addressing method 0.
   **/
  u64 scsilun_to_int(struct scsi_lun *scsilun)
  {
 @@ -1279,7 +1280,7 @@ u64 scsilun_to_int(struct scsi_lun *scsilun)
  
 lun = 0;
 for (i = 0; i  sizeof(lun); i += 2)
 -   lun = lun | (((scsilun-scsi_lun[i]  8) |
 +   lun = lun | (((scsilun-scsi_lun[i]  ((i + 1) *8)) |
   scsilun-scsi_lun[i + 1])  (i * 8));
 return lun;
  }

 The above code doesn't match the comment header. Parentheses have been
 placed such that each byte with an even index is shifted left (2*i+1)*8
 bits instead of (i+1)*8.
 
 I don't see that in the code, which parentheses do you mean?

This patch should change the code into what I think was intended by the
comment above scsilun_to_int():

--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1280,8 +1280,8 @@ u64 scsilun_to_int(struct scsi_lun *scsilun)
 
lun = 0;
for (i = 0; i  sizeof(lun); i += 2)
-   lun = lun | (((scsilun-scsi_lun[i]  ((i + 1) *8)) |
- scsilun-scsi_lun[i + 1])  (i * 8));
+   lun = lun | (((u64)scsilun-scsi_lun[i]  ((i + 1) *8)) |
+((u64)scsilun-scsi_lun[i + 1]  (i * 8)));
return lun;
 }
 EXPORT_SYMBOL(scsilun_to_int);

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: [PATCH 6/6] scsi_scan: Fixup scsilun_to_int()

[Douglas Gilbert]

On 14-06-10 10:06 AM, James Bottomley wrote:

On Tue, 2014-06-10 at 13:37 +0200, Bart Van Assche wrote:

On 06/03/14 10:58, Hannes Reinecke wrote:

+ * Given a struct scsi_lun of: d2 04 0b 03 00 00 00 00, this function
+ * returns the integer: 0x0b03d204
+ *
+ * This encoding will return a standard integer LUN for LUNs smaller
+ * than 256, which typically use a single level LUN structure with
+ * addressing method 0.
   **/
  u64 scsilun_to_int(struct scsi_lun *scsilun)
  {
@@ -1279,7 +1280,7 @@ u64 scsilun_to_int(struct scsi_lun *scsilun)

lun = 0;
for (i = 0; i  sizeof(lun); i += 2)
-   lun = lun | (((scsilun-scsi_lun[i]  8) |
+   lun = lun | (((scsilun-scsi_lun[i]  ((i + 1) *8)) |
  scsilun-scsi_lun[i + 1])  (i * 8));
return lun;
  }


The above code doesn't match the comment header. Parentheses have been
placed such that each byte with an even index is shifted left (2*i+1)*8
bits instead of (i+1)*8.


I don't see that in the code, which parentheses do you mean?


For sizeof(int)==4 then
  unsigned char  32
  unsigned char  40
etc
does _not_ give a 64 bit quantity. It is undefined but seems to
wrap on a 32 bit unsigned int (i.e. 32 bits). One solution: the
left argument to  needs to be a 64 bit quantity
(e.g. uint64_t).

Get sg_luns (from sg3_utils version 1.36 or later) and try this
hierarchical LUN:
  sg_luns -t 0105020603070408
Decoded LUN:
  Peripheral device addressing: bus_id=1, target=5
  Second level addressing:
Peripheral device addressing: bus_id=2, target=6
  Third level addressing:
Peripheral device addressing: bus_id=3, target=7
  Fourth level addressing:
Peripheral device addressing: bus_id=4, target=8

Now ask for a Linux integer translation (in hex) using the
first function I showed in my previous post:
  sg_luns -t 0105020603070408L -H
Linux 'word flipped' integer LUN representation: 0x408030702060105
Decoded LUN:
same as before

As expected.

Now ask for a Linux integer translation (in hex) using the
second function (that Bart is objecting to):
  $ sg_luns -t 0105020603070408W -H
64 bit LUN in T10 preferred (hex) format:  01 05 02 06 03 07 04 08
Linux internal 64 bit LUN representation: 0x60e0307
Decoded LUN:
same as before

The undocumented W suffix calls sg_luns' t10_2linux_lun64bitBR()
function. That function never sets any bits between bit 32 and 63.

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: [PATCH 6/6] scsi_scan: Fixup scsilun_to_int()

[James Bottomley]

On Tue, 2014-06-10 at 16:48 +0200, Bart Van Assche wrote:
 On 06/10/14 16:06, James Bottomley wrote:
  On Tue, 2014-06-10 at 13:37 +0200, Bart Van Assche wrote:
  On 06/03/14 10:58, Hannes Reinecke wrote:
  + * Given a struct scsi_lun of: d2 04 0b 03 00 00 00 00, this function
  + * returns the integer: 0x0b03d204
  + *
  + * This encoding will return a standard integer LUN for LUNs smaller
  + * than 256, which typically use a single level LUN structure with
  + * addressing method 0.
**/
   u64 scsilun_to_int(struct scsi_lun *scsilun)
   {
  @@ -1279,7 +1280,7 @@ u64 scsilun_to_int(struct scsi_lun *scsilun)
   
lun = 0;
for (i = 0; i  sizeof(lun); i += 2)
  - lun = lun | (((scsilun-scsi_lun[i]  8) |
  + lun = lun | (((scsilun-scsi_lun[i]  ((i + 1) *8)) |
  scsilun-scsi_lun[i + 1])  (i * 8));
return lun;
   }
 
  The above code doesn't match the comment header. Parentheses have been
  placed such that each byte with an even index is shifted left (2*i+1)*8
  bits instead of (i+1)*8.
  
  I don't see that in the code, which parentheses do you mean?
 
 This patch should change the code into what I think was intended by the
 comment above scsilun_to_int():
 
 --- a/drivers/scsi/scsi_scan.c
 +++ b/drivers/scsi/scsi_scan.c
 @@ -1280,8 +1280,8 @@ u64 scsilun_to_int(struct scsi_lun *scsilun)
  
   lun = 0;
   for (i = 0; i  sizeof(lun); i += 2)
 - lun = lun | (((scsilun-scsi_lun[i]  ((i + 1) *8)) |
 -   scsilun-scsi_lun[i + 1])  (i * 8));
 + lun = lun | (((u64)scsilun-scsi_lun[i]  ((i + 1) *8)) |
 +  ((u64)scsilun-scsi_lun[i + 1]  (i * 8)));
   return lun;
  }
  EXPORT_SYMBOL(scsilun_to_int);

So this is nothing to do with a wrong 2*i+1 step, which was your initial
complaint?  Now it's an arithmetic conversion problem (which looks
reasonable: on 32 bits, we'll do the shift at the natural size, which is
u32, so we'll overshift for i4.  If we're using sizeof(lun) in the for
loop, the converter should probably be typeof(lun) for consistency).

I don't see your second set of brackets being necessary bitwise or is
one of the lowest precedence non-logical operators; certainly it's lower
than shift:

http://en.cppreference.com/w/c/language/operator_precedence

James

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

Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[Stephen Rothwell]

Hi Michael,

On Tue, 10 Jun 2014 16:02:08 +0300 Michael S. Tsirkin m...@redhat.com wrote:

 I don't see how that's possible.
 Here's a point you might have missed.
 Nicholas's patch isn't just introducing a merge conflict.
 It is also buggy.

Ah ha!  Indeed that is a different kettle of fish. :-)

-- 
Cheers,
Stephen Rothwells...@canb.auug.org.au


signature.asc
Description: PGP signature

Re: [Open-FCoE] [PATCH] fc: ensure scan_work isn't active when freeing fc_rport

[Neil Horman]

On Tue, Jun 10, 2014 at 04:38:41AM -0700, Christoph Hellwig wrote:
 On Mon, Jun 09, 2014 at 03:16:37PM -0400, Neil Horman wrote:
   Given fcoe is quite mature now and its patches volume is very low, so
   getting its kernel patches directly to scsi subsystem should work fine
   and should be okay with James or Christophs to pull into scsi subsystem
   directly once I've my non-author signoff ACK there as described in this
   announcement at http://marc.info/?l=linux-scsim=140050839729415w=2 
   
   If no alternate suggestion or objection to this then I'll formally
   announce this on fcoe mailing list.
   
   However for any huges patches series bomb or RFCs, I'll request fcoe
   developers to send patches against scsi tree at fcoe devel list first
   and then if needed I can roll them up.
   
   Thanks,
   Vasu
   
  Copy that Vasu, Christoph, is that ok with you?
 
 That's fine with me.  It would help greatly if you could make sure
 all the paches get a review or two very quickly so I can just pick them
 up immediately after reviewing them.
 
 
Roger that, Vasu has already acked this.  I thought there was a second, but I'm
not sure, my mailbox is a bit messed up at the moment.

Sorry for not cc-ing you on this sooner, I thought it was going to go through
the FCoE tree initially

Best
Neil

--
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 6/6] scsi_scan: Fixup scsilun_to_int()

[Bart Van Assche]

On 06/10/14 17:01, James Bottomley wrote:
 On Tue, 2014-06-10 at 16:48 +0200, Bart Van Assche wrote:
 On 06/10/14 16:06, James Bottomley wrote:
 On Tue, 2014-06-10 at 13:37 +0200, Bart Van Assche wrote:
 On 06/03/14 10:58, Hannes Reinecke wrote:
 + * Given a struct scsi_lun of: d2 04 0b 03 00 00 00 00, this function
 + * returns the integer: 0x0b03d204
 + *
 + * This encoding will return a standard integer LUN for LUNs smaller
 + * than 256, which typically use a single level LUN structure with
 + * addressing method 0.
   **/
  u64 scsilun_to_int(struct scsi_lun *scsilun)
  {
 @@ -1279,7 +1280,7 @@ u64 scsilun_to_int(struct scsi_lun *scsilun)
  
   lun = 0;
   for (i = 0; i  sizeof(lun); i += 2)
 - lun = lun | (((scsilun-scsi_lun[i]  8) |
 + lun = lun | (((scsilun-scsi_lun[i]  ((i + 1) *8)) |
 scsilun-scsi_lun[i + 1])  (i * 8));
   return lun;
  }

 The above code doesn't match the comment header. Parentheses have been
 placed such that each byte with an even index is shifted left (2*i+1)*8
 bits instead of (i+1)*8.

 I don't see that in the code, which parentheses do you mean?

 This patch should change the code into what I think was intended by the
 comment above scsilun_to_int():

 --- a/drivers/scsi/scsi_scan.c
 +++ b/drivers/scsi/scsi_scan.c
 @@ -1280,8 +1280,8 @@ u64 scsilun_to_int(struct scsi_lun *scsilun)
  
  lun = 0;
  for (i = 0; i  sizeof(lun); i += 2)
 -lun = lun | (((scsilun-scsi_lun[i]  ((i + 1) *8)) |
 -  scsilun-scsi_lun[i + 1])  (i * 8));
 +lun = lun | (((u64)scsilun-scsi_lun[i]  ((i + 1) *8)) |
 + ((u64)scsilun-scsi_lun[i + 1]  (i * 8)));
  return lun;
  }
  EXPORT_SYMBOL(scsilun_to_int);
 
 So this is nothing to do with a wrong 2*i+1 step, which was your initial
 complaint?  Now it's an arithmetic conversion problem (which looks
 reasonable: on 32 bits, we'll do the shift at the natural size, which is
 u32, so we'll overshift for i4.  If we're using sizeof(lun) in the for
 loop, the converter should probably be typeof(lun) for consistency).
 
 I don't see your second set of brackets being necessary bitwise or is
 one of the lowest precedence non-logical operators; certainly it's lower
 than shift:
 
 http://en.cppreference.com/w/c/language/operator_precedence

Hello James,

In case you would be wondering why I made the above comments about
scsilun_to_int(): both issues - the misplaced parentheses and the
missing casts - were discovered by testing Hannes' patch series with an
LLD driver that had been modified to support 64-bit LUNs and by running
a test with LUN numbers above 2**32.

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

[Bug 77631] New: task scsi_eh_6:537 blocked for more than 120 seconds.

[bugzilla-daemon]

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

Bug ID: 77631
   Summary: task scsi_eh_6:537 blocked for more than 120 seconds.
   Product: IO/Storage
   Version: 2.5
Kernel Version: 3.14.6
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: SCSI
  Assignee: linux-scsi@vger.kernel.org
  Reporter: mikhail.v.gavri...@gmail.com
Regression: No

Created attachment 138941
  -- https://bugzilla.kernel.org/attachment.cgi?id=138941action=edit
kernel log

task scsi_eh_6:537 blocked for more than 120 seconds.


[  240.036323] INFO: task scsi_eh_6:537 blocked for more than 120 seconds.
[  240.036333]   Not tainted 3.14.6-200.fc20.x86_64+debug #1
[  240.036337] echo 0  /proc/sys/kernel/hung_task_timeout_secs disables this
message.
[  240.036341] scsi_eh_6   D 880813528000  6184   537  2 0x
[  240.036355]  8808027cdc50 0046 88007c988000
8808027cdfd8
[  240.036366]  001d5800 001d5800 88007c988000
880810ca6bf0
[  240.036375]  880810ca6bf8 0246 88007c988000
88080c532600
[  240.036384] Call Trace:
[  240.036400]  [817dafe1] schedule_preempt_disabled+0x31/0x80
[  240.036409]  [817dc204] mutex_lock_nested+0x194/0x430
[  240.036434]  [a035d9b4] ? device_reset+0x24/0x50 [usb_storage]
[  240.036450]  [a035d9b4] ? device_reset+0x24/0x50 [usb_storage]
[  240.036466]  [a035d9b4] device_reset+0x24/0x50 [usb_storage]
[  240.036478]  [81523f0a] scsi_try_bus_device_reset+0x2a/0x50
[  240.036487]  [8152601f] scsi_eh_ready_devs+0x4df/0xc20
[  240.036495]  [81527886] ? scsi_eh_get_sense+0x176/0x2a0
[  240.036504]  [81527eec] scsi_error_handler+0x53c/0x830
[  240.036512]  [815279b0] ? scsi_eh_get_sense+0x2a0/0x2a0
[  240.036522]  [810c452f] kthread+0xff/0x120
[  240.036530]  [810c4430] ? insert_kthread_work+0x80/0x80
[  240.036540]  [817eaafc] ret_from_fork+0x7c/0xb0
[  240.036547]  [810c4430] ? insert_kthread_work+0x80/0x80
[  240.036553] 1 lock held by scsi_eh_6/537:
[  240.036555]  #0:  (us_interface_key[i]){+.+.+.}, at: [a035d9b4]
device_reset+0x24/0x50 [usb_storage]
[  240.036575] INFO: task usb-storage:539 blocked for more than 120 seconds.
[  240.036579]   Not tainted 3.14.6-200.fc20.x86_64+debug #1
[  240.036582] echo 0  /proc/sys/kernel/hung_task_timeout_secs disables this
message.
[  240.036585] usb-storage D 88081351a590  5032   539  2 0x
[  240.036595]  8808025ebae0 0046 880800c28000
8808025ebfd8
[  240.036604]  001d5800 001d5800 880800c28000
7fff
[  240.036612]  880810ca6dd0 880810ca6dc8 880800c28000

[  240.036621] Call Trace:
[  240.036629]  [817daad9] schedule+0x29/0x70
[  240.036637]  [817d9a59] schedule_timeout+0x1f9/0x350
[  240.036648]  [810fb9e9] ? mark_held_locks+0xb9/0x140
[  240.036657]  [817dff8c] ? _raw_spin_unlock_irq+0x2c/0x40
[  240.036665]  [810fbb75] ? trace_hardirqs_on_caller+0x105/0x1d0
[  240.036673]  [817dc02c] wait_for_completion+0xfc/0x140
[  240.036684]  [810d9a50] ? wake_up_state+0x20/0x20
[  240.036692]  [815845fd] usb_sg_wait+0x12d/0x190
[  240.036709]  [a035e4db]
usb_stor_bulk_transfer_sglist.part.4+0x8b/0xf0 [usb_storage]
[  240.036725]  [a035e5b1] usb_stor_bulk_srb+0x71/0x80 [usb_storage]
[  240.036741]  [a035e71c] usb_stor_Bulk_transport+0x15c/0x360
[usb_storage]
[  240.036758]  [a035ef9b] usb_stor_invoke_transport+0x3b/0x570
[usb_storage]
[  240.036766]  [810fb9e9] ? mark_held_locks+0xb9/0x140
[  240.036774]  [817dff8c] ? _raw_spin_unlock_irq+0x2c/0x40
[  240.036790]  [a035dbce] usb_stor_transparent_scsi_command+0xe/0x10
[usb_storage]
[  240.036807]  [a035fe96] usb_stor_control_thread+0x176/0x290
[usb_storage]
[  240.036824]  [a035fd20] ? fill_inquiry_response+0x20/0x20
[usb_storage]
[  240.036840]  [a035fd20] ? fill_inquiry_response+0x20/0x20
[usb_storage]
[  240.036847]  [810c452f] kthread+0xff/0x120
[  240.036854]  [810c4430] ? insert_kthread_work+0x80/0x80
[  240.036862]  [817eaafc] ret_from_fork+0x7c/0xb0
[  240.036869]  [810c4430] ? insert_kthread_work+0x80/0x80
[  240.036873] 1 lock held by usb-storage/539:
[  240.036876]  #0:  (us_interface_key[i]){+.+.+.}, at: [a035fdaa]
usb_stor_control_thread+0x8a/0x290 [usb_storage]
[  359.971932] INFO: task scsi_eh_6:537 blocked for more than 120 seconds.
[  359.971935]   Not tainted 3.14.6-200.fc20.x86_64+debug #1
[  359.971936] echo 0  /proc/sys/kernel/hung_task_timeout_secs disables this
message.
[  359.971937] scsi_eh_6   D 880813528000  6184   537  

Re: [Bug 77631] New: task scsi_eh_6:537 blocked for more than 120 seconds.

[James Bottomley]

From the trace below, this looks to be a USB issue (USB added to cc):
the scsi error handler thread is waiting for usb storage to complete the
reset.  It's a 3.14.5 kernel, so the previous reset hang because of
spurious sense requests should be fixed.

James


On Tue, 2014-06-10 at 16:50 +, bugzilla-dae...@bugzilla.kernel.org
wrote:
 https://bugzilla.kernel.org/show_bug.cgi?id=77631
 
 Bug ID: 77631
Summary: task scsi_eh_6:537 blocked for more than 120 seconds.
Product: IO/Storage
Version: 2.5
 Kernel Version: 3.14.6
   Hardware: All
 OS: Linux
   Tree: Mainline
 Status: NEW
   Severity: normal
   Priority: P1
  Component: SCSI
   Assignee: linux-scsi@vger.kernel.org
   Reporter: mikhail.v.gavri...@gmail.com
 Regression: No
 
 Created attachment 138941
   -- https://bugzilla.kernel.org/attachment.cgi?id=138941action=edit
 kernel log
 
 task scsi_eh_6:537 blocked for more than 120 seconds.
 
 
 [  240.036323] INFO: task scsi_eh_6:537 blocked for more than 120 seconds.
 [  240.036333]   Not tainted 3.14.6-200.fc20.x86_64+debug #1
 [  240.036337] echo 0  /proc/sys/kernel/hung_task_timeout_secs disables 
 this
 message.
 [  240.036341] scsi_eh_6   D 880813528000  6184   537  2 
 0x
 [  240.036355]  8808027cdc50 0046 88007c988000
 8808027cdfd8
 [  240.036366]  001d5800 001d5800 88007c988000
 880810ca6bf0
 [  240.036375]  880810ca6bf8 0246 88007c988000
 88080c532600
 [  240.036384] Call Trace:
 [  240.036400]  [817dafe1] schedule_preempt_disabled+0x31/0x80
 [  240.036409]  [817dc204] mutex_lock_nested+0x194/0x430
 [  240.036434]  [a035d9b4] ? device_reset+0x24/0x50 [usb_storage]
 [  240.036450]  [a035d9b4] ? device_reset+0x24/0x50 [usb_storage]
 [  240.036466]  [a035d9b4] device_reset+0x24/0x50 [usb_storage]
 [  240.036478]  [81523f0a] scsi_try_bus_device_reset+0x2a/0x50
 [  240.036487]  [8152601f] scsi_eh_ready_devs+0x4df/0xc20
 [  240.036495]  [81527886] ? scsi_eh_get_sense+0x176/0x2a0
 [  240.036504]  [81527eec] scsi_error_handler+0x53c/0x830
 [  240.036512]  [815279b0] ? scsi_eh_get_sense+0x2a0/0x2a0
 [  240.036522]  [810c452f] kthread+0xff/0x120
 [  240.036530]  [810c4430] ? insert_kthread_work+0x80/0x80
 [  240.036540]  [817eaafc] ret_from_fork+0x7c/0xb0
 [  240.036547]  [810c4430] ? insert_kthread_work+0x80/0x80
 [  240.036553] 1 lock held by scsi_eh_6/537:
 [  240.036555]  #0:  (us_interface_key[i]){+.+.+.}, at: [a035d9b4]
 device_reset+0x24/0x50 [usb_storage]
 [  240.036575] INFO: task usb-storage:539 blocked for more than 120 seconds.
 [  240.036579]   Not tainted 3.14.6-200.fc20.x86_64+debug #1
 [  240.036582] echo 0  /proc/sys/kernel/hung_task_timeout_secs disables 
 this
 message.
 [  240.036585] usb-storage D 88081351a590  5032   539  2 
 0x
 [  240.036595]  8808025ebae0 0046 880800c28000
 8808025ebfd8
 [  240.036604]  001d5800 001d5800 880800c28000
 7fff
 [  240.036612]  880810ca6dd0 880810ca6dc8 880800c28000
 
 [  240.036621] Call Trace:
 [  240.036629]  [817daad9] schedule+0x29/0x70
 [  240.036637]  [817d9a59] schedule_timeout+0x1f9/0x350
 [  240.036648]  [810fb9e9] ? mark_held_locks+0xb9/0x140
 [  240.036657]  [817dff8c] ? _raw_spin_unlock_irq+0x2c/0x40
 [  240.036665]  [810fbb75] ? trace_hardirqs_on_caller+0x105/0x1d0
 [  240.036673]  [817dc02c] wait_for_completion+0xfc/0x140
 [  240.036684]  [810d9a50] ? wake_up_state+0x20/0x20
 [  240.036692]  [815845fd] usb_sg_wait+0x12d/0x190
 [  240.036709]  [a035e4db]
 usb_stor_bulk_transfer_sglist.part.4+0x8b/0xf0 [usb_storage]
 [  240.036725]  [a035e5b1] usb_stor_bulk_srb+0x71/0x80 [usb_storage]
 [  240.036741]  [a035e71c] usb_stor_Bulk_transport+0x15c/0x360
 [usb_storage]
 [  240.036758]  [a035ef9b] usb_stor_invoke_transport+0x3b/0x570
 [usb_storage]
 [  240.036766]  [810fb9e9] ? mark_held_locks+0xb9/0x140
 [  240.036774]  [817dff8c] ? _raw_spin_unlock_irq+0x2c/0x40
 [  240.036790]  [a035dbce] 
 usb_stor_transparent_scsi_command+0xe/0x10
 [usb_storage]
 [  240.036807]  [a035fe96] usb_stor_control_thread+0x176/0x290
 [usb_storage]
 [  240.036824]  [a035fd20] ? fill_inquiry_response+0x20/0x20
 [usb_storage]
 [  240.036840]  [a035fd20] ? fill_inquiry_response+0x20/0x20
 [usb_storage]
 [  240.036847]  [810c452f] kthread+0xff/0x120
 [  240.036854]  [810c4430] ? insert_kthread_work+0x80/0x80
 [  240.036862]  [817eaafc] ret_from_fork+0x7c/0xb0
 [  240.036869]  [810c4430] ? insert_kthread_work+0x80/0x80
 [  240.036873] 

Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[Nicholas A. Bellinger]

On Tue, 2014-06-10 at 16:02 +0300, Michael S. Tsirkin wrote:
 On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote:
  Hi Michael,
  
  On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin m...@redhat.com 
  wrote:
  
   So I see two options:
   - I go ahead with my changes and you with yours and let Linus resolve
 the conflict.  This means bisect build will be broken since the
 breakage will likely not be noticed until after the merge.
  
  Well, since the resolution is known, the one who submits their tree
  later should tell Linus (as suggested by Nicholas).  That is part of
  the point of the linux-next tree ... and therefore there would be no
  bisect problem.
  
Stephen (CC'ed) has included a fix in today's linux-next for the merge
conflict here:

https://lkml.org/lkml/2014/6/10/3

Please confirm, as it will be a pointer to Linus within the
target-pending/for-next PULL request.
   
   Yes but this does mean people trying to bisect will
   hit build breakages, not nice.
  
  Not necessarily.
  
  -- 
  Cheers,
  Stephen Rothwells...@canb.auug.org.au
 
 
 I don't see how that's possible.
 Here's a point you might have missed.
 Nicholas's patch isn't just introducing a merge conflict.
 It is also buggy.
 Replacing bit access with has_feature silently fixes the bug.
 
 So if we want to avoid bisect breakage target tree will
 have to be rebased.
 
 And if doing that anyway, I don't see any reason not
 to merge everything through the vhost tree, esp
 since I already put the patches there. Less work for
 everyone involved.
 

The problem is with Sagi's recent changes wrt to including T10 PI bytes
into expected data transfer length in target-core, you'll end up
introducing a different bug into your tree..  ;)

Why don't I simply add Stephen's patch to use vhost_has_feature() in
target-pending/for-next, and we just make sure that the vhost PULL
request goes out after target-pending..?

--nab

--
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: [PATCHv4 0/6] Support 64-bit LUNs

[Bart Van Assche]

On 06/03/14 10:58, Hannes Reinecke wrote:
 this patchset updates the SCSI stack to support full 64-bit LUNs.
 The first patch is a simple fix; the next patch updates
 the sequential scan logic to be compliant with SPC.
 The third patch addresses a firmware issue with earlier
 qla2xxx HBAs.
 The next two patches update the SCSI stack and all drivers
 to use 64-bit LUNs where appropriate.
 And finally we need to update the conversion routines
 scsilun_to_int to cope with 64bit LUNs.
 
 Two drivers have issues with 64bit LUNs:
 - The qla2xxx driver uses a 32-bit LUN value for TMFs.
   But as the driver uses a max_lun value from 0x
   we should be safe for the time being.
 - The zfcp driver uses a 32-bit LUN for debug records; the
   record format would need to be updated to cope with
   64-bit LUNs. But again, this driver uses 0x
   for max_lun, so it doesn't do any harm.
 
 The other changes have been pretty straightforward.

Hello Hannes,

Many SCSI LLD's use int_to_scsilun() in the hot path (queuecommand()).
This patch series makes the int_to_scsilun() function slightly more
expensive. Has it been considered to cache the result of int_to_scsilun()
such that LLD's can copy the cached int_to_scsilun() result instead of
having to call int_to_scsilun() in the queuecommand() function ?
Something like the (untested) patch below might be sufficient:

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index e02b3aa..9e50d78 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -244,6 +244,7 @@ static struct scsi_device *scsi_alloc_sdev(struct 
scsi_target *starget,
sdev-queue_ramp_up_period = SCSI_DEFAULT_RAMP_UP_PERIOD;
sdev-id = starget-id;
sdev-lun = lun;
+   int_to_scsilun(lun, sdev-scsi_lun);
sdev-channel = starget-channel;
sdev-sdev_state = SDEV_CREATED;
INIT_LIST_HEAD(sdev-siblings);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 5853c91..48ea68e 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -100,6 +100,8 @@ struct scsi_device {
 
unsigned int id, lun, channel;
 
+   struct scsi_lun scsi_lun;   /* int_to_scsilun(lun) */
+
unsigned int manufacturer;  /* Manufacturer of device, for using 
 * vendor-specific cmd's */
unsigned sector_size;   /* size in bytes */

Thanks,

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

Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[Michael S. Tsirkin]

On Tue, Jun 10, 2014 at 10:39:17AM -0700, Nicholas A. Bellinger wrote:
 On Tue, 2014-06-10 at 16:02 +0300, Michael S. Tsirkin wrote:
  On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote:
   Hi Michael,
   
   On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin m...@redhat.com 
   wrote:
   
So I see two options:
- I go ahead with my changes and you with yours and let Linus resolve
  the conflict.  This means bisect build will be broken since the
  breakage will likely not be noticed until after the merge.
   
   Well, since the resolution is known, the one who submits their tree
   later should tell Linus (as suggested by Nicholas).  That is part of
   the point of the linux-next tree ... and therefore there would be no
   bisect problem.
   
 Stephen (CC'ed) has included a fix in today's linux-next for the merge
 conflict here:
 
 https://lkml.org/lkml/2014/6/10/3
 
 Please confirm, as it will be a pointer to Linus within the
 target-pending/for-next PULL request.

Yes but this does mean people trying to bisect will
hit build breakages, not nice.
   
   Not necessarily.
   
   -- 
   Cheers,
   Stephen Rothwells...@canb.auug.org.au
  
  
  I don't see how that's possible.
  Here's a point you might have missed.
  Nicholas's patch isn't just introducing a merge conflict.
  It is also buggy.
  Replacing bit access with has_feature silently fixes the bug.
  
  So if we want to avoid bisect breakage target tree will
  have to be rebased.
  
  And if doing that anyway, I don't see any reason not
  to merge everything through the vhost tree, esp
  since I already put the patches there. Less work for
  everyone involved.
  
 
 The problem is with Sagi's recent changes wrt to including T10 PI bytes
 into expected data transfer length in target-core, you'll end up
 introducing a different bug into your tree..  ;)
 
 Why don't I simply add Stephen's patch to use vhost_has_feature() in
 target-pending/for-next, and we just make sure that the vhost PULL
 request goes out after target-pending..?
 
 --nab

Because that depends on vhost API changes :)

-- 
MST
--
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 v1 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

[Sagi Grimberg]

On 6/10/2014 10:02 PM, Martin K. Petersen wrote:

Sagi == Sagi Grimberg sa...@mellanox.com writes:

+static inline unsigned scsi_prot_length(unsigned data_length,
+   unsigned sector_size)
+{
+   switch (sector_size) {
+   case 512:
+   return (data_length  9) * 8;
+   case 1024:
+   return (data_length  10) * 8;
+   case 2048:
+   return (data_length  11) * 8;
+   case 4096:
+   return (data_length  12) * 8;
+   default:
+   return (data_length  ilog2(sector_size)) * 8;
+   }
+}
+
+static inline unsigned scsi_transfer_length(struct scsi_cmnd *cmd)
+{
+   unsigned data_length;
+
+   if (cmd-sc_data_direction == DMA_FROM_DEVICE) {
+   data_length = scsi_in(cmd)-length;
+   if (scsi_get_prot_op(cmd) ==  SCSI_PROT_NORMAL ||
+   scsi_get_prot_op(cmd) ==  SCSI_PROT_READ_INSERT)
+   return data_length;
+   } else {
+   data_length = scsi_out(cmd)-length;
+   if (scsi_get_prot_op(cmd) ==  SCSI_PROT_NORMAL ||
+   scsi_get_prot_op(cmd) ==  SCSI_PROT_WRITE_STRIP)
+   return data_length;
+   }
+
+   /* Protection information exists on the wire */
+   return data_length + scsi_prot_length(data_length,
+ cmd-device-sector_size);
+}

Let's do this for 3.16:

static inline unsigned int scsi_transfer_length(struct scsi_cmnd *scmd)
{
 unsigned int xfer_len = blk_rq_bytes(scmd-request);
 unsigned int prot_op = scsi_get_prot_op(scmd);

 switch (prot_op) {
 case SCSI_PROT_NORMAL:
 case SCSI_PROT_WRITE_STRIP:
 case SCSI_PROT_READ_INSERT:
  return xfer_len;
 }

 return xfer_len + (xfer_len  ilog2(sector_size)) * 8;
}

And then in 3.17 we'll have:

static inline unsigned int scsi_transfer_length(struct scsi_cmnd *scmd)
{
 unsigned int xfer_len = blk_rq_bytes(scmd-request);
  
 if (scsi_prot_flagged(SCSI_PROT_TRANSFER_PI))

xfer_len += (xfer_len  ilog2(scsi_prot_interval(scmd))) * 8;

 return xfer_len;
}



No problem,

I'll send out v2 tomorrow (your tonight...)

Thanks,
Sagi.
--
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 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[Michael S. Tsirkin]

On Tue, Jun 10, 2014 at 10:39:17AM -0700, Nicholas A. Bellinger wrote:
 On Tue, 2014-06-10 at 16:02 +0300, Michael S. Tsirkin wrote:
  On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote:
   Hi Michael,
   
   On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin m...@redhat.com 
   wrote:
   
So I see two options:
- I go ahead with my changes and you with yours and let Linus resolve
  the conflict.  This means bisect build will be broken since the
  breakage will likely not be noticed until after the merge.
   
   Well, since the resolution is known, the one who submits their tree
   later should tell Linus (as suggested by Nicholas).  That is part of
   the point of the linux-next tree ... and therefore there would be no
   bisect problem.
   
 Stephen (CC'ed) has included a fix in today's linux-next for the merge
 conflict here:
 
 https://lkml.org/lkml/2014/6/10/3
 
 Please confirm, as it will be a pointer to Linus within the
 target-pending/for-next PULL request.

Yes but this does mean people trying to bisect will
hit build breakages, not nice.
   
   Not necessarily.
   
   -- 
   Cheers,
   Stephen Rothwells...@canb.auug.org.au
  
  
  I don't see how that's possible.
  Here's a point you might have missed.
  Nicholas's patch isn't just introducing a merge conflict.
  It is also buggy.
  Replacing bit access with has_feature silently fixes the bug.
  
  So if we want to avoid bisect breakage target tree will
  have to be rebased.
  
  And if doing that anyway, I don't see any reason not
  to merge everything through the vhost tree, esp
  since I already put the patches there. Less work for
  everyone involved.
  
 
 The problem is with Sagi's recent changes wrt to including T10 PI bytes
 into expected data transfer length in target-core, you'll end up
 introducing a different bug into your tree..  ;)

I thought you wanted to fix it after -rc1 anyway?

 Why don't I simply add Stephen's patch to use vhost_has_feature() in
 target-pending/for-next, and we just make sure that the vhost PULL
 request goes out after target-pending..?
 
 --nab

I can drop the PI feature bit in rc1. You will apply Sagi's changes and
then enable the feature in rc2?
--
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 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[Nicholas A. Bellinger]

On Tue, 2014-06-10 at 21:45 +0300, Michael S. Tsirkin wrote:
 On Tue, Jun 10, 2014 at 10:39:17AM -0700, Nicholas A. Bellinger wrote:
  On Tue, 2014-06-10 at 16:02 +0300, Michael S. Tsirkin wrote:
   On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote:
Hi Michael,

On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin 
m...@redhat.com wrote:

 So I see two options:
 - I go ahead with my changes and you with yours and let Linus resolve
   the conflict.  This means bisect build will be broken since the
   breakage will likely not be noticed until after the merge.

Well, since the resolution is known, the one who submits their tree
later should tell Linus (as suggested by Nicholas).  That is part of
the point of the linux-next tree ... and therefore there would be no
bisect problem.

  Stephen (CC'ed) has included a fix in today's linux-next for the 
  merge
  conflict here:
  
  https://lkml.org/lkml/2014/6/10/3
  
  Please confirm, as it will be a pointer to Linus within the
  target-pending/for-next PULL request.
 
 Yes but this does mean people trying to bisect will
 hit build breakages, not nice.

Not necessarily.

-- 
Cheers,
Stephen Rothwells...@canb.auug.org.au
   
   
   I don't see how that's possible.
   Here's a point you might have missed.
   Nicholas's patch isn't just introducing a merge conflict.
   It is also buggy.
   Replacing bit access with has_feature silently fixes the bug.
   
   So if we want to avoid bisect breakage target tree will
   have to be rebased.
   
   And if doing that anyway, I don't see any reason not
   to merge everything through the vhost tree, esp
   since I already put the patches there. Less work for
   everyone involved.
   
  
  The problem is with Sagi's recent changes wrt to including T10 PI bytes
  into expected data transfer length in target-core, you'll end up
  introducing a different bug into your tree..  ;)
  
  Why don't I simply add Stephen's patch to use vhost_has_feature() in
  target-pending/for-next, and we just make sure that the vhost PULL
  request goes out after target-pending..?
  
  --nab
 
 Because that depends on vhost API changes :)
 

Ugh, right.

In that case, let's see what Linus (CC'ed) prefers to do..

--nab

--
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 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[Nicholas A. Bellinger]

On Tue, 2014-06-10 at 22:35 +0300, Michael S. Tsirkin wrote:
 On Tue, Jun 10, 2014 at 10:39:17AM -0700, Nicholas A. Bellinger wrote:
  On Tue, 2014-06-10 at 16:02 +0300, Michael S. Tsirkin wrote:
   On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote:
Hi Michael,

On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin 
m...@redhat.com wrote:

 So I see two options:
 - I go ahead with my changes and you with yours and let Linus resolve
   the conflict.  This means bisect build will be broken since the
   breakage will likely not be noticed until after the merge.

Well, since the resolution is known, the one who submits their tree
later should tell Linus (as suggested by Nicholas).  That is part of
the point of the linux-next tree ... and therefore there would be no
bisect problem.

  Stephen (CC'ed) has included a fix in today's linux-next for the 
  merge
  conflict here:
  
  https://lkml.org/lkml/2014/6/10/3
  
  Please confirm, as it will be a pointer to Linus within the
  target-pending/for-next PULL request.
 
 Yes but this does mean people trying to bisect will
 hit build breakages, not nice.

Not necessarily.

-- 
Cheers,
Stephen Rothwells...@canb.auug.org.au
   
   
   I don't see how that's possible.
   Here's a point you might have missed.
   Nicholas's patch isn't just introducing a merge conflict.
   It is also buggy.
   Replacing bit access with has_feature silently fixes the bug.
   
   So if we want to avoid bisect breakage target tree will
   have to be rebased.
   
   And if doing that anyway, I don't see any reason not
   to merge everything through the vhost tree, esp
   since I already put the patches there. Less work for
   everyone involved.
   
  
  The problem is with Sagi's recent changes wrt to including T10 PI bytes
  into expected data transfer length in target-core, you'll end up
  introducing a different bug into your tree..  ;)
 
 I thought you wanted to fix it after -rc1 anyway?
 

I've merged Sagi's patches and fixed up the resulting vhost-scsi
breakage in target-pending/for-next here:

https://git.kernel.org/cgit/linux/kernel/git/nab/target-pending.git/commit/?h=for-nextid=4101fc0abffeef604b14a707a3f9c9e2a8e39085

Only the qla2xxx target DIF related breakage remains, because those
patches are going through the scsi/for-next tree and will need to be
fixed in -rc2.

  Why don't I simply add Stephen's patch to use vhost_has_feature() in
  target-pending/for-next, and we just make sure that the vhost PULL
  request goes out after target-pending..?
  
  --nab
 
 I can drop the PI feature bit in rc1. You will apply Sagi's changes and
 then enable the feature in rc2?

That is not necessary.  I'll just apply Stephen's patch to use
vhost_has_feature(), and then we make sure that target-pending is merged
before vhost to avoid the build breakage.

I'll be sending out the target-pending PULL request tomorrow afternoon
and will CC' you.

--nab

--
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 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[James Bottomley]

On Tue, 2014-06-10 at 12:57 -0700, Nicholas A. Bellinger wrote:
 On Tue, 2014-06-10 at 21:45 +0300, Michael S. Tsirkin wrote:
  On Tue, Jun 10, 2014 at 10:39:17AM -0700, Nicholas A. Bellinger wrote:
   On Tue, 2014-06-10 at 16:02 +0300, Michael S. Tsirkin wrote:
On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote:
 Hi Michael,
 
 On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin 
 m...@redhat.com wrote:
 
  So I see two options:
  - I go ahead with my changes and you with yours and let Linus 
  resolve
the conflict.  This means bisect build will be broken since the
breakage will likely not be noticed until after the merge.
 
 Well, since the resolution is known, the one who submits their tree
 later should tell Linus (as suggested by Nicholas).  That is part of
 the point of the linux-next tree ... and therefore there would be no
 bisect problem.
 
   Stephen (CC'ed) has included a fix in today's linux-next for the 
   merge
   conflict here:
   
   https://lkml.org/lkml/2014/6/10/3
   
   Please confirm, as it will be a pointer to Linus within the
   target-pending/for-next PULL request.
  
  Yes but this does mean people trying to bisect will
  hit build breakages, not nice.
 
 Not necessarily.
 
 -- 
 Cheers,
 Stephen Rothwells...@canb.auug.org.au


I don't see how that's possible.
Here's a point you might have missed.
Nicholas's patch isn't just introducing a merge conflict.
It is also buggy.
Replacing bit access with has_feature silently fixes the bug.

So if we want to avoid bisect breakage target tree will
have to be rebased.

And if doing that anyway, I don't see any reason not
to merge everything through the vhost tree, esp
since I already put the patches there. Less work for
everyone involved.

   
   The problem is with Sagi's recent changes wrt to including T10 PI bytes
   into expected data transfer length in target-core, you'll end up
   introducing a different bug into your tree..  ;)
   
   Why don't I simply add Stephen's patch to use vhost_has_feature() in
   target-pending/for-next, and we just make sure that the vhost PULL
   request goes out after target-pending..?
   
   --nab
  
  Because that depends on vhost API changes :)
  
 
 Ugh, right.
 
 In that case, let's see what Linus (CC'ed) prefers to do..

Build a branch with all the patches dependent on the new API based on
top of the vhost tree.  Then you send it to Linus after the vhost tree
is merged (otherwise you end up being the person sending the vhost
tree).

That way there's no API breakage and we get a consistent always
buildable tree.

James

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

Re: [PATCH v1 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

[Or Gerlitz]

On Tue, Jun 10, 2014 at 10:02 PM, Martin K. Petersen
martin.peter...@oracle.com wrote:
 Sagi == Sagi Grimberg sa...@mellanox.com writes:

 +static inline unsigned scsi_prot_length(unsigned data_length,
 +   unsigned sector_size)
 +{
 +   switch (sector_size) {
 +   case 512:
 +   return (data_length  9) * 8;
 +   case 1024:
 +   return (data_length  10) * 8;
 +   case 2048:
 +   return (data_length  11) * 8;
 +   case 4096:
 +   return (data_length  12) * 8;
 +   default:
 +   return (data_length  ilog2(sector_size)) * 8;
 +   }
 +}
 +
 +static inline unsigned scsi_transfer_length(struct scsi_cmnd *cmd)
 +{
 +   unsigned data_length;
 +
 +   if (cmd-sc_data_direction == DMA_FROM_DEVICE) {
 +   data_length = scsi_in(cmd)-length;
 +   if (scsi_get_prot_op(cmd) ==  SCSI_PROT_NORMAL ||
 +   scsi_get_prot_op(cmd) ==  SCSI_PROT_READ_INSERT)
 +   return data_length;
 +   } else {
 +   data_length = scsi_out(cmd)-length;
 +   if (scsi_get_prot_op(cmd) ==  SCSI_PROT_NORMAL ||
 +   scsi_get_prot_op(cmd) ==  SCSI_PROT_WRITE_STRIP)
 +   return data_length;
 +   }
 +
 +   /* Protection information exists on the wire */
 +   return data_length + scsi_prot_length(data_length,
 + cmd-device-sector_size);
 +}

 Let's do this for 3.16:

Just to make sure, by 3.16 you also mean 3.15.y, right?
--
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 v1 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

[Martin K. Petersen]

 Or == Or Gerlitz or.gerl...@gmail.com writes:

Or Just to make sure, by 3.16 you also mean 3.15.y, right?

Yes.

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

Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[Nicholas A. Bellinger]

On Tue, 2014-06-10 at 13:09 -0700, James Bottomley wrote:
 On Tue, 2014-06-10 at 12:57 -0700, Nicholas A. Bellinger wrote:
  On Tue, 2014-06-10 at 21:45 +0300, Michael S. Tsirkin wrote:
   On Tue, Jun 10, 2014 at 10:39:17AM -0700, Nicholas A. Bellinger wrote:
On Tue, 2014-06-10 at 16:02 +0300, Michael S. Tsirkin wrote:
 On Tue, Jun 10, 2014 at 09:52:17PM +1000, Stephen Rothwell wrote:
  Hi Michael,
  
  On Tue, 10 Jun 2014 12:42:54 +0300 Michael S. Tsirkin 
  m...@redhat.com wrote:
  
   So I see two options:
   - I go ahead with my changes and you with yours and let Linus 
   resolve
 the conflict.  This means bisect build will be broken since the
 breakage will likely not be noticed until after the merge.
  
  Well, since the resolution is known, the one who submits their tree
  later should tell Linus (as suggested by Nicholas).  That is part of
  the point of the linux-next tree ... and therefore there would be no
  bisect problem.
  
Stephen (CC'ed) has included a fix in today's linux-next for 
the merge
conflict here:

https://lkml.org/lkml/2014/6/10/3

Please confirm, as it will be a pointer to Linus within the
target-pending/for-next PULL request.
   
   Yes but this does mean people trying to bisect will
   hit build breakages, not nice.
  
  Not necessarily.
  
  -- 
  Cheers,
  Stephen Rothwells...@canb.auug.org.au
 
 
 I don't see how that's possible.
 Here's a point you might have missed.
 Nicholas's patch isn't just introducing a merge conflict.
 It is also buggy.
 Replacing bit access with has_feature silently fixes the bug.
 
 So if we want to avoid bisect breakage target tree will
 have to be rebased.
 
 And if doing that anyway, I don't see any reason not
 to merge everything through the vhost tree, esp
 since I already put the patches there. Less work for
 everyone involved.
 

The problem is with Sagi's recent changes wrt to including T10 PI bytes
into expected data transfer length in target-core, you'll end up
introducing a different bug into your tree..  ;)

Why don't I simply add Stephen's patch to use vhost_has_feature() in
target-pending/for-next, and we just make sure that the vhost PULL
request goes out after target-pending..?

--nab
   
   Because that depends on vhost API changes :)
   
  
  Ugh, right.
  
  In that case, let's see what Linus (CC'ed) prefers to do..
 
 Build a branch with all the patches dependent on the new API based on
 top of the vhost tree.  Then you send it to Linus after the vhost tree
 is merged (otherwise you end up being the person sending the vhost
 tree).
 
 That way there's no API breakage and we get a consistent always
 buildable tree.
 

That would work, or I can simply include a pointer to Stephen's patch in
the target-pending PULL request after the vhost API changes are merged
and Linus can apply himself..

Linus, which do you prefer..?

--nab

--
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

Debug flag parameter for SCSI tape driver

[Laurence Oberman]

Hello

I am tired of building modules to enable SCSI tape driver debug so I am hoping 
this patch is acceptable.
Tested using kernel 3.14.6

Usage example:
modprobe st debug_flag=1

diff -Nur a/st.c b/st.c
--- a/st.c  2014-06-10 16:45:18.522354105 -0400
+++ b/st.c  2014-06-10 16:45:33.953765908 -0400
@@ -80,6 +80,7 @@
 static int try_direct_io = TRY_DIRECT_IO;
 static int try_rdio = 1;
 static int try_wdio = 1;
+static int debug_flag = 0;
 
 static struct class st_sysfs_class;
 static const struct attribute_group *st_dev_groups[];
@@ -100,6 +101,9 @@
 MODULE_PARM_DESC(max_sg_segs, Maximum number of scatter/gather segments to 
use (256));
 module_param_named(try_direct_io, try_direct_io, int, 0);
 MODULE_PARM_DESC(try_direct_io, Try direct I/O between user buffer and tape 
drive (1));
+module_param_named(debug_flag, debug_flag, int, 0);
+MODULE_PARM_DESC(debug_flag, Enable DEBUG, same as setting DEBUG 1 in 
source);
+
 
 /* Extra parameters for testing */
 module_param_named(try_rdio, try_rdio, int, 0);
@@ -124,6 +128,9 @@
},
{
try_direct_io, try_direct_io
+   },
+   {
+   debug_flag, debug_flag
}
 };
 #endif
@@ -4277,7 +4284,9 @@
 static int __init init_st(void)
 {
int err;
-
+   debugging = (debug_flag  0) ? debug_flag : DEBUG;
+if (debugging) 
+   printk(KERN_INFO st: Debugging enabled debug_flag = 
%d\n,debugging);
validate_options();
 
printk(KERN_INFO st: Version %s, fixed bufsize %d, s/g segs %d\n,

Thanks
Laurence
--
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

[Bug 77631] task scsi_eh_6:537 blocked for more than 120 seconds.

[bugzilla-daemon]

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

--- Comment #1 from Alan Stern st...@rowland.harvard.edu ---
On Tue, 10 Jun 2014, James Bottomley wrote:

 From the trace below, this looks to be a USB issue (USB added to cc):
 the scsi error handler thread is waiting for usb storage to complete the
 reset.  It's a 3.14.5 kernel, so the previous reset hang because of
 spurious sense requests should be fixed.
 
 James
 
 
 On Tue, 2014-06-10 at 16:50 +, bugzilla-dae...@bugzilla.kernel.org
 wrote:
  https://bugzilla.kernel.org/show_bug.cgi?id=77631
  
  Bug ID: 77631
 Summary: task scsi_eh_6:537 blocked for more than 120 seconds.
 Product: IO/Storage
 Version: 2.5
  Kernel Version: 3.14.6
Hardware: All
  OS: Linux
Tree: Mainline
  Status: NEW
Severity: normal
Priority: P1
   Component: SCSI
Assignee: linux-scsi@vger.kernel.org
Reporter: mikhail.v.gavri...@gmail.com
  Regression: No
  
  Created attachment 138941
-- https://bugzilla.kernel.org/attachment.cgi?id=138941action=edit
  kernel log
  
  task scsi_eh_6:537 blocked for more than 120 seconds.

This line from earlier in the kernel log is highly suspicious:

 [   46.330470] usb-storage: Error in queuecommand_lck: us-srb = 
 88080c532600

This means that the SCSI midlayer called the queuecommand routine twice 
with no intervening completion, even though the maximum queue depth is 
only 1.

Mikhail, is this bug reproducible?  If it is, can you post a usbmon 
trace showing the events leading up to the bug?

Does the bug occur if the device is plugged into a USB-2 bus rather 
than a USB-3 bus?

Alan Stern

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

Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[Linus Torvalds]

On Tue, Jun 10, 2014 at 1:25 PM, Nicholas A. Bellinger
n...@linux-iscsi.org wrote:

 That would work, or I can simply include a pointer to Stephen's patch in
 the target-pending PULL request after the vhost API changes are merged
 and Linus can apply himself..

Yes. That way I'll include it in the merge, and everything should just work.

  Linus
--
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

debug_flag added to st tape driver

[Laurence Oberman]

Hello

I am tired of building modules to enable SCSI tape driver debug so I
am hoping this patch is acceptable.
Tested using kernel 3.14.6

Usage example:
modprobe st debug_flag=1

diff -Nur a/st.c b/st.c
--- a/st.c2014-06-10 16:45:18.522354105 -0400
+++ b/st.c2014-06-10 16:45:33.953765908 -0400
@@ -80,6 +80,7 @@
 static int try_direct_io = TRY_DIRECT_IO;
 static int try_rdio = 1;
 static int try_wdio = 1;
+static int debug_flag = 0;

 static struct class st_sysfs_class;
 static const struct attribute_group *st_dev_groups[];
@@ -100,6 +101,9 @@
 MODULE_PARM_DESC(max_sg_segs, Maximum number of scatter/gather
segments to use (256));
 module_param_named(try_direct_io, try_direct_io, int, 0);
 MODULE_PARM_DESC(try_direct_io, Try direct I/O between user buffer
and tape drive (1));
+module_param_named(debug_flag, debug_flag, int, 0);
+MODULE_PARM_DESC(debug_flag, Enable DEBUG, same as setting DEBUG 1
in source);
+

 /* Extra parameters for testing */
 module_param_named(try_rdio, try_rdio, int, 0);
@@ -124,6 +128,9 @@
 },
 {
 try_direct_io, try_direct_io
+},
+{
+debug_flag, debug_flag
 }
 };
 #endif
@@ -4277,7 +4284,9 @@
 static int __init init_st(void)
 {
 int err;
-
+debugging = (debug_flag  0) ? debug_flag : DEBUG;
+if (debugging)
+printk(KERN_INFO st: Debugging enabled debug_flag =
%d\n,debugging);
 validate_options();

 printk(KERN_INFO st: Version %s, fixed bufsize %d, s/g segs %d\n,

Thanks
Laurence
--
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 v1 3/3] TARGET/sbc,loopback: Adjust command data length in case pi exists on the wire

[Quinn Tran]

All,

Comments inline.

Regards,
Quinn Tran




On 6/10/14 1:04 AM, Nicholas A. Bellinger n...@linux-iscsi.org wrote:

Hi Sagi  Co,

On Sun, 2014-06-08 at 13:27 +0300, Sagi Grimberg wrote:
 In various areas of the code, it is assumed that
 se_cmd-data_length describes pure data. In case
 that protection information exists over the wire
 (protect bits is are on) the target core decrease
 the protection length from the data length (instead
 of each transport peeking in the cdb).

 Modify loopback device to include protection information
 in the transferred data length (like other scsi transports).

 Signed-off-by: Sagi Grimberg sa...@mellanox.com
 ---
  drivers/target/loopback/tcm_loop.c |   15 ---
  drivers/target/target_core_sbc.c   |   15 +--
  2 files changed, 25 insertions(+), 5 deletions(-)

 diff --git a/drivers/target/loopback/tcm_loop.c
b/drivers/target/loopback/tcm_loop.c
 index c886ad1..1f4c015 100644
 --- a/drivers/target/loopback/tcm_loop.c
 +++ b/drivers/target/loopback/tcm_loop.c
 @@ -179,7 +179,7 @@ static void tcm_loop_submission_work(struct
work_struct *work)
  struct tcm_loop_hba *tl_hba;
  struct tcm_loop_tpg *tl_tpg;
  struct scatterlist *sgl_bidi = NULL;
 -u32 sgl_bidi_count = 0;
 +u32 sgl_bidi_count = 0, transfer_length;
  int rc;

  tl_hba = *(struct tcm_loop_hba **)shost_priv(sc-device-host);
 @@ -213,12 +213,21 @@ static void tcm_loop_submission_work(struct
work_struct *work)

  }

 -if (!scsi_prot_sg_count(sc)  scsi_get_prot_op(sc) !=
SCSI_PROT_NORMAL)
 +transfer_length = scsi_transfer_length(sc);
 +if (!scsi_prot_sg_count(sc) 
 +scsi_get_prot_op(sc) != SCSI_PROT_NORMAL) {
  se_cmd-prot_pto = true;
 +/*
 + * loopback transport doesn't support
 + * WRITE_GENERATE, READ_STRIP protection
 + * information operations, go ahead unprotected.
 + */
 +transfer_length = scsi_bufflen(sc);
 +}

  rc = target_submit_cmd_map_sgls(se_cmd, tl_nexus-se_sess, sc-cmnd,
  tl_cmd-tl_sense_buf[0], tl_cmd-sc-device-lun,
 -scsi_bufflen(sc), tcm_loop_sam_attr(sc),
 +transfer_length, tcm_loop_sam_attr(sc),
  sc-sc_data_direction, 0,
  scsi_sglist(sc), scsi_sg_count(sc),
  sgl_bidi, sgl_bidi_count,
 diff --git a/drivers/target/target_core_sbc.c
b/drivers/target/target_core_sbc.c
 index e022959..06f8ecd 100644
 --- a/drivers/target/target_core_sbc.c
 +++ b/drivers/target/target_core_sbc.c
 @@ -665,8 +665,19 @@ sbc_check_prot(struct se_device *dev, struct
se_cmd *cmd, unsigned char *cdb,

  cmd-prot_type = dev-dev_attrib.pi_prot_type;
  cmd-prot_length = dev-prot_length * sectors;
 -pr_debug(%s: prot_type=%d, prot_length=%d prot_op=%d
prot_checks=%d\n,
 - __func__, cmd-prot_type, cmd-prot_length,
 +
 +/**
 + * In case protection information exists over the wire
 + * we modify command data length to describe pure data.
 + * The actual transfer length is data length + protection
 + * length
 + **/
 +if (protect)
 +cmd-data_length -= cmd-prot_length;
 +

QT Instead of using existing value within cmd-data_length, can we
calculated data_length based on secstors  blocksize.

cmd-data_length = sectors * dev-dev_attrib.block_size;


From the QLogic perfective, the cmd-data_length is pull directly from the
wire (i.e. FCP header) when cmd is received.  In essence, it's whatever
the Initiator is set it to.  We does not have any indicator to spot DIF vs
none-DIF cmd when 1st received, unless the code do a peek.

With that said, the cmd-data_length does not guarantee to contain both
data length  protection length when cmd is submit to
TCM/target_submit_cmd().  In Dif-Insert mode, data_length will only
contain the actual data(no Dif).

It's best that the SBC code re-calculate the actual data length and dif
data length based on the number of sectors derived from the cmd.

Thanks.


This fabric wide assumption breaks the vhost-scsi + tcm_qla2xxx PI code
already queued for v3.16-rc1..

A vhost-scsi side conversion to combine exp_data_len + prot_bytes is
easy enough in target-pending/for-next (see below), given that
vhost-scsi's exp_data_len is coming from virtio buffer iovecs, so
changing virtio-scsi LLD to use scsi_transfer_length() doesn't really
make any sense. (Adding CC's for MST + Paolo)

The qla2xxx target T10 PI patch in scsi/for-next (adding Quinn CC) will
also need a similar change to update qlt_do_work() to include PI bytes
for data_length being passed into tgt_ops-handle_cmd(), following what
qlt_build_ctio_crc2_pkt() is already doing for calculating
transfer_length for HW offload.

That said, there is one other small qla2xxx change to enable per-session
PI that is currently missing in Quinn's patch in scsi/for-next code.

QT I've been bog down with other front, I have 

Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support

[Nicholas A. Bellinger]

On Tue, 2014-06-10 at 13:56 -0700, Linus Torvalds wrote:
 On Tue, Jun 10, 2014 at 1:25 PM, Nicholas A. Bellinger
 n...@linux-iscsi.org wrote:
 
  That would work, or I can simply include a pointer to Stephen's patch in
  the target-pending PULL request after the vhost API changes are merged
  and Linus can apply himself..
 
 Yes. That way I'll include it in the merge, and everything should just work.
 

nod, sounds good.

MST, please drop the target related patches from your tree, and go ahead
and send your PULL request now.

--nab

--
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] bnx2fc: Do not log error for netevents that need no action

[Eddie Wai]

On Fri, 2014-06-06 at 13:05 -0500, shirishpargaon...@gmail.com wrote:
 From: Shirish Pargaonkar shirishpargaon...@gmail.com
 
 
 Do not log error for netevents that need no action such as
 NETDEV_REGISTER 0x0005, NETDEV_CHANGEADDR, and NETDEV_CHANGENAME.
 It results in logging error messages such as these
 
 [   35.315872] bnx2fc: Unknown netevent 5
 [   35.315935] bnx2fc: Unknown netevent 8
 [   35.353866] bnx2fc: Unknown netevent 10
 
 and generating bug reports.
 Remove logging this message as an ERROR instead of turning them into
 either DEBUG or INFO level messages.
 
 
 Signed-by: Shirish Pargaonkar spargaon...@suse.com
 ---
  drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 1 -
  1 file changed, 1 deletion(-)
 
 diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c 
 b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
 index 1d41f4b..c7388a8 100644
 --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
 +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
 @@ -856,7 +856,6 @@ static void bnx2fc_indicate_netevent(void *context, 
 unsigned long event,
   return;
  
   default:
 - printk(KERN_ERR PFX Unknown netevent %ld, event);
   return;
   }
  
Thanks for fixing this, Shirish.  It makes sense to suppress the
unwanted netevent messages.

The patch looks good to me, but we should really get the current bnx2fc
maintainer from Qlogic to ACK this.

Acked-by: Eddie Wai eddie@broadcom.com


--
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

[Bug 77631] task scsi_eh_6:537 blocked for more than 120 seconds.

[bugzilla-daemon]

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

--- Comment #2 from Mikhail mikhail.v.gavri...@gmail.com ---
This occurred when I did first reboot after kernel update from 3.14.5 to
3.14.6.
After second reboot I have not seen this issue.
And I did not plug/unplug any USB devices when it occurred.

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

Re: debug_flag added to st tape driver

[Bryn M. Reeves]

On Tue, Jun 10, 2014 at 04:57:06PM -0400, Laurence Oberman wrote:
 I am tired of building modules to enable SCSI tape driver debug so I
 am hoping this patch is acceptable.
 Tested using kernel 3.14.6
 
 Usage example:
 modprobe st debug_flag=1

Missing Signed-off-by :-)
 
 +module_param_named(debug_flag, debug_flag, int, 0);

It's probably not worth making this a sysfs knob as most distros
still compile st as a module although why not just set debugging
directly from the module parameter?

 +MODULE_PARM_DESC(debug_flag, Enable DEBUG, same as setting DEBUG 1
 in source);
 +

The description is a bit misleading as a bunch of stuff gets
compiled out when DEBUG is unset at compile time.

Maybe same as setting debugging=1 instead?

--
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

[Bug 77631] task scsi_eh_6:537 blocked for more than 120 seconds.

[bugzilla-daemon]

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

--- Comment #3 from Mikhail mikhail.v.gavri...@gmail.com ---
When I reconnect my USB3.0 10 port hub only occurs this:

[  835.657491] usb 4-3: USB disconnect, device number 3
[  835.657506] usb 4-3.4: USB disconnect, device number 11
[  835.658118] usb 4-3.4: Failed to set U1 timeout to 0x0,error code -19
[  835.658134] usb 4-3.4: Failed to set U1 timeout to 0x43,error code -19
[  835.658146] usb 4-3.4: Failed to set U2 timeout to 0x28,error code -19
[  835.694393] usb 4-3.4: Failed to set U1 timeout to 0x0,error code -19
[  835.694434] usb 4-3.4: Failed to set U1 timeout to 0x29,error code -19
[  835.694440] usb 4-3.4: Failed to set U2 timeout to 0x28,error code -19
[  835.802188] usb 3-3: USB disconnect, device number 11
[  835.802191] usb 3-3.1: USB disconnect, device number 12
[  835.802193] usb 3-3.1.3: USB disconnect, device number 14
[  835.868161] usb 3-3.1.4: USB disconnect, device number 15
[  835.875955] usb 3-3.1.6: USB disconnect, device number 16
[  835.914734] usb 3-3.3: USB disconnect, device number 13
[  841.411016] usb 3-7: USB disconnect, device number 3
[  859.635511] usb 3-7: new high-speed USB device number 20 using xhci_hcd
[  859.800823] usb 3-7: New USB device found, idVendor=2109, idProduct=2812
[  859.800833] usb 3-7: New USB device strings: Mfr=0, Product=1,
SerialNumber=0
[  859.800838] usb 3-7: Product: USB 2.0 HUB

[  859.803390] hub 3-7:1.0: USB hub found
[  859.803648] hub 3-7:1.0: 4 ports detected
[  860.088189] usb 3-7.1: new high-speed USB device number 21 using xhci_hcd
[  860.174264] usb 3-7.1: New USB device found, idVendor=1a40, idProduct=0201
[  860.174266] usb 3-7.1: New USB device strings: Mfr=0, Product=1,
SerialNumber=0
[  860.174268] usb 3-7.1: Product: USB 2.0 Hub [MTT]
[  860.174915] hub 3-7.1:1.0: USB hub found
[  860.174941] hub 3-7.1:1.0: 7 ports detected
[  860.251194] usb 3-7.3: new high-speed USB device number 22 using xhci_hcd
[  860.338906] usb 3-7.3: New USB device found, idVendor=18d1, idProduct=4ee1
[  860.338908] usb 3-7.3: New USB device strings: Mfr=1, Product=2,
SerialNumber=3
[  860.338909] usb 3-7.3: Product: Nexus 5
[  860.338910] usb 3-7.3: Manufacturer: LGE
[  860.338911] usb 3-7.3: SerialNumber: 07606f5400e1b31a
[  860.375934] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.375967] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.376052] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.376646] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.376686] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.376723] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.376763] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.376803] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.376906] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.376942] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.377140] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.377215] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.377254] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.377369] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.377451] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.377494] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.377534] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.377570] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.377651] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.377693] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.377745] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.377828] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378002] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378120] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378160] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378198] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378240] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378279] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378320] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378363] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378414] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378455] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378501] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378665] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378704] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378747] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378787] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378828] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378881] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378920] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.378970] hub 3-7.1:1.0: hub_port_status failed (err = -71)
[  860.379011] hub 3-7.1:1.0: hub_port_status failed (err = 

[Bug 77631] task scsi_eh_6:537 blocked for more than 120 seconds.

[bugzilla-daemon]

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

--- Comment #4 from Mikhail mikhail.v.gavri...@gmail.com ---
Created attachment 138991
  -- https://bugzilla.kernel.org/attachment.cgi?id=138991action=edit
kernel log (second reboot) and plug/unplug USB3.0 hub

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

[PATCH]: add debug flag parameter for SCSI tape driver

[Laurence Oberman]

Hello

Take 2 of this patch, changed module description and subject line.

This patch adds a debug_flag parameter that can be set on module load, and 
allows the DEBUG facility without a module recompile.
Usage: mpdprobe st debug_flag=1

Signed-off-by: Laurence Oberman lober...@redhat.com

diff -Nur a/st.c b/st.c
--- a/st.c  2014-06-10 16:45:18.522354105 -0400
+++ b/st.c  2014-06-10 19:40:39.774387990 -0400
@@ -80,6 +80,7 @@
 static int try_direct_io = TRY_DIRECT_IO;
 static int try_rdio = 1;
 static int try_wdio = 1;
+static int debug_flag = 0;
 
 static struct class st_sysfs_class;
 static const struct attribute_group *st_dev_groups[];
@@ -100,6 +101,9 @@
 MODULE_PARM_DESC(max_sg_segs, Maximum number of scatter/gather segments to 
use (256));
 module_param_named(try_direct_io, try_direct_io, int, 0);
 MODULE_PARM_DESC(try_direct_io, Try direct I/O between user buffer and tape 
drive (1));
+module_param_named(debug_flag, debug_flag, int, 0);
+MODULE_PARM_DESC(debug_flag, Enable DEBUG, same as setting debugging=1);
+
 
 /* Extra parameters for testing */
 module_param_named(try_rdio, try_rdio, int, 0);
@@ -124,6 +128,9 @@
},
{
try_direct_io, try_direct_io
+   },
+   {
+   debug_flag, debug_flag
}
 };
 #endif
@@ -4277,7 +4284,9 @@
 static int __init init_st(void)
 {
int err;
-
+   debugging = (debug_flag  0) ? debug_flag : DEBUG;
+if (debugging) 
+   printk(KERN_INFO st: Debugging enabled debug_flag = 
%d\n,debugging);
validate_options();
 
printk(KERN_INFO st: Version %s, fixed bufsize %d, s/g segs %d\n,


Thanks
Laurence
--
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