On Thu, 2010-08-19 at 08:04 +0200, Bart Van Assche wrote: > On Thu, Aug 19, 2010 at 1:44 AM, David Dillow <[email protected]> wrote: > > > > On Mon, 2010-08-16 at 20:54 +0200, Bart Van Assche wrote: > > > @@ -989,19 +989,21 @@ static void srp_send_completion(struct ib_cq *cq, > > > void *target_ptr) > > > static struct srp_iu *__srp_get_tx_iu(struct srp_target_port *target, > > > enum srp_request_type req_type) > > > { > > > - s32 min = (req_type == SRP_REQ_TASK_MGMT) ? 1 : 2; > > > + s32 rsv; > > > + > > > + rsv = (req_type == SRP_REQ_TASK_MGMT) ? 0 : SRP_TASK_MGMT_SQ_SIZE; > > > > > > srp_send_completion(target->send_cq, target); > > > > > > if (target->tx_head - target->tx_tail >= SRP_SQ_SIZE) > > > return NULL; > > > > > > - if (target->req_lim < min) { > > > + if (target->req_lim <= rsv) { > > > ++target->zero_req_lim; > > > return NULL; > > > } > > > > > > - return target->tx_ring[target->tx_head & SRP_SQ_SIZE]; > > > + return target->tx_ring[target->tx_head & SRP_SQ_MASK]; > > > } > > > > Did you forget to remove the rest of the changes? The only change to > > this function should be s/SRP_SQ_SIZE/SRP_SQ_MASK/ > > What are your objections against the above changes ? No functionality > has been changed - all the above changes do is to introduce the > symbolic constants SRP_TASK_MGMT_SQ_SIZE and SRP_SQ_MASK > __srp_get_tx_iu().
Style. Aesthetics. I was fine with SRP_SQ_MASK, but I didn't like the rest of the changes. Mainly, I seriously dislike the name SRP_TASK_MGMT_SQ_SIZE. But I've given up on the naming issue for now since I don't have a better suggestion, and you do remove a magic constant, which is good. Put the assignment on the same line as the declaration and I'll deal with the name. 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
