On 5/1/2018 7:20 PM, Doug Ledford wrote:
On Mon, 2018-04-30 at 09:27 +0200, Roman Pen wrote:
From: Roman Pen <r.peniaev@xxxxxxxxx>
Hi all,
This is an attempt to make a device pool API out of a common code,
which caches pair of ib_device and ib_pd pointers. I found 4 places,
where this common functionality can be replaced by some lib calls:
nvme, nvmet, iser and isert. Total deduplication gain in loc is not
quite significant, but eventually new ULP IB code can also require
the same device/pd pair cache, e.g. in our IBTRS module the same
code has to be repeated again, which was observed by Sagi and he
suggested to make a common helper function instead of producing
another copy.
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.
I'm sorry, I also don't think it saves us a lot. I guess each ULP that
is using ib_device will always wrap it with some ulp_device structure
with it's own functionality.
-Max.
--
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
