>Subject: Re: [PATCH rdma-next v4 10/12] RDMA/efa: Add EFA verbs >implementation > >On 28-Mar-19 19:27, Jason Gunthorpe wrote: >>>>> + unsigned long max_page_shift, >>>>> + int *count, u8 *shift, u32 *ncont) { >>>>> + unsigned long page_shift = umem->page_shift; >>>>> + struct scatterlist *sg; >>>>> + u64 base = ~0, p = 0; >>>>> + unsigned long tmp; >>>>> + unsigned long m; >>>>> + u64 len, pfn; >>>>> + int i = 0; >>>>> + int entry; >>>>> + >>>>> + addr = addr >> page_shift; >>>>> + tmp = (unsigned long)addr; >>>>> + m = find_first_bit(&tmp, BITS_PER_LONG); >>>>> + if (max_page_shift) >>>>> + m = min_t(unsigned long, max_page_shift - page_shift, m); >>>>> + >>>>> + for_each_sg(umem->sg_head.sgl, sg, umem->nmap, entry) { >>>>> + len = sg_dma_len(sg) >> page_shift; >>>> >>>> Shouldn't assume the offset is zero here. >>> >>> Because elements are not necessarily PAGE_SIZE? >>> If page combining is already done by ib_core, this whole function can >>> probably be deleted. >> >> If you test and ack the patch above then page combining will be done >> by the ib_core tomorrow :) >> >>>> This whole function should really better match what Shiraz is doing. >>>> It looks like it is just ib_umem_find_single_pg_size() ?? >>> >>> Yea, I think ib_umem_find_single_pg_size() could probably replace >>> this whole function, this work isn't merged yet though right? >> >> Not yes, but if you test it and say it works and you need it, then it >> can accelerate. > >Hey Shiraz and Jason - >Looks like the last patchset that contains these new APIs >(ib_umem_find_single_pg_size, return aligned memory blocks from SGL) was >submitted around Feb 19 and got some comments. >Should I wait for the new patches or cherry-pick v1? Sounds like the interface is >going to change, not sure if I should rebase the driver on top of that. Hi Gal - Sorry for the delayed response. Looks like you already tested the page combining patch. Thanks! :) Additionally, if you want to try out the API to find the best driver supported page size in the MR (ib_umem_find_single_pg_size()) and see if it fits your need, then you can grab that patch from v1 patchset. I don’t intend to update this API (well maybe a naming change). So you probably want to cherry-pick this one on top of the page combining patch. [v1 2/6] RDMA/umem: Add API to find best driver supported page size in an MR] https://patchwork.kernel.org/patch/10819995/ The iterator that returns aligned memory blocks [v1 3/6 RDMA/umem: Add API to return aligned memory blocks from SGL] is undergoing a re-spin and is not ready. Shiraz