From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 37EFA6DD1E for ; Tue, 30 Jan 2024 14:27:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706624830; cv=none; b=UqgGOyxrp2sMQdVAr8Qzd+qOHRpr1DvOcv9WgUNaCWun42MwOg1rJYxuok06wJ1Mft1Byy5c+aWEYU32H+H/iju+gd0eOvU4wOSrPFWjzgv9truAe09rleJbDGyHkvpVLpA0FOIrhNWF7iXblFiHUtfNEpgaYhJmb9DZTFDLdOo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706624830; c=relaxed/simple; bh=5p3LgCdEGKk3NWs/IAlbX6OPgZW0pLlNEZJxIYqkGoM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=MYhYj2qckK/Mt8sdfSHWiezEOUnGZq/KjOh8Qb5V3WvwPufR/ZDMWWOhcNhVLRyFVyxTbJZM6azeKPVJcMLSY0Dkwks2W2HoCvrhDBKQmoej2u+9ehYOTL0ydOeKDHSpBbRZSZ7Smws1sWap5c1yVltT+tcQu0xyOdAn5xpKf1s= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=UOfnV6yr; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="UOfnV6yr" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1706624828; 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=zaujk7yqT3tCcvpOY/dPJbxkbTmDPNwYZxgb8W+DENY=; b=UOfnV6yrXwvWYuaai4G4snjEZn+cvTnJ6QrLcrTEG4zJ/s6ceDKVBpDJK30fh6J/NRSsUw 3Bw70MkEz7eB3V+b2CgCFmOFxx0mNeVoOj6xsTyWkTLVc5zO33UBXw4i3pd8WbOs+nPu20 +p6tOe6IV/HQjxLYyrcs8q1d0JdF7MY= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-678-iuQoh3B7NLejN0iWnFWjiw-1; Tue, 30 Jan 2024 09:27:05 -0500 X-MC-Unique: iuQoh3B7NLejN0iWnFWjiw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (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 6E1723806711; Tue, 30 Jan 2024 14:27:04 +0000 (UTC) Received: from bfoster (unknown [10.22.32.186]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E9862134; Tue, 30 Jan 2024 14:27:03 +0000 (UTC) Date: Tue, 30 Jan 2024 09:28:22 -0500 From: Brian Foster To: Christoph Hellwig Cc: linux-mm@kvack.org, Matthew Wilcox , Jan Kara , David Howells , Christian Brauner , "Darrick J. Wong" , linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Jan Kara , Dave Chinner Subject: Re: [PATCH 06/19] writeback: Factor out writeback_finish() Message-ID: References: <20240125085758.2393327-1-hch@lst.de> <20240125085758.2393327-7-hch@lst.de> <20240130140459.GA31126@lst.de> Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240130140459.GA31126@lst.de> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.1 On Tue, Jan 30, 2024 at 03:04:59PM +0100, Christoph Hellwig wrote: > On Mon, Jan 29, 2024 at 03:13:47PM -0500, Brian Foster wrote: > > > @@ -2481,6 +2500,9 @@ int write_cache_pages(struct address_space *mapping, > > > folio_unlock(folio); > > > error = 0; > > > } > > > + > > > > JFYI: whitespace damage on the above line. > > Thanks, fixed. > > > > > > + if (error && !wbc->err) > > > + wbc->err = error; > > > > > > > Also what happened to the return of the above "first error encountered" > > for the WB_SYNC_ALL case? Is that not needed for some reason (and so the > > comment just below might require an update)? > > No, this got broken during the various rebases (and is fixed again later > in the series). We need to return wbc->err from write_cache_pages at > this stage, I'll fix it. > Ok, I noticed it was added back once I got to more of the iter abstraction bits and so figured it was a transient/unintentional thing. The above tweak makes sense to me. FWIW, I haven't stared at the final patch long enough to have a strong opinion. I tend to agree with Jan that the error handling logic in the current series is a little wonky in that it's one of those things I'd have to go read the implementation every time to remember what it does, but the broader changes all seem reasonable to me. So for patches 1-18 and with the above tweak: Reviewed-by: Brian Foster