Re: [PATCH 01/11] ext2fs: cache the superblock

[Samuel Thibault]

Justus Winter, le Mon 12 May 2014 12:05:39 +0200, a écrit :
> Previously, the superblock was mmaped and a pointer stored in sblock
> by map_hypermetadata.  This memory is backed by our disk pager.
> 
> This is rather unfortunate, as this means that whenever we read a
> value from that location, we might generate a request our disk pager.
> This amplifies the so-called thread-storm problem.
> 
> Rather than relying on a mmaped region of memory, just use the data
> loaded by get_hypermetadata.

Please also fix the comment in the declaration of map_hypermetadata in
ext2fs.h, then Ack.

> * ext2fs/hyper.c (get_hypermetadata): Do not free sblock.
> (mapped_sblock): New variable.
> (map_hypermetadata): Map the superblock to mapped_sblock instead.
> (diskfs_set_hypermetadata): Copy superblock into mapped_superblock.
> ---
>  ext2fs/hyper.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/ext2fs/hyper.c b/ext2fs/hyper.c
> index 5bcc2ab..5f288bf 100644
> --- a/ext2fs/hyper.c
> +++ b/ext2fs/hyper.c
> @@ -61,7 +61,9 @@ get_hypermetadata (void)
>    error_t err;
>    size_t read = 0;
>  
> -  assert (! sblock);
> +  if (sblock != NULL)
> +    munmap (sblock, SBLOCK_SIZE);
> +
>    err = store_read (store, SBLOCK_OFFS >> store->log2_block_size,
>                   SBLOCK_SIZE, (void **)&sblock, &read);
>    if (err || read != SBLOCK_SIZE)
> @@ -161,19 +163,19 @@ get_hypermetadata (void)
>        zeroblock = (vm_address_t) mmap (0, block_size, PROT_READ, MAP_ANON, 
> 0, 0);
>        assert (zeroblock != (vm_address_t) MAP_FAILED);
>      }
> -
> -  munmap (sblock, SBLOCK_SIZE);
> -  sblock = NULL;
>  }
>  
> +static struct ext2_super_block *mapped_sblock;
> +
>  void
>  map_hypermetadata (void)
>  {
> -  sblock = (struct ext2_super_block *) boffs_ptr (SBLOCK_OFFS);
> +  mapped_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 (mapped_sblock) + 1);
>  }
>  
>  error_t
> @@ -196,8 +198,9 @@ diskfs_set_hypermetadata (int wait, int clean)
>   if (sblock_dirty)
>     {
>       sblock_dirty = 0;
> -     disk_cache_block_ref_ptr (sblock);
> -     record_global_poke (sblock);
> +     memcpy (mapped_sblock, sblock, SBLOCK_SIZE);
> +     disk_cache_block_ref_ptr (mapped_sblock);
> +     record_global_poke (mapped_sblock);
>     }
>  
>    sync_global (wait);
> -- 
> 2.0.0.rc0
> 

-- 
Samuel
Progress (n.): The process through which the Internet has evolved from
smart people in front of dumb terminals to dumb people in front of smart
terminals.