I was hoping for more dedup of LoC. Oh well. It still has value in
that it puts the locking and such in one place so we don't have as many
potential spots for bugs to creep in, but one way you usually measure
that impact is when you remove 100 lines of complex code and replace it
with 20 lines of simpler library calls. Because of the need for
callbacks, you don't get 20 lines of simpler library calls, you get 20
lines of simpler calls and then 60 lines of callbacks that reintroduce
complexity into that which was supposed to be simplified.
Anyway, I'm ambivalent on this one. If it simplified things more, I'd
be much more enthusiastic. In the end, I'll leave the decision up to
Sagi and Christoph. It's their code that this is modifying mainly. If
they think this qualifies as a worthwhile cleanup in their code, then
I'll take it in.
Hi Sagi and Christoph,
Guys, could you please comment on this patch set? These patches were
created because of some later Sagi comments on IBTRS code, where I
repeat the same dev_find_or_add() pattern, which is common for nvme
and iser. So this is an attempt to deduplicate a bit and then reuse
the API.
Indeed, this change does not bring a lot in terms of LoC, but e.g. for
IBTRS it simplifies the stuff and I just want to decide can I use this
API or I should move those functions back to ibtrs-core module, without
touching infiniband/core/.
Hi Roman,
Thanks for taking a stab at it!
I was hoping this would simplify things even further but looking at this
I'm not sure it makes things any better.
What I had in mind was something different. I would try and tie this
to ib_client interface. In this case, a ULP would create its device
representation in .add() and destroy it in .remove() callouts. For the
binding of a ULP device represntation to a ULP queue I would offer a
single generic lookup that takes cm_id->device and return the ib_client
class device (pointer to the ULP device).
This way, no need for refcounting (comes for free) nor optional
callouts... Thoughts?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
