On Tue, 2012-04-03 at 15:51 -0700, Andy Grover wrote:
> We originally changed iscsi to allocate its own buffers just as an
> intermediate step to clean up some core buffer allocation mechanisms. Now
> we can put it back.
>
> Also had to change allocate_iovecs to use data_length instead of
> t_data_nents because iovecs are now allocated before the data buffer, thus
> t_data_nents is not yet initialized.
>
> Signed-off-by: Andy Grover <agrover@xxxxxxxxxx>
> ---
> drivers/target/iscsi/iscsi_target.c | 77 ++---------------------------
> drivers/target/iscsi/iscsi_target_core.h | 3 -
> drivers/target/iscsi/iscsi_target_util.c | 6 --
> 3 files changed, 6 insertions(+), 80 deletions(-)
>
The conversion back to using se_cmd->t_data_sg[] looks OK to me.
Thanks again Andy!
--nab
> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> index da4e779..37028da 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c
> @@ -686,9 +686,7 @@ int iscsit_add_reject_from_cmd(
>
> /*
> * Map some portion of the allocated scatterlist to an iovec, suitable for
> - * kernel sockets to copy data in/out. This handles both pages and slab-allocated
> - * buffers, since we have been tricky and mapped t_mem_sg to the buffer in
> - * either case (see iscsit_alloc_buffs)
> + * kernel sockets to copy data in/out.
> */
> static int iscsit_map_iovec(
> struct iscsi_cmd *cmd,
> @@ -701,10 +699,9 @@ static int iscsit_map_iovec(
> unsigned int page_off;
>
> /*
> - * We have a private mapping of the allocated pages in t_mem_sg.
> - * At this point, we also know each contains a page.
> + * We know each entry in t_data_sg contains a page.
> */
> - sg = &cmd->t_mem_sg[data_offset / PAGE_SIZE];
> + sg = &cmd->se_cmd.t_data_sg[data_offset / PAGE_SIZE];
> page_off = (data_offset % PAGE_SIZE);
>
> cmd->first_data_sg = sg;
> @@ -762,8 +759,7 @@ static void iscsit_ack_from_expstatsn(struct iscsi_conn *conn, u32 exp_statsn)
>
> static int iscsit_allocate_iovecs(struct iscsi_cmd *cmd)
> {
> - u32 iov_count = (cmd->se_cmd.t_data_nents == 0) ? 1 :
> - cmd->se_cmd.t_data_nents;
> + u32 iov_count = min(1UL, DIV_ROUND_UP(cmd->se_cmd.data_length, PAGE_SIZE));
>
> iov_count += ISCSI_IOV_DATA_BUFFER;
>
> @@ -777,64 +773,6 @@ static int iscsit_allocate_iovecs(struct iscsi_cmd *cmd)
> return 0;
> }
>
> -static int iscsit_alloc_buffs(struct iscsi_cmd *cmd)
> -{
> - struct scatterlist *sgl;
> - u32 length = cmd->se_cmd.data_length;
> - int nents = DIV_ROUND_UP(length, PAGE_SIZE);
> - int i = 0, j = 0, ret;
> - /*
> - * If no SCSI payload is present, allocate the default iovecs used for
> - * iSCSI PDU Header
> - */
> - if (!length)
> - return iscsit_allocate_iovecs(cmd);
> -
> - sgl = kzalloc(sizeof(*sgl) * nents, GFP_KERNEL);
> - if (!sgl)
> - return -ENOMEM;
> -
> - sg_init_table(sgl, nents);
> -
> - while (length) {
> - int buf_size = min_t(int, length, PAGE_SIZE);
> - struct page *page;
> -
> - page = alloc_page(GFP_KERNEL | __GFP_ZERO);
> - if (!page)
> - goto page_alloc_failed;
> -
> - sg_set_page(&sgl[i], page, buf_size, 0);
> -
> - length -= buf_size;
> - i++;
> - }
> -
> - cmd->t_mem_sg = sgl;
> - cmd->t_mem_sg_nents = nents;
> -
> - /* BIDI ops not supported */
> -
> - /* Tell the core about our preallocated memory */
> - transport_generic_map_mem_to_cmd(&cmd->se_cmd, sgl, nents, NULL, 0);
> - /*
> - * Allocate iovecs for SCSI payload after transport_generic_map_mem_to_cmd
> - * so that cmd->se_cmd.t_tasks_se_num has been set.
> - */
> - ret = iscsit_allocate_iovecs(cmd);
> - if (ret < 0)
> - return -ENOMEM;
> -
> - return 0;
> -
> -page_alloc_failed:
> - while (j < i)
> - __free_page(sg_page(&sgl[j++]));
> -
> - kfree(sgl);
> - return -ENOMEM;
> -}
> -
> static int iscsit_handle_scsi_cmd(
> struct iscsi_conn *conn,
> unsigned char *buf)
> @@ -1076,11 +1014,8 @@ attach_cmd:
> * Active/NonOptimized primary access state..
> */
> core_alua_check_nonop_delay(&cmd->se_cmd);
> - /*
> - * Allocate and setup SGL used with transport_generic_map_mem_to_cmd().
> - * also call iscsit_allocate_iovecs()
> - */
> - ret = iscsit_alloc_buffs(cmd);
> +
> + ret = iscsit_allocate_iovecs(cmd);
> if (ret < 0)
> return iscsit_add_reject_from_cmd(
> ISCSI_REASON_BOOKMARK_NO_RESOURCES,
> diff --git a/drivers/target/iscsi/iscsi_target_core.h b/drivers/target/iscsi/iscsi_target_core.h
> index 94c736e..1c70144 100644
> --- a/drivers/target/iscsi/iscsi_target_core.h
> +++ b/drivers/target/iscsi/iscsi_target_core.h
> @@ -468,9 +468,6 @@ struct iscsi_cmd {
> #define ISCSI_SENSE_BUFFER_LEN (TRANSPORT_SENSE_BUFFER + 2)
> unsigned char sense_buffer[ISCSI_SENSE_BUFFER_LEN];
>
> - struct scatterlist *t_mem_sg;
> - u32 t_mem_sg_nents;
> -
> u32 padding;
> u8 pad_bytes[4];
>
> diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
> index 9cd9f30..b42cdeb 100644
> --- a/drivers/target/iscsi/iscsi_target_util.c
> +++ b/drivers/target/iscsi/iscsi_target_util.c
> @@ -645,7 +645,6 @@ void iscsit_free_queue_reqs_for_conn(struct iscsi_conn *conn)
> void iscsit_release_cmd(struct iscsi_cmd *cmd)
> {
> struct iscsi_conn *conn = cmd->conn;
> - int i;
>
> iscsit_free_r2ts_from_list(cmd);
> iscsit_free_all_datain_reqs(cmd);
> @@ -656,11 +655,6 @@ void iscsit_release_cmd(struct iscsi_cmd *cmd)
> kfree(cmd->tmr_req);
> kfree(cmd->iov_data);
>
> - for (i = 0; i < cmd->t_mem_sg_nents; i++)
> - __free_page(sg_page(&cmd->t_mem_sg[i]));
> -
> - kfree(cmd->t_mem_sg);
> -
> if (conn) {
> iscsit_remove_cmd_from_immediate_queue(cmd, conn);
> iscsit_remove_cmd_from_response_queue(cmd, conn);
--
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
