Re: [PATCH 1/2] nvme: remove workarounds for gcc bug wrt unnamed fields in initializers
Hi Niklas,
I love your patch! Yet something to improve:
[auto build test ERROR on block/for-next]
[also build test ERROR on linus/master v5.8-rc1 next-20200618]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Niklas-Cassel/nvme-remove-workarounds-for-gcc-bug-wrt-unnamed-fields-in-initializers/20200618-223525
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git
for-next
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project
487ca07fcc75d52755c9fe2ee05bcb3b6c44)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
>> drivers/nvme/host/rdma.c:1814:20: error: use of undeclared identifier 'priv'
.private_data = &priv,
^
drivers/nvme/host/rdma.c:1815:30: error: use of undeclared identifier 'priv'
.private_data_len = sizeof(priv),
^
2 errors generated.
vim +/priv +1814 drivers/nvme/host/rdma.c
1803
1804 static int nvme_rdma_route_resolved(struct nvme_rdma_queue *queue)
1805 {
1806 struct nvme_rdma_ctrl *ctrl = queue->ctrl;
1807 struct rdma_conn_param param = {
1808 .qp_num = queue->qp->qp_num,
1809 .flow_control = 1,
1810 .responder_resources =
queue->device->dev->attrs.max_qp_rd_atom,
1811 /* maximum retry count */
1812 .retry_count = 7,
1813 .rnr_retry_count = 7,
> 1814 .private_data = &priv,
1815 .private_data_len = sizeof(priv),
1816 };
1817 struct nvme_rdma_cm_req priv = {
1818 .recfmt = cpu_to_le16(NVME_RDMA_CM_FMT_1_0),
1819 .qid = cpu_to_le16(nvme_rdma_queue_idx(queue)),
1820 };
1821 int ret;
1822
1823 /*
1824 * set the admin queue depth to the minimum size
1825 * specified by the Fabrics standard.
1826 */
1827 if (priv.qid == 0) {
1828 priv.hrqsize = cpu_to_le16(NVME_AQ_DEPTH);
1829 priv.hsqsize = cpu_to_le16(NVME_AQ_DEPTH - 1);
1830 } else {
1831 /*
1832 * current interpretation of the fabrics spec
1833 * is at minimum you make hrqsize sqsize+1, or a
1834 * 1's based representation of sqsize.
1835 */
1836 priv.hrqsize = cpu_to_le16(queue->queue_size);
1837 priv.hsqsize = cpu_to_le16(queue->ctrl->ctrl.sqsize);
1838 }
1839
1840 ret = rdma_connect(queue->cm_id, ¶m);
1841 if (ret) {
1842 dev_err(ctrl->ctrl.device,
1843 "rdma_connect failed (%d).\n", ret);
1844 goto out_destroy_queue_ib;
1845 }
1846
1847 return 0;
1848
1849 out_destroy_queue_ib:
1850 nvme_rdma_destroy_queue_ib(queue);
1851 return ret;
1852 }
1853
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH 1/2] nvme: remove workarounds for gcc bug wrt unnamed fields in initializers
Hi Niklas,
I love your patch! Yet something to improve:
[auto build test ERROR on block/for-next]
[also build test ERROR on linus/master v5.8-rc1 next-20200618]
[cannot apply to hch-configfs/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Niklas-Cassel/nvme-remove-workarounds-for-gcc-bug-wrt-unnamed-fields-in-initializers/20200618-223525
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git
for-next
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=m68k
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
In file included from arch/m68k/include/asm/io_mm.h:25,
from arch/m68k/include/asm/io.h:8,
from include/linux/scatterlist.h:9,
from include/linux/dma-mapping.h:11,
from include/rdma/ib_verbs.h:44,
from include/rdma/mr_pool.h:8,
from drivers/nvme/host/rdma.c:10:
arch/m68k/include/asm/raw_io.h: In function 'raw_rom_outsb':
arch/m68k/include/asm/raw_io.h:83:7: warning: variable '__w' set but not
used [-Wunused-but-set-variable]
83 | ({u8 __w, __v = (b); u32 _addr = ((u32) (addr)); \
| ^~~
arch/m68k/include/asm/raw_io.h:430:3: note: in expansion of macro 'rom_out_8'
430 | rom_out_8(port, *buf++);
| ^
arch/m68k/include/asm/raw_io.h: In function 'raw_rom_outsw':
arch/m68k/include/asm/raw_io.h:86:8: warning: variable '__w' set but not
used [-Wunused-but-set-variable]
86 | ({u16 __w, __v = (w); u32 _addr = ((u32) (addr)); \
|^~~
arch/m68k/include/asm/raw_io.h:448:3: note: in expansion of macro
'rom_out_be16'
448 | rom_out_be16(port, *buf++);
| ^~~~
arch/m68k/include/asm/raw_io.h: In function 'raw_rom_outsw_swapw':
arch/m68k/include/asm/raw_io.h:90:8: warning: variable '__w' set but not
used [-Wunused-but-set-variable]
90 | ({u16 __w, __v = (w); u32 _addr = ((u32) (addr)); \
|^~~
arch/m68k/include/asm/raw_io.h:466:3: note: in expansion of macro
'rom_out_le16'
466 | rom_out_le16(port, *buf++);
| ^~~~
In file included from include/linux/kernel.h:11,
from include/linux/list.h:9,
from include/linux/module.h:12,
from drivers/nvme/host/rdma.c:7:
include/linux/scatterlist.h: In function 'sg_set_buf':
arch/m68k/include/asm/page_mm.h:169:49: warning: ordered comparison of
pointer with null pointer [-Wextra]
169 | #define virt_addr_valid(kaddr) ((void *)(kaddr) >= (void
*)PAGE_OFFSET && (void *)(kaddr) < high_memory)
| ^~
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON'
143 | BUG_ON(!virt_addr_valid(buf));
| ^~
include/linux/scatterlist.h:143:10: note: in expansion of macro
'virt_addr_valid'
143 | BUG_ON(!virt_addr_valid(buf));
| ^~~
In file included from arch/m68k/include/asm/bug.h:32,
from include/linux/bug.h:5,
from include/linux/thread_info.h:12,
from include/asm-generic/preempt.h:5,
from ./arch/m68k/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from include/linux/stat.h:19,
from include/linux/module.h:13,
from drivers/nvme/host/rdma.c:7:
include/linux/dma-mapping.h: In function 'dma_map_resource':
arch/m68k/include/asm/page_mm.h:169:49: warning: ordered comparison of
pointer with null pointer [-Wextra]
169 | #define virt_addr_valid(kaddr) ((void *)(kaddr) >= (void
*)PAGE_OFFSET && (void *)(kaddr) < high_memory)
| ^~
include/asm-generic/bug.h:144:27: note: in definition of macro 'WARN_ON_ONCE'
144 | int __ret_warn_once = !!(condition); \
| ^~~
Re: [PATCH 1/2] nvme: remove workarounds for gcc bug wrt unnamed fields in initializers
On Thu, Jun 18, 2020 at 07:11:24PM +0200, Daniel Wagner wrote: > On Thu, Jun 18, 2020 at 04:32:40PM +0200, Niklas Cassel wrote: > > If, for some reason, we want to allow builds with gcc < 4.6.0 > > even though the minimum gcc version is now 4.8.0, > > Just one thing to watch out: the stable trees are still using > older version of gcc. Note sure how relevant this is though. While the AUTOSEL can be a bit annoying when autoselecting patches to backport that you didn't intend, I think that it in most cases backports fixes that has a Fixes-tag, which this doesn't. Kind regards, Niklas
Re: [PATCH 1/2] nvme: remove workarounds for gcc bug wrt unnamed fields in initializers
On Thu, Jun 18, 2020 at 04:32:40PM +0200, Niklas Cassel wrote: > If, for some reason, we want to allow builds with gcc < 4.6.0 > even though the minimum gcc version is now 4.8.0, Just one thing to watch out: the stable trees are still using older version of gcc. Note sure how relevant this is though.
[PATCH 1/2] nvme: remove workarounds for gcc bug wrt unnamed fields in initializers
Workarounds for gcc issues with initializers and anon unions was first
introduced in commit e44ac588cd61 ("drivers/block/nvme-core.c: fix build
with gcc-4.4.4").
The gcc bug in question has been fixed since gcc 4.6.0:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=10676
The minimum gcc version for building the kernel has been 4.6.0 since
commit cafa0010cd51 ("Raise the minimum required gcc version to 4.6"),
and has since been updated to gcc 4.8.0 in
commit 5429ef62bcf3 ("compiler/gcc: Raise minimum GCC version for
kernel builds to 4.8").
For that reason, it should now be safe to remove these workarounds
and make the code look like it did before
commit e44ac588cd61 ("drivers/block/nvme-core.c: fix build with gcc-4.4.4")
was introduced.
Signed-off-by: Niklas Cassel
---
If, for some reason, we want to allow builds with gcc < 4.6.0
even though the minimum gcc version is now 4.8.0,
there is another less intrusive workaround where you add an extra pair of
curly braces, see e.g. commit 6cc65be4f6f2 ("locking/qspinlock: Fix build
for anonymous union in older GCC compilers").
drivers/nvme/host/core.c | 59 ++--
drivers/nvme/host/lightnvm.c | 32 +--
drivers/nvme/host/rdma.c | 28 -
3 files changed, 59 insertions(+), 60 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 9491dbcfe81a..99059340d723 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1038,13 +1038,12 @@ static bool nvme_ctrl_limited_cns(struct nvme_ctrl
*ctrl)
static int nvme_identify_ctrl(struct nvme_ctrl *dev, struct nvme_id_ctrl **id)
{
- struct nvme_command c = { };
+ struct nvme_command c = {
+ .identify.opcode = nvme_admin_identify,
+ .identify.cns = NVME_ID_CNS_CTRL,
+ };
int error;
- /* gcc-4.4.4 (at least) has issues with initializers and anon unions */
- c.identify.opcode = nvme_admin_identify;
- c.identify.cns = NVME_ID_CNS_CTRL;
-
*id = kmalloc(sizeof(struct nvme_id_ctrl), GFP_KERNEL);
if (!*id)
return -ENOMEM;
@@ -1096,16 +1095,16 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl,
struct nvme_ns_ids *ids,
static int nvme_identify_ns_descs(struct nvme_ctrl *ctrl, unsigned nsid,
struct nvme_ns_ids *ids)
{
- struct nvme_command c = { };
+ struct nvme_command c = {
+ .identify.opcode = nvme_admin_identify,
+ .identify.nsid = cpu_to_le32(nsid),
+ .identify.cns = NVME_ID_CNS_NS_DESC_LIST,
+ };
int status;
void *data;
int pos;
int len;
- c.identify.opcode = nvme_admin_identify;
- c.identify.nsid = cpu_to_le32(nsid);
- c.identify.cns = NVME_ID_CNS_NS_DESC_LIST;
-
data = kzalloc(NVME_IDENTIFY_DATA_SIZE, GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -1143,11 +1142,12 @@ static int nvme_identify_ns_descs(struct nvme_ctrl
*ctrl, unsigned nsid,
static int nvme_identify_ns_list(struct nvme_ctrl *dev, unsigned nsid, __le32
*ns_list)
{
- struct nvme_command c = { };
+ struct nvme_command c = {
+ .identify.opcode = nvme_admin_identify,
+ .identify.cns = NVME_ID_CNS_NS_ACTIVE_LIST,
+ .identify.nsid = cpu_to_le32(nsid),
+ };
- c.identify.opcode = nvme_admin_identify;
- c.identify.cns = NVME_ID_CNS_NS_ACTIVE_LIST;
- c.identify.nsid = cpu_to_le32(nsid);
return nvme_submit_sync_cmd(dev->admin_q, &c, ns_list,
NVME_IDENTIFY_DATA_SIZE);
}
@@ -1155,14 +1155,13 @@ static int nvme_identify_ns_list(struct nvme_ctrl *dev,
unsigned nsid, __le32 *n
static int nvme_identify_ns(struct nvme_ctrl *ctrl,
unsigned nsid, struct nvme_id_ns **id)
{
- struct nvme_command c = { };
+ struct nvme_command c = {
+ .identify.opcode = nvme_admin_identify,
+ .identify.nsid = cpu_to_le32(nsid),
+ .identify.cns = NVME_ID_CNS_NS,
+ };
int error;
- /* gcc-4.4.4 (at least) has issues with initializers and anon unions */
- c.identify.opcode = nvme_admin_identify;
- c.identify.nsid = cpu_to_le32(nsid);
- c.identify.cns = NVME_ID_CNS_NS;
-
*id = kmalloc(sizeof(**id), GFP_KERNEL);
if (!*id)
return -ENOMEM;
@@ -2815,17 +2814,17 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl,
struct nvme_id_ctrl *id)
int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp,
void *log, size_t size, u64 offset)
{
- struct nvme_command c = { };
u32 dwlen = nvme_bytes_to_numd(size);
-
- c.get_log_page.opcode = nvme_admin_get_log_page;
- c.get_log_page.nsid = cpu_to_le32(nsid);
- c.get_log_page.lid = log_page;
- c.get_log_page.lsp = lsp;
- c.get_log_page.numdl =
