On Fri, May 03, 2019 at 12:53:55PM +0300, Gal Pressman wrote:
> On 02-May-19 21:14, Jason Gunthorpe wrote:
> > On Wed, May 01, 2019 at 01:48:22PM +0300, Gal Pressman wrote:
> >
> >> +/* create a page buffer list from a mapped user memory region */
> >> +static int pbl_create(struct efa_dev *dev,
> >> + struct pbl_context *pbl,
> >> + struct ib_umem *umem,
> >> + int hp_cnt,
> >> + u8 hp_shift)
> >> +{
> >> + int err;
> >> +
> >> + pbl->pbl_buf_size_in_bytes = hp_cnt * EFA_CHUNK_PAYLOAD_PTR_SIZE;
> >> + pbl->pbl_buf = kzalloc(pbl->pbl_buf_size_in_bytes,
> >> + GFP_KERNEL | __GFP_NOWARN);
> >> + if (pbl->pbl_buf) {
> >> + pbl->physically_continuous = 1;
> >> + err = umem_to_page_list(dev, umem, pbl->pbl_buf, hp_cnt,
> >> + hp_shift);
> >> + if (err)
> >> + goto err_continuous;
> >> + err = pbl_continuous_initialize(dev, pbl);
> >> + if (err)
> >> + goto err_continuous;
> >> + } else {
> >> + pbl->physically_continuous = 0;
> >> + pbl->pbl_buf = vzalloc(pbl->pbl_buf_size_in_bytes);
> >> + if (!pbl->pbl_buf)
> >> + return -ENOMEM;
> >
> > This way to fallback seems ugly, I think you should just call kvzalloc
> > and check for continuity during the umem_to_page_list
>
> I've considered using kvzalloc, but it doesn't really fit this use case.
It does, you just check for continuity when building the pbl instead
of assuming it based on how it was created. It isn't hard, and drivers
shouldn't abuse APIs like this
Jason
