linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brendan Higgins <brendanhiggins@google.com>
To: Alan Maguire <alan.maguire@oracle.com>,
	Kees Cook <keescook@chromium.org>
Cc: "open list:KERNEL SELFTEST FRAMEWORK" 
	<linux-kselftest@vger.kernel.org>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Iurii Zaikin <yzaikin@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Masahiro Yamada <yamada.masahiro@socionext.com>,
	catalin.marinas@arm.com, joe.lawrence@redhat.com,
	penguin-kernel@i-love.sakura.ne.jp, schowdary@nvidia.com,
	urezki@gmail.com, andriy.shevchenko@linux.intel.com,
	changbin.du@intel.com, kunit-dev@googlegroups.com,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Knut Omang <knut.omang@oracle.com>
Subject: Re: [PATCH v2 linux-kselftest-test 1/3] kunit: allow kunit tests to be loaded as a module
Date: Fri, 11 Oct 2019 02:47:49 -0700	[thread overview]
Message-ID: <CAFd5g46_6McK06XSrX=EZ9AaYYitQzd2CTvPMX+rPymisDq5uQ@mail.gmail.com> (raw)
In-Reply-To: <alpine.LRH.2.20.1910091726010.2517@dhcp-10-175-191-127.vpn.oracle.com>

Sorry for the delayed reply. I will be on vacation until Wednesday,
October 16th.

On Wed, Oct 9, 2019 at 9:36 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On Tue, 8 Oct 2019, Brendan Higgins wrote:
>
> > On Tue, Oct 08, 2019 at 03:55:44PM +0100, Alan Maguire wrote:
[...]
> > > diff --git a/lib/kunit/string-stream.c b/lib/kunit/string-stream.c
> > > index e6d17aa..e4f3a97 100644
> > > --- a/lib/kunit/string-stream.c
> > > +++ b/lib/kunit/string-stream.c
> > > @@ -100,6 +100,7 @@ int string_stream_vadd(struct string_stream *stream,
> > >
> > >     return 0;
> > >  }
> > > +EXPORT_SYMBOL_GPL(string_stream_vadd);
> >
> > Is this actually needed by anything other than lib/kunit/test.c right
> > now? Maybe we should move the include file into the kunit/ directory to
> > hide these so no one else can use them.
> >
>
> I tried this, and it's the right answer I think but it exposes
> a problem with symbol visibility when kunit is compiled as a module.
> More on this below...
>
> > >  int string_stream_add(struct string_stream *stream, const char *fmt, ...)
> > >  {
> > > @@ -112,6 +113,7 @@ int string_stream_add(struct string_stream *stream, const char *fmt, ...)
> > >
> > >     return result;
> > >  }
> > > +EXPORT_SYMBOL_GPL(string_stream_add);
> > [...]
> > > diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> > > index c83c0fa..e7896f1 100644
> > > --- a/lib/kunit/test.c
> > > +++ b/lib/kunit/test.c
> > [...]
> > > @@ -50,6 +51,7 @@ static unsigned long kunit_test_timeout(void)
> > >      * For more background on this topic, see:
> > >      * https://mike-bland.com/2011/11/01/small-medium-large.html
> > >      */
> > > +#ifndef MODULE
> >
> > Why is this block of code "ifndef MODULE"?
> >
>
> Symbol visibility is the problem again; sysctl_hung_task_timeout_secs
> isn't exported so when kunit is a module it can't find the symbol.
>
> I think I saw Kees mentioned something about symbol lookup too; in KTF
> Knut solved this by defining ktf_find_symbol(). I'd suggest we may need a
> kunit_find_symbol() with a function signature

I thought we were just talking about exposing symbols for linking
outside of a compilation unit (static vs. not static); nevertheless, I
think you are right that it is relevant here. Kees, thoughts?

> void *kunit_find_symbol(const char *modname, const char *symbol_name);
>
> ...which does a [module_]kallsyms_lookup_sym().
>
> If the above makes sense I can look at adding it as a patch (and adding
> a test of it of course!). What do you think?

So that won't work if you are trying to link against a symbol not in a
module, right? Also, it won't work against a static symbol, right?

Even so, I think it is pretty wonky to expect users to either a)
export any symbol name to be tested, or b) have to access them via
kunit_find_symbol. I think it is fine to have some tests that cannot
be compiled as modules, if there is no other user friendly way to make
this work in those cases.

Thoughts?

  reply	other threads:[~2019-10-11  9:48 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-08 14:55 [PATCH v2 linux-kselftest-test 0/3] kunit: support building core/tests as modules Alan Maguire
2019-10-08 14:55 ` [PATCH v2 linux-kselftest-test 1/3] kunit: allow kunit tests to be loaded as a module Alan Maguire
2019-10-08 21:35   ` Brendan Higgins
2019-10-09 16:35     ` Alan Maguire
2019-10-11  9:47       ` Brendan Higgins [this message]
2019-10-11 10:25         ` Alan Maguire
2019-10-16 23:01           ` Brendan Higgins
2019-10-17 18:32             ` Alan Maguire
2019-10-18 12:21               ` Luis Chamberlain
2019-10-24  1:33                 ` Brendan Higgins
2019-10-08 14:55 ` [PATCH v2 linux-kselftest-test 2/3] kunit: allow kunit " Alan Maguire
2019-10-08 15:13   ` Randy Dunlap
2019-10-08 14:55 ` [PATCH v2 linux-kselftest-test 3/3] kunit: update documentation to describe module-based build Alan Maguire
2019-10-08 21:47   ` Brendan Higgins
2019-10-08 21:00 ` [PATCH v2 linux-kselftest-test 0/3] kunit: support building core/tests as modules Brendan Higgins
2019-10-14  9:20 ` Luis Chamberlain
2019-10-14 14:02   ` Alan Maguire
2019-10-16 12:47     ` Luis Chamberlain

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='CAFd5g46_6McK06XSrX=EZ9AaYYitQzd2CTvPMX+rPymisDq5uQ@mail.gmail.com' \
    --to=brendanhiggins@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=alan.maguire@oracle.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=catalin.marinas@arm.com \
    --cc=changbin.du@intel.com \
    --cc=joe.lawrence@redhat.com \
    --cc=keescook@chromium.org \
    --cc=knut.omang@oracle.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=schowdary@nvidia.com \
    --cc=skhan@linuxfoundation.org \
    --cc=urezki@gmail.com \
    --cc=yamada.masahiro@socionext.com \
    --cc=yzaikin@google.com \
    --subject='Re: [PATCH v2 linux-kselftest-test 1/3] kunit: allow kunit tests to be loaded as a module' \
    /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

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).