* [PATCH net] gve: DQO: Suppress unused var warnings
@ 2021-07-23 23:19 Bailey Forrest
2021-09-27 9:58 ` Arnd Bergmann
0 siblings, 1 reply; 7+ messages in thread
From: Bailey Forrest @ 2021-07-23 23:19 UTC (permalink / raw)
To: Bailey Forrest, David S . Miller, Jakub Kicinski, Arnd Bergmann; +Cc: netdev
Some variables become unused when `CONFIG_NEED_DMA_MAP_STATE=n`.
We only suppress when `CONFIG_NEED_DMA_MAP_STATE=n` in order to avoid
false negatives.
Fixes: a57e5de476be ("gve: DQO: Add TX path")
Signed-off-by: Bailey Forrest <bcf@google.com>
---
drivers/net/ethernet/google/gve/gve_tx_dqo.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/google/gve/gve_tx_dqo.c b/drivers/net/ethernet/google/gve/gve_tx_dqo.c
index 05ddb6a75c38..f873321c022e 100644
--- a/drivers/net/ethernet/google/gve/gve_tx_dqo.c
+++ b/drivers/net/ethernet/google/gve/gve_tx_dqo.c
@@ -87,6 +87,9 @@ static void gve_tx_clean_pending_packets(struct gve_tx_ring *tx)
for (j = 0; j < cur_state->num_bufs; j++) {
struct gve_tx_dma_buf *buf = &cur_state->bufs[j];
+#ifndef CONFIG_NEED_DMA_MAP_STATE
+ (void)buf; // Suppress unused variable.
+#endif
if (j == 0) {
dma_unmap_single(tx->dev,
dma_unmap_addr(buf, dma),
@@ -459,9 +462,14 @@ static int gve_tx_add_skb_no_copy_dqo(struct gve_tx_ring *tx,
struct gve_tx_pending_packet_dqo *pending_packet;
struct gve_tx_metadata_dqo metadata;
+ struct gve_tx_dma_buf *buf;
s16 completion_tag;
int i;
+#ifndef CONFIG_NEED_DMA_MAP_STATE
+ (void)buf; // Suppress unused variable.
+#endif
+
pending_packet = gve_alloc_pending_packet(tx);
pending_packet->skb = skb;
pending_packet->num_bufs = 0;
@@ -493,8 +501,6 @@ static int gve_tx_add_skb_no_copy_dqo(struct gve_tx_ring *tx,
/* Map the linear portion of skb */
{
- struct gve_tx_dma_buf *buf =
- &pending_packet->bufs[pending_packet->num_bufs];
u32 len = skb_headlen(skb);
dma_addr_t addr;
@@ -502,6 +508,7 @@ static int gve_tx_add_skb_no_copy_dqo(struct gve_tx_ring *tx,
if (unlikely(dma_mapping_error(tx->dev, addr)))
goto err;
+ buf = &pending_packet->bufs[pending_packet->num_bufs];
dma_unmap_len_set(buf, len, len);
dma_unmap_addr_set(buf, dma, addr);
++pending_packet->num_bufs;
@@ -512,8 +519,6 @@ static int gve_tx_add_skb_no_copy_dqo(struct gve_tx_ring *tx,
}
for (i = 0; i < shinfo->nr_frags; i++) {
- struct gve_tx_dma_buf *buf =
- &pending_packet->bufs[pending_packet->num_bufs];
const skb_frag_t *frag = &shinfo->frags[i];
bool is_eop = i == (shinfo->nr_frags - 1);
u32 len = skb_frag_size(frag);
@@ -523,6 +528,7 @@ static int gve_tx_add_skb_no_copy_dqo(struct gve_tx_ring *tx,
if (unlikely(dma_mapping_error(tx->dev, addr)))
goto err;
+ buf = &pending_packet->bufs[pending_packet->num_bufs];
dma_unmap_len_set(buf, len, len);
dma_unmap_addr_set(buf, dma, addr);
++pending_packet->num_bufs;
@@ -555,6 +561,9 @@ static int gve_tx_add_skb_no_copy_dqo(struct gve_tx_ring *tx,
for (i = 0; i < pending_packet->num_bufs; i++) {
struct gve_tx_dma_buf *buf = &pending_packet->bufs[i];
+#ifndef CONFIG_NEED_DMA_MAP_STATE
+ (void)buf; // Suppress unused variable.
+#endif
if (i == 0) {
dma_unmap_single(tx->dev, dma_unmap_addr(buf, dma),
dma_unmap_len(buf, len),
--
2.32.0.432.gabb21c7263-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net] gve: DQO: Suppress unused var warnings
2021-07-23 23:19 [PATCH net] gve: DQO: Suppress unused var warnings Bailey Forrest
@ 2021-09-27 9:58 ` Arnd Bergmann
2021-09-27 20:21 ` Bailey Forrest
0 siblings, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2021-09-27 9:58 UTC (permalink / raw)
To: Bailey Forrest; +Cc: David S . Miller, Jakub Kicinski, Networking
On Sat, Jul 24, 2021 at 1:19 AM Bailey Forrest <bcf@google.com> wrote:
>
> Some variables become unused when `CONFIG_NEED_DMA_MAP_STATE=n`.
>
> We only suppress when `CONFIG_NEED_DMA_MAP_STATE=n` in order to avoid
> false negatives.
>
> Fixes: a57e5de476be ("gve: DQO: Add TX path")
> Signed-off-by: Bailey Forrest <bcf@google.com>
Hi Bailey,
I see that the warning still exists in linux-5.15-rc3 and net-next,
I'm building with my original patch[1] to get around the -Werror
warnings.
Can you resend your patch, or should I resend mine after all?
Arnd
[1] https://lore.kernel.org/all/20210721151100.2042139-1-arnd@kernel.org/
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] gve: DQO: Suppress unused var warnings
2021-09-27 9:58 ` Arnd Bergmann
@ 2021-09-27 20:21 ` Bailey Forrest
2021-09-27 23:21 ` Jakub Kicinski
0 siblings, 1 reply; 7+ messages in thread
From: Bailey Forrest @ 2021-09-27 20:21 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: David S . Miller, Jakub Kicinski, Networking
Apologies, resending as text
On Mon, Sep 27, 2021 at 2:59 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> On Sat, Jul 24, 2021 at 1:19 AM Bailey Forrest <bcf@google.com> wrote:
> >
> > Some variables become unused when `CONFIG_NEED_DMA_MAP_STATE=n`.
> >
> > We only suppress when `CONFIG_NEED_DMA_MAP_STATE=n` in order to avoid
> > false negatives.
> >
> > Fixes: a57e5de476be ("gve: DQO: Add TX path")
> > Signed-off-by: Bailey Forrest <bcf@google.com>
>
> Hi Bailey,
>
> I see that the warning still exists in linux-5.15-rc3 and net-next,
> I'm building with my original patch[1] to get around the -Werror
> warnings.
>
> Can you resend your patch, or should I resend mine after all?
>
> Arnd
>
> [1] https://lore.kernel.org/all/20210721151100.2042139-1-arnd@kernel.org/
Hi David/Jakub,
Any thoughts on my patch? I'm open to alternative suggestions for how
to resolve this.
This patch still works and merges cleanly on HEAD.
Thanks,
Bailey
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] gve: DQO: Suppress unused var warnings
2021-09-27 20:21 ` Bailey Forrest
@ 2021-09-27 23:21 ` Jakub Kicinski
2021-09-27 23:59 ` Bailey Forrest
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2021-09-27 23:21 UTC (permalink / raw)
To: Bailey Forrest; +Cc: Arnd Bergmann, David S . Miller, Networking
On Mon, 27 Sep 2021 13:21:30 -0700 Bailey Forrest wrote:
> Apologies, resending as text
>
> On Mon, Sep 27, 2021 at 2:59 AM Arnd Bergmann <arnd@kernel.org> wrote:
> >
> > On Sat, Jul 24, 2021 at 1:19 AM Bailey Forrest <bcf@google.com> wrote:
> > >
> > > Some variables become unused when `CONFIG_NEED_DMA_MAP_STATE=n`.
> > >
> > > We only suppress when `CONFIG_NEED_DMA_MAP_STATE=n` in order to avoid
> > > false negatives.
> > >
> > > Fixes: a57e5de476be ("gve: DQO: Add TX path")
> > > Signed-off-by: Bailey Forrest <bcf@google.com>
> >
> > Hi Bailey,
> >
> > I see that the warning still exists in linux-5.15-rc3 and net-next,
> > I'm building with my original patch[1] to get around the -Werror
> > warnings.
> >
> > Can you resend your patch, or should I resend mine after all?
> >
> > Arnd
> >
> > [1] https://lore.kernel.org/all/20210721151100.2042139-1-arnd@kernel.org/
>
> Hi David/Jakub,
>
> Any thoughts on my patch? I'm open to alternative suggestions for how
> to resolve this.
>
> This patch still works and merges cleanly on HEAD.
Looks like fixing this on the wrong end, dma_unmap_len_set()
and friends should always evaluate their arguments.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] gve: DQO: Suppress unused var warnings
2021-09-27 23:21 ` Jakub Kicinski
@ 2021-09-27 23:59 ` Bailey Forrest
2021-09-28 14:15 ` Arnd Bergmann
0 siblings, 1 reply; 7+ messages in thread
From: Bailey Forrest @ 2021-09-27 23:59 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Arnd Bergmann, David S . Miller, Networking
On Mon, Sep 27, 2021 at 4:21 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 27 Sep 2021 13:21:30 -0700 Bailey Forrest wrote:
> > Apologies, resending as text
> >
> > On Mon, Sep 27, 2021 at 2:59 AM Arnd Bergmann <arnd@kernel.org> wrote:
> > >
> > > On Sat, Jul 24, 2021 at 1:19 AM Bailey Forrest <bcf@google.com> wrote:
> > > >
> > > > Some variables become unused when `CONFIG_NEED_DMA_MAP_STATE=n`.
> > > >
> > > > We only suppress when `CONFIG_NEED_DMA_MAP_STATE=n` in order to avoid
> > > > false negatives.
> > > >
> > > > Fixes: a57e5de476be ("gve: DQO: Add TX path")
> > > > Signed-off-by: Bailey Forrest <bcf@google.com>
> > >
> > > Hi Bailey,
> > >
> > > I see that the warning still exists in linux-5.15-rc3 and net-next,
> > > I'm building with my original patch[1] to get around the -Werror
> > > warnings.
> > >
> > > Can you resend your patch, or should I resend mine after all?
> > >
> > > Arnd
> > >
> > > [1] https://lore.kernel.org/all/20210721151100.2042139-1-arnd@kernel.org/
> >
> > Hi David/Jakub,
> >
> > Any thoughts on my patch? I'm open to alternative suggestions for how
> > to resolve this.
> >
> > This patch still works and merges cleanly on HEAD.
>
> Looks like fixing this on the wrong end, dma_unmap_len_set()
> and friends should always evaluate their arguments.
That makes sense.
Arnd, if you want to fix this inside of the dma_* macros, the
following diff resolves the errors reported for this driver:
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index dca2b1355bb1..f51eee0f678e 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -590,10 +590,21 @@ static inline int dma_mmap_wc(struct device *dev,
#else
#define DEFINE_DMA_UNMAP_ADDR(ADDR_NAME)
#define DEFINE_DMA_UNMAP_LEN(LEN_NAME)
-#define dma_unmap_addr(PTR, ADDR_NAME) (0)
-#define dma_unmap_addr_set(PTR, ADDR_NAME, VAL) do { } while (0)
-#define dma_unmap_len(PTR, LEN_NAME) (0)
-#define dma_unmap_len_set(PTR, LEN_NAME, VAL) do { } while (0)
+
+#define dma_unmap_addr(PTR, ADDR_NAME) ({ (void)PTR; 0; })
+
+#define dma_unmap_addr_set(PTR, ADDR_NAME, VAL) do { \
+ (void)PTR; \
+ (void)VAL; \
+} while (0)
+
+#define dma_unmap_len(PTR, LEN_NAME) ({ (void)PTR; 0; })
+
+#define dma_unmap_len_set(PTR, LEN_NAME, VAL) do { \
+ (void)PTR; \
+ (void)VAL; \
+} while (0)
+
#endif
#endif /* _LINUX_DMA_MAPPING_H */
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net] gve: DQO: Suppress unused var warnings
2021-09-27 23:59 ` Bailey Forrest
@ 2021-09-28 14:15 ` Arnd Bergmann
2021-09-28 20:04 ` Bailey Forrest
0 siblings, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2021-09-28 14:15 UTC (permalink / raw)
To: Bailey Forrest; +Cc: Jakub Kicinski, David S . Miller, Networking
On Tue, Sep 28, 2021 at 2:00 AM Bailey Forrest <bcf@google.com> wrote:
> On Mon, Sep 27, 2021 at 4:21 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > On Mon, 27 Sep 2021 13:21:30 -0700 Bailey Forrest wrote:
> >
> > Looks like fixing this on the wrong end, dma_unmap_len_set()
> > and friends should always evaluate their arguments.
>
> That makes sense.
>
> Arnd, if you want to fix this inside of the dma_* macros, the
> following diff resolves the errors reported for this driver:
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index dca2b1355bb1..f51eee0f678e 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -590,10 +590,21 @@ static inline int dma_mmap_wc(struct device *dev,
> #else
> #define DEFINE_DMA_UNMAP_ADDR(ADDR_NAME)
> #define DEFINE_DMA_UNMAP_LEN(LEN_NAME)
> -#define dma_unmap_addr(PTR, ADDR_NAME) (0)
> -#define dma_unmap_addr_set(PTR, ADDR_NAME, VAL) do { } while (0)
> -#define dma_unmap_len(PTR, LEN_NAME) (0)
> -#define dma_unmap_len_set(PTR, LEN_NAME, VAL) do { } while (0)
> +
> +#define dma_unmap_addr(PTR, ADDR_NAME) ({ (void)PTR; 0; })
> +
> +#define dma_unmap_addr_set(PTR, ADDR_NAME, VAL) do { \
Unfortunately, this breaks every other driver using these macros, as the
point of them is that the unmap-address is not actually defined
and not taking up space in data structure. Referencing it by name
is a compile-time bug.
I've come up with a new patch to gve that just removes the
"struct gve_tx_dma_buf" and open-codes the access everywhere,
sending that as v2 now. Feel free to take that and modify as needed
before sending on, or doing yet another patch.
Arnd
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] gve: DQO: Suppress unused var warnings
2021-09-28 14:15 ` Arnd Bergmann
@ 2021-09-28 20:04 ` Bailey Forrest
0 siblings, 0 replies; 7+ messages in thread
From: Bailey Forrest @ 2021-09-28 20:04 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Jakub Kicinski, David S . Miller, Networking
On Tue, Sep 28, 2021 at 7:15 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> On Tue, Sep 28, 2021 at 2:00 AM Bailey Forrest <bcf@google.com> wrote:
> > On Mon, Sep 27, 2021 at 4:21 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > > On Mon, 27 Sep 2021 13:21:30 -0700 Bailey Forrest wrote:
> > >
> > > Looks like fixing this on the wrong end, dma_unmap_len_set()
> > > and friends should always evaluate their arguments.
> >
> > That makes sense.
> >
> > Arnd, if you want to fix this inside of the dma_* macros, the
> > following diff resolves the errors reported for this driver:
>
> > diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> > index dca2b1355bb1..f51eee0f678e 100644
> > --- a/include/linux/dma-mapping.h
> > +++ b/include/linux/dma-mapping.h
> > @@ -590,10 +590,21 @@ static inline int dma_mmap_wc(struct device *dev,
> > #else
> > #define DEFINE_DMA_UNMAP_ADDR(ADDR_NAME)
> > #define DEFINE_DMA_UNMAP_LEN(LEN_NAME)
> > -#define dma_unmap_addr(PTR, ADDR_NAME) (0)
> > -#define dma_unmap_addr_set(PTR, ADDR_NAME, VAL) do { } while (0)
> > -#define dma_unmap_len(PTR, LEN_NAME) (0)
> > -#define dma_unmap_len_set(PTR, LEN_NAME, VAL) do { } while (0)
> > +
> > +#define dma_unmap_addr(PTR, ADDR_NAME) ({ (void)PTR; 0; })
> > +
> > +#define dma_unmap_addr_set(PTR, ADDR_NAME, VAL) do { \
>
> Unfortunately, this breaks every other driver using these macros, as the
> point of them is that the unmap-address is not actually defined
> and not taking up space in data structure. Referencing it by name
> is a compile-time bug.
My patch works with a small modification:
```
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index dca2b1355bb1..04ca5467562e 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -590,10 +590,10 @@ static inline int dma_mmap_wc(struct device *dev,
#else
#define DEFINE_DMA_UNMAP_ADDR(ADDR_NAME)
#define DEFINE_DMA_UNMAP_LEN(LEN_NAME)
-#define dma_unmap_addr(PTR, ADDR_NAME) (0)
-#define dma_unmap_addr_set(PTR, ADDR_NAME, VAL) do { } while (0)
-#define dma_unmap_len(PTR, LEN_NAME) (0)
-#define dma_unmap_len_set(PTR, LEN_NAME, VAL) do { } while (0)
+#define dma_unmap_addr(PTR, ADDR_NAME) ({ (void)PTR; 0; })
+#define dma_unmap_addr_set(PTR, ADDR_NAME, VAL) do { (void)PTR; } while (0)
+#define dma_unmap_len(PTR, LEN_NAME) ({ (void)PTR; 0; })
+#define dma_unmap_len_set(PTR, LEN_NAME, VAL) do { (void)PTR; } while (0)
#endif
#endif /* _LINUX_DMA_MAPPING_H */
```
To me, this is still the preferred solution. However, your latest
patch looked fine to me.
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-09-28 20:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-23 23:19 [PATCH net] gve: DQO: Suppress unused var warnings Bailey Forrest
2021-09-27 9:58 ` Arnd Bergmann
2021-09-27 20:21 ` Bailey Forrest
2021-09-27 23:21 ` Jakub Kicinski
2021-09-27 23:59 ` Bailey Forrest
2021-09-28 14:15 ` Arnd Bergmann
2021-09-28 20:04 ` Bailey Forrest
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).