* Re: remove the last set_fs() in common code, and remove it for x86 and powerpc v2
From: Matthew Wilcox @ 2020-09-01 17:42 UTC (permalink / raw)
To: Al Viro
Cc: linux-arch, Kees Cook, x86, linuxppc-dev, linux-kernel,
linux-fsdevel, Linus Torvalds, Christoph Hellwig
In-Reply-To: <20200901172512.GI1236603@ZenIV.linux.org.uk>
On Tue, Sep 01, 2020 at 06:25:12PM +0100, Al Viro wrote:
> On Tue, Sep 01, 2020 at 07:13:00PM +0200, Christophe Leroy wrote:
>
> > 10.92% dd [kernel.kallsyms] [k] iov_iter_zero
>
> Interesting... Could you get an instruction-level profile inside iov_iter_zero(),
> along with the disassembly of that sucker?
Also, does [1] make any difference? Probably not since it's translating
O flags into IOCB flags instead of RWF flags into IOCB flags. I wonder
if there's a useful trick we can play here ... something like:
static inline int iocb_flags(struct file *file)
{
int res = 0;
if (likely(!file->f_flags & O_APPEND | O_DIRECT | O_DSYNC | __O_SYNC)) && !IS_SYNC(file->f_mapping->host))
return res;
if (file->f_flags & O_APPEND)
res |= IOCB_APPEND;
if (file->f_flags & O_DIRECT)
res |= IOCB_DIRECT;
if ((file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host))
res |= IOCB_DSYNC;
if (file->f_flags & __O_SYNC)
res |= IOCB_SYNC;
return res;
}
Can we do something like force O_DSYNC to be set if the inode IS_SYNC()
at the time of open? Or is setting the sync bit on the inode required
to affect currently-open files?
[1] https://lore.kernel.org/linux-fsdevel/95de7ce4-9254-39f1-304f-4455f66bf0f4@kernel.dk/
^ permalink raw reply
* Re: remove the last set_fs() in common code, and remove it for x86 and powerpc v2
From: Al Viro @ 2020-09-01 17:25 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, Kees Cook, x86, linuxppc-dev, linux-kernel,
linux-fsdevel, Linus Torvalds, Christoph Hellwig
In-Reply-To: <a8bb0319-0928-4687-9e9c-777c5860dbdd@csgroup.eu>
On Tue, Sep 01, 2020 at 07:13:00PM +0200, Christophe Leroy wrote:
> 10.92% dd [kernel.kallsyms] [k] iov_iter_zero
Interesting... Could you get an instruction-level profile inside iov_iter_zero(),
along with the disassembly of that sucker?
^ permalink raw reply
* Re: remove the last set_fs() in common code, and remove it for x86 and powerpc v2
From: Christophe Leroy @ 2020-09-01 17:13 UTC (permalink / raw)
To: Christoph Hellwig, Linus Torvalds, Al Viro, Michael Ellerman, x86
Cc: linux-fsdevel, linux-arch, linuxppc-dev, Kees Cook, linux-kernel
In-Reply-To: <20200827150030.282762-1-hch@lst.de>
Hi Christoph,
Le 27/08/2020 à 17:00, Christoph Hellwig a écrit :
> Hi all,
>
> this series removes the last set_fs() used to force a kernel address
> space for the uaccess code in the kernel read/write/splice code, and then
> stops implementing the address space overrides entirely for x86 and
> powerpc.
>
> The file system part has been posted a few times, and the read/write side
> has been pretty much unchanced. For splice this series drops the
> conversion of the seq_file and sysctl code to the iter ops, and thus loses
> the splice support for them. The reasons for that is that it caused a lot
> of churn for not much use - splice for these small files really isn't much
> of a win, even if existing userspace uses it. All callers I found do the
> proper fallback, but if this turns out to be an issue the conversion can
> be resurrected.
>
> Besides x86 and powerpc I plan to eventually convert all other
> architectures, although this will be a slow process, starting with the
> easier ones once the infrastructure is merged. The process to convert
> architectures is roughtly:
>
> (1) ensure there is no set_fs(KERNEL_DS) left in arch specific code
> (2) implement __get_kernel_nofault and __put_kernel_nofault
> (3) remove the arch specific address limitation functionality
>
> Changes since v1:
> - drop the patch to remove the non-iter ops for /dev/zero and
> /dev/null as they caused a performance regression
> - don't enable user access in __get_kernel on powerpc
> - xfail the set_fs() based lkdtm tests
>
> Diffstat:
>
I'm still sceptic with the results I get.
With 5.9-rc2:
root@vgoippro:~# time dd if=/dev/zero of=/dev/null count=1M
1048576+0 records in
1048576+0 records out
536870912 bytes (512.0MB) copied, 5.585880 seconds, 91.7MB/s
real 0m 5.59s
user 0m 1.40s
sys 0m 4.19s
With your series:
root@vgoippro:/tmp# time dd if=/dev/zero of=/dev/null count=1M
1048576+0 records in
1048576+0 records out
536870912 bytes (512.0MB) copied, 7.780540 seconds, 65.8MB/s
real 0m 7.79s
user 0m 2.12s
sys 0m 5.66s
Top of perf report of a standard perf record:
With 5.9-rc2:
20.31% dd [kernel.kallsyms] [k] __arch_clear_user
8.37% dd [kernel.kallsyms] [k] transfer_to_syscall
7.37% dd [kernel.kallsyms] [k] __fsnotify_parent
6.95% dd [kernel.kallsyms] [k] iov_iter_zero
5.72% dd [kernel.kallsyms] [k] new_sync_read
4.87% dd [kernel.kallsyms] [k] vfs_write
4.47% dd [kernel.kallsyms] [k] vfs_read
3.07% dd [kernel.kallsyms] [k] ksys_write
2.77% dd [kernel.kallsyms] [k] ksys_read
2.65% dd [kernel.kallsyms] [k] __fget_light
2.37% dd [kernel.kallsyms] [k] __fdget_pos
2.35% dd [kernel.kallsyms] [k] memset
1.53% dd [kernel.kallsyms] [k] rw_verify_area
1.52% dd [kernel.kallsyms] [k] read_iter_zero
With your series:
19.60% dd [kernel.kallsyms] [k] __arch_clear_user
10.92% dd [kernel.kallsyms] [k] iov_iter_zero
9.50% dd [kernel.kallsyms] [k] vfs_write
8.97% dd [kernel.kallsyms] [k] __fsnotify_parent
5.46% dd [kernel.kallsyms] [k] transfer_to_syscall
5.42% dd [kernel.kallsyms] [k] vfs_read
3.58% dd [kernel.kallsyms] [k] ksys_read
2.84% dd [kernel.kallsyms] [k] read_iter_zero
2.24% dd [kernel.kallsyms] [k] ksys_write
1.80% dd [kernel.kallsyms] [k] __fget_light
1.34% dd [kernel.kallsyms] [k] __fdget_pos
0.91% dd [kernel.kallsyms] [k] memset
0.91% dd [kernel.kallsyms] [k] rw_verify_area
Christophe
^ permalink raw reply
* Re: [PATCH] selftests/powerpc: Skip PROT_SAO test in guests/LPARS
From: Sachin Sant @ 2020-09-01 15:33 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20200901124653.523182-1-mpe@ellerman.id.au>
> On 01-Sep-2020, at 6:16 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> In commit 9b725a90a8f1 ("powerpc/64s: Disallow PROT_SAO in LPARs by
> default") PROT_SAO was disabled in guests/LPARs by default. So skip
> the test if we are running in a guest to avoid a spurious failure.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> —
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
With the fix test is skipped while running in a guest
# ./prot_sao
test: prot-sao
tags: git_version:unknown
[SKIP] Test skipped on line 25
skip: prot-sao
#
^ permalink raw reply
* [PATCH v3 16/23] powerpc: use asm-generic/mmu_context.h for no-op implementations
From: Nicholas Piggin @ 2020-09-01 14:15 UTC (permalink / raw)
To: linux-arch
Cc: Arnd Bergmann, linux-kernel, Nicholas Piggin, linux-mm,
linuxppc-dev
In-Reply-To: <20200901141539.1757549-1-npiggin@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/mmu_context.h | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 7f3658a97384..a3a12a8341b2 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -14,7 +14,9 @@
/*
* Most if the context management is out of line
*/
+#define init_new_context init_new_context
extern int init_new_context(struct task_struct *tsk, struct mm_struct *mm);
+#define destroy_context destroy_context
extern void destroy_context(struct mm_struct *mm);
#ifdef CONFIG_SPAPR_TCE_IOMMU
struct mm_iommu_table_group_mem_t;
@@ -235,27 +237,26 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
}
#define switch_mm_irqs_off switch_mm_irqs_off
-
-#define deactivate_mm(tsk,mm) do { } while (0)
-
/*
* After we have set current->mm to a new value, this activates
* the context for the new mm so we see the new mappings.
*/
+#define activate_mm activate_mm
static inline void activate_mm(struct mm_struct *prev, struct mm_struct *next)
{
switch_mm(prev, next, current);
}
/* We don't currently use enter_lazy_tlb() for anything */
+#ifdef CONFIG_PPC_BOOK3E_64
+#define enter_lazy_tlb enter_lazy_tlb
static inline void enter_lazy_tlb(struct mm_struct *mm,
struct task_struct *tsk)
{
/* 64-bit Book3E keeps track of current PGD in the PACA */
-#ifdef CONFIG_PPC_BOOK3E_64
get_paca()->pgd = NULL;
-#endif
}
+#endif
extern void arch_exit_mmap(struct mm_struct *mm);
@@ -298,5 +299,7 @@ static inline int arch_dup_mmap(struct mm_struct *oldmm,
return 0;
}
+#include <asm-generic/mmu_context.h>
+
#endif /* __KERNEL__ */
#endif /* __ASM_POWERPC_MMU_CONTEXT_H */
--
2.23.0
^ permalink raw reply related
* [PATCH] cpuidle-pseries: Fix CEDE latency conversion from tb to us
From: Gautham R. Shenoy @ 2020-09-01 14:08 UTC (permalink / raw)
To: Michael Ellerman, Rafael J. Wysocki, Vaidyanathan Srinivasan
Cc: Gautham R. Shenoy, linuxppc-dev, linux-kernel, linux-pm
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
CEDE(0)") sets the exit latency of CEDE(0) based on the latency values
of the Extended CEDE states advertised by the platform. The values
advertised by the platform are in timebase ticks. However the cpuidle
framework requires the latency values in microseconds.
If the tb-ticks value advertised by the platform correspond to a value
smaller than 1us, during the conversion from tb-ticks to microseconds,
in the current code, the result becomes zero. This is incorrect as it
puts a CEDE state on par with the snooze state.
This patch fixes this by rounding up the result obtained while
converting the latency value from tb-ticks to microseconds.
Fixes: commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
CEDE(0)")
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
drivers/cpuidle/cpuidle-pseries.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index ff6d99e..9043358 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -361,7 +361,7 @@ static void __init fixup_cede0_latency(void)
for (i = 0; i < nr_xcede_records; i++) {
struct xcede_latency_record *record = &payload->records[i];
u64 latency_tb = be64_to_cpu(record->latency_ticks);
- u64 latency_us = tb_to_ns(latency_tb) / NSEC_PER_USEC;
+ u64 latency_us = DIV_ROUND_UP_ULL(tb_to_ns(latency_tb), NSEC_PER_USEC);
if (latency_us < min_latency_us)
min_latency_us = latency_us;
--
1.9.4
^ permalink raw reply related
* Re: [PATCH] powerpc/powernv/pci: Drop pnv_phb->initialized
From: Michael Ellerman @ 2020-09-01 13:52 UTC (permalink / raw)
To: Oliver O'Halloran, linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20200831061500.1646445-1-oohall@gmail.com>
Oliver O'Halloran <oohall@gmail.com> writes:
> The pnv_phb->initialized flag is an odd beast. It was added back in 2012 in
> commit db1266c85261 ("powerpc/powernv: Skip check on PE if necessary") to
> allow devices to be enabled even if their PE assignments hadn't been
> completed yet. I can't think of any situation where we would (or should)
> have PCI devices being enabled before their PEs are assigned, so I can only
> assume it was a workaround for a bug or some other undesirable behaviour
> from the PCI core.
>
> Since commit dc3d8f85bb57 ("powerpc/powernv/pci: Re-work bus PE
> configuration") the PE setup occurs before the PCI core allows driver to
> attach to the device so the problem should no longer exist. Even it does
> allowing the device to be enabled before we have assigned the device to a
> PE is almost certainly broken and will cause spurious EEH events so we
> should probably just remove it.
>
> It's also worth pointing out that ->initialized flag is set in
> pnv_pci_ioda_create_dbgfs() which has the entire function body wrapped in
> flag.
"body wrapped in flag." ?
I guess you meant:
"wrapped in #ifdef CONFIG_DEBUG_FS" ?
> That has the fun side effect of bypassing any other checks in
> pnv_pci_enable_device_hook() which is probably not what anybody wants.
That would only be true for CONFIG_DEBUG_FS=n builds though.
cheers
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 023a4f987bb2..6ac3c637b313 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -2410,9 +2410,6 @@ static void pnv_pci_ioda_create_dbgfs(void)
> list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
> phb = hose->private_data;
>
> - /* Notify initialization of PHB done */
> - phb->initialized = 1;
> -
> sprintf(name, "PCI%04x", hose->global_number);
> phb->dbgfs = debugfs_create_dir(name, powerpc_debugfs_root);
>
> @@ -2609,17 +2606,8 @@ static resource_size_t pnv_pci_default_alignment(void)
> */
> static bool pnv_pci_enable_device_hook(struct pci_dev *dev)
> {
> - struct pnv_phb *phb = pci_bus_to_pnvhb(dev->bus);
> struct pci_dn *pdn;
>
> - /* The function is probably called while the PEs have
> - * not be created yet. For example, resource reassignment
> - * during PCI probe period. We just skip the check if
> - * PEs isn't ready.
> - */
> - if (!phb->initialized)
> - return true;
> -
> pdn = pci_get_pdn(dev);
> if (!pdn || pdn->pe_number == IODA_INVALID_PE)
> return false;
> @@ -2629,14 +2617,9 @@ static bool pnv_pci_enable_device_hook(struct pci_dev *dev)
>
> static bool pnv_ocapi_enable_device_hook(struct pci_dev *dev)
> {
> - struct pci_controller *hose = pci_bus_to_host(dev->bus);
> - struct pnv_phb *phb = hose->private_data;
> struct pci_dn *pdn;
> struct pnv_ioda_pe *pe;
>
> - if (!phb->initialized)
> - return true;
> -
> pdn = pci_get_pdn(dev);
> if (!pdn)
> return false;
> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
> index 739a0b3b72e1..36d22920f5a3 100644
> --- a/arch/powerpc/platforms/powernv/pci.h
> +++ b/arch/powerpc/platforms/powernv/pci.h
> @@ -119,7 +119,6 @@ struct pnv_phb {
> int flags;
> void __iomem *regs;
> u64 regs_phys;
> - int initialized;
> spinlock_t lock;
>
> #ifdef CONFIG_DEBUG_FS
> --
> 2.26.2
^ permalink raw reply
* Re: [RESEND][PATCH 1/7] powerpc/iommu: Avoid overflow at boundary_size
From: Michael Ellerman @ 2020-09-01 13:27 UTC (permalink / raw)
To: Nicolin Chen, benh, paulus, rth, ink, mattst88, tony.luck,
fenghua.yu, schnelle, gerald.schaefer, hca, gor, borntraeger,
davem, tglx, mingo, bp, x86, hpa, James.Bottomley, deller
Cc: sfr, linux-ia64, linux-parisc, linux-s390, linux-kernel,
linux-alpha, sparclinux, linuxppc-dev, hch
In-Reply-To: <20200831203811.8494-2-nicoleotsuka@gmail.com>
Nicolin Chen <nicoleotsuka@gmail.com> writes:
> The boundary_size might be as large as ULONG_MAX, which means
> that a device has no specific boundary limit. So either "+ 1"
> or passing it to ALIGN() would potentially overflow.
>
> According to kernel defines:
> #define ALIGN_MASK(x, mask) (((x) + (mask)) & ~(mask))
> #define ALIGN(x, a) ALIGN_MASK(x, (typeof(x))(a) - 1)
>
> We can simplify the logic here:
> ALIGN(boundary + 1, 1 << shift) >> shift
> = ALIGN_MASK(b + 1, (1 << s) - 1) >> s
> = {[b + 1 + (1 << s) - 1] & ~[(1 << s) - 1]} >> s
> = [b + 1 + (1 << s) - 1] >> s
> = [b + (1 << s)] >> s
> = (b >> s) + 1
>
> So fixing a potential overflow with the safer shortcut.
>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
> Cc: Christoph Hellwig <hch@lst.de>
> ---
> arch/powerpc/kernel/iommu.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
Are you asking for acks, or for maintainers to merge the patches
individually?
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index 9704f3f76e63..c01ccbf8afdd 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -236,15 +236,14 @@ static unsigned long iommu_range_alloc(struct device *dev,
> }
> }
>
> - if (dev)
> - boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
> - 1 << tbl->it_page_shift);
> - else
> - boundary_size = ALIGN(1UL << 32, 1 << tbl->it_page_shift);
> /* 4GB boundary for iseries_hv_alloc and iseries_hv_map */
> + boundary_size = dev ? dma_get_seg_boundary(dev) : U32_MAX;
Is there any path that passes a NULL dev anymore?
Both iseries_hv_alloc() and iseries_hv_map() were removed years ago.
See:
8ee3e0d69623 ("powerpc: Remove the main legacy iSerie platform code")
So maybe we should do a lead-up patch that drops the NULL dev support,
which will then make this patch simpler.
cheers
> + /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
> + boundary_size = (boundary_size >> tbl->it_page_shift) + 1;
>
> n = iommu_area_alloc(tbl->it_map, limit, start, npages, tbl->it_offset,
> - boundary_size >> tbl->it_page_shift, align_mask);
> + boundary_size, align_mask);
> if (n == -1) {
> if (likely(pass == 0)) {
> /* First try the pool from the start */
> --
> 2.17.1
^ permalink raw reply
* [PATCH] selftests/powerpc: Skip PROT_SAO test in guests/LPARS
From: Michael Ellerman @ 2020-09-01 12:46 UTC (permalink / raw)
To: linuxppc-dev
In commit 9b725a90a8f1 ("powerpc/64s: Disallow PROT_SAO in LPARs by
default") PROT_SAO was disabled in guests/LPARs by default. So skip
the test if we are running in a guest to avoid a spurious failure.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
tools/testing/selftests/powerpc/mm/prot_sao.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/powerpc/mm/prot_sao.c b/tools/testing/selftests/powerpc/mm/prot_sao.c
index e0cf8ebbf8cd..30b71b1d78d5 100644
--- a/tools/testing/selftests/powerpc/mm/prot_sao.c
+++ b/tools/testing/selftests/powerpc/mm/prot_sao.c
@@ -7,6 +7,7 @@
#include <stdlib.h>
#include <string.h>
#include <sys/mman.h>
+#include <unistd.h>
#include <asm/cputable.h>
@@ -18,9 +19,13 @@ int test_prot_sao(void)
{
char *p;
- /* SAO was introduced in 2.06 and removed in 3.1 */
+ /*
+ * SAO was introduced in 2.06 and removed in 3.1. It's disabled in
+ * guests/LPARs by default, so also skip if we are running in a guest.
+ */
SKIP_IF(!have_hwcap(PPC_FEATURE_ARCH_2_06) ||
- have_hwcap2(PPC_FEATURE2_ARCH_3_1));
+ have_hwcap2(PPC_FEATURE2_ARCH_3_1) ||
+ access("/proc/device-tree/rtas/ibm,hypertas-functions", F_OK) == 0);
/*
* Ensure we can ask for PROT_SAO.
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 4/4] powerpc/64s/radix: Fix mm_cpumask trimming race vs kthread_use_mm
From: Michael Ellerman @ 2020-09-01 12:00 UTC (permalink / raw)
To: Nicholas Piggin, linux-mm
Cc: linux-arch, Jens Axboe, Peter Zijlstra, Aneesh Kumar K.V,
linux-kernel, Nicholas Piggin, Andrew Morton, linuxppc-dev,
David S. Miller
In-Reply-To: <20200828100022.1099682-5-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> Commit 0cef77c7798a7 ("powerpc/64s/radix: flush remote CPUs out of
> single-threaded mm_cpumask") added a mechanism to trim the mm_cpumask of
> a process under certain conditions. One of the assumptions is that
> mm_users would not be incremented via a reference outside the process
> context with mmget_not_zero() then go on to kthread_use_mm() via that
> reference.
>
> That invariant was broken by io_uring code (see previous sparc64 fix),
> but I'll point Fixes: to the original powerpc commit because we are
> changing that assumption going forward, so this will make backports
> match up.
>
> Fix this by no longer relying on that assumption, but by having each CPU
> check the mm is not being used, and clearing their own bit from the mask
> if it's okay. This fix relies on commit 38cf307c1f20 ("mm: fix
> kthread_use_mm() vs TLB invalidate") to disable irqs over the mm switch,
> and ARCH_WANT_IRQS_OFF_ACTIVATE_MM to be enabled.
You could use:
Depends-on: 38cf307c1f20 ("mm: fix kthread_use_mm() vs TLB invalidate")
> Fixes: 0cef77c7798a7 ("powerpc/64s/radix: flush remote CPUs out of single-threaded mm_cpumask")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/tlb.h | 13 -------------
> arch/powerpc/mm/book3s64/radix_tlb.c | 23 ++++++++++++++++-------
> 2 files changed, 16 insertions(+), 20 deletions(-)
One minor nit below if you're respinning anyway.
You know this stuff better than me, but I still reviewed it and it seems
good to me.
Reviewed-by: Michael Ellerman <mpe@ellerman.id.au>
> diff --git a/arch/powerpc/include/asm/tlb.h b/arch/powerpc/include/asm/tlb.h
> index fbc6f3002f23..d97f061fecac 100644
> --- a/arch/powerpc/include/asm/tlb.h
> +++ b/arch/powerpc/include/asm/tlb.h
> @@ -66,19 +66,6 @@ static inline int mm_is_thread_local(struct mm_struct *mm)
> return false;
> return cpumask_test_cpu(smp_processor_id(), mm_cpumask(mm));
> }
> -static inline void mm_reset_thread_local(struct mm_struct *mm)
> -{
> - WARN_ON(atomic_read(&mm->context.copros) > 0);
> - /*
> - * It's possible for mm_access to take a reference on mm_users to
> - * access the remote mm from another thread, but it's not allowed
> - * to set mm_cpumask, so mm_users may be > 1 here.
> - */
> - WARN_ON(current->mm != mm);
> - atomic_set(&mm->context.active_cpus, 1);
> - cpumask_clear(mm_cpumask(mm));
> - cpumask_set_cpu(smp_processor_id(), mm_cpumask(mm));
> -}
> #else /* CONFIG_PPC_BOOK3S_64 */
> static inline int mm_is_thread_local(struct mm_struct *mm)
> {
> diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
> index 0d233763441f..a421a0e3f930 100644
> --- a/arch/powerpc/mm/book3s64/radix_tlb.c
> +++ b/arch/powerpc/mm/book3s64/radix_tlb.c
> @@ -645,19 +645,29 @@ static void do_exit_flush_lazy_tlb(void *arg)
> struct mm_struct *mm = arg;
> unsigned long pid = mm->context.id;
>
> + /*
> + * A kthread could have done a mmget_not_zero() after the flushing CPU
> + * checked mm_users == 1, and be in the process of kthread_use_mm when
^
in mm_is_singlethreaded()
Adding that reference would help join the dots for a new reader I think.
cheers
> + * interrupted here. In that case, current->mm will be set to mm,
> + * because kthread_use_mm() setting ->mm and switching to the mm is
> + * done with interrupts off.
> + */
> if (current->mm == mm)
> - return; /* Local CPU */
> + goto out_flush;
>
> if (current->active_mm == mm) {
> - /*
> - * Must be a kernel thread because sender is single-threaded.
> - */
> - BUG_ON(current->mm);
> + WARN_ON_ONCE(current->mm != NULL);
> + /* Is a kernel thread and is using mm as the lazy tlb */
> mmgrab(&init_mm);
> - switch_mm(mm, &init_mm, current);
> current->active_mm = &init_mm;
> + switch_mm_irqs_off(mm, &init_mm, current);
> mmdrop(mm);
> }
> +
> + atomic_dec(&mm->context.active_cpus);
> + cpumask_clear_cpu(smp_processor_id(), mm_cpumask(mm));
> +
> +out_flush:
> _tlbiel_pid(pid, RIC_FLUSH_ALL);
> }
>
> @@ -672,7 +682,6 @@ static void exit_flush_lazy_tlbs(struct mm_struct *mm)
> */
> smp_call_function_many(mm_cpumask(mm), do_exit_flush_lazy_tlb,
> (void *)mm, 1);
> - mm_reset_thread_local(mm);
> }
>
> void radix__flush_tlb_mm(struct mm_struct *mm)
^ permalink raw reply
* [PATCH] ASoC: fsl_sai: Support multiple data channel enable bits
From: Shengjiu Wang @ 2020-09-01 11:01 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel, lgirdwood
Cc: linuxppc-dev, linux-kernel
One data channel is one data line. From imx7ulp, the SAI IP is
enhanced to support multiple data channels.
If there is only two channels input and slots is 2, then enable one
data channel is enough for data transfer. So enable the TCE/RCE and
transmit/receive mask register according to the input channels and
slots configuration.
Move the data channel enablement from startup() to hw_params().
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl_sai.c | 30 ++++++++++++------------------
sound/soc/fsl/fsl_sai.h | 2 +-
2 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 62c5fdb678fc..38c7bcbb361d 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -443,6 +443,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
u32 slots = (channels == 1) ? 2 : channels;
u32 slot_width = word_width;
int adir = tx ? RX : TX;
+ u32 pins;
int ret;
if (sai->slots)
@@ -451,6 +452,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
if (sai->slot_width)
slot_width = sai->slot_width;
+ pins = DIV_ROUND_UP(channels, slots);
+
if (!sai->is_slave_mode) {
if (sai->bclk_ratio)
ret = fsl_sai_set_bclk(cpu_dai, tx,
@@ -501,13 +504,17 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
FSL_SAI_CR5_FBT_MASK, val_cr5);
}
+ regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
+ FSL_SAI_CR3_TRCE_MASK,
+ FSL_SAI_CR3_TRCE((1 << pins) - 1));
regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs),
FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
val_cr4);
regmap_update_bits(sai->regmap, FSL_SAI_xCR5(tx, ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
FSL_SAI_CR5_FBT_MASK, val_cr5);
- regmap_write(sai->regmap, FSL_SAI_xMR(tx), ~0UL - ((1 << channels) - 1));
+ regmap_write(sai->regmap, FSL_SAI_xMR(tx),
+ ~0UL - ((1 << min(channels, slots)) - 1));
return 0;
}
@@ -517,6 +524,10 @@ static int fsl_sai_hw_free(struct snd_pcm_substream *substream,
{
struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
+ unsigned int ofs = sai->soc_data->reg_offset;
+
+ regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
+ FSL_SAI_CR3_TRCE_MASK, 0);
if (!sai->is_slave_mode &&
sai->mclk_streams & BIT(substream->stream)) {
@@ -651,14 +662,9 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,
struct snd_soc_dai *cpu_dai)
{
struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
- unsigned int ofs = sai->soc_data->reg_offset;
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
int ret;
- regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
- FSL_SAI_CR3_TRCE_MASK,
- FSL_SAI_CR3_TRCE);
-
/*
* EDMA controller needs period size to be a multiple of
* tx/rx maxburst
@@ -675,17 +681,6 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,
return ret;
}
-static void fsl_sai_shutdown(struct snd_pcm_substream *substream,
- struct snd_soc_dai *cpu_dai)
-{
- struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
- unsigned int ofs = sai->soc_data->reg_offset;
- bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
-
- regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
- FSL_SAI_CR3_TRCE_MASK, 0);
-}
-
static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
.set_bclk_ratio = fsl_sai_set_dai_bclk_ratio,
.set_sysclk = fsl_sai_set_dai_sysclk,
@@ -695,7 +690,6 @@ static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
.hw_free = fsl_sai_hw_free,
.trigger = fsl_sai_trigger,
.startup = fsl_sai_startup,
- .shutdown = fsl_sai_shutdown,
};
static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai)
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 6aba7d28f5f3..5f630be74853 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -109,7 +109,7 @@
#define FSL_SAI_CR2_DIV_MASK 0xff
/* SAI Transmit and Receive Configuration 3 Register */
-#define FSL_SAI_CR3_TRCE BIT(16)
+#define FSL_SAI_CR3_TRCE(x) ((x) << 16)
#define FSL_SAI_CR3_TRCE_MASK GENMASK(23, 16)
#define FSL_SAI_CR3_WDFL(x) (x)
#define FSL_SAI_CR3_WDFL_MASK 0x1f
--
2.27.0
^ permalink raw reply related
* Re: [PATCH] arch: vdso: add vdso linker script to 'targets' instead of extra-y
From: Greentime Hu @ 2020-09-01 11:29 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-s390, Catalin Marinas, Vasily Gorbik, Nick Hu,
Linux Kbuild mailing list, Heiko Carstens, linuxppc-dev,
Linux Kernel Mailing List, Christian Borntraeger, Paul Mackerras,
Vincent Chen, Will Deacon, linux-arm-kernel
In-Reply-To: <20200831182239.480317-1-masahiroy@kernel.org>
Masahiro Yamada <masahiroy@kernel.org> 於 2020年9月1日 週二 上午2:23寫道：
>
> The vdso linker script is preprocessed on demand.
> Adding it to 'targets' is enough to include the .cmd file.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> arch/arm64/kernel/vdso/Makefile | 2 +-
> arch/arm64/kernel/vdso32/Makefile | 2 +-
> arch/nds32/kernel/vdso/Makefile | 2 +-
> arch/powerpc/kernel/vdso32/Makefile | 2 +-
> arch/powerpc/kernel/vdso64/Makefile | 2 +-
> arch/s390/kernel/vdso64/Makefile | 2 +-
> 6 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> index 45d5cfe46429..7cd8aafbe96e 100644
> --- a/arch/arm64/kernel/vdso/Makefile
> +++ b/arch/arm64/kernel/vdso/Makefile
> @@ -54,7 +54,7 @@ endif
> GCOV_PROFILE := n
>
> obj-y += vdso.o
> -extra-y += vdso.lds
> +targets += vdso.lds
> CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
>
> # Force dependency (incbin is bad)
> diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
> index d6adb4677c25..572475b7b7ed 100644
> --- a/arch/arm64/kernel/vdso32/Makefile
> +++ b/arch/arm64/kernel/vdso32/Makefile
> @@ -155,7 +155,7 @@ asm-obj-vdso := $(addprefix $(obj)/, $(asm-obj-vdso))
> obj-vdso := $(c-obj-vdso) $(c-obj-vdso-gettimeofday) $(asm-obj-vdso)
>
> obj-y += vdso.o
> -extra-y += vdso.lds
> +targets += vdso.lds
> CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
>
> # Force dependency (vdso.s includes vdso.so through incbin)
> diff --git a/arch/nds32/kernel/vdso/Makefile b/arch/nds32/kernel/vdso/Makefile
> index 7c3c1ccb196e..55df25ef0057 100644
> --- a/arch/nds32/kernel/vdso/Makefile
> +++ b/arch/nds32/kernel/vdso/Makefile
> @@ -20,7 +20,7 @@ GCOV_PROFILE := n
>
>
> obj-y += vdso.o
> -extra-y += vdso.lds
> +targets += vdso.lds
> CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
>
> # Force dependency
> diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile
> index 87ab1152d5ce..fd5072a4c73c 100644
> --- a/arch/powerpc/kernel/vdso32/Makefile
> +++ b/arch/powerpc/kernel/vdso32/Makefile
> @@ -29,7 +29,7 @@ ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
> asflags-y := -D__VDSO32__ -s
>
> obj-y += vdso32_wrapper.o
> -extra-y += vdso32.lds
> +targets += vdso32.lds
> CPPFLAGS_vdso32.lds += -P -C -Upowerpc
>
> # Force dependency (incbin is bad)
> diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
> index 38c317f25141..c737b3ea3207 100644
> --- a/arch/powerpc/kernel/vdso64/Makefile
> +++ b/arch/powerpc/kernel/vdso64/Makefile
> @@ -17,7 +17,7 @@ ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
> asflags-y := -D__VDSO64__ -s
>
> obj-y += vdso64_wrapper.o
> -extra-y += vdso64.lds
> +targets += vdso64.lds
> CPPFLAGS_vdso64.lds += -P -C -U$(ARCH)
>
> # Force dependency (incbin is bad)
> diff --git a/arch/s390/kernel/vdso64/Makefile b/arch/s390/kernel/vdso64/Makefile
> index 4a66a1cb919b..d0d406cfffa9 100644
> --- a/arch/s390/kernel/vdso64/Makefile
> +++ b/arch/s390/kernel/vdso64/Makefile
> @@ -25,7 +25,7 @@ $(targets:%=$(obj)/%.dbg): KBUILD_CFLAGS = $(KBUILD_CFLAGS_64)
> $(targets:%=$(obj)/%.dbg): KBUILD_AFLAGS = $(KBUILD_AFLAGS_64)
>
> obj-y += vdso64_wrapper.o
> -extra-y += vdso64.lds
> +targets += vdso64.lds
> CPPFLAGS_vdso64.lds += -P -C -U$(ARCH)
>
> # Disable gcov profiling, ubsan and kasan for VDSO code
For nds32:
Acked-by: Greentime Hu <green.hu@gmail.com>
^ permalink raw reply
* Re: [PATCH v2] powerpc/mm: Remove DEBUG_VM_PGTABLE support on powerpc
From: Anshuman Khandual @ 2020-09-01 10:48 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe
In-Reply-To: <20200901094423.100149-1-aneesh.kumar@linux.ibm.com>
On 09/01/2020 03:14 PM, Aneesh Kumar K.V wrote:
> The test is broken w.r.t page table update rules and results in kernel
> crash as below. Disable the support until we get the tests updated.
>
> [ 21.083519] kernel BUG at arch/powerpc/mm/pgtable.c:304!
> cpu 0x0: Vector: 700 (Program Check) at [c000000c6d1e76c0]
> pc: c00000000009a5ec: assert_pte_locked+0x14c/0x380
> lr: c0000000005eeeec: pte_update+0x11c/0x190
> sp: c000000c6d1e7950
> msr: 8000000002029033
> current = 0xc000000c6d172c80
> paca = 0xc000000003ba0000 irqmask: 0x03 irq_happened: 0x01
> pid = 1, comm = swapper/0
> kernel BUG at arch/powerpc/mm/pgtable.c:304!
> [link register ] c0000000005eeeec pte_update+0x11c/0x190
> [c000000c6d1e7950] 0000000000000001 (unreliable)
> [c000000c6d1e79b0] c0000000005eee14 pte_update+0x44/0x190
> [c000000c6d1e7a10] c000000001a2ca9c pte_advanced_tests+0x160/0x3d8
> [c000000c6d1e7ab0] c000000001a2d4fc debug_vm_pgtable+0x7e8/0x1338
> [c000000c6d1e7ba0] c0000000000116ec do_one_initcall+0xac/0x5f0
> [c000000c6d1e7c80] c0000000019e4fac kernel_init_freeable+0x4dc/0x5a4
> [c000000c6d1e7db0] c000000000012474 kernel_init+0x24/0x160
> [c000000c6d1e7e20] c00000000000cbd0 ret_from_kernel_thread+0x5c/0x6c
>
> With DEBUG_VM disabled
>
> [ 20.530152] BUG: Kernel NULL pointer dereference on read at 0x00000000
> [ 20.530183] Faulting instruction address: 0xc0000000000df330
> cpu 0x33: Vector: 380 (Data SLB Access) at [c000000c6d19f700]
> pc: c0000000000df330: memset+0x68/0x104
> lr: c00000000009f6d8: hash__pmdp_huge_get_and_clear+0xe8/0x1b0
> sp: c000000c6d19f990
> msr: 8000000002009033
> dar: 0
> current = 0xc000000c6d177480
> paca = 0xc00000001ec4f400 irqmask: 0x03 irq_happened: 0x01
> pid = 1, comm = swapper/0
> [link register ] c00000000009f6d8 hash__pmdp_huge_get_and_clear+0xe8/0x1b0
> [c000000c6d19f990] c00000000009f748 hash__pmdp_huge_get_and_clear+0x158/0x1b0 (unreliable)
> [c000000c6d19fa10] c0000000019ebf30 pmd_advanced_tests+0x1f0/0x378
> [c000000c6d19fab0] c0000000019ed088 debug_vm_pgtable+0x79c/0x1244
> [c000000c6d19fba0] c0000000000116ec do_one_initcall+0xac/0x5f0
> [c000000c6d19fc80] c0000000019a4fac kernel_init_freeable+0x4dc/0x5a4
> [c000000c6d19fdb0] c000000000012474 kernel_init+0x24/0x160
> [c000000c6d19fe20] c00000000000cbd0 ret_from_kernel_thread+0x5c/0x6c
> 33:mon>
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> arch/powerpc/Kconfig | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 65bed1fdeaad..787e829b6f25 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -116,7 +116,6 @@ config PPC
> #
> select ARCH_32BIT_OFF_T if PPC32
> select ARCH_HAS_DEBUG_VIRTUAL
> - select ARCH_HAS_DEBUG_VM_PGTABLE
> select ARCH_HAS_DEVMEM_IS_ALLOWED
> select ARCH_HAS_ELF_RANDOMIZE
> select ARCH_HAS_FORTIFY_SOURCE
>
If support for powerpc is being dropped, please update the features file
here as well. They should be in sync.
Documentation/features/debug/debug-vm-pgtable/arch-support.txt
^ permalink raw reply
* Re: [PATCH 00/10] sound: convert tasklets to use new tasklet_setup()
From: Takashi Iwai @ 2020-09-01 10:09 UTC (permalink / raw)
To: Allen
Cc: alsa-devel, Kees Cook, timur, Xiubo.Lee,
Linux Kernel Mailing List, clemens, tiwai, o-takashi,
nicoleotsuka, Allen Pais, Mark Brown, perex, linuxppc-dev
In-Reply-To: <CAOMdWSJ2VKhbnRDTNVuTKSL12k0qhryO7yznstAk8k_nBGp2=Q@mail.gmail.com>
On Tue, 01 Sep 2020 12:04:53 +0200,
Allen wrote:
>
> Takashi,
> > > > > These patches which I wasn't CCed on and which need their subject lines
> > > > > fixing :( . With the subject lines fixed I guess so so
> > >
> > > > Extremely sorry. I thought I had it covered. How would you like it
> > > > worded?
> > >
> > > ASoC:
> >
> > To be more exact, "ASoC:" prefix is for sound/soc/*, and for the rest
> > sound/*, use "ALSA:" prefix please.
>
> I could not get the generic API accepted upstream. We would stick to
> from_tasklet()
> or container_of(). Could I go ahead and send out V2 using
> from_tasklet() with subject line fixed?
Yes, please submit whatever should go into 5.9.
thanks,
Takashi
^ permalink raw reply
* Re: [PATCH 0/8] scsi: convert tasklets to use new tasklet_setup()
From: Allen @ 2020-09-01 10:06 UTC (permalink / raw)
To: jejb
Cc: Kees Cook, martin.petersen, shivasharan.srikanteshwara,
Linux Kernel Mailing List, kashyap.desai, sumit.saxena,
Allen Pais, target-devel, linux-scsi, linuxppc-dev,
megaraidlinux.pdl
In-Reply-To: <1597694252.22390.12.camel@linux.ibm.com>
> > > >
> > > > Commit 12cc923f1ccc ("tasklet: Introduce new initialization
> > > > API")' introduced a new tasklet initialization API. This series
> > > > converts all the scsi drivers to use the new tasklet_setup() API
> > >
> > > I've got to say I agree with Jens, this was a silly obfuscation:
> > >
> > > +#define from_tasklet(var, callback_tasklet, tasklet_fieldname) \
> > > + container_of(callback_tasklet, typeof(*var),
> > > tasklet_fieldname)
> > >
> > > Just use container_of directly since we all understand what it
> > > does.
> >
> > But then the lines get really long, wrapped, etc.
>
> I really don't think that's a problem but if you want to add a new
> generic container_of that does typeof instead of insisting on the type,
> I'd be sort of OK with that ... provided you don't gratuitously alter
> the argument order.
>
> The thing I object to is that this encourages everyone to roll their
> own unnecessary container_of type macros in spite of the fact that it's
> function is wholly generic. It's fine if you're eliminating one of the
> arguments, or actually making the macro specific to the type, but in
> this case you're not, you're making a completely generic macro where
> the name is the only thing that's specific to this case.
>
> > This is what the timer_struct conversion did too (added a
> > container_of wrapper), so I think it makes sense here too.
>
> I didn't see that one to object to it ...
Since we could not get the generic API accepted, can I send out V2
which would use container_of()?
Thanks,
--
- Allen
^ permalink raw reply
* Re: [PATCH 00/10] sound: convert tasklets to use new tasklet_setup()
From: Allen @ 2020-09-01 10:04 UTC (permalink / raw)
To: Takashi Iwai
Cc: alsa-devel, Kees Cook, timur, Xiubo.Lee,
Linux Kernel Mailing List, clemens, tiwai, o-takashi,
nicoleotsuka, Allen Pais, Mark Brown, perex, linuxppc-dev
In-Reply-To: <s5h4koyj2no.wl-tiwai@suse.de>
Takashi,
> > > > These patches which I wasn't CCed on and which need their subject lines
> > > > fixing :( . With the subject lines fixed I guess so so
> >
> > > Extremely sorry. I thought I had it covered. How would you like it
> > > worded?
> >
> > ASoC:
>
> To be more exact, "ASoC:" prefix is for sound/soc/*, and for the rest
> sound/*, use "ALSA:" prefix please.
I could not get the generic API accepted upstream. We would stick to
from_tasklet()
or container_of(). Could I go ahead and send out V2 using
from_tasklet() with subject line fixed?
Thanks,
--
- Allen
^ permalink raw reply
* Re: [PATCH v3 09/13] mm/debug_vm_pgtable/locks: Move non page table modifying test together
From: Anshuman Khandual @ 2020-09-01 9:58 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm
Cc: linux-arch, linux-s390, Christophe Leroy, x86, Mike Rapoport,
Qian Cai, Gerald Schaefer, Vineet Gupta, linux-snps-arc,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <58a5280f-4882-4a36-c52d-15ad879209d6@linux.ibm.com>
On 09/01/2020 03:06 PM, Aneesh Kumar K.V wrote:
>
>>>>
>>>> [ 17.080644] ------------[ cut here ]------------
>>>> [ 17.081342] kernel BUG at mm/pgtable-generic.c:164!
>>>> [ 17.082091] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>>>> [ 17.082977] Modules linked in:
>>>> [ 17.083481] CPU: 79 PID: 1 Comm: swapper/0 Tainted: G W 5.9.0-rc2-00105-g740e72cd6717 #14
>>>> [ 17.084998] Hardware name: linux,dummy-virt (DT)
>>>> [ 17.085745] pstate: 60400005 (nZCv daif +PAN -UAO BTYPE=--)
>>>> [ 17.086645] pc : pgtable_trans_huge_deposit+0x58/0x60
>>>> [ 17.087462] lr : debug_vm_pgtable+0x4dc/0x8f0
>>>> [ 17.088168] sp : ffff80001219bcf0
>>>> [ 17.088710] x29: ffff80001219bcf0 x28: ffff8000114ac000
>>>> [ 17.089574] x27: ffff8000114ac000 x26: 0020000000000fd3
>>>> [ 17.090431] x25: 0020000081400fd3 x24: fffffe00175c12c0
>>>> [ 17.091286] x23: ffff0005df04d1a8 x22: 0000f18d6e035000
>>>> [ 17.092143] x21: ffff0005dd790000 x20: ffff0005df18e1a8
>>>> [ 17.093003] x19: ffff0005df04cb80 x18: 0000000000000014
>>>> [ 17.093859] x17: 00000000b76667d0 x16: 00000000fd4e6611
>>>> [ 17.094716] x15: 0000000000000001 x14: 0000000000000002
>>>> [ 17.095572] x13: 000000000055d966 x12: fffffe001755e400
>>>> [ 17.096429] x11: 0000000000000008 x10: ffff0005fcb6e210
>>>> [ 17.097292] x9 : ffff0005fcb6e210 x8 : 0020000081590fd3
>>>> [ 17.098149] x7 : ffff0005dd71e0c0 x6 : ffff0005df18e1a8
>>>> [ 17.099006] x5 : 0020000081590fd3 x4 : 0020000081590fd3
>>>> [ 17.099862] x3 : ffff0005df18e1a8 x2 : fffffe00175c12c0
>>>> [ 17.100718] x1 : fffffe00175c1300 x0 : 0000000000000000
>>>> [ 17.101583] Call trace:
>>>> [ 17.101993] pgtable_trans_huge_deposit+0x58/0x60
>>>> [ 17.102754] do_one_initcall+0x74/0x1cc
>>>> [ 17.103381] kernel_init_freeable+0x1d0/0x238
>>>> [ 17.104089] kernel_init+0x14/0x118
>>>> [ 17.104658] ret_from_fork+0x10/0x34
>>>> [ 17.105251] Code: f9000443 f9000843 f9000822 d65f03c0 (d4210000)
>>>> [ 17.106303] ---[ end trace e63471e00f8c147f ]---
>>>>
>>>
>>> IIUC, this should happen even without the series right? This is
>>>
>>> assert_spin_locked(pmd_lockptr(mm, pmdp));
>>
>> Crash does not happen without this series. A previous patch [PATCH v3 08/13]
>> added pgtable_trans_huge_deposit/withdraw() in pmd_advanced_tests(). arm64
>> does not define __HAVE_ARCH_PGTABLE_DEPOSIT and hence falls back on generic
>> pgtable_trans_huge_deposit() which the asserts the lock.
>>
>
>
> I fixed that by moving the pgtable deposit after adding the pmd locks correctly.
>
> mm/debug_vm_pgtable/locks: Move non page table modifying test together
> mm/debug_vm_pgtable/locks: Take correct page table lock
> mm/debug_vm_pgtable/thp: Use page table depost/withdraw with THP
Right, it does fix. But then both those patches should be folded/merged in
order to preserve git bisect ability, besides test classification reasons
as mentioned in a previous response and a possible redundant movement of
hugetlb_basic_tests().
^ permalink raw reply
* Re: [PATCH v3 13/13] mm/debug_vm_pgtable: populate a pte entry before fetching it
From: Aneesh Kumar K.V @ 2020-09-01 9:58 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm, akpm
Cc: linux-arch, linux-s390, Christophe Leroy, x86, Mike Rapoport,
Qian Cai, Gerald Schaefer, Vineet Gupta, linux-snps-arc,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <e3140b44-993e-aa4b-130d-ee2230eff2b5@arm.com>
On 9/1/20 1:08 PM, Anshuman Khandual wrote:
>
>
> On 09/01/2020 12:07 PM, Aneesh Kumar K.V wrote:
>> On 9/1/20 8:55 AM, Anshuman Khandual wrote:
>>>
>>>
>>> On 08/27/2020 01:34 PM, Aneesh Kumar K.V wrote:
>>>> pte_clear_tests operate on an existing pte entry. Make sure that is not a none
>>>> pte entry.
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>>> ---
>>>> mm/debug_vm_pgtable.c | 6 ++++--
>>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>>>> index 21329c7d672f..8527ebb75f2c 100644
>>>> --- a/mm/debug_vm_pgtable.c
>>>> +++ b/mm/debug_vm_pgtable.c
>>>> @@ -546,7 +546,7 @@ static void __init pgd_populate_tests(struct mm_struct *mm, pgd_t *pgdp,
>>>> static void __init pte_clear_tests(struct mm_struct *mm, pte_t *ptep,
>>>> unsigned long vaddr)
>>>> {
>>>> - pte_t pte = ptep_get(ptep);
>>>> + pte_t pte = ptep_get_and_clear(mm, vaddr, ptep);
>>>
>>> Seems like ptep_get_and_clear() here just clears the entry in preparation
>>> for a following set_pte_at() which otherwise would have been a problem on
>>> ppc64 as you had pointed out earlier i.e set_pte_at() should not update an
>>> existing valid entry. So the commit message here is bit misleading.
>>>
>>
>> and also fetch the pte value which is used further.
>>
>>
>>>> pr_debug("Validating PTE clear\n");
>>>> pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
>>>> @@ -944,7 +944,7 @@ static int __init debug_vm_pgtable(void)
>>>> p4d_t *p4dp, *saved_p4dp;
>>>> pud_t *pudp, *saved_pudp;
>>>> pmd_t *pmdp, *saved_pmdp, pmd;
>>>> - pte_t *ptep;
>>>> + pte_t *ptep, pte;
>>>> pgtable_t saved_ptep;
>>>> pgprot_t prot, protnone;
>>>> phys_addr_t paddr;
>>>> @@ -1049,6 +1049,8 @@ static int __init debug_vm_pgtable(void)
>>>> */
>>>> ptep = pte_alloc_map_lock(mm, pmdp, vaddr, &ptl);
>>>> + pte = pfn_pte(pte_aligned, prot);
>>>> + set_pte_at(mm, vaddr, ptep, pte);
>>>
>>> Not here, creating and populating an entry must be done in respective
>>> test functions itself. Besides, this seems bit redundant as well. The
>>> test pte_clear_tests() with the above change added, already
>>>
>>> - Clears the PTEP entry with ptep_get_and_clear()
>>
>> and fetch the old value set previously.
>
> In that case, please move above two lines i.e
>
> pte = pfn_pte(pte_aligned, prot);
> set_pte_at(mm, vaddr, ptep, pte);
>
> from debug_vm_pgtable() to pte_clear_tests() and update it's arguments
> as required.
>
Frankly, I don't understand what these tests are testing. It all looks
like some random clear and set.
static void __init pte_clear_tests(struct mm_struct *mm, pte_t *ptep,
unsigned long vaddr, unsigned long pfn,
pgprot_t prot)
{
pte_t pte = pfn_pte(pfn, prot);
set_pte_at(mm, vaddr, ptep, pte);
pte = ptep_get_and_clear(mm, vaddr, ptep);
pr_debug("Validating PTE clear\n");
pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
set_pte_at(mm, vaddr, ptep, pte);
barrier();
pte_clear(mm, vaddr, ptep);
pte = ptep_get(ptep);
WARN_ON(!pte_none(pte));
}
-aneesh
^ permalink raw reply
* [PATCH v2] powerpc/mm: Remove DEBUG_VM_PGTABLE support on powerpc
From: Aneesh Kumar K.V @ 2020-09-01 9:44 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V, Anshuman Khandual
The test is broken w.r.t page table update rules and results in kernel
crash as below. Disable the support until we get the tests updated.
[ 21.083519] kernel BUG at arch/powerpc/mm/pgtable.c:304!
cpu 0x0: Vector: 700 (Program Check) at [c000000c6d1e76c0]
pc: c00000000009a5ec: assert_pte_locked+0x14c/0x380
lr: c0000000005eeeec: pte_update+0x11c/0x190
sp: c000000c6d1e7950
msr: 8000000002029033
current = 0xc000000c6d172c80
paca = 0xc000000003ba0000 irqmask: 0x03 irq_happened: 0x01
pid = 1, comm = swapper/0
kernel BUG at arch/powerpc/mm/pgtable.c:304!
[link register ] c0000000005eeeec pte_update+0x11c/0x190
[c000000c6d1e7950] 0000000000000001 (unreliable)
[c000000c6d1e79b0] c0000000005eee14 pte_update+0x44/0x190
[c000000c6d1e7a10] c000000001a2ca9c pte_advanced_tests+0x160/0x3d8
[c000000c6d1e7ab0] c000000001a2d4fc debug_vm_pgtable+0x7e8/0x1338
[c000000c6d1e7ba0] c0000000000116ec do_one_initcall+0xac/0x5f0
[c000000c6d1e7c80] c0000000019e4fac kernel_init_freeable+0x4dc/0x5a4
[c000000c6d1e7db0] c000000000012474 kernel_init+0x24/0x160
[c000000c6d1e7e20] c00000000000cbd0 ret_from_kernel_thread+0x5c/0x6c
With DEBUG_VM disabled
[ 20.530152] BUG: Kernel NULL pointer dereference on read at 0x00000000
[ 20.530183] Faulting instruction address: 0xc0000000000df330
cpu 0x33: Vector: 380 (Data SLB Access) at [c000000c6d19f700]
pc: c0000000000df330: memset+0x68/0x104
lr: c00000000009f6d8: hash__pmdp_huge_get_and_clear+0xe8/0x1b0
sp: c000000c6d19f990
msr: 8000000002009033
dar: 0
current = 0xc000000c6d177480
paca = 0xc00000001ec4f400 irqmask: 0x03 irq_happened: 0x01
pid = 1, comm = swapper/0
[link register ] c00000000009f6d8 hash__pmdp_huge_get_and_clear+0xe8/0x1b0
[c000000c6d19f990] c00000000009f748 hash__pmdp_huge_get_and_clear+0x158/0x1b0 (unreliable)
[c000000c6d19fa10] c0000000019ebf30 pmd_advanced_tests+0x1f0/0x378
[c000000c6d19fab0] c0000000019ed088 debug_vm_pgtable+0x79c/0x1244
[c000000c6d19fba0] c0000000000116ec do_one_initcall+0xac/0x5f0
[c000000c6d19fc80] c0000000019a4fac kernel_init_freeable+0x4dc/0x5a4
[c000000c6d19fdb0] c000000000012474 kernel_init+0x24/0x160
[c000000c6d19fe20] c00000000000cbd0 ret_from_kernel_thread+0x5c/0x6c
33:mon>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 65bed1fdeaad..787e829b6f25 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -116,7 +116,6 @@ config PPC
#
select ARCH_32BIT_OFF_T if PPC32
select ARCH_HAS_DEBUG_VIRTUAL
- select ARCH_HAS_DEBUG_VM_PGTABLE
select ARCH_HAS_DEVMEM_IS_ALLOWED
select ARCH_HAS_ELF_RANDOMIZE
select ARCH_HAS_FORTIFY_SOURCE
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v3 09/13] mm/debug_vm_pgtable/locks: Move non page table modifying test together
From: Aneesh Kumar K.V @ 2020-09-01 9:36 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm, akpm
Cc: linux-arch, linux-s390, Christophe Leroy, x86, Mike Rapoport,
Qian Cai, Gerald Schaefer, Vineet Gupta, linux-snps-arc,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <9b75b175-f319-d40e-a95e-b323b3db654a@arm.com>
>>>
>>> [ 17.080644] ------------[ cut here ]------------
>>> [ 17.081342] kernel BUG at mm/pgtable-generic.c:164!
>>> [ 17.082091] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>>> [ 17.082977] Modules linked in:
>>> [ 17.083481] CPU: 79 PID: 1 Comm: swapper/0 Tainted: G W 5.9.0-rc2-00105-g740e72cd6717 #14
>>> [ 17.084998] Hardware name: linux,dummy-virt (DT)
>>> [ 17.085745] pstate: 60400005 (nZCv daif +PAN -UAO BTYPE=--)
>>> [ 17.086645] pc : pgtable_trans_huge_deposit+0x58/0x60
>>> [ 17.087462] lr : debug_vm_pgtable+0x4dc/0x8f0
>>> [ 17.088168] sp : ffff80001219bcf0
>>> [ 17.088710] x29: ffff80001219bcf0 x28: ffff8000114ac000
>>> [ 17.089574] x27: ffff8000114ac000 x26: 0020000000000fd3
>>> [ 17.090431] x25: 0020000081400fd3 x24: fffffe00175c12c0
>>> [ 17.091286] x23: ffff0005df04d1a8 x22: 0000f18d6e035000
>>> [ 17.092143] x21: ffff0005dd790000 x20: ffff0005df18e1a8
>>> [ 17.093003] x19: ffff0005df04cb80 x18: 0000000000000014
>>> [ 17.093859] x17: 00000000b76667d0 x16: 00000000fd4e6611
>>> [ 17.094716] x15: 0000000000000001 x14: 0000000000000002
>>> [ 17.095572] x13: 000000000055d966 x12: fffffe001755e400
>>> [ 17.096429] x11: 0000000000000008 x10: ffff0005fcb6e210
>>> [ 17.097292] x9 : ffff0005fcb6e210 x8 : 0020000081590fd3
>>> [ 17.098149] x7 : ffff0005dd71e0c0 x6 : ffff0005df18e1a8
>>> [ 17.099006] x5 : 0020000081590fd3 x4 : 0020000081590fd3
>>> [ 17.099862] x3 : ffff0005df18e1a8 x2 : fffffe00175c12c0
>>> [ 17.100718] x1 : fffffe00175c1300 x0 : 0000000000000000
>>> [ 17.101583] Call trace:
>>> [ 17.101993] pgtable_trans_huge_deposit+0x58/0x60
>>> [ 17.102754] do_one_initcall+0x74/0x1cc
>>> [ 17.103381] kernel_init_freeable+0x1d0/0x238
>>> [ 17.104089] kernel_init+0x14/0x118
>>> [ 17.104658] ret_from_fork+0x10/0x34
>>> [ 17.105251] Code: f9000443 f9000843 f9000822 d65f03c0 (d4210000)
>>> [ 17.106303] ---[ end trace e63471e00f8c147f ]---
>>>
>>
>> IIUC, this should happen even without the series right? This is
>>
>> assert_spin_locked(pmd_lockptr(mm, pmdp));
>
> Crash does not happen without this series. A previous patch [PATCH v3 08/13]
> added pgtable_trans_huge_deposit/withdraw() in pmd_advanced_tests(). arm64
> does not define __HAVE_ARCH_PGTABLE_DEPOSIT and hence falls back on generic
> pgtable_trans_huge_deposit() which the asserts the lock.
>
I fixed that by moving the pgtable deposit after adding the pmd locks
correctly.
mm/debug_vm_pgtable/locks: Move non page table modifying test together
mm/debug_vm_pgtable/locks: Take correct page table lock
mm/debug_vm_pgtable/thp: Use page table depost/withdraw with THP
-aneesh
^ permalink raw reply
* Re: [PATCH v3 09/13] mm/debug_vm_pgtable/locks: Move non page table modifying test together
From: Anshuman Khandual @ 2020-09-01 9:33 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm
Cc: linux-arch, linux-s390, Christophe Leroy, x86, Mike Rapoport,
Qian Cai, Gerald Schaefer, Vineet Gupta, linux-snps-arc,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <b6240372-4554-8c17-186a-cdc0b0a9089c@linux.ibm.com>
On 09/01/2020 12:08 PM, Aneesh Kumar K.V wrote:
> On 9/1/20 9:11 AM, Anshuman Khandual wrote:
>>
>>
>> On 08/27/2020 01:34 PM, Aneesh Kumar K.V wrote:
>>> This will help in adding proper locks in a later patch
>>
>> It really makes sense to classify these tests here as static and dynamic.
>> Static are the ones that test via page table entry values modification
>> without changing anything on the actual page table itself. Where as the
>> dynamic tests do change the page table. Static tests would probably never
>> require any lock synchronization but the dynamic ones will do.
>>
>>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>> ---
>>> mm/debug_vm_pgtable.c | 52 ++++++++++++++++++++++++-------------------
>>> 1 file changed, 29 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>>> index 0ce5c6a24c5b..78c8af3445ac 100644
>>> --- a/mm/debug_vm_pgtable.c
>>> +++ b/mm/debug_vm_pgtable.c
>>> @@ -992,7 +992,7 @@ static int __init debug_vm_pgtable(void)
>>> p4dp = p4d_alloc(mm, pgdp, vaddr);
>>> pudp = pud_alloc(mm, p4dp, vaddr);
>>> pmdp = pmd_alloc(mm, pudp, vaddr);
>>> - ptep = pte_alloc_map_lock(mm, pmdp, vaddr, &ptl);
>>> + ptep = pte_alloc_map(mm, pmdp, vaddr);
>>> /*
>>> * Save all the page table page addresses as the page table
>>> @@ -1012,33 +1012,12 @@ static int __init debug_vm_pgtable(void)
>>> p4d_basic_tests(p4d_aligned, prot);
>>> pgd_basic_tests(pgd_aligned, prot);
>>> - pte_clear_tests(mm, ptep, vaddr);
>>> - pmd_clear_tests(mm, pmdp);
>>> - pud_clear_tests(mm, pudp);
>>> - p4d_clear_tests(mm, p4dp);
>>> - pgd_clear_tests(mm, pgdp);
>>> -
>>> - pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
>>> - pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot, saved_ptep);
>>> - pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot);
>>> - hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
>>> -
>>> pmd_leaf_tests(pmd_aligned, prot);
>>> pud_leaf_tests(pud_aligned, prot);
>>> - pmd_huge_tests(pmdp, pmd_aligned, prot);
>>> - pud_huge_tests(pudp, pud_aligned, prot);
>>> -
>>> pte_savedwrite_tests(pte_aligned, protnone);
>>> pmd_savedwrite_tests(pmd_aligned, protnone);
>>> - pte_unmap_unlock(ptep, ptl);
>>> -
>>> - pmd_populate_tests(mm, pmdp, saved_ptep);
>>> - pud_populate_tests(mm, pudp, saved_pmdp);
>>> - p4d_populate_tests(mm, p4dp, saved_pudp);
>>> - pgd_populate_tests(mm, pgdp, saved_p4dp);
>>> -
>>> pte_special_tests(pte_aligned, prot);
>>> pte_protnone_tests(pte_aligned, protnone);
>>> pmd_protnone_tests(pmd_aligned, protnone);
>>> @@ -1056,11 +1035,38 @@ static int __init debug_vm_pgtable(void)
>>> pmd_swap_tests(pmd_aligned, prot);
>>> swap_migration_tests();
>>> - hugetlb_basic_tests(pte_aligned, prot);
>>> pmd_thp_tests(pmd_aligned, prot);
>>> pud_thp_tests(pud_aligned, prot);
>>> + /*
>>> + * Page table modifying tests
>>> + */
>>
>> In this comment, please do add some more details about how these tests
>> need proper locking mechanism unlike the previous static ones. Also add
>> a similar comment section for the static tests that dont really change
>> page table and need not require corresponding locking mechanism. Both
>> comment sections will make this classification clear.
>>
>>> + pte_clear_tests(mm, ptep, vaddr);
>>> + pmd_clear_tests(mm, pmdp);
>>> + pud_clear_tests(mm, pudp);
>>> + p4d_clear_tests(mm, p4dp);
>>> + pgd_clear_tests(mm, pgdp);
>>> +
>>> + ptep = pte_alloc_map_lock(mm, pmdp, vaddr, &ptl);
>>> + pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
>>> + pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot, saved_ptep);
>>> + pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot);
>>> + hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
>>> +
>>> +
>>> + pmd_huge_tests(pmdp, pmd_aligned, prot);
>>> + pud_huge_tests(pudp, pud_aligned, prot);
>>> +
>>> + pte_unmap_unlock(ptep, ptl);
>>> +
>>> + pmd_populate_tests(mm, pmdp, saved_ptep);
>>> + pud_populate_tests(mm, pudp, saved_pmdp);
>>> + p4d_populate_tests(mm, p4dp, saved_pudp);
>>> + pgd_populate_tests(mm, pgdp, saved_p4dp);
>>> +
>>> + hugetlb_basic_tests(pte_aligned, prot);
>>
>> hugetlb_basic_tests() is a non page table modifying static test and
>> should be classified accordingly.
>>
>>> +
>>> p4d_free(mm, saved_p4dp);
>>> pud_free(mm, saved_pudp);
>>> pmd_free(mm, saved_pmdp);
>>>
>>
>> Changes till this patch fails to boot on arm64 platform. This should be
>> folded with the next patch.
>>
>> [ 17.080644] ------------[ cut here ]------------
>> [ 17.081342] kernel BUG at mm/pgtable-generic.c:164!
>> [ 17.082091] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>> [ 17.082977] Modules linked in:
>> [ 17.083481] CPU: 79 PID: 1 Comm: swapper/0 Tainted: G W 5.9.0-rc2-00105-g740e72cd6717 #14
>> [ 17.084998] Hardware name: linux,dummy-virt (DT)
>> [ 17.085745] pstate: 60400005 (nZCv daif +PAN -UAO BTYPE=--)
>> [ 17.086645] pc : pgtable_trans_huge_deposit+0x58/0x60
>> [ 17.087462] lr : debug_vm_pgtable+0x4dc/0x8f0
>> [ 17.088168] sp : ffff80001219bcf0
>> [ 17.088710] x29: ffff80001219bcf0 x28: ffff8000114ac000
>> [ 17.089574] x27: ffff8000114ac000 x26: 0020000000000fd3
>> [ 17.090431] x25: 0020000081400fd3 x24: fffffe00175c12c0
>> [ 17.091286] x23: ffff0005df04d1a8 x22: 0000f18d6e035000
>> [ 17.092143] x21: ffff0005dd790000 x20: ffff0005df18e1a8
>> [ 17.093003] x19: ffff0005df04cb80 x18: 0000000000000014
>> [ 17.093859] x17: 00000000b76667d0 x16: 00000000fd4e6611
>> [ 17.094716] x15: 0000000000000001 x14: 0000000000000002
>> [ 17.095572] x13: 000000000055d966 x12: fffffe001755e400
>> [ 17.096429] x11: 0000000000000008 x10: ffff0005fcb6e210
>> [ 17.097292] x9 : ffff0005fcb6e210 x8 : 0020000081590fd3
>> [ 17.098149] x7 : ffff0005dd71e0c0 x6 : ffff0005df18e1a8
>> [ 17.099006] x5 : 0020000081590fd3 x4 : 0020000081590fd3
>> [ 17.099862] x3 : ffff0005df18e1a8 x2 : fffffe00175c12c0
>> [ 17.100718] x1 : fffffe00175c1300 x0 : 0000000000000000
>> [ 17.101583] Call trace:
>> [ 17.101993] pgtable_trans_huge_deposit+0x58/0x60
>> [ 17.102754] do_one_initcall+0x74/0x1cc
>> [ 17.103381] kernel_init_freeable+0x1d0/0x238
>> [ 17.104089] kernel_init+0x14/0x118
>> [ 17.104658] ret_from_fork+0x10/0x34
>> [ 17.105251] Code: f9000443 f9000843 f9000822 d65f03c0 (d4210000)
>> [ 17.106303] ---[ end trace e63471e00f8c147f ]---
>>
>
> IIUC, this should happen even without the series right? This is
>
> assert_spin_locked(pmd_lockptr(mm, pmdp));
Crash does not happen without this series. A previous patch [PATCH v3 08/13]
added pgtable_trans_huge_deposit/withdraw() in pmd_advanced_tests(). arm64
does not define __HAVE_ARCH_PGTABLE_DEPOSIT and hence falls back on generic
pgtable_trans_huge_deposit() which the asserts the lock.
^ permalink raw reply
* Re: [PATCH] arch: vdso: add vdso linker script to 'targets' instead of extra-y
From: Catalin Marinas @ 2020-09-01 9:19 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-s390, Vasily Gorbik, Nick Hu, linux-kbuild, Heiko Carstens,
linuxppc-dev, linux-kernel, Christian Borntraeger, Paul Mackerras,
Greentime Hu, Vincent Chen, Will Deacon, linux-arm-kernel
In-Reply-To: <20200831182239.480317-1-masahiroy@kernel.org>
On Tue, Sep 01, 2020 at 03:22:39AM +0900, Masahiro Yamada wrote:
> The vdso linker script is preprocessed on demand.
> Adding it to 'targets' is enough to include the .cmd file.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
For arm64:
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* Re: [PATCH] powerpc/mm: Remove DEBUG_VM_PGTABLE support on ppc64
From: Aneesh Kumar K.V @ 2020-09-01 9:13 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev, mpe; +Cc: Anshuman Khandual
In-Reply-To: <49fbd2b4-31af-826d-c884-32b7cc1c6e8d@csgroup.eu>
On 9/1/20 2:40 PM, Christophe Leroy wrote:
>
>
> Le 01/09/2020 à 10:15, Christophe Leroy a écrit :
>>
>>
>> Le 01/09/2020 à 10:12, Aneesh Kumar K.V a écrit :
>>> On 9/1/20 1:40 PM, Christophe Leroy wrote:
>>>>
>>>>
>>>> Le 01/09/2020 à 10:02, Aneesh Kumar K.V a écrit :
>>>>> The test is broken w.r.t page table update rules and results in kernel
>>>>> crash as below. Disable the support untill we get the tests updated.
>>>>>
>>>>>
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>>>
>>>> Any Fixes: tag ?
>>>>
>>>>> ---
>>>>> arch/powerpc/Kconfig | 1 -
>>>>> 1 file changed, 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>>>> index 65bed1fdeaad..787e829b6f25 100644
>>>>> --- a/arch/powerpc/Kconfig
>>>>> +++ b/arch/powerpc/Kconfig
>>>>> @@ -116,7 +116,6 @@ config PPC
>>>>> #
>>>>> select ARCH_32BIT_OFF_T if PPC32
>>>>> select ARCH_HAS_DEBUG_VIRTUAL
>>>>> - select ARCH_HAS_DEBUG_VM_PGTABLE
>>>>
>>>>
>>>> You say you remove support for ppc64 but you are removing it for
>>>> both PPC64 and PPC32. Should you replace the line by:
>>>
>>> Does it work on PPC32 with DEBUG_VM enabled? I was expecting it will
>>> be broken there too.
>>
>> I was wondering. I have just started a build to test that on my 8xx.
>> I'll tell you before noon (Paris).
>>
>
> There are warnings but it boots OK. So up to you, but if you deactivate
> it for both PPC32 and PPC64, say so in the subject like.
>
I will update the subject line to indicate powerpc instead of ppc64
-aneesh
^ permalink raw reply
* Re: [PATCH v2 00/13] mm/debug_vm_pgtable fixes
From: Anshuman Khandual @ 2020-09-01 9:11 UTC (permalink / raw)
To: Christophe Leroy, Aneesh Kumar K.V, linux-mm, akpm
Cc: Linux-Arch, linux-s390@vger.kernel.org, x86@kernel.org,
Mike Rapoport, Qian Cai, Gerald Schaefer, Vineet Gupta,
linux-snps-arc@lists.infradead.org, linuxppc-dev, Linux ARM
In-Reply-To: <c0de2c68-826b-bf0f-dc2c-a501fa7bef38@csgroup.eu>
On 09/01/2020 01:33 PM, Christophe Leroy wrote:
>
>
> Le 21/08/2020 à 10:51, Anshuman Khandual a écrit :
>>
>> On 08/19/2020 06:30 PM, Aneesh Kumar K.V wrote:
>>> This patch series includes fixes for debug_vm_pgtable test code so that
>>> they follow page table updates rules correctly. The first two patches introduce
>>> changes w.r.t ppc64. The patches are included in this series for completeness. We can
>>> merge them via ppc64 tree if required.
>>>
>>> Hugetlb test is disabled on ppc64 because that needs larger change to satisfy
>>> page table update rules.
>>>
>
>>
>> Changes proposed here will impact other enabled platforms as well.
>> Adding the following folks and mailing lists, and hoping to get a
>> broader review and test coverage. Please do include them in the
>> next iteration as well.
>>
>> + linux-arm-kernel@lists.infradead.org
>> + linux-s390@vger.kernel.org
>> + linux-snps-arc@lists.infradead.org
>> + x86@kernel.org
>> + linux-arch@vger.kernel.org
>>
>> + Gerald Schaefer <gerald.schaefer@de.ibm.com>
>> + Christophe Leroy <christophe.leroy@c-s.fr>
>
> Please don't use anymore the above address. Only use the one below.
>
>> + Christophe Leroy <christophe.leroy@csgroup.eu>
Sure, noted.
>> + Vineet Gupta <vgupta@synopsys.com>
>> + Mike Rapoport <rppt@linux.ibm.com>
>> + Qian Cai <cai@lca.pw>
>>
>
> Thanks
> Christophe
>
>
^ permalink raw reply
* Re: [PATCH] powerpc/mm: Remove DEBUG_VM_PGTABLE support on ppc64
From: Christophe Leroy @ 2020-09-01 9:10 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: Anshuman Khandual
In-Reply-To: <6661a001-0a00-17b6-cb34-0f3510ca1fec@csgroup.eu>
Le 01/09/2020 à 10:15, Christophe Leroy a écrit :
>
>
> Le 01/09/2020 à 10:12, Aneesh Kumar K.V a écrit :
>> On 9/1/20 1:40 PM, Christophe Leroy wrote:
>>>
>>>
>>> Le 01/09/2020 à 10:02, Aneesh Kumar K.V a écrit :
>>>> The test is broken w.r.t page table update rules and results in kernel
>>>> crash as below. Disable the support untill we get the tests updated.
>>>>
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>>
>>> Any Fixes: tag ?
>>>
>>>> ---
>>>> arch/powerpc/Kconfig | 1 -
>>>> 1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>>> index 65bed1fdeaad..787e829b6f25 100644
>>>> --- a/arch/powerpc/Kconfig
>>>> +++ b/arch/powerpc/Kconfig
>>>> @@ -116,7 +116,6 @@ config PPC
>>>> #
>>>> select ARCH_32BIT_OFF_T if PPC32
>>>> select ARCH_HAS_DEBUG_VIRTUAL
>>>> - select ARCH_HAS_DEBUG_VM_PGTABLE
>>>
>>>
>>> You say you remove support for ppc64 but you are removing it for both
>>> PPC64 and PPC32. Should you replace the line by:
>>
>> Does it work on PPC32 with DEBUG_VM enabled? I was expecting it will
>> be broken there too.
>
> I was wondering. I have just started a build to test that on my 8xx.
> I'll tell you before noon (Paris).
>
There are warnings but it boots OK. So up to you, but if you deactivate
it for both PPC32 and PPC64, say so in the subject like.
[ 7.065399] debug_vm_pgtable: [debug_vm_pgtable ]: Validating
architecture page table helpers
[ 7.075155] ------------[ cut here ]------------
[ 7.079590] WARNING: CPU: 0 PID: 1 at arch/powerpc/mm/pgtable.c:185
set_pte_at+0x20/0xf4
[ 7.087542] CPU: 0 PID: 1 Comm: swapper Not tainted
5.9.0-rc2-s3k-dev-01348-g283e890ee4ad #3933
[ 7.096122] NIP: c000f634 LR: c07440f8 CTR: 00000000
[ 7.101124] REGS: c9023c50 TRAP: 0700 Not tainted
(5.9.0-rc2-s3k-dev-01348-g283e890ee4ad)
[ 7.109445] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 53000339 XER: 80006100
[ 7.116072]
[ 7.116072] GPR00: c07440f8 c9023d08 c60e4000 c6ba4000 1efac000
c6ba8eb0 c9023da8 00000001
[ 7.116072] GPR08: 00000004 007346c9 c6ba8ebc 00000000 93000333
00000000 c000390c 00000000
[ 7.116072] GPR16: c0840000 00000ec9 000001ec 00734000 06ba8000
c6bb0000 c05f43e8 1efac000
[ 7.116072] GPR24: fffffff0 c6b96d08 c6ba8eac c6ba4000 1efac000
007346c9 c6ba8eb0 007346c9
[ 7.150796] NIP [c000f634] set_pte_at+0x20/0xf4
[ 7.155274] LR [c07440f8] pte_advanced_tests+0xec/0x2bc
[ 7.160401] Call Trace:
[ 7.162831] [c9023d08] [c080db94] 0xc080db94 (unreliable)
[ 7.168183] [c9023d28] [c07440f8] pte_advanced_tests+0xec/0x2bc
[ 7.174036] [c9023dd8] [c0744498] debug_vm_pgtable+0x1d0/0x668
[ 7.179827] [c9023e98] [c0734cd4] do_one_initcall+0x8c/0x1cc
[ 7.185405] [c9023ef8] [c0735008] kernel_init_freeable+0x178/0x1d0
[ 7.191511] [c9023f28] [c0003920] kernel_init+0x14/0x114
[ 7.196763] [c9023f38] [c000e184] ret_from_kernel_thread+0x14/0x1c
[ 7.202818] Instruction dump:
[ 7.205754] bba10014 7c0803a6 38210020 4e800020 7c0802a6 9421ffe0
bfc10018 90010024
[ 7.213412] 83e60000 81250000 71270001 41820008 <0fe00000> 73e90040
41820080 73ea0001
[ 7.221249] ---[ end trace 95bbebcafa22d0f7 ]---
[ 7.226049] ------------[ cut here ]------------
[ 7.230438] WARNING: CPU: 0 PID: 1 at arch/powerpc/mm/pgtable.c:185
set_pte_at+0x20/0xf4
[ 7.238410] CPU: 0 PID: 1 Comm: swapper Tainted: G W
5.9.0-rc2-s3k-dev-01348-g283e890ee4ad #3933
[ 7.248363] NIP: c000f634 LR: c0744218 CTR: 00000000
[ 7.253368] REGS: c9023c50 TRAP: 0700 Tainted: G W
(5.9.0-rc2-s3k-dev-01348-g283e890ee4ad)
[ 7.263064] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 53000335 XER: a0006100
[ 7.269690]
[ 7.269690] GPR00: c0744218 c9023d08 c60e4000 c6ba4000 1efac000
c6ba8eb0 c9023da8 00000001
[ 7.269690] GPR08: 00000000 007341c9 00000000 007341c9 93000333
00000000 c000390c 00000000
[ 7.269690] GPR16: c0840000 00000ec9 000001ec 00734000 06ba8000
c6bb0000 c05f43e8 1efac000
[ 7.269690] GPR24: fffffff0 c6b96d08 c6ba8eac c6ba4000 1efac000
007346c9 c6ba8eb0 007346c9
[ 7.304418] NIP [c000f634] set_pte_at+0x20/0xf4
[ 7.308892] LR [c0744218] pte_advanced_tests+0x20c/0x2bc
[ 7.314105] Call Trace:
[ 7.316535] [c9023d08] [c080db94] 0xc080db94 (unreliable)
[ 7.321888] [c9023d28] [c0744218] pte_advanced_tests+0x20c/0x2bc
[ 7.327826] [c9023dd8] [c0744498] debug_vm_pgtable+0x1d0/0x668
[ 7.333613] [c9023e98] [c0734cd4] do_one_initcall+0x8c/0x1cc
[ 7.339196] [c9023ef8] [c0735008] kernel_init_freeable+0x178/0x1d0
[ 7.345300] [c9023f28] [c0003920] kernel_init+0x14/0x114
[ 7.350551] [c9023f38] [c000e184] ret_from_kernel_thread+0x14/0x1c
[ 7.356609] Instruction dump:
[ 7.359545] bba10014 7c0803a6 38210020 4e800020 7c0802a6 9421ffe0
bfc10018 90010024
[ 7.367203] 83e60000 81250000 71270001 41820008 <0fe00000> 73e90040
41820080 73ea0001
[ 7.375039] ---[ end trace 95bbebcafa22d0f8 ]---
[ 7.379783] ------------[ cut here ]------------
[ 7.384228] WARNING: CPU: 0 PID: 1 at arch/powerpc/mm/pgtable.c:276
set_huge_pte_at+0x104/0x134
[ 7.392803] CPU: 0 PID: 1 Comm: swapper Tainted: G W
5.9.0-rc2-s3k-dev-01348-g283e890ee4ad #3933
[ 7.402756] NIP: c000f8fc LR: c074465c CTR: 00000000
[ 7.407761] REGS: c9023d00 TRAP: 0700 Tainted: G W
(5.9.0-rc2-s3k-dev-01348-g283e890ee4ad)
[ 7.417456] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 53000339 XER: a0006100
[ 7.424082]
[ 7.424082] GPR00: c074465c c9023db8 c60e4000 c6ba4000 000001ec
c6ba8eb0 c9023e48 00000001
[ 7.424082] GPR08: c6ba90ac 0000000b 007346c9 007341c9 93000333
00000000 c000390c 00000000
[ 7.424082] GPR16: c0840000 c6ba8eac 000001ec 00734000 06ba8000
c6bb0000 c05f43e8 1efac000
[ 7.424082] GPR24: c7fb39a0 c6ba8000 c6b96d08 000001cd 000006c9
c0840000 007346c9 00000080
[ 7.458810] NIP [c000f8fc] set_huge_pte_at+0x104/0x134
[ 7.463885] LR [c074465c] debug_vm_pgtable+0x394/0x668
[ 7.468928] Call Trace:
[ 7.471358] [c9023db8] [c6ba8000] 0xc6ba8000 (unreliable)
[ 7.476709] [c9023dd8] [c074465c] debug_vm_pgtable+0x394/0x668
[ 7.482497] [c9023e98] [c0734cd4] do_one_initcall+0x8c/0x1cc
[ 7.488081] [c9023ef8] [c0735008] kernel_init_freeable+0x178/0x1d0
[ 7.494182] [c9023f28] [c0003920] kernel_init+0x14/0x114
[ 7.499436] [c9023f38] [c000e184] ret_from_kernel_thread+0x14/0x1c
[ 7.505494] Instruction dump:
[ 7.508429] 7d2a482e 712a0800 40a2ff78 812204b4 2f890000 419e0014
812900a0 55290034
[ 7.516088] 2b890400 419e0014 57de06b0 4bffff54 <0fe00000> 4bffff3c
7fa3eb78 90a10008
[ 7.523925] ---[ end trace 95bbebcafa22d0f9 ]---
Christophe
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox