reserved field in struct io_manager

reserved field in struct io_manager

[Andreas Dilger]

Ted,
I notice in the struct io_manager that there are reserved fields at the
end, presumably for adding new methods to this array.  Unfortunately,
the reserved field is type "int" instead of "long" and as a result there
isn't necessarily enough space to hold a function pointer on a 64-bit
system.

Have you looked at this at all?  Presumably as methods are added on 64-bit
systems, the array grows slightly larger each time because the pointers
are larger than the amount of space removed from the array.  Possibly this
is harmless, because the consumers of this struct have allocated enough
space to handle all of the used fields.

I'm not 100% sure of the usage of this struct, but I thought I'd mention
it, because I noticed that the 64-bit methods were added there recently.

I was going to submit a patch to add a "readahead" method, which we've
been using in our "e2scan" tool to improve performance, and I thought
it might also be useful for e2fsck speedups.  If someone is interested
in these I can send them to the list, but the patches won't apply to
the current Git tree because of this change.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: reserved field in struct io_manager

[Theodore Tso]

On Tue, Aug 26, 2008 at 05:09:24AM -0600, Andreas Dilger wrote:
> I notice in the struct io_manager that there are reserved fields at the
> end, presumably for adding new methods to this array.  Unfortunately,
> the reserved field is type "int" instead of "long" and as a result there
> isn't necessarily enough space to hold a function pointer on a 64-bit
> system.
> 
> Have you looked at this at all?  Presumably as methods are added on 64-bit
> systems, the array grows slightly larger each time because the pointers
> are larger than the amount of space removed from the array.  Possibly this
> is harmless, because the consumers of this struct have allocated enough
> space to handle all of the used fields.

Yeah, it's mostly harmless.  We should probably make it be a long just
to be safe, but we have a pretty big buffer there.

The bigger problem is that when we added the 64-bit methods, we didn't
do it the best way, given that we have to deal with old applications
linking with new libaries, and vice versa, and the I/O manager can be
in *either* the application or the library.  We can mostly paper over
this by having ext2fs_open() check to make sure the write_blk64() and
read_blk64() are present.  But I should (and will) replace the header
file #define's for the read64 and write64 functions with a real C
function which can fall back to read/write functions if the
read64/write64 functions aren't present.

> I was going to submit a patch to add a "readahead" method, which we've
> been using in our "e2scan" tool to improve performance, and I thought
> it might also be useful for e2fsck speedups.  If someone is interested
> in these I can send them to the list, but the patches won't apply to
> the current Git tree because of this change.

I'd recommend adding a new C wrapper function instead of using a
#define in ext2_io.h, and for us to apply it in the "master" branch
after the 1.41.1 release.

Regards,

					- Ted