* Re: linux-next: tree build failure
From: Jan Beulich @ 2009-10-05 6:58 UTC (permalink / raw)
To: Hollis Blanchard
Cc: sfr, Rusty Russell, linux-kernel, kvm-ppc, linux-next, akpm,
linuxppc-dev
In-Reply-To: <1254498517.3839.17.camel@slab.beaverton.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 745 bytes --]
>>> Hollis Blanchard <hollisb@us.ibm.com> 02.10.09 17:48 >>>
>On Wed, 2009-09-30 at 07:35 +0100, Jan Beulich wrote:
>> The one Rusty suggested the other day may help here. I don't like it
>> as a drop-in replacement for BUILD_BUG_ON() though (due to it
>> deferring the error generated to the linking stage), I'd rather view
>> this as an improvement to MAYBE_BUILD_BUG_ON() (which should
>> then be used here).
>
>Can you be more specific?
>
>I have no idea what Rusty suggested where. I can't even guess what
I'm attaching Rusty's response I was referring to.
>MAYBE_BUILD_BUG_ON() is supposed to do (sounds like a terrible name).
Agreed - but presumably better than just deleting the bogus instances
altogether...
Jan
[-- Attachment #2: Type: message/rfc822, Size: 2578 bytes --]
From: Rusty Russell <rusty@rustcorp.com.au>
To: "Jan Beulich" <JBeulich@novell.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fix BUILD_BUG_ON() and a couple of bogus uses of it
Date: Wed, 23 Sep 2009 10:27:00 +0930
Message-ID: <200909231027.01006.rusty@rustcorp.com.au>
On Wed, 19 Aug 2009 01:29:25 am Jan Beulich wrote:
> gcc permitting variable length arrays makes the current construct
> used for BUILD_BUG_ON() useless, as that doesn't produce any diagnostic
> if the controlling expression isn't really constant. Instead, this
> patch makes it so that a bit field gets used here. Consequently, those
> uses where the condition isn't really constant now also need fixing.
>
> Note that in the gfp.h, kmemcheck.h, and virtio_config.h cases
> MAYBE_BUILD_BUG_ON() really just serves documentation purposes - even
> if the expression is compile time constant (__builtin_constant_p()
> yields true), the array is still deemed of variable length by gcc, and
> hence the whole expression doesn't have the intended effect.
>
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
We used to use an undefined symbol here; diagnostics are worse but it catches
more stuff.
Perhaps a hybrid is the way to go?
#ifndef __OPTIMIZE__
#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
#else
/* If it's a constant, catch it at compile time, otherwise at link time. */
extern int __build_bug_on_failed;
#define BUILD_BUG_ON(condition) \
do { \
((void)sizeof(char[1 - 2*!!(condition)])); \
if (condition) __build_bug_on_failed = 1; \
} while(0)
#endif
Thanks,
Rusty.
^ permalink raw reply
* Re: 2.6.31-git5 kernel boot hangs on powerpc
From: Sachin Sant @ 2009-10-05 6:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Tejun Heo, Linux/PPC Development
In-Reply-To: <1253872111.7103.520.camel@pasglop>
[-- Attachment #1: Type: text/plain, Size: 4023 bytes --]
Benjamin Herrenschmidt wrote:
> On Fri, 2009-09-25 at 18:01 +0900, Tejun Heo wrote:
>
>>> With this patch applied the machine boots OK :-)
>>>
>> Ah... so, the problem really is too high address. If you've got some
>> time, it might be interesting to find out how far high is safe.
>>
>>
> Might give me a clue about what the problem is but I think I'll just
> cook up a test case that forcibly vmap something high up and see how it
> goes from there. It could be a very old bug that nobody ever noticed
> because our vmalloc space on 64-bit is so huge :-)
>
I still have this problem with 2.6.32-rc3.
Here is the relevant information
0:mon> t
[link register ] c0000000001a7f78 .pcpu_alloc+0x798/0xa04
[c0000000033e37f0] c0000000001a7f08 .pcpu_alloc+0x728/0xa04 (unreliable)
[c0000000033e3920] c0000000001a8278 .__alloc_percpu+0x3c/0x58
[c0000000033e39b0] c0000000005d1ad0 .snmp_mib_init+0x64/0xb0
[c0000000033e3a40] c0000000005d1c00 .ipv4_mib_init_net+0xe4/0x1f8
[c0000000033e3b00] c00000000055b608 .setup_net+0x78/0x138
[c0000000033e3ba0] c00000000055be38 .copy_net_ns+0x9c/0x148
[c0000000033e3c30] c0000000000d06d8 .create_new_namespaces+0x120/0x1e4
[c0000000033e3ce0] c0000000000d09e0 .unshare_nsproxy_namespaces+0x7c/0xfc
[c0000000033e3d80] c00000000009dd74 .SyS_unshare+0x148/0x33c
[c0000000033e3e30] c0000000000085b4 syscall_exit+0x0/0x40
--- Exception: c01 (System Call) at 00000fff8b0ab978
SP (fffe633fe30) is in userspace
0:mon> e
cpu 0x0: Vector: 501 (Hardware Interrupt) at [c0000000033e3570]
pc: c00000000004bdc0: .memset+0x60/0xfc
lr: c0000000001a7f78: .pcpu_alloc+0x798/0xa04
sp: c0000000033e37f0
msr: 8000000000009032
current = 0xc000000003270860
paca = 0xc0000000010c2600
pid = 3442, comm = two_children_ns
0:mon> r
R00 = 0000000000000040 R07 = d00007fffff00000
R01 = c0000000033e37f0 R08 = 0000000000000000
R02 = c000000000fe7c78 R09 = c000000001700180
R03 = d00007fffff00000 R10 = c000000001095aa0
R04 = 0000000000000000 R11 = 00000000000003c0
R05 = 0000000000000000 R12 = 0000000048004428
R06 = d00007fffff00000 R13 = c0000000010c2600
pc = c00000000004bdc0 .memset+0x60/0xfc
lr = c0000000001a7f78 .pcpu_alloc+0x798/0xa04
msr = 8000000000009032 cr = 44004420
ctr = 0000000000000040 xer = 0000000020000020 trap = 501
0:mon> di $.memset
c00000000004bd60 7c0300d0 neg r0,r3
c00000000004bd64 5084442e rlwimi r4,r4,8,16,23
c00000000004bd68 70000007 andi. r0,r0,7
c00000000004bd6c 5084801e rlwimi r4,r4,16,0,15
c00000000004bd70 7c850040 cmplw cr1,r5,r0
c00000000004bd74 7884000e rldimi r4,r4,32,0
c00000000004bd78 7c101120 mtocrf 1,r0
c00000000004bd7c 7c661b78 mr r6,r3
c00000000004bd80 418400ac blt cr1,c00000000004be2c # .memset+0xcc/0xfc
c00000000004bd84 41e2002c beq+ c00000000004bdb0 # .memset+0x50/0xfc
c00000000004bd88 7ca02850 subf r5,r0,r5
c00000000004bd8c 409f000c bns cr7,c00000000004bd98 # .memset+0x38/0xfc
c00000000004bd90 98860000 stb r4,0(r6)
c00000000004bd94 38c60001 addi r6,r6,1
c00000000004bd98 409e000c bne cr7,c00000000004bda4 # .memset+0x44/0xfc
c00000000004bd9c b0860000 sth r4,0(r6)
0:mon>
c00000000004bda0 38c60002 addi r6,r6,2
c00000000004bda4 409d000c ble cr7,c00000000004bdb0 # .memset+0x50/0xfc
c00000000004bda8 90860000 stw r4,0(r6)
c00000000004bdac 38c60004 addi r6,r6,4
c00000000004bdb0 78a0d183 rldicl. r0,r5,58,6
c00000000004bdb4 78a506a0 clrldi r5,r5,58
c00000000004bdb8 7c0903a6 mtctr r0
c00000000004bdbc 4182002c beq c00000000004bde8 # .memset+0x88/0xfc
c00000000004bdc0 f8860000 std r4,0(r6)
At this point R06 contains d00007fffff00000.
Have attached the xmon log.
Thanks
-Sachin
--
---------------------------------
Sachin Sant
IBM Linux Technology Center
India Systems and Technology Labs
Bangalore, India
---------------------------------
[-- Attachment #2: xmonlog --]
[-- Type: text/plain, Size: 4349 bytes --]
0:mon> t
[link register ] c0000000001a7f78 .pcpu_alloc+0x798/0xa04
[c0000000033e37f0] c0000000001a7f08 .pcpu_alloc+0x728/0xa04 (unreliable)
[c0000000033e3920] c0000000001a8278 .__alloc_percpu+0x3c/0x58
[c0000000033e39b0] c0000000005d1ad0 .snmp_mib_init+0x64/0xb0
[c0000000033e3a40] c0000000005d1c00 .ipv4_mib_init_net+0xe4/0x1f8
[c0000000033e3b00] c00000000055b608 .setup_net+0x78/0x138
[c0000000033e3ba0] c00000000055be38 .copy_net_ns+0x9c/0x148
[c0000000033e3c30] c0000000000d06d8 .create_new_namespaces+0x120/0x1e4
[c0000000033e3ce0] c0000000000d09e0 .unshare_nsproxy_namespaces+0x7c/0xfc
[c0000000033e3d80] c00000000009dd74 .SyS_unshare+0x148/0x33c
[c0000000033e3e30] c0000000000085b4 syscall_exit+0x0/0x40
--- Exception: c01 (System Call) at 00000fff8b0ab978
SP (fffe633fe30) is in userspace
0:mon> e
cpu 0x0: Vector: 501 (Hardware Interrupt) at [c0000000033e3570]
pc: c00000000004bdc0: .memset+0x60/0xfc
lr: c0000000001a7f78: .pcpu_alloc+0x798/0xa04
sp: c0000000033e37f0
msr: 8000000000009032
current = 0xc000000003270860
paca = 0xc0000000010c2600
pid = 3442, comm = two_children_ns
0:mon> r
R00 = 0000000000000040 R07 = d00007fffff00000
R01 = c0000000033e37f0 R08 = 0000000000000000
R02 = c000000000fe7c78 R09 = c000000001700180
R03 = d00007fffff00000 R10 = c000000001095aa0
R04 = 0000000000000000 R11 = 00000000000003c0
R05 = 0000000000000000 R12 = 0000000048004428
R06 = d00007fffff00000 R13 = c0000000010c2600
pc = c00000000004bdc0 .memset+0x60/0xfc
lr = c0000000001a7f78 .pcpu_alloc+0x798/0xa04
msr = 8000000000009032 cr = 44004420
ctr = 0000000000000040 xer = 0000000020000020 trap = 501
0:mon> di $.memset
c00000000004bd60 7c0300d0 neg r0,r3
c00000000004bd64 5084442e rlwimi r4,r4,8,16,23
c00000000004bd68 70000007 andi. r0,r0,7
c00000000004bd6c 5084801e rlwimi r4,r4,16,0,15
c00000000004bd70 7c850040 cmplw cr1,r5,r0
c00000000004bd74 7884000e rldimi r4,r4,32,0
c00000000004bd78 7c101120 mtocrf 1,r0
c00000000004bd7c 7c661b78 mr r6,r3
c00000000004bd80 418400ac blt cr1,c00000000004be2c # .memset+0xcc/0xfc
c00000000004bd84 41e2002c beq+ c00000000004bdb0 # .memset+0x50/0xfc
c00000000004bd88 7ca02850 subf r5,r0,r5
c00000000004bd8c 409f000c bns cr7,c00000000004bd98 # .memset+0x38/0xfc
c00000000004bd90 98860000 stb r4,0(r6)
c00000000004bd94 38c60001 addi r6,r6,1
c00000000004bd98 409e000c bne cr7,c00000000004bda4 # .memset+0x44/0xfc
c00000000004bd9c b0860000 sth r4,0(r6)
0:mon>
c00000000004bda0 38c60002 addi r6,r6,2
c00000000004bda4 409d000c ble cr7,c00000000004bdb0 # .memset+0x50/0xfc
c00000000004bda8 90860000 stw r4,0(r6)
c00000000004bdac 38c60004 addi r6,r6,4
c00000000004bdb0 78a0d183 rldicl. r0,r5,58,6
c00000000004bdb4 78a506a0 clrldi r5,r5,58
c00000000004bdb8 7c0903a6 mtctr r0
c00000000004bdbc 4182002c beq c00000000004bde8 # .memset+0x88/0xfc
c00000000004bdc0 f8860000 std r4,0(r6)
c00000000004bdc4 f8860008 std r4,8(r6)
c00000000004bdc8 f8860010 std r4,16(r6)
c00000000004bdcc f8860018 std r4,24(r6)
c00000000004bdd0 f8860020 std r4,32(r6)
c00000000004bdd4 f8860028 std r4,40(r6)
c00000000004bdd8 f8860030 std r4,48(r6)
c00000000004bddc f8860038 std r4,56(r6)
0:mon> u
SLB contents of cpu 0
00 c000000008000000 40004f7ca3000500 1T ESID= c00000 VSID= 4f7ca3 LLP:100
01 d000000008000000 4000eb71b0000510 1T ESID= d00000 VSID= eb71b0 LLP:110
20 f000000008000000 4000235bcc000500 1T ESID= f00000 VSID= 235bcc LLP:100
27 00000f0008000000 400014e596000d90 1T ESID= f VSID= 14e596 LLP:110
28 0000000018000000 00004be47e859d90 256M ESID= 1 VSID= 4be47e859 LLP:110
29 d000070008000000 400026a7a5000400 1T ESID= d00007 VSID= 26a7a5 LLP: 0
30 d000080008000000 4000e5f87e000400 1T ESID= d00008 VSID= e5f87e LLP: 0
0:mon> S
msr = 8000000000001032 sprg0= 0000000000000000
pvr = 00000000003e0301 sprg1= c0000000010c2600
dec = 0000000058fe12dd sprg2= c0000000010c2600
sp = c00000000fffb8f0 sprg3= 0000000000000000
toc = c000000000fe7c78 dar = d00007fffff00000
^ permalink raw reply
* Re: [PATCH, RFC] powerpc, pci: fix MODPOST warning
From: Heiko Schocher @ 2009-10-05 7:06 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <4ABB69DA.7030306@denx.de>
Hello,
Heiko Schocher wrote:
> making a powerpc target with PCI support, shows the
> following warning:
>
> MODPOST vmlinux.o
> WARNING: vmlinux.o(.text+0x10430): Section mismatch in reference from the function pcibios_allocate_bus_resources() to the function .init.text:reparent_resources()
> The function pcibios_allocate_bus_resources() references
> the function __init reparent_resources().
> This is often because pcibios_allocate_bus_resources lacks a __init
> annotation or the annotation of reparent_resources is wrong.
>
> This patch fix this warning by removing the __init
> annotation before reparent_resources.
No comments? So, is this fix OK, or unusable?
bye
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply
* Re: [PATCH] sound: Don't assume i2c device probing always succeeds
From: Takashi Iwai @ 2009-10-05 5:30 UTC (permalink / raw)
To: Jean Delvare
Cc: linuxppc-dev, Johannes Berg, Tim Shepard, alsa-devel,
Jaroslav Kysela
In-Reply-To: <20091004113521.6a0c1dbb@hyperion.delvare>
At Sun, 4 Oct 2009 11:35:21 +0200,
Jean Delvare wrote:
>
> Hi Takashi,
>
> On Thu, 01 Oct 2009 08:52:59 +0200, Takashi Iwai wrote:
> > At Wed, 30 Sep 2009 18:55:05 +0200,
> > Jean Delvare wrote:
> > >
> > > On Wed, 30 Sep 2009 17:15:49 +0200, Takashi Iwai wrote:
> > > > Yes, indeed I prefer NULL check because the user can know the error
> > > > at the right place. I share your concern about the code addition,
> > > > though :)
> > > >
> > > > I already made a patch below, but it's totally untested.
> > > > It'd be helpful if someone can do review and build-test it.
> > > >
> > > >
> > > > thanks,
> > > >
> > > > Takashi
> > > >
> > > > ---
> > > > diff --git a/sound/aoa/codecs/tas.c b/sound/aoa/codecs/tas.c
> > > > index f0ebc97..0f810c8 100644
> > > > --- a/sound/aoa/codecs/tas.c
> > > > +++ b/sound/aoa/codecs/tas.c
> > > > @@ -897,6 +897,10 @@ static int tas_create(struct i2c_adapter *adapter,
> > > > client = i2c_new_device(adapter, &info);
> > > > if (!client)
> > > > return -ENODEV;
> > > > + if (!client->driver) {
> > > > + i2c_unregister_device(client);
> > > > + return -ENODEV;
> > > > + }
> > > >
> > > > /*
> > > > * Let i2c-core delete that device on driver removal.
> > > > diff --git a/sound/ppc/keywest.c b/sound/ppc/keywest.c
> > > > index 835fa19..473c5a6 100644
> > > > --- a/sound/ppc/keywest.c
> > > > +++ b/sound/ppc/keywest.c
> > > > @@ -59,6 +59,13 @@ static int keywest_attach_adapter(struct i2c_adapter *adapter)
> > > > strlcpy(info.type, "keywest", I2C_NAME_SIZE);
> > > > info.addr = keywest_ctx->addr;
> > > > keywest_ctx->client = i2c_new_device(adapter, &info);
> > > > + if (!keywest_ctx->client)
> > > > + return -ENODEV;
> > > > + if (!keywest_ctx->client->driver) {
> > > > + i2c_unregister_device(keywest_ctx->client);
> > > > + keywest_ctx->client = NULL;
> > > > + return -ENODEV;
> > > > + }
> > > >
> > > > /*
> > > > * Let i2c-core delete that device on driver removal.
> > >
> > > This looks good to me. Please add the following comment before the
> > > client->driver check in both drivers:
> > >
> > > /*
> > > * We know the driver is already loaded, so the device should be
> > > * already bound. If not it means binding failed, and then there
> > > * is no point in keeping the device instantiated.
> > > */
> > >
> > > Otherwise it's a little difficult to understand why the check is there.
> >
> > Fair enough. I applied the patch with the comment now.
> > Thanks!
>
> I see this is upstream now. While the keywest fix was essentially
> theoretical, the tas one addresses a crash which really could happen,
> so I think it would be worth sending to stable for 2.6.31. What do you
> think? Will you take care, or do you want me to?
Agreed, it's safer to send the patch to stable tree.
I'm going to submit it.
thanks,
Takashi
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Joakim Tjernlund @ 2009-10-04 20:45 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <1254688118.7122.30.camel@pasglop>
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 04/10/2009 22:28:38:
>
> > I have managed to update the TLB code to make proper use of dirty and accessed states.
> > Advantages are:
> > - I/D TLB Miss never needs to write to the linux pte, saving a few cycles
>
> That's good, that leaves us with only 40x to fix now. Also we can remove
> atomic updates of PTEs for all non-hash. It's pointless on those CPUs
> anyway.
>
> > - Accessed is only set by I/D TLB Error, should be a plus when SWAP is used.
>
> No need for that neither.
Since 8xx lacks HW support for ACCESSED, the only way is map
the page NoAccess and take a TLB Error on first access that sets
access bit (or bails to do_page_fault)
>
> ISI/DSI shouldn't touch the PTE. They should just fall back to C code
> which takes care of it all.l
Yes, that is what I do now(i.e I only read the pte). ISI and DSI is the
TLB Miss handlers on 8xx.
>
> > - _PAGE_DIRTY is mapped to 0x100, the changed bit, and is set directly
> > and there will be no extra DTLB Error to actually set the changed bit
> > when a page has been made dirty.
> > - Proper RO/RW mapping of user space.
> >
> > Cons:
> > - 4 more insn in TLB Miss handlers, but the since the linux pte isn't
> > written it should still be a win.
> >
> > However, I did this on my 2.4 tree but I can port it to 2.6 if you guys
> > can test it for me.
>
> Why don't you use and test 2.6 ? :-)
Because porting my 8xx board to 2.6 isn't going to be easy so
I havn't yet. One day I might when we can't get away with 2.4 on our
old boards.
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Joakim Tjernlund @ 2009-10-04 20:38 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <1254688002.7122.28.camel@pasglop>
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 04/10/2009 22:26:42:
> On Sun, 2009-10-04 at 10:35 +0200, Joakim Tjernlund wrote:
> > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 03/10/2009 12:57:28:
> > >
> > > On Sat, 2009-10-03 at 11:24 +0200, Joakim Tjernlund wrote:
> > > >
> > > > So yes, there is a missing _tlbil_va() missing for 8xx somewhere
> > > > but there is something more too.
> > > > Maybe your new filter functions and my
> > > > powerpc, 8xx: DTLB Error must check for more errors.
> > > > will do the trick?
> > >
> > > Well, if we can't tell between a load and a store on a TLB miss, then
> > > we should probably let it create an unpopulated entry in all cases,
> > > so that we do take a proper DSI/ISI the second time around, which
> > > would then tell us where we come from...
> >
> > While looking closer on 8xx TLB handling I noticed we were taking
> > an extra TLB Error when making a page dirty. Tracked it down to:
> > static inline pte_t pte_mkdirty(pte_t pte) {
> > pte_val(pte) |= _PAGE_DIRTY; return pte; }
> > pte_mkdirty does not set HWWRITE thus forcing a new
> > TLB error to clear it. Adding HWWRITE to pte_mkdirty fixes the
> > problem.
>
> We should just get rid of HWWRITE like I did for 44x and BookE.
I am trying :)
>
> > Looking at (especially pte_mkclean):
> > static inline pte_t pte_wrprotect(pte_t pte) {
> > pte_val(pte) &= ~(_PAGE_RW | _PAGE_HWWRITE); return pte; }
> > static inline pte_t pte_mkclean(pte_t pte) {
> > pte_val(pte) &= ~(_PAGE_DIRTY | _PAGE_HWWRITE); return pte; }
> >
> > it looks like a mistake not to include HWWRITE in pte_mkdirty(),
> > what do you think?
>
> Maybe yes. No big deal right now, we have more important problems
> to fix no ? :-)
The missing invalidate you guys need to work out. I have no clue
where to put it. If we are really lucky, getting rid of HWWRITE
might help :)
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Benjamin Herrenschmidt @ 2009-10-04 20:28 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: Scott Wood, linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <OF7C895670.A7F16A1A-ONC1257645.00620A4D-C1257645.006ED40F@transmode.se>
> I have managed to update the TLB code to make proper use of dirty and accessed states.
> Advantages are:
> - I/D TLB Miss never needs to write to the linux pte, saving a few cycles
That's good, that leaves us with only 40x to fix now. Also we can remove
atomic updates of PTEs for all non-hash. It's pointless on those CPUs
anyway.
> - Accessed is only set by I/D TLB Error, should be a plus when SWAP is used.
No need for that neither.
ISI/DSI shouldn't touch the PTE. They should just fall back to C code
which takes care of it all.l
> - _PAGE_DIRTY is mapped to 0x100, the changed bit, and is set directly
> and there will be no extra DTLB Error to actually set the changed bit
> when a page has been made dirty.
> - Proper RO/RW mapping of user space.
>
> Cons:
> - 4 more insn in TLB Miss handlers, but the since the linux pte isn't
> written it should still be a win.
>
> However, I did this on my 2.4 tree but I can port it to 2.6 if you guys
> can test it for me.
Why don't you use and test 2.6 ? :-)
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Benjamin Herrenschmidt @ 2009-10-04 20:26 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: Scott Wood, linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <OF1CF373A5.3E3C833B-ONC1257645.002E46F1-C1257645.002F3070@transmode.se>
On Sun, 2009-10-04 at 10:35 +0200, Joakim Tjernlund wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 03/10/2009 12:57:28:
> >
> > On Sat, 2009-10-03 at 11:24 +0200, Joakim Tjernlund wrote:
> > >
> > > So yes, there is a missing _tlbil_va() missing for 8xx somewhere
> > > but there is something more too.
> > > Maybe your new filter functions and my
> > > powerpc, 8xx: DTLB Error must check for more errors.
> > > will do the trick?
> >
> > Well, if we can't tell between a load and a store on a TLB miss, then
> > we should probably let it create an unpopulated entry in all cases,
> > so that we do take a proper DSI/ISI the second time around, which
> > would then tell us where we come from...
>
> While looking closer on 8xx TLB handling I noticed we were taking
> an extra TLB Error when making a page dirty. Tracked it down to:
> static inline pte_t pte_mkdirty(pte_t pte) {
> pte_val(pte) |= _PAGE_DIRTY; return pte; }
> pte_mkdirty does not set HWWRITE thus forcing a new
> TLB error to clear it. Adding HWWRITE to pte_mkdirty fixes the
> problem.
We should just get rid of HWWRITE like I did for 44x and BookE.
> Looking at (especially pte_mkclean):
> static inline pte_t pte_wrprotect(pte_t pte) {
> pte_val(pte) &= ~(_PAGE_RW | _PAGE_HWWRITE); return pte; }
> static inline pte_t pte_mkclean(pte_t pte) {
> pte_val(pte) &= ~(_PAGE_DIRTY | _PAGE_HWWRITE); return pte; }
>
> it looks like a mistake not to include HWWRITE in pte_mkdirty(),
> what do you think?
Maybe yes. No big deal right now, we have more important problems
to fix no ? :-)
Ben.
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Joakim Tjernlund @ 2009-10-04 20:10 UTC (permalink / raw)
To: Scott Wood, Rex Feany; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <20091002214949.GA20514@b07421-ec1.am.freescale.net>
Scott Wood <scottwood@freescale.com> wrote on 02/10/2009 23:49:49:
>
> On Thu, Oct 01, 2009 at 08:35:59AM +1000, Benjamin Herrenschmidt wrote:
> > >From what I can see, the TLB miss code will check _PAGE_PRESENT, and
> > when not set, it will -still- insert something into the TLB (unlike
> > all other CPU types that go straight to data access faults from there).
> >
> > So we end up populating with those unpopulated entries that will then
> > cause us to take a DSI (or ISI) instead of a TLB miss the next time
> > around ... and so we need to remove them once we do that, no ? IE. Once
> > we have actually put a valid PTE in.
> >
> > At least that's my understanding and why I believe we should always have
> > tlbil_va() in set_pte() and ptep_set_access_flags(), which boils down
> > in putting it into the 2 "filter" functions in the new code.
> >
> > Well.. actually, the ptep_set_access_flags() will already do a
> > flush_tlb_page_nohash() when the PTE is changed. So I suppose all we
> > really need here would be in set_pte_filter(), to do the same if we are
> > writing a PTE that has _PAGE_PRESENT, at least on 8xx.
> >
> > But just unconditionally doing a tlbil_va() in both the filter functions
> > shouldn't harm and also fix the problem, though Rex seems to indicate
> > that is not the case.
>
> Adding a tlbil_va to do_page_fault makes the problem go away for me (on
> top of your "merge" branch) -- none of the other changes in this thread
> do (assuming I didn't miss any). FWIW, when it gets stuck on a fault,
> DSISR is 0xc0000000, and handle_mm_fault returns zero.
Scott and Rex
I have managed to update the TLB code to make proper use of dirty and accessed states.
Advantages are:
- I/D TLB Miss never needs to write to the linux pte, saving a few cycles
- Accessed is only set by I/D TLB Error, should be a plus when SWAP is used.
- _PAGE_DIRTY is mapped to 0x100, the changed bit, and is set directly
and there will be no extra DTLB Error to actually set the changed bit
when a page has been made dirty.
- Proper RO/RW mapping of user space.
Cons:
- 4 more insn in TLB Miss handlers, but the since the linux pte isn't
written it should still be a win.
However, I did this on my 2.4 tree but I can port it to 2.6 if you guys
can test it for me.
Jocke
^ permalink raw reply
* Re: [patch 1/3] powerpc: Move 64bit VDSO to improve context switch performance
From: Andreas Schwab @ 2009-10-04 12:35 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Anton Blanchard
In-Reply-To: <1254606807.7122.23.camel@pasglop>
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> Maybe a better fix is to force alignment in the kernel by requesting
> size + 64k - 4k and aligning it.
Sure you are right.
Andreas.
>From b9441a3d2148d439e2730def3222a7b70dccc432 Mon Sep 17 00:00:00 2001
From: Andreas Schwab <schwab@linux-m68k.org>
Date: Sun, 4 Oct 2009 14:29:04 +0200
Subject: [PATCH] powerpc: align vDSO base address
Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
---
arch/powerpc/kernel/vdso.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index 94e2df3..137dc22 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -50,6 +50,9 @@
/* Max supported size for symbol names */
#define MAX_SYMNAME 64
+/* The alignment of the vDSO */
+#define VDSO_ALIGNMENT (1 << 16)
+
extern char vdso32_start, vdso32_end;
static void *vdso32_kbase = &vdso32_start;
static unsigned int vdso32_pages;
@@ -231,15 +234,21 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
* pick a base address for the vDSO in process space. We try to put it
* at vdso_base which is the "natural" base for it, but we might fail
* and end up putting it elsewhere.
+ * Add enough to the size so that the result can be aligned.
*/
down_write(&mm->mmap_sem);
vdso_base = get_unmapped_area(NULL, vdso_base,
- vdso_pages << PAGE_SHIFT, 0, 0);
+ (vdso_pages << PAGE_SHIFT) +
+ ((VDSO_ALIGNMENT - 1) & PAGE_MASK),
+ 0, 0);
if (IS_ERR_VALUE(vdso_base)) {
rc = vdso_base;
goto fail_mmapsem;
}
+ /* Add required alignment. */
+ vdso_base = ALIGN(vdso_base, VDSO_ALIGNMENT);
+
/*
* Put vDSO base into mm struct. We need to do this before calling
* install_special_mapping or the perf counter mmap tracking code
--
1.6.4.4
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply related
* Re: [PATCH] sound: Don't assume i2c device probing always succeeds
From: Jean Delvare @ 2009-10-04 9:35 UTC (permalink / raw)
To: Takashi Iwai
Cc: linuxppc-dev, Johannes Berg, Tim Shepard, alsa-devel,
Jaroslav Kysela
In-Reply-To: <s5hiqezk3lg.wl%tiwai@suse.de>
Hi Takashi,
On Thu, 01 Oct 2009 08:52:59 +0200, Takashi Iwai wrote:
> At Wed, 30 Sep 2009 18:55:05 +0200,
> Jean Delvare wrote:
> >
> > On Wed, 30 Sep 2009 17:15:49 +0200, Takashi Iwai wrote:
> > > Yes, indeed I prefer NULL check because the user can know the error
> > > at the right place. I share your concern about the code addition,
> > > though :)
> > >
> > > I already made a patch below, but it's totally untested.
> > > It'd be helpful if someone can do review and build-test it.
> > >
> > >
> > > thanks,
> > >
> > > Takashi
> > >
> > > ---
> > > diff --git a/sound/aoa/codecs/tas.c b/sound/aoa/codecs/tas.c
> > > index f0ebc97..0f810c8 100644
> > > --- a/sound/aoa/codecs/tas.c
> > > +++ b/sound/aoa/codecs/tas.c
> > > @@ -897,6 +897,10 @@ static int tas_create(struct i2c_adapter *adapter,
> > > client = i2c_new_device(adapter, &info);
> > > if (!client)
> > > return -ENODEV;
> > > + if (!client->driver) {
> > > + i2c_unregister_device(client);
> > > + return -ENODEV;
> > > + }
> > >
> > > /*
> > > * Let i2c-core delete that device on driver removal.
> > > diff --git a/sound/ppc/keywest.c b/sound/ppc/keywest.c
> > > index 835fa19..473c5a6 100644
> > > --- a/sound/ppc/keywest.c
> > > +++ b/sound/ppc/keywest.c
> > > @@ -59,6 +59,13 @@ static int keywest_attach_adapter(struct i2c_adapter *adapter)
> > > strlcpy(info.type, "keywest", I2C_NAME_SIZE);
> > > info.addr = keywest_ctx->addr;
> > > keywest_ctx->client = i2c_new_device(adapter, &info);
> > > + if (!keywest_ctx->client)
> > > + return -ENODEV;
> > > + if (!keywest_ctx->client->driver) {
> > > + i2c_unregister_device(keywest_ctx->client);
> > > + keywest_ctx->client = NULL;
> > > + return -ENODEV;
> > > + }
> > >
> > > /*
> > > * Let i2c-core delete that device on driver removal.
> >
> > This looks good to me. Please add the following comment before the
> > client->driver check in both drivers:
> >
> > /*
> > * We know the driver is already loaded, so the device should be
> > * already bound. If not it means binding failed, and then there
> > * is no point in keeping the device instantiated.
> > */
> >
> > Otherwise it's a little difficult to understand why the check is there.
>
> Fair enough. I applied the patch with the comment now.
> Thanks!
I see this is upstream now. While the keywest fix was essentially
theoretical, the tas one addresses a crash which really could happen,
so I think it would be worth sending to stable for 2.6.31. What do you
think? Will you take care, or do you want me to?
Thanks,
--
Jean Delvare
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Joakim Tjernlund @ 2009-10-04 8:35 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <1254567448.7122.8.camel@pasglop>
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 03/10/2009 12:57:28:
>
> On Sat, 2009-10-03 at 11:24 +0200, Joakim Tjernlund wrote:
> >
> > So yes, there is a missing _tlbil_va() missing for 8xx somewhere
> > but there is something more too.
> > Maybe your new filter functions and my
> > powerpc, 8xx: DTLB Error must check for more errors.
> > will do the trick?
>
> Well, if we can't tell between a load and a store on a TLB miss, then
> we should probably let it create an unpopulated entry in all cases,
> so that we do take a proper DSI/ISI the second time around, which
> would then tell us where we come from...
While looking closer on 8xx TLB handling I noticed we were taking
an extra TLB Error when making a page dirty. Tracked it down to:
static inline pte_t pte_mkdirty(pte_t pte) {
pte_val(pte) |= _PAGE_DIRTY; return pte; }
pte_mkdirty does not set HWWRITE thus forcing a new
TLB error to clear it. Adding HWWRITE to pte_mkdirty fixes the
problem.
Looking at (especially pte_mkclean):
static inline pte_t pte_wrprotect(pte_t pte) {
pte_val(pte) &= ~(_PAGE_RW | _PAGE_HWWRITE); return pte; }
static inline pte_t pte_mkclean(pte_t pte) {
pte_val(pte) &= ~(_PAGE_DIRTY | _PAGE_HWWRITE); return pte; }
it looks like a mistake not to include HWWRITE in pte_mkdirty(),
what do you think?
Jocke
^ permalink raw reply
* Re: [patch 1/3] powerpc: Move 64bit VDSO to improve context switch performance
From: Benjamin Herrenschmidt @ 2009-10-03 21:53 UTC (permalink / raw)
To: Andreas Schwab; +Cc: linuxppc-dev, Anton Blanchard
In-Reply-To: <m2ocoo1qfi.fsf@igel.home>
On Sat, 2009-10-03 at 16:51 +0200, Andreas Schwab wrote:
> Andreas Schwab <schwab@linux-m68k.org> writes:
>
> > Andreas Schwab <schwab@linux-m68k.org> writes:
> >
> >> Anton Blanchard <anton@samba.org> writes:
> >>
> >>> On 64bit applications the VDSO is the only thing in segment 0. Since the VDSO
> >>> is position independent we can remove the hint and let get_unmapped_area pick
> >>> an area.
> >>
> >> This breaks gdb. The section table in the VDSO image when mapped into
> >> the process no longer contains meaningful values, and gdb rejects it.
> >
> > The problem is that the load segment requires 64k alignment, but the
> > page allocater of course only provides PAGE_SIZE alignment, causing the
> > image to be unaligned in memory.
>
> Here is a patch. The disadvantage is that now gdb complains that the
> vdso does not contain section headers, because they are no longer
> covered by the load segment.
Maybe a better fix is to force alignment in the kernel by requesting
size + 64k - 4k and aligning it.
Ben.
> Andreas.
> ---
> >From 003c323c753d8717e58220c9560329882bcfa2c2 Mon Sep 17 00:00:00 2001
> From: Andreas Schwab <schwab@linux-m68k.org>
> Date: Sat, 3 Oct 2009 16:46:45 +0200
> Subject: [PATCH] powerpc: fix unaligned load segment in vdso64
>
> Since the page allocator can only guarantee PAGE_SIZE alignment the load
> segment in the vdso object cannot claim more than that alignment. Use 4k
> alignment which is the minimum possible page size.
>
> Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
> ---
> arch/powerpc/kernel/vdso64/Makefile | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
> index 79da65d..d4be303 100644
> --- a/arch/powerpc/kernel/vdso64/Makefile
> +++ b/arch/powerpc/kernel/vdso64/Makefile
> @@ -10,7 +10,9 @@ obj-vdso64 := $(addprefix $(obj)/, $(obj-vdso64))
> GCOV_PROFILE := n
>
> EXTRA_CFLAGS := -shared -fno-common -fno-builtin
> +# Force maximum page size of 4k so that it works with any page size
> EXTRA_CFLAGS += -nostdlib -Wl,-soname=linux-vdso64.so.1 \
> + -Wl,-z,max-page-size=4096 \
> $(call cc-ldoption, -Wl$(comma)--hash-style=sysv)
> EXTRA_AFLAGS := -D__VDSO64__ -s
>
> --
> 1.6.4.4
>
>
^ permalink raw reply
* Re: [PATCH] powerpc/5200: make BestComm gen_bd microcode exchangeable
From: Albrecht Dreß @ 2009-10-03 15:09 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linuxppc-dev
In-Reply-To: <20091003144026.GA27519@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 857 bytes --]
Am 03.10.09 16:40 schrieb(en) Wolfram Sang:
> Sorry, I hardly know anything about the microcode. From what I know,
> it shouldn't be much fun due to various bugs in the Bestcomm engine.
Ummm. That's not encouraging! :-/
> Hey! No need for insults! ;)
Sorry, that wasn't my intention, but it's a MUA I *know* it doesn't
understand it... most OSS clients do (like Balsa, Thunderbird, kmail,
etc.).
> > Or is '3676 in general not acceptable for patches?
>
> Maybe. Could you try without next time, and we will see if it works
> better?
Actually, it's in Documentation/email-clients.txt - '3676 must not be
used. Sorry, should have read that before! (However, messages
*without* folding *and* with lines > 78 characters violate RFC2822...)
> Yup, since -rc1. (git log --grep=mtd-ram)
Thanks!
Cheers, Albrecht.
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [patch 1/3] powerpc: Move 64bit VDSO to improve context switch performance
From: Andreas Schwab @ 2009-10-03 14:51 UTC (permalink / raw)
To: Anton Blanchard; +Cc: linuxppc-dev
In-Reply-To: <m28wfsef79.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> Anton Blanchard <anton@samba.org> writes:
>>
>>> On 64bit applications the VDSO is the only thing in segment 0. Since the VDSO
>>> is position independent we can remove the hint and let get_unmapped_area pick
>>> an area.
>>
>> This breaks gdb. The section table in the VDSO image when mapped into
>> the process no longer contains meaningful values, and gdb rejects it.
>
> The problem is that the load segment requires 64k alignment, but the
> page allocater of course only provides PAGE_SIZE alignment, causing the
> image to be unaligned in memory.
Here is a patch. The disadvantage is that now gdb complains that the
vdso does not contain section headers, because they are no longer
covered by the load segment.
Andreas.
---
>From 003c323c753d8717e58220c9560329882bcfa2c2 Mon Sep 17 00:00:00 2001
From: Andreas Schwab <schwab@linux-m68k.org>
Date: Sat, 3 Oct 2009 16:46:45 +0200
Subject: [PATCH] powerpc: fix unaligned load segment in vdso64
Since the page allocator can only guarantee PAGE_SIZE alignment the load
segment in the vdso object cannot claim more than that alignment. Use 4k
alignment which is the minimum possible page size.
Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
---
arch/powerpc/kernel/vdso64/Makefile | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
index 79da65d..d4be303 100644
--- a/arch/powerpc/kernel/vdso64/Makefile
+++ b/arch/powerpc/kernel/vdso64/Makefile
@@ -10,7 +10,9 @@ obj-vdso64 := $(addprefix $(obj)/, $(obj-vdso64))
GCOV_PROFILE := n
EXTRA_CFLAGS := -shared -fno-common -fno-builtin
+# Force maximum page size of 4k so that it works with any page size
EXTRA_CFLAGS += -nostdlib -Wl,-soname=linux-vdso64.so.1 \
+ -Wl,-z,max-page-size=4096 \
$(call cc-ldoption, -Wl$(comma)--hash-style=sysv)
EXTRA_AFLAGS := -D__VDSO64__ -s
--
1.6.4.4
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply related
* Re: [PATCH] powerpc/5200: make BestComm gen_bd microcode exchangeable
From: Wolfram Sang @ 2009-10-03 14:40 UTC (permalink / raw)
To: Albrecht Dreß; +Cc: linuxppc-dev
In-Reply-To: <1254577821.3331.0@antares>
[-- Attachment #1: Type: text/plain, Size: 1304 bytes --]
> doesn't give any support, so it's all trial and error and error and
> error... If you have any idea, pointers would be appreciated!
Sorry, I hardly know anything about the microcode. From what I know, it
shouldn't be much fun due to various bugs in the Bestcomm engine.
> I don't think so - the message body is a "format=flowed" (RFC 3676)
> text/plain part. Maybe your MUA doesn't display that format correctly
> (Outlook?)?
Hey! No need for insults! ;)
Have to check if my MUA (see headers) supports it. The archives seem to have
split it as well:
http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-October/076236.html
I recall a similar problem, where some archives had the patch correct and some
didn't.
> Or is '3676 in general not acceptable for patches?
Maybe. Could you try without next time, and we will see if it works better?
> P.S.: Did your mtd-ram/physmap_of patch you submitted a while ago make
> it into the stock kernel? I have some pending extra work (16-bit LPB
> RAM access) building on top of it...
Yup, since -rc1. (git log --grep=mtd-ram)
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [patch 1/3] powerpc: Move 64bit VDSO to improve context switch performance
From: Andreas Schwab @ 2009-10-03 14:15 UTC (permalink / raw)
To: Anton Blanchard; +Cc: linuxppc-dev
In-Reply-To: <m2ocophale.fsf__34527.2158309401$1254511503$gmane$org@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> Anton Blanchard <anton@samba.org> writes:
>
>> On 64bit applications the VDSO is the only thing in segment 0. Since the VDSO
>> is position independent we can remove the hint and let get_unmapped_area pick
>> an area.
>
> This breaks gdb. The section table in the VDSO image when mapped into
> the process no longer contains meaningful values, and gdb rejects it.
The problem is that the load segment requires 64k alignment, but the
page allocater of course only provides PAGE_SIZE alignment, causing the
image to be unaligned in memory.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH] powerpc/5200: make BestComm gen_bd microcode exchangeable
From: Albrecht Dreß @ 2009-10-03 13:50 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linuxppc-dev
In-Reply-To: <20091003094417.GA24206@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1258 bytes --]
Hi Wolfram:
Am 03.10.09 11:44 schrieb(en) Wolfram Sang:
> you wrote your own microcode? :)
I modified the bcom_gen_bd_rx_task for a LPB peripheral as to perform
Endianess swapping during the transfer (works meanwhile :-).
Modifying the standard kernel code for testing seemed to be the wrong
approach for me, so I wrote that patch...
If possible, I would also like to have a crc32 calculation performed by
BestComm, but unfortunately there is no documentation and Freescale
doesn't give any support, so it's all trial and error and error and
error... If you have any idea, pointers would be appreciated!
> approach looks ok to me in general, but this patch is line-wrapped.
I don't think so - the message body is a "format=flowed" (RFC 3676)
text/plain part. Maybe your MUA doesn't display that format correctly
(Outlook?)? Or is '3676 in general not acceptable for patches?
[snip]
> spaces instead of tabs
[snip]
> Two empty lines.
Thanks - will submit a fixed patch next week, stay tuned...
Cheers, Albrecht.
P.S.: Did your mtd-ram/physmap_of patch you submitted a while ago make
it into the stock kernel? I have some pending extra work (16-bit LPB
RAM access) building on top of it...
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Joakim Tjernlund @ 2009-10-03 11:47 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <1254567448.7122.8.camel@pasglop>
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 03/10/2009 12:57:28:
> On Sat, 2009-10-03 at 11:24 +0200, Joakim Tjernlund wrote:
> >
> > So yes, there is a missing _tlbil_va() missing for 8xx somewhere
> > but there is something more too.
> > Maybe your new filter functions and my
> > powerpc, 8xx: DTLB Error must check for more errors.
> > will do the trick?
>
> Well, if we can't tell between a load and a store on a TLB miss, then
> we should probably let it create an unpopulated entry in all cases,
> so that we do take a proper DSI/ISI the second time around, which
> would then tell us where we come from...
Not quite sure what you mean here?
Always branch to DSI at TLB Miss and create a unpopulated
entry? That does not feel right.
The current method is better. The only odd thing is
the null pmd entry and what to load into RPN. I am not convinced
that it is a problem but I would like to see a generic impl. of
a null pmd and use that instead. A 8xx impl. of that
looks like this(tested on 2.4, works fine)
lwz r11, 0(r10) /* Get the level 1 entry */
rlwinm. r10, r11,0,0,19 /* Extract page descriptor page address */
bne+ 4f /* If zero, use a null pmd */
lis r11, null_pmd_dir@h
ori r11, r11, null_pmd_dir@l
4: tophys(r11,r11)
...
.data
.globl null_pmd_dir
null_pmd_dir:
.space 4096
If the pmd_none() and friends are updated to test for a null_pmd_dir
we can loose the test above and save 4 insn :)
Anyhow did you post a patch(can't find one) about your suggested
filter functions for 8xx? I sure would like to see one :)
^ permalink raw reply
* Re: [PATCH] powerpc: fix segment mapping in vdso32
From: Benjamin Herrenschmidt @ 2009-10-03 11:22 UTC (permalink / raw)
To: Andreas Schwab; +Cc: linuxppc-dev
In-Reply-To: <m2bpkoomlw.fsf@whitebox.home>
On Sat, 2009-10-03 at 11:25 +0200, Andreas Schwab wrote:
> Due to missing segment assignments the .text section was put in the NOTES
> segment (and marked as NOTE section), and the .got was put in the DYNAMIC
> segment.
Ouch, good catch ! Thanks.
Cheers,
Ben.
> Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
> ---
> arch/powerpc/kernel/vdso32/vdso32.lds.S | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/vdso32/vdso32.lds.S b/arch/powerpc/kernel/vdso32/vdso32.lds.S
> index 904ef13..0546bcd 100644
> --- a/arch/powerpc/kernel/vdso32/vdso32.lds.S
> +++ b/arch/powerpc/kernel/vdso32/vdso32.lds.S
> @@ -25,7 +25,7 @@ SECTIONS
> . = ALIGN(16);
> .text : {
> *(.text .stub .text.* .gnu.linkonce.t.* __ftr_alt_*)
> - }
> + } :text
> PROVIDE(__etext = .);
> PROVIDE(_etext = .);
> PROVIDE(etext = .);
> @@ -56,7 +56,7 @@ SECTIONS
> .fixup : { *(.fixup) }
>
> .dynamic : { *(.dynamic) } :text :dynamic
> - .got : { *(.got) }
> + .got : { *(.got) } :text
> .plt : { *(.plt) }
>
> _end = .;
> --
> 1.6.4.4
>
^ permalink raw reply
* Re: Is volatile always verboten for FSL QE structures?
From: Benjamin Herrenschmidt @ 2009-10-03 11:20 UTC (permalink / raw)
To: Simon Richter; +Cc: linuxppc-dev
In-Reply-To: <20091003095537.GA15992@honey.hogyros.de>
> Making the target of foo volatile properly rechecks the condition on
> each iteration.
>
> OTOH my PPC box runs fine, so I'm probably missing something obvious.
Probably because the IO accessors do -both- volatile casts and
add the barriers :-)
Ben.
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Benjamin Herrenschmidt @ 2009-10-03 10:57 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: Scott Wood, linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <OF0216D5A2.8B77DD30-ONC1257644.0031FDC4-C1257644.0033A760@transmode.se>
On Sat, 2009-10-03 at 11:24 +0200, Joakim Tjernlund wrote:
>
> So yes, there is a missing _tlbil_va() missing for 8xx somewhere
> but there is something more too.
> Maybe your new filter functions and my
> powerpc, 8xx: DTLB Error must check for more errors.
> will do the trick?
Well, if we can't tell between a load and a store on a TLB miss, then
we should probably let it create an unpopulated entry in all cases,
so that we do take a proper DSI/ISI the second time around, which
would then tell us where we come from...
Ben.
^ permalink raw reply
* Re: Is volatile always verboten for FSL QE structures?
From: Simon Richter @ 2009-10-03 9:55 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20091002200848.06be4c5a@xilun.lan.proformatique.com>
Hi,
> > >> 'volatile' just doesn't really do what you think it should do. The
> > >> PowerPC architecture is too complicated w.r.t. ordering of reads and
> > >> writes. In other words, you can't trust it.
It's not sufficient on PowerPC.
It might be necessary, depending on the compiler's mood for moving stuff
out of loops.
Consider:
| unsigned int *foo = (unsigned int *)0x12345678;
| void bar(void) { while(*foo != 0) asm("eieio"); }
gcc 4.3.4 with -O3 compiles this to
00000000 <bar>:
0: 3d 20 00 00 lis r9,0
2: R_PPC_ADDR16_HA foo
4: 81 69 00 00 lwz r11,0(r9)
6: R_PPC_ADDR16_LO foo
8: 80 0b 00 00 lwz r0,0(r11)
c: 2f 80 00 00 cmpwi cr7,r0,0
10: 4d 9e 00 20 beqlr cr7
14: 7c 00 06 ac eieio
18: 7c 00 06 ac eieio
1c: 4b ff ff f8 b 14 <bar+0x14>
Making the target of foo volatile properly rechecks the condition on
each iteration.
OTOH my PPC box runs fine, so I'm probably missing something obvious.
Simon
^ permalink raw reply
* Re: [PATCH] powerpc/5200: make BestComm gen_bd microcode exchangeable
From: Wolfram Sang @ 2009-10-03 9:44 UTC (permalink / raw)
To: Albrecht Dreß; +Cc: linuxppc-dev
In-Reply-To: <1254426938.3252.1@antares>
[-- Attachment #1: Type: text/plain, Size: 3948 bytes --]
Hi Albrecht,
you wrote your own microcode? :)
approach looks ok to me in general, but this patch is line-wrapped.
On Thu, Oct 01, 2009 at 09:55:38PM +0200, Albrecht Dreß wrote:
> This patch adds a method for defining different microcodes than the
> pe-defined ones for the MPC52xx processor's BestComm General Buffer
pre-defined
> Descriptor (gen_db) tasks. The default microcode is still the one from
> bcom_gen_bd_[rt]x_task, but it can be replaced by calling
> bcom_gen_bd_set_microcode() which is more efficient than explicitly
> loading it via bcom_load_image() after each bcom_gen_bd_[rt]x_reset().
>
> Signed-off-by: Albrecht Dreß <albrecht.dress@arcor.de>
>
>
> ---
>
>
> diff -uprN -X linux-2.6.30.3/Documentation/dontdiff
> linux-2.6.30.3.orig/arch/powerpc/sysdev/bestcomm/gen_bd.c
> linux-2.6.30.3/arch/powerpc/sysdev/bestcomm/gen_bd.c
> --- linux-2.6.30.3.orig/arch/powerpc/sysdev/bestcomm/gen_bd.c 2009-07-24
> 23:47:51.000000000 +0200
> +++ linux-2.6.30.3/arch/powerpc/sysdev/bestcomm/gen_bd.c 2009-10-01
> 14:26:33.000000000 +0200
> @@ -78,6 +78,7 @@ struct bcom_gen_bd_priv {
> int initiator;
> int ipr;
> int maxbufsize;
> + u32 *microcode;
spaces instead of tabs
> };
>
>
> @@ -104,6 +105,7 @@ bcom_gen_bd_rx_init(int queue_len, phys_
> priv->initiator = initiator;
> priv->ipr = ipr;
> priv->maxbufsize = maxbufsize;
> + priv->microcode = bcom_gen_bd_rx_task;
>
> if (bcom_gen_bd_rx_reset(tsk)) {
> bcom_task_free(tsk);
> @@ -128,7 +130,7 @@ bcom_gen_bd_rx_reset(struct bcom_task *t
> var = (struct bcom_gen_bd_rx_var *) bcom_task_var(tsk->tasknum);
> inc = (struct bcom_gen_bd_rx_inc *) bcom_task_inc(tsk->tasknum);
>
> - if (bcom_load_image(tsk->tasknum, bcom_gen_bd_rx_task))
> + if (bcom_load_image(tsk->tasknum, priv->microcode))
> return -1;
>
> var->enable = bcom_eng->regs_base +
> @@ -188,6 +190,7 @@ bcom_gen_bd_tx_init(int queue_len, phys_
> priv->fifo = fifo;
> priv->initiator = initiator;
> priv->ipr = ipr;
> + priv->microcode = bcom_gen_bd_tx_task;
>
> if (bcom_gen_bd_tx_reset(tsk)) {
> bcom_task_free(tsk);
> @@ -212,7 +215,7 @@ bcom_gen_bd_tx_reset(struct bcom_task *t
> var = (struct bcom_gen_bd_tx_var *) bcom_task_var(tsk->tasknum);
> inc = (struct bcom_gen_bd_tx_inc *) bcom_task_inc(tsk->tasknum);
>
> - if (bcom_load_image(tsk->tasknum, bcom_gen_bd_tx_task))
> + if (bcom_load_image(tsk->tasknum, priv->microcode))
> return -1;
>
> var->enable = bcom_eng->regs_base +
> @@ -253,6 +256,16 @@ bcom_gen_bd_tx_release(struct bcom_task
> }
> EXPORT_SYMBOL_GPL(bcom_gen_bd_tx_release);
>
> +void
> +bcom_gen_bd_set_microcode(struct bcom_task *tsk, u32 *microcode)
> +{
> + struct bcom_gen_bd_priv *priv = tsk->priv;
> +
> + priv->microcode = microcode;
> +}
> +EXPORT_SYMBOL_GPL(bcom_gen_bd_set_microcode);
> +
> +
Two empty lines.
> /*
> ---------------------------------------------------------------------
> * PSC support code
> */
> diff -uprN -X linux-2.6.30.3/Documentation/dontdiff
> linux-2.6.30.3.orig/arch/powerpc/sysdev/bestcomm/gen_bd.h
> linux-2.6.30.3/arch/powerpc/sysdev/bestcomm/gen_bd.h
> --- linux-2.6.30.3.orig/arch/powerpc/sysdev/bestcomm/gen_bd.h 2009-07-24
> 23:47:51.000000000 +0200
> +++ linux-2.6.30.3/arch/powerpc/sysdev/bestcomm/gen_bd.h 2009-10-01
> 14:26:50.000000000 +0200
> @@ -43,6 +43,9 @@ bcom_gen_bd_tx_reset(struct bcom_task *t
> extern void
> bcom_gen_bd_tx_release(struct bcom_task *tsk);
>
> +extern void
> +bcom_gen_bd_set_microcode(struct bcom_task *tsk, u32 *microcode);
> +
>
> /* PSC support utility wrappers */
> struct bcom_task * bcom_psc_gen_bd_rx_init(unsigned psc_num, int
> queue_len,
>
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Joakim Tjernlund @ 2009-10-03 9:24 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <1254558678.7122.7.camel@pasglop>
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 03/10/2009 10:31:18:
>
> On Sat, 2009-10-03 at 10:05 +0200, Joakim Tjernlund wrote:
> > Cannot shake the feeling that it this snip of code that causes it
> > lwz r11, 0(r10) /* Get the level 1 entry */
> > rlwinm. r10, r11,0,0,19 /* Extract page descriptor page
> > address */
> > beq 2f /* If zero, don't try to find a pte */
> > Perhaps we can do something better? I still feel that we need to
> > force a TLB Error as the TLBMiss does not set DSISR so we have no way
> > of
> > knowing if it is an load or store.
>
> Can't we manufacture a DSISR and branch to the right function ?
Not if we want know if it is a load or store. There is no info to manufacture
a DSISR from. The best we can do here is try getting the RPN physical page
number correct. Perhaps something like this will do:
/* Copy 20 msb from MD_EPN to r20 to get the correct page
* number. Do not rely on DAR since the dcxx instructions fails
* to update DAR when they cause a DTLB Miss
*/
mfspr r21, MD_EPN
li r20, 0x0
rlwimi r20, r21, 0, 0, 19
Then go back and set the RPN accordingly.
The 8xx is different as as it will force a TLB error every time
it needs to deal with a page fault.
I suspect adding
if (!ret)
_tlbil_va(address);
in do_page_fault() will do the trick too.
So yes, there is a missing _tlbil_va() missing for 8xx somewhere
but there is something more too.
Maybe your new filter functions and my
powerpc, 8xx: DTLB Error must check for more errors.
will do the trick?
Jocke
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox