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;
From:   Alistair Popple <apopple@nvidia.com>
To:     Felix Kuehling <felix.kuehling@amd.com>
Cc:     Jason Gunthorpe <jgg@nvidia.com>,
        David Hildenbrand <david@redhat.com>,
        Christoph Hellwig <hch@lst.de>,
        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, willy@infradead.org
Subject: Re: [PATCH v6 01/10] mm: add zone device coherent type memory support
Date:   Fri, 18 Feb 2022 11:59:37 +1100
References: <beb38138-2266-1ff8-cc82-8fe914bed862@redhat.com>
 <877d9vd10u.fsf@nvdebian.thelocal> <20220216020357.GD4160@nvidia.com>
 <6156515.kVgMqSaHHm@nvdebian>
 <98d8bbc5-ffc2-8966-fdc1-a844874e7ae8@redhat.com>
 <20220216122600.GG4160@nvidia.com>
 <bf16195e-2570-3687-2b53-3f597ebfcfec@amd.com>
User-agent: mu4e 1.6.9; emacs 27.1
In-reply-to: <bf16195e-2570-3687-2b53-3f597ebfcfec@amd.com>
Message-ID: <87y229askb.fsf@nvdebian.thelocal>
Content-Type: multipart/mixed; boundary="=-=-="
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MFdSTFVkcUthSFJoR0I1OUNTajFQNE5BcFhEQmNycENjOGR4WWdMWm9BbXhX?=
 =?utf-8?B?dEZKd1NzaXZBZm04YlBVNnZ4eGRQVndZWmZGSkVWdm5zSGNLWE94ckt1QVJ1?=
 =?utf-8?B?UzJucEg1QVVEKy9kQVJsQnMvZHRtcVdPOVF3MldXWTU3Q1hJQTZBTnNNS0hY?=
 =?utf-8?B?NVZubGRYLzg2VkVsTDN1akprZ2RWVTIyenNtRVN0cUpqVVZkN3djL09Sb0Nq?=
 =?utf-8?B?UFFDbXlKS09tbHpaZ0p4MlFNNGl0SEZ0bkN4QnpJaHd2TW9ucWdTVSt3cEdw?=
 =?utf-8?B?ZkRjTlF4S3BDbkd6QWV4RmdvRFl2c2dDMlowVTJVZVN6eDlOa2QyU1VzSllY?=
 =?utf-8?B?QWdwUzFMQVplMXE1RVJMeU4wMDlveU5YOXAybXh3cVNEaUhiSElmVDNlbUNj?=
 =?utf-8?B?cjZpN2ZTZEdIVTN2N1pKb2xQU2hDMGpuU2ZoRnltbis4N1F5Rlp3M0Q4cjl6?=
 =?utf-8?B?dHpXMzk0VU90amZmaHM1aVRpNm50YitOajNOYWhsMlpCTUducHNWTkpMRk5N?=
 =?utf-8?B?dEM4VEkwZmx0c0tweFRuQjA3b3o0bnd5OWxBVlNRbTNST2xPRElTZDdQSjd1?=
 =?utf-8?B?ZjVqZGRnaXJkaWVGU1F1U2RtaEdDa01xVUZGKy95bE9IazNLL0k4WHpNV3Jm?=
 =?utf-8?B?MjdrVFIwL1FCcER6WEJYYlRobllTOTZ4UjFiaVg1blh3ZnAxeGt6S1FTaktW?=
 =?utf-8?B?cldrbXQ1VmU3T2o4QzJjZEc1aDRsRVpyOVJIUE1acUVyVkdZOUVBU3hheGRJ?=
 =?utf-8?B?RVlIVVVwUnRhQ0lLelhTNW9zaTVhcy9uUS9mTnNQRzI4V1NFZ0ZBZjlMS2Fy?=
 =?utf-8?B?R0dkc0lFczdRRDBYMk44Ry94MHlLb0pEemtWWHl1VlVCM1dOVkIrMEJxV1Bo?=
 =?utf-8?B?U1FLa3RVaS93b05XMUR1Snl6T0hmZUFvZ0hTNWxDK212VXVKUWVPZ3RHOTB5?=
 =?utf-8?B?bTJSa05FeWN3ZGJxTHZhdGRVTythUDREc0NMbHNsOTg1OVM3QlFzV1FuV2FU?=
 =?utf-8?B?ZlBCVy9jT3A2dnFZemRDMy9LbHdaMHFXUHQ3SXVhcU5vby9HdVpIRDNCaGEv?=
 =?utf-8?B?eEhOeHJyNmczTlNqK1A3engza3V2Y3pzVWJqUmFLY3NNRmdtVXBrR2xGRlly?=
 =?utf-8?B?L0dFdTBhNnJxcmh6U0NmcmptTkdvYk5weVBXc0RHSENjYSs1T0xFQ2gvN1VE?=
 =?utf-8?B?RlZHQXhQaHNwS1p4MGdFTU5mbkFvUC9zaDk0UCtsdWIwd0pRSnpsRzV1c1U3?=
 =?utf-8?B?QzBtZ2dUa1ZITUl4emoxbVBQeEovaURuRjF3YVJJV0xNNzUvSFk2SHR3b21n?=
 =?utf-8?B?d2hSdUQzK1ZVOTc2aVBEdHh4UW51Q2R3dThET01JNmpnN2FRQ3FCYTVLNXNM?=
 =?utf-8?B?dE5VaklES3plZWJrSGFzaXpDa2g3eFBTMTJoQzJmaXR1eXpqY1ZyVGpGeTcy?=
 =?utf-8?B?VGNoelRyTXBQSWg1ZUFEMiswbHVtZXNqYTFLU0ZXMy9ZRk5GUGxSYXFMNkJK?=
 =?utf-8?B?YmFtR05uRVBaKzZJRHFKVlpPdUtUYzNRY1FCdURJWlJBYjhWMU5UZ2tMMjBO?=
 =?utf-8?B?cGprd2lkU3VUT2hJNUZLaGEvdmlYaWQ0RVhka004cFo4eld4Nzh2ZytRL3pW?=
 =?utf-8?B?MmNSaDJaVmNGRDZPVUtrQ3ExVWQva090QW9aMW02bzREQnk1UU9sSWE4bmVX?=
 =?utf-8?B?Q0F5elJsc1NOZjYzZjNmVzlSM3gyV1NCczFuVVdRNDNGSjV6eFllU1gwbDE0?=
 =?utf-8?B?SEtmKzVaM3lqa010WDhodzhRQlBwa291RW11bVRsTk9KT0VWRllCbWM3S2Mr?=
 =?utf-8?B?RFVSOEZzaGdENG5pYW1SSXZDLzFzWjZoOSs4Y2lWTTJsaE5CNjdSUGRUVFJS?=
 =?utf-8?B?azRsQnVjU3dRVzMzNHVFaGV3My9JMW5nQ3dkbHdBRDBvWlpCTzlSQnNRb044?=
 =?utf-8?B?WWEyVFl6cVFORjJIRW83bXVDS3hOcytTQ1RTbDkxVU1JS044ak1Oc0I0Mmxs?=
 =?utf-8?B?bGNQK1E4WE5Kd0dnekhOVllOemhqd0hYMnZRelFHVXpPQ0x3MDRrRHlkRTVG?=
 =?utf-8?B?WDUwVlVIMlFNZE9rK0V0eU95YXpQTXFIMFJ3Qkc5UW5IdnJMWG53TG5NTDJu?=
 =?utf-8?B?RHNJVmpTT0tzdytBckRHR3dEd1FVWWJQOXVOZ05FdlZwVFpqTHR3VS9VbnNn?=
 =?utf-8?Q?7fIpfaNfkoXiq6araxzfHto=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d6e61b08-0bf2-42f9-a08e-08d9f27b6210
