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 8D2C2C43334 for ; Fri, 8 Jul 2022 14:50:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 22B036B0072; Fri, 8 Jul 2022 10:50:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1DC146B0073; Fri, 8 Jul 2022 10:50:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0A2976B0074; Fri, 8 Jul 2022 10:50:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id EE29A6B0072 for ; Fri, 8 Jul 2022 10:50:07 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay12.hostedemail.com (Postfix) with ESMTP id BBB85120416 for ; Fri, 8 Jul 2022 14:50:07 +0000 (UTC) X-FDA: 79664217654.13.B00ED88 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by imf13.hostedemail.com (Postfix) with ESMTP id 9B9DF20027 for ; Fri, 8 Jul 2022 14:50:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1657291805; x=1688827805; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to; bh=7oUj1VswY6dlFRQ4E+ASrn3hkrxe8AfoLr2tRQoMka8=; b=eQTO46BT+4OUorRllUy3Pp5+1MrMjfkVG6JtZw5OYR+t/IZO2NdcYFcr lflzRe9b4IGP48X4K5pB8spGqR+0XGbHRlMpsu21Z6KZuAXzUy41xuPZc ZcXLOvbI8AX1R/jfqPWpv6nTSk/GjBPTiZ0jXrS4ncHtaF/pQmsg3Op1q 4vviADbpDe/du6WbxXfnXQTzoqdMfHfe+WtFO8SOCYBIrHLT6ROpKChAA Hvk8Ke2HyG4O/1CoED3I2ON9j73WB7ZkJ5Gu0LUz1RfdbsROGjn/0k8BD 7xLegbIAtfY0CQPLndJvobBLU4eNlAwo9l35vwZd6JET6tmvz8jdH4wJY A==; X-IronPort-AV: E=McAfee;i="6400,9594,10401"; a="285037227" X-IronPort-AV: E=Sophos;i="5.92,255,1650956400"; d="scan'208";a="285037227" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jul 2022 07:50:03 -0700 X-IronPort-AV: E=Sophos;i="5.92,255,1650956400"; d="scan'208";a="626723098" Received: from yyan2-mobl2.amr.corp.intel.com (HELO [10.212.242.16]) ([10.212.242.16]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jul 2022 07:50:03 -0700 Content-Type: multipart/mixed; boundary="------------9cLz13md01RHbRqOUGKisaoY" Message-ID: Date: Fri, 8 Jul 2022 07:49:55 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH] x86/mm/tlb: ignore f->new_tlb_gen when zero Content-Language: en-US To: Nadav Amit , linux-kernel@vger.kernel.org, Hugh Dickins , Thomas Gleixner Cc: Ingo Molnar , Borislav Petkov , x86@kernel.org, Linux MM , Nadav Amit , Dave Hansen , Peter Zijlstra , Andy Lutomirski References: <20220708003053.158480-1-namit@vmware.com> From: Dave Hansen In-Reply-To: <20220708003053.158480-1-namit@vmware.com> ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1657291806; a=rsa-sha256; cv=none; b=Sddk9RlD92V7RxFrIihVdNA5pEmjXWGDKk+33AtVh38ViIEvzHj3AWXHlpgz/H8OGpc/cB lONrtBVGJ8H6r3NTX0yiwiFj+PUJSDeLGGUYThbxEQPXMnltdpH02y2pSPlX3IrcEUfo9W 0xyJBvU0XGMetsWsP3LXTQVVR4PI81U= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=eQTO46BT; spf=none (imf13.hostedemail.com: domain of dave.hansen@intel.com has no SPF policy when checking 134.134.136.24) smtp.mailfrom=dave.hansen@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1657291806; 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=LaejvingvM4Q0igeuSuQPVtEcH1/Wa+t82pTQ7KrgLY=; b=6lmWxtbPKXQbvuFGE8FJ2vlz6HXfieU/uw84hHUjxLUE3wGlvDzsytimSWwABiXuCOoiht fXcbzcQV2q4peL+fvPZDabhLewkTiF9YKd9tmRGlAVzY/lWkDNGvllWfVBYBOwQ8Vi5/bm SnXxDQuIt79DMNlniTEMHr5XmTMrD+g= X-Rspamd-Server: rspam08 X-Rspam-User: Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=eQTO46BT; spf=none (imf13.hostedemail.com: domain of dave.hansen@intel.com has no SPF policy when checking 134.134.136.24) smtp.mailfrom=dave.hansen@intel.com; dmarc=pass (policy=none) header.from=intel.com X-Stat-Signature: 4bexqgzr38dwyn7pexhmfhzgoszppfph X-Rspamd-Queue-Id: 9B9DF20027 X-HE-Tag: 1657291805-575979 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: This is a multi-part message in MIME format. --------------9cLz13md01RHbRqOUGKisaoY Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 7/7/22 17:30, Nadav Amit wrote: You might want to fix the clock on the system from which you sent this. I was really scratching my head trying to figure out how you got this patch out before Hugh's bug report. > From: Nadav Amit > > Commit aa44284960d5 ("x86/mm/tlb: Avoid reading mm_tlb_gen when > possible") introduced an optimization of skipping the flush if the TLB > generation that is flushed (as provided in flush_tlb_info) was already > flushed. > > However, arch_tlbbatch_flush() does not provide any generation in > flush_tlb_info. As a result, try_to_unmap_one() would not perform any > TLB flushes. > > Fix it by checking whether f->new_tlb_gen is nonzero. Zero value is > anyhow is an invalid generation value. It is, but the check below uses 'f->end == TLB_FLUSH_ALL' as the marker for f->new_tlb_gen being invalid. Being consistent seems like a good idea on this stuff. > In addition, add the missing unlikely() and jump to get tracing right. There are currently five routes out of flush_tlb_func(): * Three early returns * One 'goto done' * One implicit return The tracing code doesn't get run for any of the early returns, but that's intentional because they don't *actually* flush the TLB. We don't want to record that flush_tlb_func() flushed the TLB when it didn't. There's another tracepoint on the TLB_REMOTE_SEND_IPI side to tell where the flushes were requested. That said, I think the if (unlikely(local_tlb_gen == mm_tlb_gen)) goto done; is arguably buggy, as is the 'goto done' in this hunk: > arch/x86/mm/tlb.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c > index d9314cc8b81f..d81b4084bb8a 100644 > --- a/arch/x86/mm/tlb.c > +++ b/arch/x86/mm/tlb.c > @@ -771,14 +771,14 @@ static void flush_tlb_func(void *info) > return; > } > > - if (f->new_tlb_gen <= local_tlb_gen) { > + if (unlikely(f->new_tlb_gen != 0 && f->new_tlb_gen <= local_tlb_gen)) { > /* > * The TLB is already up to date in respect to f->new_tlb_gen. > * While the core might be still behind mm_tlb_gen, checking > * mm_tlb_gen unnecessarily would have negative caching effects > * so avoid it. > */ > - return; > + goto done; > } > > /* We might want to (eventually) think about doing something like the attached patch to make the skipped flushes explicit in the tracing and make the return paths out of this function more consistent. --------------9cLz13md01RHbRqOUGKisaoY Content-Type: text/x-patch; charset=UTF-8; name="tlbtrace.patch" Content-Disposition: attachment; filename="tlbtrace.patch" Content-Transfer-Encoding: base64 ZGlmZiAtLWdpdCBhL2FyY2gveDg2L21tL3RsYi5jIGIvYXJjaC94ODYvbW0vdGxiLmMKaW5k ZXggZDQwMGI2ZDlkMjQ2Li40NGJhNzM2MDFmNTAgMTAwNjQ0Ci0tLSBhL2FyY2gveDg2L21t L3RsYi5jCisrKyBiL2FyY2gveDg2L21tL3RsYi5jCkBAIC03MjAsNyArNzIwLDcgQEAgdm9p ZCBpbml0aWFsaXplX3RsYnN0YXRlX2FuZF9mbHVzaCh2b2lkKQogICogYmVjYXVzZSBhbGwg eDg2IGZsdXNoIG9wZXJhdGlvbnMgYXJlIHNlcmlhbGl6aW5nIGFuZCB0aGUKICAqIGF0b21p YzY0X3JlYWQgb3BlcmF0aW9uIHdvbid0IGJlIHJlb3JkZXJlZCBieSB0aGUgY29tcGlsZXIu CiAgKi8KLXN0YXRpYyB2b2lkIGZsdXNoX3RsYl9mdW5jKHZvaWQgKmluZm8pCitzdGF0aWMg Zmx1c2hfdGxiX2Z1bmModm9pZCAqaW5mbykKIHsKIAkvKgogCSAqIFdlIGhhdmUgdGhyZWUg ZGlmZmVyZW50IHRsYl9nZW4gdmFsdWVzIGluIGhlcmUuICBUaGV5IGFyZToKQEAgLTczOCw2 ICs3MzgsNyBAQCBzdGF0aWMgdm9pZCBmbHVzaF90bGJfZnVuYyh2b2lkICppbmZvKQogCXU2 NCBsb2NhbF90bGJfZ2VuID0gdGhpc19jcHVfcmVhZChjcHVfdGxic3RhdGUuY3R4c1tsb2Fk ZWRfbW1fYXNpZF0udGxiX2dlbik7CiAJYm9vbCBsb2NhbCA9IHNtcF9wcm9jZXNzb3JfaWQo KSA9PSBmLT5pbml0aWF0aW5nX2NwdTsKIAl1bnNpZ25lZCBsb25nIG5yX2ludmFsaWRhdGUg PSAwOworCWVudW0gdGxiX2ZsdXNoX3JlYXNvbjsKIAogCS8qIFRoaXMgY29kZSBjYW5ub3Qg cHJlc2VudGx5IGhhbmRsZSBiZWluZyByZWVudGVyZWQuICovCiAJVk1fV0FSTl9PTighaXJx c19kaXNhYmxlZCgpKTsKQEAgLTc0NywxMiArNzQ4LDIxIEBAIHN0YXRpYyB2b2lkIGZsdXNo X3RsYl9mdW5jKHZvaWQgKmluZm8pCiAJCWNvdW50X3ZtX3RsYl9ldmVudChOUl9UTEJfUkVN T1RFX0ZMVVNIX1JFQ0VJVkVEKTsKIAogCQkvKiBDYW4gb25seSBoYXBwZW4gb24gcmVtb3Rl IENQVXMgKi8KLQkJaWYgKGYtPm1tICYmIGYtPm1tICE9IGxvYWRlZF9tbSkKLQkJCXJldHVy bjsKKwkJaWYgKGYtPm1tICYmIGYtPm1tICE9IGxvYWRlZF9tbSkgeworCQkJZmx1c2hfcmVh c29uID0gVExCX0ZMVVNIX1NLSVBQRUQ7CisJCQlnb3RvIGRvbmU7CisJCX0KKwkJZmx1c2hf cmVhc29uID0gVExCX1JFTU9URV9TSE9PVERPV047CisJfSBlbHNlIGlmIChmLT5tbSA9PSBO VUxMKSB7CisJCWZsdXNoX3JlYXNvbiA9IFRMQl9MT0NBTF9TSE9PVERPV047CisJfSBlbHNl IHsKKwkJZmx1c2hfcmVhc29uID0gVExCX0xPQ0FMX01NX1NIT09URE9XTjsKIAl9CiAKLQlp ZiAodW5saWtlbHkobG9hZGVkX21tID09ICZpbml0X21tKSkKLQkJcmV0dXJuOworCWlmICh1 bmxpa2VseShsb2FkZWRfbW0gPT0gJmluaXRfbW0pKSB7CisJCWZsdXNoX3JlYXNvbiA9IFRM Ql9GTFVTSF9TS0lQUEVEOworCQlnb3RvIGRvbmU7CisJfQogCiAJVk1fV0FSTl9PTih0aGlz X2NwdV9yZWFkKGNwdV90bGJzdGF0ZS5jdHhzW2xvYWRlZF9tbV9hc2lkXS5jdHhfaWQpICE9 CiAJCSAgIGxvYWRlZF9tbS0+Y29udGV4dC5jdHhfaWQpOwpAQCAtNzY4LDcgKzc3OCw4IEBA IHN0YXRpYyB2b2lkIGZsdXNoX3RsYl9mdW5jKHZvaWQgKmluZm8pCiAJCSAqIElQSXMgdG8g bGF6eSBUTEIgbW9kZSBDUFVzLgogCQkgKi8KIAkJc3dpdGNoX21tX2lycXNfb2ZmKE5VTEws ICZpbml0X21tLCBOVUxMKTsKLQkJcmV0dXJuOworCQlmbHVzaF9yZWFzb24gPSBUTEJfRkxV U0hfU0tJUFBFRDsKKwkJZ290byBkb25lOwogCX0KIAogCWlmICh1bmxpa2VseShsb2NhbF90 bGJfZ2VuID09IG1tX3RsYl9nZW4pKSB7CkBAIC03NzgsNiArNzg5LDcgQEAgc3RhdGljIHZv aWQgZmx1c2hfdGxiX2Z1bmModm9pZCAqaW5mbykKIAkJICogYmUgaGFuZGxlZCBjYW4gY2F0 Y2ggdXMgYWxsIHRoZSB3YXkgdXAsIGxlYXZpbmcgbm8gd29yayBmb3IKIAkJICogdGhlIHNl Y29uZCBmbHVzaC4KIAkJICovCisJCWZsdXNoX3JlYXNvbiA9IFRMQl9GTFVTSF9TS0lQUEVE OwogCQlnb3RvIGRvbmU7CiAJfQogCkBAIC04NDksMTAgKzg2MSw3IEBAIHN0YXRpYyB2b2lk IGZsdXNoX3RsYl9mdW5jKHZvaWQgKmluZm8pCiAKIAkvKiBUcmFjaW5nIGlzIGRvbmUgaW4g YSB1bmlmaWVkIG1hbm5lciB0byByZWR1Y2UgdGhlIGNvZGUgc2l6ZSAqLwogZG9uZToKLQl0 cmFjZV90bGJfZmx1c2goIWxvY2FsID8gVExCX1JFTU9URV9TSE9PVERPV04gOgotCQkJCShm LT5tbSA9PSBOVUxMKSA/IFRMQl9MT0NBTF9TSE9PVERPV04gOgotCQkJCQkJICBUTEJfTE9D QUxfTU1fU0hPT1RET1dOLAotCQkJbnJfaW52YWxpZGF0ZSk7CisJdHJhY2VfdGxiX2ZsdXNo KGZsdXNoX3JlYXNvbiwgbnJfaW52YWxpZGF0ZSk7CiB9CiAKIHN0YXRpYyBib29sIHRsYl9p c19ub3RfbGF6eShpbnQgY3B1LCB2b2lkICpkYXRhKQo= --------------9cLz13md01RHbRqOUGKisaoY--