On 10/22/21 9:29 AM, Jens Axboe wrote:

On 10/22/21 8:19 AM, Jens Axboe wrote:

quoted

On 10/21/21 3:03 PM, Jeff Moyer wrote:

quoted

Jeff Moyer [off-list ref] writes:

quoted

Jens Axboe [off-list ref] writes:

quoted

On 10/21/21 12:05 PM, Jeff Moyer wrote:

quoted

quoted

quoted

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.

I think this incremental should fix it, also providing a helper to
properly pack these. The more I look at the gadget stuff the more I also
get the feeling that it really is wonky and nobody uses res2, which
would be a nice cleanup to continue. But I think it should be separate.

For the record, deferring all of this until next week when I'm back.
I'll pick it back up then, obviously this isn't an urgent thing at all,
would just love to sort out the useless argument going down the line.
We'd need to pack all of them, not just the odd ones out.

-- 
Jens Axboe