From ffc36c7610731115c77700dcc53901920361c235 Mon Sep 17 00:00:00 2001 From: Bartlomiej Zolnierkiewicz Date: Tue, 23 Jun 2009 03:43:00 -0700 Subject: ide: fix handling of unexpected IRQs vs request_irq() Add ide_host_enable_irqs() helper and use it in ide_host_register() before registering ports. Then remove no longer needed IRQ unmasking from in init_irq(). This should fix the problem with "screaming" shared IRQ on the first port (after request_irq() call while we have the unexpected IRQ pending on the second port) which was uncovered by my rework of the serialized interfaces support. Reported-by: Frans Pop Tested-by: Frans Pop Signed-off-by: Bartlomiej Zolnierkiewicz Signed-off-by: David S. Miller --- drivers/ide/ide-probe.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) (limited to 'drivers/ide') diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c index 51af4eea0d3..1bb106f6221 100644 --- a/drivers/ide/ide-probe.c +++ b/drivers/ide/ide-probe.c @@ -818,6 +818,24 @@ static int ide_port_setup_devices(ide_hwif_t *hwif) return j; } +static void ide_host_enable_irqs(struct ide_host *host) +{ + ide_hwif_t *hwif; + int i; + + ide_host_for_each_port(i, hwif, host) { + if (hwif == NULL) + continue; + + /* clear any pending IRQs */ + hwif->tp_ops->read_status(hwif); + + /* unmask IRQs */ + if (hwif->io_ports.ctl_addr) + hwif->tp_ops->write_devctl(hwif, ATA_DEVCTL_OBS); + } +} + /* * This routine sets up the IRQ for an IDE interface. */ @@ -831,9 +849,6 @@ static int init_irq (ide_hwif_t *hwif) if (irq_handler == NULL) irq_handler = ide_intr; - if (io_ports->ctl_addr) - hwif->tp_ops->write_devctl(hwif, ATA_DEVCTL_OBS); - if (request_irq(hwif->irq, irq_handler, sa, hwif->name, hwif)) goto out_up; @@ -1404,6 +1419,8 @@ int ide_host_register(struct ide_host *host, const struct ide_port_info *d, ide_port_tune_devices(hwif); } + ide_host_enable_irqs(host); + ide_host_for_each_port(i, hwif, host) { if (hwif == NULL) continue; -- cgit v1.2.3-70-g09d2 From af054ed0018f0a69f8ea6f7546cbf34385edf13b Mon Sep 17 00:00:00 2001 From: "David S. Miller" Date: Tue, 23 Jun 2009 16:01:06 -0700 Subject: ide-cd: Don't warn on bogus block size unless it actually matters. Frans Pop reported that his CDROM drive reports a blocksize of 2352, and this causes new warnings due to commit e8e7b9eb11c34ee18bde8b7011af41938d1ad667 ("ide-cd: fix oops when using growisofs"). What we're trying to do is make sure that "blocklen >> SECTOR_BITS" is something the block layer won't choke on. And for Frans' case "2352 >> SECTOR_BITS" is equal to "2048 >> SECTOR_BITS", and thats "4". So warning in this case gives no real benefit. Reported-by: Frans Pop Tested-by: Frans Pop Signed-off-by: David S. Miller --- drivers/ide/ide-cd.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/ide') diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c index 4a19686fcfe..a9a1bfb14e7 100644 --- a/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -876,9 +876,12 @@ static int cdrom_read_capacity(ide_drive_t *drive, unsigned long *capacity, return stat; /* - * Sanity check the given block size + * Sanity check the given block size, in so far as making + * sure the sectors_per_frame we give to the caller won't + * end up being bogus. */ blocklen = be32_to_cpu(capbuf.blocklen); + blocklen = (blocklen >> SECTOR_BITS) << SECTOR_BITS; switch (blocklen) { case 512: case 1024: -- cgit v1.2.3-70-g09d2 From d9ae62433e46909fc9e7d97ce74202c2851667b8 Mon Sep 17 00:00:00 2001 From: Frans Pop Date: Tue, 23 Jun 2009 16:02:58 -0700 Subject: ide-cd: Improve "weird block size" error message Currently the error gets repeated too frequently, for example each time HAL polls the device when a disc is present. Avoid that by using printk_once instead of printk. Also join the error and corrective action messages into a single line. Signed-off-by: Frans Pop Acked-by: Borislav Petkov Signed-off-by: David S. Miller --- drivers/ide/ide-cd.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/ide') diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c index a9a1bfb14e7..f0ede5953af 100644 --- a/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -889,10 +889,9 @@ static int cdrom_read_capacity(ide_drive_t *drive, unsigned long *capacity, case 4096: break; default: - printk(KERN_ERR PFX "%s: weird block size %u\n", + printk_once(KERN_ERR PFX "%s: weird block size %u; " + "setting default block size to 2048\n", drive->name, blocklen); - printk(KERN_ERR PFX "%s: default to 2kb block size\n", - drive->name); blocklen = 2048; break; } -- cgit v1.2.3-70-g09d2 From 346c17a6cf60375323adfaa4b8a9d841049f890e Mon Sep 17 00:00:00 2001 From: Bartlomiej Zolnierkiewicz Date: Mon, 22 Jun 2009 07:38:26 +0000 Subject: ide: relax DMA info validity checking There are some broken devices that report multiple DMA xfer modes enabled at once (ATA spec doesn't allow it) but otherwise work fine with DMA so just delete ide_id_dma_bug(). [ As discovered by detective work by Frans and Bart, due to how handling of the ID block was handled before commit c419993 ("ide-iops: only clear DMA words on setting DMA mode") this check was always seeing zeros in the fields or other similar garbage. Therefore this check wasn't actually checking anything. Now that the tests actually check the real bits, all we see are devices that trigger the check yet work perfectly fine, therefore killing this useless check is the best thing to do. -DaveM ] Reported-by: Frans Pop Signed-off-by: Bartlomiej Zolnierkiewicz Signed-off-by: David S. Miller --- drivers/ide/ide-dma.c | 21 --------------------- drivers/ide/ide-iops.c | 3 --- include/linux/ide.h | 2 -- 3 files changed, 26 deletions(-) (limited to 'drivers/ide') diff --git a/drivers/ide/ide-dma.c b/drivers/ide/ide-dma.c index 219e6fb78dc..ee58c88dee5 100644 --- a/drivers/ide/ide-dma.c +++ b/drivers/ide/ide-dma.c @@ -361,9 +361,6 @@ static int ide_tune_dma(ide_drive_t *drive) if (__ide_dma_bad_drive(drive)) return 0; - if (ide_id_dma_bug(drive)) - return 0; - if (hwif->host_flags & IDE_HFLAG_TRUST_BIOS_FOR_DMA) return config_drive_for_dma(drive); @@ -394,24 +391,6 @@ static int ide_dma_check(ide_drive_t *drive) return -1; } -int ide_id_dma_bug(ide_drive_t *drive) -{ - u16 *id = drive->id; - - if (id[ATA_ID_FIELD_VALID] & 4) { - if ((id[ATA_ID_UDMA_MODES] >> 8) && - (id[ATA_ID_MWDMA_MODES] >> 8)) - goto err_out; - } else if ((id[ATA_ID_MWDMA_MODES] >> 8) && - (id[ATA_ID_SWDMA_MODES] >> 8)) - goto err_out; - - return 0; -err_out: - printk(KERN_ERR "%s: bad DMA info in identify block\n", drive->name); - return 1; -} - int ide_set_dma(ide_drive_t *drive) { int rc; diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c index fa047150a1c..917186ec496 100644 --- a/drivers/ide/ide-iops.c +++ b/drivers/ide/ide-iops.c @@ -329,9 +329,6 @@ int ide_driveid_update(ide_drive_t *drive) kfree(id); - if ((drive->dev_flags & IDE_DFLAG_USING_DMA) && ide_id_dma_bug(drive)) - ide_dma_off(drive); - return 1; out_err: if (rc == 2) diff --git a/include/linux/ide.h b/include/linux/ide.h index 95c6e00a72e..cf1f3888067 100644 --- a/include/linux/ide.h +++ b/include/linux/ide.h @@ -1361,7 +1361,6 @@ int ide_in_drive_list(u16 *, const struct drive_list_entry *); #ifdef CONFIG_BLK_DEV_IDEDMA int ide_dma_good_drive(ide_drive_t *); int __ide_dma_bad_drive(ide_drive_t *); -int ide_id_dma_bug(ide_drive_t *); u8 ide_find_dma_mode(ide_drive_t *, u8); @@ -1402,7 +1401,6 @@ void ide_dma_lost_irq(ide_drive_t *); ide_startstop_t ide_dma_timeout_retry(ide_drive_t *, int); #else -static inline int ide_id_dma_bug(ide_drive_t *drive) { return 0; } static inline u8 ide_find_dma_mode(ide_drive_t *drive, u8 speed) { return 0; } static inline u8 ide_max_dma_mode(ide_drive_t *drive) { return 0; } static inline void ide_dma_off_quietly(ide_drive_t *drive) { ; } -- cgit v1.2.3-70-g09d2 From ba9413bd284e79ea43b0ae406a7a29526aaf82b3 Mon Sep 17 00:00:00 2001 From: Bartlomiej Zolnierkiewicz Date: Tue, 23 Jun 2009 16:11:10 -0700 Subject: ide: add QUANTUM FIREBALLct20 30 with firmware APL.090 to ivb_list[] Signed-off-by: Bartlomiej Zolnierkiewicz Signed-off-by: David S. Miller --- drivers/ide/ide-iops.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/ide') diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c index 917186ec496..2892b242bbe 100644 --- a/drivers/ide/ide-iops.c +++ b/drivers/ide/ide-iops.c @@ -210,6 +210,7 @@ EXPORT_SYMBOL_GPL(ide_in_drive_list); */ static const struct drive_list_entry ivb_list[] = { { "QUANTUM FIREBALLlct10 05" , "A03.0900" }, + { "QUANTUM FIREBALLlct20 30" , "APL.0900" }, { "TSSTcorp CDDVDW SH-S202J" , "SB00" }, { "TSSTcorp CDDVDW SH-S202J" , "SB01" }, { "TSSTcorp CDDVDW SH-S202N" , "SB00" }, -- cgit v1.2.3-70-g09d2 From a1317f714af7aed60ddc182d0122477cbe36ee9b Mon Sep 17 00:00:00 2001 From: Bartlomiej Zolnierkiewicz Date: Tue, 23 Jun 2009 23:52:17 -0700 Subject: ide: improve handling of Power Management requests Make hwif->rq point to PM request during PM sequence and do not allow any other types of requests to slip in (the old comment was never correct as there should be no such requests generated during PM sequence). Signed-off-by: Bartlomiej Zolnierkiewicz Signed-off-by: David S. Miller --- drivers/ide/ide-io.c | 54 +++++++++++++++++++++------------------------------- 1 file changed, 22 insertions(+), 32 deletions(-) (limited to 'drivers/ide') diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c index 1059f809b80..93b7886a2d6 100644 --- a/drivers/ide/ide-io.c +++ b/drivers/ide/ide-io.c @@ -476,10 +476,14 @@ void do_ide_request(struct request_queue *q) if (!ide_lock_port(hwif)) { ide_hwif_t *prev_port; - - WARN_ON_ONCE(hwif->rq); repeat: prev_port = hwif->host->cur_port; + + if (drive->dev_flags & IDE_DFLAG_BLOCKED) + rq = hwif->rq; + else + WARN_ON_ONCE(hwif->rq); + if (drive->dev_flags & IDE_DFLAG_SLEEPING && time_after(drive->sleep, jiffies)) { ide_unlock_port(hwif); @@ -506,43 +510,29 @@ repeat: hwif->cur_dev = drive; drive->dev_flags &= ~(IDE_DFLAG_SLEEPING | IDE_DFLAG_PARKED); - spin_unlock_irq(&hwif->lock); - spin_lock_irq(q->queue_lock); - /* - * we know that the queue isn't empty, but this can happen - * if the q->prep_rq_fn() decides to kill a request - */ - if (!rq) + if (rq == NULL) { + spin_unlock_irq(&hwif->lock); + spin_lock_irq(q->queue_lock); + /* + * we know that the queue isn't empty, but this can + * happen if ->prep_rq_fn() decides to kill a request + */ rq = blk_fetch_request(drive->queue); + spin_unlock_irq(q->queue_lock); + spin_lock_irq(&hwif->lock); - spin_unlock_irq(q->queue_lock); - spin_lock_irq(&hwif->lock); - - if (!rq) { - ide_unlock_port(hwif); - goto out; + if (rq == NULL) { + ide_unlock_port(hwif); + goto out; + } } /* * Sanity: don't accept a request that isn't a PM request - * if we are currently power managed. This is very important as - * blk_stop_queue() doesn't prevent the blk_fetch_request() - * above to return us whatever is in the queue. Since we call - * ide_do_request() ourselves, we end up taking requests while - * the queue is blocked... - * - * We let requests forced at head of queue with ide-preempt - * though. I hope that doesn't happen too much, hopefully not - * unless the subdriver triggers such a thing in its own PM - * state machine. + * if we are currently power managed. */ - if ((drive->dev_flags & IDE_DFLAG_BLOCKED) && - blk_pm_request(rq) == 0 && - (rq->cmd_flags & REQ_PREEMPT) == 0) { - /* there should be no pending command at this point */ - ide_unlock_port(hwif); - goto plug_device; - } + BUG_ON((drive->dev_flags & IDE_DFLAG_BLOCKED) && + blk_pm_request(rq) == 0); hwif->rq = rq; -- cgit v1.2.3-70-g09d2 From d7e2f36d9a92284754ed5254562766cb3d61c7ca Mon Sep 17 00:00:00 2001 From: "David S. Miller" Date: Wed, 24 Jun 2009 02:36:17 -0700 Subject: ide cs5520: Initialize second port's interrupt number. In 86ccf37c6acd74cf7e4b7751ee045de19943c5a0 the driver was modified to deal with the removal of the pciirq argument to ide_pci_setup_ports(). But in the conversion only the first port's IRQ gets setup. Inspired by a patch by Bartlomiej Zolnierkiewicz., and with help from Alan Cox. Signed-off-by: David S. Miller --- drivers/ide/cs5520.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/ide') diff --git a/drivers/ide/cs5520.c b/drivers/ide/cs5520.c index bd066bb9d61..09f98ed0731 100644 --- a/drivers/ide/cs5520.c +++ b/drivers/ide/cs5520.c @@ -135,6 +135,7 @@ static int __devinit cs5520_init_one(struct pci_dev *dev, const struct pci_devic ide_pci_setup_ports(dev, d, &hw[0], &hws[0]); hw[0].irq = 14; + hw[1].irq = 15; return ide_host_add(d, hws, 2, NULL); } -- cgit v1.2.3-70-g09d2 From 789547508f22e482825f52f813b59680408ec2c7 Mon Sep 17 00:00:00 2001 From: Bartlomiej Zolnierkiewicz Date: Tue, 23 Jun 2009 11:26:06 +0000 Subject: ide: fix ide_kill_rq() for special ide-{floppy,tape} driver requests Such requests should be failed with -EIO (like all other requests in this function) instead of being completed successfully. Signed-off-by: Bartlomiej Zolnierkiewicz Signed-off-by: David S. Miller --- drivers/ide/ide-io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/ide') diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c index 93b7886a2d6..95db5f03f6a 100644 --- a/drivers/ide/ide-io.c +++ b/drivers/ide/ide-io.c @@ -152,7 +152,7 @@ void ide_kill_rq(ide_drive_t *drive, struct request *rq) if ((media == ide_floppy || media == ide_tape) && drv_req) { rq->errors = 0; - ide_complete_rq(drive, 0, blk_rq_bytes(rq)); + ide_complete_rq(drive, -EIO, blk_rq_bytes(rq)); } else { if (media == ide_tape) rq->errors = IDE_DRV_ERROR_GENERAL; -- cgit v1.2.3-70-g09d2 From 5e955245d6cf49c5ed26c7add7392ff5a6762bf4 Mon Sep 17 00:00:00 2001 From: Bartlomiej Zolnierkiewicz Date: Tue, 23 Jun 2009 11:27:27 +0000 Subject: ide: always kill the whole request on error * Use blk_rq_bytes() instead of obsolete ide_rq_bytes() in ide_kill_rq() and ide_floppy_do_request() for failed requests. [ bugfix part ] * Use blk_rq_bytes() instead of obsolete ide_rq_bytes() in ide_do_devset() and ide_complete_drive_reset(). Then remove ide_rq_bytes(). [ cleanup part ] Signed-off-by: Bartlomiej Zolnierkiewicz Signed-off-by: David S. Miller --- drivers/ide/ide-devsets.c | 2 +- drivers/ide/ide-eh.c | 2 +- drivers/ide/ide-floppy.c | 2 +- drivers/ide/ide-io.c | 14 ++------------ include/linux/ide.h | 1 - 5 files changed, 5 insertions(+), 16 deletions(-) (limited to 'drivers/ide') diff --git a/drivers/ide/ide-devsets.c b/drivers/ide/ide-devsets.c index 5bf958e5b1d..1099bf7cf96 100644 --- a/drivers/ide/ide-devsets.c +++ b/drivers/ide/ide-devsets.c @@ -183,6 +183,6 @@ ide_startstop_t ide_do_devset(ide_drive_t *drive, struct request *rq) err = setfunc(drive, *(int *)&rq->cmd[1]); if (err) rq->errors = err; - ide_complete_rq(drive, err, ide_rq_bytes(rq)); + ide_complete_rq(drive, err, blk_rq_bytes(rq)); return ide_stopped; } diff --git a/drivers/ide/ide-eh.c b/drivers/ide/ide-eh.c index 2b914197961..e9abf2c3c33 100644 --- a/drivers/ide/ide-eh.c +++ b/drivers/ide/ide-eh.c @@ -149,7 +149,7 @@ static inline void ide_complete_drive_reset(ide_drive_t *drive, int err) if (rq && blk_special_request(rq) && rq->cmd[0] == REQ_DRIVE_RESET) { if (err <= 0 && rq->errors == 0) rq->errors = -EIO; - ide_complete_rq(drive, err ? err : 0, ide_rq_bytes(rq)); + ide_complete_rq(drive, err ? err : 0, blk_rq_bytes(rq)); } } diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c index 8b3f204f7d7..fefbdfc8db0 100644 --- a/drivers/ide/ide-floppy.c +++ b/drivers/ide/ide-floppy.c @@ -293,7 +293,7 @@ out_end: drive->failed_pc = NULL; if (blk_fs_request(rq) == 0 && rq->errors == 0) rq->errors = -EIO; - ide_complete_rq(drive, -EIO, ide_rq_bytes(rq)); + ide_complete_rq(drive, -EIO, blk_rq_bytes(rq)); return ide_stopped; } diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c index 95db5f03f6a..d5f3c77bead 100644 --- a/drivers/ide/ide-io.c +++ b/drivers/ide/ide-io.c @@ -112,16 +112,6 @@ void ide_complete_cmd(ide_drive_t *drive, struct ide_cmd *cmd, u8 stat, u8 err) } } -/* obsolete, blk_rq_bytes() should be used instead */ -unsigned int ide_rq_bytes(struct request *rq) -{ - if (blk_pc_request(rq)) - return blk_rq_bytes(rq); - else - return blk_rq_cur_sectors(rq) << 9; -} -EXPORT_SYMBOL_GPL(ide_rq_bytes); - int ide_complete_rq(ide_drive_t *drive, int error, unsigned int nr_bytes) { ide_hwif_t *hwif = drive->hwif; @@ -152,14 +142,14 @@ void ide_kill_rq(ide_drive_t *drive, struct request *rq) if ((media == ide_floppy || media == ide_tape) && drv_req) { rq->errors = 0; - ide_complete_rq(drive, -EIO, blk_rq_bytes(rq)); } else { if (media == ide_tape) rq->errors = IDE_DRV_ERROR_GENERAL; else if (blk_fs_request(rq) == 0 && rq->errors == 0) rq->errors = -EIO; - ide_complete_rq(drive, -EIO, ide_rq_bytes(rq)); } + + ide_complete_rq(drive, -EIO, blk_rq_bytes(rq)); } static void ide_tf_set_specify_cmd(ide_drive_t *drive, struct ide_taskfile *tf) diff --git a/include/linux/ide.h b/include/linux/ide.h index cf1f3888067..c6af7c44d46 100644 --- a/include/linux/ide.h +++ b/include/linux/ide.h @@ -1062,7 +1062,6 @@ int generic_ide_ioctl(ide_drive_t *, struct block_device *, unsigned, unsigned l extern int ide_vlb_clk; extern int ide_pci_clk; -unsigned int ide_rq_bytes(struct request *); int ide_end_rq(ide_drive_t *, struct request *, int, unsigned int); void ide_kill_rq(ide_drive_t *, struct request *); -- cgit v1.2.3-70-g09d2 From 9c72ebef5aabf3532469d602a9d87beceea268b1 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Fri, 26 Jun 2009 11:22:37 -0700 Subject: ide-cd: handle fragmented packet commands gracefully There are some devices in the wild that clear the DRQ bit during the last word of a packet command and therefore could use a "second chance" for that last word of data to be xferred instead of simply failing the request. Do that by attempting to suck in those last bytes in PIO mode. In addition, the ATA_ERR bit has to be cleared for we cannot be sure the data is valid otherwise. See http://bugzilla.kernel.org/show_bug.cgi?id=13399 for details. Signed-off-by: Borislav Petkov Acked-by: Bartlomiej Zolnierkiewicz Signed-off-by: David S. Miller --- drivers/ide/ide-cd.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'drivers/ide') diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c index f0ede5953af..6a9a769bffc 100644 --- a/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -592,9 +592,19 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive) } } else if (!blk_pc_request(rq)) { ide_cd_request_sense_fixup(drive, cmd); - /* complain if we still have data left to transfer */ + uptodate = cmd->nleft ? 0 : 1; - if (uptodate == 0) + + /* + * suck out the remaining bytes from the drive in an + * attempt to complete the data xfer. (see BZ#13399) + */ + if (!(stat & ATA_ERR) && !uptodate && thislen) { + ide_pio_bytes(drive, cmd, write, thislen); + uptodate = cmd->nleft ? 0 : 1; + } + + if (!uptodate) rq->cmd_flags |= REQ_FAILED; } goto out_end; -- cgit v1.2.3-70-g09d2 From 2bf427b25b79eb7cea27963a66c3d4684cae0e0c Mon Sep 17 00:00:00 2001 From: Bartlomiej Zolnierkiewicz Date: Mon, 29 Jun 2009 19:20:42 -0700 Subject: ide: fix resume for CONFIG_BLK_DEV_IDEACPI=y commit 2f0d0fd2a605666d38e290c5c0d2907484352dc4 ("ide-acpi: cleanup do_drive_get_GTF()") didn't account for the lack of hwif->acpidata check in generic_ide_suspend() [ indirect user of do_drive_get_GTF() through ide_acpi_exec_tfs() ] resulting in broken resume when ACPI support is enabled but ACPI data is unavailable. Fix it by adding ide_port_acpi() helper for checking if port needs ACPI handling and cleaning generic_ide_{suspend,resume}() to use it instead of hiding hwif->acpidata and ide_noacpi checks in IDE ACPI helpers (this should help in preventing similar bugs in the future). While at it: - kill superfluous debugging printks in ide_acpi_{get,push}_timing() Reported-and-tested-by: Etienne Basset Also-reported-and-tested-by: Jeff Chua Signed-off-by: Bartlomiej Zolnierkiewicz Signed-off-by: David S. Miller --- drivers/ide/ide-acpi.c | 37 +++++++------------------------------ drivers/ide/ide-pm.c | 30 ++++++++++++++++++------------ include/linux/ide.h | 2 ++ 3 files changed, 27 insertions(+), 42 deletions(-) (limited to 'drivers/ide') diff --git a/drivers/ide/ide-acpi.c b/drivers/ide/ide-acpi.c index 77f79d26b26..c509c991646 100644 --- a/drivers/ide/ide-acpi.c +++ b/drivers/ide/ide-acpi.c @@ -92,6 +92,11 @@ int ide_acpi_init(void) return 0; } +bool ide_port_acpi(ide_hwif_t *hwif) +{ + return ide_noacpi == 0 && hwif->acpidata; +} + /** * ide_get_dev_handle - finds acpi_handle and PCI device.function * @dev: device to locate @@ -352,9 +357,6 @@ int ide_acpi_exec_tfs(ide_drive_t *drive) unsigned long gtf_address; unsigned long obj_loc; - if (ide_noacpi) - return 0; - DEBPRINT("call get_GTF, drive=%s port=%d\n", drive->name, drive->dn); ret = do_drive_get_GTF(drive, >f_length, >f_address, &obj_loc); @@ -389,16 +391,6 @@ void ide_acpi_get_timing(ide_hwif_t *hwif) struct acpi_buffer output; union acpi_object *out_obj; - if (ide_noacpi) - return; - - DEBPRINT("ENTER:\n"); - - if (!hwif->acpidata) { - DEBPRINT("no ACPI data for %s\n", hwif->name); - return; - } - /* Setting up output buffer for _GTM */ output.length = ACPI_ALLOCATE_BUFFER; output.pointer = NULL; /* ACPI-CA sets this; save/free it later */ @@ -479,16 +471,6 @@ void ide_acpi_push_timing(ide_hwif_t *hwif) struct ide_acpi_drive_link *master = &hwif->acpidata->master; struct ide_acpi_drive_link *slave = &hwif->acpidata->slave; - if (ide_noacpi) - return; - - DEBPRINT("ENTER:\n"); - - if (!hwif->acpidata) { - DEBPRINT("no ACPI data for %s\n", hwif->name); - return; - } - /* Give the GTM buffer + drive Identify data to the channel via the * _STM method: */ /* setup input parameters buffer for _STM */ @@ -527,16 +509,11 @@ void ide_acpi_set_state(ide_hwif_t *hwif, int on) ide_drive_t *drive; int i; - if (ide_noacpi || ide_noacpi_psx) + if (ide_noacpi_psx) return; DEBPRINT("ENTER:\n"); - if (!hwif->acpidata) { - DEBPRINT("no ACPI data for %s\n", hwif->name); - return; - } - /* channel first and then drives for power on and verse versa for power off */ if (on) acpi_bus_set_power(hwif->acpidata->obj_handle, ACPI_STATE_D0); @@ -616,7 +593,7 @@ void ide_acpi_port_init_devices(ide_hwif_t *hwif) drive->name, err); } - if (!ide_acpionboot) { + if (ide_noacpi || ide_acpionboot == 0) { DEBPRINT("ACPI methods disabled on boot\n"); return; } diff --git a/drivers/ide/ide-pm.c b/drivers/ide/ide-pm.c index c14ca144cff..ad7be2669dc 100644 --- a/drivers/ide/ide-pm.c +++ b/drivers/ide/ide-pm.c @@ -10,9 +10,11 @@ int generic_ide_suspend(struct device *dev, pm_message_t mesg) struct request_pm_state rqpm; int ret; - /* call ACPI _GTM only once */ - if ((drive->dn & 1) == 0 || pair == NULL) - ide_acpi_get_timing(hwif); + if (ide_port_acpi(hwif)) { + /* call ACPI _GTM only once */ + if ((drive->dn & 1) == 0 || pair == NULL) + ide_acpi_get_timing(hwif); + } memset(&rqpm, 0, sizeof(rqpm)); rq = blk_get_request(drive->queue, READ, __GFP_WAIT); @@ -26,9 +28,11 @@ int generic_ide_suspend(struct device *dev, pm_message_t mesg) ret = blk_execute_rq(drive->queue, NULL, rq, 0); blk_put_request(rq); - /* call ACPI _PS3 only after both devices are suspended */ - if (ret == 0 && ((drive->dn & 1) || pair == NULL)) - ide_acpi_set_state(hwif, 0); + if (ret == 0 && ide_port_acpi(hwif)) { + /* call ACPI _PS3 only after both devices are suspended */ + if ((drive->dn & 1) || pair == NULL) + ide_acpi_set_state(hwif, 0); + } return ret; } @@ -42,13 +46,15 @@ int generic_ide_resume(struct device *dev) struct request_pm_state rqpm; int err; - /* call ACPI _PS0 / _STM only once */ - if ((drive->dn & 1) == 0 || pair == NULL) { - ide_acpi_set_state(hwif, 1); - ide_acpi_push_timing(hwif); - } + if (ide_port_acpi(hwif)) { + /* call ACPI _PS0 / _STM only once */ + if ((drive->dn & 1) == 0 || pair == NULL) { + ide_acpi_set_state(hwif, 1); + ide_acpi_push_timing(hwif); + } - ide_acpi_exec_tfs(drive); + ide_acpi_exec_tfs(drive); + } memset(&rqpm, 0, sizeof(rqpm)); rq = blk_get_request(drive->queue, READ, __GFP_WAIT); diff --git a/include/linux/ide.h b/include/linux/ide.h index c6af7c44d46..edc93a6d931 100644 --- a/include/linux/ide.h +++ b/include/linux/ide.h @@ -1419,6 +1419,7 @@ static inline void ide_dma_unmap_sg(ide_drive_t *drive, #ifdef CONFIG_BLK_DEV_IDEACPI int ide_acpi_init(void); +bool ide_port_acpi(ide_hwif_t *hwif); extern int ide_acpi_exec_tfs(ide_drive_t *drive); extern void ide_acpi_get_timing(ide_hwif_t *hwif); extern void ide_acpi_push_timing(ide_hwif_t *hwif); @@ -1427,6 +1428,7 @@ void ide_acpi_port_init_devices(ide_hwif_t *); extern void ide_acpi_set_state(ide_hwif_t *hwif, int on); #else static inline int ide_acpi_init(void) { return 0; } +static inline bool ide_port_acpi(ide_hwif_t *hwif) { return 0; } static inline int ide_acpi_exec_tfs(ide_drive_t *drive) { return 0; } static inline void ide_acpi_get_timing(ide_hwif_t *hwif) { ; } static inline void ide_acpi_push_timing(ide_hwif_t *hwif) { ; } -- cgit v1.2.3-70-g09d2 From e18ed145c7f556f1de8350c32739bf35b26df705 Mon Sep 17 00:00:00 2001 From: Christian Engelmayer Date: Mon, 29 Jun 2009 19:31:41 -0700 Subject: ide: memory overrun in ide_get_identity_ioctl() on big endian machines using ioctl HDIO_OBSOLETE_IDENTITY This patch fixes a memory overrun in function ide_get_identity_ioctl() which chooses the size of a memory buffer depending on the ioctl command that led to the function call, however, passes that buffer to a function which needs the buffer size to be always chosen unconditionally. Due to conditional compilation the memory overrun can only happen on big endian machines. The error can be triggered using ioctl HDIO_OBSOLETE_IDENTITY. Usage of ioctl HDIO_GET_IDENTITY is safe. Signed-off-by: Christian Engelmayer Acked-by: Bartlomiej Zolnierkiewicz Signed-off-by: David S. Miller --- drivers/ide/ide-ioctls.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/ide') diff --git a/drivers/ide/ide-ioctls.c b/drivers/ide/ide-ioctls.c index 82f252c3ee6..e246d3d3fbc 100644 --- a/drivers/ide/ide-ioctls.c +++ b/drivers/ide/ide-ioctls.c @@ -64,7 +64,8 @@ static int ide_get_identity_ioctl(ide_drive_t *drive, unsigned int cmd, goto out; } - id = kmalloc(size, GFP_KERNEL); + /* ata_id_to_hd_driveid() relies on 'id' to be fully allocated. */ + id = kmalloc(ATA_ID_WORDS * 2, GFP_KERNEL); if (id == NULL) { rc = -ENOMEM; goto out; -- cgit v1.2.3-70-g09d2 From 3503e0acbfab0dbcd24ccadd5fe841f3f8290e81 Mon Sep 17 00:00:00 2001 From: "David S. Miller" Date: Mon, 6 Jul 2009 12:39:27 -0700 Subject: Revert "ide: improve handling of Power Management requests" This reverts commit a1317f714af7aed60ddc182d0122477cbe36ee9b. --- drivers/ide/ide-io.c | 54 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 22 deletions(-) (limited to 'drivers/ide') diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c index d5f3c77bead..db96138fefc 100644 --- a/drivers/ide/ide-io.c +++ b/drivers/ide/ide-io.c @@ -466,14 +466,10 @@ void do_ide_request(struct request_queue *q) if (!ide_lock_port(hwif)) { ide_hwif_t *prev_port; + + WARN_ON_ONCE(hwif->rq); repeat: prev_port = hwif->host->cur_port; - - if (drive->dev_flags & IDE_DFLAG_BLOCKED) - rq = hwif->rq; - else - WARN_ON_ONCE(hwif->rq); - if (drive->dev_flags & IDE_DFLAG_SLEEPING && time_after(drive->sleep, jiffies)) { ide_unlock_port(hwif); @@ -500,29 +496,43 @@ repeat: hwif->cur_dev = drive; drive->dev_flags &= ~(IDE_DFLAG_SLEEPING | IDE_DFLAG_PARKED); - if (rq == NULL) { - spin_unlock_irq(&hwif->lock); - spin_lock_irq(q->queue_lock); - /* - * we know that the queue isn't empty, but this can - * happen if ->prep_rq_fn() decides to kill a request - */ + spin_unlock_irq(&hwif->lock); + spin_lock_irq(q->queue_lock); + /* + * we know that the queue isn't empty, but this can happen + * if the q->prep_rq_fn() decides to kill a request + */ + if (!rq) rq = blk_fetch_request(drive->queue); - spin_unlock_irq(q->queue_lock); - spin_lock_irq(&hwif->lock); - if (rq == NULL) { - ide_unlock_port(hwif); - goto out; - } + spin_unlock_irq(q->queue_lock); + spin_lock_irq(&hwif->lock); + + if (!rq) { + ide_unlock_port(hwif); + goto out; } /* * Sanity: don't accept a request that isn't a PM request - * if we are currently power managed. + * if we are currently power managed. This is very important as + * blk_stop_queue() doesn't prevent the blk_fetch_request() + * above to return us whatever is in the queue. Since we call + * ide_do_request() ourselves, we end up taking requests while + * the queue is blocked... + * + * We let requests forced at head of queue with ide-preempt + * though. I hope that doesn't happen too much, hopefully not + * unless the subdriver triggers such a thing in its own PM + * state machine. */ - BUG_ON((drive->dev_flags & IDE_DFLAG_BLOCKED) && - blk_pm_request(rq) == 0); + if ((drive->dev_flags & IDE_DFLAG_BLOCKED) && + blk_pm_request(rq) == 0 && + (rq->cmd_flags & REQ_PREEMPT) == 0) { + /* there should be no pending command at this point */ + ide_unlock_port(hwif); + goto plug_device; + } hwif->rq = rq; -- cgit v1.2.3-70-g09d2 From bc146d23d1358af43f03793c3ad8c9f16bbcffcb Mon Sep 17 00:00:00 2001 From: Maxime Bizon Date: Thu, 16 Jul 2009 06:32:52 +0000 Subject: ide: fix memory leak when flush command is issued I'm using ide on 2.6.30.1 with xfs filesystem. I noticed a kernel memory leak after writing lots of data, the kmalloc-96 slab cache keeps growing. It seems the struct ide_cmd kmalloced by idedisk_prepare_flush is never kfreed. Commit a09485df9cda49fbde2766c86eb18a9cae585162 ("ide: move request type specific code from ide_end_drive_cmd() to callers (v3)") and f505d49ffd25ed062e76ffd17568d3937fcd338c ("ide: fix barriers support") cause this regression, cmd->rq must now be set for ide_complete_cmd to honor the IDE_TFLAG_DYN flag. Signed-off-by: Maxime Bizon Acked-by: Bartlomiej Zolnierkiewicz Signed-off-by: David S. Miller --- drivers/ide/ide-disk.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/ide') diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c index 695181120cd..7f878017b73 100644 --- a/drivers/ide/ide-disk.c +++ b/drivers/ide/ide-disk.c @@ -455,6 +455,7 @@ static void idedisk_prepare_flush(struct request_queue *q, struct request *rq) rq->cmd_type = REQ_TYPE_ATA_TASKFILE; rq->special = cmd; + cmd->rq = rq; } ide_devset_get(multcount, mult_count); -- cgit v1.2.3-70-g09d2 From 2fc2111c2729462b99b6e37f39a48917054776f5 Mon Sep 17 00:00:00 2001 From: Michael Buesch Date: Sun, 19 Jul 2009 09:15:19 +0000 Subject: ide-tape: Don't leak kernel stack information Don't leak kernel stack information through uninitialized structure members. Signed-off-by: Michael Buesch Acked-by: Borislav Petkov . Signed-off-by: David S. Miller --- drivers/ide/ide-tape.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/ide') diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c index 013dc595fab..bc5fb12b913 100644 --- a/drivers/ide/ide-tape.c +++ b/drivers/ide/ide-tape.c @@ -1064,6 +1064,7 @@ static int idetape_blkdev_ioctl(ide_drive_t *drive, unsigned int cmd, tape->best_dsc_rw_freq = config.dsc_rw_frequency; break; case 0x0350: + memset(&config, 0, sizeof(config)); config.dsc_rw_frequency = (int) tape->best_dsc_rw_freq; config.nr_stages = 1; if (copy_to_user(argp, &config, sizeof(config))) -- cgit v1.2.3-70-g09d2