Re: [PATCH v3 3/5] hugetlb: only set HPageMigratable for migratable hstates

[Mike Kravetz]

On 2/1/21 3:49 AM, Michal Hocko wrote:
> On Fri 29-01-21 10:46:15, Mike Kravetz wrote:
>> On 1/28/21 2:15 PM, Andrew Morton wrote:
>>> On Thu, 28 Jan 2021 14:00:29 -0800 Mike Kravetz <mike.kravetz@xxxxxxxxxx> wrote:
>>>>
>>>> Michal suggested that comments describing synchronization be added for each
>>>> flag.  Since I did 'one patch per flag', that would be an update to each patch.
>>>> Or, I could simply add a patch to update the comment block based on what you
>>>> already have.
>>>>
>>>> Let me know what is best/easiest for you.
>>>
>>> I guess just one patch is best for reviewers.  Then I'll split up into
>>> a sprinkle of -fix patches if I'm feeling energetic ;)
>>
>> Here is a patch to update the comments for all those flags.  It should
>> apply on top of what is in your tree.
>>
>> From: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
>> Date: Fri, 29 Jan 2021 10:36:12 -0800
>> Subject: [PATCH] huegtlb: add synchronization information for new hugetlb
>>  specific flags
>>
>> Adding comments, no functional change.
>>
>> Signed-off-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
> 
> Acked-by: Michal Hocko <mhocko@xxxxxxxx>
> 
> Thanks Mike!

Hi Andrew,

You may have missed this due to the depth of email thread.  Can you pick
this up, or would you like me to resend?

Thanks,
-- 
Mike Kravetz

> 
>> ---
>>  include/linux/hugetlb.h | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
>> index e5e6ffd55392..cf70795d2209 100644
>> --- a/include/linux/hugetlb.h
>> +++ b/include/linux/hugetlb.h
>> @@ -480,14 +480,24 @@ unsigned long hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
>>   * HPG_restore_reserve - Set when a hugetlb page consumes a reservation at
>>   *	allocation time.  Cleared when page is fully instantiated.  Free
>>   *	routine checks flag to restore a reservation on error paths.
>> + *	Synchronization:  Examined or modified by code that knows it has
>> + *	the only reference to page.  i.e. After allocation but before use
>> + *	or when the page is being freed.
>>   * HPG_migratable  - Set after a newly allocated page is added to the page
>>   *	cache and/or page tables.  Indicates the page is a candidate for
>>   *	migration.
>> + *	Synchronization:  Initially set after new page allocation with no
>> + *	locking.  When examined and modified during migration processing
>> + *	(isolate, migrate, putback) the hugetlb_lock is held.
>>   * HPG_temporary - - Set on a page that is temporarily allocated from the buddy
>>   *	allocator.  Typically used for migration target pages when no pages
>>   *	are available in the pool.  The hugetlb free page path will
>>   *	immediately free pages with this flag set to the buddy allocator.
>> + *	Synchronization: Can be set after huge page allocation from buddy when
>> + *	code knows it has only reference.  All other examinations and
>> + *	modifications require hugetlb_lock.
>>   * HPG_freed - Set when page is on the free lists.
>> + *	Synchronization: hugetlb_lock held for examination and modification.
>>   */
>>  enum hugetlb_page_flags {
>>  	HPG_restore_reserve = 0,
>> -- 
>> 2.29.2
>>
>