[PATCH RESEND] scsi: use platform_{get,set}_drvdata()

2013-08-21 Thread Jingoo Han
Use the wrapper functions for getting and setting the driver data using
platform_device instead of using dev_{get,set}_drvdata() with pdev-dev,
so we can directly pass a struct platform_device. This is a cosmetic
change in order to enhance the readability and make the code simpler.

Also, unnecessary dev_set_drvdata() is removed, because the driver core
clears the driver data to NULL after device_release or on probe failure.

Signed-off-by: Jingoo Han jg1@samsung.com
---
 drivers/scsi/jazz_esp.c   |4 ++--
 drivers/scsi/qlogicpti.c  |4 ++--
 drivers/scsi/sni_53c710.c |4 ++--
 drivers/scsi/sun3x_esp.c  |4 ++--
 drivers/scsi/sun_esp.c|6 ++
 5 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/jazz_esp.c b/drivers/scsi/jazz_esp.c
index 69efbf1..0063b1b 100644
--- a/drivers/scsi/jazz_esp.c
+++ b/drivers/scsi/jazz_esp.c
@@ -180,7 +180,7 @@ static int esp_jazz_probe(struct platform_device *dev)
esp-scsi_id_mask = (1  esp-scsi_id);
esp-cfreq = 4000;
 
-   dev_set_drvdata(dev-dev, esp);
+   platform_set_drvdata(dev, esp);
 
err = scsi_esp_register(esp, dev-dev);
if (err)
@@ -203,7 +203,7 @@ fail:
 
 static int esp_jazz_remove(struct platform_device *dev)
 {
-   struct esp *esp = dev_get_drvdata(dev-dev);
+   struct esp *esp = platform_get_drvdata(dev);
unsigned int irq = esp-host-irq;
 
scsi_esp_unregister(esp);
diff --git a/drivers/scsi/qlogicpti.c b/drivers/scsi/qlogicpti.c
index 6d48d30..4ad93e8 100644
--- a/drivers/scsi/qlogicpti.c
+++ b/drivers/scsi/qlogicpti.c
@@ -1367,7 +1367,7 @@ static int qpti_sbus_probe(struct platform_device *op)
goto fail_unmap_queues;
}
 
-   dev_set_drvdata(op-dev, qpti);
+   platform_set_drvdata(op, qpti);
 
qpti_chain_add(qpti);
 
@@ -1404,7 +1404,7 @@ fail_unlink:
 
 static int qpti_sbus_remove(struct platform_device *op)
 {
-   struct qlogicpti *qpti = dev_get_drvdata(op-dev);
+   struct qlogicpti *qpti = platform_get_drvdata(op);
 
qpti_chain_del(qpti);
 
diff --git a/drivers/scsi/sni_53c710.c b/drivers/scsi/sni_53c710.c
index 52d54e7..4f5f3a2 100644
--- a/drivers/scsi/sni_53c710.c
+++ b/drivers/scsi/sni_53c710.c
@@ -104,7 +104,7 @@ static int snirm710_probe(struct platform_device *dev)
goto out_put_host;
}
 
-   dev_set_drvdata(dev-dev, host);
+   platform_set_drvdata(dev, host);
scsi_scan_host(host);
 
return 0;
@@ -119,7 +119,7 @@ static int snirm710_probe(struct platform_device *dev)
 
 static int __exit snirm710_driver_remove(struct platform_device *dev)
 {
-   struct Scsi_Host *host = dev_get_drvdata(dev-dev);
+   struct Scsi_Host *host = platform_get_drvdata(dev);
struct NCR_700_Host_Parameters *hostdata =
(struct NCR_700_Host_Parameters *)host-hostdata[0];
 
diff --git a/drivers/scsi/sun3x_esp.c b/drivers/scsi/sun3x_esp.c
index 534eb96..2edbce3 100644
--- a/drivers/scsi/sun3x_esp.c
+++ b/drivers/scsi/sun3x_esp.c
@@ -244,7 +244,7 @@ static int esp_sun3x_probe(struct platform_device *dev)
esp-scsi_id_mask = (1  esp-scsi_id);
esp-cfreq = 2000;
 
-   dev_set_drvdata(dev-dev, esp);
+   platform_set_drvdata(dev, esp);
 
err = scsi_esp_register(esp, dev-dev);
if (err)
@@ -270,7 +270,7 @@ fail:
 
 static int esp_sun3x_remove(struct platform_device *dev)
 {
-   struct esp *esp = dev_get_drvdata(dev-dev);
+   struct esp *esp = platform_get_drvdata(dev);
unsigned int irq = esp-host-irq;
u32 val;
 
diff --git a/drivers/scsi/sun_esp.c b/drivers/scsi/sun_esp.c
index f2e6845..db1397e 100644
--- a/drivers/scsi/sun_esp.c
+++ b/drivers/scsi/sun_esp.c
@@ -538,7 +538,7 @@ static int esp_sbus_probe_one(struct platform_device *op,
dma_write32(val  ~DMA_RST_SCSI, DMA_CSR);
}
 
-   dev_set_drvdata(op-dev, esp);
+   platform_set_drvdata(op, esp);
 
err = scsi_esp_register(esp, op-dev);
if (err)
@@ -585,7 +585,7 @@ static int esp_sbus_probe(struct platform_device *op)
 
 static int esp_sbus_remove(struct platform_device *op)
 {
-   struct esp *esp = dev_get_drvdata(op-dev);
+   struct esp *esp = platform_get_drvdata(op);
struct platform_device *dma_of = esp-dma;
unsigned int irq = esp-host-irq;
bool is_hme;
@@ -611,8 +611,6 @@ static int esp_sbus_remove(struct platform_device *op)
 
scsi_host_put(esp-host);
 
-   dev_set_drvdata(op-dev, NULL);
-
return 0;
 }
 
-- 
1.7.10.4


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


Re: [PATCH 3/5] target/iscsi: Remove macros that contain typecasts

2013-08-21 Thread Christoph Hellwig
On Tue, Aug 20, 2013 at 06:00:05PM -0700, Andy Grover wrote:
 These just want to return a pointer instead of a value, but are otherwise
 the same.
 
 ISCSI_TPG_LUN macro was unused.
 
 Signed-off-by: Andy Grover agro...@redhat.com

Looks good,

Reviewed-by: Christoph Hellwig h...@lst.de
--
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 1/5] target: Remove TF_CIT_TMPL macro

2013-08-21 Thread Christoph Hellwig
On Tue, Aug 20, 2013 at 06:00:03PM -0700, Andy Grover wrote:
 Remove a lingering macro that just hid a dereference.
 
 Signed-off-by: Andy Grover agro...@redhat.com

Looks good,

Reviewed-by: Christoph Hellwig h...@lst.de

--
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 4/5] target/qla2xxx: Define NPIV ops in terms of normal ops

2013-08-21 Thread Christoph Hellwig
On Tue, Aug 20, 2013 at 06:00:06PM -0700, Andy Grover wrote:
 Instead of defining a second target_core_fabric_ops struct, use the
 same one as normal (tcm_qla2xxx_ops) and then fixup the changed methods.
 
 This should make it a little easier to pick out the npiv differences, and
 also save a little space.
 
 Signed-off-by: Andy Grover agro...@redhat.com

Can't say I'm a huge fan of either the old or new way, I'd rather have
the methods contain both the NPIV and non-NPIV code inline.  If that's
what you're preparing for I'm supportive of this, otherwise I don't
really care too much about it.  Looks correct at least..

--
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/5] target/qla2xxx: Remove QLA_TPG_ATTRIB macro

2013-08-21 Thread Christoph Hellwig
On Tue, Aug 20, 2013 at 06:00:07PM -0700, Andy Grover wrote:
 Just a dereference, don't need a macro.
 
 Signed-off-by: Andy Grover agro...@redhat.com

Looks good,

Reviewed-by: Christoph Hellwig h...@lst.de
--
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 1/9] scsi: Add CDB definition for COMPARE_AND_WRITE

2013-08-21 Thread Christoph Hellwig
Looks good,

Reviewed-by: Christoph Hellwig h...@lst.de
--
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/9] target: Skip -queue_data_in() callbacks for COMPARE_AND_WRITE

2013-08-21 Thread Christoph Hellwig
 @@ -1832,7 +1832,8 @@ static void transport_complete_qf(struct se_cmd *cmd)
   ret = cmd-se_tfo-queue_data_in(cmd);
   break;
   case DMA_TO_DEVICE:
 - if (cmd-t_bidi_data_sg) {
 + if (cmd-t_bidi_data_sg 
 + cmd-t_task_cdb[0] != COMPARE_AND_WRITE) {

This is not the place to hardcode specific cdb opcodes.  Should be
a flag with a defined meaning on the command.

--
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/9] target: Allow sbc_ops-execute_rw() to accept SGLs + data_direction

2013-08-21 Thread Christoph Hellwig
On Tue, Aug 20, 2013 at 08:07:57PM +, Nicholas A. Bellinger wrote:
 From: Nicholas Bellinger n...@daterainc.com
 
 COMPARE_AND_WRITE expects to be able to send down a DMA_FROM_DEVICE
 to obtain the necessary READ payload for comparision against the
 first half of the WRITE payload containing the verify user data.
 
 Currently virtual backends expect to internally reference SGLs,
 SGL nents, and data_direction, so change IBLOCK, FILEIO and RD
 sbc_ops-execute_rw() to accept this values as function parameters.
 
 Also add the sbc_execute_rw() wrapper to handle the special case
 for the initial COMPARE_AND_WRITE DMA_FROM_DEVICE - READ I/O
 submission.

I don't like the way this is structured with the new method.  It seems
like we should just pass the sgl and associated data to execute_cmd
and have the read vs write logic driven by command code, using generic
flags instead of specificly checking for compare and write.

--
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 9/9] tcm_qla2xxx: Add special case for COMPARE_AND_WRITE data_direction

2013-08-21 Thread Christoph Hellwig
On Tue, Aug 20, 2013 at 08:08:00PM +, Nicholas A. Bellinger wrote:
 From: Nicholas Bellinger n...@daterainc.com
 
 Add a special case for COMPARE_AND_WRITE for the reverse data direction
 mapping used for pci_map_sg() + friends.

A low level driver is an even worse place to hardcode a specific cdb
opcode.  As written before this should be done by a flag on the command.

Also it might make sense to lift this helper to get a dma direction from
a command into common code.

--
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: SCSI core patches for kernel 3.12

2013-08-21 Thread Christoph Hellwig
On Tue, Aug 20, 2013 at 01:38:04PM -0700, Nicholas A. Bellinger wrote:
 yawn, I only care about the performance against upstream code, so that
 would mean scsi_debug here.  Typically the onus of demonstrating a
 performance improvement is on the patch submitter (eg: not the
 reviewer).

Do you really care?  If the patchset introduced a lot of code or
ugliness I might agreee to your above statement, but it actually
makes the code more obvious, simpler and also fixes some small issues
so I don't think we'll need an exact performance improvement to justify
it.

 But it would be at least useful to know the actual benefit with results
 as an incremental step, short of avoiding this code entirely for
 scsi-mq.

I don't think you can avoid the device put/get entirely for that case.
I've been looking over the mq code a bit more lately, and in a few
places it just seems to try to cut a few too many corners.  To get
it out of the prototype status we'll need to make sure all edge cases
are handled correctly.

--
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] [TRIVIAL] Remove braces in drivers/scsi/in2000.h to fix build for clang. No functional change otherwise.

2013-08-21 Thread Christoph Hellwig
On Tue, Aug 20, 2013 at 11:19:06PM +0200, dl...@gmx.de wrote:
 From: Jan-Simon M??ller dl...@gmx.de
 
 Author:  PaX Team pageexec at freemail.hu
 ML-Post: 
 http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120507/142707.html
 URL: http://llvm.linuxfoundation.org
 
 Merge:   Jan-Simon M??ller dl9pf at gmx.de
 
 Description:
   Clang chokes on the notation insw (%%dx) but works for insw %%dx (outsw 
 likewise);
   GNU as accepts both forms.
 
 Signed-off-by: Jan-Simon M??ller dl...@gmx.de

I think the right fix is to simply kill the FAST_READ_IO/FAST_WRITE_IO
defines and their associated inline assembly entirely.

--
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/9] target: Skip -queue_data_in() callbacks for COMPARE_AND_WRITE

2013-08-21 Thread Nicholas A. Bellinger
On Tue, 2013-08-20 at 23:32 -0700, Christoph Hellwig wrote:
  @@ -1832,7 +1832,8 @@ static void transport_complete_qf(struct se_cmd *cmd)
  ret = cmd-se_tfo-queue_data_in(cmd);
  break;
  case DMA_TO_DEVICE:
  -   if (cmd-t_bidi_data_sg) {
  +   if (cmd-t_bidi_data_sg 
  +   cmd-t_task_cdb[0] != COMPARE_AND_WRITE) {
 
 This is not the place to hardcode specific cdb opcodes.  Should be
 a flag with a defined meaning on the command.
 

Since this code path is only ever reached after a successful comparison
+ submission of the subsequent write payload, this is fine to change to
use SCF_COMPARE_AND_WRITE_POST flag..

Folding in the following patch.

diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 60d1336..70a6adb 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1833,7 +1833,7 @@ static void transport_complete_qf(struct se_cmd *cmd)
break;
case DMA_TO_DEVICE:
if (cmd-t_bidi_data_sg 
-   cmd-t_task_cdb[0] != COMPARE_AND_WRITE) {
+   !(cmd-se_cmd_flags  SCF_COMPARE_AND_WRITE_POST)) {
ret = cmd-se_tfo-queue_data_in(cmd);
if (ret  0)
break;
@@ -1949,7 +1949,7 @@ static void target_complete_ok_work(struct work_struct 
*work)
 * Check if we need to send READ payload for BIDI-COMMAND
 */
if (cmd-t_bidi_data_sg 
-   cmd-t_task_cdb[0] != COMPARE_AND_WRITE) {
+   !(cmd-se_cmd_flags  SCF_COMPARE_AND_WRITE_POST)) {
spin_lock(cmd-se_lun-lun_sep_lock);
if (cmd-se_lun-lun_sep) {
cmd-se_lun-lun_sep-sep_stats.tx_data_octets 
+=


--
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/9] target: Allow sbc_ops-execute_rw() to accept SGLs + data_direction

2013-08-21 Thread Nicholas A. Bellinger
On Tue, 2013-08-20 at 23:35 -0700, Christoph Hellwig wrote:
 On Tue, Aug 20, 2013 at 08:07:57PM +, Nicholas A. Bellinger wrote:
  From: Nicholas Bellinger n...@daterainc.com
  
  COMPARE_AND_WRITE expects to be able to send down a DMA_FROM_DEVICE
  to obtain the necessary READ payload for comparision against the
  first half of the WRITE payload containing the verify user data.
  
  Currently virtual backends expect to internally reference SGLs,
  SGL nents, and data_direction, so change IBLOCK, FILEIO and RD
  sbc_ops-execute_rw() to accept this values as function parameters.
  
  Also add the sbc_execute_rw() wrapper to handle the special case
  for the initial COMPARE_AND_WRITE DMA_FROM_DEVICE - READ I/O
  submission.
 
 I don't like the way this is structured with the new method.  It seems
 like we should just pass the sgl and associated data to execute_cmd
 and have the read vs write logic driven by command code, using generic
 flags instead of specificly checking for compare and write.

I considered that approach as well, but in the end all of the non
sbc_ops-execute_rw() users of se_cmd-execute_cmd() will never make use
of a passed *sgl, sgl_nents, or data_direction that is different than
se_cmd assignments.

So in the end, the approach of changing all se_cmd-execute_cmd() users
to accommodate COMPARE_AND_WRITE did not end up making sense outside of
this particular special case..

--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 9/9] tcm_qla2xxx: Add special case for COMPARE_AND_WRITE data_direction

2013-08-21 Thread Nicholas A. Bellinger
On Tue, 2013-08-20 at 23:37 -0700, Christoph Hellwig wrote:
 On Tue, Aug 20, 2013 at 08:08:00PM +, Nicholas A. Bellinger wrote:
  From: Nicholas Bellinger n...@daterainc.com
  
  Add a special case for COMPARE_AND_WRITE for the reverse data direction
  mapping used for pci_map_sg() + friends.
 
 A low level driver is an even worse place to hardcode a specific cdb
 opcode.  As written before this should be done by a flag on the command.
 

Which means adding another COMPARE_AND_WRITE specific flag to
se_cmd_flags_Table, as the SCF_COMPARE_AND_WRITE_POST is ony set after
the comparsion of the verify instance data is complete..

Is it really worth having two se_cmd_flags for COMPARE_AND_WRITE..?

 Also it might make sense to lift this helper to get a dma direction from
 a command into common code.
 

Mmm, perhaps.  I don't recall of the top of my head why tcm_qla2xxx
actually needed to reverse it's dma direction (I'm sure that Roland
knows, CC'ed), but IIRC it was a tcm_qla2xxx specific thing..?

--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] [TRIVIAL] Remove braces in drivers/scsi/in2000.h to fix build for clang. No functional change otherwise.

2013-08-21 Thread Jan-Simon Möller
  
  Description:
Clang chokes on the notation insw (%%dx) but works for insw %%dx
(outsw likewise); GNU as accepts both forms.
  
  Signed-off-by: Jan-Simon M??ller dl...@gmx.de
 
 I think the right fix is to simply kill the FAST_READ_IO/FAST_WRITE_IO
 defines and their associated inline assembly entirely.

No objection ;) .
--
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: SCSI core patches for kernel 3.12

2013-08-21 Thread Nicholas A. Bellinger
On Tue, 2013-08-20 at 23:48 -0700, Christoph Hellwig wrote:
 On Tue, Aug 20, 2013 at 01:38:04PM -0700, Nicholas A. Bellinger wrote:
  yawn, I only care about the performance against upstream code, so that
  would mean scsi_debug here.  Typically the onus of demonstrating a
  performance improvement is on the patch submitter (eg: not the
  reviewer).
 
 Do you really care?  If the patchset introduced a lot of code or
 ugliness I might agreee to your above statement, but it actually
 makes the code more obvious, simpler and also fixes some small issues
 so I don't think we'll need an exact performance improvement to justify
 it.

Of course not.  I'm curious 1) how fast Bart has a SCSI client going
using scsi_request_fn(), and if it's a measurable difference 2) what
that difference is.

 
  But it would be at least useful to know the actual benefit with results
  as an incremental step, short of avoiding this code entirely for
  scsi-mq.
 
 I don't think you can avoid the device put/get entirely for that case.
 I've been looking over the mq code a bit more lately, and in a few
 places it just seems to try to cut a few too many corners.

Yes, it's a prototype!

 To get it out of the prototype status we'll need to make sure all edge cases
 are handled correctly.
 

Ohhh, yes.  I'm not claiming that it's anything beyond a very early
alpha at this stage, and have no plans for a mainline push anytime
before 2014 arrives, before scsi_error.c supports per-device context
recovery, or before hell freezes over.

Which ever comes first..  ;)

--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: [PATCHv2] pm80xx: fix Adaptec 71605H hang

2013-08-21 Thread Jack Wang
On 07/31/2013 05:19 PM, Jack Wang wrote:
 On 07/26/2013 06:43 PM, Hans Verkuil wrote:
 The IO command size is 128 bytes for these new controllers as opposed to 64
 for the old 8001 controller.

 The Adaptec out-of-tree driver did this correctly. After comparing the two
 this turned out to be the crucial difference.

 So don't hardcode the IO command size, instead use pm8001_ha-iomb_size as
 that is the correct value for both old and new controllers.

 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 Cc: sta...@vger.kernel.org  # for v3.10 and up
 snip
 
 Thanks Hans.
 Looks good now.
 
 Acked-by: Jack Wang xjtu...@gmail.com
 
 James,
 
 Could you consider to include this fix into your fixes tree, without
 this fix new Adaptec controller support is broken, sorry for that.
 
 
 Jack
 
Hi, James,

Could you include this patch, or you need we resend?

KR
Jack
--
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] [TRIVIAL] Remove braces in drivers/scsi/in2000.h to fix build for clang. No functional change otherwise.

2013-08-21 Thread Jan-Simon Möller
On Wednesday 21 August 2013 09:26:14 Jan-Simon Möller wrote:
   Description:
 Clang chokes on the notation insw (%%dx) but works for insw %%dx
 (outsw likewise); GNU as accepts both forms.
   
   Signed-off-by: Jan-Simon M??ller dl...@gmx.de
  
  I think the right fix is to simply kill the FAST_READ_IO/FAST_WRITE_IO
  defines and their associated inline assembly entirely.
 
 No objection ;) .
@Christoph Hellwig:
Do you take care of removing it then ?

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


Re: [PATCH 2/4] scsi_debug: fix logical block provisioning support when unmap_alignment != 0

2013-08-21 Thread Akinobu Mita
2013/8/21 Douglas Gilbert dgilb...@interlog.com:
 On 13-08-19 10:16 AM, Akinobu Mita wrote:

 Hi Douglas, Martin,

 Could you review this patch when you have a time?  I would like to
 submit at least this patch 2/4, and 1/4 which has already been acked
 by Douglas for the next merge window.

 Although the patches 2/4 ~ 4/4 are all related to the logical block
 provisioning support, the problems that fixed by 3/4 and 4/4 only
 happen with virtual_gb option enabled, too.  On the other hand, the
 problem that fixed by 2/4 is easily reproduced by, for example,
 'modprobe scsi_debug lbpu=1 unmap_alignment=1 unmap_granularity=4'.
 So the patch 2/4 has rather higher severity than others.


 This is Martin's area of expertise so I hope he also
 acks it.

 Acked-by: Douglas Gilbert dgilb...@interlog.com

Thanks.

BTW, I realized that the commit log for this change didn't fully
describe the problem.  So I'll update it so that it includes a
concrete example like below.

How to reproduce this problem:
# modprobe scsi_debug lbpu=1 unmap_alignment=1 unmap_granularity=4
# dd if=/dev/zero of=/dev/sdb
# sg_unmap --lba=1 --num=4 /dev/sdb

GET LBA STATUS command for lba=1 shows that the last UNMAP command didn't work:
# sg_get_lba_status --lba=1 /dev/sdb
descriptor LBA: 0x0001  blocks: 16383  mapped
--
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 9/9] tcm_qla2xxx: Add special case for COMPARE_AND_WRITE data_direction

2013-08-21 Thread Roland Dreier
On Tue, Aug 20, 2013 at 1:08 PM, Nicholas A. Bellinger
n...@daterainc.com wrote:
 Add a special case for COMPARE_AND_WRITE for the reverse data direction
 mapping used for pci_map_sg() + friends.

I don't understand this.  In fact the whole patch series looks quite
confused.  COMPARE AND WRITE is a normal Data-Out command, with no
requirement for special bidirectional handling or anything like that.
The only slightly unusual thing is that a CAW command with a NUMBER OF
LOGICAL BLOCKS equal to N will actually transfer 2*N worth of data --
one set of data for the compare operation and a second set to write if
the compare succeeds.  But just to be clear, the transfer of those 2*N
blocks happens as a single transfer during the Data-Out phase.

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


[PATCH 0/8] zfcp features and bugfixes for 3.12 merge window

2013-08-21 Thread Steffen Maier
James,

here is a series of zfcp features and bugfixes
for the upcoming merge window preparing kernel v3.12.
The patches apply on top of the current misc branch of your scsi.git.

Steffen

Linux on System z Development

IBM Deutschland Research  Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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


[PATCH 3/8] zfcp: dead code removal

2013-08-21 Thread Steffen Maier
From: Martin Peschke mpesc...@linux.vnet.ibm.com

Get rid of unused function zfcp_fsf_get_req and corresponding
prototype definition.

Commit a54ca0f62f953898b05549391ac2a8a4dad6482b in v2.6.28
[SCSI] zfcp: Redesign of the debug tracing for HBA records.
accidentally introduced this code which was dead in the first place.

Signed-off-by: Martin Peschke mpesc...@linux.vnet.ibm.com
Signed-off-by: Steffen Maier ma...@linux.vnet.ibm.com
---
 drivers/s390/scsi/zfcp_ext.h |2 --
 drivers/s390/scsi/zfcp_fsf.c |9 -
 2 files changed, 11 deletions(-)

--- a/drivers/s390/scsi/zfcp_ext.h
+++ b/drivers/s390/scsi/zfcp_ext.h
@@ -126,8 +126,6 @@ extern int zfcp_qdio_sbals_from_sg(struc
 extern int zfcp_qdio_open(struct zfcp_qdio *);
 extern void zfcp_qdio_close(struct zfcp_qdio *);
 extern void zfcp_qdio_siosl(struct zfcp_adapter *);
-extern struct zfcp_fsf_req *zfcp_fsf_get_req(struct zfcp_qdio *,
-struct qdio_buffer *);
 
 /* zfcp_scsi.c */
 extern struct scsi_transport_template *zfcp_scsi_transport_template;
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -2388,12 +2388,3 @@ void zfcp_fsf_reqid_check(struct zfcp_qd
break;
}
 }
-
-struct zfcp_fsf_req *zfcp_fsf_get_req(struct zfcp_qdio *qdio,
- struct qdio_buffer *sbal)
-{
-   struct qdio_buffer_element *sbale = sbal-element[0];
-   u64 req_id = (unsigned long) sbale-addr;
-
-   return zfcp_reqlist_find(qdio-adapter-req_list, req_id);
-}

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


[PATCH 2/8] zfcp: cleanup use of obsolete strict_strto* functions

2013-08-21 Thread Steffen Maier
From: Martin Peschke mpesc...@linux.vnet.ibm.com

strict_strtoul and friends are obsolete. Use kstrtoul functions
instead.

Signed-off-by: Martin Peschke mpesc...@linux.vnet.ibm.com
Cc: Jingoo Han jg1@samsung.com
Signed-off-by: Steffen Maier ma...@linux.vnet.ibm.com
---
 drivers/s390/scsi/zfcp_aux.c   |4 ++--
 drivers/s390/scsi/zfcp_sysfs.c |   12 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

--- a/drivers/s390/scsi/zfcp_aux.c
+++ b/drivers/s390/scsi/zfcp_aux.c
@@ -104,11 +104,11 @@ static void __init zfcp_init_device_setu
strncpy(busid, token, ZFCP_BUS_ID_SIZE);
 
token = strsep(str, ,);
-   if (!token || strict_strtoull(token, 0, (unsigned long long *) wwpn))
+   if (!token || kstrtoull(token, 0, (unsigned long long *) wwpn))
goto err_out;
 
token = strsep(str, ,);
-   if (!token || strict_strtoull(token, 0, (unsigned long long *) lun))
+   if (!token || kstrtoull(token, 0, (unsigned long long *) lun))
goto err_out;
 
kfree(str_saved);
--- a/drivers/s390/scsi/zfcp_sysfs.c
+++ b/drivers/s390/scsi/zfcp_sysfs.c
@@ -95,7 +95,7 @@ static ssize_t zfcp_sysfs_port_failed_st
struct zfcp_port *port = container_of(dev, struct zfcp_port, dev);
unsigned long val;
 
-   if (strict_strtoul(buf, 0, val) || val != 0)
+   if (kstrtoul(buf, 0, val) || val != 0)
return -EINVAL;
 
zfcp_erp_set_port_status(port, ZFCP_STATUS_COMMON_RUNNING);
@@ -134,7 +134,7 @@ static ssize_t zfcp_sysfs_unit_failed_st
unsigned long val;
struct scsi_device *sdev;
 
-   if (strict_strtoul(buf, 0, val) || val != 0)
+   if (kstrtoul(buf, 0, val) || val != 0)
return -EINVAL;
 
sdev = zfcp_unit_sdev(unit);
@@ -184,7 +184,7 @@ static ssize_t zfcp_sysfs_adapter_failed
if (!adapter)
return -ENODEV;
 
