Received-SPF: pass (google.com: domain of linux-kernel+bounces-159672-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <db87a92d-9813-4fdd-bea5-e3d8a6838b30@oracle.com>
Date: Fri, 26 Apr 2024 09:02:31 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH RFC 2/7] filemap: Change mapping_set_folio_min_order() ->
 mapping_set_folio_orders()
To: "Pankaj Raghav (Samsung)" <kernel@pankajraghav.com>
Cc: axboe@kernel.dk, brauner@kernel.org, djwong@kernel.org,
        viro@zeniv.linux.org.uk, jack@suse.cz, akpm@linux-foundation.org,
        willy@infradead.org, dchinner@redhat.com, tytso@mit.edu, hch@lst.de,
        martin.petersen@oracle.com, nilay@linux.ibm.com, ritesh.list@gmail.com,
        mcgrof@kernel.org, linux-block@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-xfs@vger.kernel.org,
        linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
        ojaswin@linux.ibm.com, p.raghav@samsung.com, jbongio@google.com,
        okiselev@amazon.com
References: <20240422143923.3927601-1-john.g.garry@oracle.com>
 <20240422143923.3927601-3-john.g.garry@oracle.com>
 <20240425144741.houv6uoflhwmcc2u@quentin>
Content-Language: en-US
From: John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <20240425144741.houv6uoflhwmcc2u@quentin>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?QU1rOVh0U09TRG1CSSt0OENCQVkwVmFGR1RTUWRUcngvM0JDSktkeXZTKy8v?=
 =?utf-8?B?cWlOMDNlWFZhMFBLWTVETWlLa2hwUHVwK0NaTFZRL0ZZVHFMUEl5eTdFS2ox?=
 =?utf-8?B?aHR3L0J2eXRJQnhOZndBekxhRVYyeGxMUlhnRGZnS0tPby9rNkphSW8rSitq?=
 =?utf-8?B?cXJkWHJNMVFScjZsYUVIMHovWE56bWYrdXZFam0zM3h0alVmTjI0NGEwY0pm?=
 =?utf-8?B?Q0VmSDZjT3lNSmFZaGhqOEhzMnRnOURvUmNWVnRUclVEbitIWVhzekxSUGY0?=
 =?utf-8?B?anVJS0JpZnlmaEtXVU1hMFIxYWZqR2xCMHZIZmgvZk02d1c5azg5a0NuUEx2?=
 =?utf-8?B?aHlTRStMejZjUUxYMVc4dmM2TzhielpCbkRtREJRT2VFMUNPVVdQNmg5QlEw?=
 =?utf-8?B?blIycHhSZysvWDVsMjUyTmVPRC92dFhDZlJyejhudmkwZ3J5b2l6LzRsYklw?=
 =?utf-8?B?V05DcHMvZGt4T1R5b09qU0tHWXNFclhUajFhUU9oUlBQSzRCQkdBeVc3dmVx?=
 =?utf-8?B?b2Vjc2tqSFdBa3d3ay9IUStWamZNT0t1cy9tTk5wOXVCQWVqL3Q4Wkc3TlQ2?=
 =?utf-8?B?ckUyU1NueUNRM1VDaXBPSmJCekxxQnBBSHF5WXBHaklEelVGemFPZ1Vud1Fv?=
 =?utf-8?B?YUJWS2doa2I2WWNybUlTWXRlZVlsNkp5cnR4UUNscHBadmVjdDRIcUxROVJF?=
 =?utf-8?B?SXpvZG5SRHdiZDFlOFFPZmM1d01iNjhUVjB1QlJWZDJEay91VkxGVmYvdVQv?=
 =?utf-8?B?RVZiQm5kZjFxT3pHaFU1VjVzcDlJNFZoUGRGMVMybUZGYjRrMVRoNmFJNDBT?=
 =?utf-8?B?MW9ibDRBRVRmZlFwOE0vWm14TE1yMFlsWWxaQnF5RGhxRk5USXFLbkU5Qjhw?=
 =?utf-8?B?cDlWZGNFR0JUaG9TMGF2d1ZSNENMN0pJWTNuU29BcDNjbFdaS0dSS05HcUM0?=
 =?utf-8?B?ZTNlR1k4U2JoaU1wNFJCa29aN0tKWE43WnltdzVvNzBtbXVZaW91OUlTOUpj?=
 =?utf-8?B?cjN3V3A5ZXZldzJsY1RsTjhpOU9UL2N5c21Yam1SeTdRTHNHeHhMV0JnVzk2?=
 =?utf-8?B?b0xpTEdsc1V5ZldsaFplTEgzVmtZZ1VzTUE2b0FhVVhZbTNNQUN6S0ZjODZm?=
 =?utf-8?B?RXRrTnhjQTNIaXE4VVNNaUE3SVE1VDNXU2JVQ09kZXNxRHFxdTBoWWxZYzNH?=
 =?utf-8?B?STFLLzIxeWpEUnZNSWVUVE0wS0ZrU0pET3RiS2lFTlpiR0l2czhwZ0ZVOTNQ?=
 =?utf-8?B?Y29UL3RVMHBmUFVXUG0wMVNrMVF0cVFScUNLcmNGaUlBNFBDV1Q1MzgyN2FI?=
 =?utf-8?B?T0FFTkN1RFo3WWEzWUNBakhUUTBKck4walpsWEt0VTM1UWFYbFlPM3dUbE40?=
 =?utf-8?B?MUVURjhTMEE1ckZ2NnRDMzlhb0RlK1B6QUFEc3FLcWtKbXVqUmZkSStRRHFv?=
 =?utf-8?B?U1M4eXRNdzFZdjdzRllLNVFjdW9IenRzM3F2emNSRFRwU0RTTUJpeitKaU1G?=
 =?utf-8?B?VFkyK2c3R3F0M0MvVHY0Smcwa2dKY3p0T0FXSkZFZEF1Ulo5YmZSdlFkbFh0?=
 =?utf-8?B?MnBpSWFMUTZtYUtKbmhsdUxvVzlCSkFLMVIxTmp0eUg1eFBHeGFFanl2Z0VB?=
 =?utf-8?B?MC9NMXJ2WVRMNVVYVk1RQUFaMXpjRFdGSDNkK3E3NmNSMUZQdjdRT3FuaGdx?=
 =?utf-8?B?L0l2NkdIRXErZUx5QnQ4aG1lMG1aOEJTU2JvR0g2S2s1aHY0enp6R2ZCbDY1?=
 =?utf-8?B?QXpXN3dlQXI5WWZPZXgrK2VTNm5TK3hjRFFqRklkVXJmMUx1eG1hSjB5cWZ6?=
 =?utf-8?B?Mng5bVZJNDhXcTltQmFyaGNWOERJL2tZQ2R1Ym44MXRJSU50ZmltTDVRakNu?=
 =?utf-8?B?QUdINklmdTV4aEdoL0c3VTZCbW8zS0Evd1VpdEwvczlBMm1oVGxMUGFaeEdj?=
 =?utf-8?B?anBPNFIyTVdMQ0ZZbkU5UEJwYXVEbU1CMnVhVFd5emRXb1FEQTJBZlBmMDVo?=
 =?utf-8?B?eWt3dk5UOG95NHJlcTdNWlFBamU0UUNWbEdldGtYVVVkazk5NzQ4QzZFUHJT?=
 =?utf-8?B?YlNyZ0ZqUVFpakZMeDRiM1grQnJ5YXl2YXErZDA1Mnh0VHFLZ2Z5TDlZcXV1?=
 =?utf-8?B?eENld0pzaXVLNENHTDNCNktOVVFCaHJWVjVBNTVyelpQYWFIUGxXNGZLNnor?=
 =?utf-8?B?S2c9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	CGlfwlcpMy5ecsGyI1NNnFl6028onGWafui1Yrwrxat4EkuITOUl6371OhQdAzq1W33D+YewPr4hfpgS3tPm6TIHc8gmLDGaRBfRn04/wBJlv8MQuHMLbb01SOuxiqwGoTt2g3vSQAZe0Jspm2sTlRFlSRnh5KshLehsEsxKsZCuuwYl61VH+/7/OYNy09eKcPgzv6rjzSAwK8bA5sh9lr2K40GK+cAzbiVKVlLpziOSw1iL3p55vpyfLwiZ2NHUA4pqBI+Thz/rJFYmUxYbOYAVQTxbcFx9rJSnFjooueOIC0/DhF0dnYusq3CD4U41tsbPNeM79Z563WOicwIhJwn9OA/ixVp+yJrREy2qmZ04UV2kAXR2ln6FOIz5Afxp9NRUbKftAtUnTFdGUFDNdZCa5dkRUlvfdG5FxjlexzU1TP/IJoUIy09bdVAA3oFB3qr3dTDS8kMPuwayZ9yCy/2h37DJly4tptm6qMo4WQdXJuS3d2qBfOVFonZDWuXCz9hPvQWyiB5momDfwCCfEg+mSLgyWt5l+axFN6cdM1wka2vhnOmD+4vrKFQGj+CnXjIcaGphMQHaWK+OWawtpAO9ajHok15mcviigMnRGVU=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0cf0b7de-2f66-4b6b-263e-08dc65c73cc9
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Apr 2024 08:02:37.5319
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ATLs+PdzljorOortdv/q7bpdlKVU/nWnxKrdJkcnNM/7sJyPV1kAoX6fXnsUUTWyZu+MlNMlYuSZeiFDKjHuNg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB6155
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1011,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-04-26_07,2024-04-26_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxlogscore=999
 mlxscore=0 phishscore=0 spamscore=0 adultscore=0 bulkscore=0
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2404010000 definitions=main-2404260050
X-Proofpoint-ORIG-GUID: 9CWikY-ymdwDVl5JWCpqLn62IR1_rjlG
X-Proofpoint-GUID: 9CWikY-ymdwDVl5JWCpqLn62IR1_rjlG

On 25/04/2024 15:47, Pankaj Raghav (Samsung) wrote:
> On Mon, Apr 22, 2024 at 02:39:18PM +0000, John Garry wrote:
>> Borrowed from:
>>
>> https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20240213093713.1753368-2-kernel@pankajraghav.com/__;!!ACWV5N9M2RV99hQ!LvajFab0xQx8oBWDlDtVY8duiLDjOKX91G4YqadoCu6gqatA2H0FzBUvdSC69dqXNoe2QvStSwrxIZ142MXOKk8$
>> (credit given in due course)
>>
>> We will need to be able to only use a single folio order for buffered
>> atomic writes, so allow the mapping folio order min and max be set.
> 
>>
>> We still have the restriction of not being able to support order-1
>> folios - it will be required to lift this limit at some stage.
> 
> This is already supported upstream for file-backed folios:
> commit: 8897277acfef7f70fdecc054073bea2542fc7a1b

ok

> 
>> index fc8eb9c94e9c..c22455fa28a1 100644
>> --- a/include/linux/pagemap.h
>> +++ b/include/linux/pagemap.h
>> @@ -363,9 +363,10 @@ static inline void mapping_set_gfp_mask(struct address_space *m, gfp_t mask)
>>   #endif
>>   
>>   /*
>> - * mapping_set_folio_min_order() - Set the minimum folio order
>> + * mapping_set_folio_orders() - Set the minimum and max folio order
> 
> In the new series (sorry forgot to CC you),

no worries, I saw it

> I added a new helper called
> mapping_set_folio_order_range() which does something similar to avoid
> confusion based on willy's suggestion:
> https://urldefense.com/v3/__https://lore.kernel.org/linux-xfs/20240425113746.335530-3-kernel@pankajraghav.com/__;!!ACWV5N9M2RV99hQ!LvajFab0xQx8oBWDlDtVY8duiLDjOKX91G4YqadoCu6gqatA2H0FzBUvdSC69dqXNoe2QvStSwrxIZ14opzAoso$
> 

Fine, I can include that

> mapping_set_folio_min_order() also sets max folio order to be
> MAX_PAGECACHE_ORDER order anyway. So no need of explicitly calling it
> here?
> 

Here mapping_set_folio_min_order() is being replaced with 
mapping_set_folio_order_range(), so not sure why you mention that. 
Regardless, I'll use your mapping_set_folio_order_range().

>>   /**
>> @@ -400,7 +406,7 @@ static inline void mapping_set_folio_min_order(struct address_space *mapping,
>>    */
>>   static inline void mapping_set_large_folios(struct address_space *mapping)
>>   {
>> -	mapping_set_folio_min_order(mapping, 0);
>> +	mapping_set_folio_orders(mapping, 0, MAX_PAGECACHE_ORDER);
>>   }
>>   
>>   static inline unsigned int mapping_max_folio_order(struct address_space *mapping)
>> diff --git a/mm/filemap.c b/mm/filemap.c
>> index d81530b0aac0..d5effe50ddcb 100644
>> --- a/mm/filemap.c
>> +++ b/mm/filemap.c
>> @@ -1898,9 +1898,15 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
>>   no_page:
>>   	if (!folio && (fgp_flags & FGP_CREAT)) {
>>   		unsigned int min_order = mapping_min_folio_order(mapping);
>> -		unsigned int order = max(min_order, FGF_GET_ORDER(fgp_flags));
>> +		unsigned int max_order = mapping_max_folio_order(mapping);
>> +		unsigned int order = FGF_GET_ORDER(fgp_flags);
>>   		int err;
>>   
>> +		if (order > max_order)
>> +			order = max_order;
>> +		else if (order < min_order)
>> +			order = max_order;
> 
> order = min_order; ?

right

Thanks,
John