On 2023/8/24 16:32, Heiko Carstens wrote:
On Thu, Aug 24, 2023 at 10:16:33AM +0200, Alexander Gordeev wrote:
On Mon, Aug 21, 2023 at 08:30:50PM +0800, Kefeng Wang wrote:
Use new try_vma_locked_page_fault() helper to simplify code.
No functional change intended.
Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
---
arch/s390/mm/fault.c | 66 ++++++++++++++++++--------------------------
1 file changed, 27 insertions(+), 39 deletions(-)
...
- fault = handle_mm_fault(vma, address, flags | FAULT_FLAG_VMA_LOCK, regs);
- if (!(fault & (VM_FAULT_RETRY | VM_FAULT_COMPLETED)))
- vma_end_read(vma);
- if (!(fault & VM_FAULT_RETRY)) {
- count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
- if (likely(!(fault & VM_FAULT_ERROR)))
- fault = 0;
This fault fixup is removed in the new version.
...
+ vmf.vm_flags = VM_WRITE;
+ if (vmf.vm_flags == VM_WRITE)
+ vmf.flags |= FAULT_FLAG_WRITE;
+
+ fault = try_vma_locked_page_fault(&vmf);
+ if (fault == VM_FAULT_NONE)
+ goto lock_mm;
Because VM_FAULT_NONE is set to 0 it gets confused with
the success code of 0 returned by a fault handler. In the
former case we want to continue, while in the latter -
successfully return. I think it applies to all archs.
...
FWIW, this series ends up with kernel BUG at arch/s390/mm/fault.c:341!
I didn't test and only built, this is a RFC to want to know whether
the way to add three more numbers into vmf and using vmf in arch's page
fault is feasible or not.
Without having looked in detail into this patch: all of this is likely
because s390's fault handling is quite odd. Not only because fault is set
to 0, but also because of the private VM_FAULT values like
VM_FAULT_BADCONTEXT. I'm just cleaning up all of this, but it won't make it
for the next merge window.
Sure, if re-post, will drop the s390's change, but as mentioned above,
the abstract of the generic vma locked and changes may be not perfect,
let's wait for more response.
Thanks all.
Therefore I'd like to ask to drop the s390 conversion of this series, and
if this series is supposed to be merged the s390 conversion needs to be
done later. Let's not waste more time on the current implementation,
please.
