Re: RDMA/rpma + fsdax(ext4) was broken since 36f30e486d

["lizhijian@fujitsu.com" <lizhijian@fujitsu.com>]

Hi all

I have verified that below changes can solve this problem

diff --git a/mm/hmm.c b/mm/hmm.c
index 24f9ff95f3ae..2c9a3e3eefce 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -294,7 +294,7 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr,
          * Since each architecture defines a struct page for the zero page, just
          * fall through and treat it like a normal page.
          */
-       if (pte_special(pte) && !is_zero_pfn(pte_pfn(pte))) {
+       if (!pte_devmap(pte) && pte_special(pte) && !is_zero_pfn(pte_pfn(pte))) {
                 if (hmm_pte_need_fault(hmm_vma_walk, pfn_req_flags, 0)) {
                         pte_unmap(ptep);
                         return -EFAULT;


i looked over the change-log of hmm_vma_handle_pte(), and found that before
4055062 ("mm/hmm: add missing call to hmm_pte_need_fault in HMM_PFN_SPECIAL handling")

hmm_vma_handle_pte() will not check pte_special(pte) if pte_devmap(pte) is true.

when we reached
"if (pte_special(pte) && !is_zero_pfn(pte_pfn(pte))) {"
the pte have already presented and its pte's flag already fulfilled the request flags.


My question is that
Per https://01.org/blogs/dave/2020/linux-consumption-x86-page-table-bits,
pte_devmap(pte) and pte_special(pte) could be both true in fsdax user case, right ?

if so, what's the expected code path for fsdax case ？


commit 4055062749229101365e5f9e87cb1c5a93e292f8
Author: Jason Gunthorpe <jgg@mellanox.com>
Date:   Thu Mar 5 14:27:20 2020 -0400

     mm/hmm: add missing call to hmm_pte_need_fault in HMM_PFN_SPECIAL handling

     Currently if a special PTE is encountered hmm_range_fault() immediately
     returns EFAULT and sets the HMM_PFN_SPECIAL error output (which nothing
     uses).

     EFAULT should only be returned after testing with hmm_pte_need_fault().

     Also pte_devmap() and pte_special() are exclusive, and there is no need to
     check IS_ENABLED, pte_special() is stubbed out to return false on
     unsupported architectures.

     Fixes: 992de9a8b751 ("mm/hmm: allow to mirror vma of a file on a DAX backed filesystem")
     Reviewed-by: Ralph Campbell <rcampbell@nvidia.com>
     Reviewed-by: Christoph Hellwig <hch@lst.de>
     Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>

diff --git a/mm/hmm.c b/mm/hmm.c
index 3a03fcf..9c82ea9 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -339,16 +339,21 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr,
                         pte_unmap(ptep);
                         return -EBUSY;
                 }
-       } else if (IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL) && pte_special(pte)) {
-               if (!is_zero_pfn(pte_pfn(pte))) {
+       }
+
+       /*
+        * Since each architecture defines a struct page for the zero page, just
+        * fall through and treat it like a normal page.
+        */
+       if (pte_special(pte) && !is_zero_pfn(pte_pfn(pte))) {
+               hmm_pte_need_fault(hmm_vma_walk, orig_pfn, 0, &fault,
+                                  &write_fault);
+               if (fault || write_fault) {
                         pte_unmap(ptep);
-                       *pfn = range->values[HMM_PFN_SPECIAL];
                         return -EFAULT;
                 }
-               /*
-                * Since each architecture defines a struct page for the zero
-                * page, just fall through and treat it like a normal page.
-                */
+               *pfn = range->values[HMM_PFN_SPECIAL];
+               return 0;
         }

         *pfn = hmm_device_entry_from_pfn(range, pte_pfn(pte)) | cpu_flags;


Thanks
Zhijian



On 06/08/2021 11:20, Li, Zhijian wrote:
> Hi Jason
>
> thank you for your advice.
>
> CCing nvdimm
>
> both ext4 and xfs are impacted.
>
> Thanks
>
>
> at 2021/8/6 9:45, Jason Gunthorpe wrote:
>> On Wed, Aug 04, 2021 at 04:06:53PM +0800, Li, Zhijian/李 智坚 wrote:
>>> convert to text and send again
>>>
>>> 2021/8/4 15:55, Li, Zhijian wrote:
>>>> Hey all:
>>>>
>>>> Recently, i reported a issue to rpmahttps://github.com/pmem/rpma/issues/1142
>>>> where we found that the native rpma + fsdax example failed in recent kernel.
>>>>
>>>> Below is the bisect log
>>>>
>>>> [lizhijian@yl linux]$ git bisect log
>>>> git bisect start
>>>> # good: [bbf5c979011a099af5dc76498918ed7df445635b] Linux 5.9
>>>> git bisect good bbf5c979011a099af5dc76498918ed7df445635b
>>>> # bad: [2c85ebc57b3e1817b6ce1a6b703928e113a90442] Linux 5.10
>>>> git bisect bad 2c85ebc57b3e1817b6ce1a6b703928e113a90442
>>>> # good: [4d0e9df5e43dba52d38b251e3b909df8fa1110be] lib, uaccess: add failure injection to usercopy functions
>>>> git bisect good 4d0e9df5e43dba52d38b251e3b909df8fa1110be
>>>> # bad: [6694875ef8045cdb1e6712ee9b68fe08763507d8] ext4: indicate that fast_commit is available via /sys/fs/ext4/feature/...
>>>> git bisect bad 6694875ef8045cdb1e6712ee9b68fe08763507d8
>>>> # good: [14c914fcb515c424177bb6848cc2858ebfe717a8] Merge tag 'wireless-drivers-next-2020-10-02' of git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next
>>>> git bisect good 14c914fcb515c424177bb6848cc2858ebfe717a8
>>>> # good: [6f78b9acf04fbf9ede7f4265e7282f9fb39d2c8c] Merge tag 'mtd/for-5.10' of git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux
>>>> git bisect good 6f78b9acf04fbf9ede7f4265e7282f9fb39d2c8c
>>>> # bad: [bbe85027ce8019c73ab99ad1c2603e2dcd1afa49] Merge tag 'xfs-5.10-merge-5' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux
>>>> git bisect bad bbe85027ce8019c73ab99ad1c2603e2dcd1afa49
>>>> # bad: [9d9af1007bc08971953ae915d88dc9bb21344b53] Merge tag 'perf-tools-for-v5.10-2020-10-15' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux
>>>> git bisect bad 9d9af1007bc08971953ae915d88dc9bb21344b53
>>>> # good: [21c2fe94abb2abe894e6aabe6b4e84a255c8d339] RDMA/mthca: Combine special QP struct with mthca QP
>>>> git bisect good 21c2fe94abb2abe894e6aabe6b4e84a255c8d339
>>>> # good: [dbaa1b3d9afba3c050d365245a36616ae3f425a7] Merge branch 'perf/urgent' into perf/core
>>>> git bisect good dbaa1b3d9afba3c050d365245a36616ae3f425a7
>>>> # bad: [c7a198c700763ac89abbb166378f546aeb9afb33] RDMA/ucma: Fix use after free in destroy id flow
>>>> git bisect bad c7a198c700763ac89abbb166378f546aeb9afb33
>>>> # bad: [5ce2dced8e95e76ff7439863a118a053a7fc6f91] RDMA/ipoib: Set rtnl_link_ops for ipoib interfaces
>>>> git bisect bad 5ce2dced8e95e76ff7439863a118a053a7fc6f91
>>>> # bad: [a03bfc37d59de316436c46f5691c5a972ed57c82] RDMA/mlx5: Sync device with CPU pages upon ODP MR registration
>>>> git bisect bad a03bfc37d59de316436c46f5691c5a972ed57c82
>>>> # good: [a6f0b08dbaf289c3c57284e16ac8043140f2139b] RDMA/core: Remove ucontext->closing
>>>> git bisect good a6f0b08dbaf289c3c57284e16ac8043140f2139b
>>>> # bad: [36f30e486dce22345c2dd3a3ba439c12cd67f6ba] IB/core: Improve ODP to use hmm_range_fault()
>>>> git bisect bad 36f30e486dce22345c2dd3a3ba439c12cd67f6ba
>>>> # good: [2ee9bf346fbfd1dad0933b9eb3a4c2c0979b633e] RDMA/addr: Fix race with netevent_callback()/rdma_addr_cancel()
>>>> git bisect good 2ee9bf346fbfd1dad0933b9eb3a4c2c0979b633e
>>>> # first bad commit: [36f30e486dce22345c2dd3a3ba439c12cd67f6ba] IB/core: Improve ODP to use hmm_range_fault()
>> This is perhaps not so surprising, but I think you should report it to
>> the dax people that hmm_range_fault and dax don't work together..
>>
>> Though I think it is supposed to, and I'm surprised it doesn't.
>>
>> Jason
>>
>>