Linux-RDMA Archive on lore.kernel.org
 help / color / Atom feed
* [bug report] IB/hfi1: Eliminate allocation while atomic
@ 2019-10-02 12:15 Dan Carpenter
  2019-10-03 11:58 ` Dennis Dalessandro
  0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2019-10-02 12:15 UTC (permalink / raw)
  To: don.hiatt; +Cc: linux-rdma, Laura Abbott, Greg KH

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

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

* Re: [bug report] IB/hfi1: Eliminate allocation while atomic
  2019-10-02 12:15 [bug report] IB/hfi1: Eliminate allocation while atomic Dan Carpenter
@ 2019-10-03 11:58 ` Dennis Dalessandro
  2019-10-04 21:09   ` Marciniszyn, Mike
  0 siblings, 1 reply; 4+ messages in thread
From: Dennis Dalessandro @ 2019-10-03 11:58 UTC (permalink / raw)
  To: Dan Carpenter, don.hiatt
  Cc: linux-rdma, Laura Abbott, Greg KH, Marciniszyn, Mike

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

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

* RE: [bug report] IB/hfi1: Eliminate allocation while atomic
  2019-10-03 11:58 ` Dennis Dalessandro
@ 2019-10-04 21:09   ` Marciniszyn, Mike
  2019-10-05  5:20     ` Dan Carpenter
  0 siblings, 1 reply; 4+ messages in thread
From: Marciniszyn, Mike @ 2019-10-04 21:09 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: linux-rdma, Laura Abbott, Greg KH, Dalessandro, Dennis, Hiatt, Don

> 
> 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

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

* Re: [bug report] IB/hfi1: Eliminate allocation while atomic
  2019-10-04 21:09   ` Marciniszyn, Mike
@ 2019-10-05  5:20     ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2019-10-05  5:20 UTC (permalink / raw)
  To: Marciniszyn, Mike
  Cc: linux-rdma, Laura Abbott, Greg KH, Dalessandro, Dennis, Hiatt,
	Don, Kees Cook

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


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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-02 12:15 [bug report] IB/hfi1: Eliminate allocation while atomic Dan Carpenter
2019-10-03 11:58 ` Dennis Dalessandro
2019-10-04 21:09   ` Marciniszyn, Mike
2019-10-05  5:20     ` Dan Carpenter

Linux-RDMA Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rdma/0 linux-rdma/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rdma linux-rdma/ https://lore.kernel.org/linux-rdma \
		linux-rdma@vger.kernel.org linux-rdma@archiver.kernel.org
	public-inbox-index linux-rdma

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rdma


AGPL code for this site: git clone https://public-inbox.org/ public-inbox