Skip to contentSkip to conversation

add BUG_ON to 2.4 #2

PATCH New Robert Love 2 participants 4 messages Jan 25, 2002

Subsystems (7)
Memory management MM core Scheduler exec and binfmt +3 more subsystems
lore
Share & tools
lore

Robert Love

author
#0 lore raw

PATCH 1/1 add BUG_ON to 2.4 #2

This patch, which requires my previous patch which adds BUG_ON, changes some uses of BUG() to BUG_ON() in a few fast path areas in the kernel.

Patch against 2.4.18-pre7. Marcelo, please apply.

Robert Love

arch/i386/kernel/smp.c +1 -2
diff -urN linux-2.4.18-pre7/arch/i386/kernel/smp.c linux/arch/i386/kernel/smp.c
@@ -301,8 +301,7 @@
  */
 static void inline leave_mm (unsigned long cpu)
 {
-	if (cpu_tlbstate[cpu].state == TLBSTATE_OK)
-		BUG();
+	BUG_ON(cpu_tlbstate[cpu].state == TLBSTATE_OK);
 	clear_bit(cpu, &cpu_tlbstate[cpu].active_mm->cpu_vm_mask);
 }
arch/i386/kernel/smpboot.c +2 -4
diff -urN linux-2.4.18-pre7/arch/i386/kernel/smpboot.c linux/arch/i386/kernel/smpboot.c
@@ -130,8 +130,7 @@
 	 * Has to be in very low memory so we can execute
 	 * real-mode AP code.
 	 */
-	if (__pa(trampoline_base) >= 0x9F000)
-		BUG();
+	BUG_ON(__pa(trampoline_base) >= 0x9F000);
 }
 
 /*

@@ -1088,8 +1087,7 @@
 	connect_bsp_APIC();
 	setup_local_APIC();
 
-	if (GET_APIC_ID(apic_read(APIC_ID)) != boot_cpu_physical_apicid)
-		BUG();
+	BUG_ON(GET_APIC_ID(apic_read(APIC_ID)) != boot_cpu_physical_apicid);
 
 	/*
 	 * Scan the CPU present map and fire up the other CPUs via do_boot_cpu
kernel/exit.c +1 -1
diff -urN linux-2.4.18-pre7/kernel/exit.c linux/kernel/exit.c
@@ -316,7 +316,7 @@
 	mm_release();
 	if (mm) {
 		atomic_inc(&mm->mm_count);
-		if (mm != tsk->active_mm) BUG();
+		BUG_ON(mm != tsk->active_mm);
 		/* more a memory barrier than a real lock */
 		task_lock(tsk);
 		tsk->mm = NULL;
kernel/fork.c +1 -1
diff -urN linux-2.4.18-pre7/kernel/fork.c linux/kernel/fork.c
@@ -249,7 +249,7 @@
  */
 inline void __mmdrop(struct mm_struct *mm)
 {
-	if (mm == &init_mm) BUG();
+	BUG_ON(mm == &init_mm);
 	pgd_free(mm->pgd);
 	destroy_context(mm);
 	free_mm(mm);
kernel/sched.c +1 -1
diff -urN linux-2.4.18-pre7/kernel/sched.c linux/kernel/sched.c
@@ -556,7 +556,7 @@
 
 	spin_lock_prefetch(&runqueue_lock);
 
-	if (!current->active_mm) BUG();
+	BUG_ON(!current->active_mm);
 need_resched_back:
 	prev = current;
 	this_cpu = prev->processor;
mm/filemap.c +3 -5
diff -urN linux-2.4.18-pre7/mm/filemap.c linux/mm/filemap.c
@@ -120,7 +120,7 @@
  */
 void __remove_inode_page(struct page *page)
 {
-	if (PageDirty(page)) BUG();
+	BUG_ON(PageDirty(page));
 	remove_page_from_inode_queue(page);
 	remove_page_from_hash_queue(page);
 }

@@ -628,8 +628,7 @@
  */
 void add_to_page_cache_locked(struct page * page, struct address_space *mapping, unsigned long index)
 {
-	if (!PageLocked(page))
-		BUG();
+	BUG_ON(!PageLocked(page));
 
 	page->index = index;
 	page_cache_get(page);

@@ -2078,8 +2077,7 @@
 
 	dir = pgd_offset(vma->vm_mm, address);
 	flush_cache_range(vma->vm_mm, end - size, end);
-	if (address >= end)
-		BUG();
+	BUG_ON(address >= end);
 	do {
 		error |= filemap_sync_pmd_range(dir, address, end - address, vma, flags);
 		address = (address + PGDIR_SIZE) & PGDIR_MASK;
mm/slab.c +4 -6
diff -urN linux-2.4.18-pre7/mm/slab.c linux/mm/slab.c
@@ -666,8 +666,7 @@
 	 * Always checks flags, a caller might be expecting debug
 	 * support which isn't available.
 	 */
-	if (flags & ~CREATE_MASK)
-		BUG();
+	BUG_ON(flags & ~CREATE_MASK);
 
 	/* Get cache's description obj. */
 	cachep = (kmem_cache_t *) kmem_cache_alloc(&cache_cache, SLAB_KERNEL);

@@ -811,8 +810,7 @@
 			kmem_cache_t *pc = list_entry(p, kmem_cache_t, next);
 
 			/* The name field is constant - no lock needed. */
-			if (!strcmp(pc->name, name))
-				BUG();
+			BUG_ON(!strcmp(pc->name, name));
 		}
 	}
 

@@ -1095,8 +1093,8 @@
 	/* Be lazy and only check for valid flags here,
  	 * keeping it out of the critical path in kmem_cache_alloc().
 	 */
-	if (flags & ~(SLAB_DMA|SLAB_LEVEL_MASK|SLAB_NO_GROW))
-		BUG();
+	BUG_ON(flags & ~(SLAB_DMA|SLAB_LEVEL_MASK|SLAB_NO_GROW));
+
 	if (flags & SLAB_NO_GROW)
 		return 0;
mm/vmscan.c +4 -6
diff -urN linux-2.4.18-pre7/mm/vmscan.c linux/mm/vmscan.c
@@ -234,8 +234,8 @@
 	pgdir = pgd_offset(mm, address);
 
 	end = vma->vm_end;
-	if (address >= end)
-		BUG();
+	BUG_ON(address >= end);
+
 	do {
 		count = swap_out_pgd(mm, vma, pgdir, address, end, count, classzone);
 		if (!count)

@@ -354,10 +354,8 @@
 
 		page = list_entry(entry, struct page, lru);
 
-		if (unlikely(!PageLRU(page)))
-			BUG();
-		if (unlikely(PageActive(page)))
-			BUG();
+		BUG_ON(unlikely(!PageLRU(page)));
+		BUG_ON(unlikely(PageActive(page)))
 
 		list_del(entry);
 		list_add(entry, &inactive_list);
#1 lore raw

PATCH 1/1 add BUG_ON to 2.4 #2

Robert Love writes:
...

Quoted 3 lines

diff -urN linux-2.4.18-pre7/mm/vmscan.c linux/mm/vmscan.c

mm/vmscan.c +0 -0

...

Quoted 4 lines
mm/vmscan.c +0 -0
@@ -354,10 +354,8 @@
 
 		page = list_entry(entry, struct page, lru);

mm/vmscan.c line 361

Quoted 10 lines
mm/vmscan.c +2 -4
 
-		if (unlikely(!PageLRU(page)))
-			BUG();
-		if (unlikely(PageActive(page)))
-			BUG();
+		BUG_ON(unlikely(!PageLRU(page)));
+		BUG_ON(unlikely(PageActive(page)))
 
 		list_del(entry);
 		list_add(entry, &inactive_list);

While I don't know the exact definitions, this works out to:

        if (unlikely(unlikely(!PageLRU(page)))) BUG();

which I doubt is a good idea.

--David Garfield

author
#2 lore raw
Quoted 5 lines · David Garfield

While I don't know the exact definitions, this works out to:

        if (unlikely(unlikely(!PageLRU(page)))) BUG();

which I doubt is a good idea.

Probably doesn't hurt but obviously not what I intended. Attached patch is fixed. Sorry.

Robert Love

diff -urN linux-2.4.18-pre7/arch/i386/kernel/smp.c linux/arch/i386/kernel/smp.c

arch/i386/kernel/smp.c +1 -2
@@ -301,8 +301,7 @@
  */
 static void inline leave_mm (unsigned long cpu)
 {
-	if (cpu_tlbstate[cpu].state == TLBSTATE_OK)
-		BUG();
+	BUG_ON(cpu_tlbstate[cpu].state == TLBSTATE_OK);
 	clear_bit(cpu, &cpu_tlbstate[cpu].active_mm->cpu_vm_mask);
 }

diff -urN linux-2.4.18-pre7/arch/i386/kernel/smpboot.c linux/arch/i386/kernel/smpboot.c
--- linux-2.4.18-pre7/arch/i386/kernel/smpboot.c Thu Jan 24 13:48:49
2002
+++ linux/arch/i386/kernel/smpboot.c Fri Jan 25 18:00:50 2002
@@ -130,8 +130,7 @@
* Has to be in very low memory so we can execute
* real-mode AP code.
*/
- if (__pa(trampoline_base) >= 0x9F000)
- BUG();
+ BUG_ON(__pa(trampoline_base) >= 0x9F000);
}

/*
@@ -1088,8 +1087,7 @@
connect_bsp_APIC();
setup_local_APIC();

- if (GET_APIC_ID(apic_read(APIC_ID)) != boot_cpu_physical_apicid)
- BUG();
+ BUG_ON(GET_APIC_ID(apic_read(APIC_ID)) != boot_cpu_physical_apicid);

/*
* Scan the CPU present map and fire up the other CPUs via do_boot_cpu

kernel/exit.c +1 -1
diff -urN linux-2.4.18-pre7/kernel/exit.c linux/kernel/exit.c
@@ -316,7 +316,7 @@
 	mm_release();
 	if (mm) {
 		atomic_inc(&mm->mm_count);
-		if (mm != tsk->active_mm) BUG();
+		BUG_ON(mm != tsk->active_mm);
 		/* more a memory barrier than a real lock */
 		task_lock(tsk);
 		tsk->mm = NULL;
kernel/fork.c +1 -1
diff -urN linux-2.4.18-pre7/kernel/fork.c linux/kernel/fork.c
@@ -249,7 +249,7 @@
  */
 inline void __mmdrop(struct mm_struct *mm)
 {
-	if (mm == &init_mm) BUG();
+	BUG_ON(mm == &init_mm);
 	pgd_free(mm->pgd);
 	destroy_context(mm);
 	free_mm(mm);
kernel/sched.c +1 -1
diff -urN linux-2.4.18-pre7/kernel/sched.c linux/kernel/sched.c
@@ -556,7 +556,7 @@
 
 	spin_lock_prefetch(&runqueue_lock);
 
-	if (!current->active_mm) BUG();
+	BUG_ON(!current->active_mm);
 need_resched_back:
 	prev = current;
 	this_cpu = prev->processor;
mm/filemap.c +1 -1
diff -urN linux-2.4.18-pre7/mm/filemap.c linux/mm/filemap.c
@@ -120,7 +120,7 @@
  */
 void __remove_inode_page(struct page *page)
 {
-	if (PageDirty(page)) BUG();
+	BUG_ON(PageDirty(page));
 	remove_page_from_inode_queue(page);
 	remove_page_from_hash_queue(page);
 }

@@ -628,8 +628,7 @@
  */
 void add_to_page_cache_locked(struct page * page, struct address_space

*mapping, unsigned long index)
{
- if (!PageLocked(page))
- BUG();
+ BUG_ON(!PageLocked(page));

page->index = index;
page_cache_get(page);
@@ -2078,8 +2077,7 @@

dir = pgd_offset(vma->vm_mm, address); flush_cache_range(vma->vm_mm, end - size, end);
- if (address >= end)
- BUG();
+ BUG_ON(address >= end);
do {
error |= filemap_sync_pmd_range(dir, address, end - address, vma,
flags);
address = (address + PGDIR_SIZE) & PGDIR_MASK;

mm/slab.c +4 -6
diff -urN linux-2.4.18-pre7/mm/slab.c linux/mm/slab.c
@@ -666,8 +666,7 @@
 	 * Always checks flags, a caller might be expecting debug
 	 * support which isn't available.
 	 */
-	if (flags & ~CREATE_MASK)
-		BUG();
+	BUG_ON(flags & ~CREATE_MASK);
 
 	/* Get cache's description obj. */
 	cachep = (kmem_cache_t *) kmem_cache_alloc(&cache_cache, SLAB_KERNEL);

@@ -811,8 +810,7 @@
 			kmem_cache_t *pc = list_entry(p, kmem_cache_t, next);
 
 			/* The name field is constant - no lock needed. */
-			if (!strcmp(pc->name, name))
-				BUG();
+			BUG_ON(!strcmp(pc->name, name));
 		}
 	}
 

@@ -1095,8 +1093,8 @@
 	/* Be lazy and only check for valid flags here,
  	 * keeping it out of the critical path in kmem_cache_alloc().
 	 */
-	if (flags & ~(SLAB_DMA|SLAB_LEVEL_MASK|SLAB_NO_GROW))
-		BUG();
+	BUG_ON(flags & ~(SLAB_DMA|SLAB_LEVEL_MASK|SLAB_NO_GROW));
+
 	if (flags & SLAB_NO_GROW)
 		return 0;
mm/vmscan.c +4 -6
diff -urN linux-2.4.18-pre7/mm/vmscan.c linux/mm/vmscan.c
@@ -234,8 +234,8 @@
 	pgdir = pgd_offset(mm, address);
 
 	end = vma->vm_end;
-	if (address >= end)
-		BUG();
+	BUG_ON(address >= end);
+
 	do {
 		count = swap_out_pgd(mm, vma, pgdir, address, end, count, classzone);
 		if (!count)

@@ -354,10 +354,8 @@
 
 		page = list_entry(entry, struct page, lru);
 
-		if (unlikely(!PageLRU(page)))
-			BUG();
-		if (unlikely(PageActive(page)))
-			BUG();
+		BUG_ON(!PageLRU(page));
+		BUG_ON(PageActive(page))
 
 		list_del(entry);
 		list_add(entry, &inactive_list);
author
#3 lore raw
Quoted 2 lines · Robert Love

Probably doesn't hurt but obviously not what I intended. Attached patch is fixed. Sorry.

Ugh my formatting was bad there. Attached patch is correct. Sorry, Marcelo.

Robert Love

arch/i386/kernel/smp.c +1 -2
diff -urN linux-2.4.18-pre7/arch/i386/kernel/smp.c linux/arch/i386/kernel/smp.c
@@ -301,8 +301,7 @@
  */
 static void inline leave_mm (unsigned long cpu)
 {
-	if (cpu_tlbstate[cpu].state == TLBSTATE_OK)
-		BUG();
+	BUG_ON(cpu_tlbstate[cpu].state == TLBSTATE_OK);
 	clear_bit(cpu, &cpu_tlbstate[cpu].active_mm->cpu_vm_mask);
 }
arch/i386/kernel/smpboot.c +2 -4
diff -urN linux-2.4.18-pre7/arch/i386/kernel/smpboot.c linux/arch/i386/kernel/smpboot.c
@@ -130,8 +130,7 @@
 	 * Has to be in very low memory so we can execute
 	 * real-mode AP code.
 	 */
-	if (__pa(trampoline_base) >= 0x9F000)
-		BUG();
+	BUG_ON(__pa(trampoline_base) >= 0x9F000);
 }
 
 /*

@@ -1088,8 +1087,7 @@
 	connect_bsp_APIC();
 	setup_local_APIC();
 
-	if (GET_APIC_ID(apic_read(APIC_ID)) != boot_cpu_physical_apicid)
-		BUG();
+	BUG_ON(GET_APIC_ID(apic_read(APIC_ID)) != boot_cpu_physical_apicid);
 
 	/*
 	 * Scan the CPU present map and fire up the other CPUs via do_boot_cpu
kernel/exit.c +1 -1
diff -urN linux-2.4.18-pre7/kernel/exit.c linux/kernel/exit.c
@@ -316,7 +316,7 @@
 	mm_release();
 	if (mm) {
 		atomic_inc(&mm->mm_count);
-		if (mm != tsk->active_mm) BUG();
+		BUG_ON(mm != tsk->active_mm);
 		/* more a memory barrier than a real lock */
 		task_lock(tsk);
 		tsk->mm = NULL;
kernel/fork.c +1 -1
diff -urN linux-2.4.18-pre7/kernel/fork.c linux/kernel/fork.c
@@ -249,7 +249,7 @@
  */
 inline void __mmdrop(struct mm_struct *mm)
 {
-	if (mm == &init_mm) BUG();
+	BUG_ON(mm == &init_mm);
 	pgd_free(mm->pgd);
 	destroy_context(mm);
 	free_mm(mm);
kernel/sched.c +1 -1
diff -urN linux-2.4.18-pre7/kernel/sched.c linux/kernel/sched.c
@@ -556,7 +556,7 @@
 
 	spin_lock_prefetch(&runqueue_lock);
 
-	if (!current->active_mm) BUG();
+	BUG_ON(!current->active_mm);
 need_resched_back:
 	prev = current;
 	this_cpu = prev->processor;
mm/filemap.c +3 -5
diff -urN linux-2.4.18-pre7/mm/filemap.c linux/mm/filemap.c
@@ -120,7 +120,7 @@
  */
 void __remove_inode_page(struct page *page)
 {
-	if (PageDirty(page)) BUG();
+	BUG_ON(PageDirty(page));
 	remove_page_from_inode_queue(page);
 	remove_page_from_hash_queue(page);
 }

@@ -628,8 +628,7 @@
  */
 void add_to_page_cache_locked(struct page * page, struct address_space *mapping, unsigned long index)
 {
-	if (!PageLocked(page))
-		BUG();
+	BUG_ON(!PageLocked(page));
 
 	page->index = index;
 	page_cache_get(page);

@@ -2078,8 +2077,7 @@
 
 	dir = pgd_offset(vma->vm_mm, address);
 	flush_cache_range(vma->vm_mm, end - size, end);
-	if (address >= end)
-		BUG();
+	BUG_ON(address >= end);
 	do {
 		error |= filemap_sync_pmd_range(dir, address, end - address, vma, flags);
 		address = (address + PGDIR_SIZE) & PGDIR_MASK;
mm/slab.c +4 -6
diff -urN linux-2.4.18-pre7/mm/slab.c linux/mm/slab.c
@@ -666,8 +666,7 @@
 	 * Always checks flags, a caller might be expecting debug
 	 * support which isn't available.
 	 */
-	if (flags & ~CREATE_MASK)
-		BUG();
+	BUG_ON(flags & ~CREATE_MASK);
 
 	/* Get cache's description obj. */
 	cachep = (kmem_cache_t *) kmem_cache_alloc(&cache_cache, SLAB_KERNEL);

@@ -811,8 +810,7 @@
 			kmem_cache_t *pc = list_entry(p, kmem_cache_t, next);
 
 			/* The name field is constant - no lock needed. */
-			if (!strcmp(pc->name, name))
-				BUG();
+			BUG_ON(!strcmp(pc->name, name));
 		}
 	}
 

@@ -1095,8 +1093,8 @@
 	/* Be lazy and only check for valid flags here,
  	 * keeping it out of the critical path in kmem_cache_alloc().
 	 */
-	if (flags & ~(SLAB_DMA|SLAB_LEVEL_MASK|SLAB_NO_GROW))
-		BUG();
+	BUG_ON(flags & ~(SLAB_DMA|SLAB_LEVEL_MASK|SLAB_NO_GROW));
+
 	if (flags & SLAB_NO_GROW)
 		return 0;
mm/vmscan.c +4 -6
diff -urN linux-2.4.18-pre7/mm/vmscan.c linux/mm/vmscan.c
@@ -234,8 +234,8 @@
 	pgdir = pgd_offset(mm, address);
 
 	end = vma->vm_end;
-	if (address >= end)
-		BUG();
+	BUG_ON(address >= end);
+
 	do {
 		count = swap_out_pgd(mm, vma, pgdir, address, end, count, classzone);
 		if (!count)

@@ -354,10 +354,8 @@
 
 		page = list_entry(entry, struct page, lru);
 
-		if (unlikely(!PageLRU(page)))
-			BUG();
-		if (unlikely(PageActive(page)))
-			BUG();
+		BUG_ON(!PageLRU(page));
+		BUG_ON(PageActive(page))
 
 		list_del(entry);
 		list_add(entry, &inactive_list);