Looks pretty good; a few minor comments:

 > +static int srp_response_common(struct srp_target_port *target, s32 
 > req_delta,
 > +                           void *rsp, int len);

Didn't check -- can we reorder things to avoid this forward declaration?

 > +    shost_printk(KERN_ERR, target->scsi_host, PFX
 > +                 "ignoring AER for LUN %llu\n", be64_to_cpu(req->lun));

I wonder if we should dump the sense data here, while we're at it.  Do
any targets even send this?

 >  static int __srp_post_send(struct srp_target_port *target,
 > -                       struct srp_iu *iu, int len)
 > +                       struct srp_iu *iu, int len, int credits)

I wonder if this would be slightly clearer/safer if we made the
parameter "bool consume_credit" or something like that?  Since we should
never pass in anything except 0 or 1, right?

 > +    /* SRP_RESP_RESV sets the number of queue entries reserved for
 > +     * unsolicited messages from the target and the responses to them.
 > +     */
 > +    SRP_RESP_RESV           = 2,

Looking at this -- where does the value 2 come from?  We currently
always keep one receive posted, right?  I guess the issue is that we
might get a CRED_REQ and a logout request or something like that, but
that should work OK if we process them one at a time.

Or is this change important too?

 > +} __attribute__((packed));

I don't think any of the new structures need to be packed actually.

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

Reply via email to