* Re: [PATCH v2] dma-mapping: Fix filename references
From: Andy Shevchenko @ 2019-08-13 14:42 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, Tony Luck, Fenghua Yu, Konrad Rzeszutek Wilk,
linux-doc, Christoph Hellwig, Bjorn Helgaas
In-Reply-To: <20190619141956.65696-1-andriy.shevchenko@linux.intel.com>
On Wed, Jun 19, 2019 at 05:19:55PM +0300, Andy Shevchenko wrote:
> After the commit cf65a0f6f6ff
>
> ("dma-mapping: move all DMA mapping code to kernel/dma")
>
> some of the files are referring to outdated information, i.e. old file names
> of DMA mapping sources.
>
> Fix it here.
>
> Note, the lines with "Glue code for..." have been removed completely.
Any comment on this?
>
> Cc: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> - address Bjorn's comments
> Documentation/x86/x86_64/boot-options.rst | 2 +-
> arch/ia64/kernel/setup.c | 2 +-
> arch/x86/kernel/pci-swiotlb.c | 1 -
> arch/x86/kernel/setup.c | 2 +-
> arch/x86/pci/sta2x11-fixup.c | 4 +---
> 5 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/x86/x86_64/boot-options.rst b/Documentation/x86/x86_64/boot-options.rst
> index 6a4285a3c7a4..2b98efb5ba7f 100644
> --- a/Documentation/x86/x86_64/boot-options.rst
> +++ b/Documentation/x86/x86_64/boot-options.rst
> @@ -230,7 +230,7 @@ IOMMU (input/output memory management unit)
> ===========================================
> Multiple x86-64 PCI-DMA mapping implementations exist, for example:
>
> - 1. <lib/dma-direct.c>: use no hardware/software IOMMU at all
> + 1. <kernel/dma/direct.c>: use no hardware/software IOMMU at all
> (e.g. because you have < 3 GB memory).
> Kernel boot message: "PCI-DMA: Disabling IOMMU"
>
> diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
> index c9cfa760cd57..ab8d25d3e358 100644
> --- a/arch/ia64/kernel/setup.c
> +++ b/arch/ia64/kernel/setup.c
> @@ -256,7 +256,7 @@ __initcall(register_memory);
> * This function checks if the reserved crashkernel is allowed on the specific
> * IA64 machine flavour. Machines without an IO TLB use swiotlb and require
> * some memory below 4 GB (i.e. in 32 bit area), see the implementation of
> - * lib/swiotlb.c. The hpzx1 architecture has an IO TLB but cannot use that
> + * kernel/dma/swiotlb.c. The hpzx1 architecture has an IO TLB but cannot use that
> * in kdump case. See the comment in sba_init() in sba_iommu.c.
> *
> * So, the only machvec that really supports loading the kdump kernel
> diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
> index 5f5302028a9a..c2cfa5e7c152 100644
> --- a/arch/x86/kernel/pci-swiotlb.c
> +++ b/arch/x86/kernel/pci-swiotlb.c
> @@ -1,5 +1,4 @@
> // SPDX-License-Identifier: GPL-2.0
> -/* Glue code to lib/swiotlb.c */
>
> #include <linux/pci.h>
> #include <linux/cache.h>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 08a5f4a131f5..8655bf374893 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -477,7 +477,7 @@ static int __init reserve_crashkernel_low(void)
> ret = parse_crashkernel_low(boot_command_line, total_low_mem, &low_size, &base);
> if (ret) {
> /*
> - * two parts from lib/swiotlb.c:
> + * two parts from kernel/dma/swiotlb.c:
> * -swiotlb size: user-specified with swiotlb= or default.
> *
> * -swiotlb overflow buffer: now hardcoded to 32k. We round it
> diff --git a/arch/x86/pci/sta2x11-fixup.c b/arch/x86/pci/sta2x11-fixup.c
> index 97bbc12dd6b2..6269a175385d 100644
> --- a/arch/x86/pci/sta2x11-fixup.c
> +++ b/arch/x86/pci/sta2x11-fixup.c
> @@ -1,8 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0-only
> /*
> - * arch/x86/pci/sta2x11-fixup.c
> - * glue code for lib/swiotlb.c and DMA translation between STA2x11
> - * AMBA memory mapping and the X86 memory mapping
> + * DMA translation between STA2x11 AMBA memory mapping and the x86 memory mapping
> *
> * ST Microelectronics ConneXt (STA2X11/STA2X10)
> *
> --
> 2.20.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v5 1/6] mm/page_idle: Add per-pid idle page tracking using virtual index
From: Joel Fernandes @ 2019-08-13 14:45 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, linux-kernel, Alexey Dobriyan, Borislav Petkov,
Brendan Gregg, Catalin Marinas, Christian Hansen, dancol, fmayer,
H. Peter Anvin, Ingo Molnar, Jonathan Corbet, Kees Cook,
kernel-team, linux-api, linux-doc, linux-fsdevel, linux-mm,
Mike Rapoport, minchan, namhyung, paulmck, Robin Murphy,
Roman Gushchin, Stephen Rothwell, surenb, Thomas Gleixner, tkjos,
Vladimir Davydov, Vlastimil Babka, Will Deacon
In-Reply-To: <20190813141432.GL17933@dhcp22.suse.cz>
On Tue, Aug 13, 2019 at 04:14:32PM +0200, Michal Hocko wrote:
[snip]
> > > If the API is flawed then this is likely going
> > > to kick us later and will be hard to fix. I am still not convinced about
> > > the swap part of the thing TBH.
> >
> > Ok, then let us discuss it. As I mentioned before, without this we lose the
> > access information due to MADVISE or swapping. Minchan and Konstantin both
> > suggested it that's why I also added it (other than me also realizing that it
> > is neeed).
>
> I have described my concerns about the general idle bit behavior after
> unmapping pointing to discrepancy with !anon pages. And I believe those
> haven't been addressed yet.
You are referring to this post right?
https://lkml.org/lkml/2019/8/6/637
Specifically your question was:
How are you going to handle situation when the page is unmapped and refaulted again (e.g. a normal reclaim of a pagecache)?
Currently I don't know how to implement that. Would it work if I stored the
page-idle bit information in the pte of the file page (after the page is
unmapped by reclaim?).
Also, this could be a future extension - the Android heap profiler does not
need it right now. I know that's not a good argument but it is useful to say
that it doesn't affect a real world usecase.. the swap issue on the other
hand, is a real usecase. Since the profiler should not get affected by
swapping or MADVISE_COLD hints.
> Besides that I am still not seeing any
> description of the usecase that would suffer from the lack of the
> functionality in changelogs.
You are talking about the swap usecase? The usecase is well layed out in v5
2/6. Did you see it? https://lore.kernel.org/patchwork/patch/1112283/
thanks,
- Joel
^ permalink raw reply
* Re: [PATCH v2] dma-mapping: Fix filename references
From: Christoph Hellwig @ 2019-08-13 14:45 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, Tony Luck, Fenghua Yu, Konrad Rzeszutek Wilk,
linux-doc, Christoph Hellwig, Bjorn Helgaas
In-Reply-To: <20190813144222.GF30120@smile.fi.intel.com>
On Tue, Aug 13, 2019 at 05:42:22PM +0300, Andy Shevchenko wrote:
> On Wed, Jun 19, 2019 at 05:19:55PM +0300, Andy Shevchenko wrote:
> > After the commit cf65a0f6f6ff
> >
> > ("dma-mapping: move all DMA mapping code to kernel/dma")
> >
> > some of the files are referring to outdated information, i.e. old file names
> > of DMA mapping sources.
> >
> > Fix it here.
> >
> > Note, the lines with "Glue code for..." have been removed completely.
>
> Any comment on this?
Fine with me, and I also agree with the glue code comment.
^ permalink raw reply
* Re: [PATCH v5 1/6] mm/page_idle: Add per-pid idle page tracking using virtual index
From: Michal Hocko @ 2019-08-13 14:57 UTC (permalink / raw)
To: Joel Fernandes
Cc: Andrew Morton, linux-kernel, Alexey Dobriyan, Borislav Petkov,
Brendan Gregg, Catalin Marinas, Christian Hansen, dancol, fmayer,
H. Peter Anvin, Ingo Molnar, Jonathan Corbet, Kees Cook,
kernel-team, linux-api, linux-doc, linux-fsdevel, linux-mm,
Mike Rapoport, minchan, namhyung, paulmck, Robin Murphy,
Roman Gushchin, Stephen Rothwell, surenb, Thomas Gleixner, tkjos,
Vladimir Davydov, Vlastimil Babka, Will Deacon
In-Reply-To: <20190813144517.GE258732@google.com>
On Tue 13-08-19 10:45:17, Joel Fernandes wrote:
> On Tue, Aug 13, 2019 at 04:14:32PM +0200, Michal Hocko wrote:
> [snip]
> > > > If the API is flawed then this is likely going
> > > > to kick us later and will be hard to fix. I am still not convinced about
> > > > the swap part of the thing TBH.
> > >
> > > Ok, then let us discuss it. As I mentioned before, without this we lose the
> > > access information due to MADVISE or swapping. Minchan and Konstantin both
> > > suggested it that's why I also added it (other than me also realizing that it
> > > is neeed).
> >
> > I have described my concerns about the general idle bit behavior after
> > unmapping pointing to discrepancy with !anon pages. And I believe those
> > haven't been addressed yet.
>
> You are referring to this post right?
> https://lkml.org/lkml/2019/8/6/637
>
> Specifically your question was:
> How are you going to handle situation when the page is unmapped and refaulted again (e.g. a normal reclaim of a pagecache)?
>
> Currently I don't know how to implement that. Would it work if I stored the
> page-idle bit information in the pte of the file page (after the page is
> unmapped by reclaim?).
It would work as long as we keep page tables around after unmap. As they
are easily reconstructable this is a good candidate for reclaim as well.
> Also, this could be a future extension - the Android heap profiler does not
> need it right now. I know that's not a good argument but it is useful to say
> that it doesn't affect a real world usecase.. the swap issue on the other
> hand, is a real usecase. Since the profiler should not get affected by
> swapping or MADVISE_COLD hints.
>
> > Besides that I am still not seeing any
> > description of the usecase that would suffer from the lack of the
> > functionality in changelogs.
>
> You are talking about the swap usecase? The usecase is well layed out in v5
> 2/6. Did you see it? https://lore.kernel.org/patchwork/patch/1112283/
For some reason I've missed it. I will coment on that.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v5 2/6] mm/page_idle: Add support for handling swapped PG_Idle pages
From: Michal Hocko @ 2019-08-13 15:04 UTC (permalink / raw)
To: Joel Fernandes (Google)
Cc: linux-kernel, Minchan Kim, Alexey Dobriyan, Andrew Morton,
Borislav Petkov, Brendan Gregg, Catalin Marinas, Christian Hansen,
dancol, fmayer, H. Peter Anvin, Ingo Molnar, joelaf,
Jonathan Corbet, Kees Cook, kernel-team, linux-api, linux-doc,
linux-fsdevel, linux-mm, Mike Rapoport, namhyung, paulmck,
Robin Murphy, Roman Gushchin, Stephen Rothwell, surenb,
Thomas Gleixner, tkjos, Vladimir Davydov, Vlastimil Babka,
Will Deacon
In-Reply-To: <20190807171559.182301-2-joel@joelfernandes.org>
On Wed 07-08-19 13:15:55, Joel Fernandes (Google) wrote:
> Idle page tracking currently does not work well in the following
> scenario:
> 1. mark page-A idle which was present at that time.
> 2. run workload
> 3. page-A is not touched by workload
> 4. *sudden* memory pressure happen so finally page A is finally swapped out
> 5. now see the page A - it appears as if it was accessed (pte unmapped
> so idle bit not set in output) - but it's incorrect.
>
> To fix this, we store the idle information into a new idle bit of the
> swap PTE during swapping of anonymous pages.
>
> Also in the future, madvise extensions will allow a system process
> manager (like Android's ActivityManager) to swap pages out of a process
> that it knows will be cold. To an external process like a heap profiler
> that is doing idle tracking on another process, this procedure will
> interfere with the idle page tracking similar to the above steps.
This could be solved by checking the !present/swapped out pages
right? Whoever decided to put the page out to the swap just made it
idle effectively. So the monitor can make some educated guess for
tracking. If that is fundamentally not possible then please describe
why.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v5 1/6] mm/page_idle: Add per-pid idle page tracking using virtual index
From: Jann Horn @ 2019-08-13 15:19 UTC (permalink / raw)
To: Joel Fernandes, Daniel Gruss
Cc: Michal Hocko, kernel list, Alexey Dobriyan, Andrew Morton,
Borislav Petkov, Brendan Gregg, Catalin Marinas, Christian Hansen,
Daniel Colascione, fmayer, H. Peter Anvin, Ingo Molnar,
Jonathan Corbet, Kees Cook, kernel-team, Linux API, linux-doc,
linux-fsdevel, Linux-MM, Mike Rapoport, Minchan Kim, namhyung,
Paul E. McKenney, Robin Murphy, Roman Gushchin, Stephen Rothwell,
Suren Baghdasaryan, Thomas Gleixner, Todd Kjos, Vladimir Davydov,
Vlastimil Babka, Will Deacon
In-Reply-To: <20190813142527.GD258732@google.com>
On Tue, Aug 13, 2019 at 4:25 PM Joel Fernandes <joel@joelfernandes.org> wrote:
> On Tue, Aug 13, 2019 at 12:08:56PM +0200, Michal Hocko wrote:
> > On Mon 12-08-19 20:14:38, Jann Horn wrote:
> > > On Wed, Aug 7, 2019 at 7:16 PM Joel Fernandes (Google)
> > > <joel@joelfernandes.org> wrote:
> > > > The page_idle tracking feature currently requires looking up the pagemap
> > > > for a process followed by interacting with /sys/kernel/mm/page_idle.
> > > > Looking up PFN from pagemap in Android devices is not supported by
> > > > unprivileged process and requires SYS_ADMIN and gives 0 for the PFN.
> > > >
> > > > This patch adds support to directly interact with page_idle tracking at
> > > > the PID level by introducing a /proc/<pid>/page_idle file. It follows
> > > > the exact same semantics as the global /sys/kernel/mm/page_idle, but now
> > > > looking up PFN through pagemap is not needed since the interface uses
> > > > virtual frame numbers, and at the same time also does not require
> > > > SYS_ADMIN.
> > > >
> > > > In Android, we are using this for the heap profiler (heapprofd) which
> > > > profiles and pin points code paths which allocates and leaves memory
> > > > idle for long periods of time. This method solves the security issue
> > > > with userspace learning the PFN, and while at it is also shown to yield
> > > > better results than the pagemap lookup, the theory being that the window
> > > > where the address space can change is reduced by eliminating the
> > > > intermediate pagemap look up stage. In virtual address indexing, the
> > > > process's mmap_sem is held for the duration of the access.
> > >
> > > What happens when you use this interface on shared pages, like memory
> > > inherited from the zygote, library file mappings and so on? If two
> > > profilers ran concurrently for two different processes that both map
> > > the same libraries, would they end up messing up each other's data?
> >
> > Yup PageIdle state is shared. That is the page_idle semantic even now
> > IIRC.
>
> Yes, that's right. This patch doesn't change that semantic. Idle page
> tracking at the core is a global procedure which is based on pages that can
> be shared.
>
> One of the usecases of the heap profiler is to enable profiling of pages that
> are shared between zygote and any processes that are forked. In this case,
> I am told by our team working on the heap profiler, that the monitoring of
> shared pages will help.
>
> > > Can this be used to observe which library pages other processes are
> > > accessing, even if you don't have access to those processes, as long
> > > as you can map the same libraries? I realize that there are already a
> > > bunch of ways to do that with side channels and such; but if you're
> > > adding an interface that allows this by design, it seems to me like
> > > something that should be gated behind some sort of privilege check.
> >
> > Hmm, you need to be priviledged to get the pfn now and without that you
> > cannot get to any page so the new interface is weakening the rules.
> > Maybe we should limit setting the idle state to processes with the write
> > status. Or do you think that even observing idle status is useful for
> > practical side channel attacks? If yes, is that a problem of the
> > profiler which does potentially dangerous things?
>
> The heap profiler is currently unprivileged. Would it help the concern Jann
> raised, if the new interface was limited to only anonymous private/shared
> pages and not to file pages? Or, is this even a real concern?
+Daniel Gruss in case he wants to provide some more detail; he has
been involved in a lot of the public research around this topic.
It is a bit of a concern when code that wasn't hardened as rigorously
as cryptographic library code operates on secret values.
A paper was published this year that abused mincore() in combination
with tricks for flushing the page cache to obtain information about
when shared read-only memory was accessed:
<https://arxiv.org/pdf/1901.01161.pdf>. In response to that, the
semantics of mincore() were changed to prevent that information from
leaking (see commit 134fca9063ad4851de767d1768180e5dede9a881).
On the other hand, an attacker could also use things like cache timing
attacks instead of abusing operating system features; that is more
hardware-specific, but it has a higher spatial granularity (typically
64 bytes instead of 4096 bytes). Timing-granularity-wise, I'm not sure
whether the proposed interface would be more or less bad than existing
cache side-channels on common architectures. There are papers that
demonstrate things like being able to distinguish some classes of
keyboard keys from others on an Android phone:
<https://www.usenix.org/system/files/conference/usenixsecurity16/sec16_paper_lipp.pdf>
I don't think limiting it to anonymous pages is necessarily enough to
completely solve this; in a normal Linux environment, it might be good
enough, but on Android, I'm worried about the CoW private memory from
the zygote.
^ permalink raw reply
* Re: [PATCH v2] driver/core: Fix build error when SRCU and lockdep disabled
From: Greg Kroah-Hartman @ 2019-08-13 15:25 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, kernel-team, kbuild test robot, Josh Triplett,
Lai Jiangshan, linux-doc, Mathieu Desnoyers, Paul E. McKenney,
Rafael J. Wysocki, rcu, Steven Rostedt
In-Reply-To: <20190813134014.GB258732@google.com>
On Tue, Aug 13, 2019 at 09:40:14AM -0400, Joel Fernandes wrote:
> On Tue, Aug 13, 2019 at 09:39:05AM -0400, Joel Fernandes wrote:
> [snip]
> > > > drivers/base/core.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > > > index 32cf83d1c744..c22271577c84 100644
> > > > --- a/drivers/base/core.c
> > > > +++ b/drivers/base/core.c
> > > > @@ -97,10 +97,12 @@ void device_links_read_unlock(int not_used)
> > > > up_read(&device_links_lock);
> > > > }
> > > >
> > > > +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> > > > int device_links_read_lock_held(void)
> > > > {
> > > > - return lock_is_held(&device_links_lock);
> > > > + return lock_is_held(&(device_links_lock.dep_map));
> > > > }
> > > > +#endif
> > >
> > > I don't know what the original code looks like here, but I'm guessing
> > > that some .h file will need to be fixed up as you are just preventing
> > > this function from ever being present without that option enabled?
> >
> > No, it doesn't. I already thought about that and it is not an issue. I know
> > this may be confusing because the patch I am fixing is not yet in mainline
> > but in -rcu dev branch, however I did CC you on that RCU patch before but
> > understandably it is not in the series so it was harder to review.
> >
> > Let me explain, the lock checking facility that this patch uses is here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/commit/?h=dev&id=28875945ba98d1b47a8a706812b6494d165bb0a0
> >
> > If you see, the CONFIG_PROVE_RCU_LIST defines an alternate __list_check_rcu()
> > which is just a NOOP if CONFIG_PROVE_RCU_LIST=n.
> >
> > CONFIG_PROVE_RCU_LIST depends on CONFIG_PROVE_RCU which is def_bool on
> > CONFIG_PROVE_LOCKING which selects CONFIG_DEBUG_LOCK_ALLOC.
> >
> > So there cannot be a scenario where CONFIG_PROVE_RCU_LIST is enabled but
> > CONFIG_DEBUG_LOCK_ALLOC is disabled.
> >
> > To verify this, one could clone the RCU tree's dev branch and do this:
> >
> > Initially PROVE_RCU_LIST is not in config:
> >
> > joelaf@joelaf:~/repo/linux-master$ grep -i prove_rcu .config
> >
> > Neither is DEBUG_LOCK_ALLOC:
> >
> > joelaf@joelaf:~/repo/linux-master$ grep -i debug_lock .config
> > # CONFIG_DEBUG_LOCK_ALLOC is not set
> >
> > Enable all configs:
> > joelaf@joelaf:~/repo/linux-master$ ./scripts/config -e CONFIG_RCU_EXPERT
> > joelaf@joelaf:~/repo/linux-master$ ./scripts/config -e CONFIG_PROVE_LOCKING
> > joelaf@joelaf:~/repo/linux-master$ ./scripts/config -e CONFIG_PROVE_RCU
> > joelaf@joelaf:~/repo/linux-master$ ./scripts/config -e CONFIG_PROVE_RCU_LIST
> > joelaf@joelaf:~/repo/linux-master$ make olddefconfig
> >
> > DEBUG_LOCK_ALLOC shows up:
> >
> > joelaf@joelaf:~/repo/linux-master$ grep -i debug_lock_all .config
> > CONFIG_DEBUG_LOCK_ALLOC=y
> >
> > So does PROVE_RCU options:
> > joelaf@joelaf:~/repo/linux-master$ grep -i prove_rcu .config
> > CONFIG_PROVE_RCU=y
> > CONFIG_PROVE_RCU_LIST=y
> >
> > Now, force disable DEBUG_LOCK_ALLOC:
> >
> > joelaf@joelaf:~/repo/linux-master$ ./scripts/config -d CONFIG_DEBUG_LOCK_ALLOC
> >
> > joelaf@joelaf:~/repo/linux-master$ make olddefconfig
> >
> > Options are still enabled:
> >
> > joelaf@joelaf:~/repo/linux-master$ grep -i debug_lock .config
> > CONFIG_DEBUG_LOCK_ALLOC=y
> > joelaf@joelaf:~/repo/linux-master$ grep -i prove_rcu .config
> > CONFIG_PROVE_RCU=y
> > CONFIG_PROVE_RCU_LIST=y
>
> Also, appreciate if you could Ack the fix ;-) (assuming the newlines in
> commit message are fixed).
I don't know enough here to ack it, given that I can't even remember the
original patch...
If you think it's ok, that's fine with me, you can deal with the fallout
if it fails later :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v5 1/6] mm/page_idle: Add per-pid idle page tracking using virtual index
From: Jann Horn @ 2019-08-13 15:29 UTC (permalink / raw)
To: Michal Hocko, Daniel Gruss, Joel Fernandes (Google)
Cc: kernel list, Alexey Dobriyan, Andrew Morton, Borislav Petkov,
Brendan Gregg, Catalin Marinas, Christian Hansen,
Daniel Colascione, fmayer, H. Peter Anvin, Ingo Molnar,
Joel Fernandes, Jonathan Corbet, Kees Cook, kernel-team,
Linux API, linux-doc, linux-fsdevel, Linux-MM, Mike Rapoport,
Minchan Kim, namhyung, Paul E. McKenney, Robin Murphy,
Roman Gushchin, Stephen Rothwell, Suren Baghdasaryan,
Thomas Gleixner, Todd Kjos, Vladimir Davydov, Vlastimil Babka,
Will Deacon
In-Reply-To: <20190813100856.GF17933@dhcp22.suse.cz>
On Tue, Aug 13, 2019 at 12:09 PM Michal Hocko <mhocko@kernel.org> wrote:
> On Mon 12-08-19 20:14:38, Jann Horn wrote:
> > On Wed, Aug 7, 2019 at 7:16 PM Joel Fernandes (Google)
> > <joel@joelfernandes.org> wrote:
> > > The page_idle tracking feature currently requires looking up the pagemap
> > > for a process followed by interacting with /sys/kernel/mm/page_idle.
> > > Looking up PFN from pagemap in Android devices is not supported by
> > > unprivileged process and requires SYS_ADMIN and gives 0 for the PFN.
> > >
> > > This patch adds support to directly interact with page_idle tracking at
> > > the PID level by introducing a /proc/<pid>/page_idle file. It follows
> > > the exact same semantics as the global /sys/kernel/mm/page_idle, but now
> > > looking up PFN through pagemap is not needed since the interface uses
> > > virtual frame numbers, and at the same time also does not require
> > > SYS_ADMIN.
> > >
> > > In Android, we are using this for the heap profiler (heapprofd) which
> > > profiles and pin points code paths which allocates and leaves memory
> > > idle for long periods of time. This method solves the security issue
> > > with userspace learning the PFN, and while at it is also shown to yield
> > > better results than the pagemap lookup, the theory being that the window
> > > where the address space can change is reduced by eliminating the
> > > intermediate pagemap look up stage. In virtual address indexing, the
> > > process's mmap_sem is held for the duration of the access.
> >
> > What happens when you use this interface on shared pages, like memory
> > inherited from the zygote, library file mappings and so on? If two
> > profilers ran concurrently for two different processes that both map
> > the same libraries, would they end up messing up each other's data?
>
> Yup PageIdle state is shared. That is the page_idle semantic even now
> IIRC.
>
> > Can this be used to observe which library pages other processes are
> > accessing, even if you don't have access to those processes, as long
> > as you can map the same libraries? I realize that there are already a
> > bunch of ways to do that with side channels and such; but if you're
> > adding an interface that allows this by design, it seems to me like
> > something that should be gated behind some sort of privilege check.
>
> Hmm, you need to be priviledged to get the pfn now and without that you
> cannot get to any page so the new interface is weakening the rules.
> Maybe we should limit setting the idle state to processes with the write
> status. Or do you think that even observing idle status is useful for
> practical side channel attacks? If yes, is that a problem of the
> profiler which does potentially dangerous things?
I suppose read-only access isn't a real problem as long as the
profiler isn't writing the idle state in a very tight loop... but I
don't see a usecase where you'd actually want that? As far as I can
tell, if you can't write the idle state, being able to read it is
pretty much useless.
If the profiler only wants to profile process-private memory, then
that should be implementable in a safe way in principle, I think, but
since Joel said that they want to profile CoW memory as well, I think
that's inherently somewhat dangerous.
^ permalink raw reply
* Re: [PATCH v5 1/6] mm/page_idle: Add per-pid idle page tracking using virtual index
From: Joel Fernandes @ 2019-08-13 15:30 UTC (permalink / raw)
To: Jann Horn
Cc: kernel list, Alexey Dobriyan, Andrew Morton, Borislav Petkov,
Brendan Gregg, Catalin Marinas, Christian Hansen,
Daniel Colascione, fmayer, H. Peter Anvin, Ingo Molnar,
Jonathan Corbet, Kees Cook, kernel-team, Linux API, linux-doc,
linux-fsdevel, Linux-MM, Michal Hocko, Mike Rapoport, Minchan Kim,
namhyung, Paul E. McKenney, Robin Murphy, Roman Gushchin,
Stephen Rothwell, Suren Baghdasaryan, Thomas Gleixner, Todd Kjos,
Vladimir Davydov, Vlastimil Babka, Will Deacon
In-Reply-To: <CAG48ez0ysprvRiENhBkLeV9YPTN_MB18rbu2HDa2jsWo5FYR8g@mail.gmail.com>
On Mon, Aug 12, 2019 at 08:14:38PM +0200, Jann Horn wrote:
[snip]
> > +/* Helper to get the start and end frame given a pos and count */
> > +static int page_idle_get_frames(loff_t pos, size_t count, struct mm_struct *mm,
> > + unsigned long *start, unsigned long *end)
> > +{
> > + unsigned long max_frame;
> > +
> > + /* If an mm is not given, assume we want physical frames */
> > + max_frame = mm ? (mm->task_size >> PAGE_SHIFT) : max_pfn;
> > +
> > + if (pos % BITMAP_CHUNK_SIZE || count % BITMAP_CHUNK_SIZE)
> > + return -EINVAL;
> > +
> > + *start = pos * BITS_PER_BYTE;
> > + if (*start >= max_frame)
> > + return -ENXIO;
> > +
> > + *end = *start + count * BITS_PER_BYTE;
> > + if (*end > max_frame)
> > + *end = max_frame;
> > + return 0;
> > +}
>
> You could add some overflow checks for the multiplications. I haven't
> seen any place where it actually matters, but it seems unclean; and in
> particular, on a 32-bit architecture where the maximum user address is
> very high (like with a 4G:4G split), it looks like this function might
> theoretically return with `*start > *end`, which could be confusing to
> callers.
I could store the multiplication result in unsigned long long (since we are
bounds checking with max_frame, start > end would not occur). Something like
the following (with extraneous casts). But I'll think some more about the
point you are raising.
diff --git a/mm/page_idle.c b/mm/page_idle.c
index 1ea21bbb36cb..8fd7a5559986 100644
--- a/mm/page_idle.c
+++ b/mm/page_idle.c
@@ -135,6 +135,7 @@ static int page_idle_get_frames(loff_t pos, size_t count, struct mm_struct *mm,
unsigned long *start, unsigned long *end)
{
unsigned long max_frame;
+ unsigned long long tmp;
/* If an mm is not given, assume we want physical frames */
max_frame = mm ? (mm->task_size >> PAGE_SHIFT) : max_pfn;
@@ -142,13 +143,16 @@ static int page_idle_get_frames(loff_t pos, size_t count, struct mm_struct *mm,
if (pos % BITMAP_CHUNK_SIZE || count % BITMAP_CHUNK_SIZE)
return -EINVAL;
- *start = pos * BITS_PER_BYTE;
- if (*start >= max_frame)
+ tmp = pos * BITS_PER_BYTE;
+ if (tmp >= (unsigned long long)max_frame)
return -ENXIO;
+ *start = (unsigned long)tmp;
- *end = *start + count * BITS_PER_BYTE;
- if (*end > max_frame)
+ tmp = *start + count * BITS_PER_BYTE;
+ if (tmp > (unsigned long long)max_frame)
*end = max_frame;
+ else
+ *end = (unsigned long)tmp;
return 0;
}
> [...]
> > for (; pfn < end_pfn; pfn++) {
> > bit = pfn % BITMAP_CHUNK_BITS;
> > if (!bit)
> > *out = 0ULL;
> > - page = page_idle_get_page(pfn);
> > - if (page) {
> > - if (page_is_idle(page)) {
> > - /*
> > - * The page might have been referenced via a
> > - * pte, in which case it is not idle. Clear
> > - * refs and recheck.
> > - */
> > - page_idle_clear_pte_refs(page);
> > - if (page_is_idle(page))
> > - *out |= 1ULL << bit;
> > - }
> > + page = page_idle_get_page_pfn(pfn);
> > + if (page && page_idle_pte_check(page)) {
> > + *out |= 1ULL << bit;
> > put_page(page);
> > }
>
> The `page && !page_idle_pte_check(page)` case looks like it's missing
> a put_page(); you probably intended to write something like this?
>
> page = page_idle_get_page_pfn(pfn);
> if (page) {
> if (page_idle_pte_check(page))
> *out |= 1ULL << bit;
> put_page(page);
> }
Ah, you're right. Will fix, good eyes and thank you!
> [...]
> > +/* page_idle tracking for /proc/<pid>/page_idle */
> > +
> > +struct page_node {
> > + struct page *page;
> > + unsigned long addr;
> > + struct list_head list;
> > +};
> > +
> > +struct page_idle_proc_priv {
> > + unsigned long start_addr;
> > + char *buffer;
> > + int write;
> > +
> > + /* Pre-allocate and provide nodes to pte_page_idle_proc_add() */
> > + struct page_node *page_nodes;
> > + int cur_page_node;
> > + struct list_head *idle_page_list;
> > +};
>
> A linked list is a weird data structure to use if the list elements
> are just consecutive array elements.
Not all of the pages will be considered in the later parts of the code, some
pages are skipped.
However, in v4 I added an array to allocate all the page_node structures,
since Andrew did not want GFP_ATOMIC allocations of individual list nodes.
So I think I could get rid of the linked list and leave the unused
page_node(s) as NULL, but I don't know I have to check if that is possible. I
could be missing a corner case, regardless I'll make a note of this and try!
> > +/*
> > + * Add page to list to be set as idle later.
> > + */
> > +static void pte_page_idle_proc_add(struct page *page,
> > + unsigned long addr, struct mm_walk *walk)
> > +{
> > + struct page *page_get = NULL;
> > + struct page_node *pn;
> > + int bit;
> > + unsigned long frames;
> > + struct page_idle_proc_priv *priv = walk->private;
> > + u64 *chunk = (u64 *)priv->buffer;
> > +
> > + if (priv->write) {
> > + VM_BUG_ON(!page);
> > +
> > + /* Find whether this page was asked to be marked */
> > + frames = (addr - priv->start_addr) >> PAGE_SHIFT;
> > + bit = frames % BITMAP_CHUNK_BITS;
> > + chunk = &chunk[frames / BITMAP_CHUNK_BITS];
> > + if (((*chunk >> bit) & 1) == 0)
> > + return;
>
> This means that BITMAP_CHUNK_SIZE is UAPI on big-endian systems,
> right? My opinion is that it would be slightly nicer to design the
> UAPI such that incrementing virtual addresses are mapped to
> incrementing offsets in the buffer (iow, either use bytewise access or
> use little-endian), but I'm not going to ask you to redesign the UAPI
> this late.
That would also be slow and consume more memory in userspace buffers.
Currently, a 64-bit (8 byte) chunk accounts for 64 pages worth or 256KB.
Also I wanted to keep the interface consistent with the global
/sys/kernel/mm/page_idle interface.
> [...]
> > +ssize_t page_idle_proc_generic(struct file *file, char __user *ubuff,
> > + size_t count, loff_t *pos, int write)
> > +{
> [...]
> > + down_read(&mm->mmap_sem);
> [...]
> > +
> > + if (!write && !walk_error)
> > + ret = copy_to_user(ubuff, buffer, count);
> > +
> > + up_read(&mm->mmap_sem);
>
> I'd move the up_read() above the copy_to_user(); copy_to_user() can
> block, and there's no reason to hold the mmap_sem across
> copy_to_user().
Will do.
> Sorry about only chiming in at v5 with all this.
No problem, I'm glad you did!
thanks,
- Joel
^ permalink raw reply related
* Re: [PATCH v5 2/6] mm/page_idle: Add support for handling swapped PG_Idle pages
From: Joel Fernandes @ 2019-08-13 15:36 UTC (permalink / raw)
To: Michal Hocko, khlebnikov
Cc: linux-kernel, Minchan Kim, Alexey Dobriyan, Andrew Morton,
Borislav Petkov, Brendan Gregg, Catalin Marinas, Christian Hansen,
dancol, fmayer, H. Peter Anvin, Ingo Molnar, Jonathan Corbet,
Kees Cook, kernel-team, linux-api, linux-doc, linux-fsdevel,
linux-mm, Mike Rapoport, namhyung, paulmck, Robin Murphy,
Roman Gushchin, Stephen Rothwell, surenb, Thomas Gleixner, tkjos,
Vladimir Davydov, Vlastimil Babka, Will Deacon
In-Reply-To: <20190813150450.GN17933@dhcp22.suse.cz>
On Tue, Aug 13, 2019 at 05:04:50PM +0200, Michal Hocko wrote:
> On Wed 07-08-19 13:15:55, Joel Fernandes (Google) wrote:
> > Idle page tracking currently does not work well in the following
> > scenario:
> > 1. mark page-A idle which was present at that time.
> > 2. run workload
> > 3. page-A is not touched by workload
> > 4. *sudden* memory pressure happen so finally page A is finally swapped out
> > 5. now see the page A - it appears as if it was accessed (pte unmapped
> > so idle bit not set in output) - but it's incorrect.
> >
> > To fix this, we store the idle information into a new idle bit of the
> > swap PTE during swapping of anonymous pages.
> >
> > Also in the future, madvise extensions will allow a system process
> > manager (like Android's ActivityManager) to swap pages out of a process
> > that it knows will be cold. To an external process like a heap profiler
> > that is doing idle tracking on another process, this procedure will
> > interfere with the idle page tracking similar to the above steps.
>
> This could be solved by checking the !present/swapped out pages
> right? Whoever decided to put the page out to the swap just made it
> idle effectively. So the monitor can make some educated guess for
> tracking. If that is fundamentally not possible then please describe
> why.
But the monitoring process (profiler) does not have control over the 'whoever
made it effectively idle' process.
As you said it will be a guess, it will not be accurate.
I am curious what is your concern with using a bit in the swap PTE?
(Adding Konstantin as well since we may be interested in this, since we also
suggested this idea).
thanks,
- Joel
^ permalink raw reply
* Re: [PATCH v5 1/6] mm/page_idle: Add per-pid idle page tracking using virtual index
From: Daniel Gruss @ 2019-08-13 15:34 UTC (permalink / raw)
To: Jann Horn, Michal Hocko, Joel Fernandes (Google)
Cc: kernel list, Alexey Dobriyan, Andrew Morton, Borislav Petkov,
Brendan Gregg, Catalin Marinas, Christian Hansen,
Daniel Colascione, fmayer, H. Peter Anvin, Ingo Molnar,
Joel Fernandes, Jonathan Corbet, Kees Cook, kernel-team,
Linux API, linux-doc, linux-fsdevel, Linux-MM, Mike Rapoport,
Minchan Kim, namhyung, Paul E. McKenney, Robin Murphy,
Roman Gushchin, Stephen Rothwell, Suren Baghdasaryan,
Thomas Gleixner, Todd Kjos, Vladimir Davydov, Vlastimil Babka,
Will Deacon
In-Reply-To: <CAG48ez2cuqe_VYhhaqw8Hcyswv47cmz2XmkqNdvkXEhokMVaXg@mail.gmail.com>
On 8/13/19 5:29 PM, Jann Horn wrote:
> On Tue, Aug 13, 2019 at 12:09 PM Michal Hocko <mhocko@kernel.org> wrote:
>> On Mon 12-08-19 20:14:38, Jann Horn wrote:
>>> On Wed, Aug 7, 2019 at 7:16 PM Joel Fernandes (Google)
>>> <joel@joelfernandes.org> wrote:
>>>> The page_idle tracking feature currently requires looking up the pagemap
>>>> for a process followed by interacting with /sys/kernel/mm/page_idle.
>>>> Looking up PFN from pagemap in Android devices is not supported by
>>>> unprivileged process and requires SYS_ADMIN and gives 0 for the PFN.
>>>>
>>>> This patch adds support to directly interact with page_idle tracking at
>>>> the PID level by introducing a /proc/<pid>/page_idle file. It follows
>>>> the exact same semantics as the global /sys/kernel/mm/page_idle, but now
>>>> looking up PFN through pagemap is not needed since the interface uses
>>>> virtual frame numbers, and at the same time also does not require
>>>> SYS_ADMIN.
>>>>
>>>> In Android, we are using this for the heap profiler (heapprofd) which
>>>> profiles and pin points code paths which allocates and leaves memory
>>>> idle for long periods of time. This method solves the security issue
>>>> with userspace learning the PFN, and while at it is also shown to yield
>>>> better results than the pagemap lookup, the theory being that the window
>>>> where the address space can change is reduced by eliminating the
>>>> intermediate pagemap look up stage. In virtual address indexing, the
>>>> process's mmap_sem is held for the duration of the access.
>>>
>>> What happens when you use this interface on shared pages, like memory
>>> inherited from the zygote, library file mappings and so on? If two
>>> profilers ran concurrently for two different processes that both map
>>> the same libraries, would they end up messing up each other's data?
>>
>> Yup PageIdle state is shared. That is the page_idle semantic even now
>> IIRC.
>>
>>> Can this be used to observe which library pages other processes are
>>> accessing, even if you don't have access to those processes, as long
>>> as you can map the same libraries? I realize that there are already a
>>> bunch of ways to do that with side channels and such; but if you're
>>> adding an interface that allows this by design, it seems to me like
>>> something that should be gated behind some sort of privilege check.
>>
>> Hmm, you need to be priviledged to get the pfn now and without that you
>> cannot get to any page so the new interface is weakening the rules.
>> Maybe we should limit setting the idle state to processes with the write
>> status. Or do you think that even observing idle status is useful for
>> practical side channel attacks? If yes, is that a problem of the
>> profiler which does potentially dangerous things?
>
> I suppose read-only access isn't a real problem as long as the
> profiler isn't writing the idle state in a very tight loop... but I
> don't see a usecase where you'd actually want that? As far as I can
> tell, if you can't write the idle state, being able to read it is
> pretty much useless.
>
> If the profiler only wants to profile process-private memory, then
> that should be implementable in a safe way in principle, I think, but
> since Joel said that they want to profile CoW memory as well, I think
> that's inherently somewhat dangerous.
I agree that allowing profiling of shared pages would leak information.
To me the use case is not entirely clear. This is not a feature that
would normally be run in everyday computer usage, right?
^ permalink raw reply
* Re: [PATCH v5 1/6] mm/page_idle: Add per-pid idle page tracking using virtual index
From: Jann Horn @ 2019-08-13 15:40 UTC (permalink / raw)
To: Joel Fernandes
Cc: kernel list, Alexey Dobriyan, Andrew Morton, Borislav Petkov,
Brendan Gregg, Catalin Marinas, Christian Hansen,
Daniel Colascione, fmayer, H. Peter Anvin, Ingo Molnar,
Jonathan Corbet, Kees Cook, kernel-team, Linux API, linux-doc,
linux-fsdevel, Linux-MM, Michal Hocko, Mike Rapoport, Minchan Kim,
namhyung, Paul E. McKenney, Robin Murphy, Roman Gushchin,
Stephen Rothwell, Suren Baghdasaryan, Thomas Gleixner, Todd Kjos,
Vladimir Davydov, Vlastimil Babka, Will Deacon
In-Reply-To: <20190813153020.GC14622@google.com>
On Tue, Aug 13, 2019 at 5:30 PM Joel Fernandes <joel@joelfernandes.org> wrote:
> On Mon, Aug 12, 2019 at 08:14:38PM +0200, Jann Horn wrote:
> [snip]
> > > +/* Helper to get the start and end frame given a pos and count */
> > > +static int page_idle_get_frames(loff_t pos, size_t count, struct mm_struct *mm,
> > > + unsigned long *start, unsigned long *end)
> > > +{
> > > + unsigned long max_frame;
> > > +
> > > + /* If an mm is not given, assume we want physical frames */
> > > + max_frame = mm ? (mm->task_size >> PAGE_SHIFT) : max_pfn;
> > > +
> > > + if (pos % BITMAP_CHUNK_SIZE || count % BITMAP_CHUNK_SIZE)
> > > + return -EINVAL;
> > > +
> > > + *start = pos * BITS_PER_BYTE;
> > > + if (*start >= max_frame)
> > > + return -ENXIO;
> > > +
> > > + *end = *start + count * BITS_PER_BYTE;
> > > + if (*end > max_frame)
> > > + *end = max_frame;
> > > + return 0;
> > > +}
> >
> > You could add some overflow checks for the multiplications. I haven't
> > seen any place where it actually matters, but it seems unclean; and in
> > particular, on a 32-bit architecture where the maximum user address is
> > very high (like with a 4G:4G split), it looks like this function might
> > theoretically return with `*start > *end`, which could be confusing to
> > callers.
>
> I could store the multiplication result in unsigned long long (since we are
> bounds checking with max_frame, start > end would not occur). Something like
> the following (with extraneous casts). But I'll think some more about the
> point you are raising.
check_mul_overflow() exists and could make that a bit cleaner.
> > This means that BITMAP_CHUNK_SIZE is UAPI on big-endian systems,
> > right? My opinion is that it would be slightly nicer to design the
> > UAPI such that incrementing virtual addresses are mapped to
> > incrementing offsets in the buffer (iow, either use bytewise access or
> > use little-endian), but I'm not going to ask you to redesign the UAPI
> > this late.
>
> That would also be slow and consume more memory in userspace buffers.
> Currently, a 64-bit (8 byte) chunk accounts for 64 pages worth or 256KB.
I still wanted to use one bit per page; I just wanted to rearrange the
bits. So the first byte would always contain 8 bits corresponding to
the first 8 pages, instead of corresponding to pages 56-63 on some
systems depending on endianness. Anyway, this is a moot point, since
as you said...
> Also I wanted to keep the interface consistent with the global
> /sys/kernel/mm/page_idle interface.
Sorry, I missed that this is already UAPI in the global interface. I
agree, using a different API for the per-process interface would be a
bad idea.
^ permalink raw reply
* Re: [RFC 01/19] kbuild: Fixes to rules for host-cshlib and host-cxxshlib
From: Knut Omang @ 2019-08-13 16:19 UTC (permalink / raw)
To: Masahiro Yamada
Cc: open list:KERNEL SELFTEST FRAMEWORK, Linux Kernel Mailing List,
open list:DOCUMENTATION, Linux Kbuild mailing list, Shuah Khan,
Jonathan Corbet, Michal Marek, Greg Kroah-Hartman,
Shreyans Devendra Doshi, Alan Maguire, Brendan Higgins,
Kevin Hilman, Hidenori Yamaji, Frank Rowand, Timothy Bird,
Luis Chamberlain, Theodore Ts'o, Daniel Vetter, Stephen Boyd
In-Reply-To: <CAK7LNASX4jPRxRxD+JafAfKqjck=x27HuHZgPV1VFfW8MzcwZA@mail.gmail.com>
On Tue, 2019-08-13 at 23:01 +0900, Masahiro Yamada wrote:
> On Tue, Aug 13, 2019 at 3:13 PM Knut Omang <knut.omang@oracle.com> wrote:
> >
> > C++ libraries interfacing to C APIs might sometimes need some glue
> > logic more easily written in C.
> > Allow a C++ library to also contain 0 or more C objects.
> >
> > Also fix rules for both C and C++ shared libraries:
> > - C++ shared libraries depended on .c instead of .cc files
> > - Rules were referenced as -objs instead of the intended
> > -cobjs and -cxxobjs following the pattern from hostprogs*.
> >
> > Signed-off-by: Knut Omang <knut.omang@oracle.com>
>
>
> How is this patch related to the rest of this series?
This is just my (likely naive) way I to get what I had working
using autotools in the Github version of KTF) translated into something
comparable using kbuild only. We need to build a shared library consisting
of a few C++ files and a very simple C file, and a couple of simple binaries,
and the rule in there does seem to take .c files and subject them to the
C++ compiler, which makes this difficult to achieve?
> This patch breaks GCC-plugins.
> Did you really compile-test this patch before the submission?
Sorry for my ignorance here:
I ran through the kernel build and installed the resulting kernel
on a VM that I used to test this, if that's what you are asking
about?
Do I need some unusual .config options or run a special make target
to trigger the problem you see?
I used a recent Fedora config with default values for new options,
and ran the normal default make target (also with O=) and make selftests
to test the patch itself.
Thanks,
Knut
> --
> Best Regards
>
> Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v12 03/18] kunit: test: add string_stream a std::stream like string builder
From: Stephen Boyd @ 2019-08-13 16:48 UTC (permalink / raw)
To: Brendan Higgins
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, shuah,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Michael Ellerman, Petr Mladek, Randy Dunlap,
Richard Weinberger, David Rientjes, Steven Rostedt, wfg
In-Reply-To: <CAFd5g452+-6m1eiVK0ccTDkJ2wH8GBwxRDw5owwC8h3NscE1ag@mail.gmail.com>
Quoting Brendan Higgins (2019-08-13 02:12:54)
> On Tue, Aug 13, 2019 at 2:04 AM Brendan Higgins
> <brendanhiggins@google.com> wrote:
> >
> > On Mon, Aug 12, 2019 at 10:30 PM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Brendan Higgins (2019-08-12 22:02:59)
> > > > However, now that I added the kunit_resource_destroy, I thought it
> > > > might be good to free the string_stream after I use it in each call to
> > > > kunit_assert->format(...) in which case I will be using this logic.
> > > >
> > > > So I think the right thing to do is to expose string_stream_destroy so
> > > > kunit_do_assert can clean up when it's done, and then demote
> > > > string_stream_clear to static. Sound good?
> > >
> > > Ok, sure. I don't really see how clearing it explicitly when the
> > > assertion prints vs. never allocating it to begin with is really any
> > > different. Maybe I've missed something though.
> >
> > It's for the case that we *do* print something out. Once we are doing
> > printing, we don't want the fragments anymore.
>
> Oops, sorry fat fingered: s/doing/done
Yes, but when we print something out we've run into some sort of problem
and then the test is over. So freeing the memory when it fails vs. when
the test is over seems like a minor difference. Or is it also used to
print other informational messages while the test is running?
I'm not particularly worried here, just trying to see if less code is
possible.
^ permalink raw reply
* [PATCH v3 1/2] rcu/tree: Add basic support for kfree_rcu batching
From: Joel Fernandes (Google) @ 2019-08-13 17:00 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Rao Shoaib, max.byungchul.park,
byungchul.park, kernel-team, kernel-team, Andrew Morton,
Davidlohr Bueso, Jonathan Corbet, Josh Triplett, Kees Cook,
Lai Jiangshan, linux-doc, Mathieu Desnoyers,
Mauro Carvalho Chehab, Paul E. McKenney, rcu, Steven Rostedt,
Thomas Gleixner
Recently a discussion about stability and performance of a system
involving a high rate of kfree_rcu() calls surfaced on the list [1]
which led to another discussion how to prepare for this situation.
This patch adds basic batching support for kfree_rcu(). It is "basic"
because we do none of the slab management, dynamic allocation, code
moving or any of the other things, some of which previous attempts did
[2]. These fancier improvements can be follow-up patches and there are
different ideas being discussed in those regards.
Torture tests follow in the next patch and show improvements of around
400% in reduction of number of grace periods on a 16 CPU system. More
details and test data are in that patch.
This is an effort to start simple, and build up from there. In the
future, an extension to use kfree_bulk and possibly per-slab batching
could be done to further improve performance due to cache-locality and
slab-specific bulk free optimizations. By using an array of pointers,
the worker thread processing the work would need to read lesser data
since it does not need to deal with large rcu_head(s) any longer.
There is an implication with rcu_barrier() with this patch. Since the
kfree_rcu() calls can be batched, and may not be handed yet to the RCU
machinery in fact, the monitor may not have even run yet to do the
queue_rcu_work(), there seems no easy way of implementing rcu_barrier()
to wait for those kfree_rcu()s that are already made. So this means a
kfree_rcu() followed by an rcu_barrier() does not imply that memory will
be freed once rcu_barrier() returns.
Another implication is higher active memory usage (although not
run-away..) until the kfree_rcu() flooding ends, in comparison to
without batching. More details about this are in the second patch which
adds an rcuperf test.
Finally, in the near future we will get rid of kfree_rcu special casing
within RCU such as in rcu_do_batch and switch everything to just
batching. Currently we don't do that since timer subsystem is not yet up
and we cannot schedule the kfree_rcu() monitor as the timer subsystem's
lock are not initialized. That would also mean getting rid of
kfree_call_rcu_nobatch() entirely.
[1] http://lore.kernel.org/lkml/20190723035725-mutt-send-email-mst@kernel.org
[2] https://lkml.org/lkml/2017/12/19/824
Cc: Rao Shoaib <rao.shoaib@oracle.com>
Cc: max.byungchul.park@gmail.com
Cc: byungchul.park@lge.com
Cc: kernel-team@android.com
Cc: kernel-team@lge.com
Co-developed-by: Byungchul Park <byungchul.park@lge.com>
Signed-off-by: Byungchul Park <byungchul.park@lge.com>
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
v2->v3: Just some code comment changes thanks to Byungchul.
RFCv1->PATCH v2: Removed limits on the ->head list, just let it grow.
Dropped KFREE_MAX_JIFFIES to HZ/50 from HZ/20 to reduce OOM occurrence.
Removed sleeps in rcuperf test, just using cond_resched()in loop.
Better code comments ;)
.../admin-guide/kernel-parameters.txt | 17 ++
include/linux/rcutiny.h | 5 +
include/linux/rcutree.h | 1 +
kernel/rcu/tree.c | 204 +++++++++++++++++-
4 files changed, 221 insertions(+), 6 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 7ccd158b3894..a9156ca5de24 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3895,6 +3895,23 @@
test until boot completes in order to avoid
interference.
+ rcuperf.kfree_rcu_test= [KNL]
+ Set to measure performance of kfree_rcu() flooding.
+
+ rcuperf.kfree_nthreads= [KNL]
+ The number of threads running loops of kfree_rcu().
+
+ rcuperf.kfree_alloc_num= [KNL]
+ Number of allocations and frees done in an iteration.
+
+ rcuperf.kfree_loops= [KNL]
+ Number of loops doing rcuperf.kfree_alloc_num number
+ of allocations and frees.
+
+ rcuperf.kfree_no_batch= [KNL]
+ Use the non-batching (slower) version of kfree_rcu.
+ This is useful for comparing with the batched version.
+
rcuperf.nreaders= [KNL]
Set number of RCU readers. The value -1 selects
N, where N is the number of CPUs. A value
diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
index 8e727f57d814..383f2481750f 100644
--- a/include/linux/rcutiny.h
+++ b/include/linux/rcutiny.h
@@ -39,6 +39,11 @@ static inline void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
call_rcu(head, func);
}
+static inline void kfree_call_rcu_nobatch(struct rcu_head *head, rcu_callback_t func)
+{
+ call_rcu(head, func);
+}
+
void rcu_qs(void);
static inline void rcu_softirq_qs(void)
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index 735601ac27d3..7e38b39ec634 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -34,6 +34,7 @@ static inline void rcu_virt_note_context_switch(int cpu)
void synchronize_rcu_expedited(void);
void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
+void kfree_call_rcu_nobatch(struct rcu_head *head, rcu_callback_t func);
void rcu_barrier(void);
bool rcu_eqs_special_set(int cpu);
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index a14e5fbbea46..3e62bbb0e1fa 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2593,17 +2593,195 @@ void call_rcu(struct rcu_head *head, rcu_callback_t func)
}
EXPORT_SYMBOL_GPL(call_rcu);
+
+/* Maximum number of jiffies to wait before draining a batch. */
+#define KFREE_DRAIN_JIFFIES (HZ / 50)
+
/*
- * Queue an RCU callback for lazy invocation after a grace period.
- * This will likely be later named something like "call_rcu_lazy()",
- * but this change will require some way of tagging the lazy RCU
- * callbacks in the list of pending callbacks. Until then, this
- * function may only be called from __kfree_rcu().
+ * Maximum number of kfree(s) to batch, if this limit is hit then the batch of
+ * kfree(s) is queued for freeing after a grace period, right away.
*/
-void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
+struct kfree_rcu_cpu {
+ /* The rcu_work node for queuing work with queue_rcu_work(). The work
+ * is done after a grace period.
+ */
+ struct rcu_work rcu_work;
+
+ /* The list of objects being queued in a batch but are not yet
+ * scheduled to be freed.
+ */
+ struct rcu_head *head;
+
+ /* The list of objects that have now left ->head and are queued for
+ * freeing after a grace period.
+ */
+ struct rcu_head *head_free;
+
+ /* Protect concurrent access to this structure. */
+ spinlock_t lock;
+
+ /* The delayed work that flushes ->head to ->head_free incase ->head
+ * within KFREE_DRAIN_JIFFIES. In case flushing cannot be done if RCU
+ * is busy, ->head just continues to grow and we retry flushing later.
+ */
+ struct delayed_work monitor_work;
+ bool monitor_todo; /* Is a delayed work pending execution? */
+};
+
+static DEFINE_PER_CPU(struct kfree_rcu_cpu, krc);
+
+/*
+ * This function is invoked in workqueue context after a grace period.
+ * It frees all the objects queued on ->head_free.
+ */
+static void kfree_rcu_work(struct work_struct *work)
+{
+ unsigned long flags;
+ struct rcu_head *head, *next;
+ struct kfree_rcu_cpu *krcp = container_of(to_rcu_work(work),
+ struct kfree_rcu_cpu, rcu_work);
+
+ spin_lock_irqsave(&krcp->lock, flags);
+ head = krcp->head_free;
+ krcp->head_free = NULL;
+ spin_unlock_irqrestore(&krcp->lock, flags);
+
+ /*
+ * The head is detached and not referenced from anywhere, so lockless
+ * access is Ok.
+ */
+ for (; head; head = next) {
+ next = head->next;
+ head->next = NULL;
+ /* Could be possible to optimize with kfree_bulk in future */
+ __rcu_reclaim(rcu_state.name, head);
+ cond_resched_tasks_rcu_qs();
+ }
+}
+
+/*
+ * Schedule the kfree batch RCU work to run in workqueue context after a GP.
+ *
+ * This function is invoked by kfree_rcu_monitor() when the KFREE_DRAIN_JIFFIES
+ * timeout has been reached.
+ */
+static inline bool queue_kfree_rcu_work(struct kfree_rcu_cpu *krcp)
+{
+ lockdep_assert_held(&krcp->lock);
+
+ /* If a previous RCU batch work is already in progress, we cannot queue
+ * another one, just refuse the optimization and it will be retried
+ * again in KFREE_DRAIN_JIFFIES time.
+ */
+ if (krcp->head_free)
+ return false;
+
+ krcp->head_free = krcp->head;
+ krcp->head = NULL;
+ INIT_RCU_WORK(&krcp->rcu_work, kfree_rcu_work);
+ queue_rcu_work(system_wq, &krcp->rcu_work);
+
+ return true;
+}
+
+static inline void kfree_rcu_drain_unlock(struct kfree_rcu_cpu *krcp,
+ unsigned long flags)
+{
+ /* Flush ->head to ->head_free, all objects on ->head_free will be
+ * kfree'd after a grace period.
+ */
+ if (queue_kfree_rcu_work(krcp)) {
+ /* Success! Our job is done here. */
+ spin_unlock_irqrestore(&krcp->lock, flags);
+ return;
+ }
+
+ /* Previous batch did not get free yet, let us try again soon. */
+ if (krcp->monitor_todo == false) {
+ schedule_delayed_work_on(smp_processor_id(),
+ &krcp->monitor_work, KFREE_DRAIN_JIFFIES);
+ krcp->monitor_todo = true;
+ }
+ spin_unlock_irqrestore(&krcp->lock, flags);
+}
+
+/*
+ * This function is invoked after the KFREE_DRAIN_JIFFIES timeout has elapsed,
+ * so it drains the specified kfree_rcu_cpu structure's ->head list.
+ */
+static void kfree_rcu_monitor(struct work_struct *work)
+{
+ bool todo;
+ unsigned long flags;
+ struct kfree_rcu_cpu *krcp = container_of(work, struct kfree_rcu_cpu,
+ monitor_work.work);
+
+ spin_lock_irqsave(&krcp->lock, flags);
+ todo = krcp->monitor_todo;
+ krcp->monitor_todo = false;
+ if (todo)
+ kfree_rcu_drain_unlock(krcp, flags);
+ else
+ spin_unlock_irqrestore(&krcp->lock, flags);
+}
+
+/*
+ * This version of kfree_call_rcu does not do batching of kfree_rcu() requests.
+ * Used only by rcuperf torture test for comparison with kfree_rcu_batch().
+ */
+void kfree_call_rcu_nobatch(struct rcu_head *head, rcu_callback_t func)
{
__call_rcu(head, func, -1, 1);
}
+EXPORT_SYMBOL_GPL(kfree_call_rcu_nobatch);
+
+/*
+ * Queue a request for lazy invocation of kfree() after a grace period.
+ *
+ * Each kfree_call_rcu() request is added to a batch. The batch will be drained
+ * every KFREE_DRAIN_JIFFIES number of jiffies. All the objects in the batch
+ * will be kfree'd in workqueue context. This allows us to:
+ *
+ * 1. Batch requests together to reduce the number of grace periods during
+ * heavy kfree_rcu() load.
+ *
+ * 2. In the future, makes it possible to use kfree_bulk() on a large number of
+ * kfree_rcu() requests thus reducing the per-object overhead of kfree() and
+ * also reducing cache misses.
+ */
+void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
+{
+ unsigned long flags;
+ struct kfree_rcu_cpu *krcp;
+ bool monitor_todo;
+
+ /* kfree_call_rcu() batching requires timers to be up. If the scheduler
+ * is not yet up, just skip batching and do non-batched kfree_call_rcu().
+ */
+ if (rcu_scheduler_active != RCU_SCHEDULER_RUNNING)
+ return kfree_call_rcu_nobatch(head, func);
+
+ local_irq_save(flags);
+ krcp = this_cpu_ptr(&krc);
+
+ spin_lock(&krcp->lock);
+
+ /* Queue the kfree but don't yet schedule the batch. */
+ head->func = func;
+ head->next = krcp->head;
+ krcp->head = head;
+
+ /* Schedule monitor for timely drain after KFREE_DRAIN_JIFFIES. */
+ monitor_todo = krcp->monitor_todo;
+ krcp->monitor_todo = true;
+ spin_unlock(&krcp->lock);
+
+ if (!monitor_todo) {
+ schedule_delayed_work_on(smp_processor_id(),
+ &krcp->monitor_work, KFREE_DRAIN_JIFFIES);
+ }
+ local_irq_restore(flags);
+}
EXPORT_SYMBOL_GPL(kfree_call_rcu);
/*
@@ -3455,10 +3633,24 @@ static void __init rcu_dump_rcu_node_tree(void)
struct workqueue_struct *rcu_gp_wq;
struct workqueue_struct *rcu_par_gp_wq;
+static void __init kfree_rcu_batch_init(void)
+{
+ int cpu;
+
+ for_each_possible_cpu(cpu) {
+ struct kfree_rcu_cpu *krcp = per_cpu_ptr(&krc, cpu);
+
+ spin_lock_init(&krcp->lock);
+ INIT_DELAYED_WORK(&krcp->monitor_work, kfree_rcu_monitor);
+ }
+}
+
void __init rcu_init(void)
{
int cpu;
+ kfree_rcu_batch_init();
+
rcu_early_boot_tests();
rcu_bootup_announce();
--
2.23.0.rc1.153.gdeed80330f-goog
^ permalink raw reply related
* [PATCH v3 2/2] rcuperf: Add kfree_rcu performance Tests
From: Joel Fernandes (Google) @ 2019-08-13 17:00 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Andrew Morton, byungchul.park,
Davidlohr Bueso, Jonathan Corbet, Josh Triplett, Kees Cook,
kernel-team, kernel-team, Lai Jiangshan, linux-doc,
Mathieu Desnoyers, Mauro Carvalho Chehab, max.byungchul.park,
Paul E. McKenney, Rao Shoaib, rcu, Steven Rostedt,
Thomas Gleixner
In-Reply-To: <20190813170046.81707-1-joel@joelfernandes.org>
This test runs kfree_rcu in a loop to measure performance of the new
kfree_rcu batching functionality.
The following table shows results when booting with arguments:
rcuperf.kfree_loops=200000 rcuperf.kfree_alloc_num=1000 rcuperf.kfree_rcu_test=1
In addition, rcuperf.kfree_no_batch is used to toggle the batching of
kfree_rcu()s for a test run.
rcuperf.kfree_no_batch GPs time (seconds)
0 (default) 1732 15.9
1 9133 14.5
Note that the results are the same for the case:
1. Patch is not applied and rcuperf.kfree_no_batch=0
2. Patch is applied and rcuperf.kfree_no_batch=1
On a 16 CPU system with the above boot parameters, we see that the total
number of grace periods that elapse during the test drops from 9133 when
not batching to 1732 when batching (a ~427% improvement). The
kfree_rcu() flood itself slows down a bit when batching, though, as
shown. This is likely due to rcuperf threads contending with the
additional worker threads that are now running both before (the monitor)
and after (the work done to kfree()) the grace period.
Note that the active memory consumption during the kfree_rcu() flood
does increase to around 300-400MB due to the batching (from around 50MB
without batching). However, this memory consumption is relatively
constant and is just an effect of the buffering. In other words, the
system is able to keep up with the kfree_rcu() load.
Also, when running the test, please disable CONFIG_DEBUG_PREEMPT and
CONFIG_PROVE_RCU for realistic comparisons with/without batching.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
kernel/rcu/rcuperf.c | 189 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 181 insertions(+), 8 deletions(-)
diff --git a/kernel/rcu/rcuperf.c b/kernel/rcu/rcuperf.c
index 7a6890b23c5f..70d6ac19cbff 100644
--- a/kernel/rcu/rcuperf.c
+++ b/kernel/rcu/rcuperf.c
@@ -86,6 +86,7 @@ torture_param(bool, shutdown, RCUPERF_SHUTDOWN,
"Shutdown at end of performance tests.");
torture_param(int, verbose, 1, "Enable verbose debugging printk()s");
torture_param(int, writer_holdoff, 0, "Holdoff (us) between GPs, zero to disable");
+torture_param(int, kfree_rcu_test, 0, "Do we run a kfree_rcu perf test?");
static char *perf_type = "rcu";
module_param(perf_type, charp, 0444);
@@ -105,8 +106,8 @@ static atomic_t n_rcu_perf_writer_finished;
static wait_queue_head_t shutdown_wq;
static u64 t_rcu_perf_writer_started;
static u64 t_rcu_perf_writer_finished;
-static unsigned long b_rcu_perf_writer_started;
-static unsigned long b_rcu_perf_writer_finished;
+static unsigned long b_rcu_gp_test_started;
+static unsigned long b_rcu_gp_test_finished;
static DEFINE_PER_CPU(atomic_t, n_async_inflight);
static int rcu_perf_writer_state;
@@ -379,10 +380,10 @@ rcu_perf_writer(void *arg)
if (atomic_inc_return(&n_rcu_perf_writer_started) >= nrealwriters) {
t_rcu_perf_writer_started = t;
if (gp_exp) {
- b_rcu_perf_writer_started =
+ b_rcu_gp_test_started =
cur_ops->exp_completed() / 2;
} else {
- b_rcu_perf_writer_started = cur_ops->get_gp_seq();
+ b_rcu_gp_test_started = cur_ops->get_gp_seq();
}
}
@@ -435,10 +436,10 @@ rcu_perf_writer(void *arg)
PERFOUT_STRING("Test complete");
t_rcu_perf_writer_finished = t;
if (gp_exp) {
- b_rcu_perf_writer_finished =
+ b_rcu_gp_test_finished =
cur_ops->exp_completed() / 2;
} else {
- b_rcu_perf_writer_finished =
+ b_rcu_gp_test_finished =
cur_ops->get_gp_seq();
}
if (shutdown) {
@@ -523,8 +524,8 @@ rcu_perf_cleanup(void)
t_rcu_perf_writer_finished -
t_rcu_perf_writer_started,
ngps,
- rcuperf_seq_diff(b_rcu_perf_writer_finished,
- b_rcu_perf_writer_started));
+ rcuperf_seq_diff(b_rcu_gp_test_finished,
+ b_rcu_gp_test_started));
for (i = 0; i < nrealwriters; i++) {
if (!writer_durations)
break;
@@ -592,6 +593,175 @@ rcu_perf_shutdown(void *arg)
return -EINVAL;
}
+/*
+ * kfree_rcu performance tests: Start a kfree_rcu loop on all CPUs for number
+ * of iterations and measure total time and number of GP for all iterations to complete.
+ */
+
+torture_param(int, kfree_nthreads, -1, "Number of threads running loops of kfree_rcu().");
+torture_param(int, kfree_alloc_num, 8000, "Number of allocations and frees done in an iteration.");
+torture_param(int, kfree_loops, 10, "Number of loops doing kfree_alloc_num allocations and frees.");
+torture_param(int, kfree_no_batch, 0, "Use the non-batching (slower) version of kfree_rcu.");
+
+static struct task_struct **kfree_reader_tasks;
+static int kfree_nrealthreads;
+static atomic_t n_kfree_perf_thread_started;
+static atomic_t n_kfree_perf_thread_ended;
+
+struct kfree_obj {
+ char kfree_obj[8];
+ struct rcu_head rh;
+};
+
+static int
+kfree_perf_thread(void *arg)
+{
+ int i, loop = 0;
+ long me = (long)arg;
+ struct kfree_obj **alloc_ptrs;
+ u64 start_time, end_time;
+
+ VERBOSE_PERFOUT_STRING("kfree_perf_thread task started");
+ set_cpus_allowed_ptr(current, cpumask_of(me % nr_cpu_ids));
+ set_user_nice(current, MAX_NICE);
+
+ alloc_ptrs = (struct kfree_obj **)kmalloc(sizeof(struct kfree_obj *) * kfree_alloc_num,
+ GFP_KERNEL);
+ if (!alloc_ptrs)
+ return -ENOMEM;
+
+ start_time = ktime_get_mono_fast_ns();
+
+ if (atomic_inc_return(&n_kfree_perf_thread_started) >= kfree_nrealthreads) {
+ if (gp_exp)
+ b_rcu_gp_test_started = cur_ops->exp_completed() / 2;
+ else
+ b_rcu_gp_test_started = cur_ops->get_gp_seq();
+ }
+
+ do {
+ for (i = 0; i < kfree_alloc_num; i++) {
+ alloc_ptrs[i] = kmalloc(sizeof(struct kfree_obj), GFP_KERNEL);
+ if (!alloc_ptrs[i])
+ return -ENOMEM;
+ }
+
+ for (i = 0; i < kfree_alloc_num; i++) {
+ if (!kfree_no_batch) {
+ kfree_rcu(alloc_ptrs[i], rh);
+ } else {
+ rcu_callback_t cb;
+
+ cb = (rcu_callback_t)(unsigned long)offsetof(struct kfree_obj, rh);
+ kfree_call_rcu_nobatch(&(alloc_ptrs[i]->rh), cb);
+ }
+ }
+
+ cond_resched();
+ } while (!torture_must_stop() && ++loop < kfree_loops);
+
+ if (atomic_inc_return(&n_kfree_perf_thread_ended) >= kfree_nrealthreads) {
+ end_time = ktime_get_mono_fast_ns();
+
+ if (gp_exp)
+ b_rcu_gp_test_finished = cur_ops->exp_completed() / 2;
+ else
+ b_rcu_gp_test_finished = cur_ops->get_gp_seq();
+
+ pr_alert("Total time taken by all kfree'ers: %llu ns, loops: %d, batches: %ld\n",
+ (unsigned long long)(end_time - start_time), kfree_loops,
+ rcuperf_seq_diff(b_rcu_gp_test_finished, b_rcu_gp_test_started));
+ if (shutdown) {
+ smp_mb(); /* Assign before wake. */
+ wake_up(&shutdown_wq);
+ }
+ }
+
+ kfree(alloc_ptrs);
+ torture_kthread_stopping("kfree_perf_thread");
+ return 0;
+}
+
+static void
+kfree_perf_cleanup(void)
+{
+ int i;
+
+ if (torture_cleanup_begin())
+ return;
+
+ if (kfree_reader_tasks) {
+ for (i = 0; i < kfree_nrealthreads; i++)
+ torture_stop_kthread(kfree_perf_thread,
+ kfree_reader_tasks[i]);
+ kfree(kfree_reader_tasks);
+ }
+
+ torture_cleanup_end();
+}
+
+/*
+ * shutdown kthread. Just waits to be awakened, then shuts down system.
+ */
+static int
+kfree_perf_shutdown(void *arg)
+{
+ do {
+ wait_event(shutdown_wq,
+ atomic_read(&n_kfree_perf_thread_ended) >=
+ kfree_nrealthreads);
+ } while (atomic_read(&n_kfree_perf_thread_ended) < kfree_nrealthreads);
+
+ smp_mb(); /* Wake before output. */
+
+ kfree_perf_cleanup();
+ kernel_power_off();
+ return -EINVAL;
+}
+
+static int __init
+kfree_perf_init(void)
+{
+ long i;
+ int firsterr = 0;
+
+ kfree_nrealthreads = compute_real(kfree_nthreads);
+ /* Start up the kthreads. */
+ if (shutdown) {
+ init_waitqueue_head(&shutdown_wq);
+ firsterr = torture_create_kthread(kfree_perf_shutdown, NULL,
+ shutdown_task);
+ if (firsterr)
+ goto unwind;
+ schedule_timeout_uninterruptible(1);
+ }
+
+ kfree_reader_tasks = kcalloc(kfree_nrealthreads, sizeof(kfree_reader_tasks[0]),
+ GFP_KERNEL);
+ if (kfree_reader_tasks == NULL) {
+ firsterr = -ENOMEM;
+ goto unwind;
+ }
+
+ for (i = 0; i < kfree_nrealthreads; i++) {
+ firsterr = torture_create_kthread(kfree_perf_thread, (void *)i,
+ kfree_reader_tasks[i]);
+ if (firsterr)
+ goto unwind;
+ }
+
+ while (atomic_read(&n_kfree_perf_thread_started) < kfree_nrealthreads)
+ schedule_timeout_uninterruptible(1);
+
+ torture_init_end();
+ return 0;
+
+unwind:
+ torture_init_end();
+ kfree_perf_cleanup();
+ return firsterr;
+}
+
static int __init
rcu_perf_init(void)
{
@@ -624,6 +794,9 @@ rcu_perf_init(void)
if (cur_ops->init)
cur_ops->init();
+ if (kfree_rcu_test)
+ return kfree_perf_init();
+
nrealwriters = compute_real(nwriters);
nrealreaders = compute_real(nreaders);
atomic_set(&n_rcu_perf_reader_started, 0);
--
2.23.0.rc1.153.gdeed80330f-goog
^ permalink raw reply related
* Re: [RFC 00/19] Integration of Kernel Test Framework (KTF) into the kernel tree
From: Brendan Higgins @ 2019-08-13 17:02 UTC (permalink / raw)
To: Knut Omang
Cc: Greg Kroah-Hartman, open list:KERNEL SELFTEST FRAMEWORK,
Linux Kernel Mailing List, open list:DOCUMENTATION, linux-kbuild,
Shuah Khan, Jonathan Corbet, Masahiro Yamada, Michal Marek,
Shreyans Devendra Doshi, Alan Maguire, Kevin Hilman,
Hidenori Yamaji, Frank Rowand, Timothy Bird, Luis Chamberlain,
Theodore Ts'o, Daniel Vetter, Stephen Boyd
In-Reply-To: <f0e1a6efa8f21ab93935c4c19e13b904d4a71f98.camel@oracle.com>
On Tue, Aug 13, 2019 at 2:51 AM Knut Omang <knut.omang@oracle.com> wrote:
>
> On Tue, 2019-08-13 at 10:23 +0200, Greg Kroah-Hartman wrote:
> > On Tue, Aug 13, 2019 at 08:09:15AM +0200, Knut Omang wrote:
> > > and in the making::
> > >
> > > kunit/ (kernel only (UML))
> >
> > You are going to have to integrate this with kunit, to come up with a
> > superset of both in the end.
>
> Yes, I agree - getting to a unified approach has been my intention since I first brought this
> up at LPC'17.
>
> > And I do not think that kunit is only UML, it's just that seems to be
> > what Brendan tests with, but should work with other arches as well.
>
> If I get Brendan right, it is UML only now but can be extended to also support
> kernels running on real hardware. Still it is kernel only, while KTF also has the
> hybrid mode, where a test can have code and assertions both in user mode and kernel mode.
> This is made easier and more streamlined by letting all reporting happen from user mode.
Nope, the KUnit patchset currently under review *does* support any
architecture; we have tested it on x86, ARM, and UML, but it should
work on any architecture.
I added support for that a while ago due to popular demand.
^ permalink raw reply
* Re: [PATCH v12 09/18] kunit: test: add support for test abort
From: Stephen Boyd @ 2019-08-13 17:06 UTC (permalink / raw)
To: Brendan Higgins
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, shuah,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Michael Ellerman, Petr Mladek, Randy Dunlap,
Richard Weinberger, David Rientjes, Steven Rostedt, wfg
In-Reply-To: <CAFd5g4415URtJBKPhsEw98GxiExJr-fstW6SQ6nmV9ts9ggK-g@mail.gmail.com>
Quoting Brendan Higgins (2019-08-13 00:52:03)
> On Mon, Aug 12, 2019 at 10:56 PM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Brendan Higgins (2019-08-12 21:57:55)
> > > On Mon, Aug 12, 2019 at 9:22 PM Stephen Boyd <sboyd@kernel.org> wrote:
> > > >
> > > > Quoting Brendan Higgins (2019-08-12 11:24:12)
> > > > > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > > > > index 2625bcfeb19ac..93381f841e09f 100644
> > > > > --- a/include/kunit/test.h
> > > > > +++ b/include/kunit/test.h
> > > > > @@ -184,6 +191,13 @@ struct kunit {
> > > > > struct list_head resources; /* Protected by lock. */
> > > > > };
> > > > >
> > > > > +static inline void kunit_set_death_test(struct kunit *test, bool death_test)
> > > > > +{
> > > > > + spin_lock(&test->lock);
> > > > > + test->death_test = death_test;
> > > > > + spin_unlock(&test->lock);
> > > > > +}
> > > >
> > > > These getters and setters are using spinlocks again. It doesn't make any
> > > > sense. It probably needs a rework like was done for the other bool
> > > > member, success.
> > >
> > > No, this is intentional. death_test can transition from false to true
> > > and then back to false within the same test. Maybe that deserves a
> > > comment?
> >
> > Yes. How does it transition from true to false again?
>
> The purpose is to tell try_catch that it was expected for the test to
> bail out. Given the default implementation there is no way for this to
> happen aside from abort() being called, but in some implementations it
> is possible to implement a fault catcher which allows a test suite to
> recover from an unexpected failure.
>
> Maybe it would be best to drop this until I add one of those
> alternative implementations.
Ok.
>
> > Either way, having a spinlock around a read/write API doesn't make sense
> > because it just makes sure that two writes don't overlap, but otherwise
> > does nothing to keep things synchronized. For example a set to true
> > after a set to false when the two calls to set true or false aren't
> > synchronized means they can happen in any order. So I don't see how it
> > needs a spinlock. The lock needs to be one level higher.
>
> There shouldn't be any cases where one thread is trying to set it
> while another is trying to unset it. The thing I am worried about here
> is making sure all the cores see the write, and making sure no reads
> or writes get reordered before it. So I guess I just want a fence. So
> I take it I should probably have is a WRITE_ONCE here and READ_ONCE in
> the getter?
>
Are the gets and sets in program order? If so, WRITE_ONCE and READ_ONCE
aren't required. Otherwise, if it's possible for one thread to write it
and another to read it but the threads are ordered with some other
barrier like a completion or lock, then again the macros aren't needed.
It would be good to read memory-barriers.txt to understand when to use
the read/write macros.
^ permalink raw reply
* Re: [PATCH v12 12/18] kunit: test: add tests for KUnit managed resources
From: Stephen Boyd @ 2019-08-13 17:07 UTC (permalink / raw)
To: Brendan Higgins
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, shuah,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Michael Ellerman, Petr Mladek, Randy Dunlap,
Richard Weinberger, David Rientjes, Steven Rostedt, wfg,
Avinash Kondareddy
In-Reply-To: <CAFd5g44Es4emKyQSxUkqckGJ02_o3sAcDLwUCW8ZFGX14j5=xg@mail.gmail.com>
Quoting Brendan Higgins (2019-08-13 00:57:33)
> On Mon, Aug 12, 2019 at 9:31 PM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > BTW, maybe kunit allocation APIs should
> > fail the test if they fail to allocate in general. Unless we're unit
> > testing failure to allocate problems.
>
> Yeah, I thought about that. I wasn't sure how people would feel about
> it, and I thought it would be a pain to tease out all the issues
> arising from aborting in different contexts when someone might not
> expect it.
>
> I am thinking later we can have kunit_kmalloc_or_abort variants? And
> then we can punt this issue to a later time?
>
Sure. Sounds good.
^ permalink raw reply
* Re: [RFC 00/19] Integration of Kernel Test Framework (KTF) into the kernel tree
From: Brendan Higgins @ 2019-08-13 17:50 UTC (permalink / raw)
To: Knut Omang
Cc: open list:KERNEL SELFTEST FRAMEWORK, Linux Kernel Mailing List,
open list:DOCUMENTATION, linux-kbuild, Shuah Khan,
Jonathan Corbet, Masahiro Yamada, Michal Marek,
Greg Kroah-Hartman, Shreyans Devendra Doshi, Alan Maguire,
Kevin Hilman, Hidenori Yamaji, Frank Rowand, Timothy Bird,
Luis Chamberlain, Theodore Ts'o, Daniel Vetter, Stephen Boyd
In-Reply-To: <b4d2cf8635103e30313773761b095f3a1cfd9a82.camel@oracle.com>
On Tue, Aug 13, 2019 at 4:29 AM Knut Omang <knut.omang@oracle.com> wrote:
>
> On Tue, 2019-08-13 at 01:17 -0700, Brendan Higgins wrote:
> > On Mon, Aug 12, 2019 at 11:11 PM Knut Omang <knut.omang@oracle.com> wrote:
> > [...]
> > > Alan Maguire (3):
> > > ktf: Implementation of ktf support for overriding function entry and return.
> > > ktf: A simple debugfs interface to test results
> > > ktf: Simple coverage support
> > >
> > > Knut Omang (16):
> > > kbuild: Fixes to rules for host-cshlib and host-cxxshlib
> > > ktf: Introduce the main part of the kernel side of ktf
> > > ktf: Introduce a generic netlink protocol for test result communication
> > > ktf: An implementation of a generic associative array container
> > > ktf: Configurable context support for network info setup
> > > ktf: resolve: A helper utility to aid in exposing private kernel symbols to KTF tests.
> > > ktf: Add documentation for Kernel Test Framework (KTF)
> > > ktf: Add a small test suite with a few tests to test KTF itself
> > > ktf: Main part of user land library for executing tests
> > > ktf: Integration logic for running ktf tests from googletest
> > > ktf: Internal debugging facilities
> > > ktf: Some simple examples
> > > ktf: Some user applications to run tests
> > > ktf: Toplevel ktf Makefile/makefile includes and scripts to run from kselftest
> > > kselftests: Enable building ktf
> > > Documentation/dev-tools: Add index entry for KTF documentation
> > >
> > > Documentation/dev-tools/index.rst | 1 +-
> > > Documentation/dev-tools/ktf/concepts.rst | 242 +++-
> > > Documentation/dev-tools/ktf/debugging.rst | 248 +++-
> > > Documentation/dev-tools/ktf/examples.rst | 26 +-
> > > Documentation/dev-tools/ktf/features.rst | 307 ++++-
> > > Documentation/dev-tools/ktf/implementation.rst | 70 +-
> > > Documentation/dev-tools/ktf/index.rst | 14 +-
> > > Documentation/dev-tools/ktf/installation.rst | 73 +-
> > > Documentation/dev-tools/ktf/introduction.rst | 134 ++-
> > > Documentation/dev-tools/ktf/progref.rst | 144 ++-
> > > scripts/Makefile.host | 17 +-
> > > tools/testing/selftests/Makefile | 1 +-
> > > tools/testing/selftests/ktf/Makefile | 21 +-
> > > tools/testing/selftests/ktf/examples/Makefile | 17 +-
> > > tools/testing/selftests/ktf/examples/h2.c | 45 +-
> > > tools/testing/selftests/ktf/examples/h3.c | 84 +-
> > > tools/testing/selftests/ktf/examples/h4.c | 62 +-
> > > tools/testing/selftests/ktf/examples/hello.c | 38 +-
> > > tools/testing/selftests/ktf/examples/kgdemo.c | 61 +-
> > > tools/testing/selftests/ktf/kernel/Makefile | 15 +-
> > > tools/testing/selftests/ktf/kernel/ktf.h | 604 +++++++-
> > > tools/testing/selftests/ktf/kernel/ktf_context.c | 409 +++++-
> > > tools/testing/selftests/ktf/kernel/ktf_cov.c | 690 ++++++++-
> > > tools/testing/selftests/ktf/kernel/ktf_cov.h | 94 +-
> > > tools/testing/selftests/ktf/kernel/ktf_debugfs.c | 356 ++++-
> > > tools/testing/selftests/ktf/kernel/ktf_debugfs.h | 34 +-
> > > tools/testing/selftests/ktf/kernel/ktf_map.c | 261 +++-
> > > tools/testing/selftests/ktf/kernel/ktf_map.h | 154 ++-
> > > tools/testing/selftests/ktf/kernel/ktf_netctx.c | 132 ++-
> > > tools/testing/selftests/ktf/kernel/ktf_netctx.h | 64 +-
> > > tools/testing/selftests/ktf/kernel/ktf_nl.c | 516 ++++++-
> > > tools/testing/selftests/ktf/kernel/ktf_nl.h | 15 +-
> > > tools/testing/selftests/ktf/kernel/ktf_override.c | 45 +-
> > > tools/testing/selftests/ktf/kernel/ktf_override.h | 15 +-
> > > tools/testing/selftests/ktf/kernel/ktf_test.c | 397 +++++-
> > > tools/testing/selftests/ktf/kernel/ktf_test.h | 381 ++++-
> > > tools/testing/selftests/ktf/kernel/ktf_unlproto.h | 105 +-
> > > tools/testing/selftests/ktf/lib/Makefile | 21 +-
> > > tools/testing/selftests/ktf/lib/ktf.h | 114 +-
> > > tools/testing/selftests/ktf/lib/ktf_debug.cc | 20 +-
> > > tools/testing/selftests/ktf/lib/ktf_debug.h | 59 +-
> > > tools/testing/selftests/ktf/lib/ktf_int.cc | 1031 ++++++++++++-
> > > tools/testing/selftests/ktf/lib/ktf_int.h | 84 +-
> > > tools/testing/selftests/ktf/lib/ktf_run.cc | 177 ++-
> > > tools/testing/selftests/ktf/lib/ktf_unlproto.c | 21 +-
> > > tools/testing/selftests/ktf/scripts/ktf_syms.mk | 16 +-
> > > tools/testing/selftests/ktf/scripts/resolve | 188 ++-
> > > tools/testing/selftests/ktf/scripts/runtests.mk | 3 +-
> > > tools/testing/selftests/ktf/scripts/runtests.sh | 100 +-
> > > tools/testing/selftests/ktf/scripts/top_make.mk | 14 +-
> > > tools/testing/selftests/ktf/selftest/Makefile | 17 +-
> > > tools/testing/selftests/ktf/selftest/context.c | 149 ++-
> > > tools/testing/selftests/ktf/selftest/context.h | 15 +-
> > > tools/testing/selftests/ktf/selftest/context_self.h | 34 +-
> > > tools/testing/selftests/ktf/selftest/hybrid.c | 35 +-
> > > tools/testing/selftests/ktf/selftest/hybrid.h | 24 +-
> > > tools/testing/selftests/ktf/selftest/hybrid_self.h | 27 +-
> > > tools/testing/selftests/ktf/selftest/ktf_syms.txt | 17 +-
> > > tools/testing/selftests/ktf/selftest/self.c | 661 ++++++++-
> > > tools/testing/selftests/ktf/user/Makefile | 26 +-
> > > tools/testing/selftests/ktf/user/hybrid.cc | 39 +-
> > > tools/testing/selftests/ktf/user/ktfcov.cc | 68 +-
> > > tools/testing/selftests/ktf/user/ktfrun.cc | 20 +-
> > > tools/testing/selftests/ktf/user/ktftest.cc | 46 +-
> > > 64 files changed, 8909 insertions(+), 9 deletions(-)
> >
> > It also looks like all your test code lives outside of the kernel
> > source dir. I take it you intend for tests to live in
> > tools/testing/selftests/ktf/ ?
>
> Good point,
> with this RFC it would be just to add another directory under ktf/
> but this was just to find a simple way to integrate it below selftests,
> without interfering with the current structure.
>
> I imagine a tighter integration/unification between normal Kbuild targets and selftests
> targets that also took kernel module building into consideration would be a better solution.
I think tests should live alongside the code that they test, so if I
understand what you are saying, then I agree. Where do you think KTF
should go then?
> I think this is a good topic for discussion.
> As I indicate above, I think it is problematic that test code has to be explicitly
> configured in as we configure code features of the kernel, which changes the "logical"
> kernel we build.
Yep, Luis Chamberlain and I have been discussing this for a while (I
should probably try to open up that conversation and add you to it);
it's a very tricky problem. I think that in the long term a good goal
is to have a way to express code dependencies separate from the
configuration system, but that would probably mean some substantial
changes to Kbuild and friends. Even then, we have all these macros
that generate different code (not just on/off) depending on how the
kernel is configured.
> So some more "native" support for test modules are desired, IMHO.
What do you mean by "native"? Just having the tests be more aware of the code?
^ permalink raw reply
* [PATCH] devicetree: Expose dtbs_check and dt_binding_check some more
From: Stephen Boyd @ 2019-08-13 18:38 UTC (permalink / raw)
To: Rob Herring
Cc: linux-kernel, Masahiro Yamada, linux-kbuild, devicetree,
linux-doc
It wasn't obvious that this was a command to run based on 'make help',
so add it to the top-level help for devicetree builds. Also, add an
example to the documentation to show that db_binding_check can be run
with DT_SCHEMA_FILES= to only check one schema file instead of all of
them.
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: <linux-kbuild@vger.kernel.org>
Cc: <devicetree@vger.kernel.org>
Cc: <linux-doc@vger.kernel.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
I didn't find this sent, so sending again!
Documentation/devicetree/writing-schema.md | 1 +
Makefile | 6 ++++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/writing-schema.md b/Documentation/devicetree/writing-schema.md
index dc032db36262..17ad67887fde 100644
--- a/Documentation/devicetree/writing-schema.md
+++ b/Documentation/devicetree/writing-schema.md
@@ -120,6 +120,7 @@ This will first run the `dt_binding_check` which generates the processed schema.
It is also possible to run checks with a single schema file by setting the
'DT_SCHEMA_FILES' variable to a specific schema file.
+`make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/trivial-devices.yaml`
`make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/trivial-devices.yaml`
diff --git a/Makefile b/Makefile
index 9be5834073f8..96bb28aa1c46 100644
--- a/Makefile
+++ b/Makefile
@@ -1503,8 +1503,10 @@ help:
@echo ''
@$(if $(dtstree), \
echo 'Devicetree:'; \
- echo '* dtbs - Build device tree blobs for enabled boards'; \
- echo ' dtbs_install - Install dtbs to $(INSTALL_DTBS_PATH)'; \
+ echo '* dtbs - Build device tree blobs for enabled boards'; \
+ echo ' dtbs_install - Install dtbs to $(INSTALL_DTBS_PATH)'; \
+ echo ' dt_binding_check - Validate device tree binding documents'; \
+ echo ' dtbs_check - Validate device tree source files';\
echo '')
@echo 'Userspace tools targets:'
--
Sent by a computer through tubes
^ permalink raw reply related
* Re: [PATCH v3 1/2] rcu/tree: Add basic support for kfree_rcu batching
From: Paul E. McKenney @ 2019-08-13 19:07 UTC (permalink / raw)
To: Joel Fernandes (Google)
Cc: linux-kernel, Rao Shoaib, max.byungchul.park, byungchul.park,
kernel-team, kernel-team, Andrew Morton, Davidlohr Bueso,
Jonathan Corbet, Josh Triplett, Kees Cook, Lai Jiangshan,
linux-doc, Mathieu Desnoyers, Mauro Carvalho Chehab, rcu,
Steven Rostedt, Thomas Gleixner
In-Reply-To: <20190813170046.81707-1-joel@joelfernandes.org>
On Tue, Aug 13, 2019 at 01:00:45PM -0400, Joel Fernandes (Google) wrote:
> Recently a discussion about stability and performance of a system
> involving a high rate of kfree_rcu() calls surfaced on the list [1]
> which led to another discussion how to prepare for this situation.
Looks much improved! More questions and comments inline.
Thanx, Paul
> This patch adds basic batching support for kfree_rcu(). It is "basic"
> because we do none of the slab management, dynamic allocation, code
> moving or any of the other things, some of which previous attempts did
> [2]. These fancier improvements can be follow-up patches and there are
> different ideas being discussed in those regards.
>
> Torture tests follow in the next patch and show improvements of around
> 400% in reduction of number of grace periods on a 16 CPU system. More
s/400% in reduction/a 5x reduction/
Oddly enough, a reduction of more than 100% would require a negative
number of grace periods. RCU being what it is, although this seems
unlikely, I hesitate to rule it out. ;-)
> details and test data are in that patch.
>
> This is an effort to start simple, and build up from there. In the
> future, an extension to use kfree_bulk and possibly per-slab batching
> could be done to further improve performance due to cache-locality and
> slab-specific bulk free optimizations. By using an array of pointers,
> the worker thread processing the work would need to read lesser data
> since it does not need to deal with large rcu_head(s) any longer.
This should be combined with the second paragraph -- they both discuss
possible follow-on work.
> There is an implication with rcu_barrier() with this patch. Since the
> kfree_rcu() calls can be batched, and may not be handed yet to the RCU
> machinery in fact, the monitor may not have even run yet to do the
> queue_rcu_work(), there seems no easy way of implementing rcu_barrier()
> to wait for those kfree_rcu()s that are already made. So this means a
> kfree_rcu() followed by an rcu_barrier() does not imply that memory will
> be freed once rcu_barrier() returns.
The usual approach (should kfree_rcu_barrier() in fact be needed) is to
record the address of the most recent block being kfree_rcu()'d on
each CPU, count the number of recorded addresses, and have the
kfree() side check the address, and do the usual atomic_dec_and_test()
and wakeup dance. This gets a bit more crazy should the kfree_rcu()
requests be grouped per slab, of course! So yes, good to avoid in
the meantime.
> Another implication is higher active memory usage (although not
> run-away..) until the kfree_rcu() flooding ends, in comparison to
> without batching. More details about this are in the second patch which
> adds an rcuperf test.
But this is adjustable, at least via patching at build time, via
KFREE_DRAIN_JIFFIES, right?
> Finally, in the near future we will get rid of kfree_rcu special casing
> within RCU such as in rcu_do_batch and switch everything to just
> batching. Currently we don't do that since timer subsystem is not yet up
> and we cannot schedule the kfree_rcu() monitor as the timer subsystem's
> lock are not initialized. That would also mean getting rid of
> kfree_call_rcu_nobatch() entirely.
Please consistently put "()" after function names.
> [1] http://lore.kernel.org/lkml/20190723035725-mutt-send-email-mst@kernel.org
> [2] https://lkml.org/lkml/2017/12/19/824
>
> Cc: Rao Shoaib <rao.shoaib@oracle.com>
> Cc: max.byungchul.park@gmail.com
> Cc: byungchul.park@lge.com
> Cc: kernel-team@android.com
> Cc: kernel-team@lge.com
> Co-developed-by: Byungchul Park <byungchul.park@lge.com>
> Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
>
> ---
> v2->v3: Just some code comment changes thanks to Byungchul.
>
> RFCv1->PATCH v2: Removed limits on the ->head list, just let it grow.
> Dropped KFREE_MAX_JIFFIES to HZ/50 from HZ/20 to reduce OOM occurrence.
> Removed sleeps in rcuperf test, just using cond_resched()in loop.
> Better code comments ;)
>
> .../admin-guide/kernel-parameters.txt | 17 ++
> include/linux/rcutiny.h | 5 +
> include/linux/rcutree.h | 1 +
> kernel/rcu/tree.c | 204 +++++++++++++++++-
> 4 files changed, 221 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 7ccd158b3894..a9156ca5de24 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3895,6 +3895,23 @@
> test until boot completes in order to avoid
> interference.
>
> + rcuperf.kfree_rcu_test= [KNL]
> + Set to measure performance of kfree_rcu() flooding.
> +
> + rcuperf.kfree_nthreads= [KNL]
> + The number of threads running loops of kfree_rcu().
> +
> + rcuperf.kfree_alloc_num= [KNL]
> + Number of allocations and frees done in an iteration.
> +
> + rcuperf.kfree_loops= [KNL]
> + Number of loops doing rcuperf.kfree_alloc_num number
> + of allocations and frees.
> +
> + rcuperf.kfree_no_batch= [KNL]
> + Use the non-batching (slower) version of kfree_rcu.
> + This is useful for comparing with the batched version.
> +
> rcuperf.nreaders= [KNL]
> Set number of RCU readers. The value -1 selects
> N, where N is the number of CPUs. A value
This change to kernel-parameters.txt really needs to be in the
rcuperf patch rather than this one.
> diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
> index 8e727f57d814..383f2481750f 100644
> --- a/include/linux/rcutiny.h
> +++ b/include/linux/rcutiny.h
> @@ -39,6 +39,11 @@ static inline void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> call_rcu(head, func);
> }
>
> +static inline void kfree_call_rcu_nobatch(struct rcu_head *head, rcu_callback_t func)
> +{
> + call_rcu(head, func);
> +}
> +
> void rcu_qs(void);
>
> static inline void rcu_softirq_qs(void)
> diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
> index 735601ac27d3..7e38b39ec634 100644
> --- a/include/linux/rcutree.h
> +++ b/include/linux/rcutree.h
> @@ -34,6 +34,7 @@ static inline void rcu_virt_note_context_switch(int cpu)
>
> void synchronize_rcu_expedited(void);
> void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
> +void kfree_call_rcu_nobatch(struct rcu_head *head, rcu_callback_t func);
>
> void rcu_barrier(void);
> bool rcu_eqs_special_set(int cpu);
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index a14e5fbbea46..3e62bbb0e1fa 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -2593,17 +2593,195 @@ void call_rcu(struct rcu_head *head, rcu_callback_t func)
> }
> EXPORT_SYMBOL_GPL(call_rcu);
>
> +
> +/* Maximum number of jiffies to wait before draining a batch. */
> +#define KFREE_DRAIN_JIFFIES (HZ / 50)
> +
> /*
> - * Queue an RCU callback for lazy invocation after a grace period.
> - * This will likely be later named something like "call_rcu_lazy()",
> - * but this change will require some way of tagging the lazy RCU
> - * callbacks in the list of pending callbacks. Until then, this
> - * function may only be called from __kfree_rcu().
> + * Maximum number of kfree(s) to batch, if this limit is hit then the batch of
> + * kfree(s) is queued for freeing after a grace period, right away.
> */
> -void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> +struct kfree_rcu_cpu {
> + /* The rcu_work node for queuing work with queue_rcu_work(). The work
> + * is done after a grace period.
> + */
> + struct rcu_work rcu_work;
> +
> + /* The list of objects being queued in a batch but are not yet
> + * scheduled to be freed.
> + */
> + struct rcu_head *head;
> +
> + /* The list of objects that have now left ->head and are queued for
> + * freeing after a grace period.
> + */
> + struct rcu_head *head_free;
So this is not yet the one that does multiple batches concurrently
awaiting grace periods, correct? Or am I missing something subtle?
If it is not doing the multiple-batch optimization, what does the
400% in the commit log refer to?
> + /* Protect concurrent access to this structure. */
> + spinlock_t lock;
> +
> + /* The delayed work that flushes ->head to ->head_free incase ->head
> + * within KFREE_DRAIN_JIFFIES. In case flushing cannot be done if RCU
> + * is busy, ->head just continues to grow and we retry flushing later.
> + */
> + struct delayed_work monitor_work;
> + bool monitor_todo; /* Is a delayed work pending execution? */
> +};
> +
> +static DEFINE_PER_CPU(struct kfree_rcu_cpu, krc);
> +
> +/*
> + * This function is invoked in workqueue context after a grace period.
> + * It frees all the objects queued on ->head_free.
> + */
> +static void kfree_rcu_work(struct work_struct *work)
> +{
> + unsigned long flags;
> + struct rcu_head *head, *next;
> + struct kfree_rcu_cpu *krcp = container_of(to_rcu_work(work),
> + struct kfree_rcu_cpu, rcu_work);
> +
> + spin_lock_irqsave(&krcp->lock, flags);
> + head = krcp->head_free;
> + krcp->head_free = NULL;
> + spin_unlock_irqrestore(&krcp->lock, flags);
> +
> + /*
> + * The head is detached and not referenced from anywhere, so lockless
> + * access is Ok.
> + */
> + for (; head; head = next) {
> + next = head->next;
> + head->next = NULL;
Why do we need to NULL ->next? Could produce an expensive cache miss,
and I don't see how it helps anything. What am I missing here?
> + /* Could be possible to optimize with kfree_bulk in future */
> + __rcu_reclaim(rcu_state.name, head);
> + cond_resched_tasks_rcu_qs();
> + }
> +}
> +
> +/*
> + * Schedule the kfree batch RCU work to run in workqueue context after a GP.
> + *
> + * This function is invoked by kfree_rcu_monitor() when the KFREE_DRAIN_JIFFIES
> + * timeout has been reached.
> + */
> +static inline bool queue_kfree_rcu_work(struct kfree_rcu_cpu *krcp)
> +{
> + lockdep_assert_held(&krcp->lock);
> +
> + /* If a previous RCU batch work is already in progress, we cannot queue
> + * another one, just refuse the optimization and it will be retried
> + * again in KFREE_DRAIN_JIFFIES time.
> + */
> + if (krcp->head_free)
> + return false;
> +
> + krcp->head_free = krcp->head;
> + krcp->head = NULL;
> + INIT_RCU_WORK(&krcp->rcu_work, kfree_rcu_work);
> + queue_rcu_work(system_wq, &krcp->rcu_work);
> +
> + return true;
> +}
> +
> +static inline void kfree_rcu_drain_unlock(struct kfree_rcu_cpu *krcp,
> + unsigned long flags)
> +{
> + /* Flush ->head to ->head_free, all objects on ->head_free will be
> + * kfree'd after a grace period.
> + */
> + if (queue_kfree_rcu_work(krcp)) {
> + /* Success! Our job is done here. */
> + spin_unlock_irqrestore(&krcp->lock, flags);
> + return;
> + }
> +
> + /* Previous batch did not get free yet, let us try again soon. */
> + if (krcp->monitor_todo == false) {
> + schedule_delayed_work_on(smp_processor_id(),
> + &krcp->monitor_work, KFREE_DRAIN_JIFFIES);
Given that we are using a lock, do we need to restrict to the current
CPU? In any case, we do need to handle the case where the current
CPU has gone offline before we get around to sending a batch off to
wait for a grace period, correct?
> + krcp->monitor_todo = true;
> + }
> + spin_unlock_irqrestore(&krcp->lock, flags);
> +}
> +
> +/*
> + * This function is invoked after the KFREE_DRAIN_JIFFIES timeout has elapsed,
> + * so it drains the specified kfree_rcu_cpu structure's ->head list.
> + */
> +static void kfree_rcu_monitor(struct work_struct *work)
> +{
> + bool todo;
> + unsigned long flags;
> + struct kfree_rcu_cpu *krcp = container_of(work, struct kfree_rcu_cpu,
> + monitor_work.work);
> +
> + spin_lock_irqsave(&krcp->lock, flags);
> + todo = krcp->monitor_todo;
> + krcp->monitor_todo = false;
> + if (todo)
> + kfree_rcu_drain_unlock(krcp, flags);
> + else
> + spin_unlock_irqrestore(&krcp->lock, flags);
> +}
> +
> +/*
> + * This version of kfree_call_rcu does not do batching of kfree_rcu() requests.
> + * Used only by rcuperf torture test for comparison with kfree_rcu_batch().
> + */
> +void kfree_call_rcu_nobatch(struct rcu_head *head, rcu_callback_t func)
> {
> __call_rcu(head, func, -1, 1);
> }
> +EXPORT_SYMBOL_GPL(kfree_call_rcu_nobatch);
> +
> +/*
> + * Queue a request for lazy invocation of kfree() after a grace period.
> + *
> + * Each kfree_call_rcu() request is added to a batch. The batch will be drained
> + * every KFREE_DRAIN_JIFFIES number of jiffies. All the objects in the batch
> + * will be kfree'd in workqueue context. This allows us to:
> + *
> + * 1. Batch requests together to reduce the number of grace periods during
> + * heavy kfree_rcu() load.
> + *
> + * 2. In the future, makes it possible to use kfree_bulk() on a large number of
> + * kfree_rcu() requests thus reducing the per-object overhead of kfree() and
> + * also reducing cache misses.
> + */
> +void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> +{
> + unsigned long flags;
> + struct kfree_rcu_cpu *krcp;
> + bool monitor_todo;
> +
> + /* kfree_call_rcu() batching requires timers to be up. If the scheduler
> + * is not yet up, just skip batching and do non-batched kfree_call_rcu().
> + */
> + if (rcu_scheduler_active != RCU_SCHEDULER_RUNNING)
> + return kfree_call_rcu_nobatch(head, func);
> +
> + local_irq_save(flags);
> + krcp = this_cpu_ptr(&krc);
> +
> + spin_lock(&krcp->lock);
> +
> + /* Queue the kfree but don't yet schedule the batch. */
> + head->func = func;
> + head->next = krcp->head;
> + krcp->head = head;
> +
> + /* Schedule monitor for timely drain after KFREE_DRAIN_JIFFIES. */
> + monitor_todo = krcp->monitor_todo;
> + krcp->monitor_todo = true;
> + spin_unlock(&krcp->lock);
> +
> + if (!monitor_todo) {
> + schedule_delayed_work_on(smp_processor_id(),
> + &krcp->monitor_work, KFREE_DRAIN_JIFFIES);
> + }
> + local_irq_restore(flags);
> +}
> EXPORT_SYMBOL_GPL(kfree_call_rcu);
>
> /*
> @@ -3455,10 +3633,24 @@ static void __init rcu_dump_rcu_node_tree(void)
> struct workqueue_struct *rcu_gp_wq;
> struct workqueue_struct *rcu_par_gp_wq;
>
> +static void __init kfree_rcu_batch_init(void)
> +{
> + int cpu;
> +
> + for_each_possible_cpu(cpu) {
> + struct kfree_rcu_cpu *krcp = per_cpu_ptr(&krc, cpu);
> +
> + spin_lock_init(&krcp->lock);
> + INIT_DELAYED_WORK(&krcp->monitor_work, kfree_rcu_monitor);
> + }
> +}
> +
> void __init rcu_init(void)
> {
> int cpu;
>
> + kfree_rcu_batch_init();
> +
> rcu_early_boot_tests();
>
> rcu_bootup_announce();
> --
> 2.23.0.rc1.153.gdeed80330f-goog
>
^ permalink raw reply
* Re: [PATCH v5 1/6] mm/page_idle: Add per-pid idle page tracking using virtual index
From: Joel Fernandes @ 2019-08-13 19:18 UTC (permalink / raw)
To: Daniel Gruss
Cc: Jann Horn, Michal Hocko, kernel list, Alexey Dobriyan,
Andrew Morton, Borislav Petkov, Brendan Gregg, Catalin Marinas,
Christian Hansen, Daniel Colascione, fmayer, H. Peter Anvin,
Ingo Molnar, Jonathan Corbet, Kees Cook, kernel-team, Linux API,
linux-doc, linux-fsdevel, Linux-MM, Mike Rapoport, Minchan Kim,
namhyung, Paul E. McKenney, Robin Murphy, Roman Gushchin,
Stephen Rothwell, Suren Baghdasaryan, Thomas Gleixner, Todd Kjos,
Vladimir Davydov, Vlastimil Babka, Will Deacon
In-Reply-To: <d6ae7f06-f0ef-ec00-a020-98e7cfada281@iaik.tugraz.at>
On Tue, Aug 13, 2019 at 05:34:16PM +0200, Daniel Gruss wrote:
> On 8/13/19 5:29 PM, Jann Horn wrote:
> > On Tue, Aug 13, 2019 at 12:09 PM Michal Hocko <mhocko@kernel.org> wrote:
> >> On Mon 12-08-19 20:14:38, Jann Horn wrote:
> >>> On Wed, Aug 7, 2019 at 7:16 PM Joel Fernandes (Google)
> >>> <joel@joelfernandes.org> wrote:
> >>>> The page_idle tracking feature currently requires looking up the pagemap
> >>>> for a process followed by interacting with /sys/kernel/mm/page_idle.
> >>>> Looking up PFN from pagemap in Android devices is not supported by
> >>>> unprivileged process and requires SYS_ADMIN and gives 0 for the PFN.
> >>>>
> >>>> This patch adds support to directly interact with page_idle tracking at
> >>>> the PID level by introducing a /proc/<pid>/page_idle file. It follows
> >>>> the exact same semantics as the global /sys/kernel/mm/page_idle, but now
> >>>> looking up PFN through pagemap is not needed since the interface uses
> >>>> virtual frame numbers, and at the same time also does not require
> >>>> SYS_ADMIN.
> >>>>
> >>>> In Android, we are using this for the heap profiler (heapprofd) which
> >>>> profiles and pin points code paths which allocates and leaves memory
> >>>> idle for long periods of time. This method solves the security issue
> >>>> with userspace learning the PFN, and while at it is also shown to yield
> >>>> better results than the pagemap lookup, the theory being that the window
> >>>> where the address space can change is reduced by eliminating the
> >>>> intermediate pagemap look up stage. In virtual address indexing, the
> >>>> process's mmap_sem is held for the duration of the access.
> >>>
> >>> What happens when you use this interface on shared pages, like memory
> >>> inherited from the zygote, library file mappings and so on? If two
> >>> profilers ran concurrently for two different processes that both map
> >>> the same libraries, would they end up messing up each other's data?
> >>
> >> Yup PageIdle state is shared. That is the page_idle semantic even now
> >> IIRC.
> >>
> >>> Can this be used to observe which library pages other processes are
> >>> accessing, even if you don't have access to those processes, as long
> >>> as you can map the same libraries? I realize that there are already a
> >>> bunch of ways to do that with side channels and such; but if you're
> >>> adding an interface that allows this by design, it seems to me like
> >>> something that should be gated behind some sort of privilege check.
> >>
> >> Hmm, you need to be priviledged to get the pfn now and without that you
> >> cannot get to any page so the new interface is weakening the rules.
> >> Maybe we should limit setting the idle state to processes with the write
> >> status. Or do you think that even observing idle status is useful for
> >> practical side channel attacks? If yes, is that a problem of the
> >> profiler which does potentially dangerous things?
> >
> > I suppose read-only access isn't a real problem as long as the
> > profiler isn't writing the idle state in a very tight loop... but I
> > don't see a usecase where you'd actually want that? As far as I can
> > tell, if you can't write the idle state, being able to read it is
> > pretty much useless.
> >
> > If the profiler only wants to profile process-private memory, then
> > that should be implementable in a safe way in principle, I think, but
> > since Joel said that they want to profile CoW memory as well, I think
> > that's inherently somewhat dangerous.
>
> I agree that allowing profiling of shared pages would leak information.
Will think more about it. If we limit it to private pages, then it could
become useless. Consider a scenario where:
A process allocates a some memory, then forks a bunch of worker processes
that read that memory and perform some work with them. Per-PID page idle
tracking is now run on the parent processes. Now it should appear that the
pages are actively accessed (not-idle). If we don't track shared pages, then
we cannot detect if those pages are really due to memory leaking, or if they
are there for a purpose and are actively used.
> To me the use case is not entirely clear. This is not a feature that
> would normally be run in everyday computer usage, right?
Generally, this to be used as a debugging feature that helps developers
detect memory leaks in their programs.
thanks,
- Joel
^ permalink raw reply
* Re: [PATCH v5 2/6] mm/page_idle: Add support for handling swapped PG_Idle pages
From: Konstantin Khlebnikov @ 2019-08-13 19:24 UTC (permalink / raw)
To: Joel Fernandes
Cc: Michal Hocko,
Константин Хлебников,
Linux Kernel Mailing List, Minchan Kim, Alexey Dobriyan,
Andrew Morton, Borislav Petkov, Brendan Gregg, Catalin Marinas,
Christian Hansen, dancol, fmayer, H. Peter Anvin, Ingo Molnar,
Jonathan Corbet, Kees Cook, kernel-team, Linux API, linux-doc,
linux-fsdevel, linux-mm, Mike Rapoport, namhyung, paulmck,
Robin Murphy, Roman Gushchin, Stephen Rothwell, surenb,
Thomas Gleixner, tkjos, Vladimir Davydov, Vlastimil Babka,
Will Deacon
In-Reply-To: <20190813153659.GD14622@google.com>
On Tue, Aug 13, 2019 at 6:37 PM Joel Fernandes <joel@joelfernandes.org> wrote:
>
> On Tue, Aug 13, 2019 at 05:04:50PM +0200, Michal Hocko wrote:
> > On Wed 07-08-19 13:15:55, Joel Fernandes (Google) wrote:
> > > Idle page tracking currently does not work well in the following
> > > scenario:
> > > 1. mark page-A idle which was present at that time.
> > > 2. run workload
> > > 3. page-A is not touched by workload
> > > 4. *sudden* memory pressure happen so finally page A is finally swapped out
> > > 5. now see the page A - it appears as if it was accessed (pte unmapped
> > > so idle bit not set in output) - but it's incorrect.
> > >
> > > To fix this, we store the idle information into a new idle bit of the
> > > swap PTE during swapping of anonymous pages.
> > >
> > > Also in the future, madvise extensions will allow a system process
> > > manager (like Android's ActivityManager) to swap pages out of a process
> > > that it knows will be cold. To an external process like a heap profiler
> > > that is doing idle tracking on another process, this procedure will
> > > interfere with the idle page tracking similar to the above steps.
> >
> > This could be solved by checking the !present/swapped out pages
> > right? Whoever decided to put the page out to the swap just made it
> > idle effectively. So the monitor can make some educated guess for
> > tracking. If that is fundamentally not possible then please describe
> > why.
>
> But the monitoring process (profiler) does not have control over the 'whoever
> made it effectively idle' process.
>
> As you said it will be a guess, it will not be accurate.
Yep. Without saving idle bit in swap entry (and presuming that all swap is idle)
profiler could miss access. This patch adds accurate tracking almost for free.
After that profiler could work with any pace without races.
>
> I am curious what is your concern with using a bit in the swap PTE?
>
> (Adding Konstantin as well since we may be interested in this, since we also
> suggested this idea).
>
> thanks,
>
> - Joel
>
>
^ permalink raw reply
* [PATCH v8 00/27] Control-flow Enforcement: Shadow Stack
From: Yu-cheng Yu @ 2019-08-13 20:51 UTC (permalink / raw)
To: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
Cc: Yu-cheng Yu
Intel has published Control-flow Enforcement (CET) in the Architecture
Instruction Set Extensions Programming Reference:
https://software.intel.com/en-us/download/intel-architecture-instruction-set-
extensions-programming-reference
The previous version (v7) of CET Shadow Stack patches is here:
https://lkml.org/lkml/2019/6/6/1003
Summary of changes from v7:
Rewrite ELF GNU property parsing (Patch #22). Look at PT_GNU_PROPERTY now.
Rebase to v5.3-rc4.
Small fixes in response to comments.
Yu-cheng Yu (27):
Documentation/x86: Add CET description
x86/cpufeatures: Add CET CPU feature flags for Control-flow
Enforcement Technology (CET)
x86/fpu/xstate: Change names to separate XSAVES system and user states
x86/fpu/xstate: Introduce XSAVES system states
x86/fpu/xstate: Introduce CET MSR system states
x86/cet: Add control protection exception handler
x86/cet/shstk: Add Kconfig option for user-mode shadow stack
mm: Introduce VM_SHSTK for shadow stack memory
mm/mmap: Prevent Shadow Stack VMA merges
x86/mm: Change _PAGE_DIRTY to _PAGE_DIRTY_HW
x86/mm: Introduce _PAGE_DIRTY_SW
drm/i915/gvt: Update _PAGE_DIRTY to _PAGE_DIRTY_BITS
x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for
_PAGE_DIRTY_SW
x86/mm: Shadow stack page fault error checking
mm: Handle shadow stack page fault
mm: Handle THP/HugeTLB shadow stack page fault
mm: Update can_follow_write_pte/pmd for shadow stack
mm: Introduce do_mmap_locked()
x86/cet/shstk: User-mode shadow stack support
x86/cet/shstk: Introduce WRUSS instruction
x86/cet/shstk: Handle signals for shadow stack
binfmt_elf: Extract .note.gnu.property from an ELF file
x86/cet/shstk: ELF header parsing of Shadow Stack
x86/cet/shstk: Handle thread shadow stack
mm/mmap: Add Shadow stack pages to memory accounting
x86/cet/shstk: Add arch_prctl functions for Shadow Stack
x86/cet/shstk: Add Shadow Stack instructions to opcode map
.../admin-guide/kernel-parameters.txt | 6 +
Documentation/x86/index.rst | 1 +
Documentation/x86/intel_cet.rst | 269 ++++++++++++++
arch/x86/Kconfig | 27 ++
arch/x86/Makefile | 7 +
arch/x86/entry/entry_64.S | 2 +-
arch/x86/ia32/ia32_signal.c | 8 +
arch/x86/include/asm/cet.h | 48 +++
arch/x86/include/asm/cpufeatures.h | 2 +
arch/x86/include/asm/disabled-features.h | 8 +-
arch/x86/include/asm/elf.h | 13 +
arch/x86/include/asm/fpu/internal.h | 27 +-
arch/x86/include/asm/fpu/signal.h | 2 +
arch/x86/include/asm/fpu/types.h | 22 ++
arch/x86/include/asm/fpu/xstate.h | 26 +-
arch/x86/include/asm/mmu_context.h | 3 +
arch/x86/include/asm/msr-index.h | 18 +
arch/x86/include/asm/pgtable.h | 191 ++++++++--
arch/x86/include/asm/pgtable_types.h | 38 +-
arch/x86/include/asm/processor.h | 5 +
arch/x86/include/asm/special_insns.h | 32 ++
arch/x86/include/asm/traps.h | 5 +
arch/x86/include/uapi/asm/prctl.h | 5 +
arch/x86/include/uapi/asm/processor-flags.h | 2 +
arch/x86/include/uapi/asm/sigcontext.h | 15 +
arch/x86/kernel/Makefile | 2 +
arch/x86/kernel/cet.c | 327 ++++++++++++++++++
arch/x86/kernel/cet_prctl.c | 85 +++++
arch/x86/kernel/cpu/common.c | 25 ++
arch/x86/kernel/cpu/cpuid-deps.c | 2 +
arch/x86/kernel/fpu/core.c | 26 +-
arch/x86/kernel/fpu/init.c | 10 -
arch/x86/kernel/fpu/signal.c | 81 ++++-
arch/x86/kernel/fpu/xstate.c | 169 +++++----
arch/x86/kernel/idt.c | 4 +
arch/x86/kernel/process.c | 8 +-
arch/x86/kernel/process_64.c | 41 +++
arch/x86/kernel/relocate_kernel_64.S | 2 +-
arch/x86/kernel/signal.c | 10 +-
arch/x86/kernel/signal_compat.c | 2 +-
arch/x86/kernel/traps.c | 57 +++
arch/x86/kvm/vmx/vmx.c | 2 +-
arch/x86/lib/x86-opcode-map.txt | 26 +-
arch/x86/mm/fault.c | 18 +
arch/x86/mm/pgtable.c | 41 +++
drivers/gpu/drm/i915/gvt/gtt.c | 2 +-
fs/Kconfig.binfmt | 3 +
fs/Makefile | 1 +
fs/binfmt_elf.c | 20 ++
fs/gnu_property.c | 178 ++++++++++
fs/proc/task_mmu.c | 3 +
include/asm-generic/pgtable.h | 33 ++
include/linux/elf.h | 11 +
include/linux/mm.h | 26 ++
include/uapi/asm-generic/siginfo.h | 3 +-
include/uapi/linux/elf.h | 14 +
mm/gup.c | 8 +-
mm/huge_memory.c | 12 +-
mm/memory.c | 7 +-
mm/mmap.c | 11 +
.../arch/x86/include/asm/disabled-features.h | 8 +-
tools/objtool/arch/x86/lib/x86-opcode-map.txt | 26 +-
62 files changed, 1920 insertions(+), 166 deletions(-)
create mode 100644 Documentation/x86/intel_cet.rst
create mode 100644 arch/x86/include/asm/cet.h
create mode 100644 arch/x86/kernel/cet.c
create mode 100644 arch/x86/kernel/cet_prctl.c
create mode 100644 fs/gnu_property.c
--
2.17.1
^ 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