2013-04-29 19:52:09

by Steven Rostedt

[permalink] [raw]
Subject: [PATCH 0/2] [GIT PULL] localmodconfig: Fix missing depends of config files included in if statements


Linus,

A bug was recently found (today) in the make localmodconfig where it would miss
dependencies of config files that are included in other config files inside
an if statement.

Also added a debug print that helped in solving this bug.

Please pull the latest ktest-v3.10 tree, which can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-ktest.git
ktest-v3.10

Head SHA1: 7714fe5ac0cf752eae7a20ef23408aa26271a3eb


Steven Rostedt (2):
localmodconfig: Add debug prints for dependencies of module configs
localmodconfig: Process source kconfig files as they are found

----
scripts/kconfig/streamline_config.pl | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)


Attachments:
signature.asc (490.00 B)
This is a digitally signed message part

2013-04-29 21:01:45

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 0/2] [GIT PULL] localmodconfig: Fix missing depends of config files included in if statements

On Mon, Apr 29, 2013 at 12:51 PM, Steven Rostedt <[email protected]> wrote:
>
>
> Please pull the latest ktest-v3.10 tree, which can be found at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-ktest.git
> ktest-v3.10

Hmm. This is your ktest tree (which I already pulled the ktest changes
from), not your localmodconfig tree. But then all the explanations are
about your localmodconfig changes.

I assume you *meant* to ask me to pull your linux-kconfig.git tree?
That has a "ktest-v3.10" tag too, but I think that's a mistake too,
and you *meant* to make a tag called "localconfig" like you have done
in the past.

There's just too much confusion here for me to touch anything at all,
so please fix things up.

Linus

2013-04-29 21:43:39

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 0/2] [GIT PULL] localmodconfig: Fix missing depends of config files included in if statements

On Mon, Apr 29, 2013 at 2:01 PM, Linus Torvalds
<[email protected]> wrote:
>
> There's just too much confusion here for me to touch anything at all,
> so please fix things up.

Oh, and you might as well check that I resolved the conflicts in the
trace pull correctly while you're at it. They looked pretty obvious,
and it compiles for me, but ..

Linus

2013-04-29 23:23:58

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 0/2] [GIT PULL] localmodconfig: Fix missing depends of config files included in if statements

On Mon, 2013-04-29 at 14:01 -0700, Linus Torvalds wrote:
> On Mon, Apr 29, 2013 at 12:51 PM, Steven Rostedt <[email protected]> wrote:
> >
> >
> > Please pull the latest ktest-v3.10 tree, which can be found at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-ktest.git
> > ktest-v3.10
>
> Hmm. This is your ktest tree (which I already pulled the ktest changes
> from), not your localmodconfig tree. But then all the explanations are
> about your localmodconfig changes.
>
> I assume you *meant* to ask me to pull your linux-kconfig.git tree?

Face palm!

> That has a "ktest-v3.10" tag too, but I think that's a mistake too,
> and you *meant* to make a tag called "localconfig" like you have done
> in the past.
>
> There's just too much confusion here for me to touch anything at all,
> so please fix things up.

Yeah, sorry. I have no idea what I was thinking. I was doing my double
check of ktest changes and must have still had that on my mind when I
switched over to the localmodconfig stuff.

I'll clean it up.

-- Steve

2013-04-30 00:08:17

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 0/2] [GIT PULL] localmodconfig: Fix missing depends of config files included in if statements

On Mon, 2013-04-29 at 14:42 -0700, Linus Torvalds wrote:
> On Mon, Apr 29, 2013 at 2:01 PM, Linus Torvalds
> <[email protected]> wrote:
> >
> > There's just too much confusion here for me to touch anything at all,
> > so please fix things up.
>
> Oh, and you might as well check that I resolved the conflicts in the
> trace pull correctly while you're at it. They looked pretty obvious,
> and it compiles for me, but ..

Almost.

I'll also run my full ftrace test suite on your latest kernel, and see
if it finds anything else.

Please apply:

---
tracing: Fix small merge bug

During the 3.10 merge, a conflict happened and the resolution was
almost, but not quite, correct. An if statement was reversed.

Signed-off-by: Steven Rostedt <[email protected]>

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 581630a..ae6fa2d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -904,7 +904,7 @@ update_max_tr_single(struct trace_array *tr, struct task_struct *tsk, int cpu)
return;

WARN_ON_ONCE(!irqs_disabled());
- if (tr->allocated_snapshot) {
+ if (!tr->allocated_snapshot) {
/* Only the nop tracer should hit this when disabling */
WARN_ON_ONCE(tr->current_trace != &nop_trace);
return;

2013-04-30 00:17:22

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 0/2] [GIT PULL] localmodconfig: Fix missing depends of config files included in if statements

On Mon, 2013-04-29 at 20:08 -0400, Steven Rostedt wrote:
> On Mon, 2013-04-29 at 14:42 -0700, Linus Torvalds wrote:
> > On Mon, Apr 29, 2013 at 2:01 PM, Linus Torvalds
> > <[email protected]> wrote:
> > >
> > > There's just too much confusion here for me to touch anything at all,
> > > so please fix things up.
> >
> > Oh, and you might as well check that I resolved the conflicts in the
> > trace pull correctly while you're at it. They looked pretty obvious,
> > and it compiles for me, but ..
>
> Almost.
>

BTW, what's the preferred method for this. I already posted a lot of
work to linux-next when I found bugs that required going into your tree.
I backported the fixes knowing that it will cause conflicts when you
merge.

IIRC, you stated that you don't mind doing conflict resolutions
yourself, so I did not try to fix it a head of time, as the conflicts
were rather minor.

Should I have merged your tree and done the conflict resolutions myself,
or was it OK to do what I did, and let you do the conflict resolution
and send you any fixes that needed to be done afterward?

-- Steve

2013-04-30 01:01:19

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 0/2] [GIT PULL] localmodconfig: Fix missing depends of config files included in if statements

On Mon, 2013-04-29 at 20:08 -0400, Steven Rostedt wrote:

> I'll also run my full ftrace test suite on your latest kernel, and see
> if it finds anything else.
>

With the fix I sent, everything passed.

Thanks,

-- Steve

2013-04-30 14:28:33

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 0/2] [GIT PULL] localmodconfig: Fix missing depends of config files included in if statements

On Mon, Apr 29, 2013 at 5:17 PM, Steven Rostedt <[email protected]> wrote:
>
> BTW, what's the preferred method for this. I already posted a lot of
> work to linux-next when I found bugs that required going into your tree.
> I backported the fixes knowing that it will cause conflicts when you
> merge.
>
> IIRC, you stated that you don't mind doing conflict resolutions
> yourself, so I did not try to fix it a head of time, as the conflicts
> were rather minor.
>
> Should I have merged your tree and done the conflict resolutions myself,
> or was it OK to do what I did, and let you do the conflict resolution
> and send you any fixes that needed to be done afterward?

You did the right thing. In general, if you know there will be
conflicts, it's nice if you mention then in the pull request, but for
simple stuff like this it's really not a big deal. The fact that I
screwed up and then missed a "!" when editing it all is embarrassing,
but it wasn't because the conflict was really *complicated*, it was
just stupid editing error.

If the conflicts are really complex, at some point I really enjoy
getting a "here's a pre-merged branch if you prefer it", and if people
send that, I still tend to do the merge myself, but then I often just
compare against the pre-merged one afterwards to verify. But that's
actually extra work, so I'd suggest doing that only for things that
really warrant it. It's generally a bad thing if it happens, because
it means that we had some bad workflow and people stepped on each
others toes (or we had unlucky backports etc that ended up being in
the same area as much more work).

Linus