headers
Rabin Vincent | 1 Jan 2011 11:36
Picon

[PATCH] mmci: call flush_dcache_page() outside of atomic kmap

While booting a QEMU Versatile/PB system with the root file system on
SD, the following WARN_ON hits:

------------[ cut here ]------------
WARNING: at /home/rabin/kernel/arm/lib/scatterlist.c:426 sg_miter_stop+0x64/0x9c()
Modules linked in:
[<c0031da4>] (unwind_backtrace+0x0/0xe4) from [<c003d5c0>] (warn_slowpath_common+0x4c/0x64)
[<c003d5c0>] (warn_slowpath_common+0x4c/0x64) from [<c003d5f0>] (warn_slowpath_null+0x18/0x1c)
[<c003d5f0>] (warn_slowpath_null+0x18/0x1c) from [<c0148338>] (sg_miter_stop+0x64/0x9c)
[<c0148338>] (sg_miter_stop+0x64/0x9c) from [<c01a892c>] (mmci_pio_irq+0x1fc/0x270)
[<c01a892c>] (mmci_pio_irq+0x1fc/0x270) from [<c0065928>] (handle_IRQ_event+0x24/0xf0)
[<c0065928>] (handle_IRQ_event+0x24/0xf0) from [<c0067930>] (handle_level_irq+0xa4/0x114)
[<c0067930>] (handle_level_irq+0xa4/0x114) from [<c0036258>] (sic_handle_irq+0x50/0x60)
[<c0036258>] (sic_handle_irq+0x50/0x60) from [<c0022070>] (asm_do_IRQ+0x70/0x94)
[<c0022070>] (asm_do_IRQ+0x70/0x94) from [<c002c0b4>] (__irq_svc+0x34/0xa0)

It's the WARN_ON(!irqs_disabled()) in sg_miter_stop():

                if (miter->__flags & SG_MITER_ATOMIC) {
                        WARN_ON(!irqs_disabled());
                        kunmap_atomic(miter->addr, KM_BIO_SRC_IRQ);
                }

This is because if the cache is VIVT, flush_dcache_page() calls
__flush_dcache_aliases() when a user space mapping exists.  That
function uses flush_dcache_mmap_unlock() which is spin_unlock_irq(),
which enables interrupts.   Fix this by calling flush_dcache_page() only
after the sg_miter is stopped.

Signed-off-by: Rabin Vincent <rabin <at> rab.in>
(Continue reading)

Permalink | Reply |
headers
Russell King - ARM Linux | 1 Jan 2011 13:05
Picon

Re: [PATCH] mmci: call flush_dcache_page() outside of atomic kmap

On Sat, Jan 01, 2011 at 04:06:08PM +0530, Rabin Vincent wrote:
> While booting a QEMU Versatile/PB system with the root file system on
> SD, the following WARN_ON hits:
> 
> ------------[ cut here ]------------
> WARNING: at /home/rabin/kernel/arm/lib/scatterlist.c:426 sg_miter_stop+0x64/0x9c()
> Modules linked in:
> [<c0031da4>] (unwind_backtrace+0x0/0xe4) from [<c003d5c0>] (warn_slowpath_common+0x4c/0x64)
> [<c003d5c0>] (warn_slowpath_common+0x4c/0x64) from [<c003d5f0>] (warn_slowpath_null+0x18/0x1c)
> [<c003d5f0>] (warn_slowpath_null+0x18/0x1c) from [<c0148338>] (sg_miter_stop+0x64/0x9c)
> [<c0148338>] (sg_miter_stop+0x64/0x9c) from [<c01a892c>] (mmci_pio_irq+0x1fc/0x270)
> [<c01a892c>] (mmci_pio_irq+0x1fc/0x270) from [<c0065928>] (handle_IRQ_event+0x24/0xf0)
> [<c0065928>] (handle_IRQ_event+0x24/0xf0) from [<c0067930>] (handle_level_irq+0xa4/0x114)
> [<c0067930>] (handle_level_irq+0xa4/0x114) from [<c0036258>] (sic_handle_irq+0x50/0x60)
> [<c0036258>] (sic_handle_irq+0x50/0x60) from [<c0022070>] (asm_do_IRQ+0x70/0x94)
> [<c0022070>] (asm_do_IRQ+0x70/0x94) from [<c002c0b4>] (__irq_svc+0x34/0xa0)
> 
> It's the WARN_ON(!irqs_disabled()) in sg_miter_stop():
> 
>                 if (miter->__flags & SG_MITER_ATOMIC) {
>                         WARN_ON(!irqs_disabled());
>                         kunmap_atomic(miter->addr, KM_BIO_SRC_IRQ);
>                 }
> 
> This is because if the cache is VIVT, flush_dcache_page() calls
> __flush_dcache_aliases() when a user space mapping exists.  That
> function uses flush_dcache_mmap_unlock() which is spin_unlock_irq(),
> which enables interrupts.   Fix this by calling flush_dcache_page() only
> after the sg_miter is stopped.
(Continue reading)

