Strange s390 code in 2.4.19-pre8

Strange s390 code in 2.4.19-pre8

[Pete Zaitcev]

I reviewed the -pre8 briefly, and is seems that the vast
majority of s390/s390x changes are good, but I cannot discern
the intent of some of them. I would like Martin or Ulrich
to comment.

#1 - smp_call_function in a driver, trying to be ultra smart
or IBM's hardware is too broken and asymmetric?

+       iucv_debug(1, "entering");
+       if (smp_processor_id() == 0)
+               iucv_declare_buffer_cpu0(&b2f0_result);
+       else
+               smp_call_function(iucv_declare_buffer_cpu0, &b2f0_result, 0, 1);+       iucv_debug(1, "Address of EIB = %p", iucv_external_int_buffer);
+       if (b2f0_result == 0x0deadbeef)
+           b2f0_result = 0xaa;
+       iucv_debug(1, "exiting");
        return b2f0_result;

#2 - strange changes to net Makefile

--- linux.orig/drivers/s390/net/Makefile        Thu May  2 22:37:01 2002
+++ linux/drivers/s390/net/Makefile     Thu May  2 22:25:05 2002
@@ -9,8 +9,8 @@

 ctc-objs := ctcmain.o ctctty.o

-obj-y += iucv.o fsm.o
-obj-$(CONFIG_CTC) += ctc.o
+obj-$(CONFIG_IUCV) += iucv.o fsm.o
+obj-$(CONFIG_CTC) += ctc.o fsm.o
 obj-$(CONFIG_IUCV) += netiucv.o

 include $(TOPDIR)/Rules.make

I guarantee you that this blows up, unless CTC and IUCV are
built in. And why does it add wo CONFIG_IUCV lines? It is
legal, but unconventional.

#3 - config.in patch inconsistent or deadwood

--- linux.orig/arch/s390/config.in      Thu May  2 22:36:59 2002
+++ linux/arch/s390/config.in   Thu May  2 22:25:01 2002
@@ -37,6 +37,8 @@
 mainmenu_option next_comment
 comment 'General setup'
 bool 'Fast IRQ handling' CONFIG_FAST_IRQ
+bool 'Process warning machine checks' CONFIG_MACHCHK_WARNING
+bool 'Use chscs for Common I/O' CONFIG_CHSC
 bool 'Builtin IPL record support' CONFIG_IPL
 if [ "$CONFIG_IPL" = "y" ]; then
   choice 'IPL method generated into head.S' \

This is wonderful, but why did you merge it to Marcelo if the
rest of the code is missing?

And, while we are on topic, when are you guys going to merge
changes to the partitioning code?

-- Pete

Re: Strange s390 code in 2.4.19-pre8

[Martin Schwidefsky]

Hi Pete,
> I reviewed the -pre8 briefly, and is seems that the vast
> majority of s390/s390x changes are good, but I cannot discern
> the intent of some of them. I would like Martin or Ulrich
> to comment.
Ahh, its great that someone is doing reviews of our code.

> #1 - smp_call_function in a driver, trying to be ultra smart
> or IBM's hardware is too broken and asymmetric?
The iucv stuff in VM is a bit asymmetric ...

> #2 - strange changes to net Makefile
The intention of this is to have fsm.o built as a module if ctc
and iucv are built as modules too. I agree that this is broken
if one of {iucv,ctc} is built as a module and the other is built
in.

> #3 - config.in patch inconsistent or deadwood
> This is wonderful, but why did you merge it to Marcelo if the
> rest of the code is missing?
The rest is missing because we are waiting for the legal ok.
That the config.in contains these two lines is a mistake of mine.

> And, while we are on topic, when are you guys going to merge
> changes to the partitioning code?
I sent the patch together with the lcs driver to Marcelo last
week but I haven't heard of him yet. Wait and see ?

blue skies,
   Martin

Linux/390 Design & Development, IBM Deutschland Entwicklung GmbH
Schönaicherstr. 220, D-71032 Böblingen, Telefon: 49 - (0)7031 - 16-2247
E-Mail: schwidefsky@de.ibm.com

Re: Strange s390 code in 2.4.19-pre8

[Pete Zaitcev]

> From: "Martin Schwidefsky" <schwidefsky@de.ibm.com>
> Date: Mon, 13 May 2002 09:54:59 +0200

