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 36FEFC5479D for ; Mon, 9 Jan 2023 15:43:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 85F408E0003; Mon, 9 Jan 2023 10:43:28 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 80F108E0001; Mon, 9 Jan 2023 10:43:28 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6D6D98E0003; Mon, 9 Jan 2023 10:43:28 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 5CC718E0001 for ; Mon, 9 Jan 2023 10:43:28 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 21240160427 for ; Mon, 9 Jan 2023 15:43:28 +0000 (UTC) X-FDA: 80335680096.04.1A00744 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf19.hostedemail.com (Postfix) with ESMTP id 338041A0017 for ; Mon, 9 Jan 2023 15:43:26 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=M7Ro0Mp5; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf19.hostedemail.com: domain of jlayton@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=jlayton@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1673279006; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=whInaJDAMaytofnzPQt8C7WADWMQcuXC7OHunvK9Rys=; b=29sLB5PJDlwUQqrjAba/P1MeB08BsVEofBNdlzNAxNrqTPyNeVsuvi+Y5bi9RAOaBRUjNO 7gCsKLzPwmS23kti7tMfMtieJpQgsfebd9qta9ZtGQk0pTfLEoSrg1AtQhc0xTiLn1ibsC F1WoN3yE3jKvVKstigP+6bn/1Ta2r8A= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=M7Ro0Mp5; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf19.hostedemail.com: domain of jlayton@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=jlayton@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1673279006; a=rsa-sha256; cv=none; b=h7ETk+zFTN20cYuNLUQeYoutV3mI7lkU+ayTIG8HrmVRVKXlyRhEju+0IiQ2DbvVkhPv2/ rJBm7jj8IpXt6op/BgqhRHK8GGx05P6q4+c6Nq7qrAz3z6qvwiXu5ADjcYR00uIKGlFfzK TnrhZK3gdf+GzkiGnVhl11/nqCd6AXo= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1673279005; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=whInaJDAMaytofnzPQt8C7WADWMQcuXC7OHunvK9Rys=; b=M7Ro0Mp5RjZ/6CeX/0u4QVMiQlHJIp1k/2g63jImhGkmHEF0H+sFMVQ+9BKvZi+W3s2owA fgiXxecgvrIHA6bkgYoIo+LIk8u6jTnHt7nmzhCOtTgeNkREAE9KGJJC9S8VnDvqT0cyFW iwRbvDqNw5k7O0aF+BDLxQZWGCwDaA0= Received: from mail-vs1-f72.google.com (mail-vs1-f72.google.com [209.85.217.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-635-bR_DdRxsMm248B6f11UJlQ-1; Mon, 09 Jan 2023 10:43:24 -0500 X-MC-Unique: bR_DdRxsMm248B6f11UJlQ-1 Received: by mail-vs1-f72.google.com with SMTP id q8-20020a056102204800b003d0a28475bfso631653vsr.2 for ; Mon, 09 Jan 2023 07:43:24 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=XAX19TTJR2VgCMVi/IEI/+PRXbV5myNOhGD9Pu+JHuY=; b=V9QLkkMPkpcVV9g10Wd3fsm7CZMMXFhO90m79rO3a07KbEXQCYw4l0Td5+qO96RVsb pekA9QokAy+aaaWE/2YNxrcUNxT3XgMyCb6d3dtJfCYdTyjccfdwTsv6yywtmJG8xnb6 RKfJjjXNpjG0VFzZPzkhMgVhFGGKLiCmGrPphIJgMnnGty/PsibnDeYTQ+b1vGJstvI/ PXvBqsRcmQyhE0XGiSqPhiJnCepX+3n7pjqOTzVrN6aFGhlCWAeOq/AdFcS8ft7tSCsI EzJ3KI74TNoM5nW3XodManSgvWawWBEmsbkEmkIi9R+dCpZ8VxWVumJHB5f7QUxNkdBJ hPqg== X-Gm-Message-State: AFqh2kpESpuLsLTiSI0yQPyVWqR6DcIrCzxnmVk3NNKvZV7bK1DNnf5b yRhZMgKDLz7X/bqU66XOB9QpJnkiTqNpBjArRNQoJd+z6PhcbH7mKTLOXr9KDl+09BPvP+HuuXl JIOS3Zmac5ww= X-Received: by 2002:a67:fe11:0:b0:3ce:b8ab:3361 with SMTP id l17-20020a67fe11000000b003ceb8ab3361mr11051764vsr.31.1673279003662; Mon, 09 Jan 2023 07:43:23 -0800 (PST) X-Google-Smtp-Source: AMrXdXuIH6ysnvkOmi5DSNvrXcQWqB8jTFyv7H0UbloWZCjfUbifQMxhAHyuiD5gxHgCeoN7m69bxw== X-Received: by 2002:a67:fe11:0:b0:3ce:b8ab:3361 with SMTP id l17-20020a67fe11000000b003ceb8ab3361mr11051737vsr.31.1673279003376; Mon, 09 Jan 2023 07:43:23 -0800 (PST) Received: from [192.168.1.3] (68-20-15-154.lightspeed.rlghnc.sbcglobal.net. [68.20.15.154]) by smtp.gmail.com with ESMTPSA id s11-20020a05620a0bcb00b00704a9942708sm5520303qki.73.2023.01.09.07.43.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Jan 2023 07:43:22 -0800 (PST) Message-ID: <5296e372cf8da5eeef37400d31f33b6611731228.camel@redhat.com> Subject: Re: [PATCH 08/11] cifs: Remove call to filemap_check_wb_err() From: Jeff Layton To: Matthew Wilcox Cc: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Christoph Hellwig Date: Mon, 09 Jan 2023 10:43:22 -0500 In-Reply-To: References: <20230109051823.480289-1-willy@infradead.org> <20230109051823.480289-9-willy@infradead.org> <7d1499fadf42052711e39f0d8c7656f4d3a4bc9d.camel@redhat.com> <74c40f813d4dc2bf90fbf80a80a5f0ba15365a90.camel@redhat.com> User-Agent: Evolution 3.46.2 (3.46.2-1.fc37) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="ISO-8859-15" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 338041A0017 X-Stat-Signature: nzden7ydd1hrgua7inzqd1dckzx1r3qb X-HE-Tag: 1673279006-668105 X-HE-Meta: U2FsdGVkX18YC16CIcYVFk9/bevggYZ6ck4jmMNO/M9lFvVeNcW7mzo8pPWFGJcY7ZOJhQp//MzfwMFmAEv6r24aa7pLZ7szDgJvFEw3AQIXsoV2qoH3WCkajWl2AAU169qIk4/8NzW0WQoyDtL690QNdlfRQ099YHKTLzLM5vs3cF15x0kL0OHCHee5siYamRcJuz+NX9Mbj4JjorzY9QdwNueygN60L4ES6UMzHKm2cUVHrg+3sb8Ao9M2CWKHVVc7KSxC1K+aul0CtHJryh2SK6yhHztV9GgrcTgEpzOiuDqvSPZV9lannqKJL049zOOf/d5wRgovJhQ3sIwvMsPfG6+EghiRNl1wV0jji9GFKhaigUJYPS/3OkimM+VPTDlkwV6M5XBkaVExTjK/sQqnYxuTreUiVt25bhjgydyDAFwoMVV63m6loX+glnupAttsVpZ7pG4g4GeAixXMzs/NWz8kVOJH47e2ip1MyD9SATyKc++x4+1HRY88ZjXuXx7vQIjqXquKp4D7fG2/vbgMzVkqP6kR1gwefq9PzihlOOL2pwPmKNPbC4st5bFrtg1vwJ6SAOlMIfgitamgqM367OGPZQHsAk+P9vCumaRDZyHDLHTeOlZ7Mvafbx90KbWnLCiOmVR9hTW1g61J8rXSJ/ey76y1Tl0PdXb+4THnwnmJjsqzeokeoE5zsQmWsoSELIZn6thwXA/hIyeOO2JVyokK2dh2EJ8wEZQRNLrPnvcdLiWjfxvtHQdFMUcNfzlJU09JyQhL5MJCwYy14AiIPSTxeZn89TBb+5hGoH2vXu52rkzY0OVN7qQy31BLPIr7ubdMs1rCuyninNLJfBZC4yveQ2d6gpkF6geZnuyywxI8x0ez2g7y50M9EaZMpO1uLwIfSbj29s8/qkEQrJmVYxPGKaCUsAhKgCxfLLOCdskKBPgZVjGiGbWVV0xO3sfr7a5Av9y/whFVSJb 65M5u0V0 vuLsNW7Sux/cDj8hKnYhEcRdMeig9ymzHQIbS7ly7TZJ7Ybg3ie/a7I7rqK1Gl5YYaexPrripXFshp8G0uEoH1lwCfE2opLmpmJY0SdxmT4rVFC4ES4Kh40fTuIm6tuv/EsSDMx71mKaVdsF1aXUYfXeDaA== 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, 2023-01-09 at 15:30 +0000, Matthew Wilcox wrote: > 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 ca= nnot > > > > glean any additional information by calling it ourselves. It may a= lso > > > > 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. > > > >=20 > > > > Signed-off-by: Matthew Wilcox (Oracle) > > > > --- > > > > fs/cifs/file.c | 8 +++----- > > > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > >=20 > > > > 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) > > > > =09int rc =3D 0; > > > > =20 > > > > =09if (file->f_mode & FMODE_WRITE) > > > > -=09=09rc =3D filemap_write_and_wait(inode->i_mapping); > > > > +=09=09rc =3D filemap_write_and_wait(file->f_mapping); > > >=20 > > > If we're calling ->flush, then the file is being closed. Should this > > > just be? > > > =09=09rc =3D file_write_and_wait(file); > > >=20 > > > It's not like we need to worry about corrupting ->f_wb_err at that > > > point. > > >=20 > >=20 > > OTOH, I suppose it is possible for there to be racing fsync syscall wit= h > > 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. >=20 > 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. >=20 close() is not the worry, as it does return the error from ->flush. It's the other callers of filp_close that concern me. A lot of those are dealing with special "internal" struct files, but not all of them. There's no benefit to advancing f_wb_err in ->flush, so I think we ought to just avoid it. I think we don't even want to mark it SEEN in that case either, so filemap_check_wb_err ought to be fine. > > That said...wonder if we ought to consider making filp_close and ->flus= h > > 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. > >=20 > > If you care about writeback errors, you have to call fsync -- full stop= . >=20 > 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: >=20 > 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. >=20 > 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"). >=20 It's a mess. On Linux we even=A0tear down the fd on EINTR and EAGAIN, so retrying a close() on failure will always get you a EBADF. The only sane thing for userland to do is to just ignore errors on close (aside from maybe EBADF). --=20 Jeff Layton