Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <002ad572-4d32-7133-06f3-aa680c297be2@amd.com>
Date:   Tue, 15 Feb 2022 17:49:07 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH v6 01/10] mm: add zone device coherent type memory support
Content-Language: en-US
To:     Jason Gunthorpe <jgg@nvidia.com>
Cc:     Christoph Hellwig <hch@lst.de>,
        David Hildenbrand <david@redhat.com>,
        Alex Sierra <alex.sierra@amd.com>, akpm@linux-foundation.org,
        linux-mm@kvack.org, rcampbell@nvidia.com,
        linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,
        amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
        jglisse@redhat.com, apopple@nvidia.com, willy@infradead.org
References: <20220201154901.7921-1-alex.sierra@amd.com>
 <20220201154901.7921-2-alex.sierra@amd.com>
 <beb38138-2266-1ff8-cc82-8fe914bed862@redhat.com>
 <f2af73c1-396b-168f-7f86-eb10b3b68a26@redhat.com>
 <a24d82d9-daf9-fa1a-8b9d-5db7fe10655e@amd.com>
 <078dd84e-ebbc-5c89-0407-f5ecc2ca3ebf@redhat.com>
 <20220215144524.GR4160@nvidia.com> <20220215183209.GA24409@lst.de>
 <20220215194107.GZ4160@nvidia.com>
 <ac3d5157-9251-f9fb-a973-f268ce58b4e0@amd.com>
 <20220215214749.GA4160@nvidia.com>
From:   Felix Kuehling <felix.kuehling@amd.com>
Organization: AMD Inc.
In-Reply-To: <20220215214749.GA4160@nvidia.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?ZCtPb2RxbEo4Q3l0eG1FQ09oVktYOGxyZkI1NW9XSzlJNnVTUFBYNkZIc2RX?=
 =?utf-8?B?djNSTFVpQUs1TDcwNGZsUDFNZG83K1h4NGdId0l1ajNGUW1OU3NHdXJCTmxt?=
 =?utf-8?B?RGhoSlBvVFFVUW04bllhZFN5M3g3MWU0L3BqSVZkdzlNMStRSTdlMGJYUCt6?=
 =?utf-8?B?SXl4RU0zTXYrVC9qSENqVnRRczhyaDdKejFnUU8vZkJ4STJrbEYzMkxCZkEz?=
 =?utf-8?B?SUZtUmZFWUYwdFdnSmk1N1IyYjJncklvR29GUCtvWElmTzl0OGZSbWg1ZGFM?=
 =?utf-8?B?VFM3MkszbTkyTnhCMTlZeTROKy9IN3NLUVVOU0hELzVYWmYzUXQyRlZLcXgy?=
 =?utf-8?B?UFJPYTVVaXJzVHZXR05NV2RvdEhSZ1dtQmVHRy80NWd3RU5xaGRUbzY3M010?=
 =?utf-8?B?cHRsK2J4blRhTk51amg5Nm01K1h6eTR4elBMRFJocEpvZ1NkN1l2cURWZ2tC?=
 =?utf-8?B?Y1orWDR6QTdLWitQMVdrMUV1dWQ3ckYzeWNKM0FQcnRjaDNlc3BXcEtzOTdq?=
 =?utf-8?B?RjMwQlpmOEVLZzYwZFI5WnFlb3ovY3owZ0FyeVlySDFjY2dBQUE0WXdYZ1FP?=
 =?utf-8?B?eU9QNmpHTUJEUU54Z0tqZEIyNURJdFNnQkRVa2NUMG5HUTdmZDJsKzJYcXlH?=
 =?utf-8?B?RGViT3h5dnVHTk10M1lUMlJVblloRDdvTW03UHU1VG1BNmlEUHR4WmZCSUwx?=
 =?utf-8?B?NndOQ3lsOGVxN1BkVTVhbGpTVGlYSGJuTVBvcmV0RGl5eE0waWxGOWRzdGlZ?=
 =?utf-8?B?SlVjV3RXN3dxMmR1Yk1TWW1ZSWxHOWNWbVVFVVd3TGV1dzBjVlR1RHMyT0pP?=
 =?utf-8?B?UnZncXMxbzBCeTFPTzZoaG0yQmQyRUYzQ0VyYzF2YzQ1dGlDTGVCaGhyUm5t?=
 =?utf-8?B?RkdBYUx4eEZtTlphanFSb1Mwa0dXc2RhU2hNSGpnTUlLY1R1VHg2QVhLeGlX?=
 =?utf-8?B?ODN0UE5iRjVhV0RKcEtqZFhQSkcrbnZoSS83bERFQ01HSDRBNzk0bEw5VlND?=
 =?utf-8?B?QnV2YmdZanB0Umw4S1F6MVh2V1h1ZUFJRDR4SnJxWUZQQTBXRFR3L2FGNlZj?=
 =?utf-8?B?Q2p6cTNUdGJYZzJldVllbXNpa1VXN0Z0djQrcG02ZUZ2ZFYvTkZ3T2lkV0Rp?=
 =?utf-8?B?djQ3MTlUZ0pzS2gvSGtUSUJjTjBubzNvWVVSYVROdFc2Q2pTWTlHdmFQaFhz?=
 =?utf-8?B?dU1qeU9qK0VLRVhUOHR1MjBXZHdkR0dIYkZIVEFNbU9Gc1JWaXNhYlAxdFdY?=
 =?utf-8?B?dU5BVnE0TEtPNWk1bWFacUNNeUtmdmgwcFduRHdxR05yVXltR3ZYTGR6MWNQ?=
 =?utf-8?B?dzJNUGJ5dWlpbkJLVG9sNjRPd3BmbzllOFMrd2lKc1hkOW05dGluTkxSMDBX?=
 =?utf-8?B?UXNiTVBvQnlxMEZxTmZwdWhtaHdqRDNYVDVRbUVqWXExRFNVYTR0bVVKM0pI?=
 =?utf-8?B?eWJKZjJ2bFVvaENuSWlKSHlrWFVsOFdXZTd3S3FkZWFNVGd4TVp3OHBKYkdj?=
 =?utf-8?B?cjlPZW9HUDNReDJLTlZTRTJ4UEN2QVRxbXBSQ01aY2JBUjJMdnI0aDNTZEVm?=
 =?utf-8?B?bDdtRGcyTGx0SGptNGU5TklJYkYwOC9wdXJiYmlOa3RvSjI1TlZWTklZendt?=
 =?utf-8?B?c1dzMGVJaTVnam9Eb3FDRE5nSko2eUQ5SGtxdU04anhOSytCRjRkQ0RaeW5t?=
 =?utf-8?B?UW5zMmxLZjhmb05ySXBZR1NMQlBOTzEwUUhCRnUwbXNSajdSRC9ZVWtWdm82?=
 =?utf-8?B?d3JvVFZJNW43bHd2eDFMU1ZJMVRMS2cvYnQ1TklXLyt6bURsWVB0Tzd3RlFJ?=
 =?utf-8?B?R0s1N1V6QmNRU3UzckQ0MjdkMnc1REp5RGtmUjNwbXhRTDJmeFpJa0QrZDBq?=
 =?utf-8?B?NDM5NWd0cXVFZHBzWVd2UytYOVc5ZGRxYjZseFBMVVd4elFvbmJnRWlMVkRH?=
 =?utf-8?B?N3RQRTVBMDFIN2RCWkJTc1JzVXJSY0tWVCtvMlF4bVA3ZmlWYk5JUk9QSzZJ?=
 =?utf-8?B?bFFiU0tLZFFxeVZodzZNMlcrSGQ3QkhuOXZQNmxFaEFDUXQvcTZwRCtNRDF2?=
 =?utf-8?B?aG5sdDVnbUpVU2VkK2FpRGUrMDFFT2dsSHRoNnN6aHpRTmI1UWtsYzFTdEJI?=
 =?utf-8?B?all3a3RIdHVIL3pzQUIyUmMwRURvdGZUczhoVXVPZktHN043VjQ0aitVNjNK?=
 =?utf-8?Q?mFNip+ddCxkKXH8Z0iLcvRE=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 77273886-dc0e-45d2-32f4-08d9f0d560a1
X-MS-Exchange-CrossTenant-AuthSource: BN9PR12MB5115.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Feb 2022 22:49:09.2637
 (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: QONoPdzKWv3FanV2/r5pa94Ql8HjBuoFdOTpYjpAcz3JcHbdqgmrnbTATbiWUPOIVqHhoYfBOBESMbgRrDXcDg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4254
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-ext4.vger.kernel.org>
X-Mailing-List: linux-ext4@vger.kernel.org


On 2022-02-15 16:47, Jason Gunthorpe wrote:
> On Tue, Feb 15, 2022 at 04:35:56PM -0500, Felix Kuehling wrote:
>> On 2022-02-15 14:41, Jason Gunthorpe wrote:
>>> On Tue, Feb 15, 2022 at 07:32:09PM +0100, Christoph Hellwig wrote:
>>>> On Tue, Feb 15, 2022 at 10:45:24AM -0400, Jason Gunthorpe wrote:
>>>>>> Do you know if DEVICE_GENERIC pages would end up as PageAnon()? My
>>>>>> assumption was that they would be part of a special mapping.
>>>>> We need to stop using the special PTEs and VMAs for things that have a
>>>>> struct page. This is a mistake DAX created that must be undone.
>>>> Yes, we'll get to it.  Maybe we can do it for the non-DAX devmap
>>>> ptes first given that DAX is more complicated.
>>> Probably, I think we can check the page->pgmap type to tell the
>>> difference.
>>>
>>> I'm not sure how the DEVICE_GENERIC can work without this, as DAX was
>>> made safe by using the unmap_mapping_range(), which won't work
>>> here. Is there some other trick being used to keep track of references
>>> inside the AMD driver?
>> Not sure I'm following all the discussion about VMAs and DAX. So I may be
>> answering the wrong question: We treat each ZONE_DEVICE page as a reference
>> to the BO (buffer object) that backs the page. We increment the BO refcount
>> for each page we migrate into it. In the dev_pagemap_ops.page_free callback
>> we drop that reference. Once all pages backed by a BO are freed, the BO
>> refcount reaches 0 [*] and we can free the BO allocation.
> Userspace does
>   1) mmap(MAP_PRIVATE) to allocate anon memory
>   2) something to trigger migration to install a ZONE_DEVICE page
>   3) munmap()
>
> Who decrements the refcout on the munmap?
>
> When a ZONE_DEVICE page is installed in the PTE is supposed to be
> marked as pte_devmap and that disables all the normal page refcounting
> during munmap().
>
> fsdax makes this work by working the refcounts backwards, the page is
> refcounted while it exists in the driver, when the driver decides to
> remove it then unmap_mapping_range() is called to purge it from all
> PTEs and then refcount is decrd. munmap/fork/etc don't change the
> refcount.

Hmm, that just means, whether or not there are PTEs doesn't really 
matter. It should work the same as it does for DEVICE_PRIVATE pages. I'm 
not sure where DEVICE_PRIVATE page's refcounts are decremented on unmap, 
TBH. But I can't find it in our driver, or in the test_hmm driver for 
that matter.

Regards,
   Felix

>
> Jason