qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Roman Kagan <rkagan@virtuozzo.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	"berrange@redhat.com" <berrange@redhat.com>,
	Denis Lunev <den@virtuozzo.com>,
	"qemu-block@nongnu.org" <qemu-block@nongnu.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"mreitz@redhat.com" <mreitz@redhat.com>,
	Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: Re: [Qemu-devel] [PATCH v2 2/7] iotests: exclude killed processes from running under Valgrind
Date: Mon, 17 Jun 2019 13:20:47 +0000	[thread overview]
Message-ID: <20190617132043.GE32624@rkaganb.sw.ru> (raw)
In-Reply-To: <20190617125355.GH7397@linux.fritz.box>

On Mon, Jun 17, 2019 at 02:53:55PM +0200, Kevin Wolf wrote:
> Am 17.06.2019 um 14:18 hat Roman Kagan geschrieben:
> > On Mon, Jun 17, 2019 at 01:15:04PM +0200, Kevin Wolf wrote:
> > > Am 11.06.2019 um 20:02 hat Andrey Shinkevich geschrieben:
> > > > The Valgrind tool fails to manage its termination when QEMU raises the
> > > > signal SIGKILL. Lets exclude such test cases from running under the
> > > > Valgrind because there is no sense to check memory issues that way.
> > > > 
> > > > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> > > 
> > > I don't fully understand the reasoning here. Most interesting memory
> > > access errors happen before a process terminates. (I'm not talking about
> > > leaks here, but use-after-free, buffer overflows, uninitialised memory
> > > etc.)
> > 
> > Nothing of the above, and nothing in general, happens in the usermode
> > process upon SIGKILL delivery.
> 
> My point is, the interesting part is what the program does before
> SIGKILL happens. There is value in reporting memory errors as long as we
> can, even if the final check doesn't happen because of SIGKILL.

Agreed in general, but here the testcases that include 'sigraise 9' only
do simple operations before that which are covered elsewhere too.  So
the extra effort on making valgrind work with these testcases arguably
isn't worth the extra value to be gained.

> > > However, I do see that running these test cases with -valgrind ends in a
> > > hang because the valgrind process keeps hanging around as a zombie
> > > process and the test case doesn't reap it. I'm not exactly sure why that
> > > is, but it looks more like a problem with the parent process (i.e. the
> > > bash script).
> > 
> > It rather looks like valgrind getting confused about what to do with
> > raise(SIGKILL) in the multithreaded case.
> 
> Well, valgrind can't do anything with SIGKILL, obviously, because it's
> killed immediately.

Right, but it can do whatever it wants with raise(SIGKILL).  I haven't
looked at valgrind sources, but

  # strace -ff valgind qemu-io -c 'sigraise 9'

shows SIGKILL neither sent nor received by any thread; it just shows the
main thread exit and the second thread getting stuck waiting on a futex.

> But maybe the kernel does get confused for some
> reason. I get the main threads as a zombie, but a second is still
> running. Sending SIGKILL to the second thread, too, makes the test case
> complete successfully.
> 
> So I guess the main question is why the second thread isn't
> automatically killed when the main thread receives SIGKILL.

I don't see any thread receive SIGKILL.  So I tend to think this is
valgrind's bug/feature.

Anyway the problem is outside of QEMU, so I think we need to weigh the
costs of investigating it and implementing a workaround with the
potential benefit.

Roman.


  reply	other threads:[~2019-06-17 13:21 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-11 18:02 [Qemu-devel] [PATCH v2 0/7] Allow Valgrind checking all QEMU processes Andrey Shinkevich
2019-06-11 18:02 ` [Qemu-devel] [PATCH v2 1/7] iotests: allow " Andrey Shinkevich
2019-06-13  9:44   ` Vladimir Sementsov-Ogievskiy
2019-06-27 15:08     ` Andrey Shinkevich
2019-06-13  9:45   ` Vladimir Sementsov-Ogievskiy
2019-06-11 18:02 ` [Qemu-devel] [PATCH v2 2/7] iotests: exclude killed processes from running under Valgrind Andrey Shinkevich
2019-06-13  9:47   ` Vladimir Sementsov-Ogievskiy
2019-06-17 11:57     ` Roman Kagan
2019-06-17 11:15   ` Kevin Wolf
2019-06-17 12:18     ` Roman Kagan
2019-06-17 12:53       ` Kevin Wolf
2019-06-17 13:20         ` Roman Kagan [this message]
2019-06-17 14:51           ` Kevin Wolf
2019-06-24 16:55             ` Andrey Shinkevich
2019-06-11 18:02 ` [Qemu-devel] [PATCH v2 3/7] iotests: Valgrind fails to work with nonexistent directory Andrey Shinkevich
2019-06-13  9:52   ` Vladimir Sementsov-Ogievskiy
2019-06-17 11:22     ` Kevin Wolf
2019-06-11 18:02 ` [Qemu-devel] [PATCH v2 4/7] iotests: extended timeout under Valgrind Andrey Shinkevich
2019-06-13  9:54   ` Vladimir Sementsov-Ogievskiy
2019-06-11 18:02 ` [Qemu-devel] [PATCH v2 5/7] iotests: extend sleeping time " Andrey Shinkevich
2019-06-13  9:55   ` Vladimir Sementsov-Ogievskiy
2019-06-11 18:02 ` [Qemu-devel] [PATCH v2 6/7] iotests: amend QEMU NBD process synchronization Andrey Shinkevich
2019-06-13  9:59   ` Vladimir Sementsov-Ogievskiy
2019-06-17 12:45     ` Roman Kagan
2019-06-17 12:38   ` Roman Kagan
2019-06-11 18:02 ` [Qemu-devel] [PATCH v2 7/7] iotests: new file to suppress Valgrind errors Andrey Shinkevich
2019-06-13 10:06   ` Vladimir Sementsov-Ogievskiy
2019-06-24 16:55     ` Andrey Shinkevich
2019-06-24 17:09       ` Eric Blake
2019-06-24 17:23         ` Andrey Shinkevich
2019-06-17 11:45   ` Kevin Wolf
2019-06-24 16:55     ` Andrey Shinkevich
2019-06-25  8:13       ` Kevin Wolf

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=20190617132043.GE32624@rkaganb.sw.ru \
    --to=rkagan@virtuozzo.com \
    --cc=andrey.shinkevich@virtuozzo.com \
    --cc=berrange@redhat.com \
    --cc=den@virtuozzo.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@virtuozzo.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).