Re: [RFC, PATCH 13/22] mm, rmap: Free encrypted pages once mapcount drops to zero
On Tue, Mar 06, 2018 at 06:59:04AM -0800, Dave Hansen wrote:
> On 03/06/2018 12:27 AM, Kirill A. Shutemov wrote:
> > + anon_vma = page_anon_vma(page);
> > + if (anon_vma_encrypted(anon_vma)) {
> > + int keyid = anon_vma_keyid(anon_vma);
> > + free_encrypt_page(page, keyid, compound_order(page));
> > + }
> > }
>
> So, just double-checking: free_encrypt_page() neither "frees and
> encrypts the page"" nor "free an encrypted page"?
>
> That seems a bit suboptimal. :)
Yes, I'm bad with words :)
--
Kirill A. Shutemov
Re: [RFC, PATCH 13/22] mm, rmap: Free encrypted pages once mapcount drops to zero
On Tue, Mar 06, 2018 at 06:59:04AM -0800, Dave Hansen wrote:
> On 03/06/2018 12:27 AM, Kirill A. Shutemov wrote:
> > + anon_vma = page_anon_vma(page);
> > + if (anon_vma_encrypted(anon_vma)) {
> > + int keyid = anon_vma_keyid(anon_vma);
> > + free_encrypt_page(page, keyid, compound_order(page));
> > + }
> > }
>
> So, just double-checking: free_encrypt_page() neither "frees and
> encrypts the page"" nor "free an encrypted page"?
>
> That seems a bit suboptimal. :)
Yes, I'm bad with words :)
--
Kirill A. Shutemov
Re: [RFC, PATCH 13/22] mm, rmap: Free encrypted pages once mapcount drops to zero
On 03/06/2018 12:27 AM, Kirill A. Shutemov wrote:
> + anon_vma = page_anon_vma(page);
> + if (anon_vma_encrypted(anon_vma)) {
> + int keyid = anon_vma_keyid(anon_vma);
> + free_encrypt_page(page, keyid, compound_order(page));
> + }
> }
So, just double-checking: free_encrypt_page() neither "frees and
encrypts the page"" nor "free an encrypted page"?
That seems a bit suboptimal. :)
Re: [RFC, PATCH 13/22] mm, rmap: Free encrypted pages once mapcount drops to zero
On 03/06/2018 12:27 AM, Kirill A. Shutemov wrote:
> + anon_vma = page_anon_vma(page);
> + if (anon_vma_encrypted(anon_vma)) {
> + int keyid = anon_vma_keyid(anon_vma);
> + free_encrypt_page(page, keyid, compound_order(page));
> + }
> }
So, just double-checking: free_encrypt_page() neither "frees and
encrypts the page"" nor "free an encrypted page"?
That seems a bit suboptimal. :)
Re: [RFC, PATCH 13/22] mm, rmap: Free encrypted pages once mapcount drops to zero
On Mon, Mar 05, 2018 at 11:13:36AM -0800, Dave Hansen wrote:
> On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote:
> > @@ -1292,6 +1308,12 @@ static void page_remove_anon_compound_rmap(struct
> > page *page)
> > __mod_node_page_state(page_pgdat(page), NR_ANON_MAPPED, -nr);
> > deferred_split_huge_page(page);
> > }
> > +
> > + anon_vma = page_anon_vma(page);
> > + if (anon_vma_encrypted(anon_vma)) {
> > + int keyid = anon_vma_keyid(anon_vma);
> > + free_encrypt_page(page, keyid, compound_order(page));
> > + }
> > }
>
> It's not covered in the description and I'm to lazy to dig into it, so:
> Without this code, where do they get freed? Why does it not cause any
> problems to free them here?
It's the only place where we get it freed. "Freeing" is not the best
terminology here, but I failed to come up with something batter.
We prepare the encryption page to being freed: flush the cache in MKTME
case.
The page itself gets freed later in a usual manner: once refcount drops to
zero. The problem is that we may not have valid anon_vma around once
mapcount drops to zero, so we have to do "freeing" here.
For anonymous memory once mapcount dropped to zero there's no way it will
get mapped back to userspace. page_remove_anon
Kernel still will be able to access the page with kmap() and I will need
to be very careful to get it right wrt cache management.
I'll update the description.
--
Kirill A. Shutemov
Re: [RFC, PATCH 13/22] mm, rmap: Free encrypted pages once mapcount drops to zero
On Mon, Mar 05, 2018 at 11:13:36AM -0800, Dave Hansen wrote:
> On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote:
> > @@ -1292,6 +1308,12 @@ static void page_remove_anon_compound_rmap(struct
> > page *page)
> > __mod_node_page_state(page_pgdat(page), NR_ANON_MAPPED, -nr);
> > deferred_split_huge_page(page);
> > }
> > +
> > + anon_vma = page_anon_vma(page);
> > + if (anon_vma_encrypted(anon_vma)) {
> > + int keyid = anon_vma_keyid(anon_vma);
> > + free_encrypt_page(page, keyid, compound_order(page));
> > + }
> > }
>
> It's not covered in the description and I'm to lazy to dig into it, so:
> Without this code, where do they get freed? Why does it not cause any
> problems to free them here?
It's the only place where we get it freed. "Freeing" is not the best
terminology here, but I failed to come up with something batter.
We prepare the encryption page to being freed: flush the cache in MKTME
case.
The page itself gets freed later in a usual manner: once refcount drops to
zero. The problem is that we may not have valid anon_vma around once
mapcount drops to zero, so we have to do "freeing" here.
For anonymous memory once mapcount dropped to zero there's no way it will
get mapped back to userspace. page_remove_anon
Kernel still will be able to access the page with kmap() and I will need
to be very careful to get it right wrt cache management.
I'll update the description.
--
Kirill A. Shutemov
Re: [RFC, PATCH 13/22] mm, rmap: Free encrypted pages once mapcount drops to zero
On Mon, Mar 05, 2018 at 11:12:15AM -0800, Dave Hansen wrote: > On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote: > > extern void prep_encrypt_page(struct page *page, gfp_t gfp, unsigned int > > order); > > +extern void free_encrypt_page(struct page *page, int keyid, unsigned int > > order); > > The grammar here is weird, I think. > > Why not free_encrypted_page()? Okay, I'll fix this. -- Kirill A. Shutemov
Re: [RFC, PATCH 13/22] mm, rmap: Free encrypted pages once mapcount drops to zero
On Mon, Mar 05, 2018 at 11:12:15AM -0800, Dave Hansen wrote: > On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote: > > extern void prep_encrypt_page(struct page *page, gfp_t gfp, unsigned int > > order); > > +extern void free_encrypt_page(struct page *page, int keyid, unsigned int > > order); > > The grammar here is weird, I think. > > Why not free_encrypted_page()? Okay, I'll fix this. -- Kirill A. Shutemov
Re: [RFC, PATCH 13/22] mm, rmap: Free encrypted pages once mapcount drops to zero
On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote:
> @@ -1292,6 +1308,12 @@ static void page_remove_anon_compound_rmap(struct page
> *page)
> __mod_node_page_state(page_pgdat(page), NR_ANON_MAPPED, -nr);
> deferred_split_huge_page(page);
> }
> +
> + anon_vma = page_anon_vma(page);
> + if (anon_vma_encrypted(anon_vma)) {
> + int keyid = anon_vma_keyid(anon_vma);
> + free_encrypt_page(page, keyid, compound_order(page));
> + }
> }
It's not covered in the description and I'm to lazy to dig into it, so:
Without this code, where do they get freed? Why does it not cause any
problems to free them here?
Re: [RFC, PATCH 13/22] mm, rmap: Free encrypted pages once mapcount drops to zero
On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote:
> @@ -1292,6 +1308,12 @@ static void page_remove_anon_compound_rmap(struct page
> *page)
> __mod_node_page_state(page_pgdat(page), NR_ANON_MAPPED, -nr);
> deferred_split_huge_page(page);
> }
> +
> + anon_vma = page_anon_vma(page);
> + if (anon_vma_encrypted(anon_vma)) {
> + int keyid = anon_vma_keyid(anon_vma);
> + free_encrypt_page(page, keyid, compound_order(page));
> + }
> }
It's not covered in the description and I'm to lazy to dig into it, so:
Without this code, where do they get freed? Why does it not cause any
problems to free them here?
Re: [RFC, PATCH 13/22] mm, rmap: Free encrypted pages once mapcount drops to zero
On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote: > extern void prep_encrypt_page(struct page *page, gfp_t gfp, unsigned int > order); > +extern void free_encrypt_page(struct page *page, int keyid, unsigned int > order); The grammar here is weird, I think. Why not free_encrypted_page()?
Re: [RFC, PATCH 13/22] mm, rmap: Free encrypted pages once mapcount drops to zero
On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote: > extern void prep_encrypt_page(struct page *page, gfp_t gfp, unsigned int > order); > +extern void free_encrypt_page(struct page *page, int keyid, unsigned int > order); The grammar here is weird, I think. Why not free_encrypted_page()?
