Re: [PATCH 3/7] PCI PM: Fix saving of device state in pci_legacy_suspend

On Thu, 2009-02-05 at 12:29 +1100, Benjamin Herrenschmidt wrote:
Yup, missed that change in the patch. In fact, I even missed the
existence of a WARN_ONCE that takes those text arguments... baaaah. I
suppose I should try to read more of lkml :-)

But don't expect to see them on powerpc, they never make it to the
console.

Allright, that's the problem with our implementation of WARN using a
conditional trap, we don't carry all the variable arguments for the
printf over.

I'm tempted to turn our implementation back to normal C code with an if
and a branch out of line, but Michael seems to say that even with
appropriate use of unlikely() etc... gcc decides to generate the worst
possible code every single time ...

<rant>
such as always inlining the whole printf mumbo jumbo and having the
not-warning case branch over it, not -all- CPUs have good branch
prediction dude, so the icache is going to scream but gcc folks know
better of course....
</rant>

Anyway, I'll see Michael what exactly the situation is here and if we
can fix it a way or another.

Well I'm glad you listened to me when I told you not to fire off a rant
email before I had another look at it ;)

Because Arjan has patched the generic bug.h so that we at least get the
printk on powerpc, and then use our version of __WARN().

So you should see the message at least.


As far as the code gen, with gcc 4.3.1 (debian).

WARN looks like:

#define WARN(condition, format...) ({ \
int __ret_warn_on = !!(condition); \
if (unlikely(__ret_warn_on)) \
__WARN_printf(format); \
unlikely(__ret_warn_on); \
})


Called from:

int smp_request_message_ipi(int virq, int msg)
{
int err;
...
err = request_irq(virq, smp_ipi_action[msg], IRQF_DISABLED|IRQF_PERCPU,
smp_ipi_name[msg], 0);
WARN(err < 0, "unable to request_irq %d for %s (rc %d)\n",
virq, smp_ipi_name[msg], err);

return err;
}


Comes out as:

c00000000002c9e8: 48 0a 88 c9 bl c0000000000d52b0 <.request_irq>
c00000000002c9ec: 60 00 00 00 nop
c00000000002c9f0: 2f 83 00 00 cmpwi cr7,r3,0
c00000000002c9f4: 7c 7f 1b 78 mr r31,r3
c00000000002c9f8: 40 bc 00 20 bge+ cr7,c00000000002ca18 <.smp_request_message_ipi+0x84>
c00000000002c9fc: 7c bc e8 2a ldx r5,r28,r29
c00000000002ca00: e8 7e 80 d0 ld r3,-32560(r30)
c00000000002ca04: 7f 64 db 78 mr r4,r27
c00000000002ca08: 7f e6 fb 78 mr r6,r31
c00000000002ca0c: 48 5f 53 7d bl c000000000621d88 <.printk>
c00000000002ca10: 60 00 00 00 nop
c00000000002ca14: 0f e0 00 00 twi 31,r0,0
c00000000002ca18: 38 21 00 a0 addi r1,r1,160
c00000000002ca1c: 7f e3 fb 78 mr r3,r31
c00000000002ca20: e8 01 00 10 ld r0,16(r1)
c00000000002ca24: eb 61 ff d8 ld r27,-40(r1)
c00000000002ca28: eb 81 ff e0 ld r28,-32(r1)
c00000000002ca2c: eb a1 ff e8 ld r29,-24(r1)
c00000000002ca30: eb c1 ff f0 ld r30,-16(r1)
c00000000002ca34: eb e1 ff f8 ld r31,-8(r1)
c00000000002ca38: 7c 08 03 a6 mtlr r0
c00000000002ca3c: 4e 80 00 20 blr


So it's jumping over the if body, rather than moving it out of the
straight line. It is hinting it right, so we should check if that helps
us at all.

cheers

--
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

Attachment: signature.asc
Description: This is a digitally signed message part

Relevant Pages

  • Re: sched.c: Remove unused variable relative
    ... The warning drowned in the usual flood of gcc warnings that a ... struct kobject *kobj) ... int res; ... int err, version; ...
    (Linux-Kernel)
  • Re: data types
    ... Usually short is smaller than an int. ... Your compiler is either *really* old or broken or both. ... recommend either a version of gcc,, or Visual Studio ... Microsoft's Visual Studio Express has an onerous EULA). ...
    (comp.lang.c)
  • Re: stream io in c
    ... zero to 255, putchar'ing in the body of the loop, then redirecting output ... int main ... // gcc -o chars mkchars1.c ... This compiles but gcc warns of incompatible pointer types. ...
    (comp.lang.c)
  • Re: gcc bug? Openoffice port impossibel to compile on 4.8
    ... remove this silly "bitten by the Linux bug" and the red-herring of gcc ... struct bar {int a; int b;} dapper; ... The *warning* emitted by gcc when enough analysis is done (e.g. ...
    (freebsd-hackers)
  • Re: Amusing C, amusing compiler
    ... int foo ... FWIW, gcc 3.3.3, when fed ... int qux(struct foo *f) { ... lda $30,-32 ...
    (comp.lang.c)
Loading