All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Safonov <0x7f454c46@gmail.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>,
	Dmitry Safonov <dsafonov@virtuozzo.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>, X86 ML <x86@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	Shuah Khan <shuahkh@osg.samsung.com>,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCHv9 2/2] selftest/x86: add mremap vdso test
Date: Sun, 22 May 2016 08:43:53 +0300	[thread overview]
Message-ID: <CAJwJo6a9j+AcQ4PVhW21V32EW9Ur4XqG8KSuAWPKmMrFhS3fhQ@mail.gmail.com> (raw)
In-Reply-To: <20160521202752.GA31710@gmail.com>

2016-05-21 23:27 GMT+03:00 Ingo Molnar <mingo@kernel.org>:
>
> * Andy Lutomirski <luto@amacapital.net> wrote:
>
>> On Thu, May 19, 2016 at 11:48 PM, Ingo Molnar <mingo@kernel.org> wrote:
>> >
>> > * Dmitry Safonov <dsafonov@virtuozzo.com> wrote:
>> >
>> >> Should print on success:
>> >> [root@localhost ~]# ./test_mremap_vdso_32
>> >>       AT_SYSINFO_EHDR is 0xf773f000
>> >> [NOTE]        Moving vDSO: [f773f000, f7740000] -> [a000000, a001000]
>> >> [OK]
>> >> Or segfault if landing was bad (before patches):
>> >> [root@localhost ~]# ./test_mremap_vdso_32
>> >>       AT_SYSINFO_EHDR is 0xf774f000
>> >> [NOTE]        Moving vDSO: [f774f000, f7750000] -> [a000000, a001000]
>> >> Segmentation fault (core dumped)
>> >
>> > So I still think that generating potential segfaults is not a proper way to test a
>> > new feature. How are we supposed to tell the feature still works? I realize that
>> > glibc is a problem here - but that doesn't really change the QA equation: we are
>> > adding new kernel code to help essentially a single application out of tens of
>> > thousands of applications.
>> >
>> > At minimum we should have a robust testcase ...
>>
>> I think it's robust enough.  It will print "[OK]" and exit with 0 on
>> success and it will crash on failure.  The latter should cause make
>> run_tests to fail reliably.
>
> Indeed, you are right - I somehow mis-read it as potentially segfaulting on fixed
> kernels as well...
>
> Will look at applying this after the merge window.

Great! Thanks, Ingo - maybe I should have wrote test's patch description better.
Thanks again, Andy.

WARNING: multiple messages have this Message-ID (diff)
From: Dmitry Safonov <0x7f454c46@gmail.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>,
	Dmitry Safonov <dsafonov@virtuozzo.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>, X86 ML <x86@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	Shuah Khan <shuahkh@osg.samsung.com>,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCHv9 2/2] selftest/x86: add mremap vdso test
Date: Sun, 22 May 2016 08:43:53 +0300	[thread overview]
Message-ID: <CAJwJo6a9j+AcQ4PVhW21V32EW9Ur4XqG8KSuAWPKmMrFhS3fhQ@mail.gmail.com> (raw)
In-Reply-To: <20160521202752.GA31710@gmail.com>

2016-05-21 23:27 GMT+03:00 Ingo Molnar <mingo@kernel.org>:
>
> * Andy Lutomirski <luto@amacapital.net> wrote:
>
>> On Thu, May 19, 2016 at 11:48 PM, Ingo Molnar <mingo@kernel.org> wrote:
>> >
>> > * Dmitry Safonov <dsafonov@virtuozzo.com> wrote:
>> >
>> >> Should print on success:
>> >> [root@localhost ~]# ./test_mremap_vdso_32
>> >>       AT_SYSINFO_EHDR is 0xf773f000
>> >> [NOTE]        Moving vDSO: [f773f000, f7740000] -> [a000000, a001000]
>> >> [OK]
>> >> Or segfault if landing was bad (before patches):
>> >> [root@localhost ~]# ./test_mremap_vdso_32
>> >>       AT_SYSINFO_EHDR is 0xf774f000
>> >> [NOTE]        Moving vDSO: [f774f000, f7750000] -> [a000000, a001000]
>> >> Segmentation fault (core dumped)
>> >
>> > So I still think that generating potential segfaults is not a proper way to test a
>> > new feature. How are we supposed to tell the feature still works? I realize that
>> > glibc is a problem here - but that doesn't really change the QA equation: we are
>> > adding new kernel code to help essentially a single application out of tens of
>> > thousands of applications.
>> >
>> > At minimum we should have a robust testcase ...
>>
>> I think it's robust enough.  It will print "[OK]" and exit with 0 on
>> success and it will crash on failure.  The latter should cause make
>> run_tests to fail reliably.
>
> Indeed, you are right - I somehow mis-read it as potentially segfaulting on fixed
> kernels as well...
>
> Will look at applying this after the merge window.

Great! Thanks, Ingo - maybe I should have wrote test's patch description better.
Thanks again, Andy.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2016-05-22  5:44 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-17 12:13 [PATCHv9 0/2] mremap vDSO for 32-bit Dmitry Safonov
2016-05-17 12:13 ` Dmitry Safonov
2016-05-17 12:13 ` [PATCHv9 1/2] x86/vdso: add mremap hook to vm_special_mapping Dmitry Safonov
2016-05-17 12:13   ` Dmitry Safonov
2016-05-17 12:13 ` [PATCHv9 2/2] selftest/x86: add mremap vdso test Dmitry Safonov
2016-05-17 12:13   ` Dmitry Safonov
2016-05-20  6:48   ` Ingo Molnar
2016-05-20  6:48     ` Ingo Molnar
2016-05-20 15:33     ` Andy Lutomirski
2016-05-20 15:33       ` Andy Lutomirski
2016-05-21 20:27       ` Ingo Molnar
2016-05-21 20:27         ` Ingo Molnar
2016-05-22  5:43         ` Dmitry Safonov [this message]
2016-05-22  5:43           ` Dmitry Safonov
2016-06-08 11:41         ` Dmitry Safonov
2016-06-08 11:41           ` Dmitry Safonov
2016-06-17  8:03   ` Ingo Molnar
2016-06-17  8:03     ` Ingo Molnar
2016-06-17  9:24     ` Dmitry Safonov
2016-06-17  9:24       ` Dmitry Safonov
2016-05-19  9:50 ` [PATCHv9 0/2] mremap vDSO for 32-bit Dmitry Safonov
2016-05-19  9:50   ` Dmitry Safonov

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=CAJwJo6a9j+AcQ4PVhW21V32EW9Ur4XqG8KSuAWPKmMrFhS3fhQ@mail.gmail.com \
    --to=0x7f454c46@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=dsafonov@virtuozzo.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@amacapital.net \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=shuahkh@osg.samsung.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.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.