Re: [Scst-devel] Compilation issue on OL-7.6 with MOFED-4.6

SourceForge Headquarters 1320 Columbia Street Suite 310 San Diego, CA 92101 +1 (858) 422-6466

On 5/29/19 8:01 AM, Sergey Gorenko wrote:
> Hi,
> 
> I'm trying to compile r8394 from the trunk on Oracle Linux 7.6, kernel 4.14.35-1844.2.5.el7uek.x86_64 with MLNX_OFED-4.6. And I observe the following issues.
> 
> The first issue is a redefinition of the kvcalloc function:
> /tmp/scst-trunk/iscsi-scst/kernel/isert-scst/../../../scst/include/backport.h:910:21: error: redefinition of 'kvcalloc'
>  static inline void *kvcalloc(size_t n, size_t size, gfp_t flags)
>                      ^
> In file included from ./include/linux/highmem.h:8:0,
>                  from ./include/linux/bio.h:21,
>                  from /tmp/scst-trunk/iscsi-scst/kernel/isert-scst/../../../scst/include/backport.h:24,
>                  from /tmp/scst-trunk/iscsi-scst/kernel/isert-scst/../../../scst/include/scst_debug.h:42,
>                  from /tmp/scst-trunk/iscsi-scst/kernel/isert-scst/isert_dbg.h:29,
>                  from /tmp/scst-trunk/iscsi-scst/kernel/isert-scst/iser_global.c:40:
> /usr/src/ofa_kernel/default/include/linux/mm.h:78:21: note: previous definition of 'kvcalloc' was here
>  static inline void *kvcalloc(size_t n, size_t size, gfp_t flags)
>                      ^
> 
> As you can see the function has been backported in MOFED and in SCST as well. I looked at commit history to find how similar problems were solved before. And I have found "[ r7421] scst_local: Rework the port to kernel v4.17" commit. So I made a patch that fixes the issue in the same way. The patch is listed below:
> Index: qla2x00t-32gbit/qla2x00-target/scst_qla2xxx.c
> ===================================================================
> --- qla2x00t-32gbit/qla2x00-target/scst_qla2xxx.c       (revision 8394)
> +++ qla2x00t-32gbit/qla2x00-target/scst_qla2xxx.c       (working copy)
> @@ -1299,7 +1299,13 @@
>                 goto done;
>         }
> 
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 18, 0)
> +/* See also commit 1c542f38ab8d ("mm: Introduce kvcalloc()") # v4.18. */
> +       sqa_tgt->tgt_cmd_map = kvmalloc(tag_num * tag_size,
> +                                       GFP_KERNEL | __GFP_ZERO);
> +#else
>         sqa_tgt->tgt_cmd_map = kvcalloc(tag_num, tag_size, GFP_KERNEL);
> +#endif
>         if (!sqa_tgt->tgt_cmd_map) {
>                 PRINT_ERROR("sqatgt(%ld/%d): alloc tgt_cmd_map failed",
>                             vha->host_no, vha->vp_idx);
> Index: scst/include/backport.h
> ===================================================================
> --- scst/include/backport.h     (revision 8394)
> +++ scst/include/backport.h     (working copy)
> @@ -905,14 +905,6 @@
>  }
>  #endif
> 
> -#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 18, 0)
> -/* See also commit 1c542f38ab8d ("mm: Introduce kvcalloc()") # v4.18. */
> -static inline void *kvcalloc(size_t n, size_t size, gfp_t flags)
> -{
> -       return kvmalloc(n * size, flags | __GFP_ZERO);
> -}
> -#endif
> -
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(3, 15, 0) &&                  \
>         !(LINUX_VERSION_CODE >> 8 == KERNEL_VERSION(3, 12, 0) >> 8 &&  \
>           LINUX_VERSION_CODE >= KERNEL_VERSION(3, 12, 41)) &&          \
> 
> 
> The second issue is a conflict between MOFED backports and DTrace from the OL kernel:
> 
> $ make V=1 iscsi
> ...
>   gcc -Wp,-MD,/tmp/scst-trunk.save/iscsi-scst/conftest/create_cq/.create_cq.sdtstub.o.d  -nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/4.8.5/include -Werror -I/usr/src/ofa_kernel/defau
> lt/include -I/usr/src/ofa_kernel/default/include/uapi -include /usr/src/ofa_kernel/default/include/linux/compat-2.6.h -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arc
> h/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -D__KERNEL__ -D__ASSEMBLY__ -fno-PIE -m64 -DCONFIG_AS_CF
> I=1 -DCONFIG_AS_CFI_SIGNAL_FRAME=1 -DCONFIG_AS_CFI_SECTIONS=1 -DCONFIG_AS_FXSAVEQ=1 -DCONFIG_AS_SSSE3=1 -DCONFIG_AS_CRC32=1 -DCONFIG_AS_AVX=1 -DCONFIG_AS_AVX2=1 -DCONFIG_AS_AVX512=1 -DCONFIG
> _AS_SHA1_NI=1 -DCONFIG_AS_SHA256_NI=1 -DCC_HAVE_ASM_GOTO -Wa,-gdwarf-2 -mfentry -DCC_USING_FENTRY  -c -o /tmp/scst-trunk.save/iscsi-scst/conftest/create_cq/create_cq.sdtstub.o /tmp/scst-trun
> k.save/iscsi-scst/conftest/create_cq/create_cq.sdtstub.S
> In file included from /usr/src/ofa_kernel/default/include/linux/kernel.h:6:0,
>                  from ./include/linux/uio.h:12,
>                  from ./include/linux/socket.h:8,
>                  from /usr/src/ofa_kernel/default/include/linux/socket.h:4,
>                  from ./include/uapi/linux/if.h:25,
>                  from /usr/src/ofa_kernel/default/include/linux/compat-2.6.h:13,
>                  from <command-line>:0:
> ./include/linux/kernel.h:57:0: error: "ALIGN" redefined [-Werror]
>  #define ALIGN(x, a)  __ALIGN_KERNEL((x), (a))
>  ^
> In file included from ./include/linux/kernel.h:7:0,
>                  from /usr/src/ofa_kernel/default/include/linux/kernel.h:6,
>                  from ./include/linux/uio.h:12,
>                  from ./include/linux/socket.h:8,
>                  from /usr/src/ofa_kernel/default/include/linux/socket.h:4,
>                  from ./include/uapi/linux/if.h:25,
>                  from /usr/src/ofa_kernel/default/include/linux/compat-2.6.h:13,
>                  from <command-line>:0:
> ./include/linux/linkage.h:79:0: note: this is the location of the previous definition
> #define ALIGN __ALIGN
>  ^
> In file included from ./arch/x86/include/asm/spinlock.h:8:0,
>                  from ./include/linux/spinlock.h:88,
>                  from ./include/linux/seqlock.h:36,
>                  from ./include/linux/time.h:6,
>                  from ./include/linux/stat.h:19,
>                  from ./include/linux/module.h:10,
>                  from /usr/src/ofa_kernel/default/include/linux/module.h:4,
>                  from /usr/src/ofa_kernel/default/include/linux/compat-2.6.h:21,
>                  from <command-line>:0:
> ./arch/x86/include/asm/processor.h:400:0: error: "cpu_current_top_of_stack" redefined [-Werror]
>  #define cpu_current_top_of_stack cpu_tss_rw.x86_tss.sp1
>  ^
> In file included from ./include/linux/thread_info.h:38:0,
>                  from ./include/linux/uio.h:13,
>                  from ./include/linux/socket.h:8,
>                  from /usr/src/ofa_kernel/default/include/linux/socket.h:4,
>                  from ./include/uapi/linux/if.h:25,
>                  from /usr/src/ofa_kernel/default/include/linux/compat-2.6.h:13,
>                  from <command-line>:0:
> ./arch/x86/include/asm/thread_info.h:217:0: note: this is the location of the previous definition
>  # define cpu_current_top_of_stack (cpu_tss_rw + TSS_sp1)
>  ^
> ...
> 
> As I understand this is an old unsolved issue. That's why kernel modules from MOFEDs 4.x, 3.x and 2.x are always built without DTrace. I made a patch for SCST to disable DTrace for the isert-scst module if MOFED is detected. The patch is listed below:
> Index: iscsi-scst/Makefile
> ===================================================================
> --- iscsi-scst/Makefile (revision 8394)
> +++ iscsi-scst/Makefile (working copy)
> @@ -83,6 +83,7 @@
>                         -I$(OFED_DIR)/include/uapi                      \
>                         -include $(OFED_DIR)/include/linux/compat-2.6.h
>         ISER_SYMVERS:="$(ISER_SYMVERS) $(OFED_DIR)/$(MODULE_SYMVERS)"
> +       OFED_CONFIG:=   CONFIG_DTRACE=
>  else
>         # Whether or not the OFED kernel-ib-devel RPM has been installed.
>         OFED_KERNEL_IB_DEVEL_RPM_INSTALLED:=$(shell if rpm -q kernel-ib-devel 2>/dev/null | grep -q $$(uname -r | sed 's/-/_/g'); then echo true; else echo false; fi)
> @@ -102,10 +103,12 @@
>         OFED_CFLAGS:=-I/usr/src/compat-rdma/include -include /usr/src/compat-rdma/include/linux/compat-2.6.h
>         ISER_SYMVERS:="$(ISER_SYMVERS) /usr/src/compat-rdma/$(MODULE_SYMVERS)"
>         endif
> +
> +       OFED_CONFIG:=
>  endif
> 
>  # Use make instead of $(MAKE) to get rid of command-line option -i
> -run_conftest = $(shell if [ "0$(V)" -gt 0 ]; then output=/dev/stdout; else output=/dev/null; fi; if MAKEFLAGS= make -C $(KDIR) V=$(V) M="$(shell pwd)/conftest/$1" PRE_CFLAGS="-Werror $(OFED
> _CFLAGS)" 1>&2 2>$${output}; then echo "$(strip $2)"; else echo "$(strip $3)"; fi)
> +run_conftest = $(shell if [ "0$(V)" -gt 0 ]; then output=/dev/stdout; else output=/dev/null; fi; if MAKEFLAGS= make -C $(KDIR) V=$(V) M="$(shell pwd)/conftest/$1" PRE_CFLAGS="-Werror $(OFED
> _CFLAGS)" $(OFED_CONFIG) 1>&2 2>$${output}; then echo "$(strip $2)"; else echo "$(strip $3)"; fi)
>  run_conftest_bool = $(call run_conftest,$1,-D$(strip $2)=1,-D$(strip $2)=0)
> 
>  CONFTESTS = $(shell ls -d conftest/*)
> @@ -127,7 +130,7 @@
>         if $(INFINIBAND_ENABLED); then                                         \
>           echo "  Building against $(OFED_FLAVOR) InfiniBand kernel headers."; \
>           $(MAKE) -C $(KDIR) M=$(ISERTMOD) PRE_CFLAGS="$(ISER_CFLAGS)"         \
> -         KBUILD_EXTRA_SYMBOLS=$(ISER_SYMVERS) modules;                        \
> +         KBUILD_EXTRA_SYMBOLS=$(ISER_SYMVERS) $(OFED_CONFIG) modules;         \
>         fi
> 
>  progs: include/iscsi_scst_itf_ver.h
> 
> Could you take a look at these two patches?
> 
> I would appreciate any feedback or help.

Hi Sergey,

A slightly modified version of your isert-scst patch has been checked in
that should solve the DTrace compatibility issue.

Regarding the kvcalloc() redefinition: does MOFED define macros with the
MOFED major and minor versions? I would prefer to add a test these
macros in scst/include/backport.h instead of modifying the
qla2x00t-32gbit/qla2x00-target/scst_qla2xxx.c source file. As you may
have noticed when possible ifdefs have been moved from .c files into the
scst/include/backport.h file.

Thanks,

Bart.