All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] ARM: fix PXN process with LPAE feature
@ 2021-05-29  3:41 ` Kefeng Wang
  0 siblings, 0 replies; 12+ messages in thread
From: Kefeng Wang @ 2021-05-29  3:41 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel
  Cc: Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee, Kefeng Wang

refactor and cleanup [__do|do]_page_fault, then fix the pxn process
when lpae feature enabled.

Kefeng Wang (2):
  ARM: mm: Refactor __do_page_fault()
  ARM: mm: Fix PXN process when ARM_LPAE enabled

 arch/arm/mm/fault.c | 78 +++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 48 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 0/2] ARM: fix PXN process with LPAE feature
@ 2021-05-29  3:41 ` Kefeng Wang
  0 siblings, 0 replies; 12+ messages in thread
From: Kefeng Wang @ 2021-05-29  3:41 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel
  Cc: Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee, Kefeng Wang

refactor and cleanup [__do|do]_page_fault, then fix the pxn process
when lpae feature enabled.

Kefeng Wang (2):
  ARM: mm: Refactor __do_page_fault()
  ARM: mm: Fix PXN process when ARM_LPAE enabled

 arch/arm/mm/fault.c | 78 +++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 48 deletions(-)

-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] ARM: mm: Refactor __do_page_fault()
  2021-05-29  3:41 ` Kefeng Wang
@ 2021-05-29  3:41   ` Kefeng Wang
  -1 siblings, 0 replies; 12+ messages in thread
From: Kefeng Wang @ 2021-05-29  3:41 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel
  Cc: Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee, Kefeng Wang

1. cleanup access_error(), make vma flags set and check into
   __do_page_fault() and do_page_fault() directly.

2. drop fsr and task argument, instead, using vm_flags in
   __do_page_fault().

3. cleans up the multiple goto statements in __do_page_fault().

4. use current->mm directly in do_page_fault().

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 arch/arm/mm/fault.c | 73 ++++++++++++++++-----------------------------
 1 file changed, 25 insertions(+), 48 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index efa402025031..81cf3e6e2a3d 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -183,74 +183,45 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 #define VM_FAULT_BADMAP		0x010000
 #define VM_FAULT_BADACCESS	0x020000
 
-/*
- * Check that the permissions on the VMA allow for the fault which occurred.
- * If we encountered a write fault, we must have write permission, otherwise
- * we allow any permission.
- */
-static inline bool access_error(unsigned int fsr, struct vm_area_struct *vma)
-{
-	unsigned int mask = VM_ACCESS_FLAGS;
-
-	if ((fsr & FSR_WRITE) && !(fsr & FSR_CM))
-		mask = VM_WRITE;
-	if (fsr & FSR_LNX_PF)
-		mask = VM_EXEC;
-
-	return vma->vm_flags & mask ? false : true;
-}
-
 static vm_fault_t __kprobes
-__do_page_fault(struct mm_struct *mm, unsigned long addr, unsigned int fsr,
-		unsigned int flags, struct task_struct *tsk,
-		struct pt_regs *regs)
+__do_page_fault(struct mm_struct *mm, unsigned long addr, unsigned int flags,
+		unsigned long vma_flags, struct pt_regs *regs)
 {
-	struct vm_area_struct *vma;
-	vm_fault_t fault;
-
-	vma = find_vma(mm, addr);
-	fault = VM_FAULT_BADMAP;
+	struct vm_area_struct *vma = find_vma(mm, addr);
 	if (unlikely(!vma))
-		goto out;
-	if (unlikely(vma->vm_start > addr))
-		goto check_stack;
+		return VM_FAULT_BADMAP;
+
+	if (unlikely(vma->vm_start > addr)) {
+		if (!(vma->vm_flags & VM_GROWSDOWN))
+			return VM_FAULT_BADMAP;
+		if (addr < FIRST_USER_ADDRESS)
+			return VM_FAULT_BADMAP;
+		if (expand_stack(vma, addr))
+			return VM_FAULT_BADMAP;
+	}
 
 	/*
 	 * Ok, we have a good vm_area for this
 	 * memory access, so we can handle it.
 	 */
-good_area:
-	if (access_error(fsr, vma)) {
-		fault = VM_FAULT_BADACCESS;
-		goto out;
-	}
+	if (!(vma->vm_flags & vma_flags))
+		return VM_FAULT_BADACCESS;
 
 	return handle_mm_fault(vma, addr & PAGE_MASK, flags, regs);
-
-check_stack:
-	/* Don't allow expansion below FIRST_USER_ADDRESS */
-	if (vma->vm_flags & VM_GROWSDOWN &&
-	    addr >= FIRST_USER_ADDRESS && !expand_stack(vma, addr))
-		goto good_area;
-out:
-	return fault;
 }
 
 static int __kprobes
 do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 {
-	struct task_struct *tsk;
-	struct mm_struct *mm;
+	struct mm_struct *mm = current->mm;
 	int sig, code;
 	vm_fault_t fault;
 	unsigned int flags = FAULT_FLAG_DEFAULT;
+	unsigned long vm_flags = VM_ACCESS_FLAGS;
 
 	if (kprobe_page_fault(regs, fsr))
 		return 0;
 
-	tsk = current;
-	mm  = tsk->mm;
-
 	/* Enable interrupts if they were enabled in the parent context. */
 	if (interrupts_enabled(regs))
 		local_irq_enable();
@@ -264,8 +235,14 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 
 	if (user_mode(regs))
 		flags |= FAULT_FLAG_USER;
-	if ((fsr & FSR_WRITE) && !(fsr & FSR_CM))
+
+	if ((fsr & FSR_WRITE) && !(fsr & FSR_CM)) {
 		flags |= FAULT_FLAG_WRITE;
+		vm_flags = VM_WRITE;
+	}
+
+	if (fsr & FSR_LNX_PF)
+		vm_flags = VM_EXEC;
 
 	perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, addr);
 
