Re: [Fwd: [patch #2508] ext2fs support for large store (> 1.5G)]

[Marco Gerards]

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

Hi Ognyan,

Thanks a lot for your great work so far!

> Original Submission:  Finally, the patch is prepared to enter the Hurd.

This is good to hear.  My first impression of the patch was good (by
reading over it very quickly).

I have some comments so far, I will continue to read the patch and
test it later this weekend.

The changelogs can use some work AFAICS.  For example you notice when
you add new local variables, I would not do this.  Another thing you
do is:

        (DC_DONT_REUSE): Likewise.
        (struct disk_cache_info): New struct.
        Add declarations for disk_cache_bptr, disk_cache_info,
        disk_cache_lock and disk_cache_remapping.
        Add declarations for functions disk_cache_map,
        disk_cache_block_ref, disk_cache_block_ref_ptr,
        disk_cache_block_deref and disk_cache_block_is_ref.
        Add declaration for map_hypermetadata.
        (bptr_index): New macro.
        (boffs_ptr): Instead of macro, now it's inline function that uses

I have copied some of the context.  You should do the same for
disk_cache_map, disk_cache_lock and whatever variables, functions and
macros you add as you did for bptr_index.  If you are in doubt, just
read:

http://www.gnu.org/prep/standards_40.html#SEC40

And I noticed you include the ChangeLog entries in the patch,
personally I don't like this, also because you already included it.

I wanted to test the patch before reading it in detail, so I just
did.  When I used it on a small partition it worked for me.  After
that I made a 10GB partition in GNU/Linux, I put about 2.7GB of data
on it.  When I tried copying a directory to this partition an
assertion was triggered.  Here is some information that can hopefully
help you finding the problem:


ext2fs.static.2: ../../hurd/ext2fs/ext2fs.h:330: boffs_ptr: Assertion `ptr' 
failed.


(I put the backtraces at the end of the mail)

Perhaps I can debug this problem myself, but I am not promising
anything. :)

> To have a chance RC3 or even RC2 to be committed, please don't stop
> reviewing the code at the first thing you don't like.  List precisely
> all things that need to be changed, so that number of release
> iterations is reduced.

Ok.  I will continue reading the patch and test it.  This is just what
I found so far.

> The patch requires changes in libpager and I don't claim they've been
> done in the best way.  For example, ABI compatibility of libpager is
> broken by this patch.  Again, just specify what should be changed in
> the patch.

Is there any way to prevent breaking the ABI?  And do you think the
changes can be made in a better way?

Thanks,
Marco

(gdb) bt
#0  0x080a5cbc in mach_msg_trap ()
#1  0x0807988a in mach_msg ()
#2  0x080e612d in msg_sig_post ()
#3  0x080b72d7 in kill_port.1 ()
#4  0x080b7425 in kill_pid.0 ()
#5  0x080b76cd in kill ()
#6  0x080b6de5 in raise ()
#7  0x08087ce6 in abort ()
#8  0x08083c70 in __assert_perror_fail ()
#9  0x0804d30c in ext2_alloc_inode (dir_inum=454274, mode=16877)
    at ../../hurd/ext2fs/ext2fs.h:342
#10 0x0804daa8 in diskfs_alloc_node (dir=0x814c090, mode=16877, node=0x0)
    at ../../hurd/ext2fs/ialloc.c:300
#11 0x0805cce5 in diskfs_create_node (dir=0x814c090, name=0x108df58 "doc", 
    mode=16877, newnode=0x108be24, cred=0x814b1d8, ds=0x108bdfc)
    at ../../hurd/libdiskfs/node-create.c:52
#12 0x0805ee7b in diskfs_S_dir_mkdir (dircred=0x814b1d8, name=0x108df58 "doc", 
    mode=16877) at ../../hurd/libdiskfs/dir-mkdir.c:57
#13 0x0805a0eb in _Xdir_mkdir (InHeadP=0x108df3c, OutHeadP=0x108bf3c)
    at fsServer.c:1753
#14 0x0805a93c in diskfs_fs_server (InHeadP=0x108df3c, OutHeadP=0x0)
    at fsServer.c:2679
#15 0x0805862a in diskfs_demuxer (inp=0x108df3c, outp=0x108bf3c)
---Type <return> to continue, or q <return> to quit---
    at ../../hurd/libdiskfs/demuxer.c:40
#16 0x080719ed in internal_demuxer.0 ()
    at ../../hurd/libports/manage-multithread.c:101
#17 0x08079b1b in mach_msg_server_timeout ()
#18 0x08071ade in thread_function.1 ()
    at ../../hurd/libports/manage-multithread.c:136
#19 0x08070694 in cthread_body (self=0x8144d90)
    at ../../hurd/libthreads/cthreads.c:306



(gdb) bt full
#0  0x080a5cbc in mach_msg_trap ()
No symbol table info available.
#1  0x0807988a in mach_msg ()
No symbol table info available.
#2  0x080e612d in msg_sig_post ()
No symbol table info available.
#3  0x080b72d7 in kill_port.1 ()
No symbol table info available.
#4  0x080b7425 in kill_pid.0 ()
No symbol table info available.
#5  0x080b76cd in kill ()
No symbol table info available.
#6  0x080b6de5 in raise ()
No symbol table info available.
#7  0x08087ce6 in abort ()
No symbol table info available.
#8  0x08083c70 in __assert_perror_fail ()
No symbol table info available.
#9  0x0804d30c in ext2_alloc_inode (dir_inum=454274, mode=16877)
    at ../../hurd/ext2fs/ext2fs.h:342
        bh = 0x17000 "\001"
        i = 40
        j = 0
---Type <return> to continue, or q <return> to quit---
        inum = 0
        gdp = (struct ext2_group_desc *) 0x8500
        tmp = (struct ext2_group_desc *) 0x0
#10 0x0804daa8 in diskfs_alloc_node (dir=0x814c090, mode=16877, node=0x0)
    at ../../hurd/ext2fs/ialloc.c:300
        err = 0
        sex = 0
        block = 0
        np = (struct node *) 0x108bdd4
        st = (struct stat *) 0x40000002
        inum = 4611686027017322496
#11 0x0805cce5 in diskfs_create_node (dir=0x814c090, name=0x108df58 "doc", 
    mode=16877, newnode=0x108be24, cred=0x814b1d8, ds=0x108bdfc)
    at ../../hurd/libdiskfs/node-create.c:52
        np = (struct node *) 0x0
        err = ENOENT
        newuid = 135577744
        newgid = 135577744
#12 0x0805ee7b in diskfs_S_dir_mkdir (dircred=0x814b1d8, name=0x108df58 "doc", 
    mode=16877) at ../../hurd/libdiskfs/dir-mkdir.c:57
        dnp = (struct node *) 0x814c090
        np = (struct node *) 0x0
        ds = (struct dirstat *) 0x108bdfc
---Type <return> to continue, or q <return> to quit---
        error = 1073741826
#13 0x0805a0eb in _Xdir_mkdir (InHeadP=0x108df3c, OutHeadP=0x108bf3c)
    at fsServer.c:1753
        nameCheck = {msgt_name = 12, msgt_size = 8, msgt_number = 1024, 
  msgt_inline = 1, msgt_longform = 0, msgt_deallocate = 0, msgt_unused = 0}
        modeCheck = {msgt_name = 2, msgt_size = 32, msgt_number = 1, 
  msgt_inline = 1, msgt_longform = 0, msgt_deallocate = 0, msgt_unused = 0}
#14 0x0805a93c in diskfs_fs_server (InHeadP=0x108df3c, OutHeadP=0x0)
    at fsServer.c:2679
        RetCodeType = {msgt_name = 2, msgt_size = 32, msgt_number = 1, 
  msgt_inline = 1, msgt_longform = 0, msgt_deallocate = 0, msgt_unused = 0}
        routine = 0
#15 0x0805862a in diskfs_demuxer (inp=0x108df3c, outp=0x108bf3c)
    at ../../hurd/libdiskfs/demuxer.c:40
No locals.
#16 0x080719ed in internal_demuxer.0 ()
    at ../../hurd/libports/manage-multithread.c:101
No locals.
#17 0x08079b1b in mach_msg_server_timeout ()
No symbol table info available.
#18 0x08071ade in thread_function.1 ()
    at ../../hurd/libports/manage-multithread.c:136
No locals.
---Type <return> to continue, or q <return> to quit---
#19 0x08070694 in cthread_body (self=0x8144d90)
    at ../../hurd/libthreads/cthreads.c:306
        t = 0x8144d18