From: Boris Brezillon <boris.brezillon@free-electrons.com> To: Frode Isaksen <fisaksen@baylibre.com> Cc: Vignesh R <vigneshr@ti.com>, Mark Brown <broonie@kernel.org>, Cyrille Pitchen <cyrille.pitchen@atmel.com>, Richard Weinberger <richard@nod.at>, David Woodhouse <dwmw2@infradead.org>, Brian Norris <computersforpeace@gmail.com>, Marek Vasut <marek.vasut@gmail.com>, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org, linux-spi@vger.kernel.org Subject: Re: [RFC PATCH 2/2] mtd: devices: m25p80: Enable spi-nor bounce buffer support Date: Thu, 2 Mar 2017 16:25:56 +0100 [thread overview] Message-ID: <20170302162556.76b0ae8c@bbrezillon> (raw) In-Reply-To: <341ef45d-bad5-fd7c-aa05-807041c35f42@baylibre.com> On Thu, 2 Mar 2017 16:03:17 +0100 Frode Isaksen <fisaksen@baylibre.com> wrote: > On 02/03/2017 15:29, Boris Brezillon wrote: > > On Thu, 2 Mar 2017 19:24:43 +0530 > > Vignesh R <vigneshr@ti.com> wrote: > > > >>>>>> > >>>>> Not really, I am debugging another issue with UBIFS on DRA74 EVM (ARM > >>>>> cortex-a15) wherein pages allocated by vmalloc are in highmem region > >>>>> that are not addressable using 32 bit addresses and is backed by LPAE. > >>>>> So, a 32 bit DMA cannot access these buffers at all. > >>>>> When dma_map_sg() is called to map these pages by spi_map_buf() the > >>>>> physical address is just truncated to 32 bit in pfn_to_dma() (as part of > >>>>> dma_map_sg() call). This results in random crashes as DMA starts > >>>>> accessing random memory during SPI read. > >>>>> > >>>>> IMO, there may be more undiscovered caveat with using dma_map_sg() for > >>>>> non kmalloc'd buffers and its better that spi-nor starts handling these > >>>>> buffers instead of relying on spi_map_msg() and working around every > >>>>> time something pops up. > >>>>> > >>>> Ok, I had a closer look at the SPI framework, and it seems there's a > >>>> way to tell to the core that a specific transfer cannot use DMA > >>>> (->can_dam()). The first thing you should do is fix the spi-davinci > >>>> driver: > >>>> > >>>> 1/ implement ->can_dma() > >>>> 2/ patch davinci_spi_bufs() to take the decision to do DMA or not on a > >>>> per-xfer basis and not on a per-device basis > >>>> > >> This would lead to poor perf defeating entire purpose of using DMA. > > Hm, that's not really true. For all cases where you have a DMA-able > > buffer it would still use DMA. For other cases (like the UBI+SPI-NOR > > case we're talking about here), yes, it will be slower, but slower is > > still better than buggy. > > So, in any case, I think the fixes pointed by Frode are needed. > Also, I think the UBIFS layer only uses vmalloc'ed buffers during > mount/unmount and not for read/write, so the performance hit is not > that big. It's a bit more complicated than that. You may have operations running in background that are using those big vmalloc-ed buffers at runtime. To optimize things, we really need to split LEB/PEB buffers into multiple ->max_write_size (or ->min_io_size) kmalloc-ed buffers. > In most cases the buffer is the size of the erase block, but I've seen > vmalloc'ed buffer of size only 11 bytes ! So, to optimize this, the > best solution is probably to change how the UBIFS layer is using > vmalloc'ed vs kmalloc'ed buffers, since vmalloc'ed should only be used > for large (> 128K) buffers. Hm, the buffer itself is bigger than 11 bytes, it's just that the same buffer is used in different use cases, and sometime we're only partially filling it.
WARNING: multiple messages have this Message-ID (diff)
From: Boris Brezillon <boris.brezillon@free-electrons.com> To: Frode Isaksen <fisaksen@baylibre.com> Cc: linux-omap@vger.kernel.org, Vignesh R <vigneshr@ti.com>, Richard Weinberger <richard@nod.at>, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org, Marek Vasut <marek.vasut@gmail.com>, Mark Brown <broonie@kernel.org>, linux-mtd@lists.infradead.org, Cyrille Pitchen <cyrille.pitchen@atmel.com>, Brian Norris <computersforpeace@gmail.com>, David Woodhouse <dwmw2@infradead.org> Subject: Re: [RFC PATCH 2/2] mtd: devices: m25p80: Enable spi-nor bounce buffer support Date: Thu, 2 Mar 2017 16:25:56 +0100 [thread overview] Message-ID: <20170302162556.76b0ae8c@bbrezillon> (raw) In-Reply-To: <341ef45d-bad5-fd7c-aa05-807041c35f42@baylibre.com> On Thu, 2 Mar 2017 16:03:17 +0100 Frode Isaksen <fisaksen@baylibre.com> wrote: > On 02/03/2017 15:29, Boris Brezillon wrote: > > On Thu, 2 Mar 2017 19:24:43 +0530 > > Vignesh R <vigneshr@ti.com> wrote: > > > >>>>>> > >>>>> Not really, I am debugging another issue with UBIFS on DRA74 EVM (ARM > >>>>> cortex-a15) wherein pages allocated by vmalloc are in highmem region > >>>>> that are not addressable using 32 bit addresses and is backed by LPAE. > >>>>> So, a 32 bit DMA cannot access these buffers at all. > >>>>> When dma_map_sg() is called to map these pages by spi_map_buf() the > >>>>> physical address is just truncated to 32 bit in pfn_to_dma() (as part of > >>>>> dma_map_sg() call). This results in random crashes as DMA starts > >>>>> accessing random memory during SPI read. > >>>>> > >>>>> IMO, there may be more undiscovered caveat with using dma_map_sg() for > >>>>> non kmalloc'd buffers and its better that spi-nor starts handling these > >>>>> buffers instead of relying on spi_map_msg() and working around every > >>>>> time something pops up. > >>>>> > >>>> Ok, I had a closer look at the SPI framework, and it seems there's a > >>>> way to tell to the core that a specific transfer cannot use DMA > >>>> (->can_dam()). The first thing you should do is fix the spi-davinci > >>>> driver: > >>>> > >>>> 1/ implement ->can_dma() > >>>> 2/ patch davinci_spi_bufs() to take the decision to do DMA or not on a > >>>> per-xfer basis and not on a per-device basis > >>>> > >> This would lead to poor perf defeating entire purpose of using DMA. > > Hm, that's not really true. For all cases where you have a DMA-able > > buffer it would still use DMA. For other cases (like the UBI+SPI-NOR > > case we're talking about here), yes, it will be slower, but slower is > > still better than buggy. > > So, in any case, I think the fixes pointed by Frode are needed. > Also, I think the UBIFS layer only uses vmalloc'ed buffers during > mount/unmount and not for read/write, so the performance hit is not > that big. It's a bit more complicated than that. You may have operations running in background that are using those big vmalloc-ed buffers at runtime. To optimize things, we really need to split LEB/PEB buffers into multiple ->max_write_size (or ->min_io_size) kmalloc-ed buffers. > In most cases the buffer is the size of the erase block, but I've seen > vmalloc'ed buffer of size only 11 bytes ! So, to optimize this, the > best solution is probably to change how the UBIFS layer is using > vmalloc'ed vs kmalloc'ed buffers, since vmalloc'ed should only be used > for large (> 128K) buffers. Hm, the buffer itself is bigger than 11 bytes, it's just that the same buffer is used in different use cases, and sometime we're only partially filling it. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2017-03-02 15:54 UTC|newest] Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-02-27 12:08 [RFC PATCH 0/2] mtd: spi-nor: Handle vmalloc'd buffers Vignesh R 2017-02-27 12:08 ` Vignesh R 2017-02-27 12:08 ` [RFC PATCH 1/2] mtd: spi-nor: Introduce bounce buffer to handle " Vignesh R 2017-02-27 12:08 ` Vignesh R 2017-02-28 21:39 ` Richard Weinberger 2017-02-28 21:39 ` Richard Weinberger 2017-03-01 5:13 ` Vignesh R 2017-03-01 5:13 ` Vignesh R 2017-03-01 10:09 ` Cyrille Pitchen 2017-03-01 10:09 ` Cyrille Pitchen 2017-03-01 10:18 ` Boris Brezillon 2017-03-01 10:18 ` Boris Brezillon 2017-03-01 11:18 ` Frode Isaksen 2017-03-01 11:18 ` Frode Isaksen 2017-03-01 12:12 ` Boris Brezillon 2017-03-01 12:12 ` Boris Brezillon 2017-03-01 11:50 ` Vignesh R 2017-03-01 11:50 ` Vignesh R 2017-02-27 12:08 ` [RFC PATCH 2/2] mtd: devices: m25p80: Enable spi-nor bounce buffer support Vignesh R 2017-02-27 12:08 ` Vignesh R 2017-02-28 21:41 ` Richard Weinberger 2017-03-01 4:54 ` Vignesh R 2017-03-01 4:54 ` Vignesh R 2017-03-01 10:43 ` Cyrille Pitchen 2017-03-01 10:43 ` Cyrille Pitchen 2017-03-01 11:14 ` Frode Isaksen 2017-03-01 11:14 ` Frode Isaksen 2017-03-01 11:46 ` Vignesh R 2017-03-01 11:46 ` Vignesh R 2017-03-01 11:46 ` Vignesh R 2017-03-01 12:23 ` Boris Brezillon 2017-03-01 12:23 ` Boris Brezillon 2017-03-01 14:21 ` Cyrille Pitchen 2017-03-01 14:21 ` Cyrille Pitchen 2017-03-01 14:28 ` Boris Brezillon 2017-03-01 14:28 ` Boris Brezillon 2017-03-01 14:28 ` Boris Brezillon 2017-03-01 14:30 ` Cyrille Pitchen 2017-03-01 14:30 ` Cyrille Pitchen 2017-03-01 14:30 ` Cyrille Pitchen 2017-03-01 15:52 ` Mark Brown 2017-03-01 16:04 ` Boris Brezillon 2017-03-01 16:04 ` Boris Brezillon 2017-03-01 16:55 ` Boris Brezillon 2017-03-01 16:55 ` Boris Brezillon 2017-03-02 9:06 ` Frode Isaksen 2017-03-02 9:06 ` Frode Isaksen 2017-03-02 13:54 ` Vignesh R 2017-03-02 13:54 ` Vignesh R 2017-03-02 14:29 ` Boris Brezillon 2017-03-02 14:29 ` Boris Brezillon 2017-03-02 15:03 ` Frode Isaksen 2017-03-02 15:03 ` Frode Isaksen 2017-03-02 15:25 ` Boris Brezillon [this message] 2017-03-02 15:25 ` Boris Brezillon 2017-03-03 9:02 ` Frode Isaksen 2017-03-03 9:02 ` Frode Isaksen 2017-03-02 16:45 ` Cyrille Pitchen 2017-03-02 16:45 ` Cyrille Pitchen 2017-03-02 17:00 ` Mark Brown 2017-03-02 17:00 ` Mark Brown 2017-03-02 19:49 ` Boris Brezillon 2017-03-02 19:49 ` Boris Brezillon 2017-03-03 12:50 ` Mark Brown 2017-03-03 12:50 ` Mark Brown 2017-03-06 11:47 ` Vignesh R 2017-03-06 11:47 ` Vignesh R 2017-03-14 13:21 ` Vignesh R 2017-02-27 14:03 ` [RFC PATCH 0/2] mtd: spi-nor: Handle vmalloc'd buffers Frode Isaksen 2017-02-27 14:03 ` Frode Isaksen
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=20170302162556.76b0ae8c@bbrezillon \ --to=boris.brezillon@free-electrons.com \ --cc=broonie@kernel.org \ --cc=computersforpeace@gmail.com \ --cc=cyrille.pitchen@atmel.com \ --cc=dwmw2@infradead.org \ --cc=fisaksen@baylibre.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mtd@lists.infradead.org \ --cc=linux-omap@vger.kernel.org \ --cc=linux-spi@vger.kernel.org \ --cc=marek.vasut@gmail.com \ --cc=richard@nod.at \ --cc=vigneshr@ti.com \ /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: linkBe 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.