Re: [RFC 2/2] dual scan thread bug fix

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

 



On Wed, 8 Jan 2014, James Bottomley wrote:

> OK, Agreed, but that means modifying the 1/2 patch with the below.  This
> should make the proposed diff to 2/2 correct.
> 
> James
> 
> ---
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index ef3f958..5fad646 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -417,7 +417,7 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
>  		+ shost->transportt->target_size;
>  	struct scsi_target *starget;
>  	struct scsi_target *found_target;
> -	int error;
> +	int error, ref_got;
>  
>  	starget = kzalloc(size, GFP_KERNEL);
>  	if (!starget) {
> @@ -466,15 +466,15 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
>  	return starget;
>  
>   found:
> -	if (!kref_get_unless_zero(&found_target->reap_ref))
> -		/*
> -		 * release routine already fired.  Target is dead, but
> -		 * STARGET_DEL may not yet be set (set in the release
> -		 * routine), so set here as well, just in case
> -		 */
> -		found_target->state = STARGET_DEL;
> +	/*
> +	 * release routine already fired if kref is zero, so if we can still
> +	 * take the reference, the target must be alive.  If we can't, it must
> +	 * be dying and we need to wait for a new target
> +	 */
> +	ref_got = kref_get_unless_zero(&found_target->reap_ref);
> +
>  	spin_unlock_irqrestore(shost->host_lock, flags);
> -	if (found_target->state != STARGET_DEL) {
> +	if (ref_got) {
>  		put_device(dev);
>  		return found_target;
>  	}
> @@ -482,8 +482,8 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
>  	 * Unfortunately, we found a dying target; need to wait until it's
>  	 * dead before we can get a new one.  There is an anomaly here.  We
>  	 * *should* call scsi_target_reap() to balance the kref_get() of the
> -	 * reap_ref above.  However, since the target is in state STARGET_DEL,
> -	 * it's already invisible and the reap_ref is irrelevant.  If we call
> +	 * reap_ref above.  However, since the target being released, it's
> +	 * already invisible and the reap_ref is irrelevant.  If we call
>  	 * scsi_target_reap() we might spuriously do another device_del() on
>  	 * an already invisible target.
>  	 */

In fact, most of this comment (everything after the first sentence) is
no longer needed.  If the target is dying then kref_get_unless_zero
must have failed, so there is no need to worry about unbalanced
refcounts.

Otherwise this looks good.

Alan Stern

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




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux