DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] raw/ioat: fix parameter shadow warning
@ 2021-05-07 17:20 Kevin Laatz
  2021-05-08  7:25 ` Pai G, Sunil
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Kevin Laatz @ 2021-05-07 17:20 UTC (permalink / raw)
  To: dev; +Cc: bruce.richardson, Kevin Laatz, Sunil Pai G

In the function __idxd_completed_ops() we have a parameter shadow warning
due to a local variable having the same name as one of the function
parameters. This is fixed by simply renaming the local variable.

Fixes: 245efe544d8e ("raw/ioat: report status of completed jobs")

Reported-by: Sunil Pai G <sunil.pai.g@intel.com>
Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
---
 drivers/raw/ioat/rte_idxd_rawdev_fns.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/raw/ioat/rte_idxd_rawdev_fns.h b/drivers/raw/ioat/rte_idxd_rawdev_fns.h
index 862e0eb41d..dbd8dfc507 100644
--- a/drivers/raw/ioat/rte_idxd_rawdev_fns.h
+++ b/drivers/raw/ioat/rte_idxd_rawdev_fns.h
@@ -301,11 +301,11 @@ __idxd_completed_ops(int dev_id, uint8_t max_ops, uint32_t *status, uint8_t *num
 		uint16_t idx_to_chk = idxd->batch_idx_ring[idxd->batch_idx_read];
 		volatile struct rte_idxd_completion *comp_to_chk =
 				(struct rte_idxd_completion *)&idxd->desc_ring[idx_to_chk];
-		uint8_t status = comp_to_chk->status;
-		if (status == 0)
+		uint8_t batch_status = comp_to_chk->status;
+		if (batch_status == 0)
 			break;
 		comp_to_chk->status = 0;
-		if (unlikely(status > 1)) {
+		if (unlikely(batch_status > 1)) {
 			/* error occurred somewhere in batch, start where last checked */
 			uint16_t desc_count = comp_to_chk->completed_size;
 			uint16_t batch_start = idxd->hdls_avail;
-- 
2.30.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] raw/ioat: fix parameter shadow warning
  2021-05-07 17:20 [dpdk-dev] [PATCH] raw/ioat: fix parameter shadow warning Kevin Laatz
@ 2021-05-08  7:25 ` Pai G, Sunil
  2021-05-10  9:02 ` Bruce Richardson
  2021-05-10 12:55 ` [dpdk-dev] [PATCH v2] " Kevin Laatz
  2 siblings, 0 replies; 11+ messages in thread
From: Pai G, Sunil @ 2021-05-08  7:25 UTC (permalink / raw)
  To: Laatz, Kevin, dev; +Cc: Richardson, Bruce

<snipped>

Tested-by: Sunil Pai G <sunil.pai.g@intel.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] raw/ioat: fix parameter shadow warning
  2021-05-07 17:20 [dpdk-dev] [PATCH] raw/ioat: fix parameter shadow warning Kevin Laatz
  2021-05-08  7:25 ` Pai G, Sunil
@ 2021-05-10  9:02 ` Bruce Richardson
  2021-05-10 11:07   ` Laatz, Kevin
  2021-05-10 12:55 ` [dpdk-dev] [PATCH v2] " Kevin Laatz
  2 siblings, 1 reply; 11+ messages in thread
From: Bruce Richardson @ 2021-05-10  9:02 UTC (permalink / raw)
  To: Kevin Laatz; +Cc: dev, Sunil Pai G

On Fri, May 07, 2021 at 05:20:25PM +0000, Kevin Laatz wrote:
> In the function __idxd_completed_ops() we have a parameter shadow warning
> due to a local variable having the same name as one of the function
> parameters. This is fixed by simply renaming the local variable.
> 
> Fixes: 245efe544d8e ("raw/ioat: report status of completed jobs")
> 
> Reported-by: Sunil Pai G <sunil.pai.g@intel.com>
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> ---
Please provide details in the commit log as to how/when this was seen. I
believe this issue was seen only with OVS because it passes the -Wshadow
flag when building - something DPDK probably should do, but doesn't. Is
that correct?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] raw/ioat: fix parameter shadow warning
  2021-05-10  9:02 ` Bruce Richardson
@ 2021-05-10 11:07   ` Laatz, Kevin
  0 siblings, 0 replies; 11+ messages in thread
From: Laatz, Kevin @ 2021-05-10 11:07 UTC (permalink / raw)
  To: Richardson, Bruce; +Cc: dev, Pai G, Sunil


> On Fri, May 07, 2021 at 05:20:25PM +0000, Kevin Laatz wrote:
> > In the function __idxd_completed_ops() we have a parameter shadow
> warning
> > due to a local variable having the same name as one of the function
> > parameters. This is fixed by simply renaming the local variable.
> >
> > Fixes: 245efe544d8e ("raw/ioat: report status of completed jobs")
> >
> > Reported-by: Sunil Pai G <sunil.pai.g@intel.com>
> > Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> > ---
> Please provide details in the commit log as to how/when this was seen. I
> believe this issue was seen only with OVS because it passes the -Wshadow
> flag when building - something DPDK probably should do, but doesn't. Is
> that correct?

Correct, the OVS build has -Wshadow by default. When -Wshadow is
passed to the DPDK build, this warning is also visible.

Will send a V2 to include details in commit log.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v2] raw/ioat: fix parameter shadow warning
  2021-05-07 17:20 [dpdk-dev] [PATCH] raw/ioat: fix parameter shadow warning Kevin Laatz
  2021-05-08  7:25 ` Pai G, Sunil
  2021-05-10  9:02 ` Bruce Richardson
@ 2021-05-10 12:55 ` Kevin Laatz
  2021-05-10 13:36   ` Bruce Richardson
                     ` (2 more replies)
  2 siblings, 3 replies; 11+ messages in thread
From: Kevin Laatz @ 2021-05-10 12:55 UTC (permalink / raw)
  To: dev; +Cc: bruce.richardson, Kevin Laatz, Sunil Pai G

In the function __idxd_completed_ops() we have a parameter shadow warning
due to a local variable having the same name as one of the function
parameters. This issue is fixed by simply renaming the local variable.

This warning was discovered during an OVS build with DPDK 21.05-rc2. The
OVS build passes the -Wshadow flag by default, allowing the warning to be
seen.

Fixes: 245efe544d8e ("raw/ioat: report status of completed jobs")

Reported-by: Sunil Pai G <sunil.pai.g@intel.com>
Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
Tested-by: Sunil Pai G <sunil.pai.g@intel.com>

---
v2: add details of warning discovery
---
 drivers/raw/ioat/rte_idxd_rawdev_fns.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/raw/ioat/rte_idxd_rawdev_fns.h b/drivers/raw/ioat/rte_idxd_rawdev_fns.h
index 862e0eb41d..dbd8dfc507 100644
--- a/drivers/raw/ioat/rte_idxd_rawdev_fns.h
+++ b/drivers/raw/ioat/rte_idxd_rawdev_fns.h
@@ -301,11 +301,11 @@ __idxd_completed_ops(int dev_id, uint8_t max_ops, uint32_t *status, uint8_t *num
 		uint16_t idx_to_chk = idxd->batch_idx_ring[idxd->batch_idx_read];
 		volatile struct rte_idxd_completion *comp_to_chk =
 				(struct rte_idxd_completion *)&idxd->desc_ring[idx_to_chk];
-		uint8_t status = comp_to_chk->status;
-		if (status == 0)
+		uint8_t batch_status = comp_to_chk->status;
+		if (batch_status == 0)
 			break;
 		comp_to_chk->status = 0;
-		if (unlikely(status > 1)) {
+		if (unlikely(batch_status > 1)) {
 			/* error occurred somewhere in batch, start where last checked */
 			uint16_t desc_count = comp_to_chk->completed_size;
 			uint16_t batch_start = idxd->hdls_avail;
-- 
2.30.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v2] raw/ioat: fix parameter shadow warning
  2021-05-10 12:55 ` [dpdk-dev] [PATCH v2] " Kevin Laatz
@ 2021-05-10 13:36   ` Bruce Richardson
  2021-05-10 14:06   ` David Marchand
  2021-05-12 10:47   ` [dpdk-dev] [PATCH v3] " Kevin Laatz
  2 siblings, 0 replies; 11+ messages in thread
From: Bruce Richardson @ 2021-05-10 13:36 UTC (permalink / raw)
  To: Kevin Laatz; +Cc: dev, Sunil Pai G

On Mon, May 10, 2021 at 12:55:14PM +0000, Kevin Laatz wrote:
> In the function __idxd_completed_ops() we have a parameter shadow warning
> due to a local variable having the same name as one of the function
> parameters. This issue is fixed by simply renaming the local variable.
> 
> This warning was discovered during an OVS build with DPDK 21.05-rc2. The
> OVS build passes the -Wshadow flag by default, allowing the warning to be
> seen.
> 
> Fixes: 245efe544d8e ("raw/ioat: report status of completed jobs")
> 
> Reported-by: Sunil Pai G <sunil.pai.g@intel.com>
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> Tested-by: Sunil Pai G <sunil.pai.g@intel.com>
> 
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v2] raw/ioat: fix parameter shadow warning
  2021-05-10 12:55 ` [dpdk-dev] [PATCH v2] " Kevin Laatz
  2021-05-10 13:36   ` Bruce Richardson
@ 2021-05-10 14:06   ` David Marchand
  2021-05-10 14:48     ` Bruce Richardson
  2021-05-12 10:47   ` [dpdk-dev] [PATCH v3] " Kevin Laatz
  2 siblings, 1 reply; 11+ messages in thread
From: David Marchand @ 2021-05-10 14:06 UTC (permalink / raw)
  To: Kevin Laatz; +Cc: dev, Bruce Richardson, Sunil Pai G

On Mon, May 10, 2021 at 2:55 PM Kevin Laatz <kevin.laatz@intel.com> wrote:
>
> In the function __idxd_completed_ops() we have a parameter shadow warning
> due to a local variable having the same name as one of the function
> parameters. This issue is fixed by simply renaming the local variable.
>
> This warning was discovered during an OVS build with DPDK 21.05-rc2. The
> OVS build passes the -Wshadow flag by default, allowing the warning to be
> seen.

A bit confusing.
-Wshadow only affects OVS code and there is no code calling this in
the OVS master branch.

I did not see this issue while updating my dpdk-latest OVS branch and
running builds in GHA.
So I guess Sunil caught it with his patch:
https://patchwork.ozlabs.org/project/openvswitch/patch/20201023094845.35652-2-sunil.pai.g@intel.com/


-- 
David Marchand


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v2] raw/ioat: fix parameter shadow warning
  2021-05-10 14:06   ` David Marchand
@ 2021-05-10 14:48     ` Bruce Richardson
  2021-05-11 20:49       ` Thomas Monjalon
  0 siblings, 1 reply; 11+ messages in thread
From: Bruce Richardson @ 2021-05-10 14:48 UTC (permalink / raw)
  To: David Marchand; +Cc: Kevin Laatz, dev, Sunil Pai G

On Mon, May 10, 2021 at 04:06:00PM +0200, David Marchand wrote:
> On Mon, May 10, 2021 at 2:55 PM Kevin Laatz <kevin.laatz@intel.com> wrote:
> >
> > In the function __idxd_completed_ops() we have a parameter shadow warning
> > due to a local variable having the same name as one of the function
> > parameters. This issue is fixed by simply renaming the local variable.
> >
> > This warning was discovered during an OVS build with DPDK 21.05-rc2. The
> > OVS build passes the -Wshadow flag by default, allowing the warning to be
> > seen.
> 
> A bit confusing.
> -Wshadow only affects OVS code and there is no code calling this in
> the OVS master branch.
> 
> I did not see this issue while updating my dpdk-latest OVS branch and
> running builds in GHA.
> So I guess Sunil caught it with his patch:
> https://patchwork.ozlabs.org/project/openvswitch/patch/20201023094845.35652-2-sunil.pai.g@intel.com/
>
Yes, it was caught by Sunil in the course of his work.

Ideally, I think -Wshadow would be a good flag to add to our DPDK builds,
but it causes quite a number of errors right now to do so. Hopefully in a
future release.

/Bruce

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v2] raw/ioat: fix parameter shadow warning
  2021-05-10 14:48     ` Bruce Richardson
@ 2021-05-11 20:49       ` Thomas Monjalon
  0 siblings, 0 replies; 11+ messages in thread
From: Thomas Monjalon @ 2021-05-11 20:49 UTC (permalink / raw)
  To: Kevin Laatz, Sunil Pai G; +Cc: David Marchand, dev, dev, Bruce Richardson

10/05/2021 16:48, Bruce Richardson:
> On Mon, May 10, 2021 at 04:06:00PM +0200, David Marchand wrote:
> > On Mon, May 10, 2021 at 2:55 PM Kevin Laatz <kevin.laatz@intel.com> wrote:
> > >
> > > In the function __idxd_completed_ops() we have a parameter shadow warning
> > > due to a local variable having the same name as one of the function
> > > parameters. This issue is fixed by simply renaming the local variable.
> > >
> > > This warning was discovered during an OVS build with DPDK 21.05-rc2. The
> > > OVS build passes the -Wshadow flag by default, allowing the warning to be
> > > seen.
> > 
> > A bit confusing.
> > -Wshadow only affects OVS code and there is no code calling this in
> > the OVS master branch.
> > 
> > I did not see this issue while updating my dpdk-latest OVS branch and
> > running builds in GHA.
> > So I guess Sunil caught it with his patch:
> > https://patchwork.ozlabs.org/project/openvswitch/patch/20201023094845.35652-2-sunil.pai.g@intel.com/
> >
> Yes, it was caught by Sunil in the course of his work.

So the commit message should be fixed please.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v3] raw/ioat: fix parameter shadow warning
  2021-05-10 12:55 ` [dpdk-dev] [PATCH v2] " Kevin Laatz
  2021-05-10 13:36   ` Bruce Richardson
  2021-05-10 14:06   ` David Marchand
