Re: [RFC] remove support for AVR32 architecture

[Håvard Skinnemoen]

On Wed, Mar 1, 2017 at 12:44 PM, Hans-Christian Noren Egtvedt
 wrote:
> I have prepared three patches in my for-linus branch in git tree
> https://git.kernel.org/cgit/linux/kernel/git/egtvedt/linux-avr32.git

Acked-by: Haavard Skinnemoen 

Thank you for keeping it alive for so long!

I looked through you tree superficially. There are a few references to
AVR32 remaining. Most of them are for other subsystems, but you could
consider picking up a couple of more for your patchset:

include/uapi/linux/elf-em.h still defines EM_AVR32. This could be
considered a part of the architecture code.

lib/Kconfig.debug has a list of architectures which want frame
pointers. You could probably remove AVR32 from that list.

mm/Kconfig has a special case for AVR32 needing two quicklists.

If you were planning to patch each of those separately later, please
ignore my comments.

Håvard

Re: [RFC] remove support for AVR32 architecture

[Håvard Skinnemoen]

On Wed, Mar 1, 2017 at 12:44 PM, Hans-Christian Noren Egtvedt
 wrote:
> I have prepared three patches in my for-linus branch in git tree
> https://git.kernel.org/cgit/linux/kernel/git/egtvedt/linux-avr32.git

Acked-by: Haavard Skinnemoen 

Thank you for keeping it alive for so long!

I looked through you tree superficially. There are a few references to
AVR32 remaining. Most of them are for other subsystems, but you could
consider picking up a couple of more for your patchset:

include/uapi/linux/elf-em.h still defines EM_AVR32. This could be
considered a part of the architecture code.

lib/Kconfig.debug has a list of architectures which want frame
pointers. You could probably remove AVR32 from that list.

mm/Kconfig has a special case for AVR32 needing two quicklists.

If you were planning to patch each of those separately later, please
ignore my comments.

Håvard

Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Håvard Skinnemoen]

On Tue, Feb 21, 2017 at 9:14 AM, Alexandre Belloni
 wrote:
> On 21/02/2017 at 18:43:35 +0200, Andy Shevchenko wrote:
>> >> Come on, v4.10 has just been release and
>>
>> It doesn't build anymore. And current case even worse
>> Face it. It's dead.
>>
>
> I agree it hasn't seen any significant development in a while but I'm
> not the on able to take that decision.

I've been wanting to add devicetree support for some time, but I no
longer remember how to build the toolchain, and I don't have fond
memories of that whole process. And the fact that
4.2.4-atmel.1.1.3.avr32linux.1 is still the most current version of
gcc doesn't make me very optimistic.

So while Hans-Christian and others have been doing a great job of
keeping AVR32 on life support, I tend to think that if there's not
enough enthusiasm for the architecture to build a modern toolchain or
add support for device tree, avr32-linux probably isn't going anywhere
exciting.

> A few weeks ago, I was telling Boris to let it not build for a while and
> then remove it. You already went out of your way to make it work. Again,
> feel free to send a patch removing avr32. I can only see a lot of
> benefits for the Atmel ARM SoCs and the many cleanups that will follow.

Agree, I can't help but feel that the AVR32 support is doing more harm
than good at this point.

> If nobody complains about the 4.10 breakage, You'll have plenty of time
> to remove it for 4.12

I'm fine with that, but I haven't put much effort into keeping it
alive lately. If Hans-Christian agrees, I'm willing to post a patch to
remove it, or ack someone else's patch.

Håvard

Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver

[Håvard Skinnemoen]

On Tue, Feb 21, 2017 at 9:14 AM, Alexandre Belloni
 wrote:
> On 21/02/2017 at 18:43:35 +0200, Andy Shevchenko wrote:
>> >> Come on, v4.10 has just been release and
>>
>> It doesn't build anymore. And current case even worse
>> Face it. It's dead.
>>
>
> I agree it hasn't seen any significant development in a while but I'm
> not the on able to take that decision.

I've been wanting to add devicetree support for some time, but I no
longer remember how to build the toolchain, and I don't have fond
memories of that whole process. And the fact that
4.2.4-atmel.1.1.3.avr32linux.1 is still the most current version of
gcc doesn't make me very optimistic.

So while Hans-Christian and others have been doing a great job of
keeping AVR32 on life support, I tend to think that if there's not
enough enthusiasm for the architecture to build a modern toolchain or
add support for device tree, avr32-linux probably isn't going anywhere
exciting.

> A few weeks ago, I was telling Boris to let it not build for a while and
> then remove it. You already went out of your way to make it work. Again,
> feel free to send a patch removing avr32. I can only see a lot of
> benefits for the Atmel ARM SoCs and the many cleanups that will follow.

Agree, I can't help but feel that the AVR32 support is doing more harm
than good at this point.

> If nobody complains about the 4.10 breakage, You'll have plenty of time
> to remove it for 4.12

I'm fine with that, but I haven't put much effort into keeping it
alive lately. If Hans-Christian agrees, I'm willing to post a patch to
remove it, or ack someone else's patch.

Håvard

Re: [PATCH] avr32: fix 'undefined reference to `___copy_from_user'

[Håvard Skinnemoen]

On Sat, Sep 17, 2016 at 7:56 AM, Guenter Roeck  wrote:
> avr32 builds fail with:
>
> arch/avr32/kernel/built-in.o: In function `arch_ptrace':
> (.text+0x650): undefined reference to `___copy_from_user'
> arch/avr32/kernel/built-in.o:(___ksymtab+___copy_from_user+0x0): undefined
> reference to `___copy_from_user'
> kernel/built-in.o: In function `proc_doulongvec_ms_jiffies_minmax':
> (.text+0x5dd8): undefined reference to `___copy_from_user'
> kernel/built-in.o: In function `proc_dointvec_minmax_sysadmin':
> sysctl.c:(.text+0x6174): undefined reference to `___copy_from_user'
> kernel/built-in.o: In function `ptrace_has_cap':
> ptrace.c:(.text+0x69c0): undefined reference to `___copy_from_user'
> kernel/built-in.o:ptrace.c:(.text+0x6b90): more undefined references to
> `___copy_from_user' follow
>
> Fixes: 8630c32275ba ("avr32: fix copy_from_user()")
> Cc: Al Viro 
> Signed-off-by: Guenter Roeck 

Right, gmail loves HTML. Let me try this again...

Acked-by: Havard Skinnemoen 

Thanks!

Re: [PATCH] avr32: fix 'undefined reference to `___copy_from_user'

[Håvard Skinnemoen]

On Sat, Sep 17, 2016 at 7:56 AM, Guenter Roeck  wrote:
> avr32 builds fail with:
>
> arch/avr32/kernel/built-in.o: In function `arch_ptrace':
> (.text+0x650): undefined reference to `___copy_from_user'
> arch/avr32/kernel/built-in.o:(___ksymtab+___copy_from_user+0x0): undefined
> reference to `___copy_from_user'
> kernel/built-in.o: In function `proc_doulongvec_ms_jiffies_minmax':
> (.text+0x5dd8): undefined reference to `___copy_from_user'
> kernel/built-in.o: In function `proc_dointvec_minmax_sysadmin':
> sysctl.c:(.text+0x6174): undefined reference to `___copy_from_user'
> kernel/built-in.o: In function `ptrace_has_cap':
> ptrace.c:(.text+0x69c0): undefined reference to `___copy_from_user'
> kernel/built-in.o:ptrace.c:(.text+0x6b90): more undefined references to
> `___copy_from_user' follow
>
> Fixes: 8630c32275ba ("avr32: fix copy_from_user()")
> Cc: Al Viro 
> Signed-off-by: Guenter Roeck 

