Re: [PATCH 0/29] Random iscsi cleanups and refactoring

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2012-04-03 at 15:51 -0700, Andy Grover wrote: 
> Hi nab and all,
> 
> Here are some miscellaneous cleanups I've been working on, with the goal
> to get back to core-allocated buffers (done),

I'm still having a look at these, but they all seem reasonable thus far.
I'll reply inline with any specific concerns.

> and streamline the iscsi
> handle_cmd code so it can use target_submit_cmd() (made progress but not done
> yet.) 
> 

Mmmm, due to the requirements of Immediate Data and Unsolicited Data Out
for WRITE payloads, I'm not sure if a conversion of iscsi-target to
target_submit_cmd() in it's current form with a single call is going to
work as you expect here.

The reason we have historically needed the logical split of what is now
in target_submit_cmd() logic for iscsi-target is to decode the CDB and
perform memory allocation ahead of actually dispatching for backend
device execution of command in iSCSI per session CmdSN order.. (eg: the
command submission may not always happen on the same RX context it was
received upon)

The majority of these cases where the RX context execution may be
delayed are related to multiple connection (MC/S) operation, but
Immediate Data and Unsolicited Data Out does also have special
considerations (see iscsi_target_erl1.c:iscsit_execute_cmd) that I can
see having issues with a target_submit_cmd() conversion.

I've considered an iscsi-target conversion to target_submit_cmd() in the
past months, but AFAICT this would seem to cause more problems that it's
worth for all of the iscsi-target specific cases.  Of course, I'm happy
to be proved wrong on this, but I'd likely not merge a conversion patch
until these immediate data + MC/S special cases can addressed.

> There are also many other patches unrelated to the above goal in here,
> primarily shooting for less complex and more readable code, but not changing
> what it functionally does.
> 
> Code compiled and smoketested.
> 

Thanks Andy!

--nab

> 
> 
> Regards -- Andy
> 
> The following changes since commit 317b5476e52f2e7a4077ea259e2f0c093f07aca4:
> 
>   target: Remove transport_do_task_sg_chain() and associated detritus (2012-04-02 18:20:36 -0700)
> 
> are available in the git repository at:
>   git://fedorapeople.org/home/fedora/grover/public_git/linux-2.6.git for-nab
> 
> Andy Grover (29):
>       target/iscsi: Rename iscsi_cmd.i_list to iscsi_cmd.i_conn_node
>       target/iscsi: Use cmd->immediate_cmd for conditional
>       target/iscsi: Make iscist_dataout_post_crc_passed more legible
>       target/iscsi: use max() to reduce code in build_r2ts_for_cmd()
>       target/iscsi: Remove CONFIG_SMP and if 0 ifdefs
>       target/iscsi: Replace if/goto with a while loop
>       target/iscsi: remove conn->tx_immediate_queue and tc_response_queue
>       target/iscsi: Remove unneeded locking from iscsi_target_tx_thread
>       target/iscsi: Refactor target_tx_thread immediate queue loop
>       target/iscsi: Refactor target_tx_thread response queue loop
>       target/iscsi: Put immediate and response handling in separate functions
>       target/iscsi: Make iscsit_add_reject static
>       target/iscsi: Remove data_offset_end from iscsi_datain_req
>       target/iscsi: remove "#if 0"s
>       target: Eliminate "#if 0"s from core code
>       target/usb: Remove ifdeffed code
>       target/iscsi: Rename iscsi_datain_req to cmd_datain_node
>       target/iscsi: built_r2ts_for_cmd cleanups
>       target/iscsi: Cleanup build_sendtargets_reponse
>       target: Rename generic_allocate_tasks to generic_setup_cmd_from_cdb
>       target: rewrite comment for generic_new_cmd
>       target/iscsi: Inline iscsit_allocate_se_cmd and *_for_tmr
>       target/iscsi: Move init_se_cmd closer to lookup_cmd_lun
>       target/iscsi: Eliminate iscsi_cmd.data_length
>       target/iscsi: Fold _decide_list_to_build into _build_pdu_and_seq_lists
>       target/iscsi: Update some comments in pdu/seq code
>       target/iscsi: Do not touch se_cmd in initializing pdu and seq lists
>       target: Call core_alua_check_nonop_delay in target_submit_cmd()
>       target/iscsi: Go back to core allocating data buffer for cmd
> 
>  drivers/scsi/ibmvscsi/ibmvscsis.c                 |    2 +-
>  drivers/target/iscsi/iscsi_target.c               |  823 ++++++++++-----------
>  drivers/target/iscsi/iscsi_target.h               |    3 +-
>  drivers/target/iscsi/iscsi_target_configfs.c      |    2 +-
>  drivers/target/iscsi/iscsi_target_core.h          |   12 +-
>  drivers/target/iscsi/iscsi_target_datain_values.c |   35 +-
>  drivers/target/iscsi/iscsi_target_erl0.c          |   31 +-
>  drivers/target/iscsi/iscsi_target_erl1.c          |   23 +-
>  drivers/target/iscsi/iscsi_target_erl2.c          |   28 +-
>  drivers/target/iscsi/iscsi_target_parameters.c    |    8 -
>  drivers/target/iscsi/iscsi_target_seq_pdu_list.c  |  128 +++--
>  drivers/target/iscsi/iscsi_target_seq_pdu_list.h  |    4 +-
>  drivers/target/iscsi/iscsi_target_tmr.c           |   15 +-
>  drivers/target/iscsi/iscsi_target_util.c          |  192 +-----
>  drivers/target/iscsi/iscsi_target_util.h          |    3 -
>  drivers/target/loopback/tcm_loop.c                |    4 +-
>  drivers/target/target_core_file.c                 |   11 +-
>  drivers/target/target_core_pr.c                   |   31 +-
>  drivers/target/target_core_transport.c            |   44 +-
>  drivers/target/tcm_vhost/tcm_vhost_fabric.c       |    2 +-
>  drivers/target/tcm_vhost/tcm_vhost_scsi.c         |    2 +-
>  drivers/target/usb-gadget/bot.c                   |   46 +-
>  drivers/target/usb-gadget/fabric.c                |    2 +-
>  drivers/target/usb-gadget/uas.c                   |   32 +-
>  include/target/target_core_fabric.h               |    2 +-
>  25 files changed, 638 insertions(+), 847 deletions(-)
> 
> 
> --
> 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



--
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


[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux