Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <48a4a7f2-bee5-371a-1d9d-549fc59ce4f4@redhat.com>
Date:   Wed, 1 Jun 2022 12:49:28 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.0
Subject: Re: [PATCH 1/3] mm: hugetlb_vmemmap: cleanup hugetlb_vmemmap related
 functions
Content-Language: en-US
To:     Muchun Song <songmuchun@bytedance.com>, mike.kravetz@oracle.com
Cc:     akpm@linux-foundation.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, smuchun@bytedance.com
References: <20220404074652.68024-1-songmuchun@bytedance.com>
 <20220404074652.68024-2-songmuchun@bytedance.com>
 <087817e3-98ce-09f6-9ae9-68e544f43775@redhat.com>
 <YpdD7WCYlfCkvv2L@FVFYT0MHHV2J.googleapis.com>
From:   David Hildenbrand <david@redhat.com>
Organization: Red Hat
In-Reply-To: <YpdD7WCYlfCkvv2L@FVFYT0MHHV2J.googleapis.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

>>>  #endif
>>>  #ifdef CONFIG_CGROUP_HUGETLB
>>>  	/* cgroup control files */
>>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>>> index dd642cfc538b..1f9fbdddc86b 100644
>>> --- a/mm/hugetlb.c
>>> +++ b/mm/hugetlb.c
>>> @@ -1540,7 +1540,7 @@ static void __update_and_free_page(struct hstate *h, struct page *page)
>>>  	if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
>>>  		return;
>>>  
>>> -	if (alloc_huge_page_vmemmap(h, page)) {
>>> +	if (hugetlb_vmemmap_alloc(h, page)) {
>>>  		spin_lock_irq(&hugetlb_lock);
>>>  		/*
>>>  		 * If we cannot allocate vmemmap pages, just refuse to free the
>>> @@ -1617,7 +1617,7 @@ static DECLARE_WORK(free_hpage_work, free_hpage_workfn);
>>>  
>>>  static inline void flush_free_hpage_work(struct hstate *h)
>>>  {
>>> -	if (free_vmemmap_pages_per_hpage(h))
>>> +	if (hugetlb_optimize_vmemmap_pages(h))
>>
>> It might be reasonable to call that hugetlb_should_optimize_vmemmap()
>> then, letting it return a bool.
>>
> 
> How about the name of "hugetlb_vmemmap_optimizable()"?  "should" seems to
> tell the user that this hugetlb should be optimized, however, optimization
> also depends on "hugetlb_free_vmemmap=on".  "optimizable" seems to be more
> appropriate, right?

No strong opinion. Either is clearer to me compared to what we have
right now :)


-- 
Thanks,

David / dhildenb