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: <389f9eca-7be2-d43a-bc3f-f638b955b55a@oracle.com>
Date:   Fri, 16 Dec 2022 15:10:41 -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: move folio_set_compound_order() to
 mm/internal.h
Content-Language: en-US
To:     John Hubbard <jhubbard@nvidia.com>,
        Andrew Morton <akpm@linux-foundation.org>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        songmuchun@bytedance.com, mike.kravetz@oracle.com,
        willy@infradead.org
References: <20221213212053.106058-1-sidhartha.kumar@oracle.com>
 <20221216142733.e2a716a4cd7ea55240d98391@linux-foundation.org>
 <963ba9b4-6ddf-39bc-85cf-2feef542029d@nvidia.com>
From:   Sidhartha Kumar <sidhartha.kumar@oracle.com>
In-Reply-To: <963ba9b4-6ddf-39bc-85cf-2feef542029d@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?cGhaZzFsdDE2bTBGSThibFZINzFsdHhUbmdvNjd3eTRXY1lTQ21OVmRpMmd4?=
 =?utf-8?B?NEtjcEtaUi9qMzNqRVJjeCtxd1ZFbmE0YmIyWWhEM1U1bjdCTDBmQmJtc3hC?=
 =?utf-8?B?TEZneDRhdnJFTTdzYnp4ZDVUUnpjSUxaTGsxdlltSzRWU0M4dFM3SThpVzNo?=
 =?utf-8?B?UmJtamRBMDBrT0U4bUoydUU4cGM0NzdDSVB1NEE3VjdXM1dIajhzTzZqNWRx?=
 =?utf-8?B?UXVJYTZBb0YreUc4ZVg5aUtFL2ViVUx5REdnTjQ5aWlMVTFHMExObDMva0hJ?=
 =?utf-8?B?RnVnaDdtK0hWT3NKUXZEazlPb2lZdnRUM1lwNGxKUEsrMnYyMHRRazRxL2k3?=
 =?utf-8?B?YkFKa3dCK1QxNUE2S09CUjE5ajRieDh6SmdpQXpXcHdZWkFXenVnbjh2Z2Ni?=
 =?utf-8?B?WnczSUhiR2hLTXlnNmkwVDF2VDROcGEvNGN6c0VocmowQTBLVldPb05aSGl3?=
 =?utf-8?B?aHdnMzFqVStFaC9scEpvVWlDODROMjFqMlhJbzFTUW1uc0t0TFVpSERESVEx?=
 =?utf-8?B?NzBrNEkrSDNDMlJDbnlzU1Mwa1lqSWJMekUxSFl4L3JPVDV6b3ljK1hQbmtW?=
 =?utf-8?B?ekdndEoyK1pWM2pYWkpZK3FpQkg5VHNES1hFd21oSUNVTDB6WFJ0aWhaTVZH?=
 =?utf-8?B?Nm1rR0U3b0xwUERpZlc4aE9nSFU2c3JiNGNsVU1ZUEIzbDBCVlZXWnJWNlJD?=
 =?utf-8?B?aFVyYmNmaWpsM1lYQ0FoSzAzVS9IWXFQclZheEVHMzB5OHVTcUlqNFVSaC94?=
 =?utf-8?B?c0hEdEFiN3lCUXZXVUtrY2dpQStxdDZ6WjNUMDNYYWkwTnhhU0FJUFUvQlVu?=
 =?utf-8?B?NDhLVWNIUVVxaEZLV2VZRUFhMjUrbXA0UTd4dlNQVFdEWnFQT2M1REx5OTky?=
 =?utf-8?B?TmVWYTNBNEQ1enNlbi9UcjVrVkM0eHYyNlVZTGZXOTMwVGxSRXB4Vzgxa1FY?=
 =?utf-8?B?bFVrZjJWVVRYWk5KU05PY1Vpak9xbzhSUTZpWU93TEZhUlNZN2dWQlV6OFRU?=
 =?utf-8?B?MXZINVRUekdxY1lBWjZjNUtVWGkyL3h1Y0E3UzQ1M240b0RkbFFHY1BId1ZW?=
 =?utf-8?B?MVQ2YWI5VDRLM1lWWS9XeldqeVI0TUJiTVk5c3NtRWZSZGdhMG5RVGFpYllL?=
 =?utf-8?B?aTFvd3hScUptWTl5U0RFUk9wWVM1N3JiakNQalA2UGtRQlhSd3Z0YkpHVlpY?=
 =?utf-8?B?dW51d1B1bG90dDFXZUh3WUh6d1dmeFAwdFBMeHBoRW5XUE1MN2ZpQmlkSEk0?=
 =?utf-8?B?Z1ZNUmlRNVFUQmFMSFJkZkFJTkZ2T3VHZTJTMWlQaG5HZnA0eHVpc3dXRU1Z?=
 =?utf-8?B?MmpwY00yclZwTzZtK3NNWnRWT2lPVEVsaG9VcWZJUlRZaVVMd1R3SUNqbkJY?=
 =?utf-8?B?TjdsbVdBVUhjb0RjbTJCMkJBa0QxRVkwY1RGdHBwbk5aU0NURTBWMWZiVEE1?=
 =?utf-8?B?QmxvbnBGT050NG9SaHBXWjJKdEhrcXNIZzBuNHQyWVRyRzFHMmNoeUFQUnd5?=
 =?utf-8?B?MFNzb2IxeE1BMS9uL3R2WlFRYkZVcWVMWWJwSEo5QmcyOXBidjZNZGwyUkI2?=
 =?utf-8?B?Q3UrVXliVU83OEZTejY1bW1GQU84WEpoa2o3VEZhWkZEVkVyZDBZS2VJbnRh?=
 =?utf-8?B?cW5BL0Uwd0NtSFFiUUhTOTlBZ2ZKc3NDTHB1MDVOVXFZVldBUjhOdyttMXRE?=
 =?utf-8?B?ODd3VVdaelRUQmxGMUI0WlRWdkdyM1MxMys1RXp1YUpicXBYYTFFZGE5Kytu?=
 =?utf-8?B?ZktPYllHVXI3eGhPSlVMV0V3MmNkdzJCNHAySmtpZmg3L0dCSmdPT3l1WGJL?=
 =?utf-8?B?Z3dyb1dzWUJ3K0hZV0J5eGkrU0ljWE80YUI3RStTVU5uVDY1NkFUMWJvbG00?=
 =?utf-8?B?ZUJRdk95aS9iUHNNUXlOWFFWTVRXbGp6T0RCQXRUTWpWWWFLR3VucGhjRTc4?=
 =?utf-8?B?QjZCR052NlYrK2lTRWwxS3NVeGZCS2VyeXpPNm05NWNoek1ZMzdrdEFqTFhS?=
 =?utf-8?B?aHlmenQ0cjlTQ29taGFYalFURXhMcC85d2M0WTFBbWhKYjVKUVJuSE9SbXhx?=
 =?utf-8?B?OGFEbzhlUFR3RXRKUUwxTHZsTm9sY3orVzhPQkYrc1AvbXNTZjBiWkJiNmlW?=
 =?utf-8?B?UWZjcXJTMmVaaSt5ZEdWMnVXZ0hXZm5UL3lpeHFkbzgwUFpDc1hFOFdpdytE?=
 =?utf-8?B?NkE9PQ==?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 36bd6731-559f-4179-fba4-08dadfbac296
