This patch tries to address the following
CURRENT
o remove racy spin_unlock_irq() ... foo() .. spin_lock_irq() scenarios
o moves locking from interrupt handling helpers/primitives (ide-disk.c:*_intr) to the caller.
o removes ide__sti() from some areas, personally i think this is an evil
macro for many reasons, is it supposed to be disable_irq(ide_irq); __sti(); ?
o fixes some 'FIXME' entries... adds a couple more ;)
TODO
o fix breakage in ide-floppy/tape etc...
o fix ide device setup/tear down locking
o remove ide_wait commands from interrupt paths
PERF TUNING
o reduce amount of time spent with interrupts disabled
o try and reduce lock contention between block and ide layer?
Note, this patch is far from complete and i'm currently working on the
setup time locking (ide-disk_setup etc) i'd just like to know wether this
is heading in the right direction.
Any input would be appreciated,
Thanks,
Zwane Mwaikambo
Index: linux-2.5.24/drivers/ide/ide.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.24/drivers/ide/ide.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 ide.c
--- linux-2.5.24/drivers/ide/ide.c 23 Jun 2002 13:32:22 -0000 1.1.1.1
+++ linux-2.5.24/drivers/ide/ide.c 23 Jun 2002 21:20:29 -0000
@@ -107,7 +107,7 @@
}
/*
- * Not locking variabt of the end_request method.
+ * None locking variant of the end_request method.
*
* Channel lock should be held.
*/
@@ -152,13 +152,11 @@
*/
int ata_end_request(struct ata_device *drive, struct request *rq, int uptodate)
{
- unsigned long flags;
struct ata_channel *ch = drive->channel;
int ret;
- spin_lock_irqsave(ch->lock, flags);
+ BUG_ON(!spin_is_locked(ch->lock));
ret = __ata_end_request(drive, rq, uptodate, 0);
- spin_unlock_irqrestore(drive->channel->lock, flags);
return ret;
}
@@ -374,8 +372,7 @@
unsigned long flags;
struct ata_channel *ch = drive->channel;
- __save_flags(flags); /* local CPU only */
- __cli(); /* local CPU only */
+ local_irq_save(flags);
/* For an ATAPI device, first try an ATAPI SRST. */
if (try_atapi) {
@@ -385,7 +382,7 @@
OUT_BYTE(WIN_SRST, IDE_COMMAND_REG);
ch->poll_timeout = jiffies + WAIT_WORSTCASE;
ata_set_handler(drive, atapi_reset_pollfunc, HZ/20, NULL);
- __restore_flags(flags); /* local CPU only */
+ local_irq_restore(flags);
return ide_started;
}
@@ -398,8 +395,7 @@
for (unit = 0; unit < MAX_DRIVES; ++unit)
check_crc_errors(&ch->drives[unit]);
- __restore_flags(flags); /* local CPU only */
-
+ local_irq_restore(flags);
return ide_started;
}
@@ -467,8 +463,10 @@
unsigned long flags;
u8 err = 0;
- __save_flags (flags); /* local CPU only */
- ide__sti(); /* local CPU only */
+ /* FIXME: This shouldn't be necessary as we should have the lock and irqs
+ * disabled on entry -zwane
+ */
+ local_irq_save(flags);
printk("%s: %s: status=0x%02x", drive->name, msg, drive->status);
dump_bits(ata_status_msgs, ARRAY_SIZE(ata_status_msgs), drive->status);
@@ -516,7 +514,8 @@
#endif
printk("\n");
}
- __restore_flags (flags); /* local CPU only */
+
+ local_irq_restore(flags);
return err;
}
@@ -626,8 +625,12 @@
++rq->errors;
if ((rq->errors & ERROR_RESET) == ERROR_RESET)
return do_reset1(drive, 1);
- if ((rq->errors & ERROR_RECAL) == ERROR_RECAL)
- return do_recalibrate(drive);
+ if ((rq->errors & ERROR_RECAL) == ERROR_RECAL) {
+ ide_startstop_t ret;
+ /* FIXME: do_recalibrate tries to acquire the channel lock -zwane */
+ ret = do_recalibrate(drive);
+ return ret;
+ }
}
return ide_stopped;
@@ -702,9 +705,7 @@
kill_rq:
if (ata_ops(drive)) {
if (ata_ops(drive)->end_request) {
- spin_unlock_irq(ch->lock);
ata_ops(drive)->end_request(drive, rq, 0);
- spin_lock_irq(ch->lock);
} else
__ata_end_request(drive, rq, 0, 0);
} else
@@ -894,7 +895,6 @@
drive->rq = rq;
- ide__sti(); /* allow other IRQs while we start this request */
startstop = start_request(drive, rq);
/* command started, we are busy */
@@ -921,7 +921,6 @@
static void do_request(struct ata_channel *channel)
{
ide_get_lock(&ide_irq_lock, ata_irq_request, channel);/* for atari only: POSSIBLY BROKEN HERE(?) */
-// __cli(); /* necessary paranoia: ensure IRQs are masked on local CPU */
while (!test_and_set_bit(IDE_BUSY, channel->active)) {
struct ata_channel *ch;
@@ -1021,15 +1020,12 @@
handler = ch->handler;
ch->handler = NULL;
- spin_unlock(ch->lock);
-
ch = drive->channel;
#if DISABLE_IRQ_NOSYNC
disable_irq_nosync(ch->irq);
#else
disable_irq(ch->irq); /* disable_irq_nosync ?? */
#endif
- __cli(); /* local CPU only, as if we were handling an interrupt */
if (ch->poll_timeout) {
ret = handler(drive, drive->rq);
} else if (drive_is_ready(drive)) {
@@ -1078,8 +1074,6 @@
enable_irq(ch->irq);
- spin_lock_irq(ch->lock);
-
if (ret == ide_stopped)
clear_bit(IDE_BUSY, ch->active);
@@ -1151,12 +1145,11 @@
void ata_irq_request(int irq, void *data, struct pt_regs *regs)
{
struct ata_channel *ch = data;
- unsigned long flags;
struct ata_device *drive;
ata_handler_t *handler = ch->handler;
ide_startstop_t startstop;
- spin_lock_irqsave(ch->lock, flags);
+ spin_lock(ch->lock);
if (!ide_ack_intr(ch))
goto out_lock;
@@ -1211,14 +1204,14 @@
ch->handler = NULL;
del_timer(&ch->timer);
- spin_unlock(ch->lock);
-
- if (ch->unmask)
- ide__sti(); /* local CPU only */
+ if (ch->unmask) {
+ /* FIXME: perhaps disable_irq(irq); __sti(); ? -Zwane
+ ide__sti();
+ */
+ }
/* service this interrupt, may set handler for next interrupt */
startstop = handler(drive, drive->rq);
- spin_lock_irq(ch->lock);
/*
* Note that handler() may have set things up for another
@@ -1238,7 +1231,7 @@
queue_commands(drive);
out_lock:
- spin_unlock_irqrestore(ch->lock, flags);
+ spin_unlock(ch->lock);
}
static int ide_open(struct inode * inode, struct file * filp)
Index: linux-2.5.24/drivers/ide/ide-cd.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.24/drivers/ide/ide-cd.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 ide-cd.c
--- linux-2.5.24/drivers/ide/ide-cd.c 23 Jun 2002 13:32:22 -0000 1.1.1.1
+++ linux-2.5.24/drivers/ide/ide-cd.c 23 Jun 2002 20:45:52 -0000
@@ -319,6 +319,8 @@
/****************************************************************************
* Generic packet command support and error handling routines.
+ * Note. these are called with the channel lock held and irq disabled in most
+ * cases.
*/
/* Mark that we've seen a media change, and invalidate our internal
@@ -728,13 +730,13 @@
int xferlen,
ata_handler_t handler)
{
- unsigned long flags;
struct ata_channel *ch = drive->channel;
ide_startstop_t startstop;
struct cdrom_info *info = drive->driver_data;
int ret;
- spin_lock_irqsave(ch->lock, flags);
+ BUG_ON(!spin_is_locked(ch->lock));
+
/* Wait for the controller to be idle. */
if (ata_status_poll(drive, 0, BUSY_STAT, WAIT_READY, rq, &startstop))
ret = startstop;
@@ -763,16 +765,9 @@
ret = ide_started;
} else {
OUT_BYTE (WIN_PACKETCMD, IDE_COMMAND_REG); /* packet command */
-
- /* FIXME: Oj kurwa! We have to ungrab the lock before
- * the IRQ handler gets called.
- */
- spin_unlock_irqrestore(ch->lock, flags);
ret = handler(drive, rq);
- spin_lock_irqsave(ch->lock, flags);
}
}
- spin_unlock_irqrestore(ch->lock, flags);
return ret;
}
@@ -787,10 +782,11 @@
unsigned char *cmd, unsigned long timeout,
ata_handler_t handler)
{
- unsigned long flags;
struct ata_channel *ch = drive->channel;
ide_startstop_t startstop;
+ BUG_ON(!spin_is_locked(ch->lock));
+
if (CDROM_CONFIG_FLAGS (drive)->drq_interrupt) {
/* Here we should have been called after receiving an interrupt
from the device. DRQ should how be set. */
@@ -800,24 +796,16 @@
if (cdrom_decode_status(&startstop, drive, rq, DRQ_STAT, &stat_dum))
return startstop;
} else {
- /* FIXME: make this locking go away */
- spin_lock_irqsave(ch->lock, flags);
/* Otherwise, we must wait for DRQ to get set. */
if (ata_status_poll(drive, DRQ_STAT, BUSY_STAT,
WAIT_READY, rq, &startstop)) {
- spin_unlock_irqrestore(ch->lock, flags);
-
return startstop;
}
- spin_unlock_irqrestore(ch->lock, flags);
}
/* Arm the interrupt handler and send the command to the device. */
- /* FIXME: make this locking go away */
- spin_lock_irqsave(ch->lock, flags);
ata_set_handler(drive, handler, timeout, cdrom_timer_expiry);
atapi_write(drive, cmd, CDROM_PACKET_SIZE);
- spin_unlock_irqrestore(ch->lock, flags);
return ide_started;
}
@@ -917,7 +905,6 @@
*/
static ide_startstop_t cdrom_read_intr(struct ata_device *drive, struct request *rq)
{
- unsigned long flags;
struct ata_channel *ch = drive->channel;
int stat;
int ireason, len, sectors_to_transfer, nskip;
@@ -925,6 +912,8 @@
int dma = info->dma, dma_error = 0;
ide_startstop_t startstop;
+ BUG_ON(!spin_is_locked(ch->lock));
+
/* Check for errors. */
if (dma) {
info->dma = 0;
@@ -937,14 +926,7 @@
if (dma) {
if (!dma_error) {
- /* FIXME: this locking should encompass the above register
- * file access too.
- */
-
- spin_lock_irqsave(ch->lock, flags);
__ata_end_request(drive, rq, 1, rq->nr_sectors);
- spin_unlock_irqrestore(ch->lock, flags);
-
return ide_stopped;
} else
return ata_error(drive, rq, "dma error");
@@ -1040,10 +1022,8 @@
}
/* Done moving data! Wait for another interrupt. */
- spin_lock_irqsave(ch->lock, flags);
ata_set_handler(drive, cdrom_read_intr, WAIT_CMD, NULL);
- spin_unlock_irqrestore(ch->lock, flags);
-
+
return ide_started;
}
@@ -1269,7 +1249,6 @@
/* Interrupt routine for packet command completion. */
static ide_startstop_t cdrom_pc_intr(struct ata_device *drive, struct request *rq)
{
- unsigned long flags;
struct ata_channel *ch = drive->channel;
int ireason, len, stat, thislen;
@@ -1277,6 +1256,8 @@
struct packet_command *pc = (struct packet_command *) rq->special;
ide_startstop_t startstop;
+ BUG_ON(!spin_is_locked(ch->lock));
+
/* Check for errors. */
if (cdrom_decode_status (&startstop, drive, rq, 0, &stat))
return startstop;
@@ -1363,9 +1344,7 @@
}
/* Now we wait for another interrupt. */
- spin_lock_irqsave(ch->lock, flags);
ata_set_handler(drive, cdrom_pc_intr, WAIT_CMD, cdrom_timer_expiry);
- spin_unlock_irqrestore(ch->lock, flags);
return ide_started;
}
@@ -1508,13 +1487,14 @@
static ide_startstop_t cdrom_write_intr(struct ata_device *drive, struct request *rq)
{
- unsigned long flags;
struct ata_channel *ch = drive->channel;
int stat, ireason, len, sectors_to_transfer, uptodate;
struct cdrom_info *info = drive->driver_data;
int dma_error = 0, dma = info->dma;
ide_startstop_t startstop;
+ BUG_ON(!spin_is_locked(ch->lock));
+
/* Check for errors. */
if (dma) {
info->dma = 0;
@@ -1536,14 +1516,7 @@
if (dma_error)
return ata_error(drive, rq, "dma error");
- /* FIXME: this locking should encompass the above register
- * file access too.
- */
-
- spin_lock_irqsave(ch->lock, flags);
__ata_end_request(drive, rq, 1, rq->nr_sectors);
- spin_unlock_irqrestore(ch->lock, flags);
-
return ide_stopped;
}
@@ -1607,9 +1580,7 @@
}
/* re-arm handler */
- spin_lock_irqsave(ch->lock, flags);
ata_set_handler(drive, cdrom_write_intr, 5 * WAIT_CMD, NULL);
- spin_unlock_irqrestore(ch->lock, flags);
return ide_started;
}
@@ -1662,6 +1633,8 @@
int ret;
struct cdrom_info *info = drive->driver_data;
+ BUG_ON(!spin_is_locked(ch->lock));
+
if (rq->flags & REQ_CMD) {
if (CDROM_CONFIG_FLAGS(drive)->seeking) {
unsigned long elpased = jiffies - info->start_seek;
@@ -1675,8 +1648,6 @@
}
CDROM_CONFIG_FLAGS(drive)->seeking = 0;
}
- /* FIXME: make this unlocking go away*/
- spin_unlock_irq(ch->lock);
if (IDE_LARGE_SEEK(info->last_block, block, IDECD_SEEK_THRESHOLD) && drive->dsc_overlap) {
ret = cdrom_start_seek(drive, rq, block);
} else {
@@ -1686,14 +1657,9 @@
ret = cdrom_start_write(drive, rq);
}
info->last_block = block;
- spin_lock_irq(ch->lock);
return ret;
} else if (rq->flags & (REQ_PC | REQ_SENSE)) {
- /* FIXME: make this unlocking go away*/
- spin_unlock_irq(ch->lock);
ret = cdrom_do_packet_command(drive, rq);
- spin_lock_irq(ch->lock);
-
return ret;
} else if (rq->flags & REQ_SPECIAL) {
/*
@@ -1703,10 +1669,7 @@
* right now this can only be a reset...
*/
- /* FIXME: make this unlocking go away*/
- spin_unlock_irq(ch->lock);
cdrom_end_request(drive, rq, 1);
- spin_lock_irq(ch->lock);
return ide_stopped;
} else if (rq->flags & REQ_BLOCK_PC) {
@@ -1721,9 +1684,7 @@
rq->special = (char *) &pc;
/* FIXME: make this unlocking go away*/
- spin_unlock_irq(ch->lock);
startstop = cdrom_do_packet_command(drive, rq);
- spin_lock_irq(ch->lock);
if (pc.stat)
++rq->errors;
@@ -1732,10 +1693,7 @@
}
blk_dump_rq_flags(rq, "ide-cd bad flags");
- /* FIXME: make this unlocking go away*/
- spin_unlock_irq(ch->lock);
cdrom_end_request(drive, rq, 0);
- spin_lock_irq(ch->lock);
return ide_stopped;
}
Index: linux-2.5.24/drivers/ide/ide-disk.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.24/drivers/ide/ide-disk.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 ide-disk.c
--- linux-2.5.24/drivers/ide/ide-disk.c 23 Jun 2002 13:32:22 -0000 1.1.1.1
+++ linux-2.5.24/drivers/ide/ide-disk.c 23 Jun 2002 22:59:11 -0000
@@ -90,23 +90,23 @@
return 0; /* lba_capacity value may be bad */
}
+/* TODO: remove the locking from the task_xxx_intr functions since they are primitives
+ * and should be called from callers who already have the channel->lock acquired -Zwane
+ */
+
/*
* Handler for command with PIO data-in phase.
*/
static ide_startstop_t task_in_intr(struct ata_device *drive, struct request *rq)
{
- unsigned long flags;
struct ata_channel *ch = drive->channel;
int ret;
- spin_lock_irqsave(ch->lock, flags);
+ BUG_ON(!spin_is_locked(ch->lock));
if (!ata_status(drive, DATA_READY, BAD_R_STAT)) {
- if (drive->status & (ERR_STAT | DRQ_STAT)) {
- spin_unlock_irqrestore(ch->lock, flags);
-
+ if (drive->status & (ERR_STAT | DRQ_STAT))
return ata_error(drive, rq, __FUNCTION__);
- }
/* no data yet, so wait for another interrupt */
ata_set_handler(drive, task_in_intr, WAIT_CMD, NULL);
@@ -134,8 +134,6 @@
if (rq->current_nr_sectors <= 0) {
if (!__ata_end_request(drive, rq, 1, 0)) {
// printk("Request Ended stat: %02x\n", drive->status);
- spin_unlock_irqrestore(ch->lock, flags);
-
return ide_stopped;
}
}
@@ -145,7 +143,6 @@
ret = ide_started;
}
- spin_unlock_irqrestore(ch->lock, flags);
return ret;
}
@@ -155,16 +152,13 @@
*/
static ide_startstop_t task_out_intr(struct ata_device *drive, struct request *rq)
{
- unsigned long flags;
struct ata_channel *ch = drive->channel;
int ret;
- spin_lock_irqsave(ch->lock, flags);
- if (!ata_status(drive, DRIVE_READY, drive->bad_wstat)) {
- spin_unlock_irqrestore(ch->lock, flags);
+ BUG_ON(!spin_is_locked(ch->lock));
+ if (!ata_status(drive, DRIVE_READY, drive->bad_wstat))
return ata_error(drive, rq, __FUNCTION__);
- }
if (!rq->current_nr_sectors && !__ata_end_request(drive, rq, 1, 0)) {
ret = ide_stopped;
@@ -185,7 +179,6 @@
ret = ide_started;
}
- spin_unlock_irqrestore(ch->lock, flags);
return ret;
}
@@ -195,17 +188,14 @@
*/
static ide_startstop_t task_mulin_intr(struct ata_device *drive, struct request *rq)
{
- unsigned long flags;
struct ata_channel *ch = drive->channel;
int ret;
- spin_lock_irqsave(ch->lock, flags);
- if (!ata_status(drive, DATA_READY, BAD_R_STAT)) {
- if (drive->status & (ERR_STAT | DRQ_STAT)) {
- spin_unlock_irqrestore(ch->lock, flags);
+ BUG_ON(!spin_is_locked(ch->lock));
+ if (!ata_status(drive, DATA_READY, BAD_R_STAT)) {
+ if (drive->status & (ERR_STAT | DRQ_STAT))
return ata_error(drive, rq, __FUNCTION__);
- }
/* no data yet, so wait for another interrupt */
ata_set_handler(drive, task_mulin_intr, WAIT_CMD, NULL);
@@ -242,11 +232,8 @@
/* FIXME: this seems buggy */
if (rq->current_nr_sectors <= 0) {
- if (!__ata_end_request(drive, rq, 1, 0)) {
- spin_unlock_irqrestore(ch->lock, flags);
-
+ if (!__ata_end_request(drive, rq, 1, 0))
return ide_stopped;
- }
}
msect -= nsect;
} while (msect);
@@ -256,19 +243,17 @@
ret = ide_started;
}
- spin_unlock_irqrestore(ch->lock, flags);
return ret;
}
static ide_startstop_t task_mulout_intr(struct ata_device *drive, struct request *rq)
{
- unsigned long flags;
struct ata_channel *ch = drive->channel;
int ok;
int ret;
- spin_lock_irqsave(ch->lock, flags);
+ BUG_ON(!spin_is_locked(ch->lock));
/*
* FIXME: the drive->status checks here seem to be messy.
@@ -280,11 +265,8 @@
ok = ata_status(drive, DATA_READY, BAD_R_STAT);
if (!ok || !rq->nr_sectors) {
- if (drive->status & (ERR_STAT | DRQ_STAT)) {
- spin_unlock_irqrestore(ch->lock, flags);
-
+ if (drive->status & (ERR_STAT | DRQ_STAT))
return ata_error(drive, rq, __FUNCTION__);
- }
}
if (!rq->nr_sectors) {
__ata_end_request(drive, rq, 1, rq->hard_nr_sectors);
@@ -341,7 +323,6 @@
ret = ide_started;
}
- spin_unlock_irqrestore(ch->lock, flags);
return ide_started;
}
@@ -436,6 +417,7 @@
for (i = 0; i < 100; ++i) {
if (drive_is_ready(drive))
break;
+ cpu_relax();
}
if (!drive_is_ready(drive)) {
printk(KERN_ERR "DISASTER WAITING TO HAPPEN!\n");
@@ -495,6 +477,9 @@
/* This issues a special drive command.
*/
+
+ BUG_ON(!spin_is_locked(drive->channel->lock));
+
if (rq->flags & REQ_SPECIAL)
return __do_request(drive, rq->special, rq);
@@ -622,8 +607,7 @@
} else if (drive->using_dma) {
args.cmd = WIN_READDMA;
} else if (drive->mult_count) {
- /* FIXME : Shouldn't this be task_mulin_intr?! */
- args.XXX_handler = task_in_intr;
+ args.XXX_handler = task_mulin_intr;
args.cmd = WIN_MULTREAD;
} else {
args.XXX_handler = task_in_intr;
@@ -675,7 +659,10 @@
static int idedisk_open(struct inode *inode, struct file *__fp, struct ata_device *drive)
{
+ unsigned long flags;
MOD_INC_USE_COUNT;
+
+ spin_lock_irqsave(drive->channel->lock, flags);
if (drive->removable && drive->usage == 1) {
check_disk_change(inode->i_rdev);
@@ -694,13 +681,14 @@
}
}
+ spin_unlock_irqrestore(drive->channel->lock, flags);
return 0;
}
static int flush_cache(struct ata_device *drive)
{
struct ata_taskfile args;
-
+
memset(&args, 0, sizeof(args));
if (drive->id->cfs_enable_2 & 0x2400)
@@ -713,6 +701,9 @@
static void idedisk_release(struct inode *inode, struct file *filp, struct ata_device *drive)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(drive->channel->lock, flags);
if (drive->removable && !drive->usage) {
/* XXX I don't think this is up to the lowlevel drivers.. --hch */
invalidate_bdev(inode->i_bdev, 0);
@@ -731,6 +722,7 @@
printk (KERN_INFO "%s: Write Cache FAILED Flushing!\n",
drive->name);
MOD_DEC_USE_COUNT;
+ spin_unlock_irqrestore(drive->channel->lock, flags);
}
static int idedisk_check_media_change(struct ata_device *drive)
@@ -751,17 +743,19 @@
static int set_multcount(struct ata_device *drive, int arg)
{
struct ata_taskfile args;
+ unsigned long flags;
+ int ret = -EIO;
/* Setting multi mode count on this channel type is not supported/not
* handled.
*/
if (IS_PDC4030_DRIVE)
- return -EIO;
+ return ret;
/* Hugh, we still didn't detect the devices capabilities.
*/
if (!drive->id)
- return -EIO;
+ return ret;
if (arg > drive->id->max_multsect)
arg = drive->id->max_multsect;
@@ -769,15 +763,19 @@
memset(&args, 0, sizeof(args));
args.taskfile.sector_count = arg;
args.cmd = WIN_SETMULT;
+
+ spin_lock_irqsave(drive->channel->lock, flags);
if (!ide_raw_taskfile(drive, &args)) {
/* all went well track this setting as valid */
drive->mult_count = arg;
-
- return 0;
+ ret = 0;
+ goto done;
} else
drive->mult_count = 0; /* reset */
- return -EIO;
+done:
+ spin_unlock_irqrestore(drive->channel->lock, flags);
+ return ret;
}
static int set_nowerr(struct ata_device *drive, int arg)
@@ -808,16 +806,24 @@
static int idedisk_standby(struct ata_device *drive)
{
struct ata_taskfile args;
+ unsigned long flags;
+ int ret;
memset(&args, 0, sizeof(args));
args.cmd = WIN_STANDBYNOW1;
- return ide_raw_taskfile(drive, &args);
+ spin_lock_irqsave(drive->channel->lock, flags);
+ ret = ide_raw_taskfile(drive, &args);
+ spin_unlock_irqrestore(drive->channel->lock, flags);
+
+ return ret;
}
static int set_acoustic(struct ata_device *drive, int arg)
{
struct ata_taskfile args;
+ unsigned long flags;
+ spin_lock_irqsave(drive->channel->lock, flags);
memset(&args, 0, sizeof(args));
args.taskfile.feature = (arg)?SETFEATURES_EN_AAM:SETFEATURES_DIS_AAM;
args.taskfile.sector_count = arg;
@@ -825,6 +831,7 @@
ide_raw_taskfile(drive, &args);
drive->acoustic = arg;
+ spin_unlock_irqrestore(drive->channel->lock, flags);
return 0;
}
@@ -933,6 +940,7 @@
{
struct ata_taskfile args;
unsigned long addr = 0;
+ unsigned long flags;
if (!(drive->id->command_set_1 & 0x0400) &&
!(drive->id->cfs_enable_2 & 0x0100))
@@ -942,7 +950,10 @@
memset(&args, 0, sizeof(args));
args.taskfile.device_head = 0x40;
args.cmd = WIN_READ_NATIVE_MAX;
+
+ spin_lock_irqsave(drive->channel->lock, flags);
ide_raw_taskfile(drive, &args);
+ spin_unlock_irqrestore(drive->channel->lock, flags);
/* if OK, compute maximum address value */
if (!(drive->status & ERR_STAT)) {
@@ -959,6 +970,7 @@
static u64 native_max_address_ext(struct ata_device *drive)
{
+ unsigned long flags;
struct ata_taskfile args;
u64 addr = 0;
@@ -967,7 +979,10 @@
args.taskfile.device_head = 0x40;
args.cmd = WIN_READ_NATIVE_MAX_EXT;
- ide_raw_taskfile(drive, &args);
+
+ spin_lock_irqsave(drive->channel->lock, flags);
+ ide_raw_taskfile(drive, &args);
+ spin_unlock_irqrestore(drive->channel->lock, flags);
/* if OK, compute maximum address value */
if (!(drive->status & ERR_STAT)) {
@@ -1005,7 +1020,9 @@
args.taskfile.device_head = ((addr_req >> 24) & 0x0f) | 0x40;
args.cmd = WIN_SET_MAX;
+ spin_lock_irqsave(drive->channel->lock, flags);
ide_raw_taskfile(drive, &args);
+ spin_unlock_irqrestore(drive->channel->lock, flags);
/* if OK, read new maximum address value */
if (!(drive->status & ERR_STAT)) {
@@ -1021,6 +1038,7 @@
static u64 set_max_address_ext(struct ata_device *drive, u64 addr_req)
{
struct ata_taskfile args;
+ unsigned long flags;
u64 addr_set = 0;
addr_req--;
@@ -1038,7 +1056,9 @@
args.hobfile.high_cylinder = (addr_req >>= 8);
args.hobfile.device_head = 0x40;
+ spin_lock_irqsave(drive->channel->lock, flags);
ide_raw_taskfile(drive, &args);
+ spin_unlock_irqrestore(drive->channel->lock, flags);
/* if OK, compute maximum address value */
if (!(drive->status & ERR_STAT)) {
@@ -1059,6 +1079,7 @@
{
int i;
+ unsigned long flags;
struct hd_driveid *id = drive->id;
sector_t capacity;
sector_t set_max;
@@ -1135,6 +1156,7 @@
* in above order (i.e., if value of higher priority is available,
* reset will be ignored).
*/
+
capacity = drive->cyl * drive->head * drive->sect;
set_max = native_max_address(drive);
@@ -1280,8 +1302,11 @@
drive->channel->no_io_32bit = id->dword_io ? 1 : 0;
- if (drive->id->cfs_enable_2 & 0x3000)
+ if (drive->id->cfs_enable_2 & 0x3000) {
+ spin_lock_irqsave(drive->channel->lock, flags);
write_cache(drive, (id->cfs_enable_2 & 0x3000));
+ spin_unlock_irqrestore(drive->channel->lock, flags);
+ }
probe_lba_addressing(drive, 1);
}
@@ -1289,15 +1314,19 @@
static int idedisk_cleanup(struct ata_device *drive)
{
int ret;
+ unsigned long flags;
if (!drive)
return 0;
if ((drive->id->cfs_enable_2 & 0x3000) && drive->wcache) {
+ spin_lock_irqsave(drive->channel->lock, flags);
if (flush_cache(drive))
printk (KERN_INFO "%s: Write Cache FAILED Flushing!\n",
drive->name);
+ spin_unlock_irqrestore(drive->channel->lock, flags);
}
+
ret = ide_unregister_subdriver(drive);
/* FIXME: This is killing the kernel with BUG 185 at asm/spinlocks.h
@@ -1307,7 +1336,6 @@
#if 0
put_device(&drive->device);
#endif
-
return ret;
}
@@ -1334,7 +1362,7 @@
return -EBUSY;
val = set_lba_addressing(drive, arg);
- spin_unlock_irq(drive->channel->lock);
+ spin_unlock_irq(drive->channel->lock); /* XXX */
return val;
}
@@ -1440,7 +1468,7 @@
return -EBUSY;
val = set_using_tcq(drive, arg);
- spin_unlock_irq(drive->channel->lock);
+ spin_unlock_irq(drive->channel->lock); /* XXX */
return val;
}
Index: linux-2.5.24/drivers/ide/ide-floppy.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.24/drivers/ide/ide-floppy.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 ide-floppy.c
--- linux-2.5.24/drivers/ide/ide-floppy.c 23 Jun 2002 13:32:22 -0000 1.1.1.1
+++ linux-2.5.24/drivers/ide/ide-floppy.c 23 Jun 2002 20:57:50 -0000
@@ -821,7 +821,6 @@
*/
static ide_startstop_t idefloppy_pc_intr(struct ata_device *drive, struct request *rq)
{
- unsigned long flags;
struct ata_channel *ch = drive->channel;
idefloppy_floppy_t *floppy = drive->driver_data;
idefloppy_status_reg_t status;
@@ -830,6 +829,8 @@
struct atapi_packet_command *pc = floppy->pc;
unsigned int temp;
+ BUG_ON(!spin_is_locked(ch->lock));
+
#if IDEFLOPPY_DEBUG_LOG
printk (KERN_INFO "ide-floppy: Reached idefloppy_pc_intr interrupt handler\n");
#endif
@@ -885,24 +886,16 @@
return ide_stopped;
}
#endif
- /* FIXME: this locking should encompass the above register
- * file access too.
- */
-
- spin_lock_irqsave(ch->lock, flags);
+
bcount.b.high=IN_BYTE (IDE_BCOUNTH_REG); /* Get the number of bytes to transfer */
bcount.b.low=IN_BYTE (IDE_BCOUNTL_REG); /* on this interrupt */
ireason.all=IN_BYTE (IDE_IREASON_REG);
if (ireason.b.cod) {
- spin_unlock_irqrestore(ch->lock, flags);
-
printk (KERN_ERR "ide-floppy: CoD != 0 in idefloppy_pc_intr\n");
return ide_stopped;
}
if (ireason.b.io == test_bit(PC_WRITING, &pc->flags)) { /* Hopefully, we will never get here */
- spin_unlock_irqrestore(ch->lock, flags);
-
printk (KERN_ERR "ide-floppy: We wanted to %s, ", ireason.b.io ? "Write":"Read");
printk (KERN_ERR "but the floppy wants us to %s !\n",ireason.b.io ? "Read":"Write");
return ide_stopped;
@@ -915,7 +908,6 @@
atapi_discard_data (drive,bcount.all);
ata_set_handler(drive, idefloppy_pc_intr,IDEFLOPPY_WAIT_CMD, NULL);
- spin_unlock_irqrestore(ch->lock, flags);
return ide_started;
}
@@ -939,7 +931,6 @@
pc->current_position+=bcount.all;
ata_set_handler(drive, idefloppy_pc_intr, IDEFLOPPY_WAIT_CMD, NULL); /* And set the interrupt handler again */
- spin_unlock_irqrestore(ch->lock, flags);
return ide_started;
}
@@ -951,16 +942,12 @@
*/
static ide_startstop_t idefloppy_transfer_pc(struct ata_device *drive, struct request *rq)
{
- unsigned long flags;
struct ata_channel *ch = drive->channel;
ide_startstop_t startstop;
idefloppy_floppy_t *floppy = drive->driver_data;
idefloppy_ireason_reg_t ireason;
int ret;
- /* FIXME: Move this lock upwards.
- */
- spin_lock_irqsave(ch->lock, flags);
if (ata_status_poll(drive, DRQ_STAT, BUSY_STAT,
WAIT_READY, rq, &startstop)) {
printk (KERN_ERR "ide-floppy: Strange, packet command initiated yet DRQ isn't asserted\n");
@@ -978,7 +965,6 @@
ret = ide_started;
}
}
- spin_unlock_irqrestore(ch->lock, flags);
return ret;
}
@@ -1006,13 +992,14 @@
static ide_startstop_t idefloppy_transfer_pc1(struct ata_device *drive, struct request *rq)
{
- unsigned long flags;
struct ata_channel *ch = drive->channel;
idefloppy_floppy_t *floppy = drive->driver_data;
ide_startstop_t startstop;
idefloppy_ireason_reg_t ireason;
int ret;
+ BUG_ON(!spin_is_locked(ch->lock));
+
if (ata_status_poll(drive, DRQ_STAT, BUSY_STAT,
WAIT_READY, rq, &startstop)) {
printk (KERN_ERR "ide-floppy: Strange, packet command initiated yet DRQ isn't asserted\n");
@@ -1020,11 +1007,6 @@
return startstop;
}
- /* FIXME: this locking should encompass the above register
- * file access too.
- */
-
- spin_lock_irqsave(ch->lock, flags);
ireason.all=IN_BYTE(IDE_IREASON_REG);
if (!ireason.b.cod || ireason.b.io) {
printk (KERN_ERR "ide-floppy: (IO,CoD) != (0,1) while issuing a packet command\n");
@@ -1045,7 +1027,6 @@
idefloppy_transfer_pc2); /* fail == transfer_pc2 */
ret = ide_started;
}
- spin_unlock_irqrestore(ch->lock, flags);
return ret;
}
@@ -1061,6 +1042,8 @@
int dma_ok = 0;
ata_handler_t *pkt_xfer_routine;
+ BUG_ON(!spin_is_locked(drive->channel->lock));
+
#if IDEFLOPPY_DEBUG_BUGS
if (floppy->pc->c[0] == IDEFLOPPY_REQUEST_SENSE_CMD && pc->c[0] == IDEFLOPPY_REQUEST_SENSE_CMD) {
printk(KERN_ERR "ide-floppy: possible ide-floppy.c bug - Two request sense in serial were issued\n");
@@ -1129,14 +1112,8 @@
unsigned long flags;
struct ata_channel *ch = drive->channel;
- /* FIXME: this locking should encompass the above register
- * file access too.
- */
-
- spin_lock_irqsave(ch->lock, flags);
ata_set_handler(drive, pkt_xfer_routine, IDEFLOPPY_WAIT_CMD, NULL);
OUT_BYTE (WIN_PACKETCMD, IDE_COMMAND_REG); /* Issue the packet command */
- spin_unlock_irqrestore(ch->lock, flags);
return ide_started;
} else {
@@ -1294,20 +1271,14 @@
else
printk (KERN_ERR "ide-floppy: %s: I/O error\n", drive->name);
- /* FIXME: make this unlocking go away*/
- spin_unlock_irq(ch->lock);
idefloppy_end_request(drive, rq, 0);
- spin_lock_irq(ch->lock);
return ide_stopped;
}
if (rq->flags & REQ_CMD) {
if (rq->sector % floppy->bs_factor || rq->nr_sectors % floppy->bs_factor) {
printk ("%s: unsupported r/w request size\n", drive->name);
- /* FIXME: make this unlocking go away*/
- spin_unlock_irq(ch->lock);
idefloppy_end_request(drive, rq, 0);
- spin_lock_irq(ch->lock);
return ide_stopped;
}
@@ -1318,18 +1289,12 @@
pc = (struct atapi_packet_command *) rq->buffer;
} else {
blk_dump_rq_flags(rq, "ide-floppy: unsupported command in queue");
- /* FIXME: make this unlocking go away*/
- spin_unlock_irq(ch->lock);
idefloppy_end_request(drive, rq, 0);
- spin_lock_irq(ch->lock);
return ide_stopped;
}
- /* FIXME: make this unlocking go away*/
- spin_unlock_irq(ch->lock);
ret = idefloppy_issue_pc(drive, rq, pc);
- spin_lock_irq(ch->lock);
return ret;
}
Index: linux-2.5.24/drivers/ide/ide-taskfile.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.24/drivers/ide/ide-taskfile.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 ide-taskfile.c
--- linux-2.5.24/drivers/ide/ide-taskfile.c 23 Jun 2002 13:32:22 -0000 1.1.1.1
+++ linux-2.5.24/drivers/ide/ide-taskfile.c 23 Jun 2002 20:59:36 -0000
@@ -193,6 +193,7 @@
* and the function returns immediately without waiting for the new rq to be
* completed. This is again intended for careful use by the ATAPI tape/cdrom
* driver code.
+ *
*/
int ide_do_drive_cmd(struct ata_device *drive, struct request *rq, ide_action_t action)
{
@@ -202,6 +203,8 @@
struct list_head *queue_head = &q->queue_head;
DECLARE_COMPLETION(wait);
+ BUG_ON(!spin_is_locked(drive->channel->lock));
+
#ifdef CONFIG_BLK_DEV_PDC4030
if (drive->channel->chipset == ide_pdc4030 && rq->buffer != NULL)
return -ENOSYS; /* special drive cmds not supported */
@@ -212,8 +215,6 @@
if (action == ide_wait)
rq->waiting = &wait;
- spin_lock_irqsave(drive->channel->lock, flags);
-
if (blk_queue_empty(&drive->queue) || action == ide_preempt) {
if (action == ide_preempt)
drive->rq = NULL;
@@ -227,8 +228,7 @@
do_ide_request(q);
- spin_unlock_irqrestore(drive->channel->lock, flags);
-
+ /* FIXME: This needs to move from here to simplify things -Zwane */
if (action == ide_wait) {
wait_for_completion(&wait); /* wait for it to be serviced */
return rq->errors ? -EIO : 0; /* return -EIO if errors */
@@ -238,6 +238,9 @@
}
+/* TODO: These interrupt primitives should be called with the channel lock held and
+ * irqs disabled -zwane
+ */
/*
* Invoked on completion of a special REQ_SPECIAL command.
@@ -247,11 +250,8 @@
struct ata_taskfile *ar = rq->special;
ide_startstop_t ret = ide_stopped;
- unsigned long flags;
-
- ide__sti(); /* local CPU only */
- spin_lock_irqsave(drive->channel->lock, flags);
+ BUG_ON(!spin_is_locked(drive->channel->lock));
if (rq->buffer && ar->taskfile.sector_number) {
if (!ata_status(drive, 0, DRQ_STAT) && ar->taskfile.sector_number) {
@@ -286,8 +286,6 @@
blkdev_dequeue_request(rq);
drive->rq = NULL;
end_that_request_last(rq);
-
- spin_unlock_irqrestore(drive->channel->lock, flags);
return ret;
}
Index: linux-2.5.24/drivers/ide/pcidma.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.24/drivers/ide/pcidma.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 pcidma.c
--- linux-2.5.24/drivers/ide/pcidma.c 23 Jun 2002 13:32:22 -0000 1.1.1.1
+++ linux-2.5.24/drivers/ide/pcidma.c 23 Jun 2002 22:34:58 -0000
@@ -44,17 +44,7 @@
if (ata_status(drive, DRIVE_READY, drive->bad_wstat | DRQ_STAT)) {
if (!dma_stat) {
- unsigned long flags;
- struct ata_channel *ch = drive->channel;
-
- /* FIXME: this locking should encompass the above register
- * file access too.
- */
-
- spin_lock_irqsave(ch->lock, flags);
__ata_end_request(drive, rq, 1, rq->nr_sectors);
- spin_unlock_irqrestore(ch->lock, flags);
-
return ide_stopped;
}
printk(KERN_ERR "%s: dma_intr: bad DMA status (dma_stat=%x)\n",
Index: linux-2.5.24/drivers/ide/tcq.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.24/drivers/ide/tcq.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 tcq.c
--- linux-2.5.24/drivers/ide/tcq.c 23 Jun 2002 13:32:22 -0000 1.1.1.1
+++ linux-2.5.24/drivers/ide/tcq.c 23 Jun 2002 19:37:11 -0000
@@ -229,7 +229,6 @@
*
* Also, nIEN must be set as not to need protection against ide_dmaq_intr
*
- * Channel lock should be held.
*/
static ide_startstop_t service(struct ata_device *drive, struct request *rq)
{
@@ -287,6 +286,7 @@
* should not happen, a buggy device could introduce loop
*/
if ((feat = GET_FEAT()) & NSEC_REL) {
+ /* XXX is this safe w/o the lock? -Zwane */
drive->rq = NULL;
printk("%s: release in service\n", drive->name);
return ide_stopped;
-- http://function.linuxpower.ca- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/