On Tue, Nov 06, 2018 at 08:41:54PM +0200, Leon Romanovsky wrote: > > Sorry for the late comment for Jason's point in v2 series to have > > one giant structure. > > > > With one giant structure, fill_res_entry() provider callback > > function of rdma_restrack_root doesn't deserve to be inside a > > logically well separate structure rdma_restrack_root. We should > > take out fill_res_entry() and put into ib_device_ops. This is possibly true.. It is cleaner for drivers if they don't have to reach into all sorts of different places to put function pointers. > > But I like the approach of rdma_restrack_root which keeps all > > logical entities together. And with that good example I continue > > to propose same logical separate for rest of the callbacks too. > > With that non hot control path routines can possibly stay outside > > of ib_device apart from code readability. > > Second for that, > It will also give us a chance to optimize for cache alignment for such > paths. As long as the function pointers are stored in memory allocated as part of struct ib_device, adding more structs will not help performance. The usual approach of sorting hot data together into common cache lines is appropriate here. Kamal sorted by functionality, which is not necessarily great, it would be better to sort based on some profiling, but our prior list wasn't optimized either.. > Plus review will be much cleaner, we will be able to easily spot the > need of performance numbers for changes in those flows. I think having micro structures would make a huge mess of the drivers. Every driver would have to individually split all the ops (across all the variations) into a huge number of micro structs and juggle all of that. So unnecessary and complicated.. The only value I see in this series is that it makes the drivers much simpler. I would not throw that way for micro-structures.. Jason
