Re: bad sectors, suspicious behaviour

[Artem Bokhan <aptem@ngs.ru>]

patch did not help,  but I found additional necessary condition (I 
didn't take it into account before system reboot):

to reproduce the problem from scratch it is necessary to

echo 1 >  
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:02.0/host4/target4:0:0/4:0:0:0/timeout

returning the value to 30 do not help.

With default 30 secs the problem can not be reproduced


Tejun Heo пишет:
> Hello, Mark, Artem.
>
> Mark Lord wrote:
>   
>> Mmm.. since it happens only once in a while, and not on every EH action,
>> one might assume that it's a race of some kind.
>>
>> One possibility, might be due to .qc_defer.
>>
>> The stock ata_qc_defer relies heavily on ata_tag_valid(),
>> which matches what the above WARN_ON uses.
>>
>> But sata_mv doesn't use ata_tag_valid, because it wants to know
>> about the entire port and not just a single individual link on the port.
>> So instead, it uses ap->nr_active_links for the test.
>>
>> My guess is that these two items are not kept in sync during EH.
>>     
>
> The culprit is mv_qc_defer().  If the controller is in host queueing
> mode, it allows multiple non-NCQ commands to be queued, which
> currently isn't allowed by the current libata core.  Allowing it
> shouldn't be too difficult but it would incur some core layer changes.
> For the time being, can you please test whether the following patch
> fixes the problem?
>
> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index ad169ff..80c655f 100644
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -1134,30 +1134,16 @@ static int mv_qc_defer(struct ata_queued_cmd *qc)
>  	if (ap->nr_active_links == 0)
>  		return 0;
>  
> -	if (pp->pp_flags & MV_PP_FLAG_EDMA_EN) {
> -		/*
> -		 * The port is operating in host queuing mode (EDMA).
> -		 * It can accomodate a new qc if the qc protocol
> -		 * is compatible with the current host queue mode.
> -		 */
> -		if (pp->pp_flags & MV_PP_FLAG_NCQ_EN) {
> -			/*
> -			 * The host queue (EDMA) is in NCQ mode.
> -			 * If the new qc is also an NCQ command,
> -			 * then allow the new qc.
> -			 */
> -			if (qc->tf.protocol == ATA_PROT_NCQ)
> -				return 0;
> -		} else {
> -			/*
> -			 * The host queue (EDMA) is in non-NCQ, DMA mode.
> -			 * If the new qc is also a non-NCQ, DMA command,
> -			 * then allow the new qc.
> -			 */
> -			if (qc->tf.protocol == ATA_PROT_DMA)
> -				return 0;
> -		}
> -	}
> +	/*
> +	 * The port is operating in host queuing mode (EDMA) with NCQ
> +	 * enabled, allow multiple NCQ commands.  EDMA also allows
> +	 * queueing multiple DMA commands but libata core currently
> +	 * doesn't allow it.
> +	 */
> +	if ((pp->pp_flags & MV_PP_FLAG_EDMA_EN) &&
> +	    (pp->pp_flags & MV_PP_FLAG_NCQ_EN) && ata_is_ncq(qc->tf.protocol))
> +		return 0;
> +
>  	return ATA_DEFER_PORT;
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>