linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Theodore Tso <tytso@mit.edu>
To: Andreas Dilger <adilger@sun.com>
Cc: linux-ext4@vger.kernel.org
Subject: Re: reserved field in struct io_manager
Date: Wed, 27 Aug 2008 21:25:16 -0400	[thread overview]
Message-ID: <20080828012516.GE26987@mit.edu> (raw)
In-Reply-To: <20080826110924.GJ3392@webber.adilger.int>

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

      reply	other threads:[~2008-08-28  1:25 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-26 11:09 reserved field in struct io_manager Andreas Dilger
2008-08-28  1:25 ` Theodore Tso [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080828012516.GE26987@mit.edu \
    --to=tytso@mit.edu \
    --cc=adilger@sun.com \
    --cc=linux-ext4@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).