This should fix an oops with PCMCIA PATA devices
http://bugzilla.kernel.org/show_bug.cgi?id=8424
This is not a full fix for the problem, but probably
still the right thing to do.
Signed-off-by: Andi Kleen <[email protected]>
---
arch/x86_64/kernel/pci-dma.c | 4 ++++
1 file changed, 4 insertions(+)
Index: linux/arch/x86_64/kernel/pci-dma.c
===================================================================
--- linux.orig/arch/x86_64/kernel/pci-dma.c
+++ linux/arch/x86_64/kernel/pci-dma.c
@@ -82,6 +82,10 @@ dma_alloc_coherent(struct device *dev, s
if (dma_mask == 0)
dma_mask = DMA_32BIT_MASK;
+ /* Device not DMA able */
+ if (dev->dma_mask == NULL)
+ return NULL;
+
/* Don't invoke OOM killer */
gfp |= __GFP_NORETRY;
From: Daniel Gollub <[email protected]>
Fixed wrong expression which enabled watchdogs even if nmi_watchdog kernel
parameter wasn't set. This regression got slightly introduced with commit
b7471c6da94d30d3deadc55986cc38d1ff57f9ca.
Introduced NMI_DISABLED (-1) which allows to switch the value of NMI_DEFAULT
without breaking the APIC NMI watchdog code (again).
Fixes:
https://bugzilla.novell.com/show_bug.cgi?id=298084
http://bugzilla.kernel.org/show_bug.cgi?id=7839
And likely some more nmi_watchdog=0 related issues.
Signed-off-by: Daniel Gollub <[email protected]>
Signed-off-by: Andi Kleen <[email protected]>
---
Index: linux/arch/i386/kernel/apic.c
===================================================================
--- linux.orig/arch/i386/kernel/apic.c
+++ linux/arch/i386/kernel/apic.c
@@ -1085,7 +1085,7 @@ static int __init detect_init_APIC (void
if (l & MSR_IA32_APICBASE_ENABLE)
mp_lapic_addr = l & MSR_IA32_APICBASE_BASE;
- if (nmi_watchdog != NMI_NONE)
+ if (nmi_watchdog != NMI_NONE && nmi_watchdog != NMI_DISABLED)
nmi_watchdog = NMI_LOCAL_APIC;
printk(KERN_INFO "Found and enabled local APIC!\n");
Index: linux/arch/i386/kernel/nmi.c
===================================================================
--- linux.orig/arch/i386/kernel/nmi.c
+++ linux/arch/i386/kernel/nmi.c
@@ -77,7 +77,7 @@ static int __init check_nmi_watchdog(voi
unsigned int *prev_nmi_count;
int cpu;
- if ((nmi_watchdog == NMI_NONE) || (nmi_watchdog == NMI_DEFAULT))
+ if ((nmi_watchdog == NMI_NONE) || (nmi_watchdog == NMI_DISABLED))
return 0;
if (!atomic_read(&nmi_active))
@@ -424,7 +424,7 @@ int proc_nmi_enabled(struct ctl_table *t
if (!!old_state == !!nmi_watchdog_enabled)
return 0;
- if (atomic_read(&nmi_active) < 0) {
+ if (atomic_read(&nmi_active) < 0 || nmi_watchdog == NMI_DISABLED) {
printk( KERN_WARNING "NMI watchdog is permanently disabled\n");
return -EIO;
}
Index: linux/arch/x86_64/kernel/nmi.c
===================================================================
--- linux.orig/arch/x86_64/kernel/nmi.c
+++ linux/arch/x86_64/kernel/nmi.c
@@ -85,7 +85,7 @@ int __init check_nmi_watchdog (void)
int *counts;
int cpu;
- if ((nmi_watchdog == NMI_NONE) || (nmi_watchdog == NMI_DEFAULT))
+ if ((nmi_watchdog == NMI_NONE) || (nmi_watchdog == NMI_DISABLED))
return 0;
if (!atomic_read(&nmi_active))
@@ -442,7 +442,7 @@ int proc_nmi_enabled(struct ctl_table *t
if (!!old_state == !!nmi_watchdog_enabled)
return 0;
- if (atomic_read(&nmi_active) < 0) {
+ if (atomic_read(&nmi_active) < 0 || nmi_watchdog == NMI_DISABLED) {
printk( KERN_WARNING "NMI watchdog is permanently disabled\n");
return -EIO;
}
Index: linux/include/asm-i386/nmi.h
===================================================================
--- linux.orig/include/asm-i386/nmi.h
+++ linux/include/asm-i386/nmi.h
@@ -33,11 +33,12 @@ extern int nmi_watchdog_tick (struct pt_
extern atomic_t nmi_active;
extern unsigned int nmi_watchdog;
-#define NMI_DEFAULT -1
+#define NMI_DISABLED -1
#define NMI_NONE 0
#define NMI_IO_APIC 1
#define NMI_LOCAL_APIC 2
#define NMI_INVALID 3
+#define NMI_DEFAULT NMI_DISABLED
struct ctl_table;
struct file;
Index: linux/include/asm-x86_64/nmi.h
===================================================================
--- linux.orig/include/asm-x86_64/nmi.h
+++ linux/include/asm-x86_64/nmi.h
@@ -64,11 +64,12 @@ extern int setup_nmi_watchdog(char *);
extern atomic_t nmi_active;
extern unsigned int nmi_watchdog;
-#define NMI_DEFAULT -1
+#define NMI_DISABLED -1
#define NMI_NONE 0
#define NMI_IO_APIC 1
#define NMI_LOCAL_APIC 2
#define NMI_INVALID 3
+#define NMI_DEFAULT NMI_DISABLED
struct ctl_table;
struct file;
Very old binutils (2.12.90...) seem to have trouble with newlines
in assembler macro invocation. They put them into the resulting
argument expansion. In this case this lead to a parse error because
a .rept expression ended up spread over multiple lines. Change the PMDS()
invocation to a single line.
Signed-off-by: Andi Kleen <[email protected]>
Index: linux/arch/x86_64/kernel/head.S
===================================================================
--- linux.orig/arch/x86_64/kernel/head.S
+++ linux/arch/x86_64/kernel/head.S
@@ -345,8 +345,7 @@ NEXT_PAGE(level2_kernel_pgt)
/* 40MB kernel mapping. The kernel code cannot be bigger than that.
When you change this change KERNEL_TEXT_SIZE in page.h too. */
/* (2^48-(2*1024*1024*1024)-((2^39)*511)-((2^30)*510)) = 0 */
- PMDS(0x0000000000000000, __PAGE_KERNEL_LARGE_EXEC|_PAGE_GLOBAL,
- KERNEL_TEXT_SIZE/PMD_SIZE)
+ PMDS(0x0000000000000000, __PAGE_KERNEL_LARGE_EXEC|_PAGE_GLOBAL, KERNEL_TEXT_SIZE/PMD_SIZE)
/* Module mapping starts here */
.fill (PTRS_PER_PMD - (KERNEL_TEXT_SIZE/PMD_SIZE)),8,0
Very old 64bit binutils have .cfi_startproc/endproc, but
no .cfi_rel_offset. Check for .cfi_rel_offset too.
Cc: [email protected]
Cc: [email protected]
Index: linux/arch/x86_64/Makefile
===================================================================
--- linux.orig/arch/x86_64/Makefile
+++ linux/arch/x86_64/Makefile
@@ -57,8 +57,8 @@ cflags-y += $(call cc-option,-mno-sse -m
cflags-y += -maccumulate-outgoing-args
# do binutils support CFI?
-cflags-y += $(call as-instr,.cfi_startproc\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
-AFLAGS += $(call as-instr,.cfi_startproc\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
+cflags-y += $(call as-instr,.cfi_startproc\n.cfi_rel_offset rsp${comma}0\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
+AFLAGS += $(call as-instr,.cfi_startproc\n.cfi_rel_offset rsp${comma}0\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
# is .cfi_signal_frame supported too?
cflags-y += $(call as-instr,.cfi_startproc\n.cfi_signal_frame\n.cfi_endproc,-DCONFIG_AS_CFI_SIGNAL_FRAME=1,)
Index: linux/arch/i386/Makefile
===================================================================
--- linux.orig/arch/i386/Makefile
+++ linux/arch/i386/Makefile
@@ -51,8 +51,8 @@ cflags-y += -maccumulate-outgoing-args
CFLAGS += $(shell if [ $(call cc-version) -lt 0400 ] ; then echo $(call cc-option,-fno-unit-at-a-time); fi ;)
# do binutils support CFI?
-cflags-y += $(call as-instr,.cfi_startproc\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
-AFLAGS += $(call as-instr,.cfi_startproc\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
+cflags-y += $(call as-instr,.cfi_startproc\n.cfi_rel_offset esp${comma}0\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
+AFLAGS += $(call as-instr,.cfi_startproc\n.cfi_rel_offset esp${comma}0\n.cfi_endproc,-DCONFIG_AS_CFI=1,)
# is .cfi_signal_frame supported too?
cflags-y += $(call as-instr,.cfi_startproc\n.cfi_signal_frame\n.cfi_endproc,-DCONFIG_AS_CFI_SIGNAL_FRAME=1,)
Andi Kleen schrieb:
> Very old binutils (2.12.90...) seem to have trouble with newlines
> in assembler macro invocation. They put them into the resulting
> argument expansion. In this case this lead to a parse error because
> a .rept expression ended up spread over multiple lines. Change the PMDS()
> invocation to a single line.
>
> Signed-off-by: Andi Kleen <[email protected]>
>
> Index: linux/arch/x86_64/kernel/head.S
> ===================================================================
> --- linux.orig/arch/x86_64/kernel/head.S
> +++ linux/arch/x86_64/kernel/head.S
> @@ -345,8 +345,7 @@ NEXT_PAGE(level2_kernel_pgt)
> /* 40MB kernel mapping. The kernel code cannot be bigger than that.
> When you change this change KERNEL_TEXT_SIZE in page.h too. */
> /* (2^48-(2*1024*1024*1024)-((2^39)*511)-((2^30)*510)) = 0 */
> - PMDS(0x0000000000000000, __PAGE_KERNEL_LARGE_EXEC|_PAGE_GLOBAL,
> - KERNEL_TEXT_SIZE/PMD_SIZE)
> + PMDS(0x0000000000000000, __PAGE_KERNEL_LARGE_EXEC|_PAGE_GLOBAL, KERNEL_TEXT_SIZE/PMD_SIZE)
Can you please add a comment, that this line must stay in a single line
for the above reason? I would expect that the next who does some code
clean up will break it again.
> /* Module mapping starts here */
> .fill (PTRS_PER_PMD - (KERNEL_TEXT_SIZE/PMD_SIZE)),8,0
Regards,
--
Clemens Koller
_______________________________
R&D Imaging Devices
Anagramm GmbH
Rupert-Mayer-Str. 45/1
81379 Muenchen
Germany
http://www.anagramm-technology.com
Phone: +49-89-741518-50
Fax: +49-89-741518-19
> Can you please add a comment, that this line must stay in a single line
> for the above reason? I would expect that the next who does some code
> clean up will break it again.
Then we would need to add it to all macros essentially. That wouldn't
be economical.
-Andi
Hmm. I think this is wrong.
Why? Because the regular 32-bit x86 code does this all completely
differently, and doesn't use dma_mask at all. Instead, it _only_ uses
dev->coherent_dma_mask (which, considering the name of the function,
would seem to make sense).
Considering that the oops comes from this:
/* Kludge to make it bug-to-bug compatible with i386. i386
uses the normal dma_mask for alloc_coherent. */
dma_mask &= *dev->dma_mask;
and that that code is *old*, and comes from when this file was called
arch/x86_64/kernel/pci-gart.c, and the comment doesn't seem to even be
correct any more, I really think the proper fix is likely to just *remove*
that kludge that causes the oops entirely.
Anyway, I'll apply the patch, because clearly it's not going to make
things *worse* (it does avoid the oops that we get now), but I don't think
it's really even "probably still the right thing to do". I really think we
should just remove the line that causes the oops instead, but that might
change behaviour for non-oopsing cases, so I'm not ready to do that at
this point.
Hmm? Who feels in charge of the DMA mapping stuff? Muli? James? Anybody?
Linus
On Wed, 15 Aug 2007, Andi Kleen wrote:
>
> This should fix an oops with PCMCIA PATA devices
>
> http://bugzilla.kernel.org/show_bug.cgi?id=8424
>
> This is not a full fix for the problem, but probably
> still the right thing to do.
>
> Signed-off-by: Andi Kleen <[email protected]>
>
> ---
> arch/x86_64/kernel/pci-dma.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> Index: linux/arch/x86_64/kernel/pci-dma.c
> ===================================================================
> --- linux.orig/arch/x86_64/kernel/pci-dma.c
> +++ linux/arch/x86_64/kernel/pci-dma.c
> @@ -82,6 +82,10 @@ dma_alloc_coherent(struct device *dev, s
> if (dma_mask == 0)
> dma_mask = DMA_32BIT_MASK;
>
> + /* Device not DMA able */
> + if (dev->dma_mask == NULL)
> + return NULL;
> +
> /* Don't invoke OOM killer */
> gfp |= __GFP_NORETRY;
>
>
On Saturday 18 August 2007 19:22:18 Linus Torvalds wrote:
>
> Hmm. I think this is wrong.
>
> Why? Because the regular 32-bit x86 code does this all completely
> differently, and doesn't use dma_mask at all. Instead, it _only_ uses
> dev->coherent_dma_mask (which, considering the name of the function,
> would seem to make sense).
Yes, see my discussion with Alan. Likely this needs to be handled
in the caller to really fix the problem (pata_pcmcia oopsing when
it passes a DMA incapable pcmcia device to dma_alloc_coherent)
Another possible fix proposed by James was to give the pcmcia
devices a dma_mask of 0 which would also work.
> Considering that the oops comes from this:
>
> /* Kludge to make it bug-to-bug compatible with i386. i386
> uses the normal dma_mask for alloc_coherent. */
> dma_mask &= *dev->dma_mask;
>
> and that that code is *old*, and comes from when this file was called
> arch/x86_64/kernel/pci-gart.c, and the comment doesn't seem to even be
It might be outdated or it might now. The kludge was needed for Alsa because old
i386 ignored the consistent mask and they didn't always set it correctly, but that
should be obsolete now? I'm not quite sure because sound devices
are not always well tested on large memory systems which are the only
ones who show this problem. Takashi, do you know if all alsa drivers
set consistent mask correctly now?
But I didn't want to touch that that late -- can do it for .24.
Still failing is probably correct in this case so I included the patch.
-Andi
At Sat, 18 Aug 2007 21:03:25 +0200,
Andi Kleen wrote:
>
> > Considering that the oops comes from this:
> >
> > /* Kludge to make it bug-to-bug compatible with i386. i386
> > uses the normal dma_mask for alloc_coherent. */
> > dma_mask &= *dev->dma_mask;
>
> >
> > and that that code is *old*, and comes from when this file was called
> > arch/x86_64/kernel/pci-gart.c, and the comment doesn't seem to even be
>
> It might be outdated or it might now. The kludge was needed for Alsa because old
> i386 ignored the consistent mask and they didn't always set it correctly, but that
> should be obsolete now? I'm not quite sure because sound devices
> are not always well tested on large memory systems which are the only
> ones who show this problem. Takashi, do you know if all alsa drivers
> set consistent mask correctly now?
Yes, ALSA driver set both masks from the very beginning :)
There are some OSS drivers still calling only pci_set_dma_mask().
Through a quick look, maybe only trident.c needs a fix because it uses
a mask < 32bit for ALI devices. Others use 32bit DMA mask.
Takashi