-   if (strict_strtoul(buf, 0, val) || val != 0) {
+   if (kstrtoul(buf, 0, val) || val != 0) {
retval = -EINVAL;
goto out;
}
@@ -236,7 +236,7 @@ static ssize_t zfcp_sysfs_port_remove_st
if (!adapter)
return -ENODEV;
 
-   if (strict_strtoull(buf, 0, (unsigned long long *) wwpn))
+   if (kstrtoull(buf, 0, (unsigned long long *) wwpn))
goto out;
 
port = zfcp_get_port_by_wwpn(adapter, wwpn);
@@ -297,7 +297,7 @@ static ssize_t zfcp_sysfs_unit_add_store
u64 fcp_lun;
int retval;
 
-   if (strict_strtoull(buf, 0, (unsigned long long *) fcp_lun))
+   if (kstrtoull(buf, 0, (unsigned long long *) fcp_lun))
return -EINVAL;
 
retval = zfcp_unit_add(port, fcp_lun);
@@ -315,7 +315,7 @@ static ssize_t zfcp_sysfs_unit_remove_st
struct zfcp_port *port = container_of(dev, struct zfcp_port, dev);
u64 fcp_lun;
 
-   if (strict_strtoull(buf, 0, (unsigned long long *) fcp_lun))
+   if (kstrtoull(buf, 0, (unsigned long long *) fcp_lun))
return -EINVAL;
 
if (zfcp_unit_remove(port, fcp_lun))

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


[PATCH 1/8] zfcp: consistently use appropriate SBAL flag definitions

2013-08-21 Thread Steffen Maier
From: Martin Peschke mpesc...@linux.vnet.ibm.com

minor cleanup for status read request

Signed-off-by: Martin Peschke mpesc...@linux.vnet.ibm.com
Signed-off-by: Steffen Maier ma...@linux.vnet.ibm.com
---
 drivers/s390/scsi/zfcp_fsf.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -770,7 +770,8 @@ int zfcp_fsf_status_read(struct zfcp_qdi
if (zfcp_qdio_sbal_get(qdio))
goto out;
 
-   req = zfcp_fsf_req_create(qdio, FSF_QTCB_UNSOLICITED_STATUS, 0,
+   req = zfcp_fsf_req_create(qdio, FSF_QTCB_UNSOLICITED_STATUS,
+ SBAL_SFLAGS0_TYPE_STATUS,
  adapter-pool.status_read_req);
if (IS_ERR(req)) {
retval = PTR_ERR(req);

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


[PATCH 7/8] zfcp: remove access control tables interface (port leftovers)

2013-08-21 Thread Steffen Maier
From: Martin Peschke mpesc...@linux.vnet.ibm.com

This patch removes some leftovers for commit
663e0890e31cb85f0cca5ac1faaee0d2d52880b5
[SCSI] zfcp: remove access control tables interface.

The access denied case for ports is gone, as well.
The corresponding flag was cleared, but never set.
So clean it up.

Sysfs flag is kept, though, for backward-compatibility.
Now it returns always 0.

Signed-off-by: Martin Peschke mpesc...@linux.vnet.ibm.com
Signed-off-by: Steffen Maier ma...@linux.vnet.ibm.com
---
 drivers/s390/scsi/zfcp_erp.c   |7 ---
 drivers/s390/scsi/zfcp_fsf.c   |3 +--
 drivers/s390/scsi/zfcp_sysfs.c |4 +---
 3 files changed, 2 insertions(+), 12 deletions(-)

--- a/drivers/s390/scsi/zfcp_erp.c
+++ b/drivers/s390/scsi/zfcp_erp.c
@@ -821,11 +821,6 @@ static int zfcp_erp_port_forced_strategy
return ZFCP_ERP_CONTINUES;
 }
 
-static void zfcp_erp_port_strategy_clearstati(struct zfcp_port *port)
-{
-   atomic_clear_mask(ZFCP_STATUS_COMMON_ACCESS_DENIED, port-status);
-}
-
 static int zfcp_erp_port_forced_strategy(struct zfcp_erp_action *erp_action)
 {
struct zfcp_port *port = erp_action-port;
@@ -833,7 +828,6 @@ static int zfcp_erp_port_forced_strategy
 
switch (erp_action-step) {
case ZFCP_ERP_STEP_UNINITIALIZED:
-   zfcp_erp_port_strategy_clearstati(port);
if ((status  ZFCP_STATUS_PORT_PHYS_OPEN) 
(status  ZFCP_STATUS_COMMON_OPEN))
return zfcp_erp_port_forced_strategy_close(erp_action);
@@ -933,7 +927,6 @@ static int zfcp_erp_port_strategy(struct
 
switch (erp_action-step) {
case ZFCP_ERP_STEP_UNINITIALIZED:
-   zfcp_erp_port_strategy_clearstati(port);
if (p_status  ZFCP_STATUS_COMMON_OPEN)
return zfcp_erp_port_strategy_close(erp_action);
break;
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -1396,8 +1396,7 @@ static void zfcp_fsf_open_port_handler(s
port-handle = header-port_handle;
atomic_set_mask(ZFCP_STATUS_COMMON_OPEN |
ZFCP_STATUS_PORT_PHYS_OPEN, port-status);
-   atomic_clear_mask(ZFCP_STATUS_COMMON_ACCESS_DENIED |
- ZFCP_STATUS_COMMON_ACCESS_BOXED,
+   atomic_clear_mask(ZFCP_STATUS_COMMON_ACCESS_BOXED,
  port-status);
/* check whether D_ID has changed during open */
/*
--- a/drivers/s390/scsi/zfcp_sysfs.c
+++ b/drivers/s390/scsi/zfcp_sysfs.c
@@ -73,9 +73,7 @@ ZFCP_DEFINE_ATTR(zfcp_port, port, status
 ZFCP_DEFINE_ATTR(zfcp_port, port, in_recovery, %d\n,
 (atomic_read(port-status) 
  ZFCP_STATUS_COMMON_ERP_INUSE) != 0);
-ZFCP_DEFINE_ATTR(zfcp_port, port, access_denied, %d\n,
-(atomic_read(port-status) 
- ZFCP_STATUS_COMMON_ACCESS_DENIED) != 0);
+ZFCP_DEFINE_ATTR_CONST(port, access_denied, %d\n, 0);
 
 ZFCP_DEFINE_ATTR(zfcp_unit, unit, status, 0x%08x\n,
 zfcp_unit_sdev_status(unit));

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


[PATCH 6/8] zfcp: remove access control tables interface (keep sysfs files)

2013-08-21 Thread Steffen Maier
From: Martin Peschke mpesc...@linux.vnet.ibm.com

By popular demand, this patch brings back a couple of sysfs attributes
removed by commit 663e0890e31cb85f0cca5ac1faaee0d2d52880b5
[SCSI] zfcp: remove access control tables interface.
The content has been irrelevant for years, but the files must be
there forever for whatever user space tools that may rely on them.

Since these files always return a constant value, a new stripped
down show-macro was required. Otherwise build warnings would have
been introduced.

Signed-off-by: Martin Peschke mpesc...@linux.vnet.ibm.com
Cc: sta...@vger.kernel.org #3.11+
Signed-off-by: Steffen Maier ma...@linux.vnet.ibm.com
---
 drivers/s390/scsi/zfcp_sysfs.c |   14 ++
 1 file changed, 14 insertions(+)

--- a/drivers/s390/scsi/zfcp_sysfs.c
+++ b/drivers/s390/scsi/zfcp_sysfs.c
@@ -27,6 +27,16 @@ static ssize_t zfcp_sysfs_##_feat##_##_n
 static ZFCP_DEV_ATTR(_feat, _name, S_IRUGO,   \
 zfcp_sysfs_##_feat##_##_name##_show, NULL);
 
+#define ZFCP_DEFINE_ATTR_CONST(_feat, _name, _format, _value) \
+static ssize_t zfcp_sysfs_##_feat##_##_name##_show(struct device *dev,\
+  struct device_attribute *at,\
+  char *buf)  \
+{ \
+   return sprintf(buf, _format, _value);  \
+} \
+static ZFCP_DEV_ATTR(_feat, _name, S_IRUGO,   \
+zfcp_sysfs_##_feat##_##_name##_show, NULL);
+
 #define ZFCP_DEFINE_A_ATTR(_name, _format, _value)  \
 static ssize_t zfcp_sysfs_adapter_##_name##_show(struct device *dev,\
 struct device_attribute *at,\
@@ -75,6 +85,8 @@ ZFCP_DEFINE_ATTR(zfcp_unit, unit, in_rec
 ZFCP_DEFINE_ATTR(zfcp_unit, unit, access_denied, %d\n,
 (zfcp_unit_sdev_status(unit) 
  ZFCP_STATUS_COMMON_ACCESS_DENIED) != 0);
+ZFCP_DEFINE_ATTR_CONST(unit, access_shared, %d\n, 0);
+ZFCP_DEFINE_ATTR_CONST(unit, access_readonly, %d\n, 0);
 
 static ssize_t zfcp_sysfs_port_failed_show(struct device *dev,
   struct device_attribute *attr,
@@ -347,6 +359,8 @@ static struct attribute *zfcp_unit_attrs
dev_attr_unit_in_recovery.attr,
dev_attr_unit_status.attr,
dev_attr_unit_access_denied.attr,
+   dev_attr_unit_access_shared.attr,
+   dev_attr_unit_access_readonly.attr,
NULL
 };
 static struct attribute_group zfcp_unit_attr_group = {

--
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 4/8] zfcp: fix lock imbalance by reworking request queue locking

2013-08-21 Thread Steffen Maier
From: Martin Peschke mpesc...@linux.vnet.ibm.com

This patch adds wait_event_interruptible_lock_irq_timeout(), which is a
straight-forward descendant of wait_event_interruptible_timeout() and
wait_event_interruptible_lock_irq().

The zfcp driver used to call wait_event_interruptible_timeout()
in combination with some intricate and error-prone locking. Using
wait_event_interruptible_lock_irq_timeout() as a replacement
nicely cleans up that locking.

This rework removes a situation that resulted in a locking imbalance
in zfcp_qdio_sbal_get():

BUG: workqueue leaked lock or atomic: events/1/0xff00/10
last function: zfcp_fc_wka_port_offline+0x0/0xa0 [zfcp]

It was introduced by commit c2af7545aaff3495d9bf9a7608c52f0af86fb194
[SCSI] zfcp: Do not wait for SBALs on stopped queue, which had a new
code path related to ZFCP_STATUS_ADAPTER_QDIOUP that took an early exit
without a required lock being held. The problem occured when a
special, non-SCSI I/O request was being submitted in process context,
when the adapter's queues had been torn down. In this case the bug
surfaced when the Fibre Channel port connection for a well-known address
was closed during a concurrent adapter shut-down procedure, which is a
rare constellation.

This patch also fixes these warnings from the sparse tool (make C=1):

drivers/s390/scsi/zfcp_qdio.c:224:12: warning: context imbalance in
 'zfcp_qdio_sbal_check' - wrong count at exit
drivers/s390/scsi/zfcp_qdio.c:244:5: warning: context imbalance in
 'zfcp_qdio_sbal_get' - unexpected unlock

Last but not least, we get rid of that crappy lock-unlock-lock
sequence at the beginning of the critical section.

It is okay to call zfcp_erp_adapter_reopen() with req_q_lock held.

Reported-by: Mikulas Patocka mpato...@redhat.com
Reported-by: Heiko Carstens heiko.carst...@de.ibm.com
Signed-off-by: Martin Peschke mpesc...@linux.vnet.ibm.com
Cc: sta...@vger.kernel.org #2.6.35+
Cc: Andrew Morton a...@linux-foundation.org
Cc: David Howells dhowe...@redhat.com
Cc: Jens Axboe ax...@kernel.dk
Cc: Paul E. McKenney paul...@linux.vnet.ibm.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Steffen Maier ma...@linux.vnet.ibm.com
---
 drivers/s390/scsi/zfcp_qdio.c |8 +
 include/linux/wait.h  |   57 ++
 2 files changed, 59 insertions(+), 6 deletions(-)

--- a/drivers/s390/scsi/zfcp_qdio.c
+++ b/drivers/s390/scsi/zfcp_qdio.c
@@ -224,11 +224,9 @@ int zfcp_qdio_sbals_from_sg(struct zfcp_
 
 static int zfcp_qdio_sbal_check(struct zfcp_qdio *qdio)
 {
-   spin_lock_irq(qdio-req_q_lock);
if (atomic_read(qdio-req_q_free) ||
!(atomic_read(qdio-adapter-status)  ZFCP_STATUS_ADAPTER_QDIOUP))
return 1;
-   spin_unlock_irq(qdio-req_q_lock);
return 0;
 }
 
@@ -246,9 +244,8 @@ int zfcp_qdio_sbal_get(struct zfcp_qdio
 {
long ret;
 
-   spin_unlock_irq(qdio-req_q_lock);
-   ret = wait_event_interruptible_timeout(qdio-req_q_wq,
-  zfcp_qdio_sbal_check(qdio), 5 * HZ);
+   ret = wait_event_interruptible_lock_irq_timeout(qdio-req_q_wq,
+  zfcp_qdio_sbal_check(qdio), qdio-req_q_lock, 5 * HZ);
 
if (!(atomic_read(qdio-adapter-status)  ZFCP_STATUS_ADAPTER_QDIOUP))
return -EIO;
@@ -262,7 +259,6 @@ int zfcp_qdio_sbal_get(struct zfcp_qdio
zfcp_erp_adapter_reopen(qdio-adapter, 0, qdsbg_1);
}
 
-   spin_lock_irq(qdio-req_q_lock);
return -EIO;
 }
 
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -811,6 +811,63 @@ do {   
\
__ret;  \
 })
 
+#define __wait_event_interruptible_lock_irq_timeout(wq, condition, \
+   lock, ret)  \
+do {   \
+   DEFINE_WAIT(__wait);\
+   \
+   for (;;) {  \
+   prepare_to_wait(wq, __wait, TASK_INTERRUPTIBLE);  \
+   if (condition)  \
+   break;  \
+   if (signal_pending(current)) {  \
+   ret = -ERESTARTSYS; \
+   break;  \
+   }   \
+   spin_unlock_irq(lock); \
+   ret = schedule_timeout(ret);\
+   spin_lock_irq(lock);   \
+   if (!ret)   

[PATCH 8/8] zfcp: enable FCP hardware data router by default

2013-08-21 Thread Steffen Maier
Enabling the data router support by default
can increase performance in certain situations.
It is safe to do so and tolerated in LPAR and under z/VM
in case there is no data router support in that environment.

Signed-off-by: Steffen Maier ma...@linux.vnet.ibm.com
Reviewed-by: Martin Peschke mpesc...@linux.vnet.ibm.com
---
 drivers/s390/scsi/zfcp_qdio.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/drivers/s390/scsi/zfcp_qdio.c
+++ b/drivers/s390/scsi/zfcp_qdio.c
@@ -16,9 +16,9 @@
 
 #define QBUFF_PER_PAGE (PAGE_SIZE / sizeof(struct qdio_buffer))
 
-static bool enable_multibuffer;
+static bool enable_multibuffer = 1;
 module_param_named(datarouter, enable_multibuffer, bool, 0400);
-MODULE_PARM_DESC(datarouter, Enable hardware data router support);
+MODULE_PARM_DESC(datarouter, Enable hardware data router support (default 
on));
 
 static int zfcp_qdio_buffers_enqueue(struct qdio_buffer **sbal)
 {

--
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 5/8] zfcp: fix schedule-inside-lock in scsi_device list loops

2013-08-21 Thread Steffen Maier
From: Martin Peschke mpesc...@linux.vnet.ibm.com

BUG: sleeping function called from invalid context at kernel/workqueue.c:2752
in_atomic(): 1, irqs_disabled(): 1, pid: 360, name: zfcperp0.0.1700
CPU: 1 Not tainted 3.9.3+ #69
Process zfcperp0.0.1700 (pid: 360, task: 75b7e080, ksp: 
7476bc30)
snip
Call Trace:
([001165de] show_trace+0x106/0x154)
 [001166a0] show_stack+0x74/0xf4
 [006ff646] dump_stack+0xc6/0xd4
 [0017f3a0] __might_sleep+0x128/0x148
 [0015ece8] flush_work+0x54/0x1f8
 [001630de] __cancel_work_timer+0xc6/0x128
 [005067ac] scsi_device_dev_release_usercontext+0x164/0x23c
 [00161816] execute_in_process_context+0x96/0xa8
 [004d33d8] device_release+0x60/0xc0
 [0048af48] kobject_release+0xa8/0x1c4
 [004f4bf2] __scsi_iterate_devices+0xfa/0x130
 [03ff801b307a] zfcp_erp_strategy+0x4da/0x1014 [zfcp]
 [03ff801b3caa] zfcp_erp_thread+0xf6/0x2b0 [zfcp]
 [0016b75a] kthread+0xf2/0xfc
 [0070c9de] kernel_thread_starter+0x6/0xc
 [0070c9d8] kernel_thread_starter+0x0/0xc

Apparently, the ref_count for some scsi_device drops down to zero,
triggering device removal through execute_in_process_context(), while
the lldd error recovery thread iterates through a scsi device list.
Unfortunately, execute_in_process_context() decides to immediately
execute that device removal function, instead of scheduling asynchronous
execution, since it detects process context and thinks it is safe to do
so. But almost all calls to shost_for_each_device() in our lldd are
inside spin_lock_irq, even in thread context. Obviously, schedule()
inside spin_lock_irq sections is a bad idea.

Change the lldd to use the proper iterator function,
__shost_for_each_device(), in combination with required locking.

Occurences that need to be changed include all calls in zfcp_erp.c,
since those might be executed in zfcp error recovery thread context
with a lock held.

Other occurences of shost_for_each_device() in zfcp_fsf.c do not
need to be changed (no process context, no surrounding locking).

The problem was introduced in Linux 2.6.37 by commit
b62a8d9b45b971a67a0f8413338c230e3117dff5
[SCSI] zfcp: Use SCSI device data zfcp_scsi_dev instead of zfcp_unit.

Reported-by: Christian Borntraeger borntrae...@de.ibm.com
Signed-off-by: Martin Peschke mpesc...@linux.vnet.ibm.com
Cc: sta...@vger.kernel.org #2.6.37+
Signed-off-by: Steffen Maier ma...@linux.vnet.ibm.com
---
 drivers/s390/scsi/zfcp_erp.c |   29 ++---
 1 file changed, 22 insertions(+), 7 deletions(-)

--- a/drivers/s390/scsi/zfcp_erp.c
+++ b/drivers/s390/scsi/zfcp_erp.c
@@ -102,10 +102,13 @@ static void zfcp_erp_action_dismiss_port
 
if (atomic_read(port-status)  ZFCP_STATUS_COMMON_ERP_INUSE)
zfcp_erp_action_dismiss(port-erp_action);
-   else
-   shost_for_each_device(sdev, port-adapter-scsi_host)
+   else {
+   spin_lock(port-adapter-scsi_host-host_lock);
+   __shost_for_each_device(sdev, port-adapter-scsi_host)
if (sdev_to_zfcp(sdev)-port == port)
zfcp_erp_action_dismiss_lun(sdev);
+   spin_unlock(port-adapter-scsi_host-host_lock);
+   }
 }
 
 static void zfcp_erp_action_dismiss_adapter(struct zfcp_adapter *adapter)
@@ -592,9 +595,11 @@ static void _zfcp_erp_lun_reopen_all(str
 {
struct scsi_device *sdev;
 
-   shost_for_each_device(sdev, port-adapter-scsi_host)
+   spin_lock(port-adapter-scsi_host-host_lock);
+   __shost_for_each_device(sdev, port-adapter-scsi_host)
if (sdev_to_zfcp(sdev)-port == port)
_zfcp_erp_lun_reopen(sdev, clear, id, 0);
+   spin_unlock(port-adapter-scsi_host-host_lock);
 }
 
 static void zfcp_erp_strategy_followup_failed(struct zfcp_erp_action *act)
@@ -1434,8 +1439,10 @@ void zfcp_erp_set_adapter_status(struct
atomic_set_mask(common_mask, port-status);
read_unlock_irqrestore(adapter-port_list_lock, flags);
 
-   shost_for_each_device(sdev, adapter-scsi_host)
+   spin_lock_irqsave(adapter-scsi_host-host_lock, flags);
+   __shost_for_each_device(sdev, adapter-scsi_host)
atomic_set_mask(common_mask, sdev_to_zfcp(sdev)-status);
+   spin_unlock_irqrestore(adapter-scsi_host-host_lock, flags);
 }
 
 /**
@@ -1469,11 +1476,13 @@ void zfcp_erp_clear_adapter_status(struc
}
read_unlock_irqrestore(adapter-port_list_lock, flags);
 
-   shost_for_each_device(sdev, adapter-scsi_host) {
+   spin_lock_irqsave(adapter-scsi_host-host_lock, flags);
+   __shost_for_each_device(sdev, adapter-scsi_host) {
atomic_clear_mask(common_mask, sdev_to_zfcp(sdev)-status);
if (clear_counter)
atomic_set(sdev_to_zfcp(sdev)-erp_counter, 0);
}
+   spin_unlock_irqrestore(adapter-scsi_host-host_lock, 

Re: [PATCH 0/8] zfcp features and bugfixes for 3.12 merge window

2013-08-21 Thread James Bottomley
On Wed, 2013-08-21 at 17:05 +0200, Steffen Maier wrote:
 James,
 
 here is a series of zfcp features and bugfixes
 for the upcoming merge window preparing kernel v3.12.
 The patches apply on top of the current misc branch of your scsi.git.

OK, so this isn't going to work any more given Greg's recent statements.
Patches for stable have to go through the -rc fixes process, so could
you redo this as two patch sets: one for fixes with the cc stable tags
and the rest for misc?

Thanks,

James


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


Re: [PATCH 9/9] tcm_qla2xxx: Add special case for COMPARE_AND_WRITE data_direction

2013-08-21 Thread Christoph Hellwig
On Wed, Aug 21, 2013 at 07:38:21AM -0700, Roland Dreier wrote:
 I don't understand this.  In fact the whole patch series looks quite
 confused.  COMPARE AND WRITE is a normal Data-Out command, with no
 requirement for special bidirectional handling or anything like that.
 The only slightly unusual thing is that a CAW command with a NUMBER OF
 LOGICAL BLOCKS equal to N will actually transfer 2*N worth of data --
 one set of data for the compare operation and a second set to write if
 the compare succeeds.  But just to be clear, the transfer of those 2*N
 blocks happens as a single transfer during the Data-Out phase.

I think the confusion is that the implementation of COMPARE AND WRITE
obviously requires a read and a write phase, and the implementation
tries to mix this up with an actual bidirectional scsi command.

If the core stopped keying off t_bidi_data_sg and used better flag
this could be easily solved.
--
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 4/5] target/qla2xxx: Define NPIV ops in terms of normal ops

2013-08-21 Thread Andy Grover

On 08/20/2013 11:30 PM, Christoph Hellwig wrote:

On Tue, Aug 20, 2013 at 06:00:06PM -0700, Andy Grover wrote:

Instead of defining a second target_core_fabric_ops struct, use the
same one as normal (tcm_qla2xxx_ops) and then fixup the changed methods.

This should make it a little easier to pick out the npiv differences, and
also save a little space.

Signed-off-by: Andy Grover agro...@redhat.com


Can't say I'm a huge fan of either the old or new way, I'd rather have
the methods contain both the NPIV and non-NPIV code inline.  If that's
what you're preparing for I'm supportive of this, otherwise I don't
really care too much about it.  Looks correct at least..


It reduces the binary size and makes it easier to spot the differences 
between the two cases, which was the goal. So I think it's a small 
improvement but if others disagree then I'm fine with the code as-is.


Regards -- Andy

--
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 9/9] tcm_qla2xxx: Add special case for COMPARE_AND_WRITE data_direction

2013-08-21 Thread Christoph Hellwig
On Wed, Aug 21, 2013 at 12:31:07AM -0700, Nicholas A. Bellinger wrote:
 Is it really worth having two se_cmd_flags for COMPARE_AND_WRITE..?

Not leaking the abstraction into the driver is always worth the effort.

But looking at the other patches I haven't reviewed yet I think the
issue is more severe anyway, see my next reply.

  Also it might make sense to lift this helper to get a dma direction from
  a command into common code.
  
 
 Mmm, perhaps.  I don't recall of the top of my head why tcm_qla2xxx
 actually needed to reverse it's dma direction (I'm sure that Roland
 knows, CC'ed), but IIRC it was a tcm_qla2xxx specific thing..?

It's the same issue for any hardware driver that directly maps a
se_cmd - the direction the target expects is reversed to what the
driver expects, in addition any BIDI or other special meanings will
need handling.

--
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 8/9] target: Add support for COMPARE_AND_WRITE emulation

2013-08-21 Thread Christoph Hellwig
I don't like the layering here.  The re-execution of the same command
for both reading and writing the data from/to the backend device already
looks sketchy here due to doubling work of task attribute handling, the
various state bits, etc.  And it will only get more complicated when
the required locking is added.  In addition we have all that confusion
about overloading the data direction.

I think the way this should be handled is:


 - cmd-execute_cmd gets set to a new sbc_emulate_compare_and_write
 - sbc_emulate_compare_and_write does all the setup for the locking,
   sets up the read buffer, then calls ops-execute_rw to do the
   read.  The complete callback does the comparism, then calls
   ops-execute_rw to do the write, and the second time we hit
   the complete callback we teard down the read buffer, stop the
   locking, etc.

This also avoids bloating the command with another function pointer
or having to change all execute_cmd prototypes.
--
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 9/9] tcm_qla2xxx: Add special case for COMPARE_AND_WRITE data_direction

2013-08-21 Thread Roland Dreier
On Wed, Aug 21, 2013 at 7:38 AM, Roland Dreier rol...@purestorage.com wrote:
 I don't understand this.  In fact the whole patch series looks quite
 confused.  COMPARE AND WRITE is a normal Data-Out command, with no
 requirement for special bidirectional handling or anything like that.
 The only slightly unusual thing is that a CAW command with a NUMBER OF
 LOGICAL BLOCKS equal to N will actually transfer 2*N worth of data --
 one set of data for the compare operation and a second set to write if
 the compare succeeds.  But just to be clear, the transfer of those 2*N
 blocks happens as a single transfer during the Data-Out phase.

OK, I understand the patch set a bit better.  You're using the bidi
infrastructure to have a place to stick the data that you internally
read to implement the compare, but then you end up having places like
this where you have to say, oh it's not really a bidi command, it's
just a compare and write.

Shouldn't there be a way to confine the COMPARE AND WRITE handling to
the actual implementation of that command?  Or maybe make the bidi
handling more generic so that this becomes clearer?

 - R.
--
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 8/9] target: Add support for COMPARE_AND_WRITE emulation

2013-08-21 Thread Nicholas A. Bellinger
On Wed, 2013-08-21 at 09:14 -0700, Christoph Hellwig wrote:
 I don't like the layering here.  The re-execution of the same command
 for both reading and writing the data from/to the backend device already
 looks sketchy here due to doubling work of task attribute handling, the
 various state bits, etc.  And it will only get more complicated when
 the required locking is added.  In addition we have all that confusion
 about overloading the data direction.
 
 I think the way this should be handled is:
 
 
  - cmd-execute_cmd gets set to a new sbc_emulate_compare_and_write
  - sbc_emulate_compare_and_write does all the setup for the locking,
sets up the read buffer, then calls ops-execute_rw to do the
read.  The complete callback does the comparism, then calls
ops-execute_rw to do the write, and the second time we hit
the complete callback we teard down the read buffer, stop the
locking, etc.
 

I do like this approach better, however calling ops-execute_rw() the
second time around does require at least the TRANSPORT_PROCESSING and
other transport_state bits from target_execute_cmd() to be set for
things to work correctly.  Bypassing the aborted + CMD_*STOP checks
should be OK for the write submission, and will kick (if necessary)
during the final completion.

Setting up the read buffer from sbc_emulate_compare_and_write() would
require two extra COMPARE_AND_WRITE specific se_cmd elements, so I'm
tempted to continue to use the bidi fields for this (because they
already exist) with transport_generic_get_mem_bidi(), and use a
SCF_COMPARE_AND_WRITE flag to avoid any CDB specific checks in
transport_complete_ok(), and reverse dma direction mapping case.

 This also avoids bloating the command with another function pointer
 or having to change all execute_cmd prototypes.

Indeed.

--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 9/9] tcm_qla2xxx: Add special case for COMPARE_AND_WRITE data_direction

2013-08-21 Thread Nicholas A. Bellinger
On Wed, 2013-08-21 at 08:53 -0700, Christoph Hellwig wrote:
 On Wed, Aug 21, 2013 at 07:38:21AM -0700, Roland Dreier wrote:
  I don't understand this.  In fact the whole patch series looks quite
  confused.  COMPARE AND WRITE is a normal Data-Out command, with no
  requirement for special bidirectional handling or anything like that.
  The only slightly unusual thing is that a CAW command with a NUMBER OF
  LOGICAL BLOCKS equal to N will actually transfer 2*N worth of data --
  one set of data for the compare operation and a second set to write if
  the compare succeeds.  But just to be clear, the transfer of those 2*N
  blocks happens as a single transfer during the Data-Out phase.
 
 I think the confusion is that the implementation of COMPARE AND WRITE
 obviously requires a read and a write phase, and the implementation
 tries to mix this up with an actual bidirectional scsi command.
 
 If the core stopped keying off t_bidi_data_sg and used better flag
 this could be easily solved.

Good point here as well..  Changing these cases to check for SCF_BIDI
instead, and adding a extra SCF_COMPARE_AND_WRITE check for the case in
transport_generic_new_cmd() to call transport_generic_get_mem_bidi().

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


[PATCH] scsi_dh_rdac: Add new IBM 1813 product id to rdac devlist

2013-08-21 Thread Stewart, Sean
Add new IBM product id to the RDAC devlist

Signed-off-by: Sean Stewart sean.stew...@netapp.com
---
diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c 
b/drivers/scsi/device_handler/scsi_dh_rdac.c
index 69c915a..4b9cf93 100644
--- a/drivers/scsi/device_handler/scsi_dh_rdac.c
+++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
@@ -786,6 +786,7 @@ static const struct scsi_dh_devlist rdac_dev_list[] = {
{IBM, 1742},
{IBM, 1745},
{IBM, 1746},
+   {IBM, 1813},
{IBM, 1814},
{IBM, 1815},
{IBM, 1818},
--
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/8] zfcp: cleanup use of obsolete strict_strto* functions

2013-08-21 Thread Jingoo Han
On Thursday, August 22, 2013 12:05 AM, Steffen Maier wrote:
 
 From: Martin Peschke mpesc...@linux.vnet.ibm.com
 
 strict_strtoul and friends are obsolete. Use kstrtoul functions
 instead.
 
 Signed-off-by: Martin Peschke mpesc...@linux.vnet.ibm.com
 Cc: Jingoo Han jg1@samsung.com

Reviewed-by: Jingoo Han jg1@samsung.com

Best regards,
Jingoo Han

 Signed-off-by: Steffen Maier ma...@linux.vnet.ibm.com
 ---
  drivers/s390/scsi/zfcp_aux.c   |4 ++--
  drivers/s390/scsi/zfcp_sysfs.c |   12 ++--
  2 files changed, 8 insertions(+), 8 deletions(-)

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


[PATCH] scsi: fix the build warning

2013-08-21 Thread zwu . kernel
From: Zhi Yong Wu wu...@linux.vnet.ibm.com

  Initialize csum variable to fix the build warning.

drivers/scsi/scsi_debug.c: In function ‘dif_verify’:
drivers/scsi/scsi_debug.c:1755:3: warning: ‘csum’ may be used uninitialized in 
this function [-Wmaybe-uninitialized]
  LD  drivers/built-in.o
  CHK include/generated/uapi/linux/version.h
make[2]: Nothing to be done for `all'.
make[2]: Nothing to be done for `relocs'.

Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com
---
 drivers/scsi/scsi_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cb4fefa..50d70c3 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1733,7 +1733,7 @@ static int do_device_access(struct scsi_cmnd *scmd,
 
 static u16 dif_compute_csum(const void *buf, int len)
 {
-   u16 csum;
+   u16 csum = -1;
 
switch (scsi_debug_guard) {
case 1:
-- 
1.7.11.7

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


Re: [PATCH] scsi: fix the build warning

2013-08-21 Thread Joe Perches
On Thu, 2013-08-22 at 08:44 +0800, zwu.ker...@gmail.com wrote:
 From: Zhi Yong Wu wu...@linux.vnet.ibm.com
 
   Initialize csum variable to fix the build warning.

Maybe it'd be better to change the variable
scsi_debug_guard type to bool?

Something like:
---
 drivers/scsi/scsi_debug.c | 22 +++---
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cb4fefa..58375c3 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -97,7 +97,7 @@ static const char * scsi_debug_version_date = 20100324;
 #define DEF_D_SENSE   0
 #define DEF_EVERY_NTH   0
 #define DEF_FAKE_RW0
-#define DEF_GUARD 0
+#define DEF_GUARD false
 #define DEF_LBPU 0
 #define DEF_LBPWS 0
 #define DEF_LBPWS10 0
@@ -169,7 +169,7 @@ static int scsi_debug_dix = DEF_DIX;
 static int scsi_debug_dsense = DEF_D_SENSE;
 static int scsi_debug_every_nth = DEF_EVERY_NTH;
 static int scsi_debug_fake_rw = DEF_FAKE_RW;
-static int scsi_debug_guard = DEF_GUARD;
+static bool scsi_debug_guard = DEF_GUARD;
 static int scsi_debug_lowest_aligned = DEF_LOWEST_ALIGNED;
 static int scsi_debug_max_luns = DEF_MAX_LUNS;
 static int scsi_debug_max_queue = SCSI_DEBUG_CANQUEUE;
@@ -1735,14 +1735,11 @@ static u16 dif_compute_csum(const void *buf, int len)
 {
u16 csum;
 
-   switch (scsi_debug_guard) {
-   case 1:
+   if (scsi_debug_guard)
csum = ip_compute_csum(buf, len);
-   break;
-   case 0:
+   else
csum = cpu_to_be16(crc_t10dif(buf, len));
-   break;
-   }
+
return csum;
 }
 
@@ -2736,7 +2733,7 @@ module_param_named(dix, scsi_debug_dix, int, S_IRUGO);
 module_param_named(dsense, scsi_debug_dsense, int, S_IRUGO | S_IWUSR);
 module_param_named(every_nth, scsi_debug_every_nth, int, S_IRUGO | S_IWUSR);
 module_param_named(fake_rw, scsi_debug_fake_rw, int, S_IRUGO | S_IWUSR);
-module_param_named(guard, scsi_debug_guard, int, S_IRUGO);
+module_param_named(guard, scsi_debug_guard, bool, S_IRUGO);
 module_param_named(lbpu, scsi_debug_lbpu, int, S_IRUGO);
 module_param_named(lbpws, scsi_debug_lbpws, int, S_IRUGO);
 module_param_named(lbpws10, scsi_debug_lbpws10, int, S_IRUGO);
@@ -3312,11 +3309,6 @@ static int __init scsi_debug_init(void)
return -EINVAL;
}
 
-   if (scsi_debug_guard  1) {
-   printk(KERN_ERR scsi_debug_init: guard must be 0 or 1\n);
-   return -EINVAL;
-   }
-
if (scsi_debug_ato  1) {
printk(KERN_ERR scsi_debug_init: ato must be 0 or 1\n);
return -EINVAL;
@@ -4028,7 +4020,7 @@ static int sdebug_driver_probe(struct device * dev)
   (host_prot  SHOST_DIX_TYPE2_PROTECTION) ?  DIX2 : ,
   (host_prot  SHOST_DIX_TYPE3_PROTECTION) ?  DIX3 : );
 
-   if (scsi_debug_guard == 1)
+   if (scsi_debug_guard)
scsi_host_set_guard(hpnt, SHOST_DIX_GUARD_IP);
else
scsi_host_set_guard(hpnt, SHOST_DIX_GUARD_CRC);


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


Re: [PATCH] scsi: fix the build warning

2013-08-21 Thread Zhi Yong Wu
HI,

If you'd like, you should draft one patch for this warning.

On Thu, Aug 22, 2013 at 9:02 AM, Joe Perches j...@perches.com wrote:
 On Thu, 2013-08-22 at 08:44 +0800, zwu.ker...@gmail.com wrote:
 From: Zhi Yong Wu wu...@linux.vnet.ibm.com

   Initialize csum variable to fix the build warning.

 Maybe it'd be better to change the variable
 scsi_debug_guard type to bool?

 Something like:
 ---
  drivers/scsi/scsi_debug.c | 22 +++---
  1 file changed, 7 insertions(+), 15 deletions(-)

 diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
 index cb4fefa..58375c3 100644
 --- a/drivers/scsi/scsi_debug.c
 +++ b/drivers/scsi/scsi_debug.c
 @@ -97,7 +97,7 @@ static const char * scsi_debug_version_date = 20100324;
  #define DEF_D_SENSE   0
  #define DEF_EVERY_NTH   0
  #define DEF_FAKE_RW0
 -#define DEF_GUARD 0
 +#define DEF_GUARD false
  #define DEF_LBPU 0
  #define DEF_LBPWS 0
  #define DEF_LBPWS10 0
 @@ -169,7 +169,7 @@ static int scsi_debug_dix = DEF_DIX;
  static int scsi_debug_dsense = DEF_D_SENSE;
  static int scsi_debug_every_nth = DEF_EVERY_NTH;
  static int scsi_debug_fake_rw = DEF_FAKE_RW;
 -static int scsi_debug_guard = DEF_GUARD;
 +static bool scsi_debug_guard = DEF_GUARD;
  static int scsi_debug_lowest_aligned = DEF_LOWEST_ALIGNED;
  static int scsi_debug_max_luns = DEF_MAX_LUNS;
  static int scsi_debug_max_queue = SCSI_DEBUG_CANQUEUE;
 @@ -1735,14 +1735,11 @@ static u16 dif_compute_csum(const void *buf, int len)
  {
 u16 csum;

 -   switch (scsi_debug_guard) {
 -   case 1:
 +   if (scsi_debug_guard)
 csum = ip_compute_csum(buf, len);
 -   break;
 -   case 0:
 +   else
 csum = cpu_to_be16(crc_t10dif(buf, len));
 -   break;
 -   }
 +
 return csum;
  }

 @@ -2736,7 +2733,7 @@ module_param_named(dix, scsi_debug_dix, int, S_IRUGO);
  module_param_named(dsense, scsi_debug_dsense, int, S_IRUGO | S_IWUSR);
  module_param_named(every_nth, scsi_debug_every_nth, int, S_IRUGO | S_IWUSR);
  module_param_named(fake_rw, scsi_debug_fake_rw, int, S_IRUGO | S_IWUSR);
 -module_param_named(guard, scsi_debug_guard, int, S_IRUGO);
 +module_param_named(guard, scsi_debug_guard, bool, S_IRUGO);
  module_param_named(lbpu, scsi_debug_lbpu, int, S_IRUGO);
  module_param_named(lbpws, scsi_debug_lbpws, int, S_IRUGO);
  module_param_named(lbpws10, scsi_debug_lbpws10, int, S_IRUGO);
 @@ -3312,11 +3309,6 @@ static int __init scsi_debug_init(void)
 return -EINVAL;
 }

 -   if (scsi_debug_guard  1) {
 -   printk(KERN_ERR scsi_debug_init: guard must be 0 or 1\n);
 -   return -EINVAL;
 -   }
 -
 if (scsi_debug_ato  1) {
 printk(KERN_ERR scsi_debug_init: ato must be 0 or 1\n);
 return -EINVAL;
 @@ -4028,7 +4020,7 @@ static int sdebug_driver_probe(struct device * dev)
(host_prot  SHOST_DIX_TYPE2_PROTECTION) ?  DIX2 : ,
(host_prot  SHOST_DIX_TYPE3_PROTECTION) ?  DIX3 : );

 -   if (scsi_debug_guard == 1)
 +   if (scsi_debug_guard)
 scsi_host_set_guard(hpnt, SHOST_DIX_GUARD_IP);
 else
 scsi_host_set_guard(hpnt, SHOST_DIX_GUARD_CRC);





-- 
Regards,

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


Re: [PATCH] scsi: fix the build warning

2013-08-21 Thread Joe Perches
On Thu, 2013-08-22 at 09:05 +0800, Zhi Yong Wu wrote:
 HI,

Hi.

 If you'd like, you should draft one patch for this warning.

Not for me.

I don't get this build warning in the first place and
I think the scsi_debug file is quite old and probably
doesn't need to be changed at all.

I think scsi_debug_guard could be set to a negative
value and then the csum will always be -1.

Dunno if that's a good thing.

Anyway, just a suggestion, do what you want with it.

cheers, Joe

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


Re: [PATCH] scsi: fix the build warning

2013-08-21 Thread Martin K. Petersen
 Joe == Joe Perches j...@perches.com writes:

Joe I don't get this build warning in the first place and I think the
Joe scsi_debug file is quite old and probably doesn't need to be
Joe changed at all.

guard isn't a boolean, it selects the checksum algorithm used.

Also, I believe Akinobu's recent reorganization of this code in question
fixed the warning.

-- 
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 2/4] scsi_debug: fix logical block provisioning support when unmap_alignment != 0

2013-08-21 Thread Martin K. Petersen
 Doug == Douglas Gilbert dgilb...@interlog.com writes:

Doug This is Martin's area of expertise so I hope he also acks it.

Doug Acked-by: Douglas Gilbert dgilb...@interlog.com

I believe I already did, but:

Acked-by: Martin K. Petersen martin.peter...@oracle.com

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


Re: [PATCH] scsi: fix the build warning

2013-08-21 Thread Joe Perches
On Wed, 2013-08-21 at 21:25 -0400, Martin K. Petersen wrote:
  Joe == Joe Perches j...@perches.com writes:
 
 Joe I don't get this build warning in the first place and I think the
 Joe scsi_debug file is quite old and probably doesn't need to be
 Joe changed at all.
 
 guard isn't a boolean, it selects the checksum algorithm used.

OK, maybe this then...
---
 drivers/scsi/scsi_debug.c | 2 +-
 1 file changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cb4fefa..6fc2831 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3312,7 +3312,7 @@ static int __init scsi_debug_init(void)
return -EINVAL;
}
 
-   if (scsi_debug_guard  1) {
+   if (scsi_debug_guard  0 || scsi_debug_guard  1) {
printk(KERN_ERR scsi_debug_init: guard must be 0 or 1\n);
return -EINVAL;
}


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


Re: [PATCH] scsi: fix the build warning

2013-08-21 Thread Zhi Yong Wu
On Thu, Aug 22, 2013 at 9:55 AM, Joe Perches j...@perches.com wrote:
 On Wed, 2013-08-21 at 21:25 -0400, Martin K. Petersen wrote:
  Joe == Joe Perches j...@perches.com writes:

 Joe I don't get this build warning in the first place and I think the
 Joe scsi_debug file is quite old and probably doesn't need to be
 Joe changed at all.

 guard isn't a boolean, it selects the checksum algorithm used.

 OK, maybe this then...
 ---
  drivers/scsi/scsi_debug.c | 2 +-
  1 file changed, 1 insertions(+), 1 deletions(-)

 diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
 index cb4fefa..6fc2831 100644
 --- a/drivers/scsi/scsi_debug.c
 +++ b/drivers/scsi/scsi_debug.c
 @@ -3312,7 +3312,7 @@ static int __init scsi_debug_init(void)
 return -EINVAL;
 }

 -   if (scsi_debug_guard  1) {
 +   if (scsi_debug_guard  0 || scsi_debug_guard  1) {
I don't think that it can fix that issue.
 printk(KERN_ERR scsi_debug_init: guard must be 0 or 1\n);
 return -EINVAL;
 }





-- 
Regards,

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


Re: [PATCH] scsi: fix the build warning

2013-08-21 Thread Joe Perches
On Thu, 2013-08-22 at 09:59 +0800, Zhi Yong Wu wrote:
 On Thu, Aug 22, 2013 at 9:55 AM, Joe Perches j...@perches.com wrote:
  On Wed, 2013-08-21 at 21:25 -0400, Martin K. Petersen wrote:
   Joe == Joe Perches j...@perches.com writes:
 
  Joe I don't get this build warning in the first place and I think the
  Joe scsi_debug file is quite old and probably doesn't need to be
  Joe changed at all.
 
  guard isn't a boolean, it selects the checksum algorithm used.
 
  OK, maybe this then...
  ---
   drivers/scsi/scsi_debug.c | 2 +-
   1 file changed, 1 insertions(+), 1 deletions(-)
 
  diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
  index cb4fefa..6fc2831 100644
  --- a/drivers/scsi/scsi_debug.c
  +++ b/drivers/scsi/scsi_debug.c
  @@ -3312,7 +3312,7 @@ static int __init scsi_debug_init(void)
  return -EINVAL;
  }
 
  -   if (scsi_debug_guard  1) {
  +   if (scsi_debug_guard  0 || scsi_debug_guard  1) {
 I don't think that it can fix that issue.

No, it doesn't fix any compile warning.  Your patch, if
it's needed, would still apply.

This patch suggestion fixes an issue where debug_guard
could be set to a negative value.

I didn't sign it, it's up to Martin or I suppose James
to actually want it done.

cheers, Joe

--
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 12/12] target: Release COMPARE_AND_WRITE mutex in generic failure path

2013-08-21 Thread Nicholas A. Bellinger
From: Nicholas Bellinger n...@daterainc.com

This patch adds a extra check for SCF_COMPARE_AND_WRITE within
transport_generic_request_failure() to invoke the callback for
compare_and_write_callback() or compare_and_write_done(), in
order to release se_dev-caw_mutex from the generic failure
path.

It also adds to checks within compare_and_write_callback() to
avoid processing when transport_generic_request_failure() occurs
early enough that cmd-t_data_sg or cmd-t_bidi_data_sg have not
been setup yet, nor se_dev-caw_mutex obtained from within
sbc_compare_and_write().

Cc: Christoph Hellwig h...@lst.de
Cc: Hannes Reinecke h...@suse.de
Cc: Martin Petersen martin.peter...@oracle.com
Cc: Chris Mason chris.ma...@fusionio.com
Cc: James Bottomley jbottom...@parallels.com
Cc: Nicholas Bellinger n...@linux-iscsi.org
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
 drivers/target/target_core_sbc.c   |7 +++
 drivers/target/target_core_transport.c |6 ++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index bb1b42b..287e960 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -372,6 +372,13 @@ static sense_reason_t compare_and_write_callback(struct 
se_cmd *cmd)
sense_reason_t ret = TCM_NO_SENSE;
int rc, i;
 
+   /*
+* Handle early failure in transport_generic_request_failure(),
+* which will not have taken -caw_mutex yet..
+*/
+   if (!cmd-t_data_sg || !cmd-t_bidi_data_sg)
+   return TCM_NO_SENSE;
+
buf = kzalloc(cmd-data_length, GFP_KERNEL);
if (!buf) {
pr_err(Unable to allocate compare_and_write buf\n);
diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index a95e799..3009cda 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1560,6 +1560,12 @@ void transport_generic_request_failure(struct se_cmd 
*cmd,
 * For SAM Task Attribute emulation for failed struct se_cmd
 */
transport_complete_task_attr(cmd);
+   /*
+* Handle special case for COMPARE_AND_WRITE failure, where the
+* callback is expected to drop the per device -caw_mutex.
+*/
+   if (cmd-se_cmd_flags  SCF_COMPARE_AND_WRITE)
+   cmd-transport_complete_callback(cmd);
 
switch (sense_reason) {
case TCM_NON_EXISTENT_LUN:
-- 
1.7.2.5

--
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 10/12] target: Add support for COMPARE_AND_WRITE emulation

2013-08-21 Thread Nicholas A. Bellinger
From: Nicholas Bellinger n...@daterainc.com

This patch adds support for COMPARE_AND_WRITE emulation on a per block
basis.  This logic is used as an atomic test and set primative currently
used by VMWare ESX VAAI for performing array side locking of individual
VMFS extent ownership.

This includes the COMPARE_AND_WRITE CDB parsing within sbc_parse_cdb(),
and does the majority of the work within the compare_and_write_callback()
to perform the verify instance user data comparision, and subsequent
write instance user data I/O submission upon a successfull comparision.

The synchronization is enforced by se_device-caw_mutex, that is obtained
before the initial READ I/O submission in sbc_compare_and_write().  The
mutex is then released upon MISCOMPARE in compare_and_write_callback(),
or upon WRITE instance user-data completion in compare_and_write_post().

The implementation currently assumes a single logical block (NoLB=1).

v2 changes:
 - Set SCF_COMPARE_AND_WRITE and cmd-execute_cmd() to
   sbc_compare_and_write() during setup in sbc_parse_cdb()
 - Use sbc_compare_and_write() for initial READ submission with
   DMA_FROM_DEVICE
 - Reset cmd-execute_cmd() to sbc_execute_rw() for write instance
   user-data in compare_and_write_callback()
 - Drop SCF_BIDI command flag usage
 - Set TRANSPORT_PROCESSING + transport_state flags before write
   instance submission, and convert to __target_execute_cmd()
 - Prevent sbc_get_size() from being being called twice to
   generate incorrect size in sbc_parse_cdb()
 - Enforce se_device-caw_mutex synchronization between initial
   READ I/O submission, and final WRITE I/O completion.

Cc: Christoph Hellwig h...@lst.de
Cc: Hannes Reinecke h...@suse.de
Cc: Martin Petersen martin.peter...@oracle.com
Cc: Chris Mason chris.ma...@fusionio.com
Cc: James Bottomley jbottom...@parallels.com
Cc: Nicholas Bellinger n...@linux-iscsi.org
Signed-off-by: Nicholas Bellinger n...@daterainc.com
---
 drivers/target/target_core_device.c |1 +
 drivers/target/target_core_sbc.c|  190 ++-
 include/target/target_core_base.h   |1 +
 3 files changed, 191 insertions(+), 1 deletions(-)

diff --git a/drivers/target/target_core_device.c 
b/drivers/target/target_core_device.c
index 0b5f868..de89046 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -1413,6 +1413,7 @@ struct se_device *target_alloc_device(struct se_hba *hba, 
const char *name)
spin_lock_init(dev-se_port_lock);
spin_lock_init(dev-se_tmr_lock);
spin_lock_init(dev-qf_cmd_lock);
+   mutex_init(dev-caw_mutex);
atomic_set(dev-dev_ordered_id, 0);
INIT_LIST_HEAD(dev-t10_wwn.t10_vpd_list);
spin_lock_init(dev-t10_wwn.t10_vpd_lock);
diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index 5569b36..4076828 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -25,6 +25,7 @@
 #include linux/ratelimit.h
 #include asm/unaligned.h
 #include scsi/scsi.h
+#include scsi/scsi_tcq.h
 
 #include target/target_core_base.h
 #include target/target_core_backend.h
@@ -344,6 +345,170 @@ sbc_execute_rw(struct se_cmd *cmd)
   cmd-data_direction);
 }
 
+static sense_reason_t compare_and_write_post(struct se_cmd *cmd)
+{
+   struct se_device *dev = cmd-se_dev;
+
+   cmd-se_cmd_flags |= SCF_COMPARE_AND_WRITE_POST;
+   /*
+* Unlock -caw_mutex originally obtained during sbc_compare_and_write()
+* before the original READ I/O submission.
+*/
+   mutex_unlock(dev-caw_mutex);
+
+   return TCM_NO_SENSE;
+}
+
+static sense_reason_t compare_and_write_callback(struct se_cmd *cmd)
+{
+   struct se_device *dev = cmd-se_dev;
+   struct scatterlist *write_sg = NULL, *sg;
+   unsigned char *buf, *addr;
+   struct sg_mapping_iter m;
+   unsigned int offset = 0, len;
+   unsigned int nlbas = cmd-t_task_nolb;
+   unsigned int block_size = dev-dev_attrib.block_size;
+   unsigned int compare_len = (nlbas * block_size);
+   sense_reason_t ret = TCM_NO_SENSE;
+   int rc, i;
+
+   buf = kzalloc(cmd-data_length, GFP_KERNEL);
+   if (!buf) {
+   pr_err(Unable to allocate compare_and_write buf\n);
+   return TCM_OUT_OF_RESOURCES;
+   }
+
+   write_sg = kzalloc(sizeof(struct scatterlist) * cmd-t_data_nents,
+  GFP_KERNEL);
+   if (!write_sg) {
+   pr_err(Unable to allocate compare_and_write sg\n);
+   ret = TCM_OUT_OF_RESOURCES;
+   goto out;
+   }
+   /*
+* Setup verify and write data payloads from total NumberLBAs.
+*/
+   rc = sg_copy_to_buffer(cmd-t_data_sg, cmd-t_data_nents, buf,
+  cmd-data_length);
+   if (!rc) {
+   pr_err(sg_copy_to_buffer() failed for compare_and_write\n);
+   ret = 

[PATCH-v2 11/12] target: Add compare_and_write_post() completion callback fall through

2013-08-21 Thread Nicholas A. Bellinger
From: Nicholas Bellinger n...@daterainc.com

This patch changes target_complete_ok_work() to fall through
after calling the se_cmd-transport_complete_callback() -
compare_and_write_post() callback, by keying off the existance
of SCF_COMPARE_AND_WRITE_POST.

This is necessary because once SCF_COMPARE_AND_WRITE_POST has
been set by compare_and_write_post(), the SCSI response needs
to be sent via TFO-queue_status().

Cc: Christoph Hellwig h...@lst.de
Cc: Hannes Reinecke h...@suse.de
Cc: Martin Petersen martin.peter...@oracle.com
Cc: Chris Mason chris.ma...@fusionio.com
Cc: James Bottomley jbottom...@parallels.com
Cc: Nicholas Bellinger n...@linux-iscsi.org
Signed-off-by: Nicholas Bellinger n...@daterainc.com
---
 drivers/target/target_core_sbc.c   |3 ++-
 drivers/target/target_core_transport.c |   21 +++--
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index 4076828..bb1b42b 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -375,7 +375,8 @@ static sense_reason_t compare_and_write_callback(struct 
se_cmd *cmd)
buf = kzalloc(cmd-data_length, GFP_KERNEL);
if (!buf) {
pr_err(Unable to allocate compare_and_write buf\n);
-   return TCM_OUT_OF_RESOURCES;
+   ret = TCM_OUT_OF_RESOURCES;
+   goto out;
}
 
write_sg = kzalloc(sizeof(struct scatterlist) * cmd-t_data_nents,
diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index dc39f1f..a95e799 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1910,17 +1910,18 @@ static void target_complete_ok_work(struct work_struct 
*work)
sense_reason_t rc;
 
rc = cmd-transport_complete_callback(cmd);
-   if (!rc)
+   if (!rc  !(cmd-se_cmd_flags  SCF_COMPARE_AND_WRITE_POST)) {
return;
+   } else if (rc) {
+   ret = transport_send_check_condition_and_sense(cmd,
+   rc, 0);
+   if (ret == -EAGAIN || ret == -ENOMEM)
+   goto queue_full;
 
-   ret = transport_send_check_condition_and_sense(cmd,
-   rc, 0);
-   if (ret == -EAGAIN || ret == -ENOMEM)
-   goto queue_full;
-
-   transport_lun_remove_cmd(cmd);
-   transport_cmd_check_stop_to_fabric(cmd);
-   return;
+   transport_lun_remove_cmd(cmd);
+   transport_cmd_check_stop_to_fabric(cmd);
+   return;
+   }
}
 
switch (cmd-data_direction) {
-- 
1.7.2.5

--
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 09/12] target: Add MAXIMUM COMPARE AND WRITE LENGTH in Block Limits VPD

2013-08-21 Thread Nicholas A. Bellinger
From: Nicholas Bellinger n...@daterainc.com

This patch adds the MAXIMUM COMPARE AND WRITE LENGTH bit, currently
hardcoded to a single logical block (NoLB=1) within the Block Limits
VPD in spc_emulate_evpd_b0().

Also add emulate_caw device attribute in configfs (enabled by default)
to allow the exposure of this bit to be disabled, if necessary.

Cc: Christoph Hellwig h...@lst.de
Cc: Hannes Reinecke h...@suse.de
Cc: Martin Petersen martin.peter...@oracle.com
Cc: Chris Mason chris.ma...@fusionio.com
Cc: James Bottomley jbottom...@parallels.com
Cc: Nicholas Bellinger n...@linux-iscsi.org
Signed-off-by: Nicholas Bellinger n...@daterainc.com
---
 drivers/target/target_core_configfs.c |4 
 drivers/target/target_core_device.c   |   14 ++
 drivers/target/target_core_internal.h |1 +
 drivers/target/target_core_spc.c  |5 +
 include/target/target_core_base.h |3 +++
 5 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/drivers/target/target_core_configfs.c 
b/drivers/target/target_core_configfs.c
index f67a9af..24517d4 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -636,6 +636,9 @@ SE_DEV_ATTR(emulate_tpu, S_IRUGO | S_IWUSR);
 DEF_DEV_ATTRIB(emulate_tpws);
 SE_DEV_ATTR(emulate_tpws, S_IRUGO | S_IWUSR);
 
+DEF_DEV_ATTRIB(emulate_caw);
+SE_DEV_ATTR(emulate_caw, S_IRUGO | S_IWUSR);
+
 DEF_DEV_ATTRIB(enforce_pr_isids);
 SE_DEV_ATTR(enforce_pr_isids, S_IRUGO | S_IWUSR);
 
@@ -693,6 +696,7 @@ static struct configfs_attribute 
*target_core_dev_attrib_attrs[] = {
target_core_dev_attrib_emulate_tas.attr,
target_core_dev_attrib_emulate_tpu.attr,
target_core_dev_attrib_emulate_tpws.attr,
+   target_core_dev_attrib_emulate_caw.attr,
target_core_dev_attrib_enforce_pr_isids.attr,
target_core_dev_attrib_is_nonrot.attr,
target_core_dev_attrib_emulate_rest_reord.attr,
diff --git a/drivers/target/target_core_device.c 
b/drivers/target/target_core_device.c
index 8f4142f..0b5f868 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -890,6 +890,19 @@ int se_dev_set_emulate_tpws(struct se_device *dev, int 
flag)
return 0;
 }
 
+int se_dev_set_emulate_caw(struct se_device *dev, int flag)
+{
+   if (flag != 0  flag != 1) {
+   pr_err(Illegal value %d\n, flag);
+   return -EINVAL;
+   }
+   dev-dev_attrib.emulate_caw = flag;
+   pr_debug(dev[%p]: SE Device CompareAndWrite (AtomicTestandSet): %d\n,
+dev, flag);
+
+   return 0;
+}
+
 int se_dev_set_enforce_pr_isids(struct se_device *dev, int flag)
 {
if ((flag != 0)  (flag != 1)) {
@@ -1423,6 +1436,7 @@ struct se_device *target_alloc_device(struct se_hba *hba, 
const char *name)
dev-dev_attrib.emulate_tas = DA_EMULATE_TAS;
dev-dev_attrib.emulate_tpu = DA_EMULATE_TPU;
dev-dev_attrib.emulate_tpws = DA_EMULATE_TPWS;
+   dev-dev_attrib.emulate_caw = DA_EMULATE_CAW;
dev-dev_attrib.enforce_pr_isids = DA_ENFORCE_PR_ISIDS;
dev-dev_attrib.is_nonrot = DA_IS_NONROT;
dev-dev_attrib.emulate_rest_reord = DA_EMULATE_REST_REORD;
diff --git a/drivers/target/target_core_internal.h 
b/drivers/target/target_core_internal.h
index 18d49df..805ceb4 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -33,6 +33,7 @@ int   se_dev_set_emulate_ua_intlck_ctrl(struct se_device *, 
int);
 intse_dev_set_emulate_tas(struct se_device *, int);
 intse_dev_set_emulate_tpu(struct se_device *, int);
 intse_dev_set_emulate_tpws(struct se_device *, int);
+intse_dev_set_emulate_caw(struct se_device *, int);
 intse_dev_set_enforce_pr_isids(struct se_device *, int);
 intse_dev_set_is_nonrot(struct se_device *, int);
 intse_dev_set_emulate_rest_reord(struct se_device *dev, int);
diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 4cb667d..ed7077a 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -457,6 +457,11 @@ spc_emulate_evpd_b0(struct se_cmd *cmd, unsigned char *buf)
 
/* Set WSNZ to 1 */
buf[4] = 0x01;
+   /*
+* Set MAXIMUM COMPARE AND WRITE LENGTH
+*/
+   if (dev-dev_attrib.emulate_caw)
+   buf[5] = 0x01;
 
/*
 * Set OPTIMAL TRANSFER LENGTH GRANULARITY
diff --git a/include/target/target_core_base.h 
b/include/target/target_core_base.h
index eb439e7..53eea33 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -97,6 +97,8 @@
  * block/blk-lib.c:blkdev_issue_discard()
  */
 #define DA_EMULATE_TPWS0
+/* Emulation for CompareAndWrite (AtomicTestandSet) by default */
+#define DA_EMULATE_CAW 1
 /* No Emulation for PSCSI by default */
 #define DA_EMULATE_ALUA0
 /* Enforce SCSI 

[PATCH-v2 07/12] target: Add transport_reset_sgl_orig() for COMPARE_AND_WRITE

2013-08-21 Thread Nicholas A. Bellinger
From: Nicholas Bellinger n...@daterainc.com

After COMPARE_AND_WRITE completes it's comparision, the WRITE
payload SGLs head expect to be updated to point from the verify
instance of user data, to the write instance of user data.

So for this special case, add transport_reset_sgl_orig() usage
within transport_free_pages() and add se_cmd-t_data_[sg,nents]_orig
members to save the original assignments.

Cc: Christoph Hellwig h...@lst.de
Cc: Hannes Reinecke h...@suse.de
Cc: Martin Petersen martin.peter...@oracle.com
Cc: Chris Mason chris.ma...@fusionio.com
Cc: James Bottomley jbottom...@parallels.com
Cc: Nicholas Bellinger n...@linux-iscsi.org
Signed-off-by: Nicholas Bellinger n...@daterainc.com
---
 drivers/target/target_core_transport.c |   21 -
 include/target/target_core_base.h  |2 ++
 2 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 967dac7..5236a80 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1992,10 +1992,29 @@ static inline void transport_free_sgl(struct 
scatterlist *sgl, int nents)
kfree(sgl);
 }
 
+static inline void transport_reset_sgl_orig(struct se_cmd *cmd)
+{
+   /*
+* Check for saved t_data_sg that may be used for COMPARE_AND_WRITE
+* emulation, and free + reset pointers if necessary..
+*/
+   if (!cmd-t_data_sg_orig)
+   return;
+
+   kfree(cmd-t_data_sg);
+   cmd-t_data_sg = cmd-t_data_sg_orig;
+   cmd-t_data_sg_orig = NULL;
+   cmd-t_data_nents = cmd-t_data_nents_orig;
+   cmd-t_data_nents_orig = 0;
+}
+
 static inline void transport_free_pages(struct se_cmd *cmd)
 {
-   if (cmd-se_cmd_flags  SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC)
+   if (cmd-se_cmd_flags  SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC) {
+   transport_reset_sgl_orig(cmd);
return;
+   }
+   transport_reset_sgl_orig(cmd);
 
transport_free_sgl(cmd-t_data_sg, cmd-t_data_nents);
cmd-t_data_sg = NULL;
diff --git a/include/target/target_core_base.h 
b/include/target/target_core_base.h
index 0c3f47f..eb439e7 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -478,7 +478,9 @@ struct se_cmd {
struct work_struct  work;
 
struct scatterlist  *t_data_sg;
+   struct scatterlist  *t_data_sg_orig;
unsigned intt_data_nents;
+   unsigned intt_data_nents_orig;
void*t_data_vmap;
struct scatterlist  *t_bidi_data_sg;
unsigned intt_bidi_data_nents;
-- 
1.7.2.5

--
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 05/12] target: Convert se_cmd-t_bidi_data_sg checks to use SCF_BIDI

2013-08-21 Thread Nicholas A. Bellinger
From: Nicholas Bellinger n...@daterainc.com

Stop keying off se_cmd-t_bidi_data_sg within transport_complete_qf()
+ target_complete_ok_work(), and just use SCF_BIDI instead.

Cc: Christoph Hellwig h...@lst.de
Cc: Hannes Reinecke h...@suse.de
Cc: Martin Petersen martin.peter...@oracle.com
Cc: Chris Mason chris.ma...@fusionio.com
Cc: James Bottomley jbottom...@parallels.com
Cc: Nicholas Bellinger n...@linux-iscsi.org
Signed-off-by: Nicholas Bellinger n...@daterainc.com
---
 drivers/target/target_core_transport.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 3d3dc97..781859e 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1832,7 +1832,7 @@ static void transport_complete_qf(struct se_cmd *cmd)
ret = cmd-se_tfo-queue_data_in(cmd);
break;
case DMA_TO_DEVICE:
-   if (cmd-t_bidi_data_sg) {
+   if (cmd-se_cmd_flags  SCF_BIDI) {
ret = cmd-se_tfo-queue_data_in(cmd);
if (ret  0)
break;
@@ -1947,7 +1947,7 @@ static void target_complete_ok_work(struct work_struct 
*work)
/*
 * Check if we need to send READ payload for BIDI-COMMAND
 */
-   if (cmd-t_bidi_data_sg) {
+   if (cmd-se_cmd_flags  SCF_BIDI) {
spin_lock(cmd-se_lun-lun_sep_lock);
if (cmd-se_lun-lun_sep) {
cmd-se_lun-lun_sep-sep_stats.tx_data_octets 
+=
-- 
1.7.2.5

--
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 06/12] target: Add memory allocation for bidirectional commands

2013-08-21 Thread Nicholas A. Bellinger
From: Nicholas Bellinger n...@daterainc.com

This adds transport_generic_get_mem_bidi() to perform scatterlist
allocation for bidirectional commands.

Also, update transport_generic_new_cmd() to call this new function
when SCF_BIDI has been set.

v2 Changes:
  - Use SCF_COMPARE_AND_WRITE instead of CDB based check for
calculating length in transport_generic_get_mem_bidi().
  - Use SCF_COMPARE_AND_WRITE in transport_generic_new_cmd()
for determing when to call transport_generic_get_mem_bidi()

Cc: Christoph Hellwig h...@lst.de
Cc: Hannes Reinecke h...@suse.de
Cc: Martin Petersen martin.peter...@oracle.com
Cc: Chris Mason chris.ma...@fusionio.com
Cc: James Bottomley jbottom...@parallels.com
Cc: Nicholas Bellinger n...@linux-iscsi.org
Signed-off-by: Nicholas Bellinger n...@daterainc.com
---
 drivers/target/target_core_transport.c |   54 
 1 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 781859e..967dac7 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -2092,6 +2092,53 @@ void transport_kunmap_data_sg(struct se_cmd *cmd)
 EXPORT_SYMBOL(transport_kunmap_data_sg);
 
 static int
+transport_generic_get_mem_bidi(struct se_cmd *cmd)
+{
+   struct se_device *dev = cmd-se_dev;
+   struct page *page;
+   gfp_t zero_flag;
+   u32 length;
+   unsigned int nents;
+   int i = 0;
+
+   if (cmd-se_cmd_flags  SCF_COMPARE_AND_WRITE)
+   length = cmd-t_task_nolb * dev-dev_attrib.block_size;
+   else
+   length = cmd-data_length;
+
+   nents = DIV_ROUND_UP(length, PAGE_SIZE);
+   cmd-t_bidi_data_sg = kmalloc(sizeof(struct scatterlist) * nents, 
GFP_KERNEL);
+   if (!cmd-t_bidi_data_sg)
+   return -ENOMEM;
+
+   cmd-t_bidi_data_nents = nents;
+   sg_init_table(cmd-t_bidi_data_sg, nents);
+
+   zero_flag = cmd-se_cmd_flags  SCF_SCSI_DATA_CDB ? 0 : __GFP_ZERO;
+
+   while (length) {
+   u32 page_len = min_t(u32, length, PAGE_SIZE);
+   page = alloc_page(GFP_KERNEL | zero_flag);
+   if (!page)
+   goto out;
+
+   sg_set_page(cmd-t_bidi_data_sg[i], page, page_len, 0);
+   length -= page_len;
+   i++;
+   }
+   return 0;
+
+out:
+   while (i  0) {
+   i--;
+   __free_page(sg_page(cmd-t_bidi_data_sg[i]));
+   }
+   kfree(cmd-t_bidi_data_sg);
+   cmd-t_bidi_data_sg = NULL;
+   return -ENOMEM;
+}
+
+static int
 transport_generic_get_mem(struct se_cmd *cmd)
 {
u32 length = cmd-data_length;
@@ -2149,6 +2196,13 @@ transport_generic_new_cmd(struct se_cmd *cmd)
 */
if (!(cmd-se_cmd_flags  SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC) 
cmd-data_length) {
+   if ((cmd-se_cmd_flags  SCF_BIDI) ||
+   (cmd-se_cmd_flags  SCF_COMPARE_AND_WRITE)) {
+   ret = transport_generic_get_mem_bidi(cmd);
+   if (ret  0)
+   return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+   }
+
ret = transport_generic_get_mem(cmd);
if (ret  0)
return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
-- 
1.7.2.5

--
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 01/12] scsi: Add CDB definition for COMPARE_AND_WRITE

2013-08-21 Thread Nicholas A. Bellinger
From: Nicholas Bellinger n...@daterainc.com

Reviewed-by: Christoph Hellwig h...@lst.de
Cc: Hannes Reinecke h...@suse.de
Cc: Martin Petersen martin.peter...@oracle.com
Cc: Chris Mason chris.ma...@fusionio.com
Cc: James Bottomley jbottom...@parallels.com
Cc: Nicholas Bellinger n...@linux-iscsi.org
Signed-off-by: Nicholas Bellinger n...@daterainc.com
---
 include/scsi/scsi.h |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 4b87d99..6268062 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -144,6 +144,7 @@ enum scsi_timeouts {
 #define ACCESS_CONTROL_IN 0x86
 #define ACCESS_CONTROL_OUT0x87
 #define READ_16   0x88
+#define COMPARE_AND_WRITE 0x89
 #define WRITE_16  0x8a
 #define READ_ATTRIBUTE0x8c
 #define WRITE_ATTRIBUTE  0x8d
-- 
1.7.2.5

--
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 03/12] target: Add TCM_MISCOMPARE_VERIFY sense handling

2013-08-21 Thread Nicholas A. Bellinger
From: Nicholas Bellinger n...@daterainc.com

This patch adds TCM_MISCOMPARE_VERIFY (ASC=0x1d, ASCQ=0x00) sense
handling to transport_send_check_condition_and_sense(), which is
required for a COMPARE_AND_WRITE comparision failure.

Cc: Christoph Hellwig h...@lst.de
Cc: Hannes Reinecke h...@suse.de
Cc: Martin Petersen martin.peter...@oracle.com
Cc: Chris Mason chris.ma...@fusionio.com
Cc: James Bottomley jbottom...@parallels.com
Cc: Nicholas Bellinger n...@linux-iscsi.org
Signed-off-by: Nicholas Bellinger n...@daterainc.com
---
 drivers/target/target_core_transport.c |9 +
 include/target/target_core_base.h  |1 +
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 53d1d75..3d3dc97 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -2791,6 +2791,15 @@ transport_send_check_condition_and_sense(struct se_cmd 
*cmd,
buffer[SPC_ASC_KEY_OFFSET] = asc;
buffer[SPC_ASCQ_KEY_OFFSET] = ascq;
break;
+   case TCM_MISCOMPARE_VERIFY:
+   /* CURRENT ERROR */
+   buffer[0] = 0x70;
+   buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10;
+   buffer[SPC_SENSE_KEY_OFFSET] = MISCOMPARE;
+   /* MISCOMPARE DURING VERIFY OPERATION */
+   buffer[SPC_ASC_KEY_OFFSET] = 0x1d;
+   buffer[SPC_ASCQ_KEY_OFFSET] = 0x00;
+   break;
case TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE:
default:
/* CURRENT ERROR */
diff --git a/include/target/target_core_base.h 
b/include/target/target_core_base.h
index 6e946f3..fac25c5 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -197,6 +197,7 @@ enum tcm_sense_reason_table {
TCM_ADDRESS_OUT_OF_RANGE= R(0x11),
TCM_OUT_OF_RESOURCES= R(0x12),
TCM_PARAMETER_LIST_LENGTH_ERROR = R(0x13),
+   TCM_MISCOMPARE_VERIFY   = R(0x14),
 #undef R
 };
 
-- 
1.7.2.5

--
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 04/12] target: Allow sbc_ops-execute_rw() to accept SGLs + data_direction

2013-08-21 Thread Nicholas A. Bellinger
From: Nicholas Bellinger n...@daterainc.com

COMPARE_AND_WRITE expects to be able to send down a DMA_FROM_DEVICE
to obtain the necessary READ payload for comparision against the
first half of the WRITE payload containing the verify user data.

Currently virtual backends expect to internally reference SGLs,
SGL nents, and data_direction, so change IBLOCK, FILEIO and RD
sbc_ops-execute_rw() to accept this values as function parameters.

Also add default sbc_execute_rw() handler for the typical case for
cmd-execute_rw() submission using cmd-t_data_sg, cmd-t_data_nents,
and cmd-data_direction).

v2 Changes:
  - Add SCF_COMPARE_AND_WRITE command flag
  - Use sbc_execute_rw() for normal cmd-execute_rw() submission
with expected se_cmd members.

Cc: Christoph Hellwig h...@lst.de
Cc: Hannes Reinecke h...@suse.de
Cc: Martin Petersen martin.peter...@oracle.com
Cc: Chris Mason chris.ma...@fusionio.com
Cc: James Bottomley jbottom...@parallels.com
Cc: Nicholas Bellinger n...@linux-iscsi.org
Signed-off-by: Nicholas Bellinger n...@daterainc.com
---
 drivers/target/target_core_file.c|6 +---
 drivers/target/target_core_iblock.c  |6 +---
 drivers/target/target_core_rd.c  |6 +---
 drivers/target/target_core_sbc.c |   37 -
 include/target/target_core_backend.h |3 +-
 include/target/target_core_base.h|5 
 6 files changed, 40 insertions(+), 23 deletions(-)

diff --git a/drivers/target/target_core_file.c 
b/drivers/target/target_core_file.c
index bc3245d..c5448a5 100644
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -547,11 +547,9 @@ fd_execute_unmap(struct se_cmd *cmd)
 }
 
 static sense_reason_t
-fd_execute_rw(struct se_cmd *cmd)
+fd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
+ enum dma_data_direction data_direction)
 {
-   struct scatterlist *sgl = cmd-t_data_sg;
-   u32 sgl_nents = cmd-t_data_nents;
-   enum dma_data_direction data_direction = cmd-data_direction;
struct se_device *dev = cmd-se_dev;
int ret = 0;
 
diff --git a/drivers/target/target_core_iblock.c 
b/drivers/target/target_core_iblock.c
index 0a460f3..81464eb 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -587,11 +587,9 @@ static ssize_t iblock_show_configfs_dev_params(struct 
se_device *dev, char *b)
 }
 
 static sense_reason_t
-iblock_execute_rw(struct se_cmd *cmd)
+iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
+ enum dma_data_direction data_direction)
 {
-   struct scatterlist *sgl = cmd-t_data_sg;
-   u32 sgl_nents = cmd-t_data_nents;
-   enum dma_data_direction data_direction = cmd-data_direction;
struct se_device *dev = cmd-se_dev;
struct iblock_req *ibr;
struct bio *bio;
diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
index 51127d1..958d17ad 100644
--- a/drivers/target/target_core_rd.c
+++ b/drivers/target/target_core_rd.c
@@ -280,11 +280,9 @@ static struct rd_dev_sg_table *rd_get_sg_table(struct 
rd_dev *rd_dev, u32 page)
 }
 
 static sense_reason_t
-rd_execute_rw(struct se_cmd *cmd)
+rd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
+ enum dma_data_direction data_direction)
 {
-   struct scatterlist *sgl = cmd-t_data_sg;
-   u32 sgl_nents = cmd-t_data_nents;
-   enum dma_data_direction data_direction = cmd-data_direction;
struct se_device *se_dev = cmd-se_dev;
struct rd_dev *dev = RD_DEV(se_dev);
struct rd_dev_sg_table *table;
diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index be5234a..5569b36 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -337,6 +337,13 @@ out:
return ret;
 }
 
+static sense_reason_t
+sbc_execute_rw(struct se_cmd *cmd)
+{
+   return cmd-execute_rw(cmd, cmd-t_data_sg, cmd-t_data_nents,
+  cmd-data_direction);
+}
+
 sense_reason_t
 sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
 {
@@ -351,31 +358,36 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
sectors = transport_get_sectors_6(cdb);
cmd-t_task_lba = transport_lba_21(cdb);
cmd-se_cmd_flags |= SCF_SCSI_DATA_CDB;
-   cmd-execute_cmd = ops-execute_rw;
+   cmd-execute_rw = ops-execute_rw;
+   cmd-execute_cmd = sbc_execute_rw;
break;
case READ_10:
sectors = transport_get_sectors_10(cdb);
cmd-t_task_lba = transport_lba_32(cdb);
cmd-se_cmd_flags |= SCF_SCSI_DATA_CDB;
-   cmd-execute_cmd = ops-execute_rw;
+   cmd-execute_rw = ops-execute_rw;
+   cmd-execute_cmd = sbc_execute_rw;
break;
case READ_12:
sectors = 

[PATCH-v2 02/12] target: Add return for se_cmd-transport_complete_callback

2013-08-21 Thread Nicholas A. Bellinger
From: Nicholas Bellinger n...@daterainc.com

This patch adds a sense_reason_t return to -transport_complete_callback(),
and updates target_complete_ok_work() to invoke the call if necessary to
transport_send_check_condition_and_sense() during the failure case.

Also update xdreadwrite_callback() to use this return value.

Cc: Christoph Hellwig h...@lst.de
Cc: Hannes Reinecke h...@suse.de
Cc: Martin Petersen martin.peter...@oracle.com
Cc: Chris Mason chris.ma...@fusionio.com
Cc: James Bottomley jbottom...@parallels.com
Cc: Nicholas Bellinger n...@linux-iscsi.org
Signed-off-by: Nicholas Bellinger n...@daterainc.com
---
 drivers/target/target_core_sbc.c   |   13 -
 drivers/target/target_core_transport.c |   20 +---
 include/target/target_core_base.h  |2 +-
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index 8a46277..be5234a 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -280,13 +280,13 @@ sbc_setup_write_same(struct se_cmd *cmd, unsigned char 
*flags, struct sbc_ops *o
return 0;
 }
 
-static void xdreadwrite_callback(struct se_cmd *cmd)
+static sense_reason_t xdreadwrite_callback(struct se_cmd *cmd)
 {
unsigned char *buf, *addr;
struct scatterlist *sg;
unsigned int offset;
-   int i;
-   int count;
+   sense_reason_t ret = TCM_NO_SENSE;
+   int i, count;
/*
 * From sbc3r22.pdf section 5.48 XDWRITEREAD (10) command
 *
@@ -301,7 +301,7 @@ static void xdreadwrite_callback(struct se_cmd *cmd)
buf = kmalloc(cmd-data_length, GFP_KERNEL);
if (!buf) {
pr_err(Unable to allocate xor_callback buf\n);
-   return;
+   return TCM_OUT_OF_RESOURCES;
}
/*
 * Copy the scatterlist WRITE buffer located at cmd-t_data_sg
@@ -320,8 +320,10 @@ static void xdreadwrite_callback(struct se_cmd *cmd)
offset = 0;
for_each_sg(cmd-t_bidi_data_sg, sg, cmd-t_bidi_data_nents, count) {
addr = kmap_atomic(sg_page(sg));
-   if (!addr)
+   if (!addr) {
+   ret = TCM_OUT_OF_RESOURCES;
goto out;
+   }
 
for (i = 0; i  sg-length; i++)
*(addr + sg-offset + i) ^= *(buf + offset + i);
@@ -332,6 +334,7 @@ static void xdreadwrite_callback(struct se_cmd *cmd)
 
 out:
kfree(buf);
+   return ret;
 }
 
 sense_reason_t
diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 98ec711..53d1d75 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1904,10 +1904,24 @@ static void target_complete_ok_work(struct work_struct 
*work)
}
/*
 * Check for a callback, used by amongst other things
-* XDWRITE_READ_10 emulation.
+* XDWRITE_READ_10 and COMPARE_AND_WRITE emulation.
 */
-   if (cmd-transport_complete_callback)
-   cmd-transport_complete_callback(cmd);
+   if (cmd-transport_complete_callback) {
+   sense_reason_t rc;
+
+   rc = cmd-transport_complete_callback(cmd);
+   if (!rc)
+   return;
+
+   ret = transport_send_check_condition_and_sense(cmd,
+   rc, 0);
+   if (ret == -EAGAIN || ret == -ENOMEM)
+   goto queue_full;
+
+   transport_lun_remove_cmd(cmd);
+   transport_cmd_check_stop_to_fabric(cmd);
+   return;
+   }
 
switch (cmd-data_direction) {
case DMA_FROM_DEVICE:
diff --git a/include/target/target_core_base.h 
b/include/target/target_core_base.h
index 360e4a3..6e946f3 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -447,7 +447,7 @@ struct se_cmd {
struct kref cmd_kref;
struct target_core_fabric_ops *se_tfo;
sense_reason_t  (*execute_cmd)(struct se_cmd *);
-   void (*transport_complete_callback)(struct se_cmd *);
+   sense_reason_t (*transport_complete_callback)(struct se_cmd *);
 
unsigned char   *t_task_cdb;
unsigned char   __t_task_cdb[TCM_MAX_COMMAND_SIZE];
-- 
1.7.2.5

--
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 00/12] target: Add support for COMPARE_AND_WRITE (VAAI) emulation

2013-08-21 Thread Nicholas A. Bellinger
From: Nicholas Bellinger n...@daterainc.com

Hi folks,

This series -v2 adds support to target-core for generic COMPARE_AND_WRITE
emulation as defined by SBC-3 using virtual (IBLOCK, FILEIO, RAMDISK)
backends.

COMPARE_AND_WRITE is a VMWare ESX VAAI primitive that is currently used
by VMFS to perform array side locking of filesystem extents.  The logic
is the functional equivilent of an atomic test and set, which allows a
cluster filesystem to scale across multiple clients by locking individual
regions, without having to obtain a traditional SCSI reservation for
exclusive access to the entire logical unit.

Note this implemenation is currently limited to a single number of
logical blocks (NoLB).

As this point, a se_device-caw_mutex is in place to synchronize
between sbc_compare_and_write() - compare_and_write_callback() -
compare_and_write_post() callbacks and failure paths, and the code is
fully functional.  The use of mutex_lock() - mutex_unlock() across
multiple functions looks a bit strange, so comments have been added
to clarify the rather unusual looking usage.

The one point that was not addressed in hch's comments was dropping
se_cmd-execute_rw(), which ended up not being possible considering
that sbc_ops is not accessable beyond setup in sbc_parse_cdb(), and
saving this pointer in se_cmd would end up defeating the purpose of
the abstraction between SPC/SBC code.

The full changes for -v2 from hch's comments include:

  - Add SCF_COMPARE_AND_WRITE command flag
  - Use sbc_execute_rw() for normal cmd-execute_rw() submission
with expected se_cmd members.
  - Use SCF_COMPARE_AND_WRITE instead of CDB based check for
calculating length in transport_generic_get_mem_bidi().
  - Use SCF_COMPARE_AND_WRITE in transport_generic_new_cmd()
for determing when to call transport_generic_get_mem_bidi()
  - Make __target_execute_cmd() available as extern for WRITE
I/O submission within compare_and_write_callback()
  - Set SCF_COMPARE_AND_WRITE and cmd-execute_cmd() to
sbc_compare_and_write() during setup in sbc_parse_cdb()
  - Use sbc_compare_and_write() for initial READ submission with
DMA_FROM_DEVICE
  - Reset cmd-execute_cmd() to sbc_execute_rw() for write instance
user-data in compare_and_write_callback()
  - Drop SCF_BIDI command flag usage
  - Set TRANSPORT_PROCESSING + transport_state flags before write
instance submission, and convert to __target_execute_cmd()
  - Prevent sbc_get_size() from being being called twice to
generate incorrect size in sbc_parse_cdb()
  - Enforce se_device-caw_mutex synchronization between initial
READ I/O submission, and final WRITE I/O completion.
  - Drop tcm_qla2xxx patch, and will include as seperate patch
for common target_reverse_dma_direction().

Please review as v3.12 material.

Thanks!

--nab

Nicholas Bellinger (12):
  scsi: Add CDB definition for COMPARE_AND_WRITE
  target: Add return for se_cmd-transport_complete_callback
  target: Add TCM_MISCOMPARE_VERIFY sense handling
  target: Allow sbc_ops-execute_rw() to accept SGLs + data_direction
  target: Convert se_cmd-t_bidi_data_sg checks to use SCF_BIDI
  target: Add memory allocation for bidirectional commands
  target: Add transport_reset_sgl_orig() for COMPARE_AND_WRITE
  target: Make __target_execute_cmd() available as extern
  target: Add MAXIMUM COMPARE AND WRITE LENGTH in Block Limits VPD
  target: Add support for COMPARE_AND_WRITE emulation
  target: Add compare_and_write_post() completion callback fall through
  target: Release COMPARE_AND_WRITE mutex in generic failure path

 drivers/target/target_core_configfs.c  |4 +
 drivers/target/target_core_device.c|   15 ++
 drivers/target/target_core_file.c  |6 +-
 drivers/target/target_core_iblock.c|6 +-
 drivers/target/target_core_internal.h  |1 +
 drivers/target/target_core_rd.c|6 +-
 drivers/target/target_core_sbc.c   |  248 +--
 drivers/target/target_core_spc.c   |5 +
 drivers/target/target_core_transport.c |  117 ++-
 include/scsi/scsi.h|1 +
 include/target/target_core_backend.h   |3 +-
 include/target/target_core_base.h  |   14 ++-
 include/target/target_core_fabric.h|1 +
 13 files changed, 390 insertions(+), 37 deletions(-)

-- 
1.7.2.5

--
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] target/tcm_qla2xxx: Add/use target_reverse_dma_direction() in target_core_fabric.h

2013-08-21 Thread Nicholas A. Bellinger
From: Nicholas Bellinger n...@daterainc.com

Reversing the dma_data_direction for pci_map_sg() friends is useful
for other drivers, so move it from tcm_qla2xxx into inline code
within target_core_fabric.h.

Also drop internal usage of equivlient in tcm_qla2xxx fabric code.

Reported-by: Christoph Hellwig h...@lst.de
Cc: Roland Dreier rol...@purestorage.com
Cc: Giridhar Malavali giridhar.malav...@qlogic.com
Cc: Chad Dupuis chad.dup...@qlogic.com
Cc: Nicholas Bellinger n...@linux-iscsi.org
Signed-off-by: Nicholas Bellinger n...@daterainc.com
---
 drivers/scsi/qla2xxx/tcm_qla2xxx.c  |   31 +++
 include/target/target_core_fabric.h |   26 ++
 2 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c 
b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 6a93a91..f790fca 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -497,38 +497,13 @@ static u32 tcm_qla2xxx_sess_get_index(struct se_session 
*se_sess)
return 0;
 }
 
-/*
- * The LIO target core uses DMA_TO_DEVICE to mean that data is going
- * to the target (eg handling a WRITE) and DMA_FROM_DEVICE to mean
- * that data is coming from the target (eg handling a READ).  However,
- * this is just the opposite of what we have to tell the DMA mapping
- * layer -- eg when handling a READ, the HBA will have to DMA the data
- * out of memory so it can send it to the initiator, which means we
- * need to use DMA_TO_DEVICE when we map the data.
- */
-static enum dma_data_direction tcm_qla2xxx_mapping_dir(struct se_cmd *se_cmd)
-{
-   if (se_cmd-se_cmd_flags  SCF_BIDI)
-   return DMA_BIDIRECTIONAL;
-
-   switch (se_cmd-data_direction) {
-   case DMA_TO_DEVICE:
-   return DMA_FROM_DEVICE;
-   case DMA_FROM_DEVICE:
-   return DMA_TO_DEVICE;
-   case DMA_NONE:
-   default:
-   return DMA_NONE;
-   }
-}
-
 static int tcm_qla2xxx_write_pending(struct se_cmd *se_cmd)
 {
struct qla_tgt_cmd *cmd = container_of(se_cmd,
struct qla_tgt_cmd, se_cmd);
 
cmd-bufflen = se_cmd-data_length;
-   cmd-dma_data_direction = tcm_qla2xxx_mapping_dir(se_cmd);
+   cmd-dma_data_direction = target_reverse_dma_direction(se_cmd);
 
cmd-sg_cnt = se_cmd-t_data_nents;
cmd-sg = se_cmd-t_data_sg;
@@ -664,7 +639,7 @@ static int tcm_qla2xxx_queue_data_in(struct se_cmd *se_cmd)
struct qla_tgt_cmd, se_cmd);
 
cmd-bufflen = se_cmd-data_length;
-   cmd-dma_data_direction = tcm_qla2xxx_mapping_dir(se_cmd);
+   cmd-dma_data_direction = target_reverse_dma_direction(se_cmd);
cmd-aborted = (se_cmd-transport_state  CMD_T_ABORTED);
 
cmd-sg_cnt = se_cmd-t_data_nents;
@@ -688,7 +663,7 @@ static int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd)
cmd-sg = NULL;
cmd-sg_cnt = 0;
cmd-offset = 0;
-   cmd-dma_data_direction = tcm_qla2xxx_mapping_dir(se_cmd);
+   cmd-dma_data_direction = target_reverse_dma_direction(se_cmd);
cmd-aborted = (se_cmd-transport_state  CMD_T_ABORTED);
 
if (se_cmd-data_direction == DMA_FROM_DEVICE) {
diff --git a/include/target/target_core_fabric.h 
b/include/target/target_core_fabric.h
index 192eb52..882b650e 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -179,4 +179,30 @@ u32iscsi_get_pr_transport_id_len(struct 
se_portal_group *, struct se_node_acl *
 char   *iscsi_parse_pr_out_transport_id(struct se_portal_group *, const char *,
u32 *, char **);
 
+/*
+ * The LIO target core uses DMA_TO_DEVICE to mean that data is going
+ * to the target (eg handling a WRITE) and DMA_FROM_DEVICE to mean
+ * that data is coming from the target (eg handling a READ).  However,
+ * this is just the opposite of what we have to tell the DMA mapping
+ * layer -- eg when handling a READ, the HBA will have to DMA the data
+ * out of memory so it can send it to the initiator, which means we
+ * need to use DMA_TO_DEVICE when we map the data.
+ */
+static inline enum dma_data_direction
+target_reverse_dma_direction(struct se_cmd *se_cmd)
+{
+   if (se_cmd-se_cmd_flags  SCF_BIDI)
+   return DMA_BIDIRECTIONAL;
+
+   switch (se_cmd-data_direction) {
+   case DMA_TO_DEVICE:
+   return DMA_FROM_DEVICE;
+   case DMA_FROM_DEVICE:
+   return DMA_TO_DEVICE;
+   case DMA_NONE:
+   default:
+   return DMA_NONE;
+   }
+}
+
 #endif /* TARGET_CORE_FABRICH */
-- 
1.7.10.4

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