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 9695CC3DA59 for ; Mon, 22 Jul 2024 20:54:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 280E36B0085; Mon, 22 Jul 2024 16:54:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 231CB6B0088; Mon, 22 Jul 2024 16:54:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 083BD6B0089; Mon, 22 Jul 2024 16:54:18 -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 D9E6E6B0085 for ; Mon, 22 Jul 2024 16:54:17 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 7CB38141A29 for ; Mon, 22 Jul 2024 20:54:17 +0000 (UTC) X-FDA: 82368591354.06.1B674B3 Received: from mail-qk1-f176.google.com (mail-qk1-f176.google.com [209.85.222.176]) by imf12.hostedemail.com (Postfix) with ESMTP id 50B564001D for ; Mon, 22 Jul 2024 20:54:15 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=X1gme6Qj; spf=pass (imf12.hostedemail.com: domain of boqun.feng@gmail.com designates 209.85.222.176 as permitted sender) smtp.mailfrom=boqun.feng@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721681609; 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=q6TvImu7lxiWaBHweYLVQLFGqyazvpOghutfGgukQ2U=; b=6p0Z0dt3yX+FVW7y3cVbbyQDjHnzwDF9mhTDbBlCm9FKDsxMFxJfXvCWXb5JuOPpQ6O7wx jKCkvuOFsKe7FhupYbZt0LYpb9C+0ZpwF31R6R50Zq4QOEPbq21e7W/pBQCu5lV3oqoP2L UDXa0EhDgtVS53LlYp2YDrfdRxT6ueQ= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721681609; a=rsa-sha256; cv=none; b=Hvx0wFH/uZC7lfZhOj3CDZwLMQLRr6b1OkgSk6vEpRgwU/QSgK/ZUFRwh4kt6KUc4HPP49 Ko9hMk20gsti1CvwXow+zLuqw/mlcCwsVlJdcfUdHTnGxnSGej/3nZ5GChGWtWgpXRvhJN zUmZ4woMSM1wqYqRilLIls02tTYV/W0= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=X1gme6Qj; spf=pass (imf12.hostedemail.com: domain of boqun.feng@gmail.com designates 209.85.222.176 as permitted sender) smtp.mailfrom=boqun.feng@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-qk1-f176.google.com with SMTP id af79cd13be357-7a1a6af940cso200006285a.0 for ; Mon, 22 Jul 2024 13:54:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1721681654; x=1722286454; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:feedback-id:from:to:cc:subject:date :message-id:reply-to; bh=q6TvImu7lxiWaBHweYLVQLFGqyazvpOghutfGgukQ2U=; b=X1gme6Qj9VYU/sNWFz6YpOJjGL0gpqOv//xbBmCW0+EhCfi0d63xycwQVmx8USj63c dRVmKhg1IBRSQX3T2rZTL3me7waCguEh91xKXMcxcxNrrWMDnACxazBTQhJ8USugBtXm 8X4LspdkYeYhR8Bb1vZc8S4ybP8SdAz3qdhekmnz4oK3f8DDCzCA4YZdtmOx51SYigIx FoDRRBFq3D5j2ohO7CJmC9aZPp1wi61BvPP8fVBb5dlF9ltJaBnjnywIMdXFYyUyMIxD SdywmKJteO2WceEvLaBJBxrnnwmTDajydp3CUzObgNFgpjrhyGZODOJHNEOnOQf+qkcC ekRQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721681654; x=1722286454; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:feedback-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=q6TvImu7lxiWaBHweYLVQLFGqyazvpOghutfGgukQ2U=; b=LPPda1/c0bA3w76SdKpZukan2h5KmwhEi0arw/OBIF8jnJIrUNhvAFYtMMBg2E2gz8 5gi2Z9+4kflFDP9oSB56SVcUff6c/hrYqd2gy6EMwNbTKOoBnyYAM9blLiV3AgzJvW+F uOjhQVo0Vi1qmcRMX/i8Ofw3ygxFDEUMWPcTK/hLxEagxcDormV3pJ9QN+8EzNN5hwEg is6qORmvYX2rwph5oOtD7ZYAYQCccnZ/2ROhkpSx/RrzkOD63z0t0OrkIu/lRMhumaoB +InUVgx7fINRQOO6a+R2gnVYznQRiCbBD9W8tF2JnSdydH8iK8B4rcW+kaDhJ59vl5iy sPcQ== X-Forwarded-Encrypted: i=1; AJvYcCUjnDf2h0v6hdcFUz1lhQWbFfRV6Iq7a9Q451rXBO6ZpONqL0NlenY/7GKzl+lH2FRXRU6rtCV8YTKLV1EnBMkL+kU= X-Gm-Message-State: AOJu0YyXPcBxCGsbt4q7N+hNDjH4LyMM8ePz2+wtApBtBF91M0iHqgAZ 8Ok59Mh/90wACP0HtRHVl1eXoT7t2ZYpI4xiaFNfL3H5A/QoiNFR X-Google-Smtp-Source: AGHT+IG8mTdIdLQoFIkii7HnHE3YfignHoBS5qpYfDrVTqbFjX9tSgTcfCzDyWhCt6oeodE+3Og+6w== X-Received: by 2002:a05:620a:290b:b0:79f:17f4:7154 with SMTP id af79cd13be357-7a1c2eb92cbmr16040785a.3.1721681654300; Mon, 22 Jul 2024 13:54:14 -0700 (PDT) Received: from fauth1-smtp.messagingengine.com (fauth1-smtp.messagingengine.com. [103.168.172.200]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7a198fba3cfsm402155285a.33.2024.07.22.13.54.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 22 Jul 2024 13:54:13 -0700 (PDT) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailfauth.nyi.internal (Postfix) with ESMTP id A744E1200066; Mon, 22 Jul 2024 16:54:11 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Mon, 22 Jul 2024 16:54:11 -0400 X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrheejgdduheejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvdenucfhrhhomhepuehoqhhu nhcuhfgvnhhguceosghoqhhunhdrfhgvnhhgsehgmhgrihhlrdgtohhmqeenucggtffrrg htthgvrhhnpeehudfgudffffetuedtvdehueevledvhfelleeivedtgeeuhfegueeviedu ffeivdenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpe gsohhquhhnodhmvghsmhhtphgruhhthhhpvghrshhonhgrlhhithihqdeiledvgeehtdei gedqudejjeekheehhedvqdgsohhquhhnrdhfvghngheppehgmhgrihhlrdgtohhmsehfih igmhgvrdhnrghmvg X-ME-Proxy: Feedback-ID: iad51458e:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 22 Jul 2024 16:54:11 -0400 (EDT) Date: Mon, 22 Jul 2024 13:53:52 -0700 From: Boqun Feng To: Dennis Zhou Cc: Tejun Heo , kernel test robot , Suren Baghdasaryan , oe-lkp@lists.linux.dev, lkp@intel.com, linux-kernel@vger.kernel.org, Andrew Morton , Kent Overstreet , Kees Cook , Alexander Viro , Alex Gaynor , Alice Ryhl , Andreas Hindborg , Benno Lossin , =?iso-8859-1?Q?Bj=F6rn?= Roy Baron , Christoph Lameter , Gary Guo , Miguel Ojeda , Pasha Tatashin , Peter Zijlstra , Vlastimil Babka , Wedson Almeida Filho , linux-mm@kvack.org, lkmm@lists.linux.dev Subject: Re: [linus:master] [mm] 24e44cc22a: BUG:KCSAN:data-race_in_pcpu_alloc_noprof/pcpu_block_update_hint_alloc Message-ID: References: <202407191651.f24e499d-oliver.sang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 50B564001D X-Stat-Signature: 7ai8mk7sm4qbtrck3bkoi46bf9apc3wg X-HE-Tag: 1721681655-153999 X-HE-Meta: U2FsdGVkX1/6FxSKEbqQFfPFvGkOwt641BG86o/Z3ZaI0Hvbu47tGkceZd8J8WP0DSj3V7172dKD1zPXcOjl+rEsczu+hpas16lMfegnXC2ArpuFKBlOoUUg4phvli1O4nEquePk0cLaGePMvz4O6lcGqpOJLHoz8F7x763KlRq43ZCUruz6uBKCBV2FGuvrF2iirNvjOXfLTmmXd5dhqx6opYmCXnpwzFEMHRLyRLZpJDniJKlnhJ82HzKWruzUAImineRMuCovEVkrZoldNFaw5+66ht9QJoSevcgcZzZB61zxaLDzMVkuTrhjiR3aWPkiTm2kYbgdWrfli8CupEw5b4Bhh6R8xM5S5MfmFd2JhmLmOYE4m+JAkVM5u1li74r5gueLvu/rZPN3Y4OeKvr63UfEabY+vpvOc4n/G72k8A70NL7laxktRyvQPCOYEm+4ho7iLQIAMaD7OlvnIIoc8qZ787KVKNo0wAHZQMEKY37mMlRTUf9xgAiTqzE377EzY2pkpKU7r1JlYuFs56vOs7cR2b8nsMur3+5gPT8zVpA0t0ymuvBvpKLGt+KGefoQJajRalKE2MgoeSwSJZPNbv9mziMyVgPAMXc41F8QPF/uHgfJKdkn84G+ZpKWIz81gC40QktqcGIfy3U66CTnEHGfHroEUldr2YOGZbLFM0Gdm/+QBPvf6aW8/lOV8wfcFi2Dj7EBvRCd5LsE/p/ZqSIKbMKHIVliyeOYoaLd2XcrEBnBNYDpzBf1v9nTEXiTfYU0IL9WNG3v4QiCLh1r4ZV0vjFIJ1JQAXYHofxwo5Rc/xYUZI7G9boZxc6oAd8wWzyymgxkGuZvYQH1o0XbXuk8aBEjrtyScBAKwOf1cA8MBnJf2WIBZffFKLLrdNYQA/4yPuoLXkD4VQfP5PfaB2/45b8aIvpeLB7uyuEzR21XTTM0910ivjNTH/LIvIURS+6WiW3gAnQ4GLB AIAKDuso t4LIB/OGTk+SZ+dyVN+F32JkFDZlqiU89PSoSB8Rv5ECXeA1SwMJPeu+8p9MP3u7IHup0F04aWpq8kNlEmq5rmO6+SypxHzr87YTUSXs3NSuXI3ODxLsHQIwZKrD2VD/BLfz7BcumeMy+6rFHT49KcrR3pVI3FnGMOTNMD1i6XI4+fzAsAbJiGXS9bKBAQOLFC/x434gt5KXE9hvJQujgEO+FZ7NGBtrqpf/OYWQj3dW9W8H4Ere3ltOmydWcmLQlv33EDiF8cEBl0ajinoS/yWZVrt/TbNz8uQFD++3EnrniPOlqeehOF/k+BTADWWSloLR4DlCue/sdP/LBln6hPCotZIxA7jcH37KTc2h8SHVFouAVI4e4XbMLgDctxFGoY0Y8bCBWjouFYPylQ751twhiLI91sUpTopfbeY++PKpwTlhPWjlryfiohMav8oL28BGF6sT+8R2n+r9wpWWuemseCy+7zOXiOzPmw+X+G73EEqjM7Ren2tp2ppAGcq/2jlnG 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: List-Subscribe: List-Unsubscribe: On Mon, Jul 22, 2024 at 11:27:48AM -0700, Dennis Zhou wrote: > Hello, > > On Mon, Jul 22, 2024 at 11:03:00AM -0700, Boqun Feng wrote: > > On Mon, Jul 22, 2024 at 07:52:22AM -1000, Tejun Heo wrote: > > > On Mon, Jul 22, 2024 at 10:47:30AM -0700, Boqun Feng wrote: > > > > This looks like a data race because we read pcpu_nr_empty_pop_pages out > > > > of the lock for a best effort checking, @Tejun, maybe you could confirm > > > > on this? > > > > > > That does sound plausible. > > > > > > > - if (pcpu_nr_empty_pop_pages < PCPU_EMPTY_POP_PAGES_LOW) > > > > + /* > > > > + * Checks pcpu_nr_empty_pop_pages out of the pcpu_lock, data races may > > > > + * occur but this is just a best-effort checking, everything is synced > > > > + * in pcpu_balance_work. > > > > + */ > > > > + if (data_race(pcpu_nr_empty_pop_pages) < PCPU_EMPTY_POP_PAGES_LOW) > > > > pcpu_schedule_balance_work(); > > > > > > Would it be better to use READ/WRITE_ONCE() for the variable? > > > > > > > For READ/WRITE_ONCE(), we will need to replace all write accesses and > > all out-of-lock read accesses to pcpu_nr_empty_pop_pages, like below. > > It's better in the sense that it doesn't rely on compiler behaviors on > > data races, not sure about the performance impact though. > > > > I think a better alternative is we can move it up into the lock under > area_found. The value gets updated as part of pcpu_alloc_area() as the > code above populates percpu memory that is already allocated. > Not sure I followed what exactly you suggested here because I'm not familiar with the logic, but a simpler version would be: diff --git a/mm/percpu.c b/mm/percpu.c index 20d91af8c033..fc54d27e5786 100644 --- a/mm/percpu.c +++ b/mm/percpu.c @@ -1891,8 +1891,10 @@ void __percpu *pcpu_alloc_noprof(size_t size, size_t align, bool reserved, mutex_unlock(&pcpu_alloc_mutex); } - if (pcpu_nr_empty_pop_pages < PCPU_EMPTY_POP_PAGES_LOW) - pcpu_schedule_balance_work(); + scoped_guard(spinlock_irqsave, &pcpu_lock) { + if (pcpu_nr_empty_pop_pages < PCPU_EMPTY_POP_PAGES_LOW) + pcpu_schedule_balance_work(); + } /* clear the areas and return address relative to base address */ for_each_possible_cpu(cpu) I.e. just locking while checking. Regards, Boqun > We should probably annotate pcpu_update_empty_pages() with: > lockdep_assert_held(&pcpu_lock); > > Thanks, > Dennis > > > Regards, > > Boqun > > > > ----->8 > > diff --git a/mm/percpu.c b/mm/percpu.c > > index 20d91af8c033..729e8188238b 100644 > > --- a/mm/percpu.c > > +++ b/mm/percpu.c > > @@ -570,7 +570,8 @@ static void pcpu_isolate_chunk(struct pcpu_chunk *chunk) > > > > if (!chunk->isolated) { > > chunk->isolated = true; > > - pcpu_nr_empty_pop_pages -= chunk->nr_empty_pop_pages; > > + WRITE_ONCE(pcpu_nr_empty_pop_pages, > > + pcpu_nr_empty_pop_pages - chunk->nr_empty_pop_pages); > > } > > list_move(&chunk->list, &pcpu_chunk_lists[pcpu_to_depopulate_slot]); > > } > > @@ -581,7 +582,8 @@ static void pcpu_reintegrate_chunk(struct pcpu_chunk *chunk) > > > > if (chunk->isolated) { > > chunk->isolated = false; > > - pcpu_nr_empty_pop_pages += chunk->nr_empty_pop_pages; > > + WRITE_ONCE(pcpu_nr_empty_pop_pages, > > + pcpu_nr_empty_pop_pages + chunk->nr_empty_pop_pages); > > pcpu_chunk_relocate(chunk, -1); > > } > > } > > @@ -599,7 +601,8 @@ static inline void pcpu_update_empty_pages(struct pcpu_chunk *chunk, int nr) > > { > > chunk->nr_empty_pop_pages += nr; > > if (chunk != pcpu_reserved_chunk && !chunk->isolated) > > - pcpu_nr_empty_pop_pages += nr; > > + WRITE_ONCE(pcpu_nr_empty_pop_pages, > > + pcpu_nr_empty_pop_pages + nr); > > } > > > > /* > > @@ -1891,7 +1894,7 @@ void __percpu *pcpu_alloc_noprof(size_t size, size_t align, bool reserved, > > mutex_unlock(&pcpu_alloc_mutex); > > } > > > > - if (pcpu_nr_empty_pop_pages < PCPU_EMPTY_POP_PAGES_LOW) > > + if (READ_ONCE(pcpu_nr_empty_pop_pages) < PCPU_EMPTY_POP_PAGES_LOW) > > pcpu_schedule_balance_work(); > > > > /* clear the areas and return address relative to base address */ > > @@ -2754,7 +2757,7 @@ void __init pcpu_setup_first_chunk(const struct pcpu_alloc_info *ai, > > tmp_addr = (unsigned long)base_addr + static_size + ai->reserved_size; > > pcpu_first_chunk = pcpu_alloc_first_chunk(tmp_addr, dyn_size); > > > > - pcpu_nr_empty_pop_pages = pcpu_first_chunk->nr_empty_pop_pages; > > + WRITE_ONCE(pcpu_nr_empty_pop_pages, pcpu_first_chunk->nr_empty_pop_pages); > > pcpu_chunk_relocate(pcpu_first_chunk, -1); > > > > /* include all regions of the first chunk */ > >