Re: [PATCH 1/9] mm: Introduce new vm_insert_range API

[Randy Dunlap]

On 11/16/18 12:15 AM, Souptick Joarder wrote:
> On Fri, Nov 16, 2018 at 12:11 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>>
>> On Fri, Nov 16, 2018 at 11:00:30AM +0530, Souptick Joarder wrote:
>>> On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap@xxxxxxxxxxxxx> wrote:
>>>> On 11/15/18 7:45 AM, Souptick Joarder wrote:
>>>> What is the opposite of vm_insert_range() or even of vm_insert_page()?
>>>> or is there no need for that?
>>>
>>> There is no opposite function of vm_insert_range() / vm_insert_page().
>>> My understanding is, in case of any error, mmap handlers will return the
>>> err to user process and user space will decide the next action. So next
>>> time when mmap handler is getting invoked it will map from the beginning.
>>> Correct me if I am wrong.
>>
>> The opposite function, I suppose, is unmap_region().
>>
>>>> s/no./number/
>>>
>>> I didn't get it ??
>>
>> This is a 'sed' expression.  's' is the 'substitute' command; the /
>> is a separator, 'no.' is what you wrote, and 'number' is what Randy
>> is recommending instead.
> 
> Ok. Will change it in v2.

Thanks.

>>>>> +     for (i = 0; i < page_count; i++) {
>>>>> +             ret = vm_insert_page(vma, uaddr, pages[i]);
>>>>> +             if (ret < 0)
>>>>> +                     return ret;
>>>>
>>>> For a non-trivial value of page_count:
>>>> Is it a problem if vm_insert_page() succeeds for several pages
>>>> and then fails?
>>>
>>> No, it will be considered as total failure and mmap handler will return
>>> the err to user space.
>>
>> I think what Randy means is "What happens to the inserted pages?" and
>> the answer is that mmap_region() jumps to the 'unmap_and_free_vma'
>> label, which is an accurate name.

which says:

	/* Undo any partial mapping done by a device driver. */
	unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end);

and that is what I was missing.  Thanks.

> Sorry for incorrect understanding of the question.

No problem.


-- 
~Randy