jump label v9
in [Kernel]
|
Prev: [PATCH 2/6] KVM: MMU: fix conflict access permissions in direct sp
Next: [PATCH] MAINTAINERS: update mail address
From: David Miller on 14 Jun 2010 23:50 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jason Baron on 15 Jun 2010 10:30 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. 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? thanks. And again I apologize for any wasted cycles that I've caused. -Jason -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Mathieu Desnoyers on 15 Jun 2010 11:50 * Jason Baron (jbaron(a)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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: David Miller on 15 Jun 2010 13:20 From: Jason Baron <jbaron(a)redhat.com> Date: Tue, 15 Jun 2010 10:28:11 -0400 > 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? I think most architectures will use a constant nop sequence, in fact x86 is the only one I can think of that needs variable nop sequences. Why not abstract this behind some asm/jump_label.h macro just like everything else? "jump_label_text_poke_early()" or similar. On sparc64 I would define this to: #include <asm/system.h> static inline void jump_label_text_poke_early(void *addr, const void *opcode, size_t len) { u32 new_insn = *(u32 *)opcode; u32 *insn_p = (u32 *) addr; *insn_p = new_insn; flushi(insn_p); } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 15 Jun 2010 13:30
On 06/15/2010 10:13 AM, David Miller wrote: > From: Jason Baron <jbaron(a)redhat.com> > Date: Tue, 15 Jun 2010 10:28:11 -0400 > >> 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? > > I think most architectures will use a constant nop sequence, in fact > x86 is the only one I can think of that needs variable nop sequences. > One could potentially see that for other variable-length-instructions architectures, e.g. S390, m68k or ARM/Thumb2 as well. For fixed-length-instructions architectures, well, it shouldn't be an issue. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |