Re: [PATCH 2/2] fatfs: use two distinct pager buckets for the disk and file pager

[Samuel Thibault]

Justus Winter, le Mon 05 May 2014 17:33:12 +0200, a écrit :
> fatfs 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.

> * fatfs/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_fat_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.
> ---
>  fatfs/pager.c | 87 
> ++++++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 62 insertions(+), 25 deletions(-)
> 
> diff --git a/fatfs/pager.c b/fatfs/pager.c
> index 6180aac..f855ecf 100644
> --- a/fatfs/pager.c
> +++ b/fatfs/pager.c
> @@ -18,12 +18,16 @@
>     along with this program; if not, write to the Free Software
>     Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111, USA. */
>  
> +#include <error.h>
>  #include <string.h>
>  #include <hurd/store.h>
>  #include "fatfs.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;
>  
>  /* Mapped image of the FAT.  */
>  void *fat_image;
> @@ -752,16 +756,51 @@ pager_dropweak (struct user_pager_info *p __attribute__ 
> ((unused)))
>  {
>  }
>  
> +/* 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.  */
>  void
>  create_fat_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));
>    upi->type = FAT;
> -  pager_bucket = ports_create_bucket ();
> -  diskfs_start_disk_pager (upi, pager_bucket, MAY_CACHE, 0,
> +  disk_pager_bucket = ports_create_bucket ();
> +  diskfs_start_disk_pager (upi, disk_pager_bucket, MAY_CACHE, 0,
>                          bytes_per_sector * sectors_per_fat,
>                          &fat_image);
> +
> +  /* 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.
> @@ -800,7 +839,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)
>              {
> @@ -881,14 +920,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);
>  
>    pager_sync (diskfs_disk_pager, 1);
>  
> @@ -903,13 +941,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);
>    pager_sync (diskfs_disk_pager, wait);
>  }
>  
> @@ -926,7 +963,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
> @@ -954,7 +992,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
> @@ -962,9 +1001,9 @@ enable_caching ()
>  int
>  diskfs_pager_users ()
>  {
> -  int npagers = ports_count_bucket (pager_bucket);
> +  int npagers = ports_count_bucket (file_pager_bucket);
>  
> -  if (npagers <= 1)
> +  if (npagers == 0)
>      return 0;
>  
>    if (MAY_CACHE)
> @@ -975,8 +1014,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,
> @@ -984,7 +1023,7 @@ diskfs_pager_users ()
>        enable_caching ();
>      }
>    
> -  ports_enable_bucket (pager_bucket);
> +  ports_enable_bucket (file_pager_bucket);
>  
>    return 1;
>  }
> @@ -995,17 +1034,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 is going to
>            get.  */
>            return max_prot == (VM_PROT_READ|VM_PROT_WRITE|VM_PROT_EXECUTE);
> @@ -1017,12 +1054,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;
>  }
> -- 
> 2.0.0.rc0
> 

-- 
Samuel
 Battery 1: charging, 90%, charging at zero rate - will never fully charge.
 -+- acpi - et pourtant, ca monte -+-