Received: by 10.192.165.156 with SMTP id m28csp389179imm; Wed, 11 Apr 2018 00:20:03 -0700 (PDT) X-Google-Smtp-Source: AIpwx4/+c0r7Iaw06P3EU6kshJUH9ik3+zHomGbEnZywXEiXhaNy4rTcY1FzESAzagBD/py9urpx X-Received: by 2002:a17:902:1e5:: with SMTP id b92-v6mr3834347plb.78.1523431203199; Wed, 11 Apr 2018 00:20:03 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1523431203; cv=none; d=google.com; s=arc-20160816; b=lqwum3COx4BA5t4zo2FToU6m0rUv0zCJvEmSMQO4aL4lzpmipKfuSDMyFFoHiOiDeq kqJ4+GX/hEWDizQG8dXfoQMNOGHP0PuX6ANjNZGeyqd2JwMMuc+sRgUkepylHmKsYLmE Qi2Fws3ed1tAc1CZUL/x1r3OGRbXGYLeMj8bHqDDmKNY3ejczRueuG9HdRjuCOfjFCjR v5C6KBTn3QnXUHDnbLT6qzX8IjTZWPcguqCy2CMdzDGWAJMEhUdLNA+wzpO/iPTnwcdz 146ZWWpHbSYHq279hkFgNO9qFM/sxSTGKCuZXgmBRbjJWiY/PjuuMdvuhTSSN8BtVlVZ WlWA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:subject:cc:to:from:date :arc-authentication-results; bh=e0O/LjimTBTEBSj+SXza/LCD4zRITUpRd3akTMBKrmg=; b=QQVqlGzuwon098TuFIVAEkx/xUnRP37JRcCgSktc4qI3n6CC6uJ/K5rf5FXBvHTnqQ kRl20GRr8nk5WwS4zNiC26MRSN2dQzCcIEM7IzhvcnQo6O3s6JhlpSh8UkhVuBc++Tp6 BHxushLH2RiadglnKDXBiVSTla9AKWlOrIZd7ABkrp31rsHtcz9KVYv622J3BAxxmsDT oZpsG3gFfQw4HUUUJ02HPHjpy9gnnUUj0edhitoQyTEf/wxhXgQ/COxSGfqMrdipYA4Q +KHu9Gmkie1lbMGvDQ3wD5/V4pC2+QPQTjGIjK+rRcg/oWkx2NSL1tXKDw1Drpq6C8aa SiwA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id z7si293296pgs.645.2018.04.11.00.19.26; Wed, 11 Apr 2018 00:20:03 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752750AbeDKHPl (ORCPT + 99 others); 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 Received: by mail.bootlin.com (Postfix, from userid 110) id 4260120857; Wed, 11 Apr 2018 09:15:38 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on mail.bootlin.com X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT shortcircuit=ham autolearn=disabled version=3.4.0 Received: from bbrezillon (LStLambert-657-1-97-87.w90-63.abo.wanadoo.fr [90.63.216.87]) by mail.bootlin.com (Postfix) with ESMTPSA id E5B312077B; Wed, 11 Apr 2018 09:15:27 +0200 (CEST) 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 Precedence: bulk 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; > } >