Changeset View
Standalone View
sys/kern/vfs_default.c
Show First 20 Lines • Show All 417 Lines • ▼ Show 20 Lines | |||||
/* | /* | ||||
* Advisory record locking support | * Advisory record locking support | ||||
*/ | */ | ||||
int | int | ||||
vop_stdadvlock(struct vop_advlock_args *ap) | vop_stdadvlock(struct vop_advlock_args *ap) | ||||
{ | { | ||||
struct vnode *vp; | struct vnode *vp; | ||||
struct mount *mp; | |||||
struct vattr vattr; | struct vattr vattr; | ||||
int error; | int error; | ||||
vp = ap->a_vp; | vp = ap->a_vp; | ||||
/* | |||||
* Provide atomicity of open(O_CREAT | O_EXCL | O_EXLOCK) for | |||||
* local filesystems. See vn_open_cred() for reciprocal part. | |||||
*/ | |||||
mp = vp->v_mount; | |||||
markj: So what about filesystems that do not use the default implementation? Should this be done in a… | |||||
Done Inline ActionsNo I do not think so. The idea of atomicity for O_EXCL|O_EXLOCK does not make sense for remote filesystems, because we cannot enforce anything on protocol, and they are exactly the filesystems that use non-default vop_advlock implementation. This is why the comment about local filesystems in the commit message. kib: No I do not think so. The idea of atomicity for O_EXCL|O_EXLOCK does not make sense for remote… | |||||
Not Done Inline ActionsThere is also FUSE. The latter falls back to stdadvlock in some cases, not sure if it coincides exactly with remote vs. local. markj: There is also FUSE. The latter falls back to stdadvlock in some cases, not sure if it coincides… | |||||
Not Done Inline ActionsI would suggest either putting a KASSERT that MNT_LOCAL is set or add the MNT_LOCAL test if FUSE may use it on a non-local filesystem. mckusick: I would suggest either putting a KASSERT that MNT_LOCAL is set or add the MNT_LOCAL test if… | |||||
Done Inline ActionsIs this an unlocked load? markj: Is this an unlocked load? | |||||
Done Inline ActionsYes, it is fine. The flag is never changed after mount. kib: Yes, it is fine. The flag is never changed after mount. | |||||
Not Done Inline ActionsI meant, is it guaranteed that the reference to mp itself is safe? Mount points are type-stable at least, but the flags value may be stale, unless I misunderstood. It looks harmless but might deserve a comment, again assuming I did not misunderstand. markj: I meant, is it guaranteed that the reference to `mp` itself is safe? Mount points are type… | |||||
Done Inline ActionsIn the worst case of unmount/momental mp reuse, we might see undeserved MNT_LOCAL and wait for the passing open when we should not. Or we might not see MNT_LOCAL and then at unmount it would happen that lock is obtained while open is still processing. But in this case, open(2) would return a file descriptor referencing reclaimed vnode, otherwise mp cannot be reused. So no future operations with it is possible. kib: In the worst case of unmount/momental mp reuse, we might see undeserved MNT_LOCAL and wait for… | |||||
if (mp != NULL && (mp->mnt_flag & MNT_LOCAL) != 0 && | |||||
ap->a_op == F_SETLK && (ap->a_flags & F_FIRSTOPEN) == 0) { | |||||
VI_LOCK(vp); | |||||
while ((vp->v_iflag & VI_FOPENING) != 0) | |||||
msleep(vp, VI_MTX(vp), PLOCK, "lockfo", 0); | |||||
VI_UNLOCK(vp); | |||||
} | |||||
if (ap->a_fl->l_whence == SEEK_END) { | if (ap->a_fl->l_whence == SEEK_END) { | ||||
/* | /* | ||||
* The NFSv4 server must avoid doing a vn_lock() here, since it | * The NFSv4 server must avoid doing a vn_lock() here, since it | ||||
* can deadlock the nfsd threads, due to a LOR. Fortunately | * can deadlock the nfsd threads, due to a LOR. Fortunately | ||||
* the NFSv4 server always uses SEEK_SET and this code is | * the NFSv4 server always uses SEEK_SET and this code is | ||||
* only required for the SEEK_END case. | * only required for the SEEK_END case. | ||||
*/ | */ | ||||
vn_lock(vp, LK_SHARED | LK_RETRY); | vn_lock(vp, LK_SHARED | LK_RETRY); | ||||
▲ Show 20 Lines • Show All 1,172 Lines • Show Last 20 Lines |
So what about filesystems that do not use the default implementation? Should this be done in a vop_advlock_pre() routine?