[RESEND][PATCH 5/6] Provide a filesystem-specific sync'able page bit

[RESEND][PATCH 5/6] Provide a filesystem-specific sync'able page bit

[David Howells]

Because Christoph asked so nicely, I'm resending this patch with a wider
distribution. The patch currently resides in the -mm kernels. It is being used
by the in-kernel AFS filesystem and by the NFS caching patches that haven't
yet been passed to akpm.

---

The attached patch provides a filesystem-specific page bit that a filesystem
can synchronise upon. This can be used, for example, by a netfs to synchronise
with CacheFS writing its pages to disc.

Signed-Off-By: David Howells <dhowells@redhat.com>
---

warthog>diffstat cachefs-pagefsmisc-269rc1mm2.diff 
 include/linux/page-flags.h |   10 ++++++++++
 include/linux/pagemap.h    |   11 +++++++++++
 mm/filemap.c               |   17 +++++++++++++++++
 3 files changed, 38 insertions(+)

diff -uNrp linux-2.6.9-rc1-mm2/include/linux/page-flags.h linux-2.6.9-rc1-mm2-cachefs/include/linux/page-flags.h
--- linux-2.6.9-rc1-mm2/include/linux/page-flags.h	2004-08-31 16:52:38.000000000 +0100
+++ linux-2.6.9-rc1-mm2-cachefs/include/linux/page-flags.h	2004-09-02 15:15:42.000000000 +0100
@@ -62,6 +62,7 @@
 #define PG_slab			 7	/* slab debug (Suparna wants this) */
 
 #define PG_highmem		 8
+#define PG_fs_misc		 9	/* Filesystem specific bit */
 #define PG_checked		 9	/* kill me in 2.5.<early>. */
 #define PG_arch_1		10
 #define PG_reserved		11
@@ -315,4 +316,13 @@ static inline void set_page_writeback(st
 	test_set_page_writeback(page);
 }
 
+/*
+ * Filesystem-specific page bit testing
+ */
+#define PageFsMisc(page)		test_bit(PG_fs_misc, &(page)->flags)
+#define SetPageFsMisc(page)		set_bit(PG_fs_misc, &(page)->flags)
+#define TestSetPageFsMisc(page)		test_and_set_bit(PG_fs_misc, &(page)->flags)
+#define ClearPageFsMisc(page)		clear_bit(PG_fs_misc, &(page)->flags)
+#define TestClearPageFsMisc(page)	test_and_clear_bit(PG_fs_misc, &(page)->flags)
+
 #endif	/* PAGE_FLAGS_H */
diff -uNrp linux-2.6.9-rc1-mm2/include/linux/pagemap.h linux-2.6.9-rc1-mm2-cachefs/include/linux/pagemap.h
--- linux-2.6.9-rc1-mm2/include/linux/pagemap.h	2004-08-31 16:52:38.000000000 +0100
+++ linux-2.6.9-rc1-mm2-cachefs/include/linux/pagemap.h	2004-09-02 15:17:26.000000000 +0100
@@ -192,6 +192,17 @@ static inline void wait_on_page_writebac
 extern void end_page_writeback(struct page *page);
 
 /*
+ * Wait for filesystem-specific page synchronisation to complete
+ */
+static inline void wait_on_page_fs_misc(struct page *page)
+{
+	if (PageFsMisc(page))
+		wait_on_page_bit(page, PG_fs_misc);
+}
+
+extern void fastcall end_page_fs_misc(struct page *page);
+
+/*
  * Fault a userspace page into pagetables.  Return non-zero on a fault.
  *
  * This assumes that two userspace pages are always sufficient.  That's
diff -uNrp linux-2.6.9-rc1-mm2/mm/filemap.c linux-2.6.9-rc1-mm2-cachefs/mm/filemap.c
--- linux-2.6.9-rc1-mm2/mm/filemap.c	2004-08-31 16:52:40.000000000 +0100
+++ linux-2.6.9-rc1-mm2-cachefs/mm/filemap.c	2004-09-03 16:46:58.780545691 +0100
@@ -442,6 +442,23 @@ void fastcall __lock_page(struct page *p
 EXPORT_SYMBOL(__lock_page);
 
 /*
+ * Note completion of filesystem specific page synchronisation
+ *
+ * This is used to allow a page to be written to a filesystem cache in the
+ * background without holding up the completion of readpage
+ */
+void fastcall end_page_fs_misc(struct page *page)
+{
+	smp_mb__before_clear_bit();
+	if (!TestClearPageFsMisc(page))
+		BUG();
+	smp_mb__after_clear_bit(); 
+	wake_up_page(page, PG_fs_misc);
+}
+
+EXPORT_SYMBOL(end_page_fs_misc);
+
+/*
  * a rather lightweight function, finding and getting a reference to a
  * hashed page atomically.
  */
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: [RESEND][PATCH 5/6] Provide a filesystem-specific sync'able page bit

