Skip to content

Commit

Permalink
scsi: target: avoid per-loop XCOPY buffer allocations
Browse files Browse the repository at this point in the history 
The main target_xcopy_do_work() loop unnecessarily allocates an I/O buffer
with each synchronous READ / WRITE pair. This commit significantly reduces
allocations by reusing the XCOPY I/O buffer when possible.

Link: https://lore.kernel.org/r/20200327141954.955-4-ddiss@suse.de
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: David Disseldorp <ddiss@suse.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
  • Loading branch information
David Disseldorp authored and Martin K. Petersen committed Mar 29, 2020
1 parent 267fc83 commit 0ad0899
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 66 deletions.
96 changes: 30 additions & 66 deletions drivers/target/target_core_xcopy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,6 @@ void target_xcopy_release_pt(void)
* @cdb: SCSI CDB to be copied into @xpt_cmd.
* @remote_port: If false, use the LUN through which the XCOPY command has
* been received. If true, use @se_dev->xcopy_lun.
* @alloc_mem: Whether or not to allocate an SGL list.
*
* Set up a SCSI command (READ or WRITE) that will be used to execute an
* XCOPY command.
Expand All @@ -509,12 +508,9 @@ static int target_xcopy_setup_pt_cmd(
struct xcopy_op *xop,
struct se_device *se_dev,
unsigned char *cdb,
bool remote_port,
bool alloc_mem)
bool remote_port)
{
struct se_cmd *cmd = &xpt_cmd->se_cmd;
sense_reason_t sense_rc;
int ret = 0, rc;

/*
* Setup LUN+port to honor reservations based upon xop->op_origin for
Expand All @@ -530,46 +526,17 @@ static int target_xcopy_setup_pt_cmd(
cmd->se_cmd_flags |= SCF_SE_LUN_CMD;

cmd->tag = 0;
sense_rc = target_setup_cmd_from_cdb(cmd, cdb);
if (sense_rc) {
ret = -EINVAL;
goto out;
}
if (target_setup_cmd_from_cdb(cmd, cdb))
return -EINVAL;

if (alloc_mem) {
rc = target_alloc_sgl(&cmd->t_data_sg, &cmd->t_data_nents,
cmd->data_length, false, false);
if (rc < 0) {
ret = rc;
goto out;
}
/*
* Set this bit so that transport_free_pages() allows the
* caller to release SGLs + physical memory allocated by
* transport_generic_get_mem()..
*/
cmd->se_cmd_flags |= SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC;
} else {
/*
* Here the previously allocated SGLs for the internal READ
* are mapped zero-copy to the internal WRITE.
*/
sense_rc = transport_generic_map_mem_to_cmd(cmd,
xop->xop_data_sg, xop->xop_data_nents,
NULL, 0);
if (sense_rc) {
ret = -EINVAL;
goto out;
}
if (transport_generic_map_mem_to_cmd(cmd, xop->xop_data_sg,
xop->xop_data_nents, NULL, 0))
return -EINVAL;

pr_debug("Setup PASSTHROUGH_NOALLOC t_data_sg: %p t_data_nents:"
" %u\n", cmd->t_data_sg, cmd->t_data_nents);
}
pr_debug("Setup PASSTHROUGH_NOALLOC t_data_sg: %p t_data_nents:"
" %u\n", cmd->t_data_sg, cmd->t_data_nents);

return 0;

out:
return ret;
}

static int target_xcopy_issue_pt_cmd(struct xcopy_pt_cmd *xpt_cmd)
Expand Down Expand Up @@ -626,15 +593,13 @@ static int target_xcopy_read_source(
xop->src_pt_cmd = xpt_cmd;

rc = target_xcopy_setup_pt_cmd(xpt_cmd, xop, src_dev, &cdb[0],
remote_port, true);
remote_port);
if (rc < 0) {
ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status;
transport_generic_free_cmd(se_cmd, 0);
return rc;
}

xop->xop_data_sg = se_cmd->t_data_sg;
xop->xop_data_nents = se_cmd->t_data_nents;
pr_debug("XCOPY-READ: Saved xop->xop_data_sg: %p, num: %u for READ"
" memory\n", xop->xop_data_sg, xop->xop_data_nents);

