| Message ID | 1398375634-24759-1-git-send-email-kdasu.kdev@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
于 2014年04月25日 05:40, Kamal Dasu 写道: > + * This option is defined to protect against kmapped buffers > + * being passed from highmem when using DMA the dma API can not support the highmem. could you show us how do the driver use the highmem buffer? In the gpmi driver, i do a memcpy for the highmem buffer passed from upper layer. thanks Huang Shijie
Huang, "could you show us how do the driver use the highmem buffer?" This is the stack on a BMIPS platform we are doing a read dma transaction. drv_* calls are from out of tree nand controller driver. This example shows the driver trying to map highmem virtual address(0xfe1141e0) for DMA operation passed from higher layer, and we get an oops. [<80010764>] mips_dma_map_page+0x64/0x1f8 [<80394a80>] drv_nand_dma_trans+0xd0/0x23c [<80394f60>] drv_nand_read+0x374/0x3dc [<8039536c>] drv_nand_read_page+0x78/0x84 [<803903c8>] nand_do_read_ops+0x1c0/0x48c [<80390b2c>] nand_read+0xbc/0xec [<8036df10>] part_read+0x7c/0x128 [<8039fa48>] ubi_io_read+0x98/0x1bc [<8039d954>] ubi_eba_read_leb+0x130/0x320 [<8039cd80>] ubi_leb_read+0xf8/0x164 [<803a5034>] gluebi_read+0xf0/0x148 [<801d2b9c>] jffs2_flash_read+0x74/0x284 [<801c5930>] jffs2_read_dnode+0x140/0x3d0 [<801c5c88>] jffs2_read_inode_range+0xc8/0x1d0 [<801c3d0c>] jffs2_do_readpage_nolock+0x5c/0x208 [<801c4270>] jffs2_do_readpage_unlock+0x14/0x34 [<80088b68>] do_read_cache_page+0xa4/0x23c [<80088d50>] read_cache_page_async+0x20/0x2c [<801d0740>] jffs2_gc_fetch_page+0x2c/0x5c [<801cc958>] jffs2_garbage_collect_live+0x55c/0xfb0 [<801cd810>] jffs2_garbage_collect_pass+0x464/0x894 [<801cf2f8>] jffs2_garbage_collect_thread+0x108/0x224 [<800481fc>] kthread+0x90/0x98 [<8000698c>] kernel_thread_helper+0x10/0x18 "In the gpmi driver, i do a memcpy for the highmem buffer passed from upper layer." In the nand controller driver that needs to bounce buffer the "chip->option |= NAND_USE_BOUNCE_BUFFER" can be set during initialization, nand_base uses the already allocated databuf and does the memcpy. Much like the unaligned access case. So multiple drivers can use this generic feature on other platforms as well. Thanks Kamal On Thu, Apr 24, 2014 at 10:07 PM, Huang Shijie <b32955@freescale.com> wrote: > 于 2014年04月25日 05:40, Kamal Dasu 写道: > >> + * This option is defined to protect against kmapped buffers >> + * being passed from highmem when using DMA > > the dma API can not support the highmem. > > could you show us how do the driver use the highmem buffer? > > In the gpmi driver, i do a memcpy for the highmem buffer passed from upper > layer. > > thanks > Huang Shijie > >
On Thu, Apr 24, 2014 at 11:52:41PM -0400, Kamal Dasu wrote: > Huang, > > "could you show us how do the driver use the highmem buffer?" > > This is the stack on a BMIPS platform we are doing a read dma transaction. > drv_* calls are from out of tree nand controller driver. This I can read the driver since it is not in the l2-mtd. :( > example shows the driver trying to map > highmem virtual address(0xfe1141e0) for DMA operation passed from Which dma api do you use? do you use the dma_map_sg() or something else? If you use the dma_map_sg, do it support the highmem in the MIPS? thanks Huang Shijie
On Fri, Apr 25, 2014 at 03:43:13PM +0800, Huang Shijie wrote: > On Thu, Apr 24, 2014 at 11:52:41PM -0400, Kamal Dasu wrote: > > Huang, > > > > "could you show us how do the driver use the highmem buffer?" > > > > This is the stack on a BMIPS platform we are doing a read dma transaction. > > drv_* calls are from out of tree nand controller driver. This > I can read the driver since it is not in the l2-mtd. :( s/can/can't/ > > > example shows the driver trying to map > > highmem virtual address(0xfe1141e0) for DMA operation passed from > > Which dma api do you use? In the ARM platform, we use the sg_init_one to initialize the sg. It can only convert the kmalloc memory, please see the code: #define __virt_to_phys(x) ((x) - PAGE_OFFSET + PHYS_OFFSET) It does a linear convert for a kernel address. So in the ARM platform, it can _NOT_ support the highmem which is not linear. I am not familiar with MIPS. I do not know how the MIPS convert the highmem address. thanks Huang Shijie
Huang, "In the gpmi driver, i do a memcpy for the highmem buffer passed from upper layer." In other l2-mtd driver examples when the mapping fails those drivers are reverting to bouce buffering using kmall'd dma buffer and memcpy to highmem buffer. So the solutions have been implemented in the l2-mtd (lowest level nand controller) driver. In contrast this in change in nand_base uses the generic mm api virt_addr_valid() to check if the passed address is in highmem, and if it is I am not trying to convert(map) it, rather using the kmalloc'd databuf in nand_base and memcpy to/from the highmem buffer. Also using this check is optional, if l2-mtd driver can handle highmem in other ways like gpmi, then chip->option need not be set. "Which dma api do you use?" For mapping the va to pa dma_map_single() is used. "In the ARM platform, we use the sg_init_one to initialize the sg. It can only convert the kmalloc memory, please see the code: #define __virt_to_phys(x) ((x) - PAGE_OFFSET + PHYS_OFFSET) It does a linear convert for a kernel address. So in the ARM platform, it can _NOT_ support the highmem which is not linear." I am not converting the address anywhere, merely checking if it is in highmem. Are you suggesting that using virt_addr_valid() for ARM platform is not sufficient ?. I see that ret= dma_map_sg() is being used in case of the gpmi driver to achieve the same. Thanks Kamal On Fri, Apr 25, 2014 at 3:59 AM, Huang Shijie <b32955@freescale.com> wrote: > On Fri, Apr 25, 2014 at 03:43:13PM +0800, Huang Shijie wrote: >> On Thu, Apr 24, 2014 at 11:52:41PM -0400, Kamal Dasu wrote: >> > Huang, >> > >> > "could you show us how do the driver use the highmem buffer?" >> > >> > This is the stack on a BMIPS platform we are doing a read dma transaction. >> > drv_* calls are from out of tree nand controller driver. This >> I can read the driver since it is not in the l2-mtd. :( > > s/can/can't/ > >> >> > example shows the driver trying to map >> > highmem virtual address(0xfe1141e0) for DMA operation passed from >> >> Which dma api do you use? > > In the ARM platform, we use the sg_init_one to initialize the sg. > > It can only convert the kmalloc memory, please see the code: > > #define __virt_to_phys(x) ((x) - PAGE_OFFSET + PHYS_OFFSET) > > It does a linear convert for a kernel address. > So in the ARM platform, it can _NOT_ support the highmem which is not linear. > > > I am not familiar with MIPS. > > I do not know how the MIPS convert the highmem address. > > thanks > Huang Shijie
On Fri, Apr 25, 2014 at 12:10:43PM -0400, Kamal Dasu wrote: > Huang, > > "In the gpmi driver, i do a memcpy for the highmem buffer passed from > upper layer." > > In other l2-mtd driver examples when the mapping fails those drivers > are reverting to bouce buffering using kmall'd dma buffer and memcpy > to highmem buffer. So the solutions have been implemented in the > l2-mtd (lowest level nand controller) driver. > > In contrast this in change in nand_base uses the generic mm api > virt_addr_valid() to check if the passed address is in highmem, and if > it is I am not trying to convert(map) it, rather using the kmalloc'd > databuf in nand_base and memcpy to/from the highmem buffer. Also I think your idea is good. :) If your patch can be merged to kernel, it mean the driver can always do the DMA mapping sucessfully(Am i right?). And it makes the gpmi driver more simple. See my comment in the orginal patch. thanks Huang Shijie
On Thu, Apr 24, 2014 at 05:40:34PM -0400, Kamal Dasu wrote: > nand_base can be passed a kmap()'d buffers from highmem by > filesystems like jffs2. This results in failure to map the > physical address of the DMA buffer on various contoller > driver on different platforms. This change adds a chip option > to use preallocated databuf as bounce buffers used in > nand_do_read_ops() and nand_do_write_ops(). > This allows for specific nand controller driver to set this > option as needed. > > Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com> > --- > > Changes since v1: > * Fix bytes to write calculation in nand_do_write_ops > > drivers/mtd/nand/nand_base.c | 28 +++++++++++++++++++++------- > include/linux/mtd/nand.h | 5 +++++ > 2 files changed, 26 insertions(+), 7 deletions(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index 9d01c4d..d94242a 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -1501,6 +1501,7 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from, > mtd->oobavail : mtd->oobsize; > > uint8_t *bufpoi, *oob, *buf; > + int use_bufpoi; > unsigned int max_bitflips = 0; > int retry_mode = 0; > bool ecc_fail = false; > @@ -1522,10 +1523,16 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from, > > bytes = min(mtd->writesize - col, readlen); > aligned = (bytes == mtd->writesize); > + use_bufpoi = (chip->options & NAND_USE_BOUNCE_BUFFER) ? > + !virt_addr_valid(buf) : 0; I met a compiler error here in the ARM platform. I think you should add some header. > > /* Is the current page in the buffer? */ > if (realpage != chip->pagebuf || oob) { > - bufpoi = aligned ? buf : chip->buffers->databuf; > + bufpoi = (aligned && !use_bufpoi) ? buf : > + chip->buffers->databuf; > + > + if (use_bufpoi && aligned) > + pr_debug("%s: using bounce buffer\n", __func__); print more info here, such as: pr_debug("%s: using bounce buffer for buf:%p\n", __func__, buf); > > read_retry: > chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page); > @@ -1547,7 +1554,7 @@ read_retry: > ret = chip->ecc.read_page(mtd, chip, bufpoi, > oob_required, page); > if (ret < 0) { > - if (!aligned) > + if (!aligned || use_bufpoi) > /* Invalidate page cache */ > chip->pagebuf = -1; > break; > @@ -1556,7 +1563,7 @@ read_retry: > max_bitflips = max_t(unsigned int, max_bitflips, ret); > > /* Transfer not aligned data */ > - if (!aligned) { > + if (!aligned || use_bufpoi) { In actually, the " if (!aligned || use_bufpoi) " is equal to "if (bufpoi == chip->buffers->databuf)) {" I am not sure which is more readable. :) But I prefer to the later. Let Brian to judge it. thanks Huang Shijie
On Thu, Apr 24, 2014 at 05:40:34PM -0400, Kamal Dasu wrote: > nand_base can be passed a kmap()'d buffers from highmem by > filesystems like jffs2. This results in failure to map the In actually, the highmem buffer may also allocated by vmalloc(), such as used in the UBIFS. thanks Huang Shijie
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 9d01c4d..d94242a 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -1501,6 +1501,7 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from, mtd->oobavail : mtd->oobsize; uint8_t *bufpoi, *oob, *buf; + int use_bufpoi; unsigned int max_bitflips = 0; int retry_mode = 0; bool ecc_fail = false; @@ -1522,10 +1523,16 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from, bytes = min(mtd->writesize - col, readlen); aligned = (bytes == mtd->writesize); + use_bufpoi = (chip->options & NAND_USE_BOUNCE_BUFFER) ? + !virt_addr_valid(buf) : 0; /* Is the current page in the buffer? */ if (realpage != chip->pagebuf || oob) { - bufpoi = aligned ? buf : chip->buffers->databuf; + bufpoi = (aligned && !use_bufpoi) ? buf : + chip->buffers->databuf; + + if (use_bufpoi && aligned) + pr_debug("%s: using bounce buffer\n", __func__); read_retry: chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page); @@ -1547,7 +1554,7 @@ read_retry: ret = chip->ecc.read_page(mtd, chip, bufpoi, oob_required, page); if (ret < 0) { - if (!aligned) + if (!aligned || use_bufpoi) /* Invalidate page cache */ chip->pagebuf = -1; break; @@ -1556,7 +1563,7 @@ read_retry: max_bitflips = max_t(unsigned int, max_bitflips, ret); /* Transfer not aligned data */ - if (!aligned) { + if (!aligned || use_bufpoi) { if (!NAND_HAS_SUBPAGE_READ(chip) && !oob && !(mtd->ecc_stats.failed - ecc_failures) && (ops->mode != MTD_OPS_RAW)) { @@ -2376,11 +2383,18 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to, int bytes = mtd->writesize; int cached = writelen > bytes && page != blockmask; uint8_t *wbuf = buf; - - /* Partial page write? */ - if (unlikely(column || writelen < (mtd->writesize - 1))) { + int use_bufpoi = (chip->options & NAND_USE_BOUNCE_BUFFER ? + !virt_addr_valid(buf) : 0); + int part_pagewr = unlikely(column || + writelen < (mtd->writesize - 1)); + + /* Partial page write?, or need to use bounce buffer */ + if (part_pagewr || use_bufpoi) { + pr_debug("%s: using write bounce buffer\n", __func__); cached = 0; - bytes = min_t(int, bytes - column, (int) writelen); + if (part_pagewr) + bytes = min_t(int, + bytes - column, (int) writelen); chip->pagebuf = -1; memset(chip->buffers->databuf, 0xff, mtd->writesize); memcpy(&chip->buffers->databuf[column], buf, bytes); diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 450d61e..bb3e064 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -176,6 +176,11 @@ typedef enum { /* Chip may not exist, so silence any errors in scan */ #define NAND_SCAN_SILENT_NODEV 0x00040000 /* + * This option is defined to protect against kmapped buffers + * being passed from highmem when using DMA + */ +#define NAND_USE_BOUNCE_BUFFER 0x00080000 +/* * Autodetect nand buswidth with readid/onfi. * This suppose the driver will configure the hardware in 8 bits mode * when calling nand_scan_ident, and update its configuration
nand_base can be passed a kmap()'d buffers from highmem by filesystems like jffs2. This results in failure to map the physical address of the DMA buffer on various contoller driver on different platforms. This change adds a chip option to use preallocated databuf as bounce buffers used in nand_do_read_ops() and nand_do_write_ops(). This allows for specific nand controller driver to set this option as needed. Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com> --- Changes since v1: * Fix bytes to write calculation in nand_do_write_ops drivers/mtd/nand/nand_base.c | 28 +++++++++++++++++++++------- include/linux/mtd/nand.h | 5 +++++ 2 files changed, 26 insertions(+), 7 deletions(-)