* Re: [RFC 0/4] RFC - Coherent Device Memory (Not for inclusion)
From: Michal Hocko @ 2017-05-04 12:52 UTC (permalink / raw)
To: Balbir Singh
Cc: linux-mm, akpm, khandual, benh, aneesh.kumar, paulmck, srikar,
haren, jglisse, mgorman, arbab, vbabka, cl
In-Reply-To: <1493875615.7934.1.camel@gmail.com>
On Thu 04-05-17 15:26:55, Balbir Singh wrote:
> On Tue, 2017-05-02 at 16:36 +0200, Michal Hocko wrote:
> > On Wed 19-04-17 17:52:38, Balbir Singh wrote:
[...]
> > > 2. kswapd reclaim
> >
> > How is the memory reclaim handled then? How are users expected to handle
> > OOM situation?
> >
>
> 1. The fallback node list for coherent memory includes regular memory
> nodes
> 2. Direct reclaim works, I've tested it
But the direct reclaim would be effective only _after_ all other nodes
are full.
I thought that kswapd reclaim is a problem because the HW doesn't
support aging properly but as the direct reclaim works then what is the
actual problem?
> > > The reason for exposing this device memory as NUMA is to simplify
> > > the programming model, where memory allocation via malloc() or
> > > mmap() for example would seamlessly work across both kinds of
> > > memory. Since we expect the size of device memory to be smaller
> > > than system RAM, we would like to control the allocation of such
> > > memory. The proposed mechanism reuses nodemasks and explicit
> > > specification of the coherent node in the nodemask for allocation
> > > from device memory. This implementation also allows for kernel
> > > level allocation via __GFP_THISNODE and existing techniques
> > > such as page migration to work.
> >
> > so it basically resembles isol_cpus except for memory, right. I believe
> > scheduler people are more than unhappy about this interface...
> >
>
> isol_cpus were for an era when timer/interrupts and other scheduler
> infrastructure present today was not around, but I don't mean to digress.
AFAIU, it has been added to _isolate_ some cpus from the scheduling domain
and have them available for the explicit affinity usage. You are
effectivelly proposing the same thing.
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: RFC v2: post-init-read-only protection for data allocated dynamically
From: Michal Hocko @ 2017-05-04 13:11 UTC (permalink / raw)
To: Igor Stoppa; +Cc: linux-mm, linux-kernel, Dave Hansen
In-Reply-To: <83d4556c-b21c-7ae5-6e83-4621a74f9fd5@huawei.com>
On Thu 04-05-17 15:14:10, Igor Stoppa wrote:
[...]
> I wonder if you are thinking about loadable modules or maybe livepatch.
> My proposal, in its current form, is only about what is done when the
> kernel initialization is performed. So it would not take those cases
> under its umbrella. Actually it might be incompatible with livepatch, if
> any of the read-only data is supposed to be updated.
>
> Since it's meant to improve the current level of integrity, I would
> prefer to have a progressive approach and address modules/livepatch in a
> later phase, if this is not seen as a show stopper.
I believe that this is a fundamental question. Sealing sounds useful
for after-boot usecases as well and it would change the approach
considerably. Coming up with an ad-hoc solution for the boot only way
seems like a wrong way to me. And as you've said SELinux which is your
target already does the thing after the early boot.
[...]
> > Roughly it would mean that once kmem_cache_seal() is
> > called on a cache it would changed page tables to used slab pages to RO
> > state. This would obviously need some fiddling to make those pages not
> > usable for new allocations from sealed pages. It would also mean some
> > changes to kfree path but I guess this is doable.
>
> Ok, as it probably has already become evident, I have just started
> peeking into the memory subsystem, so this is the sort of guidance I was
> hoping I could receive =) - thank you
>
> Question: I see that some pages can be moved around. Would this apply to
> the slab-based solution, or can I assume that once I have certain
> physical pages sealed, they will not be altered anymore?
Slab pages are not migrateable currently. Even if they start being
migrateable it would be an opt-in because that requires pointers tracking
to make sure they are updated properly.
> >> * While I do not strictly need a new memory zone, memory zones are what
> >> kmalloc understands at the moment: AFAIK, it is not possible to tell
> >> kmalloc from which memory pool it should fish out the memory, other than
> >> having a reference to a memory zone.
> >
> > As I've said already. I think that a zone is a completely wrong
> > approach. How would it help anyway. It is the allocator on top of the
> > page allocator which has to do clever things to support sealing.
>
>
> Ok, as long as there is a way forward that fits my needs and has the
> possibility to be merged upstream, I'm fine with it.
>
> I suppose zones are the first thing one meets when reading the code, so
> they are probably the first target that comes to mind.
> That's what happened to me.
>
> I will probably come back with further questions, but I can then start
> putting together some prototype of what you described.
>
> I am fine with providing a generic solution, but I must make sure that
> it works with slub. I suppose what you proposed will do it, right?
I haven't researched that too deeply. In principle both SLAB and SLUB
maintain slab pages in a similar way so I do not see any fundamental
problems.
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: RFC v2: post-init-read-only protection for data allocated dynamically
From: Igor Stoppa @ 2017-05-04 13:37 UTC (permalink / raw)
To: Michal Hocko; +Cc: linux-mm, linux-kernel, Dave Hansen
In-Reply-To: <20170504131131.GI31540@dhcp22.suse.cz>
On 04/05/17 16:11, Michal Hocko wrote:
> On Thu 04-05-17 15:14:10, Igor Stoppa wrote:
> I believe that this is a fundamental question. Sealing sounds useful
> for after-boot usecases as well and it would change the approach
> considerably. Coming up with an ad-hoc solution for the boot only way
> seems like a wrong way to me. And as you've said SELinux which is your
> target already does the thing after the early boot.
I didn't spend too many thoughts on this so far, because the zone-based
approach seemed almost doomed, so I wanted to wait for the evolution of
the discussion :-)
The main question here is granularity, I think.
At least, as first cut, the simpler approach would be to have a master
toggle: when some legitimate operation needs to happen, the seal is
lifted across the entire range, then it is put back in place, once the
operation has concluded.
Simplicity is the main advantage.
The disadvantage is that anything can happen, undetected, while the seal
is lifted.
OTOH the amount of code that could backfire should be fairly limited, so
it doesn't seem a huge issue to me.
The alternative would be to somehow know what a write will change and
make only the appropriate page(s) writable. But it seems overkill to me.
Especially because in some cases, with huge pages, everything would fit
anyway in one page.
One more option that comes to mind - but I do not know how realistic it
would be - is to have multiple slabs, to be used for different purposes.
Ex: one for the monolithic kernel and one for modules.
It wouldn't help for livepatch, though, as it can modify both, so both
would have to be unprotected.
But live-patching is potentially a far less frequent event than module
loading/unloading (thinking about USB gadgets, for example).
[...]
> Slab pages are not migrateable currently. Even if they start being
> migrateable it would be an opt-in because that requires pointers tracking
> to make sure they are updated properly.
ok
[...]
> I haven't researched that too deeply. In principle both SLAB and SLUB
> maintain slab pages in a similar way so I do not see any fundamental
> problems.
good, then I could proceed with the prototype, if there are no further
objections/questions and we agree that, implementation aside, there are
no obvious fundamental problems preventing the merge
---
thanks, igor
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: RFC v2: post-init-read-only protection for data allocated dynamically
From: Michal Hocko @ 2017-05-04 14:01 UTC (permalink / raw)
To: Igor Stoppa; +Cc: linux-mm, linux-kernel, Dave Hansen
In-Reply-To: <df1b34fb-f90b-da9e-6723-49e8f1cb1757@huawei.com>
On Thu 04-05-17 16:37:55, Igor Stoppa wrote:
> On 04/05/17 16:11, Michal Hocko wrote:
> > On Thu 04-05-17 15:14:10, Igor Stoppa wrote:
>
> > I believe that this is a fundamental question. Sealing sounds useful
> > for after-boot usecases as well and it would change the approach
> > considerably. Coming up with an ad-hoc solution for the boot only way
> > seems like a wrong way to me. And as you've said SELinux which is your
> > target already does the thing after the early boot.
>
> I didn't spend too many thoughts on this so far, because the zone-based
> approach seemed almost doomed, so I wanted to wait for the evolution of
> the discussion :-)
>
> The main question here is granularity, I think.
>
> At least, as first cut, the simpler approach would be to have a master
> toggle: when some legitimate operation needs to happen, the seal is
> lifted across the entire range, then it is put back in place, once the
> operation has concluded.
>
> Simplicity is the main advantage.
>
> The disadvantage is that anything can happen, undetected, while the seal
> is lifted.
Yes and I think this makes it basically pointless
> OTOH the amount of code that could backfire should be fairly limited, so
> it doesn't seem a huge issue to me.
>
> The alternative would be to somehow know what a write will change and
> make only the appropriate page(s) writable. But it seems overkill to me.
> Especially because in some cases, with huge pages, everything would fit
> anyway in one page.
>
> One more option that comes to mind - but I do not know how realistic it
> would be - is to have multiple slabs, to be used for different purposes.
> Ex: one for the monolithic kernel and one for modules.
> It wouldn't help for livepatch, though, as it can modify both, so both
> would have to be unprotected.
Just to make my proposal more clear. I suggest the following workflow
cache = kmem_cache_create(foo, object_size, ..., SLAB_SEAL);
obj = kmem_cache_alloc(cache, gfp_mask);
init_obj(obj)
[more allocations]
kmem_cache_seal(cache);
All slab pages belonging to the cache would get write protection. All
new allocations from this cache would go to new slab pages. Later
kmem_cache_seal will write protect only those new pages.
The main discomfort with this approach is that you have to create those
caches in advance, obviously. We could help by creating some general
purpose caches for common sizes but this sound like an overkill to me.
The caller will know which objects will need the protection so the
appropriate cache can be created on demand. But this reall depends on
potential users...
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v5 31/32] x86: Add sysfs support for Secure Memory Encryption
From: Tom Lendacky @ 2017-05-04 14:13 UTC (permalink / raw)
To: Dave Young, Dave Hansen
Cc: linux-arch, linux-efi, kvm, linux-doc, x86, kexec, linux-kernel,
kasan-dev, linux-mm, iommu, Thomas Gleixner, Rik van Riel,
Brijesh Singh, Toshimitsu Kani, Arnd Bergmann, Jonathan Corbet,
Matt Fleming, Joerg Roedel, Radim Krčmář,
Konrad Rzeszutek Wilk, Andrey Ryabinin, Ingo Molnar,
Michael S. Tsirkin, Andy Lutomirski, H. Peter Anvin,
Borislav Petkov, Paolo Bonzini, Alexander Potapenko,
Larry Woodman, Dmitry Vyukov
In-Reply-To: <20170427072547.GB15297@dhcp-128-65.nay.redhat.com>
On 4/27/2017 2:25 AM, Dave Young wrote:
> On 04/21/17 at 02:55pm, Dave Hansen wrote:
>> On 04/18/2017 02:22 PM, Tom Lendacky wrote:
>>> Add sysfs support for SME so that user-space utilities (kdump, etc.) can
>>> determine if SME is active.
>>>
>>> A new directory will be created:
>>> /sys/kernel/mm/sme/
>>>
>>> And two entries within the new directory:
>>> /sys/kernel/mm/sme/active
>>> /sys/kernel/mm/sme/encryption_mask
>>
>> Why do they care, and what will they be doing with this information?
>
> Since kdump will copy old memory but need this to know if the old memory
> was encrypted or not. With this sysfs file we can know the previous SME
> status and pass to kdump kernel as like a kernel param.
>
> Tom, have you got chance to try if it works or not?
Sorry, I haven't had a chance to test this yet.
Thanks,
Tom
>
> Thanks
> Dave
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v5 31/32] x86: Add sysfs support for Secure Memory Encryption
From: Tom Lendacky @ 2017-05-04 14:17 UTC (permalink / raw)
To: Dave Hansen, Dave Young
Cc: linux-arch, linux-efi, kvm, linux-doc, x86, kexec, linux-kernel,
kasan-dev, linux-mm, iommu, Thomas Gleixner, Rik van Riel,
Brijesh Singh, Toshimitsu Kani, Arnd Bergmann, Jonathan Corbet,
Matt Fleming, Joerg Roedel, Radim Krčmář,
Konrad Rzeszutek Wilk, Andrey Ryabinin, Ingo Molnar,
Michael S. Tsirkin, Andy Lutomirski, H. Peter Anvin,
Borislav Petkov, Paolo Bonzini, Alexander Potapenko,
Larry Woodman, Dmitry Vyukov
In-Reply-To: <1f034974-20e6-b5e9-e6ff-434b634e1522@intel.com>
On 4/27/2017 10:52 AM, Dave Hansen wrote:
> On 04/27/2017 12:25 AM, Dave Young wrote:
>> On 04/21/17 at 02:55pm, Dave Hansen wrote:
>>> On 04/18/2017 02:22 PM, Tom Lendacky wrote:
>>>> Add sysfs support for SME so that user-space utilities (kdump, etc.) can
>>>> determine if SME is active.
>>>>
>>>> A new directory will be created:
>>>> /sys/kernel/mm/sme/
>>>>
>>>> And two entries within the new directory:
>>>> /sys/kernel/mm/sme/active
>>>> /sys/kernel/mm/sme/encryption_mask
>>>
>>> Why do they care, and what will they be doing with this information?
>>
>> Since kdump will copy old memory but need this to know if the old memory
>> was encrypted or not. With this sysfs file we can know the previous SME
>> status and pass to kdump kernel as like a kernel param.
>>
>> Tom, have you got chance to try if it works or not?
>
> What will the kdump kernel do with it though? We kexec() into that
> kernel so the SME keys will all be the same, right? So, will the kdump
Yes, the SME key will be same after a kexec.
> kernel be just setting the encryption bit in the PTE so it can copy the
> old plaintext out?
Yes, the idea would be to set the encryption bit in the PTE when mapping
and copying encrypted pages and not set it for unencrypted pages.
>
> Why do we need both 'active' and 'encryption_mask'? How could it be
> that the hardware-enumerated 'encryption_mask' changes across a kexec()?
>
We don't need both, I just added the 'encryption mask' entry for
information. It won't change across a kexec, I can remove it.
Thanks,
Tom
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v5 06/32] x86/mm: Add Secure Memory Encryption (SME) support
From: Tom Lendacky @ 2017-05-04 14:24 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-arch, linux-efi, kvm, linux-doc, x86, kexec, linux-kernel,
kasan-dev, linux-mm, iommu, Rik van Riel,
Radim Krčmář, Toshimitsu Kani, Arnd Bergmann,
Jonathan Corbet, Matt Fleming, Michael S. Tsirkin, Joerg Roedel,
Konrad Rzeszutek Wilk, Paolo Bonzini, Larry Woodman,
Brijesh Singh, Ingo Molnar, Andy Lutomirski, H. Peter Anvin,
Andrey Ryabinin, Alexander Potapenko, Dave Young, Thomas Gleixner,
Dmitry Vyukov
In-Reply-To: <20170427154631.2tsqgax4kqcvydnx@pd.tnic>
On 4/27/2017 10:46 AM, Borislav Petkov wrote:
> On Tue, Apr 18, 2017 at 04:17:27PM -0500, Tom Lendacky wrote:
>> Add support for Secure Memory Encryption (SME). This initial support
>> provides a Kconfig entry to build the SME support into the kernel and
>> defines the memory encryption mask that will be used in subsequent
>> patches to mark pages as encrypted.
>
> ...
>
>> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
>> new file mode 100644
>> index 0000000..d5c4a2b
>> --- /dev/null
>> +++ b/arch/x86/include/asm/mem_encrypt.h
>> @@ -0,0 +1,42 @@
>> +/*
>> + * AMD Memory Encryption Support
>> + *
>> + * Copyright (C) 2016 Advanced Micro Devices, Inc.
>> + *
>> + * Author: Tom Lendacky <thomas.lendacky@amd.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>
> These ifdeffery closing #endif markers look strange:
>
>> +#ifndef __X86_MEM_ENCRYPT_H__
>> +#define __X86_MEM_ENCRYPT_H__
>> +
>> +#ifndef __ASSEMBLY__
>> +
>> +#ifdef CONFIG_AMD_MEM_ENCRYPT
>> +
>> +extern unsigned long sme_me_mask;
>> +
>> +static inline bool sme_active(void)
>> +{
>> + return !!sme_me_mask;
>> +}
>> +
>> +#else /* !CONFIG_AMD_MEM_ENCRYPT */
>> +
>> +#ifndef sme_me_mask
>> +#define sme_me_mask 0UL
>> +
>> +static inline bool sme_active(void)
>> +{
>> + return false;
>> +}
>> +#endif
>
> this endif is the sme_me_mask closing one and it has sme_active() in it.
> Shouldn't it be:
>
> #ifndef sme_me_mask
> #define sme_me_mask 0UL
> #endif
>
> and have sme_active below it, in the !CONFIG_AMD_MEM_ENCRYPT branch?
>
> The same thing is in include/linux/mem_encrypt.h
I did this so that an the include order wouldn't cause issues (including
asm/mem_encrypt.h followed by later by a linux/mem_encrypt.h include).
I can make this a bit clearer by having separate #defines for each
thing, e.g.:
#ifndef sme_me_mask
#define sme_me_mask 0UL
#endif
#ifndef sme_active
#define sme_active sme_active
static inline ...
#endif
Is that better/clearer?
Thanks,
Tom
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: RFC v2: post-init-read-only protection for data allocated dynamically
From: Dave Hansen @ 2017-05-04 14:30 UTC (permalink / raw)
To: Igor Stoppa, Michal Hocko; +Cc: linux-mm, linux-kernel
In-Reply-To: <b7bb1884-3125-5c98-f1fe-53b974454ce2@huawei.com>
On 05/04/2017 01:17 AM, Igor Stoppa wrote:
> Or, let me put it differently: my goal is to not fracture more pages
> than needed.
> It will probably require some profiling to figure out what is the
> ballpark of the memory footprint.
This is easy to say, but hard to do. What if someone loads a different
set of LSMs, or uses a very different configuration? How could this
possibly work generally without vastly over-reserving in most cases?
> I might have overlooked some aspect of this, but the overall goal
> is to have a memory range (I won't call it zone, to avoid referring to a
> specific implementation) which is as tightly packed as possible, stuffed
> with all the data that is expected to become read-only.
I'm starting with the assumption that a new zone isn't feasible. :)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v5 09/32] x86/mm: Provide general kernel support for memory encryption
From: Tom Lendacky @ 2017-05-04 14:34 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-arch, linux-efi, kvm, linux-doc, x86, kexec, linux-kernel,
kasan-dev, linux-mm, iommu, Rik van Riel,
Radim Krčmář, Toshimitsu Kani, Arnd Bergmann,
Jonathan Corbet, Matt Fleming, Michael S. Tsirkin, Joerg Roedel,
Konrad Rzeszutek Wilk, Paolo Bonzini, Larry Woodman,
Brijesh Singh, Ingo Molnar, Andy Lutomirski, H. Peter Anvin,
Andrey Ryabinin, Alexander Potapenko, Dave Young, Thomas Gleixner,
Dmitry Vyukov
In-Reply-To: <20170427161227.c57dkvghz63pvmu2@pd.tnic>
On 4/27/2017 11:12 AM, Borislav Petkov wrote:
> On Tue, Apr 18, 2017 at 04:17:54PM -0500, Tom Lendacky wrote:
>> Changes to the existing page table macros will allow the SME support to
>> be enabled in a simple fashion with minimal changes to files that use these
>> macros. Since the memory encryption mask will now be part of the regular
>> pagetable macros, we introduce two new macros (_PAGE_TABLE_NOENC and
>> _KERNPG_TABLE_NOENC) to allow for early pagetable creation/initialization
>> without the encryption mask before SME becomes active. Two new pgprot()
>> macros are defined to allow setting or clearing the page encryption mask.
>
> ...
>
>> @@ -55,7 +57,7 @@ static inline void copy_user_page(void *to, void *from, unsigned long vaddr,
>> __phys_addr_symbol(__phys_reloc_hide((unsigned long)(x)))
>>
>> #ifndef __va
>> -#define __va(x) ((void *)((unsigned long)(x)+PAGE_OFFSET))
>> +#define __va(x) ((void *)(__sme_clr(x) + PAGE_OFFSET))
>> #endif
>>
>> #define __boot_va(x) __va(x)
>> diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
>> index 7bd0099..fead0a5 100644
>> --- a/arch/x86/include/asm/page_types.h
>> +++ b/arch/x86/include/asm/page_types.h
>> @@ -15,7 +15,7 @@
>> #define PUD_PAGE_SIZE (_AC(1, UL) << PUD_SHIFT)
>> #define PUD_PAGE_MASK (~(PUD_PAGE_SIZE-1))
>>
>> -#define __PHYSICAL_MASK ((phys_addr_t)((1ULL << __PHYSICAL_MASK_SHIFT) - 1))
>> +#define __PHYSICAL_MASK ((phys_addr_t)(__sme_clr((1ULL << __PHYSICAL_MASK_SHIFT) - 1)))
>
> That looks strange: poking SME mask hole into a mask...?
I masked it out here based on a previous comment from Dave Hansen:
http://marc.info/?l=linux-kernel&m=148778719826905&w=2
I could move the __sme_clr into the individual defines of:
PHYSICAL_PAGE_MASK, PHYSICAL_PMD_PAGE_MASK and PHYSICAL_PUD_PAGE_MASK
Either way works for me.
Thanks,
Tom
>
>> #define __VIRTUAL_MASK ((1UL << __VIRTUAL_MASK_SHIFT) - 1)
>>
>> /* Cast *PAGE_MASK to a signed type so that it is sign-extended if
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v5 06/32] x86/mm: Add Secure Memory Encryption (SME) support
From: Borislav Petkov @ 2017-05-04 14:36 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-arch, linux-efi, kvm, linux-doc, x86, kexec, linux-kernel,
kasan-dev, linux-mm, iommu, Rik van Riel,
Radim Krčmář, Toshimitsu Kani, Arnd Bergmann,
Jonathan Corbet, Matt Fleming, Michael S. Tsirkin, Joerg Roedel,
Konrad Rzeszutek Wilk, Paolo Bonzini, Larry Woodman,
Brijesh Singh, Ingo Molnar, Andy Lutomirski, H. Peter Anvin,
Andrey Ryabinin, Alexander Potapenko, Dave Young, Thomas Gleixner,
Dmitry Vyukov
In-Reply-To: <d9d9f10a-0ce5-53e8-41f5-f8690dbd7362@amd.com>
On Thu, May 04, 2017 at 09:24:11AM -0500, Tom Lendacky wrote:
> I did this so that an the include order wouldn't cause issues (including
> asm/mem_encrypt.h followed by later by a linux/mem_encrypt.h include).
> I can make this a bit clearer by having separate #defines for each
> thing, e.g.:
>
> #ifndef sme_me_mask
> #define sme_me_mask 0UL
> #endif
>
> #ifndef sme_active
> #define sme_active sme_active
> static inline ...
> #endif
>
> Is that better/clearer?
I guess but where do we have to include both the asm/ and the linux/
version?
IOW, can we avoid these issues altogether by partitioning symbol
declarations differently among the headers?
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v5 12/32] x86/mm: Insure that boot memory areas are mapped properly
From: Tom Lendacky @ 2017-05-04 14:39 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-arch, linux-efi, kvm, linux-doc, x86, kexec, linux-kernel,
kasan-dev, linux-mm, iommu, Rik van Riel,
Radim Krčmář, Toshimitsu Kani, Arnd Bergmann,
Jonathan Corbet, Matt Fleming, Michael S. Tsirkin, Joerg Roedel,
Konrad Rzeszutek Wilk, Paolo Bonzini, Larry Woodman,
Brijesh Singh, Ingo Molnar, Andy Lutomirski, H. Peter Anvin,
Andrey Ryabinin, Alexander Potapenko, Dave Young, Thomas Gleixner,
Dmitry Vyukov
In-Reply-To: <20170504101609.vazu4tuc3gqapaqk@pd.tnic>
On 5/4/2017 5:16 AM, Borislav Petkov wrote:
> On Tue, Apr 18, 2017 at 04:18:22PM -0500, Tom Lendacky wrote:
>> The boot data and command line data are present in memory in a decrypted
>> state and are copied early in the boot process. The early page fault
>> support will map these areas as encrypted, so before attempting to copy
>> them, add decrypted mappings so the data is accessed properly when copied.
>>
>> For the initrd, encrypt this data in place. Since the future mapping of the
>> initrd area will be mapped as encrypted the data will be accessed properly.
>>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>> arch/x86/include/asm/mem_encrypt.h | 11 +++++
>> arch/x86/include/asm/pgtable.h | 3 +
>> arch/x86/kernel/head64.c | 30 ++++++++++++--
>> arch/x86/kernel/setup.c | 10 +++++
>> arch/x86/mm/mem_encrypt.c | 77 ++++++++++++++++++++++++++++++++++++
>> 5 files changed, 127 insertions(+), 4 deletions(-)
>
> ...
>
>> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
>> index 603a166..a95800b 100644
>> --- a/arch/x86/kernel/setup.c
>> +++ b/arch/x86/kernel/setup.c
>> @@ -115,6 +115,7 @@
>> #include <asm/microcode.h>
>> #include <asm/mmu_context.h>
>> #include <asm/kaslr.h>
>> +#include <asm/mem_encrypt.h>
>>
>> /*
>> * max_low_pfn_mapped: highest direct mapped pfn under 4GB
>> @@ -374,6 +375,15 @@ static void __init reserve_initrd(void)
>> !ramdisk_image || !ramdisk_size)
>> return; /* No initrd provided by bootloader */
>>
>> + /*
>> + * If SME is active, this memory will be marked encrypted by the
>> + * kernel when it is accessed (including relocation). However, the
>> + * ramdisk image was loaded decrypted by the bootloader, so make
>> + * sure that it is encrypted before accessing it.
>> + */
>> + if (sme_active())
>
> That test is not needed here because __sme_early_enc_dec() already tests
> sme_me_mask. There you should change that test to sme_active() instead.
Yeah, I was probably thinking slightly ahead to SEV where the initrd
will already be encrypted and so we only want to do this for SME.
That change can come in the SEV support patches, though.
Thanks,
Tom
>
>> + sme_early_encrypt(ramdisk_image, ramdisk_end - ramdisk_image);
>> +
>> initrd_start = 0;
>>
>> mapped_size = memblock_mem_size(max_pfn_mapped);
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 2/2] dax: fix data corruption due to stale mmap reads
From: Jan Kara @ 2017-05-04 9:12 UTC (permalink / raw)
To: Ross Zwisler
Cc: Jan Kara, Andrew Morton, linux-kernel, Alexander Viro,
Alexey Kuznetsov, Andrey Ryabinin, Anna Schumaker,
Christoph Hellwig, Dan Williams, Darrick J. Wong,
Eric Van Hensbergen, Jens Axboe, Johannes Weiner,
Konrad Rzeszutek Wilk, Latchesar Ionkov, linux-cifs,
linux-fsdevel, linux-mm, linux-nfs, linux-nvdimm, Matthew Wilcox,
Ron Minnich, samba-technical, Steve French, Trond Myklebust,
v9fs-developer
In-Reply-To: <20170501223855.GA25862@linux.intel.com>
On Mon 01-05-17 16:38:55, Ross Zwisler wrote:
> > So for now I'm still more inclined to just stay with the radix tree lock as
> > is and just fix up the locking as I suggest and go for larger rewrite only
> > if we can demonstrate further performance wins.
>
> Sounds good.
>
> > WRT your second patch, if we go with the locking as I suggest, it is enough
> > to unmap the whole range after invalidate_inode_pages2() has cleared radix
> > tree entries (*) which will be much cheaper (for large writes) than doing
> > unmapping entry by entry.
>
> I'm still not convinced that it is safe to do the unmap in a separate step. I
> see your point about it being expensive to do a rmap walk to unmap each entry
> in __dax_invalidate_mapping_entry(), but I think we might need to because the
> unmap is part of the contract imposed by invalidate_inode_pages2_range() and
> invalidate_inode_pages2(). This exists in the header comment above each:
>
> * Any pages which are found to be mapped into pagetables are unmapped prior
> * to invalidation.
>
> If you look at the usage of invalidate_inode_pages2_range() in
> generic_file_direct_write() for example (which I realize we won't call for a
> DAX inode, but still), I think that it really does rely on the fact that
> invalidated pages are unmapped, right? If it didn't, and hole pages were
> mapped, the hole pages could remain mapped while a direct I/O write allocated
> blocks and then wrote real data.
>
> If we really want to unmap the entire range at once, maybe it would have to be
> done in invalidate_inode_pages2_range(), after the loop? My hesitation about
> this is that we'd be leaking yet more DAX special casing up into the
> mm/truncate.c code.
>
> Or am I missing something?
No, my thinking was to put the invalidation at the end of
invalidate_inode_pages2_range(). I agree it means more special-casing for
DAX in mm/truncate.c.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC 0/4] RFC - Coherent Device Memory (Not for inclusion)
From: Benjamin Herrenschmidt @ 2017-05-04 15:49 UTC (permalink / raw)
To: Michal Hocko, Balbir Singh
Cc: linux-mm, akpm, khandual, aneesh.kumar, paulmck, srikar, haren,
jglisse, mgorman, arbab, vbabka, cl
In-Reply-To: <20170504125250.GH31540@dhcp22.suse.cz>
On Thu, 2017-05-04 at 14:52 +0200, Michal Hocko wrote:
> But the direct reclaim would be effective only _after_ all other nodes
> are full.
>
> I thought that kswapd reclaim is a problem because the HW doesn't
> support aging properly but as the direct reclaim works then what is the
> actual problem?
Ageing isn't isn't completely broken. The ATS MMU supports
dirty/accessed just fine.
However the TLB invalidations are quite expensive with a GPU so too
much harvesting is detrimental, and the GPU tends to check pages out
using a special "read with intend to write" mode, which means it almost
always set the dirty bit if the page is writable to begin with.
Cheers,
Ben.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: RFC v2: post-init-read-only protection for data allocated dynamically
From: Laura Abbott @ 2017-05-04 16:49 UTC (permalink / raw)
To: Igor Stoppa, Michal Hocko
Cc: linux-mm, linux-kernel, kernel-hardening@lists.openwall.com
In-Reply-To: <9200d87d-33b6-2c70-0095-e974a30639fd@huawei.com>
[adding kernel-hardening since I think there would be interest]
On 05/03/2017 05:06 AM, Igor Stoppa wrote:
> Hello,
>
> please review my (longish) line of thoughts, below.
>
> I've restructured them so that they should be easier to follow.
>
>
> Observations
> ------------
>
> * it is currently possible, by using prefix "__read_only", to have the
> linker place a static variable into a special memory region, which will
> become write-protected at the end of the init phase.
>
> * the purpose is to write-protect data which is not expected to change,
> ever, after it has been initialized.
>
> * The mechanism used for locking down the memory region is to program
> the MMU to trap writes to said region. It is fairly efficient and
> HW-backed, so it doesn't introduce any major overhead, but the MMU deals
> only with pages or supersets of pages, hence the need to collect all the
> soon-to-be-read-only data - and only that - into the "special region".
> The "__read_only" modifier is the admission ticket.
>
> * the write-protecting feature helps supporting memory integrity in
> general and can also help spotting rogue writes, whatever their origin
> might be: uninitialized or expired pointers, wrong pointer arithmetic, etc.
>
>
>
> Problem
> -------
>
> The feature is available only for *static* data - it will not work with
> something like a linked list that is put together during init, for example.
>
>
>
> Wish
> ----
>
> My starting point are the policy DB of SE Linux and the LSM Hooks, but
> eventually I would like to extend the protection also to other
> subsystems, in a way that can be merged into mainline.
>
>
>
> Analysis
> --------
>
> * the solution I come up with has to be as little invasive as possible,
> at least for what concerns the various subsystems whose integrity I want
> to enhance.
>
> * In most, if not all, the cases that could be enhanced, the code will
> be calling kmalloc/vmalloc, indicating GFP_KERNEL as the desired type of
> memory.
>
> * I suspect/hope that the various maintainer won't object too much if my
> changes are limited to replacing GFP_KERNEL with some other macro, for
> example what I previously called GFP_LOCKABLE, provided I can ensure that:
>
> -1) no penalty is introduced, at least when the extra protection
> feature is not enabled, iow nobody has to suffer from my changes.
> This means that GFP_LOCKABLE should fall back to GFP_KERNEL, when
> it's not enabled.
>
> -2) when the extra protection feature is enabled, the code still
> works as expected, as long as the data identified for this
> enhancement is really unmodified after init.
>
> * In my quest for improved memory integrity, I will deal with very
> different memory size being allocated, so if I start writing my own
> memory allocator, starting from a page-aligned chunk of normal memory,
> at best I will end up with a replica of kmalloc, at worst with something
> buggy. Either way, it will be extremely harder to push other subsystems
> to use it.
> I probably wouldn't like it either, if I was a maintainer.
>
> * While I do not strictly need a new memory zone, memory zones are what
> kmalloc understands at the moment: AFAIK, it is not possible to tell
> kmalloc from which memory pool it should fish out the memory, other than
> having a reference to a memory zone.
> If it was possible to aim kmalloc at arbitrary memory pools, probably we
> would not be having this exchange right now. And probably there would
> not be so many other folks trying to have their memory zone of interest
> being merged. However I suspect this solution would be sub-optimal for
> the normal use cases, because there would be the extra overhead of
> passing the reference to the memory pool, instead of encoding it into
> bitfields, together with other information.
>
> * there are very slim chances (to be optimistic :) that I can get away
> with having my custom zone merged, because others are trying with
> similar proposals and they get rejected, so maybe I can have better luck
> if I propose something that can also work for others.
>
> * currently memory zones are mapped 1:1 to bits in crowded a bitmask,
> but not all these zones are really needed in a typical real system, some
> are kept for backward compatibility and supporting distros, which cannot
> know upfront the quirks of the HW they will be running on.
>
>
> Conclusions
> -----------
>
> * the solution that seems to be more likely to succeed is to remove the
> 1:1 mapping between optional zones and their respective bits.
>
> * the bits previously assigned to the optional zones would become
> available for mapping whatever zone a system integrator wants to support.
>
>
> Cons:
> There would be still a hard constraint on the maximum number of zones
> available simultaneously, so one will have to choose which of the
> optional zones to enable, and be ready to deal with own zone
> disappearing (ex: fall back to normal memory and give up some/all
> functionality)
>
> Pros:
> * No bit would go to waste: those who want to have own custom zone could
> make a better use of the allocated-but-not-necessary-to-them bits.
> * There would be a standard way for people to add non-standard zones.
> * It doesn't alter the hot paths that are critical to efficient memory
> handling.
>
> So it seems a win-win scenario, apart from the fact that I will probably
> have to reshuffle a certain amount of macros :-)
>
>
> P.S.
> There was an early advice of creating and using a custom-made memory
> allocator, I hope it's now clear why I don't think it's viable: it might
> work if I use it only for further code that I will write, but it really
> doesn't seem the best way to convince other subsystem maintainers to
> take in my changes, if I suggest them to give up the super optimized
> kmalloc (and friends) in favor of some homebrew allocator I wrote :-/
>
>
BPF takes the approach of calling set_memory_ro to mark regions as
read only. I'm certainly over simplifying but it sounds like this
is mostly a mechanism to have this happen mostly automatically.
Can you provide any more details about tradeoffs of the two approaches?
arm and arm64 have the added complexity of using larger
page sizes on the linear map so dynamic mapping/unmapping generally
doesn't work. arm64 supports DEBUG_PAGEALLOC by mapping with only
pages but this is generally only wanted as a debug mechanism.
I don't know if you've given this any thought at all.
Thanks,
Laura
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* OpenStack Application Users List
From: roxie bray @ 2017-05-04 16:53 UTC (permalink / raw)
To: laurence.ameur
[-- Attachment #1: Type: text/plain, Size: 843 bytes --]
Hi,
Hope everything is fine, and I trust this email finds you well!
We are one of the Leading B2B Email List Service Provider. I was wondering
if you would be interested in *OpenStack.*
Titles: C-Level, VP-Level, Managers, Directors and many more…
Information Fields: Name, Title, Email, Phone, & Company Details.
We also provide contact Users List for*- VMware, Amazon Web Services,
Oracle, and Salesforce *depending on the criteria you’re opting for.
We provide contacts across all Industries and from all departments/titles.
Please let me know your thoughts or pass on the message to the right person
in your company.
Thanks & Regards,
* Change* <https://profiles.google.com/?hl=en&tab=mX>
*Roxie bray*
To opt out please reply “Remove” in subject line
[-- Attachment #2: Type: text/html, Size: 5400 bytes --]
^ permalink raw reply
* Re: [PATCH v5 09/32] x86/mm: Provide general kernel support for memory encryption
From: Borislav Petkov @ 2017-05-04 17:01 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-arch, linux-efi, kvm, linux-doc, x86, kexec, linux-kernel,
kasan-dev, linux-mm, iommu, Rik van Riel,
Radim Krčmář, Toshimitsu Kani, Arnd Bergmann,
Jonathan Corbet, Matt Fleming, Michael S. Tsirkin, Joerg Roedel,
Konrad Rzeszutek Wilk, Paolo Bonzini, Larry Woodman,
Brijesh Singh, Ingo Molnar, Andy Lutomirski, H. Peter Anvin,
Andrey Ryabinin, Alexander Potapenko, Dave Young, Thomas Gleixner,
Dmitry Vyukov
In-Reply-To: <0b6e4055-8e07-3a71-3d52-12b0395c8f04@amd.com>
On Thu, May 04, 2017 at 09:34:09AM -0500, Tom Lendacky wrote:
> I masked it out here based on a previous comment from Dave Hansen:
>
> http://marc.info/?l=linux-kernel&m=148778719826905&w=2
>
> I could move the __sme_clr into the individual defines of:
Nah, it is fine as it is. I was just wondering...
Thanks.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: RFC v2: post-init-read-only protection for data allocated dynamically
From: Dave Hansen @ 2017-05-04 17:24 UTC (permalink / raw)
To: Michal Hocko, Igor Stoppa; +Cc: linux-mm, linux-kernel
In-Reply-To: <20170504140126.GJ31540@dhcp22.suse.cz>
On 05/04/2017 07:01 AM, Michal Hocko wrote:
> Just to make my proposal more clear. I suggest the following workflow
>
> cache = kmem_cache_create(foo, object_size, ..., SLAB_SEAL);
>
> obj = kmem_cache_alloc(cache, gfp_mask);
> init_obj(obj)
> [more allocations]
> kmem_cache_seal(cache);
>
> All slab pages belonging to the cache would get write protection. All
> new allocations from this cache would go to new slab pages. Later
> kmem_cache_seal will write protect only those new pages.
Igor, what sizes of objects are you after here, mostly?
I ask because slub, at least, doesn't work at all for objects
>PAGE_SIZE. It just punts those to the page allocator. But, you
_could_ still use vmalloc() for those.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC 0/4] RFC - Coherent Device Memory (Not for inclusion)
From: Dave Hansen @ 2017-05-04 17:33 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Michal Hocko, Balbir Singh
Cc: linux-mm, akpm, khandual, aneesh.kumar, paulmck, srikar, haren,
jglisse, mgorman, arbab, vbabka, cl
In-Reply-To: <1493912961.25766.379.camel@kernel.crashing.org>
On 05/04/2017 08:49 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2017-05-04 at 14:52 +0200, Michal Hocko wrote:
>> But the direct reclaim would be effective only _after_ all other nodes
>> are full.
>>
>> I thought that kswapd reclaim is a problem because the HW doesn't
>> support aging properly but as the direct reclaim works then what is the
>> actual problem?
>
> Ageing isn't isn't completely broken. The ATS MMU supports
> dirty/accessed just fine.
>
> However the TLB invalidations are quite expensive with a GPU so too
> much harvesting is detrimental, and the GPU tends to check pages out
> using a special "read with intend to write" mode, which means it almost
> always set the dirty bit if the page is writable to begin with.
Why do you have to invalidate the TLB? Does the GPU have a TLB so large
that it can keep thing in the TLB for super-long periods of time?
We don't flush the TLB on clearing Accessed on x86 normally.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH] mm, sparsemem: break out of loops early
From: Dave Hansen @ 2017-05-04 17:44 UTC (permalink / raw)
To: linux-kernel; +Cc: akpm, linux-mm, Dave Hansen, kirill.shutemov
From: Dave Hansen <dave.hansen@linux.intel.com>
There are a number of times that we loop over NR_MEM_SECTIONS,
looking for section_present() on each section. But, when we have
very large physical address spaces (large MAX_PHYSMEM_BITS),
NR_MEM_SECTIONS becomes very large, making the loops quite long.
With MAX_PHYSMEM_BITS=46 and a section size of 128MB, the current
loops are 512k iterations, which we barely notice on modern
hardware. But, raising MAX_PHYSMEM_BITS higher (like we will see
on systems that support 5-level paging) makes this 64x longer and
we start to notice, especially on slower systems like simulators.
A 10-second delay for 512k iterations is annoying. But, a 640-
second delay is crippling.
This does not help if we have extremely sparse physical address
spaces, but those are quite rare. We expect that most of the
"slow" systems where this matters will also be quite small and
non-sparse.
To fix this, we track the highest section we've ever encountered.
This lets us know when we will *never* see another
section_present(), and lets us break out of the loops earlier.
Doing the whole for_each_present_section_nr() macro is probably
overkill, but it will ensure that any future loop iterations that
we grow are more likely to be correct.
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
b/drivers/base/memory.c | 4 +++
b/include/linux/mmzone.h | 2 +
b/mm/sparse.c | 60 ++++++++++++++++++++++++++++++++++++-----------
3 files changed, 52 insertions(+), 14 deletions(-)
diff -puN mm/sparse.c~highest-section mm/sparse.c
--- a/mm/sparse.c~highest-section 2017-05-03 14:50:10.413117097 -0700
+++ b/mm/sparse.c 2017-05-04 10:43:36.725960272 -0700
@@ -168,6 +168,44 @@ void __meminit mminit_validate_memmodel_
}
}
+/*
+ * There are a number of times that we loop over NR_MEM_SECTIONS,
+ * looking for section_present() on each. But, when we have very
+ * large physical address spaces, NR_MEM_SECTIONS can also be
+ * very large which makes the loops quite long.
+ *
+ * Keeping track of this gives us an easy way to break out of
+ * those loops early.
+ */
+int __highest_present_section_nr;
+static void section_mark_present(struct mem_section *ms)
+{
+ int section_nr = __section_nr(ms);
+
+ if (section_nr > __highest_present_section_nr)
+ __highest_present_section_nr = section_nr;
+
+ ms->section_mem_map |= SECTION_MARKED_PRESENT;
+}
+
+static inline int next_present_section_nr(int section_nr)
+{
+ do {
+ section_nr++;
+ if (present_section_nr(section_nr))
+ return section_nr;
+ } while ((section_nr < NR_MEM_SECTIONS) &&
+ (section_nr <= __highest_present_section_nr));
+
+ return -1;
+}
+#define for_each_present_section_nr(start, section_nr) \
+ for (section_nr = next_present_section_nr(start-1); \
+ ((section_nr >= 0) && \
+ (section_nr < NR_MEM_SECTIONS) && \
+ (section_nr <= __highest_present_section_nr)); \
+ section_nr = next_present_section_nr(section_nr))
+
/* Record a memory area against a node. */
void __init memory_present(int nid, unsigned long start, unsigned long end)
{
@@ -183,9 +221,10 @@ void __init memory_present(int nid, unsi
set_section_nid(section, nid);
ms = __nr_to_section(section);
- if (!ms->section_mem_map)
- ms->section_mem_map = sparse_encode_early_nid(nid) |
- SECTION_MARKED_PRESENT;
+ if (!ms->section_mem_map) {
+ ms->section_mem_map = sparse_encode_early_nid(nid);
+ section_mark_present(ms);
+ }
}
}
@@ -479,23 +518,19 @@ static void __init alloc_usemap_and_memm
int nodeid_begin = 0;
unsigned long pnum_begin = 0;
- for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
+ for_each_present_section_nr(0, pnum) {
struct mem_section *ms;
- if (!present_section_nr(pnum))
- continue;
ms = __nr_to_section(pnum);
nodeid_begin = sparse_early_nid(ms);
pnum_begin = pnum;
break;
}
map_count = 1;
- for (pnum = pnum_begin + 1; pnum < NR_MEM_SECTIONS; pnum++) {
+ for_each_present_section_nr(pnum_begin + 1, pnum) {
struct mem_section *ms;
int nodeid;
- if (!present_section_nr(pnum))
- continue;
ms = __nr_to_section(pnum);
nodeid = sparse_early_nid(ms);
if (nodeid == nodeid_begin) {
@@ -564,10 +599,7 @@ void __init sparse_init(void)
(void *)map_map);
#endif
- for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
- if (!present_section_nr(pnum))
- continue;
-
+ for_each_present_section_nr(0, pnum) {
usemap = usemap_map[pnum];
if (!usemap)
continue;
@@ -725,7 +757,7 @@ int __meminit sparse_add_one_section(str
memset(memmap, 0, sizeof(struct page) * PAGES_PER_SECTION);
- ms->section_mem_map |= SECTION_MARKED_PRESENT;
+ section_mark_present(ms);
ret = sparse_init_one_section(ms, section_nr, memmap, usemap);
diff -puN include/linux/mmzone.h~highest-section include/linux/mmzone.h
--- a/include/linux/mmzone.h~highest-section 2017-05-03 14:50:10.416117232 -0700
+++ b/include/linux/mmzone.h 2017-05-03 14:50:10.437118180 -0700
@@ -1171,6 +1171,8 @@ static inline struct mem_section *__pfn_
return __nr_to_section(pfn_to_section_nr(pfn));
}
+extern int __highest_present_section_nr;
+
#ifndef CONFIG_HAVE_ARCH_PFN_VALID
static inline int pfn_valid(unsigned long pfn)
{
diff -puN drivers/base/memory.c~highest-section drivers/base/memory.c
--- a/drivers/base/memory.c~highest-section 2017-05-03 14:50:10.421117458 -0700
+++ b/drivers/base/memory.c 2017-05-03 14:50:10.439118270 -0700
@@ -820,6 +820,10 @@ int __init memory_dev_init(void)
*/
mutex_lock(&mem_sysfs_mutex);
for (i = 0; i < NR_MEM_SECTIONS; i += sections_per_block) {
+ /* Don't iterate over sections we know are !present: */
+ if (i > __highest_present_section_nr)
+ break;
+
err = add_memory_block(i);
if (!ret)
ret = err;
_
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v3 4/4] mm: Adaptive hash table scaling
From: Pasha Tatashin @ 2017-05-04 18:23 UTC (permalink / raw)
To: Michal Hocko, Andrew Morton; +Cc: linux-mm, sparclinux, linux-fsdevel, Al Viro
In-Reply-To: <20170426201126.GA32407@dhcp22.suse.cz>
Hi Michal,
I do not really want to impose any hard limit, because I do not know
what it should be.
The owners of the subsystems that use these large hash table should make
a call, and perhaps pass high_limit, if needed into
alloc_large_system_hash().
Previous growth rate was unacceptable, because in addition to allocating
large tables (which is acceptable if we take a total system memory
size), we also needed to zero that, and zeroing while we have only one
CPU available was significantly reducing the boot time.
Now, on 32T the hash table is 1G instead of 32G, so the call is 32 times
faster to finish. While it is not a good idea to waste memory, both 1G
and 32G is insignificant amount of memory compared to the total amount
of such 32T systems (0.09% and 0.003% accordingly).
Here is boot log on 32T system without this fix:
https://hastebin.com/muruzoveno.go
[ 769.622359] Dentry cache hash table entries: 2147483648 (order: 21,
17179869184 bytes)
[ 791.942136] Inode-cache hash table entries: 2147483648 (order: 21,
17179869184 bytes)
[ 810.810745] Mount-cache hash table entries: 67108864 (order: 16,
536870912 bytes)
[ 810.922322] Mountpoint-cache hash table entries: 67108864 (order: 16,
536870912 bytes)
[ 812.125398] ftrace: allocating 20650 entries in 41 pages
Total time 42.5s
With this fix (and some other unrelated for this interval fixes):
https://hastebin.com/buxucurawa.go
[ 12.621164] Dentry cache hash table entries: 134217728 (order: 17,
1073741824 bytes)
[ 12.869462] Inode-cache hash table entries: 67108864 (order: 16,
536870912 bytes)
[ 13.101963] Mount-cache hash table entries: 67108864 (order: 16,
536870912 bytes)
[ 13.331988] Mountpoint-cache hash table entries: 67108864 (order: 16,
536870912 bytes)
[ 13.364661] ftrace: allocating 20650 entries in 41 pages
Total time 0.76s.
So, it scales well for 32T systems, and will scale well for perceivable
future without adding a hard ceiling limit.
Pasha
On 04/26/2017 04:11 PM, Michal Hocko wrote:
> On Fri 03-03-17 15:32:47, Andrew Morton wrote:
>> On Thu, 2 Mar 2017 00:33:45 -0500 Pavel Tatashin <pasha.tatashin@oracle.com> wrote:
>>
>>> Allow hash tables to scale with memory but at slower pace, when HASH_ADAPT
>>> is provided every time memory quadruples the sizes of hash tables will only
>>> double instead of quadrupling as well. This algorithm starts working only
>>> when memory size reaches a certain point, currently set to 64G.
>>>
>>> This is example of dentry hash table size, before and after four various
>>> memory configurations:
>>>
>>> MEMORY SCALE HASH_SIZE
>>> old new old new
>>> 8G 13 13 8M 8M
>>> 16G 13 13 16M 16M
>>> 32G 13 13 32M 32M
>>> 64G 13 13 64M 64M
>>> 128G 13 14 128M 64M
>>> 256G 13 14 256M 128M
>>> 512G 13 15 512M 128M
>>> 1024G 13 15 1024M 256M
>>> 2048G 13 16 2048M 256M
>>> 4096G 13 16 4096M 512M
>>> 8192G 13 17 8192M 512M
>>> 16384G 13 17 16384M 1024M
>>> 32768G 13 18 32768M 1024M
>>> 65536G 13 18 65536M 2048M
>>
>> OK, but what are the runtime effects? Presumably some workloads will
>> slow down a bit. How much? How do we know that this is a worthwhile
>> tradeoff?
>>
>> If the effect of this change is "undetectable" then those hash tables
>> are simply too large, and additional tuning is needed, yes?
>
> I am playing with a 3TB and have hit the following
> [ 0.961309] Dentry cache hash table entries: 536870912 (order: 20, 4294967296 bytes)
> [ 2.300012] vmalloc: allocation failure, allocated 1383612416 of 2147487744 bytes
> [ 2.307473] swapper/0: page allocation failure: order:0, mode:0x2080020(GFP_ATOMIC)
> [ 2.315101] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 4.4.49-hotplug19-default #1
> [ 2.324017] Hardware name: Huawei 9008/IT91SMUB, BIOS BLXSV607 04/17/2017
> [ 2.330775] ffffffff8101aba5 ffffffff8130efa0 ffffffff81863f48 ffffffff81c03e40
> [ 2.338201] ffffffff8118c9a2 02080020fff00300 ffffffff81863f48 ffffffff81c03de0
> [ 2.345628] 0000000000000018 ffffffff81c03e50 ffffffff81c03df8 ffffffff811d28e6
> [ 2.353056] Call Trace:
> [ 2.355507] [<ffffffff81019a99>] dump_trace+0x59/0x310
> [ 2.360710] [<ffffffff81019e3a>] show_stack_log_lvl+0xea/0x170
> [ 2.366605] [<ffffffff8101abc1>] show_stack+0x21/0x40
> [ 2.371723] [<ffffffff8130efa0>] dump_stack+0x5c/0x7c
> [ 2.376842] [<ffffffff8118c9a2>] warn_alloc_failed+0xe2/0x150
> [ 2.382655] [<ffffffff811c2a10>] __vmalloc_node_range+0x240/0x280
> [ 2.388814] [<ffffffff811c2a97>] __vmalloc+0x47/0x50
> [ 2.393851] [<ffffffff81da02ae>] alloc_large_system_hash+0x189/0x25d
> [ 2.400264] [<ffffffff81da7625>] inode_init+0x74/0xa3
> [ 2.405381] [<ffffffff81da7483>] vfs_caches_init+0x59/0xe1
> [ 2.410930] [<ffffffff81d6f070>] start_kernel+0x474/0x4d0
> [ 2.416392] [<ffffffff81d6e719>] x86_64_start_kernel+0x147/0x156
>
> Allocating 4G for a hash table is just ridiculous. 512MB which this
> patch should give looks much reasonable, although I would argue it is
> still a _lot_.
> I cannot say I would be really happy about the chosen approach,
> though. Why HASH_ADAPT is not implicit? Which hash table would need
> gigabytes of memory and still benefit from it? Even if there is such an
> example then it should use the explicit high_limit. I do not like this
> opt-in because it is just too easy to miss that and hit the same issue
> again. And in fact only few users of alloc_large_system_hash are using
> the flag. E.g. why {dcache,inode}_init_early do not have the flag? I
> am pretty sure that having a physically contiguous hash table would be
> better over vmalloc from the TLB point of view.
>
> mount_hashtable resp. mountpoint_hashtable are another example. Other
> users just have a reasonable max value. So can we do the following
> on top of your commit? I think that we should rethink the scaling as
> well but I do not have a good answer for the maximum size so let's just
> start with a more reasonable API first.
> ---
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 808ea99062c2..363502faa328 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -3585,7 +3585,7 @@ static void __init dcache_init(void)
> sizeof(struct hlist_bl_head),
> dhash_entries,
> 13,
> - HASH_ZERO | HASH_ADAPT,
> + HASH_ZERO,
> &d_hash_shift,
> &d_hash_mask,
> 0,
> diff --git a/fs/inode.c b/fs/inode.c
> index a9caf53df446..b3c0731ec1fe 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1950,7 +1950,7 @@ void __init inode_init(void)
> sizeof(struct hlist_head),
> ihash_entries,
> 14,
> - HASH_ZERO | HASH_ADAPT,
> + HASH_ZERO,
> &i_hash_shift,
> &i_hash_mask,
> 0,
> diff --git a/include/linux/bootmem.h b/include/linux/bootmem.h
> index dbaf312b3317..e223d91b6439 100644
> --- a/include/linux/bootmem.h
> +++ b/include/linux/bootmem.h
> @@ -359,7 +359,6 @@ extern void *alloc_large_system_hash(const char *tablename,
> #define HASH_SMALL 0x00000002 /* sub-page allocation allowed, min
> * shift passed via *_hash_shift */
> #define HASH_ZERO 0x00000004 /* Zero allocated hash table */
> -#define HASH_ADAPT 0x00000008 /* Adaptive scale for large memory */
>
> /* Only NUMA needs hash distribution. 64bit NUMA architectures have
> * sufficient vmalloc space.
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index fa752de84eef..3bf60669d200 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7226,7 +7226,7 @@ void *__init alloc_large_system_hash(const char *tablename,
> if (PAGE_SHIFT < 20)
> numentries = round_up(numentries, (1<<20)/PAGE_SIZE);
>
> - if (flags & HASH_ADAPT) {
> + if (!high_limit) {
> unsigned long adapt;
>
> for (adapt = ADAPT_SCALE_NPAGES; adapt < numentries;
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v3 4/4] mm: Adaptive hash table scaling
From: Pasha Tatashin @ 2017-05-04 18:28 UTC (permalink / raw)
To: Michal Hocko, Andrew Morton; +Cc: linux-mm, sparclinux, linux-fsdevel, Al Viro
In-Reply-To: <40f72efa-3928-b3c6-acca-0740f1a15ba4@oracle.com>
BTW, I am OK with your patch on top of this "Adaptive hash table" patch,
but I do not know what high_limit should be from where HASH_ADAPT will
kick in. 128M sound reasonable to you?
Pasha
On 05/04/2017 02:23 PM, Pasha Tatashin wrote:
> Hi Michal,
>
> I do not really want to impose any hard limit, because I do not know
> what it should be.
>
> The owners of the subsystems that use these large hash table should make
> a call, and perhaps pass high_limit, if needed into
> alloc_large_system_hash().
>
> Previous growth rate was unacceptable, because in addition to allocating
> large tables (which is acceptable if we take a total system memory
> size), we also needed to zero that, and zeroing while we have only one
> CPU available was significantly reducing the boot time.
>
> Now, on 32T the hash table is 1G instead of 32G, so the call is 32 times
> faster to finish. While it is not a good idea to waste memory, both 1G
> and 32G is insignificant amount of memory compared to the total amount
> of such 32T systems (0.09% and 0.003% accordingly).
>
> Here is boot log on 32T system without this fix:
> https://hastebin.com/muruzoveno.go
>
> [ 769.622359] Dentry cache hash table entries: 2147483648 (order: 21,
> 17179869184 bytes)
> [ 791.942136] Inode-cache hash table entries: 2147483648 (order: 21,
> 17179869184 bytes)
> [ 810.810745] Mount-cache hash table entries: 67108864 (order: 16,
> 536870912 bytes)
> [ 810.922322] Mountpoint-cache hash table entries: 67108864 (order: 16,
> 536870912 bytes)
> [ 812.125398] ftrace: allocating 20650 entries in 41 pages
>
> Total time 42.5s
>
> With this fix (and some other unrelated for this interval fixes):
> https://hastebin.com/buxucurawa.go
>
> [ 12.621164] Dentry cache hash table entries: 134217728 (order: 17,
> 1073741824 bytes)
> [ 12.869462] Inode-cache hash table entries: 67108864 (order: 16,
> 536870912 bytes)
> [ 13.101963] Mount-cache hash table entries: 67108864 (order: 16,
> 536870912 bytes)
> [ 13.331988] Mountpoint-cache hash table entries: 67108864 (order: 16,
> 536870912 bytes)
> [ 13.364661] ftrace: allocating 20650 entries in 41 pages
>
> Total time 0.76s.
>
> So, it scales well for 32T systems, and will scale well for perceivable
> future without adding a hard ceiling limit.
>
> Pasha
>
> On 04/26/2017 04:11 PM, Michal Hocko wrote:
>> On Fri 03-03-17 15:32:47, Andrew Morton wrote:
>>> On Thu, 2 Mar 2017 00:33:45 -0500 Pavel Tatashin
>>> <pasha.tatashin@oracle.com> wrote:
>>>
>>>> Allow hash tables to scale with memory but at slower pace, when
>>>> HASH_ADAPT
>>>> is provided every time memory quadruples the sizes of hash tables
>>>> will only
>>>> double instead of quadrupling as well. This algorithm starts working
>>>> only
>>>> when memory size reaches a certain point, currently set to 64G.
>>>>
>>>> This is example of dentry hash table size, before and after four
>>>> various
>>>> memory configurations:
>>>>
>>>> MEMORY SCALE HASH_SIZE
>>>> old new old new
>>>> 8G 13 13 8M 8M
>>>> 16G 13 13 16M 16M
>>>> 32G 13 13 32M 32M
>>>> 64G 13 13 64M 64M
>>>> 128G 13 14 128M 64M
>>>> 256G 13 14 256M 128M
>>>> 512G 13 15 512M 128M
>>>> 1024G 13 15 1024M 256M
>>>> 2048G 13 16 2048M 256M
>>>> 4096G 13 16 4096M 512M
>>>> 8192G 13 17 8192M 512M
>>>> 16384G 13 17 16384M 1024M
>>>> 32768G 13 18 32768M 1024M
>>>> 65536G 13 18 65536M 2048M
>>>
>>> OK, but what are the runtime effects? Presumably some workloads will
>>> slow down a bit. How much? How do we know that this is a worthwhile
>>> tradeoff?
>>>
>>> If the effect of this change is "undetectable" then those hash tables
>>> are simply too large, and additional tuning is needed, yes?
>>
>> I am playing with a 3TB and have hit the following
>> [ 0.961309] Dentry cache hash table entries: 536870912 (order: 20,
>> 4294967296 bytes)
>> [ 2.300012] vmalloc: allocation failure, allocated 1383612416 of
>> 2147487744 bytes
>> [ 2.307473] swapper/0: page allocation failure: order:0,
>> mode:0x2080020(GFP_ATOMIC)
>> [ 2.315101] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G
>> W 4.4.49-hotplug19-default #1
>> [ 2.324017] Hardware name: Huawei 9008/IT91SMUB, BIOS BLXSV607
>> 04/17/2017
>> [ 2.330775] ffffffff8101aba5 ffffffff8130efa0 ffffffff81863f48
>> ffffffff81c03e40
>> [ 2.338201] ffffffff8118c9a2 02080020fff00300 ffffffff81863f48
>> ffffffff81c03de0
>> [ 2.345628] 0000000000000018 ffffffff81c03e50 ffffffff81c03df8
>> ffffffff811d28e6
>> [ 2.353056] Call Trace:
>> [ 2.355507] [<ffffffff81019a99>] dump_trace+0x59/0x310
>> [ 2.360710] [<ffffffff81019e3a>] show_stack_log_lvl+0xea/0x170
>> [ 2.366605] [<ffffffff8101abc1>] show_stack+0x21/0x40
>> [ 2.371723] [<ffffffff8130efa0>] dump_stack+0x5c/0x7c
>> [ 2.376842] [<ffffffff8118c9a2>] warn_alloc_failed+0xe2/0x150
>> [ 2.382655] [<ffffffff811c2a10>] __vmalloc_node_range+0x240/0x280
>> [ 2.388814] [<ffffffff811c2a97>] __vmalloc+0x47/0x50
>> [ 2.393851] [<ffffffff81da02ae>] alloc_large_system_hash+0x189/0x25d
>> [ 2.400264] [<ffffffff81da7625>] inode_init+0x74/0xa3
>> [ 2.405381] [<ffffffff81da7483>] vfs_caches_init+0x59/0xe1
>> [ 2.410930] [<ffffffff81d6f070>] start_kernel+0x474/0x4d0
>> [ 2.416392] [<ffffffff81d6e719>] x86_64_start_kernel+0x147/0x156
>>
>> Allocating 4G for a hash table is just ridiculous. 512MB which this
>> patch should give looks much reasonable, although I would argue it is
>> still a _lot_.
>> I cannot say I would be really happy about the chosen approach,
>> though. Why HASH_ADAPT is not implicit? Which hash table would need
>> gigabytes of memory and still benefit from it? Even if there is such an
>> example then it should use the explicit high_limit. I do not like this
>> opt-in because it is just too easy to miss that and hit the same issue
>> again. And in fact only few users of alloc_large_system_hash are using
>> the flag. E.g. why {dcache,inode}_init_early do not have the flag? I
>> am pretty sure that having a physically contiguous hash table would be
>> better over vmalloc from the TLB point of view.
>>
>> mount_hashtable resp. mountpoint_hashtable are another example. Other
>> users just have a reasonable max value. So can we do the following
>> on top of your commit? I think that we should rethink the scaling as
>> well but I do not have a good answer for the maximum size so let's just
>> start with a more reasonable API first.
>> ---
>> diff --git a/fs/dcache.c b/fs/dcache.c
>> index 808ea99062c2..363502faa328 100644
>> --- a/fs/dcache.c
>> +++ b/fs/dcache.c
>> @@ -3585,7 +3585,7 @@ static void __init dcache_init(void)
>> sizeof(struct hlist_bl_head),
>> dhash_entries,
>> 13,
>> - HASH_ZERO | HASH_ADAPT,
>> + HASH_ZERO,
>> &d_hash_shift,
>> &d_hash_mask,
>> 0,
>> diff --git a/fs/inode.c b/fs/inode.c
>> index a9caf53df446..b3c0731ec1fe 100644
>> --- a/fs/inode.c
>> +++ b/fs/inode.c
>> @@ -1950,7 +1950,7 @@ void __init inode_init(void)
>> sizeof(struct hlist_head),
>> ihash_entries,
>> 14,
>> - HASH_ZERO | HASH_ADAPT,
>> + HASH_ZERO,
>> &i_hash_shift,
>> &i_hash_mask,
>> 0,
>> diff --git a/include/linux/bootmem.h b/include/linux/bootmem.h
>> index dbaf312b3317..e223d91b6439 100644
>> --- a/include/linux/bootmem.h
>> +++ b/include/linux/bootmem.h
>> @@ -359,7 +359,6 @@ extern void *alloc_large_system_hash(const char
>> *tablename,
>> #define HASH_SMALL 0x00000002 /* sub-page allocation allowed, min
>> * shift passed via *_hash_shift */
>> #define HASH_ZERO 0x00000004 /* Zero allocated hash table */
>> -#define HASH_ADAPT 0x00000008 /* Adaptive scale for large
>> memory */
>> /* Only NUMA needs hash distribution. 64bit NUMA architectures have
>> * sufficient vmalloc space.
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index fa752de84eef..3bf60669d200 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -7226,7 +7226,7 @@ void *__init alloc_large_system_hash(const char
>> *tablename,
>> if (PAGE_SHIFT < 20)
>> numentries = round_up(numentries, (1<<20)/PAGE_SIZE);
>> - if (flags & HASH_ADAPT) {
>> + if (!high_limit) {
>> unsigned long adapt;
>> for (adapt = ADAPT_SCALE_NPAGES; adapt < numentries;
>>
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH v2 1/2] dax: prevent invalidation of mapped DAX entries
From: Ross Zwisler @ 2017-05-04 19:59 UTC (permalink / raw)
To: Andrew Morton, linux-kernel
Cc: Ross Zwisler, Alexander Viro, Alexey Kuznetsov, Andrey Ryabinin,
Anna Schumaker, Christoph Hellwig, Dan Williams, Darrick J. Wong,
Eric Van Hensbergen, Jan Kara, Jens Axboe, Johannes Weiner,
Konrad Rzeszutek Wilk, Latchesar Ionkov, linux-cifs,
linux-fsdevel, linux-mm, linux-nfs, linux-nvdimm, Matthew Wilcox,
Ron Minnich, samba-technical, Steve French, Trond Myklebust,
v9fs-developer
dax_invalidate_mapping_entry() currently removes DAX exceptional entries
only if they are clean and unlocked. This is done via:
invalidate_mapping_pages()
invalidate_exceptional_entry()
dax_invalidate_mapping_entry()
However, for page cache pages removed in invalidate_mapping_pages() there
is an additional criteria which is that the page must not be mapped. This
is noted in the comments above invalidate_mapping_pages() and is checked in
invalidate_inode_page().
For DAX entries this means that we can can end up in a situation where a
DAX exceptional entry, either a huge zero page or a regular DAX entry,
could end up mapped but without an associated radix tree entry. This is
inconsistent with the rest of the DAX code and with what happens in the
page cache case.
We aren't able to unmap the DAX exceptional entry because according to its
comments invalidate_mapping_pages() isn't allowed to block, and
unmap_mapping_range() takes a write lock on the mapping->i_mmap_rwsem.
We could potentially do an rmap walk to see if each of the entries actually
has any active mappings before we remove it, but this might end up being
very expensive and doesn't currently look to be worth it.
So, just remove dax_invalidate_mapping_entry() and leave the DAX entries in
the radix tree.
Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Fixes: c6dcf52c23d2 ("mm: Invalidate DAX radix tree entries only if appropriate")
Reported-by: Jan Kara <jack@suse.cz>
Reviewed-by: Jan Kara <jack@suse.cz>
Cc: <stable@vger.kernel.org> [4.10+]
---
fs/dax.c | 29 -----------------------------
include/linux/dax.h | 1 -
mm/truncate.c | 9 +++------
3 files changed, 3 insertions(+), 36 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index 85abd74..166504c 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -507,35 +507,6 @@ int dax_delete_mapping_entry(struct address_space *mapping, pgoff_t index)
}
/*
- * Invalidate exceptional DAX entry if easily possible. This handles DAX
- * entries for invalidate_inode_pages() so we evict the entry only if we can
- * do so without blocking.
- */
-int dax_invalidate_mapping_entry(struct address_space *mapping, pgoff_t index)
-{
- int ret = 0;
- void *entry, **slot;
- struct radix_tree_root *page_tree = &mapping->page_tree;
-
- spin_lock_irq(&mapping->tree_lock);
- entry = __radix_tree_lookup(page_tree, index, NULL, &slot);
- if (!entry || !radix_tree_exceptional_entry(entry) ||
- slot_locked(mapping, slot))
- goto out;
- if (radix_tree_tag_get(page_tree, index, PAGECACHE_TAG_DIRTY) ||
- radix_tree_tag_get(page_tree, index, PAGECACHE_TAG_TOWRITE))
- goto out;
- radix_tree_delete(page_tree, index);
- mapping->nrexceptional--;
- ret = 1;
-out:
- spin_unlock_irq(&mapping->tree_lock);
- if (ret)
- dax_wake_mapping_entry_waiter(mapping, index, entry, true);
- return ret;
-}
-
-/*
* Invalidate exceptional DAX entry if it is clean.
*/
int dax_invalidate_mapping_entry_sync(struct address_space *mapping,
diff --git a/include/linux/dax.h b/include/linux/dax.h
index d8a3dc0..f8e1833 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -41,7 +41,6 @@ ssize_t dax_iomap_rw(struct kiocb *iocb, struct iov_iter *iter,
int dax_iomap_fault(struct vm_fault *vmf, enum page_entry_size pe_size,
const struct iomap_ops *ops);
int dax_delete_mapping_entry(struct address_space *mapping, pgoff_t index);
-int dax_invalidate_mapping_entry(struct address_space *mapping, pgoff_t index);
int dax_invalidate_mapping_entry_sync(struct address_space *mapping,
pgoff_t index);
void dax_wake_mapping_entry_waiter(struct address_space *mapping,
diff --git a/mm/truncate.c b/mm/truncate.c
index 6263aff..c537184 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -67,17 +67,14 @@ static void truncate_exceptional_entry(struct address_space *mapping,
/*
* Invalidate exceptional entry if easily possible. This handles exceptional
- * entries for invalidate_inode_pages() so for DAX it evicts only unlocked and
- * clean entries.
+ * entries for invalidate_inode_pages().
*/
static int invalidate_exceptional_entry(struct address_space *mapping,
pgoff_t index, void *entry)
{
- /* Handled by shmem itself */
- if (shmem_mapping(mapping))
+ /* Handled by shmem itself, or for DAX we do nothing. */
+ if (shmem_mapping(mapping) || dax_mapping(mapping))
return 1;
- if (dax_mapping(mapping))
- return dax_invalidate_mapping_entry(mapping, index);
clear_shadow_entry(mapping, index, entry);
return 1;
}
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v2 2/2] dax: fix data corruption due to stale mmap reads
From: Ross Zwisler @ 2017-05-04 19:59 UTC (permalink / raw)
To: Andrew Morton, linux-kernel
Cc: Ross Zwisler, Alexander Viro, Alexey Kuznetsov, Andrey Ryabinin,
Anna Schumaker, Christoph Hellwig, Dan Williams, Darrick J. Wong,
Eric Van Hensbergen, Jan Kara, Jens Axboe, Johannes Weiner,
Konrad Rzeszutek Wilk, Latchesar Ionkov, linux-cifs,
linux-fsdevel, linux-mm, linux-nfs, linux-nvdimm, Matthew Wilcox,
Ron Minnich, samba-technical, Steve French, Trond Myklebust,
v9fs-developer
In-Reply-To: <20170504195910.11579-1-ross.zwisler@linux.intel.com>
Users of DAX can suffer data corruption from stale mmap reads via the
following sequence:
- open an mmap over a 2MiB hole
- read from a 2MiB hole, faulting in a 2MiB zero page
- write to the hole with write(3p). The write succeeds but we incorrectly
leave the 2MiB zero page mapping intact.
- via the mmap, read the data that was just written. Since the zero page
mapping is still intact we read back zeroes instead of the new data.
We fix this by unconditionally calling invalidate_inode_pages2_range() in
dax_iomap_actor() for new block allocations, and by enhancing
invalidate_inode_pages2_range() so that it properly unmaps the DAX entries
being removed from the radix tree.
This is based on an initial patch from Jan Kara.
I've written an fstest that triggers this error:
http://www.spinics.net/lists/linux-mm/msg126276.html
Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Fixes: c6dcf52c23d2 ("mm: Invalidate DAX radix tree entries only if appropriate")
Reported-by: Jan Kara <jack@suse.cz>
Cc: <stable@vger.kernel.org> [4.10+]
---
Changes since v1:
- Instead of unmapping each DAX entry individually in
__dax_invalidate_mapping_entry(), instead unmap the whole range at once
inside of invalidate_inode_pages2_range(). Each unmap requires an rmap
walk so this should be less expensive, plus now we don't have to drop
and re-acquire the mapping->tree_lock for each entry. (Jan)
These patches apply cleanly to v4.11 and have passed an xfstest run.
They also apply to v4.10.13 with a little help from git am's 3-way merger.
---
fs/dax.c | 8 ++++----
mm/truncate.c | 10 ++++++++++
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index 166504c..1f2c880 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -999,11 +999,11 @@ dax_iomap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
return -EIO;
/*
- * Write can allocate block for an area which has a hole page mapped
- * into page tables. We have to tear down these mappings so that data
- * written by write(2) is visible in mmap.
+ * Write can allocate block for an area which has a hole page or zero
+ * PMD entry in the radix tree. We have to tear down these mappings so
+ * that data written by write(2) is visible in mmap.
*/
- if ((iomap->flags & IOMAP_F_NEW) && inode->i_mapping->nrpages) {
+ if (iomap->flags & IOMAP_F_NEW) {
invalidate_inode_pages2_range(inode->i_mapping,
pos >> PAGE_SHIFT,
(end - 1) >> PAGE_SHIFT);
diff --git a/mm/truncate.c b/mm/truncate.c
index c537184..ad40316 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -683,6 +683,16 @@ int invalidate_inode_pages2_range(struct address_space *mapping,
cond_resched();
index++;
}
+
+ /*
+ * Ensure that any DAX exceptional entries that have been invalidated
+ * are also unmapped.
+ */
+ if (dax_mapping(mapping)) {
+ unmap_mapping_range(mapping, (loff_t)start << PAGE_SHIFT,
+ (loff_t)(1 + end - start) << PAGE_SHIFT, 0);
+ }
+
cleancache_invalidate_inode(mapping);
return ret;
}
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH] swap: add block io poll in swapin path
From: Shaohua Li @ 2017-05-04 20:42 UTC (permalink / raw)
To: linux-mm; +Cc: Andrew Morton, Kernel-team, Tim Chen, Huang Ying, Jens Axboe
For fast flash disk, async IO could introduce overhead because of
context switch. block-mq now supports IO poll, which improves
performance and latency a lot. swapin is a good place to use this
technique, because the task is waitting for the swapin page to continue
execution.
In my virtual machine, directly read 4k data from a NVMe with iopoll is
about 60% better than that without poll. With iopoll support in swapin
patch, my microbenchmark (a task does random memory write) is about 10%
~ 25% faster. CPU utilization increases a lot though, 2x and even 3x CPU
utilization. This will depend on disk speed though. While iopoll in
swapin isn't intended for all usage cases, it's a win for latency
sensistive workloads with high speed swap disk. block layer has knob to
control poll in runtime. If poll isn't enabled in block layer, there
should be no noticeable change in swapin.
The swapin readahead might read several pages in in the same time and
form a big IO request. Since the IO will take longer time, it doesn't
make sense to do poll, so the patch only does iopoll for single page
swapin.
Cc: Tim Chen <tim.c.chen@intel.com>
Cc: Huang Ying <ying.huang@intel.com>
Cc: Jens Axboe <axboe@fb.com>
Signed-off-by: Shaohua Li <shli@fb.com>
---
include/linux/swap.h | 5 +++--
mm/madvise.c | 4 ++--
mm/page_io.c | 20 ++++++++++++++++++--
mm/swap_state.c | 10 ++++++----
mm/swapfile.c | 2 +-
5 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/include/linux/swap.h b/include/linux/swap.h
index ba58824..c589e6c 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -331,7 +331,7 @@ extern void kswapd_stop(int nid);
#include <linux/blk_types.h> /* for bio_end_io_t */
/* linux/mm/page_io.c */
-extern int swap_readpage(struct page *);
+extern int swap_readpage(struct page *, bool do_poll);
extern int swap_writepage(struct page *page, struct writeback_control *wbc);
extern void end_swap_bio_write(struct bio *bio);
extern int __swap_writepage(struct page *page, struct writeback_control *wbc,
@@ -362,7 +362,8 @@ extern void free_page_and_swap_cache(struct page *);
extern void free_pages_and_swap_cache(struct page **, int);
extern struct page *lookup_swap_cache(swp_entry_t);
extern struct page *read_swap_cache_async(swp_entry_t, gfp_t,
- struct vm_area_struct *vma, unsigned long addr);
+ struct vm_area_struct *vma, unsigned long addr,
+ bool do_poll);
extern struct page *__read_swap_cache_async(swp_entry_t, gfp_t,
struct vm_area_struct *vma, unsigned long addr,
bool *new_page_allocated);
diff --git a/mm/madvise.c b/mm/madvise.c
index 25b78ee..8eda184 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -205,7 +205,7 @@ static int swapin_walk_pmd_entry(pmd_t *pmd, unsigned long start,
continue;
page = read_swap_cache_async(entry, GFP_HIGHUSER_MOVABLE,
- vma, index);
+ vma, index, false);
if (page)
put_page(page);
}
@@ -246,7 +246,7 @@ static void force_shm_swapin_readahead(struct vm_area_struct *vma,
}
swap = radix_to_swp_entry(page);
page = read_swap_cache_async(swap, GFP_HIGHUSER_MOVABLE,
- NULL, 0);
+ NULL, 0, false);
if (page)
put_page(page);
}
diff --git a/mm/page_io.c b/mm/page_io.c
index 23f6d0d..464cf16 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -117,6 +117,7 @@ static void swap_slot_free_notify(struct page *page)
static void end_swap_bio_read(struct bio *bio)
{
struct page *page = bio->bi_io_vec[0].bv_page;
+ struct task_struct *waiter = bio->bi_private;
if (bio->bi_error) {
SetPageError(page);
@@ -133,6 +134,7 @@ static void end_swap_bio_read(struct bio *bio)
out:
unlock_page(page);
bio_put(bio);
+ wake_up_process(waiter);
}
int generic_swapfile_activate(struct swap_info_struct *sis,
@@ -329,11 +331,13 @@ int __swap_writepage(struct page *page, struct writeback_control *wbc,
return ret;
}
-int swap_readpage(struct page *page)
+int swap_readpage(struct page *page, bool do_poll)
{
struct bio *bio;
int ret = 0;
struct swap_info_struct *sis = page_swap_info(page);
+ blk_qc_t qc;
+ struct block_device *bdev;
VM_BUG_ON_PAGE(!PageSwapCache(page), page);
VM_BUG_ON_PAGE(!PageLocked(page), page);
@@ -372,9 +376,21 @@ int swap_readpage(struct page *page)
ret = -ENOMEM;
goto out;
}
+ bdev = bio->bi_bdev;
+ bio->bi_private = current;
bio_set_op_attrs(bio, REQ_OP_READ, 0);
count_vm_event(PSWPIN);
- submit_bio(bio);
+ qc = submit_bio(bio);
+ while (do_poll) {
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ if (!PageLocked(page))
+ break;
+
+ if (!blk_mq_poll(bdev_get_queue(bdev), qc))
+ break;
+ }
+ __set_current_state(TASK_RUNNING);
+
out:
return ret;
}
diff --git a/mm/swap_state.c b/mm/swap_state.c
index 539b888..7c0a66c 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -404,14 +404,14 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
* the swap entry is no longer in use.
*/
struct page *read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
- struct vm_area_struct *vma, unsigned long addr)
+ struct vm_area_struct *vma, unsigned long addr, bool do_poll)
{
bool page_was_allocated;
struct page *retpage = __read_swap_cache_async(entry, gfp_mask,
vma, addr, &page_was_allocated);
if (page_was_allocated)
- swap_readpage(retpage);
+ swap_readpage(retpage, do_poll);
return retpage;
}
@@ -488,11 +488,13 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
unsigned long start_offset, end_offset;
unsigned long mask;
struct blk_plug plug;
+ bool do_poll = true;
mask = swapin_nr_pages(offset) - 1;
if (!mask)
goto skip;
+ do_poll = false;
/* Read a page_cluster sized and aligned cluster around offset. */
start_offset = offset & ~mask;
end_offset = offset | mask;
@@ -503,7 +505,7 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
for (offset = start_offset; offset <= end_offset ; offset++) {
/* Ok, do the async read-ahead now */
page = read_swap_cache_async(swp_entry(swp_type(entry), offset),
- gfp_mask, vma, addr);
+ gfp_mask, vma, addr, false);
if (!page)
continue;
if (offset != entry_offset)
@@ -514,7 +516,7 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
lru_add_drain(); /* Push any new pages onto the LRU now */
skip:
- return read_swap_cache_async(entry, gfp_mask, vma, addr);
+ return read_swap_cache_async(entry, gfp_mask, vma, addr, do_poll);
}
int init_swap_address_space(unsigned int type, unsigned long nr_pages)
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 4f6cba1..04516c1 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1719,7 +1719,7 @@ int try_to_unuse(unsigned int type, bool frontswap,
swap_map = &si->swap_map[i];
entry = swp_entry(type, i);
page = read_swap_cache_async(entry,
- GFP_HIGHUSER_MOVABLE, NULL, 0);
+ GFP_HIGHUSER_MOVABLE, NULL, 0, false);
if (!page) {
/*
* Either swap_duplicate() failed because entry
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* Re: [PATCH] swap: add block io poll in swapin path
From: Jens Axboe @ 2017-05-04 20:53 UTC (permalink / raw)
To: Shaohua Li, linux-mm; +Cc: Andrew Morton, Kernel-team, Tim Chen, Huang Ying
In-Reply-To: <7dd0349ba5d321af557d7a09e08610f2486ea29e.1493930299.git.shli@fb.com>
On 05/04/2017 02:42 PM, Shaohua Li wrote:
> For fast flash disk, async IO could introduce overhead because of
> context switch. block-mq now supports IO poll, which improves
> performance and latency a lot. swapin is a good place to use this
> technique, because the task is waitting for the swapin page to continue
> execution.
Nitfy!
> In my virtual machine, directly read 4k data from a NVMe with iopoll is
> about 60% better than that without poll. With iopoll support in swapin
> patch, my microbenchmark (a task does random memory write) is about 10%
> ~ 25% faster. CPU utilization increases a lot though, 2x and even 3x CPU
> utilization. This will depend on disk speed though. While iopoll in
> swapin isn't intended for all usage cases, it's a win for latency
> sensistive workloads with high speed swap disk. block layer has knob to
> control poll in runtime. If poll isn't enabled in block layer, there
> should be no noticeable change in swapin.
Did you try with hybrid polling enabled? We should be able to achieve
most of the latency win at much less CPU cost with that.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox