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 C49E9C433EF for ; Wed, 20 Jul 2022 19:09:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C4E646B0071; Wed, 20 Jul 2022 15:09:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BFDCA6B0073; Wed, 20 Jul 2022 15:09:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AC5936B0074; Wed, 20 Jul 2022 15:09:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 9ADBD6B0071 for ; Wed, 20 Jul 2022 15:09:17 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 5FF00C06CA for ; Wed, 20 Jul 2022 19:09:17 +0000 (UTC) X-FDA: 79708416354.03.55846A0 Received: from mail-pj1-f43.google.com (mail-pj1-f43.google.com [209.85.216.43]) by imf25.hostedemail.com (Postfix) with ESMTP id 03AB1A0009 for ; Wed, 20 Jul 2022 19:09:16 +0000 (UTC) Received: by mail-pj1-f43.google.com with SMTP id d7-20020a17090a564700b001f209736b89so3024012pji.0 for ; Wed, 20 Jul 2022 12:09:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=pj+kkQ0iRlDhBCBdwrMBBfdY/mASMiz71pQCLaAohtA=; b=OnPbctI9jJtcSd7gZ4vdxsMVCyrfYFBTDXHdDBUaQZGZI/iAjdJjUY+qm2eykhJzmJ 313tEWC3U1HJ37sA1Vn/+fwAN4YWnhmWvwQFABMwut2Rn44hsNSbNI2eJnLJhJMmh1tt ku3vgKdKFQFbX6TAJdHDoPd9Lse5BUcRZ19BDB3bdjeZe6v2QGUM97+OE8UG+szzNHMY tQ/qHZprDhGMEk+a/RcJRZ3jHI9/jwK+ayK0YjOacHD2bBO/GlXKE7TUlIeAdrGRKJWX FqUu1JZPS0PiXkDc9oJmUzmj3SpgzKMaUmHTB44cQKFJvKbSBUdihn9Bd9OgqWb1im8F i+nA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=pj+kkQ0iRlDhBCBdwrMBBfdY/mASMiz71pQCLaAohtA=; b=VTj3r5z24awEzmvpKo4Cdf8X0BLfrHd0mq+pkTB84G2hmDCQTx6viQVLUBEKy6dvOq PQbp8tXJuGr52u21TSa331OKe+EoKhRSZZaRtQklPdrHSmECr282f9e3GvFFz3UEiXFW I3JrkkHVGaWgYshbPDJ0hAdfXM4mXZNnnKLtEuYSHYcrU8SS/Hza3YBM6j+OVebRS8BE riGeDCMpjbCS/Z0Xxm7Q2eO5K0nluB4trGqacslgrECyGngS29rGI9zBg2xhqDR67M9y mV+tYDlN3tx/CYbS9nNs0QSvdXtjNHXXBCnxYKy98Qe/H5c4Ntxpu2gFalp+wR+iBjuk /DOQ== X-Gm-Message-State: AJIora8irARiq8kdPjGPB2AROuMVj/3qZqzexIVJxGKOJPWDgp9NisMO mKB7e3NWa8GAkeeq4W97K/CvJA== X-Google-Smtp-Source: AGRyM1sMkT1p/UZrE2KeTQZJh5VcVRcSDzTpqhCn9L7kxZxDadzF6P0YmSrncGOmFItKKwgNErQf5A== X-Received: by 2002:a17:90b:1643:b0:1f0:2094:57db with SMTP id il3-20020a17090b164300b001f0209457dbmr6967333pjb.140.1658344155753; Wed, 20 Jul 2022 12:09:15 -0700 (PDT) Received: from google.com (55.212.185.35.bc.googleusercontent.com. [35.185.212.55]) by smtp.gmail.com with ESMTPSA id r11-20020a170903410b00b0016c5306917fsm14110890pld.53.2022.07.20.12.09.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Jul 2022 12:09:15 -0700 (PDT) Date: Wed, 20 Jul 2022 12:09:11 -0700 From: Zach O'Keefe To: Yang Shi Cc: Andrew Morton , Linux MM , Hugh Dickins , Miaohe Lin Subject: Re: [PATCH mm-unstable 2/4] mm/khugepaged: consistently order cc->is_khugepaged and pte_* checks Message-ID: References: <20220720140603.1958773-1-zokeefe@google.com> <20220720140603.1958773-3-zokeefe@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=OnPbctI9; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf25.hostedemail.com: domain of zokeefe@google.com designates 209.85.216.43 as permitted sender) smtp.mailfrom=zokeefe@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1658344157; 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=pj+kkQ0iRlDhBCBdwrMBBfdY/mASMiz71pQCLaAohtA=; b=E/9OJ+Cr720Pj9cml4bHhtgOm0hUoBIRVLLivJFKkD9Uf/6ZWQsp6mCf6i78ccQzRgCQ4v Nr1Q5Z4QarrhX7quOgsGQX/VsStZklXuCcka0OGAqF1rJuB0s9iEkAtmm9ZOCV5ljqKeaY mwfCLZfEmAQ5dOFfzPW3yj44vbA0S44= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1658344157; a=rsa-sha256; cv=none; b=FijKQerws/E1X2e4LuEFJVioPiHWxYDCfbS9Y7mECdMzWPwF/SIJW2FeEyeDmZy7FoGc7R OIUDcyeKgYaPIfatNP0FzhF4HXFLjCpLvgLiz70YjFMTTEAkuu+HQbexWxBcLJMDWszdmJ T+VM+YYve4BHVm4uZaT5CuXNfQccGyo= X-Rspamd-Queue-Id: 03AB1A0009 Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=OnPbctI9; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf25.hostedemail.com: domain of zokeefe@google.com designates 209.85.216.43 as permitted sender) smtp.mailfrom=zokeefe@google.com X-Rspam-User: X-Rspamd-Server: rspam03 X-Stat-Signature: x4o9qf87djg9m1iq4trsp3pdbafgpdkk X-HE-Tag: 1658344156-351158 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: On Jul 20 10:27, Yang Shi wrote: > On Wed, Jul 20, 2022 at 7:06 AM Zach O'Keefe wrote: > > > > cc->is_khugepaged is used to predicate the khugepaged-only behavior > > of enforcing khugepaged heuristics limited by the sysfs knobs > > khugepaged_max_ptes_[none|swap|shared]. > > > > In branches where khugepaged_max_ptes_* is checked, consistently check > > cc->is_khugepaged first. Also, local counters (for comparison vs > > khugepaged_max_ptes_* limits) were previously incremented in the > > comparison expression. Some of these counters (unmapped) are > > additionally used outside of khugepaged_max_ptes_* enforcement, and > > all counters are communicated in tracepoints. Move the correct > > accounting of these counters before branching statements to avoid future > > errors due to C's short-circuiting evaluation. > > Yeah, it is safer to not depend on the order of branch statements to > inc the counter. > Only cost me a couple hours when I got bit by this after naively moving checks around :) Hopefully I can save the next person. Also, thanks for the reviews, Yang! > Reviewed-by: Yang Shi > > > > > Fixes: 9fab4752a181 ("mm/khugepaged: add flag to predicate khugepaged-only behavior") > > Link: https://lore.kernel.org/linux-mm/Ys2qJm6FaOQcxkha@google.com/ > > Signed-off-by: Zach O'Keefe > > --- > > mm/khugepaged.c | 49 +++++++++++++++++++++++++++++-------------------- > > 1 file changed, 29 insertions(+), 20 deletions(-) > > > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > > index ecd28bfeab60..290422577172 100644 > > --- a/mm/khugepaged.c > > +++ b/mm/khugepaged.c > > @@ -574,9 +574,10 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma, > > pte_t pteval = *_pte; > > if (pte_none(pteval) || (pte_present(pteval) && > > is_zero_pfn(pte_pfn(pteval)))) { > > + ++none_or_zero; > > if (!userfaultfd_armed(vma) && > > - (++none_or_zero <= khugepaged_max_ptes_none || > > - !cc->is_khugepaged)) { > > + (!cc->is_khugepaged || > > + none_or_zero <= khugepaged_max_ptes_none)) { > > continue; > > } else { > > result = SCAN_EXCEED_NONE_PTE; > > @@ -596,11 +597,14 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma, > > > > VM_BUG_ON_PAGE(!PageAnon(page), page); > > > > - if (cc->is_khugepaged && page_mapcount(page) > 1 && > > - ++shared > khugepaged_max_ptes_shared) { > > - result = SCAN_EXCEED_SHARED_PTE; > > - count_vm_event(THP_SCAN_EXCEED_SHARED_PTE); > > - goto out; > > + if (page_mapcount(page) > 1) { > > + ++shared; > > + if (cc->is_khugepaged && > > + shared > khugepaged_max_ptes_shared) { > > + result = SCAN_EXCEED_SHARED_PTE; > > + count_vm_event(THP_SCAN_EXCEED_SHARED_PTE); > > + goto out; > > + } > > } > > > > if (PageCompound(page)) { > > @@ -1170,8 +1174,9 @@ static int hpage_collapse_scan_pmd(struct mm_struct *mm, > > _pte++, _address += PAGE_SIZE) { > > pte_t pteval = *_pte; > > if (is_swap_pte(pteval)) { > > - if (++unmapped <= khugepaged_max_ptes_swap || > > - !cc->is_khugepaged) { > > + ++unmapped; > > + if (!cc->is_khugepaged || > > + unmapped <= khugepaged_max_ptes_swap) { > > /* > > * Always be strict with uffd-wp > > * enabled swap entries. Please see > > @@ -1189,9 +1194,10 @@ static int hpage_collapse_scan_pmd(struct mm_struct *mm, > > } > > } > > if (pte_none(pteval) || is_zero_pfn(pte_pfn(pteval))) { > > + ++none_or_zero; > > if (!userfaultfd_armed(vma) && > > - (++none_or_zero <= khugepaged_max_ptes_none || > > - !cc->is_khugepaged)) { > > + (!cc->is_khugepaged || > > + none_or_zero <= khugepaged_max_ptes_none)) { > > continue; > > } else { > > result = SCAN_EXCEED_NONE_PTE; > > @@ -1221,12 +1227,14 @@ static int hpage_collapse_scan_pmd(struct mm_struct *mm, > > goto out_unmap; > > } > > > > - if (cc->is_khugepaged && > > - page_mapcount(page) > 1 && > > - ++shared > khugepaged_max_ptes_shared) { > > - result = SCAN_EXCEED_SHARED_PTE; > > - count_vm_event(THP_SCAN_EXCEED_SHARED_PTE); > > - goto out_unmap; > > + if (page_mapcount(page) > 1) { > > + ++shared; > > + if (cc->is_khugepaged && > > + shared > khugepaged_max_ptes_shared) { > > + result = SCAN_EXCEED_SHARED_PTE; > > + count_vm_event(THP_SCAN_EXCEED_SHARED_PTE); > > + goto out_unmap; > > + } > > } > > > > page = compound_head(page); > > @@ -1961,8 +1969,9 @@ static int khugepaged_scan_file(struct mm_struct *mm, struct file *file, > > continue; > > > > if (xa_is_value(page)) { > > + ++swap; > > if (cc->is_khugepaged && > > - ++swap > khugepaged_max_ptes_swap) { > > + swap > khugepaged_max_ptes_swap) { > > result = SCAN_EXCEED_SWAP_PTE; > > count_vm_event(THP_SCAN_EXCEED_SWAP_PTE); > > break; > > @@ -2013,8 +2022,8 @@ static int khugepaged_scan_file(struct mm_struct *mm, struct file *file, > > rcu_read_unlock(); > > > > if (result == SCAN_SUCCEED) { > > - if (present < HPAGE_PMD_NR - khugepaged_max_ptes_none && > > - cc->is_khugepaged) { > > + if (cc->is_khugepaged && > > + present < HPAGE_PMD_NR - khugepaged_max_ptes_none) { > > result = SCAN_EXCEED_NONE_PTE; > > count_vm_event(THP_SCAN_EXCEED_NONE_PTE); > > } else { > > -- > > 2.37.0.170.g444d1eabd0-goog > >