All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: eguan@redhat.com, fstests@vger.kernel.org
Subject: Re: trouble with generic/081
Date: Thu, 15 Dec 2016 17:36:50 +1100	[thread overview]
Message-ID: <20161215063650.GJ4326@dastard> (raw)
In-Reply-To: <20161214164314.GA25105@infradead.org>

On Wed, Dec 14, 2016 at 08:43:14AM -0800, Christoph Hellwig wrote:
> Hi Eryu,
> 
> I'm running into a fairly reproducable issue with generic/081
> (about every other run): For some reason the umount call in
> _cleanup doesn't do anything because it thinks the file system isn't
> mounted, but then vgremove complains that there is a mounted file
> system.  This leads to the scratch device no being release and all
> subsequent tests failing.

Yup, been seeing that on my pmem test setup for months. Reported
along with the subsequent LVM configuration fuckup it resulted in:

https://www.redhat.com/archives/dm-devel/2016-July/msg00405.html

> Here is the output if I let the commands in _cleanup print to stdout:
> 
> QA output created by 081
> Silence is golden
> umount: /mnt/test/mnt_081: not mounted
>   Logical volume vg_081/snap_081 contains a filesystem in use.
>   PV /dev/sdc belongs to Volume Group vg_081 so please use vgreduce first.
> 
> You added a comment in _cleanup that sais:
> 
> # lvm may have umounted it on I/O error, but in case it does not
> 
> Does LVM really unmount filesystems on it's own?  Could we be racing
> with it?

Nope, I'm pretty sure it's a snapshot lifecycle issue - the snapshot
is still busy doing something (probably IO) for a short while after
we unmount, so LVM can't tear it down immediately like we ask. Wait
a few seconds, the snapshot work finishes, goes idle, and then it
can be torn down.

But if you consider the fuckup that occurs if generic/085 starts up
and tries to reconfigure LVM while the snapshot from generic/081 is
still in this whacky window (as reported in the above link), this is
really quite a nasty bug.

> With a "sleep 1" added before the umount call the test passes reliably
> for me, but that seems like papering over the issue.

Yup, same here. My local patch is this:

---
 tests/generic/081 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/generic/081 b/tests/generic/081
index 11755d4d89ff..ff33ffaa4fb8 100755
--- a/tests/generic/081
+++ b/tests/generic/081
@@ -36,6 +36,11 @@ _cleanup()
 	rm -f $tmp.*
 	# lvm may have umounted it on I/O error, but in case it does not
 	$UMOUNT_PROG $mnt >/dev/null 2>&1
+
+	# on a pmem device, the vgremove/pvremove commands fail immediately
+	# after unmount. Wait a bit before removing them in the hope it
+	# succeeds.
+	sleep 5
 	$LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1
 	$LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1
 }

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  parent reply	other threads:[~2016-12-15  6:36 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-14 16:43 trouble with generic/081 Christoph Hellwig
2016-12-15  6:29 ` Eryu Guan
2016-12-15  6:36 ` Dave Chinner [this message]
2016-12-15  8:42   ` Christoph Hellwig
2016-12-15  9:16     ` Zdenek Kabelac
2016-12-16  8:15       ` Christoph Hellwig
2016-12-16  9:31         ` Zdenek Kabelac
2017-01-04 23:03         ` Eric Sandeen
2017-01-05 10:35           ` Zdenek Kabelac
2017-01-05 10:35             ` Zdenek Kabelac
2017-01-05 16:26             ` Mike Snitzer
2017-01-05 17:42               ` Zdenek Kabelac
2017-01-05 17:42                 ` Zdenek Kabelac
2017-01-05 18:07                 ` Mike Snitzer
2017-01-05 18:40                 ` Eric Sandeen
2017-01-05 18:24             ` Eric Sandeen
2017-01-05 18:52               ` Mike Snitzer
2017-01-05 19:13               ` Zdenek Kabelac
2017-01-05 19:29                 ` Eric Sandeen
2017-01-05 21:12                   ` Zdenek Kabelac
2017-01-05 22:03                     ` Eric Sandeen
2017-01-05 22:46                     ` Dave Chinner
2017-01-09 13:39                       ` Christoph Hellwig
2017-01-09 14:22                         ` Zdenek Kabelac
2017-01-09 14:54                           ` Eric Sandeen
2017-01-09 15:11                             ` Zdenek Kabelac
2017-01-10  2:48                               ` Theodore Ts'o
2017-01-10  4:30                             ` Darrick J. Wong
2017-01-09 15:01                           ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161215063650.GJ4326@dastard \
    --to=david@fromorbit.com \
    --cc=eguan@redhat.com \
    --cc=fstests@vger.kernel.org \
    --cc=hch@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.