Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <434a111c-7f1a-0018-6bd2-561cb382deea@oracle.com>
Date:   Thu, 8 Dec 2022 14:33:01 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.1
Subject: Re: [PATCH mm-unstable] mm: clarify folio_set_compound_order() zero
 support
Content-Language: en-US
To:     John Hubbard <jhubbard@nvidia.com>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        Matthew Wilcox <willy@infradead.org>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        akpm@linux-foundation.org, songmuchun@bytedance.com,
        tsahu@linux.ibm.com, david@redhat.com
References: <20221207223731.32784-1-sidhartha.kumar@oracle.com>
 <92965844-c430-8b8e-d9f1-705d7578bceb@nvidia.com>
 <ec8f46ca-9ea6-4567-2038-22f6d3000ed5@oracle.com>
 <d17530ad-8e12-8069-d619-a2d72fe80e15@nvidia.com>
 <0187f9c2-e80a-9cde-68bc-c9bdbd96b6fe@oracle.com>
 <Y5I78soNmAFv7pi8@casper.infradead.org> <Y5JCi3h8bUzLf3cu@monkey>
 <2723541a-79aa-c6b5-d82c-53db76b78145@oracle.com>
 <e86ca90f-e59e-3851-7225-b5f596ad04b9@nvidia.com>
 <36ddac45-ecd0-e2d2-e974-8c85ca503053@oracle.com>
 <20cc2088-b66e-28d1-a529-414e82146336@nvidia.com>
From:   Sidhartha Kumar <sidhartha.kumar@oracle.com>
In-Reply-To: <20cc2088-b66e-28d1-a529-414e82146336@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?bFh4UkttVlFWOGJrZ1MrTzQ0a2RubWxOaXA1eWpTUE5DTmw1eEE2dkVHYUtV?=
 =?utf-8?B?Q3p1MlczQUpPNE9kcDk1OGpOT0tnM1pVdi8xS3dUYmcwb2J2b2JWVDBIZ2pV?=
 =?utf-8?B?Mk5VY1M5ZXBOOTdSbk1DWFkzbTRtVy95VzYwUE93cnBkSU1URlMvd054RnB1?=
 =?utf-8?B?dXhNQVhzeVorR0FBaFRwaytIUm1PYVovY0dmdnNnaVpCa3czMndkZ1lDSVRx?=
 =?utf-8?B?eEZ4aVN2SllQTmd3VFBKNzFqNGNkTWJ2UGs3R1ZyejI0NmNScURDRmZXQ205?=
 =?utf-8?B?Tlc0WU5uQ1ArVjNlZDc3OC9LSEM2WHdYNVlGcVBjUXpNUXozRHpwL3lDQk1r?=
 =?utf-8?B?WW11Snl4bEtNUlVoczZZVjRsZkdManpUc2ZxVDlzMkxhSE15Rk5PRlh2d2ZO?=
 =?utf-8?B?cXJkMUxGRnJDOVBTU2syeWg1S3B4b3dzSWdjNUlUS0ZMNHZ4TktVT0lSS1I1?=
 =?utf-8?B?YkY1QXhyUW4rVWJuVWxqOWpMZ1Y4aHVnRG4wYm5qMWRGS1dwTjRBdSs1NHFo?=
 =?utf-8?B?OWIrMUh4VCszb0IyenRDZ3JlZ2lQNXJtYkovWEIyR1Rwd3hGeVMzcEt4cFpn?=
 =?utf-8?B?RVNTVFc3S1RUWWJKYmw1VUJYUzhxYlVkODVObENlV1pJanBZMVZ5aTZETmJW?=
 =?utf-8?B?dERIZ2pDY2FabU1EN21UNEhrU2NES1E1WGJsVHoxTnFWQUhsdHNLMmtvbWU4?=
 =?utf-8?B?dzBCYW0veTY0RzVpcFFhMFkxNWpHbmRPMlFnQkgxSWdrOHZHYXJBeWo3dUFv?=
 =?utf-8?B?dmpnem1QSThrbW1LWWl0bjBBMEJqR3EvT1ZHaDcrelVsMkVMcmxjSWM1NzBk?=
 =?utf-8?B?Z3pRUjlrdDRaaEVHLzhjcnpPZTB3ODRnOUNDK1Q5YjRYeVJkbUpBNlZHMVJM?=
 =?utf-8?B?TFlNV3JNUVRpeS94bFJLTk1aMzBiMFJsVlRvZkJzV3AwRXBXUEFONE4zVi84?=
 =?utf-8?B?M25Vc1hKblVoNklPQ3U5RVRQVGVSR1l5VUE1VWdHaWY0UlV3MXBCbm1icWtH?=
 =?utf-8?B?RVlvVkVaUTBiUGIrcUhzQW45UWdjbFIxSG9XejNKNGdmcXJPZjlHTGIvcmFH?=
 =?utf-8?B?RWFHTHNuMUxNZWljYVNYVGtHeUtZNC9BZHNNT0pzUHZyTERVeE1PTDIwOXQ0?=
 =?utf-8?B?eTl3cWdIR3dDOFVqb01CSEJDZXIzWkxkU2x0OHNZeSsvaHNZWUtuT1pOR1By?=
 =?utf-8?B?QkozZm4zNnR1eHo5Q2tjaTdKSHRVR3pkYnFQSHlUU29obGRYbDBiZ290Q2pF?=
 =?utf-8?B?VWlsRENvLzZUZ0tzUXRmcWdyWk1JRzRUVXJyN1lKVGp0WXV4VVorbFJlNldp?=
 =?utf-8?B?dGh3RmRsNDQ0RGo0TXFVaTU0N0FzbVI5T3ZhY3NOMEJtbmxyUk1wTnhGSU5O?=
 =?utf-8?B?NjF1dDRTMUdPL3VjK3V6TnVCVDk0UkFoU3pvTFA2TUgzWmN6ZnZTbFJzZldv?=
 =?utf-8?B?WVR5ZkZ1b3BzTFNxb20rNEowZW1aNWRQRkhQREJ2VnhITlhQZG9rUzgvQksx?=
 =?utf-8?B?clNtdEhpeEw1aHc2WEwyOTlzOWRQWTF6SEdudFgzN20ybW9ZcVUwRURLMjdU?=
 =?utf-8?B?VHNIeWNLcWJFOGFYSFpTRHFERk43bFBRVnYxYjMrZlh5bnQvTk9GSGZhS3M4?=
 =?utf-8?B?YXdqSjl3YmhrVWhValpPT29HUE5KWjF3NjNGaVVDdU15b0pkNkx1V0ZFTkUx?=
 =?utf-8?B?bkFyckUzaDE0bXNYWnpYcXpGa1RhSE16YW5jaXU3eXZ1OThxelEyL3FFNjVQ?=
 =?utf-8?B?Q2dQQlFjY1lQT0pDSFFsbXBNR1M1eVd5SGI1bjhmV0pKSlB4Y29DK2pSMFFL?=
 =?utf-8?B?TlI3Um5hMlo5UDJ6VHZiRUx4RDhqNG1naWI2WnNlbXJFYXMva0xlaUJBMlNx?=
 =?utf-8?B?UXpvYW9KU0tVcjIxZ0dWNnYxd1VZVGJWQXByZllObjBKeHN0V213b3FFM2Mv?=
 =?utf-8?B?U2VFQjJBVDhGcDd0QTN5WUxuekU2OGFzNFE3RDA0c01mVEZiSEpNQjFqUGVQ?=
 =?utf-8?B?QkhLSmY2YmNvRU5nY2s5Mld4NGdLOG85TFhScmhibFVJVTdxSkZROU1nclN3?=
 =?utf-8?B?QVBGckZvRk9TSTVGMWRhUnQ1RTEwdTY3N0pBL3NIZXNUOVRnQTJvck80L2VY?=
 =?utf-8?B?WFdMc3ovZGwzemx4YnZrNUNFSHpaaVVTUUh5M2NDQzhrcmxLN1pwa3pzKzBK?=
 =?utf-8?B?ejZTMyt2akVvSzg0QmNMUzVZb0ZxbkF0SHBTZW1ZejZCRThOcXBYTXFLbDJ4?=
 =?utf-8?B?RnM3dXljQ2E0b055elJ6YzdCaW1BPT0=?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 30fc7a10-0429-4bc7-a8f4-08dad96c2cfb
X-MS-Exchange-CrossTenant-AuthSource: CH0PR10MB5113.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Dec 2022 22:33:06.2450
 (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: ytSzTC7H5rreARLX6Runj2oiDHBqI1BXNmxPyKU4lACNuqQ7FdhBbsNtR3bq/DX2DshGDoc9YVdZu5wYdOESQIx0zVgkw06RIAVMtyu2jGY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB6625
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-12-08_12,2022-12-08_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 suspectscore=0
 malwarescore=0 phishscore=0 mlxlogscore=999 spamscore=0 adultscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2210170000 definitions=main-2212080185
X-Proofpoint-ORIG-GUID: rMBVu6QQApgMgFqvbGj30l8FS45E7N_c
X-Proofpoint-GUID: rMBVu6QQApgMgFqvbGj30l8FS45E7N_c
X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE 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-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 12/8/22 2:14 PM, John Hubbard wrote:
> On 12/8/22 14:12, Sidhartha Kumar wrote:
>> On 12/8/22 2:01 PM, John Hubbard wrote:
>>> On 12/8/22 13:58, Sidhartha Kumar wrote:
>>>> Thanks John, Mike, Matthew, and Muchun for the feedback.
>>>>
>>>> To summarize this discussion and outline the next version of this 
>>>> patch, the changes I'll make include:
>>>>
>>>> 1) change the name of folio_set_compound_order() to folio_set_order()
>>>> 2) change the placement of this function from mm.h to mm/internal.h
>>>> 3) folio_set_order() will set both _folio_order and _folio_nr_pages 
>>>> and handle the zero order case correctly.
>>>> 4) remove the comment about hugetlb's specific use for zero orders
>>>> 5) improve the style of folio_set_order() by removing ifdefs from 
>>>> inside the function to doing
>>>>
>>>> #ifdef CONFIG_64BIT
>>>>   static inline void folio_set_order(struct folio *folio,
>>>>                   unsigned int order)
>>>>   {
>>>>       VM_BUG_ON_FOLIO(!folio_test_large(folio), folio);
>>>
>>> Sounds good, except for this part: why is a function named
>>> folio_set_order() BUG-ing on a non-large folio? The naming
>>> is still wrong, perhaps?
>>>
>>
>> This is because the _folio_nr_pages and _folio_order fields are part 
>> of the first tail page in the folio. folio_test_large returns if the 
>> folio is larger than one page which would be required for setting the 
>> fields.
> 
> OK, but then as I said, the name is wrong. One can either:
> 
> a) handle the non-large case, or
> 
> b) rename the function to indicate that it only works on large folios.
> 

Discussed here[1], the BUG_ON line seemed more appropriate over
  	
if (!folio_test_large(folio))
	return;

as the misuse would not be silent. I think I would be against renaming 
the function as I don't see any large folio specific function names for 
other accessors of tail page fields. Would both the BUG_ON and return on 
non-large folio be included then?


[1]: 
https://lore.kernel.org/linux-mm/20221129225039.82257-1-sidhartha.kumar@oracle.com/T/#m98cf80bb21ae533b7385f2e363c602e2c9e2802d
> 
> thanks,