umount of unreachable NFS without -f [maybe PATCH]

umount of unreachable NFS without -f [maybe PATCH]

[Stanislav Brabec]

I just analyzed causes of hang while calling umount without -f on an
unreachable NFS.

The first hang happens on in lookup_umount_fs():

stat(tgt, &st) == 0 && S_ISDIR(st.st_mode)

I don't exactly know, what its removal causes, but I tried it.

I entered into next hang in mnt_resolve_path() inside has_utab_entry().
We cannot prevent this hang in all cases, but preventing it in a
standard situation is pretty straightforward: Try to match provided
target name before trying to canonicalize.

Now my umount without -f works well at least in the case, when the mount
point is recorded in the utab.

If it is not recorded in utab, it still hangs later in
lookup_umount_fs() on:

if (statfs(tgt, &vfs) == 0)

Well, we can prevent even this hang, but it would be at cost of parsing
mtab. I read your git comments and I see, that the whole purpose of the
code is preventing of parsing of the generated mtab/mountinfo.

Would be parsing of fstab a safe way without paying this cost?

Here is my patch, which prevented at least part of these hangs. What do
you think about it?

Index: util-linux-2.25/libmount/src/context_umount.c
===================================================================
--- util-linux-2.25.orig/libmount/src/context_umount.c
+++ util-linux-2.25/libmount/src/context_umount.c
@@ -216,6 +255,14 @@ static int has_utab_entry(struct libmnt_
 
 	/* paths in utab are canonicalized */
 	cache = mnt_context_get_cache(cxt);
+
+	/* first search for target as it is - mnt_resolve_path() can fail on unreachable NFS */
+	mnt_reset_iter(&itr, MNT_ITER_BACKWARD);
+
+	while (mnt_table_next_fs(cxt->utab, &itr, &fs) == 0) {
+		if (mnt_fs_streq_target(fs, target))
+			return 1;
+	}
 	cn = mnt_resolve_path(target, cache);
 	mnt_reset_iter(&itr, MNT_ITER_BACKWARD);
 
@@ -232,7 +279,6 @@ static int has_utab_entry(struct libmnt_
 static int lookup_umount_fs(struct libmnt_context *cxt)
 {
 	const char *tgt;
-	struct stat st;
 	struct libmnt_fs *fs = NULL;
 	int rc = 0;
 
@@ -262,7 +308,6 @@ static int lookup_umount_fs(struct libmn
 	    && !mnt_context_mtab_writable(cxt)
 	    && !mnt_context_is_force(cxt)
 	    && !mnt_context_is_lazy(cxt)
-	    && stat(tgt, &st) == 0 && S_ISDIR(st.st_mode)
 	    && !has_utab_entry(cxt, tgt)) {
 
 		const char *type = mnt_fs_get_fstype(cxt->fs);


-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                          e-mail: sbrabec@suse.cz
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

Re: umount of unreachable NFS without -f [maybe PATCH]

[Karel Zak]

On Tue, Sep 02, 2014 at 09:09:38PM +0200, Stanislav Brabec wrote:
> I just analyzed causes of hang while calling umount without -f on an
> unreachable NFS.
> 
> The first hang happens on in lookup_umount_fs():
> 
> stat(tgt, &st) == 0 && S_ISDIR(st.st_mode)
> 
> I don't exactly know, what its removal causes, but I tried it.
> 
> I entered into next hang in mnt_resolve_path() inside has_utab_entry().
> We cannot prevent this hang in all cases, but preventing it in a
> standard situation is pretty straightforward: Try to match provided
> target name before trying to canonicalize.
> 
> Now my umount without -f works well at least in the case, when the mount
> point is recorded in the utab.
> 
> If it is not recorded in utab, it still hangs later in
> lookup_umount_fs() on:
> 
> if (statfs(tgt, &vfs) == 0)
> 
> Well, we can prevent even this hang, but it would be at cost of parsing
> mtab. I read your git comments and I see, that the whole purpose of the
> code is preventing of parsing of the generated mtab/mountinfo.
> 
> Would be parsing of fstab a safe way without paying this cost?

How often do you have NFS mounts without entry within utab? I think it's
pretty unusual (I guess you have mount.nfs linked with libmount).

The change in has_utab_entry() is definitely good idea, but I'm not
sure about the rest. Maybe it would be enough to call has_utab_entry()
before

    stat(tgt, &st) == 0 && S_ISDIR(st.st_mode)


then it will be good enough for almost all NFS mounts (with utab
entries).

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: umount of unreachable NFS without -f [maybe PATCH]

[Karel Zak]

On Tue, Sep 02, 2014 at 09:09:38PM +0200, Stanislav Brabec wrote:
> I just analyzed causes of hang while calling umount without -f on an
> unreachable NFS.
> 
> The first hang happens on in lookup_umount_fs():
> 
> stat(tgt, &st) == 0 && S_ISDIR(st.st_mode)
> 
> I don't exactly know, what its removal causes, but I tried it.

The stat() is there to detect situations when someone calls

  umount /dev/sda1

then you don't want to call statfs() to get fs type, because the
device is not a mountpoint. Note that statfs() works with arbitrary
file, so we have to be careful...

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: umount of unreachable NFS without -f [maybe PATCH]

[Stanislav Brabec]

Karel Zak wrote:
> On Tue, Sep 02, 2014 at 09:09:38PM +0200, Stanislav Brabec wrote:
> > I just analyzed causes of hang while calling umount without -f on an
> > unreachable NFS.
> > 
> > The first hang happens on in lookup_umount_fs():
> > 
> > stat(tgt, &st) == 0 && S_ISDIR(st.st_mode)
> > 
> > I don't exactly know, what its removal causes, but I tried it.
> > 
> > I entered into next hang in mnt_resolve_path() inside has_utab_entry().
> > We cannot prevent this hang in all cases, but preventing it in a
> > standard situation is pretty straightforward: Try to match provided
> > target name before trying to canonicalize.
> > 
> > Now my umount without -f works well at least in the case, when the mount
> > point is recorded in the utab.
> > 
> > If it is not recorded in utab, it still hangs later in
> > lookup_umount_fs() on:
> > 
> > if (statfs(tgt, &vfs) == 0)
> > 
> > Well, we can prevent even this hang, but it would be at cost of parsing
> > mtab. I read your git comments and I see, that the whole purpose of the
> > code is preventing of parsing of the generated mtab/mountinfo.
> > 
> > Would be parsing of fstab a safe way without paying this cost?
> 
> How often do you have NFS mounts without entry within utab? I think it's
> pretty unusual (I guess you have mount.nfs linked with libmount).

I did it somehow. Guessing that it could have something with my
experiments, killing hanging umount with signal 11 etc.

> The change in has_utab_entry() is definitely good idea, but I'm not
> sure about the rest. Maybe it would be enough to call has_utab_entry()
> before
> 
>     stat(tgt, &st) == 0 && S_ISDIR(st.st_mode)
> 
> 
> then it will be good enough for almost all NFS mounts (with utab
> entries).

Yes, I just verified that it helps as well.

But I found just another stat() call further in
mnt_context_find_umount_fs():
if (stat(tgt, &st) == 0 && S_ISDIR(st.st_mode)) {
before cache lookups.

And next there is:
cn_tgt = mnt_resolve_path(tgt, cache);
which will probably hang as well.

I can imagine that we will not uses stat() here, but I cannot imagine,
how to set filter correctly without canonicalization.

I am just curious why one of my umount attempts of unresponsive NFS
succeeded yesterday...

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                          e-mail: sbrabec@suse.cz
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76