Re: [RFC 7/8] IB/srp: try to use larger FMR sizes to cover our mappings

David Dillow
2011-01-20T12:40:50Z

On Thu, 2011-01-20 at 12:24 +0200, Or Gerlitz wrote:
> David Dillow wrote:
> > Now that we can get larger SG lists, we can take advantage of HCAs that
> > allow us to use larger FMR sizes. In many cases, we can use up to 512
> > entries, so start there and work our way down.
>
> I'm not clear what this patch really tries to do but before that, going
> to low level details

I want to use 512 entry FMRs instead of 256 entry ones. I'll accept 128
entries if I have to. This reduces the number of indirect entries in the
list going to the client.

> > + for (max_pages_per_fmr = SRP_FMR_SIZE;
> > + max_pages_per_fmr >= SRP_FMR_MIN_SIZE;
> > + max_pages_per_fmr /= 2, srp_dev->fmr_max_size /= 2) {
> > + fmr_param.pool_size = SRP_FMR_POOL_SIZE;
> > + fmr_param.dirty_watermark = SRP_FMR_DIRTY_SIZE;
> > + fmr_param.cache = 1;
> > + fmr_param.max_pages_per_fmr = SRP_FMR_SIZE;
> > + fmr_param.page_shift = fmr_page_shift;
>
> > + srp_dev->fmr_pool = ib_create_fmr_pool(srp_dev->pd, &fmr_param);
> > + if (!IS_ERR(srp_dev->fmr_pool))
> > + break;
> > + }
>
> aren't stepping on srp_dev->fmr_pool on each invocation of the for loop?

No, we break out of the loop if we got a good allocation -- ! IS_ERR().

> didn't you want page_shift and/or max_pages_per_fmr to change
> throughout the loop?

Uhm. Yeah. Forgot to s/SRP_FMR_SIZE/max_pages_per_fmr/ there, didn't I.
Good catch, thanks.
--
Dave Dillow
National Center for Computational Science
Oak Ridge National Laboratory
(865) 241-6602 office