Re: Bug in the unmounting code

On Fri, Apr 20, 2007 at 06:51:11PM -0700, Maxim Sobolev wrote:
> Hi,
> 
> I have noticed that there is a bug in unmounting code which could make
> filesystem unmountable when its parent filesystem has been forcefully
> unmounted. Following is quick way to reproduce the problem:
> 
> [sobomax_at_pioneer ~]$ sudo mkdir -p /tmp/1/2
> [sobomax_at_pioneer ~]$ sudo mkdir /tmp/3
> [sobomax_at_pioneer ~]$ sudo mount_nullfs /tmp/1 /tmp/3
> [sobomax_at_pioneer ~]$ sudo mount_nullfs /tmp/1 /tmp/3/2
> [sobomax_at_pioneer ~]$ sudo umount -f /tmp/3
> [sobomax_at_pioneer ~]$ sudo mount -v
> /tmp/1 on /tmp/3/2 (nullfs, local, fsid 03ff000202000000)
> [sobomax_at_pioneer ~]$ sudo umount 03ff000202000000
> umount: unmount of /tmp/3/2 failed: No such file or directory
> umount: retrying using path instead of file system ID
> umount: unmount of /tmp/3/2 failed: No such file or directory
> 
> Investigation has revealed that in this case vn_lock() call fails with
> ENOENT due to the following piece of code:
> 
> vn_lock()
> [...]
>                 if (error == 0 && vp->v_iflag & VI_DOOMED &&
>                     (flags & LK_RETRY) == 0) {
>                         VOP_UNLOCK(vp, 0, td);
>                         error = ENOENT;
>                         break;
>                 }
> [...]
> 
> Addition of LK_RETRY flag fixed the problem, but my knowledge of VFS is
> quite limited so that I would appreciate if somebody could verify that
> the fix below won't have any undesirable effects.
> 
> -Maxim
> 
> --- vfs_mount.c 2007/04/21 01:40:53     1.1
> +++ vfs_mount.c 2007/04/21 01:41:09
> _at__at_ -1155,7 +1155,7 _at__at_
>                 mnt_gen_r = mp->mnt_gen;
>                 VI_LOCK(coveredvp);
>                 vholdl(coveredvp);
> -               error = vn_lock(coveredvp, LK_EXCLUSIVE | LK_INTERLOCK, td);
> +               error = vn_lock(coveredvp, LK_EXCLUSIVE | LK_INTERLOCK |
> LK_RETRY, td);
>                 vdrop(coveredvp);
>                 /*
>                  * Check for mp being unmounted while waiting for the

Thank you for tracking this. The regression was introduced by me in rev. 1.232.
There is no need to check for error after vn_lock(LK_RETRY).