Re: PATCH: cramfs documentation & fixes

From: Linus Torvalds (torvalds@transmeta.com)
Date: Tue Jan 11 2000 - 13:17:49 EST


On 11 Jan 2000, Peter Moulder wrote:
>
> I also added a sanity check that the requested length is within the
> buffer size. Handling of the check failing is somewhat suckful, but
> it "never happens" if the machine behaves according to spec.

Actually, it CAN happen.

While the compression algorithm is good, you CAN get cases where it cannot
compress a full page at all, and in fact it will add a few bytes of state
information to the compression. So at uncompression time you may actually
end up needing more than 4096 bytes.

It basically never happens with "normal" loads, but it does happen if
cramfs crams a already compressed file. Not something you normally want to
do, but if it is occasional it may make sense just to avoid special cases.

> The validation on super.root.offset was wrong. mkcramfs always sets
> this to either zero (for an empty filesystem) or sizeof(struct
> cramfs_super)>>2. mkcramfs doesn't set super.size to anything
> meaningful at all.

Right now, no. I'd like to change that, but I ended up releasing cramfs
early, just because (a) some of the LinuxCare people seemed interested and
(b) I'ma big believer in releasing early and getting comments from people.

Comments and patch very gratefully taken.

> i_ino is calculated as (diskinode->offset ? diskinode->offset << 2 :
> 1) (see CRAMINO in inode.c). Only symlinks, non-empty regular files
> and non-empty directories have a non-zero value of diskinode->offset,
> so we're breaking the promise that <st_dev, st_ino> uniquely identify
> a file (particularly for non-regular files). I suggest that we use
> inode->u.cram_i.offset to hold (diskinode->offset << 2), and store the
> inode's own location (perhaps divided by 4 or 4+sizeof(struct
> cramfs_inode)) in i_ino. This is enough to fulfil the promise made by
> the stat man page (also required by Single Unix Spec).

Sounds fine.. POSIX conformance wasn't really high on my list, simply
because I think cramfs is most useful for rescue disks and possibly
embedded devices, but you're obviously right.

> Changed maximum filesystem size that mkcramfs will create from 64MB to
> a bit over 256MB. I don't know whether the 64MB "limit" pertains to a
> property of ROMs or whether it's a miscalculation from the fact that
> diskinode->offset is 26 bits wide, but cramfs is useful not just for
> ROMs. I've tested that >64MB cramfs filesystems can be mounted & used.

The 64MB limit is originally just because my first version didn't do the
shift by two, and I added that later but didn't update mkcramfs. Thanks.

The patch looks good apart from the overzealous test for the length of the
required source stream, applied. And thanks,

                Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Sat Jan 15 2000 - 21:00:18 EST