On Tue, 2010-08-03 at 17:26 +0200, Bart Van Assche wrote: > On Tue, Aug 3, 2010 at 5:05 PM, David Dillow <[email protected]> wrote: > > I understood that you were allocating from the same queue of buffers, > > I'm just not sure you need to keep track separately. We already hold > > SRP_RSP_SQ_SIZE buffers back from sending requests, so we will always > > have those (well, one) available for sending a response. > > > > If we use too many for responses, that's not great, but it isn't > > completely fatal -- srp_queuecommand() will bounce the command back to > > the mid-layer with a BUSY status, and while the error handling (task > > mgmt) will go to the big guns and reset the connection sooner, we will > > recover. > > > > If there isn't a buffer available to send the response, it'll get > > dropped on the floor, just like in your patch. > > > > I just don't see a reason for the accounting change. > > I'm not sure it is a good idea to allow that all transmit buffers get > allocated for sending CMD_RSP information units and that none remain > for replying to incoming SRP_CRED_REQ / SRP_AER_REQ / SRP_T_LOGOUT > requests. Wouldn't that be a protocol violation ?
You mean SRP_CMD? We don't do that, we clamp the outstanding requests we generate so we'll always have a buffer to reply to SRP_CRED_REQ et al. If SRQ_SQ_SIZE is 64, we set .can_queue to 63 which will leave a buffer available for the reply. Now, if the target sends us multiple SRP_CRED_REQ without waiting for a reply, then we could use up buffers and not be able to send requests, but that would be a protocol violation on the target's side. Dave -- 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
