* Re: [PATCH 07/10] powerpc/mm/slice: Switch to 3-operand slice bitops helpers
From: Nicholas Piggin @ 2018-03-06 23:19 UTC (permalink / raw)
To: Christophe LEROY; +Cc: linuxppc-dev, Aneesh Kumar K . V
In-Reply-To: <49fb018e-50b2-a37c-e329-19b5d4f87942@c-s.fr>
On Tue, 6 Mar 2018 15:44:46 +0100
Christophe LEROY <christophe.leroy@c-s.fr> wrote:
> Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :
> > This converts the slice_mask bit operation helpers to be the usual
> > 3-operand kind, which is clearer to work with.
>
> What's the real benefit of doing that ?
One or two places where we want to combine 2 const input bitmaps
and can avoid the extra copy in the next patch.
E.g., slice_or_mask(&good_mask, maskp, compat_maskp);
>
> It is helps for a subsequent patch, say it.
Fair point, I'll add to the changelog.
> > @@ -433,25 +433,33 @@ static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
> > return slice_find_area_bottomup(mm, len, mask, psize, high_limit);
> > }
> >
> > -static inline void slice_or_mask(struct slice_mask *dst,
> > +static inline void slice_copy_mask(struct slice_mask *dst,
> > const struct slice_mask *src)
>
> This new function is not used, the compiler while probably not be happy.
I think it doesn't get so grumpy with inlines (otherwise we'd have a lot
of problems of headers). Usually I think the new function should go into
the patch where it's first used, but this being a self contained helper,
I thought it fit better to add here. Maybe I'm wrong, I can move it.
^ permalink raw reply
* Re: [PATCH 06/10] powerpc/mm/slice: implement slice_check_range_fits
From: Nicholas Piggin @ 2018-03-06 23:12 UTC (permalink / raw)
To: Christophe LEROY; +Cc: linuxppc-dev, Aneesh Kumar K . V
In-Reply-To: <e600126e-ab89-e15b-e4c3-17dec7799a57@c-s.fr>
On Tue, 6 Mar 2018 15:41:00 +0100
Christophe LEROY <christophe.leroy@c-s.fr> wrote:
> Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :
> > +static bool slice_check_range_fits(struct mm_struct *mm,
> > + const struct slice_mask *available,
> > + unsigned long start, unsigned long len)
> > {
> > - DECLARE_BITMAP(result, SLICE_NUM_HIGH);
> > - /*
> > - * Make sure we just do bit compare only to the max
> > - * addr limit and not the full bit map size.
> > - */
> > - unsigned long slice_count = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
> > + unsigned long end = start + len - 1;
> > + u64 low_slices = 0;
> >
> > - if (!SLICE_NUM_HIGH)
> > - return (mask->low_slices & available->low_slices) ==
> > - mask->low_slices;
> > + if (start < SLICE_LOW_TOP) {
> > + unsigned long mend = min(end, (SLICE_LOW_TOP - 1));
>
> See slice_range_to_mask()
>
> You'll have an issue here with PPC32, you have to cast (SLICE_LOW_TOP -
> 1) to unsigned long because SLICE_LOW_TOP is unsigned long long on PPC32
Okay thanks. Forgot to cross compiled it on 8xx, so I'll do that next
time.
> > +
> > + low_slices = (1u << (GET_LOW_SLICE_INDEX(mend) + 1))
> > + - (1u << GET_LOW_SLICE_INDEX(start));
> > + }
> > + if ((low_slices & available->low_slices) != low_slices)
> > + return false;
> > +
> > + if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
> > + unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
> > + unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
> > + unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
> > + unsigned long i;
> >
> > - bitmap_and(result, mask->high_slices,
> > - available->high_slices, slice_count);
> > + for (i = start_index; i < start_index + count; i++) {
> > + if (!test_bit(i, available->high_slices))
> > + return false;
> > + }
>
> What about using bitmap_find_next_zero_area()
I'll look at it. Perhaps in another patch, because existing
loops are not using bitmap range operations either. A series
to convert those is a good idea.
> > @@ -562,15 +571,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> > #endif
> >
> > /* First check hint if it's valid or if we have MAP_FIXED */
> > - if (addr != 0 || fixed) {
> > - /* Build a mask for the requested range */
> > - slice_range_to_mask(addr, len, &mask);
> > - slice_print_mask(" mask", &mask);
> > -
> > + if (addr || fixed) {
>
> It is cleanup, should it really be part of this patch ?
> > @@ -596,10 +601,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> > slice_or_mask(&potential_mask, &good_mask);
> > slice_print_mask(" potential", &potential_mask);
> >
> > - if ((addr != 0 || fixed) &&
> > - slice_check_fit(mm, &mask, &potential_mask)) {
> > - slice_dbg(" fits potential !\n");
> > - goto convert;
> > + if (addr || fixed) {
> > + if (slice_check_range_fits(mm, &potential_mask, addr, len)) {
> > + slice_dbg(" fits potential !\n");
> > + goto convert;
> > + }
>
> Why not keep the original structure and just replacing slice_check_fit()
> by slice_check_range_fits() ?
>
> I believe cleanups should not be mixed with real feature changes. If
> needed, you should have a cleanup patch up front the serie.
For code that is already changing, I think minor cleanups are okay if
they're very simple. Maybe this is getting to the point of needing
another patch. You've made valid points for a lot of other unnecessary
cleanups though, so I'll fix all of those.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v3 02/10] include: Move compat_timespec/ timeval to compat_time.h
From: Deepa Dinamani @ 2018-03-06 22:58 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Arnd Bergmann, Thomas Gleixner, John Stultz, Mark Rutland,
open list:RALINK MIPS ARCHITECTURE, Peter Zijlstra,
Heiko Carstens, Paul Mackerras, H. Peter Anvin, sparclinux, devel,
linux-s390, y2038 Mailman List, Helge Deller,
the arch/x86 maintainers, sebott, James E.J. Bottomley,
Will Deacon, Ingo Molnar, oprofile-list, Catalin Marinas,
Robert Richter, Chris Metcalf, Peter Oberparleiter,
Arnaldo Carvalho de Melo, Julian Wiedmann, Steven Rostedt,
Ursula Braun, gerald.schaefer, Parisc List, gregkh, cohuck,
Linux Kernel Mailing List, Ralf Baechle, Jan Hoeppner,
Stefan Haberland, Martin Schwidefsky, linuxppc-dev, David Miller
In-Reply-To: <d8480da9-afe9-8a43-9c47-50919215a2de@de.ibm.com>
On Tue, Mar 6, 2018 at 4:48 AM, Christian Borntraeger
<borntraeger@de.ibm.com> wrote:
>
>
> On 03/06/2018 01:46 PM, Arnd Bergmann wrote:
>> On Mon, Mar 5, 2018 at 10:30 AM, Christian Borntraeger
>> <borntraeger@de.ibm.com> wrote:
>>> On 01/16/2018 03:18 AM, Deepa Dinamani wrote:
>>>> All the current architecture specific defines for these
>>>> are the same. Refactor these common defines to a common
>>>> header file.
>>>>
>>>> The new common linux/compat_time.h is also useful as it
>>>> will eventually be used to hold all the defines that
>>>> are needed for compat time types that support non y2038
>>>> safe types. New architectures need not have to define these
>>>> new types as they will only use new y2038 safe syscalls.
>>>> This file can be deleted after y2038 when we stop supporting
>>>> non y2038 safe syscalls.
>>>
>>> You are now include a <linux/*.h> from several asm files
>>> (
>>> arch/arm64/include/asm/stat.h
>>> arch/s390/include/asm/elf.h
>>> arch/x86/include/asm/ftrace.h
>>> arch/x86/include/asm/sys_ia32.h
>>> )
>>> It works, and it is done in many places, but it looks somewhat weird.
>>> Would it make sense to have an asm-generic/compate-time.h instead? Asking for
>>> opinions here.
>>
>> I don't think we have such a rule. If a header file is common to all
>> architectures (i.e. no architecture uses a different implementation),
>> it should be in include/linux rather than include/asm-generic, regardless
>> of whether it can be used by assembler files or not.
>>
>>>> --- a/drivers/s390/net/qeth_core_main.c
>>>> +++ b/drivers/s390/net/qeth_core_main.c
>>>> @@ -32,7 +32,7 @@
>>>> #include <asm/chpid.h>
>>>> #include <asm/io.h>
>>>> #include <asm/sysinfo.h>
>>>> -#include <asm/compat.h>
>>>> +#include <linux/compat.h>
>>>> #include <asm/diag.h>
>>>> #include <asm/cio.h>
>>>> #include <asm/ccwdev.h>
>>>
>>> Can you move that into the other includes (where all the other <linux/*> includes are.
>>
>> Good catch, this is definitely a rule we have ;-)
>
> FWIW, this was also broken for
> arch/x86/include/asm/sys_ia32.h
The reason that this was done this way is because of the sed script
mentioned in the commit text.
I was trying to make minimal change apart from the script so that we
don't have other changes like moving the lines to keep the patch
simpler.
I will fix this by hand since this is preferred.
I will post an update.
-Deepa
^ permalink raw reply
* Re: [PATCH v12 07/11] mm: Add address parameter to arch_validate_prot()
From: Andrew Morton @ 2018-03-06 22:48 UTC (permalink / raw)
To: Khalid Aziz
Cc: benh, paulus, mpe, davem, dave.hansen, bsingharora, nborisov,
aarcange, anthony.yznaga, mgorman, linuxram, kirill.shutemov,
dan.j.williams, jack, ross.zwisler, gregkh, tglx, mhocko,
n-horiguchi, jglisse, henry.willard, aneesh.kumar, khandual,
linuxppc-dev, linux-kernel, linux-mm, sparclinux, Khalid Aziz
In-Reply-To: <349751cbd54fda6f4a223f941aa71bbfe7be77ce.1519227112.git.khalid.aziz@oracle.com>
On Wed, 21 Feb 2018 10:15:49 -0700 Khalid Aziz <khalid.aziz@oracle.com> wrote:
> A protection flag may not be valid across entire address space and
> hence arch_validate_prot() might need the address a protection bit is
> being set on to ensure it is a valid protection flag. For example, sparc
> processors support memory corruption detection (as part of ADI feature)
> flag on memory addresses mapped on to physical RAM but not on PFN mapped
> pages or addresses mapped on to devices. This patch adds address to the
> parameters being passed to arch_validate_prot() so protection bits can
> be validated in the relevant context.
>
> Signed-off-by: Khalid Aziz <khalid.aziz@oracle.com>
> Cc: Khalid Aziz <khalid@gonehiking.org>
> Reviewed-by: Anthony Yznaga <anthony.yznaga@oracle.com>
Acked-by: Andrew Morton <akpm@linux-foundation.org>
^ permalink raw reply
* Re: [PATCH] selftests/powerpc: Skip tm-unavailable if TM is not enabled
From: Gustavo Romero @ 2018-03-06 20:24 UTC (permalink / raw)
To: Cyril Bur, linuxppc-dev
In-Reply-To: <1520293795.5421.1.camel@gmail.com>
Hi Cyril,
On 03/05/2018 08:49 PM, Cyril Bur wrote:
> On Mon, 2018-03-05 at 15:48 -0500, Gustavo Romero wrote:
>> Some processor revisions do not support transactional memory, and
>> additionally kernel support can be disabled. In either case the
>> tm-unavailable test should be skipped, otherwise it will fail with
>> a SIGILL.
>>
>> That commit also sets this selftest to be called through the test
>> harness as it's done for other TM selftests.
>>
>> Finally, it avoids using "ping" as a thread name since it's
>> ambiguous and can be confusing when shown, for instance,
>> in a kernel backtrace log.
>>
>
> I spent more time than I care to admit looking at backtraces wondering
> how "ping" got in the mix ;).
heh sorry about that... :)
>> Fixes: 77fad8bfb1d2 ("selftests/powerpc: Check FP/VEC on exception in TM")
>> Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com>
>
> Reviewed-by: Cyril Bur <cyrilbur@gmail.com>
Thanks for reviewing it.
Cheers,
Gustavo
>> ---
>> .../testing/selftests/powerpc/tm/tm-unavailable.c | 24 ++++++++++++++--------
>> 1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/tools/testing/selftests/powerpc/tm/tm-unavailable.c b/tools/testing/selftests/powerpc/tm/tm-unavailable.c
>> index e6a0fad..156c8e7 100644
>> --- a/tools/testing/selftests/powerpc/tm/tm-unavailable.c
>> +++ b/tools/testing/selftests/powerpc/tm/tm-unavailable.c
>> @@ -80,7 +80,7 @@ bool is_failure(uint64_t condition_reg)
>> return ((condition_reg >> 28) & 0xa) == 0xa;
>> }
>>
>> -void *ping(void *input)
>> +void *tm_una_ping(void *input)
>> {
>>
>> /*
>> @@ -280,7 +280,7 @@ void *ping(void *input)
>> }
>>
>> /* Thread to force context switch */
>> -void *pong(void *not_used)
>> +void *tm_una_pong(void *not_used)
>> {
>> /* Wait thread get its name "pong". */
>> if (DEBUG)
>> @@ -311,11 +311,11 @@ void test_fp_vec(int fp, int vec, pthread_attr_t *attr)
>> do {
>> int rc;
>>
>> - /* Bind 'ping' to CPU 0, as specified in 'attr'. */
>> - rc = pthread_create(&t0, attr, ping, (void *) &flags);
>> + /* Bind to CPU 0, as specified in 'attr'. */
>> + rc = pthread_create(&t0, attr, tm_una_ping, (void *) &flags);
>> if (rc)
>> pr_err(rc, "pthread_create()");
>> - rc = pthread_setname_np(t0, "ping");
>> + rc = pthread_setname_np(t0, "tm_una_ping");
>> if (rc)
>> pr_warn(rc, "pthread_setname_np");
>> rc = pthread_join(t0, &ret_value);
>> @@ -333,13 +333,15 @@ void test_fp_vec(int fp, int vec, pthread_attr_t *attr)
>> }
>> }
>>
>> -int main(int argc, char **argv)
>> +int tm_unavailable_test(void)
>> {
>> int rc, exception; /* FP = 0, VEC = 1, VSX = 2 */
>> pthread_t t1;
>> pthread_attr_t attr;
>> cpu_set_t cpuset;
>>
>> + SKIP_IF(!have_htm());
>> +
>> /* Set only CPU 0 in the mask. Both threads will be bound to CPU 0. */
>> CPU_ZERO(&cpuset);
>> CPU_SET(0, &cpuset);
>> @@ -354,12 +356,12 @@ int main(int argc, char **argv)
>> if (rc)
>> pr_err(rc, "pthread_attr_setaffinity_np()");
>>
>> - rc = pthread_create(&t1, &attr /* Bind 'pong' to CPU 0 */, pong, NULL);
>> + rc = pthread_create(&t1, &attr /* Bind to CPU 0 */, tm_una_pong, NULL);
>> if (rc)
>> pr_err(rc, "pthread_create()");
>>
>> /* Name it for systemtap convenience */
>> - rc = pthread_setname_np(t1, "pong");
>> + rc = pthread_setname_np(t1, "tm_una_pong");
>> if (rc)
>> pr_warn(rc, "pthread_create()");
>>
>> @@ -394,3 +396,9 @@ int main(int argc, char **argv)
>> exit(0);
>> }
>> }
>> +
>> +int main(int argc, char **argv)
>> +{
>> + test_harness_set_timeout(220);
>> + return test_harness(tm_unavailable_test, "tm_unavailable_test");
>> +}
>
^ permalink raw reply
* Re: [PATCH v2 00/38] cxlflash: OCXL transport support
From: Martin K. Petersen @ 2018-03-06 18:01 UTC (permalink / raw)
To: Uma Krishnan
Cc: linux-scsi, James Bottomley, Martin K. Petersen, Matthew R. Ochs,
Manoj N. Kumar, linuxppc-dev, Andrew Donnellan, Frederic Barrat,
Christophe Lombard
In-Reply-To: <1519683513-16731-1-git-send-email-ukrishn@linux.vnet.ibm.com>
> This patch series adds OCXL support to the cxlflash driver. With this
> support, new devices using the OCXL transport will be supported by the
> cxlflash driver along with the existing CXL devices. An effort is made
> to keep this transport specific function independent of the existing
> core driver that communicates with the AFU.
This one has been sitting for a week without any reviews. Andrew, when
will you have time to take a look?
Being a pretty big chunk of code, I would like to merge it sooner rather
than later. Or we can postpone until 4.18, of course.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH v10 2/2] cxl: read PHB indications from the device tree
From: christophe lombard @ 2018-03-06 16:57 UTC (permalink / raw)
To: Philippe Bergheaud, linuxppc-dev; +Cc: fbarrat, benh
In-Reply-To: <20180302095612.19493-2-felix@linux.vnet.ibm.com>
Le 02/03/2018 à 10:56, Philippe Bergheaud a écrit :
> Configure the P9 XSL_DSNCTL register with PHB indications found
> in the device tree, or else use legacy hard-coded values.
>
> Signed-off-by: Philippe Bergheaud <felix@linux.vnet.ibm.com>
> Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> ---
> Changelog:
>
> v2: New patch. Use the new device tree property "ibm,phb-indications".
>
> v3: No change.
>
> v4: No functional change.
> Drop cosmetic fix in comment.
>
> v5: get_phb_indications():
> - make static variables local to function.
> - return static variable values by arguments.
>
> v6: get_phb_indications():
> - acquire a mutex before setting the phb indications.
>
> v7: get_phb_indications():
> cxl_get_xsl9_dsnctl():
> - return -ENODEV instead of -1.
>
> v8: get_phb_indications():
> - stay on the safe side: acquire the mutex unconditionally
>
> v9,v10: No change.
> ---
Reviewed-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
^ permalink raw reply
* Re: [PATCH v10 1/2] powerpc/powernv: Enable tunneled operations
From: christophe lombard @ 2018-03-06 16:55 UTC (permalink / raw)
To: Philippe Bergheaud, linuxppc-dev; +Cc: fbarrat, benh
In-Reply-To: <20180302095612.19493-1-felix@linux.vnet.ibm.com>
Le 02/03/2018 à 10:56, Philippe Bergheaud a écrit :
> P9 supports PCI tunneled operations (atomics and as_notify). This
> patch adds support for tunneled operations on powernv, with a new
> API, to be called by device drivers:
>
> pnv_pci_enable_tunnel()
> Enable tunnel operations, tell driver the 16-bit ASN indication
> used by kernel.
>
> pnv_pci_disable_tunnel()
> Disable tunnel operations.
>
> pnv_pci_set_tunnel_bar()
> Tell kernel the Tunnel BAR Response address used by driver.
> This function uses two new OPAL calls, as the PBCQ Tunnel BAR
> register is configured by skiboot.
>
> pnv_pci_get_as_notify_info()
> Return the ASN info of the thread to be woken up.
>
> Signed-off-by: Philippe Bergheaud <felix@linux.vnet.ibm.com>
> Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> ---
> Changelog:
>
> v2: Do not set the ASN indication. Get it from the device tree.
>
> v3: Make pnv_pci_get_phb_node() available when compiling without cxl.
>
> v4: Add pnv_pci_get_as_notify_info().
> Rebase opal call numbers on skiboot 5.9.6.
>
> v5: pnv_pci_get_tunnel_ind():
> - fix node reference count
> pnv_pci_get_as_notify_info():
> - fail if task == NULL
> - read pid from mm->context.id
> - explain that thread.tidr require CONFIG_PPC64
>
> v6: pnv_pci_get_tunnel_ind():
> - check if radix is enabled, or else return an error
> pnv_pci_get_as_notify_info():
> - remove a capi-specific comment, irrelevant for pci
>
> v7: pnv_pci_set_tunnel_bar():
> - setting the tunnel bar more than once with the same value
> is not an error
>
> v8: No change
>
> v9: Rename pnv_pci_get_tunnel_ind() into pnv_pci_enable_tunnel():
> - Increase real window size to accept as_notify messages.
> New api pnv_pci_disable_tunnel():
> - Restore real window size to its default value.
> Adjust opal call numbers.
>
> v10: Adjust opal call numbers to their final values.
> ---
Reviewed-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
^ permalink raw reply
* Re: [PATCH 10/10] powerpc/mm/slice: remove radix calls to the slice code
From: Christophe LEROY @ 2018-03-06 15:12 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Aneesh Kumar K . V
In-Reply-To: <20180306132507.10649-11-npiggin@gmail.com>
Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :
> This is a tidy up which removes radix MMU calls into the slice
> code.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/hugetlb.h | 9 ++++++---
> arch/powerpc/mm/hugetlbpage.c | 5 +++--
> arch/powerpc/mm/slice.c | 17 ++++-------------
> 3 files changed, 13 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/hugetlb.h b/arch/powerpc/include/asm/hugetlb.h
> index 1a4847f67ea8..59885d444695 100644
> --- a/arch/powerpc/include/asm/hugetlb.h
> +++ b/arch/powerpc/include/asm/hugetlb.h
> @@ -90,16 +90,19 @@ pte_t *huge_pte_offset_and_shift(struct mm_struct *mm,
> void flush_dcache_icache_hugepage(struct page *page);
>
> #if defined(CONFIG_PPC_MM_SLICES)
> -int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
> +int slice_is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
> unsigned long len);
> -#else
> +#endif
> static inline int is_hugepage_only_range(struct mm_struct *mm,
> unsigned long addr,
> unsigned long len)
> {
> +#if defined(CONFIG_PPC_MM_SLICES)
> + if (!radix_enabled())
> + return slice_is_hugepage_only_range(mm, addr, len);
> +#endif
> return 0;
Might be easier to understand as
if (!IS_ENABLED(CONFIG_PPC_MM_SLICES) || radix_enabled())
return 0;
return slice_is_hugepage_only_range(mm, addr, len);
> }
> -#endif
>
> void book3e_hugetlb_preload(struct vm_area_struct *vma, unsigned long ea,
> pte_t pte);
> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
> index 590be3fa0ce2..b29d40889d1c 100644
> --- a/arch/powerpc/mm/hugetlbpage.c
> +++ b/arch/powerpc/mm/hugetlbpage.c
> @@ -565,10 +565,11 @@ unsigned long hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
> unsigned long vma_mmu_pagesize(struct vm_area_struct *vma)
> {
> #ifdef CONFIG_PPC_MM_SLICES
> - unsigned int psize = get_slice_psize(vma->vm_mm, vma->vm_start);
> /* With radix we don't use slice, so derive it from vma*/
> - if (!radix_enabled())
> + if (!radix_enabled()) {
> + unsigned int psize = get_slice_psize(vma->vm_mm, vma->vm_start);
Insert a blank line here.
Christophe
> return 1UL << mmu_psize_to_shift(psize);
> + }
> #endif
> if (!is_vm_hugetlb_page(vma))
> return PAGE_SIZE;
> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index 507d17e2cfcd..15a857772617 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -697,16 +697,8 @@ unsigned int get_slice_psize(struct mm_struct *mm, unsigned long addr)
> unsigned char *psizes;
> int index, mask_index;
>
> - /*
> - * Radix doesn't use slice, but can get enabled along with MMU_SLICE
> - */
> - if (radix_enabled()) {
> -#ifdef CONFIG_PPC_64K_PAGES
> - return MMU_PAGE_64K;
> -#else
> - return MMU_PAGE_4K;
> -#endif
> - }
> + VM_BUG_ON(radix_enabled());
> +
> if (addr < SLICE_LOW_TOP) {
> psizes = mm->context.low_slices_psize;
> index = GET_LOW_SLICE_INDEX(addr);
> @@ -788,14 +780,13 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
> * for now as we only use slices with hugetlbfs enabled. This should
> * be fixed as the generic code gets fixed.
> */
> -int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
> +int slice_is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
> unsigned long len)
> {
> const struct slice_mask *maskp;
> unsigned int psize = mm->context.user_psize;
>
> - if (radix_enabled())
> - return 0;
> + VM_BUG_ON(radix_enabled());
>
> maskp = slice_mask_for_size(mm, psize);
> #ifdef CONFIG_PPC_64K_PAGES
>
^ permalink raw reply
* Re: [PATCH] powerpc/perf: Fix kernel address leaks via Sampling registers
From: kbuild test robot @ 2018-03-06 15:06 UTC (permalink / raw)
To: Madhavan Srinivasan; +Cc: kbuild-all, mpe, Madhavan Srinivasan, linuxppc-dev
In-Reply-To: <1520165709-20524-1-git-send-email-maddy@linux.vnet.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 1009 bytes --]
Hi Michael,
I love your patch! Yet something to improve:
[auto build test ERROR on v4.16-rc1]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Madhavan-Srinivasan/powerpc-perf-Fix-kernel-address-leaks-via-Sampling-registers/20180306-041036
config: powerpc-pmac32_defconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
{standard input}: Assembler messages:
>> {standard input}:2476: Error: unsupported relocation against SPRN_SDAR
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23336 bytes --]
^ permalink raw reply
* Re: [PATCH 09/10] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations
From: Christophe LEROY @ 2018-03-06 15:02 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Aneesh Kumar K . V
In-Reply-To: <20180306132507.10649-10-npiggin@gmail.com>
Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :
> The number of high slices a process might use now depends on its
> address space size, and what allocation address it has requested.
>
> This patch uses that limit throughout call chains where possible,
> rather than use the fixed SLICE_NUM_HIGH for bitmap operations.
> This saves some cost for processes that don't use very large address
> spaces.
>
> Perormance numbers aren't changed significantly, this may change
> with larger address spaces or different mmap access patterns that
> require more slice mask building.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/mm/slice.c | 75 +++++++++++++++++++++++++++++--------------------
> 1 file changed, 45 insertions(+), 30 deletions(-)
>
> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index 086c31b8b982..507d17e2cfcd 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -61,14 +61,12 @@ static void slice_print_mask(const char *label, const struct slice_mask *mask) {
> #endif
>
> static void slice_range_to_mask(unsigned long start, unsigned long len,
> - struct slice_mask *ret)
> + struct slice_mask *ret,
> + unsigned long high_slices)
> {
> unsigned long end = start + len - 1;
>
> ret->low_slices = 0;
> - if (SLICE_NUM_HIGH)
> - bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
> -
> if (start < SLICE_LOW_TOP) {
> unsigned long mend = min(end,
> (unsigned long)(SLICE_LOW_TOP - 1));
> @@ -77,6 +75,10 @@ static void slice_range_to_mask(unsigned long start, unsigned long len,
> - (1u << GET_LOW_SLICE_INDEX(start));
> }
>
> + if (!SLICE_NUM_HIGH)
> + return;
> +
> + bitmap_zero(ret->high_slices, high_slices);
In include/linux/bitmap.h, it is said:
* Note that nbits should be always a compile time evaluable constant.
* Otherwise many inlines will generate horrible code.
Not sure that's true, but it is written ...
> if ((start + len) > SLICE_LOW_TOP) {
> unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
> unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
> @@ -120,22 +122,20 @@ static int slice_high_has_vma(struct mm_struct *mm, unsigned long slice)
> }
>
> static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret,
> - unsigned long high_limit)
> + unsigned long high_slices)
> {
> unsigned long i;
>
> ret->low_slices = 0;
> - if (SLICE_NUM_HIGH)
> - bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
> -
> for (i = 0; i < SLICE_NUM_LOW; i++)
> if (!slice_low_has_vma(mm, i))
> ret->low_slices |= 1u << i;
>
> - if (high_limit <= SLICE_LOW_TOP)
> + if (!SLICE_NUM_HIGH || !high_slices)
> return;
>
> - for (i = 0; i < GET_HIGH_SLICE_INDEX(high_limit); i++)
> + bitmap_zero(ret->high_slices, high_slices);
> + for (i = 0; i < high_slices; i++)
> if (!slice_high_has_vma(mm, i))
> __set_bit(i, ret->high_slices);
> }
> @@ -232,6 +232,7 @@ static void slice_convert(struct mm_struct *mm,
> {
> int index, mask_index;
> /* Write the new slice psize bits */
> + unsigned long high_slices;
> unsigned char *hpsizes, *lpsizes;
> struct slice_mask *psize_mask, *old_mask;
> unsigned long i, flags;
> @@ -267,7 +268,8 @@ static void slice_convert(struct mm_struct *mm,
> }
>
> hpsizes = mm->context.high_slices_psize;
> - for (i = 0; i < GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit); i++) {
> + high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
> + for (i = 0; SLICE_NUM_HIGH && i < high_slices; i++) {
> if (!test_bit(i, mask->high_slices))
> continue;
>
> @@ -434,32 +436,37 @@ static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
> }
>
> static inline void slice_copy_mask(struct slice_mask *dst,
> - const struct slice_mask *src)
> + const struct slice_mask *src,
> + unsigned long high_slices)
> {
> dst->low_slices = src->low_slices;
> if (!SLICE_NUM_HIGH)
> return;
> - bitmap_copy(dst->high_slices, src->high_slices, SLICE_NUM_HIGH);
> + bitmap_copy(dst->high_slices, src->high_slices, high_slices);
> }
>
> static inline void slice_or_mask(struct slice_mask *dst,
> const struct slice_mask *src1,
> - const struct slice_mask *src2)
> + const struct slice_mask *src2,
> + unsigned long high_slices)
> {
> dst->low_slices = src1->low_slices | src2->low_slices;
> if (!SLICE_NUM_HIGH)
> return;
> - bitmap_or(dst->high_slices, src1->high_slices, src2->high_slices, SLICE_NUM_HIGH);
> + bitmap_or(dst->high_slices, src1->high_slices, src2->high_slices,
> + high_slices);
Why a new line here, this line is shorter than before.
Or that was forgotten in a previous patch ?
> }
>
> static inline void slice_andnot_mask(struct slice_mask *dst,
> const struct slice_mask *src1,
> - const struct slice_mask *src2)
> + const struct slice_mask *src2,
> + unsigned long high_slices)
> {
> dst->low_slices = src1->low_slices & ~src2->low_slices;
> if (!SLICE_NUM_HIGH)
> return;
> - bitmap_andnot(dst->high_slices, src1->high_slices, src2->high_slices, SLICE_NUM_HIGH);
> + bitmap_andnot(dst->high_slices, src1->high_slices, src2->high_slices,
> + high_slices);
Same comment.
> }
>
> #ifdef CONFIG_PPC_64K_PAGES
> @@ -482,6 +489,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> struct mm_struct *mm = current->mm;
> unsigned long newaddr;
> unsigned long high_limit;
> + unsigned long high_slices;
>
> high_limit = DEFAULT_MAP_WINDOW;
> if (addr >= high_limit || (fixed && (addr + len > high_limit)))
> @@ -498,6 +506,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> return -ENOMEM;
> }
>
> + high_slices = GET_HIGH_SLICE_INDEX(high_limit);
> if (high_limit > mm->context.slb_addr_limit) {
> /*
> * Increasing the slb_addr_limit does not require
> @@ -557,13 +566,13 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> if (psize == MMU_PAGE_64K) {
> compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
> if (fixed)
> - slice_or_mask(&good_mask, maskp, compat_maskp);
> + slice_or_mask(&good_mask, maskp, compat_maskp, high_slices);
> else
> - slice_copy_mask(&good_mask, maskp);
> + slice_copy_mask(&good_mask, maskp, high_slices);
> } else
> #endif
> {
> - slice_copy_mask(&good_mask, maskp);
> + slice_copy_mask(&good_mask, maskp, high_slices);
> }
> slice_print_mask(" good_mask", &good_mask);
> if (compat_maskp)
> @@ -596,8 +605,8 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> * We don't fit in the good mask, check what other slices are
> * empty and thus can be converted
> */
> - slice_mask_for_free(mm, &potential_mask, high_limit);
> - slice_or_mask(&potential_mask, &potential_mask, &good_mask);
> + slice_mask_for_free(mm, &potential_mask, high_slices);
> + slice_or_mask(&potential_mask, &potential_mask, &good_mask, high_slices);
> slice_print_mask(" potential", &potential_mask);
>
> if (addr || fixed) {
> @@ -634,7 +643,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> #ifdef CONFIG_PPC_64K_PAGES
> if (addr == -ENOMEM && psize == MMU_PAGE_64K) {
> /* retry the search with 4k-page slices included */
> - slice_or_mask(&potential_mask, &potential_mask, compat_maskp);
> + slice_or_mask(&potential_mask, &potential_mask, compat_maskp, high_slices);
> addr = slice_find_area(mm, len, &potential_mask,
> psize, topdown, high_limit);
> }
> @@ -643,17 +652,17 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> if (addr == -ENOMEM)
> return -ENOMEM;
>
> - slice_range_to_mask(addr, len, &potential_mask);
> + slice_range_to_mask(addr, len, &potential_mask, high_slices);
> slice_dbg(" found potential area at 0x%lx\n", addr);
> slice_print_mask(" mask", &potential_mask);
>
> convert:
> - slice_andnot_mask(&potential_mask, &potential_mask, &good_mask);
> + slice_andnot_mask(&potential_mask, &potential_mask, &good_mask, high_slices);
> if (compat_maskp && !fixed)
> - slice_andnot_mask(&potential_mask, &potential_mask, compat_maskp);
> + slice_andnot_mask(&potential_mask, &potential_mask, compat_maskp, high_slices);
> if (potential_mask.low_slices ||
> (SLICE_NUM_HIGH &&
> - !bitmap_empty(potential_mask.high_slices, SLICE_NUM_HIGH))) {
> + !bitmap_empty(potential_mask.high_slices, high_slices))) {
Are we sure high_slices is not nul here when SLICE_NUM_HIGH is not nul ?
Christophe
> slice_convert(mm, &potential_mask, psize);
> if (psize > MMU_PAGE_BASE)
> on_each_cpu(slice_flush_segments, mm, 1);
> @@ -727,7 +736,9 @@ void slice_init_new_context_exec(struct mm_struct *mm)
> mm->context.user_psize = psize;
>
> /*
> - * Set all slice psizes to the default.
> + * Set all slice psizes to the default. High slices could
> + * be initialised up to slb_addr_limit if we ensure to
> + * initialise the rest of them as slb_addr_limit is expanded.
> */
> lpsizes = mm->context.low_slices_psize;
> memset(lpsizes, (psize << 4) | psize, SLICE_NUM_LOW >> 1);
> @@ -748,10 +759,12 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
> unsigned long len, unsigned int psize)
> {
> struct slice_mask mask;
> + unsigned long high_slices;
>
> VM_BUG_ON(radix_enabled());
>
> - slice_range_to_mask(start, len, &mask);
> + high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
> + slice_range_to_mask(start, len, &mask, high_slices);
> slice_convert(mm, &mask, psize);
> }
>
> @@ -790,9 +803,11 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
> if (psize == MMU_PAGE_64K) {
> const struct slice_mask *compat_maskp;
> struct slice_mask available;
> + unsigned long high_slices;
>
> compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
> - slice_or_mask(&available, maskp, compat_maskp);
> + high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
> + slice_or_mask(&available, maskp, compat_maskp, high_slices);
> return !slice_check_range_fits(mm, &available, addr, len);
> }
> #endif
>
^ permalink raw reply
* Re: [PATCH 08/10] powerpc/mm/slice: Use const pointers to cached slice masks where possible
From: Christophe LEROY @ 2018-03-06 14:55 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Aneesh Kumar K . V
In-Reply-To: <20180306132507.10649-9-npiggin@gmail.com>
Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :
> The slice_mask cache was a basic conversion which copied the slice
> mask into caller's structures, because that's how the original code
> worked. In most cases the pointer can be used directly instead, saving
> a copy and an on-stack structure.
>
> On POWER8, this increases vfork+exec+exit performance by 0.3%
> and reduces time to mmap+munmap a 64kB page by 2%.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/mm/slice.c | 77 +++++++++++++++++++++----------------------------
> 1 file changed, 33 insertions(+), 44 deletions(-)
>
> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index 46daa1d1794f..086c31b8b982 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -472,10 +472,10 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> unsigned long flags, unsigned int psize,
> int topdown)
> {
> - struct slice_mask mask;
> struct slice_mask good_mask;
> struct slice_mask potential_mask;
> - struct slice_mask compat_mask;
> + const struct slice_mask *maskp;
> + const struct slice_mask *compat_maskp = NULL;
> int fixed = (flags & MAP_FIXED);
> int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);
> unsigned long page_size = 1UL << pshift;
> @@ -509,22 +509,6 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> on_each_cpu(slice_flush_segments, mm, 1);
> }
>
> - /*
> - * init different masks
> - */
> - mask.low_slices = 0;
> -
> - /* silence stupid warning */;
> - potential_mask.low_slices = 0;
> -
> - compat_mask.low_slices = 0;
> -
> - if (SLICE_NUM_HIGH) {
> - bitmap_zero(mask.high_slices, SLICE_NUM_HIGH);
> - bitmap_zero(potential_mask.high_slices, SLICE_NUM_HIGH);
> - bitmap_zero(compat_mask.high_slices, SLICE_NUM_HIGH);
> - }
> -
> /* Sanity checks */
> BUG_ON(mm->task_size == 0);
> BUG_ON(mm->context.slb_addr_limit == 0);
> @@ -547,8 +531,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> /* First make up a "good" mask of slices that have the right size
> * already
> */
> - good_mask = *slice_mask_for_size(mm, psize);
> - slice_print_mask(" good_mask", &good_mask);
> + maskp = slice_mask_for_size(mm, psize);
>
> /*
> * Here "good" means slices that are already the right page size,
> @@ -572,11 +555,19 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> #ifdef CONFIG_PPC_64K_PAGES
> /* If we support combo pages, we can allow 64k pages in 4k slices */
> if (psize == MMU_PAGE_64K) {
> - compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
> + compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
> if (fixed)
> - slice_or_mask(&good_mask, &good_mask, &compat_mask);
> - }
> + slice_or_mask(&good_mask, maskp, compat_maskp);
> + else
> + slice_copy_mask(&good_mask, maskp);
> + } else
> #endif
> + {
> + slice_copy_mask(&good_mask, maskp);
> + }
You could get something nicer by removing that #ifdef and doing instead:
if (IS_ENABLED(CONFIG_PPC_64K_PAGES) && psize == MMU_PAGE_64K) {
...
} else {
slice_copy_mask(&good_mask, maskp);
}
> + slice_print_mask(" good_mask", &good_mask);
> + if (compat_maskp)
> + slice_print_mask(" compat_mask", compat_maskp);
>
> /* First check hint if it's valid or if we have MAP_FIXED */
> if (addr || fixed) {
> @@ -643,7 +634,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> #ifdef CONFIG_PPC_64K_PAGES
> if (addr == -ENOMEM && psize == MMU_PAGE_64K) {
> /* retry the search with 4k-page slices included */
> - slice_or_mask(&potential_mask, &potential_mask, &compat_mask);
> + slice_or_mask(&potential_mask, &potential_mask, compat_maskp);
> addr = slice_find_area(mm, len, &potential_mask,
> psize, topdown, high_limit);
> }
> @@ -652,17 +643,18 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> if (addr == -ENOMEM)
> return -ENOMEM;
>
> - slice_range_to_mask(addr, len, &mask);
> + slice_range_to_mask(addr, len, &potential_mask);
> slice_dbg(" found potential area at 0x%lx\n", addr);
> - slice_print_mask(" mask", &mask);
> + slice_print_mask(" mask", &potential_mask);
>
> convert:
> - slice_andnot_mask(&mask, &mask, &good_mask);
> - slice_andnot_mask(&mask, &mask, &compat_mask);
> - if (mask.low_slices ||
> - (SLICE_NUM_HIGH &&
> - !bitmap_empty(mask.high_slices, SLICE_NUM_HIGH))) {
> - slice_convert(mm, &mask, psize);
> + slice_andnot_mask(&potential_mask, &potential_mask, &good_mask);
> + if (compat_maskp && !fixed)
> + slice_andnot_mask(&potential_mask, &potential_mask, compat_maskp);
> + if (potential_mask.low_slices ||
> + (SLICE_NUM_HIGH &&
> + !bitmap_empty(potential_mask.high_slices, SLICE_NUM_HIGH))) {
> + slice_convert(mm, &potential_mask, psize);
> if (psize > MMU_PAGE_BASE)
> on_each_cpu(slice_flush_segments, mm, 1);
> }
> @@ -786,28 +778,25 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
> int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
> unsigned long len)
> {
> - struct slice_mask available;
> + const struct slice_mask *maskp;
> unsigned int psize = mm->context.user_psize;
>
> if (radix_enabled())
> return 0;
>
> - available = *slice_mask_for_size(mm, psize);
> + maskp = slice_mask_for_size(mm, psize);
> #ifdef CONFIG_PPC_64K_PAGES
> /* We need to account for 4k slices too */
> if (psize == MMU_PAGE_64K) {
> - struct slice_mask compat_mask;
> - compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
> - slice_or_mask(&available, &available, &compat_mask);
> + const struct slice_mask *compat_maskp;
> + struct slice_mask available;
> +
> + compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
> + slice_or_mask(&available, maskp, compat_maskp);
> + return !slice_check_range_fits(mm, &available, addr, len);
> }
> #endif
>
> -#if 0 /* too verbose */
> - slice_dbg("is_hugepage_only_range(mm=%p, addr=%lx, len=%lx)\n",
> - mm, addr, len);
> - slice_print_mask(" mask", &mask);
> - slice_print_mask(" available", &available);
> -#endif
That's cleanup, should be in a previous patch.
Christophe
> - return !slice_check_range_fits(mm, &available, addr, len);
> + return !slice_check_range_fits(mm, maskp, addr, len);
> }
> #endif
>
^ permalink raw reply
* Re: [PATCH 07/10] powerpc/mm/slice: Switch to 3-operand slice bitops helpers
From: Christophe LEROY @ 2018-03-06 14:44 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Aneesh Kumar K . V
In-Reply-To: <20180306132507.10649-8-npiggin@gmail.com>
Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :
> This converts the slice_mask bit operation helpers to be the usual
> 3-operand kind, which is clearer to work with.
What's the real benefit of doing that ?
It is helps for a subsequent patch, say it.
Otherwise, I really can't see the point.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/mm/slice.c | 38 +++++++++++++++++++++++---------------
> 1 file changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index 3841fca75006..46daa1d1794f 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -433,25 +433,33 @@ static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
> return slice_find_area_bottomup(mm, len, mask, psize, high_limit);
> }
>
> -static inline void slice_or_mask(struct slice_mask *dst,
> +static inline void slice_copy_mask(struct slice_mask *dst,
> const struct slice_mask *src)
This new function is not used, the compiler while probably not be happy.
Christophe
> {
> - dst->low_slices |= src->low_slices;
> + dst->low_slices = src->low_slices;
> if (!SLICE_NUM_HIGH)
> return;
> - bitmap_or(dst->high_slices, dst->high_slices, src->high_slices,
> - SLICE_NUM_HIGH);
> + bitmap_copy(dst->high_slices, src->high_slices, SLICE_NUM_HIGH);
> }
>
> -static inline void slice_andnot_mask(struct slice_mask *dst,
> - const struct slice_mask *src)
> +static inline void slice_or_mask(struct slice_mask *dst,
> + const struct slice_mask *src1,
> + const struct slice_mask *src2)
> {
> - dst->low_slices &= ~src->low_slices;
> + dst->low_slices = src1->low_slices | src2->low_slices;
> + if (!SLICE_NUM_HIGH)
> + return;
> + bitmap_or(dst->high_slices, src1->high_slices, src2->high_slices, SLICE_NUM_HIGH);
> +}
>
> +static inline void slice_andnot_mask(struct slice_mask *dst,
> + const struct slice_mask *src1,
> + const struct slice_mask *src2)
> +{
> + dst->low_slices = src1->low_slices & ~src2->low_slices;
> if (!SLICE_NUM_HIGH)
> return;
> - bitmap_andnot(dst->high_slices, dst->high_slices, src->high_slices,
> - SLICE_NUM_HIGH);
> + bitmap_andnot(dst->high_slices, src1->high_slices, src2->high_slices, SLICE_NUM_HIGH);
> }
>
> #ifdef CONFIG_PPC_64K_PAGES
> @@ -566,7 +574,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> if (psize == MMU_PAGE_64K) {
> compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
> if (fixed)
> - slice_or_mask(&good_mask, &compat_mask);
> + slice_or_mask(&good_mask, &good_mask, &compat_mask);
> }
> #endif
>
> @@ -598,7 +606,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> * empty and thus can be converted
> */
> slice_mask_for_free(mm, &potential_mask, high_limit);
> - slice_or_mask(&potential_mask, &good_mask);
> + slice_or_mask(&potential_mask, &potential_mask, &good_mask);
> slice_print_mask(" potential", &potential_mask);
>
> if (addr || fixed) {
> @@ -635,7 +643,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> #ifdef CONFIG_PPC_64K_PAGES
> if (addr == -ENOMEM && psize == MMU_PAGE_64K) {
> /* retry the search with 4k-page slices included */
> - slice_or_mask(&potential_mask, &compat_mask);
> + slice_or_mask(&potential_mask, &potential_mask, &compat_mask);
> addr = slice_find_area(mm, len, &potential_mask,
> psize, topdown, high_limit);
> }
> @@ -649,8 +657,8 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> slice_print_mask(" mask", &mask);
>
> convert:
> - slice_andnot_mask(&mask, &good_mask);
> - slice_andnot_mask(&mask, &compat_mask);
> + slice_andnot_mask(&mask, &mask, &good_mask);
> + slice_andnot_mask(&mask, &mask, &compat_mask);
> if (mask.low_slices ||
> (SLICE_NUM_HIGH &&
> !bitmap_empty(mask.high_slices, SLICE_NUM_HIGH))) {
> @@ -790,7 +798,7 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
> if (psize == MMU_PAGE_64K) {
> struct slice_mask compat_mask;
> compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
> - slice_or_mask(&available, &compat_mask);
> + slice_or_mask(&available, &available, &compat_mask);
> }
> #endif
>
>
^ permalink raw reply
* Re: [PATCH 06/10] powerpc/mm/slice: implement slice_check_range_fits
From: Christophe LEROY @ 2018-03-06 14:41 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Aneesh Kumar K . V
In-Reply-To: <20180306132507.10649-7-npiggin@gmail.com>
Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :
> Rather than build slice masks from a range then use that to check for
> fit in a candidate mask, implement slice_check_range_fits that checks
> if a range fits in a mask directly.
>
> This allows several structures to be removed from stacks, and also we
> don't expect a huge range in a lot of these cases, so building and
> comparing a full mask is going to be more expensive than testing just
> one or two bits of the range.
>
> On POWER8, this increases vfork+exec+exit performance by 0.3%
> and reduces time to mmap+munmap a 64kB page by 5%.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/mm/slice.c | 63 ++++++++++++++++++++++++++-----------------------
> 1 file changed, 34 insertions(+), 29 deletions(-)
>
> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index 2115efe5e869..3841fca75006 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -179,26 +179,35 @@ static struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int psize)
> #error "Must define the slice masks for page sizes supported by the platform"
> #endif
>
> -static int slice_check_fit(struct mm_struct *mm,
> - const struct slice_mask *mask,
> - const struct slice_mask *available)
> +static bool slice_check_range_fits(struct mm_struct *mm,
> + const struct slice_mask *available,
> + unsigned long start, unsigned long len)
> {
> - DECLARE_BITMAP(result, SLICE_NUM_HIGH);
> - /*
> - * Make sure we just do bit compare only to the max
> - * addr limit and not the full bit map size.
> - */
> - unsigned long slice_count = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
> + unsigned long end = start + len - 1;
> + u64 low_slices = 0;
>
> - if (!SLICE_NUM_HIGH)
> - return (mask->low_slices & available->low_slices) ==
> - mask->low_slices;
> + if (start < SLICE_LOW_TOP) {
> + unsigned long mend = min(end, (SLICE_LOW_TOP - 1));
See slice_range_to_mask()
You'll have an issue here with PPC32, you have to cast (SLICE_LOW_TOP -
1) to unsigned long because SLICE_LOW_TOP is unsigned long long on PPC32
> +
> + low_slices = (1u << (GET_LOW_SLICE_INDEX(mend) + 1))
> + - (1u << GET_LOW_SLICE_INDEX(start));
> + }
> + if ((low_slices & available->low_slices) != low_slices)
> + return false;
> +
> + if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
> + unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
> + unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
> + unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
> + unsigned long i;
>
> - bitmap_and(result, mask->high_slices,
> - available->high_slices, slice_count);
> + for (i = start_index; i < start_index + count; i++) {
> + if (!test_bit(i, available->high_slices))
> + return false;
> + }
What about using bitmap_find_next_zero_area()
> + }
>
> - return (mask->low_slices & available->low_slices) == mask->low_slices &&
> - bitmap_equal(result, mask->high_slices, slice_count);
> + return true;
> }
>
> static void slice_flush_segments(void *parm)
> @@ -562,15 +571,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> #endif
>
> /* First check hint if it's valid or if we have MAP_FIXED */
> - if (addr != 0 || fixed) {
> - /* Build a mask for the requested range */
> - slice_range_to_mask(addr, len, &mask);
> - slice_print_mask(" mask", &mask);
> -
> + if (addr || fixed) {
It is cleanup, should it really be part of this patch ?
> /* Check if we fit in the good mask. If we do, we just return,
> * nothing else to do
> */
> - if (slice_check_fit(mm, &mask, &good_mask)) {
> + if (slice_check_range_fits(mm, &good_mask, addr, len)) {
> slice_dbg(" fits good !\n");
> return addr;
> }
> @@ -596,10 +601,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> slice_or_mask(&potential_mask, &good_mask);
> slice_print_mask(" potential", &potential_mask);
>
> - if ((addr != 0 || fixed) &&
> - slice_check_fit(mm, &mask, &potential_mask)) {
> - slice_dbg(" fits potential !\n");
> - goto convert;
> + if (addr || fixed) {
> + if (slice_check_range_fits(mm, &potential_mask, addr, len)) {
> + slice_dbg(" fits potential !\n");
> + goto convert;
> + }
Why not keep the original structure and just replacing slice_check_fit()
by slice_check_range_fits() ?
I believe cleanups should not be mixed with real feature changes. If
needed, you should have a cleanup patch up front the serie.
Christophe
> }
>
> /* If we have MAP_FIXED and failed the above steps, then error out */
> @@ -772,13 +778,12 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
> int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
> unsigned long len)
> {
> - struct slice_mask mask, available;
> + struct slice_mask available;
> unsigned int psize = mm->context.user_psize;
>
> if (radix_enabled())
> return 0;
>
> - slice_range_to_mask(addr, len, &mask);
> available = *slice_mask_for_size(mm, psize);
> #ifdef CONFIG_PPC_64K_PAGES
> /* We need to account for 4k slices too */
> @@ -795,6 +800,6 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
> slice_print_mask(" mask", &mask);
> slice_print_mask(" available", &available);
> #endif
> - return !slice_check_fit(mm, &mask, &available);
> + return !slice_check_range_fits(mm, &available, addr, len);
> }
> #endif
>
^ permalink raw reply
* Re: [PATCH 02/10] powerpc/mm/slice: Simplify and optimise slice context initialisation
From: Nicholas Piggin @ 2018-03-06 14:32 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Aneesh Kumar K . V, Christophe Leroy
In-Reply-To: <20180306132507.10649-3-npiggin@gmail.com>
On Tue, 6 Mar 2018 23:24:59 +1000
Nicholas Piggin <npiggin@gmail.com> wrote:
> diff --git a/arch/powerpc/mm/mmu_context_book3s64.c b/arch/powerpc/mm/mmu_context_book3s64.c
> index 929d9ef7083f..80acad52b006 100644
> --- a/arch/powerpc/mm/mmu_context_book3s64.c
> +++ b/arch/powerpc/mm/mmu_context_book3s64.c
> @@ -93,13 +93,6 @@ static int hash__init_new_context(struct mm_struct *mm)
> if (index < 0)
> return index;
>
> - /*
> - * In the case of exec, use the default limit,
> - * otherwise inherit it from the mm we are duplicating.
> - */
> - if (!mm->context.slb_addr_limit)
> - mm->context.slb_addr_limit = DEFAULT_MAP_WINDOW_USER64;
[...]
> diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c
> index d98f7e5c141b..be8f5c9d4d08 100644
> --- a/arch/powerpc/mm/mmu_context_nohash.c
> +++ b/arch/powerpc/mm/mmu_context_nohash.c
> @@ -332,9 +332,6 @@ int init_new_context(struct task_struct *t, struct mm_struct *mm)
> pr_hard("initing context for mm @%p\n", mm);
>
> #ifdef CONFIG_PPC_MM_SLICES
> - if (!mm->context.slb_addr_limit)
> - mm->context.slb_addr_limit = DEFAULT_MAP_WINDOW;
> -
[...]
> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index 5e9e1e57d580..af4351b15d01 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -671,70 +671,29 @@ unsigned int get_slice_psize(struct mm_struct *mm, unsigned long addr)
> }
> EXPORT_SYMBOL_GPL(get_slice_psize);
>
> -/*
> - * This is called by hash_page when it needs to do a lazy conversion of
> - * an address space from real 64K pages to combo 4K pages (typically
> - * when hitting a non cacheable mapping on a processor or hypervisor
> - * that won't allow them for 64K pages).
> - *
> - * This is also called in init_new_context() to change back the user
> - * psize from whatever the parent context had it set to
> - * N.B. This may be called before mm->context.id has been set.
> - *
> - * This function will only change the content of the {low,high)_slice_psize
> - * masks, it will not flush SLBs as this shall be handled lazily by the
> - * caller.
> - */
> -void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
> +void slice_init_new_context_exec(struct mm_struct *mm)
> {
> - int index, mask_index;
> unsigned char *hpsizes, *lpsizes;
> - unsigned long flags;
> - unsigned int old_psize;
> - int i;
> -
> - slice_dbg("slice_set_user_psize(mm=%p, psize=%d)\n", mm, psize);
> -
> - VM_BUG_ON(radix_enabled());
> - spin_lock_irqsave(&slice_convert_lock, flags);
> + unsigned int psize = mmu_virtual_psize;
>
> - old_psize = mm->context.user_psize;
> - slice_dbg(" old_psize=%d\n", old_psize);
> - if (old_psize == psize)
> - goto bail;
> + slice_dbg("slice_init_new_context_exec(mm=%p)\n", mm);
>
> + /*
> + * In the case of exec, use the default limit. In the
> + * case of fork it is just inherited from the mm being
> + * duplicated.
> + */
> + mm->context.slb_addr_limit = DEFAULT_MAP_WINDOW_USER64;
Okay this looks wrong for 8xx now I just noticed. But... isn't existing
code wrong for 32-bit tasks on BOOK3S_64? It should be DEFAULT_MAP_WINDOW
here I think?
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 05/10] powerpc/mm/slice: implement a slice mask cache
From: Nicholas Piggin @ 2018-03-06 14:01 UTC (permalink / raw)
To: Christophe LEROY
Cc: linuxppc-dev, Aneesh Kumar K . V, Benjamin Herrenschmidt,
Anton Blanchard
In-Reply-To: <81a073ab-398f-be65-eb3c-5d5ac180ec94@c-s.fr>
On Tue, 6 Mar 2018 14:49:57 +0100
Christophe LEROY <christophe.leroy@c-s.fr> wrote:
> Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :
> > @@ -201,6 +206,15 @@ typedef struct {
> > unsigned char low_slices_psize[SLICE_ARRAY_SIZE];
> > unsigned char high_slices_psize[0];
> > unsigned long slb_addr_limit;
> > +# ifdef CONFIG_PPC_16K_PAGES
> > + struct slice_mask mask_16k;
> > +# else
> > + struct slice_mask mask_4k;
> > +# endif
>
> Could we just call it mask_base or something like that regardless of the
> standard page size ?
[...]
> > +#elif defined(CONFIG_PPC_8xx)
> > +static struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int psize)
> > +{
> > +#ifdef CONFIG_PPC_16K_PAGES
> > + if (psize == MMU_PAGE_16K)
> > + return &mm->context.mask_16k;
> > +#else
> > + if (psize == MMU_PAGE_4K)
> > + return &mm->context.mask_4k;
> > +#endif
>
> What about the following instead:
> + if (psize == mmu_virtual_size)
> + return &mm->context.mask_base;
Sure if you prefer. It should generate the same code right?
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 04/10] powerpc/mm/slice: pass pointers to struct slice_mask where possible
From: Nicholas Piggin @ 2018-03-06 13:59 UTC (permalink / raw)
To: Christophe LEROY; +Cc: linuxppc-dev, Aneesh Kumar K . V
In-Reply-To: <c0f290dd-33dd-6ba7-631a-1389b9a6d779@c-s.fr>
On Tue, 6 Mar 2018 14:43:40 +0100
Christophe LEROY <christophe.leroy@c-s.fr> wrote:
> Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :
> > @@ -147,7 +149,8 @@ static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret,
> > __set_bit(i, ret->high_slices);
> > }
> >
> > -static void slice_mask_for_size(struct mm_struct *mm, int psize, struct slice_mask *ret,
> > +static void slice_mask_for_size(struct mm_struct *mm, int psize,
> > + struct slice_mask *ret,
>
> That seems to be cleanup only, is it worth including that in this patch ?
Yeah that can go.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 05/10] powerpc/mm/slice: implement a slice mask cache
From: Christophe LEROY @ 2018-03-06 13:49 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: Aneesh Kumar K . V, Benjamin Herrenschmidt, Anton Blanchard
In-Reply-To: <20180306132507.10649-6-npiggin@gmail.com>
Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :
> Calculating the slice mask can become a signifcant overhead for
> get_unmapped_area. This patch adds a struct slice_mask for
> each page size in the mm_context, and keeps these in synch with
> the slices psize arrays and slb_addr_limit.
>
> On Book3S/64 this adds 288 bytes to the mm_context_t for the
> slice mask caches.
>
> On POWER8, this increases vfork+exec+exit performance by 9.9%
> and reduces time to mmap+munmap a 64kB page by 28%.
>
> Reduces time to mmap+munmap by about 10% on 8xx.
>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Anton Blanchard <anton@samba.org>
> ---
> arch/powerpc/include/asm/book3s/64/mmu.h | 18 +++++
> arch/powerpc/include/asm/mmu-8xx.h | 14 ++++
> arch/powerpc/mm/slice.c | 118 ++++++++++++++++++++-----------
> 3 files changed, 107 insertions(+), 43 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
> index bef6e39ed63a..777778579305 100644
> --- a/arch/powerpc/include/asm/book3s/64/mmu.h
> +++ b/arch/powerpc/include/asm/book3s/64/mmu.h
> @@ -80,6 +80,16 @@ struct spinlock;
> /* Maximum possible number of NPUs in a system. */
> #define NV_MAX_NPUS 8
>
> +/*
> + * One bit per slice. We have lower slices which cover 256MB segments
> + * upto 4G range. That gets us 16 low slices. For the rest we track slices
> + * in 1TB size.
> + */
> +struct slice_mask {
> + u64 low_slices;
> + DECLARE_BITMAP(high_slices, SLICE_NUM_HIGH);
> +};
> +
> typedef struct {
> mm_context_id_t id;
> u16 user_psize; /* page size index */
> @@ -95,6 +105,14 @@ typedef struct {
> unsigned char low_slices_psize[BITS_PER_LONG / BITS_PER_BYTE];
> unsigned char high_slices_psize[SLICE_ARRAY_SIZE];
> unsigned long slb_addr_limit;
> +# ifdef CONFIG_PPC_64K_PAGES
> + struct slice_mask mask_64k;
> +# endif
> + struct slice_mask mask_4k;
> +# ifdef CONFIG_HUGETLB_PAGE
> + struct slice_mask mask_16m;
> + struct slice_mask mask_16g;
> +# endif
> #else
> u16 sllp; /* SLB page size encoding */
> #endif
> diff --git a/arch/powerpc/include/asm/mmu-8xx.h b/arch/powerpc/include/asm/mmu-8xx.h
> index d3d7e79140c6..4c3b14703b3e 100644
> --- a/arch/powerpc/include/asm/mmu-8xx.h
> +++ b/arch/powerpc/include/asm/mmu-8xx.h
> @@ -192,6 +192,11 @@
> #endif
>
> #ifndef __ASSEMBLY__
> +struct slice_mask {
> + u64 low_slices;
> + DECLARE_BITMAP(high_slices, 0);
> +};
> +
> typedef struct {
> unsigned int id;
> unsigned int active;
> @@ -201,6 +206,15 @@ typedef struct {
> unsigned char low_slices_psize[SLICE_ARRAY_SIZE];
> unsigned char high_slices_psize[0];
> unsigned long slb_addr_limit;
> +# ifdef CONFIG_PPC_16K_PAGES
> + struct slice_mask mask_16k;
> +# else
> + struct slice_mask mask_4k;
> +# endif
Could we just call it mask_base or something like that regardless of the
standard page size ?
> +# ifdef CONFIG_HUGETLB_PAGE
> + struct slice_mask mask_512k;
> + struct slice_mask mask_8m;
> +# endif
> #endif
> } mm_context_t;
>
> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index 233c42d593dc..2115efe5e869 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -37,15 +37,6 @@
> #include <asm/hugetlb.h>
>
> static DEFINE_SPINLOCK(slice_convert_lock);
> -/*
> - * One bit per slice. We have lower slices which cover 256MB segments
> - * upto 4G range. That gets us 16 low slices. For the rest we track slices
> - * in 1TB size.
> - */
> -struct slice_mask {
> - u64 low_slices;
> - DECLARE_BITMAP(high_slices, SLICE_NUM_HIGH);
> -};
>
> #ifdef DEBUG
> int _slice_debug = 1;
> @@ -149,37 +140,44 @@ static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret,
> __set_bit(i, ret->high_slices);
> }
>
> -static void slice_mask_for_size(struct mm_struct *mm, int psize,
> - struct slice_mask *ret,
> - unsigned long high_limit)
> +#ifdef CONFIG_PPC_BOOK3S_64
> +static struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int psize)
> {
> - unsigned char *hpsizes, *lpsizes;
> - int index, mask_index;
> - unsigned long i;
> -
> - ret->low_slices = 0;
> - if (SLICE_NUM_HIGH)
> - bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
> -
> - lpsizes = mm->context.low_slices_psize;
> - for (i = 0; i < SLICE_NUM_LOW; i++) {
> - mask_index = i & 0x1;
> - index = i >> 1;
> - if (((lpsizes[index] >> (mask_index * 4)) & 0xf) == psize)
> - ret->low_slices |= 1u << i;
> - }
> -
> - if (high_limit <= SLICE_LOW_TOP)
> - return;
> -
> - hpsizes = mm->context.high_slices_psize;
> - for (i = 0; i < GET_HIGH_SLICE_INDEX(high_limit); i++) {
> - mask_index = i & 0x1;
> - index = i >> 1;
> - if (((hpsizes[index] >> (mask_index * 4)) & 0xf) == psize)
> - __set_bit(i, ret->high_slices);
> - }
> +#ifdef CONFIG_PPC_64K_PAGES
> + if (psize == MMU_PAGE_64K)
> + return &mm->context.mask_64k;
> +#endif
> + if (psize == MMU_PAGE_4K)
> + return &mm->context.mask_4k;
> +#ifdef CONFIG_HUGETLB_PAGE
> + if (psize == MMU_PAGE_16M)
> + return &mm->context.mask_16m;
> + if (psize == MMU_PAGE_16G)
> + return &mm->context.mask_16g;
> +#endif
> + BUG();
> }
> +#elif defined(CONFIG_PPC_8xx)
> +static struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int psize)
> +{
> +#ifdef CONFIG_PPC_16K_PAGES
> + if (psize == MMU_PAGE_16K)
> + return &mm->context.mask_16k;
> +#else
> + if (psize == MMU_PAGE_4K)
> + return &mm->context.mask_4k;
> +#endif
What about the following instead:
+ if (psize == mmu_virtual_size)
+ return &mm->context.mask_base;
Christophe
> +#ifdef CONFIG_HUGETLB_PAGE
> + if (psize == MMU_PAGE_512K)
> + return &mm->context.mask_512k;
> + if (psize == MMU_PAGE_8M)
> + return &mm->context.mask_8m;
> +#endif
> + BUG();
> +}
> +#else
> +#error "Must define the slice masks for page sizes supported by the platform"
> +#endif
>
> static int slice_check_fit(struct mm_struct *mm,
> const struct slice_mask *mask,
> @@ -226,11 +224,15 @@ static void slice_convert(struct mm_struct *mm,
> int index, mask_index;
> /* Write the new slice psize bits */
> unsigned char *hpsizes, *lpsizes;
> + struct slice_mask *psize_mask, *old_mask;
> unsigned long i, flags;
> + int old_psize;
>
> slice_dbg("slice_convert(mm=%p, psize=%d)\n", mm, psize);
> slice_print_mask(" mask", mask);
>
> + psize_mask = slice_mask_for_size(mm, psize);
> +
> /* We need to use a spinlock here to protect against
> * concurrent 64k -> 4k demotion ...
> */
> @@ -243,6 +245,14 @@ static void slice_convert(struct mm_struct *mm,
>
> mask_index = i & 0x1;
> index = i >> 1;
> +
> + /* Update the slice_mask */
> + old_psize = (lpsizes[index] >> (mask_index * 4)) & 0xf;
> + old_mask = slice_mask_for_size(mm, old_psize);
> + old_mask->low_slices &= ~(1u << i);
> + psize_mask->low_slices |= 1u << i;
> +
> + /* Update the sizes array */
> lpsizes[index] = (lpsizes[index] & ~(0xf << (mask_index * 4))) |
> (((unsigned long)psize) << (mask_index * 4));
> }
> @@ -254,6 +264,14 @@ static void slice_convert(struct mm_struct *mm,
>
> mask_index = i & 0x1;
> index = i >> 1;
> +
> + /* Update the slice_mask */
> + old_psize = (hpsizes[index] >> (mask_index * 4)) & 0xf;
> + old_mask = slice_mask_for_size(mm, old_psize);
> + __clear_bit(i, old_mask->high_slices);
> + __set_bit(i, psize_mask->high_slices);
> +
> + /* Update the sizes array */
> hpsizes[index] = (hpsizes[index] & ~(0xf << (mask_index * 4))) |
> (((unsigned long)psize) << (mask_index * 4));
> }
> @@ -464,7 +482,13 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> }
>
> if (high_limit > mm->context.slb_addr_limit) {
> + /*
> + * Increasing the slb_addr_limit does not require
> + * slice mask cache to be recalculated because it should
> + * be already initialised beyond the old address limit.
> + */
> mm->context.slb_addr_limit = high_limit;
> +
> on_each_cpu(slice_flush_segments, mm, 1);
> }
>
> @@ -506,7 +530,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> /* First make up a "good" mask of slices that have the right size
> * already
> */
> - slice_mask_for_size(mm, psize, &good_mask, high_limit);
> + good_mask = *slice_mask_for_size(mm, psize);
> slice_print_mask(" good_mask", &good_mask);
>
> /*
> @@ -531,7 +555,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> #ifdef CONFIG_PPC_64K_PAGES
> /* If we support combo pages, we can allow 64k pages in 4k slices */
> if (psize == MMU_PAGE_64K) {
> - slice_mask_for_size(mm, MMU_PAGE_4K, &compat_mask, high_limit);
> + compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
> if (fixed)
> slice_or_mask(&good_mask, &compat_mask);
> }
> @@ -683,6 +707,7 @@ EXPORT_SYMBOL_GPL(get_slice_psize);
> void slice_init_new_context_exec(struct mm_struct *mm)
> {
> unsigned char *hpsizes, *lpsizes;
> + struct slice_mask *mask;
> unsigned int psize = mmu_virtual_psize;
>
> slice_dbg("slice_init_new_context_exec(mm=%p)\n", mm);
> @@ -703,6 +728,14 @@ void slice_init_new_context_exec(struct mm_struct *mm)
>
> hpsizes = mm->context.high_slices_psize;
> memset(hpsizes, (psize << 4) | psize, SLICE_NUM_HIGH >> 1);
> +
> + /*
> + * Slice mask cache starts zeroed, fill the default size cache.
> + */
> + mask = slice_mask_for_size(mm, psize);
> + mask->low_slices = ~0UL;
> + if (SLICE_NUM_HIGH)
> + bitmap_fill(mask->high_slices, SLICE_NUM_HIGH);
> }
>
> void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
> @@ -741,18 +774,17 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
> {
> struct slice_mask mask, available;
> unsigned int psize = mm->context.user_psize;
> - unsigned long high_limit = mm->context.slb_addr_limit;
>
> if (radix_enabled())
> return 0;
>
> slice_range_to_mask(addr, len, &mask);
> - slice_mask_for_size(mm, psize, &available, high_limit);
> + available = *slice_mask_for_size(mm, psize);
> #ifdef CONFIG_PPC_64K_PAGES
> /* We need to account for 4k slices too */
> if (psize == MMU_PAGE_64K) {
> struct slice_mask compat_mask;
> - slice_mask_for_size(mm, MMU_PAGE_4K, &compat_mask, high_limit);
> + compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
> slice_or_mask(&available, &compat_mask);
> }
> #endif
>
^ permalink raw reply
* Re: [PATCH 04/10] powerpc/mm/slice: pass pointers to struct slice_mask where possible
From: Christophe LEROY @ 2018-03-06 13:43 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Aneesh Kumar K . V
In-Reply-To: <20180306132507.10649-5-npiggin@gmail.com>
Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :
> Pass around const pointers to struct slice_mask where possible, rather
> than copies of slice_mask, to reduce stack and call overhead.
>
> checkstack.pl gives, before:
> 0x00000d1c slice_get_unmapped_area [slice.o]: 592
> 0x00001864 is_hugepage_only_range [slice.o]: 448
> 0x00000754 slice_find_area_topdown [slice.o]: 400
> 0x00000484 slice_find_area_bottomup.isra.1 [slice.o]: 272
> 0x000017b4 slice_set_range_psize [slice.o]: 224
> 0x00000a4c slice_find_area [slice.o]: 128
> 0x00000160 slice_check_fit [slice.o]: 112
>
> after:
> 0x00000ad0 slice_get_unmapped_area [slice.o]: 448
> 0x00001464 is_hugepage_only_range [slice.o]: 288
> 0x000006c0 slice_find_area [slice.o]: 144
> 0x0000016c slice_check_fit [slice.o]: 128
> 0x00000528 slice_find_area_bottomup.isra.2 [slice.o]: 128
> 0x000013e4 slice_set_range_psize [slice.o]: 128
>
> This increases vfork+exec+exit performance by 1.5%.
>
> Reduces time to mmap+munmap a 64kB page by 17%.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/mm/slice.c | 87 ++++++++++++++++++++++++++-----------------------
> 1 file changed, 47 insertions(+), 40 deletions(-)
>
> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index 9625ceb35685..233c42d593dc 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -50,19 +50,21 @@ struct slice_mask {
> #ifdef DEBUG
> int _slice_debug = 1;
>
> -static void slice_print_mask(const char *label, struct slice_mask mask)
> +static void slice_print_mask(const char *label, const struct slice_mask *mask)
> {
> if (!_slice_debug)
> return;
> - pr_devel("%s low_slice: %*pbl\n", label, (int)SLICE_NUM_LOW, &mask.low_slices);
> - pr_devel("%s high_slice: %*pbl\n", label, (int)SLICE_NUM_HIGH, mask.high_slices);
> + pr_devel("%s low_slice: %*pbl\n", label,
> + (int)SLICE_NUM_LOW, &mask->low_slices);
> + pr_devel("%s high_slice: %*pbl\n", label,
> + (int)SLICE_NUM_HIGH, mask->high_slices);
> }
>
> #define slice_dbg(fmt...) do { if (_slice_debug) pr_devel(fmt); } while (0)
>
> #else
>
> -static void slice_print_mask(const char *label, struct slice_mask mask) {}
> +static void slice_print_mask(const char *label, const struct slice_mask *mask) {}
> #define slice_dbg(fmt...)
>
> #endif
> @@ -147,7 +149,8 @@ static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret,
> __set_bit(i, ret->high_slices);
> }
>
> -static void slice_mask_for_size(struct mm_struct *mm, int psize, struct slice_mask *ret,
> +static void slice_mask_for_size(struct mm_struct *mm, int psize,
> + struct slice_mask *ret,
That seems to be cleanup only, is it worth including that in this patch ?
Christophe
> unsigned long high_limit)
> {
> unsigned char *hpsizes, *lpsizes;
> @@ -179,7 +182,8 @@ static void slice_mask_for_size(struct mm_struct *mm, int psize, struct slice_ma
> }
>
> static int slice_check_fit(struct mm_struct *mm,
> - struct slice_mask mask, struct slice_mask available)
> + const struct slice_mask *mask,
> + const struct slice_mask *available)
> {
> DECLARE_BITMAP(result, SLICE_NUM_HIGH);
> /*
> @@ -189,14 +193,14 @@ static int slice_check_fit(struct mm_struct *mm,
> unsigned long slice_count = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
>
> if (!SLICE_NUM_HIGH)
> - return (mask.low_slices & available.low_slices) ==
> - mask.low_slices;
> + return (mask->low_slices & available->low_slices) ==
> + mask->low_slices;
>
> - bitmap_and(result, mask.high_slices,
> - available.high_slices, slice_count);
> + bitmap_and(result, mask->high_slices,
> + available->high_slices, slice_count);
>
> - return (mask.low_slices & available.low_slices) == mask.low_slices &&
> - bitmap_equal(result, mask.high_slices, slice_count);
> + return (mask->low_slices & available->low_slices) == mask->low_slices &&
> + bitmap_equal(result, mask->high_slices, slice_count);
> }
>
> static void slice_flush_segments(void *parm)
> @@ -216,7 +220,8 @@ static void slice_flush_segments(void *parm)
> #endif
> }
>
> -static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psize)
> +static void slice_convert(struct mm_struct *mm,
> + const struct slice_mask *mask, int psize)
> {
> int index, mask_index;
> /* Write the new slice psize bits */
> @@ -233,7 +238,7 @@ static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psiz
>
> lpsizes = mm->context.low_slices_psize;
> for (i = 0; i < SLICE_NUM_LOW; i++) {
> - if (!(mask.low_slices & (1u << i)))
> + if (!(mask->low_slices & (1u << i)))
> continue;
>
> mask_index = i & 0x1;
> @@ -244,7 +249,7 @@ static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psiz
>
> hpsizes = mm->context.high_slices_psize;
> for (i = 0; i < GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit); i++) {
> - if (!test_bit(i, mask.high_slices))
> + if (!test_bit(i, mask->high_slices))
> continue;
>
> mask_index = i & 0x1;
> @@ -270,26 +275,25 @@ static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psiz
> * 'available' slice_mark.
> */
> static bool slice_scan_available(unsigned long addr,
> - struct slice_mask available,
> - int end,
> - unsigned long *boundary_addr)
> + const struct slice_mask *available,
> + int end, unsigned long *boundary_addr)
> {
> unsigned long slice;
> if (addr < SLICE_LOW_TOP) {
> slice = GET_LOW_SLICE_INDEX(addr);
> *boundary_addr = (slice + end) << SLICE_LOW_SHIFT;
> - return !!(available.low_slices & (1u << slice));
> + return !!(available->low_slices & (1u << slice));
> } else {
> slice = GET_HIGH_SLICE_INDEX(addr);
> *boundary_addr = (slice + end) ?
> ((slice + end) << SLICE_HIGH_SHIFT) : SLICE_LOW_TOP;
> - return !!test_bit(slice, available.high_slices);
> + return !!test_bit(slice, available->high_slices);
> }
> }
>
> static unsigned long slice_find_area_bottomup(struct mm_struct *mm,
> unsigned long len,
> - struct slice_mask available,
> + const struct slice_mask *available,
> int psize, unsigned long high_limit)
> {
> int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);
> @@ -335,7 +339,7 @@ static unsigned long slice_find_area_bottomup(struct mm_struct *mm,
>
> static unsigned long slice_find_area_topdown(struct mm_struct *mm,
> unsigned long len,
> - struct slice_mask available,
> + const struct slice_mask *available,
> int psize, unsigned long high_limit)
> {
> int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);
> @@ -393,7 +397,7 @@ static unsigned long slice_find_area_topdown(struct mm_struct *mm,
>
>
> static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
> - struct slice_mask mask, int psize,
> + const struct slice_mask *mask, int psize,
> int topdown, unsigned long high_limit)
> {
> if (topdown)
> @@ -402,7 +406,8 @@ static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
> return slice_find_area_bottomup(mm, len, mask, psize, high_limit);
> }
>
> -static inline void slice_or_mask(struct slice_mask *dst, struct slice_mask *src)
> +static inline void slice_or_mask(struct slice_mask *dst,
> + const struct slice_mask *src)
> {
> dst->low_slices |= src->low_slices;
> if (!SLICE_NUM_HIGH)
> @@ -411,7 +416,8 @@ static inline void slice_or_mask(struct slice_mask *dst, struct slice_mask *src)
> SLICE_NUM_HIGH);
> }
>
> -static inline void slice_andnot_mask(struct slice_mask *dst, struct slice_mask *src)
> +static inline void slice_andnot_mask(struct slice_mask *dst,
> + const struct slice_mask *src)
> {
> dst->low_slices &= ~src->low_slices;
>
> @@ -501,7 +507,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> * already
> */
> slice_mask_for_size(mm, psize, &good_mask, high_limit);
> - slice_print_mask(" good_mask", good_mask);
> + slice_print_mask(" good_mask", &good_mask);
>
> /*
> * Here "good" means slices that are already the right page size,
> @@ -535,12 +541,12 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> if (addr != 0 || fixed) {
> /* Build a mask for the requested range */
> slice_range_to_mask(addr, len, &mask);
> - slice_print_mask(" mask", mask);
> + slice_print_mask(" mask", &mask);
>
> /* Check if we fit in the good mask. If we do, we just return,
> * nothing else to do
> */
> - if (slice_check_fit(mm, mask, good_mask)) {
> + if (slice_check_fit(mm, &mask, &good_mask)) {
> slice_dbg(" fits good !\n");
> return addr;
> }
> @@ -548,7 +554,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> /* Now let's see if we can find something in the existing
> * slices for that size
> */
> - newaddr = slice_find_area(mm, len, good_mask,
> + newaddr = slice_find_area(mm, len, &good_mask,
> psize, topdown, high_limit);
> if (newaddr != -ENOMEM) {
> /* Found within the good mask, we don't have to setup,
> @@ -564,9 +570,10 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> */
> slice_mask_for_free(mm, &potential_mask, high_limit);
> slice_or_mask(&potential_mask, &good_mask);
> - slice_print_mask(" potential", potential_mask);
> + slice_print_mask(" potential", &potential_mask);
>
> - if ((addr != 0 || fixed) && slice_check_fit(mm, mask, potential_mask)) {
> + if ((addr != 0 || fixed) &&
> + slice_check_fit(mm, &mask, &potential_mask)) {
> slice_dbg(" fits potential !\n");
> goto convert;
> }
> @@ -581,7 +588,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> * anywhere in the good area.
> */
> if (addr) {
> - addr = slice_find_area(mm, len, good_mask,
> + addr = slice_find_area(mm, len, &good_mask,
> psize, topdown, high_limit);
> if (addr != -ENOMEM) {
> slice_dbg(" found area at 0x%lx\n", addr);
> @@ -592,14 +599,14 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> /* Now let's see if we can find something in the existing slices
> * for that size plus free slices
> */
> - addr = slice_find_area(mm, len, potential_mask,
> + addr = slice_find_area(mm, len, &potential_mask,
> psize, topdown, high_limit);
>
> #ifdef CONFIG_PPC_64K_PAGES
> if (addr == -ENOMEM && psize == MMU_PAGE_64K) {
> /* retry the search with 4k-page slices included */
> slice_or_mask(&potential_mask, &compat_mask);
> - addr = slice_find_area(mm, len, potential_mask,
> + addr = slice_find_area(mm, len, &potential_mask,
> psize, topdown, high_limit);
> }
> #endif
> @@ -609,7 +616,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>
> slice_range_to_mask(addr, len, &mask);
> slice_dbg(" found potential area at 0x%lx\n", addr);
> - slice_print_mask(" mask", mask);
> + slice_print_mask(" mask", &mask);
>
> convert:
> slice_andnot_mask(&mask, &good_mask);
> @@ -617,7 +624,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> if (mask.low_slices ||
> (SLICE_NUM_HIGH &&
> !bitmap_empty(mask.high_slices, SLICE_NUM_HIGH))) {
> - slice_convert(mm, mask, psize);
> + slice_convert(mm, &mask, psize);
> if (psize > MMU_PAGE_BASE)
> on_each_cpu(slice_flush_segments, mm, 1);
> }
> @@ -706,7 +713,7 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
> VM_BUG_ON(radix_enabled());
>
> slice_range_to_mask(start, len, &mask);
> - slice_convert(mm, mask, psize);
> + slice_convert(mm, &mask, psize);
> }
>
> #ifdef CONFIG_HUGETLB_PAGE
> @@ -753,9 +760,9 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
> #if 0 /* too verbose */
> slice_dbg("is_hugepage_only_range(mm=%p, addr=%lx, len=%lx)\n",
> mm, addr, len);
> - slice_print_mask(" mask", mask);
> - slice_print_mask(" available", available);
> + slice_print_mask(" mask", &mask);
> + slice_print_mask(" available", &available);
> #endif
> - return !slice_check_fit(mm, mask, available);
> + return !slice_check_fit(mm, &mask, &available);
> }
> #endif
>
^ permalink raw reply
* Re: powerpc: Keep const vars out of writable .sdata
From: Michael Ellerman @ 2018-03-06 13:31 UTC (permalink / raw)
To: Kees Cook; +Cc: linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <20180301010249.GA18370@beast>
On Thu, 2018-03-01 at 01:02:49 UTC, Kees Cook wrote:
> From: Segher Boessenkool <segher@kernel.crashing.org>
>
> Newer gcc will support "-mno-readonly-in-sdata"[1], which makes sure that
> the optimization on PPC32 for variables getting moved into the .sdata
> section will not apply to const variables (which must be in .rodata).
>
> This was originally noticed in mm/rodata_test.c when rodata_test_data
> was not static:
>
> c0695034 g O .data 00000004 rodata_test_data
>
> After this patch with an updated compiler, this is correctly in .rodata.
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82411
>
> Reported-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Segher Boessenkool <segher@kernel.crashing.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/51d42f0f5fd6c74144d19bf6a66352
cheers
^ permalink raw reply
* Re: [v5,1/6] powerpc/mm/slice: Remove intermediate bitmap copy
From: Michael Ellerman @ 2018-03-06 13:31 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Scott Wood, aneesh.kumar
Cc: linuxppc-dev, linux-kernel, Nicholas Piggin
In-Reply-To: <455276187b369bf019e91715b7488e86cd932d49.1519305410.git.christophe.leroy@c-s.fr>
On Thu, 2018-02-22 at 14:27:20 UTC, Christophe Leroy wrote:
> bitmap_or() and bitmap_andnot() can work properly with dst identical
> to src1 or src2. There is no need of an intermediate result bitmap
> that is copied back to dst in a second step.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/326691ad4f179e6edc7eb1271e618d
cheers
^ permalink raw reply
* Re: [FIX] powerpc/pseries: Fix vector5 in ibm architecture vector table
From: Michael Ellerman @ 2018-03-06 13:29 UTC (permalink / raw)
To: Bharata B Rao, linuxppc-dev; +Cc: nfont, mwb, Bharata B Rao
In-Reply-To: <20180306081432.3653-1-bharata@linux.vnet.ibm.com>
On Tue, 2018-03-06 at 08:14:32 UTC, Bharata B Rao wrote:
> With ibm,dynamic-memory-v2 and ibm,drc-info coming around the
> same time, byte22 in vector5 of ibm architecture vector table
> got set twice separately. The end result is that guest kernel
> isn't advertising support for ibm,dynamic-memory-v2.
>
> Fix this by removing the duplicate assignment of byte22.
>
> Fixes: 02ef6dd8109b5 (powerpc: Enable support for ibm,drc-info devtree property)
> Fixes: c7a3275e0f9e4 (powerpc/pseries: Revert support for ibm,drc-info devtree property)
>
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/b0c41b8b6e43120d7c35e4709508a3
cheers
^ permalink raw reply
* [PATCH 10/10] powerpc/mm/slice: remove radix calls to the slice code
From: Nicholas Piggin @ 2018-03-06 13:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K . V, Christophe Leroy
In-Reply-To: <20180306132507.10649-1-npiggin@gmail.com>
This is a tidy up which removes radix MMU calls into the slice
code.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/hugetlb.h | 9 ++++++---
arch/powerpc/mm/hugetlbpage.c | 5 +++--
arch/powerpc/mm/slice.c | 17 ++++-------------
3 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/hugetlb.h b/arch/powerpc/include/asm/hugetlb.h
index 1a4847f67ea8..59885d444695 100644
--- a/arch/powerpc/include/asm/hugetlb.h
+++ b/arch/powerpc/include/asm/hugetlb.h
@@ -90,16 +90,19 @@ pte_t *huge_pte_offset_and_shift(struct mm_struct *mm,
void flush_dcache_icache_hugepage(struct page *page);
#if defined(CONFIG_PPC_MM_SLICES)
-int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
+int slice_is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
unsigned long len);
-#else
+#endif
static inline int is_hugepage_only_range(struct mm_struct *mm,
unsigned long addr,
unsigned long len)
{
+#if defined(CONFIG_PPC_MM_SLICES)
+ if (!radix_enabled())
+ return slice_is_hugepage_only_range(mm, addr, len);
+#endif
return 0;
}
-#endif
void book3e_hugetlb_preload(struct vm_area_struct *vma, unsigned long ea,
pte_t pte);
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 590be3fa0ce2..b29d40889d1c 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -565,10 +565,11 @@ unsigned long hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
unsigned long vma_mmu_pagesize(struct vm_area_struct *vma)
{
#ifdef CONFIG_PPC_MM_SLICES
- unsigned int psize = get_slice_psize(vma->vm_mm, vma->vm_start);
/* With radix we don't use slice, so derive it from vma*/
- if (!radix_enabled())
+ if (!radix_enabled()) {
+ unsigned int psize = get_slice_psize(vma->vm_mm, vma->vm_start);
return 1UL << mmu_psize_to_shift(psize);
+ }
#endif
if (!is_vm_hugetlb_page(vma))
return PAGE_SIZE;
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index 507d17e2cfcd..15a857772617 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -697,16 +697,8 @@ unsigned int get_slice_psize(struct mm_struct *mm, unsigned long addr)
unsigned char *psizes;
int index, mask_index;
- /*
- * Radix doesn't use slice, but can get enabled along with MMU_SLICE
- */
- if (radix_enabled()) {
-#ifdef CONFIG_PPC_64K_PAGES
- return MMU_PAGE_64K;
-#else
- return MMU_PAGE_4K;
-#endif
- }
+ VM_BUG_ON(radix_enabled());
+
if (addr < SLICE_LOW_TOP) {
psizes = mm->context.low_slices_psize;
index = GET_LOW_SLICE_INDEX(addr);
@@ -788,14 +780,13 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
* for now as we only use slices with hugetlbfs enabled. This should
* be fixed as the generic code gets fixed.
*/
-int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
+int slice_is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
unsigned long len)
{
const struct slice_mask *maskp;
unsigned int psize = mm->context.user_psize;
- if (radix_enabled())
- return 0;
+ VM_BUG_ON(radix_enabled());
maskp = slice_mask_for_size(mm, psize);
#ifdef CONFIG_PPC_64K_PAGES
--
2.16.1
^ permalink raw reply related
* [PATCH 09/10] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations
From: Nicholas Piggin @ 2018-03-06 13:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K . V, Christophe Leroy
In-Reply-To: <20180306132507.10649-1-npiggin@gmail.com>
The number of high slices a process might use now depends on its
address space size, and what allocation address it has requested.
This patch uses that limit throughout call chains where possible,
rather than use the fixed SLICE_NUM_HIGH for bitmap operations.
This saves some cost for processes that don't use very large address
spaces.
Perormance numbers aren't changed significantly, this may change
with larger address spaces or different mmap access patterns that
require more slice mask building.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/mm/slice.c | 75 +++++++++++++++++++++++++++++--------------------
1 file changed, 45 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index 086c31b8b982..507d17e2cfcd 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -61,14 +61,12 @@ static void slice_print_mask(const char *label, const struct slice_mask *mask) {
#endif
static void slice_range_to_mask(unsigned long start, unsigned long len,
- struct slice_mask *ret)
+ struct slice_mask *ret,
+ unsigned long high_slices)
{
unsigned long end = start + len - 1;
ret->low_slices = 0;
- if (SLICE_NUM_HIGH)
- bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
-
if (start < SLICE_LOW_TOP) {
unsigned long mend = min(end,
(unsigned long)(SLICE_LOW_TOP - 1));
@@ -77,6 +75,10 @@ static void slice_range_to_mask(unsigned long start, unsigned long len,
- (1u << GET_LOW_SLICE_INDEX(start));
}
+ if (!SLICE_NUM_HIGH)
+ return;
+
+ bitmap_zero(ret->high_slices, high_slices);
if ((start + len) > SLICE_LOW_TOP) {
unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
@@ -120,22 +122,20 @@ static int slice_high_has_vma(struct mm_struct *mm, unsigned long slice)
}
static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret,
- unsigned long high_limit)
+ unsigned long high_slices)
{
unsigned long i;
ret->low_slices = 0;
- if (SLICE_NUM_HIGH)
- bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
-
for (i = 0; i < SLICE_NUM_LOW; i++)
if (!slice_low_has_vma(mm, i))
ret->low_slices |= 1u << i;
- if (high_limit <= SLICE_LOW_TOP)
+ if (!SLICE_NUM_HIGH || !high_slices)
return;
- for (i = 0; i < GET_HIGH_SLICE_INDEX(high_limit); i++)
+ bitmap_zero(ret->high_slices, high_slices);
+ for (i = 0; i < high_slices; i++)
if (!slice_high_has_vma(mm, i))
__set_bit(i, ret->high_slices);
}
@@ -232,6 +232,7 @@ static void slice_convert(struct mm_struct *mm,
{
int index, mask_index;
/* Write the new slice psize bits */
+ unsigned long high_slices;
unsigned char *hpsizes, *lpsizes;
struct slice_mask *psize_mask, *old_mask;
unsigned long i, flags;
@@ -267,7 +268,8 @@ static void slice_convert(struct mm_struct *mm,
}
hpsizes = mm->context.high_slices_psize;
- for (i = 0; i < GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit); i++) {
+ high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
+ for (i = 0; SLICE_NUM_HIGH && i < high_slices; i++) {
if (!test_bit(i, mask->high_slices))
continue;
@@ -434,32 +436,37 @@ static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
}
static inline void slice_copy_mask(struct slice_mask *dst,
- const struct slice_mask *src)
+ const struct slice_mask *src,
+ unsigned long high_slices)
{
dst->low_slices = src->low_slices;
if (!SLICE_NUM_HIGH)
return;
- bitmap_copy(dst->high_slices, src->high_slices, SLICE_NUM_HIGH);
+ bitmap_copy(dst->high_slices, src->high_slices, high_slices);
}
static inline void slice_or_mask(struct slice_mask *dst,
const struct slice_mask *src1,
- const struct slice_mask *src2)
+ const struct slice_mask *src2,
+ unsigned long high_slices)
{
dst->low_slices = src1->low_slices | src2->low_slices;
if (!SLICE_NUM_HIGH)
return;
- bitmap_or(dst->high_slices, src1->high_slices, src2->high_slices, SLICE_NUM_HIGH);
+ bitmap_or(dst->high_slices, src1->high_slices, src2->high_slices,
+ high_slices);
}
static inline void slice_andnot_mask(struct slice_mask *dst,
const struct slice_mask *src1,
- const struct slice_mask *src2)
+ const struct slice_mask *src2,
+ unsigned long high_slices)
{
dst->low_slices = src1->low_slices & ~src2->low_slices;
if (!SLICE_NUM_HIGH)
return;
- bitmap_andnot(dst->high_slices, src1->high_slices, src2->high_slices, SLICE_NUM_HIGH);
+ bitmap_andnot(dst->high_slices, src1->high_slices, src2->high_slices,
+ high_slices);
}
#ifdef CONFIG_PPC_64K_PAGES
@@ -482,6 +489,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
struct mm_struct *mm = current->mm;
unsigned long newaddr;
unsigned long high_limit;
+ unsigned long high_slices;
high_limit = DEFAULT_MAP_WINDOW;
if (addr >= high_limit || (fixed && (addr + len > high_limit)))
@@ -498,6 +506,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
return -ENOMEM;
}
+ high_slices = GET_HIGH_SLICE_INDEX(high_limit);
if (high_limit > mm->context.slb_addr_limit) {
/*
* Increasing the slb_addr_limit does not require
@@ -557,13 +566,13 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
if (psize == MMU_PAGE_64K) {
compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
if (fixed)
- slice_or_mask(&good_mask, maskp, compat_maskp);
+ slice_or_mask(&good_mask, maskp, compat_maskp, high_slices);
else
- slice_copy_mask(&good_mask, maskp);
+ slice_copy_mask(&good_mask, maskp, high_slices);
} else
#endif
{
- slice_copy_mask(&good_mask, maskp);
+ slice_copy_mask(&good_mask, maskp, high_slices);
}
slice_print_mask(" good_mask", &good_mask);
if (compat_maskp)
@@ -596,8 +605,8 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
* We don't fit in the good mask, check what other slices are
* empty and thus can be converted
*/
- slice_mask_for_free(mm, &potential_mask, high_limit);
- slice_or_mask(&potential_mask, &potential_mask, &good_mask);
+ slice_mask_for_free(mm, &potential_mask, high_slices);
+ slice_or_mask(&potential_mask, &potential_mask, &good_mask, high_slices);
slice_print_mask(" potential", &potential_mask);
if (addr || fixed) {
@@ -634,7 +643,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
#ifdef CONFIG_PPC_64K_PAGES
if (addr == -ENOMEM && psize == MMU_PAGE_64K) {
/* retry the search with 4k-page slices included */
- slice_or_mask(&potential_mask, &potential_mask, compat_maskp);
+ slice_or_mask(&potential_mask, &potential_mask, compat_maskp, high_slices);
addr = slice_find_area(mm, len, &potential_mask,
psize, topdown, high_limit);
}
@@ -643,17 +652,17 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
if (addr == -ENOMEM)
return -ENOMEM;
- slice_range_to_mask(addr, len, &potential_mask);
+ slice_range_to_mask(addr, len, &potential_mask, high_slices);
slice_dbg(" found potential area at 0x%lx\n", addr);
slice_print_mask(" mask", &potential_mask);
convert:
- slice_andnot_mask(&potential_mask, &potential_mask, &good_mask);
+ slice_andnot_mask(&potential_mask, &potential_mask, &good_mask, high_slices);
if (compat_maskp && !fixed)
- slice_andnot_mask(&potential_mask, &potential_mask, compat_maskp);
+ slice_andnot_mask(&potential_mask, &potential_mask, compat_maskp, high_slices);
if (potential_mask.low_slices ||
(SLICE_NUM_HIGH &&
- !bitmap_empty(potential_mask.high_slices, SLICE_NUM_HIGH))) {
+ !bitmap_empty(potential_mask.high_slices, high_slices))) {
slice_convert(mm, &potential_mask, psize);
if (psize > MMU_PAGE_BASE)
on_each_cpu(slice_flush_segments, mm, 1);
@@ -727,7 +736,9 @@ void slice_init_new_context_exec(struct mm_struct *mm)
mm->context.user_psize = psize;
/*
- * Set all slice psizes to the default.
+ * Set all slice psizes to the default. High slices could
+ * be initialised up to slb_addr_limit if we ensure to
+ * initialise the rest of them as slb_addr_limit is expanded.
*/
lpsizes = mm->context.low_slices_psize;
memset(lpsizes, (psize << 4) | psize, SLICE_NUM_LOW >> 1);
@@ -748,10 +759,12 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
unsigned long len, unsigned int psize)
{
struct slice_mask mask;
+ unsigned long high_slices;
VM_BUG_ON(radix_enabled());
- slice_range_to_mask(start, len, &mask);
+ high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
+ slice_range_to_mask(start, len, &mask, high_slices);
slice_convert(mm, &mask, psize);
}
@@ -790,9 +803,11 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
if (psize == MMU_PAGE_64K) {
const struct slice_mask *compat_maskp;
struct slice_mask available;
+ unsigned long high_slices;
compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
- slice_or_mask(&available, maskp, compat_maskp);
+ high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
+ slice_or_mask(&available, maskp, compat_maskp, high_slices);
return !slice_check_range_fits(mm, &available, addr, len);
}
#endif
--
2.16.1
^ permalink raw reply related
* [PATCH 08/10] powerpc/mm/slice: Use const pointers to cached slice masks where possible
From: Nicholas Piggin @ 2018-03-06 13:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K . V, Christophe Leroy
In-Reply-To: <20180306132507.10649-1-npiggin@gmail.com>
The slice_mask cache was a basic conversion which copied the slice
mask into caller's structures, because that's how the original code
worked. In most cases the pointer can be used directly instead, saving
a copy and an on-stack structure.
On POWER8, this increases vfork+exec+exit performance by 0.3%
and reduces time to mmap+munmap a 64kB page by 2%.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/mm/slice.c | 77 +++++++++++++++++++++----------------------------
1 file changed, 33 insertions(+), 44 deletions(-)
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index 46daa1d1794f..086c31b8b982 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -472,10 +472,10 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
unsigned long flags, unsigned int psize,
int topdown)
{
- struct slice_mask mask;
struct slice_mask good_mask;
struct slice_mask potential_mask;
- struct slice_mask compat_mask;
+ const struct slice_mask *maskp;
+ const struct slice_mask *compat_maskp = NULL;
int fixed = (flags & MAP_FIXED);
int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);
unsigned long page_size = 1UL << pshift;
@@ -509,22 +509,6 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
on_each_cpu(slice_flush_segments, mm, 1);
}
- /*
- * init different masks
- */
- mask.low_slices = 0;
-
- /* silence stupid warning */;
- potential_mask.low_slices = 0;
-
- compat_mask.low_slices = 0;
-
- if (SLICE_NUM_HIGH) {
- bitmap_zero(mask.high_slices, SLICE_NUM_HIGH);
- bitmap_zero(potential_mask.high_slices, SLICE_NUM_HIGH);
- bitmap_zero(compat_mask.high_slices, SLICE_NUM_HIGH);
- }
-
/* Sanity checks */
BUG_ON(mm->task_size == 0);
BUG_ON(mm->context.slb_addr_limit == 0);
@@ -547,8 +531,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
/* First make up a "good" mask of slices that have the right size
* already
*/
- good_mask = *slice_mask_for_size(mm, psize);
- slice_print_mask(" good_mask", &good_mask);
+ maskp = slice_mask_for_size(mm, psize);
/*
* Here "good" means slices that are already the right page size,
@@ -572,11 +555,19 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
#ifdef CONFIG_PPC_64K_PAGES
/* If we support combo pages, we can allow 64k pages in 4k slices */
if (psize == MMU_PAGE_64K) {
- compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
+ compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
if (fixed)
- slice_or_mask(&good_mask, &good_mask, &compat_mask);
- }
+ slice_or_mask(&good_mask, maskp, compat_maskp);
+ else
+ slice_copy_mask(&good_mask, maskp);
+ } else
#endif
+ {
+ slice_copy_mask(&good_mask, maskp);
+ }
+ slice_print_mask(" good_mask", &good_mask);
+ if (compat_maskp)
+ slice_print_mask(" compat_mask", compat_maskp);
/* First check hint if it's valid or if we have MAP_FIXED */
if (addr || fixed) {
@@ -643,7 +634,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
#ifdef CONFIG_PPC_64K_PAGES
if (addr == -ENOMEM && psize == MMU_PAGE_64K) {
/* retry the search with 4k-page slices included */
- slice_or_mask(&potential_mask, &potential_mask, &compat_mask);
+ slice_or_mask(&potential_mask, &potential_mask, compat_maskp);
addr = slice_find_area(mm, len, &potential_mask,
psize, topdown, high_limit);
}
@@ -652,17 +643,18 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
if (addr == -ENOMEM)
return -ENOMEM;
- slice_range_to_mask(addr, len, &mask);
+ slice_range_to_mask(addr, len, &potential_mask);
slice_dbg(" found potential area at 0x%lx\n", addr);
- slice_print_mask(" mask", &mask);
+ slice_print_mask(" mask", &potential_mask);
convert:
- slice_andnot_mask(&mask, &mask, &good_mask);
- slice_andnot_mask(&mask, &mask, &compat_mask);
- if (mask.low_slices ||
- (SLICE_NUM_HIGH &&
- !bitmap_empty(mask.high_slices, SLICE_NUM_HIGH))) {
- slice_convert(mm, &mask, psize);
+ slice_andnot_mask(&potential_mask, &potential_mask, &good_mask);
+ if (compat_maskp && !fixed)
+ slice_andnot_mask(&potential_mask, &potential_mask, compat_maskp);
+ if (potential_mask.low_slices ||
+ (SLICE_NUM_HIGH &&
+ !bitmap_empty(potential_mask.high_slices, SLICE_NUM_HIGH))) {
+ slice_convert(mm, &potential_mask, psize);
if (psize > MMU_PAGE_BASE)
on_each_cpu(slice_flush_segments, mm, 1);
}
@@ -786,28 +778,25 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
unsigned long len)
{
- struct slice_mask available;
+ const struct slice_mask *maskp;
unsigned int psize = mm->context.user_psize;
if (radix_enabled())
return 0;
- available = *slice_mask_for_size(mm, psize);
+ maskp = slice_mask_for_size(mm, psize);
#ifdef CONFIG_PPC_64K_PAGES
/* We need to account for 4k slices too */
if (psize == MMU_PAGE_64K) {
- struct slice_mask compat_mask;
- compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
- slice_or_mask(&available, &available, &compat_mask);
+ const struct slice_mask *compat_maskp;
+ struct slice_mask available;
+
+ compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
+ slice_or_mask(&available, maskp, compat_maskp);
+ return !slice_check_range_fits(mm, &available, addr, len);
}
#endif
-#if 0 /* too verbose */
- slice_dbg("is_hugepage_only_range(mm=%p, addr=%lx, len=%lx)\n",
- mm, addr, len);
- slice_print_mask(" mask", &mask);
- slice_print_mask(" available", &available);
-#endif
- return !slice_check_range_fits(mm, &available, addr, len);
+ return !slice_check_range_fits(mm, maskp, addr, len);
}
#endif
--
2.16.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