diff mbox series

m68k: mm: Remove check for VM_IO to fix deferred I/O

Message ID 20220128173006.1713210-1-geert@linux-m68k.org (mailing list archive)
State Not Applicable
Headers show
Series m68k: mm: Remove check for VM_IO to fix deferred I/O | expand

Commit Message

Geert Uytterhoeven Jan. 28, 2022, 5:30 p.m. UTC
When an application accesses a mapped frame buffer backed by deferred
I/O, it receives a segmentation fault.  Fix this by removing the check
for VM_IO in do_page_fault().

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
This check was never present in a fault handler on any other
architecture than m68k.
Some digging revealed that it was added in v2.1.106, but I couldn't find
an email with a patch adding it.  That same kernel version extended the
use of the hwreg_present() helper to HP9000/300, so the check might have
been needed there, perhaps only during development?
The Atari kernel relies heavily on hwreg_present() (both the success and
failure cases), and these still work, at least on ARAnyM.
---
 arch/m68k/mm/fault.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Michael Schmitz Jan. 28, 2022, 9:26 p.m. UTC | #1
Hi Geert,

for hwregs_present(), the exception fixup will handle any access error 
(through send_fault_sig()), so this should continue to work.

Why the special handling of VM_IO pages? Maybe hp300 had marked all IO 
register pages VM_IO to distinguish IO faults from VM faults...

The only other area I can imagine this might have an impact is the Mac's 
pseudo-DMA - FInn might want to give this some testing.

Cheers,

	Michael


Am 29.01.2022 um 06:30 schrieb Geert Uytterhoeven:
> When an application accesses a mapped frame buffer backed by deferred
> I/O, it receives a segmentation fault.  Fix this by removing the check
> for VM_IO in do_page_fault().
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> This check was never present in a fault handler on any other
> architecture than m68k.
> Some digging revealed that it was added in v2.1.106, but I couldn't find
> an email with a patch adding it.  That same kernel version extended the
> use of the hwreg_present() helper to HP9000/300, so the check might have
> been needed there, perhaps only during development?
> The Atari kernel relies heavily on hwreg_present() (both the success and
> failure cases), and these still work, at least on ARAnyM.
> ---
>  arch/m68k/mm/fault.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/arch/m68k/mm/fault.c b/arch/m68k/mm/fault.c
> index 1493cf5eac1e7a39..71aa9f6315dc8028 100644
> --- a/arch/m68k/mm/fault.c
> +++ b/arch/m68k/mm/fault.c
> @@ -93,8 +93,6 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
>  	vma = find_vma(mm, address);
>  	if (!vma)
>  		goto map_err;
> -	if (vma->vm_flags & VM_IO)
> -		goto acc_err;
>  	if (vma->vm_start <= address)
>  		goto good_area;
>  	if (!(vma->vm_flags & VM_GROWSDOWN))
>
Finn Thain Jan. 28, 2022, 11:55 p.m. UTC | #2
On Sat, 29 Jan 2022, Michael Schmitz wrote:

> Hi Geert,
> 
> for hwregs_present(), the exception fixup will handle any access error 
> (through send_fault_sig()), so this should continue to work.
> 
> Why the special handling of VM_IO pages? Maybe hp300 had marked all IO 
> register pages VM_IO to distinguish IO faults from VM faults...
> 
> The only other area I can imagine this might have an impact is the Mac's 
> pseudo-DMA - FInn might want to give this some testing.
> 

mac_scsi.c and mac_esp.c don't use ioremap(). They rely on head.S:

        mmu_map_eq      #0x50000000,#0x03000000,%d3

Having said that, I will run some tests if you still think it necessary.
Michael Schmitz Jan. 29, 2022, 2:08 a.m. UTC | #3
Hi Finn,

Am 29.01.2022 um 12:55 schrieb Finn Thain:
> On Sat, 29 Jan 2022, Michael Schmitz wrote:
>
>> Hi Geert,
>>
>> for hwregs_present(), the exception fixup will handle any access error
>> (through send_fault_sig()), so this should continue to work.
>>
>> Why the special handling of VM_IO pages? Maybe hp300 had marked all IO
>> register pages VM_IO to distinguish IO faults from VM faults...
>>
>> The only other area I can imagine this might have an impact is the Mac's
>> pseudo-DMA - FInn might want to give this some testing.
>>
>
> mac_scsi.c and mac_esp.c don't use ioremap(). They rely on head.S:
>
>         mmu_map_eq      #0x50000000,#0x03000000,%d3
>
> Having said that, I will run some tests if you still think it necessary.

No need for test then, thanks!

Cheers,

	Michael
Michael Schmitz Jan. 30, 2022, 12:32 a.m. UTC | #4
Hi Geert,

testing this patch on my Falcon 030, I'm seeing a weird error checking 
and mounting the root filesystem (pata-falcon). The system appears to 
sit idle, never completing the journal recovery and mount. Still 
investigating that.

Can't see how that would be caused by your patch, just saying I could 
not yet test it.

Cheers,

	Michael


