1. 07 May, 2017 3 commits
    • Bart Van Assche's avatar
      IB/srpt: Avoid that aborting a command triggers a kernel warning · bd2c52d7
      Bart Van Assche authored
      Avoid that the following warning is triggered:
      
      WARNING: CPU: 10 PID: 166 at ../drivers/infiniband/ulp/srpt/ib_srpt.c:2674 srpt_release_cmd+0x139/0x140 [ib_srpt]
      CPU: 10 PID: 166 Comm: kworker/u24:8 Not tainted 4.9.4-1-default #1
      Workqueue: tmr-fileio target_tmr_work [target_core_mod]
      Call Trace:
       [<ffffffffaa3c4f70>] dump_stack+0x63/0x83
       [<ffffffffaa0844eb>] __warn+0xcb/0xf0
       [<ffffffffaa0845dd>] warn_slowpath_null+0x1d/0x20
       [<ffffffffc06ba429>] srpt_release_cmd+0x139/0x140 [ib_srpt]
       [<ffffffffc06e4377>] target_release_cmd_kref+0xb7/0x120 [target_core_mod]
       [<ffffffffc06e4d7f>] target_put_sess_cmd+0x2f/0x60 [target_core_mod]
       [<ffffffffc06e15e0>] core_tmr_lun_reset+0x340/0x790 [target_core_mod]
       [<ffffffffc06e4816>] target_tmr_work+0xe6/0x140 [target_core_mod]
       [<ffffffffaa09e4d3>] process_one_work+0x1f3/0x4d0
       [<ffffffffaa09e7f8>] worker_thread+0x48/0x4e0
       [<ffffffffaa09e7b0>] ? process_one_work+0x4d0/0x4d0
       [<ffffffffaa0a46da>] kthread+0xca/0xe0
       [<ffffffffaa0a4610>] ? kthread_park+0x60/0x60
       [<ffffffffaa71b775>] ret_from_fork+0x25/0x30
      Signed-off-by: default avatarBart Van Assche <bart.vanassche@sandisk.com>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
      Cc: Doug Ledford <dledford@redhat.com>
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: David Disseldorp <ddiss@suse.de>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      bd2c52d7
    • Bart Van Assche's avatar
      IB/srpt: Fix abort handling · 55d69427
      Bart Van Assche authored
      Let the target core check the CMD_T_ABORTED flag instead of the SRP
      target driver. Hence remove the transport_check_aborted_status()
      call. Since state == SRPT_STATE_CMD_RSP_SENT is something that really
      should not happen, do not try to recover if srpt_queue_response() is
      called for an I/O context that is in that state. This patch is a bug
      fix because the srpt_abort_cmd() call is misplaced - if that function
      is called from srpt_queue_response() it should either be called
      before the command state is changed or after the response has been
      sent.
      Signed-off-by: default avatarBart Van Assche <bart.vanassche@sandisk.com>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
      Cc: Doug Ledford <dledford@redhat.com>
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Andy Grover <agrover@redhat.com>
      Cc: David Disseldorp <ddiss@suse.de>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      55d69427
    • Bart Van Assche's avatar
      target/fileio: Fix zero-length READ and WRITE handling · 59ac9c07
      Bart Van Assche authored
      This patch fixes zero-length READ and WRITE handling in target/FILEIO,
      which was broken a long time back by:
      
      Since:
      
        commit d81cb447
        Author: Paolo Bonzini <pbonzini@redhat.com>
        Date:   Mon Sep 17 16:36:11 2012 -0700
      
            target: go through normal processing for all zero-length commands
      
      which moved zero-length READ and WRITE completion out of target-core,
      to doing submission into backend driver code.
      
      To address this, go ahead and invoke target_complete_cmd() for any
      non negative return value in fd_do_rw().
      Signed-off-by: default avatarBart Van Assche <bart.vanassche@sandisk.com>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Cc: Andy Grover <agrover@redhat.com>
      Cc: David Disseldorp <ddiss@suse.de>
      Cc: <stable@vger.kernel.org> # v3.7+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      59ac9c07
  2. 06 May, 2017 1 commit
    • Bryant G. Ly's avatar
      ibmvscsis: Do not send aborted task response · 25e78531
      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: default avatarBryant G. Ly <bryantly@linux.vnet.ibm.com>
      Signed-off-by: default avatarMichael Cyr <mikecyr@linux.vnet.ibm.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      25e78531
  3. 05 May, 2017 4 commits
    • Mike Christie's avatar
      tcmu: fix module removal due to stuck thread · d906d8af
      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: default avatarMike Christie <mchristi@redhat.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      d906d8af
    • Nicholas Bellinger's avatar
      target: Don't force session reset if queue_depth does not change · 46861cdd
      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: default avatarRaghu Krishnamurthy <rk@datera.io>
      Cc: Raghu Krishnamurthy <rk@datera.io>
      Tested-by: default avatarGary Guo <ghg@datera.io>
      Cc: Gary Guo <ghg@datera.io>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      46861cdd
    • Nicholas Bellinger's avatar
      iscsi-target: Set session_fall_back_to_erl0 when forcing reinstatement · 197b806a
      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: default avatarGary Guo <ghg@datera.io>
      Tested-by: default avatarGary Guo <ghg@datera.io>
      Cc: Gary Guo <ghg@datera.io>
      Cc: <stable@vger.kernel.org> # v4.1+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      197b806a
    • Nicholas Bellinger's avatar
      target: Fix compare_and_write_callback handling for non GOOD status · a71a5dc7
      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: default avatarBill Borsari <wgb@datera.io>
      Cc: Bill Borsari <wgb@datera.io>
      Tested-by: default avatarGary Guo <ghg@datera.io>
      Cc: Gary Guo <ghg@datera.io>
      Cc: <stable@vger.kernel.org> # v4.1+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      a71a5dc7
  4. 03 May, 2017 1 commit
    • Xiubo Li's avatar
      tcmu: Recalculate the tcmu_cmd size to save cmd area memories · fe25cc34
      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: default avatarXiubo Li <lixiubo@cmss.chinamobile.com>
      Acked-by: default avatarMike Christie <mchristi@redhat.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      fe25cc34
  5. 02 May, 2017 20 commits
  6. 02 Apr, 2017 2 commits
    • Xiubo Li's avatar
      tcmu: Skip Data-Out blocks before gathering Data-In buffer for BIDI case · a5d68ba8
      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: default avatarIlias Tsitsimpis <iliastsi@arrikto.com>
      Tested-by: default avatarIlias Tsitsimpis <iliastsi@arrikto.com>
      Signed-off-by: default avatarXiubo Li <lixiubo@cmss.chinamobile.com>
      Cc: stable@vger.kernel.org # 4.6+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      a5d68ba8
    • Nicholas Bellinger's avatar
      iscsi-target: Drop work-around for legacy GlobalSAN initiator · 1c99de98
      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: default avatarMartin 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: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      1c99de98
  7. 31 Mar, 2017 5 commits
  8. 30 Mar, 2017 4 commits
    • Xiubo Li's avatar
      tcmu: Fix wrongly calculating of the base_command_size · abe342a5
      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: default avatarIlias Tsitsimpis <iliastsi@arrikto.com>
      Reviewed-by: default avatarMike Christie <mchristi@redhat.com>
      Signed-off-by: default avatarXiubo Li <lixiubo@cmss.chinamobile.com>
      Cc: stable@vger.kernel.org # 3.18+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      abe342a5
    • Xiubo Li's avatar
      tcmu: Fix possible overwrite of t_data_sg's last iov[] · ab22d260
      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: default avatarMike Christie <mchristi@redhat.com>
      Tested-by: default avatarIlias Tsitsimpis <iliastsi@arrikto.com>
      Reviewed-by: default avatarBryant G. Ly <bryantly@linux.vnet.ibm.com>
      Signed-off-by: default avatarXiubo Li <lixiubo@cmss.chinamobile.com>
      Cc: stable@vger.kernel.org # 3.18+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      ab22d260
    • Nicholas Bellinger's avatar
      target: Avoid mappedlun symlink creation during lun shutdown · 49cb77e2
      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: default avatarJames Shen <jcs@datera.io>
      Cc: James Shen <jcs@datera.io>
      Tested-by: default avatarJames Shen <jcs@datera.io>
      Cc: stable@vger.kernel.org # 3.10+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      49cb77e2
    • Nicholas Bellinger's avatar
      iscsi-target: Fix TMR reference leak during session shutdown · efb2ea77
      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: default avatarRob Millner <rlm@daterainc.com>
      Cc: Rob Millner <rlm@daterainc.com>
      Reported-by: default avatarChu Yuan Lin <cyl@datera.io>
      Cc: Chu Yuan Lin <cyl@datera.io>
      Tested-by: default avatarChu Yuan Lin <cyl@datera.io>
      Cc: stable@vger.kernel.org # 3.10+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      efb2ea77