Hi Varun, On Sun, 2017-05-07 at 18:22 +0530, Varun Prakash wrote: > On Mon, May 01, 2017 at 09:33:31PM -0700, Nicholas A. Bellinger wrote: <SNIP> > > Thanks for the heads up. Dropping this patch for now. > > > > To address this, AFAICT the cleanest approach is to simply do the > > unmapping of DDP SGLs currently done by cxgbit_release_cmd(), directly > > from a cxgbit specific iscsit_transport->iscsit_queue_rsp() callback. > > > > That way, we can unmap the DDP SGLs immediately before invoking > > iscsit_queue_rsp() from a cxgbit specific handler, and drop the > > iscsi_transport->iscsit_release_cmd() callback alltogether. > > > > WDYT..? > > This approach will work in success case, but in failure > cases(digest errors, ...) ->iscsit_queue_status() is not called so dma umap > will not happen. Indeed. Looking at other hw drivers like qla2xxx that have this same requirement, they do *_unmap_sg() once a completion interrupt has triggered, but before target_core_fabric_ops->release_cmd() is invoked and se_cmd->t_task_sg has already been freed. So I'm open to adding a target_core_fabric_ops->unmap_sg() to do this ahead of target_core_transport.c:transport_free_pages(). That said, snother option is to perform *_unmap_sg() internally in cxgbit once DDP completion for WRITEs has completed, but before it's submitted into iscsi_target -> target_core. AFAICT from a quick scan of cxgbit code, the two scenarios for this would be: *) For ISCSI_OP_SCSI_CMD with immediate data, once cxgbit_get_immediate_data() is invoked. Either for all cases when this is invoked (eg: does the DDP SGLs both immediate, unsolicited and solicited data when mapped..?), or only when iscsi_cmd->immediate_data = 1 && iscsi_cmd_flags & ICF_GOT_LAST_DATAOUT is true. *) For ISCSI_OP_SCSI_DATA_OUT with FINAL_BIT set, before iscsit_check_dataout_payload() is called to invoke target_execute_cmd() WDYT..?