Yin Fengwei <fengwei.yin@xxxxxxxxx> writes:
> free_transhuge_page() acquires split queue lock then check
> whether the THP was added to deferred list or not. It brings
> high deferred queue lock contention.
>
> It's safe to check whether the THP is in deferred list or not
> without holding the deferred queue lock in free_transhuge_page()
> because when code hit free_transhuge_page(), there is no one
> tries to add the folio to _deferred_list.
>
> Running page_fault1 of will-it-scale + order 2 folio for anonymous
> mapping with 96 processes on an Ice Lake 48C/96T test box, we could
> see the 61% split_queue_lock contention:
> - 63.02% 0.01% page_fault1_pro [kernel.kallsyms] [k] free_transhuge_page
> - 63.01% free_transhuge_page
> + 62.91% _raw_spin_lock_irqsave
>
> With this patch applied, the split_queue_lock contention is less
> than 1%.
>
> Signed-off-by: Yin Fengwei <fengwei.yin@xxxxxxxxx>
Thanks!
Reviewed-by: "Huang, Ying" <ying.huang@xxxxxxxxx>
> ---
> mm/huge_memory.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 032fb0ef9cd1..2a1df2c24c8e 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2799,12 +2799,19 @@ void free_transhuge_page(struct page *page)
> struct deferred_split *ds_queue = get_deferred_split_queue(folio);
> unsigned long flags;
>
> - spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
> - if (!list_empty(&folio->_deferred_list)) {
> - ds_queue->split_queue_len--;
> - list_del(&folio->_deferred_list);
> + /*
> + * At this point, there is no one trying to add the folio to
> + * deferred_list. If folio is not in deferred_list, it's safe
> + * to check without acquiring the split_queue_lock.
> + */
> + if (data_race(!list_empty(&folio->_deferred_list))) {
> + spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
> + if (!list_empty(&folio->_deferred_list)) {
> + ds_queue->split_queue_len--;
> + list_del(&folio->_deferred_list);
> + }
> + spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
> }
> - spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
> free_compound_page(page);
> }
