On Sat, Aug 14, 2010 at 7:04 PM, David Dillow <d...@thedillows.org> wrote: > > On Sat, 2010-08-14 at 09:35 +0200, Bart Van Assche wrote: > > On Fri, Aug 13, 2010 at 8:24 PM, David Dillow <d...@thedillows.org> wrote: > > > > > > > + > > > > + rsv = (req_type == SRP_REQ_NORMAL) ? SRP_TSK_MGMT_RSV : 0; > > > > > > > > srp_send_completion(target->send_cq, target); > > > > > > > > - if (target->tx_head - target->tx_tail >= SRP_SQ_SIZE) > > > > + if (target->tx_head - target->tx_tail >= SRP_SQ_SIZE - rsv) > > > > return NULL; > > > > > > This isn't needed per my previous message. It is just a guard to make > > > sure we don't use more buffers than are on the ring. The real limiter is > > > the number of credits. Perhaps swapping it with the req_limit check > > > below and adding a WARN_ONCE() may be a good idea, since I don't think > > > we should ever hit it. > > > > Replacing the above if-statement by a WARN_ON_ONCE() statement would > > make sense if the value of tx_head - tx_tail did only depend on the > > implementation of ib_srp. > > I didn't say replace the guard, I said add a message so we knew we hit > this case. The "isn't needed" was referring to the change to the line. > WARN_ON_ONCE may not be the best thing to use, but perhaps a > rate-limited shost_printk would be good. We really should never hit the > limit on tx buffers, unless we are dealing with a non-conforming target > as you point out.
I'll leave out the check altogether - it has never been the primary goal of ib_srp to serve as a debugging aid for target implementers. I'll post a new patch series that should address all comments that have been posted so far. Bart. -- 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
