Messages in this thread |  | | | Subject | Re: [openib-general] [PATCH/RFC] IB: Add SCSI RDMA Protocol (SRP) initiator | | From | Roland Dreier <> | | Date | Mon, 31 Oct 2005 20:51:49 -0800 |
| |
> Has anyone purchased IB SRP target and for use with linux? > I've seen references to "Cisco SFS 3001 Multifabric Server Switch" > (TS90) with the optional FC gateway stuff. Yes, we have actually sold some...
> Are any native IB/SRP native storage devices available? I don't know what the release status of the various products are, but Data Direct, Engenio and Mellanox have all talked about native IB/SRP targets, and judging by John Kingman's activity, it's a safe bet that StorageGear has something cooking as well.
> Implies the driver hasn't changed since Jan 11. Is that correct? Nope, I bumped it to 0.2 and put it in the modinfo.
> I'd add "initiator" here unless you think this driver could > support targets in the future too. It's definitely an initiator, so I changed that.
> Don't need the NULL assignment here. Fixed.
> Could this be "adjusted" to read: > if (ret = PTR_ERR(target->qp)) { > ... > > I'm sure I do NOT understand the utility of "IS_ERR" in this case. > Most uses of "IS_ERR" seem superfluous. I don't think this sort of change will work. IS_ERR() is only true if the pointer (as an unsigned long) is in the range -1000L ... -1L. But PTR_ERR() will be true if the pointer is non-NULL.
> There are still 6 "XXX" markers...don't want to suggest they need > to be fixed. I fixed the easy ones...
> Are these retry counts specified by some standard or just > "this ought to be enough" kind of numbers? > If the latter, another "XXX" about making them system tunables > (e.g. MOD_PARM or /sys) would be good. Nope, no spec. I added a comment talking about this issue.
> Is it explained somplace why we drop the old cm_id and create > a new one in this case? > I'm hoping this was explained elsewhere and I just missed it. Yes, a few lines earlier:
/* * Now get a new local CM ID so that we avoid confusing the * target in case things are really fouled up. */ > > + while (ib_poll_cq(target->cq, 1, &wc) > 0) > > + ; /* nothing */ > does a "relax_cpu()" belong in here? I don't think so. No entries can be added to the CQ while we're in that loop -- I just want to go through the CQ and throw away any of the entries that are there. So it's not busy-waiting -- it's just iterating through the queue until it drains it.
Thanks, Roland - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |