lkml.org 
[lkml]   [2013]   [Aug]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 8/9] target: Add support for COMPARE_AND_WRITE emulation
From
Date
On Wed, 2013-08-21 at 09:14 -0700, Christoph Hellwig wrote:
> I don't like the layering here. The re-execution of the same command
> for both reading and writing the data from/to the backend device already
> looks sketchy here due to doubling work of task attribute handling, the
> various state bits, etc. And it will only get more complicated when
> the required locking is added. In addition we have all that confusion
> about overloading the data direction.
>
> I think the way this should be handled is:
>
>
> - cmd->execute_cmd gets set to a new sbc_emulate_compare_and_write
> - sbc_emulate_compare_and_write does all the setup for the locking,
> sets up the read buffer, then calls ops->execute_rw to do the
> read. The complete callback does the comparism, then calls
> ops->execute_rw to do the write, and the second time we hit
> the complete callback we teard down the read buffer, stop the
> locking, etc.
>

I do like this approach better, however calling ops->execute_rw() the
second time around does require at least the TRANSPORT_PROCESSING and
other transport_state bits from target_execute_cmd() to be set for
things to work correctly. Bypassing the aborted + CMD_*STOP checks
should be OK for the write submission, and will kick (if necessary)
during the final completion.

Setting up the read buffer from sbc_emulate_compare_and_write() would
require two extra COMPARE_AND_WRITE specific se_cmd elements, so I'm
tempted to continue to use the bidi fields for this (because they
already exist) with transport_generic_get_mem_bidi(), and use a
SCF_COMPARE_AND_WRITE flag to avoid any CDB specific checks in
transport_complete_ok(), and reverse dma direction mapping case.

> This also avoids bloating the command with another function pointer
> or having to change all execute_cmd prototypes.

Indeed.

--nab



\
 
 \ /
  Last update: 2013-08-21 20:01    [W:0.054 / U:0.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site