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 DEFFAC4829A for ; Tue, 13 Feb 2024 13:14:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 79D466B0080; Tue, 13 Feb 2024 08:14:01 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 74DD46B0095; Tue, 13 Feb 2024 08:14:01 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6163A6B0096; Tue, 13 Feb 2024 08:14:01 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 50DA16B0080 for ; Tue, 13 Feb 2024 08:14:01 -0500 (EST) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id EDD27120B90 for ; Tue, 13 Feb 2024 13:14:00 +0000 (UTC) X-FDA: 81786823440.10.E9A96E0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf27.hostedemail.com (Postfix) with ESMTP id 2F1BC4002A for ; Tue, 13 Feb 2024 13:13:58 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=cJcdEXDg; spf=pass (imf27.hostedemail.com: domain of bfoster@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707830038; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=lrJK7/vivRjYFBV9aMR7eUFZf9Hk2Ry4XzHvi+/bIaM=; b=wNL58/1WH9PZmfZE9ryX/r2jCkXRzgK7Mqv8Z6jZrwXcCJs8mSueqMcSq1Zo/9AQzLJZvY rLPDClwMY5lT1De3doi53o8rGSe2vxj524Xcm1hMdTrRiEtMVIGoRRIpa3WCMNOLTOxKbu jZ5lWZpZG5yPYR1s1MPTtVUZo3nLIpw= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707830038; a=rsa-sha256; cv=none; b=3k4lcxhyqhFGCenTaO6HFx72JnGs/NWAXTlTzH+i4o9Fb28Iuea1Q/HCw7Dtr1EpTWNq5R nNghY7tYEYZ9JzlaiZ7Ivy/jsxqVDjqi0yL0l7hwOPMX54+lfY20B/U5eaDYm1rt78k7Gm H2O2m3xuKftFdlrbIXoF7dP6sUCBJeo= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=cJcdEXDg; spf=pass (imf27.hostedemail.com: domain of bfoster@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com; dmarc=pass (policy=none) header.from=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1707830037; h=from:from: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; bh=lrJK7/vivRjYFBV9aMR7eUFZf9Hk2Ry4XzHvi+/bIaM=; b=cJcdEXDgTOGi52GYgTCl2BbrKp4BrXXUeceDTKJgAufUGf738PwF/SbzjjUXHXZEe3iw31 YJcx9GVDapmThSitZy4o7f2ye3eY+Ts2EW/3/Z15zFFFkoiWCk5Z2TTQ0IxrHQpxrjH5uV SH/lg1FpiK3R/WgK6ty5KbqYipkx7jY= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-586-Rve8DbtKO0WXuzTho1yCQQ-1; Tue, 13 Feb 2024 08:13:53 -0500 X-MC-Unique: Rve8DbtKO0WXuzTho1yCQQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 88428101A52A; Tue, 13 Feb 2024 13:13:52 +0000 (UTC) Received: from bfoster (unknown [10.22.16.56]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2308F2022AAC; Tue, 13 Feb 2024 13:13:52 +0000 (UTC) Date: Tue, 13 Feb 2024 08:15:29 -0500 From: Brian Foster To: Jan Kara Cc: Christoph Hellwig , linux-mm@kvack.org, Matthew Wilcox , Jan Kara , David Howells , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 01/14] writeback: don't call mapping_set_error in writepage_cb Message-ID: References: <20240212071348.1369918-1-hch@lst.de> <20240212071348.1369918-2-hch@lst.de> <20240213130713.ysuxaqcwizqwjke2@quack3> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240213130713.ysuxaqcwizqwjke2@quack3> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.4 X-Stat-Signature: k491ctpj8r6da4ccpcmw8yku7ojy46jz X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 2F1BC4002A X-Rspam-User: X-HE-Tag: 1707830038-34502 X-HE-Meta: U2FsdGVkX187YxoGsZqqIO5ri9/pxvuRvhsM58yoM8tws5CVIqAb9WIlb96bvfPsF956ijhcxqqw9SPZE41N24hdzO+eJQDwiL6jbUoKXMwSklEeRDuFfq35gMu5WGt9OypgQ3PnKSqvBCkevWCoF65xsAeIL7IePL/HcboaNV8O1QwwoJoiRCM7/zuuQ9szvWFE3IognETwn8wDWgSoCDZrGmO+tvYciwAyEe4b3Y56cTCyJ43/2o7/S9GzMXNlnnh7RvTkIAKkHtGNg2feXc5lWAcPIibc5TJyv6QDO1tclL8qwXFSxSBiDN8dEeDa+546AYzuKm7/V1IyreNZx+CvaWEMMfFdkm3kW4yk3k88ZxtGNk0pLKXEhMmQUAGGr5X2opi5yma3L4u0IDr++IOo4qSppKlEliNT7i+n6p9tC/B23Nya5ou48PrfxatGFSnDI2SE2iyk3mgExXhfpi3FYmFc48w/70l/0tUI0aGusjCjxqpiH7zDAe0hOCKF4U7s//5WJf4Wafs4Md63EX+ESta0XSfHk8x1zMZ53MykiaCXiNpuNOvrt06cZFyt6FXGAxpNvhosmKtipg6W5yj9rYUYMlIkEuMnA5WKdQycqp09muIcBLEM6qCeHB8MNny6JPslIlPTdPQ3ZccNZNk4TOieQULafTi21SEOuQU0Nn6XJfAdt88+qrPKSVM+hLOwas7AK9+s2W2eN1NaHcJcKhv9MxgrcL2AZkBqXQpOfz5NxOMjDO3ACe8e5fS5yO2PWpW2Q8+Ii7TMxxzHbo613NIG83GAOv4E9nDVx72PDtRirIGFofKr1zc5uniWh+sotBSvSwl8T2w4gjghV5FAtDa3UzmaQqR6fRo52FbF0EYStfrPOKnirfHBSaNbuGD6hMyflrEqIT+yKywTs3pDpisffsr1nA2dn8tZPbwZ3ckFX2rM8nWh76vlJTf9cz4F06qFLHJi7PMUibD Yd7bqAGh F5r4TRSvKfbEyfxVvMa+FB6j04OijmRu8zy6L1FKtPEuvMmpDqY04G4LEzGNbZxTcKnbKC44jpGSHq3beOTYNOG81LQJSHYEd52zobYAX/okc2mRbFy0r4o2woMVLvEUHOsvOZdtk0EYINcloL6FbjWy4c/nVUo5noq+kVFgbsUElb1lReX3Cw/kfMnIjJe1KTDKOXGn//QoLgK7Zi3p5QWfCoVQNc8hn3xLGFRqc2nqRvMVNkYC8r3PdUCIxmRGownHw93CNMy3T99k= 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: List-Subscribe: List-Unsubscribe: On Tue, Feb 13, 2024 at 02:07:13PM +0100, Jan Kara wrote: > On Mon 12-02-24 08:13:35, Christoph Hellwig wrote: > > writepage_cb is the iterator callback for write_cache_pages, which > > already tracks all errors and returns them to the caller. There is > > no need to additionally cal mapping_set_error which is intended > ^^^ call > > > for contexts where the error can't be directly returned (e.g. the > > I/O completion handlers). > > > > Remove the mapping_set_error call in writepage_cb which is not only > > superfluous but also buggy as it can be called with the error argument > > set to AOP_WRITEPAGE_ACTIVATE, which is not actually an error but a > > magic return value asking the caller to unlock the page. > > > > Signed-off-by: Christoph Hellwig > > Our error handling in writeback has always been ... spotty. E.g. > block_write_full_page() and iomap_writepage_map() call mapping_set_error() > as well so this seems to be a common way to do things, OTOH ext4 calls > mapping_set_error() only on IO completion. I guess the question is how > an error in ->writepages from background writeback should propagate to > eventual fsync(2) caller? Because currently such error propagates all the > way up to writeback_sb_inodes() where it is silently dropped... > A couple related notes from skimming around: - Things like iomap might make this call in I/O completion paths, but then invoke bio completion paths on submission side errors (i.e. iomap_submit_ioend() -> bio_endio()). - __writeback_single_inode() calls filemap_fdatawait() shortly after do_writepages(), which basically looks like it relies on mapping error state to propagate error within the writeback path. The call removed by this path only seems to apply to contexts that don't define their own .writepages, so it's not clear to me how much this really matters. It just seems like it's a little hard to quantify whether this is an undesireable change in behavior or not. Brian > Honza > > > --- > > mm/page-writeback.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > > index 3f255534986a2f..62901fa905f01e 100644 > > --- a/mm/page-writeback.c > > +++ b/mm/page-writeback.c > > @@ -2534,9 +2534,8 @@ static int writepage_cb(struct folio *folio, struct writeback_control *wbc, > > void *data) > > { > > struct address_space *mapping = data; > > - int ret = mapping->a_ops->writepage(&folio->page, wbc); > > - mapping_set_error(mapping, ret); > > - return ret; > > + > > + return mapping->a_ops->writepage(&folio->page, wbc); > > } > > > > int do_writepages(struct address_space *mapping, struct writeback_control *wbc) > > -- > > 2.39.2 > > > -- > Jan Kara > SUSE Labs, CR >