Re: [PATCH v2] target/file: add support of direct and async I/O
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" >> Cc: Christoph Hellwig >> Cc: Bryant G. Ly >> Tested-by: Bryant G. Ly >> Signed-off-by: Andrei Vagin >> --- >> 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, 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 * dev->dev_attrib.block_size; >> +aio_cmd->iocb.ki_filp = file; >> +aio_cmd->iocb.ki_complete = cmd_rw_
[PATCH v2] target: Fix Fortify_panic kernel exception
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 Reviewed-by: Steven Royer Tested-by: Taylor Jakobson Cc: Christoph Hellwig Cc: Nicholas Bellinger Cc: --- 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 = &cmd->t_data_sg[0]; - unsigned char *buf, zero = 0x00, *p = &zero; - 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
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
[ 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 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 Reviewed-by: Steven Royer Tested-by: Taylor Jakobson Cc: Christoph Hellwig Cc: Nicholas Bellinger Cc: --- 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_cor
Re: [PATCH v2] target/file: add support of direct and async I/O
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, &aio_cmd->iocb, &iter); >> +else >> +ret = call_read_iter(file, &aio_cmd->iocb, &iter); >> + >> +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 > 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 -Bryant
Re: [PATCH] [RFC] target/file: add support of direct and async I/O
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" > Signed-off-by: Andrei Vagin > --- > 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 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
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
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 --- 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(&args[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(&opts, ":")) != NULL) { + if (!*ptr) + continue; + + token = match_token(ptr, tokens, args); + switch (token) { + case Opt_dev_config: + if (match_strlcpy(udev->dev_config, &args[0], + TCMU_CONFIG_LEN) == 0) { + pr_err("Could not reconfigure dev_config"); + } + ret = tcmu_update_uio_info
[PATCH] ibmvscsis: add DRC indices to debug statements
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 Signed-off-by: Brad Warrum --- 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(&vscsi->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(&vscsi->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(&vscsi->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(&vscsi->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(&vscsi->active_q), -(int)list_empty(&vscsi->schedule_q)); + dev_dbg(&vscsi->dev, "disconnect start wait, active %d, sched %d\n", + (int)list_empty(&vscsi->active_q), + (int)list_empty(&vscsi->schedule_q)); if (!list_empty(&vscsi->active_q) || !list_empty(&vscsi->schedule_q)) { vscsi->flags |= WAIT_FOR_IDLE; - pr_debug("disconnect flags 0x%x\n", vscsi->flags); + dev_dbg(&vscsi->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(&vscsi->wait_idle); spin_lock_bh(&vscsi->intr_lock); } - pr_debug("disconnect stop wait\n"); + dev_dbg(&vscsi->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(&vscsi->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,
Re: [PATCH 2/8] target: remove iblock WRITE_SAME passthrough support
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 = &cmd->t_data_sg[0]; > - struct page *page = NULL; > - int ret; > + unsigned char *buf, zero = 0x00, *p = &zero; > + 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 nf_nat_masquerade_ipv4 iptable_nat
Re: [PATCH] tcmu: clean up the scatter helper
> 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 >>>> >>>> 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. >> > 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()
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
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
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
> From: Nicholas Bellinger > > 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 > Cc: Bryant G. Ly > Cc: Mike Christie > Cc: Hannes Reinecke > Signed-off-by: Nicholas Bellinger > --- > Reviewed-by: Bryant G. Ly
Re: [PATCH 3/6] target: Fix quiese during transport_write_pending_qf endless loop
> From: Nicholas Bellinger > > 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 > Cc: Hannes Reinecke > Cc: Bryant G. Ly > Cc: Michael Cyr > Cc: Potnuri Bharat Teja > Cc: Sagi Grimberg > Signed-off-by: Nicholas Bellinger > --- > Reviewed-by: Bryant G. Ly
Re: [PATCH] ibmvscsis: Fix write_pending failure path
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
From: "Bryant G. Ly" 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 --- 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, &vio_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
From: Xiubo Li 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 Tested-by: Damien Le Moal Signed-off-by: Xiubo Li --- 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 -Bryant
Re: [PATCH 0/5] target: Zoned block device support and bug fixes
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
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
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" Cc: Michael Cyr Cc: "James E.J. Bottomley" Cc: "Martin K. Petersen" Cc: Cc: Signed-off-by: Greg Kroah-Hartman --- 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
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 Signed-off-by: Mike Christie --- 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, &tpgt); + if (rc) + return ERR_PTR(rc); + tport->tport_tpgt = tpgt; + tport->releasing = false; rc = core_tpg_register(&tport->tport_wwn, &tport->se_tpg, -- 2.5.4 (Apple Git-61)
[PATCH v4 3/5] tcmu: Make dev_size configurable via userspace
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 --- 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, &val); + 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[] = { + &tcmu_attr_cmd_time_out, + &tcmu_attr_dev_size, + &tcmu_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] = &tcmu_attr_cmd_time_out; - i++; - tcmu_attrs[i] = &tcmu_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(&tcmu_ops); -- 2.5.4 (Apple Git-61)
[PATCH v4 5/5] tcmu: Add Type of reconfig into netlink
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 --- 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(&tcmu_genl_family, skb, 0, @@ -1301,7 +1306,7 @@ static int tcmu_configure_device(struct se_device *dev) kref_get(&udev->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(&udev->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
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 --- 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[] = { &tcmu_attr_cmd_time_out, + &tcmu_attr_dev_path, &tcmu_attr_dev_size, &tcmu_attr_emulate_write_cache, NULL, -- 2.5.4 (Apple Git-61)
[PATCH v4 2/5] tcmu: Add netlink for device reconfiguration
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 Reviewed-By: Mike Christie --- 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
From: "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. 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
This will enable the toggling of write_cache in tcmu through targetcli-fb Signed-off-by: Bryant G. Ly Reviewed-By: Mike Christie --- 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, &val); + 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] = &tcmu_attr_cmd_time_out; + i++; + tcmu_attrs[i] = &tcmu_attr_emulate_write_cache; tcmu_ops.tb_dev_attrib_attrs = tcmu_attrs; ret = transport_backend_register(&tcmu_ops); -- 2.5.4 (Apple Git-61)
[PATCH v3 4/5] tcmu: Make dev_config configurable
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 --- 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[] = { &tcmu_attr_cmd_time_out, + &tcmu_attr_dev_path, &tcmu_attr_dev_size, &tcmu_attr_emulate_write_cache, NULL, -- 2.5.4 (Apple Git-61)
[PATCH v2 1/5] tcmu: Support emulate_write_cache
This will enable the toggling of write_cache in tcmu through targetcli-fb Signed-off-by: Bryant G. Ly Reviewed-By: Mike Christie --- 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, &val); + 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] = &tcmu_attr_cmd_time_out; + i++; + tcmu_attrs[i] = &tcmu_attr_emulate_write_cache; tcmu_ops.tb_dev_attrib_attrs = tcmu_attrs; ret = transport_backend_register(&tcmu_ops); -- 2.5.4 (Apple Git-61)
[PATCH v2 5/5] tcmu: Add Type of reconfig into netlink
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 --- 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(&tcmu_genl_family, skb, 0, @@ -1301,7 +1313,7 @@ static int tcmu_configure_device(struct se_device *dev) kref_get(&udev->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(&udev->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
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
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 Reviewed-By: Mike Christie --- 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
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 --- 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[] = { &tcmu_attr_cmd_time_out, + &tcmu_attr_dev_path, &tcmu_attr_dev_size, &tcmu_attr_emulate_write_cache, NULL, -- 2.5.4 (Apple Git-61)
[PATCH v2 3/5] tcmu: Make dev_size configurable via userspace
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 --- 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, &val); + 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[] = { + &tcmu_attr_cmd_time_out, + &tcmu_attr_dev_size, + &tcmu_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] = &tcmu_attr_cmd_time_out; - i++; - tcmu_attrs[i] = &tcmu_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(&tcmu_ops); -- 2.5.4 (Apple Git-61)
[PATCH v1 4/4] tcmu: Make dev_config configurable
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 --- 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[] = { &tcmu_attr_cmd_time_out, + &tcmu_attr_dev_path, &tcmu_attr_dev_size, &tcmu_attr_emulate_write_cache, NULL, -- 2.5.4 (Apple Git-61)
[PATCH v1 1/4] tcmu: Support emulate_write_cache
This will enable the toggling of write_cache in tcmu through targetcli-fb Signed-off-by: Bryant G. Ly Reviewed-By: Mike Christie --- 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, &val); + 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] = &tcmu_attr_cmd_time_out; + i++; + tcmu_attrs[i] = &tcmu_attr_emulate_write_cache; tcmu_ops.tb_dev_attrib_attrs = tcmu_attrs; ret = transport_backend_register(&tcmu_ops); -- 2.5.4 (Apple Git-61)
[PATCH v1 3/4] tcmu: Make dev_size configurable via userspace
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 --- 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, &val); + 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[] = { + &tcmu_attr_cmd_time_out, + &tcmu_attr_dev_size, + &tcmu_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] = &tcmu_attr_cmd_time_out; - i++; - tcmu_attrs[i] = &tcmu_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(&tcmu_ops); -- 2.5.4 (Apple Git-61)
[PATCH v1 0/4] TCMU Enable Reconfiguration Patches
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
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 Reviewed-By: Mike Christie --- 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
From: Michael Cyr 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 Signed-off-by: Bryant G. Ly --- 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(&vscsi->intr_lock); + vscsi->flags &= ~PREP_FOR_SUSPEND_FLAGS; + spin_unlock_bh(&vscsi->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"); + } + + vscsi->flags &= ~(PREP_FOR_
Re: [PATCH v1] ibmvscsis: Fix cleaning up pointers
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 Reviewed-by: Michael Cyr Cc: # 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
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 Reviewed-by: Michael Cyr Cc: # 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(&cmd->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
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 Reviewed-by: Michael Cyr Cc: # 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(&vscsi->intr_lock); + vscsi->credit += 1; + spin_unlock_bh(&vscsi->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
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 Reviewed-by: Michael Cyr Cc: # 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(&vscsi->free_cmd, struct ibmvscsis_cmd, list); if (cmd) { + if (cmd->abort_cmd) + cmd->abort_cmd = NULL; cmd->flags &= ~(DELAY_SEND); list_del(&cmd->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
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: # v4.8+ Signed-off-by: Bryant G. Ly Signed-off-by: Michael Cyr --- 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(&vscsi->free_cmd, struct ibmvscsis_cmd, list); if (cmd) { + if (cmd->abort_cmd) + cmd->abort_cmd = NULL; + cmd->flags &= ~(DELAY_SEND); list_del(&cmd->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 *)&msg_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, &vscsi->waiting_rsp, list) { - iue = cmd->iue; + do { + retry = false; + list_for_each_entry_safe(cmd, nxt, &vscsi->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(&cmd->list); + ibmvscsis_free_cmd_resources(vscsi, +
[PATCH v3] ibmvscsis: Do not send aborted task response
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: # v4.8+ Signed-off-by: Bryant G. Ly Signed-off-by: Michael Cyr --- 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(&vscsi->free_cmd, struct ibmvscsis_cmd, list); if (cmd) { + cmd->flags &= ~(DELAY_SEND); list_del(&cmd->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 *)&msg_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, &vscsi->waiting_rsp, list) { - iue = cmd->iue; + do { + retry = false; + list_for_each_entry_safe(cmd, nxt, &vscsi->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(&cmd->list); + ibmvscsis_free_cmd_resources(vscsi, +cmd); + } else { + iue = cmd->iue; - crq->IU_length = cpu_to_be16(cmd->rsp.len); + crq->valid
Re: [PATCH v2] ibmvscsis: Do not send aborted task response
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: # v4.8+ Signed-off-by: Bryant G. Ly Reviewed-by: Tyrel Datwyler --- 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
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: # v4.8+ Signed-off-by: Bryant G. Ly Reviewed-by: Tyrel Datwyler --- 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
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: # v4.8+ Signed-off-by: Bryant G. Ly Reviewed-by: Tyrel Datwyler --- 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
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: # v4.8+ Signed-off-by: Bryant G. Ly Reviewed-by: Tyrel Datwyler --- 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, &vscsi->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(&cmd->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(&cmd->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(&cmd->list); + + ibmvscsis_free_cmd_resources(vscsi, +cmd); + } else { + srp_snd_msg_failed(vscsi, rc); + break; + } } } @@ -3581,9 +3594,20 @@ static int ibmvscsis_write_pending(struct se_cmd *se_c
[PATCH v3] target/user: PGR Support
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 --- 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->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
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 --- 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->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
This patch addresses clients who needs write_verify_16 for large volume groups such as AIX. Signed-off-by: Bryant G. Ly --- 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, §ors, &size); 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
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: # v4.8+ Signed-off-by: Bryant G. Ly --- 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, &vscsi->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(&cmd->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(&cmd->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(&cmd->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; + /* +* If CLIENT_F
Re: [PATCH] ibmvscsis: Do not send aborted task response
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
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: # v4.8+ Signed-off-by: Bryant G. Ly --- 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(&vscsi->free_cmd, struct ibmvscsis_cmd, list); if (cmd) { + cmd->flags &= ~(CMD_ABORTED); list_del(&cmd->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, &vscsi->waiting_rsp, list) { - iue = cmd->iue; + /* +* If an Abort flag is set then dont send response +*/ + if (cmd->flags & CMD_ABORTED) { + list_del(&cmd->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(&cmd->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(&cmd->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
[PATCH] ibmvscsis: Do not send aborted task response
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: # v4.8+ Signed-off-by: Bryant G. Ly --- 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(&vscsi->free_cmd, struct ibmvscsis_cmd, list); if (cmd) { + cmd->flags &= ~(CMD_ABORTED); list_del(&cmd->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, &vscsi->waiting_rsp, list) { - iue = cmd->iue; + /* +* If an Abort flag is set then dont send response +*/ + if (cmd->flags & CMD_ABORTED) { + list_del(&cmd->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(&cmd->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(&cmd->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; + if
[PATCH] target/user: PGR Support
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 --- 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->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/drivers/target/target_core_pr.c index e180
Re: [PATCHv4 0/4] tcmu: bug fix and dynamic growing data area support
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
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, &udev->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(&udev->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, &base_command_size, &command_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[]
On 3/8/17 2:45 AM, lixi...@cmss.chinamobile.com wrote: From: Xiubo Li 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 --- 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
Re: [PATCH] target: Fix NULL dereference during LUN lookup + active I/O shutdown
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] target: Add WRITE_VERIFY_16 Support
For clients who want to utilize more than 2GB you need WRITE AND VERIFY 16 support. Reported-by: Michael Cyr Signed-off-by: Bryant G. Ly --- drivers/target/target_core_sbc.c | 1 + include/scsi/scsi_proto.h| 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index 04f616b..e26acf2 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -918,6 +918,7 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops) cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB; cmd->execute_cmd = sbc_execute_rw; break; + case WRITE_VERIFY_16: case WRITE_16: sectors = transport_get_sectors_16(cdb); cmd->t_task_lba = transport_lba_64(cdb); diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h index d1defd1..b39c321 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) -- 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] target: Add WRITE_VERIFY_16 Support
On Fri, 2017-01-27 at 14:49 -0600, Bryant G. Ly wrote: For clients who want to utilize more than 2GB you need WRITE AND VERIFY 16 support. Hello Bryant, This patch introduces a bug for WRITE AND VERIFY(16) parsing that already exists for WRITE AND VERIFY(10) parsing, namely that the value of the BYTCHK field is ignored. Is it OK for you to wait a few weeks with adding WRITE AND VERIFY(16) support and to come back to this after I have posted my WRITE AND VERIFY(10) fix? Bart. Sounds good, thanks. I'll have to look at your patch sets that fixes that bug. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ibmvscsis: Fix max transfer length
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 Tested-by: Steven Royer Signed-off-by: Bryant G. Ly --- 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(&info->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
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 Tested-by: Steven Royer Signed-off-by: Bryant G. Ly --- 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(&info->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
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 Tested-by: Steven Royer Signed-off-by: Bryant G. Ly --- 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(&info->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
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 Tested-by: Steven Royer Signed-off-by: Bryant G. Ly --- 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(&info->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
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 Reviewed-by: Michael Cyr Signed-off-by: Bryant G. Ly --- 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(&vscsi->dma_dev->dev, sizeof(*info), &token, - GFP_KERNEL); + GFP_ATOMIC); if (!info) { dev_err(&vscsi->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(&vscsi->dma_dev->dev, olen, &token, -GFP_KERNEL); +GFP_ATOMIC); if (!cap) { dev_err(&vscsi->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
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 Tested-by: Steven Royer Signed-off-by: Bryant G. Ly --- 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(&info->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
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" 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 Tested-by: Steven Royer Signed-off-by: Bryant G. Ly --- 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
From: "Bryant G. Ly" 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 Tested-by: Steven Royer Signed-off-by: Bryant G. Ly --- 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
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
Signed-off-by: Bryant G. Ly --- 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
Signed-off-by: Bryant G. Ly Signed-off-by: Michael Cyr --- 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, &vscsi->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", -&cmd->se_cmd, scsilun_to_int(&srp->lun), (int)srp->cdb[0], -attr); - rc = target_submit_cmd(&cmd->se_cmd, nexus->se_sess, srp->cdb, cmd->sense_buf, scsilun_to_int(&srp->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(&vscsi->intr_lock); /* Remove from active_q */ list_del(&cmd->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, &vio_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, &vio_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
SUPPORTED_FORMATS is 1 << 1 so it's never zero. Signed-off-by: Bryant G. Ly Signed-off-by: Michael Cyr --- 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
The driver currently doesn't properly deregisters target sessions completely, so this will address that. Signed-off-by: Bryant G. Ly Signed-off-by: Michael Cyr --- 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
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
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
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
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 Signed-off-by: Bryant G. Ly --- 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(&dev->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
On 6/29/16, 9:54 AM, "Bart Van Assche" 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
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 timeout scripts [ 133.472687] dracut-initqueue[353]: Warning: dracut-initqueue
Re: [PATCH v8] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver
On 6/27/16, 3:17 PM, "Michael Cyr" wrote: + cmd->se_cmd.tag = be64_to_cpu(srp->tag); + + spin_lock_bh(&vscsi->intr_lock); + list_add_tail(&cmd->list, &vscsi->active_q); + spin_unlock_bh(&vscsi->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", +&cmd->se_cmd, scsilun_to_int(&srp->lun), (int)srp->cdb[0], +attr); + + rc = target_submit_cmd(&cmd->se_cmd, nexus->se_sess, srp->cdb, + cmd->sense_buf, scsilun_to_int(&srp->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(&vscsi->intr_lock); + list_add_tail(&cmd->list, &vscsi->active_q); + spin_unlock_bh(&vscsi->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(&cmd->se_cmd, nexus->se_sess, NULL, + scsilun_to_int(&srp_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
On 6/23/16, 9:22 AM, "Christoph Hellwig" 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 >> + * Copyright (C) 2010 Nicholas A. Bellinger >> + * Copyright (C) 2016 Bryant G. Ly IBM Corp. >> + * >> + * Authors: Bryant G. Ly >> + * Authors: Michael Cyr > >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(&vscsi->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(&cmd->work, ibmvscsis_scheduler); >> +list_add_tail(&cmd->list, &vscsi->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, &vio_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; >> +/* additional length (length - 8) */ >> +sd[7] = 10; >> +/* asc/ascq 0x801 = Logical Unit C
Re: [PATCH v4] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver
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
On 6/14/16, 3:46 AM, "Bart Van Assche" 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(&buf[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(&buf[16], "VOPTA ", repl_len); >>>> + else >>>> + memcpy(&buf[16], "3303 NVDISK", repl_len); >>>> + } >>>> + if (cmd_len > 32) { >>>> + repl_len = 4; >>>> + if (cmd_len - 32 < repl_len) >>>> + repl_len = cmd_len - 32; >>>> + memcpy(&buf[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-&g
Re: [PATCH v4] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver
On 6/14/16, 3:09 AM, "Bart Van Assche" 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
From: Nicholas Bellinger 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 Signed-off-by: Nicholas Bellinger Signed-off-by: Bryant G. Ly --- 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(&cmd->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(&cmd->t_state_lock); - cmd->transport_state &= ~(CMD_T_BUSY|CMD_T_SENT); - spin_unlock_irq(&cmd->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); + if (!ret) + return; +err: + spin_lock_i
[PATCH v3] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver
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&r=1&w=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 Signed-off-by: Tyrel Datwyler Signed-off-by: Bryant G. Ly --- 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 +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 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_SCSI_IPR) += ipr.o -obj-$(CONFIG_SCSI_SRP) += libsrp.o obj-$(CONFIG_SCSI_IBMVSCSI)+= ibmvscsi/ +obj-$(CONFIG_SCSI_IBMVSCSIS) += ib
[PATCH v2] ibmvscsis: Initial commit of IBM VSCSI Tgt 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&r=1&w=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 Signed-off-by: Steven Royer Signed-off-by: Tyrel Datwyler --- 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 +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 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. The module will + be called libsrp. + config SCSI_BFA_FC tristate "Brocade BFA Fibre Channel Support"
[PATCH 2/3] ibmvscsis: Addressing Bart's comments
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) += libsrp.o diff --git a/drivers/scsi/ibmvscsi/ibmvscsis.c b/drivers/scsi/ibmvscsi
[PATCH 3/3] ibmvscsis: clean up functions
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 format); -static int send_adapter_info(struct iu_entry *iue, -
IBM VSCSI Target Driver Initial Patch Sets
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
Quoting Bart Van Assche : 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(&buf[8], "IBM", 8); + if (dev->transport->get_device_type(dev) == TYPE_ROM) + memcpy(&buf[16], "VOPTA ", 16); + else + memcpy(&buf[16], "3303 NVDISK", 16); + memcpy(&buf[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_queue
[PATCH] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver
From: bgly 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 --- 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 +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 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 + * Copyright (C) 2010 Nicholas A. Bellinger + * Copyright (C) 2016 Bryant G. Ly IBM Corp. + * + * Authors: Bryant G. Ly + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2
Re: [PATCH] Fix for hang of Ordered task in TCM
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(&cmd->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(&cmd->t_state_lock); - cmd->transport_state &= ~(CMD_T_BUSY|CMD_T_SENT); - spin_unlock_irq(&cmd->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(&cmd->t_state_lock); + cmd->transport_state &= ~(CMD_T_BUSY|CMD_T_SENT); + spin_unlock_irq(&cmd->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(&cmd->se_delayed_node); spin_unlock(&dev->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 core_allo