diff mbox

target: up caw_sem if COMPARE_AND_WRITE got aborted

Message ID 6b4bfba1-8c59-eb14-0307-1dcf828e1ca3@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jiang Yi June 3, 2017, 9:32 a.m. UTC
Hi Nic,

If a se_cmd of COMPARE_AND_WRITE got aborted, "struct semaphore caw_sem" of the corresponding se_device will never being called up().

I propose a patch:


Signed-off-by: Jiang Yi <jiangyilism@gmail.com>
---
 drivers/target/target_core_transport.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Nicholas A. Bellinger June 25, 2017, 7:11 p.m. UTC | #1
Hi Jiang,

Apologies for the delayed follow-up on this.  Comments below.

On Sat, 2017-06-03 at 17:32 +0800, Jiang Yi wrote:
> Hi Nic,
> 
> If a se_cmd of COMPARE_AND_WRITE got aborted, "struct semaphore caw_sem" of the corresponding se_device will never being called up().
> 
> I propose a patch:
> 
> 
> Signed-off-by: Jiang Yi <jiangyilism@gmail.com>
> ---
>  drivers/target/target_core_transport.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index 6025935..02734ca 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -726,6 +726,9 @@ void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
>       */
>      if (cmd->transport_state & CMD_T_ABORTED ||
>          cmd->transport_state & CMD_T_STOP) {
> +        if ((cmd->se_cmd_flags & SCF_COMPARE_AND_WRITE) && success) {
> +            up(&dev->caw_sem);
> +        }
>          spin_unlock_irqrestore(&cmd->t_state_lock, flags);
>          complete_all(&cmd->t_transport_stop_comp);
>          return;

After thinking about this some more, releasing cam_sem here probably
makes the most sense, instead of say invoking the completion callback
based on se_cmd->transport_state, or invoking the completion callback
from __transport_wait_for_tasks().

Although, the extra '&& success' check here is incorrect, as the
releasing of cam_sem needs to happen for this special case regardless of
which scsi_status that target_complete_cmd() is called with.

So that said, applying this to target-pending/for-next, and dropping the
extra check based on scsi_status.

Thank you.





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

Patch

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 6025935..02734ca 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -726,6 +726,9 @@  void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
      */
     if (cmd->transport_state & CMD_T_ABORTED ||
         cmd->transport_state & CMD_T_STOP) {
+        if ((cmd->se_cmd_flags & SCF_COMPARE_AND_WRITE) && success) {
+            up(&dev->caw_sem);
+        }
         spin_unlock_irqrestore(&cmd->t_state_lock, flags);
         complete_all(&cmd->t_transport_stop_comp);
         return;