Skip to contentSkip to conversation

Should PAGE_CACHE_SIZE be discarded?

DISCUSSION David Howells 4 participants 9 messages Nov 14, 2007 – Nov 15, 2007

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Are we ever going to have PAGE_CACHE_SIZE != PAGE_SIZE? If not, why not discard PAGE_CACHE_SIZE as it's then redundant.

PAGE_CACHE_SIZE is also an ill-defined concept. Just grep in Documentation/ and it only comes up in a couple of places, neither particularly informative.

If there's a possibility that it will be used, then someone who knows how it's supposed to work needs to sit down and document what it is, what it represents, where it applies, how it interacts with PG_compound and how the page flags distribute over a page cache slot.

One further thing to consider: actually making PAGE_CACHE_SIZE > PAGE_SIZE work will be an interesting problem, as I'm certain most filesystems will break horribly without a lot of work (ramfs might be only exception).

David

#1 lore raw
Quoted 4 lines · David Howells

Are we ever going to have PAGE_CACHE_SIZE != PAGE_SIZE? If not, why not discard PAGE_CACHE_SIZE as it's then redundant.

Christoph Lameter has patches exactly to make PAGE_CACHE_SIZE larger than PAGE_SIZE, and they seem to work without much effort. I happen to hate the patches ;) but that doesn't change the fact that PAGE_CACHE_SIZE is relatively useful and it is not at all an ill-defined concept.

Quoted 7 lines

PAGE_CACHE_SIZE is also an ill-defined concept. Just grep in Documentation/ and it only comes up in a couple of places, neither particularly informative.

If there's a possibility that it will be used, then someone who knows how it's supposed to work needs to sit down and document what it is, what it represents, where it applies, how it interacts with PG_compound and how the page flags distribute over a page cache slot.

No, this was an *example*. It has nothing to do with PG_compound upsream. That was just an example. Basically, anything that goes in the page cache is in units of PAGE_CACHE_SIZE, and nothing else. For filesystems it should be pretty easy...

Quoted 3 lines

One further thing to consider: actually making PAGE_CACHE_SIZE > PAGE_SIZE work will be an interesting problem, as I'm certain most filesystems will break horribly without a lot of work (ramfs might be only exception).

I think most filesystems actually don't have much problem with it. mm code has bigger problems, eg. due to ptes <-> pagecache no longer being equal size.... but why would filesystems care?

#2 lore raw
Quoted 4 lines · Nick Piggin

Christoph Lameter has patches exactly to make PAGE_CACHE_SIZE larger than PAGE_SIZE, and they seem to work without much effort. I happen to hate the patches ;) but that doesn't change the fact that PAGE_CACHE_SIZE is relatively useful and it is not at all an ill-defined concept.

Where, please? mm kernels?

Quoted 3 lines

Basically, anything that goes in the page cache is in units of PAGE_CACHE_SIZE, and nothing else. For filesystems it should be pretty easy...

That depends on what the coverage of struct page is. I don't actually know whether this is PAGE_SIZE or PAGE_CACHE_SIZE; I assumed it to be the former, but from what you've said, I'm not actually sure.

David

#3 lore raw
Quoted 8 lines · David Howells
Quoted 4 lines · Nick Piggin

Christoph Lameter has patches exactly to make PAGE_CACHE_SIZE larger than PAGE_SIZE, and they seem to work without much effort. I happen to hate the patches ;) but that doesn't change the fact that PAGE_CACHE_SIZE is relatively useful and it is not at all an ill-defined concept.

Where, please? mm kernels?

Floating around. I'm not saying it will even get upstream. It's just an example.

Quoted 7 lines
Quoted 3 lines

Basically, anything that goes in the page cache is in units of PAGE_CACHE_SIZE, and nothing else. For filesystems it should be pretty easy...

That depends on what the coverage of struct page is. I don't actually know whether this is PAGE_SIZE or PAGE_CACHE_SIZE; I assumed it to be the former, but from what you've said, I'm not actually sure.

It can be pretty well any power of 2 from PAGE_SIZE upwards, with compound pages. None of the filesystems should really care at all. It's not even a new concept, hugetlbfs uses HPAGE_SIZE...

#4 lore raw
Quoted 3 lines · Nick Piggin

It can be pretty well any power of 2 from PAGE_SIZE upwards, with compound pages. None of the filesystems should really care at all. It's not even a new concept, hugetlbfs uses HPAGE_SIZE...

Ummm... The filesystem has to care. If the VFS/VM says 'fill this page' you do need to know how big the page is or whether it's even actually several pages.

David

#5 lore raw
Quoted 9 lines · David Howells
Quoted 3 lines · Nick Piggin

It can be pretty well any power of 2 from PAGE_SIZE upwards, with compound pages. None of the filesystems should really care at all. It's not even a new concept, hugetlbfs uses HPAGE_SIZE...

Ummm... The filesystem has to care. If the VFS/VM says 'fill this page' you do need to know how big the page is or whether it's even actually several pages.

I think that what Nick was trying to say is that PAGE_CACHE_SIZE should always be used properly as the size of the memory struct Page covers (while PAGE_SIZE is the hardware page size and the constraint is that PAGE_CACHE_SIZE == (PAGE_SIZE << k) for some k >= 0). If everybody does that then "None of the filesystems should really care at all". That said, it doesn't seem like the current usage in fs/ and drivers/ is consistent with this convention.

Quoted 3 lines

David
-

#6 lore raw
Quoted 7 lines · Benny Halevy

I think that what Nick was trying to say is that PAGE_CACHE_SIZE should always be used properly as the size of the memory struct Page covers (while PAGE_SIZE is the hardware page size and the constraint is that PAGE_CACHE_SIZE == (PAGE_SIZE << k) for some k >= 0). If everybody does that then "None of the filesystems should really care at all". That said, it doesn't seem like the current usage in fs/ and drivers/ is consistent with this convention.

Indeed. One thing you have to consider is kmap(). I would expect it to present an area of PAGE_SIZE for access. However, if the filesystem gets an area of PAGE_CACHE_SIZE to fill, then I would have to do multiple kmap() calls in the process of filling that 'pagecache page' in AFS.

Furthermore, if a page struct covers a PAGE_CACHE_SIZE chunk of memory, then I suspect the page allocator is also wrong, as it I believe it deals with PAGE_SIZE chunks of memory, assuming a struct page for each.

David

#7 lore raw
Quoted 10 lines · David Howells
Quoted 7 lines · Benny Halevy

I think that what Nick was trying to say is that PAGE_CACHE_SIZE should always be used properly as the size of the memory struct Page covers (while PAGE_SIZE is the hardware page size and the constraint is that PAGE_CACHE_SIZE == (PAGE_SIZE << k) for some k >= 0). If everybody does that then "None of the filesystems should really care at all". That said, it doesn't seem like the current usage in fs/ and drivers/ is consistent with this convention.

Quoted 8 lines

Indeed. One thing you have to consider is kmap(). I would expect it to present an area of PAGE_SIZE for access. However, if the filesystem gets an area of PAGE_CACHE_SIZE to fill, then I would have to do multiple kmap() calls in the process of filling that 'pagecache page' in AFS.

Furthermore, if a page struct covers a PAGE_CACHE_SIZE chunk of memory, then I suspect the page allocator is also wrong, as it I believe it deals with PAGE_SIZE chunks of memory, assuming a struct page for each.

That's because you're thinking about all these difficulties outside the filesystem. Really, pagecache is in PAGE_CACHE_SIZE. Nobody said pagecache

Quoted 1 line

PAGE_SIZE will work by changing a single define, but filesystems are

about the least problematic here.

#8 lore raw
In reply to David Howells
Quoted 3 lines · David Howells

Are we ever going to have PAGE_CACHE_SIZE != PAGE_SIZE? If not, why not discard PAGE_CACHE_SIZE as it's then redundant.

I did some digging. The code was merged between 28.4.1999 and 11.5.1999, as it first appears in 2.2.8 and 2.3.0:

/*
* 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.
*/

#define PAGE_CACHE_SHIFT        PAGE_SHIFT
#define PAGE_CACHE_SIZE         PAGE_SIZE
#define PAGE_CACHE_MASK         PAGE_MASK
#define page_cache_alloc()      __get_free_page(GFP_USER)
#define page_cache_free(x)      free_page(x)
#define page_cache_release(x)   __free_page(x)

Looks like PAGE_CACHE_SIZE>PAGE_SIZE was planned before 2.3 opened, but never went very far. So judging by the last eight years of history, PAGE_CACHE_SIZE is dead.

Completely discarding it might be a bad idea, as it serves as code annotation. Christoph Lameters "Large Blocksize Support" patches seem to globally replace PAGE_CACHE_SIZE with a different macro. Having PAGE_CACHE_SIZE makes such an effort easier. Dropping existing annotation seems useless, when maintaining them is zero effort.

Large code audits for the PAGE_SIZE/PAGE_CACHE_SIZE distinction seem completely useless, though. For as long as the macro existed, it never ever mattered. My take is to keep it and let it bitrot until someone actually cares.

Jörn

Signature · 6 lines
Joern's library part 6:
http://www.gzip.org/zlib/feldspar.html
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html