Re: [PATCH] ibmvscsi driver - next version

[Brian King <brking@us.ibm.com>]

Christoph Hellwig wrote:

> +	found_evt = NULL;
> +	spin_lock_irqsave(&hostdata->lock, flags);
> +	if(!list_empty(&hostdata->sent)) {
> +		list_for_each_safe(pos, next, &hostdata->sent) {
> +			tmp_evt = list_entry(pos, struct srp_event_struct, list);
> +			
> +			if (tmp_evt->cmnd == cmd) {
> +				found_evt = tmp_evt;
> +				break;
> +			}
> +		}
> +	}
> +	spin_unlock_irqrestore(&hostdata->lock, flags);
> +	
> +	/* If we can't find this event, just return false */
> +	if (found_evt == NULL) {
> +		printk(KERN_ERR "ibmvscsi: failed to abort command\n");
> +		return FAILED;
> +	}
> 
> 	This can't ever happen.  You will only get called for commands
> 	that you have accepted in queuecommand and not called the completion
> 	routine for.

I'm not sure I agree with this statement. Looking at scsi_error.c, it looks
to me like there is a race condition in that the midlayer decides whether or
not to abort an op, then grabs the host lock, then calls eh_abort. From the
time that the midlayer decides to abort an op and then grabs the host lock, the
op may have finished. The LLD should return SUCCESS in this scenario, however,
rather than FAILED.



-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center