diff mbox

[1/5] Fix spurious segmentation faults

Message ID 1277960546-32336-2-git-send-email-bryan.wu@canonical.com
State Rejected
Headers show

Commit Message

Bryan Wu July 1, 2010, 5:02 a.m. UTC 
From: Rob Clark <rob@ti.com>

This patch helps fixing some 'segmentation faults' seen on Maverick FS.
It is an import of a part of an ARM patch.

Signed-off-by: Sebastien Jan <s-jan@ti.com>
---
 arch/arm/mm/proc-macros.S |    9 ++++++++-
 arch/arm/mm/proc-v7.S     |    3 +++
 2 files changed, 11 insertions(+), 1 deletions(-)

Comments

Bryan Wu July 1, 2010, 5:07 a.m. UTC | #1 
Sebjan,

I heard this bug from Ogra before. And this patch touches the ARM common MM
code, I also failed to find it in upstream mainline.

I think if you or Rob can pointed out in the git log, it is better for us to review.

Ogra, do we have a bug in LP about this?

BTW, CONFIG_CPU_USE_DOMAINS=n in our ti-omap4 config

Thanks,
-Bryan

On 07/01/2010 01:02 PM, Bryan Wu wrote:
> From: Rob Clark <rob@ti.com>
> 
> This patch helps fixing some 'segmentation faults' seen on Maverick FS.
> It is an import of a part of an ARM patch.
> 
> Signed-off-by: Sebastien Jan <s-jan@ti.com>
> ---
>  arch/arm/mm/proc-macros.S |    9 ++++++++-
>  arch/arm/mm/proc-v7.S     |    3 +++
>  2 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> index 7d63bea..1ccf579 100644
> --- a/arch/arm/mm/proc-macros.S
> +++ b/arch/arm/mm/proc-macros.S
> @@ -99,7 +99,11 @@
>   *  110x   0   1   0	r/w	r/o
>   *  11x0   0   1   0	r/w	r/o
>   *  1111   0   1   1	r/w	r/w
> - */
> + *
> + * If !CONFIG_CPU_USE_DOMAINS, the following permissions are changed:
> + *  110x   1   1   1	r/o	r/o
> + *  11x0   1   1   1	r/o	r/o
> +  */
>  	.macro	armv6_mt_table pfx
>  \pfx\()_mt_table:
>  	.long	0x00						@ L_PTE_MT_UNCACHED
> @@ -138,8 +142,11 @@
>  
>  	tst	r1, #L_PTE_USER
>  	orrne	r3, r3, #PTE_EXT_AP1
> +#ifdef CONFIG_CPU_USE_DOMAINS
> +	@ allow kernel read/write access to read-only user pages
>  	tstne	r3, #PTE_EXT_APX
>  	bicne	r3, r3, #PTE_EXT_APX | PTE_EXT_AP0
> +#endif
>  
>  	tst	r1, #L_PTE_EXEC
>  	orreq	r3, r3, #PTE_EXT_XN
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index df74916..c1c3fe0 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -152,8 +152,11 @@ ENTRY(cpu_v7_set_pte_ext)
>  
>  	tst	r1, #L_PTE_USER
>  	orrne	r3, r3, #PTE_EXT_AP1
> +#ifdef CONFIG_CPU_USE_DOMAINS
> +	@ allow kernel read/write access to read-only user pages
>  	tstne	r3, #PTE_EXT_APX
>  	bicne	r3, r3, #PTE_EXT_APX | PTE_EXT_AP0
> +#endif
>  
>  	tst	r1, #L_PTE_EXEC
>  	orreq	r3, r3, #PTE_EXT_XN
Tim Gardner July 1, 2010, 2:10 p.m. UTC | #2 
On 06/30/2010 11:02 PM, Bryan Wu wrote:
> From: Rob Clark<rob@ti.com>
>
> This patch helps fixing some 'segmentation faults' seen on Maverick FS.
> It is an import of a part of an ARM patch.
>
> Signed-off-by: Sebastien Jan<s-jan@ti.com>
> ---

The provenance of this patch is a bit sketchy. Please provide a 
reference to the original patch from whence this was extracted, 
_especially_ if it was from an upstream patch developed by non-TI personnel.

