Patchwork [1/2] UBI: allocate verification buffer on demand

login
register
mail settings
Submitter Artem Bityutskiy
Date March 8, 2012, 2:14 p.m.
Message ID <1331216091-6560-1-git-send-email-dedekind1@gmail.com>
Download mbox | patch
Permalink /patch/145533/
State New
Headers show

Comments

Artem Bityutskiy - March 8, 2012, 2:14 p.m.
From: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>

Remove the pre-allocated 'peb_buf2' buffer because we do not really need it.
The only reason UBI has it is to check that the data were written correctly.
But we do not have to have 2 buffers for this and waste RAM - we can just
compare CRC checksums instead.

Artem bityutskiy: massaged the patch and commit message

Signed-off-by: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 drivers/mtd/ubi/build.c |    6 ------
 drivers/mtd/ubi/eba.c   |    6 +++---
 drivers/mtd/ubi/ubi.h   |    4 +---
 3 files changed, 4 insertions(+), 12 deletions(-)
Shmulik Ladkani - March 8, 2012, 4:08 p.m.
On Thu,  8 Mar 2012 16:14:50 +0200 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> From: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>
> 
> Remove the pre-allocated 'peb_buf2' buffer because we do not really need it.
> The only reason UBI has it is to check that the data were written correctly.
> But we do not have to have 2 buffers for this and waste RAM - we can just
> compare CRC checksums instead.
> 
> Artem bityutskiy: massaged the patch and commit message
> 
> Signed-off-by: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Josselin's original title "allocate verification buffer on demand" does
no longer describe this patch.
Should be "Remove the pre-allocated 'peb_buf2'" or alike.

Regards,
Shmulik
Artem Bityutskiy - March 9, 2012, 7:45 a.m.
On Thu, 2012-03-08 at 18:08 +0200, Shmulik Ladkani wrote:
> On Thu,  8 Mar 2012 16:14:50 +0200 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > From: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>
> > 
> > Remove the pre-allocated 'peb_buf2' buffer because we do not really need it.
> > The only reason UBI has it is to check that the data were written correctly.
> > But we do not have to have 2 buffers for this and waste RAM - we can just
> > compare CRC checksums instead.
> > 
> > Artem bityutskiy: massaged the patch and commit message
> > 
> > Signed-off-by: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>
> > Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> 
> Josselin's original title "allocate verification buffer on demand" does
> no longer describe this patch.
> Should be "Remove the pre-allocated 'peb_buf2'" or alike.

Fixed as well and pushed out, thanks! Waiting for Josselin's reply
anyway.

commit 43b043e78b876ce27034f167897b57fd2556ad29
Author: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>
Date:   Wed Feb 22 16:37:05 2012 +0100

    UBI: reduce memory consumption
    
    Remove the pre-allocated 'peb_buf2' buffer because we do not really need it.
    The only reason UBI has it is to check that the data were written correctly.
    But we do not have to have 2 buffers for this and waste RAM - we can just
    compare CRC checksums instead. This reduces UBI memory consumption.
    
    Artem bityutskiy: massaged the patch and commit message
    
    Signed-off-by: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>
    Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
josselin.costanzi@mobile-devices.fr - March 9, 2012, 9:13 a.m.
2012/3/9 Artem Bityutskiy <dedekind1@gmail.com>:
> On Thu, 2012-03-08 at 18:08 +0200, Shmulik Ladkani wrote:
>> On Thu,  8 Mar 2012 16:14:50 +0200 Artem Bityutskiy <dedekind1@gmail.com> wrote:
>> > From: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>
>> >
>> > Remove the pre-allocated 'peb_buf2' buffer because we do not really need it.
>> > The only reason UBI has it is to check that the data were written correctly.
>> > But we do not have to have 2 buffers for this and waste RAM - we can just
>> > compare CRC checksums instead.
>> >
>> > Artem bityutskiy: massaged the patch and commit message
>> >
>> > Signed-off-by: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>
>> > Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>>
>> Josselin's original title "allocate verification buffer on demand" does
>> no longer describe this patch.
>> Should be "Remove the pre-allocated 'peb_buf2'" or alike.
>
> Fixed as well and pushed out, thanks! Waiting for Josselin's reply
> anyway.
I will try to find time to test your patches today and will give my
feedback ASAP

> commit 43b043e78b876ce27034f167897b57fd2556ad29
> Author: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>
> Date:   Wed Feb 22 16:37:05 2012 +0100
>
>    UBI: reduce memory consumption
>
>    Remove the pre-allocated 'peb_buf2' buffer because we do not really need it.
>    The only reason UBI has it is to check that the data were written correctly.
>    But we do not have to have 2 buffers for this and waste RAM - we can just
>    compare CRC checksums instead. This reduces UBI memory consumption.
>
>    Artem bityutskiy: massaged the patch and commit message
>
>    Signed-off-by: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>
>    Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>
>
> --
> Best Regards,
> Artem Bityutskiy
>
josselin.costanzi@mobile-devices.fr - March 9, 2012, 4:42 p.m.
2012/3/9 Josselin Costanzi <josselin.costanzi@mobile-devices.fr>:
> 2012/3/9 Artem Bityutskiy <dedekind1@gmail.com>:
>> On Thu, 2012-03-08 at 18:08 +0200, Shmulik Ladkani wrote:
>>> On Thu,  8 Mar 2012 16:14:50 +0200 Artem Bityutskiy <dedekind1@gmail.com> wrote:
>>> > From: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>
>>> >
>>> > Remove the pre-allocated 'peb_buf2' buffer because we do not really need it.
>>> > The only reason UBI has it is to check that the data were written correctly.
>>> > But we do not have to have 2 buffers for this and waste RAM - we can just
>>> > compare CRC checksums instead.
>>> >
>>> > Artem bityutskiy: massaged the patch and commit message
>>> >
>>> > Signed-off-by: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>
>>> > Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>>>
>>> Josselin's original title "allocate verification buffer on demand" does
>>> no longer describe this patch.
>>> Should be "Remove the pre-allocated 'peb_buf2'" or alike.
>>
>> Fixed as well and pushed out, thanks! Waiting for Josselin's reply
>> anyway.
> I will try to find time to test your patches today and will give my
> feedback ASAP
I'm okay with your version of the patch.
Thanks.

