Re: [PATCH for-next v3 4/6] RDMA-rxe: Isolate mr code from atomic_write_reply()

[Bob Pearson <rpearsonhpe@gmail.com>]

On 1/17/23 07:47, Jason Gunthorpe wrote:
> On Tue, Jan 17, 2023 at 09:36:02AM +0800, Zhu Yanjun wrote:
>> On Sat, Jan 14, 2023 at 7:28 AM Bob Pearson <rpearsonhpe@gmail.com> wrote:
>>>
>>> Isolate mr specific code from atomic_write_reply() in rxe_resp.c into
>>> a subroutine rxe_mr_do_atomic_write() in rxe_mr.c.
>>> Check length for atomic write operation.
>>> Make iova_to_vaddr() static.
>>>
>>> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
>>> ---
>>> v3:
>>>   Fixed bug reported by kernel test robot. Ifdef'ed out atomic 8 byte
>>>   write if CONFIG_64BIT is not defined as orignally intended by the
>>>   developers of the atomic write implementation.
>>> link: https://lore.kernel.org/linux-rdma/202301131143.CmoyVcul-lkp@intel.com/
>>>
>>>  drivers/infiniband/sw/rxe/rxe_loc.h  |  1 +
>>>  drivers/infiniband/sw/rxe/rxe_mr.c   | 50 ++++++++++++++++++++++++
>>>  drivers/infiniband/sw/rxe/rxe_resp.c | 58 +++++++++++-----------------
>>>  3 files changed, 73 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
>>> index bcb1bbcf50df..fd70c71a9e4e 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_loc.h
>>> +++ b/drivers/infiniband/sw/rxe/rxe_loc.h
>>> @@ -74,6 +74,7 @@ int rxe_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg,
>>>  void *iova_to_vaddr(struct rxe_mr *mr, u64 iova, int length);
>>>  int rxe_mr_do_atomic_op(struct rxe_mr *mr, u64 iova, int opcode,
>>>                         u64 compare, u64 swap_add, u64 *orig_val);
>>> +int rxe_mr_do_atomic_write(struct rxe_mr *mr, u64 iova, void *addr);
>>>  struct rxe_mr *lookup_mr(struct rxe_pd *pd, int access, u32 key,
>>>                          enum rxe_mr_lookup_type type);
>>>  int mr_check_range(struct rxe_mr *mr, u64 iova, size_t length);
>>> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
>>> index 791731be6067..1e74f5e8e10b 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_mr.c
>>> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
>>> @@ -568,6 +568,56 @@ int rxe_mr_do_atomic_op(struct rxe_mr *mr, u64 iova, int opcode,
>>>         return 0;
>>>  }
>>>
>>> +/**
>>> + * rxe_mr_do_atomic_write() - write 64bit value to iova from addr
>>> + * @mr: memory region
>>> + * @iova: iova in mr
>>> + * @addr: source of data to write
>>> + *
>>> + * Returns:
>>> + *      0 on success
>>> + *     -1 for misaligned address
>>> + *     -2 for access errors
>>> + *     -3 for cpu without native 64 bit support
>>> + */
>>> +int rxe_mr_do_atomic_write(struct rxe_mr *mr, u64 iova, void *addr)
>>> +{
>>> +#if defined CONFIG_64BIT
>>
>> IS_ENABLED is better?
> 
> is_enabled won't work here because the code doesn't compile.
> 
> Jason

exactly.