rtg
Sebastien Jan July 1, 2010, 2:15 p.m. UTC | #3 
On 07/01/2010 07:07 AM, Bryan Wu wrote:
> Sebjan,
> 
> I heard this bug from Ogra before. And this patch touches the ARM common MM
> code, I also failed to find it in upstream mainline.
> 
> I think if you or Rob can pointed out in the git log, it is better for us to review.
> 
> Ogra, do we have a bug in LP about this?
> 
> BTW, CONFIG_CPU_USE_DOMAINS=n in our ti-omap4 config
> 
> Thanks,
> -Bryan
> 
> On 07/01/2010 01:02 PM, Bryan Wu wrote:
>> From: Rob Clark <rob@ti.com>
>>
>> This patch helps fixing some 'segmentation faults' seen on Maverick FS.
>> It is an import of a part of an ARM patch.
>>
>> Signed-off-by: Sebastien Jan <s-jan@ti.com>
>> ---
>>  arch/arm/mm/proc-macros.S |    9 ++++++++-
>>  arch/arm/mm/proc-v7.S     |    3 +++
>>  2 files changed, 11 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
>> index 7d63bea..1ccf579 100644
>> --- a/arch/arm/mm/proc-macros.S
>> +++ b/arch/arm/mm/proc-macros.S
>> @@ -99,7 +99,11 @@
>>   *  110x   0   1   0	r/w	r/o
>>   *  11x0   0   1   0	r/w	r/o
>>   *  1111   0   1   1	r/w	r/w
>> - */
>> + *
>> + * If !CONFIG_CPU_USE_DOMAINS, the following permissions are changed:
>> + *  110x   1   1   1	r/o	r/o
>> + *  11x0   1   1   1	r/o	r/o
>> +  */
>>  	.macro	armv6_mt_table pfx
>>  \pfx\()_mt_table:
>>  	.long	0x00						@ L_PTE_MT_UNCACHED
>> @@ -138,8 +142,11 @@
>>  
>>  	tst	r1, #L_PTE_USER
>>  	orrne	r3, r3, #PTE_EXT_AP1
>> +#ifdef CONFIG_CPU_USE_DOMAINS
>> +	@ allow kernel read/write access to read-only user pages
>>  	tstne	r3, #PTE_EXT_APX
>>  	bicne	r3, r3, #PTE_EXT_APX | PTE_EXT_AP0
>> +#endif
>>  
>>  	tst	r1, #L_PTE_EXEC
>>  	orreq	r3, r3, #PTE_EXT_XN
>> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
>> index df74916..c1c3fe0 100644
>> --- a/arch/arm/mm/proc-v7.S
>> +++ b/arch/arm/mm/proc-v7.S
>> @@ -152,8 +152,11 @@ ENTRY(cpu_v7_set_pte_ext)
>>  
>>  	tst	r1, #L_PTE_USER
>>  	orrne	r3, r3, #PTE_EXT_AP1
>> +#ifdef CONFIG_CPU_USE_DOMAINS
>> +	@ allow kernel read/write access to read-only user pages
>>  	tstne	r3, #PTE_EXT_APX
>>  	bicne	r3, r3, #PTE_EXT_APX | PTE_EXT_AP0
>> +#endif
>>  
>>  	tst	r1, #L_PTE_EXEC
>>  	orreq	r3, r3, #PTE_EXT_XN

Right, I shall have provided this info in the commit itself:
An early version of the following patch from Catalin was integrated into our omap4 branch:
http://www.spinics.net/lists/arm-kernel/msg91260.html
(see commit f22c4b02)

But there have been some updates to this patch series, which the current patch aims to cover, as it fixes some very ugly side effects.

Regarding patch author: I specified Rob as he did the investigations on the segfault we had, and find out the missing parts. We may have to credit Catalin instead/additionaly.
Bryan Wu July 2, 2010, 5:06 a.m. UTC | #4 
On 07/01/2010 10:15 PM, Sebastien Jan wrote:
> On 07/01/2010 07:07 AM, Bryan Wu wrote:
>> Sebjan,
>>
>> I heard this bug from Ogra before. And this patch touches the ARM common MM
>> code, I also failed to find it in upstream mainline.
>>
>> I think if you or Rob can pointed out in the git log, it is better for us to review.
>>
>> Ogra, do we have a bug in LP about this?
>>
>> BTW, CONFIG_CPU_USE_DOMAINS=n in our ti-omap4 config
>>
>> Thanks,
>> -Bryan
>>

[snip]

> 
> Right, I shall have provided this info in the commit itself:
> An early version of the following patch from Catalin was integrated into our omap4 branch:
> http://www.spinics.net/lists/arm-kernel/msg91260.html
> (see commit f22c4b02)
> 

Which tree is this patch in? I didn't find it.

> But there have been some updates to this patch series, which the current patch aims to cover, as it fixes some very ugly side effects.
> 

Yeah, I think this patch will be helpful, but we need some testing. Do we have
launchpad bug tracker of this bug? Ogra and Lee, are you guys aware of this bug?
I am not playing with the Maverick rootfs.

> Regarding patch author: I specified Rob as he did the investigations on the segfault we had, and find out the missing parts. We may have to credit Catalin instead/additionaly.

Rob, thanks for fixing this. But we need know more information about the
original patches and the status of this patch? Will this patch in mainline or in
the OMAP4 upstream queue? That will be much easier for us to review.

Thanks,
Sebastien Jan July 2, 2010, 7:31 a.m. UTC | #5 
On 07/02/2010 07:06 AM, Bryan Wu wrote:
> On 07/01/2010 10:15 PM, Sebastien Jan wrote:
>> On 07/01/2010 07:07 AM, Bryan Wu wrote:
>>> Sebjan,
>>>
>>> I heard this bug from Ogra before. And this patch touches the ARM common MM
>>> code, I also failed to find it in upstream mainline.
>>>
>>> I think if you or Rob can pointed out in the git log, it is better for us to review.
>>>
>>> Ogra, do we have a bug in LP about this?
>>>
>>> BTW, CONFIG_CPU_USE_DOMAINS=n in our ti-omap4 config
>>>
>>> Thanks,
>>> -Bryan
>>>
> 
> [snip]
> 
>>
>> Right, I shall have provided this info in the commit itself:
>> An early version of the following patch from Catalin was integrated into our omap4 branch:
>> http://www.spinics.net/lists/arm-kernel/msg91260.html
>> (see commit f22c4b02)
>>
> 
> Which tree is this patch in? I didn't find it.

This commit (f22c4b02) appears in the kernel-ubuntu tree (in the for-ubuntu-2.6.34 branch for example) (and comes from our kernel-omap4 tree, both on dev.omapzoom.org).
See this link: http://dev.omapzoom.org/?p=integration/kernel-ubuntu.git;a=commit;h=f22c4b0221b283f05a648d35f19facf80d8a0b23

>> But there have been some updates to this patch series, which the current patch aims to cover, as it fixes some very ugly side effects.
>>
> 
> Yeah, I think this patch will be helpful, but we need some testing. Do we have
> launchpad bug tracker of this bug? Ogra and Lee, are you guys aware of this bug?
> I am not playing with the Maverick rootfs.
> 
>> Regarding patch author: I specified Rob as he did the investigations on the segfault we had, and find out the missing parts. We may have to credit Catalin instead/additionaly.
> 
> Rob, thanks for fixing this. But we need know more information about the
> original patches and the status of this patch? Will this patch in mainline or in
> the OMAP4 upstream queue? That will be much easier for us to review.

The 1st version of this patch series has been integrated into our omap4 tree (see above commit information).
Since then, new versions of this series are being discussed on the ARM ML [1]. From what I can read, it seems to be a proposal for the 2.6.36 merge window.

Rob's patch contains a part of the new patch series, which fixes a memory related issue (which used to cause the segfaults when using git v1.7.0 for example).
Without this patch, I also used to see segfaults when using dpkg or gcc, making my maverick FS impossible to use for intensive build tasks.
With this patch, I was able to build the latest kernel image on my omap4 board.

We plan to integrate the complete updated patch series in a coming update.


[1] http://www.spinics.net/lists/arm-kernel/msg91260.html
    full thread: http://www.spinics.net/lists/arm-kernel/msg91259.html
Nicolas Pitre July 3, 2010, 5:15 a.m. UTC | #6 
On Fri, 2 Jul 2010, Sebastien Jan wrote:

> This commit (f22c4b02) appears in the kernel-ubuntu tree (in the for-ubuntu-2.6.34 branch for example) (and comes from our kernel-omap4 tree, both on dev.omapzoom.org).
> See this link: http://dev.omapzoom.org/?p=integration/kernel-ubuntu.git;a=commit;h=f22c4b0221b283f05a648d35f19facf80d8a0b23
> 
> >> But there have been some updates to this patch series, which the current patch aims to cover, as it fixes some very ugly side effects.
> >>
> > 
> > Yeah, I think this patch will be helpful, but we need some testing. Do we have
> > launchpad bug tracker of this bug? Ogra and Lee, are you guys aware of this bug?
> > I am not playing with the Maverick rootfs.
> > 
> >> Regarding patch author: I specified Rob as he did the investigations on the segfault we had, and find out the missing parts. We may have to credit Catalin instead/additionaly.
> > 
> > Rob, thanks for fixing this. But we need know more information about the
> > original patches and the status of this patch? Will this patch in mainline or in
> > the OMAP4 upstream queue? That will be much easier for us to review.
> 
> The 1st version of this patch series has been integrated into our omap4 tree (see above commit information).
> Since then, new versions of this series are being discussed on the ARM ML [1]. From what I can read, it seems to be a proposal for the 2.6.36 merge window.
> 
> Rob's patch contains a part of the new patch series, which fixes a memory related issue (which used to cause the segfaults when using git v1.7.0 for example).
> Without this patch, I also used to see segfaults when using dpkg or gcc, making my maverick FS impossible to use for intensive build tasks.
> With this patch, I was able to build the latest kernel image on my omap4 board.

