Re: unmount mountpoints

[Alexander Bluhm]

On Tue, Jan 10, 2017 at 09:46:30PM +0100, Alexander Bluhm wrote:
> When I force to unmount a filesystem where another mountpoint is
> located, an unlinked mountpoint will remain.  I have not found a
> way to restore the kernel to a sane state.
> 
> ok?

anyone?

bluhm

Index: kern/vfs_syscalls.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.268
diff -u -p -r1.268 vfs_syscalls.c
--- kern/vfs_syscalls.c 15 Jan 2017 23:18:05 -  1.268
+++ kern/vfs_syscalls.c 16 Jan 2017 00:01:54 -
@@ -88,6 +88,8 @@ int domkdirat(struct proc *, int, const 
 int doutimensat(struct proc *, int, const char *, struct timespec [2], int);
 int dovutimens(struct proc *, struct vnode *, struct timespec [2]);
 int dofutimens(struct proc *, int, struct timespec [2]);
+int dounmount_leaf(struct mount *, int, struct proc *);
+int unmount_vnode(struct vnode *, void *);
 
 /*
  * Virtual File System System Calls
@@ -368,11 +370,67 @@ sys_unmount(struct proc *p, void *v, reg
return (dounmount(mp, SCARG(uap, flags) & MNT_FORCE, p));
 }
 
+struct unmount_args {
+   TAILQ_HEAD(ua_mphead, mount)ua_mplist;
+   int ua_flags;
+};
+
+int
+unmount_vnode(struct vnode *vp, void *args)
+{
+   struct unmount_args *ua = args;
+   struct mount *mp;
+
+   if (vp->v_type != VDIR)
+   return (0);
+   if ((mp = vp->v_mountedhere) == NULL)
+   return (0);
+   if (!(ua->ua_flags & MNT_FORCE))
+   return (EBUSY);
+   if (vfs_busy(mp, VB_WRITE|VB_WAIT))
+   return ((ua->ua_flags & MNT_DOOMED) ? 0 : EBUSY);
+
+   TAILQ_INSERT_HEAD(>ua_mplist, mp, mnt_dounmount);
+
+   return (0);
+}
+
 /*
  * Do the actual file system unmount.
  */
 int
 dounmount(struct mount *mp, int flags, struct proc *p)
+{
+   struct unmount_args ua;
+   int error;
+
+   TAILQ_INIT(_mplist);
+   TAILQ_INSERT_HEAD(_mplist, mp, mnt_dounmount);
+   ua.ua_flags = flags;
+   TAILQ_FOREACH_REVERSE(mp, _mplist, ua_mphead, mnt_dounmount) {
+   error = vfs_mount_foreach_vnode(mp, unmount_vnode, );
+   if (error)
+   goto err;
+   }
+
+   while ((mp = TAILQ_FIRST(_mplist))) {
+   TAILQ_REMOVE(_mplist, mp, mnt_dounmount);
+   error = dounmount_leaf(mp, flags, p);
+   if (error)
+   goto err;
+   }
+   return (0);
+
+ err:
+   while ((mp = TAILQ_FIRST(_mplist))) {
+   TAILQ_REMOVE(_mplist, mp, mnt_dounmount);
+   vfs_unbusy(mp);
+   }
+   return (error);
+}
+
+int
+dounmount_leaf(struct mount *mp, int flags, struct proc *p)
 {
struct vnode *coveredvp;
int error;
Index: sys/mount.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mount.h,v
retrieving revision 1.128
diff -u -p -r1.128 mount.h
--- sys/mount.h 10 Jan 2017 19:48:32 -  1.128
+++ sys/mount.h 16 Jan 2017 00:01:54 -
@@ -347,6 +347,7 @@ LIST_HEAD(vnodelst, vnode);
 
 struct mount {
TAILQ_ENTRY(mount) mnt_list;/* mount list */
+   TAILQ_ENTRY(mount) mnt_dounmount;   /* unmount work queue */
const struct vfsops *mnt_op;/* operations on fs */
struct vfsconf  *mnt_vfc;   /* configuration info */
struct vnode*mnt_vnodecovered;  /* vnode we mounted on */

Re: unmount mountpoints

[Alexander Bluhm]

On Tue, Jan 10, 2017 at 09:46:30PM +0100, Alexander Bluhm wrote:
> The fix could be to unmount recursively.

Recursion in the kernel is bad.  Root could create deeply nested
mount points and hit the stack limit when unmounting the tree.

I have converted the recursion to a list.

ok?

bluhm

Index: kern/vfs_syscalls.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.267
diff -u -p -r1.267 vfs_syscalls.c
--- kern/vfs_syscalls.c 10 Jan 2017 20:13:17 -  1.267
+++ kern/vfs_syscalls.c 11 Jan 2017 20:00:02 -
@@ -88,6 +88,8 @@ int domkdirat(struct proc *, int, const 
 int doutimensat(struct proc *, int, const char *, struct timespec [2], int);
 int dovutimens(struct proc *, struct vnode *, struct timespec [2]);
 int dofutimens(struct proc *, int, struct timespec [2]);
+int dounmount_leaf(struct mount *, int, struct proc *);
+int unmount_vnode(struct vnode *, void *);
 
 /*
  * Virtual File System System Calls
@@ -368,11 +370,67 @@ sys_unmount(struct proc *p, void *v, reg
return (dounmount(mp, SCARG(uap, flags) & MNT_FORCE, p));
 }
 
+struct unmount_args {
+   TAILQ_HEAD(ua_mphead, mount)ua_mplist;
+   int ua_flags;
+};
+
+int
+unmount_vnode(struct vnode *vp, void *args)
+{
+   struct unmount_args *ua = args;
+   struct mount *mp;
+
+   if (vp->v_type != VDIR)
+   return (0);
+   if ((mp = vp->v_mountedhere) == NULL)
+   return (0);
+   if (!(ua->ua_flags & MNT_FORCE))
+   return (EBUSY);
+   if (vfs_busy(mp, VB_WRITE|VB_WAIT))
+   return ((ua->ua_flags & MNT_DOOMED) ? 0 : EBUSY);
+
+   TAILQ_INSERT_HEAD(>ua_mplist, mp, mnt_dounmount);
+
+   return (0);
+}
+
 /*
  * Do the actual file system unmount.
  */
 int
 dounmount(struct mount *mp, int flags, struct proc *p)
+{
+   struct unmount_args ua;
+   int error;
+
+   TAILQ_INIT(_mplist);
+   TAILQ_INSERT_HEAD(_mplist, mp, mnt_dounmount);
+   ua.ua_flags = flags;
+   TAILQ_FOREACH_REVERSE(mp, _mplist, ua_mphead, mnt_dounmount) {
+   error = vfs_mount_foreach_vnode(mp, unmount_vnode, );
+   if (error)
+   goto err;
+   }
+
+   while ((mp = TAILQ_FIRST(_mplist))) {
+   TAILQ_REMOVE(_mplist, mp, mnt_dounmount);
+   error = dounmount_leaf(mp, flags, p);
+   if (error)
+   goto err;
+   }
+   return (0);
+
+ err:
+   while ((mp = TAILQ_FIRST(_mplist))) {
+   TAILQ_REMOVE(_mplist, mp, mnt_dounmount);
+   vfs_unbusy(mp);
+   }
+   return (error);
+}
+
+int
+dounmount_leaf(struct mount *mp, int flags, struct proc *p)
 {
struct vnode *coveredvp;
int error;
Index: sys/mount.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mount.h,v
retrieving revision 1.128
diff -u -p -r1.128 mount.h
--- sys/mount.h 10 Jan 2017 19:48:32 -  1.128
+++ sys/mount.h 11 Jan 2017 19:45:34 -
@@ -347,6 +347,7 @@ LIST_HEAD(vnodelst, vnode);
 
 struct mount {
TAILQ_ENTRY(mount) mnt_list;/* mount list */
+   TAILQ_ENTRY(mount) mnt_dounmount;   /* unmount work queue */
const struct vfsops *mnt_op;/* operations on fs */
struct vfsconf  *mnt_vfc;   /* configuration info */
struct vnode*mnt_vnodecovered;  /* vnode we mounted on */

Re: unmount mountpoints

[Alexander Bluhm]

On Wed, Jan 11, 2017 at 10:20:17AM -0800, patrick keshishian wrote:
> does
> 
> # umount /dev/vnd1a
> 
> not do the trick?

No.

I have written a test to create the problem.

# cd /usr/src/regress/sys/kern/mount
# make
...
# mount
/dev/sd0a on / type ffs (local)
/dev/vnd0b on /mnt/b type ffs (local)
/dev/vnd0d on /mnt/d type ffs (local)
/dev/vnd0e on /mnt/d/e type ffs (local)
/dev/vnd0f on /mnt/d/e/f type ffs (local)
/dev/vnd0g on /mnt/d/e/f/g type ffs (local)
/dev/vnd0h on /mnt/d/e/f/g/h type ffs (local)
/dev/vnd0i on /mnt/d/e/f/g/h/i type ffs (local)
/dev/vnd0j on /mnt/d/e/f/g/h/i/j type ffs (local)
/dev/vnd0k on /mnt/d/e/f/g/h/i/j/k type ffs (local)
/dev/vnd0l on /mnt/d/e/f/g/h/i/j/k/l type ffs (local)
/dev/vnd0m on /mnt/d/e/f/g/h/i/j/k/l/m type ffs (local)
/dev/vnd0n on /mnt/d/e/f/g/h/i/j/k/l/m/n type ffs (local)
/dev/vnd0o on /mnt/d/e/f/g/h/i/j/k/l/m/n/o type ffs (local)
/dev/vnd0p on /mnt/d/e/f/g/h/i/j/k/l/m/n/o/p type ffs (local)

Depening in wether /mnt/? exists you get:

# umount /dev/vnd0b
umount: /mnt/b: Invalid argument

# umount /dev/vnd0d
umount: /mnt/d: No such file or directory

bluhm

Re: unmount mountpoints

[patrick keshishian]

my initial attempt to send a response is not moving out of the
queue...so here is a second attempt.

On 1/10/17, Alexander Bluhm  wrote:
> Hi,
>
> When I force to unmount a filesystem where another mountpoint is
> located, an unlinked mountpoint will remain.  I have not found a
> way to restore the kernel to a sane state.
>
> # mount /dev/vnd0a /mnt
> # mkdir /mnt/mnt
> # mount /dev/vnd1a /mnt/mnt
> # mount
> /dev/sd0a on / type ffs (local)
> /dev/vnd0a on /mnt type ffs (local)
> /dev/vnd1a on /mnt/mnt type ffs (local)
> # umount /mnt
> umount: /mnt: Device busy
> # umount -f /mnt
> # mount
> /dev/sd0a on / type ffs (local)
> /dev/vnd1a on /mnt/mnt type ffs (local)
> # umount /mnt/mnt
> umount: /mnt/mnt: No such file or directory

does

# umount /dev/vnd1a

not do the trick?

--patrick


> # mkdir /mnt/mnt
> # umount /mnt/mnt
> umount: /mnt/mnt: Invalid argument
> # vnconfig -u vnd1
> vnconfig: VNDIOCCLR: Device busy
>
> The fix could be to unmount recursively.
>
> ok?
>
> bluhm
>
> Index: kern/vfs_syscalls.c
> ===
> RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
> retrieving revision 1.267
> diff -u -p -r1.267 vfs_syscalls.c
> --- kern/vfs_syscalls.c   10 Jan 2017 20:13:17 -  1.267
> +++ kern/vfs_syscalls.c   10 Jan 2017 20:36:05 -
> @@ -88,6 +88,7 @@ int domkdirat(struct proc *, int, const
>  int doutimensat(struct proc *, int, const char *, struct timespec [2],
> int);
>  int dovutimens(struct proc *, struct vnode *, struct timespec [2]);
>  int dofutimens(struct proc *, int, struct timespec [2]);
> +int unmount_vnode(struct vnode *, void *);
>
>  /*
>   * Virtual File System System Calls
> @@ -368,15 +369,56 @@ sys_unmount(struct proc *p, void *v, reg
>   return (dounmount(mp, SCARG(uap, flags) & MNT_FORCE, p));
>  }
>
> +struct unmount_args {
> + struct proc *ua_proc;
> + int  ua_flags;
> + int  ua_error;
> +};
> +
> +int
> +unmount_vnode(struct vnode *vp, void *args)
> +{
> + struct unmount_args *ua = args;
> + struct mount *mp;
> + int error;
> +
> + if (vp->v_type != VDIR)
> + return (0);
> + if ((mp = vp->v_mountedhere) == NULL)
> + return (0);
> + if (!(ua->ua_flags & MNT_FORCE)) {
> + ua->ua_error = EBUSY;
> + return (EBUSY);
> + }
> + if (vfs_busy(mp, VB_WRITE|VB_WAIT)) {
> + ua->ua_error = EBUSY;
> + return (0);
> + }
> + error = dounmount(mp, ua->ua_flags, ua->ua_proc);
> + if (error)
> + ua->ua_error = error;
> + return (0);
> +}
> +
>  /*
>   * Do the actual file system unmount.
>   */
>  int
>  dounmount(struct mount *mp, int flags, struct proc *p)
>  {
> + struct unmount_args ua;
>   struct vnode *coveredvp;
>   int error;
>   int hadsyncer = 0;
> +
> + ua.ua_proc = p;
> + ua.ua_flags = flags;
> + ua.ua_error = 0;
> + vfs_mount_foreach_vnode(mp, unmount_vnode, );
> + if (ua.ua_error && !(flags & MNT_DOOMED)) {
> + vfs_unbusy(mp);
> + return (ua.ua_error);
> + }
>
>   mp->mnt_flag &=~ MNT_ASYNC;
>   cache_purgevfs(mp); /* remove cache entries for this file sys */
>
>