Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1264577 > unrolled thread

Re: [GIT PULL] tracing: Updates for 4.4

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [GIT PULL] tracing: Updates for 4.4 Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-06 22:40 +0100
    Re: [GIT PULL] tracing: Updates for 4.4 Stephen Rothwell <sfr@canb.auug.org.au> - 2015-11-06 23:10 +0100
      Re: [GIT PULL] tracing: Updates for 4.4 Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-06 23:20 +0100
        Re: [GIT PULL] tracing: Updates for 4.4 Steven Rostedt <rostedt@goodmis.org> - 2015-11-07 00:00 +0100
          Re: [GIT PULL] tracing: Updates for 4.4 Steven Rostedt <rostedt@goodmis.org> - 2015-11-07 00:10 +0100

#1264577 — Re: [GIT PULL] tracing: Updates for 4.4

On Fri, Nov 6, 2015 at 6:10 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Most of the changes are clean ups and small fixes. Some of them have
> stable tags to them. I searched through my INBOX just as the merge window
> opened and found lots of patches to pull. I ran them through all my tests
> and they were in linux-next for a few days.

Clearly they got zero actual testing, though.

I get several very big and ugly warnings about scheduler tracing:

  kernel/trace/trace_events.c: In function ‘__ftrace_clear_event_pids’:
  kernel/trace/trace_events.c:579:32: warning: passing argument 1 of
‘unregister_trace_sched_switch’ from incompatible pointer type
[-Wincompatible-pointer-types]
    unregister_trace_sched_switch(event_filter_pid_sched_switch_probe_pre, tr);
                                  ^
  In file included from kernel/trace/trace_events.c:25:0:
  include/trace/events/sched.h:124:1095: note: expected ‘void (*)(void
*, bool,  struct task_struct *, struct task_struct *) {aka void
(*)(void *, _Bool,  struct task_struct *, struct task_struct *)}’ but
argument is of type ‘void (*)(void *, struct task_struct *, struct
task_struct *)’

which clearly can't work, and is due to the new "bool preempt"
argument in scheduler tracing.

That *should* have shown up in linux-next, and you *should* have been
aware of it, and in turn let me know about it. Yes, yes, I notice
these things on my own, but I also expect that maintainers look out
for these things, especially when they were involved on both sides, so
it shouldn't have taken them - and this me - by surprise.

But something clearly failed in that whole process.

This is why we do *not* do some last-minute "let's just look through
my mailbox as the merge window is opening" crap.

I've done the merge, and I have it fixed up in my tree, but I'm
annoyed enough that I'm considering just unpulling. You *knew* about
this, because you are marked as having reviewed that commit
c73464b1c843 ("sched/core: Fix trace_sched_switch()") that added the
preempt argument.

So where did this all fail? Nobody ever looked at the warnings from
linux-next? Or it wasn't even in linux-next long enough to really ever
trigger?

I very much suspect that "look through my INBOX as the merge window
opened" is the real problem here. That is *not* how the merge window
works, and you damn well should know it.

                     Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1264594

Hi Linus,

On Fri, 6 Nov 2015 13:37:48 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
> So where did this all fail? Nobody ever looked at the warnings from
> linux-next? Or it wasn't even in linux-next long enough to really ever
> trigger?

This was reported against linux-next on Nov 2 by Sergey Senozhatsky who
supplied a fix patch that I then used as the merge conflict resolution
from Nov 3 onward (which everyone involved knew about) ...

I also noticed it, but was a bit under the weather and tired to do
anything about it immediately.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1264601

On Fri, Nov 6, 2015 at 2:09 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> This was reported against linux-next on Nov 2 by Sergey Senozhatsky who
> supplied a fix patch that I then used as the merge conflict resolution
> from Nov 3 onward (which everyone involved knew about) ...

Ok, so the problem is that even though a maintainer is aware of the
semantic conflict, the pull request doesn't talk about it.

A lot of maintainers *do* let me know, which I really appreciate, not
only because it avoids the surprise, but because it happens that I
miss these semantic conflicts.

Sometimes the semantic conflict is something that simply doesn't show
up on x86-64 build I do.  Or I was on the road and didn't do a full
allmodconfig build. Or it needs very specific config options etc. Or I
just screw up. When the pull message warns me about it, that will
help.

But maybe it's one of those things that isn't written down, and people
don't think about as part of doing their pull request, so they forget.

                  Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1264634

On Fri, 6 Nov 2015 14:15:48 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Fri, Nov 6, 2015 at 2:09 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > This was reported against linux-next on Nov 2 by Sergey Senozhatsky who
> > supplied a fix patch that I then used as the merge conflict resolution
> > from Nov 3 onward (which everyone involved knew about) ...
> 
> Ok, so the problem is that even though a maintainer is aware of the
> semantic conflict, the pull request doesn't talk about it.

Damn, that was my fault. I simply forgot to mention it :-(


> 
> A lot of maintainers *do* let me know, which I really appreciate, not
> only because it avoids the surprise, but because it happens that I
> miss these semantic conflicts.
> 
> Sometimes the semantic conflict is something that simply doesn't show
> up on x86-64 build I do.  Or I was on the road and didn't do a full
> allmodconfig build. Or it needs very specific config options etc. Or I
> just screw up. When the pull message warns me about it, that will
> help.
> 
> But maybe it's one of those things that isn't written down, and people
> don't think about as part of doing their pull request, so they forget.
> 

Yep, I was going to add that, but I've been working on internal Red Hat
Bugzilla's all week that my mind was not fully focused on that pull
request.

I'll have to add something to my scripts that pulls in any merge
breakage, such that when I run the scripts to generate the pull
request, it adds them in automatically. Because obviously, the manual
approach isn't working for me.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1264639

On Fri, 6 Nov 2015 17:52:06 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> I'll have to add something to my scripts that pulls in any merge
> breakage, such that when I run the scripts to generate the pull
> request, it adds them in automatically. Because obviously, the manual
> approach isn't working for me.

I updated my scripts such that if I add a 'notes' file in the repo that
I'm pushing, it will include that into the pull request. Now if there's
a conflict like this again, I can add a comment about it in the notes
file and it will be included in my next pull request.

I just need to remember to delete it after I'm done ;-)

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web