Re: [PATCH 7/9] ext2fs: use two distinct pager buckets for the disk and file pager

[Samuel Thibault]

Justus Winter, le Mon 28 Apr 2014 12:20:02 +0200, a écrit :
> ext2fs has two kinds of pagers.  One for the files, one for the disk.
> Previously, both were in the same port bucket.
> 
> If a request for a file pager arrives, it most likely touches some
> metadata (like the superblock).  This is in turn backed by the disk
> pager, so another request is generated for the disk pager.
> 
> Seperate all pagers clearly by using two port buckets.  This will
> enable us to use a single thread per port bucket in the future.

Ack.

> * ext2fs/pager.c (pager_bucket): Rename to...
> (disk_pager_bucket): ... this to make the change explicit at every
> occurrence.
> (file_pager_bucket): New variable.
> (service_paging_requests): New function.
> (create_disk_pager): Also create the file pager.
> (diskfs_get_filemap): Handout pagers from the file_pager_bucket.
> (diskfs_shutdown_pager): This is only concerned with the file pager.
> Simplify code accordingly.
> (diskfs_sync_everything): Likewise.
> (diskfs_pager_users): Likewise.
> (diskfs_max_user_pager_prot): Likewise.
> (disable_caching): Iterate over both buckets.
> (enable_caching): Likewise.
> ---
>  ext2fs/pager.c | 87 
> +++++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 62 insertions(+), 25 deletions(-)
> 
> diff --git a/ext2fs/pager.c b/ext2fs/pager.c
> index 9116b8c..017efcc 100644
> --- a/ext2fs/pager.c
> +++ b/ext2fs/pager.c
> @@ -21,14 +21,18 @@
>  #include <unistd.h>
>  #include <string.h>
>  #include <errno.h>
> +#include <error.h>
>  #include <hurd/store.h>
>  #include "ext2fs.h"
>  
>  /* XXX */
>  #include "../libpager/priv.h"
>  
> -/* A ports bucket to hold pager ports.  */
> -struct port_bucket *pager_bucket;
> +/* A ports bucket to hold disk pager ports.  */
> +struct port_bucket *disk_pager_bucket;
> +
> +/* A ports bucket to hold file pager ports.  */
> +struct port_bucket *file_pager_bucket;
>  
>  pthread_spinlock_t node_to_page_lock = PTHREAD_SPINLOCK_INITIALIZER;
>  
> @@ -1188,21 +1192,56 @@ disk_cache_block_is_ref (block_t block)
>    return ref;
>  }
>  
> -/* Create the DISK pager.  */
> +/* A top-level function for the paging thread that just services paging
> +   requests.  */
> +static void *
> +service_paging_requests (void *arg)
> +{
> +  struct port_bucket *pager_bucket = arg;
> +  ports_manage_port_operations_multithread (pager_bucket,
> +                                         pager_demuxer,
> +                                         1000,
> +                                         0,
> +                                         NULL);
> +  /* Not reached.  */
> +  return NULL;
> +}
> +
> +/* Create the disk pager, and the file pager.  */
>  void
>  create_disk_pager (void)
>  {
> +  pthread_t thread;
> +  pthread_attr_t attr;
> +  error_t err;
> +
> +  /* The disk pager.  */
>    struct user_pager_info *upi = malloc (sizeof (struct user_pager_info));
>    if (!upi)
>      ext2_panic ("can't create disk pager: %s", strerror (errno));
>    upi->type = DISK;
> -  pager_bucket = ports_create_bucket ();
> +  disk_pager_bucket = ports_create_bucket ();
>    get_hypermetadata ();
>    disk_cache_blocks = DISK_CACHE_BLOCKS;
>    disk_cache_size = disk_cache_blocks << log2_block_size;
> -  diskfs_start_disk_pager (upi, pager_bucket, MAY_CACHE, 1,
> +  diskfs_start_disk_pager (upi, disk_pager_bucket, MAY_CACHE, 1,
>                          disk_cache_size, &disk_cache);
>    disk_cache_init ();
> +
> +  /* The file pager.  */
> +  file_pager_bucket = ports_create_bucket ();
> +
> +#define STACK_SIZE (64 * 1024)
> +  pthread_attr_init (&attr);
> +  pthread_attr_setstacksize (&attr, STACK_SIZE);
> +#undef STACK_SIZE
> +
> +  /* Make a thread to service file paging requests.  */
> +  err = pthread_create (&thread, &attr,
> +                     service_paging_requests, file_pager_bucket);
> +  if (err)
> +    error (2, err, "pthread_create");
> +  pthread_detach (thread);
>  }
>  
>  /* Call this to create a FILE_DATA pager and return a send right.
> @@ -1241,7 +1280,7 @@ diskfs_get_filemap (struct node *node, vm_prot_t prot)
>         upi->max_prot = prot;
>         diskfs_nref_light (node);
>         node->dn->pager =
> -         pager_create (upi, pager_bucket, MAY_CACHE,
> +         pager_create (upi, file_pager_bucket, MAY_CACHE,
>                         MEMORY_OBJECT_COPY_DELAY, 0);
>         if (node->dn->pager == 0)
>           {
> @@ -1324,14 +1363,13 @@ diskfs_shutdown_pager ()
>    error_t shutdown_one (void *v_p)
>      {
>        struct pager *p = v_p;
> -      if (p != diskfs_disk_pager)
> -     pager_shutdown (p);
> +      pager_shutdown (p);
>        return 0;
>      }
>  
>    write_all_disknodes ();
>  
> -  ports_bucket_iterate (pager_bucket, shutdown_one);
> +  ports_bucket_iterate (file_pager_bucket, shutdown_one);
>  
>    /* Sync everything on the the disk pager.  */
>    sync_global (1);
> @@ -1347,13 +1385,12 @@ diskfs_sync_everything (int wait)
>    error_t sync_one (void *v_p)
>      {
>        struct pager *p = v_p;
> -      if (p != diskfs_disk_pager)
> -     pager_sync (p, wait);
> +      pager_sync (p, wait);
>        return 0;
>      }
>  
>    write_all_disknodes ();
> -  ports_bucket_iterate (pager_bucket, sync_one);
> +  ports_bucket_iterate (file_pager_bucket, sync_one);
>  
>    /* Do things on the the disk pager.  */
>    sync_global (wait);
> @@ -1372,7 +1409,8 @@ disable_caching ()
>  
>    /* Loop through the pagers and turn off caching one by one,
>       synchronously.  That should cause termination of each pager. */
> -  ports_bucket_iterate (pager_bucket, block_cache);
> +  ports_bucket_iterate (disk_pager_bucket, block_cache);
> +  ports_bucket_iterate (file_pager_bucket, block_cache);
>  }
>  
>  static void
> @@ -1400,7 +1438,8 @@ enable_caching ()
>        return 0;
>      }
>  
> -  ports_bucket_iterate (pager_bucket, enable_cache);
> +  ports_bucket_iterate (disk_pager_bucket, enable_cache);
> +  ports_bucket_iterate (file_pager_bucket, enable_cache);
>  }
>  
>  /* Tell diskfs if there are pagers exported, and if none, then
> @@ -1408,7 +1447,7 @@ enable_caching ()
>  int
>  diskfs_pager_users ()
>  {
> -  int npagers = ports_count_bucket (pager_bucket);
> +  int npagers = ports_count_bucket (file_pager_bucket);
>  
>    if (npagers <= 1)
>      return 0;
> @@ -1421,8 +1460,8 @@ diskfs_pager_users ()
>        immediately.  XXX */
>        sleep (1);
>  
> -      npagers = ports_count_bucket (pager_bucket);
> -      if (npagers <= 1)
> +      npagers = ports_count_bucket (file_pager_bucket);
> +      if (npagers == 0)
>       return 0;
>  
>        /* Darn, there are actual honest users.  Turn caching back on,
> @@ -1430,7 +1469,7 @@ diskfs_pager_users ()
>        enable_caching ();
>      }
>  
> -  ports_enable_bucket (pager_bucket);
> +  ports_enable_bucket (file_pager_bucket);
>  
>    return 1;
>  }
> @@ -1441,17 +1480,15 @@ vm_prot_t
>  diskfs_max_user_pager_prot ()
>  {
>    vm_prot_t max_prot = 0;
> -  int npagers = ports_count_bucket (pager_bucket);
> +  int npagers = ports_count_bucket (file_pager_bucket);
>  
> -  if (npagers > 1)
> -    /* More than just the disk pager.  */
> +  if (npagers > 0)
>      {
>        error_t add_pager_max_prot (void *v_p)
>       {
>         struct pager *p = v_p;
>         struct user_pager_info *upi = pager_get_upi (p);
> -       if (upi->type == FILE_DATA)
> -         max_prot |= upi->max_prot;
> +       max_prot |= upi->max_prot;
>         /* Stop iterating if MAX_PROT is as filled as it's going to get. */
>         return max_prot == (VM_PROT_READ|VM_PROT_WRITE|VM_PROT_EXECUTE);
>       }
> @@ -1462,12 +1499,12 @@ diskfs_max_user_pager_prot ()
>        immediately.  XXX */
>        sleep (1);
>  
> -      ports_bucket_iterate (pager_bucket, add_pager_max_prot);
> +      ports_bucket_iterate (file_pager_bucket, add_pager_max_prot);
>  
>        enable_caching ();
>      }
>  
> -  ports_enable_bucket (pager_bucket);
> +  ports_enable_bucket (file_pager_bucket);
>  
>    return max_prot;
>  }
> -- 
> 1.9.2
> 

-- 
Samuel
 >bah moi j'aime bien le flash et je cherche plus a comprendre
 >crosoft. Ca plante : je reinstalle
 Ca à le mérite de créer des emplois jeunes : "réinstalleur de crosoft"
 -+- BD in NPC : Bill Gates au secours de l'emploi -+-