From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8B426C433FE for ; Thu, 13 Oct 2022 16:56:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EB7426B0071; Thu, 13 Oct 2022 12:56:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E67556B0073; Thu, 13 Oct 2022 12:56:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D07C86B0074; Thu, 13 Oct 2022 12:56:22 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id B778C6B0071 for ; Thu, 13 Oct 2022 12:56:22 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 840B3801F4 for ; Thu, 13 Oct 2022 16:56:22 +0000 (UTC) X-FDA: 80016529404.09.8651666 Received: from mail-qt1-f174.google.com (mail-qt1-f174.google.com [209.85.160.174]) by imf08.hostedemail.com (Postfix) with ESMTP id BCBB416002B for ; Thu, 13 Oct 2022 16:56:21 +0000 (UTC) Received: by mail-qt1-f174.google.com with SMTP id c23so1870130qtw.8 for ; Thu, 13 Oct 2022 09:56:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20210112.gappssmtp.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=temoX/mCDi5vWtNS2Q8kfWID2qV1xMevStvQBSR088s=; b=3o2CdU/9gr3qQ+KNLeAp985uAk95TKlxvBGGaSmJRulnbg6uOPe4pJrrrxoCokf05I sTEqqKmRU5DH1OqBDaS6a9NZAEv6JMx8NVQPgBzLtEjESgW8IVg5zCZ8JVOq0dtUhHdp gHNOuihZM4xwOClIPC+MDOiDJgVNlmTL/2uWF2zOPwrLaMXUbKHsnlhTz8xPYyF24twe gPzFudLYsBg6W9731neuc2tKFzQfL/1oRN/ZqsCGFxfVOm+3WR69d5U9L9z9BHe0/OmS dcgj0+qqIFfwFiCtbGKI1C3IVm7mbpmVXS4vo61T3hrGOKlVSJf3ACKr9wXBm9NeLm70 pbyg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=temoX/mCDi5vWtNS2Q8kfWID2qV1xMevStvQBSR088s=; b=jrLpED/GXvGqnVW+4+d1GwQgvTGV5cLyNsiAVpqVh4Pja79xrLIeNH6GIfETGlERFm D2cjMuQUv0i72mFQocWngFQguHHezn6XpIbTjq28woUZph6s+dJB77JhhsKv2+fL2GVP u/JUFA9msPR3AWm7eLhj4i8yQd4VcWi/8Pf33o46IvJ4WR5a7mGr9ckv94WKxewJH8hB VeV1uETMiYcMxEtLYfYfO0j/0BoxIdRn7U5ZbvThjJmMZKjF5Up+ZkFBjDBHPyu8c9PU llO4gDw/3bgwPTQ5wcrvOoZlK+oWY2HpampiukLdyzW90shUY7Qw80LjuF3l1SnMOlZk F7SA== X-Gm-Message-State: ACrzQf16TshWGcC4PDa4HocP4OD48qRBMsfske+uucEmDugP05sRRvqC qLsFk01k5IewHY24seBv+6sXhA== X-Google-Smtp-Source: AMsMyM4T2I9Almdk6Vkjr/vhNZjALBzXzbeR3eqyNLgedNS1WsK8OfEVL9MtSmk9/6lZ915IYb27Rw== X-Received: by 2002:ac8:7fd3:0:b0:39c:c89d:4953 with SMTP id b19-20020ac87fd3000000b0039cc89d4953mr635536qtk.161.1665680180858; Thu, 13 Oct 2022 09:56:20 -0700 (PDT) Received: from localhost ([2620:10d:c091:480::3a61]) by smtp.gmail.com with ESMTPSA id h11-20020a05620a244b00b006ccc96c78easm130006qkn.134.2022.10.13.09.56.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Oct 2022 09:56:20 -0700 (PDT) Date: Thu, 13 Oct 2022 12:56:19 -0400 From: Johannes Weiner To: alexlzhu@fb.com Cc: linux-mm@kvack.org, kernel-team@fb.com, willy@infradead.org, riel@surriel.com Subject: Re: [PATCH v3 3/3] mm: THP low utilization shrinker Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1665680182; a=rsa-sha256; cv=none; b=HtidblU80bx1+EvdyAC+vg68+o8mYyes5G9xeDJNksTcgu+Kd69Xj+GR9YlZjPCr/qsH5R arEntscpGL/MIwWsRjmBFPFtvOtDDHglqwzgDZpc9DqS4JUDAV3j8YZ/70UaPG4Dsqy3SI CWIdPfWA25xFFNBw37K0L7/j3asoM6s= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b="3o2CdU/9"; spf=pass (imf08.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.160.174 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1665680182; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=temoX/mCDi5vWtNS2Q8kfWID2qV1xMevStvQBSR088s=; b=0vAV1vTgAse21eku2LVWZN/oYgP8fpQzpqoJHhftB/4Idx5KAVsqWMY1TY3/mI9vYWyOwA yGf8r3xfIfZd+y8c0Bz1+JkKCLVXXDMNdTWm8SQED4rof4KtoIv39ItGFx6BoOdv5qBq42 OiL9xTYuf7i9gamVBBR94gyy2dR6HV0= X-Stat-Signature: t4ihzkywdj564yf16ueir3r757pzzfz8 X-Rspamd-Queue-Id: BCBB416002B X-Rspam-User: X-Rspamd-Server: rspam08 Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b="3o2CdU/9"; spf=pass (imf08.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.160.174 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org X-HE-Tag: 1665680181-943471 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Oof, fatfingered send vs postpone. Here is the rest ;) On Thu, Oct 13, 2022 at 12:39:53PM -0400, Johannes Weiner wrote: > On Wed, Oct 12, 2022 at 03:51:47PM -0700, alexlzhu@fb.com wrote: > > + > > + if (get_page_unless_zero(head)) { > > + if (!trylock_page(head)) { > > + put_page(head); > > + return LRU_SKIP; > > + } > > The trylock could use a comment: /* Inverse lock order from add_underutilized_thp() */ > > + list_lru_isolate(lru, item); > > + spin_unlock_irq(lock); > > + num_utilized_pages = thp_number_utilized_pages(head); > > + bucket = thp_utilization_bucket(num_utilized_pages); > > + if (bucket < THP_UTIL_BUCKET_NR - 1) { > > + split_huge_page(head); > > + spin_lock_irq(lock); The re-lock also needs to be unconditional, or you'll get a double unlock in the not-split case. > > @@ -2737,6 +2800,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list) > > struct folio *folio = page_folio(page); > > struct deferred_split *ds_queue = get_deferred_split_queue(&folio->page); > > XA_STATE(xas, &folio->mapping->i_pages, folio->index); > > + struct list_head *underutilized_thp_list = page_underutilized_thp_list(&folio->page); > > struct anon_vma *anon_vma = NULL; > > struct address_space *mapping = NULL; > > int extra_pins, ret; > > @@ -2844,6 +2908,9 @@ int split_huge_page_to_list(struct page *page, struct list_head *list) > > list_del(page_deferred_list(&folio->page)); > > } > > spin_unlock(&ds_queue->split_queue_lock); A brief comment would be useful: /* Frozen refs lock out additions, test can be lockless */ > > + if (!list_empty(underutilized_thp_list)) > > + list_lru_del_page(&huge_low_util_page_lru, &folio->page, > > + underutilized_thp_list); > > if (mapping) { > > int nr = folio_nr_pages(folio); > > > > @@ -2886,6 +2953,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list) > > void free_transhuge_page(struct page *page) > > { > > struct deferred_split *ds_queue = get_deferred_split_queue(page); > > + struct list_head *underutilized_thp_list = page_underutilized_thp_list(page); > > unsigned long flags; > > > > spin_lock_irqsave(&ds_queue->split_queue_lock, flags); > > @@ -2894,6 +2962,12 @@ void free_transhuge_page(struct page *page) > > list_del(page_deferred_list(page)); > > } > > spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags); Here as well: /* A dead page cannot be re-added, test can be lockless */ > > + if (!list_empty(underutilized_thp_list)) > > + list_lru_del_page(&huge_low_util_page_lru, page, underutilized_thp_list); > > + > > + if (PageLRU(page)) > > + __folio_clear_lru_flags(page_folio(page)); > > + > > free_compound_page(page); > > } > > > > @@ -2934,6 +3008,39 @@ void deferred_split_huge_page(struct page *page) > > spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags); > > } > > > > +void add_underutilized_thp(struct page *page) > > +{ > > + VM_BUG_ON_PAGE(!PageTransHuge(page), page); > > + > > + if (PageSwapCache(page)) > > + return; Why is that? > > + > > + /* > > + * Need to take a reference on the page to prevent the page from getting free'd from > > + * under us while we are adding the THP to the shrinker. > > + */ > > + if (!get_page_unless_zero(page)) > > + return; > > + > > + if (!is_anon_transparent_hugepage(page)) > > + goto out_put; > > + > > + if (is_huge_zero_page(page)) > > + goto out_put; > > + And here: /* Stabilize page->memcg to allocate and add to the same list */ > > + lock_page(page); > > + > > + if (memcg_list_lru_alloc(page_memcg(page), &huge_low_util_page_lru, GFP_KERNEL)) > > + goto out_unlock; The testbot pointed this out already, but this allocation call needs an #ifdef CONFIG_MEMCG_KMEM guard.