[Matthew Wilcox]

On Thu, Oct 14, 2004 at 08:05:07PM +0100, David Howells wrote:
>  #define PG_highmem		 8
> +#define PG_fs_misc		 9	/* Filesystem specific bit */
>  #define PG_checked		 9	/* kill me in 2.5.<early>. */
>  #define PG_arch_1		10
>  #define PG_reserved		11
> @@ -315,4 +316,13 @@ static inline void set_page_writeback(st
>  	test_set_page_writeback(page);
>  }
>  
> +/*
> + * Filesystem-specific page bit testing
> + */
> +#define PageFsMisc(page)		test_bit(PG_fs_misc, &(page)->flags)
> +#define SetPageFsMisc(page)		set_bit(PG_fs_misc, &(page)->flags)
> +#define TestSetPageFsMisc(page)		test_and_set_bit(PG_fs_misc, &(page)->flags)
> +#define ClearPageFsMisc(page)		clear_bit(PG_fs_misc, &(page)->flags)
> +#define TestClearPageFsMisc(page)	test_and_clear_bit(PG_fs_misc, &(page)->flags)
> +
>  #endif	/* PAGE_FLAGS_H */

That's not really enough documentation.  Who sets this flag?  Who clears this
flag?  Currently, mm/page_alloc.c clears this flag:

./mm/page_alloc.c:                      1 << PG_checked | 1 << PG_mappedtodisk);

which probably wouldn't be noticed by someone grepping for uses.
If you're going to not kill this flag, at least rename it so we don't
have two defines for the same bit.

The other 'misc bit' has documentation of the form:

 * PG_arch_1 is an architecture specific page state bit.  The generic code
 * guarantees that this bit is cleared for a page when it first is entered into
 * the page cache.

which really ought to at least mention Documentation/cachetlb.txt

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: [RESEND][PATCH 5/6] Provide a filesystem-specific sync'able page bit

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 221 bytes --]

hi :)

On Thu, Oct 14, 2004 at 08:05:07PM +0100, David Howells wrote:
> +#define PG_fs_misc		 9	/* Filesystem specific bit */

name it PG_fs_private if it is intended to be used by the fs only?

-- 
Martin Waitz

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [RESEND][PATCH 5/6] Provide a filesystem-specific sync'able page bit

[David Howells]

> > +#define PG_fs_misc		 9	/* Filesystem specific bit */
> ...
> That's not really enough documentation.  Who sets this flag?  Who clears this
> flag?

That's up to the owning filesystem (or device file, I suppose); hence
"filesystem-specific". I could expand this a little, but there isn't much to
say - it's entirely up to the filesystem, though I think I should probably
require it to be cleared before the page is freed.

> Currently, mm/page_alloc.c clears this flag:

Not really; that's irrelevant. It checks to see if it is set when it allocates
a page, and if it is it complains bitterly. It then splats this bit and all
others to make sure struct page has all its flags in the ground state.

> If you're going to not kill this flag, at least rename it so we don't
> have two defines for the same bit.

I did have it as a different number, but I was told to make it the same as
PG_checked. I could rename all instances of PG_checked, I suppose...

> which really ought to at least mention Documentation/cachetlb.txt

Fix it then.

David
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>