tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

struct fid/filehandle size and lfs



A while back the filehandle type for ffs was fixed to have a 64-bit
inode number instead of silently truncating to 32 bits. Yesterday I
naively merged that change into lfs and it exploded the world.

It seems that lfs has an extra 32-bit field in its filehandle (that
ffs doesn't) -- it is a copy of the 32-bit "uuid" in the lfs
superblock that helps avoid tripping on old superblock copies after a
newfs. Which is well and good. However, with this field present,
making the inode number 64 bits wide causes four further bytes of
padding (because of alignment restrictions) ... and worse, on lp64
platforms only and not their 32-bit counterparts.

This would be only a nuisance, except that this padding pushes the
total size of the filehandle to 32, and in sys/fstypes.h we have

   #define FHANDLE_SIZE_COMPAT     28
   #define FHANDLE_SIZE_MIN        FHANDLE_SIZE_COMPAT

and related to this lfs has as part of its public kernel interface the
following lie:

   struct lfs_fhandle {
           /* FHANDLE_SIZE_COMPAT (but used from userland too) */
           char space[28];
   };

This is the type formally used by the fcntl the cleaner uses to get to
the ifile, so if the total filehandle size is 32 it fails and the
cleaner won't attach and nothing works. Worse, because this is passed
to fhopen() it has to be the same as other filehandles, at least in
the absence of evil hacks, so doing a special thing for the ifile
handle isn't very tenable.

This creates the following hassle: making struct lfs_fhandle bigger
requires compat, not just for the fcntl used to fetch a handle for the
ifile but also for fhtovp and any other pathway reachable via fhopen,
fhstat, etc. Which requires keying the compat off the size and keeping
the old lfs filehandle structure (with 32-bit inode field) around.

Meanwhile since 28 is not actually large enough any more,
FHANDLE_SIZE_COMPAT/FHANDLE_SIZE_MIN needs bumping, and I have no idea
what the other consequences of that might be. (The constant appears to
be kernel-only but that isn't conclusive.)

Furthermore since this lfs_fhandle struct is an abomination it should
really be dropped entirely in favor of an interface that works like
getfh(2). But you can't really cram that into an fcntl (or an ioctl
either) so it's not clear how to do that. (It can't use getfh(2)
because there's no path for the ifile; the ifile is internal.
Rewriting the cleaner not to access the ifile directly would probably
be better in the long run but isn't feasible in the short term.)

I am becoming fairly strongly inclined to enter the 64-bit inode
number into the lfs filehandle structure as two 32-bit values instead
of one 64-bit value, because this way the size will be 28 (it was 24
before -- I don't know where the 28 originally came from) and none of
the above rubbish is needed.

This would be kind of a hack, but it seems like the best approach for
the time being... anyone want to comment?

-- 
David A. Holland
dholland%netbsd.org@localhost


Home | Main Index | Thread Index | Old Index