* Re: [PATCH] kvm: external module: fix unifdef problem
2008-10-24 18:10 [PATCH] kvm: external module: fix unifdef problem Hollis Blanchard
@ 2008-10-26 12:38 ` Avi Kivity
2008-10-27 0:58 ` Sheng Yang
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2008-10-26 12:38 UTC (permalink / raw)
To: kvm-ppc
Hollis Blanchard wrote:
> Guys, I don't mind if you add new things that aren't enabled for other
> architectures, but please try to be a little more careful about breaking
> us.
>
> This patch results in the following on PowerPC:
> mv $i $i.orig && unifdef -DCONFIG_POWERPC -UCONFIG_X86 IA64
> $i.orig > $i; [ $? -le 2 ] && rm $i.orig; done
> unifdef: can only do one file
>
>
Aw.
> Here's my proposed fix:
>
> kvm: external module: Treat NONARCH_CONFIG as a list, not a single item.
>
> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
>
> diff --git a/kernel/Makefile b/kernel/Makefile
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -25,8 +25,9 @@ _hack = mv $1 $1.orig && \
> gawk -v version=$(version) -f $(ARCH_DIR)/hack-module.awk $1.orig \
> | sed '/\#include/! s/\blapic\b/l_apic/g' > $1 && rm $1.orig
>
> +unifdef_uflags = $(foreach arch, $(NONARCH_CONFIG), -UCONFIG_$(arch))
>
$(patsubst ...), or even $(NONARCH_CONFIG:%=-UCONFIG_%)
But I think NONARCH_CONFIG needs to be adjusted as well.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] kvm: external module: fix unifdef problem
2008-10-24 18:10 [PATCH] kvm: external module: fix unifdef problem Hollis Blanchard
2008-10-26 12:38 ` Avi Kivity
@ 2008-10-27 0:58 ` Sheng Yang
2008-10-27 16:07 ` Christian Ehrhardt
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Sheng Yang @ 2008-10-27 0:58 UTC (permalink / raw)
To: kvm-ppc
On Saturday 25 October 2008 02:10:54 Hollis Blanchard wrote:
> On Thu, 2008-10-23 at 06:41 +0000, Avi Kivity wrote:
> > From: Sheng Yang <sheng@linux.intel.com>
> >
> > unifdef needs a -U parameter to deal with undefined macros, otherwise
> > it can't deal with #if defined() || defined().
> >
> > Also fix a historic bug on never execute unifdef...
> >
> > Also discard "set -e" before unifdef, because unifdef would return 1 if
> > it have done something to the file.
> >
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > Signed-off-by: Avi Kivity <avi@redhat.com>
> >
> > diff --git a/kernel/Makefile b/kernel/Makefile
> > index ef18fa6..fed3bd4 100644
> > --- a/kernel/Makefile
> > +++ b/kernel/Makefile
> > @@ -3,6 +3,8 @@ include config.kbuild
> >
> > ARCH_DIR = $(if $(filter $(ARCH),x86_64 i386),x86,$(ARCH))
> > ARCH_CONFIG := $(shell echo $(ARCH_DIR) | tr '[:lower:]' '[:upper:]')
> > +# NONARCH_CONFIG used for unifdef, and only cover X86 and IA64 now
> > +NONARCH_CONFIG = $(filter-out $(ARCH_CONFIG),X86 IA64)
> >
> > KVERREL = $(patsubst /lib/modules/%/build,%,$(KERNELDIR))
> >
> > @@ -24,8 +26,8 @@ _hack = mv $1 $1.orig && \
> >
> > | sed '/\#include/! s/\blapic\b/l_apic/g' > $1 && rm $1.orig
> >
> > unifdef = mv $1 $1.orig && \
> > - unifdef -DCONFIG_$(ARCH_CONFIG) $1.orig > $1; \
> > - [ $$? -le 1 ] && rm $1.orig
> > + unifdef -DCONFIG_$(ARCH_CONFIG) -UCONFIG_$(NONARCH_CONFIG) $1.orig >
> > $1; \ + [ $$? -le 2 ] && rm $1.orig
> >
> > hack = $(call _hack,$T/$(strip $1))
>
> Guys, I don't mind if you add new things that aren't enabled for other
> architectures, but please try to be a little more careful about breaking
> us.
Oh, sorry. I've informed the Christian Ehrhardt to checked the issue, but he
is too busy at the time... I would be more careful next time...
Sorry for the inconvenient...
--
regards
Yang, Sheng
>
> This patch results in the following on PowerPC:
> mv $i $i.orig && unifdef -DCONFIG_POWERPC -UCONFIG_X86 IA64
> $i.orig > $i; [ $? -le 2 ] && rm $i.orig; done
> unifdef: can only do one file
>
> Here's my proposed fix:
>
> kvm: external module: Treat NONARCH_CONFIG as a list, not a single item.
>
> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
>
> diff --git a/kernel/Makefile b/kernel/Makefile
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -25,8 +25,9 @@ _hack = mv $1 $1.orig && \
> gawk -v version=$(version) -f $(ARCH_DIR)/hack-module.awk $1.orig \
>
> | sed '/\#include/! s/\blapic\b/l_apic/g' > $1 && rm $1.orig
>
> +unifdef_uflags = $(foreach arch, $(NONARCH_CONFIG), -UCONFIG_$(arch))
> unifdef = mv $1 $1.orig && \
> - unifdef -DCONFIG_$(ARCH_CONFIG) -UCONFIG_$(NONARCH_CONFIG) $1.orig >
> $1; \ + unifdef -DCONFIG_$(ARCH_CONFIG) $(unifdef_uflags) $1.orig > $1; \
> [ $$? -le 2 ] && rm $1.orig
>
> hack = $(call _hack,$T/$(strip $1))
>
> Comments?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] kvm: external module: fix unifdef problem
2008-10-24 18:10 [PATCH] kvm: external module: fix unifdef problem Hollis Blanchard
2008-10-26 12:38 ` Avi Kivity
2008-10-27 0:58 ` Sheng Yang
@ 2008-10-27 16:07 ` Christian Ehrhardt
2008-10-27 16:29 ` Avi Kivity
2008-10-28 1:02 ` Zhang, Xiantao
4 siblings, 0 replies; 6+ messages in thread
From: Christian Ehrhardt @ 2008-10-27 16:07 UTC (permalink / raw)
To: kvm-ppc
Hi Avi,
Hollis and I discussed how to continue on that.
Atm the upstream code is broken for powerpc and your response is too
vague for me to extend our patch in some way.
So which way should we go? Will you apply (or should I resubmit ?)
Hollis patch for now to fix upstream for powerpc. And we/you extend it
later or what else would you prefer?
Avi Kivity wrote:
> Hollis Blanchard wrote:
>> Guys, I don't mind if you add new things that aren't enabled for other
>> architectures, but please try to be a little more careful about breaking
>> us.
>>
>> This patch results in the following on PowerPC:
>> mv $i $i.orig && unifdef -DCONFIG_POWERPC -UCONFIG_X86 IA64
>> $i.orig > $i; [ $? -le 2 ] && rm $i.orig; done
>> unifdef: can only do one file
>>
>>
>
> Aw.
>
>> Here's my proposed fix:
>>
>> kvm: external module: Treat NONARCH_CONFIG as a list, not a single item.
>>
>> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
>>
>> diff --git a/kernel/Makefile b/kernel/Makefile
>> --- a/kernel/Makefile
>> +++ b/kernel/Makefile
>> @@ -25,8 +25,9 @@ _hack = mv $1 $1.orig && \
>> gawk -v version=$(version) -f $(ARCH_DIR)/hack-module.awk $1.orig \
>> | sed '/\#include/! s/\blapic\b/l_apic/g' > $1 && rm $1.orig
>>
>> +unifdef_uflags = $(foreach arch, $(NONARCH_CONFIG), -UCONFIG_$(arch))
>>
>
> $(patsubst ...), or even $(NONARCH_CONFIG:%=-UCONFIG_%)
>
> But I think NONARCH_CONFIG needs to be adjusted as well.
>
--
Grüsse / regards,
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kvm: external module: fix unifdef problem
2008-10-24 18:10 [PATCH] kvm: external module: fix unifdef problem Hollis Blanchard
` (2 preceding siblings ...)
2008-10-27 16:07 ` Christian Ehrhardt
@ 2008-10-27 16:29 ` Avi Kivity
2008-10-28 1:02 ` Zhang, Xiantao
4 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2008-10-27 16:29 UTC (permalink / raw)
To: kvm-ppc
Christian Ehrhardt wrote:
> Hollis and I discussed how to continue on that.
> Atm the upstream code is broken for powerpc and your response is too
> vague for me to extend our patch in some way.
> So which way should we go? Will you apply (or should I resubmit ?)
> Hollis patch for now to fix upstream for powerpc. And we/you extend it
> later or what else would you prefer?
My thinking is that we convert CONFIG_* to __i386__, __x86_64__,
__powerpc__, and the like, using the hack-modules pass in 'make sync'.
The advantages of that are:
- 'make sync' headers are architecture independent. Currently once you
run 'make sync', the headers become tied to the arch, even the ones in
include/linux
- The headers are standalone -- no need for -DCONFIG_* in anything that
includes it
- We already mangle things, so it's not a big change
I think Xiantao was going to try it, but if not, then please do it
yourself. I could too, of course, but I prefer someone that can test on
non-x86 do it.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH] kvm: external module: fix unifdef problem
2008-10-24 18:10 [PATCH] kvm: external module: fix unifdef problem Hollis Blanchard
` (3 preceding siblings ...)
2008-10-27 16:29 ` Avi Kivity
@ 2008-10-28 1:02 ` Zhang, Xiantao
4 siblings, 0 replies; 6+ messages in thread
From: Zhang, Xiantao @ 2008-10-28 1:02 UTC (permalink / raw)
To: kvm-ppc
Avi Kivity wrote:
> Christian Ehrhardt wrote:
>> Hollis and I discussed how to continue on that.
>> Atm the upstream code is broken for powerpc and your response is too
>> vague for me to extend our patch in some way.
>> So which way should we go? Will you apply (or should I resubmit ?)
>> Hollis patch for now to fix upstream for powerpc. And we/you extend
>> it later or what else would you prefer?
>
> My thinking is that we convert CONFIG_* to __i386__, __x86_64__,
> __powerpc__, and the like, using the hack-modules pass in 'make sync'.
>
> The advantages of that are:
>
> - 'make sync' headers are architecture independent. Currently once
> you run 'make sync', the headers become tied to the arch, even the
> ones in include/linux
> - The headers are standalone -- no need for -DCONFIG_* in anything
> that includes it
> - We already mangle things, so it's not a big change
>
> I think Xiantao was going to try it, but if not, then please do it
> yourself. I could too, of course, but I prefer someone that can test
> on non-x86 do it.
Okay, I will try it out, and send to you to have the test on non-ia64 platforms.
Thanks
Xiantao
^ permalink raw reply [flat|nested] 6+ messages in thread