diff options
author | Jeff Garzik <jgarzik@pobox.com> | 2005-09-16 06:01:48 -0400 |
---|---|---|
committer | Jeff Garzik <jgarzik@pobox.com> | 2005-09-16 06:01:48 -0400 |
commit | 7fb6ec287a05d7a71ec086d8bc9a452d5e16ff1a (patch) | |
tree | fd0a4a8df9eec8b22d1ea9f18443dfc2e32e80eb /drivers/scsi/libata-core.c | |
parent | 065d9cac98a5406ecd5a1368f8fd38f55739dee9 (diff) |
[libata] fix PIO completion race
Make sure we that completion is the final action we take; prior to this
change, another CPU may have changed ap->pio_task_state before we tested
it a final time.
Spotted by, and original patch by Albert Lee @ IBM.
Also includes a minor optimization: eliminate a ton of unnecessary
queue_work() calls, simply by jumping to the beginning of the FSM
function ata_pio_task().
Diffstat (limited to 'drivers/scsi/libata-core.c')
-rw-r--r-- | drivers/scsi/libata-core.c | 37 |
1 files changed, 23 insertions, 14 deletions
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c index 5cc53cd9323e..d92273cbe0de 100644 --- a/drivers/scsi/libata-core.c +++ b/drivers/scsi/libata-core.c | |||
@@ -2465,9 +2465,12 @@ static unsigned long ata_pio_poll(struct ata_port *ap) | |||
2465 | * | 2465 | * |
2466 | * LOCKING: | 2466 | * LOCKING: |
2467 | * None. (executing in kernel thread context) | 2467 | * None. (executing in kernel thread context) |
2468 | * | ||
2469 | * RETURNS: | ||
2470 | * Non-zero if qc completed, zero otherwise. | ||
2468 | */ | 2471 | */ |
2469 | 2472 | ||
2470 | static void ata_pio_complete (struct ata_port *ap) | 2473 | static int ata_pio_complete (struct ata_port *ap) |
2471 | { | 2474 | { |
2472 | struct ata_queued_cmd *qc; | 2475 | struct ata_queued_cmd *qc; |
2473 | u8 drv_stat; | 2476 | u8 drv_stat; |
@@ -2486,14 +2489,14 @@ static void ata_pio_complete (struct ata_port *ap) | |||
2486 | if (drv_stat & (ATA_BUSY | ATA_DRQ)) { | 2489 | if (drv_stat & (ATA_BUSY | ATA_DRQ)) { |
2487 | ap->pio_task_state = PIO_ST_LAST_POLL; | 2490 | ap->pio_task_state = PIO_ST_LAST_POLL; |
2488 | ap->pio_task_timeout = jiffies + ATA_TMOUT_PIO; | 2491 | ap->pio_task_timeout = jiffies + ATA_TMOUT_PIO; |
2489 | return; | 2492 | return 0; |
2490 | } | 2493 | } |
2491 | } | 2494 | } |
2492 | 2495 | ||
2493 | drv_stat = ata_wait_idle(ap); | 2496 | drv_stat = ata_wait_idle(ap); |
2494 | if (!ata_ok(drv_stat)) { | 2497 | if (!ata_ok(drv_stat)) { |
2495 | ap->pio_task_state = PIO_ST_ERR; | 2498 | ap->pio_task_state = PIO_ST_ERR; |
2496 | return; | 2499 | return 0; |
2497 | } | 2500 | } |
2498 | 2501 | ||
2499 | qc = ata_qc_from_tag(ap, ap->active_tag); | 2502 | qc = ata_qc_from_tag(ap, ap->active_tag); |
@@ -2502,6 +2505,10 @@ static void ata_pio_complete (struct ata_port *ap) | |||
2502 | ap->pio_task_state = PIO_ST_IDLE; | 2505 | ap->pio_task_state = PIO_ST_IDLE; |
2503 | 2506 | ||
2504 | ata_poll_qc_complete(qc, drv_stat); | 2507 | ata_poll_qc_complete(qc, drv_stat); |
2508 | |||
2509 | /* another command may start at this point */ | ||
2510 | |||
2511 | return 1; | ||
2505 | } | 2512 | } |
2506 | 2513 | ||
2507 | 2514 | ||
@@ -2709,7 +2716,7 @@ static void __atapi_pio_bytes(struct ata_queued_cmd *qc, unsigned int bytes) | |||
2709 | 2716 | ||
2710 | next_sg: | 2717 | next_sg: |
2711 | if (unlikely(qc->cursg >= qc->n_elem)) { | 2718 | if (unlikely(qc->cursg >= qc->n_elem)) { |
2712 | /* | 2719 | /* |
2713 | * The end of qc->sg is reached and the device expects | 2720 | * The end of qc->sg is reached and the device expects |
2714 | * more data to transfer. In order not to overrun qc->sg | 2721 | * more data to transfer. In order not to overrun qc->sg |
2715 | * and fulfill length specified in the byte count register, | 2722 | * and fulfill length specified in the byte count register, |
@@ -2721,7 +2728,7 @@ next_sg: | |||
2721 | unsigned int i; | 2728 | unsigned int i; |
2722 | 2729 | ||
2723 | if (words) /* warning if bytes > 1 */ | 2730 | if (words) /* warning if bytes > 1 */ |
2724 | printk(KERN_WARNING "ata%u: %u bytes trailing data\n", | 2731 | printk(KERN_WARNING "ata%u: %u bytes trailing data\n", |
2725 | ap->id, bytes); | 2732 | ap->id, bytes); |
2726 | 2733 | ||
2727 | for (i = 0; i < words; i++) | 2734 | for (i = 0; i < words; i++) |
@@ -2849,9 +2856,7 @@ static void ata_pio_block(struct ata_port *ap) | |||
2849 | if (is_atapi_taskfile(&qc->tf)) { | 2856 | if (is_atapi_taskfile(&qc->tf)) { |
2850 | /* no more data to transfer or unsupported ATAPI command */ | 2857 | /* no more data to transfer or unsupported ATAPI command */ |
2851 | if ((status & ATA_DRQ) == 0) { | 2858 | if ((status & ATA_DRQ) == 0) { |
2852 | ap->pio_task_state = PIO_ST_IDLE; | 2859 | ap->pio_task_state = PIO_ST_LAST; |
2853 | |||
2854 | ata_poll_qc_complete(qc, status); | ||
2855 | return; | 2860 | return; |
2856 | } | 2861 | } |
2857 | 2862 | ||
@@ -2887,7 +2892,12 @@ static void ata_pio_error(struct ata_port *ap) | |||
2887 | static void ata_pio_task(void *_data) | 2892 | static void ata_pio_task(void *_data) |
2888 | { | 2893 | { |
2889 | struct ata_port *ap = _data; | 2894 | struct ata_port *ap = _data; |
2890 | unsigned long timeout = 0; | 2895 | unsigned long timeout; |
2896 | int qc_completed; | ||
2897 | |||
2898 | fsm_start: | ||
2899 | timeout = 0; | ||
2900 | qc_completed = 0; | ||
2891 | 2901 | ||
2892 | switch (ap->pio_task_state) { | 2902 | switch (ap->pio_task_state) { |
2893 | case PIO_ST_IDLE: | 2903 | case PIO_ST_IDLE: |
@@ -2898,7 +2908,7 @@ static void ata_pio_task(void *_data) | |||
2898 | break; | 2908 | break; |
2899 | 2909 | ||
2900 | case PIO_ST_LAST: | 2910 | case PIO_ST_LAST: |
2901 | ata_pio_complete(ap); | 2911 | qc_completed = ata_pio_complete(ap); |
2902 | break; | 2912 | break; |
2903 | 2913 | ||
2904 | case PIO_ST_POLL: | 2914 | case PIO_ST_POLL: |
@@ -2913,10 +2923,9 @@ static void ata_pio_task(void *_data) | |||
2913 | } | 2923 | } |
2914 | 2924 | ||
2915 | if (timeout) | 2925 | if (timeout) |
2916 | queue_delayed_work(ata_wq, &ap->pio_task, | 2926 | queue_delayed_work(ata_wq, &ap->pio_task, timeout); |
2917 | timeout); | 2927 | else if (!qc_completed) |
2918 | else | 2928 | goto fsm_start; |
2919 | queue_work(ata_wq, &ap->pio_task); | ||
2920 | } | 2929 | } |
2921 | 2930 | ||
2922 | static void atapi_request_sense(struct ata_port *ap, struct ata_device *dev, | 2931 | static void atapi_request_sense(struct ata_port *ap, struct ata_device *dev, |