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 88003C433F5 for ; Mon, 30 May 2022 15:04:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240290AbiE3PD6 (ORCPT ); Mon, 30 May 2022 11:03:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38932 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S243235AbiE3PBs (ORCPT ); Mon, 30 May 2022 11:01:48 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 71BAC74DEB; Mon, 30 May 2022 07:02:09 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 0E02261138; Mon, 30 May 2022 14:02:09 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 872F7C385B8; Mon, 30 May 2022 14:02:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1653919328; bh=C2X5kSVfJKWy7iJa8rZT/0Escr5C771N57orGsEtS1E=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=m2VzrdOoPJY4ZJttjh9yFNKKEJwm9RcAA9PY+8FsNzXxT6aAEq1JV9+owT9gl/EQZ 7hl+yP33+Pjd9g3H+G0JFCraxieuBqqgJNgzUqdOvdTI+7yzcq1bmfYgOAXPixaseQ Y6P7yZALek06IX16wvn3SYbrThPO2siGOkGD8q5tOK2hlUaenKjEks7cv7U/f2iBDV SqDFpxhRToU4Kxh8qRPlgKDUSNKoP1jPaWRYcs2eIWwey90Ay9P7Vhx9KAgrMT/xM6 jtEVfKJwQL/lafRqwqvyUUGIufPMicAKUEZUGsWPlVWFUUXBjGVQuHctK1XrVjv7ch oHSFGfaKkprZQ== Message-ID: <9915b7b556106d2a525941141755adcca9e50163.camel@kernel.org> Subject: Re: [PATCH -next,v2] fuse: return the more nuanced writeback error on close() From: Jeff Layton To: Miklos Szeredi , Al Viro Cc: ChenXiaoSong , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, liuyongqiang13@huawei.com, "zhangyi (F)" , zhangxiaoxu5@huawei.com, Steve French , NeilBrown Date: Mon, 30 May 2022 10:02:06 -0400 In-Reply-To: References: <20220523014838.1647498-1-chenxiaosong2@huawei.com> Content-Type: text/plain; charset="ISO-8859-15" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.44.1 (3.44.1-1.fc36) MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2022-05-30 at 14:13 +0200, Miklos Szeredi wrote: > On Mon, 23 May 2022 at 03:35, ChenXiaoSong wro= te: > >=20 > > As filemap_check_errors() only report -EIO or -ENOSPC, we return more n= uanced > > writeback error -(file->f_mapping->wb_err & MAX_ERRNO). > >=20 > > filemap_write_and_wait > > filemap_write_and_wait_range > > filemap_check_errors > > -ENOSPC or -EIO > > filemap_check_wb_err > > errseq_check > > return -(file->f_mapping->wb_err & MAX_ERRNO) > >=20 > > Signed-off-by: ChenXiaoSong > > --- > > fs/fuse/file.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > >=20 > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > > index f18d14d5fea1..9917bc2795e6 100644 > > --- a/fs/fuse/file.c > > +++ b/fs/fuse/file.c > > @@ -488,10 +488,10 @@ static int fuse_flush(struct file *file, fl_owner= _t id) > > inode_unlock(inode); > >=20 > > err =3D filemap_check_errors(file->f_mapping); > > + /* return more nuanced writeback errors */ > > if (err) > > - return err; > > + return filemap_check_wb_err(file->f_mapping, 0); >=20 > I'm wondering if this should be file_check_and_advance_wb_err() instead. >=20 I think that it probably shouldn't be, actually. Reason below... > Is there a difference between ->flush() and ->fsync()? >=20 > Jeff, can you please help? >=20 >=20 The main difference is that ->flush is called from filp_close, so it's called when a file descriptor (or equivalent) is being torn down out, whereas ->fsync is (obviously) called from the fsync codepath. We _must_ report writeback errors on fsync, but reporting them on the close() syscall is less clear. The thing about close() is that it's going be successful no matter what is returned. The file descriptor will no longer work afterward regardless. fsync also must also initiate writeback of all the buffered data, but it's not required for filesystems to do that on close() (and in fact, there are good reasons not to if you can). A successful close() tells you nothing about whether your data made it to the backing store. It might just not have been synced out yet. Personally, I think it's probably best to _not_ return writeback errors on close at all. The only "legitimate" error on close is -EBADF. Arguably, we should make ->flush be void return. Note that most filp_close callers ignore the error anyway, so it's not much of a stretch. In any case, if you do decide to return errors in fuse_flush, then advancing the cursor would also have the effect of masking writeback errors on dup()'ed file descriptors, and I don't think you want to do that. --=20 Jeff Layton