Re: Ext2 superblock fault

[Samuel Thibault]

While having a beer with Gianluca, I thought about a possible case:

> Thread 7 (thread 588.7):
> #5  0x08052472 in write_all_disknodes ()
>     at /build/mbanck/hurd-20071119/build-tree/hurd/ext2fs/inode.c:592
> #6  0x08054a90 in diskfs_sync_everything (wait=0)

This thread is syncing everything, i.e. asking a lot of writes, which
triggers the creation of a lot of threads.  Unfortunately the superblock
was paged out, so they all block on reading it.  Unfortunately, since in
Debian there is a patch which limits the number of created threads,
the read of the superblock doesn't actually create a new thread, that is
delayed.  But since none of the existing threads can progress (since
they are all waiting for the super block), things are just dead
locked...

I'm currently using a quite ugly patch (on top of the >2GB patch) to
avoid this by just always keeping a copy of the superblock, attached.
I'll commit it in the Debian package.

Samuel

--- hurd/ext2fs/hyper.c 2008-03-10 01:14:16.643969000 +0000
+++ /var/tmp/hurd-20071119/build-tree/hurd/ext2fs/hyper.c       2008-03-01 
23:41:53.227511000 +0000
@@ -55,17 +55,22 @@
 
 static int ext2fs_clean;       /* fs clean before we started writing? */
 
+struct ext2_super_block *__sblock = NULL;
 void
 get_hypermetadata (void)
 {
   error_t err;
   size_t read;
+  static struct ext2_super_block _sblock;
 
   assert (! sblock);
   err = store_read (store, SBLOCK_OFFS >> store->log2_block_size,
-                   SBLOCK_SIZE, (void **)&sblock, &read);
+                   SBLOCK_SIZE, (void **)&__sblock, &read);
   if (err || read != SBLOCK_SIZE)
     ext2_panic ("Cannot read hypermetadata");
+
+  sblock = &_sblock;
+  *sblock = *__sblock;
   
   if (sblock->s_magic != EXT2_SUPER_MAGIC
 #ifdef EXT2FS_PRE_02B_COMPAT
@@ -159,18 +164,18 @@
   if (zeroblock == 0)
     zeroblock = (vm_address_t) mmap (0, block_size, PROT_READ, MAP_ANON, 0, 0);
 
-  munmap (sblock, SBLOCK_SIZE);
-  sblock = NULL;
+  munmap (__sblock, SBLOCK_SIZE);
+  __sblock = NULL;
 }
 
 void
 map_hypermetadata (void)
 {
-  sblock = (struct ext2_super_block *) boffs_ptr (SBLOCK_OFFS);
+  __sblock = (struct ext2_super_block *) boffs_ptr (SBLOCK_OFFS);
 
   /* Cache a convenient pointer to the block group descriptors for allocation.
      These are stored in the filesystem blocks following the superblock.  */
-  group_desc_image = (struct ext2_group_desc *) bptr (bptr_block (sblock) + 1);
+  group_desc_image = (struct ext2_group_desc *) bptr (bptr_block (__sblock) + 
1);
 }
 
 error_t
@@ -193,8 +198,9 @@
  if (sblock_dirty)
    {
      sblock_dirty = 0;
-     disk_cache_block_ref_ptr (sblock);
-     record_global_poke (sblock);
+     *__sblock = *sblock;
+     disk_cache_block_ref_ptr (__sblock);
+     record_global_poke (__sblock);
    }
 
   sync_global (wait);