Re: Ext2fs 2GB limit status?

[Marco Gerards]

Ognyan Kulev <ogi@fmi.uni-sofia.bg> writes:

> Marco Gerards wrote:
> > Ogi (I hope he is subscribed to this list) wrote some patches for the Hurd 
> > so
> > ext2fs can support large partitions.  It seems to me that not many
> > people have looked at these patches :(.  Having a >2GB capable ext2fs
> > is very important to me.
> 
> The people that commented on this patch are less that the fingers of
> one of my hands ;-)

:(
 
> > The thing that bothered me when reading these patches was that byte
> > offsets were used quite often.  This will make it not possible to use
> > partitions bigger than 4GB in some circumstances.
> 
> All used byte offsets should be 64-bit (off_t).  My main target was to
> _quickly_ release patch that works.  That's the reason that I made as
> few changes to the original design as possible.  Applying ext2fs patch
> that works fine is higher priority than improving the code.

Right.  I just think it is better to use block offsets instead of byte
offsets when this is possible.  Is off_t always 64 bits or only with
FILE_OFFSET_BITS=64?  (So if someone compiles ext2fs outside the hurd
sourcetree he should set FILE_OFFSET_BITS=64).

I understand you did a quick release, I'm glad you did that. :)

> > Most likely I forgot some.  Especially the pokel stuff might be bad.
> > Another use for these functions was to read/write the superblock;
> > although it isn't that bad, it would be better if you didn't use byte
> > offsets for this.
> > Perhaps this problem can be fixed by removing all these functions so
> > they can't be used accidently.  I see no reason to have these
> > functions anyway.
> 
> I will need some days to go deeper about these.

Yeah, I even think you do not need pokels at all.  I'm not too sure
about it though, I should read the other cod first.

> > Another thing I've noticed in the patch were changes in the amount of
> > whitespaces.  This isn't a big problem (for me), it is just something
> > to lookout for when sending in the definite patch, here is an example:
> > -     * We have succeeded in finding a free byte in the block
> > -     * bitmap.  Now search backwards up to 7 bits to find the
> > -     * start of this group of free blocks.
> > +   * We have succeeded in finding a free byte in the block
> > +   * bitmap.  Now search backwards up to 7 bits to find the
> > +   * start of this group of free blocks.
> 
> Yes, there is changed whitespace in some places.  In all of them, it's
> done the way Emacs do it.  In this particular example, I think that *
> are aligned to the leading /* or something like that.  AFAIR I've made
> such whitespace changes in the legacy ext2 code from Linux.

Ok, I just wanted to know if it was accidental or not.

> > The last thing I mentioned was this:
> > +#if 1
> >        err = find_block (node, offset, &block, &lock);
> >        if (err)
> >         break;
> >        assert (block);
> > +#else
> > +      /* XXX: I still haven't investigated why this is needed...  */
> > +      err = ext2_getblk (node, offset >> log2_block_size, 1, &block);
> > +      if (err)
> > +       break;
> > +#endif
> > I'm not really an ext2fs expert, but I think this is used for sparse
> > files.  Ext2fs doesn't allocate blocks when seeking, it just makes the
> > file sparse.  So the blocks get allocated just before they are written
> > to disk.
> 
> As you see, the original code is compiled by default.  The #else is
> just garbage, it _was_ required because of some bugs that are gone
> now.  I believe it can be removed safely, as other #if 0 in the code.

Ok.

> > I also have a question about the code.  The assembly code "int $3" is
> > used.  AFAIK this is used to tell the debugger a breakpoint is
> > reached, right?  When is this function (abort) called?
> 
> Well, this is why it is called alpha ;-)  There was a time when
> assertions failed very often.  When they fail, they call abort(),
> which I replaced with the function you saw and which contains this
> "int $3". This is solely for debugging purposes.

Interesting, I didn't know that.
 
> Now, about the future of ext2fs.  I won't be able to work on it much
> till the end of November.  So if you, or anyone else, want to work on
> it, it will be best if we put it on hurd-extras or some other public
> place where changes can be made by many people.  (We can forget about
> changelogs till release candidate status.)

Too bad you can't work on it :(.  I think it would be nice if you'd
put this in hurd-extras.  Unfortunately I don't have the time to work
on this the first 5 months (perhaps longer).

Thanks,
Marco