Re: [PATCH 2/2] xen/privcmd: Convert get_user_pages*() to pin_user_pages*()

John Hubbard
2020-06-26T06:54:32Z

On 2020-06-25 22:26, Souptick Joarder wrote:
> On Thu, Jun 25, 2020 at 11:19 AM John Hubbard <jhubbard@nvidia.com> wrote:
>> On 2020-06-24 20:02, Souptick Joarder wrote:
...
>>> @@ -612,13 +612,7 @@ static int lock_pages(
>>>
>>> static void unlock_pages(struct page *pages[], unsigned int nr_pages)
>>> {
>>> - unsigned int i;
>>> -
>>> - for (i = 0; i < nr_pages; i++) {
>>> - if (!PageDirty(page))
>>> - set_page_dirty_lock(page);
>>> - put_page(pages[i]);
>>> - }
>>> + unpin_user_pages_dirty_lock(pages, nr_pages, 1);
>>
>> "true", not "1", is the correct way to call that function.
>
> Ok.
>
>>
>> Also, this approach changes the behavior slightly, but I think it's

Correction, I forgot that I put that same if(!PageDirty(page)) check into
unpin_user_pages_dirty_lock(). So it doesn't change behavior. That's good.

>> reasonable to just set_page_dirty_lock() on the whole range--hard to
>> see much benefit in checking PageDirty first.
>
> unpin_user_pages_dirty_lock() internally will do the same check after
> patch [2/2]
> So I thought to keep old and new code in sync. Shall we avoid this check ?
>
Just leave it as you have it, but of course use "true" instead of 1, please.

thanks,
--
John Hubbard
NVIDIA