On Tue, Jan 17, 2023 at 02:41:53PM -0600, Bob Pearson wrote: > On 1/17/23 12:05, Jason Gunthorpe wrote: > > On Tue, Jan 17, 2023 at 11:04:29AM -0600, Bob Pearson wrote: > >> On 1/17/23 10:59, Jason Gunthorpe wrote: > >>> On Tue, Jan 17, 2023 at 10:57:31AM -0600, Bob Pearson wrote: > >>> > >>>>>> - dst = iova_to_vaddr(mr, qp->resp.va + qp->resp.offset, payload); > >>>>>> - /* check vaddr is 8 bytes aligned. */ > >>>>>> - if (!dst || (uintptr_t)dst & 7) > >>>>>> - return RESPST_ERR_MISALIGNED_ATOMIC; > >>>>>> + if (res->replay) > >>>>>> + return RESPST_ACKNOWLEDGE; > >>>>>> > >>>>>> - /* Do atomic write after all prior operations have completed */ > >>>>>> - smp_store_release(dst, src); > >>>>>> + mr = qp->resp.mr; > >>>>>> + value = *(u64 *)payload_addr(pkt); > >>>>>> + iova = qp->resp.va + qp->resp.offset; > >>>>>> > >>>>>> - /* decrease resp.resid to zero */ > >>>>>> - qp->resp.resid -= sizeof(payload); > >>>>>> +#if defined CONFIG_64BIT > >>>>> > >>>>> Shouldn't need a #ifdef here > >>>> > >>>> This avoids a new special error (i.e. NOT_64_bit) and makes it clear we > >>>> won't call the code in mr. > >>> > >>> ? That doesn't seem right > >> > >> that was the -3 of the -1, -2, -3 that we just fixed. there are three error paths out > >> of this state and we need a way to get to them. The #ifdef provides > >> that third path. > > > > I feel like it should be solvable without this ifdef though > > > > Jason > > You could get rid of the ifdef in the atomic_write_reply() routine but then the rxe_mr_do_atomic_write() routine would have to have a second version in the #else case > that would have to return something different so that the third exit could be taken i.e. > whatever replaces the original -3. I really think this is simpler. You really should just return the RESPST_* from these functions. Jason
