All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
To: Jason Baron <jbaron@redhat.com>
Cc: David Miller <davem@davemloft.net>,
	linux-kernel@vger.kernel.org, mingo@elte.hu, hpa@zytor.com,
	tglx@linutronix.de, rostedt@goodmis.org, andi@firstfloor.org,
	roland@redhat.com, rth@redhat.com, mhiramat@redhat.com,
	fweisbec@gmail.com, avi@redhat.com, vgoyal@redhat.com,
	sam@ravnborg.org
Subject: Re: [PATCH 00/13] jump label v9
Date: Tue, 15 Jun 2010 11:44:40 -0400	[thread overview]
Message-ID: <20100615154440.GA9224@Krystal> (raw)
In-Reply-To: <20100615142811.GB2750@redhat.com>

* Jason Baron (jbaron@redhat.com) wrote:
> On Mon, Jun 14, 2010 at 08:47:59PM -0700, David Miller wrote:
> > Jason, I'm really at wits end about this patch set.  To say
> > that trying to test our your patches is frustrating for me
> > so far would be an understatement.
> > 
> > Nothing you ever post builds for me, not one patch set has
> > built properly.
> > 
> > I can also tell that you're just blindly making changes to the
> > sparc bits and not trying to build test them at all:
> > 
> > 1) Even though you created the jump_label_t, and made it properly
> >    a u32 on sparc, you left the assembler using ".xword" to
> >    record the entries.
> > 
> > 2) The sparc "struct jump_label" still calls it's third member "name",
> >    it needs to be "key" or else the build breaks.
> > 
> > 3) Eventhough the sparc JUMP_LABEL macro was fixed to have two args,
> >    the first arg was left as "tag" instead of being renamed to "key"
> >    and that name change propaged into the asm in the macro expansion.
> > 
> > I took care of that locally to try and test this, but then I hit the
> > current major problem which is that you're using things like
> > text_poke_early() unconditionally, but that is an X86-only facility
> > implemented by x86's alternative mechanism.
> > 
> > Also, kernel/jump_label.c only gets the ERR_PTR() definitions
> > indirectly on the x86 platform, it needs to include linux/err.h
> > directly to make sure those things are available on every platform.
> > 
> > You gave me the impression a few iterations ago that you were doing
> > build testing on sparc64 using cross-compilers, or that you would
> > start to do so.  You're obviously not, could you please start doing so
> > and let me know when you've at least build tested your jump-label
> > patch series on sparc64 and at least one architecture that lacks
> > jump-label support?
> > 
> > Thanks.
> 
> Hi David,
> 
> Yes, I've tried to help re-write the sparc bits to the current api.
> However, I did not test the sparc enabled jump-label bits, b/c I need an
> updated cross compiler to do so (that has jump label support). However, I
> certainly did build test the patches on powerpc, which lacks jump-label support,
> so I know it builds on at least one architecture that lacks jump-label support
> as you've mentioned. And I did this specifically, since you requested this
> testing.
> 
> I guess I was hoping we could work more collaboratively on the sparc
> bits. A couple lines of code have fixed the issues that you've brought up.
> Sorry, if i mislead you. I really just want to do what is best for the linux
> kernel, if that's going off and figuring out how to compile a new sparc
> enabled jump label compiler for sparc, I will do it.

Hi Jason,

It makes me wonder if anyone had success building a gcc 4.5
Intel-to-sparc64 cross-compiler ? Usually, the crosstool-like suites are
a few versions behind. I'm aware that this is not trivial, as
cross-compilers have a tendency to refuse to get built in certain
occasions (such as being a recent less tested version). I'd recommend
you focus on this as a first step before resubmitting.

>                                                      And I do agree,
> that leaving text_poke_early() is my mistake. However, maybe we can
> discuss this issue? For example, the reason I have it in the code is b/c
> x86 determines the best no-op at run-time. Are other architectures going
> to have to require this kind of functionality. Or like sparc, are we
> going to be able to generally hard-code the nops on non-x86 at
> compile-time?

You might want to create "dumb" text_poke() and text_poke_early()
implementations on other architectures that wraps kernel text updates
pretty simply. The implementation is trivial if the architecture does
not write-protect the text pages, but becomes more evolved when it does.

Thanks,

Mathieu

> 
> thanks. And again I apologize for any wasted cycles that I've caused.
> 
> -Jason
>  
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

  reply	other threads:[~2010-06-15 15:44 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-09 21:38 [PATCH 00/13] jump label v9 Jason Baron
2010-06-09 21:38 ` [PATCH 01/13] jump label v9: notifier atomic call chain notrace Jason Baron
2010-06-09 21:58   ` Frederic Weisbecker
2010-06-10 15:34     ` Jason Baron
2010-06-09 21:38 ` [PATCH 02/13] jump label v9: base patch Jason Baron
2010-06-09 22:35   ` Frederic Weisbecker
2010-06-10 15:44     ` Jason Baron
2010-06-10 16:22       ` Ingo Molnar
2010-06-10 17:11       ` Frederic Weisbecker
2010-06-09 22:36   ` Frederic Weisbecker
2010-06-10 12:06   ` Peter Zijlstra
2010-06-10 12:18   ` Peter Zijlstra
2010-06-09 21:39 ` [PATCH 03/13] jump label v9: x86 support Jason Baron
2010-06-10 12:12   ` Peter Zijlstra
2010-06-10 12:14     ` Ingo Molnar
2010-06-10 13:26       ` Andi Kleen
2010-06-10 14:12         ` Peter Zijlstra
2010-06-10 14:28           ` Andi Kleen
2010-06-10 15:37         ` Ingo Molnar
2010-06-10 16:24           ` Andi Kleen
2010-06-11  8:12             ` Ingo Molnar
2010-06-11  8:30               ` Andi Kleen
2010-06-10 16:29         ` Steven Rostedt
2010-06-10 15:04       ` Jason Baron
2010-06-10 16:13         ` Mathieu Desnoyers
2010-06-11  0:52           ` Jason Baron
2010-06-11  6:18       ` H. Peter Anvin
2010-06-11  7:58         ` Ingo Molnar
2010-06-10 12:15   ` Peter Zijlstra
2010-06-10 12:33   ` Peter Zijlstra
2010-06-09 21:39 ` [PATCH 04/13] jump label v9: tracepoint support Jason Baron
2010-06-10 12:22   ` Peter Zijlstra
2010-06-09 21:39 ` [PATCH 05/13] jump label v9: add module support Jason Baron
2010-06-09 21:39 ` [PATCH 06/13] jump label v9: move ftrace_dyn_arch_init to common code Jason Baron
2010-06-19  3:24   ` Steven Rostedt
2010-06-09 21:39 ` [PATCH 07/13] jump label v9: sort jump table at build-time Jason Baron
2010-06-09 21:39 ` [PATCH 08/13] jump label v9: initialize workqueue tracepoints *before* they are registered Jason Baron
2010-06-09 21:39 ` [PATCH 09/13] jump label v9: jump_label_text_reserved() to reserve our jump points Jason Baron
2010-06-09 21:39 ` [PATCH 10/13] jump label v9: convert jump label to use a key Jason Baron
2010-06-10 12:38   ` Peter Zijlstra
2010-06-10 12:43   ` Peter Zijlstra
2010-06-10 13:57     ` Jason Baron
2010-06-10 14:46       ` Peter Zijlstra
2010-06-09 21:39 ` [PATCH 11/13] jump label v9: convert dynamic debug to use jump labels Jason Baron
2010-06-09 21:39 ` [PATCH 12/13] jump label v9: sparc64 add jump_label support Jason Baron
2010-06-09 21:39 ` [PATCH 13/13] jump label v9: add docs Jason Baron
2010-06-10 12:49   ` Peter Zijlstra
2010-06-15  3:47 ` [PATCH 00/13] jump label v9 David Miller
2010-06-15 14:28   ` Jason Baron
2010-06-15 15:44     ` Mathieu Desnoyers [this message]
2010-06-18  3:45       ` Tony Breeds
2010-06-18 15:18         ` Mathieu Desnoyers
2010-06-15 17:13     ` David Miller
2010-06-15 17:28       ` H. Peter Anvin

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=20100615154440.GA9224@Krystal \
    --to=mathieu.desnoyers@polymtl.ca \
    --cc=andi@firstfloor.org \
    --cc=avi@redhat.com \
    --cc=davem@davemloft.net \
    --cc=fweisbec@gmail.com \
    --cc=hpa@zytor.com \
    --cc=jbaron@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@redhat.com \
    --cc=mingo@elte.hu \
    --cc=roland@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=rth@redhat.com \
    --cc=sam@ravnborg.org \
    --cc=tglx@linutronix.de \
    --cc=vgoyal@redhat.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 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.