@@ -293,7 +270,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 #endif
 	}
 
-	fault = __do_page_fault(mm, addr, fsr, flags, tsk, regs);
+	fault = __do_page_fault(mm, addr, flags, vm_flags, regs);
 
 	/* If we need to retry but a fatal signal is pending, handle the
 	 * signal first. We do not need to release the mmap_lock because
-- 
2.26.2


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] ARM: mm: Refactor __do_page_fault()
@ 2021-05-29  3:41   ` Kefeng Wang
  0 siblings, 0 replies; 12+ messages in thread
From: Kefeng Wang @ 2021-05-29  3:41 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel
  Cc: Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee, Kefeng Wang

1. cleanup access_error(), make vma flags set and check into
   __do_page_fault() and do_page_fault() directly.

2. drop fsr and task argument, instead, using vm_flags in
   __do_page_fault().

3. cleans up the multiple goto statements in __do_page_fault().

4. use current->mm directly in do_page_fault().

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 arch/arm/mm/fault.c | 73 ++++++++++++++++-----------------------------
 1 file changed, 25 insertions(+), 48 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index efa402025031..81cf3e6e2a3d 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -183,74 +183,45 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 #define VM_FAULT_BADMAP		0x010000
 #define VM_FAULT_BADACCESS	0x020000
 
-/*
- * Check that the permissions on the VMA allow for the fault which occurred.
- * If we encountered a write fault, we must have write permission, otherwise
- * we allow any permission.
- */
-static inline bool access_error(unsigned int fsr, struct vm_area_struct *vma)
-{
-	unsigned int mask = VM_ACCESS_FLAGS;
-
-	if ((fsr & FSR_WRITE) && !(fsr & FSR_CM))
-		mask = VM_WRITE;
-	if (fsr & FSR_LNX_PF)
-		mask = VM_EXEC;
-
-	return vma->vm_flags & mask ? false : true;
-}
-
 static vm_fault_t __kprobes
