Re: [PATCH 09/20] target: Invoke release_cmd() callback without holding a spinlock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Oct 12, 2015 at 10:10:20AM -0700, Bart Van Assche wrote:
> This patch fixes the following kernel warning because it avoids that
> IRQs are disabled while ft_release_cmd() is invoked (fc_seq_set_resp()
> invokes spin_unlock_bh()):

This looks fine to me.  It also removes the only user of
kref_put_spinlock_irqsave so you should be able to remove it as well.

Joern, I think the use of kref_get_unless_zero in Barts patch sould deal
with the issue that you added kref_put_spinlock_irqsave for just fine,
but can you review it as well?

> +++ b/drivers/target/target_core_tmr.c
> @@ -130,6 +130,9 @@ void core_tmr_abort_task(
>  		if (tmr->ref_task_tag != ref_tag)
>  			continue;
>  
> +		if (!kref_get_unless_zero(&se_cmd->cmd_kref))
> +			continue;
> +
>  		printk("ABORT_TASK: Found referenced %s task_tag: %llu\n",
>  			se_cmd->se_tfo->get_fabric_name(), ref_tag);
>  
> @@ -139,13 +142,15 @@ void core_tmr_abort_task(
>  			       " skipping\n", ref_tag);
>  			spin_unlock(&se_cmd->t_state_lock);
>  			spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
> +
> +			target_put_sess_cmd(se_cmd);
> +
>  			goto out;
>  		}
>  		se_cmd->transport_state |= CMD_T_ABORTED;
>  		spin_unlock(&se_cmd->t_state_lock);
>  
>  		list_del_init(&se_cmd->se_cmd_list);
> -		kref_get(&se_cmd->cmd_kref);
>  		spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
>  
>  		cancel_work_sync(&se_cmd->work);
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index eae4924..7f4d3ac 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -2507,23 +2507,24 @@ out:
>  EXPORT_SYMBOL(target_get_sess_cmd);
>  
>  static void target_release_cmd_kref(struct kref *kref)
> -		__releases(&se_cmd->se_sess->sess_cmd_lock)
>  {
>  	struct se_cmd *se_cmd = container_of(kref, struct se_cmd, cmd_kref);
>  	struct se_session *se_sess = se_cmd->se_sess;
> +	unsigned long flags;
>  
> +	spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);
>  	if (list_empty(&se_cmd->se_cmd_list)) {
> -		spin_unlock(&se_sess->sess_cmd_lock);
> +		spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
>  		se_cmd->se_tfo->release_cmd(se_cmd);
>  		return;
>  	}
>  	if (se_sess->sess_tearing_down && se_cmd->cmd_wait_set) {
> -		spin_unlock(&se_sess->sess_cmd_lock);
> +		spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
>  		complete(&se_cmd->cmd_wait_comp);
>  		return;
>  	}
>  	list_del(&se_cmd->se_cmd_list);
> -	spin_unlock(&se_sess->sess_cmd_lock);
> +	spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
>  
>  	se_cmd->se_tfo->release_cmd(se_cmd);
>  }
> @@ -2539,8 +2540,7 @@ int target_put_sess_cmd(struct se_cmd *se_cmd)
>  		se_cmd->se_tfo->release_cmd(se_cmd);
>  		return 1;
>  	}
> -	return kref_put_spinlock_irqsave(&se_cmd->cmd_kref, target_release_cmd_kref,
> -			&se_sess->sess_cmd_lock);
> +	return kref_put(&se_cmd->cmd_kref, target_release_cmd_kref);
>  }
>  EXPORT_SYMBOL(target_put_sess_cmd);
>  
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe target-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux