return hidden bug and unlock bugs

PATCH In review Roel Kluin 1 participant 2 messages Oct 22, 2007

Subsystems (13)

Networking Memory management Arm (32-bit) VFS +9 more subsystems

lore

Share & tools

lore

Roel Kluin

author

2007-10-22 17:51 UTC #0 lore raw

PATCH 1/1 return hidden bug and unlock bugs

This patches shouldn't alter behavior when CONFIG_BUG is disabled. It is meant as a replacement for the previous patches.

Concerning the patch changing fs/buffer.c, I am still wondering whether "page_cache_release(page)" should be placed before or after the BUG(). --
Unlock before BUG(), but don't change behavior in the case when CONFIG_BUG is disabled

Signed-off-by Roel Kluin <[email protected]>

---

arch/alpha/kernel/pci_iommu.c +1 -1

index e1c4707..6a69425 100644
@@ -365,8 +365,8 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t dma_addr, size_t size,
 		printk(KERN_ERR "Bogus pci_unmap_single: dma_addr %lx "
 		       " base %lx size %x\n", dma_addr, arena->dma_base,
 		       arena->size);
-		return;
 		BUG();
+		return;
 	}
 
 	npages = calc_npages((dma_addr & ~PAGE_MASK) + size);

arch/arm/mach-omap2/gpmc.c +1 -1

index 5a4cc20..12bd0eb 100644
@@ -357,8 +357,8 @@ void gpmc_cs_free(int cs)
 	spin_lock(&gpmc_mem_lock);
 	if (cs >= GPMC_CS_NUM || !gpmc_cs_reserved(cs)) {
 		printk(KERN_ERR "Trying to free non-reserved GPMC CS%d\n", cs);
-		BUG();
 		spin_unlock(&gpmc_mem_lock);
+		BUG();
 		return;
 	}
 	gpmc_cs_disable_mem(cs);

drivers/mtd/ubi/vmt.c +4 -1

index 88629a3..e72bead 100644
@@ -100,8 +100,11 @@ static ssize_t vol_attribute_show(struct device *dev,
 		ret = sprintf(buf, "%lld\n", vol->used_bytes);
 	else if (attr == &attr_vol_upd_marker)
 		ret = sprintf(buf, "%d\n", vol->upd_marker);
-	else
+	else {
+		spin_unlock(&vol->ubi->volumes_lock);
 		BUG();
+		return -EINVAL;
+	}
 	spin_unlock(&vol->ubi->volumes_lock);
 	return ret;
 }

drivers/net/wireless/ipw2200.c +2 -0

index e3c8284..f337578 100644
@@ -8722,7 +8722,9 @@ static int ipw_wx_get_freq(struct net_device *dev,
 			break;
 
 		default:
+			mutex_unlock(&priv->mutex);
 			BUG();
+			return -EINVAL;
 		}
 	} else
 		wrqu->freq.m = 0;

fs/buffer.c +1 -1

index 7249e01..90a3785 100644
@@ -1051,9 +1051,9 @@ grow_dev_page(struct block_device *bdev, sector_t block,
 	return page;
 
 failed:
-	BUG();
 	unlock_page(page);
 	page_cache_release(page);
+	BUG();
 	return NULL;
 }

mm/slab.c +7 -2

index cfa6be4..2dd48c1 100644
@@ -1604,11 +1604,16 @@ void __init kmem_cache_init(void)
 	/* 6) resize the head arrays to their final sizes */
 	{
 		struct kmem_cache *cachep;
+		int fail = 0;
 		mutex_lock(&cache_chain_mutex);
 		list_for_each_entry(cachep, &cache_chain, next)
-			if (enable_cpucache(cachep))
+			if (enable_cpucache(cachep)) {
+				fail = 1;
+				mutex_unlock(&cache_chain_mutex);
 				BUG();
-		mutex_unlock(&cache_chain_mutex);
+			}
+		if (fail != 1)
+			mutex_unlock(&cache_chain_mutex);
 	}
 
 	/* Annotate slab for lockdep -- annotate the malloc caches */

net/rxrpc/ar-ack.c +4 -1

index 657ee69..e551b0b 100644
@@ -752,8 +752,11 @@ all_acked:
 		sp->call = call;
 		rxrpc_get_call(call);
 		spin_lock_bh(&call->lock);
-		if (rxrpc_queue_rcv_skb(call, skb, true, true) < 0)
+		if (rxrpc_queue_rcv_skb(call, skb, true, true) < 0) {
+			spin_unlock_bh(&call->lock);
 			BUG();
+			goto process_further;
+		}
 		spin_unlock_bh(&call->lock);
 		goto process_further;
 	}

net/rxrpc/ar-call.c +5 -2

index 3c04b00..48804e1 100644
@@ -426,9 +426,12 @@ void rxrpc_release_call(struct rxrpc_call *call)
 	       call->rx_first_oos);
 
 	spin_lock_bh(&call->lock);
-	if (test_and_set_bit(RXRPC_CALL_RELEASED, &call->flags))
+	if (test_and_set_bit(RXRPC_CALL_RELEASED, &call->flags)) {
+		spin_unlock_bh(&call->lock);
 		BUG();
-	spin_unlock_bh(&call->lock);
+	} else {
+		spin_unlock_bh(&call->lock);
+	}
 
 	/* dissociate from the socket
 	 * - the socket's ref on the call is passed to the death timer

Roel Kluin

author

2007-10-22 19:12 UTC #1 lore raw

Quoted 5 lines · Roel Kluin

This patches shouldn't alter behavior when CONFIG_BUG is disabled. It is meant as a replacement for the previous patches.
Concerning the patch changing fs/buffer.c, I am still wondering whether "page_cache_release(page)" should be placed before or after the BUG().

...

Review pci_unmap_single(struct pci_dev *pdev, dma_addr_t dma_addr, size_t size, · line 370

Quoted 8 lines

(quoted diff) +1 -1

@@ -365,8 +365,8 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t dma_addr, size_t size,
 		printk(KERN_ERR "Bogus pci_unmap_single: dma_addr %lx "
 		       " base %lx size %x\n", dma_addr, arena->dma_base,
 		       arena->size);
-		return;
 		BUG();
+		return;
 	}

As suggested by Ray Lee, BUG() is changed into WARN_ON(1)

--
Unlock before BUG(), but don't change behavior in the case when CONFIG_BUG is disabled. Also changes a return-hidden BUG() in a WARN_ON(1)