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 98610C7619A for ; Sat, 8 Apr 2023 07:13:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 72057900002; Sat, 8 Apr 2023 03:13:33 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6D0886B0074; Sat, 8 Apr 2023 03:13:33 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 597FA900002; Sat, 8 Apr 2023 03:13:33 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 44E9E6B0072 for ; Sat, 8 Apr 2023 03:13:33 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 0E93C16057D for ; Sat, 8 Apr 2023 07:13:33 +0000 (UTC) X-FDA: 80657358306.26.E5A5A4B Received: from mail-pj1-f44.google.com (mail-pj1-f44.google.com [209.85.216.44]) by imf07.hostedemail.com (Postfix) with ESMTP id 436A240016 for ; Sat, 8 Apr 2023 07:13:31 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none); spf=pass (imf07.hostedemail.com: domain of dennisszhou@gmail.com designates 209.85.216.44 as permitted sender) smtp.mailfrom=dennisszhou@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1680938011; 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; bh=WFPAwgiHpT6Plk0qWcWWjF87lkMYlj7GI2OBZM12Wk8=; b=0UfuSbnmBdaJDjJGUcT464OUqgJP4VhaEPRolLFvqYlEZkPUCCX14AXLL02q/m04w1C5kL EdeWfl2yfuhIiwbAooyb4MDpdyClYHiMAZPhR6aMNeVY17lMkluKis+G3dLSARfDAyI3V1 8MhAzbdq9sK9/Dn7wW5X9Qw/6rQg8fo= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none); spf=pass (imf07.hostedemail.com: domain of dennisszhou@gmail.com designates 209.85.216.44 as permitted sender) smtp.mailfrom=dennisszhou@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680938011; a=rsa-sha256; cv=none; b=NRiqvLdqxjaDr2UNAV+k4zeoww54gYkYsNYImApJo/DAEAdiYtMILd37Xb4fQs/HXwb30/ +Zj/ymZSX7bKmmpo7zVc3vECDYRauhTShd2AuUltlI3rouKLvrA+l4R1/7pvfQ/qcqGgko DImhSW4cWd1L1n0PXJI+0qqOgBStTy4= Received: by mail-pj1-f44.google.com with SMTP id r13-20020a17090a940d00b0023f07253a2cso433583pjo.3 for ; Sat, 08 Apr 2023 00:13:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680938010; x=1683530010; 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=WFPAwgiHpT6Plk0qWcWWjF87lkMYlj7GI2OBZM12Wk8=; b=7rD9l7NBEhOauhesloIVw0cTuInHjFB3wKlr5m46/3p5m2nroQsEMvS1vC5GpwEEvS 8nfK5Cv/lo7UGhK8by4IQR1hOkSkNUY2AI+zSDTQJ5PzMggB6wgEfFoAU+q6xDhKnPNZ EkLY7zyQZ0lRr/MbjUE4eoE/CmiRgkTgvoue3QpqG8gi9pVjx1mYQsvSmwxw5eHMp7T/ laqymYRd6KqeNZA93osdgd4/ER7E3zHcU4vMZHZkTxi2mJIMSWw4G7TaB1m7tpUXRm2i A0bN14JHXxHt1w+3kcQL0b43FdO77eiKLNaV6JdY3A+CxwihLJU/H1KOFwzSYfub01lU SKZA== X-Gm-Message-State: AAQBX9fV2RiuOOE/pqPmUvfq/NN2eeYcZUAL533LpHOaqS8ecxcw46fV h1HF84WaFRY+383TzGMRpLk= X-Google-Smtp-Source: AKy350akPQ2x1a+/y+SQRvlEsDWR2888PrYXbK3d3/S5x1PK1YzPSjyuY3yt62l7K2TKcbhc0OSOrw== X-Received: by 2002:a17:903:784:b0:19f:3b86:4710 with SMTP id kn4-20020a170903078400b0019f3b864710mr1073029plb.49.1680938009852; Sat, 08 Apr 2023 00:13:29 -0700 (PDT) Received: from snowbird (136-24-99-118.cab.webpass.net. [136.24.99.118]) by smtp.gmail.com with ESMTPSA id jk15-20020a170903330f00b001a5157d1a9dsm2080078plb.14.2023.04.08.00.13.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 08 Apr 2023 00:13:29 -0700 (PDT) Date: Sat, 8 Apr 2023 00:13:26 -0700 From: Dennis Zhou To: Ye Bin Cc: tj@kernel.org, cl@linux.com, linux-mm@kvack.org, yury.norov@gmail.com, andriy.shevchenko@linux.intel.com, linux@rasmusvillemoes.dk, linux-kernel@vger.kernel.org, yebin10@huawei.com, dchinner@redhat.com Subject: Re: [PATCH v2 2/2] lib/percpu_counter: fix dying cpu compare race Message-ID: References: <20230406015629.1804722-1-yebin@huaweicloud.com> <20230406015629.1804722-3-yebin@huaweicloud.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230406015629.1804722-3-yebin@huaweicloud.com> X-Rspamd-Queue-Id: 436A240016 X-Stat-Signature: aueygm5msff48yub34pxejz1pjkwzha3 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1680938011-156698 X-HE-Meta: U2FsdGVkX18f38wkbChuLTDEK4g3KYkZQUVPAQR2KzNLOh6un9BSsyViRc2Z/XmWlqz9MerZDRFAcNg9RVmuO/as00GNHBaMp7KXChMtW9EKzRsFl3jrkXRK+F61l9aX7T5aqlNR+II7kIrFNJ58vZN9mAWem2jBYMIihq1ojHtXHm3OomB5PbDgSUyIgZHP3b/LyAMXRGkidmtnDBYMetQfF1Hnq3fZ1wERuNjQzzAiIHU/McRBANR19uQsVavlXD8bJ5DsyLGh6R2guhoCrH9ItTlPGzr9lxchYDWdGjx9+eyMuvUb9X3vNG7wn7SKaEfrnUk4FVunbf3h5VlwSDjne+fEHxaalJ3T+5zTl6bvtuQk6IlkCiR7kmSo9UqkRGlXpdAKJUxJn6/yedeKs7rM9wp9+0LMgjvKH0dog8XMg7l5zXZmTayQp6izr4l5tboMQGFFF/JC5Dz/KhyL7bs4oHsQ/f5ND4q8GoMelSXmPeu7gOCdXTEsMVfxtJaj1l6sQzck15tQB+UtkSEZi008QcRXXsU+HtE5TkbERJgvyhI+xnE6ieDbMuKpu/9HCtJGg643Bay+5wA8ZRL3rGWZfHnn93+JAtFMYTUaeAlhqP4bTlyoRlF26H7dfnvgrHo0vp4SaWeY0z/xSjWorbOKdtfk+EQ762veZwPgUUcaYVlOz1zQndFkFov910q8IAo8JhJV9dHuVpI00Dqh9Y9YvL9oitdqqvLHE3WWX3eOzH5/CGE/8s8cnHCDxaRNkU8uvVvr7+GrOxnOYVzcB8WT1TiAKL9BYNFl7rv4OkM5Irzb8uq0dl/ewEkmln6qsI5EyPeyB0mkNr3Rh0pWJ8sbh9+H7ScmOqZlyH1zyPl2/DadQggt6Uqa2yx9cBONPrk4ZPMhmiHg2YQ0+vj00yLiUAYJ/tuJmLabrm0Esdc2ufYpdb+vm/BJh36LT5QLmPO7EPnR4PiWmA/8Swm bsJ47DUU hEYUF/pqwu+ssV1gmubc/Kr0c7RXycyBOrSBJxv7WGAEMX/TOIuu1pfnoZkMhcEVl5kNsWFq01MH7Xl9w4JuzZNHmBrgZ74uPsR0fOkD7FibWLqSapTrYzQFT2oRARoNnSXRdTxjbognVJeLMMtvTA/2GbNWe6brhKX8HXJX6qlw7S1VRjrwUGcRHjHouGX/XVa/2/36x5zoGxLBJPkiphVGBg4RKtSgUBQEo4vb3/ov6h/9FpaspL76nka9ijVBJECUNBBNIzcjqGjEhTh/FbwLEWnGU3SRdM/s4pzHEmZI9khz9lRR4pUinHbSzDSnaFh6aE4llXyOxwzVYcKTLiyVYYpnLS9sif5LPtgQo5a2S348QkeDTC7NL8pNplmcm7C1llXFJ92sM6dX3dU0U4n627LNQxjHEJ/2UILVISPckOjXcUVTLAwI3QaA4Ur5T6SmqPLFw5Cj5CpGG4dGx3RDA3lCAZPisg62fWUMFx84ZeznuhKj4YRSTX4VybUySybOqFhMckf3PNfHL9akQoMR/Xve/YuOEwjAN3bf/3pc+ITphtOKRFmAbK0jNH4KyGglmO6XChx0yaKhNuR3MCKUwnMdCMfReLMgphT0opcplDdCdqe515ISRsgeAbBfg/cKWYU3avnKpuBBerqAtCBwQY1NlDKjylIMO7MAHrhXFe3U= 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: Hello, On Thu, Apr 06, 2023 at 09:56:29AM +0800, Ye Bin wrote: > From: Ye Bin > > In commit 8b57b11cca88 ("pcpcntrs: fix dying cpu summation race") a race > condition between a cpu dying and percpu_counter_sum() iterating online CPUs > was identified. > Acctually, there's the same race condition between a cpu dying and > __percpu_counter_compare(). Here, use 'num_online_cpus()' for quick judgment. > But 'num_online_cpus()' will be decreased before call 'percpu_counter_cpu_dead()', > then maybe return incorrect result. > To solve above issue, also need to add dying CPUs count when do quick judgment > in __percpu_counter_compare(). > I've thought a lot of about this since you've sent v1. For the general problem, you haven't addressed Dave's concerns from [1]. I agree you've found a valid race condition, but as Dave mentioned, there's no synchronization in __percpu_counter_compare() and consequently no guarantees about the accuracy of the value. However, I might be missing something, but I do think the use case in 5825bea05265 ("xfs: __percpu_counter_compare() inode count debug too expensive") is potentially valid. If the rhs is an expected lower bound or upper bound (depending on if you're counting up or down, but not both) and the count you're accounting has the same expectations as percpu_refcount (you can only subtract what you've already added), then should the percpu_counter_sum() ever be on the wrong side of rhs, that should be an error and visible via percpu_counter_compare(). I need to think a little longer, but my initial thought is while you close a race condition, the function itself is inherently vulnerable. [1] https://lore.kernel.org/lkml/ZCu9LtdA+NMrfG9x@rh/ Thanks, Dennis > Signed-off-by: Ye Bin > --- > lib/percpu_counter.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/lib/percpu_counter.c b/lib/percpu_counter.c > index 5004463c4f9f..399840cb0012 100644 > --- a/lib/percpu_counter.c > +++ b/lib/percpu_counter.c > @@ -227,6 +227,15 @@ static int percpu_counter_cpu_dead(unsigned int cpu) > return 0; > } > > +static __always_inline unsigned int num_count_cpus(void) > +{ > +#ifdef CONFIG_HOTPLUG_CPU > + return (num_online_cpus() + num_dying_cpus()); > +#else > + return num_online_cpus(); > +#endif > +} > + > /* > * Compare counter against given value. > * Return 1 if greater, 0 if equal and -1 if less > @@ -237,7 +246,7 @@ int __percpu_counter_compare(struct percpu_counter *fbc, s64 rhs, s32 batch) > > count = percpu_counter_read(fbc); > /* Check to see if rough count will be sufficient for comparison */ > - if (abs(count - rhs) > (batch * num_online_cpus())) { > + if (abs(count - rhs) > (batch * num_count_cpus())) { > if (count > rhs) > return 1; > else > -- > 2.31.1 > >