tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: struct vnode::v_mount access rules



> On 1. Oct 2024, at 18:57, Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost> wrote:
> 
> Under what circumstances is access to struct vnode::v_mount allowed?
> 
> A vnode may be concurrently revoked at any time.  Part of revoking a
> vnode vp, in vcache_reclaim, involves vfs_insmntque to set vp->v_mount
> to dead_rootmount.  So there must be some circumstances under which
> access to v_mount is forbidden -- those circumstances that can race
> with that part of vcache_reclaim.
> 
> Holding the vnode lock makes access to vp->v_mount safe, because
> vcache_reclaim runs with the exclusive vnode lock held.  But
> _acquiring_ the vnode lock first requires reading vp->v_mount inside
> VOP_LOCK, before the vnode lock is actually acquired.
> 
> Is it safe to read between fd_getvnode and fd_putfile, like here?
> 
> int
> do_sys_fstatvfs(struct lwp *l, int fd, int flags, struct statvfs *sb)
> {
> file_t *fp;
> struct mount *mp;
> int error;
> 
> /* fd_getvnode() will use the descriptor for us */
> if ((error = fd_getvnode(fd, &fp)) != 0)
> return (error);
> mp = fp->f_vnode->v_mount;
> error = dostatvfs(mp, sb, curlwp, flags, 1);
> fd_putfile(fd);
> return error;
> }
> 
> I think the answer is no, nothing here precludes a concurrent
> vcache_reclaim from writing to fp->f_vnode->v_mount at the same time
> do_sys_fstatvfs is reading from it.  Although both the before and
> after states are OK for dostatvfs, if this concurrent read/write is
> supposed to be allowed, it should be done with atomic_load/store_* --
> and the same needs to apply to all other use of v_mount that isn't,
> e.g., serialized by the vnode lock.

Always thought while writing a pointer to aligned storage a
concurrent reader will get either the old or the new value but
never gets garbage.  Is this assumption wrong?

--
J. Hannken-Illjes - hannken%mailbox.org@localhost


Home | Main Index | Thread Index | Old Index