Message-ID: <3f8a48d6-7ee7-fb30-5942-29054c34aac5@amd.com>
Date:   Wed, 8 Dec 2021 12:55:26 -0600
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.2
Subject: Re: [PATCH v2 03/11] mm/gup: migrate PIN_LONGTERM dev coherent pages
 to system
Content-Language: en-US
To:     Felix Kuehling <felix.kuehling@amd.com>,
        Alistair Popple <apopple@nvidia.com>,
        akpm@linux-foundation.org, linux-mm@kvack.org,
        rcampbell@nvidia.com, linux-ext4@vger.kernel.org,
        linux-xfs@vger.kernel.org
Cc:     amd-gfx@lists.freedesktop.org, willy@infradead.org,
        jglisse@redhat.com, dri-devel@lists.freedesktop.org,
        jgg@nvidia.com, hch@lst.de
References: <20211206185251.20646-1-alex.sierra@amd.com>
 <20211206185251.20646-4-alex.sierra@amd.com> <2858338.J0npWUQLIM@nvdebian>
 <9fa224e6-d51a-d3cc-575b-3d56517a331a@amd.com>
 <2b996383-ebe6-e9d8-d794-58ecfd4a16e8@amd.com>
From:   "Sierra Guiza, Alejandro (Alex)" <alex.sierra@amd.com>
In-Reply-To: <2b996383-ebe6-e9d8-d794-58ecfd4a16e8@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TnUvcFFoWGlkRGkrUjdBTlRpNXlEZUlUTDdEQzkyd0RraFM3TFdNVU9qa1Ro?=
 =?utf-8?B?WkZXazNCcHQ5WDFhVjFmK2xoZVNTODZRQ2gwR0NxRkJOeU41SEMxa3o5QmJN?=
 =?utf-8?B?S2RIcXhnRnRWUkRXVzdWenE4WUpYdWhzdjdYMll4c0lKTGlHMFg2d2cxMG1D?=
 =?utf-8?B?ejdYWjhTalh6WGppME5mLzVVK2MvUVJHdE1obDN1NElNWDMxZGZMbDlyVU5Q?=
 =?utf-8?B?RFpLVXFwME9NYmJVMlBXT0Q0Y0RnUStJSjFBUWIyWVJ6cys5Y1hINFAyUHJU?=
 =?utf-8?B?LzdzYWlBd1JuelBzZk93anZqSzRTazlmd0hFTHp1Y2ZFZURWZ3haRnNRcXJ0?=
 =?utf-8?B?dnVpOUdGQlk1L2wrZlBqa0R5ZG93Nkd5VUpKeWRHQkhMbEU3REdRRUVwSkY3?=
 =?utf-8?B?U0ZUV2dDK3U1LzIzblAvOU9FbnY0Mmg1VFIxMHhTSkEwR2J2Skw0SFNORUdw?=
 =?utf-8?B?RDBjTllkc1JhbjRrNlN3NEJBcjJoS2l3WldORmptS2VkbEtlM3lGUjBHUGhy?=
 =?utf-8?B?aTFJNnJtKzBsK1F5Q2lGL3phRzRsMGVJUzEvMHNNMyt2SUdEQzc0SXhwMHFP?=
 =?utf-8?B?MHJSMkQ2QkNzd3pvK2haUUp2Z1pSRnpuOUVhVkhRMkwrTVJYbzVCem9iZXZ4?=
 =?utf-8?B?TkxLOWJkK0F3VWZ5VkgreS9wV2lxWlpUd2lTNG5EcGRyYWxMK3UxUmxwM2hk?=
 =?utf-8?B?MnpYRVRJa25CQXFnd2JYUlhJUkEyNHJ1NzlMSXI1akZxc0Zwb1BpZVcrWGZU?=
 =?utf-8?B?SmhTU1IrVmFLZkhGVEFOa0FUZVdPSXdHQnJPd2NCN0lzZEhMakFCSUhVbDZt?=
 =?utf-8?B?OWtKZys0cG83QTFvV3F1MGthK1lIeVJhUmJGYnc2ejlMWnRJejdkazRPU3Br?=
 =?utf-8?B?dmxDTStDSmNLOTlzNlB6aitIdjhrcGRFUTRxYmc4VWVnbFBjUmo0cVRiL0Zx?=
 =?utf-8?B?eDdkam5DQnpmTjh6bmRqOElOT1RRK1lQVkRQek1wRU1Pd0FQMDU2cjRwU2c4?=
 =?utf-8?B?L1VoZlhaME9FQUpVTnlib2I3S05LOW05SlBsSEZNOEZtZFdNaHdKSlVyVElm?=
 =?utf-8?B?QUJGL0VQQXZzdEZNa0VtQVNuaTNsTG1rOG9KUW5oMFp3eFZIR2lHU290Wk5p?=
 =?utf-8?B?ZW9JZ0F0MHN0amZ3bFE1YWdrZk0rTXQ5MWFMTEpRNUxGZzJzLzlUQ1dCUXlI?=
 =?utf-8?B?MXNrMlU2Wmpyelhxendaei8zNmlQYURlTWlPcmpWVUZwTWpKK0FrWEU2Rm1M?=
 =?utf-8?B?VVZXdWYzM0MrajNSeXd6L1JRdU5HMmU4LzgwQ1NKdVNLMmtsQTVGV3g2K1dj?=
 =?utf-8?B?UU4rNTBDeDhYR3p6bjRQcDc2eUZSV0h0Y0dTb0VKWlF6MTJLWEp2L1IyN3hS?=
 =?utf-8?B?bGxwcHlMN2FvRDZ5cG1UeXZXWjVHUFk3bEgxazM3bVpNQnNQOHQ4Q1VhYzhZ?=
 =?utf-8?B?OFhOdm1JaitpWStqS1dMRWJteDJqemJyR091WFc0eW1rdXFweTh1SmhiYlln?=
 =?utf-8?B?alRmSitTeEhDNE5WQmNHNlFHQlRkd1BCbk12VGdPM2duVHFicHU4WlMwZ0xJ?=
 =?utf-8?B?ZldxTWQzb2l3L0pJMWVqeWR2UUhiZkFLa3RZb1NsaE40R3oxSHE3NVFPK29C?=
 =?utf-8?B?SGIzVjd6ZmwydktKQmJVcTArQlF5MjhBeGthdkcyOVR5OUFadldiWUgvb3Aw?=
 =?utf-8?B?TzMyMGJZejJDK2FSeGl0Rmd2L1NEOTBIZ3RHZ3E5OTI3MEJmTXBZaTYzcjlt?=
 =?utf-8?B?eTVDUmRwd2lHRlUrTERIRUtRbnRzcWJBWEoybHFJcEhmLzVZZ1JyV2ptOHlY?=
 =?utf-8?B?TzB2K0FQc2lSdU8waGp3Z2RGVnkxN0Vtai80U3R0V3lIVVhSbG9WeU9YQXBv?=
 =?utf-8?B?eUNrTHJjMXVYWWFtTXNTeVdESzllYWY0NjEzR2lqWmhDaTI2WFR0Mkk2WHBK?=
 =?utf-8?B?Q0xDME96YmFNMFhSb2ZlNUV4S0thd1VHNWFJRmNQb2pxTFFGOUtORTlSNmtK?=
 =?utf-8?B?RVQ4eWNSOXlKbHkydkMyRmN4dGNMSGNHTGE0OTRNeWdEb2txVFkvdTloc1VK?=
 =?utf-8?B?YTIyNUNmcE93Q3JBZjIzdVZmVWhDOTFHb0k5SEhNUVM2OWsrTjl4RDBCK0VN?=
 =?utf-8?B?akU1Vy96WFAxaDA3bEFnZDVxMVJKU25EMkhYYkc5b3htM1ZxODh6L0dQL1dE?=
 =?utf-8?Q?tFSGeaCe+8b5e9yPB1hUfio=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1b8e9e90-beb2-4149-db5a-08d9ba7c4ef4
X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB2717.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Dec 2021 18:55:31.6254
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: MoyvTilro8opjzyep8DPXXlj3eXdDfnn5ji5jb9VRJFz4DJX0skvcPzNNcwUsE9WwCfX543lA+06xAtKmpbxwQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR12MB2413
Precedence: bulk
List-ID: <linux-ext4.vger.kernel.org>
X-Mailing-List: linux-ext4@vger.kernel.org


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@amd.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
>
>