[PATCH] UBI: vtbl: Use ubi_eba_atomic_leb_change()

[PATCH] UBI: vtbl: Use ubi_eba_atomic_leb_change()

[Richard Weinberger]

This is more a cosmetic change than a fix.
By using ubi_eba_atomic_leb_change()
we can guarantee that the first VTBL record is always
correct and we don't really need the second one anymore.
But we have to keep the second one to not break anything.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 drivers/mtd/ubi/vtbl.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/ubi/vtbl.c b/drivers/mtd/ubi/vtbl.c
index 07cac5f..0d26051 100644
--- a/drivers/mtd/ubi/vtbl.c
+++ b/drivers/mtd/ubi/vtbl.c
@@ -96,12 +96,8 @@ int ubi_change_vtbl_record(struct ubi_device *ubi, int idx,
 
 	memcpy(&ubi->vtbl[idx], vtbl_rec, sizeof(struct ubi_vtbl_record));
 	for (i = 0; i < UBI_LAYOUT_VOLUME_EBS; i++) {
-		err = ubi_eba_unmap_leb(ubi, layout_vol, i);
-		if (err)
-			return err;
-
-		err = ubi_eba_write_leb(ubi, layout_vol, i, ubi->vtbl, 0,
-					ubi->vtbl_size);
+		err = ubi_eba_atomic_leb_change(ubi, layout_vol, i, ubi->vtbl,
+						ubi->vtbl_size);
 		if (err)
 			return err;
 	}
@@ -148,12 +144,8 @@ int ubi_vtbl_rename_volumes(struct ubi_device *ubi,
 
 	layout_vol = ubi->volumes[vol_id2idx(ubi, UBI_LAYOUT_VOLUME_ID)];
 	for (i = 0; i < UBI_LAYOUT_VOLUME_EBS; i++) {
-		err = ubi_eba_unmap_leb(ubi, layout_vol, i);
-		if (err)
-			return err;
-
-		err = ubi_eba_write_leb(ubi, layout_vol, i, ubi->vtbl, 0,
-					ubi->vtbl_size);
+		err = ubi_eba_atomic_leb_change(ubi, layout_vol, i, ubi->vtbl,
+						ubi->vtbl_size);
 		if (err)
 			return err;
 	}
-- 
1.8.4.5

[PATCH] UBI: vtbl: Use ubi_eba_atomic_leb_change()

[Richard Weinberger]

This is more a cosmetic change than a fix.
By using ubi_eba_atomic_leb_change()
we can guarantee that the first VTBL record is always
correct and we don't really need the second one anymore.
But we have to keep the second one to not break anything.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 drivers/mtd/ubi/vtbl.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/ubi/vtbl.c b/drivers/mtd/ubi/vtbl.c
index 07cac5f..0d26051 100644
--- a/drivers/mtd/ubi/vtbl.c
+++ b/drivers/mtd/ubi/vtbl.c
@@ -96,12 +96,8 @@ int ubi_change_vtbl_record(struct ubi_device *ubi, int idx,
 
 	memcpy(&ubi->vtbl[idx], vtbl_rec, sizeof(struct ubi_vtbl_record));
 	for (i = 0; i < UBI_LAYOUT_VOLUME_EBS; i++) {
-		err = ubi_eba_unmap_leb(ubi, layout_vol, i);
-		if (err)
-			return err;
-
-		err = ubi_eba_write_leb(ubi, layout_vol, i, ubi->vtbl, 0,
-					ubi->vtbl_size);
+		err = ubi_eba_atomic_leb_change(ubi, layout_vol, i, ubi->vtbl,
+						ubi->vtbl_size);
 		if (err)
 			return err;
 	}
@@ -148,12 +144,8 @@ int ubi_vtbl_rename_volumes(struct ubi_device *ubi,
 
 	layout_vol = ubi->volumes[vol_id2idx(ubi, UBI_LAYOUT_VOLUME_ID)];
 	for (i = 0; i < UBI_LAYOUT_VOLUME_EBS; i++) {
-		err = ubi_eba_unmap_leb(ubi, layout_vol, i);
-		if (err)
-			return err;
-
-		err = ubi_eba_write_leb(ubi, layout_vol, i, ubi->vtbl, 0,
-					ubi->vtbl_size);
+		err = ubi_eba_atomic_leb_change(ubi, layout_vol, i, ubi->vtbl,
+						ubi->vtbl_size);
 		if (err)
 			return err;
 	}
-- 
1.8.4.5

Re: [PATCH] UBI: vtbl: Use ubi_eba_atomic_leb_change()

[Artem Bityutskiy]

On Sat, 2014-10-25 at 19:43 +0200, Richard Weinberger wrote:
> This is more a cosmetic change than a fix.
> By using ubi_eba_atomic_leb_change()
> we can guarantee that the first VTBL record is always
> correct and we don't really need the second one anymore.
> But we have to keep the second one to not break anything.
> 
> Signed-off-by: Richard Weinberger <richard@nod.at>

Yeah, this atomic change stuff was added later, and we had not
envisioned originally. Your patch adds robustness, but makes volume
creation slower, which is probably not a problem.

I've added a small comment and pushed it, thanks!

Artem

Re: [PATCH] UBI: vtbl: Use ubi_eba_atomic_leb_change()

[Artem Bityutskiy]

On Sat, 2014-10-25 at 19:43 +0200, Richard Weinberger wrote:
> This is more a cosmetic change than a fix.
> By using ubi_eba_atomic_leb_change()
> we can guarantee that the first VTBL record is always
> correct and we don't really need the second one anymore.
> But we have to keep the second one to not break anything.
> 
> Signed-off-by: Richard Weinberger <richard@nod.at>

Yeah, this atomic change stuff was added later, and we had not
envisioned originally. Your patch adds robustness, but makes volume
creation slower, which is probably not a problem.

I've added a small comment and pushed it, thanks!

Artem

Re: [PATCH] UBI: vtbl: Use ubi_eba_atomic_leb_change()

[hujianyang]

On 2014/10/30 16:55, Artem Bityutskiy wrote:
> On Sat, 2014-10-25 at 19:43 +0200, Richard Weinberger wrote:
>> This is more a cosmetic change than a fix.
>> By using ubi_eba_atomic_leb_change()
>> we can guarantee that the first VTBL record is always
>> correct and we don't really need the second one anymore.
>> But we have to keep the second one to not break anything.
>>
>> Signed-off-by: Richard Weinberger <richard@nod.at>
> 
> Yeah, this atomic change stuff was added later, and we had not
> envisioned originally. Your patch adds robustness, but makes volume
> creation slower, which is probably not a problem.
> 
> I've added a small comment and pushed it, thanks!
> 
> Artem
> 
> 

Hi Artem and Richard,

We are using atomic operation, leb_change(), for master_node
in ubifs-level. We use two lebs for master_node even if they
are changed with atomic operation.

I think volume_table and master_node play similar roles. Do
you think changing VTBL record into one peb is OK? I just
what to know if I missed something. Could you please take
some time to explain that?

Thanks very much~!

Hu

Re: [PATCH] UBI: vtbl: Use ubi_eba_atomic_leb_change()

[hujianyang]

On 2014/10/30 16:55, Artem Bityutskiy wrote:
> On Sat, 2014-10-25 at 19:43 +0200, Richard Weinberger wrote:
>> This is more a cosmetic change than a fix.
>> By using ubi_eba_atomic_leb_change()
>> we can guarantee that the first VTBL record is always
>> correct and we don't really need the second one anymore.
>> But we have to keep the second one to not break anything.
>>
>> Signed-off-by: Richard Weinberger <richard@nod.at>
> 
> Yeah, this atomic change stuff was added later, and we had not
> envisioned originally. Your patch adds robustness, but makes volume
> creation slower, which is probably not a problem.
> 
> I've added a small comment and pushed it, thanks!
> 
> Artem
> 
> 

Hi Artem and Richard,

We are using atomic operation, leb_change(), for master_node
in ubifs-level. We use two lebs for master_node even if they
are changed with atomic operation.

I think volume_table and master_node play similar roles. Do
you think changing VTBL record into one peb is OK? I just
what to know if I missed something. Could you please take
some time to explain that?

Thanks very much~!

Hu

Re: [PATCH] UBI: vtbl: Use ubi_eba_atomic_leb_change()

[Richard Weinberger]

Hujianyang,

Am 31.10.2014 um 05:03 schrieb hujianyang:
> Hi Artem and Richard,
> 
> We are using atomic operation, leb_change(), for master_node
> in ubifs-level. We use two lebs for master_node even if they
> are changed with atomic operation.
> 
> I think volume_table and master_node play similar roles. Do
> you think changing VTBL record into one peb is OK? I just
> what to know if I missed something. Could you please take
> some time to explain that?

I'm not sure if I correctly understand your question.

If we use only one PEB for the VTBL existing UBI implementations
would break as they assume we have two.

Thanks,
//richard

Re: [PATCH] UBI: vtbl: Use ubi_eba_atomic_leb_change()

[Richard Weinberger]

Hujianyang,

Am 31.10.2014 um 05:03 schrieb hujianyang:
> Hi Artem and Richard,
> 
> We are using atomic operation, leb_change(), for master_node
> in ubifs-level. We use two lebs for master_node even if they
> are changed with atomic operation.
> 
> I think volume_table and master_node play similar roles. Do
> you think changing VTBL record into one peb is OK? I just
> what to know if I missed something. Could you please take
> some time to explain that?

I'm not sure if I correctly understand your question.

If we use only one PEB for the VTBL existing UBI implementations
would break as they assume we have two.

Thanks,
//richard

Re: [PATCH] UBI: vtbl: Use ubi_eba_atomic_leb_change()

[hujianyang]

On 2014/10/31 16:09, Richard Weinberger wrote:
> Hujianyang,
> 
> Am 31.10.2014 um 05:03 schrieb hujianyang:
>> Hi Artem and Richard,
>>
>> We are using atomic operation, leb_change(), for master_node
>> in ubifs-level. We use two lebs for master_node even if they
>> are changed with atomic operation.
>>
>> I think volume_table and master_node play similar roles. Do
>> you think changing VTBL record into one peb is OK? I just
>> what to know if I missed something. Could you please take
>> some time to explain that?
> 
> I'm not sure if I correctly understand your question.
> 
> If we use only one PEB for the VTBL existing UBI implementations
> would break as they assume we have two.
> 
> Thanks,
> //richard
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
> 

This question is basing on your comment for this patch:

"""
we can guarantee that the first VTBL record is always
correct and we don't really need the second one anymore.
"""

I think that means one PEB is enough in your considering.
So I want to know if you are sure about this. Because
we use two leb for master_node in ubifs-level. So maybe
VTBL is like super_node, not master_node, right?

Re: [PATCH] UBI: vtbl: Use ubi_eba_atomic_leb_change()

[hujianyang]

On 2014/10/31 16:09, Richard Weinberger wrote:
> Hujianyang,
> 
> Am 31.10.2014 um 05:03 schrieb hujianyang:
>> Hi Artem and Richard,
>>
>> We are using atomic operation, leb_change(), for master_node
>> in ubifs-level. We use two lebs for master_node even if they
>> are changed with atomic operation.
>>
>> I think volume_table and master_node play similar roles. Do
>> you think changing VTBL record into one peb is OK? I just
>> what to know if I missed something. Could you please take
>> some time to explain that?
> 
> I'm not sure if I correctly understand your question.
> 
> If we use only one PEB for the VTBL existing UBI implementations
> would break as they assume we have two.
> 
> Thanks,
> //richard
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
> 

This question is basing on your comment for this patch:

"""
we can guarantee that the first VTBL record is always
correct and we don't really need the second one anymore.
"""

I think that means one PEB is enough in your considering.
So I want to know if you are sure about this. Because
we use two leb for master_node in ubifs-level. So maybe
VTBL is like super_node, not master_node, right?

Re: [PATCH] UBI: vtbl: Use ubi_eba_atomic_leb_change()

[Richard Weinberger]

Am 31.10.2014 um 11:45 schrieb hujianyang:
> This question is basing on your comment for this patch:
> 
> """
> we can guarantee that the first VTBL record is always
> correct and we don't really need the second one anymore.
> """
> 
> I think that means one PEB is enough in your considering.
> So I want to know if you are sure about this. Because
> we use two leb for master_node in ubifs-level. So maybe
> VTBL is like super_node, not master_node, right?

Yes, technically one PEB is enough if atomic leb change is used.
But existing UBI implementations want a second one
and a backup VTBL PEB is good for robustness.
i.e. if the PEB turns bad we have a backup and do not lose
all volume meta information.

Thanks,
//richard

Re: [PATCH] UBI: vtbl: Use ubi_eba_atomic_leb_change()

[Richard Weinberger]

Am 31.10.2014 um 11:45 schrieb hujianyang:
> This question is basing on your comment for this patch:
> 
> """
> we can guarantee that the first VTBL record is always
> correct and we don't really need the second one anymore.
> """
> 
> I think that means one PEB is enough in your considering.
> So I want to know if you are sure about this. Because
> we use two leb for master_node in ubifs-level. So maybe
> VTBL is like super_node, not master_node, right?

Yes, technically one PEB is enough if atomic leb change is used.
But existing UBI implementations want a second one
and a backup VTBL PEB is good for robustness.
i.e. if the PEB turns bad we have a backup and do not lose
all volume meta information.

Thanks,
//richard