- 06 May, 2017 1 commit
-
-
Bryant G. Ly authored
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: <stable@vger.kernel.org> # v4.8+ Signed-off-by:
Bryant G. Ly <bryantly@linux.vnet.ibm.com> Signed-off-by:
Michael Cyr <mikecyr@linux.vnet.ibm.com> Signed-off-by:
Nicholas Bellinger <nab@linux-iscsi.org>
-
- 05 May, 2017 4 commits
-
-
Mike Christie authored
We need to do a kthread_should_stop to check when kthread_stop has been called. This was a regression added in b6df4b79 tcmu: Add global data block pool support so not sure if you wanted to merge it in with that patch or what. Signed-off-by:
Mike Christie <mchristi@redhat.com> Signed-off-by:
-
Nicholas Bellinger authored
Keeping in the idempotent nature of target_core_fabric_configfs.c, if a queue_depth value is set and it's the same as the existing value, don't attempt to force session reinstatement. Reported-by:
Raghu Krishnamurthy <rk@datera.io> Cc: Raghu Krishnamurthy <rk@datera.io> Tested-by:
Gary Guo <ghg@datera.io> Cc: Gary Guo <ghg@datera.io> Signed-off-by:
-
Nicholas Bellinger authored
While testing modification of per se_node_acl queue_depth forcing session reinstatement via lio_target_nacl_cmdsn_depth_store() -> core_tpg_set_initiator_node_queue_depth(), a hung task bug triggered when changing cmdsn_depth invoked session reinstatement while an iscsi login was already waiting for session reinstatement to complete. This can happen when an outstanding se_cmd descriptor is taking a long time to complete, and session reinstatement from iscsi login or cmdsn_depth change occurs concurrently. To address this bug, explicitly set session_fall_back_to_erl0 = 1 when forcing session reinstatement, so session reinstatement is not attempted if an active session is already being shutdown. This patch has been tested with two scenarios. The first when iscsi login is blocked waiting for iscsi session reinstatement to complete followed by queue_depth change via configfs, and second when queue_depth change via configfs us blocked followed by a iscsi login driven session reinstatement. Note this patch depends on commit d36ad77f to handle multiple sessions per se_node_acl when changing cmdsn_depth, and for pre v4.5 kernels will need to be included for stable as well. Reported-by:
-
Nicholas Bellinger authored
Following the bugfix for handling non SAM_STAT_GOOD COMPARE_AND_WRITE status during COMMIT phase in commit 9b2792c3, the same bug exists for the READ phase as well. This would manifest first as a lost SCSI response, and eventual hung task during fabric driver logout or re-login, as existing shutdown logic waited for the COMPARE_AND_WRITE se_cmd->cmd_kref to reach zero. To address this bug, compare_and_write_callback() has been changed to set post_ret = 1 and return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE as necessary to signal failure status. Reported-by:
Bill Borsari <wgb@datera.io> Cc: Bill Borsari <wgb@datera.io> Tested-by:
-
- 03 May, 2017 1 commit
-
-
Xiubo Li authored
For the "struct tcmu_cmd_entry" in cmd area, the minimum size will be sizeof(struct tcmu_cmd_entry) == 112 Bytes. And it could fill about (sizeof(struct rsp) - sizeof(struct req)) / sizeof(struct iovec) == 68 / 16 ~= 4 data regions(iov[4]) by default. For most tcmu_cmds, the data block indexes allocated from the data area will be continuous. And for the continuous blocks they will be merged into the same region using only one iovec. For the current code, it will always allocates the same number of iovecs with blocks for each tcmu_cmd, and it will wastes much memories. For example, when the block size is 4K and the DATA_OUT buffer size is 64K, and the regions needed is less than 5(on my environment is almost 99.7%). The current code will allocate about 16 iovecs, and there will be (16 - 4) * sizeof(struct iovec) = 192 Bytes cmd area memories wasted. Here adds two helpers to calculate the base size and full size of the tcmu_cmd. And will recalculate them again when it make sure how many iovs is needed before insert it to cmd area. Signed-off-by:
Xiubo Li <lixiubo@cmss.chinamobile.com> Acked-by:
-
- 02 May, 2017 20 commits
-
-
Xiubo Li authored
For each target there will be one ring, when the target number grows larger and larger, it could eventually runs out of the system memories. In this patch for each target ring, currently for the cmd area the size will be fixed to 8MB and for the data area the size will grow from 0 to max 256K * PAGE_SIZE(1G for 4K page size). For all the targets' data areas, they will get empty blocks from the "global data block pool", which has limited to 512K * PAGE_SIZE(2G for 4K page size) for now. When the "global data block pool" has been used up, then any target could wake up the unmap thread routine to shrink other targets' data area memories. And the unmap thread routine will always try to truncate the ring vma from the last using block offset. When user space has touched the data blocks out of tcmu_cmd iov[], the tcmu_page_fault() will try to return one zeroed blocks. Here we move the timeout's tcmu_handle_completions() into unmap thread routine, that's to say when the timeout fired, it will only do the tcmu_check_expired_cmd() and then wake up the unmap thread to do the completions() and then try to shrink its idle memories. Then the cmdr_lock could be a mutex and could simplify this patch because the unmap_mapping_range() or zap_* may go to sleep. Signed-off-by:
Jianfei Hu <hujianfei@cmss.chinamobile.com> Acked-by:
-
Xiubo Li authored
Currently for the TCMU, the ring buffer size is fixed to 64K cmd area + 1M data area, and this will be bottlenecks for high iops. The struct tcmu_cmd_entry {} size is fixed about 112 bytes with iovec[N] & N <= 4, and the size of struct iovec is about 16 bytes. If N == 0, the ratio will be sizeof(cmd entry) : sizeof(datas) == 112Bytes : (N * 4096)Bytes = 28 : 0, no data area is need. If 0 < N <=4, the ratio will be sizeof(cmd entry) : sizeof(datas) == 112Bytes : (N * 4096)Bytes = 28 : (N * 1024), so the max will be 28 : 1024. If N > 4, the sizeof(cmd entry) will be [(N - 4) *16 + 112] bytes, and its corresponding data size will be [N * 4096], so the ratio of sizeof(cmd entry) : sizeof(datas) == [(N - 4) * 16 + 112)Bytes : (N * 4096)Bytes == 4/1024 - 12/(N * 1024), so the max is about 4 : 1024. When N is bigger, the ratio will be smaller. As the initial patch, we will set the cmd area size to 2M, and the cmd area size to 32M. The TCMU will dynamically grows the data area from 0 to max 32M size as needed. The cmd area memory will be allocated through vmalloc(), and the data area's blocks will be allocated individually later when needed. The allocated data area block memory will be managed via radix tree. For now the bitmap still be the most efficient way to search and manage the block index, this could be update later. Signed-off-by: -
Hannes Reinecke authored
When setting up an ALUA target port group with an invalid ID the error message kstrtoul() returned -22 for tg_pt_gp_id is displayed, which is not really helpful. Convert it to something sane. And while we're at it, join the messages onto a single line. Signed-by:
Hannes Reinecke <hare@suse.com> Reviewed-by:
Bart van Assche <bart.vanassche@sandisk.com> Signed-off-by:
-
Hannes Reinecke authored
When setting up a target the error message: Unable to do set ##_name ALUA state on non valid tg_pt_gp ID: 0 is displayed. Apparently concatenation doesn't work in a string; one should be using implicit string concatenation here. Signed-off-by:
-
Bryant G. Ly authored
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 authored
This patch addresses clients who needs write_verify_16 for large volume groups such as AIX. Signed-off-by:
-
Christophe Vu-Brugier authored
The script illustrates how to interact with configfs to create a very simple LIO iSCSI target with a file or block device backstore. The script can serve as a starting point for people that cannot use targetcli because Python is not available on their machine. Signed-off-by:
Christophe Vu-Brugier <cvubrugier@fastmail.fm> Signed-off-by:
Vincent Donnefort <vdonnefort@gmail.com> Signed-off-by:
-
Markus Elfring authored
A multiplication for the size determination of a memory allocation indicated that an array data structure should be processed. Thus use the corresponding function "kmalloc_array". This issue was detected by using the Coccinelle software. Signed-off-by:
Markus Elfring <elfring@users.sourceforge.net> Signed-off-by:
-
Markus Elfring authored
* A multiplication for the size determination of a memory allocation indicated that an array data structure should be processed. Thus use the corresponding function "kmalloc_array". This issue was detected by using the Coccinelle software. * Replace the specification of a data structure by a pointer dereference to make the corresponding size determination a bit safer according to the Linux coding style convention. Signed-off-by:
-
Markus Elfring authored
Replace the specification of two data structures by pointer dereferences as the parameter for the operator "sizeof" to make the corresponding size determinations a bit safer according to the Linux coding style convention. Signed-off-by:
-
Markus Elfring authored
The script "checkpatch.pl" pointed information out like the following. WARNING: Possible unnecessary 'out of memory' message Thus remove such statements here. Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdfSigned-off-by:
-
Markus Elfring authored
* Multiplications for the size determination of memory allocations indicated that array data structures should be processed. Thus use the corresponding function "kcalloc". This issue was detected by using the Coccinelle software. * Replace the specification of data structures by pointer dereferences to make the corresponding size determination a bit safer according to the Linux coding style convention. Signed-off-by:
-
Markus Elfring authored
Replace the specification of four data structures by pointer dereferences as the parameter for the operator "sizeof" to make the corresponding size determinations a bit safer according to the Linux coding style convention. Signed-off-by:
-
Markus Elfring authored
The script "checkpatch.pl" pointed information out like the following. WARNING: Possible unnecessary 'out of memory' message Thus remove such statements here. Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdfSigned-off-by:
-
Markus Elfring authored
* A multiplication for the size determination of a memory allocation indicated that an array data structure should be processed. Thus use the corresponding function "kcalloc". This issue was detected by using the Coccinelle software. * Replace the specification of a data structure by a pointer dereference to make the corresponding size determination a bit safer according to the Linux coding style convention. Signed-off-by:
-
Dmitry Monakhov authored
Currently ramdisk and fileio always perform PI verification before and after backend IO. This approach is not very flexible. Because some one may want to postpone this work to other layers in IO stack. For example if we want to test blk_integrity_profile testcase: https://github.com/dmonakhov/xfstests/commit/dee408c868861d6b6871dbb3381facee7effdbe4Signed-off-by:
Dmitry Monakhov <dmonakhov@openvz.org> Signed-off-by:
-
Dmitry Monakhov authored
If we failed to read data from backing file (probably because some one truncate file under us), we must zerofill cmd's data, otherwise it will be returned as is. Most likely cmd's data are unitialized pages from page cache. This result in information leak. (Change BUG_ON into -EINVAL se_cmd failure - nab) testcase: https://github.com/dmonakhov/xfstests/commit/e11a1b7b907ca67b1be51a1594025600767366d5Signed-off-by:
-
Bart Van Assche authored
Use the value of the BYTCHK field to determine the size of the Data-Out buffer. For VERIFY, honor the VRPROTECT, DPO and FUA fields. This patch avoids that LIO complains about a mismatch between the expected transfer length and the SCSI CDB length if the value of the BYTCHK field is 0. Signed-off-by:
-
Zhu Lingshan authored
This commit updated persistent revervation out service action code table in SPC-5 for development. Signed-off-by:
Zhu Lingshan <lszhu@suse.com> Signed-off-by:
-
Elena Reshetova authored
refcount_t type and corresponding API should be used instead of atomic_t when the variable is used as a reference counter. This allows to avoid accidental refcounter overflows that might lead to use-after-free situations. Signed-off-by:
Elena Reshetova <elena.reshetova@intel.com> Signed-off-by:
Hans Liljestrand <ishkamiel@gmail.com> Signed-off-by:
Kees Cook <keescook@chromium.org> Signed-off-by:
David Windsor <dwindsor@gmail.com> Signed-off-by:
-
- 02 Apr, 2017 2 commits
-
-
Xiubo Li authored
For the bidirectional case, the Data-Out buffer blocks will always at the head of the tcmu_cmd's bitmap, and before gathering the Data-In buffer, first of all it should skip the Data-Out ones, or the device supporting BIDI commands won't work. Fixed: 26418649 ("target/user: Introduce data_bitmap, replace data_length/data_head/data_tail") Reported-by:
Ilias Tsitsimpis <iliastsi@arrikto.com> Tested-by:
-
Nicholas Bellinger authored
Once upon a time back in 2009, a work-around was added to support the GlobalSAN iSCSI initiator v3.3 for MacOSX, which during login did not propose nor respond to MaxBurstLength, FirstBurstLength, DefaultTime2Wait and DefaultTime2Retain keys. The work-around in iscsi_check_proposer_for_optional_reply() allowed the missing keys to be proposed, but did not require waiting for a response before moving to full feature phase operation. This allowed GlobalSAN v3.3 to work out-of-the box, and for many years we didn't run into login interopt issues with any other initiators.. Until recently, when Martin tried a QLogic 57840S iSCSI Offload HBA on Windows 2016 which completed login, but subsequently failed with: Got unknown iSCSI OpCode: 0x43 The issue was QLogic MSFT side did not propose DefaultTime2Wait + DefaultTime2Retain, so LIO proposes them itself, and immediately transitions to full feature phase because of the GlobalSAN hack. However, the QLogic MSFT side still attempts to respond to DefaultTime2Retain + DefaultTime2Wait, even though LIO has set ISCSI_FLAG_LOGIN_NEXT_STAGE3 + ISCSI_FLAG_LOGIN_TRANSIT in last login response. So while the QLogic MSFT side should have been proposing these two keys to start, it was doing the correct thing per RFC-3720 attempting to respond to proposed keys before transitioning to full feature phase. All that said, recent versions of GlobalSAN iSCSI (v5.3.0.541) does correctly propose the four keys during login, making the original work-around moot. So in order to allow QLogic MSFT to run unmodified as-is, go ahead and drop this long standing work-around. Reported-by:Martin Svec <martin.svec@zoner.cz> Cc: Martin Svec <martin.svec@zoner.cz> Cc: Himanshu Madhani <Himanshu.Madhani@cavium.com> Cc: Arun Easi <arun.easi@cavium.com> Cc: stable@vger.kernel.org # 3.1+ Signed-off-by:
-
- 31 Mar, 2017 5 commits
-
-
Mike Christie authored
Multiple threads could be writing to alua_access_state at the same time, or there could be multiple STPGs in flight (different initiators sending them or one initiator sending them to different ports), or a combo of both and the core_alua_do_transition_tg_pt calls will race with each other. Because from the last patches we no longer delay running core_alua_do_transition_tg_pt_work, there does not seem to be any point in running that in a workqueue. And, we always wait for it to complete one way or another, so we can sleep in this code path. So, this patch made over target-pending just adds a mutex and does the work core_alua_do_transition_tg_pt_work was doing in core_alua_do_transition_tg_pt. There is also no need to use an atomic for the tg_pt_gp_alua_access_state. In core_alua_do_transition_tg_pt we will test and set it under the transition mutex. And, it is a int/32 bits so in the other places where it is read, we will never see it partially updated. Signed-off-by:
-
Sagi Grimberg authored
We pre-allocate our send-queues and might overflow them in case we have multi work-request operations which tend to occur for large RDMA transfers over devices with limited allowed sg elements. When we get to a queue-full condition we might retry again later, so track our receive buffers so we don't repost them for a retry case. Reported-by:
Potnuri Bharat Teja <bharat@chelsio.com> Tested-by:
Sagi Grimberg <sagi@grimberg.me> Signed-off-by:
-
Nicholas Bellinger authored
This patch addresses two queue-full handling bugs in iser-target. The first is propagating isert_rdma_rw_ctx_post() return back to target-core via isert_put_datain() + isert_get_dataout() callbacks, in order to trigger queue-full logic in target-core. Note target-core expects -EAGAIN or -ENOMEM error to signal RDMA WRITE/READ data-transfer callbacks should be retried, after queue-full logic been invoked. Other types of errors propagated up from RDMA RW API will result in target-core generating internal CHECK_CONDITION status, avoiding subsequent isert_put_datain() and isert_get_dataout() iscsit_transport callback retry attempts. The second is to use transport_generic_request_failure() during T10-PI hw-offload errors in isert_rdma_write_done() and isert_rdma_read_done(), so CHECK_CONDITION queue-full is handled internally by target-core. Also add isert_put_response() T10-PI failure case fixme in isert_rdma_write_done(), which is currently not internally retried or released until session reinstatement. Reported-by:
Steve Wise <swise@opengridcomputing.com> Cc: Steve Wise <swise@opengridcomputing.com> Cc: Sagi Grimberg <sagi@grimberg.me> Signed-off-by:
-
Nicholas Bellinger authored
This patch changes iscsi-target to propagate iscsit_transport ->iscsit_queue_data_in() and ->iscsit_queue_status() callback errors, back up into target-core. This allows target-core to retry failed iscsit_transport callbacks using internal queue-full logic. Reported-by:
-
Nicholas Bellinger authored
This patch fixes a set of queue-full response handling bugs, where outgoing responses are leaked when a fabric driver is propagating non -EAGAIN or -ENOMEM errors to target-core. It introduces TRANSPORT_COMPLETE_QF_ERR state used to signal when CHECK_CONDITION status should be generated, when fabric driver ->write_pending(), ->queue_data_in(), or ->queue_status() callbacks fail with non -EAGAIN or -ENOMEM errors, and data-transfer should not be retried. Note all fabric driver -EAGAIN and -ENOMEM errors are still retried indefinately with associated data-transfer callbacks, following existing queue-full logic. Also fix two missing ->queue_status() queue-full cases related to CMD_T_ABORTED w/ TAS status handling. Reported-by:
-
- 30 Mar, 2017 6 commits
-
-
Xiubo Li authored
The t_data_nents and t_bidi_data_nents are the numbers of the segments, but it couldn't be sure the block size equals to size of the segment. For the worst case, all the blocks are discontiguous and there will need the same number of iovecs, that's to say: blocks == iovs. So here just set the number of iovs to block count needed by tcmu cmd. Tested-by:
-
Xiubo Li authored
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. Reviewed-by:
-
Nicholas Bellinger authored
This patch closes a race between se_lun deletion during configfs unlink in target_fabric_port_unlink() -> core_dev_del_lun() -> core_tpg_remove_lun(), when transport_clear_lun_ref() blocks waiting for percpu_ref RCU grace period to finish, but a new NodeACL mappedlun is added before the RCU grace period has completed. This can happen in target_fabric_mappedlun_link() because it only checks for se_lun->lun_se_dev, which is not cleared until after transport_clear_lun_ref() percpu_ref RCU grace period finishes. This bug originally manifested as NULL pointer dereference OOPsen in target_stat_scsi_att_intr_port_show_attr_dev() on v4.1.y code, because it dereferences lun->lun_se_dev without a explicit NULL pointer check. In post v4.1 code with target-core RCU conversion, the code in target_stat_scsi_att_intr_port_show_attr_dev() no longer uses se_lun->lun_se_dev, but the same race still exists. To address the bug, go ahead and set se_lun>lun_shutdown as early as possible in core_tpg_remove_lun(), and ensure new NodeACL mappedlun creation in target_fabric_mappedlun_link() fails during se_lun shutdown. Reported-by:
James Shen <jcs@datera.io> Cc: James Shen <jcs@datera.io> Tested-by:
-
Nicholas Bellinger authored
This patch fixes a iscsi-target specific TMR reference leak during session shutdown, that could occur when a TMR was quiesced before the hand-off back to iscsi-target code via transport_cmd_check_stop_to_fabric(). The reference leak happens because iscsit_free_cmd() was incorrectly skipping the final target_put_sess_cmd() for TMRs when transport_generic_free_cmd() returned zero because the se_cmd->cmd_kref did not reach zero, due to the missing se_cmd assignment in original code. The result was iscsi_cmd and it's associated se_cmd memory would be freed once se_sess->sess_cmd_map where released, but the associated se_tmr_req was leaked and remained part of se_device->dev_tmr_list. This bug would manfiest itself as kernel paging request OOPsen in core_tmr_lun_reset(), when a left-over se_tmr_req attempted to dereference it's se_cmd pointer that had already been released during normal session shutdown. To address this bug, go ahead and treat ISCSI_OP_SCSI_CMD and ISCSI_OP_SCSI_TMFUNC the same when there is an extra se_cmd->cmd_kref to drop in iscsit_free_cmd(), and use op_scsi to signal __iscsit_free_cmd() when the former needs to clear any further iscsi related I/O state. Reported-by:
Rob Millner <rlm@daterainc.com> Cc: Rob Millner <rlm@daterainc.com> Reported-by:
Chu Yuan Lin <cyl@datera.io> Cc: Chu Yuan Lin <cyl@datera.io> Tested-by:
-
Manish Narani authored
This patch corrects the argument in usb_ep_free_request as it is mistakenly set to ep_out. It should be ep_in for status request. Signed-off-by:
Manish Narani <mnarani@xilinx.com> Acked-by:
Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by:
-
Nicholas Bellinger authored
The new cmd_time_out configfs attribute for TCMU is allowed to be disabled, so go ahead and drop the tcmu_cmd_time_out_store() check. Reported-by:
-
- 19 Mar, 2017 1 commit
-
-
Himanshu Madhani authored
Signed-off-by:
Himanshu Madhani <himanshu.madhani@cavium.com> signed-off-by:
Giridhar Malavali <giridhar.malavali@cavium.com> Signed-off-by:
-
