aboutsummaryrefslogtreecommitdiffstats
path: root/drivers/block/hd.c
diff options
context:
space:
mode:
authorTejun Heo <tj@kernel.org>2009-04-27 23:38:33 -0400
committerJens Axboe <jens.axboe@oracle.com>2009-04-28 01:36:56 -0400
commite93b9fb7d85da4fd9d5171649e5ddcac1dd572bf (patch)
tree72671e9cad1d1fa6acadd7e389515f3579f3ff26 /drivers/block/hd.c
parent7090a0a97f55cbf47547a140fcc5a349f32c598c (diff)
hd: fix locking
hd dance around local irq and HD_IRQ enable without achieving much. It ends up transferring data from irq handler with both local irq and HD_IRQ disabled. The only place it actually does something is while transferring the first block of a request which it does with HD_IRQ disabled but local irq enabled. Unfortunately, the dancing is horribly broken from locking POV. IRQ and timeout handlers access block queue without grabbing the queue lock and running the driver in SMP configuration crashes the whole machine pretty quickly. Remove meaningless irq enable/disable dancing and add proper locking in issue, irq and timeout paths. Signed-off-by: Tejun Heo <tj@kernel.org>
Diffstat (limited to 'drivers/block/hd.c')
-rw-r--r--drivers/block/hd.c17
1 files changed, 7 insertions, 10 deletions
diff --git a/drivers/block/hd.c b/drivers/block/hd.c
index 3c11f062a18c..baaa9e486e50 100644
--- a/drivers/block/hd.c
+++ b/drivers/block/hd.c
@@ -509,7 +509,6 @@ ok_to_write:
509 if (i > 0) { 509 if (i > 0) {
510 SET_HANDLER(&write_intr); 510 SET_HANDLER(&write_intr);
511 outsw(HD_DATA, req->buffer, 256); 511 outsw(HD_DATA, req->buffer, 256);
512 local_irq_enable();
513 } else { 512 } else {
514#if (HD_DELAY > 0) 513#if (HD_DELAY > 0)
515 last_req = read_timer(); 514 last_req = read_timer();
@@ -541,8 +540,7 @@ static void hd_times_out(unsigned long dummy)
541 if (!CURRENT) 540 if (!CURRENT)
542 return; 541 return;
543 542
544 disable_irq(HD_IRQ); 543 spin_lock_irq(hd_queue->queue_lock);
545 local_irq_enable();
546 reset = 1; 544 reset = 1;
547 name = CURRENT->rq_disk->disk_name; 545 name = CURRENT->rq_disk->disk_name;
548 printk("%s: timeout\n", name); 546 printk("%s: timeout\n", name);
@@ -552,9 +550,8 @@ static void hd_times_out(unsigned long dummy)
552#endif 550#endif
553 end_request(CURRENT, 0); 551 end_request(CURRENT, 0);
554 } 552 }
555 local_irq_disable();
556 hd_request(); 553 hd_request();
557 enable_irq(HD_IRQ); 554 spin_unlock_irq(hd_queue->queue_lock);
558} 555}
559 556
560static int do_special_op(struct hd_i_struct *disk, struct request *req) 557static int do_special_op(struct hd_i_struct *disk, struct request *req)
@@ -592,7 +589,6 @@ static void hd_request(void)
592 return; 589 return;
593repeat: 590repeat:
594 del_timer(&device_timer); 591 del_timer(&device_timer);
595 local_irq_enable();
596 592
597 req = CURRENT; 593 req = CURRENT;
598 if (!req) { 594 if (!req) {
@@ -601,7 +597,6 @@ repeat:
601 } 597 }
602 598
603 if (reset) { 599 if (reset) {
604 local_irq_disable();
605 reset_hd(); 600 reset_hd();
606 return; 601 return;
607 } 602 }
@@ -660,9 +655,7 @@ repeat:
660 655
661static void do_hd_request(struct request_queue *q) 656static void do_hd_request(struct request_queue *q)
662{ 657{
663 disable_irq(HD_IRQ);
664 hd_request(); 658 hd_request();
665 enable_irq(HD_IRQ);
666} 659}
667 660
668static int hd_getgeo(struct block_device *bdev, struct hd_geometry *geo) 661static int hd_getgeo(struct block_device *bdev, struct hd_geometry *geo)
@@ -684,12 +677,16 @@ static irqreturn_t hd_interrupt(int irq, void *dev_id)
684{ 677{
685 void (*handler)(void) = do_hd; 678 void (*handler)(void) = do_hd;
686 679
680 spin_lock(hd_queue->queue_lock);
681
687 do_hd = NULL; 682 do_hd = NULL;
688 del_timer(&device_timer); 683 del_timer(&device_timer);
689 if (!handler) 684 if (!handler)
690 handler = unexpected_hd_interrupt; 685 handler = unexpected_hd_interrupt;
691 handler(); 686 handler();
692 local_irq_enable(); 687
688 spin_unlock(hd_queue->queue_lock);
689
693 return IRQ_HANDLED; 690 return IRQ_HANDLED;
694} 691}
695 692