From nobody Thu Jan 06 22:25:10 2022 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 81DC31935731; Thu, 6 Jan 2022 22:25:11 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4JVLWq1tBGz3qvp; Thu, 6 Jan 2022 22:25:11 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 1C0E227FAE; Thu, 6 Jan 2022 22:25:11 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 206MPA31020745; Thu, 6 Jan 2022 22:25:10 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 206MPA10020744; Thu, 6 Jan 2022 22:25:10 GMT (envelope-from git) Date: Thu, 6 Jan 2022 22:25:10 GMT Message-Id: <202201062225.206MPA10020744@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Rick Macklem Subject: git: e4df1036f66d - main - nfscl: Always invalidate buffers for append writes List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rmacklem X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: e4df1036f66dc360606fea053ec04ccabb69df14 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1641507911; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=s5yXJQVio506+TBQjGhNaoLEr2RlCCyfERS0sxSdO5w=; b=bDBT38aGRHcAuIAiqQQBzr7Ys1ETZBhEN6oZkJQCWo/GEmum4hEOqkbBj2rQmmUQeiMmYN O5IkcBCZ0DZKwwVxIepYmCw2EHeUHu6jnwasjUIEREeUsMG1VYe7a6SAXX5TKPZWydIoAj r63/ugNUD0LMajHbQVJnZ6c6asGCq5fjy0QQyY4qR5ne0GqGMxkKwMMnHUGXko9D/N1jgu iEQm1YjJbJC/8b0Y+o2+Lm+egGaQGU60Irqogmaw87sPxqtovdMgQI68OmixBtxzkHUrw8 BlRorxIxs9fvIOWHrt/1/Z8U4TTUjKetrJyoJ6d/mEEM5EFJ9qrlOGM+nwGVnw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1641507911; a=rsa-sha256; cv=none; b=QyGpmo1eajzG4fX26bd5jYJJXyTri2MrjDFLJpL9bGCLFJrRWVqMnzWxBHWOiJf4gne599 uuMPDOnXpK4YNCbmr9H5WMnRZ4V+PeDZJ3DH2yogUyDh5uBw0/uLF6s61SxLzTUeLxc9zJ y5oq/bMRckCp7w63NgzABTdTrzmL8bHxzPYSZ+CbimvPduEN4vOF42qUXJ1AaZSZ6kueXt IMuGbzo3woquU3h0Axg5KZylcnJnG8cwEnmaX3ET584bTLMp58J39ufBccm0f4SoQTexho W+hRu6LjH77lkpxYVmnEP9ye8BTUUJiUHlhCavlXnzrMRTSrgtDAeHgomWxUcg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=e4df1036f66dc360606fea053ec04ccabb69df14 commit e4df1036f66dc360606fea053ec04ccabb69df14 Author: Rick Macklem AuthorDate: 2022-01-06 22:18:36 +0000 Commit: Rick Macklem CommitDate: 2022-01-06 22:18:36 +0000 nfscl: Always invalidate buffers for append writes kib@ reported a problem which was resolved by reverting commit 867c27c23a5c, which changed the NFS client to use direct RPCs to the server for IO_APPEND writes. He also spotted that the code only invalidated buffer cache buffers when they were marked NMODIFIED (had been written into). This patch modifies the NFS VOP_WRITE() to always invalidate the buffer cache buffers and pages for the file when IO_APPEND is specified. It also includes some cleanup suggested by kib@. Reported by: kib Tested by: kib Reviewed by: kib MFC after: 10 weeks --- sys/fs/nfsclient/nfs_clbio.c | 75 +++++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/sys/fs/nfsclient/nfs_clbio.c b/sys/fs/nfsclient/nfs_clbio.c index 06b51c050d34..00a3e5d12dd7 100644 --- a/sys/fs/nfsclient/nfs_clbio.c +++ b/sys/fs/nfsclient/nfs_clbio.c @@ -950,27 +950,33 @@ ncl_write(struct vop_write_args *ap) * Synchronously flush pending buffers if we are in synchronous * mode or if we are appending. */ - if (ioflag & (IO_APPEND | IO_SYNC)) { - NFSLOCKNODE(np); - if (np->n_flag & NMODIFIED) { - NFSUNLOCKNODE(np); -#ifdef notyet /* Needs matching nonblock semantics elsewhere, too. */ - /* - * Require non-blocking, synchronous writes to - * dirty files to inform the program it needs - * to fsync(2) explicitly. - */ - if (ioflag & IO_NDELAY) - return (EAGAIN); -#endif - np->n_attrstamp = 0; - KDTRACE_NFS_ATTRCACHE_FLUSH_DONE(vp); - error = ncl_vinvalbuf(vp, V_SAVE | ((ioflag & - IO_VMIO) != 0 ? V_VMIO : 0), td, 1); - if (error != 0) - return (error); - } else - NFSUNLOCKNODE(np); + if ((ioflag & IO_APPEND) || ((ioflag & IO_SYNC) && (np->n_flag & + NMODIFIED))) { + /* + * For the case where IO_APPEND is being done using a + * direct output (to the NFS server) RPC and + * newnfs_directio_enable is 0, all buffer cache buffers, + * including ones not modified, must be invalidated. + * This ensures that stale data is not read out of the + * buffer cache. The call also invalidates all mapped + * pages and, since the exclusive lock is held on the vnode, + * new pages cannot be faulted in. + * + * For the case where newnfs_directio_enable is set + * (which is not the default), it is not obvious that + * stale data should be left in the buffer cache, but + * the code has been this way for over a decade without + * complaints. Note that, unlike doing IO_APPEND via + * a direct write RPC when newnfs_directio_enable is not set, + * when newnfs_directio_enable is set, reading is done via + * direct to NFS server RPCs as well. + */ + np->n_attrstamp = 0; + KDTRACE_NFS_ATTRCACHE_FLUSH_DONE(vp); + error = ncl_vinvalbuf(vp, V_SAVE | ((ioflag & + IO_VMIO) != 0 ? V_VMIO : 0), td, 1); + if (error != 0) + return (error); } orig_resid = uio->uio_resid; @@ -1002,23 +1008,20 @@ ncl_write(struct vop_write_args *ap) return (0); /* - * If the file in not mmap()'d, do IO_APPEND writing via a - * synchronous direct write. This can result in a significant - * performance improvement. - * If the file is mmap()'d, this cannot be done, since there - * is no way to maintain consistency between the file on the - * NFS server and the file's mmap()'d pages. + * Do IO_APPEND writing via a synchronous direct write. + * This can result in a significant performance improvement. */ - NFSLOCKNODE(np); - if (vp->v_type == VREG && ((newnfs_directio_enable && (ioflag & - IO_DIRECT)) || ((ioflag & IO_APPEND) && - (vp->v_object == NULL || !vm_object_is_active(vp->v_object))))) { - NFSUNLOCKNODE(np); - if ((ioflag & IO_APPEND) != 0) - ioflag |= IO_SYNC; - return nfs_directio_write(vp, uio, cred, ioflag); + if ((newnfs_directio_enable && (ioflag & IO_DIRECT)) || + (ioflag & IO_APPEND)) { + /* + * Direct writes to the server must be done NFSWRITE_FILESYNC, + * because the write data is not cached and, therefore, the + * write cannot be redone after a server reboot. + * Set IO_SYNC to make this happen. + */ + ioflag |= IO_SYNC; + return (nfs_directio_write(vp, uio, cred, ioflag)); } - NFSUNLOCKNODE(np); /* * Maybe this should be above the vnode op call, but so long as