-__do_page_fault(struct mm_struct *mm, unsigned long addr, unsigned int fsr,
-		unsigned int flags, struct task_struct *tsk,
-		struct pt_regs *regs)
+__do_page_fault(struct mm_struct *mm, unsigned long addr, unsigned int flags,
+		unsigned long vma_flags, struct pt_regs *regs)
 {
-	struct vm_area_struct *vma;
-	vm_fault_t fault;
-
-	vma = find_vma(mm, addr);
-	fault = VM_FAULT_BADMAP;
+	struct vm_area_struct *vma = find_vma(mm, addr);
 	if (unlikely(!vma))
-		goto out;
-	if (unlikely(vma->vm_start > addr))
-		goto check_stack;
+		return VM_FAULT_BADMAP;
+
+	if (unlikely(vma->vm_start > addr)) {
+		if (!(vma->vm_flags & VM_GROWSDOWN))
+			return VM_FAULT_BADMAP;
+		if (addr < FIRST_USER_ADDRESS)
+			return VM_FAULT_BADMAP;
+		if (expand_stack(vma, addr))
+			return VM_FAULT_BADMAP;
+	}
 
 	/*
 	 * Ok, we have a good vm_area for this
 	 * memory access, so we can handle it.
 	 */
-good_area:
-	if (access_error(fsr, vma)) {
-		fault = VM_FAULT_BADACCESS;
-		goto out;
-	}
+	if (!(vma->vm_flags & vma_flags))
+		return VM_FAULT_BADACCESS;
 
 	return handle_mm_fault(vma, addr & PAGE_MASK, flags, regs);
-
-check_stack:
-	/* Don't allow expansion below FIRST_USER_ADDRESS */
-	if (vma->vm_flags & VM_GROWSDOWN &&
-	    addr >= FIRST_USER_ADDRESS && !expand_stack(vma, addr))
-		goto good_area;
-out:
-	return fault;
 }
 
 static int __kprobes
 do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 {
-	struct task_struct *tsk;
-	struct mm_struct *mm;
+	struct mm_struct *mm = current->mm;
 	int sig, code;
 	vm_fault_t fault;
 	unsigned int flags = FAULT_FLAG_DEFAULT;
+	unsigned long vm_flags = VM_ACCESS_FLAGS;
 
 	if (kprobe_page_fault(regs, fsr))
 		return 0;
 
-	tsk = current;
-	mm  = tsk->mm;
-
 	/* Enable interrupts if they were enabled in the parent context. */
 	if (interrupts_enabled(regs))
 		local_irq_enable();
@@ -264,8 +235,14 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 
 	if (user_mode(regs))
 		flags |= FAULT_FLAG_USER;
-	if ((fsr & FSR_WRITE) && !(fsr & FSR_CM))
+
+	if ((fsr & FSR_WRITE) && !(fsr & FSR_CM)) {
 		flags |= FAULT_FLAG_WRITE;
+		vm_flags = VM_WRITE;
+	}
+
+	if (fsr & FSR_LNX_PF)
+		vm_flags = VM_EXEC;
 
 	perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, addr);
 
@@ -293,7 +270,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 #endif
 	}
 
