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 4D49FC4332F for ; Fri, 4 Nov 2022 03:37:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230231AbiKDDhn (ORCPT ); Thu, 3 Nov 2022 23:37:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37154 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229572AbiKDDhQ (ORCPT ); Thu, 3 Nov 2022 23:37:16 -0400 Received: from mail-pg1-x536.google.com (mail-pg1-x536.google.com [IPv6:2607:f8b0:4864:20::536]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C54F460E7 for ; Thu, 3 Nov 2022 20:37:03 -0700 (PDT) Received: by mail-pg1-x536.google.com with SMTP id b62so3394751pgc.0 for ; Thu, 03 Nov 2022 20:37:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20210112.gappssmtp.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=kCztCif6DGnUVPea69d1j6HBte1tInJ//NGbT2adCtA=; b=uj2thbHtG/d+GBBCufhO7rN+QkuEm7zsR1utnZpf1jQ6jQI6apkQTpUS2wcEmA9aNC VnDMe2rmlwjjtPUJR6RVdf+UJIK/YCtwiDAFrNIV7Nt++fXhl9nREiFkHf0RyMEPfvfO onGWmNN1ovJwkvkYC6fzzHZz7zyvVev9H3OguSEusghpv0YPPtdfeKpV8TCgCnMYqtfc Ksic71C8032fBMagPD7RLDMZgSbzkcCWA6HBFsZEVIK8PskhHmcNukEVkFYsBkmvmeoC g/qCQdT1fcwEOyEG1ZYfH8GugnyK0CKLKk6hzKwIFkwwhNd+ojc6bS3gdGaV6YzySSoe prLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=kCztCif6DGnUVPea69d1j6HBte1tInJ//NGbT2adCtA=; b=DYYWsYSlonusd8JM9dFUOySCHg84n61oZLdSEaIfs9as9Ti4g/FctpyS7HOK+iGw3S szMxyv95AQ9qaNms0cAVlEC1NScd+czPMYJqx1sRQKBYPfXDWgTzCMvFkMN6zv2B/g2k ySbK5b+qvCml3Zn/XSNupS4f6UziQiQ4Id6/nb6Wma4gcrHs+eYa1ndReN08gtMyVdeh I/zpa51Eg5VcAQSJZEBN0AGWr7AdGoOPLhOKaeQQvkpGskHYmZjNvEmWUJSwxJdBbwRZ 7ABALWNNcHTcBWu4g8I4K/o1keV7SPbCqupZr6fEwg7ba2lMNBItN5VDLbUoWycAh7S4 xhEw== X-Gm-Message-State: ACrzQf0QlZ38R/mwB9X2U0PH5QDqnSzMnWOPASNCR4fzaz5cbjnejKZB /Xs68VLp2+0Pdtcc3KDd5mUPSw== X-Google-Smtp-Source: AMsMyM7NwhgMGw/Ejg/10+QrYuOi1WfTIe60hww1ZNfzsfJwCZnIyT2jB8JwYZIzMahLdSnJ6wzgKQ== X-Received: by 2002:a05:6a00:2396:b0:56c:b770:eda6 with SMTP id f22-20020a056a00239600b0056cb770eda6mr33239633pfc.38.1667533023291; Thu, 03 Nov 2022 20:37:03 -0700 (PDT) Received: from dread.disaster.area (pa49-181-106-210.pa.nsw.optusnet.com.au. [49.181.106.210]) by smtp.gmail.com with ESMTPSA id x13-20020a170902a38d00b001830ed575c3sm1430190pla.117.2022.11.03.20.37.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 03 Nov 2022 20:37:02 -0700 (PDT) Received: from dave by dread.disaster.area with local (Exim 4.92.3) (envelope-from ) id 1oqnVr-00A1XB-Bm; Fri, 04 Nov 2022 14:36:59 +1100 Date: Fri, 4 Nov 2022 14:36:59 +1100 From: Dave Chinner To: "Darrick J. Wong" Cc: Vishal Moola , linux-fsdevel@vger.kernel.org, linux-afs@lists.infradead.org, linux-kernel@vger.kernel.org, linux-btrfs@vger.kernel.org, ceph-devel@vger.kernel.org, linux-cifs@vger.kernel.org, linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, cluster-devel@redhat.com, linux-nilfs@vger.kernel.org, linux-mm@kvack.org, Matthew Wilcox Subject: Re: [PATCH 04/23] page-writeback: Convert write_cache_pages() to use filemap_get_folios_tag() Message-ID: <20221104033659.GA2703033@dread.disaster.area> References: <20220901220138.182896-1-vishal.moola@gmail.com> <20220901220138.182896-5-vishal.moola@gmail.com> <20221018210152.GH2703033@dread.disaster.area> <20221104003235.GZ2703033@dread.disaster.area> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org On Thu, Nov 03, 2022 at 07:45:01PM -0700, Darrick J. Wong wrote: > On Fri, Nov 04, 2022 at 11:32:35AM +1100, Dave Chinner wrote: > > On Thu, Nov 03, 2022 at 03:28:05PM -0700, Vishal Moola wrote: > > > On Wed, Oct 19, 2022 at 08:01:52AM +1100, Dave Chinner wrote: > > > > On Thu, Sep 01, 2022 at 03:01:19PM -0700, Vishal Moola (Oracle) wrote: > > > > > - BUG_ON(PageWriteback(page)); > > > > > - if (!clear_page_dirty_for_io(page)) > > > > > + BUG_ON(folio_test_writeback(folio)); > > > > > + if (!folio_clear_dirty_for_io(folio)) > > > > > goto continue_unlock; > > > > > > > > > > trace_wbc_writepage(wbc, inode_to_bdi(mapping->host)); > > > > > - error = (*writepage)(page, wbc, data); > > > > > + error = writepage(&folio->page, wbc, data); > > > > > > > > Yet, IIUC, this treats all folios as if they are single page folios. > > > > i.e. it passes the head page of a multi-page folio to a callback > > > > that will treat it as a single PAGE_SIZE page, because that's all > > > > the writepage callbacks are currently expected to be passed... > > > > > > > > So won't this break writeback of dirty multipage folios? > > > > > > Yes, it appears it would. But it wouldn't because its already 'broken'. > > > > It is? Then why isn't XFS broken on existing kernels? Oh, we don't > > know because it hasn't been tested? > > > > Seriously - if this really is broken, and this patchset further > > propagating the brokeness, then somebody needs to explain to me why > > this is not corrupting data in XFS. > > It looks like iomap_do_writepage finds the folio size correctly > > end_pos = folio_pos(folio) + folio_size(folio); > > and iomap_writpage_map will map out the correct number of blocks > > unsigned nblocks = i_blocks_per_folio(inode, folio); > > for (i = 0; i < nblocks && pos < end_pos; i++, pos += len) { > > right? Yup, that's how I read it, too. But my recent experience with folios involved being repeatedly burnt by edge case corruptions due to multipage folios showing up when and where I least expected them. Hence doing a 1:1 conversion of page based code to folio based code and just assuming large folios will work without any testing seems akin to playing russian roulette with loose cannons that have been doused with napalm and then set on fire by an air-dropped barrel bomb... Cheers, Dave. -- Dave Chinner david@fromorbit.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Chinner Date: Fri, 4 Nov 2022 14:36:59 +1100 Subject: [Cluster-devel] [PATCH 04/23] page-writeback: Convert write_cache_pages() to use filemap_get_folios_tag() In-Reply-To: References: <20220901220138.182896-1-vishal.moola@gmail.com> <20220901220138.182896-5-vishal.moola@gmail.com> <20221018210152.GH2703033@dread.disaster.area> <20221104003235.GZ2703033@dread.disaster.area> Message-ID: <20221104033659.GA2703033@dread.disaster.area> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Thu, Nov 03, 2022 at 07:45:01PM -0700, Darrick J. Wong wrote: > On Fri, Nov 04, 2022 at 11:32:35AM +1100, Dave Chinner wrote: > > On Thu, Nov 03, 2022 at 03:28:05PM -0700, Vishal Moola wrote: > > > On Wed, Oct 19, 2022 at 08:01:52AM +1100, Dave Chinner wrote: > > > > On Thu, Sep 01, 2022 at 03:01:19PM -0700, Vishal Moola (Oracle) wrote: > > > > > - BUG_ON(PageWriteback(page)); > > > > > - if (!clear_page_dirty_for_io(page)) > > > > > + BUG_ON(folio_test_writeback(folio)); > > > > > + if (!folio_clear_dirty_for_io(folio)) > > > > > goto continue_unlock; > > > > > > > > > > trace_wbc_writepage(wbc, inode_to_bdi(mapping->host)); > > > > > - error = (*writepage)(page, wbc, data); > > > > > + error = writepage(&folio->page, wbc, data); > > > > > > > > Yet, IIUC, this treats all folios as if they are single page folios. > > > > i.e. it passes the head page of a multi-page folio to a callback > > > > that will treat it as a single PAGE_SIZE page, because that's all > > > > the writepage callbacks are currently expected to be passed... > > > > > > > > So won't this break writeback of dirty multipage folios? > > > > > > Yes, it appears it would. But it wouldn't because its already 'broken'. > > > > It is? Then why isn't XFS broken on existing kernels? Oh, we don't > > know because it hasn't been tested? > > > > Seriously - if this really is broken, and this patchset further > > propagating the brokeness, then somebody needs to explain to me why > > this is not corrupting data in XFS. > > It looks like iomap_do_writepage finds the folio size correctly > > end_pos = folio_pos(folio) + folio_size(folio); > > and iomap_writpage_map will map out the correct number of blocks > > unsigned nblocks = i_blocks_per_folio(inode, folio); > > for (i = 0; i < nblocks && pos < end_pos; i++, pos += len) { > > right? Yup, that's how I read it, too. But my recent experience with folios involved being repeatedly burnt by edge case corruptions due to multipage folios showing up when and where I least expected them. Hence doing a 1:1 conversion of page based code to folio based code and just assuming large folios will work without any testing seems akin to playing russian roulette with loose cannons that have been doused with napalm and then set on fire by an air-dropped barrel bomb... Cheers, Dave. -- Dave Chinner david at fromorbit.com 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 lists.sourceforge.net (lists.sourceforge.net [216.105.38.7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 2C288C4332F for ; Fri, 4 Nov 2022 03:37:34 +0000 (UTC) Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1oqnWO-00065M-Tw; Fri, 04 Nov 2022 03:37:32 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1oqnW9-000658-AG for linux-f2fs-devel@lists.sourceforge.net; Fri, 04 Nov 2022 03:37:17 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; 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:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=kCztCif6DGnUVPea69d1j6HBte1tInJ//NGbT2adCtA=; b=QhXx+Ok9nQeadwejoyJllrzFPV O+j0MSYgjQPpDaLchB3/uyUif36dH10PYWCIg6ypgySM8Jpr1hkfLbHL4DKtdu9PYXd93dAqvfiIP WNFkK9K6e1GPa6rPp06/waRIERK7qpvRP/C9TDx1nL0vHXIXhrZDAZNkZFIvj5HlipPk=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; 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:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=kCztCif6DGnUVPea69d1j6HBte1tInJ//NGbT2adCtA=; b=ip0/zPOyXCVsdpkwrl75kL1dts N4M4uju19U/0FaMaO08AhLTBKJ82qPk86Sro4b5vT1v98ydgxJFGatWgbeIvjSDegCZ8wmzJKZ3dn G21tum2skrTwoMpoL7+Rz6QwkChK7JDCm8mXk4g9JsGwnfdyoRu0wgIP0QiW3BOVYu6E=; Received: from mail-pg1-f169.google.com ([209.85.215.169]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1oqnW0-00BHCo-SZ for linux-f2fs-devel@lists.sourceforge.net; Fri, 04 Nov 2022 03:37:17 +0000 Received: by mail-pg1-f169.google.com with SMTP id f63so3373061pgc.2 for ; Thu, 03 Nov 2022 20:37:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20210112.gappssmtp.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=kCztCif6DGnUVPea69d1j6HBte1tInJ//NGbT2adCtA=; b=uj2thbHtG/d+GBBCufhO7rN+QkuEm7zsR1utnZpf1jQ6jQI6apkQTpUS2wcEmA9aNC VnDMe2rmlwjjtPUJR6RVdf+UJIK/YCtwiDAFrNIV7Nt++fXhl9nREiFkHf0RyMEPfvfO onGWmNN1ovJwkvkYC6fzzHZz7zyvVev9H3OguSEusghpv0YPPtdfeKpV8TCgCnMYqtfc Ksic71C8032fBMagPD7RLDMZgSbzkcCWA6HBFsZEVIK8PskhHmcNukEVkFYsBkmvmeoC g/qCQdT1fcwEOyEG1ZYfH8GugnyK0CKLKk6hzKwIFkwwhNd+ojc6bS3gdGaV6YzySSoe prLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=kCztCif6DGnUVPea69d1j6HBte1tInJ//NGbT2adCtA=; b=Hs8JDkO9ckJgzKb0x09qVk8t/xtGpHtLV3+xS4RgMg7QAAnD7sGlBEyNvaSp64wC9C tEZS5qfJqDZLdhsFC+rZJ5XiD5XZISRG3w2MQMLx2rfE04NmhZFpkRN6pQ0m0OhHptZD DA8OMuVKLS7zyCL9GiObbEJ1JH7ut5yk6rMuvNgGnjXwCT5RfM6AE/PbsGn+kLI22T7b FVaO+u5IheHfpOzzW8CBPAFzrxPk0l+F5HHtqDWXhFZOtSATqLa5qm6mb9uNuF6Pk0Ef xMOy4417bzXw7+aBq/ozCkNxVDwvauo8BdlixDMtjmKtlTCNHPYhpvuuiMN/h2JeBkj1 OvjA== X-Gm-Message-State: ACrzQf3fB2QF6bKd6tk7JIsRFlnNJ30PGwcJNyOZ5BzJH4xR+RxUYDWm sMhCL/qmAYdncqmeeHZzjArF0g== X-Google-Smtp-Source: AMsMyM7NwhgMGw/Ejg/10+QrYuOi1WfTIe60hww1ZNfzsfJwCZnIyT2jB8JwYZIzMahLdSnJ6wzgKQ== X-Received: by 2002:a05:6a00:2396:b0:56c:b770:eda6 with SMTP id f22-20020a056a00239600b0056cb770eda6mr33239633pfc.38.1667533023291; Thu, 03 Nov 2022 20:37:03 -0700 (PDT) Received: from dread.disaster.area (pa49-181-106-210.pa.nsw.optusnet.com.au. [49.181.106.210]) by smtp.gmail.com with ESMTPSA id x13-20020a170902a38d00b001830ed575c3sm1430190pla.117.2022.11.03.20.37.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 03 Nov 2022 20:37:02 -0700 (PDT) Received: from dave by dread.disaster.area with local (Exim 4.92.3) (envelope-from ) id 1oqnVr-00A1XB-Bm; Fri, 04 Nov 2022 14:36:59 +1100 Date: Fri, 4 Nov 2022 14:36:59 +1100 From: Dave Chinner To: "Darrick J. Wong" Message-ID: <20221104033659.GA2703033@dread.disaster.area> References: <20220901220138.182896-1-vishal.moola@gmail.com> <20220901220138.182896-5-vishal.moola@gmail.com> <20221018210152.GH2703033@dread.disaster.area> <20221104003235.GZ2703033@dread.disaster.area> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Headers-End: 1oqnW0-00BHCo-SZ Subject: Re: [f2fs-dev] [PATCH 04/23] page-writeback: Convert write_cache_pages() to use filemap_get_folios_tag() X-BeenThere: linux-f2fs-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-cifs@vger.kernel.org, cluster-devel@redhat.com, linux-kernel@vger.kernel.org, Matthew Wilcox , linux-f2fs-devel@lists.sourceforge.net, Vishal Moola , linux-mm@kvack.org, linux-nilfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, ceph-devel@vger.kernel.org, linux-ext4@vger.kernel.org, linux-afs@lists.infradead.org, linux-btrfs@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net On Thu, Nov 03, 2022 at 07:45:01PM -0700, Darrick J. Wong wrote: > On Fri, Nov 04, 2022 at 11:32:35AM +1100, Dave Chinner wrote: > > On Thu, Nov 03, 2022 at 03:28:05PM -0700, Vishal Moola wrote: > > > On Wed, Oct 19, 2022 at 08:01:52AM +1100, Dave Chinner wrote: > > > > On Thu, Sep 01, 2022 at 03:01:19PM -0700, Vishal Moola (Oracle) wrote: > > > > > - BUG_ON(PageWriteback(page)); > > > > > - if (!clear_page_dirty_for_io(page)) > > > > > + BUG_ON(folio_test_writeback(folio)); > > > > > + if (!folio_clear_dirty_for_io(folio)) > > > > > goto continue_unlock; > > > > > > > > > > trace_wbc_writepage(wbc, inode_to_bdi(mapping->host)); > > > > > - error = (*writepage)(page, wbc, data); > > > > > + error = writepage(&folio->page, wbc, data); > > > > > > > > Yet, IIUC, this treats all folios as if they are single page folios. > > > > i.e. it passes the head page of a multi-page folio to a callback > > > > that will treat it as a single PAGE_SIZE page, because that's all > > > > the writepage callbacks are currently expected to be passed... > > > > > > > > So won't this break writeback of dirty multipage folios? > > > > > > Yes, it appears it would. But it wouldn't because its already 'broken'. > > > > It is? Then why isn't XFS broken on existing kernels? Oh, we don't > > know because it hasn't been tested? > > > > Seriously - if this really is broken, and this patchset further > > propagating the brokeness, then somebody needs to explain to me why > > this is not corrupting data in XFS. > > It looks like iomap_do_writepage finds the folio size correctly > > end_pos = folio_pos(folio) + folio_size(folio); > > and iomap_writpage_map will map out the correct number of blocks > > unsigned nblocks = i_blocks_per_folio(inode, folio); > > for (i = 0; i < nblocks && pos < end_pos; i++, pos += len) { > > right? Yup, that's how I read it, too. But my recent experience with folios involved being repeatedly burnt by edge case corruptions due to multipage folios showing up when and where I least expected them. Hence doing a 1:1 conversion of page based code to folio based code and just assuming large folios will work without any testing seems akin to playing russian roulette with loose cannons that have been doused with napalm and then set on fire by an air-dropped barrel bomb... Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Chinner Subject: Re: [PATCH 04/23] page-writeback: Convert write_cache_pages() to use filemap_get_folios_tag() Date: Fri, 4 Nov 2022 14:36:59 +1100 Message-ID: <20221104033659.GA2703033@dread.disaster.area> References: <20220901220138.182896-1-vishal.moola@gmail.com> <20220901220138.182896-5-vishal.moola@gmail.com> <20221018210152.GH2703033@dread.disaster.area> <20221104003235.GZ2703033@dread.disaster.area> Mime-Version: 1.0 Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1667533034; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post; bh=kCztCif6DGnUVPea69d1j6HBte1tInJ//NGbT2adCtA=; b=gqO+1Jo24oUvlZF3Ebcgb6r/htzR0aTyladMXKyLjgMlUgIjEyjFrSabU4570uhmwPCFkP 7uULMs0ag054uLS5cs1OgRWt54uLji/thWgPpXJCEdUmrssyW+rFIG4KvMYqrIqjlJrjSP pgEG633xpH63XqPuoLZg9KHlxOHLJuE= In-Reply-To: List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: cluster-devel-bounces@redhat.com Sender: "Cluster-devel" Content-Disposition: inline Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: "Darrick J. Wong" Cc: linux-cifs@vger.kernel.org, cluster-devel@redhat.com, linux-kernel@vger.kernel.org, Matthew Wilcox , linux-f2fs-devel@lists.sourceforge.net, Vishal Moola , linux-mm@kvack.org, linux-nilfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, ceph-devel@vger.kernel.org, linux-ext4@vger.kernel.org, linux-afs@lists.infradead.org, linux-btrfs@vger.kernel.org On Thu, Nov 03, 2022 at 07:45:01PM -0700, Darrick J. Wong wrote: > On Fri, Nov 04, 2022 at 11:32:35AM +1100, Dave Chinner wrote: > > On Thu, Nov 03, 2022 at 03:28:05PM -0700, Vishal Moola wrote: > > > On Wed, Oct 19, 2022 at 08:01:52AM +1100, Dave Chinner wrote: > > > > On Thu, Sep 01, 2022 at 03:01:19PM -0700, Vishal Moola (Oracle) wrote: > > > > > - BUG_ON(PageWriteback(page)); > > > > > - if (!clear_page_dirty_for_io(page)) > > > > > + BUG_ON(folio_test_writeback(folio)); > > > > > + if (!folio_clear_dirty_for_io(folio)) > > > > > goto continue_unlock; > > > > > > > > > > trace_wbc_writepage(wbc, inode_to_bdi(mapping->host)); > > > > > - error = (*writepage)(page, wbc, data); > > > > > + error = writepage(&folio->page, wbc, data); > > > > > > > > Yet, IIUC, this treats all folios as if they are single page folios. > > > > i.e. it passes the head page of a multi-page folio to a callback > > > > that will treat it as a single PAGE_SIZE page, because that's all > > > > the writepage callbacks are currently expected to be passed... > > > > > > > > So won't this break writeback of dirty multipage folios? > > > > > > Yes, it appears it would. But it wouldn't because its already 'broken'. > > > > It is? Then why isn't XFS broken on existing kernels? Oh, we don't > > know because it hasn't been tested? > > > > Seriously - if this really is broken, and this patchset further > > propagating the brokeness, then somebody needs to explain to me why > > this is not corrupting data in XFS. > > It looks like iomap_do_writepage finds the folio size correctly > > end_pos = folio_pos(folio) + folio_size(folio); > > and iomap_writpage_map will map out the correct number of blocks > > unsigned nblocks = i_blocks_per_folio(inode, folio); > > for (i = 0; i < nblocks && pos < end_pos; i++, pos += len) { > > right? Yup, that's how I read it, too. But my recent experience with folios involved being repeatedly burnt by edge case corruptions due to multipage folios showing up when and where I least expected them. Hence doing a 1:1 conversion of page based code to folio based code and just assuming large folios will work without any testing seems akin to playing russian roulette with loose cannons that have been doused with napalm and then set on fire by an air-dropped barrel bomb... Cheers, Dave. -- Dave Chinner david@fromorbit.com