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 6C5C4C433EF for ; Thu, 17 Mar 2022 21:16:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BA1CC8D0002; Thu, 17 Mar 2022 17:16:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B51178D0001; Thu, 17 Mar 2022 17:16:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A18D68D0002; Thu, 17 Mar 2022 17:16:29 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0239.hostedemail.com [216.40.44.239]) by kanga.kvack.org (Postfix) with ESMTP id 8EF128D0001 for ; Thu, 17 Mar 2022 17:16:29 -0400 (EDT) Received: from smtpin25.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 4943D9D67E for ; Thu, 17 Mar 2022 21:16:29 +0000 (UTC) X-FDA: 79255136898.25.2A9F2D8 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf31.hostedemail.com (Postfix) with ESMTP id 8770F20016 for ; Thu, 17 Mar 2022 21:16:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=mQVi/8AUGelccD6B82H7IX/Q4dG4lWAcNI5AMJjQ7Ww=; b=hJe+x6ypYZMPRYM+nzGb+mzE3o M4G15eQPY+WAdTrlxvOCmJxVkqSZvod7K8f+G3t1ZECiN/R9M9rrWjBJymo282e1laNhQutHt8jE3 eaJAmEcmTMVF3VAi8UZALMXOSsqaA2iDoghRHxTzSil6AMVmdndedIPoecexanjGyfPJuviG1gGce l8MIKXuzvBfPYYSDo3bv47PYE0zQDThYogTCLghFBfK6Ph6cp7lYNFQQfMq65hF6Vpfjmw9zB806A B4Wf4GAQMQ598uNZ+qDtoA7Wxj+OJFt27QVkKml6t4YMiBuC1wUNAW/XEYNwij4+//zDgcsGCDI4y TLkcR+GQ==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1nUxTo-007KMx-A5; Thu, 17 Mar 2022 21:16:20 +0000 Date: Thu, 17 Mar 2022 21:16:20 +0000 From: Matthew Wilcox To: Linus Torvalds Cc: Brian Foster , Linux-MM , linux-fsdevel , linux-xfs , Hugh Dickins , Namjae Jeon , Ashish Sangwan , Theodore Ts'o , Jan Kara , linux-ext4@vger.kernel.org Subject: Re: writeback completion soft lockup BUG in folio_wake_bit() Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Stat-Signature: 6uggacgb33tc3qwbu7ix9pw88kogw8ct Authentication-Results: imf31.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=hJe+x6yp; spf=none (imf31.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none X-Rspam-User: X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 8770F20016 X-HE-Tag: 1647551788-222212 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 Thu, Mar 17, 2022 at 12:26:35PM -0700, Linus Torvalds wrote: > On Thu, Mar 17, 2022 at 8:04 AM Matthew Wilcox wrote: > > > > So how about we do something like this: > > > > - Make folio_start_writeback() and set_page_writeback() return void, > > fixing up AFS and NFS. > > - Add a folio_wait_start_writeback() to use in the VFS > > - Remove the calls to set_page_writeback() in the filesystems > > That sounds lovely, but it does worry me a bit. Not just the odd > 'keepwrite' thing, but also the whole ordering between the folio bit > and the tagging bits. Does the ordering possibly matter? I wouldn't change the ordering of setting the xarray bits and the writeback flag; they'd just be set a little earlier. It'd all be done while the page was still locked. But you're right, there's lots of subtle interactions here. > That whole "xyz_writeback_keepwrite()" thing seems odd. It's used in > only one place (the folio version isn't used at all): > > ext4_writepage(): > > ext4_walk_page_buffers() fails: > redirty_page_for_writepage(wbc, page); > keep_towrite = true; > ext4_bio_write_page(). > > which just looks odd. Why does it even try to continue to do the > writepage when the page buffer thing has failed? > > In the regular write path (ie ext4_write_begin()), a > ext4_walk_page_buffers() failure is fatal or causes a retry). Why is > ext4_writepage() any different? Particularly since it wants to keep > the page dirty, then trying to do the writeback just seems wrong. > > So this code is all a bit odd, I suspect there are decades of "people > continued to do what they historically did" changes, and it is all > worrisome. I found the commit: 1c8349a17137 ("ext4: fix data integrity sync in ordered mode"). Fortunately, we have a documented test for this, generic/127, so we'll know if we've broken it.