> > #2 - strange changes to net Makefile
> The intention of this is to have fsm.o built as a module if ctc
> and iucv are built as modules too. I agree that this is broken
> if one of {iucv,ctc} is built as a module and the other is built
> in.

The old Makefile was correct, and the only failing was a namespace
conflict between your fsm.c and fsm.c in ISDN, when CONFIG_MODVERSIONS
are used. The right fix is to rename your fsm.c or to create
fsm_s390_ksyms.c with all EXPORT_SYMBOL's sitting in there.
It is fixed in 2.5 with $(MODPREFIX) in Rules.make.

-- Pete

Re: Strange s390 code in 2.4.19-pre8

[Kai Germaschewski]

On Mon, 13 May 2002, Pete Zaitcev wrote:

> The old Makefile was correct, and the only failing was a namespace
> conflict between your fsm.c and fsm.c in ISDN, when CONFIG_MODVERSIONS
> are used. The right fix is to rename your fsm.c or to create
> fsm_s390_ksyms.c with all EXPORT_SYMBOL's sitting in there.
> It is fixed in 2.5 with $(MODPREFIX) in Rules.make.

It's easily possible to backport the 2.5 Rules.make change to 2.4, so the 
file doesn't have to be renamed - if you want me to do that, let me know.

--Kai

Re: Strange s390 code in 2.4.19-pre8

[Pete Zaitcev]

> Date: Mon, 13 May 2002 11:18:22 -0500 (CDT)
> From: Kai Germaschewski <kai@tp1.ruhr-uni-bochum.de>

> > The old Makefile was correct, and the only failing was a namespace
> > conflict between your fsm.c and fsm.c in ISDN, when CONFIG_MODVERSIONS
> > are used. The right fix is to rename your fsm.c or to create
> > fsm_s390_ksyms.c with all EXPORT_SYMBOL's sitting in there.
> > It is fixed in 2.5 with $(MODPREFIX) in Rules.make.
> 
> It's easily possible to backport the 2.5 Rules.make change to 2.4, so the 
> file doesn't have to be renamed - if you want me to do that, let me know.

Keith said to rename such things. Another easy thing to do
would be to move EXPORT_SYMBOL into s390_ksyms.c, under
#ifdef CONFIG_FSM_something. I understand that renaming
creates a diff with 100% change, so damage can easily
be slipped in, and then it's a hell to investigate.
[Let's see if a well known persona uses it as an excuse
for more commercial advertising of a certain software product].

-- Pete

Re: Strange s390 code in 2.4.19-pre8

[Kai Germaschewski]

On Mon, 13 May 2002, Pete Zaitcev wrote:

> Keith said to rename such things. Another easy thing to do
> would be to move EXPORT_SYMBOL into s390_ksyms.c, under
> #ifdef CONFIG_FSM_something. I understand that renaming
> creates a diff with 100% change, so damage can easily
> be slipped in, and then it's a hell to investigate.

No, putting the EXPORT_SYMBOL into s390_ksyms.c won't work for fsm.o being 
a module. Renaming has the obvious disadvantage that the module name 
changes as well.

I think I see two easy way to resolve the problem:
o backport the Rules.make change (it's been in 2.5 for months w/o even
  anybody noticing it, so it's definitely stable)
o Move the EXPORT_SYMBOL() in drivers/isdn/hisax/fsm.o into
  drivers/isdn/hisax/config.o

--Kai


> [Let's see if a well known persona uses it as an excuse
> for more commercial advertising of a certain software product].

;-)

Re: Strange s390 code in 2.4.19-pre8

[Pete Zaitcev]

> Date: Mon, 13 May 2002 11:30:06 -0500 (CDT)
> From: Kai Germaschewski <kai@tp1.ruhr-uni-bochum.de>

> I think I see two easy way to resolve the problem:
> o backport the Rules.make change (it's been in 2.5 for months w/o even
>   anybody noticing it, so it's definitely stable)
> o Move the EXPORT_SYMBOL() in drivers/isdn/hisax/fsm.o into
>   drivers/isdn/hisax/config.o

If ISDN people are willing to do that, it would be a great
relief. I did not raise this question because undoubtedly
they were using fsm.c first, so it was s390 mistake (accorting
to the comment in the drivers/s390/net/fsm.c).

-- Pete

Re: Strange s390 code in 2.4.19-pre8

[Kai Germaschewski]

On Mon, 13 May 2002, Pete Zaitcev wrote:

> If ISDN people are willing to do that, it would be a great
> relief. I did not raise this question because undoubtedly
> they were using fsm.c first, so it was s390 mistake (accorting
> to the comment in the drivers/s390/net/fsm.c).

If you ask nicely ;-)

