Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <651099d6-21ae-16a6-e500-a87002468cda@amd.com>
Date:   Thu, 10 Mar 2022 16:58:05 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH v1 1/3] mm: split vm_normal_pages for LRU and non-LRU
 handling
Content-Language: en-US
To:     Matthew Wilcox <willy@infradead.org>,
        Alex Sierra <alex.sierra@amd.com>
Cc:     jgg@nvidia.com, david@redhat.com, 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, hch@lst.de, jglisse@redhat.com,
        apopple@nvidia.com, akpm@linux-foundation.org
References: <20220310172633.9151-1-alex.sierra@amd.com>
 <20220310172633.9151-2-alex.sierra@amd.com>
 <YipQqqpTz8hZAbLZ@casper.infradead.org>
From:   Felix Kuehling <felix.kuehling@amd.com>
In-Reply-To: <YipQqqpTz8hZAbLZ@casper.infradead.org>
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?U0pkLzFNd0tIODBuYlVBUE1HTFdkUU8xK2V5RENsb0xFUmcwTi94UEluQTBj?=
 =?utf-8?B?QVRETmxKeDYyQU5sUnBCQXZIQVhna29ESDVOL0xKU21rbFlaWmh6dFVDSjlB?=
 =?utf-8?B?WFpnMHNUMVQ0bVVBMS82RVN6cDdaMjFnSGxtSnlweldFbDFueUQvL2wxb0I1?=
 =?utf-8?B?enRIVk1aRGxUeHlqZHBlVjhITTduT0xVUktnVEtpRERaMnBET01hZGFXQXg1?=
 =?utf-8?B?TTdNSlU5bUw2WDUwamwyZDQrUmwrVTlya0x5QVEzZlUxZktITHZhSy9XTXgy?=
 =?utf-8?B?TFcyVE0yYUxVSHlZdGdjT1o4citxQ3MrUEpoSVBORTVuckRxZ1diVW4zTDd3?=
 =?utf-8?B?eXNSUjFRaVdEODVxYmxHTngwYjZ2b2krT3Y4c0dQOXQyMTZlVnViWVp1THpk?=
 =?utf-8?B?aERjRkswL1dPajZUMGhvSU9KNW5hd2ZsWC80UHVMQld3UjdjZDIzdlE4ME9T?=
 =?utf-8?B?U3dPaFZVV3c3N0hsNVAySUxsOGFZd2lHdDh2dWJkRkpaUGpoZkJqSUZEQXVE?=
 =?utf-8?B?VDRTVm5pdllFL2VFZDFObmpocFh3L2Z4cm5xRkJjZ2ttbVpUM3htMDBRNUkz?=
 =?utf-8?B?SzVQQ1doTnd5MHdOUytuY1BMa3RaR05uZWdKM1VVa25zeFdGc3R1MERtT3p0?=
 =?utf-8?B?eHAreUNhVGE2YXdZVXVGMW5SS3FVSGxZUCtKNkNjMlhsU2I3Mm9SM3dIVkhO?=
 =?utf-8?B?TDZCQjJJMHdETmZCbWdYZWt2elY5bmdkRXhtV0MreVdCbW1pZkJRWDZwd0dj?=
 =?utf-8?B?RjJVMVJVWEJKUHRWLzFJUjNiSmdpeHRvQ2NuVE85VzRCTkk0ZCt1Tmkzb3F3?=
 =?utf-8?B?czhuVTkzR1VYVDZZTWFtL0JZamZSdC9id3ZsL283akFRUkNHc1h3Sm8veU4y?=
 =?utf-8?B?MjlzelFyNy9QcStvV1lmZ0RlaVVHc0c2WXN6eFliUjkrb3VWN1hIRS9wVlF2?=
 =?utf-8?B?YnhqdTRMSkR6bkN6U3ZaWHRlQUkwa0RPRVJOUVlHaE81WmFNOTlJcDU2eXQv?=
 =?utf-8?B?VDJmVnBtekVCMDl3RU16RVl5NDUwUG04K1BYeU9Ud1dZdGRRVE5qVmlFVGpt?=
 =?utf-8?B?MFdjWmlidVNWTWlDZVFjUURVVXJHaVFIYzdXUlRoQXhRdUltRHJXcUt1a1dW?=
 =?utf-8?B?REZoWkRsTmVFbkpST0pxNk01ZEdFM1pBNVpMeUNvcXBncitVMVA4ejNqZ3dE?=
 =?utf-8?B?ejFJbkpaTDcyOTM1b254ZXVrMll4NWVDWi8vZUdPUVRYcGgwdzdHd1lrdnJh?=
 =?utf-8?B?WUl3aHNCVHViZGh4Z2ZpNnd6OFQrMzhULzR4ZmRzSzNpampqUktreEsrWDRi?=
 =?utf-8?B?YkFvMmlMWktBUmEwMCsrSE1MaGhmelMrWG56ZThnRzZtQXQvVDg4RWQwV2Ur?=
 =?utf-8?B?MjNnSWw2R0xHSWE5RUNoTHpmVUdPdVMwZitMREdKVnNQM20yQ2tpY1ZQNWVa?=
 =?utf-8?B?RFJBU2hrUmNRMDZQT21veVNMMmkxUG9YdEdPNUFHM2pJRDZld1A3K3IzaFhj?=
 =?utf-8?B?aWlxbVNWUTNUQk93dUgvWkJ1VXFLaU1zNllMZnpQWi9qcmlOcFJHOXlyYjhu?=
 =?utf-8?B?R29mYjdELzJSYlBXZWxHZzQ2ZDgvY0s3MzQzUzJzUzRHa0l2MmJLVkhYRXF6?=
 =?utf-8?B?bWdlSjRBcGxNLzAvRTlLOFdDdTNkdFMveStwSDdBRjVrRGxJeGp2SVlUMzlw?=
 =?utf-8?B?ajUydy9kd1Mrd0xxYlUvcmRQd0J0MFVCTzhxUHZJK0xrc3JoWWJFVncwcW5k?=
 =?utf-8?B?SUQzaVZHN1cxZExORzlFVmpvQ3g5ZFl2UUpZOFlXb0hKN21ZTlk2ejNMcEUz?=
 =?utf-8?B?SndoY3h4eEdSMlArUnhVYjUwN24zNFBWWXRieStMQUcvR3dPbVhzQm02Y3BW?=
 =?utf-8?B?VHNjVVQ1MjVlYWpWd1FhSEtHTkFldmg3SnFNampTZVhQbWtDMjdHL0dETUtX?=
 =?utf-8?Q?Xzee7lFuVpLePmot8ezAv5pVqIhPGkwM?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0f5dbb83-4109-42a2-7ccc-08da02e10f2e
X-MS-Exchange-CrossTenant-AuthSource: BN9PR12MB5115.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Mar 2022 21:58:07.4291
 (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: +9Z+UMcgb70fml3MUG0P4z06g5ICIqZOVBEIBupFqTniLsVJuHteq6oHhfZ0I5j/fYNESpOkRCrTv7rElqP75g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5039
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham 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


Am 2022-03-10 um 14:25 schrieb Matthew Wilcox:
> On Thu, Mar 10, 2022 at 11:26:31AM -0600, Alex Sierra wrote:
>> @@ -606,7 +606,7 @@ static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr,
>>    * PFNMAP mappings in order to support COWable mappings.
>>    *
>>    */
>> -struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
>> +struct page *vm_normal_any_page(struct vm_area_struct *vma, unsigned long addr,
>>   			    pte_t pte)
>>   {
>>   	unsigned long pfn = pte_pfn(pte);
>> @@ -620,8 +620,6 @@ struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
>>   			return NULL;
>>   		if (is_zero_pfn(pfn))
>>   			return NULL;
>> -		if (pte_devmap(pte))
>> -			return NULL;
>>   
>>   		print_bad_pte(vma, addr, pte, NULL);
>>   		return NULL;
> ... what?
>
> Haven't you just made it so that a devmap page always prints a bad PTE
> message, and then returns NULL anyway?

Yeah, that was stupid. :/  I think the long-term goal was to get rid of 
pte_devmap. But for now, as long as we have pte_special with pte_devmap, 
we'll need a special case to handle that like a normal page.

I only see the PFN_DEV|PFN_MAP flags set in a few places: 
drivers/dax/device.c, drivers/nvdimm/pmem.c, fs/fuse/virtio_fs.c. I 
guess we need to test at least one of them for this patch series to make 
sure we're not breaking them.


>
> Surely this should be:
>
> 		if (pte_devmap(pte))
> -			return NULL;
> +			return pfn_to_page(pfn);
>
> or maybe
>
> +			goto check_pfn;
>
> But I don't know about that highest_memmap_pfn check.

Looks to me like it should work. highest_memmap_pfn gets updated in 
memremap_pages -> pagemap_range -> move_pfn_range_to_zone -> 
memmap_init_range.

Regards,
   Felix


>
>> @@ -661,6 +659,22 @@ struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
>>   	return pfn_to_page(pfn);
>>   }
>>   
>> +/*
>> + * vm_normal_lru_page -- This function gets the "struct page" associated
>> + * with a pte only for page cache and anon page. These pages are LRU handled.
>> + */
>> +struct page *vm_normal_lru_page(struct vm_area_struct *vma, unsigned long addr,
>> +			    pte_t pte)
> It seems a shame to add a new function without proper kernel-doc.
>