All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: David Howells <dhowells@redhat.com>
Cc: torvalds@osdl.org, akpm@osdl.org, steved@redhat.com,
	trond.myklebust@fys.uio.no, linux-fsdevel@vger.kernel.org,
	linux-cachefs@redhat.com, nfsv4@linux-nfs.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 02/14] FS-Cache: Recruit a couple of page flags for cache management
Date: Wed, 01 Aug 2007 08:17:35 +0200	[thread overview]
Message-ID: <1185949055.12034.31.camel@twins> (raw)
In-Reply-To: <20070731202505.6412.37918.stgit@warthog.cambridge.redhat.com>

On Tue, 2007-07-31 at 21:25 +0100, David Howells wrote:

> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 209d3a4..eaf9854 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -83,19 +83,24 @@
>  #define PG_private		11	/* If pagecache, has fs-private data */
>  
>  #define PG_writeback		12	/* Page is under writeback */
> +#define PG_owner_priv_2		13	/* Owner use. If pagecache, fs may use */
>  #define PG_compound		14	/* Part of a compound page */
>  #define PG_swapcache		15	/* Swap page: swp_entry_t in private */
>  
>  #define PG_mappedtodisk		16	/* Has blocks allocated on-disk */
>  #define PG_reclaim		17	/* To be reclaimed asap */
> +#define PG_owner_priv_3		18	/* Owner use. If pagecache, fs may use */
>  #define PG_buddy		19	/* Page is free, on buddy lists */
>  
>  /* PG_readahead is only used for file reads; PG_reclaim is only for writes */
>  #define PG_readahead		PG_reclaim /* Reminder to do async read-ahead */
>  
> -/* PG_owner_priv_1 users should have descriptive aliases */
> +/* PG_owner_priv_1/2/3 users should have descriptive aliases */
>  #define PG_checked		PG_owner_priv_1 /* Used by some filesystems */
>  #define PG_pinned		PG_owner_priv_1	/* Xen pinned pagetable */
> +#define PG_fscache		PG_owner_priv_2	/* Backed by local cache */
> +#define PG_fscache_write	PG_owner_priv_3	/* Writing to local cache */
> +

 
> +/**
> + * page_has_private - Determine if page has private stuff
> + * @page: The page to be checked
> + *
> + * Determine if a page has private stuff, indicating that release routines
> + * should be invoked upon it.
> + */
> +#define page_has_private(page)			\
> +	((page)->flags & ((1 << PG_private) |	\
> +			  (1 << PG_fscache)))
> +


Not sure its a good idea to overload page_has_private() with an
overloadable page-flag. What if some future FS wants to use
PG_owner_priv_2 for other purposes?

Obviously filesystems cannot use these two page-flags if they want to be
compatible with FS-cache, but need all filesystems be?


(also, ouch! - 2 pageflags)


WARNING: multiple messages have this Message-ID (diff)
From: Peter Zijlstra <peterz@infradead.org>
To: David Howells <dhowells@redhat.com>
Cc: akpm@osdl.org, linux-kernel@vger.kernel.org, nfsv4@linux-nfs.org,
	trond.myklebust@fys.uio.no, torvalds@osdl.org,
	linux-cachefs@redhat.com, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 02/14] FS-Cache: Recruit a couple of page flags for cache management
Date: Wed, 01 Aug 2007 08:17:35 +0200	[thread overview]
Message-ID: <1185949055.12034.31.camel@twins> (raw)
In-Reply-To: <20070731202505.6412.37918.stgit@warthog.cambridge.redhat.com>

On Tue, 2007-07-31 at 21:25 +0100, David Howells wrote:

> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 209d3a4..eaf9854 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -83,19 +83,24 @@
>  #define PG_private		11	/* If pagecache, has fs-private data */
>  
>  #define PG_writeback		12	/* Page is under writeback */
> +#define PG_owner_priv_2		13	/* Owner use. If pagecache, fs may use */
>  #define PG_compound		14	/* Part of a compound page */
>  #define PG_swapcache		15	/* Swap page: swp_entry_t in private */
>  
>  #define PG_mappedtodisk		16	/* Has blocks allocated on-disk */
>  #define PG_reclaim		17	/* To be reclaimed asap */
> +#define PG_owner_priv_3		18	/* Owner use. If pagecache, fs may use */
>  #define PG_buddy		19	/* Page is free, on buddy lists */
>  
>  /* PG_readahead is only used for file reads; PG_reclaim is only for writes */
>  #define PG_readahead		PG_reclaim /* Reminder to do async read-ahead */
>  
> -/* PG_owner_priv_1 users should have descriptive aliases */
> +/* PG_owner_priv_1/2/3 users should have descriptive aliases */
>  #define PG_checked		PG_owner_priv_1 /* Used by some filesystems */
>  #define PG_pinned		PG_owner_priv_1	/* Xen pinned pagetable */
> +#define PG_fscache		PG_owner_priv_2	/* Backed by local cache */
> +#define PG_fscache_write	PG_owner_priv_3	/* Writing to local cache */
> +

 
> +/**
> + * page_has_private - Determine if page has private stuff
> + * @page: The page to be checked
> + *
> + * Determine if a page has private stuff, indicating that release routines
> + * should be invoked upon it.
> + */
> +#define page_has_private(page)			\
> +	((page)->flags & ((1 << PG_private) |	\
> +			  (1 << PG_fscache)))
> +


Not sure its a good idea to overload page_has_private() with an
overloadable page-flag. What if some future FS wants to use
PG_owner_priv_2 for other purposes?

Obviously filesystems cannot use these two page-flags if they want to be
compatible with FS-cache, but need all filesystems be?


(also, ouch! - 2 pageflags)

  reply	other threads:[~2007-08-01  6:17 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-31 20:24 [PATCH 00/14] Permit filesystem local caching David Howells
2007-07-31 20:25 ` [PATCH 01/14] FS-Cache: Release page->private after failed readahead David Howells
2007-07-31 20:25 ` [PATCH 02/14] FS-Cache: Recruit a couple of page flags for cache management David Howells
2007-08-01  6:17   ` Peter Zijlstra [this message]
2007-08-01  6:17     ` Peter Zijlstra
2007-08-01  8:27     ` David Howells
2007-08-01  8:27       ` David Howells
2007-07-31 20:25 ` [PATCH 03/14] FS-Cache: Provide an add_wait_queue_tail() function David Howells
2007-07-31 20:25 ` [PATCH 04/14] FS-Cache: Generic filesystem caching facility David Howells
2007-07-31 20:25 ` [PATCH 05/14] CacheFiles: Add missing copy_page export for ia64 David Howells
2007-07-31 20:25 ` [PATCH 06/14] CacheFiles: Add a hook to write a single page of data to an inode David Howells
2007-07-31 20:25 ` [PATCH 07/14] CacheFiles: Permit the page lock state to be monitored David Howells
2007-07-31 20:25 ` [PATCH 08/14] CacheFiles: Export things for CacheFiles David Howells
2007-07-31 20:25 ` [PATCH 09/14] CacheFiles: Permit a process's create SID to be overridden David Howells
2007-07-31 20:25 ` [PATCH 10/14] CacheFiles: Add an act-as SID override in task_security_struct David Howells
2007-07-31 20:25 ` [PATCH 11/14] CacheFiles: Permit an inode's security ID to be obtained David Howells
2007-07-31 20:25 ` [PATCH 12/14] CacheFiles: Get the SID under which the CacheFiles module should operate David Howells
2007-07-31 20:26 ` [PATCH 13/14] CacheFiles: A cache that backs onto a mounted filesystem David Howells
2007-07-31 20:26 ` [PATCH 14/14] NFS: Use local caching David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1185949055.12034.31.camel@twins \
    --to=peterz@infradead.org \
    --cc=akpm@osdl.org \
    --cc=dhowells@redhat.com \
    --cc=linux-cachefs@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nfsv4@linux-nfs.org \
    --cc=steved@redhat.com \
    --cc=torvalds@osdl.org \
    --cc=trond.myklebust@fys.uio.no \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.