On Tue, May 1, 2018 at 6:20 PM, Doug Ledford <dledford@xxxxxxxxxx> 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. Indeed callbacks spoil loc numbers. Init and deinit can imply alloc and free, but then I should pass ib_device and ib_pd pointers as params to those callbacks (assuming pd is allocated inside a lib call), which will save probably 10 lines of code. Well, I did not like that. But at least these calls hide a play with a list and provide simpler callback API, which consumes lines, but logically wrapps only a sequence of container_of, malloc and free calls. -- Roman -- 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
