Re: [PATCH retry] return hidden bug and unlock bugs

2007-10-22 Thread Roel Kluin 
Roel Kluin wrote:
> 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().

...

> @@ -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)

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>

diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
index e1c4707..d04f151 100644
--- a/arch/alpha/kernel/pci_iommu.c
+++ b/arch/alpha/kernel/pci_iommu.c
@@ -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);
+   WARN_ON(1);
return;
-   BUG();
}
 
npages = calc_npages((dma_addr & ~PAGE_MASK) + size);
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 5a4cc20..12bd0eb 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -357,8 +357,8 @@ void gpmc_cs_free(int cs)
spin_lock(_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(_mem_lock);
+   BUG();
return;
}
gpmc_cs_disable_mem(cs);
diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
index 88629a3..e72bead 100644
--- a/drivers/mtd/ubi/vmt.c
+++ b/drivers/mtd/ubi/vmt.c
@@ -100,8 +100,11 @@ static ssize_t vol_attribute_show(struct device *dev,
ret = sprintf(buf, "%lld\n", vol->used_bytes);
else if (attr == _vol_upd_marker)
ret = sprintf(buf, "%d\n", vol->upd_marker);
-   else
+   else {
+   spin_unlock(>ubi->volumes_lock);
BUG();
+   return -EINVAL;
+   }
spin_unlock(>ubi->volumes_lock);
return ret;
 }
diff --git a/drivers/net/wireless/ipw2200.c b/drivers/net/wireless/ipw2200.c
index e3c8284..f337578 100644
--- a/drivers/net/wireless/ipw2200.c
+++ b/drivers/net/wireless/ipw2200.c
@@ -8722,7 +8722,9 @@ static int ipw_wx_get_freq(struct net_device *dev,
break;
 
default:
+   mutex_unlock(>mutex);
BUG();
+   return -EINVAL;
}
} else
wrqu->freq.m = 0;
diff --git a/fs/buffer.c b/fs/buffer.c
index 7249e01..90a3785 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -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;
 }
 
diff --git a/mm/slab.c b/mm/slab.c
index cfa6be4..2dd48c1 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -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(_chain_mutex);
list_for_each_entry(cachep, _chain, next)
-   if (enable_cpucache(cachep))
+   if (enable_cpucache(cachep)) {
+   fail = 1;
+   mutex_unlock(_chain_mutex);
BUG();
-   mutex_unlock(_chain_mutex);
+   }
+   if (fail != 1)
+   mutex_unlock(_chain_mutex);
}
 
/* Annotate slab for lockdep -- annotate the malloc caches */
diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
index 657ee69..e551b0b 100644
--- a/net/rxrpc/ar-ack.c
+++ b/net/rxrpc/ar-ack.c
@@ -752,8 +752,11 @@ all_acked:
sp->call = call;
rxrpc_get_call(call);
spin_lock_bh(>lock);
-   if (rxrpc_queue_rcv_skb(call, skb, true, true) < 0)
+   if (rxrpc_queue_rcv_skb(call, skb, true, true) < 0) {
+   spin_unlock_bh(>lock);
BUG();
+   goto process_further;
+   }
spin_unlock_bh(>lock);
goto process_further;

[PATCH retry] return hidden bug and unlock bugs

2007-10-22 Thread 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().
--
Unlock before BUG(), but don't change behavior in the case when
CONFIG_BUG is disabled

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
index e1c4707..6a69425 100644
--- a/arch/alpha/kernel/pci_iommu.c
+++ b/arch/alpha/kernel/pci_iommu.c
@@ -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);
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 5a4cc20..12bd0eb 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -357,8 +357,8 @@ void gpmc_cs_free(int cs)
spin_lock(_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(_mem_lock);
+   BUG();
return;
}
gpmc_cs_disable_mem(cs);
diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
index 88629a3..e72bead 100644
--- a/drivers/mtd/ubi/vmt.c
+++ b/drivers/mtd/ubi/vmt.c
@@ -100,8 +100,11 @@ static ssize_t vol_attribute_show(struct device *dev,
ret = sprintf(buf, "%lld\n", vol->used_bytes);
else if (attr == _vol_upd_marker)
ret = sprintf(buf, "%d\n", vol->upd_marker);
-   else
+   else {
+   spin_unlock(>ubi->volumes_lock);
BUG();
+   return -EINVAL;
+   }
spin_unlock(>ubi->volumes_lock);
return ret;
 }
diff --git a/drivers/net/wireless/ipw2200.c b/drivers/net/wireless/ipw2200.c
index e3c8284..f337578 100644
--- a/drivers/net/wireless/ipw2200.c
+++ b/drivers/net/wireless/ipw2200.c
@@ -8722,7 +8722,9 @@ static int ipw_wx_get_freq(struct net_device *dev,
break;
 
default:
+   mutex_unlock(>mutex);
BUG();
+   return -EINVAL;
}
} else
wrqu->freq.m = 0;
diff --git a/fs/buffer.c b/fs/buffer.c
index 7249e01..90a3785 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -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;
 }
 
diff --git a/mm/slab.c b/mm/slab.c
index cfa6be4..2dd48c1 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -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(_chain_mutex);
list_for_each_entry(cachep, _chain, next)
-   if (enable_cpucache(cachep))
+   if (enable_cpucache(cachep)) {
+   fail = 1;
+   mutex_unlock(_chain_mutex);
BUG();
-   mutex_unlock(_chain_mutex);
+   }
+   if (fail != 1)
+   mutex_unlock(_chain_mutex);
}
 
/* Annotate slab for lockdep -- annotate the malloc caches */
diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
index 657ee69..e551b0b 100644
--- a/net/rxrpc/ar-ack.c
+++ b/net/rxrpc/ar-ack.c
@@ -752,8 +752,11 @@ all_acked:
sp->call = call;
rxrpc_get_call(call);
spin_lock_bh(>lock);
-   if (rxrpc_queue_rcv_skb(call, skb, true, true) < 0)
+   if (rxrpc_queue_rcv_skb(call, skb, true, true) < 0) {
+   spin_unlock_bh(>lock);
BUG();
+   goto process_further;
+   }
spin_unlock_bh(>lock);
goto process_further;
}
diff --git a/net/rxrpc/ar-call.c b/net/rxrpc/ar-call.c
index 3c04b00..48804e1 100644
--- a/net/rxrpc/ar-call.c
+++ b/net/rxrpc/ar-call.c
@@ -426,9 +426,12 @@ void rxrpc_release_call(struct rxrpc_call *call)
   call->rx_first_oos);
 
spin_lock_bh(>lock);
-   if (test_and_set_bit(RXRPC_CALL_RELEASED, >flags))
+   if (test_and_set_bit(RXRPC_CALL_RELEASED, >flags)) {
+   spin_unlock_bh(>lock);
BUG();
-   spin_unlock_bh(>lock);
+   } else

[PATCH retry] return hidden bug and unlock bugs

2007-10-22 Thread 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().
--
Unlock before BUG(), but don't change behavior in the case when
CONFIG_BUG is disabled

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
index e1c4707..6a69425 100644
--- a/arch/alpha/kernel/pci_iommu.c
+++ b/arch/alpha/kernel/pci_iommu.c
@@ -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);
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 5a4cc20..12bd0eb 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -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);
diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
index 88629a3..e72bead 100644
--- a/drivers/mtd/ubi/vmt.c
+++ b/drivers/mtd/ubi/vmt.c
@@ -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;
 }
diff --git a/drivers/net/wireless/ipw2200.c b/drivers/net/wireless/ipw2200.c
index e3c8284..f337578 100644
--- a/drivers/net/wireless/ipw2200.c
+++ b/drivers/net/wireless/ipw2200.c
@@ -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;
diff --git a/fs/buffer.c b/fs/buffer.c
index 7249e01..90a3785 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -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;
 }
 
diff --git a/mm/slab.c b/mm/slab.c
index cfa6be4..2dd48c1 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -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 */
diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
index 657ee69..e551b0b 100644
--- a/net/rxrpc/ar-ack.c
+++ b/net/rxrpc/ar-ack.c
@@ -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;
}
diff --git a/net/rxrpc/ar-call.c b/net/rxrpc/ar-call.c
index 3c04b00..48804e1 100644
--- a/net/rxrpc/ar-call.c
+++ b/net/rxrpc/ar-call.c
@@ -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);

Re: [PATCH retry] return hidden bug and unlock bugs

2007-10-22 Thread Roel Kluin 
Roel Kluin wrote:
 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().

...

 @@ -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)

Signed-off-by: Roel Kluin [EMAIL PROTECTED]

diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
index e1c4707..d04f151 100644
--- a/arch/alpha/kernel/pci_iommu.c
+++ b/arch/alpha/kernel/pci_iommu.c
@@ -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);
+   WARN_ON(1);
return;
-   BUG();
}
 
npages = calc_npages((dma_addr  ~PAGE_MASK) + size);
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 5a4cc20..12bd0eb 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -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);
diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
index 88629a3..e72bead 100644
--- a/drivers/mtd/ubi/vmt.c
+++ b/drivers/mtd/ubi/vmt.c
@@ -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;
 }
diff --git a/drivers/net/wireless/ipw2200.c b/drivers/net/wireless/ipw2200.c
index e3c8284..f337578 100644
--- a/drivers/net/wireless/ipw2200.c
+++ b/drivers/net/wireless/ipw2200.c
@@ -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;
diff --git a/fs/buffer.c b/fs/buffer.c
index 7249e01..90a3785 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -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;
 }
 
diff --git a/mm/slab.c b/mm/slab.c
index cfa6be4..2dd48c1 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -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 */
diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
index 657ee69..e551b0b 100644
--- a/net/rxrpc/ar-ack.c
+++ b/net/rxrpc/ar-ack.c
@@ -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