On Mon, May 06, 2019 at 09:38:56AM +0300, Gal Pressman wrote:
> On 05-May-19 15:35, Jason Gunthorpe wrote:
> > On Sun, May 05, 2019 at 11:13:01AM +0300, Gal Pressman wrote:
> >> On 03-May-19 15:19, Jason Gunthorpe wrote:
> >>> 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
> >>
> >> This is by no means abusing the API..
> >
> > It is, vzalloc isn't just kzalloc followed by vzalloc and you
> > shouldn't expec the two to be the same. Most likely the above has bad
> > behavior if it triggers reclaim.
>
> Is it OK to call kvzalloc and test for is_vmalloc_addr?
Yes
Jason
