| Message ID | 20190123040711.21759-1-aik@ozlabs.ru (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [kernel] vfio-pci/nvlink2: Fix ancient gcc warnings | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/build-ppc64le | success | build succeeded & removed 0 sparse warning(s) |
| snowpatch_ozlabs/build-ppc64be | success | build succeeded & removed 0 sparse warning(s) |
| snowpatch_ozlabs/build-ppc64e | success | build succeeded & removed 0 sparse warning(s) |
| snowpatch_ozlabs/build-pmac32 | success | build succeeded & removed 0 sparse warning(s) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 44 lines checked |
Hi Geert,
The below patch comes about from the build regressions and improvements
list you've sent out, but something doesn't add up that we'd be testing
with an old compiler where initialization with { 0 } generates a
"missing braces around initialization" warning. Is this really the
case or are we missing something here? There's no harm that I can see
with Alexey's fix, but are these really just false positives from a
compiler bug that we should selectively ignore if the "fix" is less
clean? Thanks,
Alex
On Wed, 23 Jan 2019 15:07:11 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> Using the {0} construct as a generic initializer is perfectly fine in C,
> however due to a bug in old gcc there is a warning:
>
> + /kisskb/src/drivers/vfio/pci/vfio_pci_nvlink2.c: warning: (near
> initialization for 'cap.header') [-Wmissing-braces]: => 181:9
>
> Since for whatever reason we still want to compile the modern kernel
> with such an old gcc without warnings, this changes the capabilities
> initialization.
>
> The gcc bugzilla: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> drivers/vfio/pci/vfio_pci_nvlink2.c | 30 ++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_nvlink2.c b/drivers/vfio/pci/vfio_pci_nvlink2.c
> index 054a2cf..91d945b 100644
> --- a/drivers/vfio/pci/vfio_pci_nvlink2.c
> +++ b/drivers/vfio/pci/vfio_pci_nvlink2.c
> @@ -178,11 +178,11 @@ static int vfio_pci_nvgpu_add_capability(struct vfio_pci_device *vdev,
> struct vfio_pci_region *region, struct vfio_info_cap *caps)
> {
> struct vfio_pci_nvgpu_data *data = region->data;
> - struct vfio_region_info_cap_nvlink2_ssatgt cap = { 0 };
> -
> - cap.header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT;
> - cap.header.version = 1;
> - cap.tgt = data->gpu_tgt;
> + struct vfio_region_info_cap_nvlink2_ssatgt cap = {
> + .header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT,
> + .header.version = 1,
> + .tgt = data->gpu_tgt
> + };
>
> return vfio_info_add_capability(caps, &cap.header, sizeof(cap));
> }
> @@ -365,18 +365,18 @@ static int vfio_pci_npu2_add_capability(struct vfio_pci_device *vdev,
> struct vfio_pci_region *region, struct vfio_info_cap *caps)
> {
> struct vfio_pci_npu2_data *data = region->data;
> - struct vfio_region_info_cap_nvlink2_ssatgt captgt = { 0 };
> - struct vfio_region_info_cap_nvlink2_lnkspd capspd = { 0 };
> + struct vfio_region_info_cap_nvlink2_ssatgt captgt = {
> + .header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT,
> + .header.version = 1,
> + .tgt = data->gpu_tgt
> + };
> + struct vfio_region_info_cap_nvlink2_lnkspd capspd = {
> + .header.id = VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD,
> + .header.version = 1,
> + .link_speed = data->link_speed
> + };
> int ret;
>
> - captgt.header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT;
> - captgt.header.version = 1;
> - captgt.tgt = data->gpu_tgt;
> -
> - capspd.header.id = VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD;
> - capspd.header.version = 1;
> - capspd.link_speed = data->link_speed;
> -
> ret = vfio_info_add_capability(caps, &captgt.header, sizeof(captgt));
> if (ret)
> return ret;
Hi Alex, On Wed, Jan 23, 2019 at 5:30 AM Alex Williamson <alex.williamson@redhat.com> wrote: > The below patch comes about from the build regressions and improvements > list you've sent out, but something doesn't add up that we'd be testing > with an old compiler where initialization with { 0 } generates a > "missing braces around initialization" warning. Is this really the > case or are we missing something here? There's no harm that I can see > with Alexey's fix, but are these really just false positives from a > compiler bug that we should selectively ignore if the "fix" is less > clean? Thanks, Yes, they are false positives, AFAIK. > On Wed, 23 Jan 2019 15:07:11 +1100 > Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > > > Using the {0} construct as a generic initializer is perfectly fine in C, > > however due to a bug in old gcc there is a warning: > > > > + /kisskb/src/drivers/vfio/pci/vfio_pci_nvlink2.c: warning: (near > > initialization for 'cap.header') [-Wmissing-braces]: => 181:9 These all seem to come from an old gcc 4.6, which is the oldest still supported version for compiling Linux http://kisskb.ellerman.id.au/kisskb/buildresult/13663641/ Note that kisskb is also using gcc 4.6.3 for s390x and mips, which are the only other builds showing missing braces warnings. > > Since for whatever reason we still want to compile the modern kernel > > with such an old gcc without warnings, this changes the capabilities > > initialization. > > > > The gcc bugzilla: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119 > > > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > --- > > drivers/vfio/pci/vfio_pci_nvlink2.c | 30 ++++++++++++++--------------- > > 1 file changed, 15 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/vfio/pci/vfio_pci_nvlink2.c b/drivers/vfio/pci/vfio_pci_nvlink2.c > > index 054a2cf..91d945b 100644 > > --- a/drivers/vfio/pci/vfio_pci_nvlink2.c > > +++ b/drivers/vfio/pci/vfio_pci_nvlink2.c > > @@ -178,11 +178,11 @@ static int vfio_pci_nvgpu_add_capability(struct vfio_pci_device *vdev, > > struct vfio_pci_region *region, struct vfio_info_cap *caps) > > { > > struct vfio_pci_nvgpu_data *data = region->data; > > - struct vfio_region_info_cap_nvlink2_ssatgt cap = { 0 }; > > - > > - cap.header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT; > > - cap.header.version = 1; > > - cap.tgt = data->gpu_tgt; > > + struct vfio_region_info_cap_nvlink2_ssatgt cap = { > > + .header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT, > > + .header.version = 1, > > + .tgt = data->gpu_tgt > > + }; I think the simpler change - struct vfio_region_info_cap_nvlink2_ssatgt cap = { 0 }; + struct vfio_region_info_cap_nvlink2_ssatgt cap = { { 0 } }; should also work (tested with gcc 4.1). That's how most of these were fixed in the past. Gr{oetje,eeting}s, Geert
On Wed, 23 Jan 2019 15:07:11 +1100 Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > Using the {0} construct as a generic initializer is perfectly fine in C, > however due to a bug in old gcc there is a warning: > > + /kisskb/src/drivers/vfio/pci/vfio_pci_nvlink2.c: warning: (near > initialization for 'cap.header') [-Wmissing-braces]: => 181:9 > > Since for whatever reason we still want to compile the modern kernel > with such an old gcc without warnings, this changes the capabilities > initialization. > > The gcc bugzilla: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119 > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> Added Fixes: and Reported-by: tags. Applied to for-linus branch for v5.0. Thanks, Alex > --- > drivers/vfio/pci/vfio_pci_nvlink2.c | 30 ++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/drivers/vfio/pci/vfio_pci_nvlink2.c b/drivers/vfio/pci/vfio_pci_nvlink2.c > index 054a2cf..91d945b 100644 > --- a/drivers/vfio/pci/vfio_pci_nvlink2.c > +++ b/drivers/vfio/pci/vfio_pci_nvlink2.c > @@ -178,11 +178,11 @@ static int vfio_pci_nvgpu_add_capability(struct vfio_pci_device *vdev, > struct vfio_pci_region *region, struct vfio_info_cap *caps) > { > struct vfio_pci_nvgpu_data *data = region->data; > - struct vfio_region_info_cap_nvlink2_ssatgt cap = { 0 }; > - > - cap.header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT; > - cap.header.version = 1; > - cap.tgt = data->gpu_tgt; > + struct vfio_region_info_cap_nvlink2_ssatgt cap = { > + .header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT, > + .header.version = 1, > + .tgt = data->gpu_tgt > + }; > > return vfio_info_add_capability(caps, &cap.header, sizeof(cap)); > } > @@ -365,18 +365,18 @@ static int vfio_pci_npu2_add_capability(struct vfio_pci_device *vdev, > struct vfio_pci_region *region, struct vfio_info_cap *caps) > { > struct vfio_pci_npu2_data *data = region->data; > - struct vfio_region_info_cap_nvlink2_ssatgt captgt = { 0 }; > - struct vfio_region_info_cap_nvlink2_lnkspd capspd = { 0 }; > + struct vfio_region_info_cap_nvlink2_ssatgt captgt = { > + .header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT, > + .header.version = 1, > + .tgt = data->gpu_tgt > + }; > + struct vfio_region_info_cap_nvlink2_lnkspd capspd = { > + .header.id = VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD, > + .header.version = 1, > + .link_speed = data->link_speed > + }; > int ret; > > - captgt.header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT; > - captgt.header.version = 1; > - captgt.tgt = data->gpu_tgt; > - > - capspd.header.id = VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD; > - capspd.header.version = 1; > - capspd.link_speed = data->link_speed; > - > ret = vfio_info_add_capability(caps, &captgt.header, sizeof(captgt)); > if (ret) > return ret;
Geert Uytterhoeven <geert@linux-m68k.org> writes: > Hi Alex, > > On Wed, Jan 23, 2019 at 5:30 AM Alex Williamson > <alex.williamson@redhat.com> wrote: >> The below patch comes about from the build regressions and improvements >> list you've sent out, but something doesn't add up that we'd be testing >> with an old compiler where initialization with { 0 } generates a >> "missing braces around initialization" warning. Is this really the >> case or are we missing something here? There's no harm that I can see >> with Alexey's fix, but are these really just false positives from a >> compiler bug that we should selectively ignore if the "fix" is less >> clean? Thanks, > > Yes, they are false positives, AFAIK. > >> On Wed, 23 Jan 2019 15:07:11 +1100 >> Alexey Kardashevskiy <aik@ozlabs.ru> wrote: >> >> > Using the {0} construct as a generic initializer is perfectly fine in C, >> > however due to a bug in old gcc there is a warning: >> > >> > + /kisskb/src/drivers/vfio/pci/vfio_pci_nvlink2.c: warning: (near >> > initialization for 'cap.header') [-Wmissing-braces]: => 181:9 > > These all seem to come from an old gcc 4.6, which is the oldest still > supported version for compiling Linux > http://kisskb.ellerman.id.au/kisskb/buildresult/13663641/ > > Note that kisskb is also using gcc 4.6.3 for s390x and mips, which are the only > other builds showing missing braces warnings. As documented here: https://www.kernel.org/doc/html/latest/process/changes.html#current-minimal-requirements x86 has effectively dropped support for 4.6 because it doesn't support retpoline and CONFIG_RETPOLINE is default y. So it might be time to stop supporting 4.6, but I'd rather that happened by someone sending a patch to change the requirements doc above, and then kisskb can stop doing the builds with 4.6. cheers
diff --git a/drivers/vfio/pci/vfio_pci_nvlink2.c b/drivers/vfio/pci/vfio_pci_nvlink2.c index 054a2cf..91d945b 100644 --- a/drivers/vfio/pci/vfio_pci_nvlink2.c +++ b/drivers/vfio/pci/vfio_pci_nvlink2.c @@ -178,11 +178,11 @@ static int vfio_pci_nvgpu_add_capability(struct vfio_pci_device *vdev, struct vfio_pci_region *region, struct vfio_info_cap *caps) { struct vfio_pci_nvgpu_data *data = region->data; - struct vfio_region_info_cap_nvlink2_ssatgt cap = { 0 }; - - cap.header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT; - cap.header.version = 1; - cap.tgt = data->gpu_tgt; + struct vfio_region_info_cap_nvlink2_ssatgt cap = { + .header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT, + .header.version = 1, + .tgt = data->gpu_tgt + }; return vfio_info_add_capability(caps, &cap.header, sizeof(cap)); } @@ -365,18 +365,18 @@ static int vfio_pci_npu2_add_capability(struct vfio_pci_device *vdev, struct vfio_pci_region *region, struct vfio_info_cap *caps) { struct vfio_pci_npu2_data *data = region->data; - struct vfio_region_info_cap_nvlink2_ssatgt captgt = { 0 }; - struct vfio_region_info_cap_nvlink2_lnkspd capspd = { 0 }; + struct vfio_region_info_cap_nvlink2_ssatgt captgt = { + .header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT, + .header.version = 1, + .tgt = data->gpu_tgt + }; + struct vfio_region_info_cap_nvlink2_lnkspd capspd = { + .header.id = VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD, + .header.version = 1, + .link_speed = data->link_speed + }; int ret; - captgt.header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT; - captgt.header.version = 1; - captgt.tgt = data->gpu_tgt; - - capspd.header.id = VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD; - capspd.header.version = 1; - capspd.link_speed = data->link_speed; - ret = vfio_info_add_capability(caps, &captgt.header, sizeof(captgt)); if (ret) return ret;
Using the {0} construct as a generic initializer is perfectly fine in C, however due to a bug in old gcc there is a warning: + /kisskb/src/drivers/vfio/pci/vfio_pci_nvlink2.c: warning: (near initialization for 'cap.header') [-Wmissing-braces]: => 181:9 Since for whatever reason we still want to compile the modern kernel with such an old gcc without warnings, this changes the capabilities initialization. The gcc bugzilla: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119 Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- drivers/vfio/pci/vfio_pci_nvlink2.c | 30 ++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)