X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB3176.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Feb 2022 01:09:59.7291
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: oz5aC1I4Gm72YzZtfcVMmDU8vcB+SoplxNgJ8R+x37KHgNOybSQViuJ7MuaAqn00FKoc0+Sma2Un9OnOHXpZHA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4137
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,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

--=-=-=
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Felix Kuehling <felix.kuehling@amd.com> writes:

> Am 2022-02-16 um 07:26 schrieb Jason Gunthorpe:
>> The other place that needs careful audit is all the callers using
>> vm_normal_page() - they must all be able to accept a ZONE_DEVICE page
>> if we don't set pte_devmap.
>
> How much code are we talking about here? A quick search finds 26 call-sit=
es in
> 12 files in current master:
>
>    fs/proc/task_mmu.c
>    mm/hmm.c
>    mm/gup.c
>    mm/huge_memory.c (vm_normal_page_pmd)
>    mm/khugepaged.c
>    mm/madvise.c
>    mm/mempolicy.c
>    mm/memory.c
>    mm/mlock.c
>    mm/migrate.c
>    mm/mprotect.c
>    mm/memcontrol.c
>
> I'm thinking of a more theoretical approach: Instead of auditing all user=
s, I'd
> ask, what are the invariants that a vm_normal_page should have. Then chec=
k,
> whether our DEVICE_COHERENT pages satisfy them. But maybe the concept of =
a
> vm_normal_page isn't defined clearly enough for that.
>
> That said, I think we (Alex and myself) made an implicit assumption from =
the
> start, that a DEVICE_COHERENT page should behave a lot like a normal page=
 in
> terms of VMA mappings, even if we didn't know what that means in detail.

Yes I'm afraid I made a similar mistake when reviewing this, forgetting tha=
t
DEVICE_COHERENT pages are not LRU pages and therefore need special treatmen=
t in
some places. So for now I will have to withdraw my reviewed-by until this h=
as
been looked at more closely, because as you note below accidentally treatin=
g
them as LRU pages leads to a bad time.

> I can now at least name some differences between DEVICE_COHERENT and norm=
al
> pages: how the memory is allocated, how data is migrated into DEVICE_COHE=
RENT
> pages and that it can't be on any LRU list (because the lru list_head in =
struct
> page is aliased by pgmap and zone_device_data). Maybe I'll find more diff=
erences
> if I keep digging.
>
> Regards,
> =C2=A0 Felix
>
>
>>
>> Jason

--=-=-=--