[bug report] IB/hfi1: Eliminate allocation while atomic

[bug report] IB/hfi1: Eliminate allocation while atomic

[Dan Carpenter]

Hello Don Hiatt,

The patch f8195f3b14a0: "IB/hfi1: Eliminate allocation while atomic"
from Oct 9, 2017, leads to the following static checker warning:

	drivers/infiniband/hw/hfi1/verbs.c:824 build_verbs_tx_desc()
	error: doing dma on the stack (trail_buf)

drivers/infiniband/hw/hfi1/verbs.c
   147  /* Length of buffer to create verbs txreq cache name */
   148  #define TXREQ_NAME_LEN 24
   149  
   150  /* 16B trailing buffer */
   151  static const u8 trail_buf[MAX_16B_PADDING];
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^

This used to be actually allocated on the stack but now it's here.  I
believe this is still a problem.  It's not a problem for most people at
runtime, but it's technically a bug.  And I believe that soon we will
add a check in dma_map_single() which will generate a warning.

   152  
   153  static uint wss_threshold = 80;

[ snip ]

   801          } else {
   802                  ret = sdma_txinit_ahg(
   803                          &tx->txreq,
   804                          ahg_info->tx_flags,
   805                          length,
   806                          ahg_info->ahgidx,
   807                          ahg_info->ahgcount,
   808                          ahg_info->ahgdesc,
   809                          hdrbytes,
   810                          verbs_sdma_complete);
   811                  if (ret)
   812                          goto bail_txadd;
   813          }
   814          /* add the ulp payload - if any. tx->ss can be NULL for acks */
   815          if (tx->ss) {
   816                  ret = build_verbs_ulp_payload(sde, length, tx);
   817                  if (ret)
   818                          goto bail_txadd;
   819          }
   820  
   821          /* add icrc, lt byte, and padding to flit */
   822          if (extra_bytes)
   823                  ret = sdma_txadd_kvaddr(sde->dd, &tx->txreq,
   824                                          (void *)trail_buf, extra_bytes);
                                                        ^^^^^^^^^
This has to be DMAable memory.

   825  
   826  bail_txadd:
   827          return ret;
   828  }


regards,
dan carpenter

Re: [bug report] IB/hfi1: Eliminate allocation while atomic

[Dennis Dalessandro]

On 10/2/2019 8:15 AM, Dan Carpenter wrote:
> Hello Don Hiatt,
> 
> The patch f8195f3b14a0: "IB/hfi1: Eliminate allocation while atomic"
> from Oct 9, 2017, leads to the following static checker warning:
> 
> 	drivers/infiniband/hw/hfi1/verbs.c:824 build_verbs_tx_desc()
> 	error: doing dma on the stack (trail_buf)
> 
> drivers/infiniband/hw/hfi1/verbs.c
>     147  /* Length of buffer to create verbs txreq cache name */
>     148  #define TXREQ_NAME_LEN 24
>     149
>     150  /* 16B trailing buffer */
>     151  static const u8 trail_buf[MAX_16B_PADDING];
>                          ^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> This used to be actually allocated on the stack but now it's here.  I
> believe this is still a problem.  It's not a problem for most people at
> runtime, but it's technically a bug.  And I believe that soon we will
> add a check in dma_map_single() which will generate a warning.
> 
>     152
>     153  static uint wss_threshold = 80;
> 
> [ snip ]
> 
>     801          } else {
>     802                  ret = sdma_txinit_ahg(
>     803                          &tx->txreq,
>     804                          ahg_info->tx_flags,
>     805                          length,
>     806                          ahg_info->ahgidx,
>     807                          ahg_info->ahgcount,
>     808                          ahg_info->ahgdesc,
>     809                          hdrbytes,
>     810                          verbs_sdma_complete);
>     811                  if (ret)
>     812                          goto bail_txadd;
>     813          }
>     814          /* add the ulp payload - if any. tx->ss can be NULL for acks */
>     815          if (tx->ss) {
>     816                  ret = build_verbs_ulp_payload(sde, length, tx);
>     817                  if (ret)
>     818                          goto bail_txadd;
>     819          }
>     820
>     821          /* add icrc, lt byte, and padding to flit */
>     822          if (extra_bytes)
>     823                  ret = sdma_txadd_kvaddr(sde->dd, &tx->txreq,
>     824                                          (void *)trail_buf, extra_bytes);
>                                                          ^^^^^^^^^
> This has to be DMAable memory.
> 
>     825
>     826  bail_txadd:
>     827          return ret;
>     828  }
> 
> 
> regards,
> dan carpenter
> 

Thanks Dan, we actually got a separate out-of-band email about this. We 
are working up a fix right now.

-Denny

RE: [bug report] IB/hfi1: Eliminate allocation while atomic

[Marciniszyn, Mike]

> 
> Thanks Dan, we actually got a separate out-of-band email about this. We
> are working up a fix right now.
> 
> -Denny

Dan,

This just hit the list in https://marc.info/?l=linux-rdma&m=157022217927143&w=2.

Can you take a look?

You mention a change in dma_map_single().

Do you have an details on that?

Mike

Re: [bug report] IB/hfi1: Eliminate allocation while atomic

[Dan Carpenter]

On Fri, Oct 04, 2019 at 09:09:12PM +0000, Marciniszyn, Mike wrote:
> > 
> > Thanks Dan, we actually got a separate out-of-band email about this. We
> > are working up a fix right now.
> > 
> > -Denny
> 
> Dan,
> 
> This just hit the list in https://marc.info/?l=linux-rdma&m=157022217927143&w=2.
> 
> Can you take a look?
> 
> You mention a change in dma_map_single().
> 
> Do you have an details on that?

Kees posted a patch for that after I emailed you.

https://lkml.org/lkml/2019/10/2/866

regards,
dan carpenter