[PATCH] Add PAGE_CACHE_PAGES

[PATCH] Add PAGE_CACHE_PAGES

[Gilad Ben-Yossef]

This patch introduces PAGE_CACHE_PAGES which is the number of pages in a
page cache.

Why?

Thus spake mm/shmem.c:

  idx = (address - vma->vm_start) >> PAGE_CACHE_SHIFT;
  idx += vma->vm_pgoff;

But include/linux/mm.h says:

unsigned long vm_pgoff;         /* Offset (within vm_file) in PAGE_SIZE
                                   units, *not* PAGE_CACHE_SIZE */
and include/linux/pagemap.h adds:

 * The page cache can done in larger chunks than
 * one page, because it allows for more efficient
 * throughput (it can then be mapped into user
 * space in smaller chunks for same flexibility).
 *
 * Or rather, it _will_ be done in larger chunks.

I also found two other places in mm/filemap.c that treat vma->vm_pgoff
as counting in PAGE_CACHE_SIZE units.

This patch only makes sense if PAGE_CACHE_SIZE is a multiple of
PAGE_SIZE but it seems logical to me that they will continue to be so
even when they wont be equel as they are now.

Comments are welcome.

Gilad.



diff -Nur -X /home/gby/dontdiff linux-2.4.19/include/linux/pagemap.h linux/include/linux/pagemap.h
--- linux-2.4.19/include/linux/pagemap.h	Fri Jun 21 01:38:38 2002
+++ linux/include/linux/pagemap.h	Thu Aug 15 21:53:15 2002
@@ -27,6 +27,7 @@
 #define PAGE_CACHE_SIZE		PAGE_SIZE
 #define PAGE_CACHE_MASK		PAGE_MASK
 #define PAGE_CACHE_ALIGN(addr)	(((addr)+PAGE_CACHE_SIZE-1)&PAGE_CACHE_MASK)
+#define PAGE_CACHE_PAGES         (PAGE_CACHE_SIZE / PAGE_SIZE)
 
 #define page_cache_get(x)	get_page(x)
 #define page_cache_release(x)	__free_page(x)
diff -Nur -X /home/gby/dontdiff linux-2.4.19/mm/filemap.c linux/mm/filemap.c
--- linux-2.4.19/mm/filemap.c	Thu Aug 15 21:31:38 2002
+++ linux/mm/filemap.c	Thu Aug 15 21:45:48 2002
@@ -1918,8 +1918,8 @@
 	struct page *page, **hash;
 	unsigned long size, pgoff, endoff;
 
-	pgoff = ((address - area->vm_start) >> PAGE_CACHE_SHIFT) + area->vm_pgoff;
-	endoff = ((area->vm_end - area->vm_start) >> PAGE_CACHE_SHIFT) + area->vm_pgoff;
+	pgoff = ((address - area->vm_start) >> PAGE_CACHE_SHIFT) + (area->vm_pgoff / PAGE_CACHE_PAGES);
+	endoff = ((area->vm_end - area->vm_start) >> PAGE_CACHE_SHIFT) + (area->vm_pgoff / PAGE_CACHE_PAGES);
 
 retry_all:
 	/*
diff -Nur -X /home/gby/dontdiff linux-2.4.19/mm/shmem.c linux/mm/shmem.c
--- linux-2.4.19/mm/shmem.c	Thu Aug 15 21:31:38 2002
+++ linux/mm/shmem.c	Thu Aug 15 21:41:21 2002
@@ -663,7 +663,7 @@
 	struct inode * inode = vma->vm_file->f_dentry->d_inode;
 
 	idx = (address - vma->vm_start) >> PAGE_CACHE_SHIFT;
-	idx += vma->vm_pgoff;
+	idx += (vma->vm_pgoff / PAGE_CACHE_PAGES);
 
 	if (shmem_getpage(inode, idx, &page))
 		return page;


-- 
Gilad Ben-Yossef <gilad@benyossef.com>
http://benyossef.com

"Money talks, bullshit walks and GNU awks."
  -- Shachar "Sun" Shemesh, debt collector for the GNU/Yakuza

Re: [PATCH] Add PAGE_CACHE_PAGES

[Hugh Dickins]

On 15 Aug 2002, Gilad Ben-Yossef wrote:
> 
> Thus spake mm/shmem.c:
> 
>   idx = (address - vma->vm_start) >> PAGE_CACHE_SHIFT;
>   idx += vma->vm_pgoff;
> 
> But include/linux/mm.h says:
> 
> unsigned long vm_pgoff;         /* Offset (within vm_file) in PAGE_SIZE
>                                    units, *not* PAGE_CACHE_SIZE */

Good observation, but it's certainly not something for Marcelo to worry
about for 2.4.  This is only one of many confusions which would need to
be fixed to get PAGE_CACHE_SIZE > PAGE_SIZE working.  mm/shmem.c
is probably especially confused (since it's backing a PAGE_CACHE_SIZE-
orientated filesystem with PAGE_SIZE-orientated swap), but much else too.

I think it was a mistake to introduce the PAGE_CACHE_SIZE definition
before it could be allowed to diverge from PAGE_SIZE: I find it very
hard to work out the right way to go, and would prefer it to vanish. 
But Ben LaHaise did succeed in getting a PAGE_CACHE_SIZE > PAGE_SIZE
patch working on 2.4.6 (no patch to mm/shmem.c, but it was a little
different then anyway, right or wrong I don't know).

Hugh

Re: [PATCH] Add PAGE_CACHE_PAGES

[Rusty Russell]

In message <1029443580.2508.18.camel@gby.benyossef.com> you write:
> 
> This patch introduces PAGE_CACHE_PAGES which is the number of pages in a
> page cache.

AFAICT, you should simply do

/* Order of pages in page cache */
#define PAGE_CACHE_PAGE_ORDER 0

#define PAGE_CACHE_SHIFT	(PAGE_SHIFT + PAGE_CACHE_PAGE_ORDER)
#define PAGE_CACHE_SIZE		(1 << PAGE_CACHE_SHIFT)
#define PAGE_CACHE_MASK		(~(PAGE_CACHE_SIZE-1))
#define PAGE_CACHE_ALIGN(addr)	(((addr)+PAGE_CACHE_SIZE-1)&PAGE_CACHE_MASK)

*Then* fix up the misuses,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] Add PAGE_CACHE_PAGES

[Gilad Ben-Yossef]

On Mon, 2002-08-19 at 06:56, Rusty Russell wrote:
> > 
> > This patch introduces PAGE_CACHE_PAGES which is the number of pages in a
> > page cache.
> 
> AFAICT, you should simply do
> 
> /* Order of pages in page cache */
> #define PAGE_CACHE_PAGE_ORDER 0

Yes, it is simpler and better. I bow to your superiour code-fu :-)

Here is the patch done Rusty's way and against 2.5.31 as per Hugh
Dickins indication that this is not apropriate to fix in 2.4.x.

Recap: trying to solve the problem that shmem.c and filemap.c treat
vma->vm_pgoff as containing PAGE_CACHE_SIZE blocks despite the explicit
warning that it does not in include/linux/mm.h. Doesn't really hurt us
now but as is this may break horribly when PAGE_CACHE_SIZE !=
PAGE_SIZE).

Thanks,
Gilad

diff -Nur -X dontdiff linux-2.5.31/include/linux/pagemap.h linux-2.5.31-gby/include/linux/pagemap.h
--- linux-2.5.31/include/linux/pagemap.h	Sun Aug 11 04:41:17 2002
+++ linux-2.5.31-gby/include/linux/pagemap.h	Mon Aug 19 10:39:10 2002
@@ -17,10 +17,14 @@
  *
  * Or rather, it _will_ be done in larger chunks.
  */
-#define PAGE_CACHE_SHIFT	PAGE_SHIFT
-#define PAGE_CACHE_SIZE		PAGE_SIZE
-#define PAGE_CACHE_MASK		PAGE_MASK
-#define PAGE_CACHE_ALIGN(addr)	(((addr)+PAGE_CACHE_SIZE-1)&PAGE_CACHE_MASK)
+
+/* Order of pages in page cache */
+#define PAGE_CACHE_PAGE_ORDER 0
+
+#define PAGE_CACHE_SHIFT        (PAGE_SHIFT + PAGE_CACHE_PAGE_ORDER)
+#define PAGE_CACHE_SIZE         (1 << PAGE_CACHE_SHIFT)
+#define PAGE_CACHE_MASK         (~(PAGE_CACHE_SIZE-1))
+#define PAGE_CACHE_ALIGN(addr)  (((addr)+PAGE_CACHE_SIZE-1)&PAGE_CACHE_MASK)
 
 #define page_cache_get(x)	get_page(x)
 extern void FASTCALL(page_cache_release(struct page *));
diff -Nur -X dontdiff linux-2.5.31/mm/filemap.c linux-2.5.31-gby/mm/filemap.c
--- linux-2.5.31/mm/filemap.c	Sun Aug 11 04:41:26 2002
+++ linux-2.5.31-gby/mm/filemap.c	Mon Aug 19 10:59:07 2002
@@ -1186,8 +1186,8 @@
 	unsigned long size, pgoff, endoff;
 	int did_readahead;
 
-	pgoff = ((address - area->vm_start) >> PAGE_CACHE_SHIFT) + area->vm_pgoff;
-	endoff = ((area->vm_end - area->vm_start) >> PAGE_CACHE_SHIFT) + area->vm_pgoff;
+	pgoff = ((address - area->vm_start) >> PAGE_CACHE_SHIFT) + (area->vm_pgoff >> PAGE_CACHE_PAGE_ORDER);
+	endoff = ((area->vm_end - area->vm_start) >> PAGE_CACHE_SHIFT) + (area->vm_pgoff >> PAGE_CACHE_PAGE_ORDER);
 
 retry_all:
 	/*
diff -Nur -X dontdiff linux-2.5.31/mm/shmem.c linux-2.5.31-gby/mm/shmem.c
--- linux-2.5.31/mm/shmem.c	Sun Aug 11 04:41:27 2002
+++ linux-2.5.31-gby/mm/shmem.c	Mon Aug 19 10:59:54 2002
@@ -731,7 +731,7 @@
 	struct inode * inode = vma->vm_file->f_dentry->d_inode;
 
 	idx = (address - vma->vm_start) >> PAGE_CACHE_SHIFT;
-	idx += vma->vm_pgoff;
+	idx += (vma->vm_pgoff >> PAGE_CACHE_PAGE_ORDER);
 
 	if (shmem_getpage(inode, idx, &page))
 		return page;

-- 
Gilad Ben-Yossef <gilad@benyossef.com>
Code mangler, senior coffee drinker and VP SIGSEGV
Qlusters ltd.

"You got an EMP device in the server room? That is so cool."
      -- from a hackers-il thread on paranoia