Re: [PATCH] staging:mt29f_spinand: MT29F2G failing as only 16-bit arguments and variables used for addressing.
So should I change this into a revert patch, or you will revert commit
6efb21d6d0e7 ("staging:mt29f_spinand: MT29F2G failing as only 16 bits
used for addressing.") by yourself?
Thanks.
Best Regards,
Jheng-Jhong Wu (Victor Wu)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: mt29f_spinand: fix memory leak while programming pages
Dear greg k-h, Before device is removed and freed memory automatically, programming pages may run many many times. Assume we erase and rewrite a large part of the flash, then spinand_program_page() might exhaust memory if memory is not large enough. We may not remove and re-add the device between each programming page, right? In fact, OOM indeed occurred when I tested programming multi-pages by mtd_debug tool. Erased first, then programmed pages. Best Regards, ─ Jheng-Jhong Wu (Victor Wu) E-mail: goodwater...@gmail.com ─ Greg Kroah-Hartman 於 2018年8月2日 週四 下午4:03寫道： > > On Thu, Aug 02, 2018 at 03:51:30PM +0800, Jheng-Jhong Wu wrote: > > In spinand_program_page(), it uses devm_kzalloc() to allocate memory to > > wbuf dynamically if internal ECC is on, but it doesn't free memory > > allocated to wbuf at the end of this function. Before the spinand device > > is removed and frees memory automatically, programming pages may run many > > times. This leads to a memory leak issue when internal ECC is on. > > How is this a memory leak? The memory will be freed when the struct > device is removed from the system. How did you test that there was a > leak? > > > Changelog: > > > > v2: > > - use kzalloc()/kfree() to replace devm_kzalloc()/devm_kfree() > > - add some descriptions to commit message > > this changelog goes below the --- line. > > thanks, > > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: mt29f_spinand: fix memory leak while programming pages
In spinand_program_page(), it uses devm_kzalloc() to allocate memory to wbuf dynamically if internal ECC is on, but it doesn't free memory allocated to wbuf at the end of this function. Before the spinand device is removed and frees memory automatically, programming pages may run many times. This leads to a memory leak issue when internal ECC is on. Changelog: v2: - use kzalloc()/kfree() to replace devm_kzalloc()/devm_kfree() - add some descriptions to commit message Signed-off-by: Jheng-Jhong Wu --- drivers/staging/mt29f_spinand/mt29f_spinand.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c b/drivers/staging/mt29f_spinand/mt29f_spinand.c index e389009..d740c76 100644 --- a/drivers/staging/mt29f_spinand/mt29f_spinand.c +++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c @@ -492,7 +492,7 @@ static int spinand_program_page(struct spi_device *spi_nand, #ifdef CONFIG_MTD_SPINAND_ONDIEECC unsigned int i, j; - wbuf = devm_kzalloc(&spi_nand->dev, CACHE_BUF, GFP_KERNEL); + wbuf = kzalloc(CACHE_BUF, GFP_KERNEL); if (!wbuf) return -ENOMEM; @@ -553,6 +553,8 @@ static int spinand_program_page(struct spi_device *spi_nand, } enable_hw_ecc = 0; } + + kfree(wbuf); #endif return 0; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: mt29f_spinand: fix memory leak while programming pages
Dear Dan, I know what you wrote, but before the spinand device is removed and freed memory automatically, programming pages may do many many times. Assume we erase and rewrite a large part of the flash, then spinand_program_page() might exhaust memory if memory is not large enough. In fact, OOM indeed occured when I tested programming multi-pages by mtd_debug tool. If OOM was not caused by devm_kzalloc() in spinand_program_page(), what may exhaust memory? Best Regards, ─ Jheng-Jhong Wu (Victor Wu) E-mail: goodwater...@gmail.com ─ ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: mt29f_spinand: fix memory leak while programming pages
In spinand_program_page(), it uses devm_kzalloc() to allocate memory to wbuf dynamically if internal ECC is on, but it doesn't free memory allocated to wbuf at the end of this function. This leads to a memory leak issue when internal ECC is on. Signed-off-by: Jheng-Jhong Wu --- drivers/staging/mt29f_spinand/mt29f_spinand.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c b/drivers/staging/mt29f_spinand/mt29f_spinand.c index e389009..cf51ca8 100644 --- a/drivers/staging/mt29f_spinand/mt29f_spinand.c +++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c @@ -553,6 +553,8 @@ static int spinand_program_page(struct spi_device *spi_nand, } enable_hw_ecc = 0; } + + devm_kfree(&spi_nand->dev, wbuf); #endif return 0; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging:mt29f_spinand: MT29F2G failing as only 16-bit arguments and variables used for addressing.
For NAND flash chips with more than 1Gbit (e.g. MT29F2G) more than 16 bits
are necessary to address the correct page. The driver sets the address for
more than 16 bits, but it uses 16-bit arguments and variables (these are
page_id, block_id, row) to do address operations. Obviously, these
arguments and variables cannot deal with more than 16-bit address.
Signed-off-by: Jheng-Jhong Wu
---
drivers/staging/mt29f_spinand/mt29f_spinand.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c
b/drivers/staging/mt29f_spinand/mt29f_spinand.c
index 4484784..a0f4cbcb 100644
--- a/drivers/staging/mt29f_spinand/mt29f_spinand.c
+++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c
@@ -308,10 +308,10 @@ static int spinand_write_enable(struct spi_device
*spi_nand)
return spinand_cmd(spi_nand, &cmd);
}
-static int spinand_read_page_to_cache(struct spi_device *spi_nand, u16 page_id)
+static int spinand_read_page_to_cache(struct spi_device *spi_nand, u32 page_id)
{
struct spinand_cmd cmd = {0};
- u16 row;
+ u32 row;
row = page_id;
cmd.cmd = CMD_READ;
@@ -331,7 +331,7 @@ static int spinand_read_page_to_cache(struct spi_device
*spi_nand, u16 page_id)
* locations.
* No tRd delay.
*/
-static int spinand_read_from_cache(struct spi_device *spi_nand, u16 page_id,
+static int spinand_read_from_cache(struct spi_device *spi_nand, u32 page_id,
u16 byte_id, u16 len, u8 *rbuf)
{
struct spinand_cmd cmd = {0};
@@ -362,7 +362,7 @@ static int spinand_read_from_cache(struct spi_device
*spi_nand, u16 page_id,
* The read includes two commands to the Nand - 0x13 and 0x03 commands
* Poll to read status to wait for tRD time.
*/
-static int spinand_read_page(struct spi_device *spi_nand, u16 page_id,
+static int spinand_read_page(struct spi_device *spi_nand, u32 page_id,
u16 offset, u16 len, u8 *rbuf)
{
int ret;
@@ -430,7 +430,7 @@ static int spinand_read_page(struct spi_device *spi_nand,
u16 page_id,
* Since it is writing the data to cache, there is no tPROG time.
*/
static int spinand_program_data_to_cache(struct spi_device *spi_nand,
-u16 page_id, u16 byte_id,
+u32 page_id, u16 byte_id,
u16 len, u8 *wbuf)
{
struct spinand_cmd cmd = {0};
@@ -457,10 +457,10 @@ static int spinand_program_data_to_cache(struct
spi_device *spi_nand,
* the Nand array.
* Need to wait for tPROG time to finish the transaction.
*/
-static int spinand_program_execute(struct spi_device *spi_nand, u16 page_id)
+static int spinand_program_execute(struct spi_device *spi_nand, u32 page_id)
{
struct spinand_cmd cmd = {0};
- u16 row;
+ u32 row;
row = page_id;
cmd.cmd = CMD_PROG_PAGE_EXC;
@@ -486,7 +486,7 @@ static int spinand_program_execute(struct spi_device
*spi_nand, u16 page_id)
* Poll to wait for the tPROG time to finish the transaction.
*/
static int spinand_program_page(struct spi_device *spi_nand,
- u16 page_id, u16 offset, u16 len, u8 *buf)
+ u32 page_id, u16 offset, u16 len, u8 *buf)
{
int retval;
u8 status = 0;
@@ -573,10 +573,10 @@ static int spinand_program_page(struct spi_device
*spi_nand,
* one block--64 pages
* Need to wait for tERS.
*/
-static int spinand_erase_block_erase(struct spi_device *spi_nand, u16 block_id)
+static int spinand_erase_block_erase(struct spi_device *spi_nand, u32 block_id)
{
struct spinand_cmd cmd = {0};
- u16 row;
+ u32 row;
row = block_id;
cmd.cmd = CMD_ERASE_BLK;
@@ -599,7 +599,7 @@ static int spinand_erase_block_erase(struct spi_device
*spi_nand, u16 block_id)
* and then send the 0xd8 erase command
* Poll to wait for the tERS time to complete the tranaction.
*/
-static int spinand_erase_block(struct spi_device *spi_nand, u16 block_id)
+static int spinand_erase_block(struct spi_device *spi_nand, u32 block_id)
{
int retval;
u8 status = 0;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
