On 10/21/21 3:03 PM, Jeff Moyer wrote: > Jeff Moyer <jmoyer@xxxxxxxxxx> writes: > >> Jens Axboe <axboe@xxxxxxxxx> writes: >> >>> On 10/21/21 12:05 PM, Jeff Moyer wrote: >>>> >>>>>> I'll follow up if there are issues. >>>> >>>> s390 (big endian, 64 bit) is failing libaio test 21: >>>> >>>> # harness/cases/21.p >>>> Expected -EAGAIN, got 4294967285 >>>> >>>> If I print out both res and res2 using %lx, you'll see what happened: >>>> >>>> Expected -EAGAIN, got fffffff5,ffffffff >>>> >>>> The sign extension is being split up. >>> >>> Funky, does it work if you apply this on top? >>> >>> diff --git a/fs/aio.c b/fs/aio.c >>> index 3674abc43788..c56437908339 100644 >>> --- a/fs/aio.c >>> +++ b/fs/aio.c >>> @@ -1442,8 +1442,8 @@ static void aio_complete_rw(struct kiocb *kiocb, u64 res) >>> * 32-bits of value at most for either value, bundle these up and >>> * pass them in one u64 value. >>> */ >>> - iocb->ki_res.res = lower_32_bits(res); >>> - iocb->ki_res.res2 = upper_32_bits(res); >>> + iocb->ki_res.res = (long) (res & 0xffffffff); >>> + iocb->ki_res.res2 = (long) (res >> 32); >>> iocb_put(iocb); >>> } >> >> I think you'll also need to clamp any ki_complete() call sites to 32 >> bits (cast to int, or what have you). Otherwise that sign extension >> will spill over into res2. >> >> fwiw, I tested with this: >> >> iocb->ki_res.res = (long)(int)lower_32_bits(res); >> iocb->ki_res.res2 = (long)(int)upper_32_bits(res); >> >> Coupled with the call site changes, that made things work for me. > > This is all starting to feel like a minefield. If you don't have any > concrete numbers to show that there is a speedup, I think we should > shelf this change. It's really not a minefield at all, we just need a proper help to encode the value. I'm out until Tuesday, but I'll sort it out when I get back. Can also provide some numbers on this. -- Jens Axboe
