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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 73020C433EF for ; Thu, 17 Mar 2022 21:16:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230179AbiCQVRq (ORCPT ); Thu, 17 Mar 2022 17:17:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57842 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230135AbiCQVRq (ORCPT ); Thu, 17 Mar 2022 17:17:46 -0400 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 65F0A1A7770; Thu, 17 Mar 2022 14:16:29 -0700 (PDT) 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: Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org 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.