Right, gmail loves HTML. Let me try this again...

Acked-by: Havard Skinnemoen 

Thanks!

Re: [PATCH] arch: avr32: add dummy syscalls

[Håvard Skinnemoen]

On Mon, Feb 4, 2013 at 7:39 AM, Al Viro  wrote:
> On Sun, Feb 03, 2013 at 09:35:39PM -0800, H?vard Skinnemoen wrote:
>>
>> > But yes, 32bit/32bit/64bit/32bit is another interesting case -
>> > fanotify_mark is 32/32/64/32/32.  From what ABI says it would seem to
>> > be r12/r11/r8:r9/r10/stack, but if I understand you correctly, we'll
>> > end up wanting *two* arguments on stack...
>>
>> Yes, I think there may be a difference between the IAR and gcc ABI
>> here. But I could be wrong.
>
> So it will use the gap in case of 32/32/64/32; the first two calls will
> take index 0 and 1 (r12 and r11 resp.), the third will take 3 and 4
> (r9:r8) and the fourth will take 2 (r10).

Oh, cool. I guess I am wrong then. Thanks a lot for taking the time to
figure this out, and sorry I misled you.

If someone's got the toolchain installed (which I don't, sorry), it
should be relatively straightforward to verify this by looking at the
disassembly of a call to a function with a similar prototype.

Havard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arch: avr32: add dummy syscalls

[Håvard Skinnemoen]

On Mon, Feb 4, 2013 at 7:39 AM, Al Viro v...@zeniv.linux.org.uk wrote:
 On Sun, Feb 03, 2013 at 09:35:39PM -0800, H?vard Skinnemoen wrote:

  But yes, 32bit/32bit/64bit/32bit is another interesting case -
  fanotify_mark is 32/32/64/32/32.  From what ABI says it would seem to
  be r12/r11/r8:r9/r10/stack, but if I understand you correctly, we'll
  end up wanting *two* arguments on stack...

 Yes, I think there may be a difference between the IAR and gcc ABI
 here. But I could be wrong.

 So it will use the gap in case of 32/32/64/32; the first two calls will
 take index 0 and 1 (r12 and r11 resp.), the third will take 3 and 4
 (r9:r8) and the fourth will take 2 (r10).

Oh, cool. I guess I am wrong then. Thanks a lot for taking the time to
figure this out, and sorry I misled you.

If someone's got the toolchain installed (which I don't, sorry), it
should be relatively straightforward to verify this by looking at the
disassembly of a call to a function with a similar prototype.

Havard
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arch: avr32: add dummy syscalls

[Håvard Skinnemoen]

On Sun, Feb 3, 2013 at 9:05 PM, Al Viro  wrote:
> On Sun, Feb 03, 2013 at 08:52:18PM -0800, H?vard Skinnemoen wrote:
>
>> You're right on -- in this case, the compiler will skip r10, and do
>> (r12, r11, r8:r9, stack). We pass the syscall number in r8, but we
>> also unconditionally move r7 to r8 in the syscall path, so it
>> shouldn't matter (libc does the opposite when necessary).
>>
>> I remember some talk about having the compiler reuse r10 for the next
>> 32-bit argument in cases like this, but I don't think it ever
>> happened.
>
> Umm...  In case of fallocate() the next argument is 64bit one, though;
> sys_fallocate() will be looking for two 32bit words on stack, so no
> matter how do we pass them to syscall, we'd better push two words in
> the wrapper.

Right.

> But yes, 32bit/32bit/64bit/32bit is another interesting case -
> fanotify_mark is 32/32/64/32/32.  From what ABI says it would seem to
> be r12/r11/r8:r9/r10/stack, but if I understand you correctly, we'll
> end up wanting *two* arguments on stack...

Yes, I think there may be a difference between the IAR and gcc ABI
here. But I could be wrong.

Havard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arch: avr32: add dummy syscalls

[Håvard Skinnemoen]

On Sun, Feb 3, 2013 at 7:02 PM, Al Viro  wrote:
> On Mon, Feb 04, 2013 at 01:31:11AM +, Al Viro wrote:
>
>> Unless I'm misreading ocavr32.pdf, that should be (R12, R10:R11, R9, R8) and
>> (R12, R10:R11, R9:R8, stack) resp., so fadvise64 doesn't need a wrapper, but
>> fadvise64_64 does.  And something like (s32, s32, s64, s64) would turn into
>> (R12, R11, R9:R8, stack, stack); AFAICS, we don't have anything that ugly...
>
> Oh, yes, we do - fallocate(2).  int fd, int mode, loff_t offset, loff_t len.
> On something like mips or sparc32 it packs nicely; on avr32 it doesn't.
> Could you confirm that I haven't misparsed the ABI?

You're right on -- in this case, the compiler will skip r10, and do
(r12, r11, r8:r9, stack). We pass the syscall number in r8, but we
also unconditionally move r7 to r8 in the syscall path, so it
shouldn't matter (libc does the opposite when necessary).

I remember some talk about having the compiler reuse r10 for the next
32-bit argument in cases like this, but I don't think it ever
happened.

Havard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arch: avr32: add dummy syscalls

[Håvard Skinnemoen]

On Sun, Feb 3, 2013 at 7:02 PM, Al Viro v...@zeniv.linux.org.uk wrote:
 On Mon, Feb 04, 2013 at 01:31:11AM +, Al Viro wrote:

 Unless I'm misreading ocavr32.pdf, that should be (R12, R10:R11, R9, R8) and
 (R12, R10:R11, R9:R8, stack) resp., so fadvise64 doesn't need a wrapper, but
 fadvise64_64 does.  And something like (s32, s32, s64, s64) would turn into
 (R12, R11, R9:R8, stack, stack); AFAICS, we don't have anything that ugly...

 Oh, yes, we do - fallocate(2).  int fd, int mode, loff_t offset, loff_t len.
 On something like mips or sparc32 it packs nicely; on avr32 it doesn't.
 Could you confirm that I haven't misparsed the ABI?

You're right on -- in this case, the compiler will skip r10, and do
(r12, r11, r8:r9, stack). We pass the syscall number in r8, but we
also unconditionally move r7 to r8 in the syscall path, so it
shouldn't matter (libc does the opposite when necessary).

I remember some talk about having the compiler reuse r10 for the next
32-bit argument in cases like this, but I don't think it ever
happened.

Havard
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arch: avr32: add dummy syscalls

[Håvard Skinnemoen]

On Sun, Feb 3, 2013 at 9:05 PM, Al Viro v...@zeniv.linux.org.uk wrote:
 On Sun, Feb 03, 2013 at 08:52:18PM -0800, H?vard Skinnemoen wrote:

 You're right on -- in this case, the compiler will skip r10, and do
 (r12, r11, r8:r9, stack). We pass the syscall number in r8, but we
 also unconditionally move r7 to r8 in the syscall path, so it
 shouldn't matter (libc does the opposite when necessary).

 I remember some talk about having the compiler reuse r10 for the next
 32-bit argument in cases like this, but I don't think it ever
 happened.

 Umm...  In case of fallocate() the next argument is 64bit one, though;
 sys_fallocate() will be looking for two 32bit words on stack, so no
 matter how do we pass them to syscall, we'd better push two words in
 the wrapper.