Permalink | Reply |
headers
Rabin Vincent | 1 Jan 2011 15:28
Picon

Re: [PATCH] mmci: call flush_dcache_page() outside of atomic kmap

On Sat, Jan 1, 2011 at 5:35 PM, Russell King - ARM Linux
<linux <at> arm.linux.org.uk> wrote:
> On Sat, Jan 01, 2011 at 04:06:08PM +0530, Rabin Vincent wrote:
>> It's the WARN_ON(!irqs_disabled()) in sg_miter_stop():
>>
>>                 if (miter->__flags & SG_MITER_ATOMIC) {
>>                         WARN_ON(!irqs_disabled());
>>                         kunmap_atomic(miter->addr, KM_BIO_SRC_IRQ);
>>                 }
>>
>> This is because if the cache is VIVT, flush_dcache_page() calls
>> __flush_dcache_aliases() when a user space mapping exists.  That
>> function uses flush_dcache_mmap_unlock() which is spin_unlock_irq(),
>> which enables interrupts.   Fix this by calling flush_dcache_page() only
>> after the sg_miter is stopped.
>
> I think there's some questions that need to be answered here first:
>
> 1. Why does this not trigger on real PB926 hardware?

Normal read/write does not trigger this, it only happens when a file on
the SD card is executed (in my case it first happens when a page in
init's data segment is faulted in) .  Has rootfs-on-SD been tried on
real PB926 hardware after the conversion to the sg_miter API?

> 2. Why the hell is a page being submitted which is mapped into userspace
>   yet has not already been populated with data from the card?
>
> (2) is a serious error, what it means is that userspace can access the
> data which was _previously_ in the page before the page has been read
(Continue reading)

Permalink | Reply |
headers
Russell King - ARM Linux | 1 Jan 2011 23:13
Picon

Re: [PATCH] mmci: call flush_dcache_page() outside of atomic kmap

On Sat, Jan 01, 2011 at 07:58:03PM +0530, Rabin Vincent wrote:
> On Sat, Jan 1, 2011 at 5:35 PM, Russell King - ARM Linux
> <linux <at> arm.linux.org.uk> wrote:
> > On Sat, Jan 01, 2011 at 04:06:08PM +0530, Rabin Vincent wrote:
> >> It's the WARN_ON(!irqs_disabled()) in sg_miter_stop():
> >>
> >>                 if (miter->__flags & SG_MITER_ATOMIC) {
> >>                         WARN_ON(!irqs_disabled());
> >>                         kunmap_atomic(miter->addr, KM_BIO_SRC_IRQ);
> >>                 }
> >>
> >> This is because if the cache is VIVT, flush_dcache_page() calls
> >> __flush_dcache_aliases() when a user space mapping exists.  That
> >> function uses flush_dcache_mmap_unlock() which is spin_unlock_irq(),
> >> which enables interrupts.   Fix this by calling flush_dcache_page() only
> >> after the sg_miter is stopped.
> >
> > I think there's some questions that need to be answered here first:
> >
> > 1. Why does this not trigger on real PB926 hardware?
> 
> Normal read/write does not trigger this, it only happens when a file on
> the SD card is executed (in my case it first happens when a page in
> init's data segment is faulted in) .  Has rootfs-on-SD been tried on
> real PB926 hardware after the conversion to the sg_miter API?
> 
> > 2. Why the hell is a page being submitted which is mapped into userspace
> >   yet has not already been populated with data from the card?
> >
> > (2) is a serious error, what it means is that userspace can access the
(Continue reading)

Permalink | Reply |
headers
Rabin Vincent | 2 Jan 2011 05:38
Picon

Re: [PATCH] mmci: call flush_dcache_page() outside of atomic kmap

On Sun, Jan 2, 2011 at 3:43 AM, Russell King - ARM Linux
<linux <at> arm.linux.org.uk> wrote:
> Err, hang on.  The sg iter API uses flush_kernel_dcache_page(), not
> flush_dcache_page().  flush_kernel_dcache_page() is not expected to
> touch userspace mappings.

This email thread was never about the flush_kernel_dcache_page() in the
sg_miter API.  It's about the flush_dcache_page() in MMCI.
Permalink | Reply |
headers
Rabin Vincent | 2 Jan 2011 06:11
Picon

Re: [PATCH] mmci: call flush_dcache_page() outside of atomic kmap

On Sun, Jan 2, 2011 at 10:08 AM, Rabin Vincent <rabin <at> rab.in> wrote:
> On Sun, Jan 2, 2011 at 3:43 AM, Russell King - ARM Linux
> <linux <at> arm.linux.org.uk> wrote:
>> Err, hang on.  The sg iter API uses flush_kernel_dcache_page(), not
>> flush_dcache_page().  flush_kernel_dcache_page() is not expected to
>> touch userspace mappings.
>>
>> On ARM, we don't implement flush_kernel_dcache_page() because it's
>> not required (see commit f8b63c1.)  Essentially, because we now consider
>> freshly created page cache pages dirty, we always flush them before we
>> map them into userspace, so a call to flush_kernel_dcache_page() has
>> nothing to do.
>
> This email thread was never about the flush_kernel_dcache_page() in the
> sg_miter API.  It's about the flush_dcache_page() in MMCI.

This flush_dcache_page() can also be removed because of what you mention
above, right?
Permalink | Reply |
headers
Rabin Vincent | 2 Jan 2011 06:20
Picon

Re: [PATCH] mmci: call flush_dcache_page() outside of atomic kmap

On Sun, Jan 2, 2011 at 10:41 AM, Rabin Vincent <rabin <at> rab.in> wrote:
> On Sun, Jan 2, 2011 at 10:08 AM, Rabin Vincent <rabin <at> rab.in> wrote:
>> On Sun, Jan 2, 2011 at 3:43 AM, Russell King - ARM Linux
>> <linux <at> arm.linux.org.uk> wrote:
>>> Err, hang on.  The sg iter API uses flush_kernel_dcache_page(), not
>>> flush_dcache_page().  flush_kernel_dcache_page() is not expected to
>>> touch userspace mappings.
>>>
>>> On ARM, we don't implement flush_kernel_dcache_page() because it's
>>> not required (see commit f8b63c1.)  Essentially, because we now consider
>>> freshly created page cache pages dirty, we always flush them before we
>>> map them into userspace, so a call to flush_kernel_dcache_page() has
>>> nothing to do.
>>
>> This email thread was never about the flush_kernel_dcache_page() in the
>> sg_miter API.  It's about the flush_dcache_page() in MMCI.
>
> This flush_dcache_page() can also be removed because of what you mention
> above, right?

From f54a734dc2749b0e2ea93b851a8233a47a93e620 Mon Sep 17 00:00:00 2001
From: Rabin Vincent <rabin <at> rab.in>
Date: Sat, 1 Jan 2011 15:42:34 +0530
Subject: [PATCH] mmci: don't flush the dcache

Since freshly created page cache pages are considered dirty and are
always flushed before they are mapped into userspace, and an
already-mapped page will never be passed to be read into by the mmci
driver, remove the flush_dcache_page() calls.
(Continue reading)

Permalink | Reply |

Gmane