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 473FCCD37BC for ; Wed, 4 Sep 2024 01:18:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C13F08D020A; Tue, 3 Sep 2024 21:18:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BC47D8D018A; Tue, 3 Sep 2024 21:18:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A8BCD8D020A; Tue, 3 Sep 2024 21:18:48 -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 864738D018A for ; Tue, 3 Sep 2024 21:18:48 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 3E02FC09A4 for ; Wed, 4 Sep 2024 01:18:48 +0000 (UTC) X-FDA: 82525296336.12.686ECB4 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf17.hostedemail.com (Postfix) with ESMTP id 8E32A4000A for ; Wed, 4 Sep 2024 01:18:45 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=a6vaW8G9; spf=pass (imf17.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1725412631; 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-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=iFAgJzowEQf4c1Q3m0nBhQAhg6SQQ8ROFWi0ejlzwiU=; b=nS4HBAL/3nNKyouoS5tl8fgGkD9rWkIlfgxxREokM5xuuRHZPs4mnZ6PvfIXdVvWpvY3SG b1GGR2knanA6hEG8Wug33VlvWQ21qBGHa1kssvtkpRrHD/g1POjta8q7zbdBXU8U0GMwyZ fKzuR0Q+wnCBsHxT4R//HvLUUVHf3Gc= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725412631; a=rsa-sha256; cv=none; b=z8BrJoaVLaDbzAQ4yPUCFQPSbYVJg9q9/WufIK6bPvkbMoUczO90aib9RFEuNcvfRI1OiA TfPkoTD+TiTPFLO4LET4zIERl0L+wDp8xgNRjSCEA73DlM8pYJ52+q5Tyrx56kKidnvmgz DVSxyWqWIUa/8q5O/TFTF9lxLk+RgMM= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=a6vaW8G9; spf=pass (imf17.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 0E6C25C55A7; Wed, 4 Sep 2024 01:18:41 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 32F76C4CEC4; Wed, 4 Sep 2024 01:18:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1725412724; bh=y/3yN1UyvYk8Wq999clT14OsLdRzhWfqeZkW6GvxlHo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=a6vaW8G9clbLDlTrj5RrieA6o0MP9RhjYpO5XdSMVRsEzfTWOTQCmNXcFNt29K/IE Ii6ZGn7bfMcRf9/BxwyraAucHwdFQ6+7gAQoraG9nsZ6hYFSgoQZaFsRCknIYjQQ3t nBvL7hYmdoj2l2F/sOXP89SfG8UwffnmELIKoFrW2Jyt/TMEIj6cPP2xdQJ0dvs5y9 1ljtUwltwh4os+Om2vmyvBo05lsV9t6Cl0lO9NOYBns/dXhyzy7LQTR6dvnqAg7Ys8 cg6xSdBi5FKzEDciSnQDxuvFj+IllEa9oFoFHzDAcr6R3v7CJsDHuZKHuq/IFP6OqZ B+7265UR+Cssg== From: SeongJae Park To: SeongJae Park Cc: "Liam R. Howlett" , Andrew Morton , "Matthew Wilcox (Oracle)" , David Hildenbrand , Brendan Higgins , David Gow , damon@lists.linux.dev, linux-mm@kvack.org, kunit-dev@googlegroups.com, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, Guenter Roeck Subject: Re: [PATCH] mm/damon/tests/vaddr-kunit: don't use mas_lock for MM_MT_FLAGS-initialized maple tree Date: Tue, 3 Sep 2024 18:18:40 -0700 Message-Id: <20240904011840.973-1-sj@kernel.org> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240904005815.1388-1-sj@kernel.org> References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 8E32A4000A X-Stat-Signature: rbqbmkrdioc1rk76ehf865hemcarp59g X-Rspamd-Server: rspam09 X-Rspam-User: X-HE-Tag: 1725412725-761140 X-HE-Meta: U2FsdGVkX18Z1AsoKbbJGBPHo16nwWqC1zHOHi9Q/4+v0YDgUQlCkvxtWLaWYLR/T1zlGWsZiE0JegyDbliaul78LotAqp7oyvd4NoHfaFAaRjQmyqSw/+0KGghjhzb4mM5vbBh9JNWIjdQH9af/lDpkCkapNuiN6RgK5JXS4RCECZRSU952P5kbzwyg/WvaacP7YBcQeXSXhsoes3cttLaM/amHmCDtwSQUijj1Nrt4T2WUaWgvyoaJLh1Pm/WuVA5QCWURzFnWs5cG21yTObTXsIHJ3FT/3hzOLEA7abNub3dupEX89aBGTDyJF/XvUHVV4Tk3QGoj0HGTWQ934jL/nNNfYrO9vc+4EdYtFRniOXSr0JWJse+a+i5FhLQDr+x3F6LzRssjUHatetTDAbUTq/k5Su/hSG+az8lItVbjupjMknNaElmzbkbQPXhuRC4eGEswuDSdW0fIN42re7lvpkwdQA/PmHssX6AviQrDdiXyLVo6WE4DyIFsRT+CjtwIhMPj3kG0h8xOyVYObhRBLgugkSTa+VH4m5v95jBIiRyE/xne33ojyJvw5gpfuS9gpVEI+iEAUayS5RhKfCegJ2g8Gm32hZFr6bzLLTJp7sUPRUD2VUyzqYYIgXGw+9wM6ta19YnvUTnaE3uXustFNsCxliwN5gz6y98SjJlRf7a4n7DaRQUCG+Nykr3h4vg+OjFYRcy1AMhPMytlUE1aKjLnmnoqJiwGBkeWxmlLpsp7deRx1r2Uo4LvA7wEdjbncAs9LLmScRubkVNaMHJavUlcCjptw9PDatzhYLiIbvncbV2w9Xff6fLwS8rEqXGuKBf32ghYhC4l0W4ofrZrOylgM6/AoyG+gcywarqKPwC2e73B+ruJ8/XLK7eyGcrnmeNGzZF31d3t3szVZD/RNUVnL5aSEYKvl2GWIMQlOkopWcp6eW+f19GwiGK5lKMu0mh+YKQjLNpd1pi mlYm5qcV e4A9DW3RnQ/I0o2aHOIfUHAaWpCxCFYjOz0qidhUg/CkgtE4whJBtWBwvtlgYexXLiJSeVb07wxMXrV93AQlGwBcTmrAFMGBIlcMW5I0LyZrHcdy1DkI0l9THlHf2UqMIEkZ1oDVieXrqLExTigfQAL9rviHiJia6SXh+7cVoBEEWJz3ywpco6lmi2zK5Dx4opXw2voxqnIm3bTRbMqR1pxInsprACAlrYUbhvAlFgvncuzNF4Vw1Tk/xl+JmVK1p2ohHZ08/d6sTw37ykFT6LjvtK7HSGzlbfy2jOzQd9/bncn7XzkPcl4sl6HR9XZtp90tOU9Z1BzRhXjIDXiKOMw6cskL7ccAeCecqrxAgfVArUCBMYyxPqv4dzmKQzWYrBLyN 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 Tue, 3 Sep 2024 17:58:15 -0700 SeongJae Park wrote: > On Tue, 3 Sep 2024 20:48:53 -0400 "Liam R. Howlett" wrote: > > > * SeongJae Park [240903 20:45]: > > > damon_test_three_regions_in_vmas() initializes a maple tree with > > > MM_MT_FLAGS. The flags contains MT_FLAGS_LOCK_EXTERN, which means > > > mt_lock of the maple tree will not be used. And therefore the maple > > > tree initialization code skips initialization of the mt_lock. However, > > > __link_vmas(), which adds vmas for test to the maple tree, uses the > > > mt_lock. In other words, the uninitialized spinlock is used. The > > > problem becomes celar when spinlock debugging is turned on, since it > > > reports spinlock bad magic bug. Fix the issue by not using the mt_lock > > > as promised. > > > > You can't do this, lockdep will tell you this is wrong. > > Hmm, but lockdep was silence on my setup? > > > We need a lock and to use the lock for writes. > > This code is executed by a single-thread test code. Do we still need the lock? > > > > > I'd suggest using different flags so the spinlock is used. > > The reporter mentioned simply dropping MT_FLAGS_LOCK_EXTERN from the flags > causes suspicious RCU usage message. May I ask if you have a suggestion of > better flags? I was actually thinking replacing the mt_init_flags() with mt_init(), which same to mt_init_flags() with zero flag, like below. ``` --- a/mm/damon/tests/vaddr-kunit.h +++ b/mm/damon/tests/vaddr-kunit.h @@ -77,7 +77,7 @@ static void damon_test_three_regions_in_vmas(struct kunit *test) (struct vm_area_struct) {.vm_start = 307, .vm_end = 330}, }; - mt_init_flags(&mm.mm_mt, MM_MT_FLAGS); + mt_init(&mm.mm_mt); if (__link_vmas(&mm.mm_mt, vmas, ARRAY_SIZE(vmas))) kunit_skip(test, "Failed to create VMA tree"); ``` And just confirmed it also convinces the reproducer. But because I'm obviously not familiar with maple tree, would like to hear some comments from Liam or others first. FYI, I ended up writing v1 to simply remove lock usage based on my humble understanding of the documetnation. The maple tree uses a spinlock by default, but external locks can be used for tree updates as well. To use an external lock, the tree must be initialized with the ``MT_FLAGS_LOCK_EXTERN flag``, this is usually done with the MTREE_INIT_EXT() #define, which takes an external lock as an argument. (from Documentation/core-api/maple_tree.rst) I was thinking the fact that the test code is executed in single thread is same to use of external lock. I will be happy to learn if I missed something. Thanks, SJ > > > Thanks, > SJ > > > > > > > > > Reported-by: Guenter Roeck > > > Closes: https://lore.kernel.org/1453b2b2-6119-4082-ad9e-f3c5239bf87e@roeck-us.net > > > Fixes: d0cf3dd47f0d ("damon: convert __damon_va_three_regions to use the VMA iterator") > > > Signed-off-by: SeongJae Park > > > --- > > > mm/damon/tests/vaddr-kunit.h | 10 +++------- > > > 1 file changed, 3 insertions(+), 7 deletions(-) > > > > > > diff --git a/mm/damon/tests/vaddr-kunit.h b/mm/damon/tests/vaddr-kunit.h > > > index 83626483f82b..c6c7e0e0ab07 100644 > > > --- a/mm/damon/tests/vaddr-kunit.h > > > +++ b/mm/damon/tests/vaddr-kunit.h > > > @@ -17,23 +17,19 @@ > > > static int __link_vmas(struct maple_tree *mt, struct vm_area_struct *vmas, > > > ssize_t nr_vmas) > > > { > > > - int i, ret = -ENOMEM; > > > + int i; > > > MA_STATE(mas, mt, 0, 0); > > > > > > if (!nr_vmas) > > > return 0; > > > > > > - mas_lock(&mas); > > > for (i = 0; i < nr_vmas; i++) { > > > mas_set_range(&mas, vmas[i].vm_start, vmas[i].vm_end - 1); > > > if (mas_store_gfp(&mas, &vmas[i], GFP_KERNEL)) > > > - goto failed; > > > + return -ENOMEM; > > > } > > > > > > - ret = 0; > > > -failed: > > > - mas_unlock(&mas); > > > - return ret; > > > + return 0; > > > } > > > > > > /* > > > -- > > > 2.39.2 > > >