All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King <rmk+lkml@arm.linux.org.uk>
To: Adrian Hunter <adrian.hunter@nokia.com>
Cc: "dwmw2@infradead.org" <dwmw2@infradead.org>,
	"Bityutskiy Artem \(Nokia-D/Helsinki\)"
	<Artem.Bityutskiy@nokia.com>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	JiSheng Zhang <jszhang3@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [UBI UBIFS] replace vmalloc with kmalloc
Date: Fri, 7 Aug 2009 10:44:04 +0100	[thread overview]
Message-ID: <20090807094404.GA10603@flint.arm.linux.org.uk> (raw)
In-Reply-To: <4A7BF1C3.4050602@nokia.com>

On Fri, Aug 07, 2009 at 12:20:03PM +0300, Adrian Hunter wrote:
> vmalloc allows large (> 128KiB) buffers, but kmalloc doesn't.
> So we presently have no choice but to use vmalloc.
>
> I do not know what hardware you have or exactly what driver you
> are using, but we have UBIFS on OneNAND using DMA.
> See drivers/mtd/onenand/omap2.c

Hmm.  Looking at that code, it's unsafe:

        if (buf >= high_memory) {
                struct page *p1;

                if (((size_t)buf & PAGE_MASK) !=
                    ((size_t)(buf + count - 1) & PAGE_MASK))
                        goto out_copy;
                p1 = vmalloc_to_page(buf);
                if (!p1)
                        goto out_copy;
                buf = page_address(p1) + ((size_t)buf & ~PAGE_MASK);
        }
...
        dma_dst = dma_map_single(&c->pdev->dev, buf, count, DMA_FROM_DEVICE);

If you consider a VIVT cache, and with vmalloc'd pages you're passing
a *different* virtual address to the DMA functions, it's not going to
touch the cachelines associated with the vmalloc mapping.

The above *may* work for VIPT caches provided both the vmalloc and
kernel direct mappings are of the same colour.  If not, this really
isn't going to be reliable for the same reason.

Basically, what's going in there is *totally* unsafe.  I hope you
don't place important data on this NAND device.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

WARNING: multiple messages have this Message-ID (diff)
From: Russell King <rmk+lkml@arm.linux.org.uk>
To: Adrian Hunter <adrian.hunter@nokia.com>
Cc: JiSheng Zhang <jszhang3@gmail.com>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dwmw2@infradead.org" <dwmw2@infradead.org>,
	"Bityutskiy Artem (Nokia-D/Helsinki)"
	<Artem.Bityutskiy@nokia.com>
Subject: Re: [UBI UBIFS] replace vmalloc with kmalloc
Date: Fri, 7 Aug 2009 10:44:04 +0100	[thread overview]
Message-ID: <20090807094404.GA10603@flint.arm.linux.org.uk> (raw)
In-Reply-To: <4A7BF1C3.4050602@nokia.com>

On Fri, Aug 07, 2009 at 12:20:03PM +0300, Adrian Hunter wrote:
> vmalloc allows large (> 128KiB) buffers, but kmalloc doesn't.
> So we presently have no choice but to use vmalloc.
>
> I do not know what hardware you have or exactly what driver you
> are using, but we have UBIFS on OneNAND using DMA.
> See drivers/mtd/onenand/omap2.c

Hmm.  Looking at that code, it's unsafe:

        if (buf >= high_memory) {
                struct page *p1;

                if (((size_t)buf & PAGE_MASK) !=
                    ((size_t)(buf + count - 1) & PAGE_MASK))
                        goto out_copy;
                p1 = vmalloc_to_page(buf);
                if (!p1)
                        goto out_copy;
                buf = page_address(p1) + ((size_t)buf & ~PAGE_MASK);
        }
...
        dma_dst = dma_map_single(&c->pdev->dev, buf, count, DMA_FROM_DEVICE);

If you consider a VIVT cache, and with vmalloc'd pages you're passing
a *different* virtual address to the DMA functions, it's not going to
touch the cachelines associated with the vmalloc mapping.

The above *may* work for VIPT caches provided both the vmalloc and
kernel direct mappings are of the same colour.  If not, this really
isn't going to be reliable for the same reason.

Basically, what's going in there is *totally* unsafe.  I hope you
don't place important data on this NAND device.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

  reply	other threads:[~2009-08-07  9:44 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-07  9:02 [UBI UBIFS] replace vmalloc with kmalloc JiSheng Zhang
2009-08-07  9:02 ` JiSheng Zhang
2009-08-07  9:14 ` Matthieu CASTET
2009-08-07  9:14   ` Matthieu CASTET
2009-08-07  9:20 ` Adrian Hunter
2009-08-07  9:20   ` Adrian Hunter
2009-08-07  9:44   ` Russell King [this message]
2009-08-07  9:44     ` Russell King
2009-08-07 14:26   ` Josh Boyer
2009-08-07 14:26     ` Josh Boyer
2009-08-07 15:28   ` JiSheng Zhang
2009-08-07 15:28     ` JiSheng Zhang
2009-08-09  5:35     ` Artem Bityutskiy
2009-08-09  5:35       ` Artem Bityutskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090807094404.GA10603@flint.arm.linux.org.uk \
    --to=rmk+lkml@arm.linux.org.uk \
    --cc=Artem.Bityutskiy@nokia.com \
    --cc=adrian.hunter@nokia.com \
    --cc=dwmw2@infradead.org \
    --cc=jszhang3@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.