On Thu, Mar 26, 2020 at 11:15:05PM +0100, David Disseldorp wrote:
> Reads and writes in the XCOPY loop are synchronous, so needn't be
> heap allocated / freed with each loop.
>
> Signed-off-by: David Disseldorp <ddiss@xxxxxxx>
Looks good:
Reviewed-by: Christoph Hellwig <hch@xxxxxx>
But a few cleanup opportunities if you don't mind a respin:
> - rc = target_xcopy_setup_pt_cmd(xpt_cmd, xop, src_dev, &cdb[0],
> + rc = target_xcopy_setup_pt_cmd(&xpt_cmd, xop, src_dev, &cdb[0],
> remote_port);
> if (rc < 0) {
> - ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status;
> + ec_cmd->scsi_status = se_cmd->scsi_status;
> transport_generic_free_cmd(se_cmd, 0);
> return rc;
> }
> @@ -603,13 +598,14 @@ static int target_xcopy_read_source(
> pr_debug("XCOPY-READ: Saved xop->xop_data_sg: %p, num: %u for READ"
> " memory\n", xop->xop_data_sg, xop->xop_data_nents);
>
> - rc = target_xcopy_issue_pt_cmd(xpt_cmd);
> + rc = target_xcopy_issue_pt_cmd(&xpt_cmd);
> if (rc < 0) {
> - ec_cmd->scsi_status = xpt_cmd->se_cmd.scsi_status;
> + ec_cmd->scsi_status = se_cmd->scsi_status;
> transport_generic_free_cmd(se_cmd, 0);
> return rc;
> }
>
> + transport_generic_free_cmd(se_cmd, 0);
> return 0;
I think this should use a common label to free the command. Same
for the write side.
> if (rc < 0) {
> - transport_generic_free_cmd(&xop->src_pt_cmd->se_cmd, 0);
> goto out;
> }
No need for the braces now.
