linux-ext4 - Re: [PATCH v2 03/11] mm/gup: migrate PIN_LONGTERM dev coherent pages to system

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3f8a48d6-7ee7-fb30-5942-29054c34aac5@amd.com>
Date:   Wed, 8 Dec 2021 12:55:26 -0600
From:   "Sierra Guiza, Alejandro (Alex)" <alex.sierra@....com>
To:     Felix Kuehling <felix.kuehling@....com>,
        Alistair Popple <apopple@...dia.com>,
        akpm@...ux-foundation.org, linux-mm@...ck.org,
        rcampbell@...dia.com, linux-ext4@...r.kernel.org,
        linux-xfs@...r.kernel.org
Cc:     amd-gfx@...ts.freedesktop.org, willy@...radead.org,
        jglisse@...hat.com, dri-devel@...ts.freedesktop.org,
        jgg@...dia.com, hch@....de
Subject: Re: [PATCH v2 03/11] mm/gup: migrate PIN_LONGTERM dev coherent pages
 to system


On 12/8/2021 11:30 AM, Felix Kuehling wrote:
> Am 2021-12-08 um 11:58 a.m. schrieb Felix Kuehling:
>> Am 2021-12-08 um 6:31 a.m. schrieb Alistair Popple:
>>> On Tuesday, 7 December 2021 5:52:43 AM AEDT Alex Sierra wrote:
>>>> Avoid long term pinning for Coherent device type pages. This could
>>>> interfere with their own device memory manager.
>>>> If caller tries to get user device coherent pages with PIN_LONGTERM flag
>>>> set, those pages will be migrated back to system memory.
>>>>
>>>> Signed-off-by: Alex Sierra <alex.sierra@....com>
>>>> ---
>>>>   mm/gup.c | 32 ++++++++++++++++++++++++++++++--
>>>>   1 file changed, 30 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/mm/gup.c b/mm/gup.c
>>>> index 886d6148d3d0..1572eacf07f4 100644
>>>> --- a/mm/gup.c
>>>> +++ b/mm/gup.c
>>>> @@ -1689,17 +1689,37 @@ struct page *get_dump_page(unsigned long addr)
>>>>   #endif /* CONFIG_ELF_CORE */
>>>>   
>>>>   #ifdef CONFIG_MIGRATION
>>>> +static int migrate_device_page(unsigned long address,
>>>> +				struct page *page)
>>>> +{
>>>> +	struct vm_area_struct *vma = find_vma(current->mm, address);
>>>> +	struct vm_fault vmf = {
>>>> +		.vma = vma,
>>>> +		.address = address & PAGE_MASK,
>>>> +		.flags = FAULT_FLAG_USER,
>>>> +		.pgoff = linear_page_index(vma, address),
>>>> +		.gfp_mask = GFP_KERNEL,
>>>> +		.page = page,
>>>> +	};
>>>> +	if (page->pgmap && page->pgmap->ops->migrate_to_ram)
>>>> +		return page->pgmap->ops->migrate_to_ram(&vmf);
>>> How does this synchronise against pgmap being released? As I understand things
>>> at this point we're not holding a reference on either the page or pgmap, so
>>> the page and therefore the pgmap may have been freed.
>>>
>>> I think a similar problem exists for device private fault handling as well and
>>> it has been on my list of things to fix for a while. I think the solution is to
>>> call try_get_page(), except it doesn't work with device pages due to the whole
>>> refcount thing. That issue is blocking a fair bit of work now so I've started
>>> looking into it.
>> At least the page should have been pinned by the __get_user_pages_locked
>> call in __gup_longterm_locked. That refcount is dropped in
>> check_and_migrate_movable_pages when it returns 0 or an error.
> Never mind. We unpin the pages first. Alex, would the migration work if
> we unpinned them afterwards? Also, the normal CPU page fault code path
> seems to make sure the page is locked (check in pfn_swap_entry_to_page)
> before calling migrate_to_ram.

No, you can not unpinned after migration. Due to the expected_count VS 
page_count condition at migrate_page_move_mapping, during migrate_page call.

Regards,
Alex Sierra

> Regards,
>    Felix
>
>