Right.

 But yes, 32bit/32bit/64bit/32bit is another interesting case -
 fanotify_mark is 32/32/64/32/32.  From what ABI says it would seem to
 be r12/r11/r8:r9/r10/stack, but if I understand you correctly, we'll
 end up wanting *two* arguments on stack...

Yes, I think there may be a difference between the IAR and gcc ABI
here. But I could be wrong.

Havard
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arch: avr32: add dummy syscalls

[Håvard Skinnemoen]

On Sun, Jan 27, 2013 at 7:50 PM, Matthias Brugger
 wrote:
> This patch adds dummy syscalls so that compiling
> for this architecture does not provoke warnings when
> checksyscalls.sh is called.

Nice, but...

> --- a/arch/avr32/kernel/syscall_table.S
> +++ b/arch/avr32/kernel/syscall_table.S
> @@ -298,3 +298,32 @@ sys_call_table:
> .long   sys_recvmmsg
> .long   sys_setns
> .long   sys_ni_syscall  /* r8 is saturated at nr_syscalls */

This terminator needs to stay at the end. Its only purpose is to allow
us to save a cycle or two when saturating the system call number.

Also, Al's suggestion sounds good to me.

Havard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arch: avr32: add dummy syscalls

[Håvard Skinnemoen]

On Sun, Jan 27, 2013 at 7:50 PM, Matthias Brugger
matthias@gmail.com wrote:
 This patch adds dummy syscalls so that compiling
 for this architecture does not provoke warnings when
 checksyscalls.sh is called.

Nice, but...

 --- a/arch/avr32/kernel/syscall_table.S
 +++ b/arch/avr32/kernel/syscall_table.S
 @@ -298,3 +298,32 @@ sys_call_table:
 .long   sys_recvmmsg
 .long   sys_setns
 .long   sys_ni_syscall  /* r8 is saturated at nr_syscalls */

This terminator needs to stay at the end. Its only purpose is to allow
us to save a cycle or two when saturating the system call number.

Also, Al's suggestion sounds good to me.

Havard
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 06/10] net/macb: clean up ring buffer logic

[Håvard Skinnemoen]

On Tue, Oct 30, 2012 at 4:12 AM, David Laight  wrote:
>> Instead of masking head and tail every time we increment them, just let them
>> wrap through UINT_MAX and mask them when subscripting. Add simple accessor
>> functions to do the subscripting properly to minimize the chances of messing
>> this up.
> ...
>> +static unsigned int macb_tx_ring_avail(struct macb *bp)
>> +{
>> + return TX_RING_SIZE - (bp->tx_head - bp->tx_tail);
>> +}
>
> That one doesn't look quite right to me.
> Surely it should be masking with 'TX_RING_SIZE - 1'

Why is that? head and tail can never be more than TX_RING_SIZE apart,
so it shouldn't make any difference.

Havard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 06/10] net/macb: clean up ring buffer logic

[Håvard Skinnemoen]

On Tue, Oct 30, 2012 at 4:12 AM, David Laight david.lai...@aculab.com wrote:
 Instead of masking head and tail every time we increment them, just let them
 wrap through UINT_MAX and mask them when subscripting. Add simple accessor
 functions to do the subscripting properly to minimize the chances of messing
 this up.
 ...
 +static unsigned int macb_tx_ring_avail(struct macb *bp)
 +{
 + return TX_RING_SIZE - (bp-tx_head - bp-tx_tail);
 +}

 That one doesn't look quite right to me.
 Surely it should be masking with 'TX_RING_SIZE - 1'

Why is that? head and tail can never be more than TX_RING_SIZE apart,
so it shouldn't make any difference.

Havard
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] avr32: fix build error in atstk1006_defconfig

[Håvard Skinnemoen]

On Tue, Oct 16, 2012 at 12:19 AM, Josh Wu  wrote:
> fixed the following compile error when use avr32 atstk1006_defconfig:
>   drivers/mtd/nand/atmel_nand.c: In function 'pmecc_err_location':
>   drivers/mtd/nand/atmel_nand.c:639: error: implicit declaration of function 
> 'writel_relaxed'
>
> which was introduced by commit 1c7b874d33b463 ("mtd: at91: atmel_nand: add 
> Programmable Multibit ECC controller support").
> The PMECC for nand flash code uses writel_relaxed(). But in avr32, there is 
> no macro "writel_relaxed" defined. This patch add writex_relaxed macro 
> definitions.
>
> Signed-off-by: Josh Wu 

Acked-by: Havard Skinnemoen 

I think it's OK to just forward to write[bwl] -- the relaxed variants
merely provide fewer ordering guarantees than the non-relaxed
variants. The worst that could happen is a small performance hit (and
on AVR32, even that is highly unlikely).

Havard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] avr32: fix build error in atstk1006_defconfig

[Håvard Skinnemoen]

On Tue, Oct 16, 2012 at 12:19 AM, Josh Wu josh...@atmel.com wrote:
 fixed the following compile error when use avr32 atstk1006_defconfig:
   drivers/mtd/nand/atmel_nand.c: In function 'pmecc_err_location':
   drivers/mtd/nand/atmel_nand.c:639: error: implicit declaration of function 
 'writel_relaxed'

 which was introduced by commit 1c7b874d33b463 (mtd: at91: atmel_nand: add 
 Programmable Multibit ECC controller support).
 The PMECC for nand flash code uses writel_relaxed(). But in avr32, there is 
 no macro writel_relaxed defined. This patch add writex_relaxed macro 
 definitions.

 Signed-off-by: Josh Wu josh...@atmel.com

Acked-by: Havard Skinnemoen hav...@skinnemoen.net

I think it's OK to just forward to write[bwl] -- the relaxed variants
merely provide fewer ordering guarantees than the non-relaxed
variants. The worst that could happen is a small performance hit (and
on AVR32, even that is highly unlikely).

Havard
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [GIT PULL] Disintegrate UAPI for avr32

[Håvard Skinnemoen]

Hi David,

On Thu, Oct 4, 2012 at 12:51 PM, David Howells  wrote:
> Can you merge the following branch into the avr32 tree please.

I don't really have an avr32 tree anymore. Can you ask Linus to pull
this directly?

Havard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [GIT PULL] Disintegrate UAPI for avr32

[Håvard Skinnemoen]

Hi David,

On Thu, Oct 4, 2012 at 12:51 PM, David Howells dhowe...@redhat.com wrote:
 Can you merge the following branch into the avr32 tree please.

I don't really have an avr32 tree anymore. Can you ask Linus to pull
this directly?

Havard
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: the Linux kernel, testsuites, and maybe *you*

[Håvard Skinnemoen]

On 9/2/07, Andi Kleen <[EMAIL PROTECTED]> wrote:
> The other issue to test some of them properly you need unmapped pages
> etc. That gets much easier to do in user space. There are some other
> issues.

vmalloc, vmap, etc. always put a guard page after the allocation. So
if you test string operations on vmalloc()'ed memory, you have
unmapped pages in both ends.

I like the idea of a test suite for kernel internals, but I also think
it would be best to run it in kernel mode since things like atomics,
etc. may work differently in user space on some architectures.

Haavard
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: the Linux kernel, testsuites, and maybe *you*

[Håvard Skinnemoen]

On 9/2/07, Andi Kleen [EMAIL PROTECTED] wrote:
 The other issue to test some of them properly you need unmapped pages
 etc. That gets much easier to do in user space. There are some other
 issues.

vmalloc, vmap, etc. always put a guard page after the allocation. So
if you test string operations on vmalloc()'ed memory, you have
unmapped pages in both ends.

