system lockup with 2.6.29 on Cavium/Octeon

I have a system lockup problem that I have been looking at on a custom Cavium/Octeon 5010 based design. I am running on linux-2.6.29 with David Daney's latest round of PCI and ethernet patches (posted here on this list).

I have tracked the problem back to local_flush_tlb_kernel_range() in arch/mips/mm/tlb-r4k.c. At the top of this function is:

void local_flush_tlb_kernel_range(unsigned long start, unsigned long
end)

    {
        unsigned long flags;
        int size;

        ENTER_CRITICAL(flags);
        size = (end - start + (PAGE_SIZE - 1)) >> PAGE_SHIFT;
        size = (size + 1) >> 1;
        if (size <= current_cpu_data.tlbsize / 2) {

The problem is that typical example values I see passed in for start and end are:

    start = c000000000006000
    end   = ffffffffc01d8000

Now the vmalloc area starts at 0xc000000000000000 and the kernel code and data is all at 0xffffffff80000000 and above. I don't know if the start and end are reasonable values, but I can see some logic as to where they come from. The code path that leads to this is via __vunmap() and __purge_vmap_area_lazy(). So it is not too difficult to see how we end up with values like this.

But the size calculation above with these types of values will result in still a large number. Larger than the 32bit "int" that is "size". I see large negative values fall out as size, and so the following tlbsize check becomes true, and the code spins inside the loop inside that if statement for a _very_ long time trying to flush tlb entries.

This is of course easily fixed, by making that size "unsigned long". The patch below trivially does this.

But is this analysis correct?

Quoted 25 lines · Greg Ungerer

I have a system lockup problem that I have been looking at on a custom Cavium/Octeon 5010 based design. I am running on linux-2.6.29 with David Daney's latest round of PCI and ethernet patches (posted here on this list).

I have tracked the problem back to local_flush_tlb_kernel_range() in arch/mips/mm/tlb-r4k.c. At the top of this function is:

void local_flush_tlb_kernel_range(unsigned long start, unsigned long
end)

    {
        unsigned long flags;
        int size;

        ENTER_CRITICAL(flags);
        size = (end - start + (PAGE_SIZE - 1)) >> PAGE_SHIFT;
        size = (size + 1) >> 1;
        if (size <= current_cpu_data.tlbsize / 2) {

The problem is that typical example values I see passed in for start and end are:

    start = c000000000006000
    end   = ffffffffc01d8000

Quoted 6 lines

Now the vmalloc area starts at 0xc000000000000000 and the kernel code and data is all at 0xffffffff80000000 and above. I don't know if the start and end are reasonable values, but I can see some logic as to where they come from. The code path that leads to this is via __vunmap() and __purge_vmap_area_lazy(). So it is not too difficult to see how we end up with values like this.

Either start or end address is sensible but not the combination - both addresses should be in the same segment. Start is in XKSEG, end in CKSEG2 and in between there are vast wastelands of unused address space exabytes in size.

Quoted 9 lines

But the size calculation above with these types of values will result in still a large number. Larger than the 32bit "int" that is "size". I see large negative values fall out as size, and so the following tlbsize check becomes true, and the code spins inside the loop inside that if statement for a _very_ long time trying to flush tlb entries.

This is of course easily fixed, by making that size "unsigned long". The patch below trivially does this.

Quoted 1 line

But is this analysis correct?

Yes - but I think we have two issues here. The one is the calculation overflowing int for the arguments you're seeing. The other being that the arguments simply are looking wrong.

There are a few more instances of the same overflow issue which the patch below is fixing.

Ralf

 arch/mips/mm/tlb-r3k.c |    6 ++----
 arch/mips/mm/tlb-r4k.c |    6 ++----
 arch/mips/mm/tlb-r8k.c |    3 +--
 3 files changed, 5 insertions(+), 10 deletions(-)

arch/mips/mm/tlb-r3k.c +2 -4

index f0cf46a..1c0048a 100644
@@ -82,8 +82,7 @@ void local_flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
 	int cpu = smp_processor_id();
 
 	if (cpu_context(cpu, mm) != 0) {
-		unsigned long flags;
-		int size;
+		unsigned long size, flags;
 
 #ifdef DEBUG_TLB
 		printk("[tlbrange<%lu,0x%08lx,0x%08lx>]",

@@ -121,8 +120,7 @@ void local_flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
 
 void local_flush_tlb_kernel_range(unsigned long start, unsigned long end)
 {
-	unsigned long flags;
-	int size;
+	unsigned long size, flags;
 
 #ifdef DEBUG_TLB
 	printk("[tlbrange<%lu,0x%08lx,0x%08lx>]", start, end);

arch/mips/mm/tlb-r4k.c +2 -4

index 9619f66..892be42 100644
@@ -117,8 +117,7 @@ void local_flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
 	int cpu = smp_processor_id();
 
 	if (cpu_context(cpu, mm) != 0) {
-		unsigned long flags;
-		int size;
+		unsigned long size, flags;
 
 		ENTER_CRITICAL(flags);
 		size = (end - start + (PAGE_SIZE - 1)) >> PAGE_SHIFT;

@@ -160,8 +159,7 @@ void local_flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
 
 void local_flush_tlb_kernel_range(unsigned long start, unsigned long end)
 {
-	unsigned long flags;
-	int size;
+	unsigned long size, flags;
 
 	ENTER_CRITICAL(flags);
 	size = (end - start + (PAGE_SIZE - 1)) >> PAGE_SHIFT;

arch/mips/mm/tlb-r8k.c +1 -2

index 4f01a3b..4ec95cc 100644
@@ -111,8 +111,7 @@ out_restore:
 /* Usable for KV1 addresses only! */
 void local_flush_tlb_kernel_range(unsigned long start, unsigned long end)
 {
-	unsigned long flags;
-	int size;
+	unsigned long size, flags;
 
 	size = (end - start + (PAGE_SIZE - 1)) >> PAGE_SHIFT;
 	size = (size + 1) >> 1;

Interestingly the definition of MODULE_START is like this:

#if defined(CONFIG_MODULES) && defined(KBUILD_64BIT_SYM32) && \
        VMALLOC_START != CKSSEG

/* Load modules into 32bit-compatible segment. */
#define MODULE_START CKSSEG

If MODULE_START wasn't defined then the module_alloc() code would have just called vmalloc() directly - and we wouldn't be in this mess :-)

Quoted 6 lines

Now the vmalloc area starts at 0xc000000000000000 and the kernel code and data is all at 0xffffffff80000000 and above. I don't know if the start and end are reasonable values, but I can see some logic as to where they come from. The code path that leads to this is via __vunmap() and __purge_vmap_area_lazy(). So it is not too difficult to see how we end up with values like this.

Either start or end address is sensible but not the combination - both addresses should be in the same segment. Start is in XKSEG, end in CKSEG2 and in between there are vast wastelands of unused address space exabytes in size.

Quoted 6 lines

But the size calculation above with these types of values will result in still a large number. Larger than the 32bit "int" that is "size". I see large negative values fall out as size, and so the following tlbsize check becomes true, and the code spins inside the loop inside that if statement for a _very_ long time trying to flush tlb entries.

Quoted 4 lines

This is of course easily fixed, by making that size "unsigned long". The patch below trivially does this.

But is this analysis correct?

Yes - but I think we have two issues here. The one is the calculation overflowing int for the arguments you're seeing. The other being that the arguments simply are looking wrong.

Quoted 17 lines · Ralf Baechle

Quoted 6 lines

Now the vmalloc area starts at 0xc000000000000000 and the kernel code and data is all at 0xffffffff80000000 and above. I don't know if the start and end are reasonable values, but I can see some logic as to where they come from. The code path that leads to this is via __vunmap() and __purge_vmap_area_lazy(). So it is not too difficult to see how we end up with values like this.

Either start or end address is sensible but not the combination - both addresses should be in the same segment. Start is in XKSEG, end in CKSEG2 and in between there are vast wastelands of unused address space exabytes in size.

Quoted 6 lines

But the size calculation above with these types of values will result in still a large number. Larger than the 32bit "int" that is "size". I see large negative values fall out as size, and so the following tlbsize check becomes true, and the code spins inside the loop inside that if statement for a _very_ long time trying to flush tlb entries.

Quoted 7 lines

Quoted 4 lines

This is of course easily fixed, by making that size "unsigned long". The patch below trivially does this.

But is this analysis correct?

Yes - but I think we have two issues here. The one is the calculation overflowing int for the arguments you're seeing. The other being that the arguments simply are looking wrong.

The wrong combination comes from lazy vunmapping which was introduced in 2.6.28 cycle. Maybe we can add new API (non-lazy version of vfree()) to vmalloc.c to implement module_free(), but I suppose fallbacking to local_flush_tlb_all() in local_flush_tlb_kernel_range() is enough().

Quoted 26 lines · Atsushi Nemoto

Quoted 17 lines · Ralf Baechle

Quoted 6 lines

Now the vmalloc area starts at 0xc000000000000000 and the kernel code and data is all at 0xffffffff80000000 and above. I don't know if the start and end are reasonable values, but I can see some logic as to where they come from. The code path that leads to this is via __vunmap() and __purge_vmap_area_lazy(). So it is not too difficult to see how we end up with values like this.

Either start or end address is sensible but not the combination - both addresses should be in the same segment. Start is in XKSEG, end in CKSEG2 and in between there are vast wastelands of unused address space exabytes in size.

Quoted 6 lines

But the size calculation above with these types of values will result in still a large number. Larger than the 32bit "int" that is "size". I see large negative values fall out as size, and so the following tlbsize check becomes true, and the code spins inside the loop inside that if statement for a _very_ long time trying to flush tlb entries.

Quoted 7 lines

Quoted 4 lines

This is of course easily fixed, by making that size "unsigned long". The patch below trivially does this.

But is this analysis correct?

Yes - but I think we have two issues here. The one is the calculation overflowing int for the arguments you're seeing. The other being that the arguments simply are looking wrong.

Quoted 5 lines

The wrong combination comes from lazy vunmapping which was introduced in 2.6.28 cycle. Maybe we can add new API (non-lazy version of vfree()) to vmalloc.c to implement module_free(), but I suppose fallbacking to local_flush_tlb_all() in local_flush_tlb_kernel_range() is enough().

Is there any performance impact on falling back to that?

The flushing due to lazy vunmapping didn't seem to happen often in the tests I was running.

Quoted 5 lines

The wrong combination comes from lazy vunmapping which was introduced in 2.6.28 cycle. Maybe we can add new API (non-lazy version of vfree()) to vmalloc.c to implement module_free(), but I suppose fallbacking to local_flush_tlb_all() in local_flush_tlb_kernel_range() is enough().

Is there any performance impact on falling back to that?

The flushing due to lazy vunmapping didn't seem to happen often in the tests I was running.