On Thu, 2012-08-09 at 15:43 +0000, Bart Van Assche wrote:
> Avoid that the scmnd->scsi_done(scmnd) call in srp_process_rsp()
> can trigger a crash by being invoked with scsi_done == NULL. That
> could happen if a reply is received during or after a command abort.

Please leave the call trace out of the commit message; it adds nothing.

> +static struct scsi_cmnd *srp_claim_req(struct srp_target_port *target,
> +                                    struct srp_request *req,
> +                                    struct scsi_cmnd *scmnd,
> +                                    s32 req_lim_delta)
>  {
>       unsigned long flags;
>  
> -     srp_unmap_data(req->scmnd, target, req);
>       spin_lock_irqsave(&target->lock, flags);
>       target->req_lim += req_lim_delta;

I still think that adding credits back before putting the request on the
free list is a crash waiting to happen. We don't check to see if the
free request list is empty in srp_queuecommand(), so this creates a new
race window where we get a response that gives us credits back, but we
don't have a free request to back them.

Arguably, we need to check for that anyway to protect against buggy
targets giving us back more credits than we have used...

Or am I missing something obvious?
-- 
Dave Dillow
National Center for Computational Science
Oak Ridge National Laboratory
(865) 241-6602 office

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to