-----"Potnuri Bharat Teja" <bharat@xxxxxxxxxxx> wrote: -----
>To: "Bernard Metzler" <bmt@xxxxxxxxxxxxxx>
>From: "Potnuri Bharat Teja" <bharat@xxxxxxxxxxx>
>Date: 09/09/2019 04:54PM
>Cc: "linux-rdma@xxxxxxxxxxxxxxx" <linux-rdma@xxxxxxxxxxxxxxx>,
>"dledford@xxxxxxxxxx" <dledford@xxxxxxxxxx>
>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@xxxxxxxxxxxxxx>
>> ---
>> 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
>>
>
>