I like the idea of a test suite for kernel internals, but I also think
it would be best to run it in kernel mode since things like atomics,
etc. may work differently in user space on some architectures.

Haavard
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch 09/10] Remove the SLOB allocator for 2.6.23

[Håvard Skinnemoen]

On 7/10/07, Nick Piggin <[EMAIL PROTECTED]> wrote:

Håvard Skinnemoen wrote:
> On 7/10/07, Matt Mackall <[EMAIL PROTECTED]> wrote:
>
>> The only remaining known bug is arguably a problem in nommu that SLOB
>> shouldn't be papering over.
>
>
> I've got another one for you: SLOB ignores ARCH_KMALLOC_MINALIGN so
> using SLOB in combination with DMA and non-coherent architectures
> causes data corruption.

Should be fixed in mm, I believe: slob-improved-alignment-handling.patch


Indeed. Thanks, I'll give it a try later today.

Håvard
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch 09/10] Remove the SLOB allocator for 2.6.23

[Håvard Skinnemoen]

On 7/10/07, Matt Mackall <[EMAIL PROTECTED]> wrote:

The only remaining known bug is arguably a problem in nommu that SLOB
shouldn't be papering over.


I've got another one for you: SLOB ignores ARCH_KMALLOC_MINALIGN so
using SLOB in combination with DMA and non-coherent architectures
causes data corruption.

That said, there are currently very few architectures that define
ARCH_KMALLOC_MINALIGN, so I guess it might be something I'm doing
wrong on avr32. But I'd really like to know how other non-coherent
architectures handle DMA to buffers sharing cachelines with unrelated
data...

Håvard
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch 09/10] Remove the SLOB allocator for 2.6.23

[Håvard Skinnemoen]

On 7/10/07, Matt Mackall [EMAIL PROTECTED] wrote:

The only remaining known bug is arguably a problem in nommu that SLOB
shouldn't be papering over.


I've got another one for you: SLOB ignores ARCH_KMALLOC_MINALIGN so
using SLOB in combination with DMA and non-coherent architectures
causes data corruption.

That said, there are currently very few architectures that define
ARCH_KMALLOC_MINALIGN, so I guess it might be something I'm doing
wrong on avr32. But I'd really like to know how other non-coherent
architectures handle DMA to buffers sharing cachelines with unrelated
data...

Håvard
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch 09/10] Remove the SLOB allocator for 2.6.23

[Håvard Skinnemoen]

On 7/10/07, Nick Piggin [EMAIL PROTECTED] wrote:

Håvard Skinnemoen wrote:
 On 7/10/07, Matt Mackall [EMAIL PROTECTED] wrote:

 The only remaining known bug is arguably a problem in nommu that SLOB
 shouldn't be papering over.


 I've got another one for you: SLOB ignores ARCH_KMALLOC_MINALIGN so
 using SLOB in combination with DMA and non-coherent architectures
 causes data corruption.

Should be fixed in mm, I believe: slob-improved-alignment-handling.patch


Indeed. Thanks, I'll give it a try later today.

Håvard
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kernel BUG at mm/slub.c:3689!

[Håvard Skinnemoen]

On 6/11/07, Christoph Lameter <[EMAIL PROTECTED]> wrote:

Trouble is that ARCH_KMALLOC_MINALIGN is in bytes whereas we would need a
shift value for KMALLOC_MIN_SHIFT.


Ah, of course. Hrm...I just thought I had an idea, but it wouldn't work...


Does the latest patch work?


I'm sorry, but I haven't tested it yet. My board needs a hard reset,
and I can't do that over VPN. I'll test it first thing in the morning.

Håvard
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kernel BUG at mm/slub.c:3689!

[Håvard Skinnemoen]

On 6/11/07, Christoph Lameter <[EMAIL PROTECTED]> wrote:

On Mon, 11 Jun 2007, Håvard Skinnemoen wrote:
> I think it's best to ensure that memory returned by kmalloc() actually
> can be used for DMA. I used to work around this problem in the SPI
> controller driver by using a temporary DMA buffer when possible
> misalignment was detected, but David Brownell said it was the wrong
> way to do it and pointed at the above paragraph.

Well there are various ways of doing DMA. Memory returned can be used for
DMA but it may not be suitable for your DMA device if that device has
issues like alignment or physical address size restrictions.


Yes, that's true. If the DMA device has such restrictions, it probably
needs to be addressed elsewhere. My goal here is to make sure that
kmalloc()'ed memory is suitable for DMA as far as the CPU and caches
are concerned.


We should probably make the minimum slab size dependent on
ARCH_KMALLOC_MINALIGN. There is no point in having smaller slabs anyways.
They will all have the same size.


Sounds reasonable to me.

Haavard
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kernel BUG at mm/slub.c:3689!

[Håvard Skinnemoen]

On 6/11/07, Christoph Lameter <[EMAIL PROTECTED]> wrote:

On Mon, 11 Jun 2007, Håvard Skinnemoen wrote:

> > Note that I do not get why you would be aligning the objects to 32 bytes.
> > Increasing the smallest cache size wastes a lot of memory. And it is
> > usually advantageous if multiple related objects are in the same cacheline
> > unless you have heavy SMP contention.
>
> It's not about performance at all, it's about DMA buffers allocated
> using kmalloc() getting corrupted. Imagine this:

Uhhh... How about using a separate slab for the DMA buffers?


If there were just a few, known drivers that did this, sure. But as
long as Documentation/DMA-mapping.txt includes this paragraph:

   If you acquired your memory via the page allocator
   (i.e. __get_free_page*()) or the generic memory allocators
   (i.e. kmalloc() or kmem_cache_alloc()) then you may DMA to/from
   that memory using the addresses returned from those routines.

I think it's best to ensure that memory returned by kmalloc() actually
can be used for DMA. I used to work around this problem in the SPI
controller driver by using a temporary DMA buffer when possible
misalignment was detected, but David Brownell said it was the wrong
way to do it and pointed at the above paragraph.

But, as I mentioned, perhaps ARCH_KMALLOC_MINALIGN isn't the best way
to solve the problem. I'll look into the flush-caches-from-dma_unmap
approach. However, it looks like other arches set
ARCH_KMALLOC_MINALIGN to various values -- I suspect some of them
might run into the same problem as well?

[EMAIL PROTECTED]:~/git/linux$ grep -r ARCH_KMALLOC_MINALIGN
include/asm-*
include/asm-mips/mach-generic/kmalloc.h:#define ARCH_KMALLOC_MINALIGN   128
include/asm-mips/mach-ip27/kmalloc.h: * All happy, no need to define
ARCH_KMALLOC_MINALIGN
include/asm-mips/mach-ip32/kmalloc.h:#define ARCH_KMALLOC_MINALIGN  32
include/asm-mips/mach-ip32/kmalloc.h:#define ARCH_KMALLOC_MINALIGN  128
include/asm-s390/cache.h:#define ARCH_KMALLOC_MINALIGN  8
include/asm-sh64/uaccess.h:#define ARCH_KMALLOC_MINALIGN 8


> Maybe there are other solutions to this problem, but the old SLAB
> allocator did guarantee 32-byte alignment as long as SLAB debugging
> was turned off, so setting ARCH_KMALLOC_MINALIGN seemed like the
> easiest way to get back to the old, known-working behaviour.

SLABs mininum object size is 32 thus you had no problems. I see. SLAB
does not guarantee 32 byte alignment. It just happened to work. If you
switch on CONFIG_SLAB_DEBUG you will likely get into trouble.