How different is this patch from Catalin's latest?

Did you test with the latest from Catalin?  If not, then why not?

It would really be a good idea to test the latest patch instead of 
trying to fix an older incarnation.  Even more so if the latest still 
exhibits problems.  And in that case it would be really important to 
disclose those segmentation fault problems on the ARM mailing list in 
order to have the original fixed.


Nicolas
Sebastien Jan July 6, 2010, 8:52 a.m. UTC | #7 
On 07/03/2010 07:15 AM, Nicolas Pitre wrote:
> On Fri, 2 Jul 2010, Sebastien Jan wrote:
> 
>> This commit (f22c4b02) appears in the kernel-ubuntu tree (in the for-ubuntu-2.6.34 branch for example) (and comes from our kernel-omap4 tree, both on dev.omapzoom.org).
>> See this link: http://dev.omapzoom.org/?p=integration/kernel-ubuntu.git;a=commit;h=f22c4b0221b283f05a648d35f19facf80d8a0b23
>>
>>>> But there have been some updates to this patch series, which the current patch aims to cover, as it fixes some very ugly side effects.
>>>>
>>>
>>> Yeah, I think this patch will be helpful, but we need some testing. Do we have
>>> launchpad bug tracker of this bug? Ogra and Lee, are you guys aware of this bug?
>>> I am not playing with the Maverick rootfs.
>>>
>>>> Regarding patch author: I specified Rob as he did the investigations on the segfault we had, and find out the missing parts. We may have to credit Catalin instead/additionaly.
>>>
>>> Rob, thanks for fixing this. But we need know more information about the
>>> original patches and the status of this patch? Will this patch in mainline or in
>>> the OMAP4 upstream queue? That will be much easier for us to review.
>>
>> The 1st version of this patch series has been integrated into our omap4 tree (see above commit information).
>> Since then, new versions of this series are being discussed on the ARM ML [1]. From what I can read, it seems to be a proposal for the 2.6.36 merge window.
>>
>> Rob's patch contains a part of the new patch series, which fixes a memory related issue (which used to cause the segfaults when using git v1.7.0 for example).
>> Without this patch, I also used to see segfaults when using dpkg or gcc, making my maverick FS impossible to use for intensive build tasks.
>> With this patch, I was able to build the latest kernel image on my omap4 board.
> 
> How different is this patch from Catalin's latest?
> 
> Did you test with the latest from Catalin?  If not, then why not?
> 
> It would really be a good idea to test the latest patch instead of 
> trying to fix an older incarnation.  Even more so if the latest still 
> exhibits problems.  And in that case it would be really important to 
> disclose those segmentation fault problems on the ARM mailing list in 
> order to have the original fixed.

I fully agree. I have prepared an updated patch to upgrade to latest from Catalin's (v4).
I did not have segfaults with this new version either.

I will send it for review very soon.
diff mbox

Patch

diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
index 7d63bea..1ccf579 100644
--- a/arch/arm/mm/proc-macros.S
+++ b/arch/arm/mm/proc-macros.S
@@ -99,7 +99,11 @@ 
  *  110x   0   1   0	r/w	r/o
  *  11x0   0   1   0	r/w	r/o
  *  1111   0   1   1	r/w	r/w
- */
+ *
+ * If !CONFIG_CPU_USE_DOMAINS, the following permissions are changed:
+ *  110x   1   1   1	r/o	r/o
+ *  11x0   1   1   1	r/o	r/o
+  */
 	.macro	armv6_mt_table pfx
 \pfx\()_mt_table:
 	.long	0x00						@ L_PTE_MT_UNCACHED
@@ -138,8 +142,11 @@ 
 
 	tst	r1, #L_PTE_USER
 	orrne	r3, r3, #PTE_EXT_AP1
+#ifdef CONFIG_CPU_USE_DOMAINS
+	@ allow kernel read/write access to read-only user pages
 	tstne	r3, #PTE_EXT_APX
 	bicne	r3, r3, #PTE_EXT_APX | PTE_EXT_AP0
+#endif
 
 	tst	r1, #L_PTE_EXEC
 	orreq	r3, r3, #PTE_EXT_XN
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index df74916..c1c3fe0 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -152,8 +152,11 @@  ENTRY(cpu_v7_set_pte_ext)
 
 	tst	r1, #L_PTE_USER
 	orrne	r3, r3, #PTE_EXT_AP1
+#ifdef CONFIG_CPU_USE_DOMAINS
+	@ allow kernel read/write access to read-only user pages
 	tstne	r3, #PTE_EXT_APX
 	bicne	r3, r3, #PTE_EXT_APX | PTE_EXT_AP0
+#endif
 
 	tst	r1, #L_PTE_EXEC
 	orreq	r3, r3, #PTE_EXT_XN