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: <12303882-dd84-485a-28cb-cefbaac4d960@oracle.com>
Date:   Tue, 30 Aug 2022 11:09:33 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.0
Subject: Re: [PATCH 1/7] mm/hugetlb: add folio support to hugetlb specific
 flag macros
Content-Language: en-US
To:     Matthew Wilcox <willy@infradead.org>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        akpm@linux-foundation.org, songmuchun@bytedance.com,
        mike.kravetz@oracle.com, vbabka@suse.cz,
        william.kucharski@oracle.com, dhowells@redhat.com,
        peterx@redhat.com, arnd@arndb.de, ccross@google.com,
        hughd@google.com, ebiederm@xmission.com
References: <20220829230014.384722-1-sidhartha.kumar@oracle.com>
 <20220829230014.384722-2-sidhartha.kumar@oracle.com>
 <Yw2E937l+GARR3Vb@casper.infradead.org>
From:   Sidhartha Kumar <sidhartha.kumar@oracle.com>
In-Reply-To: <Yw2E937l+GARR3Vb@casper.infradead.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MUl6WWhmOGpJY1lCLytoTHhRWk9FdktBVG5DaXR1S0IyUCtxTmZkVCs0UzVt?=
 =?utf-8?B?Z0lHRTlaUk80U1BGbTJsUy9nZDBBdHNsdWFVUWR3U3JtVzNlaU4zU1ByMHY3?=
 =?utf-8?B?c0x0c1BJUXhSa2FSa1d6eTVLcWk0RjJTSVZSQnVmMC9IODhIcFBDT1B5ejJ6?=
 =?utf-8?B?aTRDeGx6LzVWSjF4YTVvZlV4WjlBcVlyajlMemprUzJJMlluTkFiSC9hVnMr?=
 =?utf-8?B?L1ZFU0Z3WU5Wb25QbitEMkZGNDZvSnhZYWpqUFNzZVNjcFJpc2IrWU1JR2ZX?=
 =?utf-8?B?SGxYZnhRT2dwVjhWK2R3UHgrRHl6SWIxTHFibG5wdmZRVU1VTk1GdDlEN3lo?=
 =?utf-8?B?MnMya3E0azJtN1k5NjhNSUkwRDJNeXZtckh4TWN1cndOQTE3ZGVmTVo2MjRD?=
 =?utf-8?B?Wk9FdVM2SFJ3UGpYekxUOEhlVTRtYlFFaVJoNGI4TE9NNlU5dURFcDBZVTVQ?=
 =?utf-8?B?dmozVEQ4eHF1bjUzb1pqV1QwNjV4RnBRbGxaOTR1TVhybXkvcnNUa1V6aEwv?=
 =?utf-8?B?bmdCaVJNc0NINW5NZnhLakd6ZVRMbFZodGVidStqOU5ON0t5MzBlQUNBbWFE?=
 =?utf-8?B?NDA5UGVObVJHVXdvZ1hLeVo5TlZ0YVQ5dUl5R3d3TUNwQ2dZTW1OK01EbUh5?=
 =?utf-8?B?eVh2Qi8yM0NMbitkcTNqcVdEREhWYzNxbEZWSFh5T3RGWWtpK29JbExaUm1E?=
 =?utf-8?B?dXJacncxcW9XWk9ZMmNzUXgyQy9GZDl6dVY2YmNyaFF6QlVtaFhFakg4ajNL?=
 =?utf-8?B?QVFFbm9samxxRW12aEhqazR1QVZnVXlyZlZEbnBWemdDVWlqTDgrTjhNVk0y?=
 =?utf-8?B?UC9MRGlvZGdsYVJ5ZDJpMllFUk02ODIwVU9KSUlJVXZ6Ym9DMEFEU3hCQVJJ?=
 =?utf-8?B?V3BJNGJsTWlIWjNiVkRGMlowSmV2cUdHNU1nN3VSMWRBL252elV0eFFXNmJK?=
 =?utf-8?B?R2lVVW8xYk41bTN4bzhZUlBiRXRWa1dPWHQyRHQzb2JKQ0pBV3lwZ0Y5K1VF?=
 =?utf-8?B?TWc1ZjNsR3dBcEZLeWt4dElHNTMwVFFFeENRZUdZVi9vazM2dHRZMU5RdUdU?=
 =?utf-8?B?Z2NjM3lPbUpMUTVKV3ROV3ZUWlpPSnpKdDZOYkhSdEFZVi9SUUxObkQ1ODkw?=
 =?utf-8?B?MXhXMVIyN0UrRVVyME04Ly9tTC8vWFgrWEltbFRvaXBzVkgxNGJ3cUF4akli?=
 =?utf-8?B?KzZPSGxCaXdNQXZBZ0xZcFB0K1YwZWNnR0FuVVhUY3d4ZjB2WEMya3hlTFpO?=
 =?utf-8?B?L0lPTzJkUU5tUHRjN0xJK0EvemFIQk15Z2QrM0podVQ0VDF5bU9vSm9VbW8x?=
 =?utf-8?B?ZVJHRXFJS0tBOHlFVWQzeGNXa3Ixbit0V0h5RzFDOXVJTCtNSEdBRkhVQy9M?=
 =?utf-8?B?ankyMU5ON3ZoVko3WnFINzNmdlNtM2FuWUNyY0xkZHF3UDlNeTBTRkI0bDN1?=
 =?utf-8?B?b1ZuNWpycmNqckRBL1lYODExVS9zVWxaUzJTSTVnYmgvV0l5Z2R2WVM5TlJJ?=
 =?utf-8?B?aE5UVFEwL2RtQnBLRVVZdUxEcWNJZ0k5Q1NoVGYrU0FWZ2xCOUs5QkZXbWdU?=
 =?utf-8?B?VDVnQkZrVE8zem1ZSzNsaGQ5eG11QklIYjRkL21UcTZLZDRDWE9qTDliajlh?=
 =?utf-8?B?UUtkS0Zodzl5dFR2QU9MSHlHdUY2K21ma1djUEZ5cmdqczN3M3VxZXJTQ3Vk?=
 =?utf-8?B?djdCV3JBUUdEd0doUkx1Mmt2ZVV0MU9JWnFIMFFCbjF0RUxmOEtrZHdoeDRF?=
 =?utf-8?B?bStwN3RnWWZySnpnZDh0b0NDWmFtcWlNL0tndEZUTm4wbmt1N3VJUDBUYkV4?=
 =?utf-8?B?ZUxURk1vQ1FhNWJ1NkJlcmpyVUprak9MdEwyOHg0K1JjbVNpOHNaSzZQamx3?=
 =?utf-8?B?MElFNEQzclhnbmNhakgweGVSRkZvVnNQSHJNV0xMQm9oQW53VGs3ajhtNDk2?=
 =?utf-8?B?YUs5U0NrZmsxUkx1M3hvU1FDRnl0alJOTmRiSWE4UmY5Nnp5dkRwYjI1Uk5H?=
 =?utf-8?B?K1VKODZiMitjZmJPTkFvU3phMnFWWE1IT3BtQnBqQUpkakxFZU9OdmhQYkdB?=
 =?utf-8?B?Y0gyUldVYUM5VzY5VjU3TUpaaHNSVHJhR3MrTnNBNFdYOFE0QUh1Q3NkN0pl?=
 =?utf-8?B?OHBpRENSZmZtbU1zcGEzS2lQaXROQnF6TmV6SG5tUGR5THR2NmZIRUx2K0Vk?=
 =?utf-8?B?YVE9PQ==?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 62c963a6-d792-4d80-4928-08da8ab2ccc8
