lkml.org 
[lkml]   [2021]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 1/4] hugetlb: add demote hugetlb page sysfs interfaces
From
Date
On 9/24/21 2:28 AM, David Hildenbrand wrote:
> On 23.09.21 19:53, Mike Kravetz wrote:
>> + spin_lock_irq(&hugetlb_lock);
>> + if (nid != NUMA_NO_NODE) {
>> + nr_available = h->free_huge_pages_node[nid];
>> + init_nodemask_of_node(&nodes_allowed, nid);
>> + n_mask = &nodes_allowed;
>> + } else {
>> + nr_available = h->free_huge_pages;
>> + n_mask = &node_states[N_MEMORY];
>> + }
>> + nr_available -= h->resv_huge_pages;
>> + if (nr_available <= 0)
>> + goto out;
>> + nr_demote = min(nr_available, nr_demote);
>> +
>> + while (nr_demote) {
>> + if (!demote_pool_huge_page(h, n_mask))
>> + break;
>> +
>> + /*
>> + * We may have dropped the lock in the routines to
>> + * demote/free a page. Recompute nr_demote as counts could
>> + * have changed and we want to make sure we do not demote
>> + * a reserved huge page.
>> + */
>> + nr_demote--;
>> + if (nid != NUMA_NO_NODE)
>> + nr_available = h->free_huge_pages_node[nid];
>> + else
>> + nr_available = h->free_huge_pages;
>> + nr_available -= h->resv_huge_pages;
>> + if (nr_available <= 0)
>> + nr_demote = 0;
>> + else
>> + nr_demote = min(nr_available, nr_demote);
>> + }
>>
>
> Wonder if you could compress that quite a bit:
>
>
> ...
> spin_lock_irq(&hugetlb_lock);
>
> if (nid != NUMA_NO_NODE) {
> init_nodemask_of_node(&nodes_allowed, nid);
> n_mask = &nodes_allowed;
> } else {
> n_mask = &node_states[N_MEMORY];
> }
>
> while (nr_demote) {
> /*
> * Update after each iteration because we might have temporarily
> * dropped the lock and our counters changes.
> */
> if (nid != NUMA_NO_NODE)
> nr_available = h->free_huge_pages_node[nid];
> else
> nr_available = h->free_huge_pages;
> nr_available -= h->resv_huge_pages;
> if (nr_available <= 0)
> break;
> if (!demote_pool_huge_page(h, n_mask))
> break;
> nr_demote--;
> };
> spin_unlock_irq(&hugetlb_lock);
>
> Not sure if that "nr_demote = min(nr_available, nr_demote);" logic is really required. Once nr_available hits <= 0 we'll just stop denoting.
>

No, it is not needed. You suggested code looks much nicer. I will
incorporate into the next version.

Thanks!
--
Mike Kravetz

\
 
 \ /
  Last update: 2021-09-29 22:43    [W:0.089 / U:4.692 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site