Am 29.01.2022 um 06:30 schrieb Geert Uytterhoeven:
> When an application accesses a mapped frame buffer backed by deferred
> I/O, it receives a segmentation fault.  Fix this by removing the check
> for VM_IO in do_page_fault().
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> This check was never present in a fault handler on any other
> architecture than m68k.
> Some digging revealed that it was added in v2.1.106, but I couldn't find
> an email with a patch adding it.  That same kernel version extended the
> use of the hwreg_present() helper to HP9000/300, so the check might have
> been needed there, perhaps only during development?
> The Atari kernel relies heavily on hwreg_present() (both the success and
> failure cases), and these still work, at least on ARAnyM.
> ---
>  arch/m68k/mm/fault.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/arch/m68k/mm/fault.c b/arch/m68k/mm/fault.c
> index 1493cf5eac1e7a39..71aa9f6315dc8028 100644
> --- a/arch/m68k/mm/fault.c
> +++ b/arch/m68k/mm/fault.c
> @@ -93,8 +93,6 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
>  	vma = find_vma(mm, address);
>  	if (!vma)
>  		goto map_err;
> -	if (vma->vm_flags & VM_IO)
> -		goto acc_err;
>  	if (vma->vm_start <= address)
>  		goto good_area;
>  	if (!(vma->vm_flags & VM_GROWSDOWN))
>
Michael Schmitz Jan. 30, 2022, 6:57 a.m. UTC | #5
Hi Geert,

Am 30.01.2022 um 13:32 schrieb Michael Schmitz:
> Hi Geert,
>
> testing this patch on my Falcon 030, I'm seeing a weird error checking
> and mounting the root filesystem (pata-falcon). The system appears to
> sit idle, never completing the journal recovery and mount. Still
> investigating that.

Belay that - not related to your patch, must be some other regression 
since v5.16 that I'm seeing there.

Just ignore the noise ...

Cheers,

	Michael


> Can't see how that would be caused by your patch, just saying I could
> not yet test it.
>
> Cheers,
>
>     Michael
>
>
> Am 29.01.2022 um 06:30 schrieb Geert Uytterhoeven:
>> When an application accesses a mapped frame buffer backed by deferred
>> I/O, it receives a segmentation fault.  Fix this by removing the check
>> for VM_IO in do_page_fault().
>>
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>> This check was never present in a fault handler on any other
>> architecture than m68k.
>> Some digging revealed that it was added in v2.1.106, but I couldn't find
>> an email with a patch adding it.  That same kernel version extended the
>> use of the hwreg_present() helper to HP9000/300, so the check might have
>> been needed there, perhaps only during development?
>> The Atari kernel relies heavily on hwreg_present() (both the success and
>> failure cases), and these still work, at least on ARAnyM.
>> ---
>>  arch/m68k/mm/fault.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/arch/m68k/mm/fault.c b/arch/m68k/mm/fault.c
>> index 1493cf5eac1e7a39..71aa9f6315dc8028 100644
>> --- a/arch/m68k/mm/fault.c
>> +++ b/arch/m68k/mm/fault.c
>> @@ -93,8 +93,6 @@ int do_page_fault(struct pt_regs *regs, unsigned
>> long address,
>>      vma = find_vma(mm, address);
>>      if (!vma)
>>          goto map_err;
>> -    if (vma->vm_flags & VM_IO)
>> -        goto acc_err;
>>      if (vma->vm_start <= address)
>>          goto good_area;
>>      if (!(vma->vm_flags & VM_GROWSDOWN))
>>
Michael Schmitz Jan. 31, 2022, 2:21 a.m. UTC | #6
Hi Geert,

Am 29.01.2022 um 06:30 schrieb Geert Uytterhoeven:
> When an application accesses a mapped frame buffer backed by deferred
> I/O, it receives a segmentation fault.  Fix this by removing the check
> for VM_IO in do_page_fault().
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

Works fine on my Falcon030 when applied to v5.16.

Tested-by: Michael Schmitz <schmitzmic@gmail.com>

> ---
> This check was never present in a fault handler on any other
> architecture than m68k.
> Some digging revealed that it was added in v2.1.106, but I couldn't find
> an email with a patch adding it.  That same kernel version extended the
> use of the hwreg_present() helper to HP9000/300, so the check might have
> been needed there, perhaps only during development?
> The Atari kernel relies heavily on hwreg_present() (both the success and
> failure cases), and these still work, at least on ARAnyM.
> ---
>  arch/m68k/mm/fault.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/arch/m68k/mm/fault.c b/arch/m68k/mm/fault.c
> index 1493cf5eac1e7a39..71aa9f6315dc8028 100644
> --- a/arch/m68k/mm/fault.c
> +++ b/arch/m68k/mm/fault.c
> @@ -93,8 +93,6 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
>  	vma = find_vma(mm, address);
>  	if (!vma)
>  		goto map_err;
> -	if (vma->vm_flags & VM_IO)
> -		goto acc_err;
>  	if (vma->vm_start <= address)
>  		goto good_area;
>  	if (!(vma->vm_flags & VM_GROWSDOWN))
>
Geert Uytterhoeven Feb. 7, 2022, 1:06 p.m. UTC | #7
On Mon, Jan 31, 2022 at 3:22 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> Am 29.01.2022 um 06:30 schrieb Geert Uytterhoeven:
> > When an application accesses a mapped frame buffer backed by deferred
> > I/O, it receives a segmentation fault.  Fix this by removing the check
> > for VM_IO in do_page_fault().
> >
> > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>
> Works fine on my Falcon030 when applied to v5.16.
>
> Tested-by: Michael Schmitz <schmitzmic@gmail.com>

Thanks, queued in the m68k for-v5.18 branch.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
diff mbox series

Patch

diff --git a/arch/m68k/mm/fault.c b/arch/m68k/mm/fault.c
index 1493cf5eac1e7a39..71aa9f6315dc8028 100644
--- a/arch/m68k/mm/fault.c
+++ b/arch/m68k/mm/fault.c
@@ -93,8 +93,6 @@  int do_page_fault(struct pt_regs *regs, unsigned long address,
 	vma = find_vma(mm, address);
 	if (!vma)
 		goto map_err;
-	if (vma->vm_flags & VM_IO)
-		goto acc_err;
 	if (vma->vm_start <= address)
 		goto good_area;
 	if (!(vma->vm_flags & VM_GROWSDOWN))