From 0d0cf14c9bd2943ed5afd15df459f564d85eacde Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 13 Jun 2011 00:51:30 -0700 Subject: [PATCH] isci: cleanup request allocation Rather than return an error code and update a pointer that was passed by reference just return the request object directly (or null if allocation failed). Signed-off-by: Dan Williams --- drivers/scsi/isci/request.c | 197 ++++++++++++------------------------ drivers/scsi/isci/request.h | 11 +- drivers/scsi/isci/task.c | 136 +++++++++---------------- 3 files changed, 121 insertions(+), 223 deletions(-) diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c index 9d7531ad9a74..f0813d076c50 100644 --- a/drivers/scsi/isci/request.c +++ b/drivers/scsi/isci/request.c @@ -3510,172 +3510,110 @@ static enum sci_status isci_io_request_build( return SCI_SUCCESS; } -/** - * isci_request_alloc_core() - This function gets the request object from the - * isci_host dma cache. - * @isci_host: This parameter specifies the ISCI host object - * @isci_request: This parameter will contain the pointer to the new - * isci_request object. - * @isci_device: This parameter is the pointer to the isci remote device object - * that is the destination for this request. - * @gfp_flags: This parameter specifies the os allocation flags. - * - * SCI_SUCCESS on successfull completion, or specific failure code. - */ -static int isci_request_alloc_core( - struct isci_host *isci_host, - struct isci_request **isci_request, - struct isci_remote_device *isci_device, - gfp_t gfp_flags) +static struct isci_request *isci_request_alloc_core(struct isci_host *ihost, + struct isci_remote_device *idev, + gfp_t gfp_flags) { - int ret = 0; dma_addr_t handle; - struct isci_request *request; - + struct isci_request *ireq; - /* get pointer to dma memory. This actually points - * to both the isci_remote_device object and the - * sci object. The isci object is at the beginning - * of the memory allocated here. - */ - request = dma_pool_alloc(isci_host->dma_pool, gfp_flags, &handle); - if (!request) { - dev_warn(&isci_host->pdev->dev, + ireq = dma_pool_alloc(ihost->dma_pool, gfp_flags, &handle); + if (!ireq) { + dev_warn(&ihost->pdev->dev, "%s: dma_pool_alloc returned NULL\n", __func__); - return -ENOMEM; + return NULL; } /* initialize the request object. */ - spin_lock_init(&request->state_lock); - request->request_daddr = handle; - request->isci_host = isci_host; - request->isci_device = isci_device; - request->io_request_completion = NULL; - request->terminated = false; + spin_lock_init(&ireq->state_lock); + ireq->request_daddr = handle; + ireq->isci_host = ihost; + ireq->isci_device = idev; + ireq->io_request_completion = NULL; + ireq->terminated = false; - request->num_sg_entries = 0; + ireq->num_sg_entries = 0; - request->complete_in_target = false; + ireq->complete_in_target = false; - INIT_LIST_HEAD(&request->completed_node); - INIT_LIST_HEAD(&request->dev_node); + INIT_LIST_HEAD(&ireq->completed_node); + INIT_LIST_HEAD(&ireq->dev_node); - *isci_request = request; - isci_request_change_state(request, allocated); + isci_request_change_state(ireq, allocated); - return ret; + return ireq; } -static int isci_request_alloc_io( - struct isci_host *isci_host, - struct sas_task *task, - struct isci_request **isci_request, - struct isci_remote_device *isci_device, - gfp_t gfp_flags) +static struct isci_request *isci_request_alloc_io(struct isci_host *ihost, + struct sas_task *task, + struct isci_remote_device *idev, + gfp_t gfp_flags) { - int retval = isci_request_alloc_core(isci_host, isci_request, - isci_device, gfp_flags); - - if (!retval) { - (*isci_request)->ttype_ptr.io_task_ptr = task; - (*isci_request)->ttype = io_task; + struct isci_request *ireq; - task->lldd_task = *isci_request; + ireq = isci_request_alloc_core(ihost, idev, gfp_flags); + if (ireq) { + ireq->ttype_ptr.io_task_ptr = task; + ireq->ttype = io_task; + task->lldd_task = ireq; } - return retval; + return ireq; } -/** - * isci_request_alloc_tmf() - This function gets the request object from the - * isci_host dma cache and initializes the relevant fields as a sas_task. - * @isci_host: This parameter specifies the ISCI host object - * @sas_task: This parameter is the task struct from the upper layer driver. - * @isci_request: This parameter will contain the pointer to the new - * isci_request object. - * @isci_device: This parameter is the pointer to the isci remote device object - * that is the destination for this request. - * @gfp_flags: This parameter specifies the os allocation flags. - * - * SCI_SUCCESS on successfull completion, or specific failure code. - */ -int isci_request_alloc_tmf( - struct isci_host *isci_host, - struct isci_tmf *isci_tmf, - struct isci_request **isci_request, - struct isci_remote_device *isci_device, - gfp_t gfp_flags) +struct isci_request *isci_request_alloc_tmf(struct isci_host *ihost, + struct isci_tmf *isci_tmf, + struct isci_remote_device *idev, + gfp_t gfp_flags) { - int retval = isci_request_alloc_core(isci_host, isci_request, - isci_device, gfp_flags); - - if (!retval) { + struct isci_request *ireq; - (*isci_request)->ttype_ptr.tmf_task_ptr = isci_tmf; - (*isci_request)->ttype = tmf_task; + ireq = isci_request_alloc_core(ihost, idev, gfp_flags); + if (ireq) { + ireq->ttype_ptr.tmf_task_ptr = isci_tmf; + ireq->ttype = tmf_task; } - return retval; + return ireq; } -/** - * isci_request_execute() - This function allocates the isci_request object, - * all fills in some common fields. - * @isci_host: This parameter specifies the ISCI host object - * @sas_task: This parameter is the task struct from the upper layer driver. - * @isci_request: This parameter will contain the pointer to the new - * isci_request object. - * @gfp_flags: This parameter specifies the os allocation flags. - * - * SCI_SUCCESS on successfull completion, or specific failure code. - */ -int isci_request_execute( - struct isci_host *isci_host, - struct sas_task *task, - struct isci_request **isci_request, - gfp_t gfp_flags) +int isci_request_execute(struct isci_host *ihost, struct sas_task *task, + gfp_t gfp_flags) { - int ret = 0; - struct scic_sds_remote_device *sci_device; enum sci_status status = SCI_FAILURE_UNSUPPORTED_PROTOCOL; - struct isci_remote_device *isci_device; - struct isci_request *request; + struct scic_sds_remote_device *sci_dev; + struct isci_remote_device *idev; + struct isci_request *ireq; unsigned long flags; + int ret = 0; - isci_device = task->dev->lldd_dev; - sci_device = &isci_device->sci; + idev = task->dev->lldd_dev; + sci_dev = &idev->sci; /* do common allocation and init of request object. */ - ret = isci_request_alloc_io( - isci_host, - task, - &request, - isci_device, - gfp_flags - ); - - if (ret) + ireq = isci_request_alloc_io(ihost, task, idev, gfp_flags); + if (!ireq) goto out; - status = isci_io_request_build(isci_host, request, isci_device); + status = isci_io_request_build(ihost, ireq, idev); if (status != SCI_SUCCESS) { - dev_warn(&isci_host->pdev->dev, + dev_warn(&ihost->pdev->dev, "%s: request_construct failed - status = 0x%x\n", __func__, status); goto out; } - spin_lock_irqsave(&isci_host->scic_lock, flags); + spin_lock_irqsave(&ihost->scic_lock, flags); /* send the request, let the core assign the IO TAG. */ - status = scic_controller_start_io(&isci_host->sci, sci_device, - &request->sci, + status = scic_controller_start_io(&ihost->sci, sci_dev, + &ireq->sci, SCI_CONTROLLER_INVALID_IO_TAG); if (status != SCI_SUCCESS && status != SCI_FAILURE_REMOTE_DEVICE_RESET_REQUIRED) { - dev_warn(&isci_host->pdev->dev, + dev_warn(&ihost->pdev->dev, "%s: failed request start (0x%x)\n", __func__, status); - spin_unlock_irqrestore(&isci_host->scic_lock, flags); + spin_unlock_irqrestore(&ihost->scic_lock, flags); goto out; } @@ -3687,21 +3625,21 @@ int isci_request_execute( * Update it's status and add it to the list in the * remote device object. */ - list_add(&request->dev_node, &isci_device->reqs_in_process); + list_add(&ireq->dev_node, &idev->reqs_in_process); if (status == SCI_SUCCESS) { /* Save the tag for possible task mgmt later. */ - request->io_tag = request->sci.io_tag; - isci_request_change_state(request, started); + ireq->io_tag = ireq->sci.io_tag; + isci_request_change_state(ireq, started); } else { /* The request did not really start in the * hardware, so clear the request handle * here so no terminations will be done. */ - request->terminated = true; - isci_request_change_state(request, completed); + ireq->terminated = true; + isci_request_change_state(ireq, completed); } - spin_unlock_irqrestore(&isci_host->scic_lock, flags); + spin_unlock_irqrestore(&ihost->scic_lock, flags); if (status == SCI_FAILURE_REMOTE_DEVICE_RESET_REQUIRED) { @@ -3716,7 +3654,7 @@ int isci_request_execute( /* Cause this task to be scheduled in the SCSI error * handler thread. */ - isci_execpath_callback(isci_host, task, + isci_execpath_callback(ihost, task, sas_task_abort); /* Change the status, since we are holding @@ -3729,11 +3667,10 @@ int isci_request_execute( out: if (status != SCI_SUCCESS) { /* release dma memory on failure. */ - isci_request_free(isci_host, request); - request = NULL; + isci_request_free(ihost, ireq); + ireq = NULL; ret = SCI_FAILURE; } - *isci_request = request; return ret; } diff --git a/drivers/scsi/isci/request.h b/drivers/scsi/isci/request.h index ac9368c5a6b5..8de2542f081f 100644 --- a/drivers/scsi/isci/request.h +++ b/drivers/scsi/isci/request.h @@ -679,16 +679,13 @@ static inline void isci_request_free(struct isci_host *isci_host, #define isci_request_access_tmf(req) ((req)->ttype_ptr.tmf_task_ptr) -int isci_request_alloc_tmf(struct isci_host *isci_host, - struct isci_tmf *isci_tmf, - struct isci_request **isci_request, - struct isci_remote_device *isci_device, - gfp_t gfp_flags); - +struct isci_request *isci_request_alloc_tmf(struct isci_host *ihost, + struct isci_tmf *isci_tmf, + struct isci_remote_device *idev, + gfp_t gfp_flags); int isci_request_execute(struct isci_host *isci_host, struct sas_task *task, - struct isci_request **request, gfp_t gfp_flags); /** diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c index 01032dc2c116..ded81cd1a781 100644 --- a/drivers/scsi/isci/task.c +++ b/drivers/scsi/isci/task.c @@ -146,7 +146,6 @@ static void isci_task_refuse(struct isci_host *ihost, struct sas_task *task, int isci_task_execute_task(struct sas_task *task, int num, gfp_t gfp_flags) { struct isci_host *ihost = dev_to_ihost(task->dev); - struct isci_request *request = NULL; struct isci_remote_device *device; unsigned long flags; int ret; @@ -226,8 +225,7 @@ int isci_task_execute_task(struct sas_task *task, int num, gfp_t gfp_flags) spin_unlock_irqrestore(&task->task_state_lock, flags); /* build and send the request. */ - status = isci_request_execute(ihost, task, &request, - gfp_flags); + status = isci_request_execute(ihost, task, gfp_flags); if (status != SCI_SUCCESS) { @@ -254,54 +252,34 @@ int isci_task_execute_task(struct sas_task *task, int num, gfp_t gfp_flags) return 0; } - - -/** - * isci_task_request_build() - This function builds the task request object. - * @isci_host: This parameter specifies the ISCI host object - * @request: This parameter points to the isci_request object allocated in the - * request construct function. - * @tmf: This parameter is the task management struct to be built - * - * SCI_SUCCESS on successfull completion, or specific failure code. - */ -static enum sci_status isci_task_request_build( - struct isci_host *isci_host, - struct isci_request **isci_request, - struct isci_tmf *isci_tmf) +static struct isci_request *isci_task_request_build(struct isci_host *ihost, + struct isci_tmf *isci_tmf) { - struct scic_sds_remote_device *sci_device; + struct scic_sds_remote_device *sci_dev; enum sci_status status = SCI_FAILURE; - struct isci_request *request = NULL; - struct isci_remote_device *isci_device; + struct isci_request *ireq = NULL; + struct isci_remote_device *idev; struct domain_device *dev; - dev_dbg(&isci_host->pdev->dev, + dev_dbg(&ihost->pdev->dev, "%s: isci_tmf = %p\n", __func__, isci_tmf); - isci_device = isci_tmf->device; - sci_device = &isci_device->sci; - dev = isci_device->domain_dev; + idev = isci_tmf->device; + sci_dev = &idev->sci; + dev = idev->domain_dev; /* do common allocation and init of request object. */ - status = isci_request_alloc_tmf( - isci_host, - isci_tmf, - &request, - isci_device, - GFP_ATOMIC - ); - - if (status != SCI_SUCCESS) - goto out; + ireq = isci_request_alloc_tmf(ihost, isci_tmf, idev, GFP_ATOMIC); + if (!ireq) + return NULL; /* let the core do it's construct. */ - status = scic_task_request_construct(&isci_host->sci, sci_device, + status = scic_task_request_construct(&ihost->sci, sci_dev, SCI_CONTROLLER_INVALID_IO_TAG, - &request->sci); + &ireq->sci); if (status != SCI_SUCCESS) { - dev_warn(&isci_host->pdev->dev, + dev_warn(&ihost->pdev->dev, "%s: scic_task_request_construct failed - " "status = 0x%x\n", __func__, @@ -312,30 +290,23 @@ static enum sci_status isci_task_request_build( /* XXX convert to get this from task->tproto like other drivers */ if (dev->dev_type == SAS_END_DEV) { isci_tmf->proto = SAS_PROTOCOL_SSP; - status = scic_task_request_construct_ssp(&request->sci); + status = scic_task_request_construct_ssp(&ireq->sci); if (status != SCI_SUCCESS) goto errout; } if (dev->dev_type == SATA_DEV || (dev->tproto & SAS_PROTOCOL_STP)) { isci_tmf->proto = SAS_PROTOCOL_SATA; - status = isci_sata_management_task_request_build(request); + status = isci_sata_management_task_request_build(ireq); if (status != SCI_SUCCESS) goto errout; } - - goto out; - + return ireq; errout: - - /* release the dma memory if we fail. */ - isci_request_free(isci_host, request); - request = NULL; - - out: - *isci_request = request; - return status; + isci_request_free(ihost, ireq); + ireq = NULL; + return ireq; } /** @@ -350,16 +321,14 @@ static enum sci_status isci_task_request_build( * TMF_RESP_FUNC_COMPLETE on successful completion of the TMF (this includes * error conditions reported in the IU status), or TMF_RESP_FUNC_FAILED. */ -int isci_task_execute_tmf( - struct isci_host *isci_host, - struct isci_tmf *tmf, - unsigned long timeout_ms) +int isci_task_execute_tmf(struct isci_host *ihost, struct isci_tmf *tmf, + unsigned long timeout_ms) { DECLARE_COMPLETION_ONSTACK(completion); enum sci_task_status status = SCI_TASK_FAILURE; struct scic_sds_remote_device *sci_device; struct isci_remote_device *isci_device = tmf->device; - struct isci_request *request; + struct isci_request *ireq; int ret = TMF_RESP_FUNC_FAILED; unsigned long flags; unsigned long timeleft; @@ -368,13 +337,13 @@ int isci_task_execute_tmf( * if the device is not there and ready. */ if (!isci_device || isci_device->status != isci_ready_for_io) { - dev_dbg(&isci_host->pdev->dev, + dev_dbg(&ihost->pdev->dev, "%s: isci_device = %p not ready (%d)\n", __func__, isci_device, isci_device->status); return TMF_RESP_FUNC_FAILED; } else - dev_dbg(&isci_host->pdev->dev, + dev_dbg(&ihost->pdev->dev, "%s: isci_device = %p\n", __func__, isci_device); @@ -383,64 +352,59 @@ int isci_task_execute_tmf( /* Assign the pointer to the TMF's completion kernel wait structure. */ tmf->complete = &completion; - isci_task_request_build( - isci_host, - &request, - tmf - ); - - if (!request) { - dev_warn(&isci_host->pdev->dev, + ireq = isci_task_request_build(ihost, tmf); + if (!ireq) { + dev_warn(&ihost->pdev->dev, "%s: isci_task_request_build failed\n", __func__); return TMF_RESP_FUNC_FAILED; } - spin_lock_irqsave(&isci_host->scic_lock, flags); + spin_lock_irqsave(&ihost->scic_lock, flags); /* start the TMF io. */ status = scic_controller_start_task( - &isci_host->sci, + &ihost->sci, sci_device, - &request->sci, + &ireq->sci, SCI_CONTROLLER_INVALID_IO_TAG); if (status != SCI_TASK_SUCCESS) { - dev_warn(&isci_host->pdev->dev, + dev_warn(&ihost->pdev->dev, "%s: start_io failed - status = 0x%x, request = %p\n", __func__, status, - request); - spin_unlock_irqrestore(&isci_host->scic_lock, flags); + ireq); + spin_unlock_irqrestore(&ihost->scic_lock, flags); goto cleanup_request; } if (tmf->cb_state_func != NULL) tmf->cb_state_func(isci_tmf_started, tmf, tmf->cb_data); - isci_request_change_state(request, started); + isci_request_change_state(ireq, started); /* add the request to the remote device request list. */ - list_add(&request->dev_node, &isci_device->reqs_in_process); + list_add(&ireq->dev_node, &isci_device->reqs_in_process); - spin_unlock_irqrestore(&isci_host->scic_lock, flags); + spin_unlock_irqrestore(&ihost->scic_lock, flags); /* Wait for the TMF to complete, or a timeout. */ timeleft = wait_for_completion_timeout(&completion, jiffies + msecs_to_jiffies(timeout_ms)); if (timeleft == 0) { - spin_lock_irqsave(&isci_host->scic_lock, flags); + spin_lock_irqsave(&ihost->scic_lock, flags); if (tmf->cb_state_func != NULL) tmf->cb_state_func(isci_tmf_timed_out, tmf, tmf->cb_data); status = scic_controller_terminate_request( - &request->isci_host->sci, - &request->isci_device->sci, - &request->sci); + &ireq->isci_host->sci, + &ireq->isci_device->sci, + &ireq->sci); - spin_unlock_irqrestore(&isci_host->scic_lock, flags); + spin_unlock_irqrestore(&ihost->scic_lock, flags); } isci_print_tmf(tmf); @@ -448,7 +412,7 @@ int isci_task_execute_tmf( if (tmf->status == SCI_SUCCESS) ret = TMF_RESP_FUNC_COMPLETE; else if (tmf->status == SCI_FAILURE_IO_RESPONSE_VALID) { - dev_dbg(&isci_host->pdev->dev, + dev_dbg(&ihost->pdev->dev, "%s: tmf.status == " "SCI_FAILURE_IO_RESPONSE_VALID\n", __func__); @@ -456,18 +420,18 @@ int isci_task_execute_tmf( } /* Else - leave the default "failed" status alone. */ - dev_dbg(&isci_host->pdev->dev, + dev_dbg(&ihost->pdev->dev, "%s: completed request = %p\n", __func__, - request); + ireq); - if (request->io_request_completion != NULL) { + if (ireq->io_request_completion != NULL) { /* A thread is waiting for this TMF to finish. */ - complete(request->io_request_completion); + complete(ireq->io_request_completion); } cleanup_request: - isci_request_free(isci_host, request); + isci_request_free(ihost, ireq); return ret; } -- 2.20.1