From fe9a643157747cf85ecc07cd341e448c5849364f Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 3 Mar 2011 14:58:11 -0800 Subject: [PATCH] isci: pad stp and smp request sizes MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Ross says: "The memory allocation for these requests doesn’t take into account the additional memory needed when the code in scic_sds_s[mst]p_request_assign_buffers() shifts the struct scu_task_context so that it is cache line aligned: In an example from my machine, total buffer that I’ve given to SCIC goes from 0x410024566f84 to 0x410024567308. From this same example, this call shifts my task_context_buffer from 0x410024567208 to 0x410024567240. This means that the task_context_buffer that used to range from 0x410024567208 to 0x410024567308 instead now goes from 0x410024567240 to 0x410024567340. When the memset() call at the end of scic_task_request_construct() clears out this task_context_buffer, it does so from 0x410024567240 to 0x410024567340, effectively killing whatever buffer follows this allocation in memory." djbw: Use the kernel's PTR_ALIGN instead of scic_sds_request_align_task_context_buffer() and SMP_CACHE_BYTES instead of the local CACHE_LINE_SIZE definition. TODO: These allocations really want to be better defined in a union rather than opaque buffers carved up by macros. Reported-by: Ross Zwisler Signed-off-by: Jacek Danecki Signed-off-by: Dan Williams --- drivers/scsi/isci/core/scic_sds_request.c | 10 +++-- drivers/scsi/isci/core/scic_sds_request.h | 22 ----------- drivers/scsi/isci/core/scic_sds_smp_request.c | 5 ++- drivers/scsi/isci/core/scic_sds_stp_request.c | 38 ++++++++----------- 4 files changed, 24 insertions(+), 51 deletions(-) diff --git a/drivers/scsi/isci/core/scic_sds_request.c b/drivers/scsi/isci/core/scic_sds_request.c index 7b9ce1e878a7..00bebb9c0692 100644 --- a/drivers/scsi/isci/core/scic_sds_request.c +++ b/drivers/scsi/isci/core/scic_sds_request.c @@ -222,7 +222,7 @@ static u32 scic_sds_ssp_request_get_object_size(void) return sizeof(struct scic_sds_request) + scic_ssp_io_request_get_object_size() + sizeof(struct scu_task_context) - + CACHE_LINE_SIZE + + SMP_CACHE_BYTES + sizeof(struct scu_sgl_element_pair) * SCU_MAX_SGL_ELEMENT_PAIRS; } @@ -395,13 +395,15 @@ static void scic_sds_ssp_io_request_assign_buffers( this_request->sgl_element_pair_buffer = scic_sds_ssp_request_get_sgl_element_buffer(this_request); this_request->sgl_element_pair_buffer = - scic_sds_request_align_sgl_element_buffer(this_request->sgl_element_pair_buffer); + PTR_ALIGN(this_request->sgl_element_pair_buffer, + sizeof(struct scu_sgl_element_pair)); if (this_request->was_tag_assigned_by_user == false) { this_request->task_context_buffer = scic_sds_ssp_request_get_task_context_buffer(this_request); this_request->task_context_buffer = - scic_sds_request_align_task_context_buffer(this_request->task_context_buffer); + PTR_ALIGN(this_request->task_context_buffer, + SMP_CACHE_BYTES); } } @@ -638,7 +640,7 @@ static void scic_sds_ssp_task_request_assign_buffers( this_request->task_context_buffer = scic_sds_ssp_task_request_get_task_context_buffer(this_request); this_request->task_context_buffer = - scic_sds_request_align_task_context_buffer(this_request->task_context_buffer); + PTR_ALIGN(this_request->task_context_buffer, SMP_CACHE_BYTES); } } diff --git a/drivers/scsi/isci/core/scic_sds_request.h b/drivers/scsi/isci/core/scic_sds_request.h index 19b6feef373e..06b53c3b0aa0 100644 --- a/drivers/scsi/isci/core/scic_sds_request.h +++ b/drivers/scsi/isci/core/scic_sds_request.h @@ -313,28 +313,6 @@ extern const struct scic_sds_io_request_state_handler scic_sds_smp_request_start #define scic_sds_request_get_task_context(request) \ ((request)->task_context_buffer) -#define CACHE_LINE_SIZE (64) -#define scic_sds_request_align_task_context_buffer(address) \ - ((struct scu_task_context *)(\ - (((unsigned long)(address)) + (CACHE_LINE_SIZE - 1)) \ - & ~(CACHE_LINE_SIZE - 1) \ - )) - -/** - * scic_sds_request_align_sgl_element_buffer() - - * - * This macro will align the memory address so that it is correct for the SCU - * hardware to DMA the SGL element pairs. - */ -#define scic_sds_request_align_sgl_element_buffer(address) \ - ((struct scu_sgl_element_pair *)(\ - ((char *)(address)) \ - + (\ - ((~(unsigned long)(address)) + 1) \ - & (sizeof(struct scu_sgl_element_pair) - 1) \ - ) \ - )) - /** * scic_sds_request_set_status() - * diff --git a/drivers/scsi/isci/core/scic_sds_smp_request.c b/drivers/scsi/isci/core/scic_sds_smp_request.c index 85c890630d60..962bd3994f10 100644 --- a/drivers/scsi/isci/core/scic_sds_smp_request.c +++ b/drivers/scsi/isci/core/scic_sds_smp_request.c @@ -80,7 +80,8 @@ u32 scic_sds_smp_request_get_object_size(void) return sizeof(struct scic_sds_request) + sizeof(struct smp_request) + sizeof(struct smp_response) - + sizeof(struct scu_task_context); + + sizeof(struct scu_task_context) + + SMP_CACHE_BYTES; } /** @@ -137,7 +138,7 @@ void scic_sds_smp_request_assign_buffers( this_request->task_context_buffer = scic_sds_smp_request_get_task_context_buffer(this_request); this_request->task_context_buffer = - scic_sds_request_align_task_context_buffer(this_request->task_context_buffer); + PTR_ALIGN(this_request->task_context_buffer, SMP_CACHE_BYTES); } } diff --git a/drivers/scsi/isci/core/scic_sds_stp_request.c b/drivers/scsi/isci/core/scic_sds_stp_request.c index 0f17a28dd196..8da309f81ac2 100644 --- a/drivers/scsi/isci/core/scic_sds_stp_request.c +++ b/drivers/scsi/isci/core/scic_sds_stp_request.c @@ -131,33 +131,25 @@ u32 scic_sds_stp_request_get_object_size(void) + sizeof(struct sata_fis_reg_h2d) + sizeof(struct sata_fis_reg_d2h) + sizeof(struct scu_task_context) + + SMP_CACHE_BYTES + sizeof(struct scu_sgl_element_pair) * SCU_MAX_SGL_ELEMENT_PAIRS; } -/** - * - * - * - */ -void scic_sds_stp_request_assign_buffers( - struct scic_sds_request *request) +void scic_sds_stp_request_assign_buffers(struct scic_sds_request *sci_req) { - struct scic_sds_stp_request *this_request = (struct scic_sds_stp_request *)request; - - this_request->parent.command_buffer = - scic_sds_stp_request_get_h2d_reg_buffer(this_request); - this_request->parent.response_buffer = - scic_sds_stp_request_get_response_buffer(this_request); - this_request->parent.sgl_element_pair_buffer = - scic_sds_stp_request_get_sgl_element_buffer(this_request); - this_request->parent.sgl_element_pair_buffer = - scic_sds_request_align_sgl_element_buffer(this_request->parent.sgl_element_pair_buffer); - - if (this_request->parent.was_tag_assigned_by_user == false) { - this_request->parent.task_context_buffer = - scic_sds_stp_request_get_task_context_buffer(this_request); - this_request->parent.task_context_buffer = - scic_sds_request_align_task_context_buffer(this_request->parent.task_context_buffer); + struct scic_sds_stp_request *stp_req = container_of(sci_req, typeof(*stp_req), parent); + + sci_req->command_buffer = scic_sds_stp_request_get_h2d_reg_buffer(stp_req); + sci_req->response_buffer = scic_sds_stp_request_get_response_buffer(stp_req); + sci_req->sgl_element_pair_buffer = scic_sds_stp_request_get_sgl_element_buffer(stp_req); + sci_req->sgl_element_pair_buffer = PTR_ALIGN(sci_req->sgl_element_pair_buffer, + sizeof(struct scu_sgl_element_pair)); + + if (sci_req->was_tag_assigned_by_user == false) { + sci_req->task_context_buffer = + scic_sds_stp_request_get_task_context_buffer(stp_req); + sci_req->task_context_buffer = PTR_ALIGN(sci_req->task_context_buffer, + SMP_CACHE_BYTES); } } -- 2.30.2