On Wed, Feb 13, 2019 at 02:41:49AM +0200, Max Gurtovoy wrote: > > On 2/12/2019 5:59 PM, Christoph Hellwig wrote: > > > @@ -1982,6 +1982,9 @@ struct ib_mr *ib_alloc_mr(struct ib_pd *pd, > > > if (!pd->device->ops.alloc_mr) > > > return ERR_PTR(-EOPNOTSUPP); > > > + if (WARN_ON_ONCE(mr_type == IB_MR_TYPE_PI)) > > > + return ERR_PTR(-EINVAL); > > > + > > So why is IB_MR_TYPE_PI a separate function, but IB_MR_TYPE_SG_GAPS > > is not? > > > > I think we either want one alloc/free helper per type, or we have > > to extend ib_alloc_mr with an arguments structure or something similar. > > my original commit modified the ib_alloc_mr to: > > struct ib_mr *ib_alloc_mr(struct ib_pd *pd, > enum ib_mr_type mr_type, > - u32 max_num_sg) > + u32 max_num_sg, u32 max_num_meta_sg) > > > and I also prefer to have 1 allocation function but I guess we should use > the style that the RDMA maintainers prefer and this is the review I got. > > So Jason/Leon/Doug in case you prefer to extend ib_alloc_mr as I originally > did, I can do it for V3. These monster 'do everything' multiplexor functions have turned out to be a maintenance pain in the past. I'm not sure MR will turn out any better.. So, CH's 'one alloc/free helper per type' seems like a better direction. At least the API should be understandable in that configuration. Jason
