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 B5D08C5479D for ; Mon, 9 Jan 2023 15:30:05 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0CA088E0002; Mon, 9 Jan 2023 10:30:05 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 078EA8E0001; Mon, 9 Jan 2023 10:30:05 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E82B98E0002; Mon, 9 Jan 2023 10:30:04 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id D4EAF8E0001 for ; Mon, 9 Jan 2023 10:30:04 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 944888013A for ; Mon, 9 Jan 2023 15:30:04 +0000 (UTC) X-FDA: 80335646328.08.2510E35 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf13.hostedemail.com (Postfix) with ESMTP id 0798420017 for ; Mon, 9 Jan 2023 15:30:01 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=s5EGoru1; dmarc=none; spf=none (imf13.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1673278202; 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=bc3idNtsAWoy+2ncthmgzQVDyaeHH0rtTXHo3T/zuhY=; b=bPjT1s9vaObrQs7Pu+40X6C+PiC5LLFEUYOB8XwDTPlC1kUipPoV1RqelclPWN9KUYQEHs 3myZSHAi/UwXLzNL3sCUeWxYkwtFjXPVJbHgU+vsqlUB8AY9k0d8hAkXJadqf2frCAyWUB Kf8JlCM5PaDwnnqRg5FuL6iDztqXXWs= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=s5EGoru1; dmarc=none; spf=none (imf13.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1673278202; a=rsa-sha256; cv=none; b=YbTYy2VDhYfO7OQZiZQTtMFMKGUod9eSNQzml5hnDJYIPYUAorrrCKt7z8d59goZtSkINJ yjYadNRar5Zcrl6Nywc2PXrI2l5Rn0tB2vE4SYUZa35IzjYQuF3gvNM6OtIdyhNfwmYGqe 0lYcyu8LCaZRgbIU/92UjHIkUBiViWA= 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=bc3idNtsAWoy+2ncthmgzQVDyaeHH0rtTXHo3T/zuhY=; b=s5EGoru1cjrQGCeJd8V5jrtw8M 4F35tTEElZxf/CtwBjrbyUMGZaLo1nfwDDFZhuUBJcOzsS3fn/fcNpCl1YLHI7d0KkjxpgxkmrHt5 DbsKYCc6pQkXM/XeUS0SLTyWuI8dz9qEyeA41HHlmPDsIdlJL9wJ0By55ryA6muYyBoUFdFqNa0zR dtLKHexYBfgPycRA7VqD2C8ovNa6hN2Mu0A2iWpf/pdRqDEU0VvGCj7TBmIu28ZY2mDggDREJ0uSI Y1YL48eJzcmwLXjeSMjaXkVOTvwqNrnHLdsqhGTIFnMUXjg5ufmnsCsJTok1VDdgcQav145biosm3 2mCAIKig==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1pEu6E-002Odi-Ac; Mon, 09 Jan 2023 15:30:10 +0000 Date: Mon, 9 Jan 2023 15:30:10 +0000 From: Matthew Wilcox To: Jeff Layton Cc: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Christoph Hellwig Subject: Re: [PATCH 08/11] cifs: Remove call to filemap_check_wb_err() Message-ID: References: <20230109051823.480289-1-willy@infradead.org> <20230109051823.480289-9-willy@infradead.org> <7d1499fadf42052711e39f0d8c7656f4d3a4bc9d.camel@redhat.com> <74c40f813d4dc2bf90fbf80a80a5f0ba15365a90.camel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <74c40f813d4dc2bf90fbf80a80a5f0ba15365a90.camel@redhat.com> X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 0798420017 X-Stat-Signature: c9z7ckh8zbgjr14rah5gkrzihj3ydjaf X-HE-Tag: 1673278201-530659 X-HE-Meta: U2FsdGVkX18Kg6WzFuHpzJ/OdIhZaJbGcJwArJRxOoDPpnVOpOybKIoRnqH1xg8Ubci5AFyhgoHkwt9M/iWoFSJACY3xja7V/W+UzuNJbdTCCcR9JW2yA0s+La5t5ul6+D/8peZedJt/tWpv705OWOVz87YQJLFeM7pGf6Y1X0J4JUqjVpQfetvamWlxSyDsK0fJ929stfWLZKsUvB1fho/lu6blywMyG/nOMLucyBcRo38zWINP8TdniQu7UVLTcG6/QM1gKYfAnGiZRXWVF/5MH0aCyVhAfkHEemBFrbs87COYjbw+GcQfVhRAvWw4do0CX0xp71o5Vqn7/rBr2UjW1vq8iDxZaazdxKAP0qN/pPOly0LjNVopThXpo3dJxJ86jtnXip6bVobiQB0ZXoN4fLbNs2GPfT7WhGSYQWk1tdtwRCZgTYf6LmoQULm9+vv+Yvm4v+fNvXu1UVA5tREwghdWBL+4YeGlMXoVd4UvbXXR/45lxyaqNZroSG9cUQrzNcCA3g/4TA/FYEkqToU09MdUi2ii8YLcfxO4o7A5RJxCyFTyNX5Sbo2oJtr0FbpBH/MMa1uY4h/HyGYfOLxEKcUc25+a6TDSosqLXQhtqMg12ZWfpsEH+knQibVKM0oysgpUirkNucyCQV4+KhVn09Z+MYzIe8gWmtpt/BHLYo1UmdMUXjwC+9XIHYPLjDFpB7v2tWLskFpERO/SIidMkgWz99/ZTcMLIaOna3scGZfTGz3UyjVE93ZfldB0Iaq++c/LBWkMQDX1P1FNhHnkYZwv4YnPe7d6y5W7DYyhfIzzVvJqd0URhwEovweWqREKsedpFT9UUNd/QOpLlhI/mMS/I18dPpTIZKJehIFB1nSDkgl38RSXYasvYCR4Nv/2uMPA7mJbdYWxVeowWA== 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 Mon, Jan 09, 2023 at 10:14:12AM -0500, Jeff Layton wrote: > On Mon, 2023-01-09 at 09:42 -0500, Jeff Layton wrote: > > On Mon, 2023-01-09 at 05:18 +0000, Matthew Wilcox (Oracle) wrote: > > > filemap_write_and_wait() now calls filemap_check_wb_err(), so we cannot > > > glean any additional information by calling it ourselves. It may also > > > be misleading as it will pick up on any errors since the beginning of > > > time which may well be since before this program opened the file. > > > > > > Signed-off-by: Matthew Wilcox (Oracle) > > > --- > > > fs/cifs/file.c | 8 +++----- > > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > > > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > > > index 22dfc1f8b4f1..7e7ee26cf77d 100644 > > > --- a/fs/cifs/file.c > > > +++ b/fs/cifs/file.c > > > @@ -3042,14 +3042,12 @@ int cifs_flush(struct file *file, fl_owner_t id) > > > int rc = 0; > > > > > > if (file->f_mode & FMODE_WRITE) > > > - rc = filemap_write_and_wait(inode->i_mapping); > > > + rc = filemap_write_and_wait(file->f_mapping); > > > > If we're calling ->flush, then the file is being closed. Should this > > just be? > > rc = file_write_and_wait(file); > > > > It's not like we need to worry about corrupting ->f_wb_err at that > > point. > > > > OTOH, I suppose it is possible for there to be racing fsync syscall with > a filp_close, and in that case advancing the f_wb_err might be a bad > idea, particularly since a lot of places ignore the return from > filp_close. It's probably best to _not_ advance the f_wb_err on ->flush > calls. There's only so much we can do to protect an application from itself. If it's racing an fsync() against close(), it might get an EBADF from fsync(), or end up fsyncing entirely the wrong file due to a close(); open(); associating the fd with a different file. > That said...wonder if we ought to consider making filp_close and ->flush > void return functions. There's no POSIX requirement to flush all of the > data on close(), so an application really shouldn't rely on seeing > writeback errors returned there since it's not reliable. > > If you care about writeback errors, you have to call fsync -- full stop. Yes, most filesystems do not writeback dirty data on close(). Applications can't depend on that behaviour. Interestingly, if you read https://pubs.opengroup.org/onlinepubs/9699919799/functions/close.html really carefully, it says: If an I/O error occurred while reading from or writing to the file system during close(), it may return -1 with errno set to [EIO]; if this error is returned, the state of fildes is unspecified. So if we return an error, userspace doesn't know if this fd is still open or not! This feels like poor underspecification on POSIX's part (and probably stems from a historical unwillingness to declare any vendor's implementation as "not Unix").