Yeah, that's true. CONFIG_SLAB_DEBUG does indeed cause DMA buffer
corruption on avr32, and so does CONFIG_SLOB. I've been wanting to fix
it, but I never understood how. Now that SLUB seems to offer a
solution that doesn't effectively turn off debugging, I thought I'd
finally found it...

Haavard
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kernel BUG at mm/slub.c:3689!

[Håvard Skinnemoen]

On 6/11/07, Christoph Lameter <[EMAIL PROTECTED]> wrote:

Ok. Drop the patch and use this one instead. This one avoids the use
of smaller slabs that cause the conflict. The first slab will be sized 32
bytes instead of 8.


Thanks, I'll test it tomorrow.


Note that I do not get why you would be aligning the objects to 32 bytes.
Increasing the smallest cache size wastes a lot of memory. And it is
usually advantageous if multiple related objects are in the same cacheline
unless you have heavy SMP contention.


It's not about performance at all, it's about DMA buffers allocated
using kmalloc() getting corrupted. Imagine this:

1. A SPI protocol driver allocates a buffer using kmalloc()
2. SPI master driver receives a request and flushes all cachelines
touched by the buffer (using dma_map_single()) before handing it to
the DMA controller.
3. While the transfer is in progress, something else comes along and
reads something from a different buffer which happens to share a
cacheline with the buffer currently being used for DMA.
4. When the transfer is complete, the protocol driver will see stale
data because a part of the buffer was fetched by the dcache before the
received data was stored in RAM by the DMA controller.

Maybe there are other solutions to this problem, but the old SLAB
allocator did guarantee 32-byte alignment as long as SLAB debugging
was turned off, so setting ARCH_KMALLOC_MINALIGN seemed like the
easiest way to get back to the old, known-working behaviour.

It could be that I've underestimated the AVR32 AP cache though; I
think it can do partial writeback of cachelines, so it could be a
solution to writeback+invalidate the parts of the buffer that may have
shared cachelines from dma_unmap_*() and dma_sync_*_for_cpu(). It will
probably hit a few false positives since it might not always see the
whole buffer, but perhaps it's worth it.

Haavard
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kernel BUG at mm/slub.c:3689!

[Håvard Skinnemoen]

On 6/11/07, Christoph Lameter <[EMAIL PROTECTED]> wrote:

Ahhh... I see its the same phenomenon as before but triggered by
a different cause.

If you set the align to 32 then the first kmalloc slabs of size

8
16
32

are all of the same size which leads to duplicate files in sysfs.


Yes, that seems to be the problem.


Does this patch fix it?


Unfortunately, no. But I get a different error message; see below...

