| Message ID | 20190909132945.30462-1-bmt@zurich.ibm.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 4db8fd4973329810c8549b08df12907e6fe921c2 |
| Headers | show |
| Series | [v1,rdma-next] RDMA/siw: Relax from kmap_atomic() use in TX path | expand |
On Monday, September 09/09/19, 2019 at 18:59:45 +0530, Bernard Metzler wrote: > Since the transmit path is never executed in an atomic > context, we do not need kmap_atomic() and can always > use less demanding kmap(). > > Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com> > --- > drivers/infiniband/sw/siw/siw_qp_tx.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/infiniband/sw/siw/siw_qp_tx.c b/drivers/infiniband/sw/siw/siw_qp_tx.c > index 8e72f955921d..5d97bba0ce6d 100644 > --- a/drivers/infiniband/sw/siw/siw_qp_tx.c > +++ b/drivers/infiniband/sw/siw/siw_qp_tx.c > @@ -76,16 +76,15 @@ static int siw_try_1seg(struct siw_iwarp_tx *c_tx, void *paddr) > if (unlikely(!p)) > return -EFAULT; > > - buffer = kmap_atomic(p); > + buffer = kmap(p); > > if (likely(PAGE_SIZE - off >= bytes)) { > memcpy(paddr, buffer + off, bytes); > - kunmap_atomic(buffer); missing kunmap() > } else { > unsigned long part = bytes - (PAGE_SIZE - off); > > memcpy(paddr, buffer + off, part); > - kunmap_atomic(buffer); > + kunmap(p); kunmap(buffer) > > if (!mem->is_pbl) > p = siw_get_upage(mem->umem, > @@ -97,11 +96,10 @@ static int siw_try_1seg(struct siw_iwarp_tx *c_tx, void *paddr) > if (unlikely(!p)) > return -EFAULT; > > - buffer = kmap_atomic(p); > - memcpy(paddr + part, buffer, > - bytes - part); > - kunmap_atomic(buffer); > + buffer = kmap(p); > + memcpy(paddr + part, buffer, bytes - part); > } > + kunmap(p); Can this be out of if()? the buffers seem to be different. > } > } > return (int)bytes; > -- > 2.17.2 >
-----"Potnuri Bharat Teja" <bharat@chelsio.com> wrote: ----- >To: "Bernard Metzler" <bmt@zurich.ibm.com> >From: "Potnuri Bharat Teja" <bharat@chelsio.com> >Date: 09/09/2019 04:54PM >Cc: "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>, >"dledford@redhat.com" <dledford@redhat.com> >Subject: [EXTERNAL] Re: [PATCH v1 rdma-next] RDMA/siw: Relax from >kmap_atomic() use in TX path > >On Monday, September 09/09/19, 2019 at 18:59:45 +0530, Bernard >Metzler wrote: >> Since the transmit path is never executed in an atomic >> context, we do not need kmap_atomic() and can always >> use less demanding kmap(). >> >> Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com> >> --- >> drivers/infiniband/sw/siw/siw_qp_tx.c | 12 +++++------- >> 1 file changed, 5 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/infiniband/sw/siw/siw_qp_tx.c >b/drivers/infiniband/sw/siw/siw_qp_tx.c >> index 8e72f955921d..5d97bba0ce6d 100644 >> --- a/drivers/infiniband/sw/siw/siw_qp_tx.c >> +++ b/drivers/infiniband/sw/siw/siw_qp_tx.c >> @@ -76,16 +76,15 @@ static int siw_try_1seg(struct siw_iwarp_tx >*c_tx, void *paddr) >> if (unlikely(!p)) >> return -EFAULT; >> >> - buffer = kmap_atomic(p); >> + buffer = kmap(p); >> >> if (likely(PAGE_SIZE - off >= bytes)) { >> memcpy(paddr, buffer + off, bytes); >> - kunmap_atomic(buffer); >missing kunmap() No it's not missing. we are in the if-path, and we unmap 'p' at the next line (skipping the else-path). >> } else { >> unsigned long part = bytes - (PAGE_SIZE - off); >> >> memcpy(paddr, buffer + off, part); >> - kunmap_atomic(buffer); >> + kunmap(p); >kunmap(buffer) >> >> if (!mem->is_pbl) >> p = siw_get_upage(mem->umem, >> @@ -97,11 +96,10 @@ static int siw_try_1seg(struct siw_iwarp_tx >*c_tx, void *paddr) >> if (unlikely(!p)) >> return -EFAULT; >> >> - buffer = kmap_atomic(p); >> - memcpy(paddr + part, buffer, >> - bytes - part); >> - kunmap_atomic(buffer); >> + buffer = kmap(p); >> + memcpy(paddr + part, buffer, bytes - part); >> } >> + kunmap(p); >Can this be out of if()? the buffers seem to be different. the page pointer gets reassigned since the data span two pages. so we unmap the first page after copying out of it what we need, get the second page into 'p', and map it. at the end, the current page p gets unmapped. if we do not have data spanning two pages, we unmap the first page. If we do have, we unmap the second page. It should be all good. Thanks & best regards, Bernard. >> } >> } >> return (int)bytes; >> -- >> 2.17.2 >> > >
diff --git a/drivers/infiniband/sw/siw/siw_qp_tx.c b/drivers/infiniband/sw/siw/siw_qp_tx.c index 8e72f955921d..5d97bba0ce6d 100644 --- a/drivers/infiniband/sw/siw/siw_qp_tx.c +++ b/drivers/infiniband/sw/siw/siw_qp_tx.c @@ -76,16 +76,15 @@ static int siw_try_1seg(struct siw_iwarp_tx *c_tx, void *paddr) if (unlikely(!p)) return -EFAULT; - buffer = kmap_atomic(p); + buffer = kmap(p); if (likely(PAGE_SIZE - off >= bytes)) { memcpy(paddr, buffer + off, bytes); - kunmap_atomic(buffer); } else { unsigned long part = bytes - (PAGE_SIZE - off); memcpy(paddr, buffer + off, part); - kunmap_atomic(buffer); + kunmap(p); if (!mem->is_pbl) p = siw_get_upage(mem->umem, @@ -97,11 +96,10 @@ static int siw_try_1seg(struct siw_iwarp_tx *c_tx, void *paddr) if (unlikely(!p)) return -EFAULT; - buffer = kmap_atomic(p); - memcpy(paddr + part, buffer, - bytes - part); - kunmap_atomic(buffer); + buffer = kmap(p); + memcpy(paddr + part, buffer, bytes - part); } + kunmap(p); } } return (int)bytes;
Since the transmit path is never executed in an atomic context, we do not need kmap_atomic() and can always use less demanding kmap(). Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com> --- drivers/infiniband/sw/siw/siw_qp_tx.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)