@ 2021-05-12 10:47   ` Kevin Laatz
  2021-05-12 12:59     ` Thomas Monjalon
  2 siblings, 1 reply; 11+ messages in thread
From: Kevin Laatz @ 2021-05-12 10:47 UTC (permalink / raw)
  To: dev; +Cc: bruce.richardson, thomas, david.marchand, Kevin Laatz, Sunil Pai G

In the function __idxd_completed_ops() we have a parameter shadow warning
due to a local variable having the same name as one of the function
parameters. This issue is fixed by simply renaming the local variable.

This warning was caught when additions were made to the OVS codebase,
which include adding calls the IOAT APIs. The OVS build passes the
-Wshadow flag by default, allowing the warning to be seen when building
OVS with DPDK 21.05-rc2.

Fixes: 245efe544d8e ("raw/ioat: report status of completed jobs")

Reported-by: Sunil Pai G <sunil.pai.g@intel.com>
Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
Tested-by: Sunil Pai G <sunil.pai.g@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

---
v2: add details of warning discovery
v3: commit log update
---
 drivers/raw/ioat/rte_idxd_rawdev_fns.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/raw/ioat/rte_idxd_rawdev_fns.h b/drivers/raw/ioat/rte_idxd_rawdev_fns.h
index 862e0eb41d..dbd8dfc507 100644
--- a/drivers/raw/ioat/rte_idxd_rawdev_fns.h
+++ b/drivers/raw/ioat/rte_idxd_rawdev_fns.h
@@ -301,11 +301,11 @@ __idxd_completed_ops(int dev_id, uint8_t max_ops, uint32_t *status, uint8_t *num
 		uint16_t idx_to_chk = idxd->batch_idx_ring[idxd->batch_idx_read];
 		volatile struct rte_idxd_completion *comp_to_chk =
 				(struct rte_idxd_completion *)&idxd->desc_ring[idx_to_chk];
-		uint8_t status = comp_to_chk->status;
-		if (status == 0)
+		uint8_t batch_status = comp_to_chk->status;
+		if (batch_status == 0)
 			break;
 		comp_to_chk->status = 0;
-		if (unlikely(status > 1)) {
+		if (unlikely(batch_status > 1)) {
 			/* error occurred somewhere in batch, start where last checked */
 			uint16_t desc_count = comp_to_chk->completed_size;
 			uint16_t batch_start = idxd->hdls_avail;
-- 
2.30.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v3] raw/ioat: fix parameter shadow warning
  2021-05-12 10:47   ` [dpdk-dev] [PATCH v3] " Kevin Laatz
@ 2021-05-12 12:59     ` Thomas Monjalon
  0 siblings, 0 replies; 11+ messages in thread
From: Thomas Monjalon @ 2021-05-12 12:59 UTC (permalink / raw)
  To: Kevin Laatz; +Cc: dev, bruce.richardson, david.marchand, Sunil Pai G

12/05/2021 12:47, Kevin Laatz:
> In the function __idxd_completed_ops() we have a parameter shadow warning
> due to a local variable having the same name as one of the function
> parameters. This issue is fixed by simply renaming the local variable.
> 
> This warning was caught when additions were made to the OVS codebase,
> which include adding calls the IOAT APIs. The OVS build passes the
> -Wshadow flag by default, allowing the warning to be seen when building
> OVS with DPDK 21.05-rc2.
> 
> Fixes: 245efe544d8e ("raw/ioat: report status of completed jobs")
> 
> Reported-by: Sunil Pai G <sunil.pai.g@intel.com>
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> Tested-by: Sunil Pai G <sunil.pai.g@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Applied, thanks.




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-05-12 13:00 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-07 17:20 [dpdk-dev] [PATCH] raw/ioat: fix parameter shadow warning Kevin Laatz
2021-05-08  7:25 ` Pai G, Sunil
2021-05-10  9:02 ` Bruce Richardson
2021-05-10 11:07   ` Laatz, Kevin
2021-05-10 12:55 ` [dpdk-dev] [PATCH v2] " Kevin Laatz
2021-05-10 13:36   ` Bruce Richardson
2021-05-10 14:06   ` David Marchand
2021-05-10 14:48     ` Bruce Richardson
2021-05-11 20:49       ` Thomas Monjalon
2021-05-12 10:47   ` [dpdk-dev] [PATCH v3] " Kevin Laatz
2021-05-12 12:59     ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).