Message ID | 1508227542-13165-4-git-send-email-chenhc@lemote.com (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
Hi Huacai, On 2017-10-17 10:05, Huacai Chen wrote: > In non-coherent DMA mode, kernel uses cache flushing operations to > maintain I/O coherency, so libsas's SMP request/response should be > aligned to ARCH_DMA_MINALIGN. Otherwise, If a DMA buffer and a kernel > structure share a same cache line, and if the kernel structure has > dirty data, cache_invalidate (no writeback) will cause data corruption. > > Cc: stable@vger.kernel.org > Signed-off-by: Huacai Chen <chenhc@lemote.com> > --- > drivers/scsi/libsas/sas_expander.c | 93 +++++++++++++++++++++++--------------- > 1 file changed, 57 insertions(+), 36 deletions(-) > > diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c > index 6b4fd23..124a44b 100644 > --- a/drivers/scsi/libsas/sas_expander.c > +++ b/drivers/scsi/libsas/sas_expander.c > @@ -164,17 +164,17 @@ static int smp_execute_task(struct domain_device *dev, void *req, int req_size, > > /* ---------- Allocations ---------- */ > > -static inline void *alloc_smp_req(int size) > +static inline void *alloc_smp_req(int size, int align) > { > - u8 *p = kzalloc(size, GFP_KERNEL); > + u8 *p = kzalloc(ALIGN(size, align), GFP_KERNEL); If I remember correctly, kernel guarantees that each kmalloced buffer is always at least aligned to the CPU cache line, so CPU cache can be invalidated on the allocated buffer without corrupting anything else. Taking this into account, I wonder if the above change make sense. Have you experienced any problems without this change? > if (p) > p[0] = SMP_REQUEST; > return p; > } > > -static inline void *alloc_smp_resp(int size) > +static inline void *alloc_smp_resp(int size, int align) > { > - return kzalloc(size, GFP_KERNEL); > + return kzalloc(ALIGN(size, align), GFP_KERNEL); Save a above. > } > > static char sas_route_char(struct domain_device *dev, struct ex_phy *phy) > @@ -403,15 +403,17 @@ static int sas_ex_phy_discover_helper(struct domain_device *dev, u8 *disc_req, > int sas_ex_phy_discover(struct domain_device *dev, int single) > { > struct expander_device *ex = &dev->ex_dev; > - int res = 0; > + int res = 0, align; > u8 *disc_req; > u8 *disc_resp; > > - disc_req = alloc_smp_req(DISCOVER_REQ_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + disc_req = alloc_smp_req(DISCOVER_REQ_SIZE, align); > if (!disc_req) > return -ENOMEM; > > - disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE); > + disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE, align); > if (!disc_resp) { > kfree(disc_req); > return -ENOMEM; > @@ -480,14 +482,15 @@ static int sas_ex_general(struct domain_device *dev) > { > u8 *rg_req; > struct smp_resp *rg_resp; > - int res; > - int i; > + int i, res, align; > > - rg_req = alloc_smp_req(RG_REQ_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + rg_req = alloc_smp_req(RG_REQ_SIZE, align); > if (!rg_req) > return -ENOMEM; > > - rg_resp = alloc_smp_resp(RG_RESP_SIZE); > + rg_resp = alloc_smp_resp(RG_RESP_SIZE, align); > if (!rg_resp) { > kfree(rg_req); > return -ENOMEM; > @@ -552,13 +555,15 @@ static int sas_ex_manuf_info(struct domain_device *dev) > { > u8 *mi_req; > u8 *mi_resp; > - int res; > + int res, align; > > - mi_req = alloc_smp_req(MI_REQ_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + mi_req = alloc_smp_req(MI_REQ_SIZE, align); > if (!mi_req) > return -ENOMEM; > > - mi_resp = alloc_smp_resp(MI_RESP_SIZE); > + mi_resp = alloc_smp_resp(MI_RESP_SIZE, align); > if (!mi_resp) { > kfree(mi_req); > return -ENOMEM; > @@ -593,13 +598,15 @@ int sas_smp_phy_control(struct domain_device *dev, int phy_id, > { > u8 *pc_req; > u8 *pc_resp; > - int res; > + int res, align; > + > + align = dma_get_cache_alignment(&dev->phy->dev); > > - pc_req = alloc_smp_req(PC_REQ_SIZE); > + pc_req = alloc_smp_req(PC_REQ_SIZE, align); > if (!pc_req) > return -ENOMEM; > > - pc_resp = alloc_smp_resp(PC_RESP_SIZE); > + pc_resp = alloc_smp_resp(PC_RESP_SIZE, align); > if (!pc_resp) { > kfree(pc_req); > return -ENOMEM; > @@ -664,17 +671,19 @@ static int sas_dev_present_in_domain(struct asd_sas_port *port, > #define RPEL_RESP_SIZE 32 > int sas_smp_get_phy_events(struct sas_phy *phy) > { > - int res; > + int res, align; > u8 *req; > u8 *resp; > struct sas_rphy *rphy = dev_to_rphy(phy->dev.parent); > struct domain_device *dev = sas_find_dev_by_rphy(rphy); > > - req = alloc_smp_req(RPEL_REQ_SIZE); > + align = dma_get_cache_alignment(&phy->dev); > + > + req = alloc_smp_req(RPEL_REQ_SIZE, align); > if (!req) > return -ENOMEM; > > - resp = alloc_smp_resp(RPEL_RESP_SIZE); > + resp = alloc_smp_resp(RPEL_RESP_SIZE, align); > if (!resp) { > kfree(req); > return -ENOMEM; > @@ -709,7 +718,8 @@ int sas_get_report_phy_sata(struct domain_device *dev, int phy_id, > struct smp_resp *rps_resp) > { > int res; > - u8 *rps_req = alloc_smp_req(RPS_REQ_SIZE); > + int align = dma_get_cache_alignment(&dev->phy->dev); > + u8 *rps_req = alloc_smp_req(RPS_REQ_SIZE, align); > u8 *resp = (u8 *)rps_resp; > > if (!rps_req) > @@ -1398,7 +1408,7 @@ static int sas_check_parent_topology(struct domain_device *child) > static int sas_configure_present(struct domain_device *dev, int phy_id, > u8 *sas_addr, int *index, int *present) > { > - int i, res = 0; > + int i, res = 0, align; > struct expander_device *ex = &dev->ex_dev; > struct ex_phy *phy = &ex->ex_phy[phy_id]; > u8 *rri_req; > @@ -1406,12 +1416,13 @@ static int sas_configure_present(struct domain_device *dev, int phy_id, > > *present = 0; > *index = 0; > + align = dma_get_cache_alignment(&dev->phy->dev); > > - rri_req = alloc_smp_req(RRI_REQ_SIZE); > + rri_req = alloc_smp_req(RRI_REQ_SIZE, align); > if (!rri_req) > return -ENOMEM; > > - rri_resp = alloc_smp_resp(RRI_RESP_SIZE); > + rri_resp = alloc_smp_resp(RRI_RESP_SIZE, align); > if (!rri_resp) { > kfree(rri_req); > return -ENOMEM; > @@ -1472,15 +1483,17 @@ static int sas_configure_present(struct domain_device *dev, int phy_id, > static int sas_configure_set(struct domain_device *dev, int phy_id, > u8 *sas_addr, int index, int include) > { > - int res; > + int res, align; > u8 *cri_req; > u8 *cri_resp; > > - cri_req = alloc_smp_req(CRI_REQ_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + cri_req = alloc_smp_req(CRI_REQ_SIZE, align); > if (!cri_req) > return -ENOMEM; > > - cri_resp = alloc_smp_resp(CRI_RESP_SIZE); > + cri_resp = alloc_smp_resp(CRI_RESP_SIZE, align); > if (!cri_resp) { > kfree(cri_req); > return -ENOMEM; > @@ -1689,10 +1702,12 @@ int sas_discover_root_expander(struct domain_device *dev) > static int sas_get_phy_discover(struct domain_device *dev, > int phy_id, struct smp_resp *disc_resp) > { > - int res; > + int res, align; > u8 *disc_req; > > - disc_req = alloc_smp_req(DISCOVER_REQ_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + disc_req = alloc_smp_req(DISCOVER_REQ_SIZE, align); > if (!disc_req) > return -ENOMEM; > > @@ -1715,10 +1730,12 @@ static int sas_get_phy_discover(struct domain_device *dev, > static int sas_get_phy_change_count(struct domain_device *dev, > int phy_id, int *pcc) > { > - int res; > + int res, align; > struct smp_resp *disc_resp; > > - disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE, align); > if (!disc_resp) > return -ENOMEM; > > @@ -1733,11 +1750,13 @@ static int sas_get_phy_change_count(struct domain_device *dev, > static int sas_get_phy_attached_dev(struct domain_device *dev, int phy_id, > u8 *sas_addr, enum sas_device_type *type) > { > - int res; > + int res, align; > struct smp_resp *disc_resp; > struct discover_resp *dr; > > - disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE, align); > if (!disc_resp) > return -ENOMEM; > dr = &disc_resp->disc; > @@ -1787,15 +1806,17 @@ static int sas_find_bcast_phy(struct domain_device *dev, int *phy_id, > > static int sas_get_ex_change_count(struct domain_device *dev, int *ecc) > { > - int res; > + int res, align; > u8 *rg_req; > struct smp_resp *rg_resp; > > - rg_req = alloc_smp_req(RG_REQ_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + rg_req = alloc_smp_req(RG_REQ_SIZE, align); > if (!rg_req) > return -ENOMEM; > > - rg_resp = alloc_smp_resp(RG_RESP_SIZE); > + rg_resp = alloc_smp_resp(RG_RESP_SIZE, align); > if (!rg_resp) { > kfree(rg_req); > return -ENOMEM; Best regards
Hi, Marek, Yes, I know in include/linux/slab.h, there is #define KMALLOC_MIN_SIZE ARCH_DMA_MINALIGN But I observed that the req/resp isn't aligned to ARCH_DMA_MINALIGN and I don't know why. Problems I experienced is: In non-coherent mode, mvsas with an expander cannot detect any disks. Huacai ------------------ Original ------------------ From: "Marek Szyprowski"<m.szyprowski@samsung.com>; Date: Tue, Oct 17, 2017 07:55 PM To: "Huacai Chen"<chenhc@lemote.com>; "Christoph Hellwig"<hch@lst.de>; Cc: "Robin Murphy"<robin.murphy@arm.com>; "Andrew Morton"<akpm@linux-foundation.org>; "Fuxin Zhang"<zhangfx@lemote.com>; "linux-kernel"<linux-kernel@vger.kernel.org>; "Ralf Baechle"<ralf@linux-mips.org>; "JamesHogan"<james.hogan@imgtec.com>; "linux-mips"<linux-mips@linux-mips.org>; "James E . J .Bottomley"<jejb@linux.vnet.ibm.com>; "Martin K . Petersen"<martin.petersen@oracle.com>; "linux-scsi"<linux-scsi@vger.kernel.org>; "Tejun Heo"<tj@kernel.org>; "linux-ide"<linux-ide@vger.kernel.org>; "stable"<stable@vger.kernel.org>; Subject: Re: [PATCH V8 4/5] libsas: Align SMP req/resp todma_get_cache_alignment() Hi Huacai, On 2017-10-17 10:05, Huacai Chen wrote: > In non-coherent DMA mode, kernel uses cache flushing operations to > maintain I/O coherency, so libsas's SMP request/response should be > aligned to ARCH_DMA_MINALIGN. Otherwise, If a DMA buffer and a kernel > structure share a same cache line, and if the kernel structure has > dirty data, cache_invalidate (no writeback) will cause data corruption. > > Cc: stable@vger.kernel.org > Signed-off-by: Huacai Chen <chenhc@lemote.com> > --- > drivers/scsi/libsas/sas_expander.c | 93 +++++++++++++++++++++++--------------- > 1 file changed, 57 insertions(+), 36 deletions(-) > > diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c > index 6b4fd23..124a44b 100644 > --- a/drivers/scsi/libsas/sas_expander.c > +++ b/drivers/scsi/libsas/sas_expander.c > @@ -164,17 +164,17 @@ static int smp_execute_task(struct domain_device *dev, void *req, int req_size, > > /* ---------- Allocations ---------- */ > > -static inline void *alloc_smp_req(int size) > +static inline void *alloc_smp_req(int size, int align) > { > - u8 *p = kzalloc(size, GFP_KERNEL); > + u8 *p = kzalloc(ALIGN(size, align), GFP_KERNEL); If I remember correctly, kernel guarantees that each kmalloced buffer is always at least aligned to the CPU cache line, so CPU cache can be invalidated on the allocated buffer without corrupting anything else. Taking this into account, I wonder if the above change make sense. Have you experienced any problems without this change? > if (p) > p[0] = SMP_REQUEST; > return p; > } > > -static inline void *alloc_smp_resp(int size) > +static inline void *alloc_smp_resp(int size, int align) > { > - return kzalloc(size, GFP_KERNEL); > + return kzalloc(ALIGN(size, align), GFP_KERNEL); Save a above. > } > > static char sas_route_char(struct domain_device *dev, struct ex_phy *phy) > @@ -403,15 +403,17 @@ static int sas_ex_phy_discover_helper(struct domain_device *dev, u8 *disc_req, > int sas_ex_phy_discover(struct domain_device *dev, int single) > { > struct expander_device *ex = &dev->ex_dev; > - int res = 0; > + int res = 0, align; > u8 *disc_req; > u8 *disc_resp; > > - disc_req = alloc_smp_req(DISCOVER_REQ_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + disc_req = alloc_smp_req(DISCOVER_REQ_SIZE, align); > if (!disc_req) > return -ENOMEM; > > - disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE); > + disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE, align); > if (!disc_resp) { > kfree(disc_req); > return -ENOMEM; > @@ -480,14 +482,15 @@ static int sas_ex_general(struct domain_device *dev) > { > u8 *rg_req; > struct smp_resp *rg_resp; > - int res; > - int i; > + int i, res, align; > > - rg_req = alloc_smp_req(RG_REQ_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + rg_req = alloc_smp_req(RG_REQ_SIZE, align); > if (!rg_req) > return -ENOMEM; > > - rg_resp = alloc_smp_resp(RG_RESP_SIZE); > + rg_resp = alloc_smp_resp(RG_RESP_SIZE, align); > if (!rg_resp) { > kfree(rg_req); > return -ENOMEM; > @@ -552,13 +555,15 @@ static int sas_ex_manuf_info(struct domain_device *dev) > { > u8 *mi_req; > u8 *mi_resp; > - int res; > + int res, align; > > - mi_req = alloc_smp_req(MI_REQ_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + mi_req = alloc_smp_req(MI_REQ_SIZE, align); > if (!mi_req) > return -ENOMEM; > > - mi_resp = alloc_smp_resp(MI_RESP_SIZE); > + mi_resp = alloc_smp_resp(MI_RESP_SIZE, align); > if (!mi_resp) { > kfree(mi_req); > return -ENOMEM; > @@ -593,13 +598,15 @@ int sas_smp_phy_control(struct domain_device *dev, int phy_id, > { > u8 *pc_req; > u8 *pc_resp; > - int res; > + int res, align; > + > + align = dma_get_cache_alignment(&dev->phy->dev); > > - pc_req = alloc_smp_req(PC_REQ_SIZE); > + pc_req = alloc_smp_req(PC_REQ_SIZE, align); > if (!pc_req) > return -ENOMEM; > > - pc_resp = alloc_smp_resp(PC_RESP_SIZE); > + pc_resp = alloc_smp_resp(PC_RESP_SIZE, align); > if (!pc_resp) { > kfree(pc_req); > return -ENOMEM; > @@ -664,17 +671,19 @@ static int sas_dev_present_in_domain(struct asd_sas_port *port, > #define RPEL_RESP_SIZE 32 > int sas_smp_get_phy_events(struct sas_phy *phy) > { > - int res; > + int res, align; > u8 *req; > u8 *resp; > struct sas_rphy *rphy = dev_to_rphy(phy->dev.parent); > struct domain_device *dev = sas_find_dev_by_rphy(rphy); > > - req = alloc_smp_req(RPEL_REQ_SIZE); > + align = dma_get_cache_alignment(&phy->dev); > + > + req = alloc_smp_req(RPEL_REQ_SIZE, align); > if (!req) > return -ENOMEM; > > - resp = alloc_smp_resp(RPEL_RESP_SIZE); > + resp = alloc_smp_resp(RPEL_RESP_SIZE, align); > if (!resp) { > kfree(req); > return -ENOMEM; > @@ -709,7 +718,8 @@ int sas_get_report_phy_sata(struct domain_device *dev, int phy_id, > struct smp_resp *rps_resp) > { > int res; > - u8 *rps_req = alloc_smp_req(RPS_REQ_SIZE); > + int align = dma_get_cache_alignment(&dev->phy->dev); > + u8 *rps_req = alloc_smp_req(RPS_REQ_SIZE, align); > u8 *resp = (u8 *)rps_resp; > > if (!rps_req) > @@ -1398,7 +1408,7 @@ static int sas_check_parent_topology(struct domain_device *child) > static int sas_configure_present(struct domain_device *dev, int phy_id, > u8 *sas_addr, int *index, int *present) > { > - int i, res = 0; > + int i, res = 0, align; > struct expander_device *ex = &dev->ex_dev; > struct ex_phy *phy = &ex->ex_phy[phy_id]; > u8 *rri_req; > @@ -1406,12 +1416,13 @@ static int sas_configure_present(struct domain_device *dev, int phy_id, > > *present = 0; > *index = 0; > + align = dma_get_cache_alignment(&dev->phy->dev); > > - rri_req = alloc_smp_req(RRI_REQ_SIZE); > + rri_req = alloc_smp_req(RRI_REQ_SIZE, align); > if (!rri_req) > return -ENOMEM; > > - rri_resp = alloc_smp_resp(RRI_RESP_SIZE); > + rri_resp = alloc_smp_resp(RRI_RESP_SIZE, align); > if (!rri_resp) { > kfree(rri_req); > return -ENOMEM; > @@ -1472,15 +1483,17 @@ static int sas_configure_present(struct domain_device *dev, int phy_id, > static int sas_configure_set(struct domain_device *dev, int phy_id, > u8 *sas_addr, int index, int include) > { > - int res; > + int res, align; > u8 *cri_req; > u8 *cri_resp; > > - cri_req = alloc_smp_req(CRI_REQ_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + cri_req = alloc_smp_req(CRI_REQ_SIZE, align); > if (!cri_req) > return -ENOMEM; > > - cri_resp = alloc_smp_resp(CRI_RESP_SIZE); > + cri_resp = alloc_smp_resp(CRI_RESP_SIZE, align); > if (!cri_resp) { > kfree(cri_req); > return -ENOMEM; > @@ -1689,10 +1702,12 @@ int sas_discover_root_expander(struct domain_device *dev) > static int sas_get_phy_discover(struct domain_device *dev, > int phy_id, struct smp_resp *disc_resp) > { > - int res; > + int res, align; > u8 *disc_req; > > - disc_req = alloc_smp_req(DISCOVER_REQ_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + disc_req = alloc_smp_req(DISCOVER_REQ_SIZE, align); > if (!disc_req) > return -ENOMEM; > > @@ -1715,10 +1730,12 @@ static int sas_get_phy_discover(struct domain_device *dev, > static int sas_get_phy_change_count(struct domain_device *dev, > int phy_id, int *pcc) > { > - int res; > + int res, align; > struct smp_resp *disc_resp; > > - disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE, align); > if (!disc_resp) > return -ENOMEM; > > @@ -1733,11 +1750,13 @@ static int sas_get_phy_change_count(struct domain_device *dev, > static int sas_get_phy_attached_dev(struct domain_device *dev, int phy_id, > u8 *sas_addr, enum sas_device_type *type) > { > - int res; > + int res, align; > struct smp_resp *disc_resp; > struct discover_resp *dr; > > - disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE, align); > if (!disc_resp) > return -ENOMEM; > dr = &disc_resp->disc; > @@ -1787,15 +1806,17 @@ static int sas_find_bcast_phy(struct domain_device *dev, int *phy_id, > > static int sas_get_ex_change_count(struct domain_device *dev, int *ecc) > { > - int res; > + int res, align; > u8 *rg_req; > struct smp_resp *rg_resp; > > - rg_req = alloc_smp_req(RG_REQ_SIZE); > + align = dma_get_cache_alignment(&dev->phy->dev); > + > + rg_req = alloc_smp_req(RG_REQ_SIZE, align); > if (!rg_req) > return -ENOMEM; > > - rg_resp = alloc_smp_resp(RG_RESP_SIZE); > + rg_resp = alloc_smp_resp(RG_RESP_SIZE, align); > if (!rg_resp) { > kfree(rg_req); > return -ENOMEM; Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
On Tue, Oct 17, 2017 at 01:55:43PM +0200, Marek Szyprowski wrote: > If I remember correctly, kernel guarantees that each kmalloced buffer is > always at least aligned to the CPU cache line, so CPU cache can be > invalidated on the allocated buffer without corrupting anything else. Yes, from slab.h: /* * Some archs want to perform DMA into kmalloc caches and need a guaranteed * alignment larger than the alignment of a 64-bit integer. * Setting ARCH_KMALLOC_MINALIGN in arch headers allows that. */ #if defined(ARCH_DMA_MINALIGN) && ARCH_DMA_MINALIGN > 8 #define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN #define KMALLOC_MIN_SIZE ARCH_DMA_MINALIGN #define KMALLOC_SHIFT_LOW ilog2(ARCH_DMA_MINALIGN) #else #define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long) #endif Mips sets this for a few subarchitectures, but it seems like you need to set it for yours as well.
Hi Huacai, [auto build test ERROR on linus/master] [also build test ERROR on v4.14-rc5 next-20171018] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Huacai-Chen/dma-mapping-Rework-dma_get_cache_alignment/20171020-050317 config: um-allyesconfig (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=um All errors (new ones prefixed by >>): drivers/scsi/libsas/sas_expander.c: In function 'sas_ex_phy_discover': >> drivers/scsi/libsas/sas_expander.c:410:10: error: implicit declaration of function 'dma_get_cache_alignment' [-Werror=implicit-function-declaration] align = dma_get_cache_alignment(&dev->phy->dev); ^~~~~~~~~~~~~~~~~~~~~~~ Cyclomatic Complexity 5 include/linux/compiler.h:__write_once_size Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:atomic_set Cyclomatic Complexity 2 arch/x86/include/asm/bitops.h:set_bit Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:constant_test_bit Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls64 Cyclomatic Complexity 1 arch/x86/include/uapi/asm/swab.h:__arch_swab64 Cyclomatic Complexity 1 include/uapi/linux/swab.h:__fswab16 Cyclomatic Complexity 1 include/uapi/linux/swab.h:__fswab64 Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u64 Cyclomatic Complexity 1 include/linux/list.h:INIT_LIST_HEAD Cyclomatic Complexity 2 include/linux/list.h:__list_add Cyclomatic Complexity 1 include/linux/list.h:list_add_tail Cyclomatic Complexity 1 include/linux/list.h:__list_del Cyclomatic Complexity 2 include/linux/list.h:__list_del_entry Cyclomatic Complexity 1 include/linux/list.h:list_del Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order Cyclomatic Complexity 1 include/linux/spinlock.h:spinlock_check Cyclomatic Complexity 1 include/linux/spinlock.h:spin_lock_irq Cyclomatic Complexity 1 include/linux/spinlock.h:spin_unlock_irq Cyclomatic Complexity 1 include/linux/spinlock.h:spin_unlock_irqrestore Cyclomatic Complexity 1 include/linux/refcount.h:refcount_set Cyclomatic Complexity 28 include/linux/slab.h:kmalloc_index Cyclomatic Complexity 68 include/linux/slab.h:kmalloc_large Cyclomatic Complexity 5 include/linux/slab.h:kmalloc Cyclomatic Complexity 1 include/linux/slab.h:kzalloc Cyclomatic Complexity 1 include/linux/kref.h:kref_init Cyclomatic Complexity 1 include/linux/kref.h:kref_get Cyclomatic Complexity 2 include/linux/kref.h:kref_put Cyclomatic Complexity 1 include/scsi/scsi.h:scsi_to_u32 Cyclomatic Complexity 1 include/scsi/sas_ata.h:dev_is_sata Cyclomatic Complexity 5 drivers/scsi/libsas/sas_internal.h:sas_fill_in_rphy Cyclomatic Complexity 2 drivers/scsi/libsas/sas_internal.h:sas_add_parent_port Cyclomatic Complexity 2 drivers/scsi/libsas/sas_internal.h:sas_alloc_device Cyclomatic Complexity 1 drivers/scsi/libsas/sas_internal.h:sas_put_device Cyclomatic Complexity 2 drivers/scsi/libsas/sas_expander.c:alloc_smp_req Cyclomatic Complexity 1 drivers/scsi/libsas/sas_expander.c:alloc_smp_resp Cyclomatic Complexity 5 drivers/scsi/libsas/sas_expander.c:sas_route_char Cyclomatic Complexity 4 drivers/scsi/libsas/sas_expander.c:to_dev_type Cyclomatic Complexity 4 drivers/scsi/libsas/sas_expander.c:dev_type_flutter Cyclomatic Complexity 3 drivers/scsi/libsas/sas_expander.c:sas_print_parent_topology_bug Cyclomatic Complexity 17 drivers/scsi/libsas/sas_expander.c:smp_execute_task_sg Cyclomatic Complexity 1 drivers/scsi/libsas/sas_expander.c:smp_execute_task Cyclomatic Complexity 21 drivers/scsi/libsas/sas_expander.c:sas_configure_present Cyclomatic Complexity 4 drivers/scsi/libsas/sas_expander.c:sas_get_phy_discover Cyclomatic Complexity 3 drivers/scsi/libsas/sas_expander.c:sas_get_phy_change_count Cyclomatic Complexity 6 drivers/scsi/libsas/sas_expander.c:sas_find_bcast_phy Cyclomatic Complexity 6 drivers/scsi/libsas/sas_expander.c:sas_get_ex_change_count Cyclomatic Complexity 2 drivers/scsi/libsas/sas_expander.c:smp_task_timedout Cyclomatic Complexity 2 drivers/scsi/libsas/sas_expander.c:smp_task_done Cyclomatic Complexity 4 drivers/scsi/libsas/sas_expander.c:ex_assign_report_general Cyclomatic Complexity 22 drivers/scsi/libsas/sas_expander.c:sas_check_eeds Cyclomatic Complexity 23 drivers/scsi/libsas/sas_expander.c:sas_check_parent_topology Cyclomatic Complexity 11 drivers/scsi/libsas/sas_expander.c:sas_configure_set Cyclomatic Complexity 3 drivers/scsi/libsas/sas_expander.c:sas_configure_phy Cyclomatic Complexity 11 drivers/scsi/libsas/sas_expander.c:sas_configure_parent Cyclomatic Complexity 2 drivers/scsi/libsas/sas_expander.c:sas_configure_routing Cyclomatic Complexity 2 drivers/scsi/libsas/sas_expander.c:sas_disable_routing Cyclomatic Complexity 5 drivers/scsi/libsas/sas_expander.c:sas_find_sub_addr Cyclomatic Complexity 4 drivers/scsi/libsas/sas_expander.c:sas_get_phy_attached_dev Cyclomatic Complexity 35 drivers/scsi/libsas/sas_expander.c:sas_set_ex_phy Cyclomatic Complexity 3 drivers/scsi/libsas/sas_expander.c:sas_ex_phy_discover_helper Cyclomatic Complexity 10 drivers/scsi/libsas/sas_expander.c:sas_ex_general Cyclomatic Complexity 3 drivers/scsi/libsas/sas_expander.c:ex_assign_manuf_info Cyclomatic Complexity 7 drivers/scsi/libsas/sas_expander.c:sas_ex_manuf_info Cyclomatic Complexity 6 drivers/scsi/libsas/sas_expander.c:sas_ex_get_linkrate Cyclomatic Complexity 5 drivers/scsi/libsas/sas_expander.c:sas_ex_join_wide_port Cyclomatic Complexity 10 drivers/scsi/libsas/sas_expander.c:sas_dev_present_in_domain Cyclomatic Complexity 12 drivers/scsi/libsas/sas_expander.c:sas_ex_discover_end_dev Cyclomatic Complexity 6 drivers/scsi/libsas/sas_expander.c:sas_unregister_ex_tree Cyclomatic Complexity 12 drivers/scsi/libsas/sas_expander.c:sas_unregister_devs_sas_addr Cyclomatic Complexity 10 drivers/scsi/libsas/sas_expander.c:sas_find_bcast_dev Cyclomatic Complexity 5 drivers/scsi/libsas/sas_expander.c:sas_ex_to_ata Cyclomatic Complexity 7 drivers/scsi/libsas/sas_expander.c:sas_ex_phy_discover Cyclomatic Complexity 3 drivers/scsi/libsas/sas_expander.c:sas_expander_discover Cyclomatic Complexity 4 drivers/scsi/libsas/sas_expander.c:sas_smp_phy_control Cyclomatic Complexity 1 drivers/scsi/libsas/sas_expander.c:sas_ex_disable_phy Cyclomatic Complexity 13 drivers/scsi/libsas/sas_expander.c:sas_check_ex_subtractive_boundary Cyclomatic Complexity 7 drivers/scsi/libsas/sas_expander.c:sas_discover_expander Cyclomatic Complexity 9 drivers/scsi/libsas/sas_expander.c:sas_ex_discover_expander Cyclomatic Complexity 6 drivers/scsi/libsas/sas_expander.c:sas_ex_disable_port Cyclomatic Complexity 14 drivers/scsi/libsas/sas_expander.c:sas_check_level_subtractive_boundary Cyclomatic Complexity 40 drivers/scsi/libsas/sas_expander.c:sas_ex_discover_dev Cyclomatic Complexity 7 drivers/scsi/libsas/sas_expander.c:sas_ex_discover_devices Cyclomatic Complexity 8 drivers/scsi/libsas/sas_expander.c:sas_ex_level_discovery Cyclomatic Complexity 2 drivers/scsi/libsas/sas_expander.c:sas_ex_bfs_disc Cyclomatic Complexity 8 drivers/scsi/libsas/sas_expander.c:sas_discover_bfs_by_root_level Cyclomatic Complexity 4 drivers/scsi/libsas/sas_expander.c:sas_discover_bfs_by_root Cyclomatic Complexity 12 drivers/scsi/libsas/sas_expander.c:sas_discover_new Cyclomatic Complexity 22 drivers/scsi/libsas/sas_expander.c:sas_rediscover_dev Cyclomatic Complexity 9 drivers/scsi/libsas/sas_expander.c:sas_rediscover Cyclomatic Complexity 5 drivers/scsi/libsas/sas_expander.c:sas_smp_get_phy_events Cyclomatic Complexity 4 drivers/scsi/libsas/sas_expander.c:sas_discover_root_expander Cyclomatic Complexity 5 drivers/scsi/libsas/sas_expander.c:sas_ex_revalidate_domain Cyclomatic Complexity 7 drivers/scsi/libsas/sas_expander.c:sas_smp_handler Cyclomatic Complexity 1 drivers/scsi/libsas/sas_expander.c:_GLOBAL__sub_I_65535_0_sas_ex_to_ata cc1: some warnings being treated as errors vim +/dma_get_cache_alignment +410 drivers/scsi/libsas/sas_expander.c 402 403 int sas_ex_phy_discover(struct domain_device *dev, int single) 404 { 405 struct expander_device *ex = &dev->ex_dev; 406 int res = 0, align; 407 u8 *disc_req; 408 u8 *disc_resp; 409 > 410 align = dma_get_cache_alignment(&dev->phy->dev); 411 412 disc_req = alloc_smp_req(DISCOVER_REQ_SIZE, align); 413 if (!disc_req) 414 return -ENOMEM; 415 416 disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE, align); 417 if (!disc_resp) { 418 kfree(disc_req); 419 return -ENOMEM; 420 } 421 422 disc_req[1] = SMP_DISCOVER; 423 424 if (0 <= single && single < ex->num_phys) { 425 res = sas_ex_phy_discover_helper(dev, disc_req, disc_resp, single); 426 } else { 427 int i; 428 429 for (i = 0; i < ex->num_phys; i++) { 430 res = sas_ex_phy_discover_helper(dev, disc_req, 431 disc_resp, i); 432 if (res) 433 goto out_err; 434 } 435 } 436 out_err: 437 kfree(disc_resp); 438 kfree(disc_req); 439 return res; 440 } 441 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c index 6b4fd23..124a44b 100644 --- a/drivers/scsi/libsas/sas_expander.c +++ b/drivers/scsi/libsas/sas_expander.c @@ -164,17 +164,17 @@ static int smp_execute_task(struct domain_device *dev, void *req, int req_size, /* ---------- Allocations ---------- */ -static inline void *alloc_smp_req(int size) +static inline void *alloc_smp_req(int size, int align) { - u8 *p = kzalloc(size, GFP_KERNEL); + u8 *p = kzalloc(ALIGN(size, align), GFP_KERNEL); if (p) p[0] = SMP_REQUEST; return p; } -static inline void *alloc_smp_resp(int size) +static inline void *alloc_smp_resp(int size, int align) { - return kzalloc(size, GFP_KERNEL); + return kzalloc(ALIGN(size, align), GFP_KERNEL); } static char sas_route_char(struct domain_device *dev, struct ex_phy *phy) @@ -403,15 +403,17 @@ static int sas_ex_phy_discover_helper(struct domain_device *dev, u8 *disc_req, int sas_ex_phy_discover(struct domain_device *dev, int single) { struct expander_device *ex = &dev->ex_dev; - int res = 0; + int res = 0, align; u8 *disc_req; u8 *disc_resp; - disc_req = alloc_smp_req(DISCOVER_REQ_SIZE); + align = dma_get_cache_alignment(&dev->phy->dev); + + disc_req = alloc_smp_req(DISCOVER_REQ_SIZE, align); if (!disc_req) return -ENOMEM; - disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE); + disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE, align); if (!disc_resp) { kfree(disc_req); return -ENOMEM; @@ -480,14 +482,15 @@ static int sas_ex_general(struct domain_device *dev) { u8 *rg_req; struct smp_resp *rg_resp; - int res; - int i; + int i, res, align; - rg_req = alloc_smp_req(RG_REQ_SIZE); + align = dma_get_cache_alignment(&dev->phy->dev); + + rg_req = alloc_smp_req(RG_REQ_SIZE, align); if (!rg_req) return -ENOMEM; - rg_resp = alloc_smp_resp(RG_RESP_SIZE); + rg_resp = alloc_smp_resp(RG_RESP_SIZE, align); if (!rg_resp) { kfree(rg_req); return -ENOMEM; @@ -552,13 +555,15 @@ static int sas_ex_manuf_info(struct domain_device *dev) { u8 *mi_req; u8 *mi_resp; - int res; + int res, align; - mi_req = alloc_smp_req(MI_REQ_SIZE); + align = dma_get_cache_alignment(&dev->phy->dev); + + mi_req = alloc_smp_req(MI_REQ_SIZE, align); if (!mi_req) return -ENOMEM; - mi_resp = alloc_smp_resp(MI_RESP_SIZE); + mi_resp = alloc_smp_resp(MI_RESP_SIZE, align); if (!mi_resp) { kfree(mi_req); return -ENOMEM; @@ -593,13 +598,15 @@ int sas_smp_phy_control(struct domain_device *dev, int phy_id, { u8 *pc_req; u8 *pc_resp; - int res; + int res, align; + + align = dma_get_cache_alignment(&dev->phy->dev); - pc_req = alloc_smp_req(PC_REQ_SIZE); + pc_req = alloc_smp_req(PC_REQ_SIZE, align); if (!pc_req) return -ENOMEM; - pc_resp = alloc_smp_resp(PC_RESP_SIZE); + pc_resp = alloc_smp_resp(PC_RESP_SIZE, align); if (!pc_resp) { kfree(pc_req); return -ENOMEM; @@ -664,17 +671,19 @@ static int sas_dev_present_in_domain(struct asd_sas_port *port, #define RPEL_RESP_SIZE 32 int sas_smp_get_phy_events(struct sas_phy *phy) { - int res; + int res, align; u8 *req; u8 *resp; struct sas_rphy *rphy = dev_to_rphy(phy->dev.parent); struct domain_device *dev = sas_find_dev_by_rphy(rphy); - req = alloc_smp_req(RPEL_REQ_SIZE); + align = dma_get_cache_alignment(&phy->dev); + + req = alloc_smp_req(RPEL_REQ_SIZE, align); if (!req) return -ENOMEM; - resp = alloc_smp_resp(RPEL_RESP_SIZE); + resp = alloc_smp_resp(RPEL_RESP_SIZE, align); if (!resp) { kfree(req); return -ENOMEM; @@ -709,7 +718,8 @@ int sas_get_report_phy_sata(struct domain_device *dev, int phy_id, struct smp_resp *rps_resp) { int res; - u8 *rps_req = alloc_smp_req(RPS_REQ_SIZE); + int align = dma_get_cache_alignment(&dev->phy->dev); + u8 *rps_req = alloc_smp_req(RPS_REQ_SIZE, align); u8 *resp = (u8 *)rps_resp; if (!rps_req) @@ -1398,7 +1408,7 @@ static int sas_check_parent_topology(struct domain_device *child) static int sas_configure_present(struct domain_device *dev, int phy_id, u8 *sas_addr, int *index, int *present) { - int i, res = 0; + int i, res = 0, align; struct expander_device *ex = &dev->ex_dev; struct ex_phy *phy = &ex->ex_phy[phy_id]; u8 *rri_req; @@ -1406,12 +1416,13 @@ static int sas_configure_present(struct domain_device *dev, int phy_id, *present = 0; *index = 0; + align = dma_get_cache_alignment(&dev->phy->dev); - rri_req = alloc_smp_req(RRI_REQ_SIZE); + rri_req = alloc_smp_req(RRI_REQ_SIZE, align); if (!rri_req) return -ENOMEM; - rri_resp = alloc_smp_resp(RRI_RESP_SIZE); + rri_resp = alloc_smp_resp(RRI_RESP_SIZE, align); if (!rri_resp) { kfree(rri_req); return -ENOMEM; @@ -1472,15 +1483,17 @@ static int sas_configure_present(struct domain_device *dev, int phy_id, static int sas_configure_set(struct domain_device *dev, int phy_id, u8 *sas_addr, int index, int include) { - int res; + int res, align; u8 *cri_req; u8 *cri_resp; - cri_req = alloc_smp_req(CRI_REQ_SIZE); + align = dma_get_cache_alignment(&dev->phy->dev); + + cri_req = alloc_smp_req(CRI_REQ_SIZE, align); if (!cri_req) return -ENOMEM; - cri_resp = alloc_smp_resp(CRI_RESP_SIZE); + cri_resp = alloc_smp_resp(CRI_RESP_SIZE, align); if (!cri_resp) { kfree(cri_req); return -ENOMEM; @@ -1689,10 +1702,12 @@ int sas_discover_root_expander(struct domain_device *dev) static int sas_get_phy_discover(struct domain_device *dev, int phy_id, struct smp_resp *disc_resp) { - int res; + int res, align; u8 *disc_req; - disc_req = alloc_smp_req(DISCOVER_REQ_SIZE); + align = dma_get_cache_alignment(&dev->phy->dev); + + disc_req = alloc_smp_req(DISCOVER_REQ_SIZE, align); if (!disc_req) return -ENOMEM; @@ -1715,10 +1730,12 @@ static int sas_get_phy_discover(struct domain_device *dev, static int sas_get_phy_change_count(struct domain_device *dev, int phy_id, int *pcc) { - int res; + int res, align; struct smp_resp *disc_resp; - disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE); + align = dma_get_cache_alignment(&dev->phy->dev); + + disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE, align); if (!disc_resp) return -ENOMEM; @@ -1733,11 +1750,13 @@ static int sas_get_phy_change_count(struct domain_device *dev, static int sas_get_phy_attached_dev(struct domain_device *dev, int phy_id, u8 *sas_addr, enum sas_device_type *type) { - int res; + int res, align; struct smp_resp *disc_resp; struct discover_resp *dr; - disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE); + align = dma_get_cache_alignment(&dev->phy->dev); + + disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE, align); if (!disc_resp) return -ENOMEM; dr = &disc_resp->disc; @@ -1787,15 +1806,17 @@ static int sas_find_bcast_phy(struct domain_device *dev, int *phy_id, static int sas_get_ex_change_count(struct domain_device *dev, int *ecc) { - int res; + int res, align; u8 *rg_req; struct smp_resp *rg_resp; - rg_req = alloc_smp_req(RG_REQ_SIZE); + align = dma_get_cache_alignment(&dev->phy->dev); + + rg_req = alloc_smp_req(RG_REQ_SIZE, align); if (!rg_req) return -ENOMEM; - rg_resp = alloc_smp_resp(RG_RESP_SIZE); + rg_resp = alloc_smp_resp(RG_RESP_SIZE, align); if (!rg_resp) { kfree(rg_req); return -ENOMEM;
In non-coherent DMA mode, kernel uses cache flushing operations to maintain I/O coherency, so libsas's SMP request/response should be aligned to ARCH_DMA_MINALIGN. Otherwise, If a DMA buffer and a kernel structure share a same cache line, and if the kernel structure has dirty data, cache_invalidate (no writeback) will cause data corruption. Cc: stable@vger.kernel.org Signed-off-by: Huacai Chen <chenhc@lemote.com> --- drivers/scsi/libsas/sas_expander.c | 93 +++++++++++++++++++++++--------------- 1 file changed, 57 insertions(+), 36 deletions(-)