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 D0ACECA0EF4 for ; Tue, 12 Sep 2023 15:07:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6943E6B0107; Tue, 12 Sep 2023 11:07:12 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 643976B0108; Tue, 12 Sep 2023 11:07:12 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 533C06B0109; Tue, 12 Sep 2023 11:07:12 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 44C836B0107 for ; Tue, 12 Sep 2023 11:07:12 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 27FA8A0002 for ; Tue, 12 Sep 2023 15:07:09 +0000 (UTC) X-FDA: 81228273378.17.A93814D Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf27.hostedemail.com (Postfix) with ESMTP id DA9A240042 for ; Tue, 12 Sep 2023 15:07:05 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=U5WNSAk9; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf27.hostedemail.com: domain of "SRS0=rctY=E4=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" designates 139.178.84.217 as permitted sender) smtp.mailfrom="SRS0=rctY=E4=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1694531226; h=from:from:sender:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=QVqlIBQnvRSUmJfqMhy4NURB+7S4oJ4fQrkgxDHj8pQ=; b=syjfMVUoG3nsz5FiZL1bZmxwVGd91bFJlLTYTvL5XbyJNOhs4tfgfQV2cENY4Dd+Vgg/oh dRdmP9EeZ7Xl3rUvWhYKlkwuUoMoo3vptWq90GdUkVDK9A5tBXjzSn8winj9YWQGJQvNJf yg3YlzVUiO7EDGq3EJYgqTIfd41NAqQ= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=U5WNSAk9; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf27.hostedemail.com: domain of "SRS0=rctY=E4=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" designates 139.178.84.217 as permitted sender) smtp.mailfrom="SRS0=rctY=E4=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1694531226; a=rsa-sha256; cv=none; b=cCSgANAlYcrR10qGPWhQEM3rSYGZCZwxIrXcKMzrC4KZXd8Upm2nMhnjIETyvaFELm+w/n OdKA5eNj1J+zWwLESR5lJqsRevG1yPc4Me76DDz9gbeBQCKnLoHjrt1eoxx+eVDPgxsnFl F1xnDZ6HO/wAtPFo+buyXq9LWIiyapc= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id CC3446142A; Tue, 12 Sep 2023 15:07:04 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9E598C433CA; Tue, 12 Sep 2023 15:07:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694531224; bh=00jvBVJ7IICw3CmIKyfFR+Wl2uOtXu1lvaB5ZZUykFY=; h=Date:From:To:Subject:Reply-To:References:In-Reply-To:From; b=U5WNSAk9l93CJSL2FAXWwKj6ZhXmyDbhTZwK17FVu+jkcTjlirVurAxow/oTuiiZD MepWvmpCQhvYMKVyOEJCK6Fjdnxn6Pfa7z17kyoZq2znxT7knPRC3Do6YcFFCeLjFO tZCGAX/Jsqi01diFf+IFuGMl+lIfyhuykU2FkDT+oScMh7WHVYhSX2fA1zzgdfmWFg Hnuh4224ob/fuHAIdxRAJQ7BdiKK99BtV6ZFx9NNpMsQx4aFW/1Ilt4Bq1A9uZI9tp 8LX1zwUhcXWT2XdC8FtNoq9YzCvkbqN5qV5cGgkmOmdzLdxXMMhB7w9L1NtULnHTSx Lr2CXWPu0T7nw== Received: by paulmck-ThinkPad-P17-Gen-1.home (Postfix, from userid 1000) id 49ACFCE07C7; Tue, 12 Sep 2023 08:07:04 -0700 (PDT) Date: Tue, 12 Sep 2023 08:07:04 -0700 From: "Paul E. McKenney" To: "Liam R. Howlett" , Geert Uytterhoeven , Andrew Morton , maple-tree@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Shanker Donthineni Subject: Re: [PATCH v2 1/2] maple_tree: Disable mas_wr_append() when other readers are possible Message-ID: <9e85adf9-2e1f-4bed-a58e-9ca629c03579@paulmck-laptop> Reply-To: paulmck@kernel.org References: <20230906172954.oq4vogeuco25zam7@revolver> <495849d6-1dc6-4f38-bce7-23c50df3a99f@paulmck-laptop> <20230911235452.xhtnt7ply7ayr53x@revolver> <33150b55-970c-4607-9015-af0e50e4112d@paulmck-laptop> <62936d98-6353-486e-8535-86c9f90bc7f4@paulmck-laptop> <20230912135617.dnhyk4h5c555l2yg@revolver> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20230912135617.dnhyk4h5c555l2yg@revolver> X-Rspamd-Queue-Id: DA9A240042 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: jbk8or3prnfcu7smfjew71kigf53feoa X-HE-Tag: 1694531225-650553 X-HE-Meta: U2FsdGVkX1/gs4jzlJATY7p1aDpv9u7Y+Rd3zUAA8cPA1fzc/SyMF/ARQ/rDc7Vt9r86LL0br8Qc1JNxcgVekoVVJQU3jYGtlZlOCiPb9hfze4B8A6PuPDtMBPNFlqvBkbQ/Qb8VKrahmc/7iM0PVKLx1PGjLt+P42Ug+G4wJy8vcy3yq5LSRCiU5YU0YxYPSeN1Hi3tB2qYjXxNKjBaDJxSwOsKVlaTbUefe1ZZTNdhgn38i63Dmq133g9zivBwp/o2GSgklXluyZ+uZozRIGPo4x1VLUd4vmMT99s8lJN/UiWoo14Lwn2jiSHc3mOXvWxmEklYNKI+SgDrIYrxQgZYgOnJWUdFsflkG2P5OKfRq5zZH+5Ya07hUNIPTOwQSrT1LZOjIcUOm+xmlVkYZBbAFb62TwRy1aeDAIcRUYyzlfzidcM472RpsSyquRdVWOYsLWzEejVdHdY/9pCz27eFzoXocINRHOuucewemXThQ2zsnbjqA6A2DvkRN9YgX3FeGlMTeAMlsM4YqC4cd+SnACmk97bETEHBULrp573tk287k51fcAzRWEil+ugymEgqBtRtXApU6eSn7GppnNklQ5WoWN05TFbPoVLtHuEFXX8xIRoDm6b1OkeOK6XPA2bu6fqzE77nfq3NuzXdKdyouGfqXHluThDVloe+Q+TDROL2uL1azjXdzRpPP9HM277aPC4jHWjNX71FgIOxd9ZM+Pv7HSB45TDmMWbJpqQhfGEtsWiZ2OonpVY7gA7bKwOohppeNelfyHp9yaguwULQaUzIX0ivycughBv41OxYhy8Cj+gjJ2xoo7yAAIWmfBC/JXkiey+Hjz5SxPE1FqvMINzes7k4EahLgoW2aqtOLdENE7m3Op87AdyeuhLpmqeMxQstyHT72PLwFM8bF0T5yDI7GmxYa+5ivg0qTczdESmRq5uSu5Aev/kb3NEgDoM4odCzGzluHiQ5xtX Mv5A/slF CZXflsV6Krv7hcTgmRitGBkiQ4wAOhzMDtTXqmV40uKoiwn/pcbJ5enoOCF/O2FvAXI+yfCsE21s5jEdzigIF5iJQ1iz7gF7d/fClsUcwF6UaUQn5zSenPipsRF9tt8vqmUZ/VLzj6p7LJVcEjG1Sz+BqTr4pwSo5UC3TwozIQrIIcmrAwPSZO9ezuModYYVwrD4S/S/5buiYfuC3HmPja7T/hdVqFHVNTZbO/3cp7d0fo0keniz0mqXSzcBdSU72G8h087K/HecITfx/E52HfEqkvCwjwoTYNhropwAfe60mO4uQ5O8LgBYt9h0CoS6wJi/AMB6LrvWBcSg4nYOyr5u72dXmH+WmHxaSr4Jq948FTLKxETWzp5IFk52pEGlyLfpTzw5fvBcLY3KeSW+oYvOBi4XEtCFiMYlCexKGUnMDdjnWNhnV5b7+uCuxkjrpLNu5jPrQm+3hXUsffjiwYHHZDUWw7bnaGmOCHZ5FmBUdfuBJk/k1eOOgUGOfkuW/wr/W4mTLrZaKVboPPGJ2wyWT3FA/YX3rBag7VlPHTP6bfMtw9oDiaG2Se0FWCXDcL4cPmGnrlUtPtq4zcV36uozS8nCsvUZljlusNUliEe8cQiGBvOUPKy7Ts8Up2SO41EW39LjPSrztE9B4UXTSx0CdJXfjHDjb/p0BTsw706RbErEmulev3tU4LAU2Rw2uRhp1a7iUBIAGnxoNg7uOe4QK4LA4azJxMUSHYGxz/Ng/oqMWJLM6QzuXkMcJDJBgGfsgzelszug0dSYkbA03uWschhLZBS5J1KCdCDGXmi9XVrQ= 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 Tue, Sep 12, 2023 at 09:56:17AM -0400, Liam R. Howlett wrote: > * Paul E. McKenney [230912 06:00]: > > On Tue, Sep 12, 2023 at 10:34:44AM +0200, Geert Uytterhoeven wrote: > > > Hi Paul, > > > > > > On Tue, Sep 12, 2023 at 10:30 AM Paul E. McKenney wrote: > > > > On Tue, Sep 12, 2023 at 10:23:37AM +0200, Geert Uytterhoeven wrote: > > > > > On Tue, Sep 12, 2023 at 10:14 AM Paul E. McKenney wrote: > > > > > > On Mon, Sep 11, 2023 at 07:54:52PM -0400, Liam R. Howlett wrote: > > > > > > > * Paul E. McKenney [230906 14:03]: > > > > > > > > On Wed, Sep 06, 2023 at 01:29:54PM -0400, Liam R. Howlett wrote: > > > > > > > > > * Paul E. McKenney [230906 13:24]: > > > > > > > > > > On Wed, Sep 06, 2023 at 11:23:25AM -0400, Liam R. Howlett wrote: > > > > > > > > > > > (Adding Paul & Shanker to Cc list.. please see below for why) > > > > > > > > > > > > > > > > > > > > > > Apologies on the late response, I was away and have been struggling to > > > > > > > > > > > get a working PPC32 test environment. > > > > > > > > > > > > > > > > > > > > > > * Geert Uytterhoeven [230829 12:42]: > > > > > > > > > > > > Hi Liam, > > > > > > > > > > > > > > > > > > > > > > > > On Fri, 18 Aug 2023, Liam R. Howlett wrote: > > > > > > > > > > > > > The current implementation of append may cause duplicate data and/or > > > > > > > > > > > > > incorrect ranges to be returned to a reader during an update. Although > > > > > > > > > > > > > this has not been reported or seen, disable the append write operation > > > > > > > > > > > > > while the tree is in rcu mode out of an abundance of caution. > > > > > > > > > > > > > > > > > > > > > > ... > > > > > > > > > > > > > > > > > > > > > > > > > > > ... > > > > > > > > > > > > > > > > > > > RCU-related configs: > > > > > > > > > > > > > > > > > > > > > > > > $ grep RCU .config > > > > > > > > > > > > # RCU Subsystem > > > > > > > > > > > > CONFIG_TINY_RCU=y > > > > > > > > > > > > I must have been asleep last time I looked at this. I was looking at > > > > > > Tree RCU. Please accept my apologies for my lapse. :-/ > > > > > > > > > > > > However, Tiny RCU's call_rcu() also avoids enabling IRQs, so I would > > > > > > have said the same thing, albeit after looking at a lot less RCU code. > > > > > > > > > > > > TL;DR: > > > > > > > > > > > > 1. Try making the __setup_irq() function's call to mutex_lock() > > > > > > instead be as follows: > > > > > > > > > > > > if (!mutex_trylock(&desc->request_mutex)) > > > > > > mutex_lock(&desc->request_mutex); > > > > > > > > > > > > This might fail if __setup_irq() has other dependencies on a > > > > > > fully operational scheduler. > > > > > > > > > > > > 2. Move that ppc32 call to __setup_irq() much later, most definitely > > > > > > after interrupts have been enabled and the scheduler is fully > > > > > > operational. Invoking mutex_lock() before that time is not a > > > > > > good idea. ;-) > > > > > > > > > > There is no call to __setup_irq() from arch/powerpc/? > > > > > > > > Glad it is not just me, given that I didn't see a direct call, either. So > > > > later in this email, I asked Liam to put a WARN_ON_ONCE(irqs_disabled()) > > > > just before that mutex_lock() in __setup_irq(). > > I had already found that this is the mutex lock that is enabling them. > I surrounded the mutex lock to ensure it was not enabled before, but was > after. Here is the findings: > > kernel/irq/manage.c:1587 __setup_irq: > [ 0.000000] [c0e65ec0] [c00e9b00] __setup_irq+0x6c4/0x840 (unreliable) > [ 0.000000] [c0e65ef0] [c00e9d74] request_threaded_irq+0xf8/0x1f4 > [ 0.000000] [c0e65f20] [c0c27168] pmac_pic_init+0x204/0x5f8 > [ 0.000000] [c0e65f80] [c0c1f544] init_IRQ+0xac/0x12c > [ 0.000000] [c0e65fa0] [c0c1cad0] start_kernel+0x544/0x6d4 > > Note your line number will be slightly different due to my debug. This > is the WARN _after_ the mutex lock. > > > > > > > > > Either way, invoking mutex_lock() early in boot before interrupts have > > > > been enabled is a bad idea. ;-) > > > > > > I'll add that WARN_ON_ONCE() too, and will report back later today... > > > > Thank you, looking forward to hearing the outcome! > > > > > > > Note that there are (possibly different) issues seen on ppc32 and on arm32 > > > > > (Renesas RZ/A in particular, but not on other Renesas ARM systems). > > > > > > > > > > I saw an issue on arm32 with cfeb6ae8bcb96ccf, but not with cfeb6ae8bcb96ccf^. > > > > > Other people saw an issue on ppc32 with both cfeb6ae8bcb96ccf and > > > > > cfeb6ae8bcb96ccf^. > > > > > > > > I look forward to hearing what is the issue in both cases. > > > > > > For RZ/A, my problem report is > > > https://lore.kernel.org/all/3f86d58e-7f36-c6b4-c43a-2a7bcffd3bd@linux-m68k.org/ > > > > Thank you, Geert! > > > > Huh. Is that patch you reverted causing Maple Tree or related code > > to attempt to acquire mutexes in early boot before interrupts have > > been enabled? > > > > If that added WARN_ON_ONCE() doesn't trigger early, another approach > > would be to put it at the beginning of mutex_lock(). Or for that matter > > at the beginning of might_sleep(). > > Yeah, I put many WARN() calls through the code as well as tracking down > where TIF_NEED_RESCHED was set; the tiny.c call_rcu(). > > > So my findings summarized: > > 1. My change to the maple tree makes call_rcu() more likely on early boot. > 2. The initial thread setup is always set to idle state > 3. call_rcu() tiny sets TIF_NEED_RESCHED since is_idle_task(current) > 4. init_IRQ() takes a mutex lock which will enable the interrupts since > TIF_NEED_RESCHED is set. > > I don't know which of these things is "wrong". Doing early-boot call_rcu() is OK. The initial thread eventually becomes the idle thread for the boot CPU. See rest_init() in init/main.c. I can certainly make Tiny call_rcu() refrain from invoking resched_cpu() during boot, as shown in the (untested) patch below. This might result in boot-time hangs, though. The thought of doing mutex_lock() before interrupts are enabled on the boot CPU strikes me as very wrong. Others might argue that the fact that __might_resched() explicitly avoids complaining when system_state is equal to SYSTEM_BOOTING constitutes evidence that such calls are OK. (Which might be why enabling debug suppressed the problem.) Except that if you actually try sleeping at that time, nothing good can possibly happen. So my question is why is it useful to setup interrupts that early, given that interrupts cannot possibly happen until the boot CPU enables them? > I also looked into the mtmsr register but decided to consult you lot > about my findings in hopes that someone with more knowledge of the > platform or early boot would alleviate the pain so that I could context > switch or sleep :) I mean, an mtmsr bug seems like a leap even for the > issues I create.. Stranger things have happened, but I agree that this would indeed be quite strange! ;-) Thanx, Paul ------------------------------------------------------------------------ diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c index fec804b79080..f00fb0855e4b 100644 --- a/kernel/rcu/tiny.c +++ b/kernel/rcu/tiny.c @@ -192,7 +192,7 @@ void call_rcu(struct rcu_head *head, rcu_callback_t func) rcu_ctrlblk.curtail = &head->next; local_irq_restore(flags); - if (unlikely(is_idle_task(current))) { + if (unlikely(is_idle_task(current)) && system_state > SYSTEM_BOOTING) { /* force scheduling for rcu_qs() */ resched_cpu(0); }