From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752753AbeDKHPl (ORCPT ); Wed, 11 Apr 2018 03:15:41 -0400 Received: from mail.bootlin.com ([62.4.15.54]:41082 "EHLO mail.bootlin.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752693AbeDKHPj (ORCPT ); Wed, 11 Apr 2018 03:15:39 -0400 Date: Wed, 11 Apr 2018 09:15:28 +0200 From: Boris Brezillon To: Ladislav Michl Cc: Andreas Kemnade , Discussions about the Letux Kernel , Boris Brezillon , Aaro Koskinen , Tony Lindgren , Linux Kernel Mailing List , Peter Ujfalusi , linux-omap , Roger Quadros Subject: Re: [Letux-kernel] [Bug]: mtd: onenand: omap2plus: kernel panic with OneNAND on OMAP3 (DM3730) device GTA04A5 Message-ID: <20180411091528.4e954c32@bbrezillon> In-Reply-To: <20180411062607.GA9179@lenoch> References: <5D496D5C-4E3E-47B4-9981-E8F4C348DE00@goldelico.com> <20180410205643.GA2228@lenoch> <20180411065836.7e1bfc3f@aktux> <20180411062607.GA9179@lenoch> X-Mailer: Claws Mail 3.15.0-dirty (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Ladislav, On Wed, 11 Apr 2018 08:26:07 +0200 Ladislav Michl wrote: > Hi Andreas, > > On Wed, Apr 11, 2018 at 06:59:03AM +0200, Andreas Kemnade wrote: > > Hi Ladis, > > > > On Tue, 10 Apr 2018 22:56:43 +0200 > > Ladislav Michl wrote: > > > > > Hi Nikolaus, > > > > > > On Tue, Apr 10, 2018 at 06:25:17PM +0200, H. Nikolaus Schaller wrote: > > > > Hi, > > > > we just started testing the v4.16 kernel and found the > > > > device no longer bootable (works with v4.15). It turned > > > > out that there was a harmful modification somewhere between > > > > v4.15.0 and v4.16-rc1. > > > > > > > > A git bisect points to this patch: > > > > > > Well, that's a shame... However, this code is in production for several > > > months now, so could you, please put 'goto out_copy' if 'buf >= high_memory' > > > condition is met, ie: > > > --- a/drivers/mtd/nand/onenand/omap2.c > > > +++ b/drivers/mtd/nand/onenand/omap2.c > > > @@ -392,6 +392,7 @@ static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area, > > > if (buf >= high_memory) { > > > struct page *p1; > > > > > > + goto out_copy; > > > if (((size_t)buf & PAGE_MASK) != > > > ((size_t)(buf + count - 1) & PAGE_MASK)) > > > goto out_copy; > > > > I had the same problem here, and that snippet helps here. ubiattach > > -p /dev/mtdX does not cause kernel oopses here anymore > > It seems reviving old code always comes at a price :-) Could you try > following patch, so far compile tested only? > (we'll need to do the same for omap2_onenand_write_bufferram, but > it sould be enough for testing purposes now) > > diff --git a/drivers/mtd/nand/onenand/omap2.c b/drivers/mtd/nand/onenand/omap2.c > index 9c159f0dd9a6..04cefd7a6487 100644 > --- a/drivers/mtd/nand/onenand/omap2.c > +++ b/drivers/mtd/nand/onenand/omap2.c > @@ -375,11 +375,12 @@ static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area, > { > struct omap2_onenand *c = container_of(mtd, struct omap2_onenand, mtd); > struct onenand_chip *this = mtd->priv; > + struct device *dev = &c->pdev->dev; > dma_addr_t dma_src, dma_dst; > int bram_offset; > void *buf = (void *)buffer; > size_t xtra; > - int ret; > + int ret, page_dma = 0; > > bram_offset = omap2_onenand_bufferram_offset(mtd, area) + area + offset; > if (bram_offset & 3 || (size_t)buf & 3 || count < 384) > @@ -389,38 +390,43 @@ static int omap2_onenand_read_bufferram(struct mtd_info *mtd, int area, > if (in_interrupt() || oops_in_progress) > goto out_copy; > > + xtra = count & 3; > + if (xtra) { > + count -= xtra; > + memcpy(buf + count, this->base + bram_offset + count, xtra); > + } > + > + /* Handle vmalloc address */ > if (buf >= high_memory) { > - struct page *p1; > + struct page *page; > > if (((size_t)buf & PAGE_MASK) != > ((size_t)(buf + count - 1) & PAGE_MASK)) > goto out_copy; > - p1 = vmalloc_to_page(buf); > - if (!p1) > + page = vmalloc_to_page(buf); Not sure this approach is safe on all archs: if the cache is VIVT or VIPT, you may have several entries pointing to the same phys page, and then, when dma_map_page() does its cache maintenance operations, it's only taking one of these entries into account. In other parts of the MTD subsystem, we tend to not do DMA on buffers that have been vmalloc-ed. You can do something like if (virt_addr_valid(buf)) /* Use DMA */ else /* * Do not use DMA, or use a bounce buffer * allocated with kmalloc */ Regards, Boris > + if (!page) > goto out_copy; > - buf = page_address(p1) + ((size_t)buf & ~PAGE_MASK); > - } > - > - xtra = count & 3; > - if (xtra) { > - count -= xtra; > - memcpy(buf + count, this->base + bram_offset + count, xtra); > + page_dma = 1; > + dma_dst = dma_map_page(dev, page, (size_t) buf & ~PAGE_MASK, > + count, DMA_FROM_DEVICE); > + } else { > + dma_dst = dma_map_single(dev, buf, count, DMA_FROM_DEVICE); > } > - > dma_src = c->phys_base + bram_offset; > - dma_dst = dma_map_single(&c->pdev->dev, buf, count, DMA_FROM_DEVICE); > - if (dma_mapping_error(&c->pdev->dev, dma_dst)) { > - dev_err(&c->pdev->dev, > - "Couldn't DMA map a %d byte buffer\n", > - count); > + > + if (dma_mapping_error(dev, dma_dst)) { > + dev_err(dev, "Couldn't DMA map a %d byte buffer\n", count); > goto out_copy; > } > > ret = omap2_onenand_dma_transfer(c, dma_src, dma_dst, count); > - dma_unmap_single(&c->pdev->dev, dma_dst, count, DMA_FROM_DEVICE); > + if (page_dma) > + dma_unmap_page(dev, dma_dst, count, DMA_FROM_DEVICE); > + else > + dma_unmap_single(dev, dma_dst, count, DMA_FROM_DEVICE); > > if (ret) { > - dev_err(&c->pdev->dev, "timeout waiting for DMA\n"); > + dev_err(dev, "timeout waiting for DMA\n"); > goto out_copy; > } >