From nobody Fri Sep 19 19:23:03 2025 X-Original-To: dev-commits-src-all@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 4cT2Tg39wFz67SQ7; Fri, 19 Sep 2025 19:23:03 +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 "R12" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4cT2Tg2TZ1z3LhS; Fri, 19 Sep 2025 19:23:03 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1758309783; 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=PD0RaN7cXKaxg9zBJhhuHvTPyGcAf4GeqrMpdGi4VkQ=; b=JYTfihmvSh21qFI3qHqVeY2tKeSiikwE8d4OKBXJU62x/Fn0HZXfdXJE4rB0rEQ7RXkWtP nKguWIM8aUACTeBKWJdqdFI6LzFeqMEph9S02DF+R9pG18t1A6lTyE5Efxi4E1sMv/VQH1 zMdDPoXWouG6mbmptrZI5qEuZS+jkryRheKhbhb9zkEqNGcOrTnWg6PhN0KEaREhfdkFVx v3bFBdqVK1rQA6j6U/vypiOl/y0IcOD61PO2dyEDI25VaLpaFK4pHxOBW1ql3KT8xHW7yW Iy9aBQ0aXgjyXzFde+eaLuik42Hr0PPQuo1yLr7TdfK6RtXAbTwR6mWtHwrH2g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1758309783; 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=PD0RaN7cXKaxg9zBJhhuHvTPyGcAf4GeqrMpdGi4VkQ=; b=ypBP44ntEqC4TDaEGUzJDeXhvWgFR+dFrVUJD6RB521g3dVUmKgOLk5eRrpnAXz4fC1e9+ vUin2qsTrUIBRuzY+yUzXNCd7ml6C5Nl4/tROKPzfybB90OqHudPbmVnCwVRdOiR8q1++O r9zYXv5BWxYWH6aDfxol7yadc7kwrrnHnym9F2y0cQdye3g7vi/TljVlBGZXFL+1Fud0TS T3NQaOiNezHI42PWgcxiFZmk973PIjqfCBTmWtlqex7ROSnT44PpWY9sznUqEowcNPTKxx 4Gn83JXy553bvWUtnzhCk4o1MGKaLkbWEqKj9bwEGhwyFe6uFhLQpd88cPu+zA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1758309783; a=rsa-sha256; cv=none; b=iD+lfE0WxhbmJlhbRohAVXVxeRt7rcuYPngTaTEHhJ2zUnG6rFxkI5ZbHd5Z85w2Lv3c0d kXel8c8/29nUOHKxJq/3hiGKi8uwd8/qjCrFsQzFr4nF/Um5XGVicRPRrhWp4SR2iXfOjd DvaEF9h250QJhHZsfDoCf0wX9I9gRTbZJbR23TlueN3WkOwau015l9fhl99NdKwuhXChDb DcjQB5yZR+VBwLnGSCzfeT/RhdArtiU27i8WCgdHWZXAOgPK5ZkNJrNriI7AeGMrsKCP0w WnWVJHpDUO927bj88pL9YeFPTYlPOQ9WhiFHG3ujvasNehtKwxDo5cTjcnNRpg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none 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 4cT2Tg21JTz19Pb; Fri, 19 Sep 2025 19:23:03 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 58JJN3uW013875; Fri, 19 Sep 2025 19:23:03 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 58JJN30J013872; Fri, 19 Sep 2025 19:23:03 GMT (envelope-from git) Date: Fri, 19 Sep 2025 19:23:03 GMT Message-Id: <202509191923.58JJN30J013872@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Colin Percival Subject: git: 1f2a65222b88 - main - Revert "kern/vfs_vnops.c: generalize the lock primitive for file foffset" List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: cperciva X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 1f2a65222b8808dfa0466d6cad165116431dd6a9 Auto-Submitted: auto-generated The branch main has been updated by cperciva: URL: https://cgit.FreeBSD.org/src/commit/?id=1f2a65222b8808dfa0466d6cad165116431dd6a9 commit 1f2a65222b8808dfa0466d6cad165116431dd6a9 Author: Colin Percival AuthorDate: 2025-09-19 19:15:31 +0000 Commit: Colin Percival CommitDate: 2025-09-19 19:22:44 +0000 Revert "kern/vfs_vnops.c: generalize the lock primitive for file foffset" Temporarily back this out to fix the tree while I try 16.0 snapshot builds. It will come back once D52626 lands. This reverts commit 94a0f9f63e164e4f3791bc7d105d6f267b4d47bc. Discussed with: kib --- sys/kern/vfs_vnops.c | 148 ++++++++++++++++++++------------------------------- sys/sys/file.h | 6 +-- 2 files changed, 60 insertions(+), 94 deletions(-) diff --git a/sys/kern/vfs_vnops.c b/sys/kern/vfs_vnops.c index f81c2033d95e..a4f41192f684 100644 --- a/sys/kern/vfs_vnops.c +++ b/sys/kern/vfs_vnops.c @@ -798,82 +798,58 @@ vn_rdwr_inchunks(enum uio_rw rw, struct vnode *vp, void *base, size_t len, } #if OFF_MAX <= LONG_MAX -static void -file_v_lock(struct file *fp, short lock_bit, short lock_wait_bit) +off_t +foffset_lock(struct file *fp, int flags) { - short *flagsp; + volatile short *flagsp; + off_t res; short state; - flagsp = &fp->f_vflags; - state = atomic_load_16(flagsp); - if ((state & lock_bit) == 0 && - atomic_cmpset_acq_16(flagsp, state, state | lock_bit)) - return; + KASSERT((flags & FOF_OFFSET) == 0, ("FOF_OFFSET passed")); + + if ((flags & FOF_NOLOCK) != 0) + return (atomic_load_long(&fp->f_offset)); - sleepq_lock(flagsp); + /* + * According to McKusick the vn lock was protecting f_offset here. + * It is now protected by the FOFFSET_LOCKED flag. + */ + flagsp = &fp->f_vnread_flags; + if (atomic_cmpset_acq_16(flagsp, 0, FOFFSET_LOCKED)) + return (atomic_load_long(&fp->f_offset)); + + sleepq_lock(&fp->f_vnread_flags); state = atomic_load_16(flagsp); for (;;) { - if ((state & lock_bit) == 0) { + if ((state & FOFFSET_LOCKED) == 0) { if (!atomic_fcmpset_acq_16(flagsp, &state, - state | lock_bit)) + FOFFSET_LOCKED)) continue; break; } - if ((state & lock_wait_bit) == 0) { + if ((state & FOFFSET_LOCK_WAITING) == 0) { if (!atomic_fcmpset_acq_16(flagsp, &state, - state | lock_wait_bit)) + state | FOFFSET_LOCK_WAITING)) continue; } DROP_GIANT(); - sleepq_add(flagsp, NULL, "vofflock", 0, 0); - sleepq_wait(flagsp, PRI_MAX_KERN); + sleepq_add(&fp->f_vnread_flags, NULL, "vofflock", 0, 0); + sleepq_wait(&fp->f_vnread_flags, PRI_MAX_KERN); PICKUP_GIANT(); - sleepq_lock(flagsp); + sleepq_lock(&fp->f_vnread_flags); state = atomic_load_16(flagsp); } - sleepq_release(flagsp); -} - -static void -file_v_unlock(struct file *fp, short lock_bit, short lock_wait_bit) -{ - short *flagsp; - short state; - - flagsp = &fp->f_vflags; - state = atomic_load_16(flagsp); - if ((state & lock_wait_bit) == 0 && - atomic_cmpset_rel_16(flagsp, state, state & ~lock_bit)) - return; - - sleepq_lock(flagsp); - MPASS((*flagsp & lock_bit) != 0); - MPASS((*flagsp & lock_wait_bit) != 0); - atomic_clear_16(flagsp, lock_bit | lock_wait_bit); - sleepq_broadcast(flagsp, SLEEPQ_SLEEP, 0, 0); - sleepq_release(flagsp); -} - -off_t -foffset_lock(struct file *fp, int flags) -{ - KASSERT((flags & FOF_OFFSET) == 0, ("FOF_OFFSET passed")); - - if ((flags & FOF_NOLOCK) == 0) { - file_v_lock(fp, FILE_V_FOFFSET_LOCKED, - FILE_V_FOFFSET_LOCK_WAITING); - } - - /* - * According to McKusick the vn lock was protecting f_offset here. - * It is now protected by the FOFFSET_LOCKED flag. - */ - return (atomic_load_long(&fp->f_offset)); + res = atomic_load_long(&fp->f_offset); + sleepq_release(&fp->f_vnread_flags); + return (res); } void foffset_unlock(struct file *fp, off_t val, int flags) { + volatile short *flagsp; + short state; + KASSERT((flags & FOF_OFFSET) == 0, ("FOF_OFFSET passed")); if ((flags & FOF_NOUPDATE) == 0) @@ -883,10 +859,21 @@ foffset_unlock(struct file *fp, off_t val, int flags) if ((flags & FOF_NEXTOFF_W) != 0) fp->f_nextoff[UIO_WRITE] = val; - if ((flags & FOF_NOLOCK) == 0) { - file_v_unlock(fp, FILE_V_FOFFSET_LOCKED, - FILE_V_FOFFSET_LOCK_WAITING); - } + if ((flags & FOF_NOLOCK) != 0) + return; + + flagsp = &fp->f_vnread_flags; + state = atomic_load_16(flagsp); + if ((state & FOFFSET_LOCK_WAITING) == 0 && + atomic_cmpset_rel_16(flagsp, state, 0)) + return; + + sleepq_lock(&fp->f_vnread_flags); + MPASS((fp->f_vnread_flags & FOFFSET_LOCKED) != 0); + MPASS((fp->f_vnread_flags & FOFFSET_LOCK_WAITING) != 0); + fp->f_vnread_flags = 0; + sleepq_broadcast(&fp->f_vnread_flags, SLEEPQ_SLEEP, 0, 0); + sleepq_release(&fp->f_vnread_flags); } static off_t @@ -895,35 +882,7 @@ foffset_read(struct file *fp) return (atomic_load_long(&fp->f_offset)); } - -#else /* OFF_MAX <= LONG_MAX */ - -static void -file_v_lock_mtxp(struct file *fp, struct mtx *mtxp, short lock_bit, - short lock_wait_bit) -{ - mtx_assert(mtxp, MA_OWNED); - - while ((fp->f_vflags & lock_bit) != 0) { - fp->f_vflags |= lock_wait_bit; - msleep(&fp->f_vflags, mtxp, PRI_MAX_KERN, - "vofflock", 0); - } - fp->f_vflags |= lock_bit; -} - -static void -file_v_unlock_mtxp(struct file *fp, struct mtx *mtxp, short lock_bit, - short lock_wait_bit) -{ - mtx_assert(mtxp, MA_OWNED); - - KASSERT((fp->f_vflags & lock_bit) != 0, ("Lost lock_bit")); - if ((fp->f_vflags & lock_wait_bit) != 0) - wakeup(&fp->f_vflags); - fp->f_vflags &= ~(lock_bit | lock_wait_bit); -} - +#else off_t foffset_lock(struct file *fp, int flags) { @@ -935,8 +894,12 @@ foffset_lock(struct file *fp, int flags) mtxp = mtx_pool_find(mtxpool_sleep, fp); mtx_lock(mtxp); if ((flags & FOF_NOLOCK) == 0) { - file_v_lock_mtxp(fp, mtxp, FILE_V_FOFFSET_LOCKED, - FILE_V_FOFFSET_LOCK_WAITING); + while (fp->f_vnread_flags & FOFFSET_LOCKED) { + fp->f_vnread_flags |= FOFFSET_LOCK_WAITING; + msleep(&fp->f_vnread_flags, mtxp, PRI_MAX_KERN, + "vofflock", 0); + } + fp->f_vnread_flags |= FOFFSET_LOCKED; } res = fp->f_offset; mtx_unlock(mtxp); @@ -959,8 +922,11 @@ foffset_unlock(struct file *fp, off_t val, int flags) if ((flags & FOF_NEXTOFF_W) != 0) fp->f_nextoff[UIO_WRITE] = val; if ((flags & FOF_NOLOCK) == 0) { - file_v_unlock_mtxp(fp, mtxp, FILE_V_FOFFSET_LOCKED, - FILE_V_FOFFSET_LOCK_WAITING); + KASSERT((fp->f_vnread_flags & FOFFSET_LOCKED) != 0, + ("Lost FOFFSET_LOCKED")); + if (fp->f_vnread_flags & FOFFSET_LOCK_WAITING) + wakeup(&fp->f_vnread_flags); + fp->f_vnread_flags = 0; } mtx_unlock(mtxp); } diff --git a/sys/sys/file.h b/sys/sys/file.h index 9a072121e25f..cc3c733580fd 100644 --- a/sys/sys/file.h +++ b/sys/sys/file.h @@ -197,7 +197,7 @@ struct file { struct vnode *f_vnode; /* NULL or applicable vnode */ struct ucred *f_cred; /* associated credentials. */ short f_type; /* descriptor type */ - short f_vflags; /* (f) Sleep lock flags for members */ + short f_vnread_flags; /* (f) Sleep lock for f_offset */ /* * DTYPE_VNODE specific fields. */ @@ -220,8 +220,8 @@ struct file { #define f_cdevpriv f_vnun.fvn_cdevpriv #define f_advice f_vnun.fvn_advice -#define FILE_V_FOFFSET_LOCKED 0x0001 -#define FILE_V_FOFFSET_LOCK_WAITING 0x0002 +#define FOFFSET_LOCKED 0x1 +#define FOFFSET_LOCK_WAITING 0x2 #endif /* __BSD_VISIBLE */ #endif /* _KERNEL || _WANT_FILE */