>> commit 43b043e78b876ce27034f167897b57fd2556ad29
>> Author: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>
>> Date:   Wed Feb 22 16:37:05 2012 +0100
>>
>>    UBI: reduce memory consumption
>>
>>    Remove the pre-allocated 'peb_buf2' buffer because we do not really need it.
>>    The only reason UBI has it is to check that the data were written correctly.
>>    But we do not have to have 2 buffers for this and waste RAM - we can just
>>    compare CRC checksums instead. This reduces UBI memory consumption.
>>
>>    Artem bityutskiy: massaged the patch and commit message
>>
>>    Signed-off-by: Josselin Costanzi <josselin.costanzi@mobile-devices.fr>
>>    Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>>
>>
>> --
>> Best Regards,
>> Artem Bityutskiy
>>
>
>
>
> --
> Josselin Costanzi
>   Embedded Linux System Engineer
>
> Tél. : +33 (0) 1 42 11 93 25
> Fax: +33 (0) 1 42 11 87 17
>
>
> Warning
> This message (and any associated files) is intended only for the use
> of its intended recipient and may contain information that is
> confidential, subject to copyright or constitutes a trade secret. If
> you are not the intended recipient you are hereby notified that any
> dissemination, copying or distribution of this message, or files
> associated with this message, is strictly prohibited. If you have
> received this message in error, please notify us immediately by
> replying to the message and deleting it from your computer. Any views
> or opinions presented are solely those of the author
> josselin.costanzi@mobile-devices.fr and do not necessarily represent
> those of the company. Although the company has taken reasonable
> precautions to ensure no viruses are present in this email, the
> company cannot accept responsibility for any loss or damage arising
> from the use of this email or attachments.

Patch

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 115749f..6e0806b 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -949,10 +949,6 @@  int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset)
 	if (!ubi->peb_buf1)
 		goto out_free;
 
-	ubi->peb_buf2 = vmalloc(ubi->peb_size);
-	if (!ubi->peb_buf2)
-		goto out_free;
-
 	err = ubi_debugging_init_dev(ubi);
 	if (err)
 		goto out_free;
@@ -1030,7 +1026,6 @@  out_debugging:
 	ubi_debugging_exit_dev(ubi);
 out_free:
 	vfree(ubi->peb_buf1);
-	vfree(ubi->peb_buf2);
 	if (ref)
 		put_device(&ubi->dev);
 	else
@@ -1102,7 +1097,6 @@  int ubi_detach_mtd_dev(int ubi_num, int anyway)
 	put_mtd_device(ubi->mtd);
 	ubi_debugging_exit_dev(ubi);
 	vfree(ubi->peb_buf1);
-	vfree(ubi->peb_buf2);
 	ubi_msg("mtd%d is detached from ubi%d", ubi->mtd->index, ubi->ubi_num);
 	put_device(&ubi->dev);
 	return 0;
diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
index cd26da8..f548af3 100644
--- a/drivers/mtd/ubi/eba.c
+++ b/drivers/mtd/ubi/eba.c
@@ -1134,8 +1134,8 @@  int ubi_eba_copy_leb(struct ubi_device *ubi, int from, int to,
 		 * We've written the data and are going to read it back to make
 		 * sure it was written correctly.
 		 */
-
-		err = ubi_io_read_data(ubi, ubi->peb_buf2, to, 0, aldata_size);
+		memset(ubi->peb_buf1, 0xFF, aldata_size);
+		err = ubi_io_read_data(ubi, ubi->peb_buf1, to, 0, aldata_size);
 		if (err) {
 			if (err != UBI_IO_BITFLIPS) {
 				ubi_warn("error %d while reading data back "
@@ -1149,7 +1149,7 @@  int ubi_eba_copy_leb(struct ubi_device *ubi, int from, int to,
 
 		cond_resched();
 
-		if (memcmp(ubi->peb_buf1, ubi->peb_buf2, aldata_size)) {
+		if (crc != crc32(UBI_CRC32_INIT, ubi->peb_buf1, data_size)) {
 			ubi_warn("read data back from PEB %d and it is "
 				 "different", to);
 			err = -EINVAL;
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index d51d75d..cb93ad9 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -388,8 +388,7 @@  struct ubi_wl_entry;
  * @mtd: MTD device descriptor
  *
  * @peb_buf1: a buffer of PEB size used for different purposes
- * @peb_buf2: another buffer of PEB size used for different purposes
- * @buf_mutex: protects @peb_buf1 and @peb_buf2
+ * @buf_mutex: protects @peb_buf1
  * @ckvol_mutex: serializes static volume checking when opening
  *
  * @dbg: debugging information for this UBI device
@@ -472,7 +471,6 @@  struct ubi_device {
 	struct mtd_info *mtd;
 
 	void *peb_buf1;
-	void *peb_buf2;
 	struct mutex buf_mutex;
 	struct mutex ckvol_mutex;