* Re: [PATCH v3 13/13] mm/debug_vm_pgtable: populate a pte entry before fetching it
From: Anshuman Khandual @ 2020-09-01 7:38 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: <abef1791-8779-6b34-3178-3bf3ab36d42b@linux.ibm.com>
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.
^ permalink raw reply
* Re: [PATCH v3 08/13] mm/debug_vm_pgtable/thp: Use page table depost/withdraw with THP
From: Aneesh Kumar K.V @ 2020-09-01 7:40 UTC (permalink / raw)
To: Christophe Leroy, 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: <d80a91c3-0edf-7e2f-8101-2d37a371f4bd@csgroup.eu>
On 9/1/20 12:20 PM, Christophe Leroy wrote:
>
>
> Le 01/09/2020 à 08:25, Aneesh Kumar K.V a écrit :
>> On 9/1/20 8:52 AM, Anshuman Khandual wrote:
>>>
>>>
>>>
>>> There is a checkpatch.pl warning here.
>>>
>>> WARNING: Possible unwrapped commit description (prefer a maximum 75
>>> chars per line)
>>> #7:
>>> Architectures like ppc64 use deposited page table while updating the
>>> huge pte
>>>
>>> total: 0 errors, 1 warnings, 40 lines checked
>>>
>>
>> I will ignore all these, because they are not really important IMHO.
>>
>
> When doing a git log in a 80 chars terminal window, having wrapping
> lines is not really convenient. It should be easy to avoid it.
>
We have been ignoring that for a long time isn't it?
For example ppc64 checkpatch already had
--max-line-length=90
There was also recent discussion whether 80 character limit is valid any
more. But I do keep it restricted to 80 character where ever it is
easy/make sense.
-aneesh
^ permalink raw reply
* Re: [PATCH v3 03/13] mm/debug_vm_pgtable/ppc64: Avoid setting top bits in radom value
From: Anshuman Khandual @ 2020-09-01 7:46 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: <e5d32d12-d904-ed8c-8963-d43d2c3744d9@linux.ibm.com>
On 09/01/2020 01:06 PM, Aneesh Kumar K.V wrote:
> On 9/1/20 1:02 PM, Anshuman Khandual wrote:
>>
>>
>> On 09/01/2020 11:51 AM, Aneesh Kumar K.V wrote:
>>> On 9/1/20 8:45 AM, Anshuman Khandual wrote:
>>>>
>>>>
>>>> On 08/27/2020 01:34 PM, Aneesh Kumar K.V wrote:
>>>>> ppc64 use bit 62 to indicate a pte entry (_PAGE_PTE). Avoid setting that bit in
>>>>> random value.
>>>>>
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>>>> ---
>>>>> mm/debug_vm_pgtable.c | 13 ++++++++++---
>>>>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>>>>> index 086309fb9b6f..bbf9df0e64c6 100644
>>>>> --- a/mm/debug_vm_pgtable.c
>>>>> +++ b/mm/debug_vm_pgtable.c
>>>>> @@ -44,10 +44,17 @@
>>>>> * entry type. But these bits might affect the ability to clear entries with
>>>>> * pxx_clear() because of how dynamic page table folding works on s390. So
>>>>> * while loading up the entries do not change the lower 4 bits. It does not
>>>>> - * have affect any other platform.
>>>>> + * have affect any other platform. Also avoid the 62nd bit on ppc64 that is
>>>>> + * used to mark a pte entry.
>>>>> */
>>>>> -#define S390_MASK_BITS 4
>>>>> -#define RANDOM_ORVALUE GENMASK(BITS_PER_LONG - 1, S390_MASK_BITS)
>>>>> +#define S390_SKIP_MASK GENMASK(3, 0)
>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>> +#define PPC64_SKIP_MASK GENMASK(62, 62)
>>>>> +#else
>>>>> +#define PPC64_SKIP_MASK 0x0
>>>>> +#endif
>>>>
>>>> Please drop the #ifdef CONFIG_PPC_BOOK3S_64 here. We already accommodate skip
>>>> bits for a s390 platform requirement and can also do so for ppc64 as well. As
>>>> mentioned before, please avoid adding any platform specific constructs in the
>>>> test.
>>>>
>>>
>>>
>>> that is needed so that it can be built on 32 bit architectures.I did face build errors with arch-linux
>>
>> Could not (#if __BITS_PER_LONG == 32) be used instead or something like
>> that. But should be a generic conditional check identifying 32 bit arch
>> not anything platform specific.
>>
>
> that _PAGE_PTE bit is pretty much specific to PPC BOOK3S_64. Not sure why other architectures need to bothered about ignoring bit 62.
Thats okay as long it does not adversely affect other architectures, ignoring
some more bits is acceptable. Like existing S390_MASK_BITS gets ignored on all
other platforms even if it is a s390 specific constraint. Not having platform
specific #ifdef here, is essential.
^ permalink raw reply
* Re: [PATCH v3 08/13] mm/debug_vm_pgtable/thp: Use page table depost/withdraw with THP
From: Christophe Leroy @ 2020-09-01 7:51 UTC (permalink / raw)
To: Aneesh Kumar K.V, Anshuman Khandual, linux-mm, akpm
Cc: linux-arch, linux-s390, x86, Mike Rapoport, Qian Cai,
Gerald Schaefer, Vineet Gupta, linux-snps-arc, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <2fb4ac88-d417-2bdd-3c56-a816c356636a@linux.ibm.com>
Le 01/09/2020 à 09:40, Aneesh Kumar K.V a écrit :
> On 9/1/20 12:20 PM, Christophe Leroy wrote:
>>
>>
>> Le 01/09/2020 à 08:25, Aneesh Kumar K.V a écrit :
>>> On 9/1/20 8:52 AM, Anshuman Khandual wrote:
>>>>
>>>>
>>>>
>>>> There is a checkpatch.pl warning here.
>>>>
>>>> WARNING: Possible unwrapped commit description (prefer a maximum 75
>>>> chars per line)
>>>> #7:
>>>> Architectures like ppc64 use deposited page table while updating the
>>>> huge pte
>>>>
>>>> total: 0 errors, 1 warnings, 40 lines checked
>>>>
>>>
>>> I will ignore all these, because they are not really important IMHO.
>>>
>>
>> When doing a git log in a 80 chars terminal window, having wrapping
>> lines is not really convenient. It should be easy to avoid it.
>>
>
> We have been ignoring that for a long time isn't it?
>
> For example ppc64 checkpatch already had
> --max-line-length=90
>
>
> There was also recent discussion whether 80 character limit is valid any
> more. But I do keep it restricted to 80 character where ever it is
> easy/make sense.
>
Here we are not talking about the code, but the commit log.
As far as I know, the discussions about 80 character lines, 90 lines in
powerpc etc ... is for the code.
We still aim at keeping lines not longer than 75 chars in the commit log.
Christophe
Christophe
^ permalink raw reply
* Re: [RESEND][PATCH 0/7] Avoid overflow at boundary_size
From: Nicolin Chen @ 2020-09-01 7:54 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-ia64, James.Bottomley, paulus, hpa, sparclinux, sfr, deller,
x86, borntraeger, mingo, mattst88, fenghua.yu, gor, schnelle, hca,
ink, tglx, gerald.schaefer, rth, tony.luck, linux-parisc,
linux-s390, linux-kernel, linux-alpha, bp, linuxppc-dev, davem
In-Reply-To: <20200901073623.GA30418@lst.de>
Hi Christoph,
On Tue, Sep 01, 2020 at 09:36:23AM +0200, Christoph Hellwig wrote:
> I really don't like all the open coded smarts in the various drivers.
> What do you think about a helper like the one in the untested patch
A helper function will be actually better. I was thinking of
one yet not very sure about the naming and where to put it.
> below (on top of your series). Also please include the original
> segment boundary patch with the next resend so that the series has
> the full context.
I will use your change instead and resend with the ULONG_MAX
change. But in that case, should I make separate changes for
different files like this series, or just one single change
like yours?
Asking this as I was expecting that those changes would get
applied by different maintainers. But now it feels like you
will merge it to your tree at once?
Thanks
Nic
> diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
> index 1ef2c647bd3ec2..6f7de4f4e191e7 100644
> --- a/arch/alpha/kernel/pci_iommu.c
> +++ b/arch/alpha/kernel/pci_iommu.c
> @@ -141,10 +141,7 @@ iommu_arena_find_pages(struct device *dev, struct pci_iommu_arena *arena,
> unsigned long boundary_size;
>
> base = arena->dma_base >> PAGE_SHIFT;
> -
> - boundary_size = dev ? dma_get_seg_boundary(dev) : U32_MAX;
> - /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
> - boundary_size = (boundary_size >> PAGE_SHIFT) + 1;
> + boundary_size = dma_get_seg_boundary_nr_pages(dev, PAGE_SHIFT);
>
> /* Search forward for the first mask-aligned sequence of N free ptes */
> ptes = arena->ptes;
> diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
> index 945954903bb0ba..b49b73a95067d2 100644
> --- a/arch/ia64/hp/common/sba_iommu.c
> +++ b/arch/ia64/hp/common/sba_iommu.c
> @@ -485,8 +485,7 @@ sba_search_bitmap(struct ioc *ioc, struct device *dev,
> ASSERT(((unsigned long) ioc->res_hint & (sizeof(unsigned long) - 1UL)) == 0);
> ASSERT(res_ptr < res_end);
>
> - /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
> - boundary_size = (dma_get_seg_boundary(dev) >> iovp_shift) + 1;
> + boundary_size = dma_get_seg_boundary_nr_pages(dev, iovp_shift);
>
> BUG_ON(ioc->ibase & ~iovp_mask);
> shift = ioc->ibase >> iovp_shift;
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index c01ccbf8afdd42..cbc2e62db597cf 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -236,11 +236,7 @@ static unsigned long iommu_range_alloc(struct device *dev,
> }
> }
>
> - /* 4GB boundary for iseries_hv_alloc and iseries_hv_map */
> - boundary_size = dev ? dma_get_seg_boundary(dev) : U32_MAX;
> -
> - /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
> - boundary_size = (boundary_size >> tbl->it_page_shift) + 1;
> + boundary_size = dma_get_seg_boundary_nr_pages(dev, tbl->it_page_shift);
>
> n = iommu_area_alloc(tbl->it_map, limit, start, npages, tbl->it_offset,
> boundary_size, align_mask);
> diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
> index ecb067acc6d532..4a37d8f4de9d9d 100644
> --- a/arch/s390/pci/pci_dma.c
> +++ b/arch/s390/pci/pci_dma.c
> @@ -261,13 +261,11 @@ static unsigned long __dma_alloc_iommu(struct device *dev,
> unsigned long start, int size)
> {
> struct zpci_dev *zdev = to_zpci(to_pci_dev(dev));
> - unsigned long boundary_size;
>
> - /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
> - boundary_size = (dma_get_seg_boundary(dev) >> PAGE_SHIFT) + 1;
> return iommu_area_alloc(zdev->iommu_bitmap, zdev->iommu_pages,
> start, size, zdev->start_dma >> PAGE_SHIFT,
> - boundary_size, 0);
> + dma_get_seg_boundary_nr_pages(dev, PAGE_SHIFT),
> + 0);
> }
>
> static dma_addr_t dma_alloc_address(struct device *dev, int size)
> diff --git a/arch/sparc/kernel/iommu-common.c b/arch/sparc/kernel/iommu-common.c
> index 843e71894d0482..e6139c99762e11 100644
> --- a/arch/sparc/kernel/iommu-common.c
> +++ b/arch/sparc/kernel/iommu-common.c
> @@ -166,10 +166,6 @@ unsigned long iommu_tbl_range_alloc(struct device *dev,
> }
> }
>
> - boundary_size = dev ? dma_get_seg_boundary(dev) : U32_MAX;
> -
> - /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
> - boundary_size = (boundary_size >> iommu->table_shift) + 1;
> /*
> * if the skip_span_boundary_check had been set during init, we set
> * things up so that iommu_is_span_boundary() merely checks if the
> @@ -178,7 +174,11 @@ unsigned long iommu_tbl_range_alloc(struct device *dev,
> if ((iommu->flags & IOMMU_NO_SPAN_BOUND) != 0) {
> shift = 0;
> boundary_size = iommu->poolsize * iommu->nr_pools;
> + } else {
> + boundary_size = dma_get_seg_boundary_nr_pages(dev,
> + iommu->table_shift);
> }
> +
> n = iommu_area_alloc(iommu->map, limit, start, npages, shift,
> boundary_size, align_mask);
> if (n == -1) {
> diff --git a/arch/sparc/kernel/iommu.c b/arch/sparc/kernel/iommu.c
> index d981c37305ae31..c3e4e2df26a8b8 100644
> --- a/arch/sparc/kernel/iommu.c
> +++ b/arch/sparc/kernel/iommu.c
> @@ -472,8 +472,7 @@ static int dma_4u_map_sg(struct device *dev, struct scatterlist *sglist,
> outs->dma_length = 0;
>
> max_seg_size = dma_get_max_seg_size(dev);
> - /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
> - seg_boundary_size = (dma_get_seg_boundary(dev) >> IO_PAGE_SHIFT) + 1;
> + seg_boundary_size = dma_get_seg_boundary_nr_pages(dev, IO_PAGE_SHIFT);
> base_shift = iommu->tbl.table_map_base >> IO_PAGE_SHIFT;
> for_each_sg(sglist, s, nelems, i) {
> unsigned long paddr, npages, entry, out_entry = 0, slen;
> diff --git a/arch/sparc/kernel/pci_sun4v.c b/arch/sparc/kernel/pci_sun4v.c
> index 233fe8a20cec33..6b92dd51c0026f 100644
> --- a/arch/sparc/kernel/pci_sun4v.c
> +++ b/arch/sparc/kernel/pci_sun4v.c
> @@ -508,8 +508,7 @@ static int dma_4v_map_sg(struct device *dev, struct scatterlist *sglist,
> iommu_batch_start(dev, prot, ~0UL);
>
> max_seg_size = dma_get_max_seg_size(dev);
> - /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
> - seg_boundary_size = (dma_get_seg_boundary(dev) >> IO_PAGE_SHIFT) + 1;
> + seg_boundary_size = dma_get_seg_boundary_nr_pages(dev, IO_PAGE_SHIFT);
>
> mask = *dev->dma_mask;
> if (!iommu_use_atu(iommu, mask))
> diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c
> index 7fa0bb490065a1..bccc5357bffd6c 100644
> --- a/arch/x86/kernel/amd_gart_64.c
> +++ b/arch/x86/kernel/amd_gart_64.c
> @@ -96,8 +96,7 @@ static unsigned long alloc_iommu(struct device *dev, int size,
>
> base_index = ALIGN(iommu_bus_base & dma_get_seg_boundary(dev),
> PAGE_SIZE) >> PAGE_SHIFT;
> - /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
> - boundary_size = (dma_get_seg_boundary(dev) >> PAGE_SHIFT) + 1;
> + boundary_size = dma_get_seg_boundary_nr_pages(dev, PAGE_SHIFT);
>
> spin_lock_irqsave(&iommu_bitmap_lock, flags);
> offset = iommu_area_alloc(iommu_gart_bitmap, iommu_pages, next_bit,
> diff --git a/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c
> index c667d6aba7646e..ba16b7f8f80612 100644
> --- a/drivers/parisc/ccio-dma.c
> +++ b/drivers/parisc/ccio-dma.c
> @@ -356,8 +356,7 @@ ccio_alloc_range(struct ioc *ioc, struct device *dev, size_t size)
> ** ggg sacrifices another 710 to the computer gods.
> */
>
> - /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
> - boundary_size = (dma_get_seg_boundary(dev) >> IOVP_SHIFT) + 1;
> + boundary_size = dma_get_seg_boundary_nr_pages(dev, IOVP_SHIFT);
>
> if (pages_needed <= 8) {
> /*
> diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c
> index 96bc2c617cbd19..959bda193b9603 100644
> --- a/drivers/parisc/sba_iommu.c
> +++ b/drivers/parisc/sba_iommu.c
> @@ -342,8 +342,7 @@ sba_search_bitmap(struct ioc *ioc, struct device *dev,
> unsigned long shift;
> int ret;
>
> - /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
> - boundary_size = (dma_get_seg_boundary(dev) >> IOVP_SHIFT) + 1;
> + boundary_size = dma_get_seg_boundary_nr_pages(dev, IOVP_SHIFT);
>
> #if defined(ZX1_SUPPORT)
> BUG_ON(ioc->ibase & ~IOVP_MASK);
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 52635e91143b25..7477a164500adb 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -632,6 +632,25 @@ static inline unsigned long dma_get_seg_boundary(struct device *dev)
> return DMA_BIT_MASK(32);
> }
>
> +/**
> + * dma_get_seg_boundary_nr_pages - return the segment boundary in "page" units
> + * @dev: device to guery the boundary for
> + * @page_shift: ilog() of the the IOMMU page size
> + *
> + * Return the segment boundary in IOMMU page units (which may be different from
> + * the CPU page size) for the passed in device.
> + *
> + * If @dev is NULL a boundary of U32_MAX is assumed, this case is just for
> + * non-DMA API callers.
> + */
> +static inline unsigned long dma_get_seg_boundary_nr_pages(struct device *dev,
> + unsigned int page_shift)
> +{
> + if (!dev)
> + return (U32_MAX >> page_shift) + 1;
> + return (dma_get_seg_boundary(dev) >> page_shift) + 1;
> +}
> +
> static inline int dma_set_seg_boundary(struct device *dev, unsigned long mask)
> {
> if (dev->dma_parms) {
^ permalink raw reply
* Re: [PATCH v3 03/13] mm/debug_vm_pgtable/ppc64: Avoid setting top bits in radom value
From: Aneesh Kumar K.V @ 2020-09-01 7:55 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: <c371e316-7533-62c7-a1c6-9b6745d8d1ea@arm.com>
On 9/1/20 1:16 PM, Anshuman Khandual wrote:
>
>
> On 09/01/2020 01:06 PM, Aneesh Kumar K.V wrote:
>> On 9/1/20 1:02 PM, Anshuman Khandual wrote:
>>>
>>>
>>> On 09/01/2020 11:51 AM, Aneesh Kumar K.V wrote:
>>>> On 9/1/20 8:45 AM, Anshuman Khandual wrote:
>>>>>
>>>>>
>>>>> On 08/27/2020 01:34 PM, Aneesh Kumar K.V wrote:
>>>>>> ppc64 use bit 62 to indicate a pte entry (_PAGE_PTE). Avoid setting that bit in
>>>>>> random value.
>>>>>>
>>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>>>>> ---
>>>>>> mm/debug_vm_pgtable.c | 13 ++++++++++---
>>>>>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>>>>>> index 086309fb9b6f..bbf9df0e64c6 100644
>>>>>> --- a/mm/debug_vm_pgtable.c
>>>>>> +++ b/mm/debug_vm_pgtable.c
>>>>>> @@ -44,10 +44,17 @@
>>>>>> * entry type. But these bits might affect the ability to clear entries with
>>>>>> * pxx_clear() because of how dynamic page table folding works on s390. So
>>>>>> * while loading up the entries do not change the lower 4 bits. It does not
>>>>>> - * have affect any other platform.
>>>>>> + * have affect any other platform. Also avoid the 62nd bit on ppc64 that is
>>>>>> + * used to mark a pte entry.
>>>>>> */
>>>>>> -#define S390_MASK_BITS 4
>>>>>> -#define RANDOM_ORVALUE GENMASK(BITS_PER_LONG - 1, S390_MASK_BITS)
>>>>>> +#define S390_SKIP_MASK GENMASK(3, 0)
>>>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>>>> +#define PPC64_SKIP_MASK GENMASK(62, 62)
>>>>>> +#else
>>>>>> +#define PPC64_SKIP_MASK 0x0
>>>>>> +#endif
>>>>>
>>>>> Please drop the #ifdef CONFIG_PPC_BOOK3S_64 here. We already accommodate skip
>>>>> bits for a s390 platform requirement and can also do so for ppc64 as well. As
>>>>> mentioned before, please avoid adding any platform specific constructs in the
>>>>> test.
>>>>>
>>>>
>>>>
>>>> that is needed so that it can be built on 32 bit architectures.I did face build errors with arch-linux
>>>
>>> Could not (#if __BITS_PER_LONG == 32) be used instead or something like
>>> that. But should be a generic conditional check identifying 32 bit arch
>>> not anything platform specific.
>>>
>>
>> that _PAGE_PTE bit is pretty much specific to PPC BOOK3S_64. Not sure why other architectures need to bothered about ignoring bit 62.
>
> Thats okay as long it does not adversely affect other architectures, ignoring
> some more bits is acceptable. Like existing S390_MASK_BITS gets ignored on all
> other platforms even if it is a s390 specific constraint. Not having platform
> specific #ifdef here, is essential.
>
Why is it essential?
-aneesh
^ permalink raw reply
* Re: [PATCH v3 12/13] mm/debug_vm_pgtable/hugetlb: Disable hugetlb test on ppc64
From: Christophe Leroy @ 2020-09-01 7:59 UTC (permalink / raw)
To: Aneesh Kumar K.V, Anshuman Khandual, linux-mm, akpm
Cc: linux-arch, linux-s390, Christophe Leroy, x86, Mike Rapoport,
Erhard F., Qian Cai, Gerald Schaefer, Vineet Gupta,
linux-snps-arc, linuxppc-dev, linux-arm-kernel
In-Reply-To: <68f90b44-b830-58be-3c21-424fee05da37@linux.ibm.com>
Le 01/09/2020 à 08:30, Aneesh Kumar K.V a écrit :
>>
>
> I actually wanted to add #ifdef BROKEN. That test is completely broken.
> Infact I would suggest to remove that test completely.
>
>
>
>> #ifdef will not be required here as there would be a stub definition
>> for hugetlb_advanced_tests() when CONFIG_PPC_BOOK3S_64 is enabled.
>>
>>> spin_lock(&mm->page_table_lock);
>>> p4d_clear_tests(mm, p4dp);
>>>
>>
>> But again, we should really try and avoid taking this path.
>>
>
> To be frank i am kind of frustrated with how this patch series is being
> looked at. We pushed a completely broken test to upstream and right now
> we have a code in upstream that crash when booted on ppc64. My attempt
> has been to make progress here and you definitely seems to be not in
> agreement to that.
>
> At this point I am tempted to suggest we remove the DEBUG_VM_PGTABLE
> support on ppc64 because AFAIU it doesn't add any value.
>
Note that a bug has been filed at
https://bugzilla.kernel.org/show_bug.cgi?id=209029
Christophe
^ permalink raw reply
* [PATCH] powerpc/mm: Remove DEBUG_VM_PGTABLE support on ppc64
From: Aneesh Kumar K.V @ 2020-09-01 8:02 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 untill we get the tests updated.
[ 21.083506] ------------[ cut here ]------------
[ 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!
Linux version 5.9.0-rc2-34902-g4da73871507c (kvaneesh@ltczz75-lp2) (gcc (Ubuntu 9.3.0-10ubuntu2) 9.3.0, GNU ld (GNU Binutils for Ubuntu) 2.34) #301 SMP Tue Sep 1 02:28:29 CDT 2020
enter ? for help
[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
Linux version 5.9.0-rc2-34902-g4da73871507c (kvaneesh@ltczz75-lp2) (gcc (Ubuntu 9.3.0-10ubuntu2) 9.3.0, GNU ld (GNU Binutils for Ubuntu) 2.34) #302 SMP Tue Sep 1 02:56:02 CDT 2020
enter ? for help
[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 v2 00/13] mm/debug_vm_pgtable fixes
From: Christophe Leroy @ 2020-09-01 8:03 UTC (permalink / raw)
To: Anshuman Khandual, 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: <52e9743e-fa2f-3fd2-f50e-2c6c38464b96@arm.com>
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>
> + 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 8:10 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: Anshuman Khandual
In-Reply-To: <20200901080245.67950-1-aneesh.kumar@linux.ibm.com>
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:
select ARCH_HAS_DEBUG_VM_PGTABLE if PPC32
> select ARCH_HAS_DEVMEM_IS_ALLOWED
> select ARCH_HAS_ELF_RANDOMIZE
> select ARCH_HAS_FORTIFY_SOURCE
>
What about Documentation/features/debug/debug-vm-pgtable/arch-support.txt ?
Christophe
^ permalink raw reply
* Re: [PATCH] powerpc/mm: Remove DEBUG_VM_PGTABLE support on ppc64
From: Aneesh Kumar K.V @ 2020-09-01 8:12 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev, mpe; +Cc: Anshuman Khandual
In-Reply-To: <4519baaa-cb95-6ebb-200f-4520badb56f6@csgroup.eu>
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.
>
> select ARCH_HAS_DEBUG_VM_PGTABLE if PPC32
>
>> select ARCH_HAS_DEVMEM_IS_ALLOWED
>> select ARCH_HAS_ELF_RANDOMIZE
>> select ARCH_HAS_FORTIFY_SOURCE
>>
>
> What about Documentation/features/debug/debug-vm-pgtable/arch-support.txt ?
>
I am hoping we can enable the config once we resolve the test issues.
may be in next merge window.
-aneesh
^ permalink raw reply
* Re: [PATCH] powerpc/mm: Remove DEBUG_VM_PGTABLE support on ppc64
From: Christophe Leroy @ 2020-09-01 8:15 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: Anshuman Khandual
In-Reply-To: <39467f79-d213-772d-9ed1-93afedc2fc7a@linux.ibm.com>
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).
Christophe
>
>>
>> select ARCH_HAS_DEBUG_VM_PGTABLE if PPC32
>>
>>> select ARCH_HAS_DEVMEM_IS_ALLOWED
>>> select ARCH_HAS_ELF_RANDOMIZE
>>> select ARCH_HAS_FORTIFY_SOURCE
>>>
>>
>> What about
>> Documentation/features/debug/debug-vm-pgtable/arch-support.txt ?
>>
>
> I am hoping we can enable the config once we resolve the test issues.
> may be in next merge window.
>
> -aneesh
>
>
>
^ permalink raw reply
* Re: [RESEND][PATCH 0/7] Avoid overflow at boundary_size
From: Christoph Hellwig @ 2020-09-01 9:11 UTC (permalink / raw)
To: Nicolin Chen
Cc: linux-ia64, James.Bottomley, paulus, hpa, sparclinux,
Christoph Hellwig, sfr, deller, x86, borntraeger, mingo, mattst88,
fenghua.yu, gor, schnelle, hca, ink, tglx, gerald.schaefer, rth,
tony.luck, linux-parisc, linux-s390, linux-kernel, linux-alpha,
bp, linuxppc-dev, davem
In-Reply-To: <20200901075401.GA5667@Asurada-Nvidia>
On Tue, Sep 01, 2020 at 12:54:01AM -0700, Nicolin Chen wrote:
> Hi Christoph,
>
> On Tue, Sep 01, 2020 at 09:36:23AM +0200, Christoph Hellwig wrote:
> > I really don't like all the open coded smarts in the various drivers.
> > What do you think about a helper like the one in the untested patch
>
> A helper function will be actually better. I was thinking of
> one yet not very sure about the naming and where to put it.
>
> > below (on top of your series). Also please include the original
> > segment boundary patch with the next resend so that the series has
> > the full context.
>
> I will use your change instead and resend with the ULONG_MAX
> change. But in that case, should I make separate changes for
> different files like this series, or just one single change
> like yours?
>
> Asking this as I was expecting that those changes would get
> applied by different maintainers. But now it feels like you
> will merge it to your tree at once?
I guess one patch is fine. I can queue it up in the dma-mapping
tree as a prep patch for the default boundary change.
^ 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
* 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: 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] 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 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 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
* [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 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
* 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 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 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: 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
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