Re: ARM/kirkwood: v3.12-rc6: kernel BUG at mm/util.c:390!

[Russell King - ARM Linux]

On Sun, Oct 27, 2013 at 09:16:53PM +0800, Ming Lei wrote:
> On Sun, Oct 27, 2013 at 8:50 PM, Aaro Koskinen <aaro.koskinen@xxxxxx> wrote:
> >
> > On ARM v3.9 or older kernels do not trigger this BUG, at seems it only
> > started to appear with the following commit (bisected):
> >
> > commit 1bc39742aab09248169ef9d3727c9def3528b3f3
> > Author: Simon Baatz <gmbnomis@xxxxxxxxx>
> > Date:   Mon Jun 10 21:10:12 2013 +0100
> >
> >     ARM: 7755/1: handle user space mapped pages in flush_kernel_dcache_page
> 
> The above commit only starts to implement the helper on ARM,
> but according to Documentation/cachetlb.txt, looks caller of
> flush_kernel_dcache_page() should make sure the passed
> 'page' is a user space page.

I think your terminology is off.  flush_kernel_dcache_page() is passed a
struct page.  These exist for every physical RAM page in the system which
is under the control of the kernel.  There's no such thing as a "user
space page" - pages are shared from kernel space into userspace.

Secondly, flush_kernel_dcache_page() gets used on such pages whether or
not they're already mapped into userspace (normally they won't be if this
is the first read of the page.)  This function is only expected to deal
with kernel-side addresses of the page, ensuring that data in the page
is visible to the underlying memory.

The last thing to realise is that we already have a function which deals
with the presence of userspace mappings.  It's called flush_dcache_page().
If flush_kernel_dcache_page() had to make that decision, then there's no
point in flush_kernel_dcache_page() existing - we might as well just call
flush_dcache_page() directly.

So...

flush_kernel_dcache_page() is expected to take a struct page pointer.
This struct page pointer is part of the kernel's array of struct pages
which identifies every single physical page under the control of the
kernel.

Arguably, it should not crash if passed a page which has been allocated
to the slab cache; as this is not a page cache page,
flush_kernel_dcache_page() should merely ignore the call to it and
simply return on these.  So this makes total sense:

 arch/arm/mm/flush.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index 6d5ba9afb16a..eebb275a67fb 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -316,6 +316,10 @@ EXPORT_SYMBOL(flush_dcache_page);
  */
 void flush_kernel_dcache_page(struct page *page)
 {
+	/* Ignore slab pages */
+	if (PageSlab(page))
+		return;
+
 	if (cache_is_vivt() || cache_is_vipt_aliasing()) {
 		struct address_space *mapping;
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/