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?
