On Jun 17, 2013, at 5:02 AM, David Holland <dholland-t...@netbsd.org> wrote:
> As has come up many times in the past, we have a long-standing > problem, or family of problems, where in the course of operating on a > particular file system we end up not calling the file system's own > vnode or fs operations, but the root file system's operations. This > causes varying degrees of lossage depending on exactly what happens. > > One of the ways this happens came up today while working on PR 47937. > The vnode structure has both a v_mount field and a v_specmountpoint > field(*); both are pointers to a struct mount (that is, a filesystem) > and it's very easy for people to use the wrong one, or not be sure > which is which. > > (*) v_specmountpoint is not actually a field, it's a fake field > provided by a macro. > > In the short term, to reduce the confusion I would like to make the > following changes: > > 1. Rename v_mount, which is the filesystem the vnode is on (almost > always / for device vnodes), to "v_myfs". What is wrong with this name? `v_mount' is the `struct mount' this vnode resides on and I don't see any confusion. > 2. Rather than renaming v_specmountpoint, which is the filesystem > mounted on a device vnode, kill it off entirely, and replace it > with an inline (or noninline) function "specvn_getmountedfs()". > Make this function assert if called on a non-VBLK vnode instead of > silently producing trash. Yes, please. While here, you could account for spec node aliases like: int spec_node_mount_by_node(vnode_t devvp, struct mount **mpp) { vnode_t *vp; KASSERT(vp->v_type == VBLK); *mpp = NULL; mutex_enter(&device_lock); for (vp = SPECHASH(devvp->v_rdev); vp; vp = vp->v_specnext) { if (vp->v_specmountpoint) { KASSERT(*mpp == NULL); *mpp = vp->v_specmount; #ifndef DIAGNOSTIC break; #endif } } mutex_exit(&device_lock); return 0; } > > This is going to be a fairly large massedit, but I think it's > worthwhile for clarity even if it doesn't turn up any mistakes. > And I expect it to turn up at least a couple howlers. :-/ > > Also, while doing the edit, where feasible stop retrieving the fs > structure from the vnode structure (or other structures) by going > through the device-mounted-on. There should in general be no reason to > do this at all; a filesystem's own structures should all have adequate > pointers to one another without going through the device. > > In the long run, I think we should rearrange the way device vnodes are > handled, so that the device vnodes seen and handled belong entirely to > specfs and the vnodes for the on-disk inodes are stubs hidden behind > the specfs vnodes. This would make it much more obvious when the wrong > fs's ops get called; also it would have the generally beneficial > effect of allowing a lot of rote cut and paste filesystem stuff for > supporting device nodes to be flushed. > > More on that when my ideas solidify a bit. > > -- > David A. Holland > dholl...@netbsd.org -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)