Also unfortunately, I won't be able to do any more testing until I get
back to work tomorrow -- I'm currently at home and I can't reset my
board remotely (Doh! Why didn't I set panic_timeout...)

Haavard

kobject_add failed for :016 with -EEXIST, don't try to register
things with the same name in the same directory.
Call trace:
[<9000f14e>] dump_stack+0x1a/0x24
[<90093128>] kobject_shadow_add+0xc4/0xe8
[<90093154>] kobject_add+0x8/0xc
[<9003dec4>] sysfs_slab_add+0xb0/0xf0
[<900046c6>] slab_sysfs_init+0x26/0x78
[<93f0>] kernel_init+0x74/0x1c0
[<90015cd4>] do_exit+0x0/0x52c

[ cut here ]
kernel BUG at /home/hskinnemoen/git/linux/mm/slub.c:3686!
Oops: Kernel BUG, sig: 9 [#1]
FRAME_POINTER chip: 0x01f:0x1e82 rev 0
Modules linked in:
PC is at slab_sysfs_init+0x28/0x78
LR is at 0x9014e654
pc : [<900046c8>]lr : [<9014e654>]Not tainted
sp : 901cbf98  r12: ffef  r11: 0001
r10: 0001  r9 :   r8 : 9014e654
r7 : 901cbf98  r6 : 9014a2b4  r5 :   r4 : 
r3 :   r2 : 90015cd4  r1 : 9000b4d0  r0 : 
Flags: qvNzc
Mode bits: hrjeg
CPU Mode: Supervisor
Process: swapper [1] (task: 901c9000 thread: 901ca000)
Stack: (0x901cbf98 to 0x901cc000)
bf80:   93f0 901cbfdc
bfa0: 90009b20      0040 9000ebd0
bfc0: 9000ebd0 901cc000      90015cd4
bfe0:      90015cd4 937c 
Call trace:
[<93f0>] kernel_init+0x74/0x1c0
[<90015cd4>] do_exit+0x0/0x52c

Kernel panic - not syncing: Attempted to kill init!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kernel BUG at mm/slub.c:3689!

[Håvard Skinnemoen]

On 6/11/07, Christoph Lameter <[EMAIL PROTECTED]> wrote:

On Mon, 11 Jun 2007, Haavard Skinnemoen wrote:

> While trying to get SLUB debugging to not break DMA on AVR32, I ran
> into this:

This is a known bug in 2.6.22-rc2/rc3. Which version are you running?


2.6.22-rc4. I did a pull from Linus' tree a few hours ago.

Haavard
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kernel BUG at mm/slub.c:3689!

[Håvard Skinnemoen]

On 6/11/07, Paul Mundt <[EMAIL PROTECTED]> wrote:

On Mon, Jun 11, 2007 at 04:19:26PM +0200, Haavard Skinnemoen wrote:
> I think the combination that triggered this bug was:
>   * CONFIG_SLUB_DEBUG=y
>   * ARCH_KMALLOC_MINALIGN=32
>   * slub_debug not set at the command line
>
ARCH_KMALLOC_MINALIGN and ARCH_SLAB_MINALIGN are both in bytes
(ARCH_KMALLOC_MINALIGN used to be defined as BYTES_PER_WORD) if I recall
correctly, are you sure this is what you want?


Yeah, I actually defined it to L1_CACHE_BYTES. The idea is to avoid
cacheline sharing between different kmalloc() allocations, which may
mess up DMA transfers. So 32 bytes alignment is indeed what I want.

Haavard
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kernel BUG at mm/slub.c:3689!

[Håvard Skinnemoen]

On 6/11/07, Paul Mundt [EMAIL PROTECTED] wrote:

On Mon, Jun 11, 2007 at 04:19:26PM +0200, Haavard Skinnemoen wrote:
 I think the combination that triggered this bug was:
   * CONFIG_SLUB_DEBUG=y
   * ARCH_KMALLOC_MINALIGN=32
   * slub_debug not set at the command line

ARCH_KMALLOC_MINALIGN and ARCH_SLAB_MINALIGN are both in bytes
(ARCH_KMALLOC_MINALIGN used to be defined as BYTES_PER_WORD) if I recall
correctly, are you sure this is what you want?


Yeah, I actually defined it to L1_CACHE_BYTES. The idea is to avoid
cacheline sharing between different kmalloc() allocations, which may
mess up DMA transfers. So 32 bytes alignment is indeed what I want.

Haavard
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kernel BUG at mm/slub.c:3689!

[Håvard Skinnemoen]

On 6/11/07, Christoph Lameter [EMAIL PROTECTED] wrote:

On Mon, 11 Jun 2007, Haavard Skinnemoen wrote:

 While trying to get SLUB debugging to not break DMA on AVR32, I ran
 into this:

This is a known bug in 2.6.22-rc2/rc3. Which version are you running?


2.6.22-rc4. I did a pull from Linus' tree a few hours ago.

Haavard
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kernel BUG at mm/slub.c:3689!

[Håvard Skinnemoen]

On 6/11/07, Christoph Lameter [EMAIL PROTECTED] wrote:

Ahhh... I see its the same phenomenon as before but triggered by
a different cause.

If you set the align to 32 then the first kmalloc slabs of size

8
16
32

are all of the same size which leads to duplicate files in sysfs.


Yes, that seems to be the problem.


Does this patch fix it?


Unfortunately, no. But I get a different error message; see below...

Also unfortunately, I won't be able to do any more testing until I get
back to work tomorrow -- I'm currently at home and I can't reset my
board remotely (Doh! Why didn't I set panic_timeout...)

Haavard

kobject_add failed for :016 with -EEXIST, don't try to register
things with the same name in the same directory.
Call trace:
[9000f14e] dump_stack+0x1a/0x24
[90093128] kobject_shadow_add+0xc4/0xe8
[90093154] kobject_add+0x8/0xc
[9003dec4] sysfs_slab_add+0xb0/0xf0
[900046c6] slab_sysfs_init+0x26/0x78
[93f0] kernel_init+0x74/0x1c0
[90015cd4] do_exit+0x0/0x52c

[ cut here ]
kernel BUG at /home/hskinnemoen/git/linux/mm/slub.c:3686!
Oops: Kernel BUG, sig: 9 [#1]
FRAME_POINTER chip: 0x01f:0x1e82 rev 0
Modules linked in:
PC is at slab_sysfs_init+0x28/0x78
LR is at 0x9014e654
pc : [900046c8]lr : [9014e654]Not tainted
sp : 901cbf98  r12: ffef  r11: 0001
r10: 0001  r9 :   r8 : 9014e654
r7 : 901cbf98  r6 : 9014a2b4  r5 :   r4 : 
r3 :   r2 : 90015cd4  r1 : 9000b4d0  r0 : 
Flags: qvNzc
Mode bits: hrjeg
CPU Mode: Supervisor
Process: swapper [1] (task: 901c9000 thread: 901ca000)
Stack: (0x901cbf98 to 0x901cc000)
bf80:   93f0 901cbfdc
bfa0: 90009b20      0040 9000ebd0
bfc0: 9000ebd0 901cc000      90015cd4
bfe0:      90015cd4 937c 
Call trace:
[93f0] kernel_init+0x74/0x1c0
[90015cd4] do_exit+0x0/0x52c

Kernel panic - not syncing: Attempted to kill init!
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kernel BUG at mm/slub.c:3689!

[Håvard Skinnemoen]

On 6/11/07, Christoph Lameter [EMAIL PROTECTED] wrote:

Ok. Drop the patch and use this one instead. This one avoids the use
of smaller slabs that cause the conflict. The first slab will be sized 32
bytes instead of 8.


Thanks, I'll test it tomorrow.


Note that I do not get why you would be aligning the objects to 32 bytes.
Increasing the smallest cache size wastes a lot of memory. And it is
usually advantageous if multiple related objects are in the same cacheline
unless you have heavy SMP contention.


It's not about performance at all, it's about DMA buffers allocated
using kmalloc() getting corrupted. Imagine this:

1. A SPI protocol driver allocates a buffer using kmalloc()
2. SPI master driver receives a request and flushes all cachelines
touched by the buffer (using dma_map_single()) before handing it to
the DMA controller.
3. While the transfer is in progress, something else comes along and
reads something from a different buffer which happens to share a
cacheline with the buffer currently being used for DMA.
4. When the transfer is complete, the protocol driver will see stale
data because a part of the buffer was fetched by the dcache before the
received data was stored in RAM by the DMA controller.

Maybe there are other solutions to this problem, but the old SLAB
allocator did guarantee 32-byte alignment as long as SLAB debugging
was turned off, so setting ARCH_KMALLOC_MINALIGN seemed like the
easiest way to get back to the old, known-working behaviour.

It could be that I've underestimated the AVR32 AP cache though; I
think it can do partial writeback of cachelines, so it could be a
solution to writeback+invalidate the parts of the buffer that may have
shared cachelines from dma_unmap_*() and dma_sync_*_for_cpu(). It will
probably hit a few false positives since it might not always see the
whole buffer, but perhaps it's worth it.

Haavard
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kernel BUG at mm/slub.c:3689!

[Håvard Skinnemoen]

On 6/11/07, Christoph Lameter [EMAIL PROTECTED] wrote:

On Mon, 11 Jun 2007, Håvard Skinnemoen wrote:

  Note that I do not get why you would be aligning the objects to 32 bytes.
  Increasing the smallest cache size wastes a lot of memory. And it is
  usually advantageous if multiple related objects are in the same cacheline
  unless you have heavy SMP contention.

 It's not about performance at all, it's about DMA buffers allocated
 using kmalloc() getting corrupted. Imagine this:

Uhhh... How about using a separate slab for the DMA buffers?


If there were just a few, known drivers that did this, sure. But as
long as Documentation/DMA-mapping.txt includes this paragraph:

   If you acquired your memory via the page allocator
   (i.e. __get_free_page*()) or the generic memory allocators
   (i.e. kmalloc() or kmem_cache_alloc()) then you may DMA to/from
   that memory using the addresses returned from those routines.

I think it's best to ensure that memory returned by kmalloc() actually
can be used for DMA. I used to work around this problem in the SPI
controller driver by using a temporary DMA buffer when possible
misalignment was detected, but David Brownell said it was the wrong
way to do it and pointed at the above paragraph.

But, as I mentioned, perhaps ARCH_KMALLOC_MINALIGN isn't the best way
to solve the problem. I'll look into the flush-caches-from-dma_unmap
approach. However, it looks like other arches set
ARCH_KMALLOC_MINALIGN to various values -- I suspect some of them
might run into the same problem as well?

[EMAIL PROTECTED]:~/git/linux$ grep -r ARCH_KMALLOC_MINALIGN
include/asm-*
include/asm-mips/mach-generic/kmalloc.h:#define ARCH_KMALLOC_MINALIGN   128
include/asm-mips/mach-ip27/kmalloc.h: * All happy, no need to define
ARCH_KMALLOC_MINALIGN
include/asm-mips/mach-ip32/kmalloc.h:#define ARCH_KMALLOC_MINALIGN  32
include/asm-mips/mach-ip32/kmalloc.h:#define ARCH_KMALLOC_MINALIGN  128
include/asm-s390/cache.h:#define ARCH_KMALLOC_MINALIGN  8
include/asm-sh64/uaccess.h:#define ARCH_KMALLOC_MINALIGN 8


 Maybe there are other solutions to this problem, but the old SLAB
 allocator did guarantee 32-byte alignment as long as SLAB debugging
 was turned off, so setting ARCH_KMALLOC_MINALIGN seemed like the
 easiest way to get back to the old, known-working behaviour.

SLABs mininum object size is 32 thus you had no problems. I see. SLAB
does not guarantee 32 byte alignment. It just happened to work. If you
switch on CONFIG_SLAB_DEBUG you will likely get into trouble.


Yeah, that's true. CONFIG_SLAB_DEBUG does indeed cause DMA buffer
corruption on avr32, and so does CONFIG_SLOB. I've been wanting to fix
it, but I never understood how. Now that SLUB seems to offer a
solution that doesn't effectively turn off debugging, I thought I'd
finally found it...

Haavard
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kernel BUG at mm/slub.c:3689!

[Håvard Skinnemoen]

On 6/11/07, Christoph Lameter [EMAIL PROTECTED] wrote:

On Mon, 11 Jun 2007, Håvard Skinnemoen wrote:
 I think it's best to ensure that memory returned by kmalloc() actually
 can be used for DMA. I used to work around this problem in the SPI
 controller driver by using a temporary DMA buffer when possible
 misalignment was detected, but David Brownell said it was the wrong
 way to do it and pointed at the above paragraph.

Well there are various ways of doing DMA. Memory returned can be used for
DMA but it may not be suitable for your DMA device if that device has
issues like alignment or physical address size restrictions.


Yes, that's true. If the DMA device has such restrictions, it probably
needs to be addressed elsewhere. My goal here is to make sure that
kmalloc()'ed memory is suitable for DMA as far as the CPU and caches
are concerned.


We should probably make the minimum slab size dependent on
ARCH_KMALLOC_MINALIGN. There is no point in having smaller slabs anyways.
They will all have the same size.


Sounds reasonable to me.

Haavard
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kernel BUG at mm/slub.c:3689!

[Håvard Skinnemoen]

On 6/11/07, Christoph Lameter [EMAIL PROTECTED] wrote:

Trouble is that ARCH_KMALLOC_MINALIGN is in bytes whereas we would need a
shift value for KMALLOC_MIN_SHIFT.


Ah, of course. Hrm...I just thought I had an idea, but it wouldn't work...


Does the latest patch work?


I'm sorry, but I haven't tested it yet. My board needs a hard reset,
and I can't do that over VPN. I'll test it first thing in the morning.

Håvard
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] atmel_spi: Pass correct DMA address to controller

[Håvard Skinnemoen]

On 5/16/07, David Brownell <[EMAIL PROTECTED]> wrote:

On Wednesday 16 May 2007, Haavard Skinnemoen wrote:
> - tx_dma = xfer->tx_dma;
> - rx_dma = xfer->rx_dma;
> + tx_dma = xfer->tx_dma + xfer->len - len;
> + rx_dma = xfer->rx_dma + xfer->len - len;
>
>   /* use scratch buffer only when rx or tx data is unspecified */
> - if (rx_dma == INVALID_DMA_ADDRESS) {
> + if (!xfer->rx_buf) {

This test ...

>   rx_dma = as->buffer_dma;
>   if (len > BUFFER_SIZE)
>   len = BUFFER_SIZE;
>   }
> - if (tx_dma == INVALID_DMA_ADDRESS) {
> + if (!xfer->tx_buf) {

... and this one must not assume that the relevant buffer
is always NULL when the tx_dma has been set up.  Keep the
test against INVALID_DMA_ADDRESS, but fetch the address
from the spi_transfer.


No it doesn't -- it's the other way around. It assumes that if
xfer->tx_buf is NULL, the user didn't provide a buffer so it has to
use the scratch buffer instead. The driver never does PIO transfers,
so if tx_buf is valid, tx_dma ought to be valid  too. This is taken
care of elsewhere by calling dma_map_single() if the user provided a
virtual pointer but not a dma address.

Checking for an invalid cpu-virtual pointer really should be
equivalent to checking for an invalid DMA address at this point.

I could perhaps add else blocks to those two ifs and add the offset to
the respective dma addresses there instead if it makes it clearer. The
tests are supposed to be the same as before, but I had to change them
one way or another because of the added offset (which would make an
invalid dma address non-invalid.)


It's legit to set up cpu-virtual (for PIO) and dma addresses
for each buffer, since the upper layer driver has no way to
know if the underlying controller driver is DMA-capable, or
for that matter PIO-capable.


Yes, but are there any drivers that will provide a valid dma address
and a NULL cpu-virtual pointer? That would indeed break my
assumptions, but it would also break any PIO-only drivers, wouldn't
it?

Haavard
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] atmel_spi: Pass correct DMA address to controller

[Håvard Skinnemoen]

On 5/16/07, David Brownell [EMAIL PROTECTED] wrote:

On Wednesday 16 May 2007, Haavard Skinnemoen wrote:
 - tx_dma = xfer-tx_dma;
 - rx_dma = xfer-rx_dma;
 + tx_dma = xfer-tx_dma + xfer-len - len;
 + rx_dma = xfer-rx_dma + xfer-len - len;

   /* use scratch buffer only when rx or tx data is unspecified */
 - if (rx_dma == INVALID_DMA_ADDRESS) {
 + if (!xfer-rx_buf) {

This test ...

   rx_dma = as-buffer_dma;
   if (len  BUFFER_SIZE)
   len = BUFFER_SIZE;
   }
 - if (tx_dma == INVALID_DMA_ADDRESS) {
 + if (!xfer-tx_buf) {

... and this one must not assume that the relevant buffer
is always NULL when the tx_dma has been set up.  Keep the
test against INVALID_DMA_ADDRESS, but fetch the address
from the spi_transfer.


No it doesn't -- it's the other way around. It assumes that if
xfer-tx_buf is NULL, the user didn't provide a buffer so it has to
use the scratch buffer instead. The driver never does PIO transfers,
so if tx_buf is valid, tx_dma ought to be valid  too. This is taken
care of elsewhere by calling dma_map_single() if the user provided a
virtual pointer but not a dma address.

Checking for an invalid cpu-virtual pointer really should be
equivalent to checking for an invalid DMA address at this point.

I could perhaps add else blocks to those two ifs and add the offset to
the respective dma addresses there instead if it makes it clearer. The
tests are supposed to be the same as before, but I had to change them
one way or another because of the added offset (which would make an
invalid dma address non-invalid.)


It's legit to set up cpu-virtual (for PIO) and dma addresses
for each buffer, since the upper layer driver has no way to
know if the underlying controller driver is DMA-capable, or
for that matter PIO-capable.


Yes, but are there any drivers that will provide a valid dma address
and a NULL cpu-virtual pointer? That would indeed break my
assumptions, but it would also break any PIO-only drivers, wouldn't
it?

Haavard
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: missing kretprobes support on avr32 and sparc64

[Håvard Skinnemoen]

On Sat, March 31, 2007 15:15, Christoph Hellwig wrote:
> Currently all avr32 and sparc64 don't support kretprobes unlike all
> other kprobes supporting architectures.  This is not nice from the
> user interface point of view and from the ugly ifdefs point of view.
> Is there a reason these ports can't support kretprobes or was this
> simply an oversight / lazyness?

Lazyness on my part, I guess. It shouldn't be too hard to implement on
avr32; It looks like the generic code sets a breakpoint on function entry,
so all I have to do is to set a breakpoint on the return address. And
handle the breakpoint somehow. I'll try to implement it when I get back to
work in about a week.

Haavard

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: missing kretprobes support on avr32 and sparc64

[Håvard Skinnemoen]

On Sat, March 31, 2007 15:15, Christoph Hellwig wrote:
 Currently all avr32 and sparc64 don't support kretprobes unlike all
 other kprobes supporting architectures.  This is not nice from the
 user interface point of view and from the ugly ifdefs point of view.
 Is there a reason these ports can't support kretprobes or was this
 simply an oversight / lazyness?

Lazyness on my part, I guess. It shouldn't be too hard to implement on
avr32; It looks like the generic code sets a breakpoint on function entry,
so all I have to do is to set a breakpoint on the return address. And
handle the breakpoint somehow. I'll try to implement it when I get back to
work in about a week.

Haavard

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] Bitbanging i2c bus driver using the GPIO API

[Håvard Skinnemoen]

On Fri, March 9, 2007 20:30, David Brownell wrote:
> On Friday 09 March 2007 10:48 am, Haavard Skinnemoen wrote:
>> This is a very simple bitbanging i2c bus driver utilizing the new
>> arch-neutral GPIO API. Useful for chips that don't have a built-in
>> i2c controller, additional i2c busses, or testing purposes.
>
> That's the right idea!  But remember that not all GPIOs support
> reading back the actual value on SCL (it's an OUT pin, so lacking
> multidrive capability the values "should" be what you wrote), so
> getscl() support should depend on a flag in platform data.  In
> the same vein, if SCL is an output-only pin, you won't be able
> to change its direction ... but then, I'm not sure why you were
> changing its direction in setscl() rather than just its value.

The idea is to keep the output value at 0 and switch the output driver on
and off. I assumed that changing the direction was the easiest way to
achieve this.

I never really thought about output-only pins. Is it actually possible to
implement i2c properly on such hardware?

> I2C has another interesting special case.  at91_set_multi_drive()
> would be appropriate (yes?) for ARCH_AT91 to use on SCL, to best
> support both clock stretching and multi-master configurations.

Isn't it better to switch the direction to input since the driver needs to
watch the pin state in order to support clock stretching and multi-master?

>> +gpio_direction_input(pdata->sda_pin);
>> +gpio_direction_input(pdata->scl_pin);
>> +gpio_set_value(pdata->sda_pin, 0);
>> +gpio_set_value(pdata->scl_pin, 0);
>
> Surely you mean "output" in both cases.  So you can set the
> value.  Setting the value on an input pin is undefined.  ;)

No I really do mean input, as I want to put the bus into an idle state
initially, which means no output drivers and passive pullup on both lines.

The gpio_set_value() calls should go away and the output value should be
explicitly set to 0 when turning on the output drivers. In its present
form, the driver happens to work on my hardware, which is all I really
cared about when writing it ;-)

>> +printk(KERN_INFO "i2c-gpio: using pins 0x%x (sda) 0x%x (scl)\n",
>> +   pdata->sda_pin, pdata->scl_pin);
>
> Please, no hex there.  I think dev_info() would be better; and it
> might be nice to report whether clock stretching is supported.

Ok, but how do I inform i2c-algo-bit about whether or not clock stretching
is supported?

>> --- a/include/linux/i2c-id.h
>> +++ b/include/linux/i2c-id.h
>> @@ -194,6 +194,7 @@
>>  #define I2C_HW_B_EM28XX 0x01001f /* em28xx video capture cards 
>> */
>>  #define I2C_HW_B_CX2341X0x010020 /* Conexant CX2341X MPEG encoder
>> cards */
>>  #define I2C_HW_B_INTELFB0x010021 /* intel framebuffer driver */
>> +#define I2C_HW_B_GPIO   0x010022 /* Generic GPIO-based driver */
>
> It'd be nice to completely abolish those IDs, starting by not
> adding new ones.  Especially, not adding unused ones!

Sure.

Haavard

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] Bitbanging i2c bus driver using the GPIO API

[Håvard Skinnemoen]

On Fri, March 9, 2007 20:30, David Brownell wrote:
 On Friday 09 March 2007 10:48 am, Haavard Skinnemoen wrote:
 This is a very simple bitbanging i2c bus driver utilizing the new
 arch-neutral GPIO API. Useful for chips that don't have a built-in
 i2c controller, additional i2c busses, or testing purposes.

 That's the right idea!  But remember that not all GPIOs support
 reading back the actual value on SCL (it's an OUT pin, so lacking
 multidrive capability the values should be what you wrote), so
 getscl() support should depend on a flag in platform data.  In
 the same vein, if SCL is an output-only pin, you won't be able
 to change its direction ... but then, I'm not sure why you were
 changing its direction in setscl() rather than just its value.

The idea is to keep the output value at 0 and switch the output driver on
and off. I assumed that changing the direction was the easiest way to
achieve this.

I never really thought about output-only pins. Is it actually possible to
implement i2c properly on such hardware?

 I2C has another interesting special case.  at91_set_multi_drive()
 would be appropriate (yes?) for ARCH_AT91 to use on SCL, to best
 support both clock stretching and multi-master configurations.

Isn't it better to switch the direction to input since the driver needs to
watch the pin state in order to support clock stretching and multi-master?

 +gpio_direction_input(pdata-sda_pin);
 +gpio_direction_input(pdata-scl_pin);
 +gpio_set_value(pdata-sda_pin, 0);
 +gpio_set_value(pdata-scl_pin, 0);

 Surely you mean output in both cases.  So you can set the
 value.  Setting the value on an input pin is undefined.  ;)

No I really do mean input, as I want to put the bus into an idle state
initially, which means no output drivers and passive pullup on both lines.

The gpio_set_value() calls should go away and the output value should be
explicitly set to 0 when turning on the output drivers. In its present
form, the driver happens to work on my hardware, which is all I really
cared about when writing it ;-)

 +printk(KERN_INFO i2c-gpio: using pins 0x%x (sda) 0x%x (scl)\n,
 +   pdata-sda_pin, pdata-scl_pin);

 Please, no hex there.  I think dev_info() would be better; and it
 might be nice to report whether clock stretching is supported.

Ok, but how do I inform i2c-algo-bit about whether or not clock stretching
is supported?

 --- a/include/linux/i2c-id.h
 +++ b/include/linux/i2c-id.h
 @@ -194,6 +194,7 @@
  #define I2C_HW_B_EM28XX 0x01001f /* em28xx video capture cards 
 */
  #define I2C_HW_B_CX2341X0x010020 /* Conexant CX2341X MPEG encoder
 cards */
  #define I2C_HW_B_INTELFB0x010021 /* intel framebuffer driver */
 +#define I2C_HW_B_GPIO   0x010022 /* Generic GPIO-based driver */

 It'd be nice to completely abolish those IDs, starting by not
 adding new ones.  Especially, not adding unused ones!

Sure.

Haavard

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch 2.6.20-rc1 0/6] arch-neutral GPIO calls

[Håvard Skinnemoen]

On 12/20/06, David Brownell <[EMAIL PROTECTED]> wrote:

Based on earlier discussion, I'm sending a refresh of the generic GPIO
patch, with several (ARM based) implementations in separate patches:

 - Core patch, doc +  + 
 - OMAP implementation
 - AT91 implementation
 - PXA implementation
 - SA1100 implementation
 - S3C2410 implementation

I know there's an AVR32 implementation too; and there's been interest
in this for some PPC support as well.


Great, thanks Dave. Unfortunately, I'm going to be more or less
offline for the rest of the year, but FWIW, the AVR32 implementation
is already in -mm as part of git-avr32.patch. I guess I should check
and see if it's in sync with the rest.

I'll refresh the atmel_spi patch when I get back to work in january.

Haavard
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch 2.6.20-rc1 0/6] arch-neutral GPIO calls

[Håvard Skinnemoen]

On 12/20/06, David Brownell [EMAIL PROTECTED] wrote:

Based on earlier discussion, I'm sending a refresh of the generic GPIO
patch, with several (ARM based) implementations in separate patches:

 - Core patch, doc + asm-arm/gpio.h + asm-generic/gpio.h
 - OMAP implementation
 - AT91 implementation
 - PXA implementation
 - SA1100 implementation
 - S3C2410 implementation

I know there's an AVR32 implementation too; and there's been interest
in this for some PPC support as well.


Great, thanks Dave. Unfortunately, I'm going to be more or less
offline for the rest of the year, but FWIW, the AVR32 implementation
is already in -mm as part of git-avr32.patch. I guess I should check
and see if it's in sync with the rest.

I'll refresh the atmel_spi patch when I get back to work in january.

Haavard
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/