* [PATCH 01/02] libata: shift host flag constants @ 2006-05-31 10:19 Tejun Heo 2006-05-31 10:20 ` [PATCH 02/02] libata: implement ata_eh_wait() Tejun Heo 2006-06-08 20:48 ` [PATCH 01/02] libata: shift host flag constants Jeff Garzik 0 siblings, 2 replies; 6+ messages in thread From: Tejun Heo @ 2006-05-31 10:19 UTC (permalink / raw) To: Jeff Garzik, linux-ide Nudge host flag constants to make a room after ATA_FLAG_EH_PENDING. New EH flag will be added. Signed-off-by: Tejun Heo <htejun@gmail.com> --- include/linux/libata.h | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) 7395acb2c840fd4d0cacc91d6fb71440057141ab diff --git a/include/linux/libata.h b/include/linux/libata.h index b0ee1c1..3f9c65f 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -149,14 +149,14 @@ enum { ATA_FLAG_NO_ATAPI = (1 << 6), /* No ATAPI support */ ATA_FLAG_PIO_DMA = (1 << 7), /* PIO cmds via DMA */ ATA_FLAG_PIO_LBA48 = (1 << 8), /* Host DMA engine is LBA28 only */ - ATA_FLAG_PIO_POLLING = (1 << 10), /* use polling PIO if LLD - * doesn't handle PIO interrupts */ - ATA_FLAG_NCQ = (1 << 11), /* host supports NCQ */ + ATA_FLAG_PIO_POLLING = (1 << 9), /* use polling PIO if LLD + * doesn't handle PIO interrupts */ + ATA_FLAG_NCQ = (1 << 10), /* host supports NCQ */ - ATA_FLAG_DEBUGMSG = (1 << 14), - ATA_FLAG_FLUSH_PORT_TASK = (1 << 15), /* flush port task */ + ATA_FLAG_DEBUGMSG = (1 << 13), + ATA_FLAG_FLUSH_PORT_TASK = (1 << 14), /* flush port task */ - ATA_FLAG_EH_PENDING = (1 << 16), /* EH pending */ + ATA_FLAG_EH_PENDING = (1 << 15), /* EH pending */ ATA_FLAG_FROZEN = (1 << 17), /* port is frozen */ ATA_FLAG_RECOVERED = (1 << 18), /* recovery action performed */ -- 1.3.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 02/02] libata: implement ata_eh_wait() 2006-05-31 10:19 [PATCH 01/02] libata: shift host flag constants Tejun Heo @ 2006-05-31 10:20 ` Tejun Heo 2006-06-08 20:49 ` Jeff Garzik 2006-06-08 20:48 ` [PATCH 01/02] libata: shift host flag constants Jeff Garzik 1 sibling, 1 reply; 6+ messages in thread From: Tejun Heo @ 2006-05-31 10:20 UTC (permalink / raw) To: Jeff Garzik, linux-ide Implement ata_eh_wait(). On return from this function, it's guaranteed that the EH which was pending or in progress when the function was called is complete - including the tailing part of SCSI EH. This will be used by hotplug and others to synchronize with EH. Signed-off-by: Tejun Heo <htejun@gmail.com> --- drivers/scsi/libata-core.c | 1 + drivers/scsi/libata-eh.c | 38 ++++++++++++++++++++++++++++++++++++++ drivers/scsi/libata.h | 1 + include/linux/libata.h | 2 ++ 4 files changed, 42 insertions(+), 0 deletions(-) c6cf9e99d1de5ca6a08fb639bb73031ffe50d802 diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c index 11df827..66df895 100644 --- a/drivers/scsi/libata-core.c +++ b/drivers/scsi/libata-core.c @@ -5189,6 +5189,7 @@ static void ata_host_init(struct ata_por INIT_WORK(&ap->port_task, NULL, NULL); INIT_LIST_HEAD(&ap->eh_done_q); + init_waitqueue_head(&ap->eh_wait_q); /* set cable type */ ap->cbl = ATA_CBL_NONE; diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c index b88f492..9173d8f 100644 --- a/drivers/scsi/libata-eh.c +++ b/drivers/scsi/libata-eh.c @@ -237,6 +237,7 @@ void ata_scsi_error(struct Scsi_Host *ho ap->eh_context.i = ap->eh_info; memset(&ap->eh_info, 0, sizeof(ap->eh_info)); + ap->flags |= ATA_FLAG_EH_IN_PROGRESS; ap->flags &= ~ATA_FLAG_EH_PENDING; spin_unlock_irqrestore(hs_lock, flags); @@ -290,12 +291,49 @@ void ata_scsi_error(struct Scsi_Host *ho ata_port_printk(ap, KERN_INFO, "EH complete\n"); ap->flags &= ~ATA_FLAG_RECOVERED; + /* tell wait_eh that we're done */ + ap->flags &= ~ATA_FLAG_EH_IN_PROGRESS; + wake_up_all(&ap->eh_wait_q); + spin_unlock_irqrestore(hs_lock, flags); DPRINTK("EXIT\n"); } /** + * ata_port_wait_eh - Wait for the currently pending EH to complete + * @ap: Port to wait EH for + * + * Wait until the currently pending EH is complete. + * + * LOCKING: + * Kernel thread context (may sleep). + */ +void ata_port_wait_eh(struct ata_port *ap) +{ + unsigned long flags; + DEFINE_WAIT(wait); + + retry: + spin_lock_irqsave(&ap->host_set->lock, flags); + + while (ap->flags & (ATA_FLAG_EH_PENDING | ATA_FLAG_EH_IN_PROGRESS)) { + prepare_to_wait(&ap->eh_wait_q, &wait, TASK_UNINTERRUPTIBLE); + spin_unlock_irqrestore(&ap->host_set->lock, flags); + schedule(); + spin_lock_irqsave(&ap->host_set->lock, flags); + } + + spin_unlock_irqrestore(&ap->host_set->lock, flags); + + /* make sure SCSI EH is complete */ + if (scsi_host_in_recovery(ap->host)) { + msleep(10); + goto retry; + } +} + +/** * ata_qc_timeout - Handle timeout of queued command * @qc: Command that timed out * diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h index b76ad7d..d56d9e1 100644 --- a/drivers/scsi/libata.h +++ b/drivers/scsi/libata.h @@ -103,6 +103,7 @@ extern void ata_schedule_scsi_eh(struct /* libata-eh.c */ extern enum scsi_eh_timer_return ata_scsi_timed_out(struct scsi_cmnd *cmd); extern void ata_scsi_error(struct Scsi_Host *host); +extern void ata_port_wait_eh(struct ata_port *ap); extern void ata_qc_schedule_eh(struct ata_queued_cmd *qc); #endif /* __LIBATA_H__ */ diff --git a/include/linux/libata.h b/include/linux/libata.h index 3f9c65f..2eb5828 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -157,6 +157,7 @@ enum { ATA_FLAG_FLUSH_PORT_TASK = (1 << 14), /* flush port task */ ATA_FLAG_EH_PENDING = (1 << 15), /* EH pending */ + ATA_FLAG_EH_IN_PROGRESS = (1 << 16), /* EH in progress */ ATA_FLAG_FROZEN = (1 << 17), /* port is frozen */ ATA_FLAG_RECOVERED = (1 << 18), /* recovery action performed */ @@ -490,6 +491,7 @@ struct ata_port { u32 msg_enable; struct list_head eh_done_q; + wait_queue_head_t eh_wait_q; void *private_data; -- 1.3.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 02/02] libata: implement ata_eh_wait() 2006-05-31 10:20 ` [PATCH 02/02] libata: implement ata_eh_wait() Tejun Heo @ 2006-06-08 20:49 ` Jeff Garzik 2006-06-09 3:53 ` Tejun Heo 0 siblings, 1 reply; 6+ messages in thread From: Jeff Garzik @ 2006-06-08 20:49 UTC (permalink / raw) To: Tejun Heo; +Cc: linux-ide Tejun Heo wrote: > + spin_lock_irqsave(&ap->host_set->lock, flags); > + > + while (ap->flags & (ATA_FLAG_EH_PENDING | ATA_FLAG_EH_IN_PROGRESS)) { > + prepare_to_wait(&ap->eh_wait_q, &wait, TASK_UNINTERRUPTIBLE); > + spin_unlock_irqrestore(&ap->host_set->lock, flags); > + schedule(); > + spin_lock_irqsave(&ap->host_set->lock, flags); > + } Two comments: * why not use completions? * don't use schedule(). If there's nothing to schedule, it IMO chews up too much CPU busy-waiting. schedule_timeout(1) will at least wait for the next timer tick. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 02/02] libata: implement ata_eh_wait() 2006-06-08 20:49 ` Jeff Garzik @ 2006-06-09 3:53 ` Tejun Heo 2006-06-09 3:58 ` Jeff Garzik 0 siblings, 1 reply; 6+ messages in thread From: Tejun Heo @ 2006-06-09 3:53 UTC (permalink / raw) To: Jeff Garzik; +Cc: linux-ide Jeff Garzik wrote: > Tejun Heo wrote: >> + spin_lock_irqsave(&ap->host_set->lock, flags); >> + >> + while (ap->flags & (ATA_FLAG_EH_PENDING | >> ATA_FLAG_EH_IN_PROGRESS)) { >> + prepare_to_wait(&ap->eh_wait_q, &wait, TASK_UNINTERRUPTIBLE); >> + spin_unlock_irqrestore(&ap->host_set->lock, flags); >> + schedule(); >> + spin_lock_irqsave(&ap->host_set->lock, flags); >> + } > > > Two comments: > > * why not use completions? Mainly because there can be more than one waiters and also because the waiting doesn't really fit completion semantics. In some places, eh_wait is used just to flush EH without actually scheduling EH. In such cases, there's no synchronization point to start waiting for completion. > * don't use schedule(). If there's nothing to schedule, it IMO chews up > too much CPU busy-waiting. schedule_timeout(1) will at least wait for > the next timer tick. I don't really understand your point here. All that schedule_timeout() does is adding a timer to wake it up after the given jiffies pass. It only adds wake up condition. ie. it only makes sleeps shorter not longer. e.g. both of the following code snippets result in busy sleep/wake up loop. __set_current_state(TASK_RUNNING); while (some_condition) schedule(); ------- __set_current_state(TASK_RUNNING); while (some_condition) schedule_timeout(1); The only behavior difference between above two code snippets is that the latter registers and unregisters timer every iteration which probably never expires as the thread would usually get scheduled again before a full tick passes. AFAICS, schedule_timeout() is to be used when the wait should be 'timed out' when the thread has other wake up condition it's waiting for but it doesn't want to wait forever. As a special case, when the condition is nil, it works as unconditional sleep. Whether a thread gets rescheduled or not is determined by current->task_state. If it's RUNNING, it will get rescheduled immediately. If it's UNINTERRUPTIBLE or INTERRUPTIBLE, it will wait till gets woken up. schedule() simply means the caller is done with the CPU at the moment and giving it away. In eh_wait(), prepare_to_wait() is called w/ TASK_UNINTERRUPTIBLE before calling schedule, which sets current->task_state to UNINTERRUPTIBLE. The thread won't be scheduled again until the wait condition triggers and restores task_state to RUNNING. Oops, I forgot calling finish_wait() after exiting the loop. I'll submit a patch as soon as #upstream is updated. Sorry. Anyways, please read workqueue.c::flush_cpu_workqueue() and sched.c::wait_for_completion(). These two functions use similar waiting loops. Thanks. -- tejun ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 02/02] libata: implement ata_eh_wait() 2006-06-09 3:53 ` Tejun Heo @ 2006-06-09 3:58 ` Jeff Garzik 0 siblings, 0 replies; 6+ messages in thread From: Jeff Garzik @ 2006-06-09 3:58 UTC (permalink / raw) To: Tejun Heo; +Cc: linux-ide Tejun Heo wrote: > Jeff Garzik wrote: >> Tejun Heo wrote: >>> + spin_lock_irqsave(&ap->host_set->lock, flags); >>> + >>> + while (ap->flags & (ATA_FLAG_EH_PENDING | >>> ATA_FLAG_EH_IN_PROGRESS)) { >>> + prepare_to_wait(&ap->eh_wait_q, &wait, TASK_UNINTERRUPTIBLE); >>> + spin_unlock_irqrestore(&ap->host_set->lock, flags); >>> + schedule(); >>> + spin_lock_irqsave(&ap->host_set->lock, flags); DUH. Ignore my objection, I read the code incorrectly, thinking it was just a simple polling delay, when obviously it is not. ACK. Jeff ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 01/02] libata: shift host flag constants 2006-05-31 10:19 [PATCH 01/02] libata: shift host flag constants Tejun Heo 2006-05-31 10:20 ` [PATCH 02/02] libata: implement ata_eh_wait() Tejun Heo @ 2006-06-08 20:48 ` Jeff Garzik 1 sibling, 0 replies; 6+ messages in thread From: Jeff Garzik @ 2006-06-08 20:48 UTC (permalink / raw) To: Tejun Heo; +Cc: linux-ide Tejun Heo wrote: > Nudge host flag constants to make a room after ATA_FLAG_EH_PENDING. > New EH flag will be added. > > Signed-off-by: Tejun Heo <htejun@gmail.com> ACK ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-06-09 3:58 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-05-31 10:19 [PATCH 01/02] libata: shift host flag constants Tejun Heo 2006-05-31 10:20 ` [PATCH 02/02] libata: implement ata_eh_wait() Tejun Heo 2006-06-08 20:49 ` Jeff Garzik 2006-06-09 3:53 ` Tejun Heo 2006-06-09 3:58 ` Jeff Garzik 2006-06-08 20:48 ` [PATCH 01/02] libata: shift host flag constants Jeff Garzik
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).