On Thu, Dec 10, 2015 at 11:49:40AM -0500, Dalessandro, Dennis wrote:
> On Tue, Dec 08, 2015 at 02:52:36PM -0500, ira. weiny wrote:
> >On Tue, Dec 08, 2015 at 01:17:48PM -0600, Sean Hefty wrote:
> >>> > > +struct ib_pd *rvt_alloc_pd(struct ib_device *ibdev,
> >>> > > + struct ib_ucontext *context,
> >>> > > + struct ib_udata *udata)
> >>> > > +{
> >>> > > + struct rvt_dev_info *dev = ib_to_rvt(ibdev);
> >>> > > + struct rvt_pd *pd;
> >>> > > + struct ib_pd *ret;
> >>> > > +
> >>> > > + pd = kmalloc(sizeof(*pd), GFP_KERNEL);
> >>> > > + if (!pd) {
> >>> > > + ret = ERR_PTR(-ENOMEM);
> >>> > > + goto bail;
> >>> > > + }
> >>> > > + /*
> >>> > > + * This is actually totally arbitrary. Some correctness
> >>tests
> >>> > > + * assume there's a maximum number of PDs that can be
> >>allocated.
> >>> > > + * We don't actually have this limit, but we fail the test if
> >>> > > + * we allow allocations of more than we report for this
> >>value.
> >>> > > + */
> >>> >
> >>> > Why not trap this in user space, rather than forcing the kernel to
> >>> support some test program?
> >>> >
> >>>
> >>> What do you mean "trap this in user space"? This code is not supporting
> >>> any
> >>> particular test program.
> >>>
> >>> If users try and allocate more protection domains then are advertised
> >>then
> >>> an
> >>> error should be returned.
> >>>
> >>> Perhaps the comment should be removed if it is confusing?
> >>
> >>There is no theoretical limit on the number of PDs, or likely most other
> >>resources. So why define and enforce an arbitrary limit? The
> >>justification
> >>given was that some test program would fail. If there's a concern about
> >>that
> >>test program passing, then enforce the limit in user space.
> >
> >I did not interpret this as being driven by a test but rather by the IBTA
> >spec.
> >
> >This may be pedantic but, wouldn't it be confusing from a user POV to see
> >an
> >advertised limit but then not be constrained by it? I'm not opposed to
> >setting
> >this limit arbitrarily high, but I still think the implementation should
> >enforce that limit.
> >
> >>
> >>I didn't find the comment confusing. Just the choice to let a test app
> >>drive the design.
> >
> >Perhaps "confusing" is the wrong word. But I did not interpreted the
> >comment
> >as saying that the implementation was driven but a test program.
>
> How about I reword the comment to say that while we could continue
> allocating PDs being only constrained by system resources, the spec says
> there is a limit so we enforce that?
I'm ok with that, Sean?
Ira
>
> -Denny
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
