[patch] clarify AIO_EVENTS_OFFSET constant

[patch] clarify AIO_EVENTS_OFFSET constant

[Chen, Kenneth W]

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 <kenneth.w.chen@intel.com>


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;			\

Re: [patch] clarify AIO_EVENTS_OFFSET constant

[Benjamin LaHaise]

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: <dont@kvack.org>.

RE: [patch] clarify AIO_EVENTS_OFFSET constant

[Chen, Kenneth W]

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))