X-MS-Exchange-CrossTenant-AuthSource: CH0PR10MB5113.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Dec 2022 23:10:45.1055
 (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: 9DLBWOfsZYC2XYLrMrd6bD1gCIyjzf3DCuzfsZmnY8iD/TPUrTOgGBSslnjvB8UxhtuARckld+ppPZxFlEljGSBrEcqK0T7kcNLMouTe+wg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR10MB7133
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-16_14,2022-12-15_02,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 malwarescore=0 phishscore=0
 mlxlogscore=999 suspectscore=0 spamscore=0 adultscore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000
 definitions=main-2212160205
X-Proofpoint-GUID: eJu63TERvmvzoKo0ZjLux_8uWx6HXiD2
X-Proofpoint-ORIG-GUID: eJu63TERvmvzoKo0ZjLux_8uWx6HXiD2
X-Spam-Status: No, score=-2.8 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/16/22 2:56 PM, John Hubbard wrote:
> On 12/16/22 14:27, Andrew Morton wrote:
>> On Tue, 13 Dec 2022 13:20:53 -0800 Sidhartha Kumar 
>> <sidhartha.kumar@oracle.com> wrote:
>>
>>> folio_set_compound_order() is moved to an mm-internal location so 
>>> external
>>> folio users cannot misuse this function. Change the name of the function
>>> to folio_set_order() and use WARN_ON_ONCE() rather than BUG_ON. Also,
>>> handle the case if a non-large folio is passed and add clarifying 
>>> comments
>>> to the function.
>>>
>>
>> This differs from the version I previously merged:
>>
>> --- 
>> a/mm/internal.h~mm-move-folio_set_compound_order-to-mm-internalh-update
>> +++ a/mm/internal.h
>> @@ -384,8 +384,10 @@ int split_free_page(struct page *free_pa
>>    */
>>   static inline void folio_set_order(struct folio *folio, unsigned int 
>> order)
>>   {
>> -    if (WARN_ON_ONCE(!folio_test_large(folio)))
>> +    if (!folio_test_large(folio)) {
>> +        WARN_ON_ONCE(order);
>>           return;
>> +    }
> 
> I think that's out of date?
> 
> We eventually settled on the version that is (as of this a few minutes
> ago) already in mm-unstable (commit fdea060a130d: "mm: move
> folio_set_compound_order() to mm/internal.h"), which has it like this:
> 
Hi Andrew, yes this version that is already in mm-unstable represents 
the v2 of this patch which is what we agreed on. I think the patch 
mm-move-folio_set_compound_order-to-mm-internalh-update with description 
"alter the folio_set_order() warning" which was just added to 
mm-unstable should be removed as our discussion lead us away from that 
version.


>         if (WARN_ON_ONCE(!folio_test_large(folio)))
>                 return;
> 
>>       folio->_folio_order = order;
>>   #ifdef CONFIG_64BIT
>>
>> Makes sense.  But wouldn't
>>
>>     if (WARN_ON_ONCE(order && !folio_test_large(folio)))
>>
>> be clearer?
> That's a little narrower of a check. But maybe that's desirable. Could

Ya I think it would helpful to have a wider catch for the warn as any 
user calling folio_set_order() with a non-large folio should be aware as 
they could misuse the folio later on even if they passed in a 0 order 
because order itself would be an OOB access.

Thanks,
Sidhartha Kumar
> someone (Mike, Muchun, Sidhartha) comment on which behavior is
> preferable, please? I think I'm a little dizzy at this point. :)
> 
> 
> thanks,