| Message ID | alpine.LFD.1.10.0810291313140.30280@casper.infradead.org |
|---|---|
| State | Changes Requested |
| Headers | show |
On Wed, 2008-10-29 at 13:14 +0000, Alexey Korolev wrote: > @@ -437,6 +440,8 @@ static int alloc_device(struct nandsim *ns) > for (i = 0; i < ns->geom.pgnum; i++) { > ns->pages[i].byte = NULL; > } > + ns->nand_pages_slab = kmem_cache_create("nandsim", > + ns->geom.pgszoob, 0, 0, NULL); Sorry, but how about checking for errors here?
On Wed, 2008-10-29 at 13:14 +0000, Alexey Korolev wrote: > Hi All, > > Nandsim consumes ~2x more RAM than the density of simulated device. > It becomes critical if we need to simulate 256MB NAND and run stress tests > on it. > > We investigated the reasons. nandsim allocates space for pages using kmalloc > function. The size of LP nand page is 2112 bytes. > kmalloc gets space from slab pools by chunks 2^n. So if we need to kmalloc > 2112 bytes, 4096 bytes will be consumed by system. > The best way to avoid this issue would be using kmem_cache allocations. AFAIK > this mechanism specially designed to handle cases when arrays of allocations > are used. Would you please send a version which checks return pointer of kmem_cache_alloc() for NULL?
Hi, > > Would you please send a version which checks return pointer of > kmem_cache_alloc() for NULL? > I think it is unnecessary to add one more check of return pointer for NULL after kmem_cache_alloc() function. PLease take a look at these lines of the patch: ---- - mypage->byte = kmalloc(ns->geom.pgszoob, GFP_NOFS); + mypage->byte = kmem_cache_alloc(ns->nand_pages_slab, GFP_NOFS); if (mypage->byte == NULL) { NS_ERR("prog_page: error allocating memory for page %d\n", ns->regs.row); return -1; ---- If I properly understand the code it already performs checking just after attempt of allocation. So this patch version should be correct. Thanks, Alexey
Hi Artem, > > Would you please send a version which checks return pointer of > kmem_cache_alloc() for NULL? Thank for reminder. I've just found your message you sent just before the holidays. It was about kmem_cache_create function. Sorry I missed it. I've sent the updated-patch. Thanks, Alexey
diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c index ae7c577..0551cbf 100644 --- a/drivers/mtd/nand/nandsim.c +++ b/drivers/mtd/nand/nandsim.c @@ -295,6 +295,9 @@ struct nandsim { /* The simulated NAND flash pages array */ union ns_mem *pages; + /* Slab allocator for nand pages */ + struct kmem_cache *nand_pages_slab; + /* Internal buffer of page + OOB size bytes */ union ns_mem buf; @@ -420,8 +423,8 @@ static struct mtd_info *nsmtd; static u_char ns_verify_buf[NS_LARGEST_PAGE_SIZE]; /* - * Allocate array of page pointers and initialize the array to NULL - * pointers. + * Allocate array of page pointers, create slab allocation for an array + * and initialize the array by NULL pointers. * * RETURNS: 0 if success, -ENOMEM if memory alloc fails. */ @@ -437,6 +440,8 @@ static int alloc_device(struct nandsim *ns) for (i = 0; i < ns->geom.pgnum; i++) { ns->pages[i].byte = NULL; } + ns->nand_pages_slab = kmem_cache_create("nandsim", + ns->geom.pgszoob, 0, 0, NULL); return 0; } @@ -451,8 +456,10 @@ static void free_device(struct nandsim *ns) if (ns->pages) { for (i = 0; i < ns->geom.pgnum; i++) { if (ns->pages[i].byte) - kfree(ns->pages[i].byte); + kmem_cache_free(ns->nand_pages_slab, + ns->pages[i].byte); } + kmem_cache_destroy(ns->nand_pages_slab); vfree(ns->pages); } } @@ -1279,7 +1286,7 @@ static void erase_sector(struct nandsim *ns) for (i = 0; i < ns->geom.pgsec; i++) { if (mypage->byte != NULL) { NS_DBG("erase_sector: freeing page %d\n", ns->regs.row+i); - kfree(mypage->byte); + kmem_cache_free(ns->nand_pages_slab, mypage->byte); mypage->byte = NULL; } mypage++; @@ -1301,10 +1308,10 @@ static int prog_page(struct nandsim *ns, int num) /* * We allocate memory with GFP_NOFS because a flash FS may * utilize this. If it is holding an FS lock, then gets here, - * then kmalloc runs writeback which goes to the FS again - * and deadlocks. This was seen in practice. + * then kernel memory alloc runs writeback which goes to the FS + * again and deadlocks. This was seen in practice. */ - mypage->byte = kmalloc(ns->geom.pgszoob, GFP_NOFS); + mypage->byte = kmem_cache_alloc(ns->nand_pages_slab, GFP_NOFS); if (mypage->byte == NULL) { NS_ERR("prog_page: error allocating memory for page %d\n", ns->regs.row); return -1;