git: 0ee6f5efdfc6 - main - kern/vfs_vnops.c: generalize the lock primitive for file foffset
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sat, 20 Sep 2025 19:34:27 UTC
The branch main has been updated by kib:
URL: https://cgit.FreeBSD.org/src/commit/?id=0ee6f5efdfc6550077f15204335a68aab9c34d67
commit 0ee6f5efdfc6550077f15204335a68aab9c34d67
Author: Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2025-09-10 16:35:01 +0000
Commit: Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2025-09-20 19:24:10 +0000
kern/vfs_vnops.c: generalize the lock primitive for file foffset
Generalize foffset_lock/unlock() by splitting the locking info
file_v_lock/unlock() (LP64 case) or file_v_lock/unlock_mtxp() for ILP32
(using mutex pool) and then taking the action to read the offset.
sys/file.h: rename struct file f_vnread_flags member into generic f_vflags
Reviewed by: markj
Tested by: pho
Sponsored by: The FreeBSD Foundation
MFC after: 2 weeks
Differential revision: https://reviews.freebsd.org/D52486
---
sys/kern/vfs_vnops.c | 148 +++++++++++++++++++++++++++++++--------------------
sys/sys/file.h | 6 +--
2 files changed, 94 insertions(+), 60 deletions(-)
diff --git a/sys/kern/vfs_vnops.c b/sys/kern/vfs_vnops.c
index a4f41192f684..f81c2033d95e 100644
--- a/sys/kern/vfs_vnops.c
+++ b/sys/kern/vfs_vnops.c
@@ -798,58 +798,82 @@ vn_rdwr_inchunks(enum uio_rw rw, struct vnode *vp, void *base, size_t len,
}
#if OFF_MAX <= LONG_MAX
-off_t
-foffset_lock(struct file *fp, int flags)
+static void
+file_v_lock(struct file *fp, short lock_bit, short lock_wait_bit)
{
- volatile short *flagsp;
- off_t res;
+ short *flagsp;
short state;
- KASSERT((flags & FOF_OFFSET) == 0, ("FOF_OFFSET passed"));
-
- if ((flags & FOF_NOLOCK) != 0)
- return (atomic_load_long(&fp->f_offset));
-
- /*
- * 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));
+ flagsp = &fp->f_vflags;
+ state = atomic_load_16(flagsp);
+ if ((state & lock_bit) == 0 &&
+ atomic_cmpset_acq_16(flagsp, state, state | lock_bit))
+ return;
- sleepq_lock(&fp->f_vnread_flags);
+ sleepq_lock(flagsp);
state = atomic_load_16(flagsp);
for (;;) {
- if ((state & FOFFSET_LOCKED) == 0) {
+ if ((state & lock_bit) == 0) {
if (!atomic_fcmpset_acq_16(flagsp, &state,
- FOFFSET_LOCKED))
+ state | lock_bit))
continue;
break;
}
- if ((state & FOFFSET_LOCK_WAITING) == 0) {
+ if ((state & lock_wait_bit) == 0) {
if (!atomic_fcmpset_acq_16(flagsp, &state,
- state | FOFFSET_LOCK_WAITING))
+ state | lock_wait_bit))
continue;
}
DROP_GIANT();
- sleepq_add(&fp->f_vnread_flags, NULL, "vofflock", 0, 0);
- sleepq_wait(&fp->f_vnread_flags, PRI_MAX_KERN);
+ sleepq_add(flagsp, NULL, "vofflock", 0, 0);
+ sleepq_wait(flagsp, PRI_MAX_KERN);
PICKUP_GIANT();
- sleepq_lock(&fp->f_vnread_flags);
+ sleepq_lock(flagsp);
state = atomic_load_16(flagsp);
}
- res = atomic_load_long(&fp->f_offset);
- sleepq_release(&fp->f_vnread_flags);
- return (res);
+ sleepq_release(flagsp);
}
-void
-foffset_unlock(struct file *fp, off_t val, int flags)
+static void
+file_v_unlock(struct file *fp, short lock_bit, short lock_wait_bit)
{
- volatile short *flagsp;
+ 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));
+}
+
+void
+foffset_unlock(struct file *fp, off_t val, int flags)
+{
KASSERT((flags & FOF_OFFSET) == 0, ("FOF_OFFSET passed"));
if ((flags & FOF_NOUPDATE) == 0)
@@ -859,21 +883,10 @@ 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)
- 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);
+ if ((flags & FOF_NOLOCK) == 0) {
+ file_v_unlock(fp, FILE_V_FOFFSET_LOCKED,
+ FILE_V_FOFFSET_LOCK_WAITING);
+ }
}
static off_t
@@ -882,7 +895,35 @@ foffset_read(struct file *fp)
return (atomic_load_long(&fp->f_offset));
}
-#else
+
+#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);
+}
+
off_t
foffset_lock(struct file *fp, int flags)
{
@@ -894,12 +935,8 @@ foffset_lock(struct file *fp, int flags)
mtxp = mtx_pool_find(mtxpool_sleep, fp);
mtx_lock(mtxp);
if ((flags & FOF_NOLOCK) == 0) {
- 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;
+ file_v_lock_mtxp(fp, mtxp, FILE_V_FOFFSET_LOCKED,
+ FILE_V_FOFFSET_LOCK_WAITING);
}
res = fp->f_offset;
mtx_unlock(mtxp);
@@ -922,11 +959,8 @@ 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) {
- 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;
+ file_v_unlock_mtxp(fp, mtxp, FILE_V_FOFFSET_LOCKED,
+ FILE_V_FOFFSET_LOCK_WAITING);
}
mtx_unlock(mtxp);
}
diff --git a/sys/sys/file.h b/sys/sys/file.h
index cc3c733580fd..9a072121e25f 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_vnread_flags; /* (f) Sleep lock for f_offset */
+ short f_vflags; /* (f) Sleep lock flags for members */
/*
* 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 FOFFSET_LOCKED 0x1
-#define FOFFSET_LOCK_WAITING 0x2
+#define FILE_V_FOFFSET_LOCKED 0x0001
+#define FILE_V_FOFFSET_LOCK_WAITING 0x0002
#endif /* __BSD_VISIBLE */
#endif /* _KERNEL || _WANT_FILE */