X-MS-Exchange-CrossTenant-AuthSource: CH0PR10MB5113.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Aug 2022 18:09:37.3023
 (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: nXS/gX79nN6EQxcCxNhXYYR6rsAJbmGxAs8PuLoUXQ+PEnrk7Ek8lT0zIHIvJCkcuP6G25KNeOa44oBF/QhCPxwttR631GRlpvA91WUz3Qw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR10MB4137
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-08-30_10,2022-08-30_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 mlxscore=0 malwarescore=0
 suspectscore=0 spamscore=0 adultscore=0 mlxlogscore=999 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2207270000
 definitions=main-2208300083
X-Proofpoint-ORIG-GUID: FCR7OYbSVMiA174ZRRaeQ6b41rpTT-es
X-Proofpoint-GUID: FCR7OYbSVMiA174ZRRaeQ6b41rpTT-es
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,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-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 8/29/22 8:33 PM, Matthew Wilcox wrote:
> On Mon, Aug 29, 2022 at 04:00:08PM -0700, Sidhartha Kumar wrote:
>>   #define TESTHPAGEFLAG(uname, flname)				\
>> +static __always_inline								\
>> +int folio_test_hugetlb_##flname(struct folio *folio)		\
> One change I made was to have folio_test_foo() return bool instead of
> int.  It helps the compiler really understand what's going on.  Maybe
> some humans too ;-)
>

I went with returning an int to stay consistent with the page version
of the macros which return an int. I'm fine with changing it to return
a bool.

>> +	{	void **private = &folio->private;		\
>> +		return test_bit(HPG_##flname, (void *)((unsigned long)private));	\
> I've made this tricky for you by making folio->private a void * instead
> of the unsigned long in page.  Would this look better as ...
>
> 	{							\
> 		void *private = &folio->private;		\
> 		return test_bit(HPG_##flname, private);		\
>
> perhaps?

Ya this looks much better and passes the tests, will add to v2.