RE: [RFC PATCH 0/2] RDMA/rxe: Add RDMA Atomic Write operation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

I have one question related to atomic write API and ibv_wr extension:

> > void (*wr_rdma_atomic_write)(struct ibv_qp_ex *qp, uint32_t rkey,
> >			     uint64_t remote_addr, uint64_t atomic_wr);

> > struct {
> >       uint64_t    remote_addr;
> >       uint32_t    rkey;
> >       uint64_t    wr_value:
> > } rdma;


In both places, atomic value is defined as uint64 but the IBTA spec defines it as 8 bytes array.

"The message size must be 8 bytes and is written to a contiguous range of the destination QP's virtual address space"

Would it be better to have 
uint8_t atomic_wr in wr_rdma_atomic_write(...) declaration

and 

uint8_t wr_value[8] in struct { ... } rdma?

I have checked CmpSwap and FetchAdd and in these cases, IBTA Spec says explicitly about 64 bits values.

Regards
Tomasz


> -----Original Message-----
> From: Tom Talpey <tom@xxxxxxxxxx>
> Sent: Tuesday, January 4, 2022 4:17 PM
> To: yangx.jy@xxxxxxxxxxx; Gromadzki, Tomasz
> <tomasz.gromadzki@xxxxxxxxx>
> Cc: linux-rdma@xxxxxxxxxxxxxxx; yanjun.zhu@xxxxxxxxx;
> rpearsonhpe@xxxxxxxxx; jgg@xxxxxxxxxx; y-goto@xxxxxxxxxxx;
> lizhijian@xxxxxxxxxxx
> Subject: Re: [RFC PATCH 0/2] RDMA/rxe: Add RDMA Atomic Write operation
> 
> 
> On 1/4/2022 4:28 AM, yangx.jy@xxxxxxxxxxx wrote:
> > On 2021/12/31 14:30, yangx.jy@xxxxxxxxxxx wrote:
> >> On 2021/12/31 5:42, Tom Talpey wrote:
> >>> On 12/30/2021 2:21 PM, Gromadzki, Tomasz wrote:
> >>>> 1)
> >>>>> rdma_post_atomic_writev(struct rdma_cm_id *id, void *context,
> >>>>> struct ibv_sge *sgl,
> >>>>>               int nsge, int flags, uint64_t remote_addr, uint32_t
> >>>>> rkey)
> >>>> Do we need this API at all?
> >>>> Other atomic operations (compare_swap/add) do not use struct
> >>>> ibv_sge at all but have a dedicated place in struct ib_send_wr {
> >>>> ...
> >>>>           struct {
> >>>>               uint64_t    remote_addr;
> >>>>               uint64_t    compare_add;
> >>>>               uint64_t    swap;
> >>>>               uint32_t    rkey;
> >>>>           } atomic;
> >>>> ...
> >>>> }
> >>>>
> >>>> Would it be better to reuse (extend) any existing field?
> >>>> i.e.
> >>>>           struct {
> >>>>               uint64_t    remote_addr;
> >>>>               uint64_t    compare_add;
> >>>>               uint64_t    swap_write;
> >>>>               uint32_t    rkey;
> >>>>           } atomic;
> >>> Agreed. Passing the data to be written as an SGE is unnatural since
> >>> it is always exactly 64 bits. Tweaking the existing atomic parameter
> >>> block as Tomasz suggests is the best approach.
> >> Hi Tomasz, Tom
> >>
> >> Thanks for your quick reply.
> >>
> >> If we want to pass the 8-byte value by tweaking struct atomic on user
> >> space, why don't we tranfer the 8-byte value by ATOMIC Extended
> >> Transport Header (AtomicETH) on kernel space?
> >> PS: IBTA defines that the 8-byte value is tranfered by RDMA Extended
> >> Transport Heade(RETH) + payload.
> >>
> >> Is it inconsistent to use struct atomic on user space and RETH +
> >> payload on kernel space?
> > Hi Tomasz, Tom
> >
> > I think the following rules are right:
> > RDMA READ/WRITE should use struct rdma in libverbs and RETH + payload
> > in kernel.
> > RDMA Atomic should use struct atomic in libverbs and AtomicETH in kernel.
> >
> > According to IBTA's definition, RDMA Atomic Write should use struct
> > rdma in libibverbs.
> 
> I don't quite understand this statement, the IBTA defines the protocol but
> does not define the API at such a level.
> 
> I do however agree with this:
> 
> > How about adding a member in struct rdma? for example:
> > struct {
> >       uint64_t    remote_addr;
> >       uint32_t    rkey;
> >       uint64_t    wr_value:
> > } rdma;
> 
> Yes, that's what Tomasz and I were suggesting - a new template for the
> ATOMIC_WRITE request payload. The three fields are to be supplied by the
> verb consumer when posting the work request.
> 
> Tom.
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux