Albert Lee wrote:
Alan Cox wrote:
ata_scsi_pass_thru() is not executed at ioctl submission time (block
queue submission time), but rather immediately before it is issued to
the drive. At that point you know the bus is idle, all other commands
have finished executing, and dev->multi_count is fresh not stale. The
code path goes from ata_scsi_pass_thru() to ata_qc_issue() without
releasing the spinlock, even.
Think up to user space
Poorusersapp set multicount to 4
Evilproprietarytuningdaemon set multicount to 8
Poorusersapp issue I/O
at which point an error is indeed best.
But the last point is true -- we should error rather than just warn
there, AFAICS.
Definitely. We've been asked "please do something stupid" and not even in
a case where the requester may know better.
It looks like the ATA passthru commands contain more information than
what libata needs to execute a command.
e.g. protocol number:
libata could possibly infer the protocol from the command opcode.
This is generally a bad practice to guess protocol based on opcode. What
if the code will have to handle a vendor unique command (or some other command
not yet known to it but known to issuer)?
e.g. multi_count:
libata caches dev->multi_count. Passing multi_count along with
each passthru command looks useless for libata.
I'd agree here.
e.g. t_dir:
libata could possible infer the direction from the command opcode
Bad idea again.
or from the protocol number (e.g. 4: PIO_IN / 5: PIO_OUT).
This is reasonable if DMA direction can also be inferred from the protocol
number.
Due to the redundant info, there is possiblely inconsistency between
the parameters. e.g. t_dir vs protocol. e.g. command vs protocol.
I think it's better to allow inconsistency then to limit functionality.
There's another option though -- let the caller specify the default protocol
for the command to be issued or override it if it *knows* what it's doing.
It seems the "redundant" parameters are designed to allow stateless SATL
implementation: The application/passthru command tells the stateless SATL
implementation the protocol and the multi_count, etc. Then SATL just
follows the instruction blindly, even if asked to do something stupid.
Currently libata
- uses the passthru protocol number blindly
(even if the application issues a DMA command with wrong PIO protocol.)
- checks and warns about multi_count
- ignores t_dir, byte_block and so on.
Maybe we need a strategy to deal with incorrect passed-thru commands?
say,
- check and reject if something wrong
- mimimal check and warn/ignore, if it doesn't hurt command execution
- let the caller use defaults based on command code or override them.
--
albert
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html