Expand All @@ -644,12 +609,6 @@ static int target_xcopy_read_source(
transport_generic_free_cmd(se_cmd, 0);
return rc;
}
/*
* Clear off the allocated t_data_sg, that has been saved for
* zero-copy WRITE submission reuse in struct xcopy_op..
*/
se_cmd->t_data_sg = NULL;
se_cmd->t_data_nents = 0;

return 0;
}
Expand Down Expand Up @@ -688,27 +647,16 @@ static int target_xcopy_write_destination(
xop->dst_pt_cmd = xpt_cmd;

rc = target_xcopy_setup_pt_cmd(xpt_cmd, xop, dst_dev, &cdb[0],
remote_port, false);
remote_port);
if (rc < 0) {
struct se_cmd *src_cmd = &xop->src_pt_cmd->se_cmd;
ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status;
/*
* If the failure happened before the t_mem_list hand-off in
* target_xcopy_setup_pt_cmd(), Reset memory + clear flag so that
* core releases this memory on error during X-COPY WRITE I/O.
*/
src_cmd->se_cmd_flags &= ~SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC;
src_cmd->t_data_sg = xop->xop_data_sg;
src_cmd->t_data_nents = xop->xop_data_nents;

transport_generic_free_cmd(se_cmd, 0);
return rc;
}

rc = target_xcopy_issue_pt_cmd(xpt_cmd);
if (rc < 0) {
ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status;
se_cmd->se_cmd_flags &= ~SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC;
transport_generic_free_cmd(se_cmd, 0);
return rc;
}
Expand All @@ -724,7 +672,7 @@ static void target_xcopy_do_work(struct work_struct *work)
sector_t src_lba, dst_lba, end_lba;
unsigned int max_sectors;
int rc = 0;
unsigned short nolb, cur_nolb, max_nolb, copied_nolb = 0;
unsigned short nolb, max_nolb, copied_nolb = 0;

if (target_parse_xcopy_cmd(xop) != TCM_NO_SENSE)
goto err_free;
Expand Down Expand Up @@ -754,7 +702,23 @@ static void target_xcopy_do_work(struct work_struct *work)
(unsigned long long)src_lba, (unsigned long long)dst_lba);

while (src_lba < end_lba) {
cur_nolb = min(nolb, max_nolb);
unsigned short cur_nolb = min(nolb, max_nolb);
u32 cur_bytes = cur_nolb * src_dev->dev_attrib.block_size;

if (cur_bytes != xop->xop_data_bytes) {
/*
* (Re)allocate a buffer large enough to hold the XCOPY
* I/O size, which can be reused each read / write loop.
*/
target_free_sgl(xop->xop_data_sg, xop->xop_data_nents);
rc = target_alloc_sgl(&xop->xop_data_sg,
&xop->xop_data_nents,
cur_bytes,
false, false);
if (rc < 0)
goto out;
xop->xop_data_bytes = cur_bytes;
}

pr_debug("target_xcopy_do_work: Calling read src_dev: %p src_lba: %llu,"
" cur_nolb: %hu\n", src_dev, (unsigned long long)src_lba, cur_nolb);
Expand Down Expand Up @@ -785,12 +749,11 @@ static void target_xcopy_do_work(struct work_struct *work)
nolb -= cur_nolb;

transport_generic_free_cmd(&xop->src_pt_cmd->se_cmd, 0);
xop->dst_pt_cmd->se_cmd.se_cmd_flags &= ~SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC;

transport_generic_free_cmd(&xop->dst_pt_cmd->se_cmd, 0);
}

xcopy_pt_undepend_remotedev(xop);
target_free_sgl(xop->xop_data_sg, xop->xop_data_nents);
kfree(xop);

pr_debug("target_xcopy_do_work: Final src_lba: %llu, dst_lba: %llu\n",
Expand All @@ -804,6 +767,7 @@ static void target_xcopy_do_work(struct work_struct *work)

out:
xcopy_pt_undepend_remotedev(xop);
target_free_sgl(xop->xop_data_sg, xop->xop_data_nents);

err_free:
kfree(xop);
Expand Down
1 change: 1 addition & 0 deletions drivers/target/target_core_xcopy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ struct xcopy_op {
struct xcopy_pt_cmd *src_pt_cmd;
struct xcopy_pt_cmd *dst_pt_cmd;

u32 xop_data_bytes;
u32 xop_data_nents;
struct scatterlist *xop_data_sg;
struct work_struct xop_work;
Expand Down

0 comments on commit 0ad0899

Please sign in to comment.