2006-10-13 00:00:48

by Chen, Kenneth W

[permalink] [raw]
Subject: [patch] clarify AIO_EVENTS_OFFSET constant

A clean up patch: I think it is a lot easier to read AIO_EVENTS_OFFSET
as an offset because of aio_ring at the beginning of a head page, instead
of doing arithmetic of (event on 2nd page - event on 1st page).


Signed-off-by: Ken Chen <[email protected]>


diff -Nurp linux-2.6.18/fs/aio.c linux-2.6.18.ken/fs/aio.c
--- linux-2.6.18/fs/aio.c 2006-09-19 20:42:06.000000000 -0700
+++ linux-2.6.18.ken/fs/aio.c 2006-10-12 13:33:09.000000000 -0700
@@ -173,9 +173,8 @@ static int aio_setup_ring(struct kioctx
/* aio_ring_event: returns a pointer to the event at the given index from
* kmap_atomic(, km). Release the pointer with put_aio_ring_event();
*/
-#define AIO_EVENTS_PER_PAGE (PAGE_SIZE / sizeof(struct io_event))
-#define AIO_EVENTS_FIRST_PAGE ((PAGE_SIZE - sizeof(struct aio_ring)) / sizeof(struct io_event))
-#define AIO_EVENTS_OFFSET (AIO_EVENTS_PER_PAGE - AIO_EVENTS_FIRST_PAGE)
+#define AIO_EVENTS_PER_PAGE (PAGE_SIZE / sizeof(struct io_event))
+#define AIO_EVENTS_OFFSET (sizeof(struct aio_ring) / sizeof(struct io_event))

#define aio_ring_event(info, nr, km) ({ \
unsigned pos = (nr) + AIO_EVENTS_OFFSET; \


2006-10-13 14:23:10

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: [patch] clarify AIO_EVENTS_OFFSET constant

On Thu, Oct 12, 2006 at 05:00:24PM -0700, Chen, Kenneth W wrote:
> A clean up patch: I think it is a lot easier to read AIO_EVENTS_OFFSET
> as an offset because of aio_ring at the beginning of a head page, instead
> of doing arithmetic of (event on 2nd page - event on 1st page).

Nak. Your change fails if aio_ring is not an exact multiple of the
io_event size due to rounding errors, while the original code rounds
correctly.

-ben
--
"Time is of no importance, Mr. President, only life is important."
Don't Email: <[email protected]>.

2006-10-13 20:38:06

by Chen, Kenneth W

[permalink] [raw]
Subject: RE: [patch] clarify AIO_EVENTS_OFFSET constant

Benjamin LaHaise wrote on Friday, October 13, 2006 7:23 AM
> On Thu, Oct 12, 2006 at 05:00:24PM -0700, Chen, Kenneth W wrote:
> > A clean up patch: I think it is a lot easier to read AIO_EVENTS_OFFSET
> > as an offset because of aio_ring at the beginning of a head page, instead
> > of doing arithmetic of (event on 2nd page - event on 1st page).
>
> Nak. Your change fails if aio_ring is not an exact multiple of the
> io_event size due to rounding errors, while the original code rounds
> correctly.


Yeah, neither form is bullet proof, even the original one will break down
when io_event size is not in power of 2. The good news is that no where
near in sight that these two data structures size will change.

To make it a real bullet proof, it should be something like the following:

#define AIO_EVENTS_OFFSET ((sizeof(struct aio_ring) + \
sizeof(struct io_event) - 1 ) / \
sizeof(struct io_event))