Re: [PATCH V8 4/5] libsas: Align SMP req/resp to dma_get_cache_alignment()

2017-10-19 Thread kbuild test robot 
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(>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

Re: [PATCH V8 4/5] libsas: Align SMP req/resp to dma_get_cache_alignment()

2017-10-19 Thread Christoph Hellwig 
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.

Re: [PATCH V8 4/5] libsas: Align SMP req/resp to dma_get_cache_alignment()

2017-10-17 Thread Marek Szyprowski 

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: sta...@vger.kernel.org
Signed-off-by: Huacai Chen 
---
  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 = >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(>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(>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(>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(>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_SIZE32
  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(>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);