ARM: 6166/1: Proper prefetch abort handling on pre-ARMv6
in [Kernel]
|
Prev: [013/200] NFSD: dont report compiled-out versions as present
Next: [patch 037/164] USB: FHCI: cq_get() should check kfifo_out()s return value
From: Kirill A. Shutemov on 1 Jul 2010 18:20 On Thu, Jul 01, 2010 at 10:31:37AM -0700, Greg KH wrote: > 2.6.32-stable review patch. If anyone has any objections, please let us know. > > ------------------ > > From: Anfei <anfei.zhou(a)gmail.com> > > commit 5e27fb78df95e027723af2c90ecc9b4527ae59e9 upstream. > > Instruction faults on pre-ARMv6 CPUs are interpreted as > a 'translation fault', but do_translation_fault doesn't > handle well if user mode trying to run instruction above > TASK_SIZE, and result in the infinite retry of that > instruction. Actually, this patch helps also on ARMv6+. Probably, better to correct commit message for stable. > Signed-off-by: Anfei Zhou <anfei.zhou(a)gmail.com> > Signed-off-by: Russell King <rmk+kernel(a)arm.linux.org.uk> > Signed-off-by: Greg Kroah-Hartman <gregkh(a)suse.de> > > --- > arch/arm/mm/fault.c | 3 +++ > 1 file changed, 3 insertions(+) > > --- a/arch/arm/mm/fault.c > +++ b/arch/arm/mm/fault.c > @@ -386,6 +386,9 @@ do_translation_fault(unsigned long addr, > if (addr < TASK_SIZE) > return do_page_fault(addr, fsr, regs); > > + if (user_mode(regs)) > + goto bad_area; > + > index = pgd_index(addr); > > /* > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo(a)vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- Kirill A. Shutemov -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 1 Jul 2010 18:20 On Fri, Jul 02, 2010 at 01:14:20AM +0300, Kirill A. Shutemov wrote: > On Thu, Jul 01, 2010 at 10:31:37AM -0700, Greg KH wrote: > > 2.6.32-stable review patch. If anyone has any objections, please let us know. > > > > ------------------ > > > > From: Anfei <anfei.zhou(a)gmail.com> > > > > commit 5e27fb78df95e027723af2c90ecc9b4527ae59e9 upstream. > > > > Instruction faults on pre-ARMv6 CPUs are interpreted as > > a 'translation fault', but do_translation_fault doesn't > > handle well if user mode trying to run instruction above > > TASK_SIZE, and result in the infinite retry of that > > instruction. > > Actually, this patch helps also on ARMv6+. Probably, better to correct > commit message for stable. We (well, I) like to keep the commit log identical to what is upstream just to make things easier all around. Otherwise people start asking for spelling fixes, clarifications, and all sorts of other stuff (like this.) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Kirill A. Shutemov on 1 Jul 2010 18:30 On Thu, Jul 01, 2010 at 03:17:28PM -0700, Greg KH wrote: > On Fri, Jul 02, 2010 at 01:14:20AM +0300, Kirill A. Shutemov wrote: > > On Thu, Jul 01, 2010 at 10:31:37AM -0700, Greg KH wrote: > > > 2.6.32-stable review patch. If anyone has any objections, please let us know. > > > > > > ------------------ > > > > > > From: Anfei <anfei.zhou(a)gmail.com> > > > > > > commit 5e27fb78df95e027723af2c90ecc9b4527ae59e9 upstream. > > > > > > Instruction faults on pre-ARMv6 CPUs are interpreted as > > > a 'translation fault', but do_translation_fault doesn't > > > handle well if user mode trying to run instruction above > > > TASK_SIZE, and result in the infinite retry of that > > > instruction. > > > > Actually, this patch helps also on ARMv6+. Probably, better to correct > > commit message for stable. > > We (well, I) like to keep the commit log identical to what is upstream > just to make things easier all around. Otherwise people start asking > for spelling fixes, clarifications, and all sorts of other stuff (like > this.) Ok, fair enough. I asked for it because I was confused by this commit message while investigate (the same) problem on ARMv7 CPU. -- Kirill A. Shutemov -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Kirill A. Shutemov on 1 Jul 2010 19:00 On Thu, Jul 01, 2010 at 11:48:37PM +0100, Russell King wrote: > On Fri, Jul 02, 2010 at 01:25:41AM +0300, Kirill A. Shutemov wrote: > > On Thu, Jul 01, 2010 at 03:17:28PM -0700, Greg KH wrote: > > > We (well, I) like to keep the commit log identical to what is upstream > > > just to make things easier all around. Otherwise people start asking > > > for spelling fixes, clarifications, and all sorts of other stuff (like > > > this.) > > > > Ok, fair enough. > > > > I asked for it because I was confused by this commit message while > > investigate (the same) problem on ARMv7 CPU. > > You shouldn't get anywhere near this on ARMv7, because we know the cause > of the prefetch abort on those CPUs. > > On pre-ARMv6 CPUs, we always treat all prefetch aborts as a translation > faults. The problem which this commit addresses occurs when userspace > tries to execute code above TASK_SIZE - we're sent into a loop of prefetch > aborts (because we are unable to determine that it is a permission fault.) > > ARMv6 and ARMv7 CPUs have an instruction fault status register, which > tells us why the abort happened. On these CPUs, permission faults go > nowhere near the translation fault handler. I know it. I was involved in writing this code. > One possibility is that for some reason you're using the legacy prefetch > abort code or pre-IFSR code, which will always tell the kernel that its > a translation fault - and in this case, this patch would improve the > situation. What kernel version are you using? 2.6.32 > The commit message is accurate for the kernel version to which it was > originally applied. Simple testcase: #include <stdio.h> #include <unistd.h> #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> int main(int argc, char **argv) { int fd; void (*p)(void); fd = open("/dev/urandom", O_RDONLY); read(0, &p, sizeof(p)); printf("p: %p\n", p); p(); return 0; } If you run this test in loop on kernel without the patch you'll finally get hung instead SIGSEGV. It seems the patch fixes more than it was written for. :) -- Kirill A. Shutemov -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Kirill A. Shutemov on 2 Jul 2010 02:30
On Fri, Jul 02, 2010 at 12:12:07AM +0100, Russell King wrote: > On Fri, Jul 02, 2010 at 01:59:11AM +0300, Kirill A. Shutemov wrote: > > On Thu, Jul 01, 2010 at 11:48:37PM +0100, Russell King wrote: > > > One possibility is that for some reason you're using the legacy prefetch > > > abort code or pre-IFSR code, which will always tell the kernel that its > > > a translation fault - and in this case, this patch would improve the > > > situation. What kernel version are you using? > > > > 2.6.32 > > Should be recent enough. > > > If you run this test in loop on kernel without the patch you'll finally > > get hung instead SIGSEGV. > > > > It seems the patch fixes more than it was written for. :) > > Have you investigated the IFSR and IFAR values, and the corresponding > page table state? In oprofile dump, I saw do_PrefetchAbort() and do_translation_fault() at the top, so I guess IFSR is 5. I don't known value IFAR, but I'll see. > I'm not going to be able to run your test code for > a few days. It's only few minutes or less if you are lucky. -- Kirill A. Shutemov -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |