Re: [PATCH 00/13] fs/dax: Fix FS DAX page reference counts

2024-06-27 Thread Alistair Popple


Dan Williams  writes:

> Alistair Popple wrote:
>> FS DAX pages have always maintained their own page reference counts
>> without following the normal rules for page reference counting. In
>> particular pages are considered free when the refcount hits one rather
>> than zero and refcounts are not added when mapping the page.
>> 
>> Tracking this requires special PTE bits (PTE_DEVMAP) and a secondary
>> mechanism for allowing GUP to hold references on the page (see
>> get_dev_pagemap). However there doesn't seem to be any reason why FS
>> DAX pages need their own reference counting scheme.
>> 
>> By treating the refcounts on these pages the same way as normal pages
>> we can remove a lot of special checks. In particular pXd_trans_huge()
>> becomes the same as pXd_leaf(), although I haven't made that change
>> here. It also frees up a valuable SW define PTE bit on architectures
>> that have devmap PTE bits defined.
>> 
>> It also almost certainly allows further clean-up of the devmap managed
>> functions, but I have left that as a future improvment.
>> 
>> This is an update to the original RFC rebased onto v6.10-rc5. Unlike
>> the original RFC it passes the same number of ndctl test suite
>> (https://github.com/pmem/ndctl) tests as my current development
>> environment does without these patches.
>
> Are you seeing the 'mmap.sh' test fail even without these patches?

No. But I also don't see it failing with these patches :)

For reference this is what I see on my test machine with or without:

[1/70] Generating version.h with a custom command
 1/13 ndctl:dax / daxdev-errors.sh  SKIP 0.06s   exit 
status 77
 2/13 ndctl:dax / multi-dax.sh  SKIP 0.05s   exit 
status 77
 3/13 ndctl:dax / sub-section.shSKIP 0.14s   exit 
status 77
 4/13 ndctl:dax / dax-dev   OK   0.02s
 5/13 ndctl:dax / dax-ext4.sh   OK  12.97s
 6/13 ndctl:dax / dax-xfs.shOK  12.44s
 7/13 ndctl:dax / device-daxOK  13.40s
 8/13 ndctl:dax / revoke-devmem FAIL 0.31s   (exit 
status 250 or signal 122 SIGinvalid)
>>> TEST_PATH=/home/apopple/ndctl/build/test 
>>> LD_LIBRARY_PATH=/home/apopple/ndctl/build/cxl/lib:/home/apopple/ndctl/build/daxctl/lib:/home/apopple/ndctl/build/ndctl/lib
>>>  NDCTL=/home/apopple/ndctl/build/ndctl/ndctl MALLOC_PERTURB_=227 
>>> DATA_PATH=/home/apopple/ndctl/test 
>>> DAXCTL=/home/apopple/ndctl/build/daxctl/daxctl 
>>> /home/apopple/ndctl/build/test/revoke_devmem

 9/13 ndctl:dax / device-dax-fio.sh OK  32.43s
10/13 ndctl:dax / daxctl-devices.sh SKIP 0.07s   exit 
status 77
11/13 ndctl:dax / daxctl-create.sh  SKIP 0.04s   exit 
status 77
12/13 ndctl:dax / dm.sh FAIL 0.08s   exit 
status 1
>>> MALLOC_PERTURB_=209 TEST_PATH=/home/apopple/ndctl/build/test 
>>> LD_LIBRARY_PATH=/home/apopple/ndctl/build/cxl/lib:/home/apopple/ndctl/build/daxctl/lib:/home/apopple/ndctl/build/ndctl/lib
>>>  NDCTL=/home/apopple/ndctl/build/ndctl/ndctl 
>>> DATA_PATH=/home/apopple/ndctl/test 
>>> DAXCTL=/home/apopple/ndctl/build/daxctl/daxctl 
>>> /home/apopple/ndctl/test/dm.sh

13/13 ndctl:dax / mmap.sh   OK 107.57s

Ok: 6   
Expected Fail:  0   
Fail:   2   
Unexpected Pass:0   
Skipped:5   
Timeout:0   

I have been using QEMU for my testing. Maybe I missed some condition in
the unmap path though so will take another look.

> I see this with the patches, will try without in the morning.
>
>  EXT4-fs (pmem0): unmounting filesystem 26ea1463-343a-464f-9f16-91cb176dbdc7.
>  XFS (pmem0): Mounting V5 Filesystem 554953fd-c9f4-460f-bc37-f43979986b68
>  XFS (pmem0): Ending clean mount
>  Oops: general protection fault, probably for non-canonical address 
> 0xdead0518: 00
> T SMP PTI
>  CPU: 15 PID: 1295 Comm: mmap Tainted: G   OEN 6.10.0-rc5+ #261
>  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
> edk2-20240524-3.fc40 05/24/2024
>  RIP: 0010:folio_mark_dirty+0x25/0x60
>  Code: 90 90 90 90 90 0f 1f 44 00 00 53 48 89 fb e8 22 18 02 00 48 85 c0 74 
> 26 48 89 c7 48
> 0 02 00 74 05 f0 80 63 02 fd <48> 8b 87 18 01 00 00 48 89 de 5b 48 8b 40 18 
> e9 77 90 c0 00
>  RSP: 0018:b073022f7b08 EFLAGS: 00010246
>  RAX: 00482000 RBX: d0d005000300 RCX: 0440
>  RDX:  RSI: 7f400620 RDI: dead0400
>  RBP:  R08: 9a4b04504a30 R09: 000f
>  R10: d0d005000300 R11:  R12: 7f400620
>  R13: 9a4b7c96c000 R14: 9a4b7daba440 R15: b073022f7cb0
>  FS:  7f4046351740() GS:9a4d7778() knlGS:
>  CS:  0010 DS:  ES:  CR0: 80050033
>  CR2: 7f40461ff000 CR3: 00027aea6000 CR4: 06f0
>  DR0: 

Re: [PATCH 06/13] mm/memory: Add dax_insert_pfn

2024-06-27 Thread Jan Kara
On Thu 27-06-24 10:54:21, Alistair Popple wrote:
> Currently to map a DAX page the DAX driver calls vmf_insert_pfn. This
> creates a special devmap PTE entry for the pfn but does not take a
> reference on the underlying struct page for the mapping. This is
> because DAX page refcounts are treated specially, as indicated by the
> presence of a devmap entry.
> 
> To allow DAX page refcounts to be managed the same as normal page
> refcounts introduce dax_insert_pfn. This will take a reference on the
> underlying page much the same as vmf_insert_page, except it also
> permits upgrading an existing mapping to be writable if
> requested/possible.
> 
> Signed-off-by: Alistair Popple 

Overall this looks good to me. Some comments below.

> ---
>  include/linux/mm.h |  4 ++-
>  mm/memory.c| 79 ++-
>  2 files changed, 76 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 9a5652c..b84368b 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1080,6 +1080,8 @@ int vma_is_stack_for_current(struct vm_area_struct 
> *vma);
>  struct mmu_gather;
>  struct inode;
>  
> +extern void prep_compound_page(struct page *page, unsigned int order);
> +

You don't seem to use this function in this patch?

> diff --git a/mm/memory.c b/mm/memory.c
> index ce48a05..4f26a1f 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1989,14 +1989,42 @@ static int validate_page_before_insert(struct page 
> *page)
>  }
>  
>  static int insert_page_into_pte_locked(struct vm_area_struct *vma, pte_t 
> *pte,
> - unsigned long addr, struct page *page, pgprot_t prot)
> + unsigned long addr, struct page *page, pgprot_t prot, 
> bool mkwrite)
>  {
>   struct folio *folio = page_folio(page);
> + pte_t entry = ptep_get(pte);
>  
> - if (!pte_none(ptep_get(pte)))
> + if (!pte_none(entry)) {
> + if (mkwrite) {
> + /*
> +  * For read faults on private mappings the PFN passed
> +  * in may not match the PFN we have mapped if the
> +  * mapped PFN is a writeable COW page.  In the mkwrite
> +  * case we are creating a writable PTE for a shared
> +  * mapping and we expect the PFNs to match. If they
> +  * don't match, we are likely racing with block
> +  * allocation and mapping invalidation so just skip the
> +  * update.
> +  */
> + if (pte_pfn(entry) != page_to_pfn(page)) {
> + WARN_ON_ONCE(!is_zero_pfn(pte_pfn(entry)));
> + return -EFAULT;
> + }
> + entry = maybe_mkwrite(entry, vma);
> + entry = pte_mkyoung(entry);
> + if (ptep_set_access_flags(vma, addr, pte, entry, 1))
> + update_mmu_cache(vma, addr, pte);
> + return 0;
> + }
>   return -EBUSY;

If you do this like:

if (!mkwrite)
return -EBUSY;

You can reduce indentation of the big block and also making the flow more
obvious...

> + }
> +
>   /* Ok, finally just insert the thing.. */
>   folio_get(folio);
> + if (mkwrite)
> + entry = maybe_mkwrite(mk_pte(page, prot), vma);
> + else
> + entry = mk_pte(page, prot);

I'd prefer:

entry = mk_pte(page, prot);
if (mkwrite)
entry = maybe_mkwrite(entry, vma);

but I don't insist. Also insert_pfn() additionally has pte_mkyoung() and
pte_mkdirty(). Why was it left out here?

Honza
-- 
Jan Kara 
SUSE Labs, CR



Re: [PATCH 00/13] fs/dax: Fix FS DAX page reference counts

2024-06-27 Thread Dan Williams
Alistair Popple wrote:
> 
> Dan Williams  writes:
> 
> > Alistair Popple wrote:
> >> FS DAX pages have always maintained their own page reference counts
> >> without following the normal rules for page reference counting. In
> >> particular pages are considered free when the refcount hits one rather
> >> than zero and refcounts are not added when mapping the page.
> >> 
> >> Tracking this requires special PTE bits (PTE_DEVMAP) and a secondary
> >> mechanism for allowing GUP to hold references on the page (see
> >> get_dev_pagemap). However there doesn't seem to be any reason why FS
> >> DAX pages need their own reference counting scheme.
> >> 
> >> By treating the refcounts on these pages the same way as normal pages
> >> we can remove a lot of special checks. In particular pXd_trans_huge()
> >> becomes the same as pXd_leaf(), although I haven't made that change
> >> here. It also frees up a valuable SW define PTE bit on architectures
> >> that have devmap PTE bits defined.
> >> 
> >> It also almost certainly allows further clean-up of the devmap managed
> >> functions, but I have left that as a future improvment.
> >> 
> >> This is an update to the original RFC rebased onto v6.10-rc5. Unlike
> >> the original RFC it passes the same number of ndctl test suite
> >> (https://github.com/pmem/ndctl) tests as my current development
> >> environment does without these patches.
> >
> > Are you seeing the 'mmap.sh' test fail even without these patches?
> 
> No. But I also don't see it failing with these patches :)
> 
> For reference this is what I see on my test machine with or without:
> 
> [1/70] Generating version.h with a custom command
>  1/13 ndctl:dax / daxdev-errors.sh  SKIP 0.06s   exit 
> status 77
>  2/13 ndctl:dax / multi-dax.sh  SKIP 0.05s   exit 
> status 77
>  3/13 ndctl:dax / sub-section.shSKIP 0.14s   exit 
> status 77

I really need to get this test built as a service as this shows a
pre-req is missing, and it's not quite fair to expect submitters to put
it all together.

>  4/13 ndctl:dax / dax-dev   OK   0.02s
>  5/13 ndctl:dax / dax-ext4.sh   OK  12.97s
>  6/13 ndctl:dax / dax-xfs.shOK  12.44s
>  7/13 ndctl:dax / device-daxOK  13.40s
>  8/13 ndctl:dax / revoke-devmem FAIL 0.31s   (exit 
> status 250 or signal 122 SIGinvalid)
> >>> TEST_PATH=/home/apopple/ndctl/build/test 
> >>> LD_LIBRARY_PATH=/home/apopple/ndctl/build/cxl/lib:/home/apopple/ndctl/build/daxctl/lib:/home/apopple/ndctl/build/ndctl/lib
> >>>  NDCTL=/home/apopple/ndctl/build/ndctl/ndctl MALLOC_PERTURB_=227 
> >>> DATA_PATH=/home/apopple/ndctl/test 
> >>> DAXCTL=/home/apopple/ndctl/build/daxctl/daxctl 
> >>> /home/apopple/ndctl/build/test/revoke_devmem
> 
>  9/13 ndctl:dax / device-dax-fio.sh OK  32.43s
> 10/13 ndctl:dax / daxctl-devices.sh SKIP 0.07s   exit 
> status 77
> 11/13 ndctl:dax / daxctl-create.sh  SKIP 0.04s   exit 
> status 77
> 12/13 ndctl:dax / dm.sh FAIL 0.08s   exit 
> status 1
> >>> MALLOC_PERTURB_=209 TEST_PATH=/home/apopple/ndctl/build/test 
> >>> LD_LIBRARY_PATH=/home/apopple/ndctl/build/cxl/lib:/home/apopple/ndctl/build/daxctl/lib:/home/apopple/ndctl/build/ndctl/lib
> >>>  NDCTL=/home/apopple/ndctl/build/ndctl/ndctl 
> >>> DATA_PATH=/home/apopple/ndctl/test 
> >>> DAXCTL=/home/apopple/ndctl/build/daxctl/daxctl 
> >>> /home/apopple/ndctl/test/dm.sh
> 
> 13/13 ndctl:dax / mmap.sh   OK 107.57s

I need to think through why this one might false succeed, but that can
wait until we get this series reviewed. For now my failure is stable
which allows it to be bisected.

> 
> Ok: 6   
> Expected Fail:  0   
> Fail:   2   
> Unexpected Pass:0   
> Skipped:5   
> Timeout:0   
> 
> I have been using QEMU for my testing. Maybe I missed some condition in
> the unmap path though so will take another look.

I was able to bisect to:

[PATCH 10/13] fs/dax: Properly refcount fs dax pages

...I will prioritize that one in my review queue.



Re: [PATCH 07/13] huge_memory: Allow mappings of PUD sized pages

2024-06-27 Thread kernel test robot
Hi Alistair,

kernel test robot noticed the following build errors:

[auto build test ERROR on f2661062f16b2de5d7b6a5c42a9a5c96326b8454]

url:
https://github.com/intel-lab-lkp/linux/commits/Alistair-Popple/mm-gup-c-Remove-redundant-check-for-PCI-P2PDMA-page/20240627-191709
base:   f2661062f16b2de5d7b6a5c42a9a5c96326b8454
patch link:
https://lore.kernel.org/r/bd332b0d3971b03152b3541f97470817c5147b51.1719386613.git-series.apopple%40nvidia.com
patch subject: [PATCH 07/13] huge_memory: Allow mappings of PUD sized pages
config: x86_64-allnoconfig 
(https://download.01.org/0day-ci/archive/20240628/202406280637.147dyrrv-...@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 
617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240628/202406280637.147dyrrv-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202406280637.147dyrrv-...@intel.com/

All errors (new ones prefixed by >>):

>> mm/rmap.c:1513:37: error: call to '__compiletime_assert_279' declared with 
>> 'error' attribute: BUILD_BUG failed
1513 | __folio_add_file_rmap(folio, page, HPAGE_PUD_NR, vma, 
RMAP_LEVEL_PUD);
 |^
   include/linux/huge_mm.h:111:26: note: expanded from macro 'HPAGE_PUD_NR'
 111 | #define HPAGE_PUD_NR (1<:72:1: note: expanded from here
  72 | __compiletime_assert_279
 | ^
   mm/rmap.c:1660:35: error: call to '__compiletime_assert_280' declared with 
'error' attribute: BUILD_BUG failed
1660 | __folio_remove_rmap(folio, page, HPAGE_PUD_NR, vma, 
RMAP_LEVEL_PUD);
 |  ^
   include/linux/huge_mm.h:111:26: note: expanded from macro 'HPAGE_PUD_NR'
 111 | #define HPAGE_PUD_NR (1<:79:1: note: expanded from here
  79 | __compiletime_assert_280
 | ^
   2 errors generated.


vim +1513 mm/rmap.c

  1498  
  1499  /**
  1500   * folio_add_file_rmap_pud - add a PUD mapping to a page range of a 
folio
  1501   * @folio:  The folio to add the mapping to
  1502   * @page:   The first page to add
  1503   * @vma:The vm area in which the mapping is added
  1504   *
  1505   * The page range of the folio is defined by [page, page + HPAGE_PUD_NR)
  1506   *
  1507   * The caller needs to hold the page table lock.
  1508   */
  1509  void folio_add_file_rmap_pud(struct folio *folio, struct page *page,
  1510  struct vm_area_struct *vma)
  1511  {
  1512  #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
> 1513  __folio_add_file_rmap(folio, page, HPAGE_PUD_NR, vma, 
> RMAP_LEVEL_PUD);
  1514  #else
  1515  WARN_ON_ONCE(true);
  1516  #endif
  1517  }
  1518  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki



Re: [PATCH 13/13] mm: Remove devmap related functions and page table bits

2024-06-27 Thread kernel test robot
Hi Alistair,

kernel test robot noticed the following build errors:

[auto build test ERROR on f2661062f16b2de5d7b6a5c42a9a5c96326b8454]

url:
https://github.com/intel-lab-lkp/linux/commits/Alistair-Popple/mm-gup-c-Remove-redundant-check-for-PCI-P2PDMA-page/20240627-191709
base:   f2661062f16b2de5d7b6a5c42a9a5c96326b8454
patch link:
https://lore.kernel.org/r/47c26640cd85f3db2e0a2796047199bb984d1b3f.1719386613.git-series.apopple%40nvidia.com
patch subject: [PATCH 13/13] mm: Remove devmap related functions and page table 
bits
config: powerpc-allmodconfig 
(https://download.01.org/0day-ci/archive/20240628/202406280658.1pp5cw2f-...@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240628/202406280658.1pp5cw2f-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202406280658.1pp5cw2f-...@intel.com/

All errors (new ones prefixed by >>):

   In file included from arch/powerpc/include/asm/book3s/64/mmu-hash.h:20,
from arch/powerpc/include/asm/book3s/64/mmu.h:32,
from arch/powerpc/include/asm/mmu.h:385,
from arch/powerpc/include/asm/paca.h:18,
from arch/powerpc/include/asm/current.h:13,
from include/linux/thread_info.h:23,
from include/asm-generic/preempt.h:5,
from ./arch/powerpc/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:79,
from include/linux/alloc_tag.h:11,
from include/linux/rhashtable-types.h:12,
from include/linux/ipc.h:7,
from include/uapi/linux/sem.h:5,
from include/linux/sem.h:5,
from include/linux/compat.h:14,
from arch/powerpc/kernel/asm-offsets.c:12:
>> arch/powerpc/include/asm/book3s/64/pgtable.h:1371:1: error: expected 
>> identifier or '(' before '}' token
1371 | }
 | ^
   make[3]: *** [scripts/Makefile.build:117: arch/powerpc/kernel/asm-offsets.s] 
Error 1
   make[3]: Target 'prepare' not remade because of errors.
   make[2]: *** [Makefile:1208: prepare0] Error 2
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:240: __sub-make] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:240: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.


vim +1371 arch/powerpc/include/asm/book3s/64/pgtable.h

953c66c2b22a30 Aneesh Kumar K.V  2016-12-12  1370  
ebd31197931d75 Oliver O'Halloran 2017-06-28 @1371  }
6a1ea36260f69f Aneesh Kumar K.V  2016-04-29  1372  #endif /* 
CONFIG_TRANSPARENT_HUGEPAGE */
ebd31197931d75 Oliver O'Halloran 2017-06-28  1373  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki



Re: [PATCH 04/13] fs/dax: Add dax_page_free callback

2024-06-27 Thread Alistair Popple


Christoph Hellwig  writes:

> On Thu, Jun 27, 2024 at 10:54:19AM +1000, Alistair Popple wrote:
>> When a fs dax page is freed it has to notify filesystems that the page
>> has been unpinned/unmapped and is free. Currently this involves
>> special code in the page free paths to detect a transition of refcount
>> from 2 to 1 and to call some fs dax specific code.
>> 
>> A future change will require this to happen when the page refcount
>> drops to zero. In this case we can use the existing
>> pgmap->ops->page_free() callback so wire that up for all devices that
>> support FS DAX (nvdimm and virtio).
>
> Given that ->page_ffree is only called from free_zone_device_folio
> and right next to a switch on the the type, can't we just do the
> wake_up_var there without the somewhat confusing indirect call that
> just back in common code without any driver logic?

Longer term I'm hoping we can get rid of that switch on type entirely as
I don't think the whole get/put_dev_pagemap() thing is very useful. Less
indirection is good though so will move the wake_up_var there.



Re: [PATCH 00/13] fs/dax: Fix FS DAX page reference counts

2024-06-27 Thread Alistair Popple


Dan Williams  writes:

> Alistair Popple wrote:
>> 
>> Dan Williams  writes:
>> 
>> > Alistair Popple wrote:
>> >> FS DAX pages have always maintained their own page reference counts
>> >> without following the normal rules for page reference counting. In
>> >> particular pages are considered free when the refcount hits one rather
>> >> than zero and refcounts are not added when mapping the page.
>> >> 
>> >> Tracking this requires special PTE bits (PTE_DEVMAP) and a secondary
>> >> mechanism for allowing GUP to hold references on the page (see
>> >> get_dev_pagemap). However there doesn't seem to be any reason why FS
>> >> DAX pages need their own reference counting scheme.
>> >> 
>> >> By treating the refcounts on these pages the same way as normal pages
>> >> we can remove a lot of special checks. In particular pXd_trans_huge()
>> >> becomes the same as pXd_leaf(), although I haven't made that change
>> >> here. It also frees up a valuable SW define PTE bit on architectures
>> >> that have devmap PTE bits defined.
>> >> 
>> >> It also almost certainly allows further clean-up of the devmap managed
>> >> functions, but I have left that as a future improvment.
>> >> 
>> >> This is an update to the original RFC rebased onto v6.10-rc5. Unlike
>> >> the original RFC it passes the same number of ndctl test suite
>> >> (https://github.com/pmem/ndctl) tests as my current development
>> >> environment does without these patches.
>> >
>> > Are you seeing the 'mmap.sh' test fail even without these patches?
>> 
>> No. But I also don't see it failing with these patches :)
>> 
>> For reference this is what I see on my test machine with or without:
>> 
>> [1/70] Generating version.h with a custom command
>>  1/13 ndctl:dax / daxdev-errors.sh  SKIP 0.06s   exit 
>> status 77
>>  2/13 ndctl:dax / multi-dax.sh  SKIP 0.05s   exit 
>> status 77
>>  3/13 ndctl:dax / sub-section.shSKIP 0.14s   exit 
>> status 77
>
> I really need to get this test built as a service as this shows a
> pre-req is missing, and it's not quite fair to expect submitters to put
> it all together.

Ok. I didn't dig into why this was being skipped but I might if I find
some time. The rest of the tests seemed more relevant anyway and turned
up enough bugs with my initial implementation to keep me busy which gave
me some confidence.

If I'm being honest though I found the whole test setup a bit of a
pain. In particular remembering you have to manually (re)build the
special test versions of the modules tripped me up a few times until I
updated my build scripts. But I got there in the end.

>>  4/13 ndctl:dax / dax-dev   OK   0.02s
>>  5/13 ndctl:dax / dax-ext4.sh   OK  12.97s
>>  6/13 ndctl:dax / dax-xfs.shOK  12.44s
>>  7/13 ndctl:dax / device-daxOK  13.40s
>>  8/13 ndctl:dax / revoke-devmem FAIL 0.31s   (exit 
>> status 250 or signal 122 SIGinvalid)
>> >>> TEST_PATH=/home/apopple/ndctl/build/test 
>> >>> LD_LIBRARY_PATH=/home/apopple/ndctl/build/cxl/lib:/home/apopple/ndctl/build/daxctl/lib:/home/apopple/ndctl/build/ndctl/lib
>> >>>  NDCTL=/home/apopple/ndctl/build/ndctl/ndctl MALLOC_PERTURB_=227 
>> >>> DATA_PATH=/home/apopple/ndctl/test 
>> >>> DAXCTL=/home/apopple/ndctl/build/daxctl/daxctl 
>> >>> /home/apopple/ndctl/build/test/revoke_devmem
>> 
>>  9/13 ndctl:dax / device-dax-fio.sh OK  32.43s
>> 10/13 ndctl:dax / daxctl-devices.sh SKIP 0.07s   exit 
>> status 77
>> 11/13 ndctl:dax / daxctl-create.sh  SKIP 0.04s   exit 
>> status 77
>> 12/13 ndctl:dax / dm.sh FAIL 0.08s   exit 
>> status 1
>> >>> MALLOC_PERTURB_=209 TEST_PATH=/home/apopple/ndctl/build/test 
>> >>> LD_LIBRARY_PATH=/home/apopple/ndctl/build/cxl/lib:/home/apopple/ndctl/build/daxctl/lib:/home/apopple/ndctl/build/ndctl/lib
>> >>>  NDCTL=/home/apopple/ndctl/build/ndctl/ndctl 
>> >>> DATA_PATH=/home/apopple/ndctl/test 
>> >>> DAXCTL=/home/apopple/ndctl/build/daxctl/daxctl 
>> >>> /home/apopple/ndctl/test/dm.sh
>> 
>> 13/13 ndctl:dax / mmap.sh   OK 107.57s
>
> I need to think through why this one might false succeed, but that can
> wait until we get this series reviewed. For now my failure is stable
> which allows it to be bisected.
>
>> 
>> Ok: 6   
>> Expected Fail:  0   
>> Fail:   2   
>> Unexpected Pass:0   
>> Skipped:5   
>> Timeout:0   
>> 
>> I have been using QEMU for my testing. Maybe I missed some condition in
>> the unmap path though so will take another look.
>
> I was able to bisect to:

I could have guessed that one, as it's pretty much the crux of this
series given it's the one that switches everything away from
pXX_devmap. That means pXX_leaf/_trans_huge will start returning true
for DAX pages.

Bas

Re: [PATCH 13/13] mm: Remove devmap related functions and page table bits

2024-06-27 Thread kernel test robot
Hi Alistair,

kernel test robot noticed the following build errors:

[auto build test ERROR on f2661062f16b2de5d7b6a5c42a9a5c96326b8454]

url:
https://github.com/intel-lab-lkp/linux/commits/Alistair-Popple/mm-gup-c-Remove-redundant-check-for-PCI-P2PDMA-page/20240627-191709
base:   f2661062f16b2de5d7b6a5c42a9a5c96326b8454
patch link:
https://lore.kernel.org/r/47c26640cd85f3db2e0a2796047199bb984d1b3f.1719386613.git-series.apopple%40nvidia.com
patch subject: [PATCH 13/13] mm: Remove devmap related functions and page table 
bits
config: powerpc-allyesconfig 
(https://download.01.org/0day-ci/archive/20240628/202406280920.vnwstzzt-...@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 
326ba38a991250a8587a399a260b0f7af2c9166a)
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240628/202406280920.vnwstzzt-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202406280920.vnwstzzt-...@intel.com/

All errors (new ones prefixed by >>):

   In file included from arch/powerpc/kernel/asm-offsets.c:12:
   In file included from include/linux/compat.h:14:
   In file included from include/linux/sem.h:5:
   In file included from include/uapi/linux/sem.h:5:
   In file included from include/linux/ipc.h:7:
   In file included from include/linux/rhashtable-types.h:12:
   In file included from include/linux/alloc_tag.h:11:
   In file included from include/linux/preempt.h:79:
   In file included from ./arch/powerpc/include/generated/asm/preempt.h:1:
   In file included from include/asm-generic/preempt.h:5:
   In file included from include/linux/thread_info.h:23:
   In file included from arch/powerpc/include/asm/current.h:13:
   In file included from arch/powerpc/include/asm/paca.h:18:
   In file included from arch/powerpc/include/asm/mmu.h:385:
   In file included from arch/powerpc/include/asm/book3s/64/mmu.h:32:
   In file included from arch/powerpc/include/asm/book3s/64/mmu-hash.h:20:
>> arch/powerpc/include/asm/book3s/64/pgtable.h:1371:1: error: extraneous 
>> closing brace ('}')
1371 | }
 | ^
   In file included from arch/powerpc/kernel/asm-offsets.c:12:
   In file included from include/linux/compat.h:17:
   In file included from include/linux/fs.h:33:
   In file included from include/linux/percpu-rwsem.h:7:
   In file included from include/linux/rcuwait.h:6:
   In file included from include/linux/sched/signal.h:6:
   include/linux/signal.h:98:11: warning: array index 3 is past the end of the 
array (that has type 'unsigned long[1]') [-Warray-bounds]
  98 | return (set->sig[3] | set->sig[2] |
 | ^~
   arch/powerpc/include/uapi/asm/signal.h:18:2: note: array 'sig' declared here
  18 | unsigned long sig[_NSIG_WORDS];
 | ^
   In file included from arch/powerpc/kernel/asm-offsets.c:12:
   In file included from include/linux/compat.h:17:
   In file included from include/linux/fs.h:33:
   In file included from include/linux/percpu-rwsem.h:7:
   In file included from include/linux/rcuwait.h:6:
   In file included from include/linux/sched/signal.h:6:
   include/linux/signal.h:98:25: warning: array index 2 is past the end of the 
array (that has type 'unsigned long[1]') [-Warray-bounds]
  98 | return (set->sig[3] | set->sig[2] |
 |   ^~
   arch/powerpc/include/uapi/asm/signal.h:18:2: note: array 'sig' declared here
  18 | unsigned long sig[_NSIG_WORDS];
 | ^
   In file included from arch/powerpc/kernel/asm-offsets.c:12:
   In file included from include/linux/compat.h:17:
   In file included from include/linux/fs.h:33:
   In file included from include/linux/percpu-rwsem.h:7:
   In file included from include/linux/rcuwait.h:6:
   In file included from include/linux/sched/signal.h:6:
   include/linux/signal.h:99:4: warning: array index 1 is past the end of the 
array (that has type 'unsigned long[1]') [-Warray-bounds]
  99 | set->sig[1] | set->sig[0]) == 0;
 | ^~
   arch/powerpc/include/uapi/asm/signal.h:18:2: note: array 'sig' declared here
  18 | unsigned long sig[_NSIG_WORDS];
 | ^
   In file included from arch/powerpc/kernel/asm-offsets.c:12:
   In file included from include/linux/compat.h:17:
   In file included from include/linux/fs.h:33:
   In file included from include/linux/percpu-rwsem.h:7:
   In file included from include/linux/rcuwait.h:6:
   In file included from include/linux/sched/signal.h:6:
   include/linux/signal.h:101:11: warning: array index 1 is past the end of th