[Cbe-oss-dev] [PATCH] ps3_storage scheduling while atomic
Florin Malita
fmalita at gmail.com
Thu Jan 18 11:44:38 EST 2007
While testing the latest PS3 patchset, I tried enabling CONFIG_PREEMPT
and the storage driver triggered this:
BUG: scheduling while atomic: ps3stor-1-0/0x00000001/183
Call Trace:
[C000000001B17AB0] [C00000000000F7D4] .show_stack+0x54/0x1f0 (unreliable)
[C000000001B17B60] [C000000000288C94] .schedule+0xbb4/0xc70
[C000000001B17CC0] [C000000000288EC8] .wait_for_completion+0xb8/0x140
[C000000001B17D80] [C0000000001B5704] .ps3_stor_common_handle_write+0x104/0x260
[C000000001B17E40] [C0000000001B5AC8] .ps3_stor_main_thread+0x108/0x160
[C000000001B17EE0] [C000000000067548] .kthread+0x158/0x170
[C000000001B17F90] [C000000000021D18] .kernel_thread+0x4c/0x68
...
Both ps3_stor_common_handle_read() and ps3_stor_common_handle_write()
call wait_for_completion() while holding a rw spinlock
(dev_info->bounce_lock). Looks like the only race is with
ps3_stor_set_max_sectors() so maybe there's a way to avoid locking
altogether but in the meantime here's a patch converting it to a
rw_semaphore.
Signed-off-by: Florin Malita <fmalita at gmail.com>
---
ps3_storage.c | 31 ++++++++++++++++---------------
ps3_storage.h | 10 +++++-----
2 files changed, 21 insertions(+), 20 deletions(-)
diff --git a/drivers/block/ps3_storage.c b/drivers/block/ps3_storage.c
index 7a9697c..a146151 100644
--- a/drivers/block/ps3_storage.c
+++ b/drivers/block/ps3_storage.c
@@ -30,6 +30,7 @@
#include <linux/blkdev.h>
#include <linux/kthread.h>
#include <linux/interrupt.h>
+#include <linux/rwsem.h>
#include <scsi/scsi.h>
#include <scsi/scsi_host.h>
#include <scsi/scsi_cmnd.h>
@@ -853,7 +854,7 @@ static int ps3_stor_common_handle_read(struct ps3_stor_dev_info * dev_info,
}
/* issue read */
- read_lock(&dev_info->bounce_lock);
+ down_read(&dev_info->bounce_sem);
lpar_addr = ps3_stor_virtual_to_lpar(dev_info, dev_info->bounce_buf);
region_id = lv1_dev_info->region_info_array[(cmnd[1] >> 5)].region_id;
init_completion(&(dev_info->irq_done));
@@ -895,7 +896,7 @@ static int ps3_stor_common_handle_read(struct ps3_stor_dev_info * dev_info,
}
ps3_stor_srb_done(dev_info);
- read_unlock(&dev_info->bounce_lock);
+ up_read(&dev_info->bounce_sem);
return ret;
}
@@ -929,7 +930,7 @@ static int ps3_stor_common_handle_write(struct ps3_stor_dev_info * dev_info,
break;
}
- read_lock(&dev_info->bounce_lock);
+ down_read(&dev_info->bounce_sem);
ret = fetch_to_dev_buffer(dev_info->srb,
dev_info->bounce_buf,
sectors * dev_info->sector_size);
@@ -973,7 +974,7 @@ static int ps3_stor_common_handle_write(struct ps3_stor_dev_info * dev_info,
}
ps3_stor_srb_done(dev_info);
- read_unlock(&dev_info->bounce_lock);
+ up_read(&dev_info->bounce_sem);
return ret;
}
@@ -1072,7 +1073,7 @@ static int ps3_stor_handle_write_flash(struct ps3_stor_dev_info * dev_info,
return ps3_stor_common_handle_write(dev_info, srb);
};
- read_lock(&dev_info->bounce_lock);
+ down_read(&dev_info->bounce_sem);
region_id = region_info->region_id;
@@ -1257,7 +1258,7 @@ static int ps3_stor_handle_write_flash(struct ps3_stor_dev_info * dev_info,
} /* for */
done:
ps3_stor_srb_done(dev_info);
- read_unlock(&dev_info->bounce_lock);
+ up_read(&dev_info->bounce_sem);
DPRINTK(KERN_ERR "%s: end\n", __FUNCTION__);
return ret;
}
@@ -1761,7 +1762,7 @@ static int ps3_stor_add_adapter(struct ps3_stor_lv1_bus_info * lv1_bus_info)
dev_info->host_info = host_info;
INIT_LIST_HEAD(&dev_info->dev_list);
spin_lock_init(&dev_info->srb_lock);
- rwlock_init(&dev_info->bounce_lock);
+ init_rwsem(&dev_info->bounce_sem);
list_add_tail(&dev_info->dev_list, &host_info->dev_info_list);
}
@@ -2165,7 +2166,7 @@ static int ps3_stor_slave_alloc(struct scsi_device * scsi_dev)
FUNC_STEP_C("3");
/* prepare dma regions for the device */
- write_lock(&dev_info->bounce_lock);
+ down_write(&dev_info->bounce_sem);
switch (get_dedicated_buffer_type(lv1_dev_info->device_type)) {
case DEDICATED_KMALLOC:
/*
@@ -2176,7 +2177,7 @@ static int ps3_stor_slave_alloc(struct scsi_device * scsi_dev)
/* create its own static bouce buffer */
dev_info->dedicated_bounce_size = get_default_max_sector(lv1_dev_info) * lv1_dev_info->sector_size;
dev_info->bounce_buf = kmalloc(dev_info->dedicated_bounce_size, GFP_KERNEL | __GFP_DMA);
- write_unlock(&dev_info->bounce_lock);
+ up_write(&dev_info->bounce_sem);
if (!dev_info->bounce_buf) {
printk(KERN_ERR "%s:kmalloc for static bounce buffer failed %#x\n", __FUNCTION__,
dev_info->dedicated_bounce_size);
@@ -2195,7 +2196,7 @@ static int ps3_stor_slave_alloc(struct scsi_device * scsi_dev)
error = -ENOMEM;
goto fail_free_irq;
}
- write_unlock(&dev_info->bounce_lock);
+ up_write(&dev_info->bounce_sem);
dev_info->bounce_type = DEDICATED_SPECIAL;
break;
}
@@ -2422,9 +2423,9 @@ static ssize_t ps3_stor_get_max_sectors(struct device *dev,
(struct ps3_stor_dev_info *)scsi_dev->hostdata;
ssize_t ret;
- read_lock(&dev_info->bounce_lock);
+ down_read(&dev_info->bounce_sem);
ret = sprintf(buf, "%u\n", scsi_dev->request_queue->max_sectors);
- read_unlock(&dev_info->bounce_lock);
+ up_read(&dev_info->bounce_sem);
return ret;
}
@@ -2446,12 +2447,12 @@ static ssize_t ps3_stor_set_max_sectors(struct device *dev,
/* FIXME: need remap dma region !!! */
return -EINVAL;
}
- write_lock(&dev_info->bounce_lock);
+ down_write(&dev_info->bounce_sem);
if (dev_info->bounce_type == DEDICATED_KMALLOC) {
/* try to allocate new bounce buffer */
bounce_buf = kmalloc(max_sectors * lv1_dev_info->sector_size, GFP_NOIO | __GFP_DMA | __GFP_NOWARN);
if (!bounce_buf) {
- write_unlock(&dev_info->bounce_lock);
+ up_write(&dev_info->bounce_sem);
return -ENOMEM;
}
kfree(dev_info->bounce_buf);
@@ -2459,7 +2460,7 @@ static ssize_t ps3_stor_set_max_sectors(struct device *dev,
dev_info->dedicated_bounce_size = max_sectors * lv1_dev_info->sector_size;
}
blk_queue_max_sectors(scsi_dev->request_queue, max_sectors);
- write_unlock(&dev_info->bounce_lock);
+ up_write(&dev_info->bounce_sem);
return strlen(buf);
}
return -EINVAL;
diff --git a/drivers/block/ps3_storage.h b/drivers/block/ps3_storage.h
index 97011f4..ea83549 100644
--- a/drivers/block/ps3_storage.h
+++ b/drivers/block/ps3_storage.h
@@ -163,11 +163,11 @@ struct ps3_stor_dev_info {
u64 sector_size; /* copied from lv1 repository at initialize */
/* devices may change these value */
- rwlock_t bounce_lock; /* protect the following members:
- * bounce_buf (pointer itself, not buffer),
- * dedicated_bounce_size
- * max_sectors in scsi_dev->request_queue
- */
+ struct rw_semaphore bounce_sem; /* protect the following members:
+ * bounce_buf (pointer itself, not buffer),
+ * dedicated_bounce_size
+ * max_sectors in scsi_dev->request_queue
+ */
int dedicated_bounce; /* set nonzero if the bounce buffer is dedicated */
int dedicated_bounce_size;
int dedicated_dma_region; /* set if partial dma region allocated */
More information about the cbe-oss-dev
mailing list