Date: Wed, 29 Aug 2012 15:45:59 +0000 (UTC) From: Konstantin Belousov <kib@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r239843 - in stable/9/sys: compat/linux fs/devfs kern sys ufs/ffs Message-ID: <201208291545.q7TFjxX0070678@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kib Date: Wed Aug 29 15:45:58 2012 New Revision: 239843 URL: http://svn.freebsd.org/changeset/base/239843 Log: MFC r238029: Extend the KPI to lock and unlock f_offset member of struct file. It now fully encapsulates all accesses to f_offset, and extends f_offset locking to other consumers that need it, in particular, to lseek() and variants of getdirentries(). Modified: stable/9/sys/compat/linux/linux_file.c stable/9/sys/fs/devfs/devfs_vnops.c stable/9/sys/kern/kern_descrip.c stable/9/sys/kern/vfs_syscalls.c stable/9/sys/kern/vfs_vnops.c stable/9/sys/sys/file.h stable/9/sys/ufs/ffs/ffs_alloc.c Directory Properties: stable/9/sys/ (props changed) stable/9/sys/fs/ (props changed) Modified: stable/9/sys/compat/linux/linux_file.c ============================================================================== --- stable/9/sys/compat/linux/linux_file.c Wed Aug 29 15:41:17 2012 (r239842) +++ stable/9/sys/compat/linux/linux_file.c Wed Aug 29 15:45:58 2012 (r239843) @@ -354,15 +354,16 @@ getdents_common(struct thread *td, struc return (EBADF); } + off = foffset_lock(fp, 0); vp = fp->f_vnode; vfslocked = VFS_LOCK_GIANT(vp->v_mount); if (vp->v_type != VDIR) { VFS_UNLOCK_GIANT(vfslocked); + foffset_unlock(fp, off, 0); fdrop(fp, td); return (EINVAL); } - off = fp->f_offset; buflen = max(LINUX_DIRBLKSIZ, nbytes); buflen = min(buflen, MAXBSIZE); @@ -511,7 +512,6 @@ getdents_common(struct thread *td, struc goto eof; } - fp->f_offset = off; if (justone) nbytes = resid + linuxreclen; @@ -524,6 +524,7 @@ out: VOP_UNLOCK(vp, 0); VFS_UNLOCK_GIANT(vfslocked); + foffset_unlock(fp, off, 0); fdrop(fp, td); free(buf, M_TEMP); free(lbuf, M_TEMP); Modified: stable/9/sys/fs/devfs/devfs_vnops.c ============================================================================== --- stable/9/sys/fs/devfs/devfs_vnops.c Wed Aug 29 15:41:17 2012 (r239842) +++ stable/9/sys/fs/devfs/devfs_vnops.c Wed Aug 29 15:45:58 2012 (r239843) @@ -1170,18 +1170,14 @@ devfs_read_f(struct file *fp, struct uio if (ioflag & O_DIRECT) ioflag |= IO_DIRECT; - if ((flags & FOF_OFFSET) == 0) - uio->uio_offset = fp->f_offset; - + foffset_lock_uio(fp, uio, flags | FOF_NOLOCK); error = dsw->d_read(dev, uio, ioflag); if (uio->uio_resid != resid || (error == 0 && resid != 0)) vfs_timestamp(&dev->si_atime); td->td_fpop = fpop; dev_relthread(dev, ref); - if ((flags & FOF_OFFSET) == 0) - fp->f_offset = uio->uio_offset; - fp->f_nextoff = uio->uio_offset; + foffset_unlock_uio(fp, uio, flags | FOF_NOLOCK | FOF_NEXTOFF); return (error); } @@ -1648,8 +1644,7 @@ devfs_write_f(struct file *fp, struct ui ioflag = fp->f_flag & (O_NONBLOCK | O_DIRECT | O_FSYNC); if (ioflag & O_DIRECT) ioflag |= IO_DIRECT; - if ((flags & FOF_OFFSET) == 0) - uio->uio_offset = fp->f_offset; + foffset_lock_uio(fp, uio, flags | FOF_NOLOCK); resid = uio->uio_resid; @@ -1661,9 +1656,7 @@ devfs_write_f(struct file *fp, struct ui td->td_fpop = fpop; dev_relthread(dev, ref); - if ((flags & FOF_OFFSET) == 0) - fp->f_offset = uio->uio_offset; - fp->f_nextoff = uio->uio_offset; + foffset_unlock_uio(fp, uio, flags | FOF_NOLOCK | FOF_NEXTOFF); return (error); } Modified: stable/9/sys/kern/kern_descrip.c ============================================================================== --- stable/9/sys/kern/kern_descrip.c Wed Aug 29 15:41:17 2012 (r239842) +++ stable/9/sys/kern/kern_descrip.c Wed Aug 29 15:45:58 2012 (r239843) @@ -472,6 +472,7 @@ kern_fcntl(struct thread *td, int fd, in int vfslocked; u_int old, new; uint64_t bsize; + off_t foffset; vfslocked = 0; error = 0; @@ -613,14 +614,15 @@ kern_fcntl(struct thread *td, int fd, in } flp = (struct flock *)arg; if (flp->l_whence == SEEK_CUR) { - if (fp->f_offset < 0 || + foffset = foffset_get(fp); + if (foffset < 0 || (flp->l_start > 0 && - fp->f_offset > OFF_MAX - flp->l_start)) { + foffset > OFF_MAX - flp->l_start)) { FILEDESC_SUNLOCK(fdp); error = EOVERFLOW; break; } - flp->l_start += fp->f_offset; + flp->l_start += foffset; } /* @@ -714,15 +716,16 @@ kern_fcntl(struct thread *td, int fd, in break; } if (flp->l_whence == SEEK_CUR) { + foffset = foffset_get(fp); if ((flp->l_start > 0 && - fp->f_offset > OFF_MAX - flp->l_start) || + foffset > OFF_MAX - flp->l_start) || (flp->l_start < 0 && - fp->f_offset < OFF_MIN - flp->l_start)) { + foffset < OFF_MIN - flp->l_start)) { FILEDESC_SUNLOCK(fdp); error = EOVERFLOW; break; } - flp->l_start += fp->f_offset; + flp->l_start += foffset; } /* * VOP_ADVLOCK() may block. @@ -2907,7 +2910,7 @@ sysctl_kern_file(SYSCTL_HANDLER_ARGS) xf.xf_type = fp->f_type; xf.xf_count = fp->f_count; xf.xf_msgcount = 0; - xf.xf_offset = fp->f_offset; + xf.xf_offset = foffset_get(fp); xf.xf_flag = fp->f_flag; error = SYSCTL_OUT(req, &xf, sizeof(xf)); if (error) @@ -3112,7 +3115,7 @@ sysctl_kern_proc_ofiledesc(SYSCTL_HANDLE kif->kf_flags |= KF_FLAG_DIRECT; if (fp->f_flag & FHASLOCK) kif->kf_flags |= KF_FLAG_HASLOCK; - kif->kf_offset = fp->f_offset; + kif->kf_offset = foffset_get(fp); if (vp != NULL) { vref(vp); switch (vp->v_type) { @@ -3456,7 +3459,7 @@ sysctl_kern_proc_filedesc(SYSCTL_HANDLER } refcnt = fp->f_count; fflags = fp->f_flag; - offset = fp->f_offset; + offset = foffset_get(fp); /* * Create sysctl entry. Modified: stable/9/sys/kern/vfs_syscalls.c ============================================================================== --- stable/9/sys/kern/vfs_syscalls.c Wed Aug 29 15:41:17 2012 (r239842) +++ stable/9/sys/kern/vfs_syscalls.c Wed Aug 29 15:45:58 2012 (r239843) @@ -1992,7 +1992,7 @@ sys_lseek(td, uap) struct file *fp; struct vnode *vp; struct vattr vattr; - off_t offset, size; + off_t foffset, offset, size; int error, noneg; int vfslocked; @@ -2004,18 +2004,19 @@ sys_lseek(td, uap) return (ESPIPE); } vp = fp->f_vnode; + foffset = foffset_lock(fp, 0); vfslocked = VFS_LOCK_GIANT(vp->v_mount); noneg = (vp->v_type != VCHR); offset = uap->offset; switch (uap->whence) { case L_INCR: if (noneg && - (fp->f_offset < 0 || - (offset > 0 && fp->f_offset > OFF_MAX - offset))) { + (foffset < 0 || + (offset > 0 && foffset > OFF_MAX - offset))) { error = EOVERFLOW; break; } - offset += fp->f_offset; + offset += foffset; break; case L_XTND: vn_lock(vp, LK_SHARED | LK_RETRY); @@ -2055,12 +2056,12 @@ sys_lseek(td, uap) error = EINVAL; if (error != 0) goto drop; - fp->f_offset = offset; VFS_KNOTE_UNLOCKED(vp, 0); - *(off_t *)(td->td_retval) = fp->f_offset; + *(off_t *)(td->td_retval) = offset; drop: fdrop(fp, td); VFS_UNLOCK_GIANT(vfslocked); + foffset_unlock(fp, offset, error != 0 ? FOF_NOUPDATE : 0); return (error); } @@ -3993,6 +3994,7 @@ kern_ogetdirentries(struct thread *td, s caddr_t dirbuf; int error, eofflag, readcnt, vfslocked; long loff; + off_t foffset; /* XXX arbitrary sanity limit on `count'. */ if (uap->count > 64 * 1024) @@ -4005,10 +4007,12 @@ kern_ogetdirentries(struct thread *td, s return (EBADF); } vp = fp->f_vnode; + foffset = foffset_lock(fp, 0); unionread: vfslocked = VFS_LOCK_GIANT(vp->v_mount); if (vp->v_type != VDIR) { VFS_UNLOCK_GIANT(vfslocked); + foffset_unlock(fp, foffset, 0); fdrop(fp, td); return (EINVAL); } @@ -4021,12 +4025,13 @@ unionread: auio.uio_td = td; auio.uio_resid = uap->count; vn_lock(vp, LK_SHARED | LK_RETRY); - loff = auio.uio_offset = fp->f_offset; + loff = auio.uio_offset = foffset; #ifdef MAC error = mac_vnode_check_readdir(td->td_ucred, vp); if (error) { VOP_UNLOCK(vp, 0); VFS_UNLOCK_GIANT(vfslocked); + foffset_unlock(fp, foffset, FOF_NOUPDATE); fdrop(fp, td); return (error); } @@ -4035,7 +4040,7 @@ unionread: if (vp->v_mount->mnt_maxsymlinklen <= 0) { error = VOP_READDIR(vp, &auio, fp->f_cred, &eofflag, NULL, NULL); - fp->f_offset = auio.uio_offset; + foffset = auio.uio_offset; } else # endif { @@ -4047,7 +4052,7 @@ unionread: kiov.iov_base = dirbuf; error = VOP_READDIR(vp, &kuio, fp->f_cred, &eofflag, NULL, NULL); - fp->f_offset = kuio.uio_offset; + foffset = kuio.uio_offset; if (error == 0) { readcnt = uap->count - kuio.uio_resid; edp = (struct dirent *)&dirbuf[readcnt]; @@ -4085,6 +4090,7 @@ unionread: if (error) { VOP_UNLOCK(vp, 0); VFS_UNLOCK_GIANT(vfslocked); + foffset_unlock(fp, foffset, 0); fdrop(fp, td); return (error); } @@ -4096,13 +4102,14 @@ unionread: VREF(vp); fp->f_vnode = vp; fp->f_data = vp; - fp->f_offset = 0; + foffset = 0; vput(tvp); VFS_UNLOCK_GIANT(vfslocked); goto unionread; } VOP_UNLOCK(vp, 0); VFS_UNLOCK_GIANT(vfslocked); + foffset_unlock(fp, foffset, 0); fdrop(fp, td); td->td_retval[0] = uap->count - auio.uio_resid; if (error == 0) @@ -4154,6 +4161,7 @@ kern_getdirentries(struct thread *td, in int vfslocked; long loff; int error, eofflag; + off_t foffset; AUDIT_ARG_FD(fd); auio.uio_resid = count; @@ -4167,6 +4175,7 @@ kern_getdirentries(struct thread *td, in return (EBADF); } vp = fp->f_vnode; + foffset = foffset_lock(fp, 0); unionread: vfslocked = VFS_LOCK_GIANT(vp->v_mount); if (vp->v_type != VDIR) { @@ -4183,14 +4192,14 @@ unionread: auio.uio_td = td; vn_lock(vp, LK_SHARED | LK_RETRY); AUDIT_ARG_VNODE1(vp); - loff = auio.uio_offset = fp->f_offset; + loff = auio.uio_offset = foffset; #ifdef MAC error = mac_vnode_check_readdir(td->td_ucred, vp); if (error == 0) #endif error = VOP_READDIR(vp, &auio, fp->f_cred, &eofflag, NULL, NULL); - fp->f_offset = auio.uio_offset; + foffset = auio.uio_offset; if (error) { VOP_UNLOCK(vp, 0); VFS_UNLOCK_GIANT(vfslocked); @@ -4204,7 +4213,7 @@ unionread: VREF(vp); fp->f_vnode = vp; fp->f_data = vp; - fp->f_offset = 0; + foffset = 0; vput(tvp); VFS_UNLOCK_GIANT(vfslocked); goto unionread; @@ -4214,6 +4223,7 @@ unionread: *basep = loff; td->td_retval[0] = count - auio.uio_resid; fail: + foffset_unlock(fp, foffset, 0); fdrop(fp, td); return (error); } Modified: stable/9/sys/kern/vfs_vnops.c ============================================================================== --- stable/9/sys/kern/vfs_vnops.c Wed Aug 29 15:41:17 2012 (r239842) +++ stable/9/sys/kern/vfs_vnops.c Wed Aug 29 15:45:58 2012 (r239843) @@ -512,13 +512,22 @@ vn_rdwr_inchunks(rw, vp, base, len, offs return (error); } -static void -foffset_lock(struct file *fp, struct uio *uio, int flags) +off_t +foffset_lock(struct file *fp, int flags) { struct mtx *mtxp; + off_t res; - if ((flags & FOF_OFFSET) != 0) - return; + KASSERT((flags & FOF_OFFSET) == 0, ("FOF_OFFSET passed")); + +#if OFF_MAX <= LONG_MAX + /* + * Caller only wants the current f_offset value. Assume that + * the long and shorter integer types reads are atomic. + */ + if ((flags & FOF_NOLOCK) != 0) + return (fp->f_offset); +#endif /* * According to McKusick the vn lock was protecting f_offset here. @@ -526,16 +535,68 @@ foffset_lock(struct file *fp, struct uio */ mtxp = mtx_pool_find(mtxpool_sleep, fp); mtx_lock(mtxp); - while (fp->f_vnread_flags & FOFFSET_LOCKED) { - fp->f_vnread_flags |= FOFFSET_LOCK_WAITING; - msleep(&fp->f_vnread_flags, mtxp, PUSER -1, - "vnread offlock", 0); + 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, PUSER -1, + "vofflock", 0); + } + fp->f_vnread_flags |= FOFFSET_LOCKED; + } + res = fp->f_offset; + mtx_unlock(mtxp); + return (res); +} + +void +foffset_unlock(struct file *fp, off_t val, int flags) +{ + struct mtx *mtxp; + + KASSERT((flags & FOF_OFFSET) == 0, ("FOF_OFFSET passed")); + +#if OFF_MAX <= LONG_MAX + if ((flags & FOF_NOLOCK) != 0) { + if ((flags & FOF_NOUPDATE) == 0) + fp->f_offset = val; + if ((flags & FOF_NEXTOFF) != 0) + fp->f_nextoff = val; + return; + } +#endif + + mtxp = mtx_pool_find(mtxpool_sleep, fp); + mtx_lock(mtxp); + if ((flags & FOF_NOUPDATE) == 0) + fp->f_offset = val; + if ((flags & FOF_NEXTOFF) != 0) + fp->f_nextoff = 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; } - fp->f_vnread_flags |= FOFFSET_LOCKED; - uio->uio_offset = fp->f_offset; mtx_unlock(mtxp); } +void +foffset_lock_uio(struct file *fp, struct uio *uio, int flags) +{ + + if ((flags & FOF_OFFSET) == 0) + uio->uio_offset = foffset_lock(fp, flags); +} + +void +foffset_unlock_uio(struct file *fp, struct uio *uio, int flags) +{ + + if ((flags & FOF_OFFSET) == 0) + foffset_unlock(fp, uio->uio_offset, flags); +} + static int get_advice(struct file *fp, struct uio *uio) { @@ -555,23 +616,6 @@ get_advice(struct file *fp, struct uio * return (ret); } -static void -foffset_unlock(struct file *fp, struct uio *uio, int flags) -{ - struct mtx *mtxp; - - if ((flags & FOF_OFFSET) != 0) - return; - - fp->f_offset = uio->uio_offset; - mtxp = mtx_pool_find(mtxpool_sleep, fp); - mtx_lock(mtxp); - if (fp->f_vnread_flags & FOFFSET_LOCK_WAITING) - wakeup(&fp->f_vnread_flags); - fp->f_vnread_flags = 0; - mtx_unlock(mtxp); -} - /* * File table vnode read routine. */ @@ -850,7 +894,7 @@ vn_io_fault(struct file *fp, struct uio else doio = vn_write; vp = fp->f_vnode; - foffset_lock(fp, uio, flags); + foffset_lock_uio(fp, uio, flags); if (uio->uio_segflg != UIO_USERSPACE || vp->v_type != VREG || ((mp = vp->v_mount) != NULL && @@ -967,7 +1011,7 @@ out: vn_rangelock_unlock(vp, rl_cookie); free(uio_clone, M_IOV); out_last: - foffset_unlock(fp, uio, flags); + foffset_unlock_uio(fp, uio, flags); return (error); } Modified: stable/9/sys/sys/file.h ============================================================================== --- stable/9/sys/sys/file.h Wed Aug 29 15:41:17 2012 (r239842) +++ stable/9/sys/sys/file.h Wed Aug 29 15:45:58 2012 (r239843) @@ -72,10 +72,25 @@ struct socket; struct file; struct ucred; +#define FOF_OFFSET 0x01 /* Use the offset in uio argument */ +#define FOF_NOLOCK 0x02 /* Do not take FOFFSET_LOCK */ +#define FOF_NEXTOFF 0x04 /* Also update f_nextoff */ +#define FOF_NOUPDATE 0x10 /* Do not update f_offset */ +off_t foffset_lock(struct file *fp, int flags); +void foffset_lock_uio(struct file *fp, struct uio *uio, int flags); +void foffset_unlock(struct file *fp, off_t val, int flags); +void foffset_unlock_uio(struct file *fp, struct uio *uio, int flags); + +static inline off_t +foffset_get(struct file *fp) +{ + + return (foffset_lock(fp, FOF_NOLOCK)); +} + typedef int fo_rdwr_t(struct file *fp, struct uio *uio, struct ucred *active_cred, int flags, struct thread *td); -#define FOF_OFFSET 1 /* Use the offset in uio argument */ typedef int fo_truncate_t(struct file *fp, off_t length, struct ucred *active_cred, struct thread *td); typedef int fo_ioctl_t(struct file *fp, u_long com, void *data, Modified: stable/9/sys/ufs/ffs/ffs_alloc.c ============================================================================== --- stable/9/sys/ufs/ffs/ffs_alloc.c Wed Aug 29 15:41:17 2012 (r239842) +++ stable/9/sys/ufs/ffs/ffs_alloc.c Wed Aug 29 15:45:58 2012 (r239843) @@ -2871,10 +2871,9 @@ buffered_write(fp, uio, active_cred, fla if (ip->i_devvp != devvp) return (EINVAL); fs = ip->i_fs; + foffset_lock_uio(fp, uio, flags); vfslocked = VFS_LOCK_GIANT(ip->i_vnode->v_mount); vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY); - if ((flags & FOF_OFFSET) == 0) - uio->uio_offset = fp->f_offset; #ifdef DEBUG if (fsckcmds) { printf("%s: buffered write for block %jd\n", @@ -2899,11 +2898,9 @@ buffered_write(fp, uio, active_cred, fla goto out; } error = bwrite(bp); - if ((flags & FOF_OFFSET) == 0) - fp->f_offset = uio->uio_offset; - fp->f_nextoff = uio->uio_offset; out: VOP_UNLOCK(devvp, 0); VFS_UNLOCK_GIANT(vfslocked); + foffset_unlock_uio(fp, uio, flags | FOF_NEXTOFF); return (error); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201208291545.q7TFjxX0070678>