Re: [PATCH RESEND 2/2] SCSI: add scsi_device-retries

2006-11-19 Thread Kai Makisara 
On Sun, 19 Nov 2006, Tejun Heo wrote:

 Add scsi_device-retries to provide generic control over command
 retries, which is very similar to sdev-timeout.  The initial value is
 -1 and high level driver is free to override on attach if negative.
 Note that -1 has the same effect as 0 (no retry) and signifies that
 it's the default value.  As with sdev-timeout, sdev-retries is
 exported under the device sysfs node.
 
 All high level drivers are converted to use it for retry value.
 However, there are exceptions.
 
 * sd_sync_cache() and sr::get_sectorsize() loop three more times
   around normal command execution on failure.
 
 * st uses three retry limits - MAX_RETRIES, MAX_WRITE_RETRIES and
   MAX_READY_RETRIES, which are all zero.  This patch only converts
   MAX_RETRIES to sdev-retries.  Defining WRITE and READY retries in
   terms of sdev-retries would make more sense.
 
I am neither acking nor naking this now. The patch does not change st 
behavior but moves part of the retry strategy out of the driver. (Which is 
also good because it makes one of the retry limits user changeable.)

Combining all retry counts is something that may not be a good thing. 
Below is justification why st has three different retry limits.

For some devices one number of retries is not perfect for all functions.

The firmware of tape devices usually retries quite thoroughly before 
returning error. This is why the default zero applies to most cases.

MAX_WRITE_RETRIES is separate from MAX_RETRIES because a plausible 
strategy might be (maybe not any more) to allow no retries for write but 
allow retries for reading and repositioning. The writes should fail 
directly so that the minimum number of marginal errors will be written. 
The reads can be retried so that even marginal data can be recovered. 
(Note that this may lead to tape positioning errors and may not be a 
good strategy in all cases.)

MAX_READY_RETRIES is used for commands that don't move the tape. This is a 
situation where retrying is probably harmless.

-- 
Kai
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND 2/2] SCSI: add scsi_device-retries

2006-11-19 Thread Tejun Heo 

Hello,

Kai Makisara wrote:

* st uses three retry limits - MAX_RETRIES, MAX_WRITE_RETRIES and
  MAX_READY_RETRIES, which are all zero.  This patch only converts
  MAX_RETRIES to sdev-retries.  Defining WRITE and READY retries in
  terms of sdev-retries would make more sense.

I am neither acking nor naking this now. The patch does not change st 
behavior but moves part of the retry strategy out of the driver. (Which is 
also good because it makes one of the retry limits user changeable.)


Combining all retry counts is something that may not be a good thing. 
Below is justification why st has three different retry limits.


For some devices one number of retries is not perfect for all functions.

The firmware of tape devices usually retries quite thoroughly before 
returning error. This is why the default zero applies to most cases.


MAX_WRITE_RETRIES is separate from MAX_RETRIES because a plausible 
strategy might be (maybe not any more) to allow no retries for write but 
allow retries for reading and repositioning. The writes should fail 
directly so that the minimum number of marginal errors will be written. 
The reads can be retried so that even marginal data can be recovered. 
(Note that this may lead to tape positioning errors and may not be a 
good strategy in all cases.)


MAX_READY_RETRIES is used for commands that don't move the tape. This is a 
situation where retrying is probably harmless.


I see.  Then, how about adding and initializing STp-write_retries and 
STp-ready_retries according to SDp-retries (some reasonable default 
value, say write_retries is always zero while ready_retries is round up 
of retries * 1.1) and export both through sysfs?  That will give lower 
level primitive control as other ULDs do and also allow users to 
configure each timeout separately if necessary.


Thanks.

--
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html