| Message ID | 20201007010603.3452458-1-jglisse@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Small step toward KSM for file back page. | expand |
On Tue, Oct 06, 2020 at 09:05:49PM -0400, jglisse@redhat.com wrote: > The present patchset just add mapping argument to the various vfs call- > backs. It does not make use of that new parameter to avoid regression. > I am posting this whole things as small contain patchset as it is rather > big and i would like to make progress step by step. Well, that's the problem. This patch set is gigantic and unreviewable. And it has no benefits. The idea you present here was discussed at LSFMM in Utah and I recall absolutely nobody being in favour of it. You claim many wonderful features will be unlocked by this, but I think they can all be achieved without doing any of this very disruptive work. > 118 files changed, 722 insertions(+), 385 deletions(-) mmm.
On Wed, Oct 07, 2020 at 04:20:13AM +0100, Matthew Wilcox wrote: > On Tue, Oct 06, 2020 at 09:05:49PM -0400, jglisse@redhat.com wrote: > > The present patchset just add mapping argument to the various vfs call- > > backs. It does not make use of that new parameter to avoid regression. > > I am posting this whole things as small contain patchset as it is rather > > big and i would like to make progress step by step. > > Well, that's the problem. This patch set is gigantic and unreviewable. > And it has no benefits. The idea you present here was discussed at > LSFMM in Utah and I recall absolutely nobody being in favour of it. > You claim many wonderful features will be unlocked by this, but I think > they can all be achieved without doing any of this very disruptive work. You have any ideas on how to achieve them without such change ? I will be more than happy for a simpler solution but i fail to see how you can work around the need for a pointer inside struct page. Given struct page can not grow it means you need to be able to overload one of the existing field, at least i do not see any otherway. Cheers, Jérôme
On Wed, Oct 07, 2020 at 10:48:35AM -0400, Jerome Glisse wrote: > On Wed, Oct 07, 2020 at 04:20:13AM +0100, Matthew Wilcox wrote: > > On Tue, Oct 06, 2020 at 09:05:49PM -0400, jglisse@redhat.com wrote: > > > The present patchset just add mapping argument to the various vfs call- > > > backs. It does not make use of that new parameter to avoid regression. > > > I am posting this whole things as small contain patchset as it is rather > > > big and i would like to make progress step by step. > > > > Well, that's the problem. This patch set is gigantic and unreviewable. > > And it has no benefits. The idea you present here was discussed at > > LSFMM in Utah and I recall absolutely nobody being in favour of it. > > You claim many wonderful features will be unlocked by this, but I think > > they can all be achieved without doing any of this very disruptive work. > > You have any ideas on how to achieve them without such change ? I will > be more than happy for a simpler solution but i fail to see how you can > work around the need for a pointer inside struct page. Given struct > page can not grow it means you need to be able to overload one of the > existing field, at least i do not see any otherway. The one I've spent the most time thinking about is sharing pages between reflinked files. My approach is to pull DAX entries into the main page cache and have them reference the PFN directly. It's not a struct page, but we can find a struct page from it if we need it. The struct page would belong to a mapping that isn't part of the file. For other things (NUMA distribution), we can point to something which isn't a struct page and can be distiguished from a real struct page by a bit somewhere (I have ideas for at least three bits in struct page that could be used for this). Then use a pointer in that data structure to point to the real page. Or do NUMA distribution at the inode level. Have a way to get from (inode, node) to an address_space which contains just regular pages. Using main memory to cache DAX could be done today without any data structure changes. It just needs the DAX entries pulled up into the main pagecache. See earlier item. Exclusive write access ... you could put a magic value in the pagecache for pages which are exclusively for someone else's use and handle those specially. I don't entirely understand this use case. I don't have time to work on all of these. If there's one that particularly interests you, let's dive deep into it and figure out how you can do it without committing this kind of violence to struct page.
On Wed, Oct 07, 2020 at 06:05:58PM +0100, Matthew Wilcox wrote: > On Wed, Oct 07, 2020 at 10:48:35AM -0400, Jerome Glisse wrote: > > On Wed, Oct 07, 2020 at 04:20:13AM +0100, Matthew Wilcox wrote: > > > On Tue, Oct 06, 2020 at 09:05:49PM -0400, jglisse@redhat.com wrote: > > > > The present patchset just add mapping argument to the various vfs call- > > > > backs. It does not make use of that new parameter to avoid regression. > > > > I am posting this whole things as small contain patchset as it is rather > > > > big and i would like to make progress step by step. > > > > > > Well, that's the problem. This patch set is gigantic and unreviewable. > > > And it has no benefits. The idea you present here was discussed at > > > LSFMM in Utah and I recall absolutely nobody being in favour of it. > > > You claim many wonderful features will be unlocked by this, but I think > > > they can all be achieved without doing any of this very disruptive work. > > > > You have any ideas on how to achieve them without such change ? I will > > be more than happy for a simpler solution but i fail to see how you can > > work around the need for a pointer inside struct page. Given struct > > page can not grow it means you need to be able to overload one of the > > existing field, at least i do not see any otherway. > > The one I've spent the most time thinking about is sharing pages between > reflinked files. My approach is to pull DAX entries into the main page > cache and have them reference the PFN directly. It's not a struct page, > but we can find a struct page from it if we need it. The struct page > would belong to a mapping that isn't part of the file. You would need to do a lot of filesystem specific change to make sure the fs understand the special mapping. It is doable but i feel it would have a lot of fs specific part. > For other things (NUMA distribution), we can point to something which > isn't a struct page and can be distiguished from a real struct page by a > bit somewhere (I have ideas for at least three bits in struct page that > could be used for this). Then use a pointer in that data structure to > point to the real page. Or do NUMA distribution at the inode level. > Have a way to get from (inode, node) to an address_space which contains > just regular pages. How do you find all the copies ? KSM maintains a list for a reasons. Same would be needed here because if you want to break the write prot you need to find all the copy first. If you intend to walk page table then how do you synchronize to avoid more copy to spawn while you walk reverse mapping, we could lock the struct page i guess. Also how do you walk device page table which are completely hidden from core mm. > Using main memory to cache DAX could be done today without any data > structure changes. It just needs the DAX entries pulled up into the > main pagecache. See earlier item. > > Exclusive write access ... you could put a magic value in the pagecache > for pages which are exclusively for someone else's use and handle those > specially. I don't entirely understand this use case. For this use case you need a callback to break the protection and it needs to handle all cases ie not only write by CPU through file mapping but also file write syscall and other syscall that can write to page (pipe, ...). > I don't have time to work on all of these. If there's one that > particularly interests you, let's dive deep into it and figure out how I care about KSM, duplicate NUMA copy (not only for CPU but also device) and write protection or exclusive write access. In each case you need a list of all the copy (for KSM of the deduplicated page) Having a special entry in the page cache does not sound like a good option in many code path you would need to re-look the page cache to find out if the page is in special state. If you use a bit flag in struct page how do you get to the callback or to the copy/alias, walk all the page tables ? > you can do it without committing this kind of violence to struct page. I do not see how i am doing violence to struct page :) The basis of my approach is to pass down the mapping. We always have the mapping at the top of the stack (either syscall entry point on a file or through the vma when working on virtual address). But we rarely pass down this mapping down the stack into the fs code. I am only passing down the mapping through the bottom of the stack so we do not need to rely of page->mapping all the time. I am not trying to remove the page->mapping field, it is still usefull, i just want to be able to overload it so that we can make KSM code generic and allow to reuse that generic part for other usecase. Cheers, Jérôme
On Wed, Oct 07, 2020 at 01:54:19PM -0400, Jerome Glisse wrote: > On Wed, Oct 07, 2020 at 06:05:58PM +0100, Matthew Wilcox wrote: > > On Wed, Oct 07, 2020 at 10:48:35AM -0400, Jerome Glisse wrote: > > > On Wed, Oct 07, 2020 at 04:20:13AM +0100, Matthew Wilcox wrote: > > > > On Tue, Oct 06, 2020 at 09:05:49PM -0400, jglisse@redhat.com wrote: > > > > > The present patchset just add mapping argument to the various vfs call- > > > > > backs. It does not make use of that new parameter to avoid regression. > > > > > I am posting this whole things as small contain patchset as it is rather > > > > > big and i would like to make progress step by step. > > > > > > > > Well, that's the problem. This patch set is gigantic and unreviewable. > > > > And it has no benefits. The idea you present here was discussed at > > > > LSFMM in Utah and I recall absolutely nobody being in favour of it. > > > > You claim many wonderful features will be unlocked by this, but I think > > > > they can all be achieved without doing any of this very disruptive work. > > > > > > You have any ideas on how to achieve them without such change ? I will > > > be more than happy for a simpler solution but i fail to see how you can > > > work around the need for a pointer inside struct page. Given struct > > > page can not grow it means you need to be able to overload one of the > > > existing field, at least i do not see any otherway. > > > > The one I've spent the most time thinking about is sharing pages between > > reflinked files. My approach is to pull DAX entries into the main page > > cache and have them reference the PFN directly. It's not a struct page, > > but we can find a struct page from it if we need it. The struct page > > would belong to a mapping that isn't part of the file. > > You would need to do a lot of filesystem specific change to make sure > the fs understand the special mapping. It is doable but i feel it would > have a lot of fs specific part. I can't see any way to make it work without filesystem cooperation. > > For other things (NUMA distribution), we can point to something which > > isn't a struct page and can be distiguished from a real struct page by a > > bit somewhere (I have ideas for at least three bits in struct page that > > could be used for this). Then use a pointer in that data structure to > > point to the real page. Or do NUMA distribution at the inode level. > > Have a way to get from (inode, node) to an address_space which contains > > just regular pages. > > How do you find all the copies ? KSM maintains a list for a reasons. > Same would be needed here because if you want to break the write prot > you need to find all the copy first. If you intend to walk page table > then how do you synchronize to avoid more copy to spawn while you > walk reverse mapping, we could lock the struct page i guess. Also how > do you walk device page table which are completely hidden from core mm. You have the inode and you iterate over each mapping, looking up the page that's in each mapping. Or you use the i_mmap tree to find the pages. > > Using main memory to cache DAX could be done today without any data > > structure changes. It just needs the DAX entries pulled up into the > > main pagecache. See earlier item. > > > > Exclusive write access ... you could put a magic value in the pagecache > > for pages which are exclusively for someone else's use and handle those > > specially. I don't entirely understand this use case. > > For this use case you need a callback to break the protection and it > needs to handle all cases ie not only write by CPU through file mapping > but also file write syscall and other syscall that can write to page > (pipe, ...). If the page can't be found in the page cache, then by definition you won't be able to write to them. > > I don't have time to work on all of these. If there's one that > > particularly interests you, let's dive deep into it and figure out how > > I care about KSM, duplicate NUMA copy (not only for CPU but also > device) and write protection or exclusive write access. In each case > you need a list of all the copy (for KSM of the deduplicated page) > Having a special entry in the page cache does not sound like a good > option in many code path you would need to re-look the page cache to > find out if the page is in special state. If you use a bit flag in > struct page how do you get to the callback or to the copy/alias, > walk all the page tables ? Like I said, something that _looks_ like a struct page. At least looks enough like a struct page that you can pull a pointer out of the page cache and check the bit. But since it's not actually a struct page, you can use the rest of the data structure for pointers to things you want to track. Like the real struct page. > I do not see how i am doing violence to struct page :) The basis of > my approach is to pass down the mapping. We always have the mapping > at the top of the stack (either syscall entry point on a file or > through the vma when working on virtual address). Yes, you explained all that in Utah. I wasn't impressed than, and I'm not impressed now.
On Wed, Oct 07, 2020 at 07:33:16PM +0100, Matthew Wilcox wrote: > On Wed, Oct 07, 2020 at 01:54:19PM -0400, Jerome Glisse wrote: > > On Wed, Oct 07, 2020 at 06:05:58PM +0100, Matthew Wilcox wrote: > > > On Wed, Oct 07, 2020 at 10:48:35AM -0400, Jerome Glisse wrote: > > > > On Wed, Oct 07, 2020 at 04:20:13AM +0100, Matthew Wilcox wrote: > > > > > On Tue, Oct 06, 2020 at 09:05:49PM -0400, jglisse@redhat.com wrote: > > > For other things (NUMA distribution), we can point to something which [...] > > > isn't a struct page and can be distiguished from a real struct page by a > > > bit somewhere (I have ideas for at least three bits in struct page that > > > could be used for this). Then use a pointer in that data structure to > > > point to the real page. Or do NUMA distribution at the inode level. > > > Have a way to get from (inode, node) to an address_space which contains > > > just regular pages. > > > > How do you find all the copies ? KSM maintains a list for a reasons. > > Same would be needed here because if you want to break the write prot > > you need to find all the copy first. If you intend to walk page table > > then how do you synchronize to avoid more copy to spawn while you > > walk reverse mapping, we could lock the struct page i guess. Also how > > do you walk device page table which are completely hidden from core mm. > > You have the inode and you iterate over each mapping, looking up the page > that's in each mapping. Or you use the i_mmap tree to find the pages. This would slow down for everyone as we would have to walk all mapping each time we try to write to page. Also we a have mechanism for page write back to avoid race between thread trying to write and write back. We would also need something similar. Without mediating this through struct page i do not see how to keep this reasonable from performance point of view. > > > I don't have time to work on all of these. If there's one that > > > particularly interests you, let's dive deep into it and figure out how > > > > I care about KSM, duplicate NUMA copy (not only for CPU but also > > device) and write protection or exclusive write access. In each case > > you need a list of all the copy (for KSM of the deduplicated page) > > Having a special entry in the page cache does not sound like a good > > option in many code path you would need to re-look the page cache to > > find out if the page is in special state. If you use a bit flag in > > struct page how do you get to the callback or to the copy/alias, > > walk all the page tables ? > > Like I said, something that _looks_ like a struct page. At least looks > enough like a struct page that you can pull a pointer out of the page > cache and check the bit. But since it's not actually a struct page, > you can use the rest of the data structure for pointers to things you > want to track. Like the real struct page. What i fear is the added cost because it means we need to do this look- up everytime to check and we also need proper locking to avoid races. Adding an ancilliary struct and trying to keep everything synchronize seems harder to me. > > > I do not see how i am doing violence to struct page :) The basis of > > my approach is to pass down the mapping. We always have the mapping > > at the top of the stack (either syscall entry point on a file or > > through the vma when working on virtual address). > > Yes, you explained all that in Utah. I wasn't impressed than, and I'm > not impressed now. Is this more of a taste thing or is there something specific you do not like ? Cheers, Jérôme
On Wed, Oct 07, 2020 at 01:54:19PM -0400, Jerome Glisse wrote: > > For other things (NUMA distribution), we can point to something which > > isn't a struct page and can be distiguished from a real struct page by a > > bit somewhere (I have ideas for at least three bits in struct page that > > could be used for this). Then use a pointer in that data structure to > > point to the real page. Or do NUMA distribution at the inode level. > > Have a way to get from (inode, node) to an address_space which contains > > just regular pages. > > How do you find all the copies ? KSM maintains a list for a reasons. > Same would be needed here because if you want to break the write prot > you need to find all the copy first. If you intend to walk page table > then how do you synchronize to avoid more copy to spawn while you > walk reverse mapping, we could lock the struct page i guess. Also how > do you walk device page table which are completely hidden from core mm. So ... why don't you put a PageKsm page in the page cache? That way you can share code with the current KSM implementation. You'd need something like this: +++ b/mm/filemap.c @@ -1622,6 +1622,9 @@ struct page *find_lock_entry(struct address_space *mapping , pgoff_t index) lock_page(page); /* Has the page been truncated? */ if (unlikely(page->mapping != mapping)) { + if (PageKsm(page)) { + ... + } unlock_page(page); put_page(page); goto repeat; @@ -1655,6 +1658,7 @@ struct page *find_lock_entry(struct address_space *mapping, pgoff_t index) * * %FGP_WRITE - The page will be written * * %FGP_NOFS - __GFP_FS will get cleared in gfp mask * * %FGP_NOWAIT - Don't get blocked by page lock + * * %FGP_KSM - Return KSM pages * * If %FGP_LOCK or %FGP_CREAT are specified then the function may sleep even * if the %GFP flags specified for %FGP_CREAT are atomic. @@ -1687,6 +1691,11 @@ struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index, /* Has the page been truncated? */ if (unlikely(page->mapping != mapping)) { + if (PageKsm(page) { + if (fgp_flags & FGP_KSM) + return page; + ... + } unlock_page(page); put_page(page); goto repeat; I don't know what you want to do when you find a KSM page, so I just left an ellipsis.
On Wed, Oct 07, 2020 at 11:09:16PM +0100, Matthew Wilcox wrote: > On Wed, Oct 07, 2020 at 01:54:19PM -0400, Jerome Glisse wrote: > > > For other things (NUMA distribution), we can point to something which > > > isn't a struct page and can be distiguished from a real struct page by a > > > bit somewhere (I have ideas for at least three bits in struct page that > > > could be used for this). Then use a pointer in that data structure to > > > point to the real page. Or do NUMA distribution at the inode level. > > > Have a way to get from (inode, node) to an address_space which contains > > > just regular pages. > > > > How do you find all the copies ? KSM maintains a list for a reasons. > > Same would be needed here because if you want to break the write prot > > you need to find all the copy first. If you intend to walk page table > > then how do you synchronize to avoid more copy to spawn while you > > walk reverse mapping, we could lock the struct page i guess. Also how > > do you walk device page table which are completely hidden from core mm. > > So ... why don't you put a PageKsm page in the page cache? That way you > can share code with the current KSM implementation. You'd need > something like this: I do just that but there is no need to change anything in page cache. So below code is not necessary. What you need is a way to find all the copies so if you have a write fault (or any write access) then from that fault you get the mapping and offset and you use that to lookup the fs specific informations and de-duplicate the page with new page and the fs specific informations. Hence the filesystem code do not need to know anything it all happens in generic common code. So flow is: Same as before: 1 - write fault (address, vma) 2 - regular write fault handler -> find page in page cache New to common page fault code: 3 - ksm check in write fault common code (same as ksm today for anonymous page fault code path). 4 - break ksm (address, vma) -> (file offset, mapping) 4.a - use mapping and file offset to lookup the proper fs specific information that were save when the page was made ksm. 4.b - allocate new page and initialize it with that information (and page content), update page cache and mappings ie all the pte who where pointing to the ksm for that mapping at that offset to now use the new page (like KSM for anonymous page today). Resume regular code path: mkwrite /|| set pte ... Roughly the same for write ioctl (other cases goes through GUP which itself goes through page fault code path). There is no need to change page cache in anyway. Just common code path that enable write to file back page. The fs specific information is page->private, some of the flags (page->flags) and page->indexi (file offset). Everytime a page is deduplicated a copy of that information is save in an alias struct which you can get to from the the share KSM page (page-> mapping is a pointer to ksm root struct which has a pointer to list of all aliases). > > +++ b/mm/filemap.c > @@ -1622,6 +1622,9 @@ struct page *find_lock_entry(struct address_space *mapping > , pgoff_t index) > lock_page(page); > /* Has the page been truncated? */ > if (unlikely(page->mapping != mapping)) { > + if (PageKsm(page)) { > + ... > + } > unlock_page(page); > put_page(page); > goto repeat; > @@ -1655,6 +1658,7 @@ struct page *find_lock_entry(struct address_space *mapping, pgoff_t index) > * * %FGP_WRITE - The page will be written > * * %FGP_NOFS - __GFP_FS will get cleared in gfp mask > * * %FGP_NOWAIT - Don't get blocked by page lock > + * * %FGP_KSM - Return KSM pages > * > * If %FGP_LOCK or %FGP_CREAT are specified then the function may sleep even > * if the %GFP flags specified for %FGP_CREAT are atomic. > @@ -1687,6 +1691,11 @@ struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index, > > /* Has the page been truncated? */ > if (unlikely(page->mapping != mapping)) { > + if (PageKsm(page) { > + if (fgp_flags & FGP_KSM) > + return page; > + ... > + } > unlock_page(page); > put_page(page); > goto repeat; > > I don't know what you want to do when you find a KSM page, so I just left > an ellipsis. >
On Thu, Oct 08, 2020 at 11:30:28AM -0400, Jerome Glisse wrote: > On Wed, Oct 07, 2020 at 11:09:16PM +0100, Matthew Wilcox wrote: > > So ... why don't you put a PageKsm page in the page cache? That way you > > can share code with the current KSM implementation. You'd need > > something like this: > > I do just that but there is no need to change anything in page cache. That's clearly untrue. If you just put a PageKsm page in the page cache today, here's what will happen on a truncate: void truncate_inode_pages_range(struct address_space *mapping, loff_t lstart, loff_t lend) { ... struct page *page = find_lock_page(mapping, start - 1); find_lock_page() does this: return pagecache_get_page(mapping, offset, FGP_LOCK, 0); pagecache_get_page(): repeat: page = find_get_entry(mapping, index); ... if (fgp_flags & FGP_LOCK) { ... if (unlikely(compound_head(page)->mapping != mapping)) { unlock_page(page); put_page(page); goto repeat; so it's just going to spin. There are plenty of other codepaths that would need to be checked. If you haven't found them, that shows you don't understand the problem deeply enough yet. I believe we should solve this problem, but I don't think you're going about it the right way. > So flow is: > > Same as before: > 1 - write fault (address, vma) > 2 - regular write fault handler -> find page in page cache > > New to common page fault code: > 3 - ksm check in write fault common code (same as ksm today > for anonymous page fault code path). > 4 - break ksm (address, vma) -> (file offset, mapping) > 4.a - use mapping and file offset to lookup the proper > fs specific information that were save when the > page was made ksm. > 4.b - allocate new page and initialize it with that > information (and page content), update page cache > and mappings ie all the pte who where pointing to > the ksm for that mapping at that offset to now use > the new page (like KSM for anonymous page today). But by putting that logic in the page fault path, you've missed the truncate path. And maybe other places. Putting the logic down in pagecache_get_page() means you _don't_ need to find all the places that call pagecache_get_page().
On Thu, Oct 08, 2020 at 04:43:41PM +0100, Matthew Wilcox wrote: > On Thu, Oct 08, 2020 at 11:30:28AM -0400, Jerome Glisse wrote: > > On Wed, Oct 07, 2020 at 11:09:16PM +0100, Matthew Wilcox wrote: > > > So ... why don't you put a PageKsm page in the page cache? That way you > > > can share code with the current KSM implementation. You'd need > > > something like this: > > > > I do just that but there is no need to change anything in page cache. > > That's clearly untrue. If you just put a PageKsm page in the page > cache today, here's what will happen on a truncate: > > void truncate_inode_pages_range(struct address_space *mapping, > loff_t lstart, loff_t lend) > { > ... > struct page *page = find_lock_page(mapping, start - 1); > > find_lock_page() does this: > return pagecache_get_page(mapping, offset, FGP_LOCK, 0); > > pagecache_get_page(): > > repeat: > page = find_get_entry(mapping, index); > ... > if (fgp_flags & FGP_LOCK) { > ... > if (unlikely(compound_head(page)->mapping != mapping)) { > unlock_page(page); > put_page(page); > goto repeat; > > so it's just going to spin. There are plenty of other codepaths that > would need to be checked. If you haven't found them, that shows you > don't understand the problem deeply enough yet. I also change truncate, splice and few other special cases that do not goes through GUP/page fault/mkwrite (memory debug too but that's a different beast). > I believe we should solve this problem, but I don't think you're going > about it the right way. I have done much more than what i posted but there is bug that i need to hammer down before posting everything and i wanted to get the discussion started. I guess i will finish tracking that one down and post the whole thing. > > So flow is: > > > > Same as before: > > 1 - write fault (address, vma) > > 2 - regular write fault handler -> find page in page cache > > > > New to common page fault code: > > 3 - ksm check in write fault common code (same as ksm today > > for anonymous page fault code path). > > 4 - break ksm (address, vma) -> (file offset, mapping) > > 4.a - use mapping and file offset to lookup the proper > > fs specific information that were save when the > > page was made ksm. > > 4.b - allocate new page and initialize it with that > > information (and page content), update page cache > > and mappings ie all the pte who where pointing to > > the ksm for that mapping at that offset to now use > > the new page (like KSM for anonymous page today). > > But by putting that logic in the page fault path, you've missed > the truncate path. And maybe other places. Putting the logic > down in pagecache_get_page() means you _don't_ need to find > all the places that call pagecache_get_page(). They are cases where pagecache is not even in the loop ie you already have the page and you do not need to look it up (page fault, some fs common code, anything that goes through GUP, memory reclaim, ...). Making all those places having to go through page cache all the times will slow them down and many are hot code path that i do not believe we want to slow even if a feature is not use. Cheers, Jérôme
From: Jérôme Glisse <jglisse@redhat.com> This patchset is a step toward a larger objective: generalize existing KSM into a mechanism allowing exclusive write control for a page; either anonymous memory (like KSM today) or file back page (modulo GUP which would block that like it does today for KSM). Exclusive write control page allow multiple different features to be implemented: - KSM kernel share page, ie de-duplicate pages with same content to use a single page for all. From many pages to one read only page. We have that today for anonymous memory only. The overall patchset extends it to file back page ie sharing the same struct page accross different file or accross same file. This can be be usefull for containers for instance ... or for deduplication in same file. - NUMA duplication, duplicate a page into multiple local read only copy. This is the opposite of KSM in a sense, instead of saving memory. Using more memory to get better memory access performance. For instance duplicating libc code to local node copy; or big read only dataset duplicated on each nodes. - Exclusive write access, owner of page write protection is the only that can write to the page (and must still abide by fs rules for fileback page in respect to writeback...). One use case is for fast atomic operation using non atomic instruction. For instance by PCIE device, if all mapping of the page is read only then PCIE device driver knows device write can not race with CPU write. This is a performance optimization. - Use main memory as cache for persistent memory ie the page is read only and write will trigger callback and different strategy can be use like write combining (ie acumulating change in main memory before copying to persistent memory). Like KSM today such protection can be broken at _any_ time. The owner of the protection gets a callback (KSM code for instance get calls) so that it can unprotect the page. Breaking protection should not block and must happens quickly (like KSM code today). Convertion of existing KSM into generic mechanism is straightforward for anonymous page (just factorize out KSM code that deals with page protection from KSM code that deals with de-duplication). The big changes here is the support for file back pages. The idea to achieve it is that we almost always have the mapping a page belongs to within the call stack as we operate on such page either from: - Syscall/kernel against a file (file -> inode -> mapping). - Syscall/kernel against virtual address (vma -> file -> mapping). - Write back for a given mapping (mapping -> pages). They are few exceptions: - Reclaim, but reclaim does not care about mapping. Reclaim wants to unmap page to free it up. So all we have to do is provide special path to do that just like KSM does today for anonymous pages. - Compaction, again we do not care about the mapping for compaction. All we need is way to move page (ie migrate). - Flush data cache on some architecture the cache line are tag with the virtual address so when flushing a page we need to find all of its virtual addresses. Again we do not care about the mapping, we just need a way to find all virtual address in all process pointing to the page. - GUP user that want to set a page dirty. This is easy, we just do not allow protection to work on GUPed page and GUP also will break the protection. There is just no way to synchronize with GUP user as they violate all mm and fs rules anyway. - Some proc fs and other memory debugging API. Here we do not care about the mapping but about the page states. Also some of those API works on virtual address for which we can easily get the vma and thus the mapping. So when we have the mapping for a page from the context and not from page->mapping then we can use it as a key to lookup private and index fields value for the page. To avoid any regression risk, only protected pages sees their fields overloaded. It means that if you are not using the page protection then the page->mapping, page->private and page->index all stays as they are today. Also page->mapping is always use as canonical place to lookup the page mapping for unprotected page so that any existing code will keep working as it does today even if the mapping we get from the context does not match the page->mapping. More on this below. Overview: ========= The core idea is pretty dumb, it is just about passing new mapping argument to every function that get a page and need the mapping corresponding to that page. Most of the changes are done through semantic patches. Adding new function argument on itself does not bring any risk. The risk is in making sure that the mapping we pass as function argument is the one corresponding to the page. To avoid any regression we keep using page->mapping as the canonical mapping even if it does not match what we get as a new argument ie: Today: foo(struct page *page) { ... local variable declaration ... struct address_space *mapping = page->mapping; ... use mapping or inode from mapping ... } After: foo(struct page *page, struct address_space *mapping) { ... local variable declaration ... mapping = fs_page_mapping(page, mapping); ... use mapping or inode from mapping ... } With: struct address_space *fs_page_mapping(struct page *page, struct address_space *mapping) { if (!PageProtected(page)) { WARN_ON(page->mapping != mapping); return page->mapping; } return mapping; } So as long as you are not using page protection the end result before and after the patchset is the same ie the same mapping value will be use. Strategy for passing down mapping of page: ========================================== To make it easier to review that correct mapping is pass down, changes are split in 3 steps: - First adds new __mapping argument to all functions that will need it and do not have it already (or inode as we can get mapping from inode). The argument is never use and thus in this step call site pass MAPPING_NULL which is just macro that alias to NULL but is easier to grep for. - Second replace MAPPING_NULL with the __mapping argument whereever possible. The rational is that if a function is passing down the mapping and already have it as a function argument then the real difficulty is not in that function but in caller of that function where we must ascertain that the mapping we pass down is the correct one. Replace any local MAPPING_NULL with local mapping variable or inode (some convertion are done manualy when they are multiple possible choice or when the automatic convertion would be wrong). At the end of this step there should not be any MAPPING_NULL left anywhere. - Finaly we replace any local mapping variable with __mapping and we add a check ie going from: foo(struct address_mapping *__mapping, struct page *page, ...){ struct address_space *mapping = page->mapping; ... } To: foo(struct address_mapping *mapping, struct page *page, ...) { ... mapping = fs_page_mapping(mapping, page); } With: fs_page_mapping(struct address_mapping *mapping, struct page *page) { if (!PageProtected(page)) return page->mapping; return mapping; } Do the same for local inode that are looked up from page->mapping. So as long as you do not use the protection mechanism you will use the same mapping/inode as today. I hope that over enough time we can build confidence so that people start using page protection and KSM for file back pages without any fear of regression. But in the meantime as explained in above section, code will keep working as it does today if page is not protected. ---------------------------------------------------------------------- The present patchset just add mapping argument to the various vfs call- backs. It does not make use of that new parameter to avoid regression. I am posting this whole things as small contain patchset as it is rather big and i would like to make progress step by step. ---------------------------------------------------------------------- FAQ: ---------------------------------------------------------------------- Why multiple pass in the coccinelle patches ? Two reasons for this: - To modify callback you first need to identify callback. It is common enough to have callback define in one file and set as callback in another. For that to work you would have one pass to identify all function use as a callback. Then anoter pass executed for each of the callback identified in the first pass to update each of them (like adding a new arugment). - Run time, some of the coccinelle patch are complex enough that they have a long runtime so to avoid spending hours on semantic patching it is better to run a first pass (with very simple semantic patch) that identify all the files that will need to be updated and then a second pass on each of the file with the actual update. Why wrapping page->flags/index/private/mapping ? A protected page can (is not necessary the case) alias to multiple original page (also see "page alias" in glossary). Each alias do correspond to a different original page which had a different file (and thus mapping), likely at a different offset (page->index) and with different properties (page->flags and page->private). So we need to keep around all the original informations and we use the page and mapping (or anon_vma) as key to lookup alias and get the correct information for it. Why keeping page->private for each page alias ? For KSM or NUMA duplication we expect the page to be uptodate and clean and thus we should be able to free buffers (or fs specific struct) and thus no need to keep around private. However for page exclusive access we will not alias the page with any other page and we might want to keep the page in a writeable state and thus we need to keep the page->private around. Why a lot of patch move local variable out of declaration ? Many patch modify code from: foo(...) { some_type1 some_name1 = some_arg1; some_type2 some_name2 = some_arg2 || some_name1; DECLARATION... STATEMENT... } To: foo(...) { some_type1 some_name1; some_type2 some_name2; DECLARATION... some_name1 = some_arg1; some_name2 = some_arg2 || some_name1; STATEMENT... } This is done so that we can add a test to check if mapping/inode we get in argument does match the page->mapping field. We need to add such test after all declaration but before any assignment to local variable. Hence why declaration initializer need to become regular statement after all declarations. Saddly no way around that and this lead to a lot of code churn. ---------------------------------------------------------------------- Glossary: ---------------------------------------------------------------------- page alias A protected page can alias with different file and/or vma (and thus anon_vma). A page alias is just the information that correspond to each of the file/vma for that that. For instance let say that PA was a file back page it had: PA->mapping == mapping corresponding to the file PA belong PA->index == offset into the file for PA PA->private == fs specific information (like buffer_head) PA->flags == fs/mm specific informations PB was a anonymous page, it had: PB->mapping == anon_vma for the vma into which PB is map PB->index == offset into the vma PB->private == usualy 0 but can contain a swap entry PB->flags == fs/mm specific informations Now if PA and PB are merge together into a protect page then the protected page will have an alias struct for each of the original page ie: struct page_alias { struct list list; unsigned long flags; unsigned long index; void *mapping; void *private; }; struct pxa { struct list aliases; ... }; So now for the PXA (protected page) that replace PA and PB we have: PP->mapping == pointer to struct pxa for the page Note that PP->index/private are undefine for now but could be use for optimization like storing the most recent alias lookup. And the alias list will have one entry for PA and one for PB: PA_alias.mapping = PA.mapping PA_alias.private = PA.private PA_alias.index = PA.index PA_alias.flags = PA.flags PB_alias.mapping = PB.mapping PB_alias.private = PB.private PB_alias.index = PB.index PB_alias.flags = PB.flags raw page A page that is not use as anonymous or file back. Many cases, can be a free page, a page use by a device driver or some kernel features. Each case can sometimes make use of the various struct page fields (mapping, private, index, lru, ...). To get proper education this patchset wrap usage of those field in those code with raw_page*() helpers. This also make it clears what kind of page we expect to see in such driver/feature. Cc: linux-mm@kvack.org Cc: linux-fsdevel@vger.kernel.org Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Tejun Heo <tj@kernel.org> Cc: Jan Kara <jack@suse.cz> Cc: Josef Bacik <jbacik@fb.com> Jérôme Glisse (14): mm/pxa: page exclusive access add header file for all helpers. fs: define filler_t as a function pointer type fs: directly use a_ops->freepage() instead of a local copy of it. mm: add struct address_space to readpage() callback mm: add struct address_space to writepage() callback mm: add struct address_space to set_page_dirty() callback mm: add struct address_space to invalidatepage() callback mm: add struct address_space to releasepage() callback mm: add struct address_space to freepage() callback mm: add struct address_space to putback_page() callback mm: add struct address_space to launder_page() callback mm: add struct address_space to is_partially_uptodate() callback mm: add struct address_space to isolate_page() callback mm: add struct address_space to is_dirty_writeback() callback drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 3 +- drivers/video/fbdev/core/fb_defio.c | 3 +- fs/9p/vfs_addr.c | 24 ++++++--- fs/adfs/inode.c | 6 ++- fs/affs/file.c | 9 ++-- fs/affs/symlink.c | 4 +- fs/afs/dir.c | 15 ++++-- fs/afs/file.c | 18 ++++--- fs/afs/internal.h | 7 +-- fs/afs/write.c | 9 ++-- fs/befs/linuxvfs.c | 13 +++-- fs/bfs/file.c | 6 ++- fs/block_dev.c | 10 ++-- fs/btrfs/ctree.h | 3 +- fs/btrfs/disk-io.c | 16 +++--- fs/btrfs/extent_io.c | 5 +- fs/btrfs/file.c | 2 +- fs/btrfs/free-space-cache.c | 2 +- fs/btrfs/inode.c | 24 +++++---- fs/btrfs/ioctl.c | 2 +- fs/btrfs/relocation.c | 2 +- fs/btrfs/send.c | 2 +- fs/buffer.c | 14 +++-- fs/cachefiles/rdwr.c | 6 +-- fs/ceph/addr.c | 26 +++++---- fs/cifs/cifssmb.c | 2 +- fs/cifs/file.c | 15 ++++-- fs/coda/symlink.c | 4 +- fs/cramfs/inode.c | 3 +- fs/ecryptfs/mmap.c | 8 ++- fs/efs/inode.c | 3 +- fs/efs/symlink.c | 4 +- fs/erofs/data.c | 4 +- fs/erofs/super.c | 8 +-- fs/erofs/zdata.c | 4 +- fs/exfat/inode.c | 6 ++- fs/ext2/inode.c | 11 ++-- fs/ext4/inode.c | 35 +++++++----- fs/f2fs/checkpoint.c | 8 +-- fs/f2fs/data.c | 20 ++++--- fs/f2fs/f2fs.h | 8 +-- fs/f2fs/node.c | 8 +-- fs/fat/inode.c | 6 ++- fs/freevxfs/vxfs_immed.c | 6 ++- fs/freevxfs/vxfs_subr.c | 6 ++- fs/fuse/dir.c | 4 +- fs/fuse/file.c | 9 ++-- fs/gfs2/aops.c | 29 ++++++---- fs/gfs2/inode.h | 3 +- fs/gfs2/meta_io.c | 4 +- fs/hfs/inode.c | 9 ++-- fs/hfsplus/inode.c | 10 ++-- fs/hostfs/hostfs_kern.c | 6 ++- fs/hpfs/file.c | 6 ++- fs/hpfs/namei.c | 4 +- fs/hugetlbfs/inode.c | 3 +- fs/iomap/buffered-io.c | 15 +++--- fs/iomap/seek.c | 2 +- fs/isofs/compress.c | 3 +- fs/isofs/inode.c | 3 +- fs/isofs/rock.c | 4 +- fs/jffs2/file.c | 11 ++-- fs/jffs2/os-linux.h | 3 +- fs/jfs/inode.c | 6 ++- fs/jfs/jfs_metapage.c | 15 ++++-- fs/libfs.c | 13 +++-- fs/minix/inode.c | 6 ++- fs/mpage.c | 2 +- fs/nfs/dir.c | 14 ++--- fs/nfs/file.c | 16 +++--- fs/nfs/read.c | 6 ++- fs/nfs/symlink.c | 7 +-- fs/nfs/write.c | 9 ++-- fs/nilfs2/inode.c | 11 ++-- fs/nilfs2/mdt.c | 3 +- fs/nilfs2/page.c | 2 +- fs/nilfs2/segment.c | 4 +- fs/ntfs/aops.c | 10 ++-- fs/ntfs/file.c | 2 +- fs/ocfs2/aops.c | 9 ++-- fs/ocfs2/symlink.c | 4 +- fs/omfs/file.c | 6 ++- fs/orangefs/inode.c | 38 +++++++------ fs/qnx4/inode.c | 3 +- fs/qnx6/inode.c | 3 +- fs/reiserfs/inode.c | 20 ++++--- fs/romfs/super.c | 3 +- fs/squashfs/file.c | 4 +- fs/squashfs/symlink.c | 4 +- fs/sysv/itree.c | 6 ++- fs/ubifs/file.c | 21 +++++--- fs/udf/file.c | 7 ++- fs/udf/inode.c | 8 +-- fs/udf/symlink.c | 4 +- fs/ufs/inode.c | 6 ++- fs/vboxsf/file.c | 6 ++- fs/xfs/xfs_aops.c | 6 +-- fs/zonefs/super.c | 6 ++- include/linux/balloon_compaction.h | 11 ++-- include/linux/buffer_head.h | 12 +++-- include/linux/fs.h | 38 +++++++------ include/linux/iomap.h | 15 +++--- include/linux/mm.h | 11 +++- include/linux/nfs_fs.h | 5 +- include/linux/page-xa.h | 66 +++++++++++++++++++++++ include/linux/pagemap.h | 6 +-- include/linux/swap.h | 10 ++-- mm/balloon_compaction.c | 5 +- mm/filemap.c | 33 ++++++------ mm/migrate.c | 6 +-- mm/page-writeback.c | 16 +++--- mm/page_io.c | 11 ++-- mm/readahead.c | 6 +-- mm/shmem.c | 5 +- mm/truncate.c | 9 ++-- mm/vmscan.c | 12 ++--- mm/z3fold.c | 6 ++- mm/zsmalloc.c | 6 ++- 118 files changed, 722 insertions(+), 385 deletions(-) create mode 100644 include/linux/page-xa.h