On 07/03/2017 09:07 PM, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (m...@redhat.com) wrote:
>> On Fri, Jun 30, 2017 at 05:31:39PM +0100, Dr. David Alan Gilbert wrote:
>>> * Christian Borntraeger (borntrae...@de.ibm.com) wrote:
>>>> On 04/26/2017 01:45 PM, Christian Borntraeger wrote:
>>>>
>>>>>> Hmm, I have a theory, if the flags field has bit 1 set, i.e.
>>>>>> RAM_SAVE_FLAG_COMPRESS
>>>>>> then try changing ram_handle_compressed to always do the memset.
>>>>>
>>>>> FWIW, changing ram_handle_compressed to always memset makes the problem
>>>>> go away.
>>>>
>>>> It is still running fine now with the "always memset change"
>>>
>>> Did we ever nail down a fix for this; as I remember Andrea said
>>> we shouldn't need to do that memset, but we came to the conclusion
>>> it was something specific to how s390 protection keys worked.
>>>
>>> Dave
>>
>> No we didn't. Let's merge that for now, with a comment that
>> we don't really understand what's going on?
>
> Hmm no, I don't really want to change the !s390 behaviour, especially
> since it causes allocation that we otherwise avoid and Andrea's
> reply tothe original post pointed out it's not necessary.
Since storage keys are per physical page we must not have shared pages.
Therefore in s390_enable_skey we already do a break_ksm (via ksm_madvise),
in other words we already allocate pages on the keyless->keyed switch.
So why not do the same for zero pages - instead of invalidating the page
table entry, lets just do a proper COW.
Something like this maybe (Andrea, Martin any better way to do that?)
diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c
index 4fb3d3c..11475c7 100644
--- a/arch/s390/mm/gmap.c
+++ b/arch/s390/mm/gmap.c
@@ -2149,13 +2149,18 @@ EXPORT_SYMBOL_GPL(s390_enable_sie);
static int __s390_enable_skey(pte_t *pte, unsigned long addr,
unsigned long next, struct mm_walk *walk)
{
+ struct page *page;
/*
- * Remove all zero page mappings,
+ * Remove all zero page mappings with a COW
* after establishing a policy to forbid zero page mappings
* following faults for that page will get fresh anonymous pages
*/
- if (is_zero_pfn(pte_pfn(*pte)))
- ptep_xchg_direct(walk->mm, addr, pte, __pte(_PAGE_INVALID));
+ if (is_zero_pfn(pte_pfn(*pte))) {
+ if (get_user_pages(addr, 1, FOLL_WRITE, &page, NULL) == 1)
+ put_page(page);
+ else
+ return -ENOMEM;
+ }
/* Clear storage key */
ptep_zap_key(walk->mm, addr, pte);
return 0;