-	fault = __do_page_fault(mm, addr, fsr, flags, tsk, regs);
+	fault = __do_page_fault(mm, addr, flags, vm_flags, regs);
 
 	/* If we need to retry but a fatal signal is pending, handle the
 	 * signal first. We do not need to release the mmap_lock because
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] ARM: mm: Fix PXN process when ARM_LPAE enabled
  2021-05-29  3:41 ` Kefeng Wang
@ 2021-05-29  3:41   ` Kefeng Wang
  -1 siblings, 0 replies; 12+ messages in thread
From: Kefeng Wang @ 2021-05-29  3:41 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel
  Cc: Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee, Kefeng Wang

If we support PXN feature, should panic when user code execution
with privilege mode, but it is not true when LAPE enabled, let's
fix it by adding the check in do_page_fault().

Fixes: 1d4d37159d01 ("ARM: 8235/1: Support for the PXN CPU feature on ARMv7")
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 arch/arm/mm/fault.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 81cf3e6e2a3d..a6ada914cdf5 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -241,8 +241,13 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 		vm_flags = VM_WRITE;
 	}
 
-	if (fsr & FSR_LNX_PF)
+	if (fsr & FSR_LNX_PF) {
 		vm_flags = VM_EXEC;
+#ifdef CONFIG_ARM_LPAE
+		if (addr < TASK_SIZE && !user_mode(regs))
+			__do_kernel_fault(mm, addr, fsr, regs);
+#endif
+	}
 
 	perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, addr);
 
-- 
2.26.2


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] ARM: mm: Fix PXN process when ARM_LPAE enabled
@ 2021-05-29  3:41   ` Kefeng Wang
  0 siblings, 0 replies; 12+ messages in thread
From: Kefeng Wang @ 2021-05-29  3:41 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel
  Cc: Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee, Kefeng Wang

If we support PXN feature, should panic when user code execution
with privilege mode, but it is not true when LAPE enabled, let's
fix it by adding the check in do_page_fault().

Fixes: 1d4d37159d01 ("ARM: 8235/1: Support for the PXN CPU feature on ARMv7")
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 arch/arm/mm/fault.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 81cf3e6e2a3d..a6ada914cdf5 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -241,8 +241,13 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 		vm_flags = VM_WRITE;
 	}
 
-	if (fsr & FSR_LNX_PF)
+	if (fsr & FSR_LNX_PF) {
 		vm_flags = VM_EXEC;
+#ifdef CONFIG_ARM_LPAE
+		if (addr < TASK_SIZE && !user_mode(regs))
+			__do_kernel_fault(mm, addr, fsr, regs);
+#endif
+	}
 
 	perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, addr);
 
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] ARM: fix PXN process with LPAE feature
  2021-05-29  3:41 ` Kefeng Wang
@ 2021-06-01 13:29   ` Kefeng Wang
  -1 siblings, 0 replies; 12+ messages in thread
From: Kefeng Wang @ 2021-06-01 13:29 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel
  Cc: Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee


On 2021/5/29 11:41, Kefeng Wang wrote:
> refactor and cleanup [__do|do]_page_fault, then fix the pxn process
> when lpae feature enabled.

Additional information:

when user code execution with privilege mode, it will lead infinite loop

in the page fault handler if ARM_LPAE enabled,  the issue could be

reproduced with  "echo EXEC_USERSPACE > 
/sys/kernel/debug/provoke-crash/DIRECT"

also I could add a die_kernel_fault() (like arm64 does) to report different

message in different scenes.

Any comments, thanks.


>
> Kefeng Wang (2):
>    ARM: mm: Refactor __do_page_fault()
>    ARM: mm: Fix PXN process when ARM_LPAE enabled
>
>   arch/arm/mm/fault.c | 78 +++++++++++++++++----------------------------
>   1 file changed, 30 insertions(+), 48 deletions(-)
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] ARM: fix PXN process with LPAE feature
@ 2021-06-01 13:29   ` Kefeng Wang
  0 siblings, 0 replies; 12+ messages in thread
From: Kefeng Wang @ 2021-06-01 13:29 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel
  Cc: Catalin Marinas, linux-kernel, Andrew Morton, Jungseung Lee


On 2021/5/29 11:41, Kefeng Wang wrote:
> refactor and cleanup [__do|do]_page_fault, then fix the pxn process
> when lpae feature enabled.

Additional information:

when user code execution with privilege mode, it will lead infinite loop

in the page fault handler if ARM_LPAE enabled,  the issue could be

reproduced with  "echo EXEC_USERSPACE > 
/sys/kernel/debug/provoke-crash/DIRECT"

also I could add a die_kernel_fault() (like arm64 does) to report different

message in different scenes.

Any comments, thanks.


>
> Kefeng Wang (2):
>    ARM: mm: Refactor __do_page_fault()
>    ARM: mm: Fix PXN process when ARM_LPAE enabled
>
>   arch/arm/mm/fault.c | 78 +++++++++++++++++----------------------------
>   1 file changed, 30 insertions(+), 48 deletions(-)
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] ARM: mm: Refactor __do_page_fault()
  2021-05-29  3:41   ` Kefeng Wang
@ 2021-06-01 14:31     ` Russell King (Oracle)
  -1 siblings, 0 replies; 12+ messages in thread
From: Russell King (Oracle) @ 2021-06-01 14:31 UTC (permalink / raw)
  To: Kefeng Wang
  Cc: linux-arm-kernel, Catalin Marinas, linux-kernel, Andrew Morton,
	Jungseung Lee

On Sat, May 29, 2021 at 11:41:37AM +0800, Kefeng Wang wrote:
> 1. cleanup access_error(), make vma flags set and check into
>    __do_page_fault() and do_page_fault() directly.
> 
> 2. drop fsr and task argument, instead, using vm_flags in
>    __do_page_fault().
> 
> 3. cleans up the multiple goto statements in __do_page_fault().
> 
> 4. use current->mm directly in do_page_fault().
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

This patch is a really good example of something that is very difficult
to review and see that there are no unintended changes.

Many people have complained about my patches, where I create a series of
many patches where each patch does exactly _one_ simple transformation to
the code. This is a good example _why_ I do that - a step by step single
transformation approach is way easier to review.

Sorry, but I'm not able to sensibly review this patch, and therefore
I won't apply it. Please split it into smaller changes.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] ARM: mm: Refactor __do_page_fault()
@ 2021-06-01 14:31     ` Russell King (Oracle)
  0 siblings, 0 replies; 12+ messages in thread
From: Russell King (Oracle) @ 2021-06-01 14:31 UTC (permalink / raw)
  To: Kefeng Wang
  Cc: linux-arm-kernel, Catalin Marinas, linux-kernel, Andrew Morton,
	Jungseung Lee

On Sat, May 29, 2021 at 11:41:37AM +0800, Kefeng Wang wrote:
> 1. cleanup access_error(), make vma flags set and check into
>    __do_page_fault() and do_page_fault() directly.
> 
> 2. drop fsr and task argument, instead, using vm_flags in
>    __do_page_fault().
> 
> 3. cleans up the multiple goto statements in __do_page_fault().
> 
> 4. use current->mm directly in do_page_fault().
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

This patch is a really good example of something that is very difficult
to review and see that there are no unintended changes.

Many people have complained about my patches, where I create a series of
many patches where each patch does exactly _one_ simple transformation to
the code. This is a good example _why_ I do that - a step by step single
transformation approach is way easier to review.

Sorry, but I'm not able to sensibly review this patch, and therefore
I won't apply it. Please split it into smaller changes.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] ARM: mm: Refactor __do_page_fault()
  2021-06-01 14:31     ` Russell King (Oracle)
@ 2021-06-02  6:42       ` Kefeng Wang
  -1 siblings, 0 replies; 12+ messages in thread
From: Kefeng Wang @ 2021-06-02  6:42 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: linux-arm-kernel, Catalin Marinas, linux-kernel, Andrew Morton,
	Jungseung Lee


On 2021/6/1 22:31, Russell King (Oracle) wrote:
> On Sat, May 29, 2021 at 11:41:37AM +0800, Kefeng Wang wrote:
>> 1. cleanup access_error(), make vma flags set and check into
>>     __do_page_fault() and do_page_fault() directly.
>>
>> 2. drop fsr and task argument, instead, using vm_flags in
>>     __do_page_fault().
>>
>> 3. cleans up the multiple goto statements in __do_page_fault().
>>
>> 4. use current->mm directly in do_page_fault().
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> This patch is a really good example of something that is very difficult
> to review and see that there are no unintended changes.
>
> Many people have complained about my patches, where I create a series of
> many patches where each patch does exactly _one_ simple transformation to
> the code. This is a good example _why_ I do that - a step by step single
> transformation approach is way easier to review.
>
> Sorry, but I'm not able to sensibly review this patch, and therefore
> I won't apply it. Please split it into smaller changes.
Ok, will split it and send v2, thanks.
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] ARM: mm: Refactor __do_page_fault()
@ 2021-06-02  6:42       ` Kefeng Wang
  0 siblings, 0 replies; 12+ messages in thread
From: Kefeng Wang @ 2021-06-02  6:42 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: linux-arm-kernel, Catalin Marinas, linux-kernel, Andrew Morton,
	Jungseung Lee


On 2021/6/1 22:31, Russell King (Oracle) wrote:
> On Sat, May 29, 2021 at 11:41:37AM +0800, Kefeng Wang wrote:
>> 1. cleanup access_error(), make vma flags set and check into
>>     __do_page_fault() and do_page_fault() directly.
>>
>> 2. drop fsr and task argument, instead, using vm_flags in
>>     __do_page_fault().
>>
>> 3. cleans up the multiple goto statements in __do_page_fault().
>>
>> 4. use current->mm directly in do_page_fault().
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> This patch is a really good example of something that is very difficult
> to review and see that there are no unintended changes.
>
> Many people have complained about my patches, where I create a series of
> many patches where each patch does exactly _one_ simple transformation to
> the code. This is a good example _why_ I do that - a step by step single
> transformation approach is way easier to review.
>
> Sorry, but I'm not able to sensibly review this patch, and therefore
> I won't apply it. Please split it into smaller changes.
Ok, will split it and send v2, thanks.
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-06-02  6:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-29  3:41 [PATCH 0/2] ARM: fix PXN process with LPAE feature Kefeng Wang
2021-05-29  3:41 ` Kefeng Wang
2021-05-29  3:41 ` [PATCH 1/2] ARM: mm: Refactor __do_page_fault() Kefeng Wang
2021-05-29  3:41   ` Kefeng Wang
2021-06-01 14:31   ` Russell King (Oracle)
2021-06-01 14:31     ` Russell King (Oracle)
2021-06-02  6:42     ` Kefeng Wang
2021-06-02  6:42       ` Kefeng Wang
2021-05-29  3:41 ` [PATCH 2/2] ARM: mm: Fix PXN process when ARM_LPAE enabled Kefeng Wang
2021-05-29  3:41   ` Kefeng Wang
2021-06-01 13:29 ` [PATCH 0/2] ARM: fix PXN process with LPAE feature Kefeng Wang
2021-06-01 13:29   ` Kefeng Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.