Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
On Wed, 26 Sep 2007 20:31:02 +0100 (BST) Hugh Dickins <[EMAIL PROTECTED]> wrote: > Would that waste a little memory? I think not with SLUB, > but perhaps with SLOB, which packs a little tighter. > maybe just depends on the amount of used anon_vma and page_mapping_info etc... I don't think a system which uses SLOB consumes such structs so much as that memory-for-alignment is considered as "waste" of memory. Anyway, I decided to go ahead with current container-info-per-page implementation. If the size of page struct is problem at mainline inclusion discussion, I'll be back. Thanks, -Kame - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
On Sat, 22 Sep 2007, KAMEZAWA Hiroyuki wrote: > On Fri, 21 Sep 2007 18:02:47 +0100 (BST) > Hugh Dickins <[EMAIL PROTECTED]> wrote: > > > Or should I now leave PG_swapcache as is, > > given your designs on page->mapping? > > > will conflict with my idea ? > == > http://marc.info/?l=linux-mm=118956492926821=2 > == I asked because I had thought it would be a serious conflict: obviously the patches as such would conflict quite a bit, but that's not serious, one or the other just gets fixed up. But now I don't see it - we both want to grab a further bit from the low bits of the page->mapping pointer, you PAGE_MAPPING_INFO and me PAGE_MAPPING_SWAP; but that's okay, so long as whoever is left using bit (1<<2) is careful about the 32-bit case and remembers to put __attribute__((aligned(sizeof(long long on the declarations of struct address_space and struct anon_vma and your struct page_mapping_info. Would that waste a little memory? I think not with SLUB, but perhaps with SLOB, which packs a little tighter. Hugh - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] page-mapping clarification [1/3] base functions
On Sat, 22 Sep 2007, KAMEZAWA Hiroyuki wrote: On Fri, 21 Sep 2007 18:02:47 +0100 (BST) Hugh Dickins [EMAIL PROTECTED] wrote: Or should I now leave PG_swapcache as is, given your designs on page-mapping? will conflict with my idea ? == http://marc.info/?l=linux-mmm=118956492926821w=2 == I asked because I had thought it would be a serious conflict: obviously the patches as such would conflict quite a bit, but that's not serious, one or the other just gets fixed up. But now I don't see it - we both want to grab a further bit from the low bits of the page-mapping pointer, you PAGE_MAPPING_INFO and me PAGE_MAPPING_SWAP; but that's okay, so long as whoever is left using bit (12) is careful about the 32-bit case and remembers to put __attribute__((aligned(sizeof(long long on the declarations of struct address_space and struct anon_vma and your struct page_mapping_info. Would that waste a little memory? I think not with SLUB, but perhaps with SLOB, which packs a little tighter. Hugh - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] page-mapping clarification [1/3] base functions
On Wed, 26 Sep 2007 20:31:02 +0100 (BST) Hugh Dickins [EMAIL PROTECTED] wrote: Would that waste a little memory? I think not with SLUB, but perhaps with SLOB, which packs a little tighter. maybe just depends on the amount of used anon_vma and page_mapping_info etc... I don't think a system which uses SLOB consumes such structs so much as that memory-for-alignment is considered as waste of memory. Anyway, I decided to go ahead with current container-info-per-page implementation. If the size of page struct is problem at mainline inclusion discussion, I'll be back. Thanks, -Kame - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
On Fri, 21 Sep 2007 18:02:47 +0100 (BST) Hugh Dickins <[EMAIL PROTECTED]> wrote: > > 3. I want to *try* page->mapping overriding... store memory resource > > controller's > >information in page->mapping. By this, memory controller doesn't enlarge > > sizeof > >struct page. (works well in my small test.) > >Before doing that, I have to hide page->mapping from direct access. > > My own vote (nothing more) would be for you to set this aside until > some future time when there aren't a dozen developers all trampling > over each other in this area. > > They're invasive little changes affecting all filesystems, whereas what > we've done so far with page->mapping hasn't affected filesystems at all. > I found that each FS doesn't touch page->mapping so much as I expected. (except for ReiserFS) But ok, I admit changing this will confuse people. > 3: well, saving memory is good, but I think it could wait until some > other time, particularly since the memory controller isn't in yet. > Yes, if extra field in page struct is not hazard to push memory controller, I don't have much motivation. Because extra 8 bytes makes page struct to be 64 bytes(in 64bit), extra 8 bytes is the last space, I think. > If we were to attack page->mapping to save memory from struct page, > then we should consider Magnus Damm's idea too: he suggested it could > be replaced by a pointer to the radixtree slot (something else needed > in the anon case), from which "index" could be deduced via alignment > instead of keeping it in struct page (details to be filled in ...) > There is a bit difference. My purpose is "avoid making struct page larger", not "making struct page smaller". > Or should I now leave PG_swapcache as is, > given your designs on page->mapping? > will conflict with my idea ? == http://marc.info/?l=linux-mm=118956492926821=2 == Anyway, I'm not in hurry about this patch-set. I'll see what memory controller will go. Other people seems to have an idea to implement pfn <-> container_info_per_page function. (But this kind of function is not welcomed always.) Thank you for comments. > p.s. Sorry to niggle, but next time, please say [PATCH 1/3] etc. > rather than [PATCH] Long Description [1/3], so it's easier to > sort the mail subjects by eye in limited columns - thanks. > sorry, I'll consider well next time. Thanks, -Kame - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
On Fri, 21 Sep 2007, KAMEZAWA Hiroyuki wrote: > On Thu, 20 Sep 2007 11:26:47 -0700 (PDT) > Christoph Lameter <[EMAIL PROTECTED]> wrote: > > > > I am still a bit confused as to what the benefit of this is. > > > Honestly, I have 3 purposes, 2 for readability/clarificaton and 1 for my > trial. > > 1. Clarify page cache <-> inode relationship before *new concept of page > cache*, >yours or someone else's is introduced. > > 2. There are some places using PAGE_MAPPING_ANON directly. I don't want to see >following line in .c file. >== >anon_vma = (struct anon_vma *)(mapping - PAGE_MAPPING_ANON); >== > > 3. I want to *try* page->mapping overriding... store memory resource > controller's >information in page->mapping. By this, memory controller doesn't enlarge > sizeof >struct page. (works well in my small test.) >Before doing that, I have to hide page->mapping from direct access. My own vote (nothing more) would be for you to set this aside until some future time when there aren't a dozen developers all trampling over each other in this area. They're invasive little changes affecting all filesystems, whereas what we've done so far with page->mapping hasn't affected filesystems at all. Purposes 1 and 2 don't score very high in my book (though I too regret how mm/migrate.c copied that PAGE_MAPPING_ANON stuff from it's rightful home in mm/rmap.c: maybe we should wrap that). There's no end to the wrappers we can add, but they're not always helpful. 3: well, saving memory is good, but I think it could wait until some other time, particularly since the memory controller isn't in yet. Wouldn't it be easier to do something with page->lru than page->mapping? Everybody is interested in page->mapping, not so many in page->lru. (Though perhaps it wouldn't work out so well, since you don't need to get uniquely from mapping to page, whereas you do from lru to page.) If we were to attack page->mapping to save memory from struct page, then we should consider Magnus Damm's idea too: he suggested it could be replaced by a pointer to the radixtree slot (something else needed in the anon case), from which "index" could be deduced via alignment instead of keeping it in struct page (details to be filled in ...) Of course, my particular prejudice is that I promised months ago to free up the PG_swapcache bit by using a PAGE_MAPPING_SWAP bit instead. That patch got buried while I tried to think up a suitable name for a further page_mapping() variant that turned out to be needed - guess I should look through your collection to see if I can steal one ;) Beyond the unsatisfactory naming, that work has been long done (and like PAGE_MAPPING_ANON, doesn't touch filesystems at all). Or should I now leave PG_swapcache as is, given your designs on page->mapping? Hugh p.s. Sorry to niggle, but next time, please say [PATCH 1/3] etc. rather than [PATCH] Long Description [1/3], so it's easier to sort the mail subjects by eye in limited columns - thanks. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
On Fri, 21 Sep 2007 13:48:28 +0200 Peter Zijlstra <[EMAIL PROTECTED]> wrote: > > Followings are moved > > * page_mapping() ... returns swapper_space or address_space a page > > is on. > > (from mm.h) > > * page_index() ... returns position of a page in its inode > > (from mm.h) > > * remove_mapping() ... a safe routine to remove page->mapping from > > page. > > (from swap.h) > > I have two other functions that might want integration with this scheme: > > page_file_mapping() ... returns backing address space > page_file_index() ... returns the index therein > > They are identical to page_mapping_cache() and page_index() for > page cache pages, but they also work on swap cache pages. > > That is, for swapcache pages they return: > > page_file_mapping: > page_swap_info(page)->swap_file->f_mapping > > page_file_index: > swp_offset((swp_offset_t)page_private(page)) > > When a filesystem uses these functions instead of page->mapping and > page->index, it allows passing swap cache pages into the regular > filesystem read/write paths. > Oh, > This is useful for things like swap over NFS, where swap is backed by a > swapfile on a 'regular' filesystem. > Okay, I'll try to add them in the next set. Thanks, -Kame - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
On Wed, 19 Sep 2007 16:43:08 +0900 KAMEZAWA Hiroyuki <[EMAIL PROTECTED]> wrote: > A clarification of page <-> fs interface (page cache). > > At first, each FS has to access to struct page->mapping directly. > But it's not just pointer. (we use special 1bit enconding for anon.) > > Although there is historical consensus that page->mapping points to its > inode's > address space, I think adding some neat helper functon is not bad. > > This patch adds page-cache.h which containes page<->address_space<->inode > function which is required (used) by subsystems. > > Following functions are added > > * page_mapping_cache() ... returns address space if a page is page cache > * page_mapping_anon() ... returns anon_vma if a page is anonymous page. > * page_is_pagecache() ... returns true if a page is page-cache. > * page_inode() ... returns inode which a page-cache belongs to. > * is_page_consistent() ... returns true if a page is still valid page cache > > Followings are moved > * page_mapping() ... returns swapper_space or address_space a page is > on. > (from mm.h) > * page_index() ... returns position of a page in its inode > (from mm.h) > * remove_mapping() ... a safe routine to remove page->mapping from page. > (from swap.h) I have two other functions that might want integration with this scheme: page_file_mapping() ... returns backing address space page_file_index() ... returns the index therein They are identical to page_mapping_cache() and page_index() for page cache pages, but they also work on swap cache pages. That is, for swapcache pages they return: page_file_mapping: page_swap_info(page)->swap_file->f_mapping page_file_index: swp_offset((swp_offset_t)page_private(page)) When a filesystem uses these functions instead of page->mapping and page->index, it allows passing swap cache pages into the regular filesystem read/write paths. This is useful for things like swap over NFS, where swap is backed by a swapfile on a 'regular' filesystem. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] page-mapping clarification [1/3] base functions
On Wed, 19 Sep 2007 16:43:08 +0900 KAMEZAWA Hiroyuki [EMAIL PROTECTED] wrote: A clarification of page - fs interface (page cache). At first, each FS has to access to struct page-mapping directly. But it's not just pointer. (we use special 1bit enconding for anon.) Although there is historical consensus that page-mapping points to its inode's address space, I think adding some neat helper functon is not bad. This patch adds page-cache.h which containes page-address_space-inode function which is required (used) by subsystems. Following functions are added * page_mapping_cache() ... returns address space if a page is page cache * page_mapping_anon() ... returns anon_vma if a page is anonymous page. * page_is_pagecache() ... returns true if a page is page-cache. * page_inode() ... returns inode which a page-cache belongs to. * is_page_consistent() ... returns true if a page is still valid page cache Followings are moved * page_mapping() ... returns swapper_space or address_space a page is on. (from mm.h) * page_index() ... returns position of a page in its inode (from mm.h) * remove_mapping() ... a safe routine to remove page-mapping from page. (from swap.h) I have two other functions that might want integration with this scheme: page_file_mapping() ... returns backing address space page_file_index() ... returns the index therein They are identical to page_mapping_cache() and page_index() for page cache pages, but they also work on swap cache pages. That is, for swapcache pages they return: page_file_mapping: page_swap_info(page)-swap_file-f_mapping page_file_index: swp_offset((swp_offset_t)page_private(page)) When a filesystem uses these functions instead of page-mapping and page-index, it allows passing swap cache pages into the regular filesystem read/write paths. This is useful for things like swap over NFS, where swap is backed by a swapfile on a 'regular' filesystem. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] page-mapping clarification [1/3] base functions
On Fri, 21 Sep 2007 13:48:28 +0200 Peter Zijlstra [EMAIL PROTECTED] wrote: Followings are moved * page_mapping() ... returns swapper_space or address_space a page is on. (from mm.h) * page_index() ... returns position of a page in its inode (from mm.h) * remove_mapping() ... a safe routine to remove page-mapping from page. (from swap.h) I have two other functions that might want integration with this scheme: page_file_mapping() ... returns backing address space page_file_index() ... returns the index therein They are identical to page_mapping_cache() and page_index() for page cache pages, but they also work on swap cache pages. That is, for swapcache pages they return: page_file_mapping: page_swap_info(page)-swap_file-f_mapping page_file_index: swp_offset((swp_offset_t)page_private(page)) When a filesystem uses these functions instead of page-mapping and page-index, it allows passing swap cache pages into the regular filesystem read/write paths. Oh, This is useful for things like swap over NFS, where swap is backed by a swapfile on a 'regular' filesystem. Okay, I'll try to add them in the next set. Thanks, -Kame - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] page-mapping clarification [1/3] base functions
On Fri, 21 Sep 2007, KAMEZAWA Hiroyuki wrote: On Thu, 20 Sep 2007 11:26:47 -0700 (PDT) Christoph Lameter [EMAIL PROTECTED] wrote: I am still a bit confused as to what the benefit of this is. Honestly, I have 3 purposes, 2 for readability/clarificaton and 1 for my trial. 1. Clarify page cache - inode relationship before *new concept of page cache*, yours or someone else's is introduced. 2. There are some places using PAGE_MAPPING_ANON directly. I don't want to see following line in .c file. == anon_vma = (struct anon_vma *)(mapping - PAGE_MAPPING_ANON); == 3. I want to *try* page-mapping overriding... store memory resource controller's information in page-mapping. By this, memory controller doesn't enlarge sizeof struct page. (works well in my small test.) Before doing that, I have to hide page-mapping from direct access. My own vote (nothing more) would be for you to set this aside until some future time when there aren't a dozen developers all trampling over each other in this area. They're invasive little changes affecting all filesystems, whereas what we've done so far with page-mapping hasn't affected filesystems at all. Purposes 1 and 2 don't score very high in my book (though I too regret how mm/migrate.c copied that PAGE_MAPPING_ANON stuff from it's rightful home in mm/rmap.c: maybe we should wrap that). There's no end to the wrappers we can add, but they're not always helpful. 3: well, saving memory is good, but I think it could wait until some other time, particularly since the memory controller isn't in yet. Wouldn't it be easier to do something with page-lru than page-mapping? Everybody is interested in page-mapping, not so many in page-lru. (Though perhaps it wouldn't work out so well, since you don't need to get uniquely from mapping to page, whereas you do from lru to page.) If we were to attack page-mapping to save memory from struct page, then we should consider Magnus Damm's idea too: he suggested it could be replaced by a pointer to the radixtree slot (something else needed in the anon case), from which index could be deduced via alignment instead of keeping it in struct page (details to be filled in ...) Of course, my particular prejudice is that I promised months ago to free up the PG_swapcache bit by using a PAGE_MAPPING_SWAP bit instead. That patch got buried while I tried to think up a suitable name for a further page_mapping() variant that turned out to be needed - guess I should look through your collection to see if I can steal one ;) Beyond the unsatisfactory naming, that work has been long done (and like PAGE_MAPPING_ANON, doesn't touch filesystems at all). Or should I now leave PG_swapcache as is, given your designs on page-mapping? Hugh p.s. Sorry to niggle, but next time, please say [PATCH 1/3] etc. rather than [PATCH] Long Description [1/3], so it's easier to sort the mail subjects by eye in limited columns - thanks. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] page-mapping clarification [1/3] base functions
On Fri, 21 Sep 2007 18:02:47 +0100 (BST) Hugh Dickins [EMAIL PROTECTED] wrote: 3. I want to *try* page-mapping overriding... store memory resource controller's information in page-mapping. By this, memory controller doesn't enlarge sizeof struct page. (works well in my small test.) Before doing that, I have to hide page-mapping from direct access. My own vote (nothing more) would be for you to set this aside until some future time when there aren't a dozen developers all trampling over each other in this area. They're invasive little changes affecting all filesystems, whereas what we've done so far with page-mapping hasn't affected filesystems at all. I found that each FS doesn't touch page-mapping so much as I expected. (except for ReiserFS) But ok, I admit changing this will confuse people. 3: well, saving memory is good, but I think it could wait until some other time, particularly since the memory controller isn't in yet. Yes, if extra field in page struct is not hazard to push memory controller, I don't have much motivation. Because extra 8 bytes makes page struct to be 64 bytes(in 64bit), extra 8 bytes is the last space, I think. If we were to attack page-mapping to save memory from struct page, then we should consider Magnus Damm's idea too: he suggested it could be replaced by a pointer to the radixtree slot (something else needed in the anon case), from which index could be deduced via alignment instead of keeping it in struct page (details to be filled in ...) There is a bit difference. My purpose is avoid making struct page larger, not making struct page smaller. Or should I now leave PG_swapcache as is, given your designs on page-mapping? will conflict with my idea ? == http://marc.info/?l=linux-mmm=118956492926821w=2 == Anyway, I'm not in hurry about this patch-set. I'll see what memory controller will go. Other people seems to have an idea to implement pfn - container_info_per_page function. (But this kind of function is not welcomed always.) Thank you for comments. p.s. Sorry to niggle, but next time, please say [PATCH 1/3] etc. rather than [PATCH] Long Description [1/3], so it's easier to sort the mail subjects by eye in limited columns - thanks. sorry, I'll consider well next time. Thanks, -Kame - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
On Thu, 20 Sep 2007 11:26:47 -0700 (PDT)
Christoph Lameter <[EMAIL PROTECTED]> wrote:
> On Wed, 19 Sep 2007, KAMEZAWA Hiroyuki wrote:
>
> > Any comments are welcome.
>
> I am still a bit confused as to what the benefit of this is.
>
Honestly, I have 3 purposes, 2 for readability/clarificaton and 1 for my trial.
1. Clarify page cache <-> inode relationship before *new concept of page cache*,
yours or someone else's is introduced.
2. There are some places using PAGE_MAPPING_ANON directly. I don't want to see
following line in .c file.
==
anon_vma = (struct anon_vma *)(mapping - PAGE_MAPPING_ANON);
==
3. I want to *try* page->mapping overriding... store memory resource
controller's
information in page->mapping. By this, memory controller doesn't enlarge
sizeof
struct page. (works well in my small test.)
Before doing that, I have to hide page->mapping from direct access.
> > +/*
> > + * On an anonymous page mapped into a user virtual memory area,
> > + * page->mapping points to its anon_vma, not to a struct address_space;
> > + * with the PAGE_MAPPING_ANON bit set to distinguish it.
> > + *
> > + * Please note that, confusingly, "page_mapping" refers to the inode
> > + * address_space which maps the page from disk; whereas "page_mapped"
> > + * refers to user virtual address space into which the page is mapped.
> > + */
> > +#define PAGE_MAPPING_ANON 1
> > +
> > +static inline bool PageAnon(struct page *page)
>
> bool??? That is unusual?
This is my first experience of using bool in Linux kernel.. :)
I know bool is not very widely used in Linux now but I tried it because
this function obviously returns yes or no, and C language supports bool as
_Bool now. If messy, I'll avoid using this in this time..
>
> > +static inline struct address_space *page_mapping_cache(struct page *page)
> > +{
> > + if (!page->mapping || PageAnon(page))
> > + return NULL;
> > + return page->mapping;
> > +}
>
> That is confusing.
>
> if (PageAnon(page))
> return NULL;
> return page->mapping;
ok,
> > +static inline struct address_space *page_mapping(struct page *page)
> > +{
> > + struct address_space *mapping = page->mapping;
> > +
> > + VM_BUG_ON(PageSlab(page));
> > + if (unlikely(PageSwapCache(page)))
> > + mapping = _space;
> > +#ifdef CONFIG_SLUB
> > + else if (unlikely(PageSlab(page)))
> > + mapping = NULL;
> > +#endif
>
> The #ifdef does not exist in rc6-mm1. No need to reintroduce it.
>
ok, thanks.
> > +static inline bool
> > +is_page_consistent(struct page *page, struct address_space *mapping)
> > +{
> > + struct address_space *check = page_mapping_cache(page);
> > + return (check == mapping);
> > +}
>
> Why do we need a special function? Why is it safer?
>
For clarify meaning of compareing page_mapping_cache() with mapping.
Does this reduce readability ?
Thank you for comments.
Regards,
-Kame
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] page->mapping clarification [1/3] base functions
On Wed, 19 Sep 2007, KAMEZAWA Hiroyuki wrote:
> Any comments are welcome.
I am still a bit confused as to what the benefit of this is.
> Following functions are added
>
> * page_mapping_cache() ... returns address space if a page is page cache
> * page_mapping_anon() ... returns anon_vma if a page is anonymous page.
> * page_is_pagecache() ... returns true if a page is page-cache.
> * page_inode() ... returns inode which a page-cache belongs to.
> * is_page_consistent() ... returns true if a page is still valid page cache
Ok this could make the code more readable.
> +/*
> + * On an anonymous page mapped into a user virtual memory area,
> + * page->mapping points to its anon_vma, not to a struct address_space;
> + * with the PAGE_MAPPING_ANON bit set to distinguish it.
> + *
> + * Please note that, confusingly, "page_mapping" refers to the inode
> + * address_space which maps the page from disk; whereas "page_mapped"
> + * refers to user virtual address space into which the page is mapped.
> + */
> +#define PAGE_MAPPING_ANON 1
> +
> +static inline bool PageAnon(struct page *page)
bool??? That is unusual?
> +static inline struct address_space *page_mapping_cache(struct page *page)
> +{
> + if (!page->mapping || PageAnon(page))
> + return NULL;
> + return page->mapping;
> +}
That is confusing.
if (PageAnon(page))
return NULL;
return page->mapping;
> +static inline struct address_space *page_mapping(struct page *page)
> +{
> + struct address_space *mapping = page->mapping;
> +
> + VM_BUG_ON(PageSlab(page));
> + if (unlikely(PageSwapCache(page)))
> + mapping = _space;
> +#ifdef CONFIG_SLUB
> + else if (unlikely(PageSlab(page)))
> + mapping = NULL;
> +#endif
The #ifdef does not exist in rc6-mm1. No need to reintroduce it.
> +static inline bool
> +is_page_consistent(struct page *page, struct address_space *mapping)
> +{
> + struct address_space *check = page_mapping_cache(page);
> + return (check == mapping);
> +}
Why do we need a special function? Why is it safer?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] page-mapping clarification [1/3] base functions
On Wed, 19 Sep 2007, KAMEZAWA Hiroyuki wrote:
Any comments are welcome.
I am still a bit confused as to what the benefit of this is.
Following functions are added
* page_mapping_cache() ... returns address space if a page is page cache
* page_mapping_anon() ... returns anon_vma if a page is anonymous page.
* page_is_pagecache() ... returns true if a page is page-cache.
* page_inode() ... returns inode which a page-cache belongs to.
* is_page_consistent() ... returns true if a page is still valid page cache
Ok this could make the code more readable.
+/*
+ * On an anonymous page mapped into a user virtual memory area,
+ * page-mapping points to its anon_vma, not to a struct address_space;
+ * with the PAGE_MAPPING_ANON bit set to distinguish it.
+ *
+ * Please note that, confusingly, page_mapping refers to the inode
+ * address_space which maps the page from disk; whereas page_mapped
+ * refers to user virtual address space into which the page is mapped.
+ */
+#define PAGE_MAPPING_ANON 1
+
+static inline bool PageAnon(struct page *page)
bool??? That is unusual?
+static inline struct address_space *page_mapping_cache(struct page *page)
+{
+ if (!page-mapping || PageAnon(page))
+ return NULL;
+ return page-mapping;
+}
That is confusing.
if (PageAnon(page))
return NULL;
return page-mapping;
+static inline struct address_space *page_mapping(struct page *page)
+{
+ struct address_space *mapping = page-mapping;
+
+ VM_BUG_ON(PageSlab(page));
+ if (unlikely(PageSwapCache(page)))
+ mapping = swapper_space;
+#ifdef CONFIG_SLUB
+ else if (unlikely(PageSlab(page)))
+ mapping = NULL;
+#endif
The #ifdef does not exist in rc6-mm1. No need to reintroduce it.
+static inline bool
+is_page_consistent(struct page *page, struct address_space *mapping)
+{
+ struct address_space *check = page_mapping_cache(page);
+ return (check == mapping);
+}
Why do we need a special function? Why is it safer?
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] page-mapping clarification [1/3] base functions
On Thu, 20 Sep 2007 11:26:47 -0700 (PDT)
Christoph Lameter [EMAIL PROTECTED] wrote:
On Wed, 19 Sep 2007, KAMEZAWA Hiroyuki wrote:
Any comments are welcome.
I am still a bit confused as to what the benefit of this is.
Honestly, I have 3 purposes, 2 for readability/clarificaton and 1 for my trial.
1. Clarify page cache - inode relationship before *new concept of page cache*,
yours or someone else's is introduced.
2. There are some places using PAGE_MAPPING_ANON directly. I don't want to see
following line in .c file.
==
anon_vma = (struct anon_vma *)(mapping - PAGE_MAPPING_ANON);
==
3. I want to *try* page-mapping overriding... store memory resource
controller's
information in page-mapping. By this, memory controller doesn't enlarge
sizeof
struct page. (works well in my small test.)
Before doing that, I have to hide page-mapping from direct access.
+/*
+ * On an anonymous page mapped into a user virtual memory area,
+ * page-mapping points to its anon_vma, not to a struct address_space;
+ * with the PAGE_MAPPING_ANON bit set to distinguish it.
+ *
+ * Please note that, confusingly, page_mapping refers to the inode
+ * address_space which maps the page from disk; whereas page_mapped
+ * refers to user virtual address space into which the page is mapped.
+ */
+#define PAGE_MAPPING_ANON 1
+
+static inline bool PageAnon(struct page *page)
bool??? That is unusual?
This is my first experience of using bool in Linux kernel.. :)
I know bool is not very widely used in Linux now but I tried it because
this function obviously returns yes or no, and C language supports bool as
_Bool now. If messy, I'll avoid using this in this time..
+static inline struct address_space *page_mapping_cache(struct page *page)
+{
+ if (!page-mapping || PageAnon(page))
+ return NULL;
+ return page-mapping;
+}
That is confusing.
if (PageAnon(page))
return NULL;
return page-mapping;
ok,
+static inline struct address_space *page_mapping(struct page *page)
+{
+ struct address_space *mapping = page-mapping;
+
+ VM_BUG_ON(PageSlab(page));
+ if (unlikely(PageSwapCache(page)))
+ mapping = swapper_space;
+#ifdef CONFIG_SLUB
+ else if (unlikely(PageSlab(page)))
+ mapping = NULL;
+#endif
The #ifdef does not exist in rc6-mm1. No need to reintroduce it.
ok, thanks.
+static inline bool
+is_page_consistent(struct page *page, struct address_space *mapping)
+{
+ struct address_space *check = page_mapping_cache(page);
+ return (check == mapping);
+}
Why do we need a special function? Why is it safer?
For clarify meaning of compareing page_mapping_cache() with mapping.
Does this reduce readability ?
Thank you for comments.
Regards,
-Kame
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC][PATCH] page->mapping clarification [1/3] base functions
Rebased to 2.6.23-rc6-mm1 and reflected comments.
Not so aggresive as previous version.
(I'm not in a hurry if -mm is busy.)
Any comments are welcome.
Thanks,
-Kame
==
A clarification of page <-> fs interface (page cache).
At first, each FS has to access to struct page->mapping directly.
But it's not just pointer. (we use special 1bit enconding for anon.)
Although there is historical consensus that page->mapping points to its inode's
address space, I think adding some neat helper functon is not bad.
This patch adds page-cache.h which containes page<->address_space<->inode
function which is required (used) by subsystems.
Following functions are added
* page_mapping_cache() ... returns address space if a page is page cache
* page_mapping_anon() ... returns anon_vma if a page is anonymous page.
* page_is_pagecache() ... returns true if a page is page-cache.
* page_inode() ... returns inode which a page-cache belongs to.
* is_page_consistent() ... returns true if a page is still valid page cache
Followings are moved
* page_mapping() ... returns swapper_space or address_space a page is on.
(from mm.h)
* page_index() ... returns position of a page in its inode
(from mm.h)
* remove_mapping() ... a safe routine to remove page->mapping from page.
(from swap.h)
Changelog V1 -> V2:
- for 2.6.23-rc6-mm1.
- use bool type.
- moved related functions to page-cache.h
- renamed some functions.
Signed-off-by: KAMEZAWA Hiroyuki <[EMAIL PROTECTED]>
---
include/linux/fs.h |6 +-
include/linux/mm.h | 40 ---
include/linux/page-cache.h | 118 +
include/linux/swap.h |1
4 files changed, 123 insertions(+), 42 deletions(-)
Index: linux-2.6.23-rc6-mm1/include/linux/page-cache.h
===
--- /dev/null
+++ linux-2.6.23-rc6-mm1/include/linux/page-cache.h
@@ -0,0 +1,118 @@
+#ifndef _LINUX_PAGE_CACHE_H
+#define _LINUX_PAGE_CACHE_H
+#ifdef __KERNEL__
+
+#include
+#include
+#include
+/*
+ * This file defines interface function among page-cache and FS.
+ *
+ * page_mapping() ... returns swapper_space or address_space a page is
on.
+ * Will be used by routines walks by LRU.
+ * page_mapping_cache() ... returns address space if a page is page cache
+ * Will be used by FS.
+ * page_mapping_anon() ... returns anon_vma if a page is anonymous page.
+ * Will be used by VM subsystem
+ * page_is_pagecache() ... returns true if a page is page-cache.
+ * page_inode() ... returns inode which a page-cache belongs to.
+ * page_index() ... returns position of a page in its inode
+ * is_page_consistent() ... returns true if a page is still valid on specified
+ * address space.
+ * remove_mapping() ... a safe routine to remove page->mapping from page.
+ */
+extern struct address_space swapper_space;
+
+/*
+ * On an anonymous page mapped into a user virtual memory area,
+ * page->mapping points to its anon_vma, not to a struct address_space;
+ * with the PAGE_MAPPING_ANON bit set to distinguish it.
+ *
+ * Please note that, confusingly, "page_mapping" refers to the inode
+ * address_space which maps the page from disk; whereas "page_mapped"
+ * refers to user virtual address space into which the page is mapped.
+ */
+#define PAGE_MAPPING_ANON 1
+
+static inline bool PageAnon(struct page *page)
+{
+ return (((unsigned long)page->mapping & PAGE_MAPPING_ANON) != 0);
+}
+
+static inline struct anon_vma *page_mapping_anon(struct page *page)
+{
+ if (!page->mapping || !PageAnon(page))
+ return NULL;
+ return (struct anon_vma *)
+ ((unsigned long)page->mapping - PAGE_MAPPING_ANON);
+}
+
+static inline struct address_space *page_mapping_cache(struct page *page)
+{
+ if (!page->mapping || PageAnon(page))
+ return NULL;
+ return page->mapping;
+}
+
+/*
+ * Automatically detect 'what the page is' and returns address_space.
+ *
+ * If page is swap cache, returns _space.
+ * If page is page cache, returns inode's address space it belongs to
+ * If page is anon, returns NULL.
+ */
+
+static inline struct address_space *page_mapping(struct page *page)
+{
+ struct address_space *mapping = page->mapping;
+
+ VM_BUG_ON(PageSlab(page));
+ if (unlikely(PageSwapCache(page)))
+ mapping = _space;
+#ifdef CONFIG_SLUB
+ else if (unlikely(PageSlab(page)))
+ mapping = NULL;
+#endif
+else if (unlikely((unsigned long)mapping & PAGE_MAPPING_ANON))
+ mapping = NULL;
+return mapping;
+
+}
+
+static inline bool page_is_pagecache(struct page *page)
+{
+ return (page_mapping_cache(page) != NULL);
+}
+
+static inline
[RFC][PATCH] page-mapping clarification [1/3] base functions
Rebased to 2.6.23-rc6-mm1 and reflected comments.
Not so aggresive as previous version.
(I'm not in a hurry if -mm is busy.)
Any comments are welcome.
Thanks,
-Kame
==
A clarification of page - fs interface (page cache).
At first, each FS has to access to struct page-mapping directly.
But it's not just pointer. (we use special 1bit enconding for anon.)
Although there is historical consensus that page-mapping points to its inode's
address space, I think adding some neat helper functon is not bad.
This patch adds page-cache.h which containes page-address_space-inode
function which is required (used) by subsystems.
Following functions are added
* page_mapping_cache() ... returns address space if a page is page cache
* page_mapping_anon() ... returns anon_vma if a page is anonymous page.
* page_is_pagecache() ... returns true if a page is page-cache.
* page_inode() ... returns inode which a page-cache belongs to.
* is_page_consistent() ... returns true if a page is still valid page cache
Followings are moved
* page_mapping() ... returns swapper_space or address_space a page is on.
(from mm.h)
* page_index() ... returns position of a page in its inode
(from mm.h)
* remove_mapping() ... a safe routine to remove page-mapping from page.
(from swap.h)
Changelog V1 - V2:
- for 2.6.23-rc6-mm1.
- use bool type.
- moved related functions to page-cache.h
- renamed some functions.
Signed-off-by: KAMEZAWA Hiroyuki [EMAIL PROTECTED]
---
include/linux/fs.h |6 +-
include/linux/mm.h | 40 ---
include/linux/page-cache.h | 118 +
include/linux/swap.h |1
4 files changed, 123 insertions(+), 42 deletions(-)
Index: linux-2.6.23-rc6-mm1/include/linux/page-cache.h
===
--- /dev/null
+++ linux-2.6.23-rc6-mm1/include/linux/page-cache.h
@@ -0,0 +1,118 @@
+#ifndef _LINUX_PAGE_CACHE_H
+#define _LINUX_PAGE_CACHE_H
+#ifdef __KERNEL__
+
+#include linux/mm.h
+#include linux/fs.h
+#include linux/rmap.h
+/*
+ * This file defines interface function among page-cache and FS.
+ *
+ * page_mapping() ... returns swapper_space or address_space a page is
on.
+ * Will be used by routines walks by LRU.
+ * page_mapping_cache() ... returns address space if a page is page cache
+ * Will be used by FS.
+ * page_mapping_anon() ... returns anon_vma if a page is anonymous page.
+ * Will be used by VM subsystem
+ * page_is_pagecache() ... returns true if a page is page-cache.
+ * page_inode() ... returns inode which a page-cache belongs to.
+ * page_index() ... returns position of a page in its inode
+ * is_page_consistent() ... returns true if a page is still valid on specified
+ * address space.
+ * remove_mapping() ... a safe routine to remove page-mapping from page.
+ */
+extern struct address_space swapper_space;
+
+/*
+ * On an anonymous page mapped into a user virtual memory area,
+ * page-mapping points to its anon_vma, not to a struct address_space;
+ * with the PAGE_MAPPING_ANON bit set to distinguish it.
+ *
+ * Please note that, confusingly, page_mapping refers to the inode
+ * address_space which maps the page from disk; whereas page_mapped
+ * refers to user virtual address space into which the page is mapped.
+ */
+#define PAGE_MAPPING_ANON 1
+
+static inline bool PageAnon(struct page *page)
+{
+ return (((unsigned long)page-mapping PAGE_MAPPING_ANON) != 0);
+}
+
+static inline struct anon_vma *page_mapping_anon(struct page *page)
+{
+ if (!page-mapping || !PageAnon(page))
+ return NULL;
+ return (struct anon_vma *)
+ ((unsigned long)page-mapping - PAGE_MAPPING_ANON);
+}
+
+static inline struct address_space *page_mapping_cache(struct page *page)
+{
+ if (!page-mapping || PageAnon(page))
+ return NULL;
+ return page-mapping;
+}
+
+/*
+ * Automatically detect 'what the page is' and returns address_space.
+ *
+ * If page is swap cache, returns swapper_space.
+ * If page is page cache, returns inode's address space it belongs to
+ * If page is anon, returns NULL.
+ */
+
+static inline struct address_space *page_mapping(struct page *page)
+{
+ struct address_space *mapping = page-mapping;
+
+ VM_BUG_ON(PageSlab(page));
+ if (unlikely(PageSwapCache(page)))
+ mapping = swapper_space;
+#ifdef CONFIG_SLUB
+ else if (unlikely(PageSlab(page)))
+ mapping = NULL;
+#endif
+else if (unlikely((unsigned long)mapping PAGE_MAPPING_ANON))
+ mapping = NULL;
+return mapping;
+
+}
+
+static inline bool page_is_pagecache(struct page *page)
+{
+ return (page_mapping_cache(page) != NULL);
