On 05/09/2015 06:50 AM, Christoph Hellwig wrote:
On Thu, May 07, 2015 at 01:39:30PM -0700, Andy Grover wrote:
How about if we just exclude the
troublesome opcodes (option #5)? They'd still be available in
full-passthrough mode...
That looks way better, but I'd defintively prefer a switch statement
for the opcode handling.
OK here's that:
https://git.kernel.org/cgit/linux/kernel/git/grover/linux.git/commit/?h=tcmu-iomode-option5
but how about this? More similar to how pscsi does it:
https://git.kernel.org/cgit/linux/kernel/git/grover/linux.git/commit/?h=tcmu-iomode-option7
although since we aren't calling sbc_parse_cdb() for passed-up opcodes
any more, userspace would have to implement the equivalents of
sbc_check_dpofua() and sbc_check_prot().
But what I still don't quite understand is why allowing COMPARE_AND_WRITE
as-is isn't an option for you. Given how it is sequenced into individual
reads and writes it should work just fine. Do you have a testcase
showing how it doesn't work?
I don't have a testcase but I've been looking at this code a LOT...
The execute_rw function uses data_direction to determine read vs. write.
TCMU doesn't use that or the passed-in sgl, it just passes the CDB and
the data buffers to userspace for handling. If the emulated sequencing
is in place for CAW, the first call to tcmu_execute_rw will result in
userspace being given the cmd, and userspace would then presumably
complete both parts of the command and return success, instead of just
doing the read part.
But then target_complete_ok_work would call compare_and_write_callback
via cmd->transport_complete_callback, which would execute the command
again for the "write" part, and the exact same command would be sent to
the userspace handler. This would likely result in the userspace handler
returning a miscompare error even though the cmd succeeded the first time.
Yes?
Regards -- Andy
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
