Re: [PATCH v2] target/file: add support of direct and async I/O

2018-05-07 Thread Bryant G. Ly
Hi Martin, 

Can you review this patch and pull it into scsi since Nicholas has
been out for awhile?

I have tested this patch and really like the performance enhancements
as a result of it.

Thanks,

Bryant


On 4/19/18 1:29 PM, Andrei Vagin wrote:

> Hello Nicholas,
>
> What do you think about this patch?
>
> Thanks,
> Andrei
>
> On Wed, Mar 21, 2018 at 11:55:02PM -0700, Andrei Vagin wrote:
>> There are two advantages:
>> * Direct I/O allows to avoid the write-back cache, so it reduces
>>   affects to other processes in the system.
>> * Async I/O allows to handle a few commands concurrently.
>>
>> DIO + AIO shows a better perfomance for random write operations:
>>
>> Mode: O_DSYNC Async: 1
>> $ ./fio --bs=4K --direct=1 --rw=randwrite --ioengine=libaio --iodepth=64 
>> --name=/dev/sda --runtime=20 --numjobs=2
>>   WRITE: bw=45.9MiB/s (48.1MB/s), 21.9MiB/s-23.0MiB/s (22.0MB/s-25.2MB/s), 
>> io=919MiB (963MB), run=20002-20020msec
>>
>> Mode: O_DSYNC Async: 0
>> $ ./fio --bs=4K --direct=1 --rw=randwrite --ioengine=libaio --iodepth=64 
>> --name=/dev/sdb --runtime=20 --numjobs=2
>>   WRITE: bw=1607KiB/s (1645kB/s), 802KiB/s-805KiB/s (821kB/s-824kB/s), 
>> io=31.8MiB (33.4MB), run=20280-20295msec
>>
>> Known issue:
>>
>> DIF (PI) emulation doesn't work when a target uses async I/O, because
>> DIF metadata is saved in a separate file, and it is another non-trivial
>> task how to synchronize writing in two files, so that a following read
>> operation always returns a consisten metadata for a specified block.
>>
>> v2: fix comments from Christoph Hellwig
>>
>> Cc: "Nicholas A. Bellinger" <n...@linux-iscsi.org>
>> Cc: Christoph Hellwig <h...@infradead.org>
>> Cc: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
>> Tested-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
>> Signed-off-by: Andrei Vagin <ava...@openvz.org>
>> ---
>>  drivers/target/target_core_file.c | 137 
>> ++
>>  drivers/target/target_core_file.h |   1 +
>>  2 files changed, 124 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/target/target_core_file.c 
>> b/drivers/target/target_core_file.c
>> index 9b2c0c773022..16751ae55d7b 100644
>> --- a/drivers/target/target_core_file.c
>> +++ b/drivers/target/target_core_file.c
>> @@ -250,6 +250,84 @@ static void fd_destroy_device(struct se_device *dev)
>>  }
>>  }
>>  
>> +struct target_core_file_cmd {
>> +unsigned long   len;
>> +struct se_cmd   *cmd;
>> +struct kiocbiocb;
>> +};
>> +
>> +static void cmd_rw_aio_complete(struct kiocb *iocb, long ret, long ret2)
>> +{
>> +struct target_core_file_cmd *cmd;
>> +
>> +cmd = container_of(iocb, struct target_core_file_cmd, iocb);
>> +
>> +if (ret != cmd->len)
>> +target_complete_cmd(cmd->cmd, SAM_STAT_CHECK_CONDITION);
>> +else
>> +target_complete_cmd(cmd->cmd, SAM_STAT_GOOD);
>> +
>> +kfree(cmd);
>> +}
>> +
>> +static sense_reason_t
>> +fd_execute_rw_aio(struct se_cmd *cmd, struct scatterlist *sgl, u32 
>> sgl_nents,
>> +  enum dma_data_direction data_direction)
>> +{
>> +int is_write = !(data_direction == DMA_FROM_DEVICE);
>> +struct se_device *dev = cmd->se_dev;
>> +struct fd_dev *fd_dev = FD_DEV(dev);
>> +struct file *file = fd_dev->fd_file;
>> +struct target_core_file_cmd *aio_cmd;
>> +struct iov_iter iter = {};
>> +struct scatterlist *sg;
>> +struct bio_vec *bvec;
>> +ssize_t len = 0;
>> +int ret = 0, i;
>> +
>> +aio_cmd = kmalloc(sizeof(struct target_core_file_cmd), GFP_KERNEL);
>> +if (!aio_cmd)
>> +return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
>> +
>> +bvec = kcalloc(sgl_nents, sizeof(struct bio_vec), GFP_KERNEL);
>> +if (!bvec) {
>> +kfree(aio_cmd);
>> +return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
>> +}
>> +
>> +for_each_sg(sgl, sg, sgl_nents, i) {
>> +bvec[i].bv_page = sg_page(sg);
>> +bvec[i].bv_len = sg->length;
>> +bvec[i].bv_offset = sg->offset;
>> +
>> +len += sg->length;
>> +}
>> +
>> +iov_iter_bvec(, ITER_BVEC | is_write, bvec, sgl_nents, len);
>> +
>> +aio_cmd->cmd = cmd;
>> +aio_cmd->len = len;
>> +aio_cmd->iocb.ki_pos = cmd->t_task_lba 

[PATCH v2] target: Fix Fortify_panic kernel exception

2018-04-17 Thread Bryant G. Ly
The bug exists in the memcmp in which the length passed in must
be guaranteed to be 1. This bug currently exists because
the second pointer passed in, can be smaller than the
cmd->data_length, which causes a fortify_panic.

The fix is to use memchr_inv instead to find whether or not
a 0 exists instead of using memcmp. This way you dont have to
worry about buffer overflow which is the reason for the
fortify_panic.

The bug was found by running a block backstore via LIO.

[  496.212958] Call Trace:
[  496.212960] [c007e58e3800] [c0cbbefc] fortify_panic+0x24/0x38 
(unreliable)
[  496.212965] [c007e58e3860] [df150c28] 
iblock_execute_write_same+0x3b8/0x3c0 [target_core_iblock]
[  496.212976] [c007e58e3910] [d6c737d4] 
__target_execute_cmd+0x54/0x150 [target_core_mod]
[  496.212982] [c007e58e3940] [d6d32ce4] 
ibmvscsis_write_pending+0x74/0xe0 [ibmvscsis]
[  496.212991] [c007e58e39b0] [d6c74fc8] 
transport_generic_new_cmd+0x318/0x370 [target_core_mod]
[  496.213001] [c007e58e3a30] [d6c75084] 
transport_handle_cdb_direct+0x64/0xd0 [target_core_mod]
[  496.213011] [c007e58e3aa0] [d6c75298] 
target_submit_cmd_map_sgls+0x1a8/0x320 [target_core_mod]
[  496.213021] [c007e58e3b30] [d6c75458] 
target_submit_cmd+0x48/0x60 [target_core_mod]
[  496.213026] [c007e58e3bd0] [d6d34c20] 
ibmvscsis_scheduler+0x370/0x600 [ibmvscsis]
[  496.213031] [c007e58e3c90] [c013135c] 
process_one_work+0x1ec/0x580
[  496.213035] [c007e58e3d20] [c0131798] worker_thread+0xa8/0x600
[  496.213039] [c007e58e3dc0] [c013a468] kthread+0x168/0x1b0
[  496.213044] [c007e58e3e30] [c000b528] 
ret_from_kernel_thread+0x5c/0xb4

Fixes: 2237498f0b5c ("target/iblock: Convert WRITE_SAME to 
blkdev_issue_zeroout")
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-by: Steven Royer <sero...@linux.vnet.ibm.com>
Tested-by: Taylor Jakobson <tjak...@us.ibm.com>
Cc: Christoph Hellwig <h...@lst.de>
Cc: Nicholas Bellinger <n...@linux-iscsi.org>
Cc: <sta...@vger.kernel.org>
---
 drivers/target/target_core_iblock.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/target/target_core_iblock.c 
b/drivers/target/target_core_iblock.c
index 07c814c..6042901 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -427,8 +427,8 @@ iblock_execute_zero_out(struct block_device *bdev, struct 
se_cmd *cmd)
 {
struct se_device *dev = cmd->se_dev;
struct scatterlist *sg = >t_data_sg[0];
-   unsigned char *buf, zero = 0x00, *p = 
-   int rc, ret;
+   unsigned char *buf, *not_zero;
+   int ret;
 
buf = kmap(sg_page(sg)) + sg->offset;
if (!buf)
@@ -437,10 +437,10 @@ iblock_execute_zero_out(struct block_device *bdev, struct 
se_cmd *cmd)
 * Fall back to block_execute_write_same() slow-path if
 * incoming WRITE_SAME payload does not contain zeros.
 */
-   rc = memcmp(buf, p, cmd->data_length);
+   not_zero = memchr_inv(buf, 0x00, cmd->data_length);
kunmap(sg_page(sg));
 
-   if (rc)
+   if (not_zero)
return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 
ret = blkdev_issue_zeroout(bdev,
-- 
2.7.2



Re: [PATCH] target: Fix Fortify_panic kernel exception

2018-04-17 Thread Bryant G. Ly

On 4/13/18 11:44 AM, Christoph Hellwig wrote:

> The patch looks fine, but in general I think descriptions of what
> you fixed in the code or more important than starting out with
> a backtrace.
>
> E.g. please explain what was wrong, how you fixed it and only after
> that mention how it was caught.  (preferably without the whole trace)
>
I will put the trace at the end next time. Do you want me to re-submit
with it moved?

-Bryant



[PATCH] target: Fix Fortify_panic kernel exception

2018-04-12 Thread Bryant G. Ly
[  496.212783] [ cut here ]
[  496.212784] kernel BUG at 
/build/linux-hwe-edge-ojNirv/linux-hwe-edge-4.15.0/lib/string.c:1052!
[  496.212789] Oops: Exception in kernel mode, sig: 5 [#1]
[  496.212791] LE SMP NR_CPUS=2048 NUMA pSeries
[  496.212795] Modules linked in: hvcs(OE) hvcserver dm_snapshot dm_bufio 
rpadlpar_io rpaphp ip6table_raw xt_CT xt_mac xt_tcpudp xt_comment xt_physdev 
xt_set ip_set_hash_net ip_set iptable_raw dccp_diag dccp tcp_diag udp_diag 
inet_diag unix_diag af_packet_diag netlink_diag target_core_pscsi(OE) 
target_core_file(OE) target_core_iblock(OE) iscsi_target_mod(OE) vxlan 
ip6_udp_tunnel udp_tunnel openvswitch nsh nf_nat_ipv6 target_core_user(OE) uio 
binfmt_misc xt_conntrack nf_conntrack_netlink nfnetlink nf_conntrack_netbios_ns 
nf_conntrack_broadcast nf_conntrack_ipv6 nf_defrag_ipv6 nbd ipt_REJECT 
nf_reject_ipv4 ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat 
nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 pseries_rng nf_nat ibmvmc(OE) 
nf_conntrack libcrc32c vmx_crypto crct10dif_vpmsum iptable_mangle iptable_filter
[  496.212854]  ip_tables ip6table_filter ip6_tables ebtables x_tables 
br_netfilter bridge stp llc ib_iser rdma_cm iw_cm ib_cm ib_core iscsi_tcp 
libiscsi_tcp libiscsi scsi_transport_iscsi autofs4 mlx4_en ses enclosure 
scsi_transport_sas uas usb_storage ibmvscsis(OE) target_core_mod(OE) 
ibmveth(OE) mlx5_core mlx4_core mlxfw crc32c_vpmsum be2net tg3 ipr devlink
[  496.212888] CPU: 1 PID: 2587 Comm: kworker/1:2 Tainted: G   OE
4.15.0-15-generic #16~16.04.1-Ubuntu
[  496.212897] Workqueue: ibmvscsis300f ibmvscsis_scheduler [ibmvscsis]
[  496.212900] NIP:  c0cbbf00 LR: c0cbbefc CTR: 00655170
[  496.212903] REGS: c007e58e3580 TRAP: 0700   Tainted: G   OE 
(4.15.0-15-generic)
[  496.212906] MSR:  8282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 
286c  XER: 2003
[  496.212915] CFAR: c018d634 SOFTE: 1
   GPR00: c0cbbefc c007e58e3800 c16bae00 
0022
   GPR04: c007fe94ce18 c007fe964368 0003 

   GPR08: 0007 c1193a74 0007fd7c 
3986
   GPR12: 2200 cfa80b00 c013a308 
c007f48adb00
   GPR16:    

   GPR20:   fef7 
0402
   GPR24:  f1a8cb40 03f0 
00648010
   GPR28: c005a360a570 c007f4095880 c000fc9e7e00 
c007f1f56000
[  496.212952] NIP [c0cbbf00] fortify_panic+0x28/0x38
[  496.212956] LR [c0cbbefc] fortify_panic+0x24/0x38
[  496.212958] Call Trace:
[  496.212960] [c007e58e3800] [c0cbbefc] fortify_panic+0x24/0x38 
(unreliable)
[  496.212965] [c007e58e3860] [df150c28] 
iblock_execute_write_same+0x3b8/0x3c0 [target_core_iblock]
[  496.212976] [c007e58e3910] [d6c737d4] 
__target_execute_cmd+0x54/0x150 [target_core_mod]
[  496.212982] [c007e58e3940] [d6d32ce4] 
ibmvscsis_write_pending+0x74/0xe0 [ibmvscsis]
[  496.212991] [c007e58e39b0] [d6c74fc8] 
transport_generic_new_cmd+0x318/0x370 [target_core_mod]
[  496.213001] [c007e58e3a30] [d6c75084] 
transport_handle_cdb_direct+0x64/0xd0 [target_core_mod]
[  496.213011] [c007e58e3aa0] [d6c75298] 
target_submit_cmd_map_sgls+0x1a8/0x320 [target_core_mod]
[  496.213021] [c007e58e3b30] [d6c75458] 
target_submit_cmd+0x48/0x60 [target_core_mod]
[  496.213026] [c007e58e3bd0] [d6d34c20] 
ibmvscsis_scheduler+0x370/0x600 [ibmvscsis]
[  496.213031] [c007e58e3c90] [c013135c] 
process_one_work+0x1ec/0x580
[  496.213035] [c007e58e3d20] [c0131798] worker_thread+0xa8/0x600
[  496.213039] [c007e58e3dc0] [c013a468] kthread+0x168/0x1b0
[  496.213044] [c007e58e3e30] [c000b528] 
ret_from_kernel_thread+0x5c/0xb4
[  496.213047] Instruction dump:
[  496.213049] 7c0803a6 4e800020 3c4c00a0 3842ef28 7c0802a6 f8010010 f821ffa1 
7c641b78
[  496.213055] 3c62ff94 3863dc00 4b4d16f1 6000 <0fe0>   

[  496.213062] ---[ end trace 4c7e8c92043f3868 ]---
[  654.577815] ibmvscsis 300f: connection lost with outstanding work

The patch fixes the above trace where the size passed into
memcmp is greater than the size of the data passed in from
ptr1 or ptr2 then a fortify_panic is posted.

Fixes: 2237498f0b5c ("target/iblock: Convert WRITE_SAME to 
blkdev_issue_zeroout")
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-by: Steven Royer <sero...@linux.vnet.ibm.com>
Tested-by: Taylor Jakobson <tjak...@us.ibm.com>
Cc: Christoph Hellwig <h...@lst.de>
Cc: Nicholas Bellinger <n...@linux-iscsi.org&

Re: [PATCH v2] target/file: add support of direct and async I/O

2018-03-22 Thread Bryant G. Ly
On 3/22/18 12:34 PM, Christoph Hellwig wrote:

>> DIF (PI) emulation doesn't work when a target uses async I/O, because
>> DIF metadata is saved in a separate file, and it is another non-trivial
>> task how to synchronize writing in two files, so that a following read
>> operation always returns a consisten metadata for a specified block.
> As said, this isn't really in any way aio specific.
>
>> +int is_write = !(data_direction == DMA_FROM_DEVICE);
>   bool is_write = data_direction != DMA_FROM_DEVICE;
>
>> +if (is_write && (cmd->se_cmd_flags & SCF_FUA))
>> +aio_cmd->iocb.ki_flags |= IOCB_DSYNC;
>> +
>> +if (is_write)
>> +ret = call_write_iter(file, _cmd->iocb, );
>> +else
>> +ret = call_read_iter(file, _cmd->iocb, );
>> +
>> +kfree(bvec);
> While this looks good to me this is the same pattern just said doesn't
> work in loop.  So it works here just fine?
>
> Otherwise this looks fine to me:
>
> Signed-off-by: Christoph Hellwig <h...@lst.de>
>
This patch created the helpers for f_op->read/write:
https://patchwork.kernel.org/patch/9580365/

That patch was queued up for 4.11 so I guess if anyone tried to port this patch
back they would hit issues if they didn't use f_op.

Reviewed-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>

-Bryant



Re: [PATCH] [RFC] target/file: add support of direct and async I/O

2018-03-15 Thread Bryant G. Ly
On 3/8/18 6:42 PM, Andrei Vagin wrote:

> Direct I/O allows to not affect the write-back cache, this is
> expected when a non-buffered mode is used.
>
> Async I/O allows to handle a few commands concurrently, so a target shows a
> better perfomance:
>
> Mode: O_DSYNC Async: 1
> $ ./fio --bs=4K --direct=1 --rw=randwrite --ioengine=libaio --iodepth=64 
> --name=/dev/sda --runtime=20 --numjobs=2
>   WRITE: bw=45.9MiB/s (48.1MB/s), 21.9MiB/s-23.0MiB/s (22.0MB/s-25.2MB/s), 
> io=919MiB (963MB), run=20002-20020msec
>
> Mode: O_DSYNC Async: 0
> $ ./fio --bs=4K --direct=1 --rw=randwrite --ioengine=libaio --iodepth=64 
> --name=/dev/sdb --runtime=20 --numjobs=2
>   WRITE: bw=1607KiB/s (1645kB/s), 802KiB/s-805KiB/s (821kB/s-824kB/s), 
> io=31.8MiB (33.4MB), run=20280-20295msec
>
> Known issue:
>
> DIF (PI) emulation doesn't work when a target uses async I/O, because
> DIF metadata is saved in a separate file, and it is another non-trivial
> task how to synchronize writing in two files, so that a following read
> operation always returns a consisten metadata for a specified block.
>
> Cc: "Nicholas A. Bellinger" <n...@linux-iscsi.org>
> Signed-off-by: Andrei Vagin <ava...@openvz.org>
> ---
>  drivers/target/target_core_file.c | 124 
> --
>  drivers/target/target_core_file.h |   1 +
>  2 files changed, 120 insertions(+), 5 deletions(-)
>
>
Tested-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>

Patch looks good to me - Thanks for the performance enhancement! 

Btw I have been running I/O tests with HTX against this patch for 24 hrs and 
have no problems.

-Bryant



Re: [PATCH] tcmu: Allow reconfig to handle multiple attributes

2018-01-29 Thread Bryant G. Ly

On 1/10/18 12:26 PM, Mike Christie wrote:

> On 01/04/2018 10:11 AM, Bryant G. Ly wrote:
>> This patch allows for multiple attributes to be reconfigured
>> and handled all in one call as compared to multiple netlinks.
>>
>> Example:
>> set attribute dev_reconfig=dev_config=fbo//home/path:dev_size=2147483648
>>
> I know I suggested this, but I think I was wrong. If we have to support
> other apps that work in reverse to targetcli+tcmu-runner where the
> tcmu-runner equivalent app sets things up then contacts the kernel,
> let's just not do passthrough operations like this for reconfig. There
> is no need to bring in the kernel.
>
> For the initial config we can still do it since we have to maintain
> compat, but for major reconfigs like this let's just have targetcli
> contact tcmu-runner, then runner can update the kernel if needed.
>
> So in targetcli and runner copy the check_config stuff, and add a
> reconfig callout that works like it. We then do not have this weird
> kernel passthrough and do not have to worry about the non
> targetcl-tcmu-runner type of model.
>
>
>
So you basically want me to use DBUS correct? Connect a GCallback function

with reconfig function and use DBUS to pass info back to kernel if necessary?

-Bryant



[PATCH] tcmu: Allow reconfig to handle multiple attributes

2018-01-04 Thread Bryant G. Ly
This patch allows for multiple attributes to be reconfigured
and handled all in one call as compared to multiple netlinks.

Example:
set attribute dev_reconfig=dev_config=fbo//home/path:dev_size=2147483648

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_user.c | 92 ++-
 include/uapi/linux/target_core_user.h |  1 +
 2 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index 4824abf92ed79..619fae5e865f1 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -152,6 +152,8 @@ struct tcmu_dev {
char dev_config[TCMU_CONFIG_LEN];
 
int nl_reply_supported;
+
+   char dev_reconfig[TCMU_CONFIG_LEN * 2];
 };
 
 #define TCMU_DEV(_se_dev) container_of(_se_dev, struct tcmu_dev, se_dev)
@@ -1452,6 +1454,10 @@ static int tcmu_netlink_event(struct tcmu_dev *udev, 
enum tcmu_genl_cmd cmd,
ret = nla_put_u8(skb, reconfig_attr,
  *((u8 *)reconfig_data));
break;
+   case TCMU_ATTR_DEV_RECFG:
+   pr_err("Put string into netlink and send it\n");
+   ret = nla_put_string(skb, reconfig_attr, reconfig_data);
+   break;
default:
BUG();
}
@@ -1637,7 +1643,7 @@ static void tcmu_destroy_device(struct se_device *dev)
 
 enum {
Opt_dev_config, Opt_dev_size, Opt_hw_block_size, Opt_hw_max_sectors,
-   Opt_nl_reply_supported, Opt_err,
+   Opt_nl_reply_supported, Opt_dev_reconfig, Opt_err,
 };
 
 static match_table_t tokens = {
@@ -1646,6 +1652,7 @@ static match_table_t tokens = {
{Opt_hw_block_size, "hw_block_size=%u"},
{Opt_hw_max_sectors, "hw_max_sectors=%u"},
{Opt_nl_reply_supported, "nl_reply_supported=%d"},
+   {Opt_dev_reconfig, "dev_reconfig=%s"},
{Opt_err, NULL}
 };
 
@@ -1731,6 +1738,14 @@ static ssize_t tcmu_set_configfs_dev_params(struct 
se_device *dev,
if (ret < 0)
pr_err("kstrtoint() failed for 
nl_reply_supported=\n");
break;
+   case Opt_dev_reconfig:
+   arg_p = match_strdup([0]);
+   if (!arg_p) {
+   ret = -ENOMEM;
+   break;
+   }
+   kfree(arg_p);
+   break;
default:
break;
}
@@ -1945,12 +1960,87 @@ static ssize_t tcmu_emulate_write_cache_store(struct 
config_item *item,
 }
 CONFIGFS_ATTR(tcmu_, emulate_write_cache);
 
+static ssize_t tcmu_dev_reconfig_show(struct config_item *item, char *page)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+
+   return snprintf(page, PAGE_SIZE, "%s\n", udev->dev_reconfig);
+}
+
+static ssize_t tcmu_dev_reconfig_store(struct config_item *item,
+  const char *page,
+  size_t count)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+   int token, ret;
+   char *orig, *ptr, *opts, *arg_p;
+   substring_t args[MAX_OPT_ARGS];
+
+   /* Check if device has been configured before */
+   if (tcmu_dev_configured(udev)) {
+   ret = tcmu_netlink_event(udev, TCMU_CMD_RECONFIG_DEVICE,
+TCMU_ATTR_DEV_RECFG, page);
+   if (ret) {
+   pr_err("Unable to reconfigure device\n");
+   return ret;
+   }
+
+   opts = kstrdup(page, GFP_KERNEL);
+   if (!opts)
+   return -ENOMEM;
+
+   orig = opts;
+   strcpy(udev->dev_reconfig, opts);
+
+   while ((ptr = strsep(, ":")) != NULL) {
+   if (!*ptr)
+   continue;
+
+   token = match_token(ptr, tokens, args);
+   switch (token) {
+   case Opt_dev_config:
+   if (match_strlcpy(udev->dev_config, [0],
+ TCMU_CONFIG_LEN) == 0) {
+   pr_err("Could not reconfigure 
dev_config");
+   }
+   ret 

[PATCH] ibmvscsis: add DRC indices to debug statements

2017-12-04 Thread Bryant G. Ly
Where applicable, changes pr_debug, pr_info, pr_err, etc. calls
to the dev_* versions.  This adds the DRC index of the device to the
corresponding trace statement.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Signed-off-by: Brad Warrum <bwar...@us.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 320 ---
 1 file changed, 170 insertions(+), 150 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 2799a6b08f736..c3a76af9f5fa9 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -122,7 +122,7 @@ static bool connection_broken(struct scsi_info *vscsi)
   cpu_to_be64(buffer[MSG_HI]),
   cpu_to_be64(buffer[MSG_LOW]));
 
-   pr_debug("connection_broken: rc %ld\n", h_return_code);
+   dev_dbg(>dev, "Connection_broken: rc %ld\n", h_return_code);
 
if (h_return_code == H_CLOSED)
rc = true;
@@ -210,7 +210,7 @@ static long ibmvscsis_unregister_command_q(struct scsi_info 
*vscsi)
}
} while (qrc != H_SUCCESS && rc == ADAPT_SUCCESS);
 
-   pr_debug("Freeing CRQ: phyp rc %ld, rc %ld\n", qrc, rc);
+   dev_dbg(>dev, "Freeing CRQ: phyp rc %ld, rc %ld\n", qrc, rc);
 
return rc;
 }
@@ -291,9 +291,9 @@ static long ibmvscsis_free_command_q(struct scsi_info 
*vscsi)
ibmvscsis_delete_client_info(vscsi, false);
}
 
-   pr_debug("free_command_q: flags 0x%x, state 0x%hx, acr_flags 
0x%x, acr_state 0x%hx\n",
-vscsi->flags, vscsi->state, vscsi->phyp_acr_flags,
-vscsi->phyp_acr_state);
+   dev_dbg(>dev, "free_command_q: flags 0x%x, state 0x%hx, 
acr_flags 0x%x, acr_state 0x%hx\n",
+   vscsi->flags, vscsi->state, vscsi->phyp_acr_flags,
+   vscsi->phyp_acr_state);
}
return rc;
 }
@@ -428,8 +428,8 @@ static void ibmvscsis_disconnect(struct work_struct *work)
vscsi->flags |= DISCONNECT_SCHEDULED;
vscsi->flags &= ~SCHEDULE_DISCONNECT;
 
-   pr_debug("disconnect: flags 0x%x, state 0x%hx\n", vscsi->flags,
-vscsi->state);
+   dev_dbg(>dev, "disconnect: flags 0x%x, state 0x%hx\n",
+   vscsi->flags, vscsi->state);
 
/*
 * check which state we are in and see if we
@@ -540,13 +540,14 @@ static void ibmvscsis_disconnect(struct work_struct *work)
}
 
if (wait_idle) {
-   pr_debug("disconnect start wait, active %d, sched %d\n",
-(int)list_empty(>active_q),
-(int)list_empty(>schedule_q));
+   dev_dbg(>dev, "disconnect start wait, active %d, sched 
%d\n",
+   (int)list_empty(>active_q),
+   (int)list_empty(>schedule_q));
if (!list_empty(>active_q) ||
!list_empty(>schedule_q)) {
vscsi->flags |= WAIT_FOR_IDLE;
-   pr_debug("disconnect flags 0x%x\n", vscsi->flags);
+   dev_dbg(>dev, "disconnect flags 0x%x\n",
+   vscsi->flags);
/*
 * This routine is can not be called with the interrupt
 * lock held.
@@ -555,7 +556,7 @@ static void ibmvscsis_disconnect(struct work_struct *work)
wait_for_completion(>wait_idle);
spin_lock_bh(>intr_lock);
}
-   pr_debug("disconnect stop wait\n");
+   dev_dbg(>dev, "disconnect stop wait\n");
 
ibmvscsis_adapter_idle(vscsi);
}
@@ -597,8 +598,8 @@ static void ibmvscsis_post_disconnect(struct scsi_info 
*vscsi, uint new_state,
 
vscsi->flags |= flag_bits;
 
-   pr_debug("post_disconnect: new_state 0x%x, flag_bits 0x%x, vscsi->flags 
0x%x, state %hx\n",
-new_state, flag_bits, vscsi->flags, vscsi->state);
+   dev_dbg(>dev, "post_disconnect: new_state 0x%x, flag_bits 0x%x, 
vscsi->flags 0x%x, state %hx\n",
+   new_state, flag_bits, vscsi->flags, vscsi->state);
 
if (!(vscsi->flags & (DISCONNECT_SCHEDULED | SCHEDULE_DISCONNECT))) {
vscsi->flags |= SCHEDULE_DISCONNECT;
@@ -648,8 +649,8 @@ static void ibmvscsis_post_disconnect(struct scsi_info 
*vscsi, uint new_state,
}
}
 
-   pr_debug("Leaving post_disconnect: flags 0x%x, new_state 0x%x\n",
-  

Re: [PATCH 2/8] target: remove iblock WRITE_SAME passthrough support

2017-11-14 Thread Bryant G. Ly
On 4/12/17 12:30 AM, Nicholas A. Bellinger wrote:

> On Mon, 2017-04-10 at 18:08 +0200, Christoph Hellwig wrote:
>> Use the pscsi driver to support arbitrary command passthrough
>> instead.
>>
> The people who are actively using iblock_execute_write_same_direct() are
> doing so in the context of ESX VAAI BlockZero, together with
> EXTENDED_COPY and COMPARE_AND_WRITE primitives.  Just using PSCSI is not
> an option for them.
>
> In practice though I've not seen any users of IBLOCK WRITE_SAME for
> anything other than VAAI BlockZero, so just using blkdev_issue_zeroout()
> when available, and falling back to iblock_execute_write_same() if the
> WRITE_SAME buffer contains anything other than zeros should be OK.
>
> How about something like the following below..?
>
> This would bring parity to how blkdev_issue_write_same() works atm wrt
> to synchronous bio completions.  However, most folks with a raw
> make_request or blk-mq backend driver that supports multiple GB/sec of
> zero bandwidth end up changing IBLOCK to support asynchronous
> REQ_WRITE_SAME completions anyways.
>
> I'd be happy to add support for that using __blkdev_issue_zeroout() once
> the basic conversion is in place.
>
> From ff74012eaff38f9fa0d74aca60507b9964f484ce Mon Sep 17 00:00:00 2001
> From: Nicholas Bellinger 
> Date: Tue, 11 Apr 2017 22:21:47 -0700
> Subject: [PATCH] target/iblock: Convert WRITE_SAME to blkdev_issue_zeroout
>
> Signed-off-by: Nicholas Bellinger 
> ---
>  drivers/target/target_core_iblock.c | 44 
> +++--
>  1 file changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/target/target_core_iblock.c 
> b/drivers/target/target_core_iblock.c
> index d316ed5..5bfde20 100644
> --- a/drivers/target/target_core_iblock.c
> +++ b/drivers/target/target_core_iblock.c
> @@ -86,6 +86,7 @@ static int iblock_configure_device(struct se_device *dev)
>   struct block_device *bd = NULL;
>   struct blk_integrity *bi;
>   fmode_t mode;
> + unsigned int max_write_zeroes_sectors;
>   int ret = -ENOMEM;
>
>   if (!(ib_dev->ibd_flags & IBDF_HAS_UDEV_PATH)) {
> @@ -129,7 +130,11 @@ static int iblock_configure_device(struct se_device *dev)
>* Enable write same emulation for IBLOCK and use 0x as
>* the smaller WRITE_SAME(10) only has a two-byte block count.
>*/
> - dev->dev_attrib.max_write_same_len = 0x;
> + max_write_zeroes_sectors = bdev_write_zeroes_sectors(bd);
> + if (max_write_zeroes_sectors)
> + dev->dev_attrib.max_write_same_len = max_write_zeroes_sectors;
> + else
> + dev->dev_attrib.max_write_same_len = 0x;
>
>   if (blk_queue_nonrot(q))
>   dev->dev_attrib.is_nonrot = 1;
> @@ -415,28 +420,31 @@ static void iblock_end_io_flush(struct bio *bio)
>  }
>
>  static sense_reason_t
> -iblock_execute_write_same_direct(struct block_device *bdev, struct se_cmd 
> *cmd)
> +iblock_execute_zero_out(struct block_device *bdev, struct se_cmd *cmd)
>  {
>   struct se_device *dev = cmd->se_dev;
>   struct scatterlist *sg = >t_data_sg[0];
> - struct page *page = NULL;
> - int ret;
> + unsigned char *buf, zero = 0x00, *p = 
> + int rc, ret;
>
> - if (sg->offset) {
> - page = alloc_page(GFP_KERNEL);
> - if (!page)
> - return TCM_OUT_OF_RESOURCES;
> - sg_copy_to_buffer(sg, cmd->t_data_nents, page_address(page),
> -   dev->dev_attrib.block_size);
> - }
> + buf = kmap(sg_page(sg)) + sg->offset;
> + if (!buf)
> + return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> + /*
> +  * Fall back to block_execute_write_same() slow-path if
> +  * incoming WRITE_SAME payload does not contain zeros.
> +  */
> + rc = memcmp(buf, p, cmd->data_length);
> + kunmap(sg_page(sg));
> +

I recently pulled in this patch and I am getting:

[  716.756756] [ cut here ]
[  716.756757] kernel BUG at 
/build/linux-hwe-edge-F6_Smd/linux-hwe-edge-4.13.0/lib/string.c:985!
[  716.756762] Oops: Exception in kernel mode, sig: 5 [#1]
[  716.756764] SMP NR_CPUS=2048 
[  716.756765] NUMA 
[  716.756767] pSeries
[  716.756769] Modules linked in: hvcs(OE) hvcserver dm_snapshot dm_bufio 
ip6table_raw xt_CT xt_mac xt_tcpudp xt_comment xt_physdev xt_set 
ip_set_hash_net ip_set rpadlpar_io rpaphp iptable_raw target_core_pscsi(OE) 
target_core_file(OE) target_core_iblock(OE) iscsi_target_mod(OE) dccp_diag dccp 
tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag vport_vxlan 
vxlan ip6_udp_tunnel udp_tunnel openvswitch nf_nat_ipv6 target_core_user(OE) 
uio binfmt_misc ibmvmc(OE) pseries_rng vmx_crypto crct10dif_vpmsum xt_conntrack 
nf_nat_ftp nf_conntrack_ftp nf_conntrack_netlink nfnetlink 
nf_conntrack_netbios_ns nf_conntrack_broadcast nf_conntrack_ipv6 nf_defrag_ipv6 
nbd ipt_REJECT nf_reject_ipv4 ipt_MASQUERADE 

Re: [PATCH] tcmu: clean up the scatter helper

2017-11-08 Thread Bryant G. Ly

> On 11/08/2017 04:39 PM, Bryant G. Ly wrote:
>> On 7/30/17 5:19 PM, Nicholas A. Bellinger wrote:
>>
>>> On Thu, 2017-07-13 at 14:33 +0800, lixi...@cmss.chinamobile.com wrote:
>>>> From: Xiubo Li <lixi...@cmss.chinamobile.com>
>>>>
>>>> Add some comments to make the scatter code to be more readable.
>>>>
>>>> Signed-off-by: Xiubo Li <lixi...@cmss.chinamobile.com>
>>>> ---
>>>>  drivers/target/target_core_user.c | 30 +-
>>>>  1 file changed, 25 insertions(+), 5 deletions(-)
>>>>
>>> Applied to target-pending/for-next.
>>>
>>> Thanks Xiubo + MNC.
>>>
>> This one seems to also be missing from the tree. 
>>
> I ported this one to my patchset that is built without the
> "tcmu: Add fifo type waiter list support to avoid starvation" patch:
>
> https://www.spinics.net/lists/target-devel/msg16153.html
>
Thanks Mike! I was trying to catch up and I missed all that. I see that the 
current for-next has everything. 

-Bryant




Re: [PATCH] tcmu: Oops in unmap_thread_fn()

2017-11-08 Thread Bryant G. Ly


On 8/6/17 6:27 PM, Nicholas A. Bellinger wrote:
> On Tue, 2017-08-01 at 23:09 +0300, Dan Carpenter wrote:
>> Calling list_del() on the iterator pointer in list_for_each_entry() will
>> cause an oops.  We need to user the _safe() version for that.
>>
>> Fixes: c73d02f63c16 ("tcmu: Add fifo type waiter list support to avoid 
>> starvation")
>> Signed-off-by: Dan Carpenter 
>>
> Applied to target-pending/for-next.
>
> Thanks DanC.
>
> --
> To unsubscribe from this list: send the line "unsubscribe target-devel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Also missing.

-Bryant



Re: [PATCH] tcmu: clean up the scatter helper

2017-11-08 Thread Bryant G. Ly
On 7/30/17 5:19 PM, Nicholas A. Bellinger wrote:

> On Thu, 2017-07-13 at 14:33 +0800, lixi...@cmss.chinamobile.com wrote:
>> From: Xiubo Li 
>>
>> Add some comments to make the scatter code to be more readable.
>>
>> Signed-off-by: Xiubo Li 
>> ---
>>  drivers/target/target_core_user.c | 30 +-
>>  1 file changed, 25 insertions(+), 5 deletions(-)
>>
> Applied to target-pending/for-next.
>
> Thanks Xiubo + MNC.
>
This one seems to also be missing from the tree. 

-Bryant



Re: [PATCHv2] tcmu: Add fifo type waiter list support to avoid starvation

2017-11-08 Thread Bryant G. Ly

On 7/30/17 5:10 PM, Nicholas A. Bellinger wrote:

> Hi Xiubo,
>
> Apologies for the delayed response.  Comments below.
>
> On Wed, 2017-07-12 at 15:16 +0800, lixi...@cmss.chinamobile.com wrote:
>> From: Xiubo Li 
>>
>> The fifo type waiter list will hold the udevs who are waiting for the
>> blocks from the data global pool. The unmap thread will try to feed the
>> first udevs in waiter list, if the global free blocks available are
>> not enough, it will stop traversing the list and abort waking up the
>> others.
>>
>> Signed-off-by: Xiubo Li 
>> ---
>>  drivers/target/target_core_user.c | 104 
>> --
>>  1 file changed, 88 insertions(+), 16 deletions(-)
>>
> Applied to target-pending/for-next.
>
> Thanks Xiubo + MNC.
>
Hi Nick, 

Do you know what ever happened to this patch? You mentioned that you had 
applied the patch but I don't see it anywhere in your tree.

Thanks, 

Bryant



Re: [PATCH 1/6] target: Fix QUEUE_FULL + SCSI task attribute handling

2017-11-08 Thread Bryant G. Ly

> From: Nicholas Bellinger <n...@linux-iscsi.org>
>
> This patch fixes a bug during QUEUE_FULL where transport_complete_qf()
> calls transport_complete_task_attr() after it's already been invoked
> by target_complete_ok_work() or transport_generic_request_failure()
> during initial completion, preceeding QUEUE_FULL.
>
> This will result in se_device->simple_cmds, se_device->dev_cur_ordered_id
> and/or se_device->dev_ordered_sync being updated multiple times for
> a single se_cmd.
>
> To address this bug, clear SCF_TASK_ATTR_SET after the first call
> to transport_complete_task_attr(), and avoid updating SCSI task
> attribute related counters for any subsequent calls.
>
> Also, when a se_cmd is deferred due to ordered tags and executed
> via target_restart_delayed_cmds(), set CMD_T_SENT before execution
> matching what target_execute_cmd() does.
>
> Cc: Michael Cyr <mike...@linux.vnet.ibm.com>
> Cc: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
> Cc: Mike Christie <mchri...@redhat.com>
> Cc: Hannes Reinecke <h...@suse.com>
> Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
> ---
>
Reviewed-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>



Re: [PATCH 3/6] target: Fix quiese during transport_write_pending_qf endless loop

2017-11-08 Thread Bryant G. Ly

> From: Nicholas Bellinger <n...@linux-iscsi.org>
>
> This patch fixes a potential end-less loop during QUEUE_FULL,
> where cmd->se_tfo->write_pending() callback fails repeatedly
> but __transport_wait_for_tasks() has already been invoked to
> quiese the outstanding se_cmd descriptor.
>
> To address this bug, this patch adds a CMD_T_STOP|CMD_T_ABORTED
> check within transport_write_pending_qf() and invokes the
> existing se_cmd->t_transport_stop_comp to signal quiese
> completion back to __transport_wait_for_tasks().
>
> Cc: Mike Christie <mchri...@redhat.com>
> Cc: Hannes Reinecke <h...@suse.com>
> Cc: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
> Cc: Michael Cyr <mike...@linux.vnet.ibm.com>
> Cc: Potnuri Bharat Teja <bha...@chelsio.com>
> Cc: Sagi Grimberg <s...@grimberg.me>
> Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
> ---
>
Reviewed-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>




Re: [PATCH] ibmvscsis: Fix write_pending failure path

2017-10-03 Thread Bryant G. Ly



On 10/2/17 9:51 PM, Martin K. Petersen wrote:

Bryant,


For write_pending if the queue is down or client failed then return
-EIO so that LIO can properly process the completed command. Prior we
returned 0 since LIO could not handle it properly. Now with: target:
Fix unknown fabric callback queue-full errors that patch addresses
LIO's ability to handle things right.

Applied to 4.14/scsi-fixes. thanks!

Thanks! I forgot to add the stable tag for 4.11+, but I guess we can 
wait for
Nick to reply since we had discussed making the dependent patch series 
stable.


[PATCH 0/3] target: Fix queue-full callback error signaling

fa7e25cf13a6d0b82b5ed1008246f44d42e8422c
a4467018c2a7228f4ef58051f0511bd037bff264
025def92dd6b5b84b0d6d9069e2bb24e51e48c17


-Bryant




[PATCH] ibmvscsis: Fix write_pending failure path

2017-10-02 Thread Bryant G. Ly
From: "Bryant G. Ly" <b...@us.ibm.com>

For write_pending if the queue is down or client failed
then return -EIO so that LIO can properly process the
completed command. Prior we returned 0 since LIO could
not handle it properly. Now with:
target: Fix unknown fabric callback queue-full errors
that patch addresses LIO's ability to handle things right.

Signed-off-by: Bryant G. Ly <b...@us.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 1f75d0380516f..fe5b9d7bc06df 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -3767,7 +3767,7 @@ static int ibmvscsis_write_pending(struct se_cmd *se_cmd)
 */
if ((vscsi->flags & (CLIENT_FAILED | RESPONSE_Q_DOWN))) {
pr_err("write_pending failed since: %d\n", vscsi->flags);
-   return 0;
+   return -EIO;
}
 
rc = srp_transfer_data(cmd, _iu(iue)->srp.cmd, ibmvscsis_rdma,
-- 
2.13.5 (Apple Git-94)



Re: [PATCHv2] tcmu: Fix possbile memory leak when recalculating the cmd base size

2017-07-11 Thread Bryant G. Ly



From: Xiubo Li <lixi...@cmss.chinamobile.com>

For all the entries allocated from the ring cmd area, the memory is
something like the stack memory, which will always reserve the old
data, so the entry->req.iov_bidi_cnt maybe none zero.

On some environments, the crash could be reporduce very easy and some
not. The following is the crash core trace:

[  240.143969] CPU: 0 PID: 1285 Comm: iscsi_trx Not tainted
4.12.0-rc1+ #3
[  240.150607] Hardware name: ASUS All Series/H87-PRO, BIOS 2104
10/28/2014
[  240.157331] task: 8807de4f5800 task.stack:
c900047dc000
[  240.163270] RIP: 0010:memcpy_erms+0x6/0x10
[  240.167377] RSP: 0018:c900047dfc68 EFLAGS: 00010202
[  240.172621] RAX: c9065db85540 RBX: 8807f798 RCX:
0010
[  240.179771] RDX: 0010 RSI: 8807de574fe0 RDI:
c9065db85540
[  240.186930] RBP: c900047dfd30 R08: 8807de41b000 R09:

[  240.194088] R10: 0040 R11: 8807e9b726f0 R12:
0006565726b0
[  240.201246] R13: c90007612ea0 R14: 00065657d540 R15:

[  240.208397] FS:  ()
GS:88081fa0()
knlGS:
[  240.216510] CS:  0010 DS:  ES:  CR0: 80050033
[  240.80] CR2: c9065db85540 CR3: 01c0f000 CR4:
001406f0
[  240.229430] Call Trace:
[  240.231887]  ? tcmu_queue_cmd+0x83c/0xa80
[  240.235916]  ? target_check_reservation+0xcd/0x6f0
[  240.240725]  __target_execute_cmd+0x27/0xa0
[  240.244918]  target_execute_cmd+0x232/0x2c0
[  240.249124]  ? __local_bh_enable_ip+0x64/0xa0
[  240.253499]  iscsit_execute_cmd+0x20d/0x270
[  240.257693]  iscsit_sequence_cmd+0x110/0x190
[  240.261985]  iscsit_get_rx_pdu+0x360/0xc80
[  240.267565]  ? iscsi_target_rx_thread+0x54/0xd0
[  240.273571]  iscsi_target_rx_thread+0x9a/0xd0
[  240.279413]  kthread+0x113/0x150
[  240.284120]  ? iscsi_target_tx_thread+0x1e0/0x1e0
[  240.290297]  ? kthread_create_on_node+0x40/0x40
[  240.296297]  ret_from_fork+0x2e/0x40
[  240.301332] Code: 90 90 90 90 90 eb 1e 0f 1f 00 48 89 f8 48
89 d1 48
c1 e9 03 83 e2 07 f3 48 a5 89 d1 f3 a4 c3 66 0f 1f 44 00 00 48
89 f8 48
89 d1  a4 c3 0f 1f 80 00 00 00 00 48 89 f8 48 83 fa 20 72 7e
40 38
[  240.321751] RIP: memcpy_erms+0x6/0x10 RSP: c900047dfc68
[  240.328838] CR2: c9065db85540
[  240.333667] ---[ end trace b7e5354cfb54d08b ]---

To fix this, just memset all the entry memory before using it, and
also to be more readable we adjust the bidi code.

Fixed: fe25cc34795(tcmu: Recalculate the tcmu_cmd size to save cmd area
memories)
Reported-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Tested-by: Damien Le Moal <damien.lem...@wdc.com>
Signed-off-by: Xiubo Li <lixi...@cmss.chinamobile.com>
---
  drivers/target/target_core_user.c | 12 +---
  1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index 2f1fa92..3b25ef3 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c


Nice! This has fixed our long standing issue with not being able to boot with 
the global data area support on power.

Tested-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>

-Bryant




Re: [PATCH 0/5] target: Zoned block device support and bug fixes

2017-06-28 Thread Bryant G. Ly



This series introduce zoned block device support for the pscsi backstore and
also fixes several problems with sense data handling for failed requests.

The first patch is only a cleanup, so not really necessary but nice to have I
think.

Patch 2 and 3 introduce support for host managed zoned block device type (14h)
in the SCSI passthrough backstore and fixes sense data handling for commands
failed by the backstore device. With these fixes, a host zoned block device
exported with the iscsi or loopback transport pass libzbc ZBC specification
conformance tests.

Finally, patch 4 and 5 fix sense data hadling with the user backstore code. A
prototype ZBC emulation tcmu-runner handler was used to test these fixes and
result in the emulated handler passing libzbc ZBC specification conformance
tests.

(Note: the ZBC emulation tcmu-runner handler will be submitted to the
tcmu-runner project on github)

Please consider these patches for inclusion with kernel 4.13.

Damien Le Moal (5):
   target: Use macro for WRITE_VERIFY_xx operation codes
   target: pscsi: Introduce TYPE_ZBC support
   target: pscsi: Fix sense data handling
   target: user: Fix sense data handling
   target: core: Fix failed command sense data handling

  drivers/target/target_core_device.c|  4 ++--
  drivers/target/target_core_pscsi.c | 20 +---
  drivers/target/target_core_transport.c |  5 +++--
  drivers/target/target_core_user.c  |  4 +++-
  include/scsi/scsi_proto.h  |  1 +
  5 files changed, 22 insertions(+), 12 deletions(-)


Hi Damien,

You should take a look at the first two patches in this series to address your 
sense data handling.

https://www.spinics.net/lists/target-devel/msg15430.html

-Bryant





Re: [PATCH] configfs: Fix race between create_link and configfs_rmdir

2017-06-08 Thread Bryant G. Ly



Thanks Nic,

applied to the configfs-for-next tree.  I'm not entirely sure if we
should bother adding this to 4.12 or if it hits rarely enough?


It hits for us pretty often when we have a GPFS setup with 10 hosts and 1k+ vms.

That is how we discovered the bug in the first place.

-Bryant



Re: [PATCH 2/7] scsi: ibmvscsi_tgt: remove use of class_attrs

2017-06-08 Thread Bryant G. Ly



The class_attrs pointer is going away and it's not even being used in
this driver, so just remove it entirely.

Cc: "Bryant G. Ly" <bryan...@linux.vnet.ibm.com>
Cc: Michael Cyr <mike...@linux.vnet.ibm.com>
Cc: "James E.J. Bottomley" <j...@linux.vnet.ibm.com>
Cc: "Martin K. Petersen" <martin.peter...@oracle.com>
Cc: <linux-scsi@vger.kernel.org>
Cc: <target-de...@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 5 -
  1 file changed, 5 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index d390325c99ec..b480878e3258 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -3915,10 +3915,6 @@ static const struct target_core_fabric_ops ibmvscsis_ops 
= {

  static void ibmvscsis_dev_release(struct device *dev) {};

-static struct class_attribute ibmvscsis_class_attrs[] = {
-   __ATTR_NULL,
-};
-
  static struct device_attribute dev_attr_system_id =
__ATTR(system_id, S_IRUGO, system_id_show, NULL);

@@ -3938,7 +3934,6 @@ ATTRIBUTE_GROUPS(ibmvscsis_dev);
  static struct class ibmvscsis_class = {
.name   = "ibmvscsis",
.dev_release= ibmvscsis_dev_release,
-   .class_attrs= ibmvscsis_class_attrs,
.dev_groups = ibmvscsis_dev_groups,
  };


Thanks Greg, ACK.

-Bryant




[PATCH v1] ibmvscsis: Use tpgt passed in by user

2017-06-06 Thread Bryant G. Ly
ibmvscsis always returned 0 for the tpg/tag, since it did not
parse the value passed in by the user.

When functions like ALUA members exports the value, it will
be incorrect because targetcli/rtslib starts the tpg numbering
at 1.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Signed-off-by: Mike Christie <mchri...@redhat.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 3571052..522d547 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -3914,8 +3914,16 @@ static struct se_portal_group *ibmvscsis_make_tpg(struct 
se_wwn *wwn,
 {
struct ibmvscsis_tport *tport =
container_of(wwn, struct ibmvscsis_tport, tport_wwn);
+   u16 tpgt;
int rc;
 
+   if (strstr(name, "tpgt_") != name)
+   return ERR_PTR(-EINVAL);
+   rc = kstrtou16(name + 5, 0, );
+   if (rc)
+   return ERR_PTR(rc);
+   tport->tport_tpgt = tpgt;
+
tport->releasing = false;
 
rc = core_tpg_register(>tport_wwn, >se_tpg,
-- 
2.5.4 (Apple Git-61)



[PATCH v4 3/5] tcmu: Make dev_size configurable via userspace

2017-06-06 Thread Bryant G. Ly
Allow tcmu backstores to be able to set the device size
after it has been configured via set attribute.

Part of support in userspace to support certain backstores
changing device size.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_user.c | 59 +++
 1 file changed, 54 insertions(+), 5 deletions(-)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index ae91822..c8c84b7 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1548,6 +1548,44 @@ static ssize_t tcmu_cmd_time_out_store(struct 
config_item *item, const char *pag
 }
 CONFIGFS_ATTR(tcmu_, cmd_time_out);
 
+static ssize_t tcmu_dev_size_show(struct config_item *item, char *page)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+
+   return snprintf(page, PAGE_SIZE, "%zu\n", udev->dev_size);
+}
+
+static ssize_t tcmu_dev_size_store(struct config_item *item, const char *page,
+  size_t count)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+   unsigned long val;
+   int ret;
+
+   ret = kstrtoul(page, 0, );
+   if (ret < 0)
+   return ret;
+   udev->dev_size = val;
+
+   /* Check if device has been configured before */
+   if (tcmu_dev_configured(udev)) {
+   ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
+udev->uio_info.name,
+udev->uio_info.uio_dev->minor);
+   if (ret) {
+   pr_err("Unable to reconfigure device\n");
+   return ret;
+   }
+   }
+
+   return count;
+}
+CONFIGFS_ATTR(tcmu_, dev_size);
+
 static ssize_t tcmu_emulate_write_cache_show(struct config_item *item,
 char *page)
 {
@@ -1586,6 +1624,13 @@ static ssize_t tcmu_emulate_write_cache_store(struct 
config_item *item,
 }
 CONFIGFS_ATTR(tcmu_, emulate_write_cache);
 
+struct configfs_attribute *tcmu_attrib_attrs[] = {
+   _attr_cmd_time_out,
+   _attr_dev_size,
+   _attr_emulate_write_cache,
+   NULL,
+};
+
 static struct configfs_attribute **tcmu_attrs;
 
 static struct target_backend_ops tcmu_ops = {
@@ -1685,7 +1730,7 @@ static int unmap_thread_fn(void *data)
 
 static int __init tcmu_module_init(void)
 {
-   int ret, i, len = 0;
+   int ret, i, k, len = 0;
 
BUILD_BUG_ON((sizeof(struct tcmu_cmd_entry) % TCMU_OP_ALIGN_SIZE) != 0);
 
@@ -1710,7 +1755,10 @@ static int __init tcmu_module_init(void)
for (i = 0; passthrough_attrib_attrs[i] != NULL; i++) {
len += sizeof(struct configfs_attribute *);
}
-   len += sizeof(struct configfs_attribute *) * 2;
+   for (i = 0; tcmu_attrib_attrs[i] != NULL; i++) {
+   len += sizeof(struct configfs_attribute *);
+   }
+   len += sizeof(struct configfs_attribute *);
 
tcmu_attrs = kzalloc(len, GFP_KERNEL);
if (!tcmu_attrs) {
@@ -1721,9 +1769,10 @@ static int __init tcmu_module_init(void)
for (i = 0; passthrough_attrib_attrs[i] != NULL; i++) {
tcmu_attrs[i] = passthrough_attrib_attrs[i];
}
-   tcmu_attrs[i] = _attr_cmd_time_out;
-   i++;
-   tcmu_attrs[i] = _attr_emulate_write_cache;
+   for (k = 0; tcmu_attrib_attrs[k] != NULL; k++) {
+   tcmu_attrs[i] = tcmu_attrib_attrs[k];
+   i++;
+   }
tcmu_ops.tb_dev_attrib_attrs = tcmu_attrs;
 
ret = transport_backend_register(_ops);
-- 
2.5.4 (Apple Git-61)



[PATCH v4 5/5] tcmu: Add Type of reconfig into netlink

2017-06-06 Thread Bryant G. Ly
This patch adds more info about the attribute being changed,
so that usersapce can easily figure out what is happening.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_user.c | 20 ++--
 include/uapi/linux/target_core_user.h |  8 
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index 7c64757..afc1fd6 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1176,7 +1176,8 @@ static int tcmu_release(struct uio_info *info, struct 
inode *inode)
return 0;
 }
 
-static int tcmu_netlink_event(enum tcmu_genl_cmd cmd, const char *name, int 
minor)
+static int tcmu_netlink_event(enum tcmu_genl_cmd cmd, const char *name,
+ int minor, int type)
 {
struct sk_buff *skb;
void *msg_header;
@@ -1198,6 +1199,10 @@ static int tcmu_netlink_event(enum tcmu_genl_cmd cmd, 
const char *name, int mino
if (ret < 0)
goto free_skb;
 
+   ret = nla_put_u32(skb, TCMU_ATTR_TYPE, type);
+   if (ret < 0)
+   goto free_skb;
+
genlmsg_end(skb, msg_header);
 
ret = genlmsg_multicast_allns(_genl_family, skb, 0,
@@ -1301,7 +1306,7 @@ static int tcmu_configure_device(struct se_device *dev)
kref_get(>kref);
 
ret = tcmu_netlink_event(TCMU_CMD_ADDED_DEVICE, udev->uio_info.name,
-udev->uio_info.uio_dev->minor);
+udev->uio_info.uio_dev->minor, NO_RECONFIG);
if (ret)
goto err_netlink;
 
@@ -1383,7 +1388,7 @@ static void tcmu_free_device(struct se_device *dev)
 
if (tcmu_dev_configured(udev)) {
tcmu_netlink_event(TCMU_CMD_REMOVED_DEVICE, udev->uio_info.name,
-  udev->uio_info.uio_dev->minor);
+  udev->uio_info.uio_dev->minor, NO_RECONFIG);
 
uio_unregister_device(>uio_info);
}
@@ -1577,7 +1582,8 @@ static ssize_t tcmu_dev_path_store(struct config_item 
*item, const char *page,
if (tcmu_dev_configured(udev)) {
ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
 udev->uio_info.name,
-udev->uio_info.uio_dev->minor);
+udev->uio_info.uio_dev->minor,
+CONFIG_PATH);
if (ret) {
pr_err("Unable to reconfigure device\n");
return ret;
@@ -1615,7 +1621,8 @@ static ssize_t tcmu_dev_size_store(struct config_item 
*item, const char *page,
if (tcmu_dev_configured(udev)) {
ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
 udev->uio_info.name,
-udev->uio_info.uio_dev->minor);
+udev->uio_info.uio_dev->minor,
+CONFIG_SIZE);
if (ret) {
pr_err("Unable to reconfigure device\n");
return ret;
@@ -1654,7 +1661,8 @@ static ssize_t tcmu_emulate_write_cache_store(struct 
config_item *item,
if (tcmu_dev_configured(udev)) {
ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
 udev->uio_info.name,
-udev->uio_info.uio_dev->minor);
+udev->uio_info.uio_dev->minor,
+CONFIG_WRITECACHE);
if (ret) {
pr_err("Unable to reconfigure device\n");
return ret;
diff --git a/include/uapi/linux/target_core_user.h 
b/include/uapi/linux/target_core_user.h
index 403a61f..5b00e35 100644
--- a/include/uapi/linux/target_core_user.h
+++ b/include/uapi/linux/target_core_user.h
@@ -139,8 +139,16 @@ enum tcmu_genl_attr {
TCMU_ATTR_UNSPEC,
TCMU_ATTR_DEVICE,
TCMU_ATTR_MINOR,
+   TCMU_ATTR_TYPE,
__TCMU_ATTR_MAX,
 };
 #define TCMU_ATTR_MAX (__TCMU_ATTR_MAX - 1)
 
+enum tcmu_reconfig_types {
+   NO_RECONFIG,
+   CONFIG_PATH,
+   CONFIG_SIZE,
+   CONFIG_WRITECACHE,
+};
+
 #endif
-- 
2.5.4 (Apple Git-61)



[PATCH v4 4/5] tcmu: Make dev_config configurable

2017-06-06 Thread Bryant G. Ly
This allows for userspace to change the device path after
it has been created. Thus giving the user the ability to change
the path. The use case for this is to allow for virtual optical
to have media change.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_user.c | 41 +++
 1 file changed, 41 insertions(+)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index c8c84b7..7c64757 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1548,6 +1548,46 @@ static ssize_t tcmu_cmd_time_out_store(struct 
config_item *item, const char *pag
 }
 CONFIGFS_ATTR(tcmu_, cmd_time_out);
 
+static ssize_t tcmu_dev_path_show(struct config_item *item, char *page)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+
+   return snprintf(page, PAGE_SIZE, "%s\n", udev->dev_config);
+}
+
+static ssize_t tcmu_dev_path_store(struct config_item *item, const char *page,
+  size_t count)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+   char *copy = NULL;
+   int ret;
+
+   copy = kstrdup(page, GFP_KERNEL);
+   if (!copy) {
+   kfree(copy);
+   return -EINVAL;
+   }
+   strlcpy(udev->dev_config, copy, TCMU_CONFIG_LEN);
+
+   /* Check if device has been configured before */
+   if (tcmu_dev_configured(udev)) {
+   ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
+udev->uio_info.name,
+udev->uio_info.uio_dev->minor);
+   if (ret) {
+   pr_err("Unable to reconfigure device\n");
+   return ret;
+   }
+   }
+
+   return count;
+}
+CONFIGFS_ATTR(tcmu_, dev_path);
+
 static ssize_t tcmu_dev_size_show(struct config_item *item, char *page)
 {
struct se_dev_attrib *da = container_of(to_config_group(item),
@@ -1626,6 +1666,7 @@ CONFIGFS_ATTR(tcmu_, emulate_write_cache);
 
 struct configfs_attribute *tcmu_attrib_attrs[] = {
_attr_cmd_time_out,
+   _attr_dev_path,
_attr_dev_size,
_attr_emulate_write_cache,
NULL,
-- 
2.5.4 (Apple Git-61)



[PATCH v4 2/5] tcmu: Add netlink for device reconfiguration

2017-06-06 Thread Bryant G. Ly
This gives tcmu the ability to handle events that can cause
reconfiguration, such as resize, path changes, write_cache, etc...

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-By: Mike Christie <mchri...@redhat.com>
---
 drivers/target/target_core_user.c | 12 
 include/uapi/linux/target_core_user.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index 0c797cc..ae91822 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1562,6 +1562,7 @@ static ssize_t tcmu_emulate_write_cache_store(struct 
config_item *item,
 {
struct se_dev_attrib *da = container_of(to_config_group(item),
struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
int val;
int ret;
 
@@ -1570,6 +1571,17 @@ static ssize_t tcmu_emulate_write_cache_store(struct 
config_item *item,
return ret;
 
da->emulate_write_cache = val;
+
+   /* Check if device has been configured before */
+   if (tcmu_dev_configured(udev)) {
+   ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
+udev->uio_info.name,
+udev->uio_info.uio_dev->minor);
+   if (ret) {
+   pr_err("Unable to reconfigure device\n");
+   return ret;
+   }
+   }
return count;
 }
 CONFIGFS_ATTR(tcmu_, emulate_write_cache);
diff --git a/include/uapi/linux/target_core_user.h 
b/include/uapi/linux/target_core_user.h
index af17b41..403a61f 100644
--- a/include/uapi/linux/target_core_user.h
+++ b/include/uapi/linux/target_core_user.h
@@ -130,6 +130,7 @@ enum tcmu_genl_cmd {
TCMU_CMD_UNSPEC,
TCMU_CMD_ADDED_DEVICE,
TCMU_CMD_REMOVED_DEVICE,
+   TCMU_CMD_RECONFIG_DEVICE,
__TCMU_CMD_MAX,
 };
 #define TCMU_CMD_MAX (__TCMU_CMD_MAX - 1)
-- 
2.5.4 (Apple Git-61)



[PATCH v4 0/5] tcmu: Add Type of reconfig into netlink

2017-06-06 Thread Bryant G. Ly
From: "Bryant G. Ly" <b...@us.ibm.com>

This patch consists of adding a netlink to allow for reconfiguration
of a device in tcmu.

It also changes and adds some attributes that are reconfigurable:
write_cache, device size, and device path.

V2 - Fixes kfree in tcmu: Make dev_config configurable
V3 - Fixes spelling error
V4 - change strcpy to strlcpy for tcmu_dev_path_store and move
 tcmu_reconfig_type into target_core_user.h


Bryant G. Ly (5):
  tcmu: Support emulate_write_cache
  tcmu: Add netlink for device reconfiguration
  tcmu: Make dev_size configurable via userspace
  tcmu: Make dev_config configurable
  tcmu: Add Type of reconfig into netlink

 drivers/target/target_core_user.c | 152 --
 include/uapi/linux/target_core_user.h |   9 ++
 2 files changed, 155 insertions(+), 6 deletions(-)

-- 
2.5.4 (Apple Git-61)



[PATCH v4 1/5] tcmu: Support emulate_write_cache

2017-06-06 Thread Bryant G. Ly
This will enable the toggling of write_cache in tcmu through targetcli-fb

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-By: Mike Christie <mchri...@redhat.com>
---
 drivers/target/target_core_user.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index beb5f09..0c797cc 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1290,6 +1290,8 @@ static int tcmu_configure_device(struct se_device *dev)
/* Other attributes can be configured in userspace */
if (!dev->dev_attrib.hw_max_sectors)
dev->dev_attrib.hw_max_sectors = 128;
+   if (!dev->dev_attrib.emulate_write_cache)
+   dev->dev_attrib.emulate_write_cache = 0;
dev->dev_attrib.hw_queue_depth = 128;
 
/*
@@ -1546,6 +1548,32 @@ static ssize_t tcmu_cmd_time_out_store(struct 
config_item *item, const char *pag
 }
 CONFIGFS_ATTR(tcmu_, cmd_time_out);
 
+static ssize_t tcmu_emulate_write_cache_show(struct config_item *item,
+char *page)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+
+   return snprintf(page, PAGE_SIZE, "%i\n", da->emulate_write_cache);
+}
+
+static ssize_t tcmu_emulate_write_cache_store(struct config_item *item,
+ const char *page, size_t count)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   int val;
+   int ret;
+
+   ret = kstrtouint(page, 0, );
+   if (ret < 0)
+   return ret;
+
+   da->emulate_write_cache = val;
+   return count;
+}
+CONFIGFS_ATTR(tcmu_, emulate_write_cache);
+
 static struct configfs_attribute **tcmu_attrs;
 
 static struct target_backend_ops tcmu_ops = {
@@ -1682,6 +1710,8 @@ static int __init tcmu_module_init(void)
tcmu_attrs[i] = passthrough_attrib_attrs[i];
}
tcmu_attrs[i] = _attr_cmd_time_out;
+   i++;
+   tcmu_attrs[i] = _attr_emulate_write_cache;
tcmu_ops.tb_dev_attrib_attrs = tcmu_attrs;
 
ret = transport_backend_register(_ops);
-- 
2.5.4 (Apple Git-61)



[PATCH v3 4/5] tcmu: Make dev_config configurable

2017-05-31 Thread Bryant G. Ly
This allows for userspace to change the device path after
it has been created. Thus giving the user the ability to change
the path. The use case for this is to allow for virtual optical
to have media change.

v3 - Fix kree spelling error to kfree

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_user.c | 41 +++
 1 file changed, 41 insertions(+)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index a4cf678..eea4630 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1577,6 +1577,46 @@ static ssize_t tcmu_cmd_time_out_store(struct 
config_item *item, const char *pag
 }
 CONFIGFS_ATTR(tcmu_, cmd_time_out);
 
+static ssize_t tcmu_dev_path_show(struct config_item *item, char *page)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+
+   return snprintf(page, PAGE_SIZE, "%s\n", udev->dev_config);
+}
+
+static ssize_t tcmu_dev_path_store(struct config_item *item, const char *page,
+  size_t count)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+   char *copy = NULL;
+   int ret;
+
+   copy = kstrdup(page, GFP_KERNEL);
+   if (!copy) {
+   kfree(copy);
+   return -EINVAL;
+   }
+   strcpy(udev->dev_config, copy);
+
+   /* Check if device has been configured before */
+   if (tcmu_dev_configured(udev)) {
+   ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
+udev->uio_info.name,
+udev->uio_info.uio_dev->minor);
+   if (ret) {
+   pr_err("Unable to reconfigure device\n");
+   return ret;
+   }
+   }
+
+   return count;
+}
+CONFIGFS_ATTR(tcmu_, dev_path);
+
 static ssize_t tcmu_dev_size_show(struct config_item *item, char *page)
 {
struct se_dev_attrib *da = container_of(to_config_group(item),
@@ -1655,6 +1695,7 @@ CONFIGFS_ATTR(tcmu_, emulate_write_cache);
 
 struct configfs_attribute *tcmu_attrib_attrs[] = {
_attr_cmd_time_out,
+   _attr_dev_path,
_attr_dev_size,
_attr_emulate_write_cache,
NULL,
-- 
2.5.4 (Apple Git-61)



[PATCH v2 1/5] tcmu: Support emulate_write_cache

2017-05-30 Thread Bryant G. Ly
This will enable the toggling of write_cache in tcmu through targetcli-fb

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-By: Mike Christie <mchri...@redhat.com>
---
 drivers/target/target_core_user.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index beb5f09..0c797cc 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1290,6 +1290,8 @@ static int tcmu_configure_device(struct se_device *dev)
/* Other attributes can be configured in userspace */
if (!dev->dev_attrib.hw_max_sectors)
dev->dev_attrib.hw_max_sectors = 128;
+   if (!dev->dev_attrib.emulate_write_cache)
+   dev->dev_attrib.emulate_write_cache = 0;
dev->dev_attrib.hw_queue_depth = 128;
 
/*
@@ -1546,6 +1548,32 @@ static ssize_t tcmu_cmd_time_out_store(struct 
config_item *item, const char *pag
 }
 CONFIGFS_ATTR(tcmu_, cmd_time_out);
 
+static ssize_t tcmu_emulate_write_cache_show(struct config_item *item,
+char *page)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+
+   return snprintf(page, PAGE_SIZE, "%i\n", da->emulate_write_cache);
+}
+
+static ssize_t tcmu_emulate_write_cache_store(struct config_item *item,
+ const char *page, size_t count)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   int val;
+   int ret;
+
+   ret = kstrtouint(page, 0, );
+   if (ret < 0)
+   return ret;
+
+   da->emulate_write_cache = val;
+   return count;
+}
+CONFIGFS_ATTR(tcmu_, emulate_write_cache);
+
 static struct configfs_attribute **tcmu_attrs;
 
 static struct target_backend_ops tcmu_ops = {
@@ -1682,6 +1710,8 @@ static int __init tcmu_module_init(void)
tcmu_attrs[i] = passthrough_attrib_attrs[i];
}
tcmu_attrs[i] = _attr_cmd_time_out;
+   i++;
+   tcmu_attrs[i] = _attr_emulate_write_cache;
tcmu_ops.tb_dev_attrib_attrs = tcmu_attrs;
 
ret = transport_backend_register(_ops);
-- 
2.5.4 (Apple Git-61)



[PATCH v2 5/5] tcmu: Add Type of reconfig into netlink

2017-05-30 Thread Bryant G. Ly
This patch adds more info about the attribute being changed,
so that usersapce can easily figure out what is happening.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_user.c | 27 +--
 include/uapi/linux/target_core_user.h |  1 +
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index 7575bc9..2d461c4 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -190,6 +190,13 @@ static struct genl_family tcmu_genl_family __ro_after_init 
= {
.netnsok = true,
 };
 
+enum tcmu_reconfig_types {
+   No_reconfig,
+   Config_path,
+   Config_size,
+   Config_writecache
+};
+
 #define tcmu_cmd_set_dbi_cur(cmd, index) ((cmd)->dbi_cur = (index))
 #define tcmu_cmd_reset_dbi_cur(cmd) tcmu_cmd_set_dbi_cur(cmd, 0)
 #define tcmu_cmd_set_dbi(cmd, index) ((cmd)->dbi[(cmd)->dbi_cur++] = (index))
@@ -1176,7 +1183,8 @@ static int tcmu_release(struct uio_info *info, struct 
inode *inode)
return 0;
 }
 
-static int tcmu_netlink_event(enum tcmu_genl_cmd cmd, const char *name, int 
minor)
+static int tcmu_netlink_event(enum tcmu_genl_cmd cmd, const char *name,
+ int minor, int type)
 {
struct sk_buff *skb;
void *msg_header;
@@ -1198,6 +1206,10 @@ static int tcmu_netlink_event(enum tcmu_genl_cmd cmd, 
const char *name, int mino
if (ret < 0)
goto free_skb;
 
+   ret = nla_put_u32(skb, TCMU_ATTR_TYPE, type);
+   if (ret < 0)
+   goto free_skb;
+
genlmsg_end(skb, msg_header);
 
ret = genlmsg_multicast_allns(_genl_family, skb, 0,
@@ -1301,7 +1313,7 @@ static int tcmu_configure_device(struct se_device *dev)
kref_get(>kref);
 
ret = tcmu_netlink_event(TCMU_CMD_ADDED_DEVICE, udev->uio_info.name,
-udev->uio_info.uio_dev->minor);
+udev->uio_info.uio_dev->minor, No_reconfig);
if (ret)
goto err_netlink;
 
@@ -1383,7 +1395,7 @@ static void tcmu_free_device(struct se_device *dev)
 
if (tcmu_dev_configured(udev)) {
tcmu_netlink_event(TCMU_CMD_REMOVED_DEVICE, udev->uio_info.name,
-  udev->uio_info.uio_dev->minor);
+  udev->uio_info.uio_dev->minor, No_reconfig);
 
uio_unregister_device(>uio_info);
}
@@ -1577,7 +1589,8 @@ static ssize_t tcmu_dev_path_store(struct config_item 
*item, const char *page,
if (tcmu_dev_configured(udev)) {
ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
 udev->uio_info.name,
-udev->uio_info.uio_dev->minor);
+udev->uio_info.uio_dev->minor,
+Config_path);
if (ret) {
pr_err("Unable to reconfigure device\n");
return ret;
@@ -1615,7 +1628,8 @@ static ssize_t tcmu_dev_size_store(struct config_item 
*item, const char *page,
if (tcmu_dev_configured(udev)) {
ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
 udev->uio_info.name,
-udev->uio_info.uio_dev->minor);
+udev->uio_info.uio_dev->minor,
+Config_size);
if (ret) {
pr_err("Unable to reconfigure device\n");
return ret;
@@ -1654,7 +1668,8 @@ static ssize_t tcmu_emulate_write_cache_store(struct 
config_item *item,
if (tcmu_dev_configured(udev)) {
ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
 udev->uio_info.name,
-udev->uio_info.uio_dev->minor);
+udev->uio_info.uio_dev->minor,
+Config_writecache);
if (ret) {
pr_err("Unable to reconfigure device\n");
return ret;
diff --git a/include/uapi/linux/target_core_user.h 
b/include/uapi/linux/target_core_user.h
index 403a61f..ebad1f8 100644
--- a/include/uapi/linux/target_core_user.h
+++ b/include/uapi/linux/target_core_user.h
@@ -139,6 +139,7 @@ enum tcmu_genl_attr {
TCMU_ATTR_UNSPEC,
TCMU_ATTR_DEVICE,
TCMU_ATTR_MINOR,
+   TCMU_ATTR_TYPE,
__TCMU_ATTR_MAX,
 };
 #define TCMU_ATTR_MAX (__TCMU_ATTR_MAX - 1)
-- 
2.5.4 (Apple Git-61)



[PATCH v2 0/5] TCMU Enable Reconfiguration Patches

2017-05-30 Thread Bryant G. Ly
This patch consists of adding a netlink to allow for reconfiguration
of a device in tcmu.

It also changes and adds some attributes that are reconfigurable:
write_cache, device size, and device path.

Bryant G. Ly (5):
  tcmu: Support emulate_write_cache
  tcmu: Add netlink for device reconfiguration
  tcmu: Make dev_size configurable via userspace
  tcmu: Make dev_config configurable
  tcmu: Add Type of reconfig into netlink

 drivers/target/target_core_user.c | 159 --
 include/uapi/linux/target_core_user.h |   2 +
 2 files changed, 155 insertions(+), 6 deletions(-)

-- 
2.5.4 (Apple Git-61)



[PATCH v2 2/5] tcmu: Add netlink for device reconfiguration

2017-05-30 Thread Bryant G. Ly
This gives tcmu the ability to handle events that can cause
reconfiguration, such as resize, path changes, write_cache, etc...

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-By: Mike Christie <mchri...@redhat.com>
---
 drivers/target/target_core_user.c | 12 
 include/uapi/linux/target_core_user.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index 0c797cc..ae91822 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1562,6 +1562,7 @@ static ssize_t tcmu_emulate_write_cache_store(struct 
config_item *item,
 {
struct se_dev_attrib *da = container_of(to_config_group(item),
struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
int val;
int ret;
 
@@ -1570,6 +1571,17 @@ static ssize_t tcmu_emulate_write_cache_store(struct 
config_item *item,
return ret;
 
da->emulate_write_cache = val;
+
+   /* Check if device has been configured before */
+   if (tcmu_dev_configured(udev)) {
+   ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
+udev->uio_info.name,
+udev->uio_info.uio_dev->minor);
+   if (ret) {
+   pr_err("Unable to reconfigure device\n");
+   return ret;
+   }
+   }
return count;
 }
 CONFIGFS_ATTR(tcmu_, emulate_write_cache);
diff --git a/include/uapi/linux/target_core_user.h 
b/include/uapi/linux/target_core_user.h
index af17b41..403a61f 100644
--- a/include/uapi/linux/target_core_user.h
+++ b/include/uapi/linux/target_core_user.h
@@ -130,6 +130,7 @@ enum tcmu_genl_cmd {
TCMU_CMD_UNSPEC,
TCMU_CMD_ADDED_DEVICE,
TCMU_CMD_REMOVED_DEVICE,
+   TCMU_CMD_RECONFIG_DEVICE,
__TCMU_CMD_MAX,
 };
 #define TCMU_CMD_MAX (__TCMU_CMD_MAX - 1)
-- 
2.5.4 (Apple Git-61)



[PATCH v2 4/5] tcmu: Make dev_config configurable

2017-05-30 Thread Bryant G. Ly
This allows for userspace to change the device path after
it has been created. Thus giving the user the ability to change
the path. The use case for this is to allow for virtual optical
to have media change.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_user.c | 41 +++
 1 file changed, 41 insertions(+)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index c8c84b7..7575bc9 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1548,6 +1548,46 @@ static ssize_t tcmu_cmd_time_out_store(struct 
config_item *item, const char *pag
 }
 CONFIGFS_ATTR(tcmu_, cmd_time_out);
 
+static ssize_t tcmu_dev_path_show(struct config_item *item, char *page)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+
+   return snprintf(page, PAGE_SIZE, "%s\n", udev->dev_config);
+}
+
+static ssize_t tcmu_dev_path_store(struct config_item *item, const char *page,
+  size_t count)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+   char *copy = NULL;
+   int ret;
+
+   copy = kstrdup(page, GFP_KERNEL);
+   if (!copy) {
+   kree(copy);
+   return -EINVAL;
+   }
+   strcpy(udev->dev_config, copy);
+
+   /* Check if device has been configured before */
+   if (tcmu_dev_configured(udev)) {
+   ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
+udev->uio_info.name,
+udev->uio_info.uio_dev->minor);
+   if (ret) {
+   pr_err("Unable to reconfigure device\n");
+   return ret;
+   }
+   }
+
+   return count;
+}
+CONFIGFS_ATTR(tcmu_, dev_path);
+
 static ssize_t tcmu_dev_size_show(struct config_item *item, char *page)
 {
struct se_dev_attrib *da = container_of(to_config_group(item),
@@ -1626,6 +1666,7 @@ CONFIGFS_ATTR(tcmu_, emulate_write_cache);
 
 struct configfs_attribute *tcmu_attrib_attrs[] = {
_attr_cmd_time_out,
+   _attr_dev_path,
_attr_dev_size,
_attr_emulate_write_cache,
NULL,
-- 
2.5.4 (Apple Git-61)



[PATCH v2 3/5] tcmu: Make dev_size configurable via userspace

2017-05-30 Thread Bryant G. Ly
Allow tcmu backstores to be able to set the device size
after it has been configured via set attribute.

Part of support in userspace to support certain backstores
changing device size.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_user.c | 59 +++
 1 file changed, 54 insertions(+), 5 deletions(-)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index ae91822..c8c84b7 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1548,6 +1548,44 @@ static ssize_t tcmu_cmd_time_out_store(struct 
config_item *item, const char *pag
 }
 CONFIGFS_ATTR(tcmu_, cmd_time_out);
 
+static ssize_t tcmu_dev_size_show(struct config_item *item, char *page)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+
+   return snprintf(page, PAGE_SIZE, "%zu\n", udev->dev_size);
+}
+
+static ssize_t tcmu_dev_size_store(struct config_item *item, const char *page,
+  size_t count)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+   unsigned long val;
+   int ret;
+
+   ret = kstrtoul(page, 0, );
+   if (ret < 0)
+   return ret;
+   udev->dev_size = val;
+
+   /* Check if device has been configured before */
+   if (tcmu_dev_configured(udev)) {
+   ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
+udev->uio_info.name,
+udev->uio_info.uio_dev->minor);
+   if (ret) {
+   pr_err("Unable to reconfigure device\n");
+   return ret;
+   }
+   }
+
+   return count;
+}
+CONFIGFS_ATTR(tcmu_, dev_size);
+
 static ssize_t tcmu_emulate_write_cache_show(struct config_item *item,
 char *page)
 {
@@ -1586,6 +1624,13 @@ static ssize_t tcmu_emulate_write_cache_store(struct 
config_item *item,
 }
 CONFIGFS_ATTR(tcmu_, emulate_write_cache);
 
+struct configfs_attribute *tcmu_attrib_attrs[] = {
+   _attr_cmd_time_out,
+   _attr_dev_size,
+   _attr_emulate_write_cache,
+   NULL,
+};
+
 static struct configfs_attribute **tcmu_attrs;
 
 static struct target_backend_ops tcmu_ops = {
@@ -1685,7 +1730,7 @@ static int unmap_thread_fn(void *data)
 
 static int __init tcmu_module_init(void)
 {
-   int ret, i, len = 0;
+   int ret, i, k, len = 0;
 
BUILD_BUG_ON((sizeof(struct tcmu_cmd_entry) % TCMU_OP_ALIGN_SIZE) != 0);
 
@@ -1710,7 +1755,10 @@ static int __init tcmu_module_init(void)
for (i = 0; passthrough_attrib_attrs[i] != NULL; i++) {
len += sizeof(struct configfs_attribute *);
}
-   len += sizeof(struct configfs_attribute *) * 2;
+   for (i = 0; tcmu_attrib_attrs[i] != NULL; i++) {
+   len += sizeof(struct configfs_attribute *);
+   }
+   len += sizeof(struct configfs_attribute *);
 
tcmu_attrs = kzalloc(len, GFP_KERNEL);
if (!tcmu_attrs) {
@@ -1721,9 +1769,10 @@ static int __init tcmu_module_init(void)
for (i = 0; passthrough_attrib_attrs[i] != NULL; i++) {
tcmu_attrs[i] = passthrough_attrib_attrs[i];
}
-   tcmu_attrs[i] = _attr_cmd_time_out;
-   i++;
-   tcmu_attrs[i] = _attr_emulate_write_cache;
+   for (k = 0; tcmu_attrib_attrs[k] != NULL; k++) {
+   tcmu_attrs[i] = tcmu_attrib_attrs[k];
+   i++;
+   }
tcmu_ops.tb_dev_attrib_attrs = tcmu_attrs;
 
ret = transport_backend_register(_ops);
-- 
2.5.4 (Apple Git-61)



[PATCH v1 4/4] tcmu: Make dev_config configurable

2017-05-26 Thread Bryant G. Ly
This allows for userspace to change the device path after
it has been created. Thus giving the user the ability to change
the path. The use case for this is to allow for virtual optical
to have media change.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_user.c | 40 +++
 1 file changed, 40 insertions(+)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index c8c84b7..3036a57 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1548,6 +1548,45 @@ static ssize_t tcmu_cmd_time_out_store(struct 
config_item *item, const char *pag
 }
 CONFIGFS_ATTR(tcmu_, cmd_time_out);
 
+static ssize_t tcmu_dev_path_show(struct config_item *item, char *page)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+
+   return snprintf(page, PAGE_SIZE, "%s\n", udev->dev_config);
+}
+
+static ssize_t tcmu_dev_path_store(struct config_item *item, const char *page,
+  size_t count)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+   char *copy = NULL;
+
+   copy = kstrdup(page, GFP_KERNEL);
+
+   if (!copy)
+   return -EINVAL;
+
+   strcpy(udev->dev_config, copy);
+
+   /* Check if device has been configured before */
+   if (tcmu_dev_configured(udev)) {
+   ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
+udev->uio_info.name,
+udev->uio_info.uio_dev->minor);
+   if (ret) {
+   pr_err("Unable to reconfigure device\n");
+   return ret;
+   }
+   }
+
+   return count;
+}
+CONFIGFS_ATTR(tcmu_, dev_path);
+
 static ssize_t tcmu_dev_size_show(struct config_item *item, char *page)
 {
struct se_dev_attrib *da = container_of(to_config_group(item),
@@ -1626,6 +1665,7 @@ CONFIGFS_ATTR(tcmu_, emulate_write_cache);
 
 struct configfs_attribute *tcmu_attrib_attrs[] = {
_attr_cmd_time_out,
+   _attr_dev_path,
_attr_dev_size,
_attr_emulate_write_cache,
NULL,
-- 
2.5.4 (Apple Git-61)



[PATCH v1 1/4] tcmu: Support emulate_write_cache

2017-05-26 Thread Bryant G. Ly
This will enable the toggling of write_cache in tcmu through targetcli-fb

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-By: Mike Christie <mchri...@redhat.com>
---
 drivers/target/target_core_user.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index beb5f09..0c797cc 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1290,6 +1290,8 @@ static int tcmu_configure_device(struct se_device *dev)
/* Other attributes can be configured in userspace */
if (!dev->dev_attrib.hw_max_sectors)
dev->dev_attrib.hw_max_sectors = 128;
+   if (!dev->dev_attrib.emulate_write_cache)
+   dev->dev_attrib.emulate_write_cache = 0;
dev->dev_attrib.hw_queue_depth = 128;
 
/*
@@ -1546,6 +1548,32 @@ static ssize_t tcmu_cmd_time_out_store(struct 
config_item *item, const char *pag
 }
 CONFIGFS_ATTR(tcmu_, cmd_time_out);
 
+static ssize_t tcmu_emulate_write_cache_show(struct config_item *item,
+char *page)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+
+   return snprintf(page, PAGE_SIZE, "%i\n", da->emulate_write_cache);
+}
+
+static ssize_t tcmu_emulate_write_cache_store(struct config_item *item,
+ const char *page, size_t count)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   int val;
+   int ret;
+
+   ret = kstrtouint(page, 0, );
+   if (ret < 0)
+   return ret;
+
+   da->emulate_write_cache = val;
+   return count;
+}
+CONFIGFS_ATTR(tcmu_, emulate_write_cache);
+
 static struct configfs_attribute **tcmu_attrs;
 
 static struct target_backend_ops tcmu_ops = {
@@ -1682,6 +1710,8 @@ static int __init tcmu_module_init(void)
tcmu_attrs[i] = passthrough_attrib_attrs[i];
}
tcmu_attrs[i] = _attr_cmd_time_out;
+   i++;
+   tcmu_attrs[i] = _attr_emulate_write_cache;
tcmu_ops.tb_dev_attrib_attrs = tcmu_attrs;
 
ret = transport_backend_register(_ops);
-- 
2.5.4 (Apple Git-61)



[PATCH v1 3/4] tcmu: Make dev_size configurable via userspace

2017-05-26 Thread Bryant G. Ly
Allow tcmu backstores to be able to set the device size
after it has been configured via set attribute.

Part of support in userspace to support certain backstores
changing device size.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_user.c | 59 +++
 1 file changed, 54 insertions(+), 5 deletions(-)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index ae91822..c8c84b7 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1548,6 +1548,44 @@ static ssize_t tcmu_cmd_time_out_store(struct 
config_item *item, const char *pag
 }
 CONFIGFS_ATTR(tcmu_, cmd_time_out);
 
+static ssize_t tcmu_dev_size_show(struct config_item *item, char *page)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+
+   return snprintf(page, PAGE_SIZE, "%zu\n", udev->dev_size);
+}
+
+static ssize_t tcmu_dev_size_store(struct config_item *item, const char *page,
+  size_t count)
+{
+   struct se_dev_attrib *da = container_of(to_config_group(item),
+   struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
+   unsigned long val;
+   int ret;
+
+   ret = kstrtoul(page, 0, );
+   if (ret < 0)
+   return ret;
+   udev->dev_size = val;
+
+   /* Check if device has been configured before */
+   if (tcmu_dev_configured(udev)) {
+   ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
+udev->uio_info.name,
+udev->uio_info.uio_dev->minor);
+   if (ret) {
+   pr_err("Unable to reconfigure device\n");
+   return ret;
+   }
+   }
+
+   return count;
+}
+CONFIGFS_ATTR(tcmu_, dev_size);
+
 static ssize_t tcmu_emulate_write_cache_show(struct config_item *item,
 char *page)
 {
@@ -1586,6 +1624,13 @@ static ssize_t tcmu_emulate_write_cache_store(struct 
config_item *item,
 }
 CONFIGFS_ATTR(tcmu_, emulate_write_cache);
 
+struct configfs_attribute *tcmu_attrib_attrs[] = {
+   _attr_cmd_time_out,
+   _attr_dev_size,
+   _attr_emulate_write_cache,
+   NULL,
+};
+
 static struct configfs_attribute **tcmu_attrs;
 
 static struct target_backend_ops tcmu_ops = {
@@ -1685,7 +1730,7 @@ static int unmap_thread_fn(void *data)
 
 static int __init tcmu_module_init(void)
 {
-   int ret, i, len = 0;
+   int ret, i, k, len = 0;
 
BUILD_BUG_ON((sizeof(struct tcmu_cmd_entry) % TCMU_OP_ALIGN_SIZE) != 0);
 
@@ -1710,7 +1755,10 @@ static int __init tcmu_module_init(void)
for (i = 0; passthrough_attrib_attrs[i] != NULL; i++) {
len += sizeof(struct configfs_attribute *);
}
-   len += sizeof(struct configfs_attribute *) * 2;
+   for (i = 0; tcmu_attrib_attrs[i] != NULL; i++) {
+   len += sizeof(struct configfs_attribute *);
+   }
+   len += sizeof(struct configfs_attribute *);
 
tcmu_attrs = kzalloc(len, GFP_KERNEL);
if (!tcmu_attrs) {
@@ -1721,9 +1769,10 @@ static int __init tcmu_module_init(void)
for (i = 0; passthrough_attrib_attrs[i] != NULL; i++) {
tcmu_attrs[i] = passthrough_attrib_attrs[i];
}
-   tcmu_attrs[i] = _attr_cmd_time_out;
-   i++;
-   tcmu_attrs[i] = _attr_emulate_write_cache;
+   for (k = 0; tcmu_attrib_attrs[k] != NULL; k++) {
+   tcmu_attrs[i] = tcmu_attrib_attrs[k];
+   i++;
+   }
tcmu_ops.tb_dev_attrib_attrs = tcmu_attrs;
 
ret = transport_backend_register(_ops);
-- 
2.5.4 (Apple Git-61)



[PATCH v1 0/4] TCMU Enable Reconfiguration Patches

2017-05-26 Thread Bryant G. Ly
This patch consists of adding a netlink to allow for reconfiguration
of a device in tcmu.

It also changes and adds some attributes that are reconfigurable:
write_cache, device size, and device path.

Bryant G. Ly (4):
  tcmu: Support emulate_write_cache
  tcmu: Add netlink for device reconfiguration
  tcmu: Make dev_size configurable via userspace
  tcmu: Make dev_config configurable

 drivers/target/target_core_user.c | 137 +-
 include/uapi/linux/target_core_user.h |   1 +
 2 files changed, 135 insertions(+), 3 deletions(-)

-- 
2.5.4 (Apple Git-61)



[PATCH v1 2/4] tcmu: Add netlink for device reconfiguration

2017-05-26 Thread Bryant G. Ly
This gives tcmu the ability to handle events that can cause
reconfiguration, such as resize, path changes, write_cache, etc...

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-By: Mike Christie <mchri...@redhat.com>
---
 drivers/target/target_core_user.c | 12 
 include/uapi/linux/target_core_user.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index 0c797cc..ae91822 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1562,6 +1562,7 @@ static ssize_t tcmu_emulate_write_cache_store(struct 
config_item *item,
 {
struct se_dev_attrib *da = container_of(to_config_group(item),
struct se_dev_attrib, da_group);
+   struct tcmu_dev *udev = TCMU_DEV(da->da_dev);
int val;
int ret;
 
@@ -1570,6 +1571,17 @@ static ssize_t tcmu_emulate_write_cache_store(struct 
config_item *item,
return ret;
 
da->emulate_write_cache = val;
+
+   /* Check if device has been configured before */
+   if (tcmu_dev_configured(udev)) {
+   ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE,
+udev->uio_info.name,
+udev->uio_info.uio_dev->minor);
+   if (ret) {
+   pr_err("Unable to reconfigure device\n");
+   return ret;
+   }
+   }
return count;
 }
 CONFIGFS_ATTR(tcmu_, emulate_write_cache);
diff --git a/include/uapi/linux/target_core_user.h 
b/include/uapi/linux/target_core_user.h
index af17b41..403a61f 100644
--- a/include/uapi/linux/target_core_user.h
+++ b/include/uapi/linux/target_core_user.h
@@ -130,6 +130,7 @@ enum tcmu_genl_cmd {
TCMU_CMD_UNSPEC,
TCMU_CMD_ADDED_DEVICE,
TCMU_CMD_REMOVED_DEVICE,
+   TCMU_CMD_RECONFIG_DEVICE,
__TCMU_CMD_MAX,
 };
 #define TCMU_CMD_MAX (__TCMU_CMD_MAX - 1)
-- 
2.5.4 (Apple Git-61)



[PATCH] ibmvscsis: Enable Logical Partition Migration Support

2017-05-16 Thread Bryant G. Ly
From: Michael Cyr <mike...@us.ibm.com>

Changes to support a new mechanism from phyp to better synchronize the
logical partition migration (LPM) of the client partition.
This includes a new VIOCTL to register that we support this new
functionality, and 2 new Transport Event types, and finally another
new VIOCTL to let phyp know once we're ready for the Suspend.

Signed-off-by: Michael Cyr <mike...@us.ibm.com>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 148 ---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h |  25 +-
 drivers/scsi/ibmvscsi_tgt/libsrp.h   |   5 +-
 3 files changed, 162 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 7a45ac0..361773f 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -154,6 +154,9 @@ static long ibmvscsis_unregister_command_q(struct scsi_info 
*vscsi)
qrc = h_free_crq(vscsi->dds.unit_id);
switch (qrc) {
case H_SUCCESS:
+   spin_lock_bh(>intr_lock);
+   vscsi->flags &= ~PREP_FOR_SUSPEND_FLAGS;
+   spin_unlock_bh(>intr_lock);
break;
 
case H_HARDWARE:
@@ -421,6 +424,9 @@ static void ibmvscsis_disconnect(struct work_struct *work)
new_state = vscsi->new_state;
vscsi->new_state = 0;
 
+   vscsi->flags |= DISCONNECT_SCHEDULED;
+   vscsi->flags &= ~SCHEDULE_DISCONNECT;
+
pr_debug("disconnect: flags 0x%x, state 0x%hx\n", vscsi->flags,
 vscsi->state);
 
@@ -801,6 +807,13 @@ static long ibmvscsis_establish_new_q(struct scsi_info 
*vscsi)
long rc = ADAPT_SUCCESS;
uint format;
 
+   rc = h_vioctl(vscsi->dds.unit_id, H_ENABLE_PREPARE_FOR_SUSPEND, 3,
+ 0, 0, 0, 0);
+   if (rc == H_SUCCESS)
+   vscsi->flags |= PREP_FOR_SUSPEND_ENABLED;
+   else if (rc != H_NOT_FOUND)
+   pr_err("Error from Enable Prepare for Suspend: %ld\n", rc);
+
vscsi->flags &= PRESERVE_FLAG_FIELDS;
vscsi->rsp_q_timer.timer_pops = 0;
vscsi->debit = 0;
@@ -950,6 +963,63 @@ static void ibmvscsis_free_cmd_resources(struct scsi_info 
*vscsi,
 }
 
 /**
+ * ibmvscsis_ready_for_suspend() - Helper function to call VIOCTL
+ * @vscsi: Pointer to our adapter structure
+ * @idle:  Indicates whether we were called from adapter_idle.  This
+ * is important to know if we need to do a disconnect, since if
+ * we're called from adapter_idle, we're still processing the
+ * current disconnect, so we can't just call post_disconnect.
+ *
+ * This function is called when the adapter is idle when phyp has sent
+ * us a Prepare for Suspend Transport Event.
+ *
+ * EXECUTION ENVIRONMENT:
+ * Process or interrupt environment called with interrupt lock held
+ */
+static long ibmvscsis_ready_for_suspend(struct scsi_info *vscsi, bool idle)
+{
+   long rc = 0;
+   struct viosrp_crq *crq;
+
+   /* See if there is a Resume event in the queue */
+   crq = vscsi->cmd_q.base_addr + vscsi->cmd_q.index;
+
+   pr_debug("ready_suspend: flags 0x%x, state 0x%hx crq_valid:%x\n",
+vscsi->flags, vscsi->state, (int)crq->valid);
+
+   if (!(vscsi->flags & PREP_FOR_SUSPEND_ABORTED) && !(crq->valid)) {
+   rc = h_vioctl(vscsi->dds.unit_id, H_READY_FOR_SUSPEND, 0, 0, 0,
+ 0, 0);
+   if (rc) {
+   pr_err("Ready for Suspend Vioctl failed: %ld\n", rc);
+   rc = 0;
+   }
+   } else if (((vscsi->flags & PREP_FOR_SUSPEND_OVERWRITE) &&
+   (vscsi->flags & PREP_FOR_SUSPEND_ABORTED)) ||
+  ((crq->valid) && ((crq->valid != VALID_TRANS_EVENT) ||
+(crq->format != RESUME_FROM_SUSP {
+   if (idle) {
+   vscsi->state = ERR_DISCONNECT_RECONNECT;
+   ibmvscsis_reset_queue(vscsi);
+   rc = -1;
+   } else if (vscsi->state == CONNECTED) {
+   ibmvscsis_post_disconnect(vscsi,
+ ERR_DISCONNECT_RECONNECT, 0);
+   }
+
+   vscsi->flags &= ~PREP_FOR_SUSPEND_OVERWRITE;
+
+   if ((crq->valid) && ((crq->valid != VALID_TRANS_EVENT) ||
+(crq->format != RESUME_FROM_SUSP)))
+   pr_err("Invalid element in CRQ after Prepare for 
Suspend");
+

Re: [PATCH v1] ibmvscsis: Fix cleaning up pointers

2017-05-11 Thread Bryant G. Ly

On 5/9/17 10:45 PM, Nicholas A. Bellinger wrote:


On Tue, 2017-05-09 at 11:50 -0500, Bryant G. Ly wrote:

This patch is dependent on:
'commit 25e78531268e ("ibmvscsis: Do not send aborted task response")'
This patch cleans up some pointers after usage.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-by: Michael Cyr <mike...@linux.vnet.ibm.com>
Cc: <sta...@vger.kernel.org> # v4.8+
---
  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 3 +++
  1 file changed, 3 insertions(+)

Applied, with a more verbose commit message.

Thanks for splitting this out into an incremental patch.


No problem, did you end up forgetting it once you reverted Bart's VERIFY/WRITE 
VERIFY patch?

It seems to be missing from your for-next now.

-Bryant



[PATCH v2] ibmvscsis: Fix the incorrect req_lim_delta

2017-05-10 Thread Bryant G. Ly
The current code is not correctly calculating the req_lim_delta.

We want to make sure vscsi->credit is always incremented when
we do not send a response for the scsi op. Thus for the case where
there is a successfully aborted task we need to make sure the
vscsi->credit is incremented.

v2 - Moves the original location of the vscsi->credit increment
to a better spot. Since if we increment credit, the next command
we send back will have increased req_lim_delta. But we probably
shouldn't be doing that until the aborted cmd is actually released.
Otherwise the client will think that it can send a new command, and
we could find ourselves short of command elements. Not likely, but could
happen.

This patch depends on both:
commit 25e78531268e ("ibmvscsis: Do not send aborted task response")
commit 38b2788edbd6 ("ibmvscsis: Clear left-over abort_cmd pointers")

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-by: Michael Cyr <mike...@linux.vnet.ibm.com>
Cc: <sta...@vger.kernel.org> # v4.8+
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index ee64241..abf6026 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1791,6 +1791,25 @@ static void ibmvscsis_send_messages(struct scsi_info 
*vscsi)
list_del(>list);
ibmvscsis_free_cmd_resources(vscsi,
 cmd);
+   /*
+* With a successfully aborted op
+* through LIO we want to increment the
+* the vscsi credit so that when we dont
+* send a rsp to the original scsi abort
+* op (h_send_crq), but the tm rsp to
+* the abort is sent, the credit is
+* correctly sent with the abort tm rsp.
+* We would need 1 for the abort tm rsp
+* and 1 credit for the aborted scsi op.
+* Thus we need to increment here.
+* Also we want to increment the credit
+* here because we want to make sure
+* cmd is actually released first
+* otherwise the client will think it
+* it can send a new cmd, and we could
+* find ourselves short of cmd elements.
+*/
+   vscsi->credit += 1;
} else {
iue = cmd->iue;
 
@@ -2965,10 +2984,7 @@ static long srp_build_response(struct scsi_info *vscsi,
 
rsp->opcode = SRP_RSP;
 
-   if (vscsi->credit > 0 && vscsi->state == SRP_PROCESSING)
-   rsp->req_lim_delta = cpu_to_be32(vscsi->credit);
-   else
-   rsp->req_lim_delta = cpu_to_be32(1 + vscsi->credit);
+   rsp->req_lim_delta = cpu_to_be32(1 + vscsi->credit);
rsp->tag = cmd->rsp.tag;
rsp->flags = 0;
 
-- 
2.5.4 (Apple Git-61)



[PATCH v1] ibmvscsis: Fix the incorrect req_lim_delta

2017-05-10 Thread Bryant G. Ly
The current code is not correctly calculating the req_lim_delta.

We want to make sure vscsi->credit is always incremented when
we do not send a response for the scsi op. Thus for the case where
there is a successfully aborted task we need to make sure the
vscsi->credit is incremented.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-by: Michael Cyr <mike...@linux.vnet.ibm.com>
Cc: <sta...@vger.kernel.org> # v4.8+
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index ee64241..2d97f02 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -2965,10 +2965,7 @@ static long srp_build_response(struct scsi_info *vscsi,
 
rsp->opcode = SRP_RSP;
 
-   if (vscsi->credit > 0 && vscsi->state == SRP_PROCESSING)
-   rsp->req_lim_delta = cpu_to_be32(vscsi->credit);
-   else
-   rsp->req_lim_delta = cpu_to_be32(1 + vscsi->credit);
+   rsp->req_lim_delta = cpu_to_be32(1 + vscsi->credit);
rsp->tag = cmd->rsp.tag;
rsp->flags = 0;
 
@@ -3739,6 +3736,22 @@ static void ibmvscsis_queue_tm_rsp(struct se_cmd *se_cmd)
 
 static void ibmvscsis_aborted_task(struct se_cmd *se_cmd)
 {
+   struct ibmvscsis_cmd *cmd = container_of(se_cmd, struct ibmvscsis_cmd,
+se_cmd);
+   struct scsi_info *vscsi = cmd->adapter;
+
+   /*
+* With a successfully aborted op through LIO we want to increment the
+* the vscsi credit so that when we dont send a rsp to the original
+* scsi abort op but the tm rsp to the abort is sent, the credit is
+* correctly sent with the abort tm rsp. We would need 1 for the abort
+* tm rsp and 1 credit for the aborted scsi op. Thus we need to
+* increment here.
+*/
+   spin_lock_bh(>intr_lock);
+   vscsi->credit += 1;
+   spin_unlock_bh(>intr_lock);
+
pr_debug("ibmvscsis_aborted_task %p task_tag: %llu\n",
 se_cmd, se_cmd->tag);
 }
-- 
2.5.4 (Apple Git-61)



[PATCH v1] ibmvscsis: Fix cleaning up pointers

2017-05-09 Thread Bryant G. Ly
This patch is dependent on:
'commit 25e78531268e ("ibmvscsis: Do not send aborted task response")'
This patch cleans up some pointers after usage.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-by: Michael Cyr <mike...@linux.vnet.ibm.com>
Cc: <sta...@vger.kernel.org> # v4.8+
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index d390325..ee64241 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1170,6 +1170,8 @@ static struct ibmvscsis_cmd 
*ibmvscsis_get_free_cmd(struct scsi_info *vscsi)
cmd = list_first_entry_or_null(>free_cmd,
   struct ibmvscsis_cmd, list);
if (cmd) {
+   if (cmd->abort_cmd)
+   cmd->abort_cmd = NULL;
cmd->flags &= ~(DELAY_SEND);
list_del(>list);
cmd->iue = iue;
@@ -1774,6 +1776,7 @@ static void ibmvscsis_send_messages(struct scsi_info 
*vscsi)
if (cmd->abort_cmd) {
retry = true;
cmd->abort_cmd->flags &= ~(DELAY_SEND);
+   cmd->abort_cmd = NULL;
}
 
/*
-- 
2.5.4 (Apple Git-61)



[PATCH v4] ibmvscsis: Do not send aborted task response

2017-05-08 Thread Bryant G. Ly
The driver is sending a response to the actual scsi op that was
aborted by an abort task TM, while LIO is sending a response to
the abort task TM.

ibmvscsis_tgt does not send the response to the client until
release_cmd time. The reason for this was because if we did it
at queue_status time, then the client would be free to reuse the
tag for that command, but we're still using the tag until the
command is released at release_cmd time, so we chose to delay
sending the response until then. That then caused this issue, because
release_cmd is always called, even if queue_status is not.

SCSI spec says that the initiator that sends the abort task
TM NEVER gets a response to the aborted op and with the current
code it will send a response. Thus this fix will remove that response
if the CMD_T_ABORTED && !CMD_T_TAS.

Another case with a small timing window is the case where if LIO sends a
TMR_DOES_NOT_EXIST, and the release_cmd callback is called for the TMR Abort
cmd before the release_cmd for the (attemped) aborted cmd, then we need to
ensure that we send the response for the (attempted) abort cmd to the client
before we send the response for the TMR Abort cmd.

Forgot to reset the cmd->abort_cmd pointer after finding it in send_messages.

Cc: <sta...@vger.kernel.org> # v4.8+
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Signed-off-by: Michael Cyr <mike...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 117 ---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h |   2 +
 2 files changed, 94 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 0f80779..ee64241 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1170,6 +1170,9 @@ static struct ibmvscsis_cmd 
*ibmvscsis_get_free_cmd(struct scsi_info *vscsi)
cmd = list_first_entry_or_null(>free_cmd,
   struct ibmvscsis_cmd, list);
if (cmd) {
+   if (cmd->abort_cmd)
+   cmd->abort_cmd = NULL;
+   cmd->flags &= ~(DELAY_SEND);
list_del(>list);
cmd->iue = iue;
cmd->type = UNSET_TYPE;
@@ -1749,45 +1752,80 @@ static void srp_snd_msg_failed(struct scsi_info *vscsi, 
long rc)
 static void ibmvscsis_send_messages(struct scsi_info *vscsi)
 {
u64 msg_hi = 0;
-   /* note do not attmempt to access the IU_data_ptr with this pointer
+   /* note do not attempt to access the IU_data_ptr with this pointer
 * it is not valid
 */
struct viosrp_crq *crq = (struct viosrp_crq *)_hi;
struct ibmvscsis_cmd *cmd, *nxt;
struct iu_entry *iue;
long rc = ADAPT_SUCCESS;
+   bool retry = false;
 
if (!(vscsi->flags & RESPONSE_Q_DOWN)) {
-   list_for_each_entry_safe(cmd, nxt, >waiting_rsp, list) {
-   iue = cmd->iue;
+   do {
+   retry = false;
+   list_for_each_entry_safe(cmd, nxt, >waiting_rsp,
+list) {
+   /*
+* Check to make sure abort cmd gets processed
+* prior to the abort tmr cmd
+*/
+   if (cmd->flags & DELAY_SEND)
+   continue;
 
-   crq->valid = VALID_CMD_RESP_EL;
-   crq->format = cmd->rsp.format;
+   if (cmd->abort_cmd) {
+   retry = true;
+   cmd->abort_cmd->flags &= ~(DELAY_SEND);
+   cmd->abort_cmd = NULL;
+   }
 
-   if (cmd->flags & CMD_FAST_FAIL)
-   crq->status = VIOSRP_ADAPTER_FAIL;
+   /*
+* If CMD_T_ABORTED w/o CMD_T_TAS scenarios and
+* the case where LIO issued a
+* ABORT_TASK: Sending TMR_TASK_DOES_NOT_EXIST
+* case then we dont send a response, since it
+* was already done.
+*/
+   if (cmd->se_cmd.transport_state & CMD_T_ABORTED 
&&
+   !(cmd->se_cmd.transport_state & CMD_T_TAS)) 
{
+   list_del(>list);
+   ibmvscsis_fr

[PATCH v3] ibmvscsis: Do not send aborted task response

2017-05-05 Thread Bryant G. Ly
The driver is sending a response to the actual scsi op that was
aborted by an abort task TM, while LIO is sending a response to
the abort task TM.

ibmvscsis_tgt does not send the response to the client until
release_cmd time. The reason for this was because if we did it
at queue_status time, then the client would be free to reuse the
tag for that command, but we're still using the tag until the
command is released at release_cmd time, so we chose to delay
sending the response until then. That then caused this issue, because
release_cmd is always called, even if queue_status is not.

SCSI spec says that the initiator that sends the abort task
TM NEVER gets a response to the aborted op and with the current
code it will send a response. Thus this fix will remove that response
if the CMD_T_ABORTED && !CMD_T_TAS.

Another case with a small timing window is the case where if LIO sends a
TMR_DOES_NOT_EXIST, and the release_cmd callback is called for the TMR Abort
cmd before the release_cmd for the (attemped) aborted cmd, then we need to
ensure that we send the response for the (attempted) abort cmd to the client
before we send the response for the TMR Abort cmd.

Cc: <sta...@vger.kernel.org> # v4.8+
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Signed-off-by: Michael Cyr <mike...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 114 ---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h |   2 +
 2 files changed, 91 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 0f80779..d390325 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1170,6 +1170,7 @@ static struct ibmvscsis_cmd 
*ibmvscsis_get_free_cmd(struct scsi_info *vscsi)
cmd = list_first_entry_or_null(>free_cmd,
   struct ibmvscsis_cmd, list);
if (cmd) {
+   cmd->flags &= ~(DELAY_SEND);
list_del(>list);
cmd->iue = iue;
cmd->type = UNSET_TYPE;
@@ -1749,45 +1750,79 @@ static void srp_snd_msg_failed(struct scsi_info *vscsi, 
long rc)
 static void ibmvscsis_send_messages(struct scsi_info *vscsi)
 {
u64 msg_hi = 0;
-   /* note do not attmempt to access the IU_data_ptr with this pointer
+   /* note do not attempt to access the IU_data_ptr with this pointer
 * it is not valid
 */
struct viosrp_crq *crq = (struct viosrp_crq *)_hi;
struct ibmvscsis_cmd *cmd, *nxt;
struct iu_entry *iue;
long rc = ADAPT_SUCCESS;
+   bool retry = false;
 
if (!(vscsi->flags & RESPONSE_Q_DOWN)) {
-   list_for_each_entry_safe(cmd, nxt, >waiting_rsp, list) {
-   iue = cmd->iue;
+   do {
+   retry = false;
+   list_for_each_entry_safe(cmd, nxt, >waiting_rsp,
+list) {
+   /*
+* Check to make sure abort cmd gets processed
+* prior to the abort tmr cmd
+*/
+   if (cmd->flags & DELAY_SEND)
+   continue;
 
-   crq->valid = VALID_CMD_RESP_EL;
-   crq->format = cmd->rsp.format;
+   if (cmd->abort_cmd) {
+   retry = true;
+   cmd->abort_cmd->flags &= ~(DELAY_SEND);
+   }
 
-   if (cmd->flags & CMD_FAST_FAIL)
-   crq->status = VIOSRP_ADAPTER_FAIL;
+   /*
+* If CMD_T_ABORTED w/o CMD_T_TAS scenarios and
+* the case where LIO issued a
+* ABORT_TASK: Sending TMR_TASK_DOES_NOT_EXIST
+* case then we dont send a response, since it
+* was already done.
+*/
+   if (cmd->se_cmd.transport_state & CMD_T_ABORTED 
&&
+   !(cmd->se_cmd.transport_state & CMD_T_TAS)) 
{
+   list_del(>list);
+   ibmvscsis_free_cmd_resources(vscsi,
+cmd);
+   } else {
+   iue = cmd->iue;
 
-   crq->IU_length = cpu_to_be16(cmd->rsp.len);
+  

Re: [PATCH v2] ibmvscsis: Do not send aborted task response

2017-05-04 Thread Bryant G. Ly



On 5/3/17 1:55 PM, Bryant G. Ly wrote:

On 5/3/17 11:38 AM, Bryant G. Ly wrote:


Hi Nick,

On 5/2/17 10:43 PM, Nicholas A. Bellinger wrote:


On Tue, 2017-05-02 at 13:54 -0500, Bryant G. Ly wrote:

The driver is sending a response to the actual scsi op that was
aborted by an abort task TM, while LIO is sending a response to
the abort task TM.

ibmvscsis_tgt does not send the response to the client until
release_cmd time. The reason for this was because if we did it
at queue_status time, then the client would be free to reuse the
tag for that command, but we're still using the tag until the
command is released at release_cmd time, so we chose to delay
sending the response until then. That then caused this issue, because
release_cmd is always called, even if queue_status is not.

SCSI spec says that the initiator that sends the abort task
TM NEVER gets a response to the aborted op and with the current
code it will send a response. Thus this fix will remove that response
if the TAS bit is set.

Cc: <sta...@vger.kernel.org> # v4.8+
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 66 
++--

  1 file changed, 45 insertions(+), 21 deletions(-)

Applied, with a small update to the last sentence of the commit log wrt
to 'if ABORTED && !TAS bit is set'.

Thanks Bryant + Tyrel.

So I have been running tests on this patch extensively and it seems 
like after running 2 hrs of consecutive aborts it fails again with 
the same problem of driver sending a response to the actual scsi op.


It looks like when LIO processes the abort and if 
(!__target_check_io_state(se_cmd, se_sess, 0)) then rsp gets set as 
TMR_TASK_DOES_NOT_EXIST.


With that response the CMD_T_ABORTED state is not set, so then the 
ibmvscsis driver still sends that duplicate response.


I think the best solution would be in driver when queue_tm_rsp gets 
called and when the driver builds the response to check for the 
TMR_TASK_DOES_NOT_EXIST and set our own flag.

Then we would also check for that flag, so it would be:

if ((cmd->se_cmd.transport_state & CMD_T_ABORTED && 
!(cmd->se_cmd.transport_state & CMD_T_TAS)) || cmd->flags & 
CMD_ABORTED) {


This will ensure in the CMD_T_ABORTED w/o CMD_T_TAS scenarios are 
handled and the ABORT_TASK: Sending TMR_TASK_DOES_NOT_EXIST case.


-Bryant



Hi Nick,

You can ignore the email about the patch being broken. The script that 
was used for testing isn't right.


So the patch is working as intended now, where on an abort the client 
does not receive an extra response to the actual scsi op.


Thanks,

Bryant Ly


Sorry Nick, but can you hold off on this patch. We are still getting extra 
responses but in a small timing window.

The majority of the aborts work but if we get an abort between LIO calling 
queue_state and LIO calling release_cmd, we get an extra response.


Working on a solution, and will still have to do the tests where we basically 
send aborts over and over on the client btwn random timing widows.

-Bryant



Re: [PATCH v2] ibmvscsis: Do not send aborted task response

2017-05-03 Thread Bryant G. Ly

On 5/3/17 11:38 AM, Bryant G. Ly wrote:


Hi Nick,

On 5/2/17 10:43 PM, Nicholas A. Bellinger wrote:


On Tue, 2017-05-02 at 13:54 -0500, Bryant G. Ly wrote:

The driver is sending a response to the actual scsi op that was
aborted by an abort task TM, while LIO is sending a response to
the abort task TM.

ibmvscsis_tgt does not send the response to the client until
release_cmd time. The reason for this was because if we did it
at queue_status time, then the client would be free to reuse the
tag for that command, but we're still using the tag until the
command is released at release_cmd time, so we chose to delay
sending the response until then. That then caused this issue, because
release_cmd is always called, even if queue_status is not.

SCSI spec says that the initiator that sends the abort task
TM NEVER gets a response to the aborted op and with the current
code it will send a response. Thus this fix will remove that response
if the TAS bit is set.

Cc: <sta...@vger.kernel.org> # v4.8+
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 66 
++--

  1 file changed, 45 insertions(+), 21 deletions(-)

Applied, with a small update to the last sentence of the commit log wrt
to 'if ABORTED && !TAS bit is set'.

Thanks Bryant + Tyrel.

So I have been running tests on this patch extensively and it seems 
like after running 2 hrs of consecutive aborts it fails again with the 
same problem of driver sending a response to the actual scsi op.


It looks like when LIO processes the abort and if 
(!__target_check_io_state(se_cmd, se_sess, 0)) then rsp gets set as 
TMR_TASK_DOES_NOT_EXIST.


With that response the CMD_T_ABORTED state is not set, so then the 
ibmvscsis driver still sends that duplicate response.


I think the best solution would be in driver when queue_tm_rsp gets 
called and when the driver builds the response to check for the 
TMR_TASK_DOES_NOT_EXIST and set our own flag.

Then we would also check for that flag, so it would be:

if ((cmd->se_cmd.transport_state & CMD_T_ABORTED && 
!(cmd->se_cmd.transport_state & CMD_T_TAS)) || cmd->flags & 
CMD_ABORTED) {


This will ensure in the CMD_T_ABORTED w/o CMD_T_TAS scenarios are 
handled and the ABORT_TASK: Sending TMR_TASK_DOES_NOT_EXIST case.


-Bryant



Hi Nick,

You can ignore the email about the patch being broken. The script that was used 
for testing isn't right.

So the patch is working as intended now, where on an abort the client does not 
receive an extra response to the actual scsi op.

Thanks,

Bryant Ly



Re: [PATCH v2] ibmvscsis: Do not send aborted task response

2017-05-03 Thread Bryant G. Ly

Hi Nick,

On 5/2/17 10:43 PM, Nicholas A. Bellinger wrote:


On Tue, 2017-05-02 at 13:54 -0500, Bryant G. Ly wrote:

The driver is sending a response to the actual scsi op that was
aborted by an abort task TM, while LIO is sending a response to
the abort task TM.

ibmvscsis_tgt does not send the response to the client until
release_cmd time. The reason for this was because if we did it
at queue_status time, then the client would be free to reuse the
tag for that command, but we're still using the tag until the
command is released at release_cmd time, so we chose to delay
sending the response until then. That then caused this issue, because
release_cmd is always called, even if queue_status is not.

SCSI spec says that the initiator that sends the abort task
TM NEVER gets a response to the aborted op and with the current
code it will send a response. Thus this fix will remove that response
if the TAS bit is set.

Cc: <sta...@vger.kernel.org> # v4.8+
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 66 ++--
  1 file changed, 45 insertions(+), 21 deletions(-)

Applied, with a small update to the last sentence of the commit log wrt
to 'if ABORTED && !TAS bit is set'.

Thanks Bryant + Tyrel.


So I have been running tests on this patch extensively and it seems like after 
running 2 hrs of consecutive aborts it fails again with the same problem of 
driver sending a response to the actual scsi op.

It looks like when LIO processes the abort and if 
(!__target_check_io_state(se_cmd, se_sess, 0)) then rsp gets set as 
TMR_TASK_DOES_NOT_EXIST.

With that response the CMD_T_ABORTED state is not set, so then the ibmvscsis 
driver still sends that duplicate response.

I think the best solution would be in driver when queue_tm_rsp gets called and 
when the driver builds the response to check for the TMR_TASK_DOES_NOT_EXIST 
and set our own flag.
Then we would also check for that flag, so it would be:

if ((cmd->se_cmd.transport_state & CMD_T_ABORTED && !(cmd->se_cmd.transport_state & 
CMD_T_TAS)) || cmd->flags & CMD_ABORTED) {

This will ensure in the CMD_T_ABORTED w/o CMD_T_TAS scenarios are handled and 
the ABORT_TASK: Sending TMR_TASK_DOES_NOT_EXIST case.

-Bryant






[PATCH v2] ibmvscsis: Do not send aborted task response

2017-05-02 Thread Bryant G. Ly
The driver is sending a response to the actual scsi op that was
aborted by an abort task TM, while LIO is sending a response to
the abort task TM.

ibmvscsis_tgt does not send the response to the client until
release_cmd time. The reason for this was because if we did it
at queue_status time, then the client would be free to reuse the
tag for that command, but we're still using the tag until the
command is released at release_cmd time, so we chose to delay
sending the response until then. That then caused this issue, because
release_cmd is always called, even if queue_status is not.

SCSI spec says that the initiator that sends the abort task
TM NEVER gets a response to the aborted op and with the current
code it will send a response. Thus this fix will remove that response
if the TAS bit is set.

Cc: <sta...@vger.kernel.org> # v4.8+
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Reviewed-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 66 ++--
 1 file changed, 45 insertions(+), 21 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 4bb5635..d6e62ce 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1758,33 +1758,46 @@ static void ibmvscsis_send_messages(struct scsi_info 
*vscsi)
 
if (!(vscsi->flags & RESPONSE_Q_DOWN)) {
list_for_each_entry_safe(cmd, nxt, >waiting_rsp, list) {
-   iue = cmd->iue;
-
-   crq->valid = VALID_CMD_RESP_EL;
-   crq->format = cmd->rsp.format;
+   /*
+* If Task Abort Status Bit is set, then dont send a
+* response.
+*/
+   if (cmd->se_cmd.transport_state & CMD_T_ABORTED &&
+   !(cmd->se_cmd.transport_state & CMD_T_TAS)) {
+   list_del(>list);
+   ibmvscsis_free_cmd_resources(vscsi, cmd);
+   } else {
+   iue = cmd->iue;
 
-   if (cmd->flags & CMD_FAST_FAIL)
-   crq->status = VIOSRP_ADAPTER_FAIL;
+   crq->valid = VALID_CMD_RESP_EL;
+   crq->format = cmd->rsp.format;
 
-   crq->IU_length = cpu_to_be16(cmd->rsp.len);
+   if (cmd->flags & CMD_FAST_FAIL)
+   crq->status = VIOSRP_ADAPTER_FAIL;
 
-   rc = h_send_crq(vscsi->dma_dev->unit_address,
-   be64_to_cpu(msg_hi),
-   be64_to_cpu(cmd->rsp.tag));
+   crq->IU_length = cpu_to_be16(cmd->rsp.len);
 
-   pr_debug("send_messages: cmd %p, tag 0x%llx, rc %ld\n",
-cmd, be64_to_cpu(cmd->rsp.tag), rc);
+   rc = h_send_crq(vscsi->dma_dev->unit_address,
+   be64_to_cpu(msg_hi),
+   be64_to_cpu(cmd->rsp.tag));
 
-   /* if all ok free up the command element resources */
-   if (rc == H_SUCCESS) {
-   /* some movement has occurred */
-   vscsi->rsp_q_timer.timer_pops = 0;
-   list_del(>list);
+   pr_debug("send_messages: cmd %p, tag 0x%llx, rc 
%ld\n",
+cmd, be64_to_cpu(cmd->rsp.tag), rc);
 
-   ibmvscsis_free_cmd_resources(vscsi, cmd);
-   } else {
-   srp_snd_msg_failed(vscsi, rc);
-   break;
+   /* if all ok free up the command element
+* resources
+*/
+   if (rc == H_SUCCESS) {
+   /* some movement has occurred */
+   vscsi->rsp_q_timer.timer_pops = 0;
+   list_del(>list);
+
+   ibmvscsis_free_cmd_resources(vscsi,
+cmd);
+   } else {
+   srp_snd_msg_failed(vscsi, rc);
+   break;
+   }
}

[PATCH v3] target/user: PGR Support

2017-04-21 Thread Bryant G. Ly
This adds initial PGR support for just TCMU, since tcmu doesn't
have the necessary IT_NEXUS info to process PGR in userspace,
so have those commands be processed in kernel.

HA support is not available yet, we will work on it if this patch
is acceptable.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_configfs.c | 10 -
 drivers/target/target_core_device.c   | 38 +++
 drivers/target/target_core_pr.c   |  2 +-
 drivers/target/target_core_pscsi.c|  3 ++-
 include/target/target_core_backend.h  |  1 +
 5 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/drivers/target/target_core_configfs.c 
b/drivers/target/target_core_configfs.c
index 38b5025..edfb098 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -1366,7 +1366,7 @@ static ssize_t target_pr_res_holder_show(struct 
config_item *item, char *page)
struct se_device *dev = pr_to_dev(item);
int ret;
 
-   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
+   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)
return sprintf(page, "Passthrough\n");
 
spin_lock(>dev_reservation_lock);
@@ -1506,7 +1506,7 @@ static ssize_t target_pr_res_type_show(struct config_item 
*item, char *page)
 {
struct se_device *dev = pr_to_dev(item);
 
-   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
+   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)
return sprintf(page, "SPC_PASSTHROUGH\n");
else if (dev->dev_reservation_flags & DRF_SPC2_RESERVATIONS)
return sprintf(page, "SPC2_RESERVATIONS\n");
@@ -1519,7 +1519,7 @@ static ssize_t target_pr_res_aptpl_active_show(struct 
config_item *item,
 {
struct se_device *dev = pr_to_dev(item);
 
-   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
+   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)
return 0;
 
return sprintf(page, "APTPL Bit Status: %s\n",
@@ -1531,7 +1531,7 @@ static ssize_t target_pr_res_aptpl_metadata_show(struct 
config_item *item,
 {
struct se_device *dev = pr_to_dev(item);
 
-   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
+   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)
return 0;
 
return sprintf(page, "Ready to process PR APTPL metadata..\n");
@@ -1577,7 +1577,7 @@ static ssize_t target_pr_res_aptpl_metadata_store(struct 
config_item *item,
u16 tpgt = 0;
u8 type = 0;
 
-   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
+   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)
return count;
if (dev->dev_reservation_flags & DRF_SPC2_RESERVATIONS)
return count;
diff --git a/drivers/target/target_core_device.c 
b/drivers/target/target_core_device.c
index c754ae3..c484567 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -1045,6 +1045,8 @@ passthrough_parse_cdb(struct se_cmd *cmd,
sense_reason_t (*exec_cmd)(struct se_cmd *cmd))
 {
unsigned char *cdb = cmd->t_task_cdb;
+   struct se_device *dev = cmd->se_dev;
+   unsigned int size;
 
/*
 * Clear a lun set in the cdb if the initiator talking to use spoke
@@ -1076,6 +1078,42 @@ passthrough_parse_cdb(struct se_cmd *cmd,
return TCM_NO_SENSE;
}
 
+   /*
+* For PERSISTENT RESERVE IN/OUT, RELEASE, and RESERVE we need to
+* emulate the response, since tcmu does not have the information
+* required to process these commands.
+*/
+   if (!(dev->transport->transport_flags &
+ TRANSPORT_FLAG_PASSTHROUGH_PGR)) {
+   if (cdb[0] == PERSISTENT_RESERVE_IN) {
+   cmd->execute_cmd = target_scsi3_emulate_pr_in;
+   size = (cdb[7] << 8) + cdb[8];
+   return target_cmd_size_check(cmd, size);
+   }
+   if (cdb[0] == PERSISTENT_RESERVE_OUT) {
+   cmd->execute_cmd = target_scsi3_emulate_pr_out;
+   size = (cdb[7] << 8) + cdb[8];
+   return target_cmd_size_check(cmd, size);
+   }
+
+   if (cdb[0] == RELEASE || cdb[0] == RELEASE_10) {
+   cmd->execute_cmd = target_scsi2_reservation_release;
+   if (cdb[0] == RELEASE_10)
+   size = (cdb[7] << 8) | cdb[8];
+   else
+   

[PATCH v2] target/user: PGR Support

2017-04-21 Thread Bryant G. Ly
This adds initial PGR support for just TCMU, since tcmu doesn't
have the necessary IT_NEXUS info to process PGR in userspace,
so have those commands be processed in kernel.

HA support is not available yet, we will work on it if this patch
is acceptable.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_configfs.c | 10 -
 drivers/target/target_core_device.c   | 38 +++
 drivers/target/target_core_pr.c   |  2 +-
 drivers/target/target_core_pscsi.c|  3 ++-
 include/target/target_core_backend.h  |  1 +
 5 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/drivers/target/target_core_configfs.c 
b/drivers/target/target_core_configfs.c
index 38b5025..edfb098 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -1366,7 +1366,7 @@ static ssize_t target_pr_res_holder_show(struct 
config_item *item, char *page)
struct se_device *dev = pr_to_dev(item);
int ret;
 
-   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
+   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)
return sprintf(page, "Passthrough\n");
 
spin_lock(>dev_reservation_lock);
@@ -1506,7 +1506,7 @@ static ssize_t target_pr_res_type_show(struct config_item 
*item, char *page)
 {
struct se_device *dev = pr_to_dev(item);
 
-   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
+   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)
return sprintf(page, "SPC_PASSTHROUGH\n");
else if (dev->dev_reservation_flags & DRF_SPC2_RESERVATIONS)
return sprintf(page, "SPC2_RESERVATIONS\n");
@@ -1519,7 +1519,7 @@ static ssize_t target_pr_res_aptpl_active_show(struct 
config_item *item,
 {
struct se_device *dev = pr_to_dev(item);
 
-   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
+   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)
return 0;
 
return sprintf(page, "APTPL Bit Status: %s\n",
@@ -1531,7 +1531,7 @@ static ssize_t target_pr_res_aptpl_metadata_show(struct 
config_item *item,
 {
struct se_device *dev = pr_to_dev(item);
 
-   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
+   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)
return 0;
 
return sprintf(page, "Ready to process PR APTPL metadata..\n");
@@ -1577,7 +1577,7 @@ static ssize_t target_pr_res_aptpl_metadata_store(struct 
config_item *item,
u16 tpgt = 0;
u8 type = 0;
 
-   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
+   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)
return count;
if (dev->dev_reservation_flags & DRF_SPC2_RESERVATIONS)
return count;
diff --git a/drivers/target/target_core_device.c 
b/drivers/target/target_core_device.c
index c754ae3..80106cc 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -1045,6 +1045,8 @@ passthrough_parse_cdb(struct se_cmd *cmd,
sense_reason_t (*exec_cmd)(struct se_cmd *cmd))
 {
unsigned char *cdb = cmd->t_task_cdb;
+   struct se_device *dev = cmd->se_dev;
+   unsigned int size;
 
/*
 * Clear a lun set in the cdb if the initiator talking to use spoke
@@ -1076,6 +1078,42 @@ passthrough_parse_cdb(struct se_cmd *cmd,
return TCM_NO_SENSE;
}
 
+   /*
+* For PERSISTENT RESERVE IN/OUT, RELEASE, and RESERVE we need to
+* emulate the response, since tcmu does not have the information
+* required to process these commands.
+*/
+   if (!(dev->transport->transport_flags &
+ TRANSPORT_FLAG_PASSTHROUGH_PGR)) {
+   if (cdb[0] == PERSISTENT_RESERVE_IN) {
+   cmd->execute_cmd = target_scsi3_emulate_pr_in;
+   size = (cdb[7] << 8) + cdb[8];
+   return target_cmd_size_check(cmd, size);
+   }
+   if (cdb[0] == PERSISTENT_RESERVE_OUT) {
+   cmd->execute_cmd = target_scsi3_emulate_pr_out;
+   size = (cdb[7] << 8) + cdb[8];
+   return target_cmd_size_check(cmd, size);
+   }
+
+   if (cdb[0] == RELEASE || cdb[0] == RELEASE_10) {
+   cmd->execute_cmd = target_scsi2_reservation_release;
+   if (cdb[0] == RELEASE_10)
+   size = (cdb[7] << 8) | cdb[8];
+   else
+  

[PATCH] target: Add WRITE_VERIFY_16

2017-04-18 Thread Bryant G. Ly
This patch addresses clients who needs write_verify_16 for
large volume groups such as AIX.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_sbc.c | 2 ++
 include/scsi/scsi_proto.h| 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index ee35c90..2060f5a 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -850,6 +850,7 @@ static sense_reason_t sbc_parse_verify(struct se_cmd *cmd, 
int *sectors,
cmd->t_task_lba = transport_lba_32(cdb);
break;
case VERIFY_16:
+   case WRITE_VERIFY_16:
*sectors = transport_get_sectors_16(cdb);
cmd->t_task_lba = transport_lba_64(cdb);
break;
@@ -962,6 +963,7 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
cmd->execute_cmd = sbc_execute_rw;
break;
case WRITE_VERIFY:
+   case WRITE_VERIFY_16:
ret = sbc_parse_verify(cmd, , );
if (ret)
return ret;
diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h
index 6ba66e0..ce78ec8 100644
--- a/include/scsi/scsi_proto.h
+++ b/include/scsi/scsi_proto.h
@@ -112,6 +112,7 @@
 #define WRITE_16  0x8a
 #define READ_ATTRIBUTE0x8c
 #define WRITE_ATTRIBUTE  0x8d
+#define WRITE_VERIFY_16  0x8e
 #define VERIFY_160x8f
 #define SYNCHRONIZE_CACHE_16  0x91
 #define WRITE_SAME_160x93
-- 
2.5.4 (Apple Git-61)



[PATCH] ibmvscsis: Do not send aborted task response

2017-04-10 Thread Bryant G. Ly
The driver is sending a response to the aborted task response
along with LIO sending the tmr response.
ibmvscsis_tgt does not send the response to the client until
release_cmd time. The reason for this was because if we did it
at queue_status time, then the client would be free to reuse the
tag for that command, but we're still using the tag until the
command is released at release_cmd time, so we chose to delay
sending the response until then. That then caused this issue, because
release_cmd is always called, even if queue_status is not.

SCSI spec says that the initiator that sends the abort task
TM NEVER gets a response to the aborted op and with the current
code it will send a response. Thus this fix will remove that response
if the TAS bit is set.

Cc: <sta...@vger.kernel.org> # v4.8+
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 60 +---
 1 file changed, 40 insertions(+), 20 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 4bb5635..f75948a 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1758,33 +1758,42 @@ static void ibmvscsis_send_messages(struct scsi_info 
*vscsi)
 
if (!(vscsi->flags & RESPONSE_Q_DOWN)) {
list_for_each_entry_safe(cmd, nxt, >waiting_rsp, list) {
-   iue = cmd->iue;
+   /*
+* If Task Abort Status Bit is set, then dont send a
+* response.
+*/
+   if (cmd->se_cmd.transport_state & CMD_T_TAS) {
+   list_del(>list);
+   ibmvscsis_free_cmd_resources(vscsi, cmd);
+   } else {
+   iue = cmd->iue;
 
-   crq->valid = VALID_CMD_RESP_EL;
-   crq->format = cmd->rsp.format;
+   crq->valid = VALID_CMD_RESP_EL;
+   crq->format = cmd->rsp.format;
 
-   if (cmd->flags & CMD_FAST_FAIL)
-   crq->status = VIOSRP_ADAPTER_FAIL;
+   if (cmd->flags & CMD_FAST_FAIL)
+   crq->status = VIOSRP_ADAPTER_FAIL;
 
-   crq->IU_length = cpu_to_be16(cmd->rsp.len);
+   crq->IU_length = cpu_to_be16(cmd->rsp.len);
 
-   rc = h_send_crq(vscsi->dma_dev->unit_address,
-   be64_to_cpu(msg_hi),
-   be64_to_cpu(cmd->rsp.tag));
+   rc = h_send_crq(vscsi->dma_dev->unit_address,
+   be64_to_cpu(msg_hi),
+   be64_to_cpu(cmd->rsp.tag));
 
-   pr_debug("send_messages: cmd %p, tag 0x%llx, rc %ld\n",
-cmd, be64_to_cpu(cmd->rsp.tag), rc);
+   pr_debug("send_messages: cmd %p, tag 0x%llx, rc 
%ld\n",
+cmd, be64_to_cpu(cmd->rsp.tag), rc);
 
-   /* if all ok free up the command element resources */
-   if (rc == H_SUCCESS) {
-   /* some movement has occurred */
-   vscsi->rsp_q_timer.timer_pops = 0;
-   list_del(>list);
+   /* if all ok free up the command element 
resources */
+   if (rc == H_SUCCESS) {
+   /* some movement has occurred */
+   vscsi->rsp_q_timer.timer_pops = 0;
+   list_del(>list);
 
-   ibmvscsis_free_cmd_resources(vscsi, cmd);
-   } else {
-   srp_snd_msg_failed(vscsi, rc);
-   break;
+   ibmvscsis_free_cmd_resources(vscsi, 
cmd);
+   } else {
+   srp_snd_msg_failed(vscsi, rc);
+   break;
+   }
}
}
 
@@ -3581,9 +3590,20 @@ static int ibmvscsis_write_pending(struct se_cmd *se_cmd)
 {
struct ibmvscsis_cmd *cmd = container_of(se_cmd, struct ibmvscsis_cmd,
 se_cmd);
+   struct scsi_info *vscsi = cmd->adapter;
struct iu_entry *iue = cmd->iue;
int rc;
 
+   /*
+*

Re: [PATCH] ibmvscsis: Do not send aborted task response

2017-04-07 Thread Bryant G. Ly



On 4/7/17 11:36 AM, Bart Van Assche wrote:

On Fri, 2017-04-07 at 11:12 -0500, Bryant G. Ly wrote:

So from this stack trace it looks like the ibmvscsis driver is sending an
extra response through send_messages even though an abort was issued.
IBMi, reported this issue internally when they were testing the driver,
because their client didn't like getting a response back for the aborted op.
They are only expecting a TMR from the abort request, NOT the aborted op.

Hello Bryant,

What is the root cause of this behavior? Why is it that the behavior of
the ibmvscsi_tgt contradicts what has been implemented in the LIO core?
Sorry but the patch at the start of this thread looks to me like an
attempt to paper over the problem instead of addressing the root cause.

Thanks,


IBMi clients received a response for an aborted operation, so they sent an 
abort tm request.
Afterwards they get a CRQ response to the op that they aborted. That should not 
happen, because they are only supposed to get a response for the tm request NOT 
the aborted operation.
Looking at the code it looks like it is due to send messages, processing a 
response without checking to see if it was an aborted op.
This patch addresses a bug within the ibmvscsis driver and the fact that it 
SENT a response to the aborted operation(which is wrong since) without looking 
at what LIO core had done.
The driver isn't supposed to send any response to the aborted operation, BUT 
only a response to the abort tm request, which LIO core currently does.

-Bryant




Re: [PATCH] ibmvscsis: Do not send aborted task response

2017-04-07 Thread Bryant G. Ly



On 4/7/17 10:49 AM, Bryant G. Ly wrote:

The driver is sending a response to the aborted task response
along with LIO sending the tmr response. SCSI spec says
that the initiator that sends the abort task TM NEVER gets a
response to the aborted op and with the current code it will
send a response. Thus this fix will remove that response if the
op is aborted.

Cc: <sta...@vger.kernel.org> # v4.8+
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 60 +---
  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h |  1 +
  2 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 4bb5635..8e2733f 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1169,6 +1169,7 @@ static struct ibmvscsis_cmd 
*ibmvscsis_get_free_cmd(struct scsi_info *vscsi)
cmd = list_first_entry_or_null(>free_cmd,
   struct ibmvscsis_cmd, list);
if (cmd) {
+   cmd->flags &= ~(CMD_ABORTED);
list_del(>list);
cmd->iue = iue;
cmd->type = UNSET_TYPE;
@@ -1758,33 +1759,41 @@ static void ibmvscsis_send_messages(struct scsi_info 
*vscsi)

if (!(vscsi->flags & RESPONSE_Q_DOWN)) {
list_for_each_entry_safe(cmd, nxt, >waiting_rsp, list) {
-   iue = cmd->iue;
+   /*
+* If an Abort flag is set then dont send response
+*/
+   if (cmd->flags & CMD_ABORTED) {
+   list_del(>list);
+   ibmvscsis_free_cmd_resources(vscsi, cmd);
+   } else {
+   iue = cmd->iue;

-   crq->valid = VALID_CMD_RESP_EL;
-   crq->format = cmd->rsp.format;
+   crq->valid = VALID_CMD_RESP_EL;
+   crq->format = cmd->rsp.format;

-   if (cmd->flags & CMD_FAST_FAIL)
-   crq->status = VIOSRP_ADAPTER_FAIL;
+   if (cmd->flags & CMD_FAST_FAIL)
+   crq->status = VIOSRP_ADAPTER_FAIL;

-   crq->IU_length = cpu_to_be16(cmd->rsp.len);
+   crq->IU_length = cpu_to_be16(cmd->rsp.len);

-   rc = h_send_crq(vscsi->dma_dev->unit_address,
-   be64_to_cpu(msg_hi),
-   be64_to_cpu(cmd->rsp.tag));
+   rc = h_send_crq(vscsi->dma_dev->unit_address,
+   be64_to_cpu(msg_hi),
+   be64_to_cpu(cmd->rsp.tag));

-   pr_debug("send_messages: cmd %p, tag 0x%llx, rc %ld\n",
-cmd, be64_to_cpu(cmd->rsp.tag), rc);
+   pr_debug("send_messages: cmd %p, tag 0x%llx, rc 
%ld\n",
+cmd, be64_to_cpu(cmd->rsp.tag), rc);

-   /* if all ok free up the command element resources */
-   if (rc == H_SUCCESS) {
-   /* some movement has occurred */
-   vscsi->rsp_q_timer.timer_pops = 0;
-   list_del(>list);
+   /* if all ok free up the command element 
resources */
+   if (rc == H_SUCCESS) {
+   /* some movement has occurred */
+   vscsi->rsp_q_timer.timer_pops = 0;
+   list_del(>list);

-   ibmvscsis_free_cmd_resources(vscsi, cmd);
-   } else {
-   srp_snd_msg_failed(vscsi, rc);
-   break;
+   ibmvscsis_free_cmd_resources(vscsi, 
cmd);
+   } else {
+   srp_snd_msg_failed(vscsi, rc);
+   break;
+   }
}
}

@@ -3581,9 +3590,15 @@ static int ibmvscsis_write_pending(struct se_cmd *se_cmd)
  {
struct ibmvscsis_cmd *cmd = container_of(se_cmd, struct ibmvscsis_cmd,
 se_cmd);
+   struct scsi_info *vscsi = cmd->adapter;
struct iu_entry *iue 

[PATCH] ibmvscsis: Do not send aborted task response

2017-04-07 Thread Bryant G. Ly
The driver is sending a response to the aborted task response
along with LIO sending the tmr response. SCSI spec says
that the initiator that sends the abort task TM NEVER gets a
response to the aborted op and with the current code it will
send a response. Thus this fix will remove that response if the
op is aborted.

Cc: <sta...@vger.kernel.org> # v4.8+
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 60 +---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h |  1 +
 2 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 4bb5635..8e2733f 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1169,6 +1169,7 @@ static struct ibmvscsis_cmd 
*ibmvscsis_get_free_cmd(struct scsi_info *vscsi)
cmd = list_first_entry_or_null(>free_cmd,
   struct ibmvscsis_cmd, list);
if (cmd) {
+   cmd->flags &= ~(CMD_ABORTED);
list_del(>list);
cmd->iue = iue;
cmd->type = UNSET_TYPE;
@@ -1758,33 +1759,41 @@ static void ibmvscsis_send_messages(struct scsi_info 
*vscsi)
 
if (!(vscsi->flags & RESPONSE_Q_DOWN)) {
list_for_each_entry_safe(cmd, nxt, >waiting_rsp, list) {
-   iue = cmd->iue;
+   /*
+* If an Abort flag is set then dont send response
+*/
+   if (cmd->flags & CMD_ABORTED) {
+   list_del(>list);
+   ibmvscsis_free_cmd_resources(vscsi, cmd);
+   } else {
+   iue = cmd->iue;
 
-   crq->valid = VALID_CMD_RESP_EL;
-   crq->format = cmd->rsp.format;
+   crq->valid = VALID_CMD_RESP_EL;
+   crq->format = cmd->rsp.format;
 
-   if (cmd->flags & CMD_FAST_FAIL)
-   crq->status = VIOSRP_ADAPTER_FAIL;
+   if (cmd->flags & CMD_FAST_FAIL)
+   crq->status = VIOSRP_ADAPTER_FAIL;
 
-   crq->IU_length = cpu_to_be16(cmd->rsp.len);
+   crq->IU_length = cpu_to_be16(cmd->rsp.len);
 
-   rc = h_send_crq(vscsi->dma_dev->unit_address,
-   be64_to_cpu(msg_hi),
-   be64_to_cpu(cmd->rsp.tag));
+   rc = h_send_crq(vscsi->dma_dev->unit_address,
+   be64_to_cpu(msg_hi),
+   be64_to_cpu(cmd->rsp.tag));
 
-   pr_debug("send_messages: cmd %p, tag 0x%llx, rc %ld\n",
-cmd, be64_to_cpu(cmd->rsp.tag), rc);
+   pr_debug("send_messages: cmd %p, tag 0x%llx, rc 
%ld\n",
+cmd, be64_to_cpu(cmd->rsp.tag), rc);
 
-   /* if all ok free up the command element resources */
-   if (rc == H_SUCCESS) {
-   /* some movement has occurred */
-   vscsi->rsp_q_timer.timer_pops = 0;
-   list_del(>list);
+   /* if all ok free up the command element 
resources */
+   if (rc == H_SUCCESS) {
+   /* some movement has occurred */
+   vscsi->rsp_q_timer.timer_pops = 0;
+   list_del(>list);
 
-   ibmvscsis_free_cmd_resources(vscsi, cmd);
-   } else {
-   srp_snd_msg_failed(vscsi, rc);
-   break;
+   ibmvscsis_free_cmd_resources(vscsi, 
cmd);
+   } else {
+   srp_snd_msg_failed(vscsi, rc);
+   break;
+   }
}
}
 
@@ -3581,9 +3590,15 @@ static int ibmvscsis_write_pending(struct se_cmd *se_cmd)
 {
struct ibmvscsis_cmd *cmd = container_of(se_cmd, struct ibmvscsis_cmd,
 se_cmd);
+   struct scsi_info *vscsi = cmd->adapter;
struct iu_entry *iue = cmd->iue;
int rc;

[PATCH] target/user: PGR Support

2017-03-30 Thread Bryant G. Ly
This adds PGR support for just TCMU, since tcmu doesn't
have the necessary IT_NEXUS info to process PGR in userspace,
so have those commands be processed in kernel.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_configfs.c | 10 +-
 drivers/target/target_core_device.c   | 27 +++
 drivers/target/target_core_pr.c   |  2 +-
 drivers/target/target_core_pscsi.c|  3 ++-
 include/target/target_core_backend.h  |  1 +
 5 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/drivers/target/target_core_configfs.c 
b/drivers/target/target_core_configfs.c
index 38b5025..edfb098 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -1366,7 +1366,7 @@ static ssize_t target_pr_res_holder_show(struct 
config_item *item, char *page)
struct se_device *dev = pr_to_dev(item);
int ret;
 
-   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
+   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)
return sprintf(page, "Passthrough\n");
 
spin_lock(>dev_reservation_lock);
@@ -1506,7 +1506,7 @@ static ssize_t target_pr_res_type_show(struct config_item 
*item, char *page)
 {
struct se_device *dev = pr_to_dev(item);
 
-   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
+   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)
return sprintf(page, "SPC_PASSTHROUGH\n");
else if (dev->dev_reservation_flags & DRF_SPC2_RESERVATIONS)
return sprintf(page, "SPC2_RESERVATIONS\n");
@@ -1519,7 +1519,7 @@ static ssize_t target_pr_res_aptpl_active_show(struct 
config_item *item,
 {
struct se_device *dev = pr_to_dev(item);
 
-   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
+   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)
return 0;
 
return sprintf(page, "APTPL Bit Status: %s\n",
@@ -1531,7 +1531,7 @@ static ssize_t target_pr_res_aptpl_metadata_show(struct 
config_item *item,
 {
struct se_device *dev = pr_to_dev(item);
 
-   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
+   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)
return 0;
 
return sprintf(page, "Ready to process PR APTPL metadata..\n");
@@ -1577,7 +1577,7 @@ static ssize_t target_pr_res_aptpl_metadata_store(struct 
config_item *item,
u16 tpgt = 0;
u8 type = 0;
 
-   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
+   if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)
return count;
if (dev->dev_reservation_flags & DRF_SPC2_RESERVATIONS)
return count;
diff --git a/drivers/target/target_core_device.c 
b/drivers/target/target_core_device.c
index c754ae3..83c0d77 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -1045,6 +1045,7 @@ passthrough_parse_cdb(struct se_cmd *cmd,
sense_reason_t (*exec_cmd)(struct se_cmd *cmd))
 {
unsigned char *cdb = cmd->t_task_cdb;
+   struct se_device *dev = cmd->se_dev;
 
/*
 * Clear a lun set in the cdb if the initiator talking to use spoke
@@ -1076,6 +1077,32 @@ passthrough_parse_cdb(struct se_cmd *cmd,
return TCM_NO_SENSE;
}
 
+   /*
+* For PERSISTENT RESERVE IN/OUT, RELEASE, and RESERVE we need to
+* emulate the response, since tcmu does not have the information
+* required to process these commands.
+*/
+   if (!(dev->transport->transport_flags &
+ TRANSPORT_FLAG_PASSTHROUGH_PGR)) {
+   if (cdb[0] == PERSISTENT_RESERVE_IN) {
+   cmd->execute_cmd = target_scsi3_emulate_pr_in;
+   return TCM_NO_SENSE;
+   }
+   if (cdb[0] == PERSISTENT_RESERVE_OUT) {
+   cmd->execute_cmd = target_scsi3_emulate_pr_out;
+   return TCM_NO_SENSE;
+   }
+
+   if (cdb[0] == RELEASE || cdb[0] == RELEASE_10) {
+   cmd->execute_cmd = target_scsi2_reservation_release;
+   return TCM_NO_SENSE;
+   }
+   if (cdb[0] == RESERVE || cdb[0] == RESERVE_10) {
+   cmd->execute_cmd = target_scsi2_reservation_reserve;
+   return TCM_NO_SENSE;
+   }
+   }
+
/* Set DATA_CDB flag for ops that should have it */
switch (cdb[0]) {
case READ_6:
diff --git a/drivers/target/target_core_pr.c b/dr

Re: [PATCHv4 0/4] tcmu: bug fix and dynamic growing data area support

2017-03-21 Thread Bryant G. Ly

On 3/21/17 3:36 AM, lixi...@cmss.chinamobile.com wrote:

From: Xiubo Li 

Changed for V4:
- re-order the #3, #4 at the head.
- merge most of the #5 to others.


Changed for V3:
- [PATCHv2 2/5] fix double usage of blocks and possible page fault
   call trace.
- [PATCHv2 5/5] fix a mistake.

Changed for V2:
- [PATCHv2 1/5] just fixes some small spelling and other mistakes.
   And as the initial patch, here sets cmd area to 8M and data area to
   1G(1M fixed and 1023M growing)
- [PATCHv2 2/5] is a new one, adding global data block pool support.
   The max total size of the pool is 2G and all the targets will get
   growing blocks from here.
   Test this using multi-targets at the same time.
- [PATCHv2 3/5] changed nothing, respin it to avoid the conflict.
- [PATCHv2 4/5] and [PATCHv2 5/5] are new ones.



Xiubo Li (4):
   tcmu: Fix possible overwrite of t_data_sg's last iov[]
   tcmu: Fix wrongly calculating of the base_command_size
   tcmu: Add dynamic growing data area feature support
   tcmu: Add global data block pool support

  drivers/target/target_core_user.c | 605 +++---
  1 file changed, 504 insertions(+), 101 deletions(-)


I have built this patch into our kernel and will get back to you.

We will be doing larger scale testing to make sure everything still works.

-Bryant




Re: [PATCHv2 5/5] target/user: Clean up tcmu_queue_cmd_ring

2017-03-16 Thread Bryant G. Ly



From: Xiubo Li 

Add two helpers to simplify the code, and move some code out of
the cmdr spin lock to reduce the size of critical region.

Signed-off-by: Xiubo Li 
---
  drivers/target/target_core_user.c | 54 ++-
  1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index 117be07..5205d2f 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c






+static inline void tcmu_cmd_get_cmd_size(struct se_cmd *se_cmd,
+   size_t *base_size, size_t *cmd_size)


Should this be tcmu_cmd_get_cmd_size(struct tcmu_cmd *tcmu_cmd instead? Look at 
bottom.


+{
+   struct se_cmd *se_cmd = tcmu_cmd->se_cmd;
+
+   *base_size = max(offsetof(struct tcmu_cmd_entry,
+req.iov[tcmu_cmd->dbi_len]),
+sizeof(struct tcmu_cmd_entry));
+
+   *cmd_size = round_up(scsi_command_size(se_cmd->t_task_cdb),
+   TCMU_OP_ALIGN_SIZE) + *base_size;
+   WARN_ON(*cmd_size & (TCMU_OP_ALIGN_SIZE-1));
+}
+
  static sense_reason_t
  tcmu_queue_cmd_ring(struct tcmu_cmd *tcmu_cmd)
  {
struct tcmu_dev *udev = tcmu_cmd->tcmu_dev;
struct se_cmd *se_cmd = tcmu_cmd->se_cmd;
size_t base_command_size, command_size;
-   struct tcmu_mailbox *mb;
+   struct tcmu_mailbox *mb = udev->mb_addr;
struct tcmu_cmd_entry *entry;
struct iovec *iov;
int iov_cnt, ret;
@@ -642,6 +660,8 @@ static bool is_ring_space_avail(struct tcmu_dev *udev, 
struct tcmu_cmd *cmd,
if (test_bit(TCMU_DEV_BIT_BROKEN, >flags))
return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;

+   if (se_cmd->se_cmd_flags & SCF_BIDI)
+   BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents));
/*
 * Must be a certain minimum size for response sense info, but
 * also may be larger if the iov array is large.
@@ -649,33 +669,19 @@ static bool is_ring_space_avail(struct tcmu_dev *udev, 
struct tcmu_cmd *cmd,
 * We prepare way too many iovs for potential uses here, because it's
 * expensive to tell how many regions are freed in the bitmap
*/
-   base_command_size = max(offsetof(struct tcmu_cmd_entry,
-   req.iov[tcmu_cmd->dbi_len]),
-   sizeof(struct tcmu_cmd_entry));
-   command_size = base_command_size
-   + round_up(scsi_command_size(se_cmd->t_task_cdb), 
TCMU_OP_ALIGN_SIZE);
-
-   WARN_ON(command_size & (TCMU_OP_ALIGN_SIZE-1));
-
-   spin_lock_irq(>cmdr_lock);
-
-   mb = udev->mb_addr;
-   cmd_head = mb->cmd_head % udev->cmdr_size; /* UAM */
-   data_length = round_up(se_cmd->data_length, DATA_BLOCK_SIZE);
-   if (se_cmd->se_cmd_flags & SCF_BIDI) {
-   BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents));
-   data_length += round_up(se_cmd->t_bidi_data_sg->length,
-   DATA_BLOCK_SIZE);
-   }
+   tcmu_cmd_get_cmd_size(tcmu_cmd, _command_size, _size);




So Based upon your definition of tcmu_cmd_get_cmd_size, why did you pass in 
tcmu_cmd here? It wont compile in its current state.

-Bryant



Re: [PATCHv2 3/5] target/user: Fix possible overwrite of t_data_sg's last iov[]

2017-03-16 Thread Bryant G. Ly



On 3/8/17 2:45 AM, lixi...@cmss.chinamobile.com wrote:

From: Xiubo Li <lixi...@cmss.chinamobile.com>

If there has BIDI data, its first iov[] will overwrite the last
iov[] for se_cmd->t_data_sg.

To fix this, we can just increase the iov pointer, but this may
introuduce a new memory leakage bug: If the se_cmd->data_length
and se_cmd->t_bidi_data_sg->length are all not aligned up to the
DATA_BLOCK_SIZE, the actual length needed maybe larger than just
sum of them.

So, this could be avoided by rounding all the data lengthes up
to DATA_BLOCK_SIZE.

Signed-off-by: Xiubo Li <lixi...@cmss.chinamobile.com>
---
  drivers/target/target_core_user.c | 32 +++-
  1 file changed, 19 insertions(+), 13 deletions(-)


I have seen this in my environment:
(gdb) print *((tcmulib_cmd->iovec)+0)
$7 = {iov_base = 0x3fff7c3d, iov_len = 8192}
(gdb) print *((tcmulib_cmd->iovec)+1)
$3 = {iov_base = 0x3fff7c3da000, iov_len = 4096}
(gdb) print *((tcmulib_cmd->iovec)+2)
$4 = {iov_base = 0x3fff7c3dc000, iov_len = 16384}
(gdb) print *((tcmulib_cmd->iovec)+3)
$5 = {iov_base = 0x3fff7c3f7000, iov_len = 12288}
(gdb) print *((tcmulib_cmd->iovec)+4)
$6 = {iov_base = 0x1306e853c0028, iov_len = 128}  <--- bad pointer and length

So this fix would be great!

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>




Re: [PATCH] target: Fix NULL dereference during LUN lookup + active I/O shutdown

2017-02-23 Thread Bryant G. Ly



From: Nicholas Bellinger 

When transport_clear_lun_ref() is shutting down a se_lun via
configfs with new I/O in-flight, it's possible to trigger a
NULL pointer dereference in transport_lookup_cmd_lun() due
to the fact percpu_ref_get() doesn't do any __PERCPU_REF_DEAD
checking before incrementing lun->lun_ref.count after
lun->lun_ref has switched to atomic_t mode.

This results in a NULL pointer dereference as LUN shutdown
code in core_tpg_remove_lun() continues running after the
existing ->release() -> core_tpg_lun_ref_release() callback
completes, and clears the RCU protected se_lun->lun_se_dev
pointer.

During the OOPs, the state of lun->lun_ref in the process
which triggered the NULL pointer dereference looks like
the following on v4.1.y stable code:

struct se_lun {
   lun_link_magic = 4294932337,
   lun_status = TRANSPORT_LUN_STATUS_FREE,

   .

   lun_se_dev = 0x0,
   lun_sep = 0x0,

   .

   lun_ref = {
 count = {
   counter = 1
 },
 percpu_count_ptr = 3,
 release = 0xa02fa1e0 ,
 confirm_switch = 0x0,
 force_atomic = false,
 rcu = {
   next = 0x88154fa1a5d0,
   func = 0x8137c4c0 
 }
   }
}

To address this bug, use percpu_ref_tryget_live() to ensure
once __PERCPU_REF_DEAD is visable on all CPUs and ->lun_ref
has switched to atomic_t, all new I/Os will fail to obtain
a new lun->lun_ref reference.

Also use an explicit percpu_ref_kill_and_confirm() callback
to block on ->lun_ref_comp to allow the first stage and
associated RCU grace period to complete, and then block on
->lun_ref_shutdown waiting for the final percpu_ref_put()
to drop the last reference via transport_lun_remove_cmd()
before continuing with core_tpg_remove_lun() shutdown.

Reported-by: Rob Millner 
Tested-by: Rob Millner 
Cc: Rob Millner 
Tested-by: Vaibhav Tandon 
Cc: Vaibhav Tandon 
Signed-off-by: Nicholas Bellinger 
---
  drivers/target/target_core_device.c| 10 --
  drivers/target/target_core_tpg.c   |  3 ++-
  drivers/target/target_core_transport.c | 31 ++-
  include/target/target_core_base.h  |  1 +
  4 files changed, 41 insertions(+), 4 deletions(-)


I have seen this and have tested this with our custom kernel.

So this looks good from me!

-Bryant




[PATCH] ibmvscsis: Fix max transfer length

2017-01-11 Thread Bryant G. Ly
Current code incorrectly calculates the max transfer length, since
it is assuming a 4k page table, but ppc64 all run on 64k page tables.

Cc: sta...@vger.kernel.org
Reported-by: Steven Royer <sero...@linux.vnet.ibm.com>
Tested-by: Steven Royer <sero...@linux.vnet.ibm.com>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 8fb5c54..9c91e75 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -46,6 +46,7 @@
 
 #defineINITIAL_SRP_LIMIT   800
 #defineDEFAULT_MAX_SECTORS 256
+#define MAX_TXU1024 * 1024
 
 static uint max_vdma_size = MAX_H_COPY_RDMA;
 
@@ -1443,7 +1444,7 @@ static long ibmvscsis_adapter_info(struct scsi_info 
*vscsi,
info->mad_version = cpu_to_be32(MAD_VERSION_1);
info->os_type = cpu_to_be32(LINUX);
memset(>port_max_txu[0], 0, sizeof(info->port_max_txu));
-   info->port_max_txu[0] = cpu_to_be32(128 * PAGE_SIZE);
+   info->port_max_txu[0] = cpu_to_be32(MAX_TXU);
 
dma_wmb();
rc = h_copy_rdma(sizeof(*info), vscsi->dds.window[LOCAL].liobn,
-- 
2.5.4 (Apple Git-61)

--
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] ibmvscsis: Fix max transfer length

2017-01-11 Thread Bryant G. Ly
Current code incorrectly calculates the max transfer length, since
it is assuming a 4k page table, but ppc64 all run on 64k page tables.

Cc: sta...@vger.kernel.org
Reported-by: Steven Royer <sero...@linux.vnet.ibm.com>
Tested-by: Steven Royer <sero...@linux.vnet.ibm.com>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 8fb5c54..3b4d36b 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -46,6 +46,7 @@
 
 #defineINITIAL_SRP_LIMIT   800
 #defineDEFAULT_MAX_SECTORS 256
+#define MAX_TXU1024
 
 static uint max_vdma_size = MAX_H_COPY_RDMA;
 
@@ -1443,7 +1444,7 @@ static long ibmvscsis_adapter_info(struct scsi_info 
*vscsi,
info->mad_version = cpu_to_be32(MAD_VERSION_1);
info->os_type = cpu_to_be32(LINUX);
memset(>port_max_txu[0], 0, sizeof(info->port_max_txu));
-   info->port_max_txu[0] = cpu_to_be32(128 * PAGE_SIZE);
+   info->port_max_txu[0] = cpu_to_be32(MAX_TXU);
 
dma_wmb();
rc = h_copy_rdma(sizeof(*info), vscsi->dds.window[LOCAL].liobn,
-- 
2.5.4 (Apple Git-61)

--
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] ibmvscsis: Fix max transfer length

2017-01-11 Thread Bryant G. Ly
Current code incorrectly calculates the max transfer length, since
it is assuming a 4k page table, but ppc64 all run on 64k page tables.

Cc: sta...@vger.kernel.org
Reported-by: Steven Royer <sero...@linux.vnet.ibm.com>
Tested-by: Steven Royer <sero...@linux.vnet.ibm.com>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 8fb5c54..3b4d36b 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -46,6 +46,7 @@
 
 #defineINITIAL_SRP_LIMIT   800
 #defineDEFAULT_MAX_SECTORS 256
+#define MAX_TXU1024
 
 static uint max_vdma_size = MAX_H_COPY_RDMA;
 
@@ -1443,7 +1444,7 @@ static long ibmvscsis_adapter_info(struct scsi_info 
*vscsi,
info->mad_version = cpu_to_be32(MAD_VERSION_1);
info->os_type = cpu_to_be32(LINUX);
memset(>port_max_txu[0], 0, sizeof(info->port_max_txu));
-   info->port_max_txu[0] = cpu_to_be32(128 * PAGE_SIZE);
+   info->port_max_txu[0] = cpu_to_be32(MAX_TXU);
 
dma_wmb();
rc = h_copy_rdma(sizeof(*info), vscsi->dds.window[LOCAL].liobn,
-- 
2.5.4 (Apple Git-61)

--
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] ibmvscsis: Fix max transfer length

2017-01-11 Thread Bryant G. Ly

On 1/11/17 12:57 PM, Bart Van Assche wrote:


On Wed, 2017-01-11 at 12:02 -0600, Bryant G. Ly wrote:

Current code incorrectly calculates the max transfer length, since
it is assuming a 4k page table, but ppc64 all run on 64k page tables.

Cc: sta...@vger.kernel.org
Reported-by: Steven Royer <sero...@linux.vnet.ibm.com>
Tested-by: Steven Royer <sero...@linux.vnet.ibm.com>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 8fb5c54..792a8bd 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1443,7 +1443,7 @@ static long ibmvscsis_adapter_info(struct scsi_info
*vscsi,
info->mad_version = cpu_to_be32(MAD_VERSION_1);
info->os_type = cpu_to_be32(LINUX);
memset(>port_max_txu[0], 0, sizeof(info->port_max_txu));
-   info->port_max_txu[0] = cpu_to_be32(128 * PAGE_SIZE);
+   info->port_max_txu[0] = cpu_to_be32(16 * PAGE_SIZE);
  
  	dma_wmb();

rc = h_copy_rdma(sizeof(*info), vscsi->dds.window[LOCAL].liobn,

Hello Bryant,

The old limit was intended to be 128 * 4 KB = 512 KB. The new limit is 16 *
64 KB = 1024 KB. Is that intended?

Do I understand correctly that the max_txu value is independent of the page
size? If so, have you considered to introduce a #define for the max_mtu
value that specifies max_mtu as a number instead of as a multiple of the
page size? At least in the SLES 10 SP3 ibmvscsis.c code max_mtu was not
defined as a multiple of the page size.


Hi Bart,

Thanks for the review. You are correct, max_txu is independent of the 
page size.

A define is probably best.

The reason for this is people who are using tcmu-runner, they have a max 
transfer
length of 1024KB, so if you exceed this value, then commands will fail. 
Thus to prevent
people from experiencing this issue, we are going to limit our driver to 
that cap.


The "allowed values" from a typical AIX disk can go all the way up to 
16MB. Typically

the max values do default at 512KB though.

-Bryant.

--
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] ibmvscsis: fix sleeping in interrupt context

2017-01-11 Thread Bryant G. Ly
Currently, dma_alloc_coherent is being called with a GFP_KERNEL
flag which allows it to sleep in an interrupt context, need to
change to GFP_ATOMIC.

Cc: sta...@vger.kernel.org
Tested-by: Steven Royer <sero...@linux.vnet.ibm.com>
Reviewed-by: Michael Cyr <mike...@linux.vnet.ibm.com>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 792a8bd..8e308fd 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1391,7 +1391,7 @@ static long ibmvscsis_adapter_info(struct scsi_info 
*vscsi,
}
 
info = dma_alloc_coherent(>dma_dev->dev, sizeof(*info), ,
- GFP_KERNEL);
+ GFP_ATOMIC);
if (!info) {
dev_err(>dev, "bad dma_alloc_coherent %p\n",
iue->target);
@@ -1509,7 +1509,7 @@ static int ibmvscsis_cap_mad(struct scsi_info *vscsi, 
struct iu_entry *iue)
}
 
cap = dma_alloc_coherent(>dma_dev->dev, olen, ,
-GFP_KERNEL);
+GFP_ATOMIC);
if (!cap) {
dev_err(>dev, "bad dma_alloc_coherent %p\n",
iue->target);
-- 
2.5.4 (Apple Git-61)

--
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] ibmvscsis: Fix max transfer length

2017-01-11 Thread Bryant G. Ly
Current code incorrectly calculates the max transfer length, since
it is assuming a 4k page table, but ppc64 all run on 64k page tables.

Cc: sta...@vger.kernel.org
Reported-by: Steven Royer <sero...@linux.vnet.ibm.com>
Tested-by: Steven Royer <sero...@linux.vnet.ibm.com>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 8fb5c54..792a8bd 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1443,7 +1443,7 @@ static long ibmvscsis_adapter_info(struct scsi_info 
*vscsi,
info->mad_version = cpu_to_be32(MAD_VERSION_1);
info->os_type = cpu_to_be32(LINUX);
memset(>port_max_txu[0], 0, sizeof(info->port_max_txu));
-   info->port_max_txu[0] = cpu_to_be32(128 * PAGE_SIZE);
+   info->port_max_txu[0] = cpu_to_be32(16 * PAGE_SIZE);
 
dma_wmb();
rc = h_copy_rdma(sizeof(*info), vscsi->dds.window[LOCAL].liobn,
-- 
2.5.4 (Apple Git-61)

--
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] ibmvscsis: Fix srp_transfer_data fail return code

2017-01-09 Thread Bryant G. Ly

On 1/9/17 10:47 AM, Greg KH wrote:


On Mon, Jan 09, 2017 at 10:21:20AM -0600, Bryant G. Ly wrote:

From: "Bryant G. Ly" <b...@us.ibm.com>

If srp_transfer_data fails within ibmvscsis_write_pending, then
the most likely scenario is that the client timed out the op and
removed the TCE mapping. Thus it will loop forever retrying the
op that is pretty much guaranteed to fail forever. A better return
code would be EIO instead of EAGAIN.

Cc: sta...@vger.kernel.org
Reported-by: Steven Royer <sero...@linux.vnet.ibm.com>
Tested-by: Steven Royer <sero...@linux.vnet.ibm.com>
Signed-off-by: Bryant G. Ly <b...@us.ibm.com>
---
  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Why are you sending this to me?

confused,

greg k-h

Sorry I meant to put --to target-de...@vger.kernel.org, and just --cc you.

-Bryant

--
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] ibmvscsis: Fix srp_transfer_data fail return code

2017-01-09 Thread Bryant G. Ly
From: "Bryant G. Ly" <b...@us.ibm.com>

If srp_transfer_data fails within ibmvscsis_write_pending, then
the most likely scenario is that the client timed out the op and
removed the TCE mapping. Thus it will loop forever retrying the
op that is pretty much guaranteed to fail forever. A better return
code would be EIO instead of EAGAIN.

Cc: sta...@vger.kernel.org
Reported-by: Steven Royer <sero...@linux.vnet.ibm.com>
Tested-by: Steven Royer <sero...@linux.vnet.ibm.com>
Signed-off-by: Bryant G. Ly <b...@us.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 3d3768a..8fb5c54 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -3585,7 +3585,7 @@ static int ibmvscsis_write_pending(struct se_cmd *se_cmd)
   1, 1);
if (rc) {
pr_err("srp_transfer_data() failed: %d\n", rc);
-   return -EAGAIN;
+   return -EIO;
}
/*
 * We now tell TCM to add this WRITE CDB directly into the TCM storage
-- 
2.5.4 (Apple Git-61)

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


Re: [PATCH v1 1/3] Synchronization of cmds during termination conditions

2016-10-12 Thread Bryant G. Ly


On 10/11/16 5:58 PM, Michael Cyr wrote:

Signed-off-by: Michael Cyr 
---
  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 1082 +-
  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h |5 +-
  2 files changed, 486 insertions(+), 601 deletions(-)


I would make the first patch reorganization and styling fixes, no actual 
code changes.
Then for each of the commits make sure you amend them to give more 
description

as to what you are doing in each specific patch.

i.e. First patch:

ibmvscsis: Re-organization of commands/styling

Need to move functions around in order for the proceeding patches to work.

Patch 2/3 and 3/3 look fine to me after you change the title to include 
"ibmvscsis:" in it.


-Bryant

--
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 v1] Ibmvscsis: Fixed unused variable

2016-09-16 Thread Bryant G. Ly
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index b29fef9..f8ba2a0 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -3193,8 +3193,6 @@ static int ibmvscsis_rdma(struct ibmvscsis_cmd *cmd, 
struct scatterlist *sg,
 server_ioba);
} else {
/* write to client */
-   struct srp_cmd *srp = (struct srp_cmd *)iue->sbuf->buf;
-
if (!READ_CMD(srp->cdb))
print_hex_dump_bytes(" data:", DUMP_PREFIX_NONE,
 sg_virt(sgp), buf_len);
-- 
2.5.4 (Apple Git-61)

--
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 v1 2/3] Ibmvscsis: Code cleanup of print statements

2016-08-31 Thread Bryant G. Ly
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Signed-off-by: Michael Cyr <mike...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 26 ++
 1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index a515cdd..d4c67b9 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1606,8 +1606,6 @@ static void ibmvscsis_send_messages(struct scsi_info 
*vscsi)
 
if (!(vscsi->flags & RESPONSE_Q_DOWN)) {
list_for_each_entry_safe(cmd, nxt, >waiting_rsp, list) {
-   pr_debug("send_messages cmd %p\n", cmd);
-
iue = cmd->iue;
 
crq->valid = VALID_CMD_RESP_EL;
@@ -2556,10 +2554,6 @@ static void ibmvscsis_parse_cmd(struct scsi_info *vscsi,
 
srp->lun.scsi_lun[0] &= 0x3f;
 
-   pr_debug("calling submit_cmd, se_cmd %p, lun 0x%llx, cdb 0x%x, 
attr:%d\n",
->se_cmd, scsilun_to_int(>lun), (int)srp->cdb[0],
-attr);
-
rc = target_submit_cmd(>se_cmd, nexus->se_sess, srp->cdb,
   cmd->sense_buf, scsilun_to_int(>lun),
   data_len, attr, dir, 0);
@@ -3144,8 +3138,6 @@ static int ibmvscsis_rdma(struct ibmvscsis_cmd *cmd, 
struct scatterlist *sg,
long tx_len;
long rc = 0;
 
-   pr_debug("rdma: dir %d, bytes 0x%x\n", dir, bytes);
-
if (bytes == 0)
return 0;
 
@@ -3197,9 +3189,6 @@ static int ibmvscsis_rdma(struct ibmvscsis_cmd *cmd, 
struct scatterlist *sg,
/* write to client */
struct srp_cmd *srp = (struct srp_cmd *)iue->sbuf->buf;
 
-   if (!READ_CMD(srp->cdb))
-   print_hex_dump_bytes(" data:", DUMP_PREFIX_NONE,
-sg_virt(sgp), buf_len);
/* The h_copy_rdma will cause phyp, running in another
 * partition, to read memory, so we need to make sure
 * the data has been written out, hence these syncs.
@@ -3324,12 +3313,9 @@ cmd_work:
rc = ibmvscsis_trans_event(vscsi, crq);
} else if (vscsi->flags & TRANS_EVENT) {
/*
-* if a tranport event has occurred leave
+* if a transport event has occurred leave
 * everything but transport events on the queue
-*/
-   pr_debug("handle_crq, ignoring\n");
-
-   /*
+*
 * need to decrement the queue index so we can
 * look at the elment again
 */
@@ -3695,8 +3681,6 @@ static void ibmvscsis_release_cmd(struct se_cmd *se_cmd)
 se_cmd);
struct scsi_info *vscsi = cmd->adapter;
 
-   pr_debug("release_cmd %p, flags %d\n", se_cmd, cmd->flags);
-
spin_lock_bh(>intr_lock);
/* Remove from active_q */
list_del(>list);
@@ -3717,9 +3701,6 @@ static int ibmvscsis_write_pending(struct se_cmd *se_cmd)
struct iu_entry *iue = cmd->iue;
int rc;
 
-   pr_debug("write_pending, se_cmd %p, length 0x%x\n",
-se_cmd, se_cmd->data_length);
-
rc = srp_transfer_data(cmd, _iu(iue)->srp.cmd, ibmvscsis_rdma,
   1, 1);
if (rc) {
@@ -3758,9 +3739,6 @@ static int ibmvscsis_queue_data_in(struct se_cmd *se_cmd)
uint len = 0;
int rc;
 
-   pr_debug("queue_data_in, se_cmd %p, length 0x%x\n",
-se_cmd, se_cmd->data_length);
-
rc = srp_transfer_data(cmd, _iu(iue)->srp.cmd, ibmvscsis_rdma, 1,
   1);
if (rc) {
-- 
2.5.4 (Apple Git-61)

--
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 v1 3/3] Ibmvscsis: Fixed a bug reported by Dan Carpenter

2016-08-31 Thread Bryant G. Ly
SUPPORTED_FORMATS is 1 << 1 so it's never zero.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Signed-off-by: Michael Cyr <mike...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index d4c67b9..35455af 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1978,7 +1978,7 @@ static long ibmvscsis_srp_login(struct scsi_info *vscsi,
reason = SRP_LOGIN_REJ_MULTI_CHANNEL_UNSUPPORTED;
else if (fmt->buffers & (~SUPPORTED_FORMATS))
reason = SRP_LOGIN_REJ_UNSUPPORTED_DESCRIPTOR_FMT;
-   else if ((fmt->buffers | SUPPORTED_FORMATS) == 0)
+   else if ((fmt->buffers & SUPPORTED_FORMATS) == 0)
reason = SRP_LOGIN_REJ_UNSUPPORTED_DESCRIPTOR_FMT;
 
if (vscsi->state == SRP_PROCESSING)
-- 
2.5.4 (Apple Git-61)

--
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 v1 1/3] Ibmvscsis: Properly deregister target sessions

2016-08-31 Thread Bryant G. Ly
The driver currently doesn't properly deregisters target sessions
completely, so this will address that.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Signed-off-by: Michael Cyr <mike...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index b29fef9..a515cdd 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1934,6 +1934,8 @@ static int ibmvscsis_drop_nexus(struct ibmvscsis_tport 
*tport)
/*
 * Release the SCSI I_T Nexus to the emulated ibmvscsis Target Port
 */
+   target_wait_for_sess_cmds(se_sess);
+   transport_deregister_session_configfs(se_sess);
transport_deregister_session(se_sess);
tport->ibmv_nexus = NULL;
kfree(nexus);
-- 
2.5.4 (Apple Git-61)

--
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 v1 0/3] Ibmvscsis fixes and code cleanup

2016-08-31 Thread Bryant G. Ly
Here are some small fixes and cleanups for Ibmvscsis Target Driver.
The only one of significance is the first one where there is a
possibility of a kernel crash due to inproper deregister of target
session since we didn't sync up work and didn't deregister_configfs.

Bryant G. Ly (3):
  Ibmvscsis: Properly deregister target sessions
  Ibmvscsis: Code cleanup of print statements
  Ibmvscsis: Fixed a bug reported by Dan Carpenter

 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 30 +-
 1 file changed, 5 insertions(+), 25 deletions(-)

-- 
2.5.4 (Apple Git-61)

--
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 -next] ibmvscsis: Use list_move_tail instead of list_del/list_add_tail

2016-08-26 Thread Bryant G. Ly
On 7/22/16, 9:03 AM, "Wei Yongjun"  wrote:

Using list_move_tail() instead of list_del() + list_add_tail().

Signed-off-by: Wei Yongjun 
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)


Looks good to me.


--
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 v9] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-07-20 Thread Bryant G. Ly
Hi Nic, 

< SNIP >

> Note the deletion of drivers/scsi/libsrp.c + include/scsi/libsrp.h has
> been dropped from the above commit, as it looks like they where changes
> specific to your local tree.

> Please have a quick look, and let me know if anything doesn't look
> right.

So the deletion is from James suggesting that I revert the deletion of libsrp 
to make
It clear that the code used to exist in the kernel and that im bringing it back 
for this driver.
Thus yeah you are right to not include it since those files don’t exist 
currently.

-Bryant

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




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


[PATCH] target: Unsupported SCSI Opcode Fix

2016-06-30 Thread Bryant G. Ly
If a Simple command is sent with a failure,
target_setup_cmd_from_cdb returns with TCM_UNSUPPORTED_SCSI_OPCODE
or TCM_INVALID_CDB_FIELD. So in the cases where target_setup_cmd_from_cdb
returns an error, we never get far enough to call target_execute_cmd to
increment simple_cmds. Since simple_cmds isn't incremented, the
result of the failure from target_setup_cmd_from_cdb causes
transport_generic_request_failure to decrement
simple_cmds, due to call to transport_complete_task_attr.
With this dev->simple_cmds or dev->dev_ordered_sync is now -1, not 0.
So when a subsequent command with an Ordered Task is sent, it causes
a hang, since dev->simple_cmds is at -1.

Signed-off-by: Michael Cyr <mike...@linux.vnet.ibm.com>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_transport.c | 6 ++
 include/target/target_core_base.h  | 3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 0a8f0a6..a2e447a 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1824,6 +1824,8 @@ static bool target_handle_task_attr(struct se_cmd *cmd)
if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
return false;
 
+   cmd->se_cmd_flags |= SCF_TASK_ATTR_SET;
+
/*
 * Check for the existence of HEAD_OF_QUEUE, and if true return 1
 * to allow the passed struct se_cmd list of tasks to the front of the 
list.
@@ -1946,6 +1948,9 @@ static void transport_complete_task_attr(struct se_cmd 
*cmd)
if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)
return;
 
+   if (!(cmd->se_cmd_flags & SCF_TASK_ATTR_SET))
+   goto restart;
+
if (cmd->sam_task_attr == TCM_SIMPLE_TAG) {
atomic_dec_mb(>simple_cmds);
dev->dev_cur_ordered_id++;
@@ -1963,6 +1968,7 @@ static void transport_complete_task_attr(struct se_cmd 
*cmd)
 dev->dev_cur_ordered_id);
}
 
+restart:
target_restart_delayed_cmds(dev);
 }
 
diff --git a/include/target/target_core_base.h 
b/include/target/target_core_base.h
index e5d0948..276f5a5 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -139,7 +139,8 @@ enum se_cmd_flags_table {
SCF_COMPARE_AND_WRITE_POST  = 0x0010,
SCF_PASSTHROUGH_PROT_SG_TO_MEM_NOALLOC = 0x0020,
SCF_ACK_KREF= 0x0040,
-   SCF_USE_CPUID   = 0x0080,
+   SCF_USE_CPUID   = 0x0080,
+   SCF_TASK_ATTR_SET   = 0x0100,
 };
 
 /* struct se_dev_entry->lun_flags and struct se_lun->lun_access */
-- 
2.5.4 (Apple Git-61)

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


Re: [PATCH v8] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-06-29 Thread Bryant G. Ly
On 6/29/16, 9:54 AM, "Bart Van Assche" <target-devel-ow...@vger.kernel.org on 
behalf of bart.vanass...@sandisk.com> wrote:

On 06/29/2016 04:43 PM, Bryant G. Ly wrote:
>> It looks like the masking fixes the device mapper’s ability to find the 
>> disk….

> Is the multipath client available in your initrd? If so, running 
> multipath -ll -v with n >= 3 should reveal why multipathd ignored 
> certain disks.

I enabled it on the initiator:

[root@vscsi-bry ~]# multipath -ll
[root@vscsi-bry ~]# multipath -ll -v with n >= 3
-bash: n: No such file or directory
[root@vscsi-bry ~]# mpathconf
multipath is enabled
find_multipaths is enabled
user_friendly_names is enabled
dm_multipath module is loaded
multipathd is running
[root@vscsi-bry ~]#

But it seems to not do much.

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




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


Re: [PATCH v8] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-06-29 Thread Bryant G. Ly


On 6/29/16, 12:49 AM, "Bart Van Assche"  wrote:

>> I was hoping someone in the community can help answer this:
>>
>> If I were to remove the masking off of the lun addressing method prior to 
>> target_submit_cmd/target_submit_tmr then I get a hang within scsi probe
>> on bootup. (srp->lun.scsi_lun[0] &= 0x3f; and srp_tsk->lun.scsi_lun[0] &= 
>> 0x3f;)
>>
>> [0.842648] ibmvscsi 300d: sent SRP login
>> [0.842667] ibmvscsi 300d: SRP_LOGIN succeeded
>> [0.842682] scsi 0:0:2:0: CD-ROMAIX  VOPTA
>>  PQ: 0 ANSI: 4
>> [  OK  ] Started Show Plymouth Boot Screen.
>> [  OK  ] Reached target Paths.
>> [  OK  ] Reached target Basic System.
>> [0.886179] sr 0:0:2:0: [sr0] scsi-1 drive
>> [0.886199] cdrom: Uniform CD-ROM driver Revision: 3.20
>> [0.888582] sd 0:0:1:0: [sda] 41943040 512-byte logical blocks: (21.4 
>> GB/20.0 GiB)
>> [0.888657] sd 0:0:1:0: [sda] Write Protect is off
>> [0.888712] sd 0:0:1:0: [sda] Cache data unavailable
>> [0.888722] sd 0:0:1:0: [sda] Assuming drive cache: write through
>> [0.901443]  sda: sda1 sda2 sda3
>> [0.901838] sd 0:0:1:0: [sda] Attached SCSI disk
>> [  124.522778] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
>> starting timeout scripts
>> [  125.151242] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
>> starting timeout scripts
>> [  125.662808] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
>> starting timeout scripts
>>
>> Does anyone know of the reasoning for having to mask off the addressing 
>> method prior
>> to submitting to target?

> How long have you waited before giving up waiting? The SCSI initiator 
> sends a REPORT LUN and other SCSI commands to LUN 0. If LUN 0 is missing 
> these commands will time out but that should not cause a hang. Anyway, 
> SysRq-w should help to determine the cause of the hang.

[0.852777] ibmvscsi 3002: SRP_LOGIN succeeded
[  OK  ] Reached target System Initialization.
 Starting dracut initqueue hook...
 Starting Show Plymouth Boot Screen...
[0.872421] ibmvscsi 300d: SRP_VERSION: 16.a
[0.872561] scsi 0:0:1:0: Direct-Access AIX  VDASD0001 
PQ: 0 ANSI: 3
[0.872635] scsi host1: IBM POWER Virtual SCSI Adapter 1.5.9
[0.872846] ibmvscsi 300d: partner initialization complete
[0.872897] ibmvscsi 300d: host srp version: 16.a, host partition  (0), 
OS 2, max io 8388608
[0.872917] ibmvscsi 300d: sent SRP login
[0.872937] ibmvscsi 300d: SRP_LOGIN succeeded
[0.872953] scsi 0:0:2:0: CD-ROMAIX  VOPTA 
PQ: 0 ANSI: 4
[  OK  ] Started Show Plymouth Boot Screen.
[  OK  ] Reached target Paths.
[  OK  ] Reached target Basic System.
[0.915670] sr 0:0:2:0: [sr0] scsi-1 drive
[0.915690] cdrom: Uniform CD-ROM driver Revision: 3.20
[0.917957] sd 0:0:1:0: [sda] 41943040 512-byte logical blocks: (21.4 
GB/20.0 GiB)
[0.918035] sd 0:0:1:0: [sda] Write Protect is off
[0.918092] sd 0:0:1:0: [sda] Cache data unavailable
[0.918104] sd 0:0:1:0: [sda] Assuming drive cache: write through
[0.929717]  sda: sda1 sda2 sda3
[0.930107] sd 0:0:1:0: [sda] Attached SCSI disk
[  124.662745] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  125.311128] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  125.822847] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  126.332803] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  126.842856] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  127.352800] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  127.862721] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  128.372729] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  128.882699] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  129.392709] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  129.902698] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  130.412724] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  130.922700] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  131.432729] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  131.942724] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  132.452690] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  132.962679] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting 

Re: [PATCH v8] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-06-28 Thread Bryant G. Ly


On 6/27/16, 3:17 PM, "Michael Cyr"  wrote:



+   cmd->se_cmd.tag = be64_to_cpu(srp->tag);
+
+   spin_lock_bh(>intr_lock);
+   list_add_tail(>list, >active_q);
+   spin_unlock_bh(>intr_lock);
+
+   srp->lun.scsi_lun[0] &= 0x3f;
+
+   pr_debug("calling submit_cmd, se_cmd %p, lun 0x%llx, cdb 0x%x, 
attr:%d\n",
+>se_cmd, scsilun_to_int(>lun), (int)srp->cdb[0],
+attr);
+
+   rc = target_submit_cmd(>se_cmd, nexus->se_sess, srp->cdb,
+  cmd->sense_buf, scsilun_to_int(>lun),
+  data_len, attr, dir, 0);
+   if (rc) {



+ * ibmvscsis_parse_task() - Parse SRP Task Management Request
+ *
+ * Parse the srp task management request; if it is valid then submit it to tcm.
+ * Note: The return code does not reflect the status of the task management
+ * request.
+ *
+ * EXECUTION ENVIRONMENT:
+ * Processor level
+ */
+static void ibmvscsis_parse_task(struct scsi_info *vscsi,
+struct ibmvscsis_cmd *cmd)
+{


+   spin_lock_bh(>intr_lock);
+   list_add_tail(>list, >active_q);
+   spin_unlock_bh(>intr_lock);
+
+   srp_tsk->lun.scsi_lun[0] &= 0x3f;
+
+   pr_debug("calling submit_tmr, func %d\n",
+srp_tsk->tsk_mgmt_func);
+   rc = target_submit_tmr(>se_cmd, nexus->se_sess, NULL,
+  scsilun_to_int(_tsk->lun), srp_tsk,
+  tcm_type, GFP_KERNEL, tag_to_abort, 0);
+   if (rc) {


I was hoping someone in the community can help answer this:

If I were to remove the masking off of the lun addressing method prior to 
target_submit_cmd/target_submit_tmr then I get a hang within scsi probe 
on bootup. (srp->lun.scsi_lun[0] &= 0x3f; and srp_tsk->lun.scsi_lun[0] &= 0x3f;)

[0.842648] ibmvscsi 300d: sent SRP login
[0.842667] ibmvscsi 300d: SRP_LOGIN succeeded
[0.842682] scsi 0:0:2:0: CD-ROMAIX  VOPTA 
PQ: 0 ANSI: 4
[  OK  ] Started Show Plymouth Boot Screen.
[  OK  ] Reached target Paths.
[  OK  ] Reached target Basic System.
[0.886179] sr 0:0:2:0: [sr0] scsi-1 drive
[0.886199] cdrom: Uniform CD-ROM driver Revision: 3.20
[0.888582] sd 0:0:1:0: [sda] 41943040 512-byte logical blocks: (21.4 
GB/20.0 GiB)
[0.888657] sd 0:0:1:0: [sda] Write Protect is off
[0.888712] sd 0:0:1:0: [sda] Cache data unavailable
[0.888722] sd 0:0:1:0: [sda] Assuming drive cache: write through
[0.901443]  sda: sda1 sda2 sda3
[0.901838] sd 0:0:1:0: [sda] Attached SCSI disk
[  124.522778] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  125.151242] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts
[  125.662808] dracut-initqueue[353]: Warning: dracut-initqueue timeout - 
starting timeout scripts

Does anyone know of the reasoning for having to mask off the addressing method 
prior
to submitting to target?

Thanks,

Bryant


--
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 v7] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-06-23 Thread Bryant G. Ly


On 6/23/16, 9:22 AM, "Christoph Hellwig" <target-devel-ow...@vger.kernel.org on 
behalf of h...@infradead.org> wrote:

>> -config SCSI_SRP
>> -tristate "SCSI RDMA Protocol helper library"
>> -depends on SCSI && PCI
>> -select SCSI_TGT
>> -help
>> -  If you wish to use SRP target drivers, say Y.
>> -
>> -  To compile this driver as a module, choose M here: the
>> -  module will be called libsrp.
>> -
>
>Please split that removal of libsrp into a separate patch before
>adding the new driver.

Why do you suggest splitting the path up for the libsrp stuff? The current 
upstream
does not contain libsrp. The only reason the patch shows that there is a 
removal of
libsrp is that James Bottomley suggested doing a revert of: 
libsrp: removal - commit f6667938cfceefd8afe6355ceb6497dce4883ae9
to make it clear that I’m bringing back what had existed a year ago within the 
upstream.

>
>> new file mode 100644
>> index 000..887574d
>> --- /dev/null
>> +++ b/drivers/scsi/ibmvscsi_tgt/Makefile
>> @@ -0,0 +1,4 @@
>> +obj-$(CONFIG_SCSI_IBMVSCSIS)+= ibmvscsis.o
>> +
>> +ibmvscsis-objs := libsrp.o ibmvscsi_tgt.o
>
>please use module-y for adding objects.  Also what's the reason
>for splitting these files?
>

I will change to module-y. The reason is we are possibly going to write
another target fabric in the future with the IBMVFC driver so just incase
we want to leave it separated for now. 

>> +/***
>> + * IBM Virtual SCSI Target Driver
>> + * Copyright (C) 2003-2005 Dave Boutcher (boutc...@us.ibm.com) IBM Corp.
>> + * Santiago Leon (san...@us.ibm.com) IBM Corp.
>> + * Linda Xie (l...@us.ibm.com) IBM Corp.
>> + *
>> + * Copyright (C) 2005-2011 FUJITA Tomonori <to...@acm.org>
>> + * Copyright (C) 2010 Nicholas A. Bellinger <n...@kernel.org>
>> + * Copyright (C) 2016 Bryant G. Ly <bryan...@linux.vnet.ibm.com> IBM Corp.
>> + *
>> + * Authors: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
>> + * Authors: Michael Cyr <mike...@linux.vnet.ibm.com>
>
>What's the reational for the copyright vs Authors lines?

No reason, ill remove the copyright. 

>
>> +#include "ibmvscsi_tgt.h"
>> +
>> +#ifndef H_GET_PARTNER_INFO
>> +#define H_GET_PARTNER_INFO  0x0008LL
>> +#endif
>
>Should this be in a header with the other hcalls?

Yes, Moved.

>
>> +static const char ibmvscsis_driver_name[] = "ibmvscsis";
>
>I think you can just assign the name directly in the driver ops
>structure.

Fixed.

>
>> +static const char ibmvscsis_workq_name[] = "ibmvscsis";
>
>This one seems unused.

Removed

>
>> +vscsi->flags &= (~PROCESSING_MAD);
>
>No need for the braces here. (ditto for many other places later on)
>
>> +static long ibmvscsis_parse_command(struct scsi_info *vscsi,
>> +struct viosrp_crq *crq);
>
>Can you avoid forward declarations where easily possible, and if not
>keep them all at the beginning of the file?

Will do.



>> +static int ibmvscsis_alloc_cmds(struct scsi_info *vscsi, int num)
>> +{
>> +struct ibmvscsis_cmd *cmd;
>> +int i;
>> +
>> +INIT_LIST_HEAD(>free_cmd);
>> +vscsi->cmd_pool = kcalloc(num, sizeof(struct ibmvscsis_cmd),
>> +  GFP_KERNEL);
>> +if (!vscsi->cmd_pool)
>> +return -ENOMEM;
>> +
>> +for (i = 0, cmd = (struct ibmvscsis_cmd *)vscsi->cmd_pool; i < num;
>> + i++, cmd++) {
>> +cmd->adapter = vscsi;
>> +INIT_WORK(>work, ibmvscsis_scheduler);
>> +list_add_tail(>list, >free_cmd);
>> +}
>> +
>> +return 0;
>> +}
>
>Why can't you use the existing infrastructure for cmd pools in the
>target core?
>

I wasn’t aware there was existing infrastructure.



>> +rc = srp_transfer_data(cmd, _iu(iue)->srp.cmd, ibmvscsis_rdma, 1,
>> +   1);
>> +if (rc) {
>> +pr_err("srp_transfer_data failed: %d\n", rc);
>> +sd = se_cmd->sense_buffer;
>> +se_cmd->scsi_sense_length = 18;
>> +memset(se_cmd->sense_buffer, 0, se_cmd->scsi_sense_length);
>> +/* Current error */
>> +sd[0] = 0x70;
>> +/* sense key = Medium Error */
>> +sd[2] = 3;
>> +  

Re: [PATCH v4] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-06-15 Thread Bryant G. Ly
On 6/14/16, 9:57 AM, "Christoph Hellwig"  wrote:

>On Tue, Jun 14, 2016 at 10:46:09AM +0200, Bart Van Assche wrote:
>> All what's needed is something like the (untested) patch below. As one
>> can see no new function pointers have been added to target_core_fabric_ops.
>> All that has been added are a few new data members. With the patch below
>> ibmvscsis_modify_std_inquiry() can be left out and the target core will
>> pick up the "IBM", "VOPTA", "3303 NVDISK" and "0001" strings through the
>> target_core_fabric_ops instance in the ibmvscsis template. I think this is
>> a much cleaner approach than keeping the ibmvscsis_modify_std_inquiry()
>> function ...
>
>If we really have to, yes.  But then again we manually allow overriding
>the values from configfs, and none of this seems to be a protocol
>requirement but just a workaround for a braindead AIX initiator.  So
>the right thing is documentation telling people how to modify their
>inquiry strings for AIX to work.
>

I can add documentation for modifying their inquiry strings, but the thing is
This is just to support older AIX clients who don’t want to update. We already
Have a working ODM patch to address this but we’d still like to support older
Customers who would like to use this driver with an AIX initiator(client).

Latest patch I have stripped everything out as per your request, which
Does work with a linux client.

-Bryant


--
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 v4] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-06-15 Thread Bryant G. Ly
On 6/14/16, 3:46 AM, "Bart Van Assche" <bart.vanass...@sandisk.com> wrote:

>On 06/14/2016 08:35 AM, Nicholas A. Bellinger wrote:
>> On Fri, 2016-06-10 at 12:05 -0700, Bart Van Assche wrote:
>>> On 06/09/2016 02:26 PM, Bryant G. Ly wrote:
>>>> +/**
>>>> + * ibmvscsis_modify_std_inquiry() - Modify STD Inquiry
>>>> + *
>>>> + * This function modifies the inquiry data prior to sending to initiator
>>>> + * so that we can support current AIX. Internally we are going to
>>>> + * add new ODM entries to support the emulation from LIO. This function
>>>> + * is temporary until those changes are done.
>>>> + */
>>>> +static void ibmvscsis_modify_std_inquiry(struct se_cmd *se_cmd)
>>>> +{
>>>> +  struct se_device *dev = se_cmd->se_dev;
>>>> +  u32 cmd_len = se_cmd->data_length;
>>>> +  u32 repl_len;
>>>> +  unsigned char *buf = NULL;
>>>> +
>>>> +  if (cmd_len <= 8)
>>>> +  return;
>>>> +
>>>> +  buf = transport_kmap_data_sg(se_cmd);
>>>> +  if (buf) {
>>>> +  repl_len = 8;
>>>> +  if (cmd_len - 8 < repl_len)
>>>> +  repl_len = cmd_len - 8;
>>>> +  memcpy([8], "IBM ", repl_len);
>>>> +
>>>> +  if (cmd_len > 16) {
>>>> +  repl_len = 16;
>>>> +  if (cmd_len - 16 < repl_len)
>>>> +  repl_len = cmd_len - 16;
>>>> +  if (dev->transport->get_device_type(dev) == TYPE_ROM)
>>>> +  memcpy([16], "VOPTA   ", repl_len);
>>>> +  else
>>>> +  memcpy([16], "3303  NVDISK", repl_len);
>>>> +  }
>>>> +  if (cmd_len > 32) {
>>>> +  repl_len = 4;
>>>> +  if (cmd_len - 32 < repl_len)
>>>> +  repl_len = cmd_len - 32;
>>>> +  memcpy([32], "0001", repl_len);
>>>> +  }
>>>> +  transport_kunmap_data_sg(se_cmd);
>>>> +  }
>>>> +}
>>>
>>> Christoph Hellwig had asked you *not* to modify the INQUIRY response 
>>> generated by the target core. Had you noticed that comment?
>> 
>> Yeah, so I wanted to avoid having to add a new target_core_fabric_ops
>> API caller just for this special case, but there might not be another
>> choice..
>
>All what's needed is something like the (untested) patch below. As one
>can see no new function pointers have been added to target_core_fabric_ops.
>All that has been added are a few new data members. With the patch below
>ibmvscsis_modify_std_inquiry() can be left out and the target core will
>pick up the "IBM", "VOPTA", "3303 NVDISK" and "0001" strings through the
>target_core_fabric_ops instance in the ibmvscsis template. I think this is
>a much cleaner approach than keeping the ibmvscsis_modify_std_inquiry()
>function ...
>
>Bart.
>
>[PATCH] target: Make INQUIRY vendor, product ID and product revision 
>configurable
>
>---
> drivers/target/target_core_configfs.c |  4 
> drivers/target/target_core_spc.c  | 22 +-
> include/target/target_core_fabric.h   |  6 ++
> 3 files changed, 27 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/target/target_core_configfs.c 
>b/drivers/target/target_core_configfs.c
>index 2001005..32a74f7 100644
>--- a/drivers/target/target_core_configfs.c
>+++ b/drivers/target/target_core_configfs.c
>@@ -442,6 +442,10 @@ static int target_fabric_tf_ops_check(const struct 
>target_core_fabric_ops *tfo)
>   pr_err("Missing tfo->fabric_drop_tpg()\n");
>   return -EINVAL;
>   }
>+  if (tfo->prod_id_cnt && !tfo->prod_id) {
>+  pr_err("Missing tfo->prod_id\n");
>+  return -EINVAL;
>+  }
>
>   return 0;
> }
>diff --git a/drivers/target/target_core_spc.c 
>b/drivers/target/target_core_spc.c
>index 2a91ed3..32c8435 100644
>--- a/drivers/target/target_core_spc.c
>+++ b/drivers/target/target_core_spc.c
>@@ -66,6 +66,9 @@ spc_emulate_inquiry_std(struct se_cmd *cmd, unsigned char 
>*buf)
>   struct se_lun *lun = cmd->se_lun;
>   struct se_device *dev = cmd->se_dev;
>   struct se_session *sess = cmd->

Re: [PATCH v4] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-06-15 Thread Bryant G. Ly
On 6/14/16, 3:09 AM, "Bart Van Assche" <bart.vanass...@sandisk.com> wrote:

>On 06/09/2016 11:26 PM, Bryant G. Ly wrote:
>> This driver is a pick up of the old IBM VIO scsi Target Driver
>> that was started by Nick and Fujita 2-4 years ago.
>> http://comments.gmane.org/gmane.linux.scsi/90119
>
>What kernel version is this patch based on? It doesn't apply cleanly on 
>top of kernel v4.7-rc1 nor on top of kernel v4.6.
>

It should be for 4.5, but latest version I based it off 4.6 stable. So you 
should look at that one.

-Bryant


--
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: Fix for hang of Ordered task in TCM

2016-06-08 Thread Bryant G. Ly
From: Nicholas Bellinger <n...@linux-iscsi.org>

If a command with a Simple task attribute is failed due to a Unit
Attention, then a subsequent command with an Ordered task attribute
will hang forever.  The reason for this is that the Unit Attention
status is checked for in target_setup_cmd_from_cdb, before the call
to target_execute_cmd, which calls target_handle_task_attr, which
in turn increments dev->simple_cmds.

However, transport_generic_request_failure still calls
transport_complete_task_attr, which will decrement dev->simple_cmds.
In this case, simple_cmds is now -1.  So when a command with the
Ordered task attribute is sent, target_handle_task_attr sees that
dev->simple_cmds is not 0, so it decides it can't execute the
command until all the (nonexistent) Simple commands have completed.

Reported-by: Michael Cyr <mike...@linux.vnet.ibm.com>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/target/target_core_internal.h  |  1 +
 drivers/target/target_core_sbc.c   |  2 +-
 drivers/target/target_core_transport.c | 62 +++---
 include/target/target_core_fabric.h|  1 -
 4 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/drivers/target/target_core_internal.h 
b/drivers/target/target_core_internal.h
index fc91e85..e2c970a 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -146,6 +146,7 @@ sense_reason_t  target_cmd_size_check(struct se_cmd 
*cmd, unsigned int size);
 void   target_qf_do_work(struct work_struct *work);
 bool   target_check_wce(struct se_device *dev);
 bool   target_check_fua(struct se_device *dev);
+void   __target_execute_cmd(struct se_cmd *, bool);
 
 /* target_core_stat.c */
 void   target_stat_setup_dev_default_groups(struct se_device *);
diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index a9057aa..04f616b 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -602,7 +602,7 @@ static sense_reason_t compare_and_write_callback(struct 
se_cmd *cmd, bool succes
cmd->transport_state |= CMD_T_ACTIVE|CMD_T_BUSY|CMD_T_SENT;
spin_unlock_irq(>t_state_lock);
 
-   __target_execute_cmd(cmd);
+   __target_execute_cmd(cmd, false);
 
kfree(buf);
return ret;
diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index e887635..7c4ea39 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1303,23 +1303,6 @@ target_setup_cmd_from_cdb(struct se_cmd *cmd, unsigned 
char *cdb)
 
trace_target_sequencer_start(cmd);
 
-   /*
-* Check for an existing UNIT ATTENTION condition
-*/
-   ret = target_scsi3_ua_check(cmd);
-   if (ret)
-   return ret;
-
-   ret = target_alua_state_check(cmd);
-   if (ret)
-   return ret;
-
-   ret = target_check_reservation(cmd);
-   if (ret) {
-   cmd->scsi_status = SAM_STAT_RESERVATION_CONFLICT;
-   return ret;
-   }
-
ret = dev->transport->parse_cdb(cmd);
if (ret == TCM_UNSUPPORTED_SCSI_OPCODE)
pr_warn_ratelimited("%s/%s: Unsupported SCSI Opcode 0x%02x, 
sending CHECK_CONDITION.\n",
@@ -1762,20 +1745,45 @@ queue_full:
 }
 EXPORT_SYMBOL(transport_generic_request_failure);
 
-void __target_execute_cmd(struct se_cmd *cmd)
+void __target_execute_cmd(struct se_cmd *cmd, bool do_checks)
 {
sense_reason_t ret;
 
-   if (cmd->execute_cmd) {
-   ret = cmd->execute_cmd(cmd);
-   if (ret) {
-   spin_lock_irq(>t_state_lock);
-   cmd->transport_state &= ~(CMD_T_BUSY|CMD_T_SENT);
-   spin_unlock_irq(>t_state_lock);
+   if (!cmd->execute_cmd) {
+   ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+   goto err;
+   }
+   if (do_checks) {
+   /*
+* Check for an existing UNIT ATTENTION condition after
+* target_handle_task_attr() has done SAM task attr
+* checking, and possibly have already defered execution
+* out to target_restart_delayed_cmds() context.
+*/
+   ret = target_scsi3_ua_check(cmd);
+   if (ret)
+   goto err;
+
+   ret = target_alua_state_check(cmd);
+   if (ret)
+   goto err;
 
-   transport_generic_request_failure(cmd, ret);
+   ret = target_check_reservation(cmd);
+   if (ret) {
+   cmd->scsi_status = SAM_STAT_RESERVATION_CONFLICT;
+   goto err;
}
}
+
+   ret = cmd->execute_cmd(cmd

[PATCH v3] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-05-27 Thread Bryant G. Ly
Version 1:
This initial commit contains WIP of the IBM VSCSI Target Fabric
Module. It currently supports read/writes, and I have tested
the ability to create a file backstore with the driver, install
RHEL, and then boot up the partition via filio backstore through
the driver.

This driver is a pick up of the old IBM VIO scsi Target Driver
that was started by Nick and Fujita 2-4 years ago.
http://comments.gmane.org/gmane.linux.scsi/90119
and http://marc.info/?t=12973408564=1=2

The driver provides a virtual SCSI device on IBM Power Servers.

When reviving the old libsrp, I stripped out all that utilized
scsi to submit commands to the target. Hence there is no more
scsi_tgt_if_*, and scsi_transport_* files and fully utilizes
LIO instead. This driver does however use the SRP protocol
for communication between guests/and or hosts, but its all
synchronous data transfers due to the utilization of
H_COPY_RDMA, a VIO mechanism which means that others like
ib_srp, ib_srpt which are asynchronous can't use this driver.
This was also the reason for moving libsrp out of the
drivers/scsi/. and into the ibmvscsi folder.

Signed-off-by: Steven Royer <sero...@linux.vnet.ibm.com>
Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 MAINTAINERS  |   10 +
 drivers/scsi/Kconfig |   27 +-
 drivers/scsi/Makefile|2 +-
 drivers/scsi/ibmvscsi/Makefile   |4 +
 drivers/scsi/ibmvscsi/ibmvscsi_tgt.c | 1932 ++
 drivers/scsi/ibmvscsi/ibmvscsi_tgt.h |  169 +++
 drivers/scsi/ibmvscsi/libsrp.c   |  386 +++
 drivers/scsi/ibmvscsi/libsrp.h   |   91 ++
 drivers/scsi/libsrp.c|  447 
 include/scsi/libsrp.h|   78 --
 10 files changed, 2610 insertions(+), 536 deletions(-)
 create mode 100644 drivers/scsi/ibmvscsi/ibmvscsi_tgt.c
 create mode 100644 drivers/scsi/ibmvscsi/ibmvscsi_tgt.h
 create mode 100644 drivers/scsi/ibmvscsi/libsrp.c
 create mode 100644 drivers/scsi/ibmvscsi/libsrp.h
 delete mode 100644 drivers/scsi/libsrp.c
 delete mode 100644 include/scsi/libsrp.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 9c567a4..0f412d4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5451,6 +5451,16 @@ S:   Supported
 F: drivers/scsi/ibmvscsi/ibmvscsi*
 F: drivers/scsi/ibmvscsi/viosrp.h
 
+IBM Power Virtual SCSI Device Target Driver
+M: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
+L: linux-scsi@vger.kernel.org
+L: target-de...@vger.kernel.org
+S: Supported
+F: drivers/scsi/ibmvscsi/ibmvscsis.c
+F: drivers/scsi/ibmvscsi/ibmvscsis.h
+F: drivers/scsi/ibmvscsi/libsrp.c
+F: drivers/scsi/ibmvscsi/libsrp.h
+
 IBM Power Virtual FC Device Drivers
 M: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
 L: linux-scsi@vger.kernel.org
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index c5883a5..0f8a1de 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -848,6 +848,23 @@ config SCSI_IBMVSCSI
  To compile this driver as a module, choose M here: the
  module will be called ibmvscsi.
 
+config SCSI_IBMVSCSIS
+   tristate "IBM Virtual SCSI Server support"
+   depends on PPC_PSERIES && TARGET_CORE && SCSI && PCI
+   help
+ This is the IBM POWER Virtual SCSI Target Server
+ This driver uses the SRP protocol for communication betwen servers
+ guest and/or the host that run on the same server.
+ More information on VSCSI protocol can be found at www.power.org
+
+ The userspace configuration needed to initialize the driver can be
+ be found here:
+
+ https://github.com/powervm/ibmvscsis/wiki/Configuration
+
+ To compile this driver as a module, choose M here: the
+ module will be called ibmvstgt.
+
 config SCSI_IBMVFC
tristate "IBM Virtual FC support"
depends on PPC_PSERIES && SCSI
@@ -1729,16 +1746,6 @@ config SCSI_PM8001
  This driver supports PMC-Sierra PCIE SAS/SATA 8x6G SPC 8001 chip
  based host adapters.
 
-config SCSI_SRP
-   tristate "SCSI RDMA Protocol helper library"
-   depends on SCSI && PCI
-   select SCSI_TGT
-   help
- If you wish to use SRP target drivers, say Y.
-
- To compile this driver as a module, choose M here: the
- module will be called libsrp.
-
 config SCSI_BFA_FC
tristate "Brocade BFA Fibre Channel Support"
depends on PCI && SCSI
diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
index 0335d28..ea2030c 100644
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -127,8 +127,8 @@ obj-$(CONFIG_SCSI_LASI700)  += 53c700.o lasi700.o
 obj-$(CONFIG_SCSI_SNI_53C710)  += 53c700.o sni_53c710.o
 obj-$(CONFIG_SCSI_NSP32)   += nsp32.o
 obj-$(CONFIG_SC

[PATCH v2] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-05-26 Thread Bryant G. Ly
This driver is a pick up of the old IBM VIO scsi Target Driver
that was started by Nick and Fujita 2-4 years ago.
http://comments.gmane.org/gmane.linux.scsi/90119
and http://marc.info/?t=12973408564=1=2

The driver provides a virtual SCSI device on IBM Power Servers.

When reviving the old libsrp, I stripped out all that utilized
scsi to submit commands to the target. Hence there is no more
scsi_tgt_if_*, and scsi_transport_* files and fully utilizes
LIO instead. This driver does however use the SRP protocol
for communication between guests/and or hosts, but its all
synchronous data transfers due to the utilization of
H_COPY_RDMA, a VIO mechanism which means that others like
ib_srp, ib_srpt which are asynchronous can't use this driver.
This was also the reason for moving libsrp out of the
drivers/scsi/. and into the ibmvscsi folder.

Version 1:
This initial commit contains WIP of the IBM VSCSI Target Fabric
Module. It currently supports read/writes, and I have tested
the ability to create a file backstore with the driver, install
RHEL, and then boot up the partition via filio backstore through
the driver.

Version 2:
Addressing Bart's Comments, contains cleaning up the code for styling
and also addresses Bart's comments.

Removed forward declarations and re-organizes thefunctions within
the driver. This patch also fixes MAINTAINERS for ibmvscsis.

Cleaned up indentations and a bug in send_adapter_info where on
the error case it wont free dma allocation.

Lastly, this disregards my previous post splitting up the changes
into patches, and makes them ammends with different versions of the
original patch. This patch also contains internal IBM sign offs.

Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
Signed-off-by: Steven Royer <sero...@linux.vnet.ibm.com>
Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 MAINTAINERS   |   10 +
 drivers/scsi/Kconfig  |   30 +
 drivers/scsi/Makefile |2 +
 drivers/scsi/ibmvscsi/Makefile|2 +
 drivers/scsi/ibmvscsi/ibmvscsis.c | 1932 +
 drivers/scsi/ibmvscsi/ibmvscsis.h |  150 +++
 drivers/scsi/ibmvscsi/libsrp.c|  386 
 drivers/scsi/ibmvscsi/libsrp.h|   91 ++
 8 files changed, 2603 insertions(+)
 create mode 100644 drivers/scsi/ibmvscsi/ibmvscsis.c
 create mode 100644 drivers/scsi/ibmvscsi/ibmvscsis.h
 create mode 100644 drivers/scsi/ibmvscsi/libsrp.c
 create mode 100644 drivers/scsi/ibmvscsi/libsrp.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 6ee06ea..3f09a15 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5381,6 +5381,16 @@ S:   Supported
 F: drivers/scsi/ibmvscsi/ibmvscsi*
 F: drivers/scsi/ibmvscsi/viosrp.h
 
+IBM Power Virtual SCSI Device Target Driver
+M: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
+L: linux-scsi@vger.kernel.org
+L: target-de...@vger.kernel.org
+S: Supported
+F: drivers/scsi/ibmvscsi/ibmvscsis.c
+F: drivers/scsi/ibmvscsi/ibmvscsis.h
+F: drivers/scsi/ibmvscsi/libsrp.c
+F: drivers/scsi/ibmvscsi/libsrp.h
+
 IBM Power Virtual FC Device Drivers
 M: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
 L: linux-scsi@vger.kernel.org
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index e2f31c9..f03c5a7 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -847,6 +847,23 @@ config SCSI_IBMVSCSI
  To compile this driver as a module, choose M here: the
  module will be called ibmvscsi.
 
+config SCSI_IBMVSCSIS
+   tristate "IBM Virtual SCSI Server support"
+   depends on PPC_PSERIES && SCSI_SRP && TARGET_CORE
+   help
+ This is the IBM POWER Virtual SCSI Target Server
+ This driver uses the SRP protocol for communication betwen servers
+ guest and/or the host that run on the same server.
+ More information on VSCSI protocol can be found at www.power.org
+
+ The userspace configuration needed to initialize the driver can be
+ be found here:
+
+ https://github.com/powervm/ibmvscsis/wiki/Configuration
+
+ To compile this driver as a module, choose M here: the
+ module will be called ibmvstgt.
+
 config SCSI_IBMVFC
tristate "IBM Virtual FC support"
depends on PPC_PSERIES && SCSI
@@ -1728,6 +1745,19 @@ config SCSI_PM8001
  This driver supports PMC-Sierra PCIE SAS/SATA 8x6G SPC 8001 chip
  based host adapters.
 
+config SCSI_SRP
+   tristate "SCSI RDMA Protocol helper library"
+   depends on SCSI && PCI
+   help
+ This SCSI SRP module is a library for ibmvscsi target driver.
+ This module can only be used by SRP drivers that utilize synchronous
+ data transfers and not by SRP drivers that use asynchronous.
+
+ If you wish to use SRP target drivers, say Y.
+
+ To compile this driver as a module, choose M here.

[PATCH 2/3] ibmvscsis: Addressing Bart's comments

2016-05-25 Thread Bryant G. Ly
From: bryantly 

This patch contains cleaning up the code for styling and also addresses Bart's 
comments.

Signed-off-by: bryantly 
---
 MAINTAINERS   |   4 +-
 drivers/scsi/Kconfig  |  36 +--
 drivers/scsi/Makefile |   4 +-
 drivers/scsi/ibmvscsi/Makefile|   3 +-
 drivers/scsi/ibmvscsi/ibmvscsis.c | 450 ++
 drivers/scsi/ibmvscsi/ibmvscsis.h |  40 ++--
 drivers/scsi/ibmvscsi/libsrp.c| 386 
 drivers/scsi/ibmvscsi/libsrp.h|  91 
 drivers/scsi/libsrp.c | 387 
 include/scsi/libsrp.h |  95 
 10 files changed, 737 insertions(+), 759 deletions(-)
 create mode 100644 drivers/scsi/ibmvscsi/libsrp.c
 create mode 100644 drivers/scsi/ibmvscsi/libsrp.h
 delete mode 100644 drivers/scsi/libsrp.c
 delete mode 100644 include/scsi/libsrp.h

diff --git a/MAINTAINERS b/MAINTAINERS
index b520e6c..a11905c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5387,9 +5387,9 @@ L:linux-scsi@vger.kernel.org
 L: target-de...@vger.kernel.org
 S: Supported
 F: drivers/scsi/ibmvscsi/ibmvscsis.c
-F:  drivers/scsi/ibmvscsi/ibmvscsis.h
+F: drivers/scsi/ibmvscsi/ibmvscsis.h
 F: drivers/scsi/libsrp.h
-F:  drivers/scsi/libsrp.c
+F: drivers/scsi/libsrp.c
 
 IBM Power Virtual FC Device Drivers
 M: Tyrel Datwyler 
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 6adf8f1..1221f6b 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -848,18 +848,21 @@ config SCSI_IBMVSCSI
  module will be called ibmvscsi.
 
 config SCSI_IBMVSCSIS
-   tristate "IBM Virtual SCSI Server support"
-   depends on PPC_PSERIES && SCSI_SRP && TARGET_CORE
-   help
- This is the IBM POWER Virtual SCSI Target Server
+   tristate "IBM Virtual SCSI Server support"
+   depends on PPC_PSERIES && SCSI_SRP && TARGET_CORE
+   help
+ This is the IBM POWER Virtual SCSI Target Server
+ This driver uses the SRP protocol for communication betwen servers
+ guest and/or the host that run on the same server.
+ More information on VSCSI protocol can be found at www.power.org
 
-  The userspace component needed to initialize the driver and
- documentation can be found:
+ The userspace component needed to initialize the driver and
+ documentation can be found:
 
-  https://github.com/powervm/ibmvscsis
+ https://github.com/powervm/ibmvscsis
 
-  To compile this driver as a module, choose M here: the
- module will be called ibmvstgt.
+ To compile this driver as a module, choose M here: the
+ module will be called ibmvstgt.
 
 config SCSI_IBMVFC
tristate "IBM Virtual FC support"
@@ -1743,14 +1746,17 @@ config SCSI_PM8001
  based host adapters.
 
 config SCSI_SRP
-   tristate "SCSI RDMA Protocol helper library"
-   depends on SCSI && PCI
-   help
- This scsi srp module is a library for ibmvscsi target driver.
+   tristate "SCSI RDMA Protocol helper library"
+   depends on SCSI && PCI
+   help
+ This SCSI SRP module is a library for ibmvscsi target driver.
+ This module can only be used by SRP drivers that utilize synchronous
+ data transfers and not by SRP drivers that use asynchronous.
+
  If you wish to use SRP target drivers, say Y.
 
- To compile this driver as a module, choose M here. The module will
- be called libsrp.
+ To compile this driver as a module, choose M here. The module will
+ be called libsrp.
 
 config SCSI_BFA_FC
tristate "Brocade BFA Fibre Channel Support"
diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
index 8692dd4..9dfa4da 100644
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -127,9 +127,9 @@ obj-$(CONFIG_SCSI_LASI700)  += 53c700.o lasi700.o
 obj-$(CONFIG_SCSI_SNI_53C710)  += 53c700.o sni_53c710.o
 obj-$(CONFIG_SCSI_NSP32)   += nsp32.o
 obj-$(CONFIG_SCSI_IPR) += ipr.o
-obj-$(CONFIG_SCSI_SRP)  += libsrp.o
+obj-$(CONFIG_SCSI_SRP) += ibmvscsi/
 obj-$(CONFIG_SCSI_IBMVSCSI)+= ibmvscsi/
-obj-$(CONFIG_SCSI_IBMVSCSIS)+= ibmvscsi/
+obj-$(CONFIG_SCSI_IBMVSCSIS)   += ibmvscsi/
 obj-$(CONFIG_SCSI_IBMVFC)  += ibmvscsi/
 obj-$(CONFIG_SCSI_HPTIOP)  += hptiop.o
 obj-$(CONFIG_SCSI_STEX)+= stex.o
diff --git a/drivers/scsi/ibmvscsi/Makefile b/drivers/scsi/ibmvscsi/Makefile
index b241a567..e7a1663 100644
--- a/drivers/scsi/ibmvscsi/Makefile
+++ b/drivers/scsi/ibmvscsi/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_SCSI_IBMVSCSI)+= ibmvscsi.o
-obj-$(CONFIG_SCSI_IBMVSCSIS)+= ibmvscsis.o
+obj-$(CONFIG_SCSI_IBMVSCSIS)   += ibmvscsis.o
 obj-$(CONFIG_SCSI_IBMVFC)  += ibmvfc.o
+obj-$(CONFIG_SCSI_SRP)

[PATCH 3/3] ibmvscsis: clean up functions

2016-05-25 Thread Bryant G. Ly
From: bryantly 

This patch removes forward declarations and re-organizes the
functions within the driver. This patch also fixes MAINTAINERS
for ibmvscsis.

Signed-off-by: bryantly 
---
 MAINTAINERS   |4 +-
 drivers/scsi/ibmvscsi/ibmvscsis.c | 2709 ++---
 2 files changed, 1315 insertions(+), 1398 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index a11905c..3f09a15 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5388,8 +5388,8 @@ L:target-de...@vger.kernel.org
 S: Supported
 F: drivers/scsi/ibmvscsi/ibmvscsis.c
 F: drivers/scsi/ibmvscsi/ibmvscsis.h
-F: drivers/scsi/libsrp.h
-F: drivers/scsi/libsrp.c
+F: drivers/scsi/ibmvscsi/libsrp.c
+F: drivers/scsi/ibmvscsi/libsrp.h
 
 IBM Power Virtual FC Device Drivers
 M: Tyrel Datwyler 
diff --git a/drivers/scsi/ibmvscsi/ibmvscsis.c 
b/drivers/scsi/ibmvscsi/ibmvscsis.c
index 6f9e9ba..610f4f5 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsis.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsis.c
@@ -62,88 +62,17 @@
 
 #define SRP_RSP_SENSE_DATA_LEN 18
 
+static const char ibmvscsis_driver_name[] = "ibmvscsis";
+static char system_id[SYS_ID_NAME_LEN] = "";
+static char partition_name[PARTITION_NAMELEN] = "UNKNOWN";
+static unsigned int partition_number = -1;
+
 static struct workqueue_struct *vtgtd;
 static unsigned max_vdma_size = MAX_H_COPY_RDMA;
 
 static DEFINE_SPINLOCK(ibmvscsis_dev_lock);
 static LIST_HEAD(ibmvscsis_dev_list);
 
-static int ibmvscsis_probe(struct vio_dev *vdev,
-  const struct vio_device_id *id);
-static void ibmvscsis_dev_release(struct device *dev);
-static void ibmvscsis_modify_rep_luns(struct se_cmd *se_cmd);
-static void ibmvscsis_modify_std_inquiry(struct se_cmd *se_cmd);
-static int read_dma_window(struct vio_dev *vdev,
-  struct ibmvscsis_adapter *adapter);
-static char *ibmvscsis_get_fabric_name(void);
-static char *ibmvscsis_get_fabric_wwn(struct se_portal_group *se_tpg);
-static u16 ibmvscsis_get_tag(struct se_portal_group *se_tpg);
-static u32 ibmvscsis_get_default_depth(struct se_portal_group *se_tpg);
-static int ibmvscsis_check_true(struct se_portal_group *se_tpg);
-static int ibmvscsis_check_false(struct se_portal_group *se_tpg);
-static u32 ibmvscsis_tpg_get_inst_index(struct se_portal_group *se_tpg);
-static int ibmvscsis_check_stop_free(struct se_cmd *se_cmd);
-static void ibmvscsis_release_cmd(struct se_cmd *se_cmd);
-static int ibmvscsis_shutdown_session(struct se_session *se_sess);
-static void ibmvscsis_close_session(struct se_session *se_sess);
-static u32 ibmvscsis_sess_get_index(struct se_session *se_sess);
-static int ibmvscsis_write_pending(struct se_cmd *se_cmd);
-static int ibmvscsis_write_pending_status(struct se_cmd *se_cmd);
-static void ibmvscsis_set_default_node_attrs(struct se_node_acl *nacl);
-static int ibmvscsis_get_cmd_state(struct se_cmd *se_cmd);
-static int ibmvscsis_queue_data_in(struct se_cmd *se_cmd);
-static int ibmvscsis_queue_status(struct se_cmd *se_cmd);
-static void ibmvscsis_queue_tm_rsp(struct se_cmd *se_cmd);
-static void ibmvscsis_aborted_task(struct se_cmd *se_cmd);
-static struct se_wwn *ibmvscsis_make_tport(struct target_fabric_configfs *tf,
-  struct config_group *group,
-  const char *name);
-static void ibmvscsis_drop_tport(struct se_wwn *wwn);
-static struct se_portal_group *ibmvscsis_make_tpg(struct se_wwn *wwn,
- struct config_group *group,
- const char *name);
-static void ibmvscsis_drop_tpg(struct se_portal_group *se_tpg);
-static int ibmvscsis_remove(struct vio_dev *vdev);
-static ssize_t system_id_show(struct device *dev,
- struct device_attribute *attr,
- char *buf);
-static ssize_t partition_number_show(struct device *dev,
-struct device_attribute *attr,
-char *buf);
-static ssize_t unit_address_show(struct device *dev,
-struct device_attribute *attr, char *buf);
-static int get_system_info(void);
-static irqreturn_t ibmvscsis_interrupt(int dummy, void *data);
-static int process_srp_iu(struct iu_entry *iue);
-static void process_iu(struct viosrp_crq *crq,
-  struct ibmvscsis_adapter *adapter);
-static void process_crq(struct viosrp_crq *crq,
-   struct ibmvscsis_adapter *adapter);
-static void handle_crq(struct work_struct *work);
-static int ibmvscsis_reset_crq_queue(struct ibmvscsis_adapter *adapter);
-static void crq_queue_destroy(struct ibmvscsis_adapter *adapter);
-static inline struct viosrp_crq *next_crq(struct crq_queue *queue);
-static int send_iu(struct iu_entry *iue, u64 length, u8 

IBM VSCSI Target Driver Initial Patch Sets

2016-05-25 Thread Bryant G. Ly
This patch series addresses comments by Joe with runing checkpatch with a
--strict. It cleans up all the misc styling and removes all the forward
declarations.

The patch also addresses all of Bart's comments besides the preallocation of
buffers before IO starts and the merging of unpack_lun with scsiluntoint.
Those will come in later patch sets.

[PATCH 2/3] ibmvscsis: Addressing Bart's comments
[PATCH 3/3] ibmvscsis: clean up functions

--
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] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-05-24 Thread Bryant G Ly


Quoting Bart Van Assche <bart.vanass...@sandisk.com>:


On 05/24/2016 06:52 AM, Bryant G. Ly wrote:

+config SCSI_IBMVSCSIS
+   tristate "IBM Virtual SCSI Server support"
+   depends on PPC_PSERIES && SCSI_SRP && TARGET_CORE
+   help
+ This is the IBM POWER Virtual SCSI Target Server
+
+  The userspace component needed to initialize the driver and
+ documentation can be found:
+
+  https://github.com/powervm/ibmvscsis
+
+  To compile this driver as a module, choose M here: the
+ module will be called ibmvstgt.
+


This driver has confused Linux users before. Most people know SRP as  
a SCSI transport layer that is used for communication between  
servers. This driver however uses the SRP protocol for communication  
between guests and/or the host that run on the same server. Please  
add this information to the above help text, together with a  
reference to the VIO protocol documentation in the POWER  
architecture manual.




Done


+#include 


Every Linux user can query the kernel version by running the uname  
command. Is it really useful to print the kernel version  
(UTS_RELEASE) when this driver is loaded?




Done


+#include "ibmvscsi.h"


The above include directive indirectly includes a whole bunch of  
SCSI initiator driver header files. We do not want that initiator  
header files are included in the source code of a target driver. If  
it is necessary to share definitions of symbolic constants between  
the initiator and the target driver please move these into a new  
header file.




Done


+static inline long h_send_crq(struct ibmvscsis_adapter *adapter,
+   u64 word1, u64 word2)
+{
+   long rc;
+   struct vio_dev *vdev = adapter->dma_dev;
+
+   pr_debug("ibmvscsis: ibmvscsis_send_crq(0x%x, 0x%016llx, 0x%016llx)\n",
+   vdev->unit_address, word1, word2);
+


As Joe Perches already asked, please define pr_fmt() instead of  
including the kernel module name in every pr_debug() statement.




Done


+static char system_id[64] = "";
+static char partition_name[97] = "UNKNOWN";


Where do these magic constants come from and what is their meaning?  
Please consider to introduce symbolic names for these constants.




Done


+static ssize_t ibmvscsis_tpg_enable_show(struct config_item *item,
+   char *page)
+{
+   struct se_portal_group *se_tpg = to_tpg(item);
+   struct ibmvscsis_tport *tport = container_of(se_tpg,
+   struct ibmvscsis_tport, se_tpg);
+
+   return snprintf(page, PAGE_SIZE, "%d\n", (tport->enabled) ? 1 : 0);
+}


Have you considered to use MODULE_VERSION() instead of introducing a  
sysfs attribute to export the module version? An advantage of  
MODULE_VERSION() is that it allows to query the module version  
without having to load a kernel module.




Done


+static void ibmvscsis_modify_std_inquiry(struct se_cmd *se_cmd)
+{
+   struct se_device *dev = se_cmd->se_dev;
+   unsigned char *buf = NULL;
+   u32 cmd_len = se_cmd->data_length;
+
+   if (cmd_len <= INQ_DATA_OFFSET)
+   return;
+
+   buf = transport_kmap_data_sg(se_cmd);
+   if (buf) {
+   memcpy([8], "IBM", 8);
+   if (dev->transport->get_device_type(dev) == TYPE_ROM)
+   memcpy([16], "VOPTA   ", 16);
+   else
+   memcpy([16], "3303  NVDISK", 16);
+   memcpy([32], "0001", 4);
+   transport_kunmap_data_sg(se_cmd);
+   }
+}


Shouldn't a sense code be returned to the initiator if this function fails?



This function never fails? Also this is a temporary function that will go away
soon. It's to address AIX not recognizing target emulation of inquiry, but
we have already discussed internally to add these ODM entries into AIX.


+   default:
+   pr_err("ibmvscsis: unknown task mgmt func %d\n",
+   srp_tsk->tsk_mgmt_func);
+   cmd->se_cmd.se_tmr_req->response =
+   TMR_TASK_MGMT_FUNCTION_NOT_SUPPORTED;
+   rc = -1;
+   break;
+   }
+
+   if (!rc) {


Please consider changing the above "break" into "goto fail" such  
that the if (!rc) can be left out and such that the indentation of  
the code below if (!rc) can be reduced.




Done


+static int ibmvscsis_queuecommand(struct ibmvscsis_adapter *adapter,
+ struct iu_entry *iue)
+{
+   struct srp_cmd *cmd = iue->sbuf->buf;
+   struct scsi_cmnd *sc;
+   struct ibmvscsis_cmnd *vsc;
+   int ret;
+
+   pr_debug("ibmvscsis: ibmvscsis_queuec

[PATCH] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-05-24 Thread Bryant G. Ly
From: bgly <b...@us.ibm.com>

This initial commit contains WIP of the IBM VSCSI Target Fabric
Module. It currently supports read/writes, and I have tested
the ability to create a file backstore with the driver and install
RHEL VIA NIM and then boot up the partition via filio backstore
through the driver.

Signed-off-by: bryantly <bryan...@linux.vnet.ibm.com>
---
 MAINTAINERS   |   10 +
 drivers/scsi/Kconfig  |   24 +
 drivers/scsi/Makefile |2 +
 drivers/scsi/ibmvscsi/Makefile|1 +
 drivers/scsi/ibmvscsi/ibmvscsis.c | 2033 +
 drivers/scsi/ibmvscsi/ibmvscsis.h |  160 +++
 drivers/scsi/libsrp.c |  387 +++
 include/scsi/libsrp.h |   95 ++
 8 files changed, 2712 insertions(+)
 create mode 100644 drivers/scsi/ibmvscsi/ibmvscsis.c
 create mode 100644 drivers/scsi/ibmvscsi/ibmvscsis.h
 create mode 100644 drivers/scsi/libsrp.c
 create mode 100644 include/scsi/libsrp.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 6ee06ea..b520e6c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5381,6 +5381,16 @@ S:   Supported
 F: drivers/scsi/ibmvscsi/ibmvscsi*
 F: drivers/scsi/ibmvscsi/viosrp.h
 
+IBM Power Virtual SCSI Device Target Driver
+M:     Bryant G. Ly <bryan...@linux.vnet.ibm.com>
+L: linux-scsi@vger.kernel.org
+L: target-de...@vger.kernel.org
+S: Supported
+F: drivers/scsi/ibmvscsi/ibmvscsis.c
+F:  drivers/scsi/ibmvscsi/ibmvscsis.h
+F: drivers/scsi/libsrp.h
+F:  drivers/scsi/libsrp.c
+
 IBM Power Virtual FC Device Drivers
 M: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
 L: linux-scsi@vger.kernel.org
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index e2f31c9..6adf8f1 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -847,6 +847,20 @@ config SCSI_IBMVSCSI
  To compile this driver as a module, choose M here: the
  module will be called ibmvscsi.
 
+config SCSI_IBMVSCSIS
+   tristate "IBM Virtual SCSI Server support"
+   depends on PPC_PSERIES && SCSI_SRP && TARGET_CORE
+   help
+ This is the IBM POWER Virtual SCSI Target Server
+
+  The userspace component needed to initialize the driver and
+ documentation can be found:
+
+  https://github.com/powervm/ibmvscsis
+
+  To compile this driver as a module, choose M here: the
+ module will be called ibmvstgt.
+
 config SCSI_IBMVFC
tristate "IBM Virtual FC support"
depends on PPC_PSERIES && SCSI
@@ -1728,6 +1742,16 @@ config SCSI_PM8001
  This driver supports PMC-Sierra PCIE SAS/SATA 8x6G SPC 8001 chip
  based host adapters.
 
+config SCSI_SRP
+   tristate "SCSI RDMA Protocol helper library"
+   depends on SCSI && PCI
+   help
+ This scsi srp module is a library for ibmvscsi target driver.
+ If you wish to use SRP target drivers, say Y.
+
+ To compile this driver as a module, choose M here. The module will
+ be called libsrp.
+
 config SCSI_BFA_FC
tristate "Brocade BFA Fibre Channel Support"
depends on PCI && SCSI
diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
index 862ab4e..8692dd4 100644
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -127,7 +127,9 @@ obj-$(CONFIG_SCSI_LASI700)  += 53c700.o lasi700.o
 obj-$(CONFIG_SCSI_SNI_53C710)  += 53c700.o sni_53c710.o
 obj-$(CONFIG_SCSI_NSP32)   += nsp32.o
 obj-$(CONFIG_SCSI_IPR) += ipr.o
+obj-$(CONFIG_SCSI_SRP)  += libsrp.o
 obj-$(CONFIG_SCSI_IBMVSCSI)+= ibmvscsi/
+obj-$(CONFIG_SCSI_IBMVSCSIS)+= ibmvscsi/
 obj-$(CONFIG_SCSI_IBMVFC)  += ibmvscsi/
 obj-$(CONFIG_SCSI_HPTIOP)  += hptiop.o
 obj-$(CONFIG_SCSI_STEX)+= stex.o
diff --git a/drivers/scsi/ibmvscsi/Makefile b/drivers/scsi/ibmvscsi/Makefile
index 3840c64..b241a567 100644
--- a/drivers/scsi/ibmvscsi/Makefile
+++ b/drivers/scsi/ibmvscsi/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_SCSI_IBMVSCSI)+= ibmvscsi.o
+obj-$(CONFIG_SCSI_IBMVSCSIS)+= ibmvscsis.o
 obj-$(CONFIG_SCSI_IBMVFC)  += ibmvfc.o
diff --git a/drivers/scsi/ibmvscsi/ibmvscsis.c 
b/drivers/scsi/ibmvscsi/ibmvscsis.c
new file mode 100644
index 000..c7eb347
--- /dev/null
+++ b/drivers/scsi/ibmvscsi/ibmvscsis.c
@@ -0,0 +1,2033 @@
+/***
+ * IBM Virtual SCSI Target Driver
+ * Copyright (C) 2003-2005 Dave Boutcher (boutc...@us.ibm.com) IBM Corp.
+ *Santiago Leon (san...@us.ibm.com) IBM Corp.
+ *Linda Xie (l...@us.ibm.com) IBM Corp.
+ *
+ * Copyright (C) 2005-2011 FUJITA Tomonori <to...@acm.org>
+ * Copyright (C) 2010 Nicholas A. Bellinger <n...@kernel.org>
+ * Copyright (C) 2016 Bryant G. Ly <b...@us.ibm.com> IBM Corp.
+ *
+ * Authors: Bryant G. Ly <bry

Re: [PATCH] Fix for hang of Ordered task in TCM

2016-05-23 Thread Bryant G Ly


Quoting "Nicholas A. Bellinger" :




So AFAICT for delayed commands, the above patch ends up skipping these
three checks subsequently when doing __target_execute_cmd() directly
from target_restart_delayed_cmds(), no..?

After pondering this some more, what about moving these checks into
__target_execute_cmd() to handle both target_core_transport.c cases
instead..?

We'll also need a parameter for internal COMPARE_AND_WRITE usage
within compare_and_write_callback(), to bypass checks upon secondary
->execute_cmd() WRITE payload submission after READ + COMPARE has
completed successfully.

WDYT..?

diff --git a/drivers/target/target_core_internal.h  
b/drivers/target/target_core_internal.h

index fc91e85..e2c970a 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -146,6 +146,7 @@ sense_reason_t	target_cmd_size_check(struct  
se_cmd *cmd, unsigned int size);

 void   target_qf_do_work(struct work_struct *work);
 bool   target_check_wce(struct se_device *dev);
 bool   target_check_fua(struct se_device *dev);
+void   __target_execute_cmd(struct se_cmd *, bool);

 /* target_core_stat.c */
 void   target_stat_setup_dev_default_groups(struct se_device *);
diff --git a/drivers/target/target_core_sbc.c  
b/drivers/target/target_core_sbc.c

index a9057aa..04f616b 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -602,7 +602,7 @@ static sense_reason_t  
compare_and_write_callback(struct se_cmd *cmd, bool succes

cmd->transport_state |= CMD_T_ACTIVE|CMD_T_BUSY|CMD_T_SENT;
spin_unlock_irq(>t_state_lock);

-   __target_execute_cmd(cmd);
+   __target_execute_cmd(cmd, false);

kfree(buf);
return ret;
diff --git a/drivers/target/target_core_transport.c  
b/drivers/target/target_core_transport.c

index 6c089af..f3e93dd 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1761,20 +1761,45 @@ queue_full:
 }
 EXPORT_SYMBOL(transport_generic_request_failure);

-void __target_execute_cmd(struct se_cmd *cmd)
+void __target_execute_cmd(struct se_cmd *cmd, bool do_checks)
 {
sense_reason_t ret;

-   if (cmd->execute_cmd) {
-   ret = cmd->execute_cmd(cmd);
-   if (ret) {
-   spin_lock_irq(>t_state_lock);
-   cmd->transport_state &= ~(CMD_T_BUSY|CMD_T_SENT);
-   spin_unlock_irq(>t_state_lock);
+   if (!cmd->execute_cmd) {
+   ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+   goto err;
+   }
+   if (do_checks) {
+   /*
+* Check for an existing UNIT ATTENTION condition after
+* target_handle_task_attr() has done SAM task attr
+* checking, and possibly have already defered execution
+* out to target_restart_delayed_cmds() context.
+*/
+   ret = target_scsi3_ua_check(cmd);
+   if (ret)
+   goto err;

-   transport_generic_request_failure(cmd, ret);
+   ret = target_alua_state_check(cmd);
+   if (ret)
+   goto err;
+
+   ret = target_check_reservation(cmd);
+   if (ret) {
+   cmd->scsi_status = SAM_STAT_RESERVATION_CONFLICT;
+   goto err;
}
}
+
+   ret = cmd->execute_cmd(cmd);
+   if (!ret)
+   return;
+err:
+   spin_lock_irq(>t_state_lock);
+   cmd->transport_state &= ~(CMD_T_BUSY|CMD_T_SENT);
+   spin_unlock_irq(>t_state_lock);
+
+   transport_generic_request_failure(cmd, ret);
 }

 static int target_write_prot_action(struct se_cmd *cmd)
@@ -1899,7 +1924,7 @@ void target_execute_cmd(struct se_cmd *cmd)
return;
}

-   __target_execute_cmd(cmd);
+   __target_execute_cmd(cmd, true);
 }
 EXPORT_SYMBOL(target_execute_cmd);

@@ -1923,7 +1948,7 @@ static void target_restart_delayed_cmds(struct  
se_device *dev)

list_del(>se_delayed_node);
spin_unlock(>delayed_cmd_lock);

-   __target_execute_cmd(cmd);
+   __target_execute_cmd(cmd, true);

if (cmd->sam_task_attr == TCM_ORDERED_TAG)
break;
diff --git a/include/target/target_core_fabric.h  
b/include/target/target_core_fabric.h

index ec79da3..334f107 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -163,7 +163,6 @@ int	core_tmr_alloc_req(struct se_cmd *, void *,  
u8, gfp_t);

 void   core_tmr_release_req(struct se_tmr_req *);
 inttransport_generic_handle_tmr(struct se_cmd *);
 void   transport_generic_request_failure(struct se_cmd *, sense_reason_t);
-void   __target_execute_cmd(struct se_cmd *);
 inttransport_lookup_tmr_lun(struct se_cmd *, u64);
 void