ide: add ide_[un]lock_hwgroup() helpers
PATCH 2/3 ide: add ide_[un]lock_hwgroup() helpers
Add ide_[un]lock_hwgroup() inline helpers for obtaining exclusive access to the given hwgroup and update the core code accordingly.
[ This change besides making code saner results in more efficient
use of ide_{get,release}_lock(). ]
--- drivers/ide/ide-io.c | 32 +++++++++++--------------------- drivers/ide/ide-park.c | 2 +- include/linux/ide.h | 20 ++++++++++++++++++++ 3 files changed, 32 insertions(+), 22 deletions(-)
drivers/ide/ide-io.c +11 -21
@@ -790,10 +790,7 @@ void do_ide_request(struct request_queue/* caller must own hwgroup->lock */ BUG_ON (!irqs_disabled ()); -while (!hwgroup->busy) { - hwgroup->busy =1 ; -/* for atari only */ -ide_get_lock (ide_intr, hwgroup); +while (!ide_lock_hwgroup (hwgroup)) { drive =choose_drive (hwgroup);if (drive ==NULL ) {int sleeping =0 ;
@@ -825,17 +822,10 @@ void do_ide_request(struct request_queue hwgroup->sleeping =1 ; hwgroup->req_gen_timer = hwgroup->req_gen;mod_timer (&hwgroup->timer, sleep); -/* we purposely leave hwgroup->busy==1 +/* we purposely leave hwgroup locked * while sleeping */ - }else { -/* Ugly, but how can we sleep for the lock -* otherwise? perhaps from tq_disk? -*/ - -/* for atari only */ -ide_release_lock (); - hwgroup->busy =0 ; - } + }else +ide_unlock_hwgroup (hwgroup);/* no more work for this hwgroup (for now) */ goto plug_device;
@@ -865,7 +855,7 @@ void do_ide_request(struct request_queue*/ rq =elv_next_request (drive->queue);if (!rq) { - hwgroup->busy =0 ; +ide_unlock_hwgroup (hwgroup);break ; }
@@ -885,8 +875,8 @@ void do_ide_request(struct request_queueif ((drive->dev_flags &IDE_DFLAG_BLOCKED ) &&blk_pm_request (rq) ==0 && (rq->cmd_flags &REQ_PREEMPT ) ==0 ) { -/* We clear busy, there should be no pending ATA command at this point. */ - hwgroup->busy =0 ; +/* there should be no pending command at this point */ +ide_unlock_hwgroup (hwgroup); goto plug_device; }
@@ -897,7 +887,7 @@ void do_ide_request(struct request_queuespin_lock_irq (&hwgroup->lock);if (startstop == ide_stopped) { - hwgroup->busy =0 ; +ide_unlock_hwgroup (hwgroup);if (!elv_queue_empty (orig_drive->queue))blk_plug_device (orig_drive->queue); }
@@ -1001,7 +991,7 @@ void ide_timer_expiry (unsigned long dat*/ if (hwgroup->sleeping) { hwgroup->sleeping =0 ; - hwgroup->busy =0 ; +ide_unlock_hwgroup (hwgroup); } }else {ide_drive_t *drive = hwgroup->drive;
@@ -1056,7 +1046,7 @@ void ide_timer_expiry (unsigned long datspin_lock_irq (&hwgroup->lock);enable_irq (hwif->irq);if (startstop == ide_stopped) { - hwgroup->busy =0 ; +ide_unlock_hwgroup (hwgroup);if (!elv_queue_empty (drive->queue))blk_plug_device (drive->queue); }
@@ -1249,7 +1239,7 @@ irqreturn_t ide_intr (int irq, void *dev drive->service_time = jiffies - drive->service_start;if (startstop == ide_stopped) {if (hwgroup->handler ==NULL ) {/* paranoia */ - hwgroup->busy =0 ; +ide_unlock_hwgroup (hwgroup);if (!elv_queue_empty (drive->queue))blk_plug_device (drive->queue); }else
drivers/ide/ide-park.c +1 -1
@@ -22,7 +22,7 @@ static void issue_park_cmd(ide_drive_t *if (reset_timer && hwgroup->sleeping &&del_timer (&hwgroup->timer)) { hwgroup->sleeping =0 ; - hwgroup->busy =0 ; +ide_unlock_hwgroup (hwgroup);blk_start_queueing (q); }spin_unlock_irq (&hwgroup->lock);
include/linux/ide.h +20 -0
@@ -1280,6 +1280,26 @@ extern void ide_stall_queue(ide_drive_textern void ide_timer_expiry (unsigned long );extern irqreturn_t ide_intr (int irq,void *dev_id); + +static inline int ide_lock_hwgroup (ide_hwgroup_t *hwgroup) +{ +if (hwgroup->busy) +return 1 ; + + hwgroup->busy =1 ; +/* for atari only */ +ide_get_lock (ide_intr, hwgroup); + +return 0 ; +} + +static inline void ide_unlock_hwgroup (ide_hwgroup_t *hwgroup) +{ +/* for atari only */ +ide_release_lock (); + hwgroup->busy =0 ; +} +extern void do_ide_request (struct request_queue *);void ide_init_disk (struct gendisk *,ide_drive_t *);
drivers/ide/ide-io.c void do_ide_request(struct request_queue · line 793
Quoted 5 lines
drivers/ide/ide-io.c +1
-4-while (!hwgroup->busy) { - hwgroup->busy =1 ; -/* for atari only */ -ide_get_lock (ide_intr, hwgroup); +while (!ide_lock_hwgroup (hwgroup)) {
Something I've run into while working on the locking stuff: what happens if the above ide_lock_hwgroup(hwgroup) sleeps for long enough to trigger the request timer?
I'll think about the ramifications of your patch in the context of what I tested WRT unlocking whenever hwgroup->busy is cleared, and get back to you.
Michael
[ Sorry for the late reply. ]
Quoted 9 lines · Michael Schmitz
drivers/ide/ide-io.c void do_ide_request(struct request_queue · line 793
Quoted 5 lines
drivers/ide/ide-io.c +1
-4-while (!hwgroup->busy) { - hwgroup->busy =1 ; -/* for atari only */ -ide_get_lock (ide_intr, hwgroup); +while (!ide_lock_hwgroup (hwgroup)) {Something I've run into while working on the locking stuff: what happens if the above
ide_lock_hwgroup(hwgroup)sleeps for long enough to trigger the request timer?
AFAICS this happens before the hwgroup timeout timer is armed and IDE is not using block layer request timers yet so we should be fine here..
Thanks,
Bart
Hi,
Quoted 1 line
[ Sorry for the late reply. ]
No worries - I haven't been very active either. It's home improvement season down under...
Quoted 6 lines
Quoted 3 lines
Something I've run into while working on the locking stuff: what happens if the above
ide_lock_hwgroup(hwgroup)sleeps for long enough to trigger the request timer?AFAICS this happens before the hwgroup timeout timer is armed and IDE is not using block layer request timers yet so we should be fine here..
OK; I'll just let it block while SCSI requests are in flight. Thanks for pondering this :-)
There's another change I'll send via Geert: since in Falcon, IDE uses stdma_intr as interrupt handler (interrupts dispatched there to whichever driver has exclusive use of the ST-DMA and associated interrupt), and this handler has been registered prior to IDE setup it is not necessary to request the IDE interrupt on Falcon when probing the IDE interface. In fact, this results in the IDE interrupt possibly getting run while the lock is not being held by IDE.
Other than that it all seems to work fine.
Michael