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 934E1C433FE for ; Mon, 28 Feb 2022 21:26:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 15E758D0003; Mon, 28 Feb 2022 16:26:14 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 10E628D0001; Mon, 28 Feb 2022 16:26:14 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EF12C8D0003; Mon, 28 Feb 2022 16:26:13 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0027.hostedemail.com [216.40.44.27]) by kanga.kvack.org (Postfix) with ESMTP id E21D48D0001 for ; Mon, 28 Feb 2022 16:26:13 -0500 (EST) Received: from smtpin19.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id A4C9F9368D for ; Mon, 28 Feb 2022 21:26:13 +0000 (UTC) X-FDA: 79193471826.19.02DAA29 Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) by imf07.hostedemail.com (Postfix) with ESMTP id 1AFDD40008 for ; Mon, 28 Feb 2022 21:26:12 +0000 (UTC) Received: from cwcc.thunk.org (pool-108-7-220-252.bstnma.fios.verizon.net [108.7.220.252]) (authenticated bits=0) (User authenticated as tytso@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 21SLP4dr006996 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 28 Feb 2022 16:25:05 -0500 Received: by cwcc.thunk.org (Postfix, from userid 15806) id 7B5D115C0038; Mon, 28 Feb 2022 16:25:04 -0500 (EST) Date: Mon, 28 Feb 2022 16:25:04 -0500 From: "Theodore Ts'o" To: Jan Kara Cc: Byungchul Park , torvalds@linux-foundation.org, damien.lemoal@opensource.wdc.com, linux-ide@vger.kernel.org, adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org, mingo@redhat.com, linux-kernel@vger.kernel.org, peterz@infradead.org, will@kernel.org, tglx@linutronix.de, rostedt@goodmis.org, joel@joelfernandes.org, sashal@kernel.org, daniel.vetter@ffwll.ch, chris@chris-wilson.co.uk, duyuyang@gmail.com, johannes.berg@intel.com, tj@kernel.org, willy@infradead.org, david@fromorbit.com, amir73il@gmail.com, bfields@fieldses.org, gregkh@linuxfoundation.org, kernel-team@lge.com, linux-mm@kvack.org, akpm@linux-foundation.org, mhocko@kernel.org, minchan@kernel.org, hannes@cmpxchg.org, vdavydov.dev@gmail.com, sj@kernel.org, jglisse@redhat.com, dennis@kernel.org, cl@linux.com, penberg@kernel.org, rientjes@google.com, vbabka@suse.cz, ngupta@vflare.org, linux-block@vger.kernel.org, axboe@kernel.dk, paolo.valente@linaro.org, josef@toxicpanda.com, linux-fsdevel@vger.kernel.org, viro@zeniv.linux.org.uk, jack@suse.com, jlayton@kernel.org, dan.j.williams@intel.com, hch@infradead.org, djwong@kernel.org, dri-devel@lists.freedesktop.org, airlied@linux.ie, rodrigosiqueiramelo@gmail.com, melissa.srw@gmail.com, hamohammed.sa@gmail.com Subject: Re: Report 2 in ext4 and journal based on v5.17-rc1 Message-ID: References: <1645095472-26530-1-git-send-email-byungchul.park@lge.com> <1645096204-31670-1-git-send-email-byungchul.park@lge.com> <1645096204-31670-2-git-send-email-byungchul.park@lge.com> <20220221190204.q675gtsb6qhylywa@quack3.lan> <20220223003534.GA26277@X58A-UD3R> <20220223144859.na2gjgl5efgw5zhn@quack3.lan> <20220224011102.GA29726@X58A-UD3R> <20220224102239.n7nzyyekuacgpnzg@quack3.lan> <20220228092826.GA5201@X58A-UD3R> <20220228101444.6frl63dn5vmgycbp@quack3.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220228101444.6frl63dn5vmgycbp@quack3.lan> X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 1AFDD40008 X-Stat-Signature: 5fzed4t8zhrm795f68n87s45xtqpqxuh Authentication-Results: imf07.hostedemail.com; dkim=none; spf=none (imf07.hostedemail.com: domain of tytso@mit.edu has no SPF policy when checking 18.9.28.11) smtp.mailfrom=tytso@mit.edu; dmarc=fail reason="No valid SPF, No valid DKIM" header.from=mit.edu (policy=none) X-Rspam-User: X-HE-Tag: 1646083572-41727 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 Mon, Feb 28, 2022 at 11:14:44AM +0100, Jan Kara wrote: > > case 1. Code with an actual circular dependency, but not deadlock. > > > > A circular dependency can be broken by a rescue wakeup source e.g. > > timeout. It's not a deadlock. If it's okay that the contexts > > participating in the circular dependency and others waiting for the > > events in the circle are stuck until it gets broken. Otherwise, say, > > if it's not meant, then it's anyway problematic. > > > > 1-1. What if we judge this code is problematic? > > 1-2. What if we judge this code is good? > > > > I've been wondering if the kernel guys esp. Linus considers code with > > any circular dependency is problematic or not, even if it won't lead to > > a deadlock, say, case 1. Even though I designed Dept based on what I > > believe is right, of course, I'm willing to change the design according > > to the majority opinion. > > > > However, I would never allow case 1 if I were the owner of the kernel > > for better stability, even though the code works anyway okay for now. Note, I used the example of the timeout as the most obvious way of explaining that a deadlock is not possible. There is also the much more complex explanation which Jan was trying to give, which is what leads to the circular dependency. It can happen that when trying to start a handle, if either (a) there is not enough space in the journal for new handles, or (b) the current transaction is so large that if we don't close the transaction and start a new hone, we will end up running out of space in the future, and so in that case, start_this_handle() will block starting any more handles, and then wake up the commit thread. The commit thread then waits for the currently running threads to complete, before it allows new handles to start, and then it will complete the commit. In the case of (a) we then need to do a journal checkpoint, which is more work to release space in the journal, and only then, can we allow new handles to start. The botom line is (a) it works, (b) there aren't significant delays, and for DEPT to complain that this is somehow wrong and we need to completely rearchitect perfectly working code because it doesn't confirm to DEPT's idea of what is "correct" is not acceptable. > We have a queue of work to do Q protected by lock L. Consumer process has > code like: > > while (1) { > lock L > prepare_to_wait(work_queued); > if (no work) { > unlock L > sleep > } else { > unlock L > do work > wake_up(work_done) > } > } > > AFAIU Dept will create dependency here that 'wakeup work_done' is after > 'wait for work_queued'. Producer has code like: > > while (1) { > lock L > prepare_to_wait(work_done) > if (too much work queued) { > unlock L > sleep > } else { > queue work > unlock L > wake_up(work_queued) > } > } > > And Dept will create dependency here that 'wakeup work_queued' is after > 'wait for work_done'. And thus we have a trivial cycle in the dependencies > despite the code being perfectly valid and safe. Cheers, - Ted