Changeset View
Standalone View
sys/kern/vfs_vnops.c
Show First 20 Lines • Show All 55 Lines • ▼ Show 20 Lines | |||||
#include <sys/kdb.h> | #include <sys/kdb.h> | ||||
#include <sys/ktr.h> | #include <sys/ktr.h> | ||||
#include <sys/stat.h> | #include <sys/stat.h> | ||||
#include <sys/priv.h> | #include <sys/priv.h> | ||||
#include <sys/proc.h> | #include <sys/proc.h> | ||||
#include <sys/limits.h> | #include <sys/limits.h> | ||||
#include <sys/lock.h> | #include <sys/lock.h> | ||||
#include <sys/mman.h> | #include <sys/mman.h> | ||||
#include <sys/malloc.h> | |||||
#include <sys/mount.h> | #include <sys/mount.h> | ||||
#include <sys/mutex.h> | #include <sys/mutex.h> | ||||
#include <sys/namei.h> | #include <sys/namei.h> | ||||
#include <sys/vnode.h> | #include <sys/vnode.h> | ||||
#include <sys/bio.h> | #include <sys/bio.h> | ||||
#include <sys/buf.h> | #include <sys/buf.h> | ||||
#include <sys/filio.h> | #include <sys/filio.h> | ||||
#include <sys/resourcevar.h> | #include <sys/resourcevar.h> | ||||
Show All 12 Lines | |||||
#include <vm/vm.h> | #include <vm/vm.h> | ||||
#include <vm/vm_extern.h> | #include <vm/vm_extern.h> | ||||
#include <vm/pmap.h> | #include <vm/pmap.h> | ||||
#include <vm/vm_map.h> | #include <vm/vm_map.h> | ||||
#include <vm/vm_object.h> | #include <vm/vm_object.h> | ||||
#include <vm/vm_page.h> | #include <vm/vm_page.h> | ||||
#include <vm/vnode_pager.h> | #include <vm/vnode_pager.h> | ||||
#include <machine/vmparam.h> | |||||
#ifdef HWPMC_HOOKS | #ifdef HWPMC_HOOKS | ||||
#include <sys/pmckern.h> | #include <sys/pmckern.h> | ||||
#endif | #endif | ||||
static fo_rdwr_t vn_read; | static fo_rdwr_t vn_read; | ||||
static fo_rdwr_t vn_write; | static fo_rdwr_t vn_write; | ||||
static fo_rdwr_t vn_io_fault; | static fo_rdwr_t vn_io_fault; | ||||
static fo_truncate_t vn_truncate; | static fo_truncate_t vn_truncate; | ||||
▲ Show 20 Lines • Show All 2,401 Lines • ▼ Show 20 Lines | |||||
vn_fsid(struct vnode *vp, struct vattr *va) | vn_fsid(struct vnode *vp, struct vattr *va) | ||||
{ | { | ||||
fsid_t *f; | fsid_t *f; | ||||
f = &vp->v_mount->mnt_stat.f_fsid; | f = &vp->v_mount->mnt_stat.f_fsid; | ||||
va->va_fsid = (uint32_t)f->val[1]; | va->va_fsid = (uint32_t)f->val[1]; | ||||
va->va_fsid <<= sizeof(f->val[1]) * NBBY; | va->va_fsid <<= sizeof(f->val[1]) * NBBY; | ||||
va->va_fsid += (uint32_t)f->val[0]; | va->va_fsid += (uint32_t)f->val[0]; | ||||
} | |||||
/* | |||||
* Copies a byte range from invp to outvp. Calls VOP_COPY_FILE_RANGE() | |||||
asomers: You can avoid the extra malloc by using the globally defined `zero_region` (see vm/vm_kern.c). | |||||
Done Inline ActionsI did a simple C version using a u_int *. Maybe it would be more efficient to use rmacklem: I did a simple C version using a u_int *. Maybe it would be more efficient to use
a uint64_t *? | |||||
* or vn_generic_copy_file_range() after rangelocking the byte ranges, | |||||
* to do the actual copy. | |||||
* vn_generic_copy_file_range() is factored out, so it can be called | |||||
* from a VOP_COPY_FILE_RANGE() call as well, but handles vnodes from | |||||
* different file systems. | |||||
*/ | |||||
int | |||||
vn_copy_file_range(struct vnode *invp, off_t *inoffp, struct vnode *outvp, | |||||
off_t *outoffp, size_t *lenp, unsigned int flags, struct ucred *incred, | |||||
struct ucred *outcred, struct thread *fsize_td) | |||||
{ | |||||
struct vattr va; | |||||
int error; | |||||
size_t len; | |||||
uint64_t uvalin, uvalout; | |||||
len = *lenp; | |||||
error = 0; | |||||
/* Do some sanity checks on the arguments. */ | |||||
uvalin = *inoffp; | |||||
uvalin += len; | |||||
uvalout = *outoffp; | |||||
Done Inline Actionsoff_t is signed type, so the overflow is undefined, and you cannot check for it by testing the wrap. We do specify -fwrapv to compiler, but there is a desire to not add new code which depends on undefined behavior, since the fight with compiler will be lost anyway. kib: off_t is signed type, so the overflow is undefined, and you cannot check for it by testing the… | |||||
Done Inline ActionsIn the code below, I used two uint64_t variables to do the overflow checks. I suppose I could define the variables as uoff_t, but I don't know if that will rmacklem: In the code below, I used two uint64_t variables to do the overflow checks.
I think I got the… | |||||
uvalout += len; | |||||
if (invp->v_type == VDIR || outvp->v_type == VDIR) | |||||
Not Done Inline ActionsYou may want to allow using VCHR. asomers: You may want to allow using `VCHR`. | |||||
Done Inline ActionsOnly defined for VREG on Linux and I'm not sure why you'd want to do this for VCHR? rmacklem: Only defined for VREG on Linux and I'm not sure why you'd want to do this for VCHR? | |||||
error = EISDIR; | |||||
else if (*inoffp < 0 || uvalin > INT64_MAX || uvalin < | |||||
(uint64_t)*inoffp || *outoffp < 0 || uvalout > INT64_MAX || | |||||
uvalout < (uint64_t)*outoffp || invp->v_type != VREG || | |||||
outvp->v_type != VREG || invp == outvp) | |||||
error = EINVAL; | |||||
Done Inline ActionsThe invp == outvp error case should return EBADF, not EINVAL. asomers: The `invp == outvp` error case should return `EBADF`, not `EINVAL`. | |||||
Done Inline ActionsThe syscall actually got this right and I don't even recall what the correct NFSv4.2 rmacklem: The syscall actually got this right and I don't even recall what the correct NFSv4.2
error is… | |||||
if (error != 0) | |||||
goto out; | |||||
error = vn_lock(invp, LK_SHARED); | |||||
if (error != 0) | |||||
goto out; | |||||
/* Check that the offset + len does not go past EOF of invp. */ | |||||
error = VOP_GETATTR(invp, &va, incred); | |||||
if (error == 0 && va.va_size < (*inoffp + len)) | |||||
error = EINVAL; | |||||
VOP_UNLOCK(invp, 0); | |||||
if (error != 0) | |||||
goto out; | |||||
bwillwrite(); | |||||
/* | |||||
* If the two vnode are for the same file system, call | |||||
* VOP_COPY_FILE_RANGE(), otherwise call vn_generic_copy_file_range() | |||||
* which can handle copies across multiple file systems. | |||||
*/ | |||||
if (invp->v_mount == outvp->v_mount) | |||||
error = VOP_COPY_FILE_RANGE(invp, inoffp, outvp, outoffp, | |||||
lenp, flags, incred, outcred, fsize_td); | |||||
Done Inline ActionsYou can avoid the malloc and VFS_STATFS by just using the cached value: invp->v_mount->mnt_stat.f_iosize. asomers: You can avoid the malloc and `VFS_STATFS` by just using the cached value: `invp->v_mount… | |||||
else | |||||
error = vn_generic_copy_file_range(invp, inoffp, outvp, | |||||
outoffp, lenp, flags, incred, outcred, fsize_td); | |||||
out: | |||||
return (error); | |||||
} | } | ||||
int | int | ||||
vn_fsync_buf(struct vnode *vp, int waitfor) | vn_fsync_buf(struct vnode *vp, int waitfor) | ||||
{ | { | ||||
struct buf *bp, *nbp; | struct buf *bp, *nbp; | ||||
Done Inline ActionsI really think you should use the least common multiple of in and out block sizes. kib: I really think you should use the least common multiple of in and out block sizes. | |||||
Done Inline ActionsMake that the LCM of in, out, and 16384. asomers: Make that the LCM of in, out, and 16384. | |||||
Done Inline ActionsBasically done. It now uses holesize if it can get that and falls back on f_iosize. rmacklem: Basically done. It now uses holesize if it can get that and falls back on f_iosize.
Assuming… | |||||
struct bufobj *bo; | struct bufobj *bo; | ||||
struct mount *mp; | struct mount *mp; | ||||
int error, maxretry; | int error, maxretry; | ||||
Done Inline ActionsThe wait in vn_start_write() is after the vnode lock in global order. You are introducing the deadlock there. kib: The wait in vn_start_write() is after the vnode lock in global order. You are introducing the… | |||||
Done Inline ActionsOops, I've made this mistake before. rmacklem: Oops, I've made this mistake before.
| |||||
error = 0; | error = 0; | ||||
maxretry = 10000; /* large, arbitrarily chosen */ | maxretry = 10000; /* large, arbitrarily chosen */ | ||||
mp = NULL; | mp = NULL; | ||||
if (vp->v_type == VCHR) { | if (vp->v_type == VCHR) { | ||||
VI_LOCK(vp); | VI_LOCK(vp); | ||||
mp = vp->v_rdev->si_mountpt; | mp = vp->v_rdev->si_mountpt; | ||||
Done Inline ActionsDo not use malloc(M_WAITOK) while holding vnode lock. Pagedaemon might need to page out a page belonging to the vnode locked by current thread.. kib: Do not use malloc(M_WAITOK) while holding vnode lock. Pagedaemon might need to page out a page… | |||||
VI_UNLOCK(vp); | VI_UNLOCK(vp); | ||||
} | } | ||||
bo = &vp->v_bufobj; | bo = &vp->v_bufobj; | ||||
BO_LOCK(bo); | BO_LOCK(bo); | ||||
loop1: | loop1: | ||||
/* | /* | ||||
* MARK/SCAN initialization to avoid infinite loops. | * MARK/SCAN initialization to avoid infinite loops. | ||||
Done Inline Actionsglibc handles this simply: the fallback is implemented in userspace instead of kernelspace. asomers: glibc handles this simply: the fallback is implemented in userspace instead of kernelspace. | |||||
Done Inline ActionsYep. Although the Linux man page claims this is done in the kernel to reduce the I think doing it across file systems in the kernel makes sense. I wanted the NFS server to do it across multiple exported file systems, so that it rmacklem: Yep. Although the Linux man page claims this is done in the kernel to reduce the
number of… | |||||
*/ | */ | ||||
TAILQ_FOREACH(bp, &bo->bo_dirty.bv_hd, b_bobufs) { | TAILQ_FOREACH(bp, &bo->bo_dirty.bv_hd, b_bobufs) { | ||||
bp->b_vflags &= ~BV_SCANNED; | bp->b_vflags &= ~BV_SCANNED; | ||||
bp->b_error = 0; | bp->b_error = 0; | ||||
} | } | ||||
/* | /* | ||||
* Flush all dirty buffers associated with a vnode. | * Flush all dirty buffers associated with a vnode. | ||||
*/ | */ | ||||
loop2: | loop2: | ||||
TAILQ_FOREACH_SAFE(bp, &bo->bo_dirty.bv_hd, b_bobufs, nbp) { | TAILQ_FOREACH_SAFE(bp, &bo->bo_dirty.bv_hd, b_bobufs, nbp) { | ||||
if ((bp->b_vflags & BV_SCANNED) != 0) | if ((bp->b_vflags & BV_SCANNED) != 0) | ||||
continue; | continue; | ||||
Done Inline ActionsEither this or next vn_rdwr() calls are done in the way which skips range locking. The result is that torn writes can be seen. FreeBSD makes the guarantee that each single write(2) syscall effects are either seen as a whole, or not seen at all, by each single read(2) syscall. kib: Either this or next vn_rdwr() calls are done in the way which skips range locking. The result… | |||||
Done Inline ActionsI'll see what others say about this. rmacklem: I'll see what others say about this.
If they want atomicity, then we need to create non… | |||||
bp->b_vflags |= BV_SCANNED; | bp->b_vflags |= BV_SCANNED; | ||||
if (BUF_LOCK(bp, LK_EXCLUSIVE | LK_NOWAIT, NULL)) { | if (BUF_LOCK(bp, LK_EXCLUSIVE | LK_NOWAIT, NULL)) { | ||||
if (waitfor != MNT_WAIT) | if (waitfor != MNT_WAIT) | ||||
continue; | continue; | ||||
if (BUF_LOCK(bp, | if (BUF_LOCK(bp, | ||||
LK_EXCLUSIVE | LK_INTERLOCK | LK_SLEEPFAIL, | LK_EXCLUSIVE | LK_INTERLOCK | LK_SLEEPFAIL, | ||||
BO_LOCKPTR(bo)) != 0) { | BO_LOCKPTR(bo)) != 0) { | ||||
BO_LOCK(bo); | BO_LOCK(bo); | ||||
goto loop1; | goto loop1; | ||||
} | } | ||||
BO_LOCK(bo); | BO_LOCK(bo); | ||||
} | } | ||||
BO_UNLOCK(bo); | BO_UNLOCK(bo); | ||||
Done Inline ActionsThis code looks like it ignores holes altogether. If the destination file contains a non-hole and the source file contains a hole, then you need to either hole-punch the destination or at least fill it with zeros. asomers: This code looks like it ignores holes altogether. If the destination file contains a non-hole… | |||||
Done Inline ActionsThanks, good catch. I was thinking of/testing with an empty output file. The code now truncates the output file if the range covers the output file rmacklem: Thanks, good catch. I was thinking of/testing with an empty output file.
The code now… | |||||
KASSERT(bp->b_bufobj == bo, | KASSERT(bp->b_bufobj == bo, | ||||
("bp %p wrong b_bufobj %p should be %p", | ("bp %p wrong b_bufobj %p should be %p", | ||||
bp, bp->b_bufobj, bo)); | bp, bp->b_bufobj, bo)); | ||||
if ((bp->b_flags & B_DELWRI) == 0) | if ((bp->b_flags & B_DELWRI) == 0) | ||||
panic("fsync: not dirty"); | panic("fsync: not dirty"); | ||||
if ((vp->v_object != NULL) && (bp->b_flags & B_CLUSTEROK)) { | if ((vp->v_object != NULL) && (bp->b_flags & B_CLUSTEROK)) { | ||||
vfs_bio_awrite(bp); | vfs_bio_awrite(bp); | ||||
} else { | } else { | ||||
Show All 13 Lines | loop2: | ||||
* retry if dirty blocks still exist. | * retry if dirty blocks still exist. | ||||
*/ | */ | ||||
if (waitfor == MNT_WAIT) { | if (waitfor == MNT_WAIT) { | ||||
bufobj_wwait(bo, 0, 0); | bufobj_wwait(bo, 0, 0); | ||||
if (bo->bo_dirty.bv_cnt > 0) { | if (bo->bo_dirty.bv_cnt > 0) { | ||||
/* | /* | ||||
* If we are unable to write any of these buffers | * If we are unable to write any of these buffers | ||||
* then we fail now rather than trying endlessly | * then we fail now rather than trying endlessly | ||||
* to write them out. | * to write them out. | ||||
Done Inline ActionsThis ignores errors that it shouldn't, like EIO. It should return for any error except EOPNOTSUPP. Better yet, the fallback logic should be moved into vop_stdcopy_file_range and that should be the default method for this vop. asomers: This ignores errors that it shouldn't, like EIO. It should return for any error except… | |||||
Done Inline ActionsWell, I am not sure which errors are going to be recoverable by doing vn_rdwr() in a loop. I have no idea what other errors a non-FreeBSD NFSv4.2 server might return which is If you think fuse (or some future local fs that chooses to implement the VOP) will I don't see putting the "fallback" in the VOP, since it is designed to work across I think working across multiple file systems could be a useful feature. rmacklem: Well, I am not sure which errors are going to be recoverable by doing vn_rdwr() in a loop.
One… | |||||
Done Inline ActionsActually any error VOP_COPY_FILE_RANGE() returns gets returned by the syscall. rmacklem: Actually any error VOP_COPY_FILE_RANGE() returns gets returned by the syscall.
The current code… | |||||
Done Inline ActionsThis ignores errors that it shouldn't, like EIO. It should return for any error except EOPNOTSUPP. Better yet, the fallback logic should be moved into vop_stdcopy_file_range and that should be the default method for this vop. asomers: This ignores errors that it shouldn't, like `EIO`. It should return for any error except… | |||||
*/ | */ | ||||
TAILQ_FOREACH(bp, &bo->bo_dirty.bv_hd, b_bobufs) | TAILQ_FOREACH(bp, &bo->bo_dirty.bv_hd, b_bobufs) | ||||
if ((error = bp->b_error) != 0) | if ((error = bp->b_error) != 0) | ||||
break; | break; | ||||
if ((mp != NULL && mp->mnt_secondary_writes > 0) || | if ((mp != NULL && mp->mnt_secondary_writes > 0) || | ||||
(error == 0 && --maxretry >= 0)) | (error == 0 && --maxretry >= 0)) | ||||
goto loop1; | goto loop1; | ||||
if (error == 0) | if (error == 0) | ||||
Show All 9 Lines |
You can avoid the extra malloc by using the globally defined zero_region (see vm/vm_kern.c). But comparing against that would still needlessly consume CPU cache. Better to write a mem_iszero helper that compares a single buffer against the literal 0.