| Message ID | 20131027195115.208f40f3@tom-ThinkPad-T410 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Sun, Oct 27, 2013 at 07:51:15PM +0800, Ming Lei wrote: > On Sat, 26 Oct 2013 15:36:17 +0100 > Will Deacon <will.deacon@arm.com> wrote: > > > On Thu, Oct 24, 2013 at 09:07:30PM +0100, Aaro Koskinen wrote: > > > > > [ 36.477203] Backtrace: > > > [ 36.535603] [<c009237c>] (page_mapping+0x0/0x50) from [<c0010dd8>] (flush_kernel_dcache_page+0x14/0x98) > > > [ 36.661070] [<c0010dc4>] (flush_kernel_dcache_page+0x0/0x98) from [<c0172b60>] (sg_miter_stop+0xc8/0x10c) > > > [ 36.792813] r4:df8a9a64 r3:00000003 > > > [ 36.857524] [<c0172a98>] (sg_miter_stop+0x0/0x10c) from [<c0172f20>] (sg_miter_next+0x14/0x13c) > > > > ... assumedly for scatter/gather DMA. How is your block driver allocating > > its buffers? If you're using the DMA API, I can't see how this would happen. > > Lots of SCSI commands(inquiry, ...) pass kmalloc buffer to block layer, > then the sg buffer copy helpers and flush_kernel_dcache_page() may see > slab page. > > That has been here from commit b1adaf65ba03( [SCSI] block: add sg buffer copy > helper functions). 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@gmail.com> Date: Mon Jun 10 21:10:12 2013 +0100 ARM: 7755/1: handle user space mapped pages in flush_kernel_dcache_page A.
On Sun, Oct 27, 2013 at 8:50 PM, Aaro Koskinen <aaro.koskinen@iki.fi> 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@gmail.com> > 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. Thanks,
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@iki.fi> 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@gmail.com> > > 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 don't think PageSlab() is the right test tho. Wouldn't testing against user_addr_max() make more sense? Thanks.
On Sun, Oct 27, 2013 at 09:42:57AM -0400, Tejun Heo wrote: > 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@iki.fi> 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@gmail.com> > > > 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 don't think PageSlab() is the right test tho. Wouldn't testing > against user_addr_max() make more sense? How does that help for a function passed a struct page pointer?
On Sun, Oct 27, 2013 at 07:51:15PM +0800, Ming Lei wrote: > diff --git a/lib/scatterlist.c b/lib/scatterlist.c > index a685c8a..eea8806 100644 > --- a/lib/scatterlist.c > +++ b/lib/scatterlist.c > @@ -577,7 +577,7 @@ void sg_miter_stop(struct sg_mapping_iter *miter) > miter->__offset += miter->consumed; > miter->__remaining -= miter->consumed; > > - if (miter->__flags & SG_MITER_TO_SG) > + if ((miter->__flags & SG_MITER_TO_SG) && !PageSlab(page)) This is what I was going to propose, but I would have used !PageSlab(miter->page) ;-) > flush_kernel_dcache_page(miter->page); With this, a kernel with DEBUG_VM now boots on Kirkwood. - Simon
On Mon, Oct 28, 2013 at 10:13 PM, Simon Baatz <gmbnomis@gmail.com> wrote: > On Sun, Oct 27, 2013 at 07:51:15PM +0800, Ming Lei wrote: >> diff --git a/lib/scatterlist.c b/lib/scatterlist.c >> index a685c8a..eea8806 100644 >> --- a/lib/scatterlist.c >> +++ b/lib/scatterlist.c >> @@ -577,7 +577,7 @@ void sg_miter_stop(struct sg_mapping_iter *miter) >> miter->__offset += miter->consumed; >> miter->__remaining -= miter->consumed; >> >> - if (miter->__flags & SG_MITER_TO_SG) >> + if ((miter->__flags & SG_MITER_TO_SG) && !PageSlab(page)) > > This is what I was going to propose, but I would have used > !PageSlab(miter->page) ;-) OK, I will send a formal one later, thank you for pointing out the above, :-) > >> flush_kernel_dcache_page(miter->page); > > With this, a kernel with DEBUG_VM now boots on Kirkwood. Thanks,
diff --git a/lib/scatterlist.c b/lib/scatterlist.c index a685c8a..eea8806 100644 --- a/lib/scatterlist.c +++ b/lib/scatterlist.c @@ -577,7 +577,7 @@ void sg_miter_stop(struct sg_mapping_iter *miter) miter->__offset += miter->consumed; miter->__remaining -= miter->consumed; - if (miter->__flags & SG_MITER_TO_SG) + if ((miter->__flags & SG_MITER_TO_SG) && !PageSlab(page)) flush_kernel_dcache_page(miter->page); if (miter->__flags & SG_MITER_ATOMIC) {