Re: [PATCH] Make udf(4) MPSAFE and use shared lookups

On Tue, Dec 02, 2008 at 06:28:36PM -0500, John Baldwin wrote:
> On Saturday 22 November 2008 06:50:28 am Kostik Belousov wrote:
> > udf_vget() does insmntque() before vnode is fully initialized, allowing
> > other threads to find the vnode on the mount list. This is typical for
> > !MPSAFE fs, and it seems corresponding call was not marked XXX for udf.
> 
> It does the same as ufs.  ufs only partially initializes the i-node (as much 
> as both cd9660 and udf do) and then exclusive locks the vnode before 
> insmntque().  They then finish initializing the i-node (bread() the d-node, 
> for example) and finally drop the vnode lock.

My bad, unjustified grumbling.

> 
> > udf_lookup for ISDOTDOT case unlocks dvp before vget'ing "..", allowing
> > the same race on forced unmount as ufs (I will finally commit ufs patch
> > today). The race happens for !MPSAFE code too, but it is easier to
> > execute without Giant.
> 
> Every fs is going to need this workaround it seems.  Would be nice if there 
> was an easier way to avoid cut and pasting this code N times.  Perhaps we 
> could make lookup() check VI_DOOMED instead?  I had changed it do that at one 
> point, but then someone pointed me at the deadfs stuff and said that was 
> sufficient.

The point of the patch is busying mp while parent vnode is locked, that
guarantees that mp is not unmounted during whole DOTDOT traversing in
vop_lookup(). The deadfs stuff works for lookup result vnode and is
sufficient.

The fragment that someone committed into UFS can be extracted into the
vfs support routine. I doubt that it can be embedded into lookup().
The problem is that some filesystems do additional operations inside
vop_lookup().