Okay, I sent Marcelo a patch to do so.

--Kai

Re: Strange s390 code in 2.4.19-pre8

[Kai Germaschewski]

On Mon, 13 May 2002, Martin Schwidefsky wrote:

> > #2 - strange changes to net Makefile
> The intention of this is to have fsm.o built as a module if ctc
> and iucv are built as modules too. I agree that this is broken
> if one of {iucv,ctc} is built as a module and the other is built
> in.

Actually, I don't think it's broken, Rules.make takes care of that case 
and compiles fsm.o into the kernel in the latter case, it does not build 
a fsm.o module.

--Kai

Re: Strange s390 code in 2.4.19-pre8

[Pete Zaitcev]

> Date: Mon, 13 May 2002 11:16:07 -0500 (CDT)
> From: Kai Germaschewski <kai@tp1.ruhr-uni-bochum.de>

> > > #2 - strange changes to net Makefile
> > The intention of this is to have fsm.o built as a module if ctc
> > and iucv are built as modules too. I agree that this is broken
> > if one of {iucv,ctc} is built as a module and the other is built
> > in.
> 
> Actually, I don't think it's broken, Rules.make takes care of that case 
> and compiles fsm.o into the kernel in the latter case, it does not build 
> a fsm.o module.
> 
> --Kai

Obviously, the guy who did the change expected it to work
like you described, but he never tested it, and it doesn't.
ctc fails to load with init_fsm undefined.
It would work if EXPORT_SYMBOL_NOVERS were involved.

-- Pete

Re: Strange s390 code in 2.4.19-pre8

[Kai Germaschewski]

On Mon, 13 May 2002, Pete Zaitcev wrote:

> > Actually, I don't think it's broken, Rules.make takes care of that case 
> > and compiles fsm.o into the kernel in the latter case, it does not build 
> > a fsm.o module.
>
> Obviously, the guy who did the change expected it to work
> like you described, but he never tested it, and it doesn't.
> ctc fails to load with init_fsm undefined.
> It would work if EXPORT_SYMBOL_NOVERS were involved.

Not sure if I understand you correctly. I mean it should work w/o 
CONFIG_MODVERSIONS, and it would also work with CONFIG_MODVERSIONS if it 
wasn't for the conflict with the ISDN fsm.o. Do we agree here?

--Kai

Re: Strange s390 code in 2.4.19-pre8

[Keith Owens]

On Mon, 13 May 2002 12:19:33 -0400, 
Pete Zaitcev <zaitcev@redhat.com> wrote:
>It would work if EXPORT_SYMBOL_NOVERS were involved.

EXPORT_SYMBOL_NOVERS should only be used when the symbol is used in
assembler.  Asm code does not get the modversion name mangling.  If you
have to use EXPORT_SYMBOL_NOVERS for symbols in C code there is almost
ceratinly something wrong.

Re: Strange s390 code in 2.4.19-pre8

[Ulrich Weigand]

Kai Germaschewski wrote:

>Not sure if I understand you correctly. I mean it should work w/o 
>CONFIG_MODVERSIONS, and it would also work with CONFIG_MODVERSIONS if it 
>wasn't for the conflict with the ISDN fsm.o. Do we agree here?

As it is not possible to configure the ISDN fsm.o into a s390
build (and there is in fact no ISDN hardware for S/390 ;-/),
how can there be any conflict?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de

Re: Strange s390 code in 2.4.19-pre8

[Kai Germaschewski]

On Tue, 14 May 2002, Ulrich Weigand wrote:

> As it is not possible to configure the ISDN fsm.o into a s390
> build (and there is in fact no ISDN hardware for S/390 ;-/),
> how can there be any conflict?

The version strings for export symbols are generated at "make dep" time, 
which iterates over all subdirectories (well, only arch/$ARCH, but all 
the rest) without caring about config options. So generating symbols will 
conflict if $ARCH == s390.

--Kai