* Re: use generic DMA mapping code in powerpc V4
From: Christoph Hellwig @ 2019-02-12 19:52 UTC (permalink / raw)
To: Christian Zigotzky
Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <f57e6d8f-d43f-e9f8-b092-e43f5019f86f@xenosoft.de>
Great! I owe you a night worth of beers at a conference or if
you come anywhere near Innsbruck!
^ permalink raw reply
* Re: use generic DMA mapping code in powerpc V4
From: Christian Zigotzky @ 2019-02-12 19:50 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev
In-Reply-To: <47bff9d1-7001-4d92-4ad1-e24215b56555@xenosoft.de>
On 12 February 2019 at 8:31PM, Christian Zigotzky wrote:
> On 12 February 2019 at 4:25PM, Christoph Hellwig wrote:
>> On Tue, Feb 12, 2019 at 01:42:56PM +0100, Christian Zigotzky wrote:
>>> On 11 February 2019 at 08:38AM, Christoph Hellwig wrote:
>>>> On Sun, Feb 10, 2019 at 01:00:20PM +0100, Christian Zigotzky wrote:
>>>>> I tested the whole series today. The kernels boot and the P.A. Semi
>>>>> Ethernet works! :-) Thanks a lot!
>>>>>
>>>>> I also tested it in a virtual e5500 QEMU machine today.
>>>>> Unfortunately the
>>>>> kernel crashes.
>>>> This looks like a patch I fixed in mainline a while ago, but which
>>>> the powerpc tree didn't have yet.
>>>>
>>>> I've cherry picked this commit
>>>> ("swiotlb: clear io_tlb_start and io_tlb_end in swiotlb_exit")
>>>>
>>>> and added it to the powerpc-dma.6 tree, please retry with that one.
>>>>
>>> Hello Christoph,
>>>
>>> Have you added it to the powerpc-dma.6 tree yet? The last commit was
>>> 4 days
>>> ago.
>> I added it, but forgot to push it out. It is there now, sorry:
>>
>> http://git.infradead.org/users/hch/misc.git/commitdiff/2cf0745b7420af4a3e871d5a970a45662dfae69c
>>
>>
> Hi Christoph
>
> Many thanks! Your Git kernel works in a virtual e5500 machine now! :-)
>
> I think we have reached the end of testing! All things are working
> with your DMA updates.
>
> I am looking forward to testing your DMA changes in the next merge
> window again. :-)
>
> Cheers
> Christian
>
>
Edit: typo
^ permalink raw reply
* Re: use generic DMA mapping code in powerpc V4
From: Christian Zigotzky @ 2019-02-12 19:31 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev
In-Reply-To: <20190212152543.GA24061@lst.de>
On 12 February 2019 at 4:25PM, Christoph Hellwig wrote:
> On Tue, Feb 12, 2019 at 01:42:56PM +0100, Christian Zigotzky wrote:
>> On 11 February 2019 at 08:38AM, Christoph Hellwig wrote:
>>> On Sun, Feb 10, 2019 at 01:00:20PM +0100, Christian Zigotzky wrote:
>>>> I tested the whole series today. The kernels boot and the P.A. Semi
>>>> Ethernet works! :-) Thanks a lot!
>>>>
>>>> I also tested it in a virtual e5500 QEMU machine today. Unfortunately the
>>>> kernel crashes.
>>> This looks like a patch I fixed in mainline a while ago, but which
>>> the powerpc tree didn't have yet.
>>>
>>> I've cherry picked this commit
>>> ("swiotlb: clear io_tlb_start and io_tlb_end in swiotlb_exit")
>>>
>>> and added it to the powerpc-dma.6 tree, please retry with that one.
>>>
>> Hello Christoph,
>>
>> Have you added it to the powerpc-dma.6 tree yet? The last commit was 4 days
>> ago.
> I added it, but forgot to push it out. It is there now, sorry:
>
> http://git.infradead.org/users/hch/misc.git/commitdiff/2cf0745b7420af4a3e871d5a970a45662dfae69c
>
Hi Christoph
Many thanks! Your Git kernel works in a virtual e5500 machine now! :-)
I think we have reached the end of testing! All things are working with
your DMA updates.
I am looking forward to test your DMA changes in the next merge window
again. :-)
Cheers
Christian
^ permalink raw reply
* Re: [PATCH 2/5] vfio/spapr_tce: use pinned_vm instead of locked_vm to account pinned pages
From: Alex Williamson @ 2019-02-12 18:56 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: dave, jack, kvm, linux-mm, linux-fpga, atull, linux-kernel,
kvm-ppc, Daniel Jordan, jgg, mdf, akpm, linuxppc-dev, cl, hao.wu
In-Reply-To: <ee4d14db-05c3-6208-503c-16e287fa78eb@ozlabs.ru>
On Tue, 12 Feb 2019 17:56:18 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> On 12/02/2019 09:44, Daniel Jordan wrote:
> > Beginning with bc3e53f682d9 ("mm: distinguish between mlocked and pinned
> > pages"), locked and pinned pages are accounted separately. The SPAPR
> > TCE VFIO IOMMU driver accounts pinned pages to locked_vm; use pinned_vm
> > instead.
> >
> > pinned_vm recently became atomic and so no longer relies on mmap_sem
> > held as writer: delete.
> >
> > Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
> > ---
> > Documentation/vfio.txt | 6 +--
> > drivers/vfio/vfio_iommu_spapr_tce.c | 64 ++++++++++++++---------------
> > 2 files changed, 33 insertions(+), 37 deletions(-)
> >
> > diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
> > index f1a4d3c3ba0b..fa37d65363f9 100644
> > --- a/Documentation/vfio.txt
> > +++ b/Documentation/vfio.txt
> > @@ -308,7 +308,7 @@ This implementation has some specifics:
> > currently there is no way to reduce the number of calls. In order to make
> > things faster, the map/unmap handling has been implemented in real mode
> > which provides an excellent performance which has limitations such as
> > - inability to do locked pages accounting in real time.
> > + inability to do pinned pages accounting in real time.
> >
> > 4) According to sPAPR specification, A Partitionable Endpoint (PE) is an I/O
> > subtree that can be treated as a unit for the purposes of partitioning and
> > @@ -324,7 +324,7 @@ This implementation has some specifics:
> > returns the size and the start of the DMA window on the PCI bus.
> >
> > VFIO_IOMMU_ENABLE
> > - enables the container. The locked pages accounting
> > + enables the container. The pinned pages accounting
> > is done at this point. This lets user first to know what
> > the DMA window is and adjust rlimit before doing any real job.
I don't know of a ulimit only covering pinned pages, so for
documentation it seems more correct to continue referring to this as
locked page accounting.
> > @@ -454,7 +454,7 @@ This implementation has some specifics:
> >
> > PPC64 paravirtualized guests generate a lot of map/unmap requests,
> > and the handling of those includes pinning/unpinning pages and updating
> > - mm::locked_vm counter to make sure we do not exceed the rlimit.
> > + mm::pinned_vm counter to make sure we do not exceed the rlimit.
> > The v2 IOMMU splits accounting and pinning into separate operations:
> >
> > - VFIO_IOMMU_SPAPR_REGISTER_MEMORY/VFIO_IOMMU_SPAPR_UNREGISTER_MEMORY ioctls
> > diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
> > index c424913324e3..f47e020dc5e4 100644
> > --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> > +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> > @@ -34,9 +34,11 @@
> > static void tce_iommu_detach_group(void *iommu_data,
> > struct iommu_group *iommu_group);
> >
> > -static long try_increment_locked_vm(struct mm_struct *mm, long npages)
> > +static long try_increment_pinned_vm(struct mm_struct *mm, long npages)
> > {
> > - long ret = 0, locked, lock_limit;
> > + long ret = 0;
> > + s64 pinned;
> > + unsigned long lock_limit;
> >
> > if (WARN_ON_ONCE(!mm))
> > return -EPERM;
> > @@ -44,39 +46,33 @@ static long try_increment_locked_vm(struct mm_struct *mm, long npages)
> > if (!npages)
> > return 0;
> >
> > - down_write(&mm->mmap_sem);
> > - locked = mm->locked_vm + npages;
> > + pinned = atomic64_add_return(npages, &mm->pinned_vm);
> > lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> > - if (locked > lock_limit && !capable(CAP_IPC_LOCK))
> > + if (pinned > lock_limit && !capable(CAP_IPC_LOCK)) {
> > ret = -ENOMEM;
> > - else
> > - mm->locked_vm += npages;
> > + atomic64_sub(npages, &mm->pinned_vm);
> > + }
> >
> > - pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%ld%s\n", current->pid,
> > + pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%lu%s\n", current->pid,
> > npages << PAGE_SHIFT,
> > - mm->locked_vm << PAGE_SHIFT,
> > - rlimit(RLIMIT_MEMLOCK),
> > - ret ? " - exceeded" : "");
> > -
> > - up_write(&mm->mmap_sem);
> > + atomic64_read(&mm->pinned_vm) << PAGE_SHIFT,
> > + rlimit(RLIMIT_MEMLOCK), ret ? " - exceeded" : "");
> >
> > return ret;
> > }
> >
> > -static void decrement_locked_vm(struct mm_struct *mm, long npages)
> > +static void decrement_pinned_vm(struct mm_struct *mm, long npages)
> > {
> > if (!mm || !npages)
> > return;
> >
> > - down_write(&mm->mmap_sem);
> > - if (WARN_ON_ONCE(npages > mm->locked_vm))
> > - npages = mm->locked_vm;
> > - mm->locked_vm -= npages;
> > - pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%ld\n", current->pid,
> > + if (WARN_ON_ONCE(npages > atomic64_read(&mm->pinned_vm)))
> > + npages = atomic64_read(&mm->pinned_vm);
> > + atomic64_sub(npages, &mm->pinned_vm);
> > + pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%lu\n", current->pid,
> > npages << PAGE_SHIFT,
> > - mm->locked_vm << PAGE_SHIFT,
> > + atomic64_read(&mm->pinned_vm) << PAGE_SHIFT,
> > rlimit(RLIMIT_MEMLOCK));
> > - up_write(&mm->mmap_sem);
>
>
> So it used to be down_write+up_write and stuff in between.
>
> Now it is 3 independent accesses (actually 4 but the last one is
> diagnostic) with no locking around them. Why do not we need a lock
> anymore precisely? Thanks,
The first 2 look pretty sketchy to me, is there a case where you don't
know how many pages you've pinned to unpin them? And can it ever
really be correct to just unpin whatever remains? The last access is
diagnostic, which leaves 1. Daniel's rework to warn on a negative
result looks more sane. Thanks,
Alex
^ permalink raw reply
* Re: [PATCH 1/5] vfio/type1: use pinned_vm instead of locked_vm to account pinned pages
From: Alex Williamson @ 2019-02-12 18:41 UTC (permalink / raw)
To: Daniel Jordan
Cc: dave, jack, kvm, linux-mm, aik, linux-fpga, atull, linux-kernel,
kvm-ppc, Jason Gunthorpe, mdf, akpm, linuxppc-dev, cl, hao.wu
In-Reply-To: <20190211231152.qflff6g2asmkb6hr@ca-dmjordan1.us.oracle.com>
On Mon, 11 Feb 2019 18:11:53 -0500
Daniel Jordan <daniel.m.jordan@oracle.com> wrote:
> On Mon, Feb 11, 2019 at 03:56:20PM -0700, Jason Gunthorpe wrote:
> > On Mon, Feb 11, 2019 at 05:44:33PM -0500, Daniel Jordan wrote:
> > > @@ -266,24 +267,15 @@ static int vfio_lock_acct(struct vfio_dma *dma, long npage, bool async)
> > > if (!mm)
> > > return -ESRCH; /* process exited */
> > >
> > > - ret = down_write_killable(&mm->mmap_sem);
> > > - if (!ret) {
> > > - if (npage > 0) {
> > > - if (!dma->lock_cap) {
> > > - unsigned long limit;
> > > -
> > > - limit = task_rlimit(dma->task,
> > > - RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> > > + pinned_vm = atomic64_add_return(npage, &mm->pinned_vm);
> > >
> > > - if (mm->locked_vm + npage > limit)
> > > - ret = -ENOMEM;
> > > - }
> > > + if (npage > 0 && !dma->lock_cap) {
> > > + unsigned long limit = task_rlimit(dma->task, RLIMIT_MEMLOCK) >>
> > > +
> > > - PAGE_SHIFT;
> >
> > I haven't looked at this super closely, but how does this stuff work?
> >
> > do_mlock doesn't touch pinned_vm, and this doesn't touch locked_vm...
> >
> > Shouldn't all this be 'if (locked_vm + pinned_vm < RLIMIT_MEMLOCK)' ?
> >
> > Otherwise MEMLOCK is really doubled..
>
> So this has been a problem for some time, but it's not as easy as adding them
> together, see [1][2] for a start.
>
> The locked_vm/pinned_vm issue definitely needs fixing, but all this series is
> trying to do is account to the right counter.
This still makes me nervous because we have userspace dependencies on
setting process locked memory. There's a user visible difference if we
account for them in the same bucket vs separate. Perhaps we're
counting in the wrong bucket now, but if we "fix" that and userspace
adapts, how do we ever go back to accounting both mlocked and pinned
memory combined against rlimit? Thanks,
Alex
^ permalink raw reply
* Re: [PATCH v2 2/2] locking/rwsem: Optimize down_read_trylock()
From: Waiman Long @ 2019-02-12 18:38 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-arch, linux-xtensa, Davidlohr Bueso, linux-ia64, Tim Chen,
Arnd Bergmann, linux-sh, linux-hexagon, x86, Will Deacon,
linux-kernel, Linus Torvalds, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, linux-alpha, sparclinux, Thomas Gleixner,
linuxppc-dev, Andrew Morton, linux-arm-kernel
In-Reply-To: <a90fa2cf-f891-d64c-101b-adc81e06e0de@redhat.com>
On 02/12/2019 01:36 PM, Waiman Long wrote:
> On 02/12/2019 08:25 AM, Peter Zijlstra wrote:
>> On Tue, Feb 12, 2019 at 02:24:04PM +0100, Peter Zijlstra wrote:
>>> On Mon, Feb 11, 2019 at 02:31:26PM -0500, Waiman Long wrote:
>>>> Modify __down_read_trylock() to make it generate slightly better code
>>>> (smaller and maybe a tiny bit faster).
>>>>
>>>> Before this patch, down_read_trylock:
>>>>
>>>> 0x0000000000000000 <+0>: callq 0x5 <down_read_trylock+5>
>>>> 0x0000000000000005 <+5>: jmp 0x18 <down_read_trylock+24>
>>>> 0x0000000000000007 <+7>: lea 0x1(%rdx),%rcx
>>>> 0x000000000000000b <+11>: mov %rdx,%rax
>>>> 0x000000000000000e <+14>: lock cmpxchg %rcx,(%rdi)
>>>> 0x0000000000000013 <+19>: cmp %rax,%rdx
>>>> 0x0000000000000016 <+22>: je 0x23 <down_read_trylock+35>
>>>> 0x0000000000000018 <+24>: mov (%rdi),%rdx
>>>> 0x000000000000001b <+27>: test %rdx,%rdx
>>>> 0x000000000000001e <+30>: jns 0x7 <down_read_trylock+7>
>>>> 0x0000000000000020 <+32>: xor %eax,%eax
>>>> 0x0000000000000022 <+34>: retq
>>>> 0x0000000000000023 <+35>: mov %gs:0x0,%rax
>>>> 0x000000000000002c <+44>: or $0x3,%rax
>>>> 0x0000000000000030 <+48>: mov %rax,0x20(%rdi)
>>>> 0x0000000000000034 <+52>: mov $0x1,%eax
>>>> 0x0000000000000039 <+57>: retq
>>>>
>>>> After patch, down_read_trylock:
>>>>
>>>> 0x0000000000000000 <+0>: callq 0x5 <down_read_trylock+5>
>>>> 0x0000000000000005 <+5>: mov (%rdi),%rax
>>>> 0x0000000000000008 <+8>: test %rax,%rax
>>>> 0x000000000000000b <+11>: js 0x2f <down_read_trylock+47>
>>>> 0x000000000000000d <+13>: lea 0x1(%rax),%rdx
>>>> 0x0000000000000011 <+17>: lock cmpxchg %rdx,(%rdi)
>>>> 0x0000000000000016 <+22>: jne 0x8 <down_read_trylock+8>
>>>> 0x0000000000000018 <+24>: mov %gs:0x0,%rax
>>>> 0x0000000000000021 <+33>: or $0x3,%rax
>>>> 0x0000000000000025 <+37>: mov %rax,0x20(%rdi)
>>>> 0x0000000000000029 <+41>: mov $0x1,%eax
>>>> 0x000000000000002e <+46>: retq
>>>> 0x000000000000002f <+47>: xor %eax,%eax
>>>> 0x0000000000000031 <+49>: retq
>>>>
>>>> By using a rwsem microbenchmark, the down_read_trylock() rate on a
>>>> x86-64 system before and after the patch were:
>>>>
>>>> Before Patch After Patch
>>>> # of Threads rlock rlock
>>>> ------------ ----- -----
>>>> 1 27,787 28,259
>>>> 2 8,359 9,234
>>> From 1/2:
>>>
>>> 1 29,201 30,143 29,458 28,615 30,172 29,201
>>> 2 6,807 13,299 1,171 7,725 15,025 1,804
>> Argh, fat fingered and send before I was done typing.
>>
>> What I wanted to say was; those rlock numbers don't match up. What
>> gives?
>>
>> The before _this_ patch number of 27k787 should be the same as the after
>> first patch number of 30k172.
> The rlock number in patch 1 refers to down_read() which uses xadd. The
> number here in patch 2 refers specifically to down_read_trylock() which
> uses cmpxchg() as this patch changes only __down_read_tryulock(). So the
> performance data differ.
You can see that the performance is worse if we use cmpxchg for
down_read instead of using xadd.
Cheers,
Longman
^ permalink raw reply
* Re: [PATCH v2 2/2] locking/rwsem: Optimize down_read_trylock()
From: Waiman Long @ 2019-02-12 18:36 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-arch, linux-xtensa, Davidlohr Bueso, linux-ia64, Tim Chen,
Arnd Bergmann, linux-sh, linux-hexagon, x86, Will Deacon,
linux-kernel, Linus Torvalds, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, linux-alpha, sparclinux, Thomas Gleixner,
linuxppc-dev, Andrew Morton, linux-arm-kernel
In-Reply-To: <20190212132537.GL32534@hirez.programming.kicks-ass.net>
On 02/12/2019 08:25 AM, Peter Zijlstra wrote:
> On Tue, Feb 12, 2019 at 02:24:04PM +0100, Peter Zijlstra wrote:
>> On Mon, Feb 11, 2019 at 02:31:26PM -0500, Waiman Long wrote:
>>> Modify __down_read_trylock() to make it generate slightly better code
>>> (smaller and maybe a tiny bit faster).
>>>
>>> Before this patch, down_read_trylock:
>>>
>>> 0x0000000000000000 <+0>: callq 0x5 <down_read_trylock+5>
>>> 0x0000000000000005 <+5>: jmp 0x18 <down_read_trylock+24>
>>> 0x0000000000000007 <+7>: lea 0x1(%rdx),%rcx
>>> 0x000000000000000b <+11>: mov %rdx,%rax
>>> 0x000000000000000e <+14>: lock cmpxchg %rcx,(%rdi)
>>> 0x0000000000000013 <+19>: cmp %rax,%rdx
>>> 0x0000000000000016 <+22>: je 0x23 <down_read_trylock+35>
>>> 0x0000000000000018 <+24>: mov (%rdi),%rdx
>>> 0x000000000000001b <+27>: test %rdx,%rdx
>>> 0x000000000000001e <+30>: jns 0x7 <down_read_trylock+7>
>>> 0x0000000000000020 <+32>: xor %eax,%eax
>>> 0x0000000000000022 <+34>: retq
>>> 0x0000000000000023 <+35>: mov %gs:0x0,%rax
>>> 0x000000000000002c <+44>: or $0x3,%rax
>>> 0x0000000000000030 <+48>: mov %rax,0x20(%rdi)
>>> 0x0000000000000034 <+52>: mov $0x1,%eax
>>> 0x0000000000000039 <+57>: retq
>>>
>>> After patch, down_read_trylock:
>>>
>>> 0x0000000000000000 <+0>: callq 0x5 <down_read_trylock+5>
>>> 0x0000000000000005 <+5>: mov (%rdi),%rax
>>> 0x0000000000000008 <+8>: test %rax,%rax
>>> 0x000000000000000b <+11>: js 0x2f <down_read_trylock+47>
>>> 0x000000000000000d <+13>: lea 0x1(%rax),%rdx
>>> 0x0000000000000011 <+17>: lock cmpxchg %rdx,(%rdi)
>>> 0x0000000000000016 <+22>: jne 0x8 <down_read_trylock+8>
>>> 0x0000000000000018 <+24>: mov %gs:0x0,%rax
>>> 0x0000000000000021 <+33>: or $0x3,%rax
>>> 0x0000000000000025 <+37>: mov %rax,0x20(%rdi)
>>> 0x0000000000000029 <+41>: mov $0x1,%eax
>>> 0x000000000000002e <+46>: retq
>>> 0x000000000000002f <+47>: xor %eax,%eax
>>> 0x0000000000000031 <+49>: retq
>>>
>>> By using a rwsem microbenchmark, the down_read_trylock() rate on a
>>> x86-64 system before and after the patch were:
>>>
>>> Before Patch After Patch
>>> # of Threads rlock rlock
>>> ------------ ----- -----
>>> 1 27,787 28,259
>>> 2 8,359 9,234
>> From 1/2:
>>
>> 1 29,201 30,143 29,458 28,615 30,172 29,201
>> 2 6,807 13,299 1,171 7,725 15,025 1,804
> Argh, fat fingered and send before I was done typing.
>
> What I wanted to say was; those rlock numbers don't match up. What
> gives?
>
> The before _this_ patch number of 27k787 should be the same as the after
> first patch number of 30k172.
The rlock number in patch 1 refers to down_read() which uses xadd. The
number here in patch 2 refers specifically to down_read_trylock() which
uses cmpxchg() as this patch changes only __down_read_tryulock(). So the
performance data differ.
Cheers,
Longman
^ permalink raw reply
* Re: [PATCH 2/5] vfio/spapr_tce: use pinned_vm instead of locked_vm to account pinned pages
From: Daniel Jordan @ 2019-02-12 17:18 UTC (permalink / raw)
To: Christopher Lameter
Cc: dave, jack, kvm, linux-mm, Alexey Kardashevskiy, linux-fpga,
atull, linux-kernel, kvm-ppc, Daniel Jordan, jgg, alex.williamson,
mdf, akpm, linuxppc-dev, hao.wu
In-Reply-To: <01000168e29daf0a-cb3a9394-e3dd-4d88-ad3c-31df1f9ec052-000000@email.amazonses.com>
On Tue, Feb 12, 2019 at 04:50:11PM +0000, Christopher Lameter wrote:
> On Tue, 12 Feb 2019, Alexey Kardashevskiy wrote:
>
> > Now it is 3 independent accesses (actually 4 but the last one is
> > diagnostic) with no locking around them. Why do not we need a lock
> > anymore precisely? Thanks,
>
> Updating a regular counter is racy and requires a lock. It was converted
> to be an atomic which can be incremented without a race.
Yes, though Alexey may have meant that the multiple reads of the atomic in
decrement_pinned_vm are racy. It only matters when there's a bug that would
make the counter go negative, but it's there.
And FWIW the debug print in try_increment_pinned_vm is also racy.
This fixes all that. It doesn't try to correct the negative pinned_vm as the
old code did because it's already a bug and adjusting the value by the negative
amount seems to do nothing but make debugging harder.
If it's ok, I'll respin the whole series this way (another point for common
helper)
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index f47e020dc5e4..b79257304de6 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -53,25 +53,24 @@ static long try_increment_pinned_vm(struct mm_struct *mm, long npages)
atomic64_sub(npages, &mm->pinned_vm);
}
- pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%lu%s\n", current->pid,
- npages << PAGE_SHIFT,
- atomic64_read(&mm->pinned_vm) << PAGE_SHIFT,
- rlimit(RLIMIT_MEMLOCK), ret ? " - exceeded" : "");
+ pr_debug("[%d] RLIMIT_MEMLOCK +%ld %lld/%lu%s\n", current->pid,
+ npages << PAGE_SHIFT, pinned << PAGE_SHIFT,
+ lock_limit, ret ? " - exceeded" : "");
return ret;
}
static void decrement_pinned_vm(struct mm_struct *mm, long npages)
{
+ s64 pinned;
+
if (!mm || !npages)
return;
- if (WARN_ON_ONCE(npages > atomic64_read(&mm->pinned_vm)))
- npages = atomic64_read(&mm->pinned_vm);
- atomic64_sub(npages, &mm->pinned_vm);
- pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%lu\n", current->pid,
- npages << PAGE_SHIFT,
- atomic64_read(&mm->pinned_vm) << PAGE_SHIFT,
+ pinned = atomic64_sub_return(npages, &mm->pinned_vm);
+ WARN_ON_ONCE(pinned < 0);
+ pr_debug("[%d] RLIMIT_MEMLOCK -%ld %lld/%lu\n", current->pid,
+ npages << PAGE_SHIFT, pinned << PAGE_SHIFT,
rlimit(RLIMIT_MEMLOCK));
}
^ permalink raw reply related
* Re: [PATCH 2/5] vfio/spapr_tce: use pinned_vm instead of locked_vm to account pinned pages
From: Christopher Lameter @ 2019-02-12 16:50 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: dave, jack, kvm, jgg, linux-fpga, atull, linux-kernel, kvm-ppc,
Daniel Jordan, linux-mm, alex.williamson, mdf, akpm, linuxppc-dev,
hao.wu
In-Reply-To: <ee4d14db-05c3-6208-503c-16e287fa78eb@ozlabs.ru>
On Tue, 12 Feb 2019, Alexey Kardashevskiy wrote:
> Now it is 3 independent accesses (actually 4 but the last one is
> diagnostic) with no locking around them. Why do not we need a lock
> anymore precisely? Thanks,
Updating a regular counter is racy and requires a lock. It was converted
to be an atomic which can be incremented without a race.
^ permalink raw reply
* Re: use generic DMA mapping code in powerpc V4
From: Christoph Hellwig @ 2019-02-12 15:25 UTC (permalink / raw)
To: Christian Zigotzky
Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <820bfeb1-30c0-3d5a-54a2-c4f9a8c15b0e@xenosoft.de>
On Tue, Feb 12, 2019 at 01:42:56PM +0100, Christian Zigotzky wrote:
> On 11 February 2019 at 08:38AM, Christoph Hellwig wrote:
>> On Sun, Feb 10, 2019 at 01:00:20PM +0100, Christian Zigotzky wrote:
>>> I tested the whole series today. The kernels boot and the P.A. Semi
>>> Ethernet works! :-) Thanks a lot!
>>>
>>> I also tested it in a virtual e5500 QEMU machine today. Unfortunately the
>>> kernel crashes.
>> This looks like a patch I fixed in mainline a while ago, but which
>> the powerpc tree didn't have yet.
>>
>> I've cherry picked this commit
>> ("swiotlb: clear io_tlb_start and io_tlb_end in swiotlb_exit")
>>
>> and added it to the powerpc-dma.6 tree, please retry with that one.
>>
> Hello Christoph,
>
> Have you added it to the powerpc-dma.6 tree yet? The last commit was 4 days
> ago.
I added it, but forgot to push it out. It is there now, sorry:
http://git.infradead.org/users/hch/misc.git/commitdiff/2cf0745b7420af4a3e871d5a970a45662dfae69c
^ permalink raw reply
* Re: [QUESTION] powerpc, libseccomp, and spu
From: Tom Hromatka @ 2019-02-12 14:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Paul Moore, Dhaval Giani
In-Reply-To: <df04c747-58c1-4757-b0ef-1ccea745defb@oracle.com>
On 2/11/19 11:54 AM, Tom Hromatka wrote:
> PowerPC experts,
>
> Paul Moore and I are working on the v2.4 release of libseccomp,
> and as part of this work I need to update the syscall table for
> each architecture.
>
> I have incorporated the new ppc syscall.tbl into libseccomp, but
> I am not familiar with the value of "spu" in the ABI column. For
> example:
>
> 22 32 umount sys_oldumount
> 22 64 umount sys_ni_syscall
> 22 spu umount sys_ni_syscall
>
> In libseccomp, we maintain a 32-bit ppc syscall table and a 64-bit
> ppc syscall table. Do we also need to add a "spu" ppc syscall
> table? Some clarification on the syscalls marked "spu" and "nospu"
> would be greatly appreciated.
>
> Thanks.
>
> Tom
Thanks for the awesome responses, Ben and Michael. I'll definitely
get Paul's input as well, but it sounds reasonable to exclude SPUs
from the newest libseccomp release.
Michael's recommendation to replace "nospu" with common" and ignore
"spu" entirely has allowed ppc and ppc64 to pass all of our internal
checks.
Thanks again!
Tom
^ permalink raw reply
* [PATCH v5 3/3] powerpc/32: Add KASAN support
From: Christophe Leroy @ 2019-02-12 13:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev
In-Reply-To: <cover.1549935247.git.christophe.leroy@c-s.fr>
This patch adds KASAN support for PPC32.
The KASAN shadow area is located between the vmalloc area and the
fixmap area.
KASAN_SHADOW_OFFSET is calculated in asm/kasan.h and extracted
by Makefile prepare rule via asm-offsets.h
Note that on book3s it will only work on the 603 because the other
ones use hash table and can therefore not share a single PTE table
covering the entire early KASAN shadow area.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/Makefile | 7 ++
arch/powerpc/include/asm/book3s/32/pgtable.h | 2 +
arch/powerpc/include/asm/kasan.h | 24 ++++++
arch/powerpc/include/asm/nohash/32/pgtable.h | 2 +
arch/powerpc/include/asm/ppc_asm.h | 4 +
arch/powerpc/include/asm/setup.h | 5 ++
arch/powerpc/include/asm/string.h | 14 ++++
arch/powerpc/kernel/Makefile | 9 ++-
arch/powerpc/kernel/asm-offsets.c | 4 +
arch/powerpc/kernel/head_32.S | 3 +
arch/powerpc/kernel/head_40x.S | 3 +
arch/powerpc/kernel/head_44x.S | 3 +
arch/powerpc/kernel/head_8xx.S | 3 +
arch/powerpc/kernel/head_fsl_booke.S | 3 +
arch/powerpc/kernel/prom_init_check.sh | 10 ++-
arch/powerpc/kernel/setup-common.c | 2 +
arch/powerpc/lib/Makefile | 8 ++
arch/powerpc/lib/copy_32.S | 9 ++-
arch/powerpc/mm/Makefile | 3 +
arch/powerpc/mm/dump_linuxpagetables.c | 8 ++
arch/powerpc/mm/kasan_init.c | 114 +++++++++++++++++++++++++++
arch/powerpc/mm/mem.c | 4 +
23 files changed, 239 insertions(+), 6 deletions(-)
create mode 100644 arch/powerpc/include/asm/kasan.h
create mode 100644 arch/powerpc/mm/kasan_init.c
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 08908219fba9..850b06def84f 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -175,6 +175,7 @@ config PPC
select GENERIC_TIME_VSYSCALL
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_JUMP_LABEL
+ select HAVE_ARCH_KASAN if PPC32
select HAVE_ARCH_KGDB
select HAVE_ARCH_MMAP_RND_BITS
select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index ac033341ed55..f0738099e31e 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -427,6 +427,13 @@ else
endif
endif
+ifdef CONFIG_KASAN
+prepare: kasan_prepare
+
+kasan_prepare: prepare0
+ $(eval KASAN_SHADOW_OFFSET = $(shell awk '{if ($$2 == "KASAN_SHADOW_OFFSET") print $$3;}' include/generated/asm-offsets.h))
+endif
+
# Check toolchain versions:
# - gcc-4.6 is the minimum kernel-wide version so nothing required.
checkbin:
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 49d76adb9bc5..4543016f80ca 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -141,6 +141,8 @@ static inline bool pte_user(pte_t pte)
*/
#ifdef CONFIG_HIGHMEM
#define KVIRT_TOP PKMAP_BASE
+#elif defined(CONFIG_KASAN)
+#define KVIRT_TOP KASAN_SHADOW_START
#else
#define KVIRT_TOP (0xfe000000UL) /* for now, could be FIXMAP_BASE ? */
#endif
diff --git a/arch/powerpc/include/asm/kasan.h b/arch/powerpc/include/asm/kasan.h
new file mode 100644
index 000000000000..5d0088429b62
--- /dev/null
+++ b/arch/powerpc/include/asm/kasan.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_KASAN_H
+#define __ASM_KASAN_H
+
+#ifndef __ASSEMBLY__
+
+#include <asm/page.h>
+#include <asm/pgtable-types.h>
+#include <asm/fixmap.h>
+
+#define KASAN_SHADOW_SCALE_SHIFT 3
+#define KASAN_SHADOW_SIZE ((~0UL - PAGE_OFFSET + 1) >> KASAN_SHADOW_SCALE_SHIFT)
+
+#define KASAN_SHADOW_START (ALIGN_DOWN(FIXADDR_START - KASAN_SHADOW_SIZE, \
+ PGDIR_SIZE))
+#define KASAN_SHADOW_END (KASAN_SHADOW_START + KASAN_SHADOW_SIZE)
+#define KASAN_SHADOW_OFFSET (KASAN_SHADOW_START - \
+ (PAGE_OFFSET >> KASAN_SHADOW_SCALE_SHIFT))
+
+void kasan_early_init(void);
+void kasan_init(void);
+
+#endif
+#endif
diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h
index bed433358260..b3b52f02be1a 100644
--- a/arch/powerpc/include/asm/nohash/32/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
@@ -71,6 +71,8 @@ extern int icache_44x_need_flush;
*/
#ifdef CONFIG_HIGHMEM
#define KVIRT_TOP PKMAP_BASE
+#elif defined(CONFIG_KASAN)
+#define KVIRT_TOP KASAN_SHADOW_START
#else
#define KVIRT_TOP (0xfe000000UL) /* for now, could be FIXMAP_BASE ? */
#endif
diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index e0637730a8e7..dba2c1038363 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -251,6 +251,10 @@ GLUE(.,name):
#define _GLOBAL_TOC(name) _GLOBAL(name)
+#define KASAN_OVERRIDE(x, y) \
+ .weak x; \
+ .set x, y
+
#endif
/*
diff --git a/arch/powerpc/include/asm/setup.h b/arch/powerpc/include/asm/setup.h
index 65676e2325b8..da7768aa996a 100644
--- a/arch/powerpc/include/asm/setup.h
+++ b/arch/powerpc/include/asm/setup.h
@@ -74,6 +74,11 @@ static inline void setup_spectre_v2(void) {};
#endif
void do_btb_flush_fixups(void);
+#ifndef CONFIG_KASAN
+static inline void kasan_early_init(void) { }
+static inline void kasan_init(void) { }
+#endif
+
#endif /* !__ASSEMBLY__ */
#endif /* _ASM_POWERPC_SETUP_H */
diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h
index 1647de15a31e..64d44d4836b4 100644
--- a/arch/powerpc/include/asm/string.h
+++ b/arch/powerpc/include/asm/string.h
@@ -27,6 +27,20 @@ extern int memcmp(const void *,const void *,__kernel_size_t);
extern void * memchr(const void *,int,__kernel_size_t);
extern void * memcpy_flushcache(void *,const void *,__kernel_size_t);
+void *__memset(void *s, int c, __kernel_size_t count);
+void *__memcpy(void *to, const void *from, __kernel_size_t n);
+void *__memmove(void *to, const void *from, __kernel_size_t n);
+
+#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
+/*
+ * For files that are not instrumented (e.g. mm/slub.c) we
+ * should use not instrumented version of mem* functions.
+ */
+#define memcpy(dst, src, len) __memcpy(dst, src, len)
+#define memmove(dst, src, len) __memmove(dst, src, len)
+#define memset(s, c, n) __memset(s, c, n)
+#endif
+
#ifdef CONFIG_PPC64
#define __HAVE_ARCH_MEMSET32
#define __HAVE_ARCH_MEMSET64
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 879b36602748..fc4c42262694 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -16,8 +16,9 @@ CFLAGS_prom_init.o += -fPIC
CFLAGS_btext.o += -fPIC
endif
-CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
-CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
+CFLAGS_early_32.o += -DDISABLE_BRANCH_PROFILING
+CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) -DDISABLE_BRANCH_PROFILING
+CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) -DDISABLE_BRANCH_PROFILING
CFLAGS_btext.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
@@ -31,6 +32,10 @@ CFLAGS_REMOVE_btext.o = $(CC_FLAGS_FTRACE)
CFLAGS_REMOVE_prom.o = $(CC_FLAGS_FTRACE)
endif
+KASAN_SANITIZE_early_32.o := n
+KASAN_SANITIZE_cputable.o := n
+KASAN_SANITIZE_prom_init.o := n
+
obj-y := cputable.o ptrace.o syscalls.o \
irq.o align.o signal_32.o pmc.o vdso.o \
process.o systbl.o idle.o \
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 9ffc72ded73a..846fb30b1190 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -783,5 +783,9 @@ int main(void)
DEFINE(VIRT_IMMR_BASE, (u64)__fix_to_virt(FIX_IMMR_BASE));
#endif
+#ifdef CONFIG_KASAN
+ DEFINE(KASAN_SHADOW_OFFSET, KASAN_SHADOW_OFFSET);
+#endif
+
return 0;
}
diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index 05b08db3901d..0ec9dec06bc2 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -962,6 +962,9 @@ start_here:
* Do early platform-specific initialization,
* and set up the MMU.
*/
+#ifdef CONFIG_KASAN
+ bl kasan_early_init
+#endif
li r3,0
mr r4,r31
bl machine_init
diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
index b19d78410511..5d6ff8fa7e2b 100644
--- a/arch/powerpc/kernel/head_40x.S
+++ b/arch/powerpc/kernel/head_40x.S
@@ -848,6 +848,9 @@ start_here:
/*
* Decide what sort of machine this is and initialize the MMU.
*/
+#ifdef CONFIG_KASAN
+ bl kasan_early_init
+#endif
li r3,0
mr r4,r31
bl machine_init
diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
index bf23c19c92d6..7ca14dff6192 100644
--- a/arch/powerpc/kernel/head_44x.S
+++ b/arch/powerpc/kernel/head_44x.S
@@ -203,6 +203,9 @@ _ENTRY(_start);
/*
* Decide what sort of machine this is and initialize the MMU.
*/
+#ifdef CONFIG_KASAN
+ bl kasan_early_init
+#endif
li r3,0
mr r4,r31
bl machine_init
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 0fea10491f3a..6a644ea2e6b6 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -823,6 +823,9 @@ start_here:
/*
* Decide what sort of machine this is and initialize the MMU.
*/
+#ifdef CONFIG_KASAN
+ bl kasan_early_init
+#endif
li r3,0
mr r4,r31
bl machine_init
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 2386ce2a9c6e..4f4585a68850 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -274,6 +274,9 @@ set_ivor:
/*
* Decide what sort of machine this is and initialize the MMU.
*/
+#ifdef CONFIG_KASAN
+ bl kasan_early_init
+#endif
mr r3,r30
mr r4,r31
bl machine_init
diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
index 667df97d2595..da6bb16e0876 100644
--- a/arch/powerpc/kernel/prom_init_check.sh
+++ b/arch/powerpc/kernel/prom_init_check.sh
@@ -16,8 +16,16 @@
# If you really need to reference something from prom_init.o add
# it to the list below:
+grep CONFIG_KASAN=y .config >/dev/null
+if [ $? -eq 0 ]
+then
+ MEMFCT="__memcpy __memset"
+else
+ MEMFCT="memcpy memset"
+fi
+
WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
-_end enter_prom memcpy memset reloc_offset __secondary_hold
+_end enter_prom $MEMFCT reloc_offset __secondary_hold
__secondary_hold_acknowledge __secondary_hold_spinloop __start
strcmp strcpy strlcpy strlen strncmp strstr kstrtobool logo_linux_clut224
reloc_got2 kernstart_addr memstart_addr linux_banner _stext
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index ca00fbb97cf8..16ff1ea66805 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -978,6 +978,8 @@ void __init setup_arch(char **cmdline_p)
paging_init();
+ kasan_init();
+
/* Initialize the MMU context management stuff. */
mmu_context_init();
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 3bf9fc6fd36c..ce8d4a9f810a 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -8,6 +8,14 @@ ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
CFLAGS_REMOVE_code-patching.o = $(CC_FLAGS_FTRACE)
CFLAGS_REMOVE_feature-fixups.o = $(CC_FLAGS_FTRACE)
+KASAN_SANITIZE_code-patching.o := n
+KASAN_SANITIZE_feature-fixups.o := n
+
+ifdef CONFIG_KASAN
+CFLAGS_code-patching.o += -DDISABLE_BRANCH_PROFILING
+CFLAGS_feature-fixups.o += -DDISABLE_BRANCH_PROFILING
+endif
+
obj-y += string.o alloc.o code-patching.o feature-fixups.o
obj-$(CONFIG_PPC32) += div64.o copy_32.o crtsavres.o strlen_32.o
diff --git a/arch/powerpc/lib/copy_32.S b/arch/powerpc/lib/copy_32.S
index ba66846fe973..4d8a1c73b4cf 100644
--- a/arch/powerpc/lib/copy_32.S
+++ b/arch/powerpc/lib/copy_32.S
@@ -91,7 +91,8 @@ EXPORT_SYMBOL(memset16)
* We therefore skip the optimised bloc that uses dcbz. This jump is
* replaced by a nop once cache is active. This is done in machine_init()
*/
-_GLOBAL(memset)
+_GLOBAL(__memset)
+KASAN_OVERRIDE(memset, __memset)
cmplwi 0,r5,4
blt 7f
@@ -163,12 +164,14 @@ EXPORT_SYMBOL(memset)
* We therefore jump to generic_memcpy which doesn't use dcbz. This jump is
* replaced by a nop once cache is active. This is done in machine_init()
*/
-_GLOBAL(memmove)
+_GLOBAL(__memmove)
+KASAN_OVERRIDE(memmove, __memmove)
cmplw 0,r3,r4
bgt backwards_memcpy
/* fall through */
-_GLOBAL(memcpy)
+_GLOBAL(__memcpy)
+KASAN_OVERRIDE(memcpy, __memcpy)
1: b generic_memcpy
patch_site 1b, patch__memcpy_nocache
diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
index f965fc33a8b7..d6b76f25f6de 100644
--- a/arch/powerpc/mm/Makefile
+++ b/arch/powerpc/mm/Makefile
@@ -7,6 +7,8 @@ ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
CFLAGS_REMOVE_slb.o = $(CC_FLAGS_FTRACE)
+KASAN_SANITIZE_kasan_init.o := n
+
obj-y := fault.o mem.o pgtable.o mmap.o \
init_$(BITS).o pgtable_$(BITS).o \
init-common.o mmu_context.o drmem.o
@@ -55,3 +57,4 @@ obj-$(CONFIG_PPC_BOOK3S_64) += dump_linuxpagetables-book3s64.o
endif
obj-$(CONFIG_PPC_HTDUMP) += dump_hashpagetable.o
obj-$(CONFIG_PPC_MEM_KEYS) += pkeys.o
+obj-$(CONFIG_KASAN) += kasan_init.o
diff --git a/arch/powerpc/mm/dump_linuxpagetables.c b/arch/powerpc/mm/dump_linuxpagetables.c
index 6aa41669ac1a..c862b48118f1 100644
--- a/arch/powerpc/mm/dump_linuxpagetables.c
+++ b/arch/powerpc/mm/dump_linuxpagetables.c
@@ -94,6 +94,10 @@ static struct addr_marker address_markers[] = {
{ 0, "Consistent mem start" },
{ 0, "Consistent mem end" },
#endif
+#ifdef CONFIG_KASAN
+ { 0, "kasan shadow mem start" },
+ { 0, "kasan shadow mem end" },
+#endif
#ifdef CONFIG_HIGHMEM
{ 0, "Highmem PTEs start" },
{ 0, "Highmem PTEs end" },
@@ -310,6 +314,10 @@ static void populate_markers(void)
address_markers[i++].start_address = IOREMAP_TOP +
CONFIG_CONSISTENT_SIZE;
#endif
+#ifdef CONFIG_KASAN
+ address_markers[i++].start_address = KASAN_SHADOW_START;
+ address_markers[i++].start_address = KASAN_SHADOW_END;
+#endif
#ifdef CONFIG_HIGHMEM
address_markers[i++].start_address = PKMAP_BASE;
address_markers[i++].start_address = PKMAP_ADDR(LAST_PKMAP);
diff --git a/arch/powerpc/mm/kasan_init.c b/arch/powerpc/mm/kasan_init.c
new file mode 100644
index 000000000000..bd8e0a263e12
--- /dev/null
+++ b/arch/powerpc/mm/kasan_init.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define DISABLE_BRANCH_PROFILING
+
+#include <linux/kasan.h>
+#include <linux/printk.h>
+#include <linux/memblock.h>
+#include <linux/sched/task.h>
+#include <asm/pgalloc.h>
+
+void __init kasan_early_init(void)
+{
+ unsigned long addr = KASAN_SHADOW_START;
+ unsigned long end = KASAN_SHADOW_END;
+ unsigned long next;
+ pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(addr), addr), addr);
+ int i;
+ phys_addr_t pa = __pa(kasan_early_shadow_page);
+
+ BUILD_BUG_ON(KASAN_SHADOW_START & ~PGDIR_MASK);
+
+ if (early_mmu_has_feature(MMU_FTR_HPTE_TABLE))
+ panic("KASAN not supported with Hash MMU\n");
+
+ for (i = 0; i < PTRS_PER_PTE; i++)
+ __set_pte_at(&init_mm, (unsigned long)kasan_early_shadow_page,
+ kasan_early_shadow_pte + i,
+ pfn_pte(PHYS_PFN(pa), PAGE_KERNEL), 0);
+
+ do {
+ next = pgd_addr_end(addr, end);
+ pmd_populate_kernel(&init_mm, pmd, kasan_early_shadow_pte);
+ } while (pmd++, addr = next, addr != end);
+}
+
+static void __init kasan_init_region(struct memblock_region *reg)
+{
+ void *start = __va(reg->base);
+ void *end = __va(reg->base + reg->size);
+ unsigned long k_start, k_end, k_cur, k_next;
+ pmd_t *pmd;
+ void *block;
+
+ if (start >= end)
+ return;
+
+ k_start = (unsigned long)kasan_mem_to_shadow(start);
+ k_end = (unsigned long)kasan_mem_to_shadow(end);
+ pmd = pmd_offset(pud_offset(pgd_offset_k(k_start), k_start), k_start);
+
+ for (k_cur = k_start; k_cur != k_end; k_cur = k_next, pmd++) {
+ k_next = pgd_addr_end(k_cur, k_end);
+ if ((void *)pmd_page_vaddr(*pmd) == kasan_early_shadow_pte) {
+ pte_t *new = pte_alloc_one_kernel(&init_mm);
+
+ if (!new)
+ panic("kasan: pte_alloc_one_kernel() failed");
+ memcpy(new, kasan_early_shadow_pte, PTE_TABLE_SIZE);
+ pmd_populate_kernel(&init_mm, pmd, new);
+ }
+ };
+
+ block = memblock_alloc(k_end - k_start, PAGE_SIZE);
+ for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE) {
+ void *va = block ? block + k_cur - k_start :
+ memblock_alloc(PAGE_SIZE, PAGE_SIZE);
+ pte_t pte = pfn_pte(PHYS_PFN(__pa(va)), PAGE_KERNEL);
+
+ if (!va)
+ panic("kasan: memblock_alloc() failed");
+ pmd = pmd_offset(pud_offset(pgd_offset_k(k_cur), k_cur), k_cur);
+ pte_update(pte_offset_kernel(pmd, k_cur), ~0, pte_val(pte));
+ }
+ flush_tlb_kernel_range(k_start, k_end);
+}
+
+static void __init kasan_remap_early_shadow_ro(void)
+{
+ unsigned long k_cur;
+ phys_addr_t pa = __pa(kasan_early_shadow_page);
+ int i;
+
+ for (i = 0; i < PTRS_PER_PTE; i++)
+ ptep_set_wrprotect(&init_mm, 0, kasan_early_shadow_pte + i);
+
+ for (k_cur = PAGE_OFFSET & PAGE_MASK; k_cur; k_cur += PAGE_SIZE) {
+ pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(k_cur), k_cur), k_cur);
+ pte_t *ptep = pte_offset_kernel(pmd, k_cur);
+
+ if ((void *)pmd_page_vaddr(*pmd) == kasan_early_shadow_pte)
+ continue;
+ if ((pte_val(*ptep) & PAGE_MASK) != pa)
+ continue;
+
+ ptep_set_wrprotect(&init_mm, k_cur, ptep);
+ }
+ flush_tlb_mm(&init_mm);
+}
+
+void __init kasan_init(void)
+{
+ struct memblock_region *reg;
+
+ for_each_memblock(memory, reg)
+ kasan_init_region(reg);
+
+ kasan_remap_early_shadow_ro();
+
+ clear_page(kasan_early_shadow_page);
+
+ /* At this point kasan is fully initialized. Enable error messages */
+ init_task.kasan_depth = 0;
+ pr_info("KASAN init done\n");
+}
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 81f251fc4169..1bb055775e60 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -336,6 +336,10 @@ void __init mem_init(void)
pr_info(" * 0x%08lx..0x%08lx : highmem PTEs\n",
PKMAP_BASE, PKMAP_ADDR(LAST_PKMAP));
#endif /* CONFIG_HIGHMEM */
+#ifdef CONFIG_KASAN
+ pr_info(" * 0x%08lx..0x%08lx : kasan shadow mem\n",
+ KASAN_SHADOW_START, KASAN_SHADOW_END);
+#endif
#ifdef CONFIG_NOT_COHERENT_CACHE
pr_info(" * 0x%08lx..0x%08lx : consistent mem\n",
IOREMAP_TOP, IOREMAP_TOP + CONFIG_CONSISTENT_SIZE);
--
2.13.3
^ permalink raw reply related
* [PATCH v5 2/3] powerpc/32: Move early_init() in a separate file
From: Christophe Leroy @ 2019-02-12 13:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev
In-Reply-To: <cover.1549935247.git.christophe.leroy@c-s.fr>
In preparation of KASAN, move early_init() into a separate
file in order to allow deactivation of KASAN for that function.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/Makefile | 2 +-
arch/powerpc/kernel/early_32.c | 35 +++++++++++++++++++++++++++++++++++
arch/powerpc/kernel/setup_32.c | 26 --------------------------
3 files changed, 36 insertions(+), 27 deletions(-)
create mode 100644 arch/powerpc/kernel/early_32.c
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index cb7f0bb9ee71..879b36602748 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -93,7 +93,7 @@ extra-y += vmlinux.lds
obj-$(CONFIG_RELOCATABLE) += reloc_$(BITS).o
-obj-$(CONFIG_PPC32) += entry_32.o setup_32.o
+obj-$(CONFIG_PPC32) += entry_32.o setup_32.o early_32.o
obj-$(CONFIG_PPC64) += dma-iommu.o iommu.o
obj-$(CONFIG_KGDB) += kgdb.o
obj-$(CONFIG_BOOTX_TEXT) += btext.o
diff --git a/arch/powerpc/kernel/early_32.c b/arch/powerpc/kernel/early_32.c
new file mode 100644
index 000000000000..b3e40d6d651c
--- /dev/null
+++ b/arch/powerpc/kernel/early_32.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Early init before relocation
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <asm/setup.h>
+#include <asm/sections.h>
+
+/*
+ * We're called here very early in the boot.
+ *
+ * Note that the kernel may be running at an address which is different
+ * from the address that it was linked at, so we must use RELOC/PTRRELOC
+ * to access static data (including strings). -- paulus
+ */
+notrace unsigned long __init early_init(unsigned long dt_ptr)
+{
+ unsigned long offset = reloc_offset();
+
+ /* First zero the BSS */
+ memset(PTRRELOC(&__bss_start), 0, __bss_stop - __bss_start);
+
+ /*
+ * Identify the CPU type and fix up code sections
+ * that depend on which cpu we have.
+ */
+ identify_cpu(offset, mfspr(SPRN_PVR));
+
+ apply_feature_fixups();
+
+ return KERNELBASE + offset;
+}
diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index 5e761eb16a6d..b46a9a33225b 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -63,32 +63,6 @@ EXPORT_SYMBOL(DMA_MODE_READ);
EXPORT_SYMBOL(DMA_MODE_WRITE);
/*
- * We're called here very early in the boot.
- *
- * Note that the kernel may be running at an address which is different
- * from the address that it was linked at, so we must use RELOC/PTRRELOC
- * to access static data (including strings). -- paulus
- */
-notrace unsigned long __init early_init(unsigned long dt_ptr)
-{
- unsigned long offset = reloc_offset();
-
- /* First zero the BSS */
- memset(PTRRELOC(&__bss_start), 0, __bss_stop - __bss_start);
-
- /*
- * Identify the CPU type and fix up code sections
- * that depend on which cpu we have.
- */
- identify_cpu(offset, mfspr(SPRN_PVR));
-
- apply_feature_fixups();
-
- return KERNELBASE + offset;
-}
-
-
-/*
* This is run before start_kernel(), the kernel has been relocated
* and we are running with enough of the MMU enabled to have our
* proper kernel virtual addresses
--
2.13.3
^ permalink raw reply related
* [PATCH v5 1/3] powerpc/mm: prepare kernel for KAsan on PPC32
From: Christophe Leroy @ 2019-02-12 13:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev
In-Reply-To: <cover.1549935247.git.christophe.leroy@c-s.fr>
In kernel/cputable.c, explicitly use memcpy() in order
to allow GCC to replace it with __memcpy() when KASAN is
selected.
Since commit 400c47d81ca38 ("powerpc32: memset: only use dcbz once cache is
enabled"), memset() can be used before activation of the cache,
so no need to use memset_io() for zeroing the BSS.
Acked-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/cputable.c | 13 ++++++++++---
arch/powerpc/kernel/setup_32.c | 6 ++----
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 1eab54bc6ee9..cd12f362b61f 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -2147,7 +2147,11 @@ void __init set_cur_cpu_spec(struct cpu_spec *s)
struct cpu_spec *t = &the_cpu_spec;
t = PTRRELOC(t);
- *t = *s;
+ /*
+ * use memcpy() instead of *t = *s so that GCC replaces it
+ * by __memcpy() when KASAN is active
+ */
+ memcpy(t, s, sizeof(*t));
*PTRRELOC(&cur_cpu_spec) = &the_cpu_spec;
}
@@ -2161,8 +2165,11 @@ static struct cpu_spec * __init setup_cpu_spec(unsigned long offset,
t = PTRRELOC(t);
old = *t;
- /* Copy everything, then do fixups */
- *t = *s;
+ /*
+ * Copy everything, then do fixups. Use memcpy() instead of *t = *s
+ * so that GCC replaces it by __memcpy() when KASAN is active
+ */
+ memcpy(t, s, sizeof(*t));
/*
* If we are overriding a previous value derived from the real
diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index 947f904688b0..5e761eb16a6d 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -73,10 +73,8 @@ notrace unsigned long __init early_init(unsigned long dt_ptr)
{
unsigned long offset = reloc_offset();
- /* First zero the BSS -- use memset_io, some platforms don't have
- * caches on yet */
- memset_io((void __iomem *)PTRRELOC(&__bss_start), 0,
- __bss_stop - __bss_start);
+ /* First zero the BSS */
+ memset(PTRRELOC(&__bss_start), 0, __bss_stop - __bss_start);
/*
* Identify the CPU type and fix up code sections
--
2.13.3
^ permalink raw reply related
* [PATCH v5 0/3] KASAN for powerpc/32
From: Christophe Leroy @ 2019-02-12 13:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev
This serie adds KASAN support to powerpc/32
Tested on nohash/32 (8xx) and book3s/32 (mpc832x ie 603)
Changes in v5:
- Added KASAN_SHADOW_OFFSET in Makefile, otherwise we fallback to KASAN_MINIMAL
and some stuff like stack instrumentation is not performed
- Moved calls to kasan_early_init() in head.S because stack instrumentation
in machine_init was performed before the call to kasan_early_init()
- Mapping kasan_early_shadow_page RW in kasan_early_init() and
remaping RO later in kasan_init()
- Allocating a big memblock() for shadow area, falling back to PAGE_SIZE blocks in case of failure.
Changes in v4:
- Comments from Andrey (DISABLE_BRANCH_PROFILING, Activation of reports)
- Proper initialisation of shadow area in kasan_init()
- Panic in case Hash table is required.
- Added comments in patch one to explain why *t = *s becomes memcpy(t, s, ...)
- Call of kasan_init_tags()
Changes in v3:
- Removed the printk() in kasan_early_init() to avoid build failure (see https://github.com/linuxppc/issues/issues/218)
- Added necessary changes in asm/book3s/32/pgtable.h to get it work on powerpc 603 family
- Added a few KASAN_SANITIZE_xxx.o := n to successfully boot on powerpc 603 family
Changes in v2:
- Rebased.
- Using __set_pte_at() to build the early table.
- Worked around and got rid of the patch adding asm/page.h in asm/pgtable-types.h
==> might be fixed independently but not needed for this serie.
For book3s/32 (not 603), it cannot work as is because due to HASHPTE flag, we
can't use the same pagetable for several PGD entries, and because Hash table
management is not not active early enough at the time being.
Christophe Leroy (3):
powerpc/mm: prepare kernel for KAsan on PPC32
powerpc/32: Move early_init() in a separate file
powerpc/32: Add KASAN support
arch/powerpc/Kconfig | 1 +
arch/powerpc/Makefile | 7 ++
arch/powerpc/include/asm/book3s/32/pgtable.h | 2 +
arch/powerpc/include/asm/kasan.h | 24 ++++++
arch/powerpc/include/asm/nohash/32/pgtable.h | 2 +
arch/powerpc/include/asm/ppc_asm.h | 4 +
arch/powerpc/include/asm/setup.h | 5 ++
arch/powerpc/include/asm/string.h | 14 ++++
arch/powerpc/kernel/Makefile | 11 ++-
arch/powerpc/kernel/asm-offsets.c | 4 +
arch/powerpc/kernel/cputable.c | 13 ++-
arch/powerpc/kernel/early_32.c | 35 ++++++++
arch/powerpc/kernel/head_32.S | 3 +
arch/powerpc/kernel/head_40x.S | 3 +
arch/powerpc/kernel/head_44x.S | 3 +
arch/powerpc/kernel/head_8xx.S | 3 +
arch/powerpc/kernel/head_fsl_booke.S | 3 +
arch/powerpc/kernel/prom_init_check.sh | 10 ++-
arch/powerpc/kernel/setup-common.c | 2 +
arch/powerpc/kernel/setup_32.c | 28 -------
arch/powerpc/lib/Makefile | 8 ++
arch/powerpc/lib/copy_32.S | 9 ++-
arch/powerpc/mm/Makefile | 3 +
arch/powerpc/mm/dump_linuxpagetables.c | 8 ++
arch/powerpc/mm/kasan_init.c | 114 +++++++++++++++++++++++++++
arch/powerpc/mm/mem.c | 4 +
26 files changed, 285 insertions(+), 38 deletions(-)
create mode 100644 arch/powerpc/include/asm/kasan.h
create mode 100644 arch/powerpc/kernel/early_32.c
create mode 100644 arch/powerpc/mm/kasan_init.c
--
2.13.3
^ permalink raw reply
* Re: [PATCH v2 2/2] locking/rwsem: Optimize down_read_trylock()
From: Peter Zijlstra @ 2019-02-12 13:25 UTC (permalink / raw)
To: Waiman Long
Cc: linux-arch, linux-xtensa, Davidlohr Bueso, linux-ia64, Tim Chen,
Arnd Bergmann, linux-sh, linux-hexagon, x86, Will Deacon,
linux-kernel, Linus Torvalds, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, linux-alpha, sparclinux, Thomas Gleixner,
linuxppc-dev, Andrew Morton, linux-arm-kernel
In-Reply-To: <20190212132404.GI32494@hirez.programming.kicks-ass.net>
On Tue, Feb 12, 2019 at 02:24:04PM +0100, Peter Zijlstra wrote:
> On Mon, Feb 11, 2019 at 02:31:26PM -0500, Waiman Long wrote:
> > Modify __down_read_trylock() to make it generate slightly better code
> > (smaller and maybe a tiny bit faster).
> >
> > Before this patch, down_read_trylock:
> >
> > 0x0000000000000000 <+0>: callq 0x5 <down_read_trylock+5>
> > 0x0000000000000005 <+5>: jmp 0x18 <down_read_trylock+24>
> > 0x0000000000000007 <+7>: lea 0x1(%rdx),%rcx
> > 0x000000000000000b <+11>: mov %rdx,%rax
> > 0x000000000000000e <+14>: lock cmpxchg %rcx,(%rdi)
> > 0x0000000000000013 <+19>: cmp %rax,%rdx
> > 0x0000000000000016 <+22>: je 0x23 <down_read_trylock+35>
> > 0x0000000000000018 <+24>: mov (%rdi),%rdx
> > 0x000000000000001b <+27>: test %rdx,%rdx
> > 0x000000000000001e <+30>: jns 0x7 <down_read_trylock+7>
> > 0x0000000000000020 <+32>: xor %eax,%eax
> > 0x0000000000000022 <+34>: retq
> > 0x0000000000000023 <+35>: mov %gs:0x0,%rax
> > 0x000000000000002c <+44>: or $0x3,%rax
> > 0x0000000000000030 <+48>: mov %rax,0x20(%rdi)
> > 0x0000000000000034 <+52>: mov $0x1,%eax
> > 0x0000000000000039 <+57>: retq
> >
> > After patch, down_read_trylock:
> >
> > 0x0000000000000000 <+0>: callq 0x5 <down_read_trylock+5>
> > 0x0000000000000005 <+5>: mov (%rdi),%rax
> > 0x0000000000000008 <+8>: test %rax,%rax
> > 0x000000000000000b <+11>: js 0x2f <down_read_trylock+47>
> > 0x000000000000000d <+13>: lea 0x1(%rax),%rdx
> > 0x0000000000000011 <+17>: lock cmpxchg %rdx,(%rdi)
> > 0x0000000000000016 <+22>: jne 0x8 <down_read_trylock+8>
> > 0x0000000000000018 <+24>: mov %gs:0x0,%rax
> > 0x0000000000000021 <+33>: or $0x3,%rax
> > 0x0000000000000025 <+37>: mov %rax,0x20(%rdi)
> > 0x0000000000000029 <+41>: mov $0x1,%eax
> > 0x000000000000002e <+46>: retq
> > 0x000000000000002f <+47>: xor %eax,%eax
> > 0x0000000000000031 <+49>: retq
> >
> > By using a rwsem microbenchmark, the down_read_trylock() rate on a
> > x86-64 system before and after the patch were:
> >
> > Before Patch After Patch
> > # of Threads rlock rlock
> > ------------ ----- -----
> > 1 27,787 28,259
> > 2 8,359 9,234
>
> From 1/2:
>
> 1 29,201 30,143 29,458 28,615 30,172 29,201
> 2 6,807 13,299 1,171 7,725 15,025 1,804
Argh, fat fingered and send before I was done typing.
What I wanted to say was; those rlock numbers don't match up. What
gives?
The before _this_ patch number of 27k787 should be the same as the after
first patch number of 30k172.
^ permalink raw reply
* Re: [PATCH v2 2/2] locking/rwsem: Optimize down_read_trylock()
From: Peter Zijlstra @ 2019-02-12 13:24 UTC (permalink / raw)
To: Waiman Long
Cc: linux-arch, linux-xtensa, Davidlohr Bueso, linux-ia64, Tim Chen,
Arnd Bergmann, linux-sh, linux-hexagon, x86, Will Deacon,
linux-kernel, Linus Torvalds, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, linux-alpha, sparclinux, Thomas Gleixner,
linuxppc-dev, Andrew Morton, linux-arm-kernel
In-Reply-To: <1549913486-16799-3-git-send-email-longman@redhat.com>
On Mon, Feb 11, 2019 at 02:31:26PM -0500, Waiman Long wrote:
> Modify __down_read_trylock() to make it generate slightly better code
> (smaller and maybe a tiny bit faster).
>
> Before this patch, down_read_trylock:
>
> 0x0000000000000000 <+0>: callq 0x5 <down_read_trylock+5>
> 0x0000000000000005 <+5>: jmp 0x18 <down_read_trylock+24>
> 0x0000000000000007 <+7>: lea 0x1(%rdx),%rcx
> 0x000000000000000b <+11>: mov %rdx,%rax
> 0x000000000000000e <+14>: lock cmpxchg %rcx,(%rdi)
> 0x0000000000000013 <+19>: cmp %rax,%rdx
> 0x0000000000000016 <+22>: je 0x23 <down_read_trylock+35>
> 0x0000000000000018 <+24>: mov (%rdi),%rdx
> 0x000000000000001b <+27>: test %rdx,%rdx
> 0x000000000000001e <+30>: jns 0x7 <down_read_trylock+7>
> 0x0000000000000020 <+32>: xor %eax,%eax
> 0x0000000000000022 <+34>: retq
> 0x0000000000000023 <+35>: mov %gs:0x0,%rax
> 0x000000000000002c <+44>: or $0x3,%rax
> 0x0000000000000030 <+48>: mov %rax,0x20(%rdi)
> 0x0000000000000034 <+52>: mov $0x1,%eax
> 0x0000000000000039 <+57>: retq
>
> After patch, down_read_trylock:
>
> 0x0000000000000000 <+0>: callq 0x5 <down_read_trylock+5>
> 0x0000000000000005 <+5>: mov (%rdi),%rax
> 0x0000000000000008 <+8>: test %rax,%rax
> 0x000000000000000b <+11>: js 0x2f <down_read_trylock+47>
> 0x000000000000000d <+13>: lea 0x1(%rax),%rdx
> 0x0000000000000011 <+17>: lock cmpxchg %rdx,(%rdi)
> 0x0000000000000016 <+22>: jne 0x8 <down_read_trylock+8>
> 0x0000000000000018 <+24>: mov %gs:0x0,%rax
> 0x0000000000000021 <+33>: or $0x3,%rax
> 0x0000000000000025 <+37>: mov %rax,0x20(%rdi)
> 0x0000000000000029 <+41>: mov $0x1,%eax
> 0x000000000000002e <+46>: retq
> 0x000000000000002f <+47>: xor %eax,%eax
> 0x0000000000000031 <+49>: retq
>
> By using a rwsem microbenchmark, the down_read_trylock() rate on a
> x86-64 system before and after the patch were:
>
> Before Patch After Patch
> # of Threads rlock rlock
> ------------ ----- -----
> 1 27,787 28,259
> 2 8,359 9,234
From 1/2:
1 29,201 30,143 29,458 28,615 30,172 29,201
2 6,807 13,299 1,171 7,725 15,025 1,804
>
> On a ARM64 system, the performance results were:
>
> Before Patch After Patch
> # of Threads rlock rlock
> ------------ ----- -----
> 1 24,155 25,000
> 2 6,820 8,699
>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
> kernel/locking/rwsem.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/locking/rwsem.h b/kernel/locking/rwsem.h
> index 067e265..028bc33 100644
> --- a/kernel/locking/rwsem.h
> +++ b/kernel/locking/rwsem.h
> @@ -175,11 +175,11 @@ static inline int __down_read_killable(struct rw_semaphore *sem)
>
> static inline int __down_read_trylock(struct rw_semaphore *sem)
> {
> - long tmp;
> + long tmp = atomic_long_read(&sem->count);
>
> - while ((tmp = atomic_long_read(&sem->count)) >= 0) {
> - if (tmp == atomic_long_cmpxchg_acquire(&sem->count, tmp,
> - tmp + RWSEM_ACTIVE_READ_BIAS)) {
> + while (tmp >= 0) {
> + if (atomic_long_try_cmpxchg_acquire(&sem->count, &tmp,
> + tmp + RWSEM_ACTIVE_READ_BIAS)) {
> return 1;
> }
> }
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [RFC PATCH] powerpc/6xx: Don't set back MSR_RI before reenabling MMU
From: Christophe Leroy @ 2019-02-12 13:24 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <17b00097-a807-e9b9-3dce-198df4a3153f@c-s.fr>
Le 01/02/2019 à 12:51, Christophe Leroy a écrit :
>
>
> Le 01/02/2019 à 12:10, Michael Ellerman a écrit :
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>
>>> By delaying the setting of MSR_RI, a 1% improvment is optained on
>>> null_syscall selftest on an mpc8321.
>>>
>>> Without this patch:
>>>
>>> root@vgoippro:~# ./null_syscall
>>> 1134.33 ns 378.11 cycles
>>>
>>> With this patch:
>>>
>>> root@vgoippro:~# ./null_syscall
>>> 1121.85 ns 373.95 cycles
>>>
>>> The drawback is that a machine check during that period
>>> would be unrecoverable, but as only main memory is accessed
>>> during that period, it shouldn't be a concern.
>>
>> On 64-bit server CPUs accessing main memory can cause a UE
>> (Uncorrectable Error) which can trigger a machine check.
>>
>> So it may still be a concern, it depends how paranoid you are.
>>
>>> diff --git a/arch/powerpc/kernel/head_32.S
>>> b/arch/powerpc/kernel/head_32.S
>>> index 146385b1c2da..ea28a6ab56ec 100644
>>> --- a/arch/powerpc/kernel/head_32.S
>>> +++ b/arch/powerpc/kernel/head_32.S
>>> @@ -282,8 +282,6 @@ __secondary_hold_acknowledge:
>>> stw r1,GPR1(r11); \
>>> stw r1,0(r11); \
>>> tovirt(r1,r11); /* set new kernel sp */ \
>>> - li r10,MSR_KERNEL & ~(MSR_IR|MSR_DR); /* can take exceptions
>>> */ \
>>> - MTMSRD(r10); /* (except for mach check in rtas) */ \
>>> stw r0,GPR0(r11); \
>>> lis r10,STACK_FRAME_REGS_MARKER@ha; /* exception frame
>>> marker */ \
>>> addi r10,r10,STACK_FRAME_REGS_MARKER@l; \
>>
>> Where does RI get enabled? I don't see it anywhere obvious.
>
> MSR_RI is part of MSR_KERNEL, it gets then enabled when reenabling MMU
> when calling the exception handler.
>
> #define EXC_XFER_TEMPLATE(n, hdlr, trap, copyee, tfer, ret) \
> li r10,trap; \
> stw r10,_TRAP(r11); \
> li r10,MSR_KERNEL; \
> copyee(r10, r9); \
> bl tfer; \
> i##n: \
> .long hdlr; \
> .long ret
>
> where tfer = transfer_to_handler.
>
> In transfer_to_handler (kernel/entry_32.S) you have:
>
> transfer_to_handler_cont:
> 3:
> mflr r9
> lwz r11,0(r9) /* virtual address of handler */
> lwz r9,4(r9) /* where to go when done */
> [...]
> mtspr SPRN_SRR0,r11
> mtspr SPRN_SRR1,r10
> mtlr r9
> SYNC
> RFI /* jump to handler, enable MMU */
>
> So MSR_RI is restored above as r10 contains MSR_KERNEL [ | MSR_EE ]
>
Looks like fast_exception_return, which is called by hash page handlers
at least, expects MSR_RI to be set. Allthough it works well on 603
(because it doesn't hash), I would most likely not work on others.
This 1% improvment is not worth it, I give up for now.
^ permalink raw reply
* Re: use generic DMA mapping code in powerpc V4
From: Christian Zigotzky @ 2019-02-12 12:42 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev
In-Reply-To: <20190211073804.GA15841@lst.de>
On 11 February 2019 at 08:38AM, Christoph Hellwig wrote:
> On Sun, Feb 10, 2019 at 01:00:20PM +0100, Christian Zigotzky wrote:
>> I tested the whole series today. The kernels boot and the P.A. Semi
>> Ethernet works! :-) Thanks a lot!
>>
>> I also tested it in a virtual e5500 QEMU machine today. Unfortunately the
>> kernel crashes.
> This looks like a patch I fixed in mainline a while ago, but which
> the powerpc tree didn't have yet.
>
> I've cherry picked this commit
> ("swiotlb: clear io_tlb_start and io_tlb_end in swiotlb_exit")
>
> and added it to the powerpc-dma.6 tree, please retry with that one.
>
Hello Christoph,
Have you added it to the powerpc-dma.6 tree yet? The last commit was 4
days ago.
Thanks,
Christian
^ permalink raw reply
* Re: [PATCH v4 3/3] powerpc/32: Add KASAN support
From: Andrey Ryabinin @ 2019-02-12 12:14 UTC (permalink / raw)
To: Christophe Leroy, Daniel Axtens, Andrey Konovalov
Cc: LKML, Nicholas Piggin, Linux Memory Management List,
Alexander Potapenko, Aneesh Kumar K.V, Paul Mackerras, kasan-dev,
PowerPC, Dmitry Vyukov
In-Reply-To: <a11adaf3-beda-ed0c-e6aa-9ef30f2e80cf@c-s.fr>
On 2/12/19 2:38 PM, Christophe Leroy wrote:
>
>
> Le 12/02/2019 à 02:08, Daniel Axtens a écrit :
>> Andrey Ryabinin <aryabinin@virtuozzo.com> writes:
>>
>>>
>>> Christophe, you can specify KASAN_SHADOW_OFFSET either in Kconfig (e.g. x86_64) or
>>> in Makefile (e.g. arm64). And make early mapping writable, because compiler generated code will write
>>> to shadow memory in function prologue/epilogue.
>>
>> Hmm. Is this limitation just that compilers have not implemented
>> out-of-line support for stack instrumentation, or is there a deeper
>> reason that stack/global instrumentation relies upon inline
>> instrumentation?
>
> No, it looks like as soon as we define KASAN_SHADOW_OFFSET in Makefile in addition to asm/kasan.h, stack instrumentation works with out-of-line.
>
I think you confusing two different things.
CONFIG_KASAN_INLINE/CONFIG_KASAN_OUTLINE affects only generation of code that checks memory accesses,
whether we call __asan_loadx()/__asan_storex() or directly insert code, checking shadow memory.
But with stack instrumentation we also need to poison redzones around stack variables and unpoison them
when we leave the function. That poisoning/unpoisoning thing is always inlined. Currently there is no option to make it out-of-line.
So you can have stack instrumentation with KASAN_OUTLINE, but it just means that checking shadow memory on memory access will be outlined,
poisoning/unpoisoing stack redzones will remain inlined.
^ permalink raw reply
* Re: [PATCH v4 3/3] powerpc/32: Add KASAN support
From: Andrey Ryabinin @ 2019-02-12 12:02 UTC (permalink / raw)
To: Daniel Axtens, Andrey Konovalov, christophe leroy
Cc: LKML, Nicholas Piggin, Linux Memory Management List,
Alexander Potapenko, Aneesh Kumar K.V, Paul Mackerras, kasan-dev,
PowerPC, Dmitry Vyukov
In-Reply-To: <87imxpak4r.fsf@linkitivity.dja.id.au>
On 2/12/19 4:08 AM, Daniel Axtens wrote:
> Andrey Ryabinin <aryabinin@virtuozzo.com> writes:
>
>> On 2/11/19 3:25 PM, Andrey Konovalov wrote:
>>> On Sat, Feb 9, 2019 at 12:55 PM christophe leroy
>>> <christophe.leroy@c-s.fr> wrote:
>>>>
>>>> Hi Andrey,
>>>>
>>>> Le 08/02/2019 à 18:40, Andrey Konovalov a écrit :
>>>>> On Fri, Feb 8, 2019 at 6:17 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>>>>>
>>>>>> Hi Daniel,
>>>>>>
>>>>>> Le 08/02/2019 à 17:18, Daniel Axtens a écrit :
>>>>>>> Hi Christophe,
>>>>>>>
>>>>>>> I've been attempting to port this to 64-bit Book3e nohash (e6500),
>>>>>>> although I think I've ended up with an approach more similar to Aneesh's
>>>>>>> much earlier (2015) series for book3s.
>>>>>>>
>>>>>>> Part of this is just due to the changes between 32 and 64 bits - we need
>>>>>>> to hack around the discontiguous mappings - but one thing that I'm
>>>>>>> particularly puzzled by is what the kasan_early_init is supposed to do.
>>>>>>
>>>>>> It should be a problem as my patch uses a 'for_each_memblock(memory,
>>>>>> reg)' loop.
>>>>>>
>>>>>>>
>>>>>>>> +void __init kasan_early_init(void)
>>>>>>>> +{
>>>>>>>> + unsigned long addr = KASAN_SHADOW_START;
>>>>>>>> + unsigned long end = KASAN_SHADOW_END;
>>>>>>>> + unsigned long next;
>>>>>>>> + pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(addr), addr), addr);
>>>>>>>> + int i;
>>>>>>>> + phys_addr_t pa = __pa(kasan_early_shadow_page);
>>>>>>>> +
>>>>>>>> + BUILD_BUG_ON(KASAN_SHADOW_START & ~PGDIR_MASK);
>>>>>>>> +
>>>>>>>> + if (early_mmu_has_feature(MMU_FTR_HPTE_TABLE))
>>>>>>>> + panic("KASAN not supported with Hash MMU\n");
>>>>>>>> +
>>>>>>>> + for (i = 0; i < PTRS_PER_PTE; i++)
>>>>>>>> + __set_pte_at(&init_mm, (unsigned long)kasan_early_shadow_page,
>>>>>>>> + kasan_early_shadow_pte + i,
>>>>>>>> + pfn_pte(PHYS_PFN(pa), PAGE_KERNEL_RO), 0);
>>>>>>>> +
>>>>>>>> + do {
>>>>>>>> + next = pgd_addr_end(addr, end);
>>>>>>>> + pmd_populate_kernel(&init_mm, pmd, kasan_early_shadow_pte);
>>>>>>>> + } while (pmd++, addr = next, addr != end);
>>>>>>>> +}
>>>>>>>
>>>>>>> As far as I can tell it's mapping the early shadow page, read-only, over
>>>>>>> the KASAN_SHADOW_START->KASAN_SHADOW_END range, and it's using the early
>>>>>>> shadow PTE array from the generic code.
>>>>>>>
>>>>>>> I haven't been able to find an answer to why this is in the docs, so I
>>>>>>> was wondering if you or anyone else could explain the early part of
>>>>>>> kasan init a bit better.
>>>>>>
>>>>>> See https://www.kernel.org/doc/html/latest/dev-tools/kasan.html for an
>>>>>> explanation of the shadow.
>>>>>>
>>>>>> When shadow is 0, it means the memory area is entirely accessible.
>>>>>>
>>>>>> It is necessary to setup a shadow area as soon as possible because all
>>>>>> data accesses check the shadow area, from the begining (except for a few
>>>>>> files where sanitizing has been disabled in Makefiles).
>>>>>>
>>>>>> Until the real shadow area is set, all access are granted thanks to the
>>>>>> zero shadow area beeing for of zeros.
>>>>>
>>>>> Not entirely correct. kasan_early_init() indeed maps the whole shadow
>>>>> memory range to the same kasan_early_shadow_page. However as kernel
>>>>> loads and memory gets allocated this shadow page gets rewritten with
>>>>> non-zero values by different KASAN allocator hooks. Since these values
>>>>> come from completely different parts of the kernel, but all land on
>>>>> the same page, kasan_early_shadow_page's content can be considered
>>>>> garbage. When KASAN checks memory accesses for validity it detects
>>>>> these garbage shadow values, but doesn't print any reports, as the
>>>>> reporting routine bails out on the current->kasan_depth check (which
>>>>> has the value of 1 initially). Only after kasan_init() completes, when
>>>>> the proper shadow memory is mapped, current->kasan_depth gets set to 0
>>>>> and we start reporting bad accesses.
>>>>
>>>> That's surprising, because in the early phase I map the shadow area
>>>> read-only, so I do not expect it to get modified unless RO protection is
>>>> failing for some reason.
>>>
>>> Actually it might be that the allocator hooks don't modify shadow at
>>> this point, as the allocator is not yet initialized. However stack
>>> should be getting poisoned and unpoisoned from the very start. But the
>>> generic statement that early shadow gets dirtied should be correct.
>>> Might it be that you don't use stack instrumentation?
>>>
>>
>> Yes, stack instrumentation is not used here, because shadow offset which we pass to
>> the -fasan-shadow-offset= cflag is not specified here. So the logic in scrpits/Makefile.kasan
>> just fallbacks to CFLAGS_KASAN_MINIMAL, which is outline and without stack instrumentation.
>>
>> Christophe, you can specify KASAN_SHADOW_OFFSET either in Kconfig (e.g. x86_64) or
>> in Makefile (e.g. arm64). And make early mapping writable, because compiler generated code will write
>> to shadow memory in function prologue/epilogue.
>
> Hmm. Is this limitation just that compilers have not implemented
> out-of-line support for stack instrumentation, or is there a deeper
> reason that stack/global instrumentation relies upon inline
> instrumentation?
>
Yes, it's simply wasn't implemented in compilers. Stack [un]poisoning code is always inlined.
But globals is the opposite of that, they all poisoned out-of-line via __asan_register_globals() call.
> I ask because it's very common on ppc64 to have the virtual address
> space split up into discontiguous blocks. I know this means we lose
> inline instrumentation, but I didn't realise we'd also lose stack and
> global instrumentation...
>
> I wonder if it would be worth, in the distant future, trying to
> implement a smarter scheme in compilers where we could insert more
> complex inline mapping schemes.
>
I'd say it depends on performance boost that inline might give for those complex inline schemes.
The whole inline instrumentation thing exists only because it gives better performance.
> Regards,
> Daniel
>
^ permalink raw reply
* Re: [PATCH v4 3/3] powerpc/32: Add KASAN support
From: Christophe Leroy @ 2019-02-12 11:38 UTC (permalink / raw)
To: Daniel Axtens, Andrey Ryabinin, Andrey Konovalov
Cc: LKML, Nicholas Piggin, Linux Memory Management List,
Alexander Potapenko, Aneesh Kumar K.V, Paul Mackerras, kasan-dev,
PowerPC, Dmitry Vyukov
In-Reply-To: <87imxpak4r.fsf@linkitivity.dja.id.au>
Le 12/02/2019 à 02:08, Daniel Axtens a écrit :
> Andrey Ryabinin <aryabinin@virtuozzo.com> writes:
>
>> On 2/11/19 3:25 PM, Andrey Konovalov wrote:
>>> On Sat, Feb 9, 2019 at 12:55 PM christophe leroy
>>> <christophe.leroy@c-s.fr> wrote:
>>>>
>>>> Hi Andrey,
>>>>
>>>> Le 08/02/2019 à 18:40, Andrey Konovalov a écrit :
>>>>> On Fri, Feb 8, 2019 at 6:17 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>>>>>
>>>>>> Hi Daniel,
>>>>>>
>>>>>> Le 08/02/2019 à 17:18, Daniel Axtens a écrit :
>>>>>>> Hi Christophe,
>>>>>>>
>>>>>>> I've been attempting to port this to 64-bit Book3e nohash (e6500),
>>>>>>> although I think I've ended up with an approach more similar to Aneesh's
>>>>>>> much earlier (2015) series for book3s.
>>>>>>>
>>>>>>> Part of this is just due to the changes between 32 and 64 bits - we need
>>>>>>> to hack around the discontiguous mappings - but one thing that I'm
>>>>>>> particularly puzzled by is what the kasan_early_init is supposed to do.
>>>>>>
>>>>>> It should be a problem as my patch uses a 'for_each_memblock(memory,
>>>>>> reg)' loop.
>>>>>>
>>>>>>>
>>>>>>>> +void __init kasan_early_init(void)
>>>>>>>> +{
>>>>>>>> + unsigned long addr = KASAN_SHADOW_START;
>>>>>>>> + unsigned long end = KASAN_SHADOW_END;
>>>>>>>> + unsigned long next;
>>>>>>>> + pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(addr), addr), addr);
>>>>>>>> + int i;
>>>>>>>> + phys_addr_t pa = __pa(kasan_early_shadow_page);
>>>>>>>> +
>>>>>>>> + BUILD_BUG_ON(KASAN_SHADOW_START & ~PGDIR_MASK);
>>>>>>>> +
>>>>>>>> + if (early_mmu_has_feature(MMU_FTR_HPTE_TABLE))
>>>>>>>> + panic("KASAN not supported with Hash MMU\n");
>>>>>>>> +
>>>>>>>> + for (i = 0; i < PTRS_PER_PTE; i++)
>>>>>>>> + __set_pte_at(&init_mm, (unsigned long)kasan_early_shadow_page,
>>>>>>>> + kasan_early_shadow_pte + i,
>>>>>>>> + pfn_pte(PHYS_PFN(pa), PAGE_KERNEL_RO), 0);
>>>>>>>> +
>>>>>>>> + do {
>>>>>>>> + next = pgd_addr_end(addr, end);
>>>>>>>> + pmd_populate_kernel(&init_mm, pmd, kasan_early_shadow_pte);
>>>>>>>> + } while (pmd++, addr = next, addr != end);
>>>>>>>> +}
>>>>>>>
>>>>>>> As far as I can tell it's mapping the early shadow page, read-only, over
>>>>>>> the KASAN_SHADOW_START->KASAN_SHADOW_END range, and it's using the early
>>>>>>> shadow PTE array from the generic code.
>>>>>>>
>>>>>>> I haven't been able to find an answer to why this is in the docs, so I
>>>>>>> was wondering if you or anyone else could explain the early part of
>>>>>>> kasan init a bit better.
>>>>>>
>>>>>> See https://www.kernel.org/doc/html/latest/dev-tools/kasan.html for an
>>>>>> explanation of the shadow.
>>>>>>
>>>>>> When shadow is 0, it means the memory area is entirely accessible.
>>>>>>
>>>>>> It is necessary to setup a shadow area as soon as possible because all
>>>>>> data accesses check the shadow area, from the begining (except for a few
>>>>>> files where sanitizing has been disabled in Makefiles).
>>>>>>
>>>>>> Until the real shadow area is set, all access are granted thanks to the
>>>>>> zero shadow area beeing for of zeros.
>>>>>
>>>>> Not entirely correct. kasan_early_init() indeed maps the whole shadow
>>>>> memory range to the same kasan_early_shadow_page. However as kernel
>>>>> loads and memory gets allocated this shadow page gets rewritten with
>>>>> non-zero values by different KASAN allocator hooks. Since these values
>>>>> come from completely different parts of the kernel, but all land on
>>>>> the same page, kasan_early_shadow_page's content can be considered
>>>>> garbage. When KASAN checks memory accesses for validity it detects
>>>>> these garbage shadow values, but doesn't print any reports, as the
>>>>> reporting routine bails out on the current->kasan_depth check (which
>>>>> has the value of 1 initially). Only after kasan_init() completes, when
>>>>> the proper shadow memory is mapped, current->kasan_depth gets set to 0
>>>>> and we start reporting bad accesses.
>>>>
>>>> That's surprising, because in the early phase I map the shadow area
>>>> read-only, so I do not expect it to get modified unless RO protection is
>>>> failing for some reason.
>>>
>>> Actually it might be that the allocator hooks don't modify shadow at
>>> this point, as the allocator is not yet initialized. However stack
>>> should be getting poisoned and unpoisoned from the very start. But the
>>> generic statement that early shadow gets dirtied should be correct.
>>> Might it be that you don't use stack instrumentation?
>>>
>>
>> Yes, stack instrumentation is not used here, because shadow offset which we pass to
>> the -fasan-shadow-offset= cflag is not specified here. So the logic in scrpits/Makefile.kasan
>> just fallbacks to CFLAGS_KASAN_MINIMAL, which is outline and without stack instrumentation.
>>
>> Christophe, you can specify KASAN_SHADOW_OFFSET either in Kconfig (e.g. x86_64) or
>> in Makefile (e.g. arm64). And make early mapping writable, because compiler generated code will write
>> to shadow memory in function prologue/epilogue.
>
> Hmm. Is this limitation just that compilers have not implemented
> out-of-line support for stack instrumentation, or is there a deeper
> reason that stack/global instrumentation relies upon inline
> instrumentation?
No, it looks like as soon as we define KASAN_SHADOW_OFFSET in Makefile
in addition to asm/kasan.h, stack instrumentation works with out-of-line.
I'll send series v5 soon.
Christophe
>
> I ask because it's very common on ppc64 to have the virtual address
> space split up into discontiguous blocks. I know this means we lose
> inline instrumentation, but I didn't realise we'd also lose stack and
> global instrumentation...
>
> I wonder if it would be worth, in the distant future, trying to
> implement a smarter scheme in compilers where we could insert more
> complex inline mapping schemes.
>
> Regards,
> Daniel
>
^ permalink raw reply
* Re: [PATCH 0/2] x86, numa: always initialize all possible nodes
From: Mike Rapoport @ 2019-02-12 10:19 UTC (permalink / raw)
To: Michal Hocko
Cc: Tony Luck, linux-ia64, Dave Hansen, Peter Zijlstra, x86, LKML,
Pingfan Liu, linux-mm, Ingo Molnar, linuxppc-dev
In-Reply-To: <20190212095343.23315-1-mhocko@kernel.org>
On Tue, Feb 12, 2019 at 10:53:41AM +0100, Michal Hocko wrote:
> Hi,
> this has been posted as an RFC previously [1]. There didn't seem to be
> any objections so I am reposting this for inclusion. I have added a
> debugging patch which prints the zonelist setup for each numa node
> for an easier debugging of a broken zonelist setup.
>
> [1] http://lkml.kernel.org/r/20190114082416.30939-1-mhocko@kernel.org
FWIW,
Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>
for the series.
--
Sincerely yours,
Mike.
^ permalink raw reply
* [PATCH 2/2] mm: be more verbose about zonelist initialization
From: Michal Hocko @ 2019-02-12 9:53 UTC (permalink / raw)
To: linux-mm
Cc: Tony Luck, linux-ia64, Peter Zijlstra, x86, LKML, Pingfan Liu,
Dave Hansen, Michal Hocko, Ingo Molnar, linuxppc-dev
In-Reply-To: <20190212095343.23315-1-mhocko@kernel.org>
From: Michal Hocko <mhocko@suse.com>
We have seen several bugs where zonelists have not been initialized
properly and it is not really straightforward to track those bugs down.
One way to help a bit at least is to dump zonelists of each node when
they are (re)initialized.
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
mm/page_alloc.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2e097f336126..c30d59f803fb 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5259,6 +5259,11 @@ static void build_zonelists(pg_data_t *pgdat)
build_zonelists_in_node_order(pgdat, node_order, nr_nodes);
build_thisnode_zonelists(pgdat);
+
+ pr_info("node[%d] zonelist: ", pgdat->node_id);
+ for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
+ pr_cont("%d:%s ", zone_to_nid(zone), zone->name);
+ pr_cont("\n");
}
#ifdef CONFIG_HAVE_MEMORYLESS_NODES
--
2.20.1
^ permalink raw reply related
* [PATCH 1/2] x86, numa: always initialize all possible nodes
From: Michal Hocko @ 2019-02-12 9:53 UTC (permalink / raw)
To: linux-mm
Cc: Tony Luck, linux-ia64, Peter Zijlstra, x86, LKML, Pingfan Liu,
Dave Hansen, Michal Hocko, Ingo Molnar, linuxppc-dev
In-Reply-To: <20190212095343.23315-1-mhocko@kernel.org>
From: Michal Hocko <mhocko@suse.com>
Pingfan Liu has reported the following splat
[ 5.772742] BUG: unable to handle kernel paging request at 0000000000002088
[ 5.773618] PGD 0 P4D 0
[ 5.773618] Oops: 0000 [#1] SMP NOPTI
[ 5.773618] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 4.20.0-rc1+ #3
[ 5.773618] Hardware name: Dell Inc. PowerEdge R7425/02MJ3T, BIOS 1.4.3 06/29/2018
[ 5.773618] RIP: 0010:__alloc_pages_nodemask+0xe2/0x2a0
[ 5.773618] Code: 00 00 44 89 ea 80 ca 80 41 83 f8 01 44 0f 44 ea 89 da c1 ea 08 83 e2 01 88 54 24 20 48 8b 54 24 08 48 85 d2 0f 85 46 01 00 00 <3b> 77 08 0f 82 3d 01 00 00 48 89 f8 44 89 ea 48 89
e1 44 89 e6 89
[ 5.773618] RSP: 0018:ffffaa600005fb20 EFLAGS: 00010246
[ 5.773618] RAX: 0000000000000000 RBX: 00000000006012c0 RCX: 0000000000000000
[ 5.773618] RDX: 0000000000000000 RSI: 0000000000000002 RDI: 0000000000002080
[ 5.773618] RBP: 00000000006012c0 R08: 0000000000000000 R09: 0000000000000002
[ 5.773618] R10: 00000000006080c0 R11: 0000000000000002 R12: 0000000000000000
[ 5.773618] R13: 0000000000000001 R14: 0000000000000000 R15: 0000000000000002
[ 5.773618] FS: 0000000000000000(0000) GS:ffff8c69afe00000(0000) knlGS:0000000000000000
[ 5.773618] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 5.773618] CR2: 0000000000002088 CR3: 000000087e00a000 CR4: 00000000003406e0
[ 5.773618] Call Trace:
[ 5.773618] new_slab+0xa9/0x570
[ 5.773618] ___slab_alloc+0x375/0x540
[ 5.773618] ? pinctrl_bind_pins+0x2b/0x2a0
[ 5.773618] __slab_alloc+0x1c/0x38
[ 5.773618] __kmalloc_node_track_caller+0xc8/0x270
[ 5.773618] ? pinctrl_bind_pins+0x2b/0x2a0
[ 5.773618] devm_kmalloc+0x28/0x60
[ 5.773618] pinctrl_bind_pins+0x2b/0x2a0
[ 5.773618] really_probe+0x73/0x420
[ 5.773618] driver_probe_device+0x115/0x130
[ 5.773618] __driver_attach+0x103/0x110
[ 5.773618] ? driver_probe_device+0x130/0x130
[ 5.773618] bus_for_each_dev+0x67/0xc0
[ 5.773618] ? klist_add_tail+0x3b/0x70
[ 5.773618] bus_add_driver+0x41/0x260
[ 5.773618] ? pcie_port_setup+0x4d/0x4d
[ 5.773618] driver_register+0x5b/0xe0
[ 5.773618] ? pcie_port_setup+0x4d/0x4d
[ 5.773618] do_one_initcall+0x4e/0x1d4
[ 5.773618] ? init_setup+0x25/0x28
[ 5.773618] kernel_init_freeable+0x1c1/0x26e
[ 5.773618] ? loglevel+0x5b/0x5b
[ 5.773618] ? rest_init+0xb0/0xb0
[ 5.773618] kernel_init+0xa/0x110
[ 5.773618] ret_from_fork+0x22/0x40
[ 5.773618] Modules linked in:
[ 5.773618] CR2: 0000000000002088
[ 5.773618] ---[ end trace 1030c9120a03d081 ]---
with his AMD machine with the following topology
NUMA node0 CPU(s): 0,8,16,24
NUMA node1 CPU(s): 2,10,18,26
NUMA node2 CPU(s): 4,12,20,28
NUMA node3 CPU(s): 6,14,22,30
NUMA node4 CPU(s): 1,9,17,25
NUMA node5 CPU(s): 3,11,19,27
NUMA node6 CPU(s): 5,13,21,29
NUMA node7 CPU(s): 7,15,23,31
[ 0.007418] Early memory node ranges
[ 0.007419] node 1: [mem 0x0000000000001000-0x000000000008efff]
[ 0.007420] node 1: [mem 0x0000000000090000-0x000000000009ffff]
[ 0.007422] node 1: [mem 0x0000000000100000-0x000000005c3d6fff]
[ 0.007422] node 1: [mem 0x00000000643df000-0x0000000068ff7fff]
[ 0.007423] node 1: [mem 0x000000006c528000-0x000000006fffffff]
[ 0.007424] node 1: [mem 0x0000000100000000-0x000000047fffffff]
[ 0.007425] node 5: [mem 0x0000000480000000-0x000000087effffff]
and nr_cpus set to 4. The underlying reason is tha the device is bound
to node 2 which doesn't have any memory and init_cpu_to_node only
initializes memory-less nodes for possible cpus which nr_cpus restrics.
This in turn means that proper zonelists are not allocated and the page
allocator blows up.
Fix the issue by reworking how x86 initializes the memory less nodes.
The current implementation is hacked into the workflow and it doesn't
allow any flexibility. There is init_memory_less_node called for each
offline node that has a CPU as already mentioned above. This will make
sure that we will have a new online node without any memory. Much later
on we build a zone list for this node and things seem to work, except
they do not (e.g. due to nr_cpus). Not to mention that it doesn't really
make much sense to consider an empty node as online because we just
consider this node whenever we want to iterate nodes to use and empty
node is obviously not the best candidate. This is all just too fragile.
The new code relies on the arch specific initialization to allocate all
possible NUMA nodes (including memory less) - numa_register_memblks in
this case. Generic code then initializes both zonelists (__build_all_zonelists)
and allocator internals (free_area_init_nodes) for all non-null pgdats
rather than online ones.
For the x86 specific part also do not make new node online in alloc_node_data
because this is too early to know that. numa_register_memblks knows that
a node has some memory so it can make the node online appropriately.
init_memory_less_node hack can be safely removed altogether now.
Reported-by: Pingfan Liu <kernelfans@gmail.com>
Tested-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
arch/x86/mm/numa.c | 27 +++------------------------
mm/page_alloc.c | 15 +++++++++------
2 files changed, 12 insertions(+), 30 deletions(-)
diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index 1308f5408bf7..b3621ee4dfe8 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -216,8 +216,6 @@ static void __init alloc_node_data(int nid)
node_data[nid] = nd;
memset(NODE_DATA(nid), 0, sizeof(pg_data_t));
-
- node_set_online(nid);
}
/**
@@ -570,7 +568,7 @@ static int __init numa_register_memblks(struct numa_meminfo *mi)
return -EINVAL;
/* Finally register nodes. */
- for_each_node_mask(nid, node_possible_map) {
+ for_each_node_mask(nid, numa_nodes_parsed) {
u64 start = PFN_PHYS(max_pfn);
u64 end = 0;
@@ -581,9 +579,6 @@ static int __init numa_register_memblks(struct numa_meminfo *mi)
end = max(mi->blk[i].end, end);
}
- if (start >= end)
- continue;
-
/*
* Don't confuse VM with a node that doesn't have the
* minimum amount of memory:
@@ -592,6 +587,8 @@ static int __init numa_register_memblks(struct numa_meminfo *mi)
continue;
alloc_node_data(nid);
+ if (end)
+ node_set_online(nid);
}
/* Dump memblock with node info and return. */
@@ -721,21 +718,6 @@ void __init x86_numa_init(void)
numa_init(dummy_numa_init);
}
-static void __init init_memory_less_node(int nid)
-{
- unsigned long zones_size[MAX_NR_ZONES] = {0};
- unsigned long zholes_size[MAX_NR_ZONES] = {0};
-
- /* Allocate and initialize node data. Memory-less node is now online.*/
- alloc_node_data(nid);
- free_area_init_node(nid, zones_size, 0, zholes_size);
-
- /*
- * All zonelists will be built later in start_kernel() after per cpu
- * areas are initialized.
- */
-}
-
/*
* Setup early cpu_to_node.
*
@@ -763,9 +745,6 @@ void __init init_cpu_to_node(void)
if (node == NUMA_NO_NODE)
continue;
- if (!node_online(node))
- init_memory_less_node(node);
-
numa_set_node(cpu, node);
}
}
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2ec9cc407216..2e097f336126 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5361,10 +5361,11 @@ static void __build_all_zonelists(void *data)
if (self && !node_online(self->node_id)) {
build_zonelists(self);
} else {
- for_each_online_node(nid) {
+ for_each_node(nid) {
pg_data_t *pgdat = NODE_DATA(nid);
- build_zonelists(pgdat);
+ if (pgdat)
+ build_zonelists(pgdat);
}
#ifdef CONFIG_HAVE_MEMORYLESS_NODES
@@ -6644,10 +6645,8 @@ static unsigned long __init find_min_pfn_for_node(int nid)
for_each_mem_pfn_range(i, nid, &start_pfn, NULL, NULL)
min_pfn = min(min_pfn, start_pfn);
- if (min_pfn == ULONG_MAX) {
- pr_warn("Could not find start_pfn for node %d\n", nid);
+ if (min_pfn == ULONG_MAX)
return 0;
- }
return min_pfn;
}
@@ -6991,8 +6990,12 @@ void __init free_area_init_nodes(unsigned long *max_zone_pfn)
mminit_verify_pageflags_layout();
setup_nr_node_ids();
zero_resv_unavail();
- for_each_online_node(nid) {
+ for_each_node(nid) {
pg_data_t *pgdat = NODE_DATA(nid);
+
+ if (!pgdat)
+ continue;
+
free_area_init_node(nid, NULL,
find_min_pfn_for_node(nid), NULL);
--
2.20.1
^ permalink raw reply related
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