Re: [PATCH 08/11] libata-eh-fw: implement new EH scheduling from PIO

[Tejun Heo <htejun@gmail.com>]

Albert Lee wrote:
> Tejun Heo wrote:
>> PIO executes without holding host_set lock, so it cannot be
>> synchronized using the same mechanism as interrupt driven execution.
>> port_task framework makes sure that EH is not entered until PIO task
>> is flushed, so PIO task can be sure the qc in progress won't go away
>> underneath it.  One thing it cannot be sure of is whether the qc has
>> already been scheduled for EH by another exception condition while
>> host_set lock was released.
>>
>> This patch makes ata_poll_qc-complete() handle such conditions
>> properly and make it freeze the port if HSM violation is detected
>> during PIO execution.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
>>
>> ---
>>
>>  drivers/scsi/libata-core.c |   23 +++++++++++++++++++----
>>  1 files changed, 19 insertions(+), 4 deletions(-)
>>
>> 52240d1d208615e461c41a5c297f521a8a892f0d
>> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
>> index 436ff5b..86d6a7d 100644
>> --- a/drivers/scsi/libata-core.c
>> +++ b/drivers/scsi/libata-core.c
>> @@ -3429,16 +3429,31 @@ skip_map:
>>   *	LOCKING:
>>   *	None.  (grabs host lock)
>>   */
>> -
>>  void ata_poll_qc_complete(struct ata_queued_cmd *qc)
>>  {
>>  	struct ata_port *ap = qc->ap;
>>  	unsigned long flags;
>>  
>>  	spin_lock_irqsave(&ap->host_set->lock, flags);
>> -	ap->flags &= ~ATA_FLAG_NOINTR;
>> -	ata_irq_on(ap);
>> -	ata_qc_complete(qc);
>> +
>> +	if (ap->ops->error_handler) {
>> +		/* EH might have kicked in while host_set lock is released */
>> +		qc = ata_qc_from_tag(ap, qc->tag);
>> +		if (qc) {
>> +			if (!(qc->err_mask & AC_ERR_HSM)) {
>> +				ap->flags &= ~ATA_FLAG_NOINTR;
>> +				ata_irq_on(ap);
>> +				ata_qc_complete(qc);
>> +			} else
>> +				ata_port_freeze(ap);
> 
> I don't quite understand here. For the AC_ERR_HSM error, how does the
> EH get triggered after ata_port_freeze()? Do we wait until time out?
> 

/**
  *	ata_port_freeze - abort & freeze port
  *	@ap: ATA port to freeze
  *
  *	Abort and freeze @ap.
         ^^^^^^
  *
  *	LOCKING:
  *	spin_lock_irqsave(host_set lock)
  *
  *	RETURNS:
  *	Number of aborted commands.
  */
int ata_port_freeze(struct ata_port *ap)
{
	int nr_aborted;

	WARN_ON(!ap->ops->error_handler);

	nr_aborted = ata_port_abort(ap);
                      ^^^^^^^^^^^^^^^^^^^
	__ata_port_freeze(ap);

	return nr_aborted;
}

-- 
tejun