| Message ID | 1337101871-31181-2-git-send-email-richard@nod.at |
|---|---|
| State | RFC |
| Headers | show |
Hi Richard, On Tue, 15 May 2012 19:11:05 +0200 Richard Weinberger <richard@nod.at> wrote: > The fastmap mechanism needs to read the next sequence nummber > directly. > > Signed-off-by: Richard Weinberger <richard@nod.at> > --- > drivers/mtd/ubi/eba.c | 18 +++++++++--------- > drivers/mtd/ubi/ubi.h | 1 + > 2 files changed, 10 insertions(+), 9 deletions(-) > > diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c > index bd5fdbf..d75677d 100644 > --- a/drivers/mtd/ubi/eba.c > +++ b/drivers/mtd/ubi/eba.c > @@ -57,7 +57,7 @@ > * global sequence counter value. It also increases the global sequence > * counter. > */ > -static unsigned long long next_sqnum(struct ubi_device *ubi) > +unsigned long long ubi_next_sqnum(struct ubi_device *ubi) Minor nit. Comment above the function should have been changed too. (look for the "next_sqnum - get next sequence number" comment) Other than that, Reviewed-by: Shmulik Ladkani <shmulik.ladkani@gmail.com> Regards, Shmulik
On Wed, 2012-05-16 at 17:03 +0300, Shmulik Ladkani wrote: > Comment above the function should have been changed too. > (look for the "next_sqnum - get next sequence number" comment) I do not think we should make these non-static. We should re-use the entire scan_peb() function instead to scan the fastmap internal volume.
Hi Artem, On Wed, 16 May 2012 17:27:37 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote: > On Wed, 2012-05-16 at 17:03 +0300, Shmulik Ladkani wrote: > > Comment above the function should have been changed too. > > (look for the "next_sqnum - get next sequence number" comment) > > I do not think we should make these non-static. We should re-use the > entire scan_peb() function instead to scan the fastmap internal volume. Sorry, couldn't follow you. The new (outside eba.c) calls to 'ubi_next_sqnum' are from 'ubi_write_fastmap' (called from the 'ubi_update_fastmap' interface, implemented in fastmap.c) - during construction of VID headers of the FM_SB and FM_FATA. IMO this is reasonable. Do you suggest to somehow use existing ubi_eba_write_xxx functions? Or place the FM_SB/FM_DATA peb writing funtions into eba.c? Regards, Shmulik
On Thu, 2012-05-17 at 12:45 +0300, Shmulik Ladkani wrote: > Hi Artem, > > On Wed, 16 May 2012 17:27:37 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote: > > On Wed, 2012-05-16 at 17:03 +0300, Shmulik Ladkani wrote: > > > Comment above the function should have been changed too. > > > (look for the "next_sqnum - get next sequence number" comment) > > > > I do not think we should make these non-static. We should re-use the > > entire scan_peb() function instead to scan the fastmap internal volume. > > Sorry, couldn't follow you. > > The new (outside eba.c) calls to 'ubi_next_sqnum' are from > 'ubi_write_fastmap' (called from the 'ubi_update_fastmap' interface, > implemented in fastmap.c) - during construction of VID headers of the > FM_SB and FM_FATA. > > IMO this is reasonable. > > Do you suggest to somehow use existing ubi_eba_write_xxx functions? > Or place the FM_SB/FM_DATA peb writing funtions into eba.c? I think I meant the compare_lebs() exporting, not this one, replied to wrong e-mail, sorry. Happens when I am in hurry - try to keep everyone happy and make sure Richard is always busy with something :-)
Am 17.05.2012 13:44, schrieb Artem Bityutskiy: > I think I meant the compare_lebs() exporting, not this one, replied to > wrong e-mail, sorry. Happens when I am in hurry - try to keep everyone > happy and make sure Richard is always busy with something :-) > Haha. No need to keep me busy. :P Thanks, //richard
On Thu, 2012-05-17 at 13:47 +0200, Richard Weinberger wrote: > Am 17.05.2012 13:44, schrieb Artem Bityutskiy: > > I think I meant the compare_lebs() exporting, not this one, replied to > > wrong e-mail, sorry. Happens when I am in hurry - try to keep everyone > > happy and make sure Richard is always busy with something :-) > > > > Haha. > No need to keep me busy. :P By this I meant - give some review feed-back ASAP to make sure you have something to do and think about. Surely it is better than giving a lot more feed-back but with a couple of weeks delay?
diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c index bd5fdbf..d75677d 100644 --- a/drivers/mtd/ubi/eba.c +++ b/drivers/mtd/ubi/eba.c @@ -57,7 +57,7 @@ * global sequence counter value. It also increases the global sequence * counter. */ -static unsigned long long next_sqnum(struct ubi_device *ubi) +unsigned long long ubi_next_sqnum(struct ubi_device *ubi) { unsigned long long sqnum; @@ -522,7 +522,7 @@ retry: goto out_put; } - vid_hdr->sqnum = cpu_to_be64(next_sqnum(ubi)); + vid_hdr->sqnum = cpu_to_be64(ubi_next_sqnum(ubi)); err = ubi_io_write_vid_hdr(ubi, new_pnum, vid_hdr); if (err) goto write_error; @@ -633,7 +633,7 @@ int ubi_eba_write_leb(struct ubi_device *ubi, struct ubi_volume *vol, int lnum, } vid_hdr->vol_type = UBI_VID_DYNAMIC; - vid_hdr->sqnum = cpu_to_be64(next_sqnum(ubi)); + vid_hdr->sqnum = cpu_to_be64(ubi_next_sqnum(ubi)); vid_hdr->vol_id = cpu_to_be32(vol_id); vid_hdr->lnum = cpu_to_be32(lnum); vid_hdr->compat = ubi_get_compat(ubi, vol_id); @@ -694,7 +694,7 @@ write_error: return err; } - vid_hdr->sqnum = cpu_to_be64(next_sqnum(ubi)); + vid_hdr->sqnum = cpu_to_be64(ubi_next_sqnum(ubi)); ubi_msg("try another PEB"); goto retry; } @@ -747,7 +747,7 @@ int ubi_eba_write_leb_st(struct ubi_device *ubi, struct ubi_volume *vol, return err; } - vid_hdr->sqnum = cpu_to_be64(next_sqnum(ubi)); + vid_hdr->sqnum = cpu_to_be64(ubi_next_sqnum(ubi)); vid_hdr->vol_id = cpu_to_be32(vol_id); vid_hdr->lnum = cpu_to_be32(lnum); vid_hdr->compat = ubi_get_compat(ubi, vol_id); @@ -812,7 +812,7 @@ write_error: return err; } - vid_hdr->sqnum = cpu_to_be64(next_sqnum(ubi)); + vid_hdr->sqnum = cpu_to_be64(ubi_next_sqnum(ubi)); ubi_msg("try another PEB"); goto retry; } @@ -864,7 +864,7 @@ int ubi_eba_atomic_leb_change(struct ubi_device *ubi, struct ubi_volume *vol, if (err) goto out_mutex; - vid_hdr->sqnum = cpu_to_be64(next_sqnum(ubi)); + vid_hdr->sqnum = cpu_to_be64(ubi_next_sqnum(ubi)); vid_hdr->vol_id = cpu_to_be32(vol_id); vid_hdr->lnum = cpu_to_be32(lnum); vid_hdr->compat = ubi_get_compat(ubi, vol_id); @@ -932,7 +932,7 @@ write_error: goto out_leb_unlock; } - vid_hdr->sqnum = cpu_to_be64(next_sqnum(ubi)); + vid_hdr->sqnum = cpu_to_be64(ubi_next_sqnum(ubi)); ubi_msg("try another PEB"); goto retry; } @@ -1092,7 +1092,7 @@ int ubi_eba_copy_leb(struct ubi_device *ubi, int from, int to, vid_hdr->data_size = cpu_to_be32(data_size); vid_hdr->data_crc = cpu_to_be32(crc); } - vid_hdr->sqnum = cpu_to_be64(next_sqnum(ubi)); + vid_hdr->sqnum = cpu_to_be64(ubi_next_sqnum(ubi)); err = ubi_io_write_vid_hdr(ubi, to, vid_hdr); if (err) { diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h index 75b9f1c..00c48f6 100644 --- a/drivers/mtd/ubi/ubi.h +++ b/drivers/mtd/ubi/ubi.h @@ -530,6 +530,7 @@ int ubi_eba_atomic_leb_change(struct ubi_device *ubi, struct ubi_volume *vol, int ubi_eba_copy_leb(struct ubi_device *ubi, int from, int to, struct ubi_vid_hdr *vid_hdr); int ubi_eba_init_scan(struct ubi_device *ubi, struct ubi_scan_info *si); +unsigned long long ubi_next_sqnum(struct ubi_device *ubi); /* wl.c */ int ubi_wl_get_peb(struct ubi_device *ubi);
The fastmap mechanism needs to read the next sequence nummber directly. Signed-off-by: Richard Weinberger <richard@nod.at> --- drivers/mtd/ubi/eba.c | 18 +++++++++--------- drivers/mtd/ubi/ubi.h | 1 + 2 files changed, 10 insertions(+), 9 deletions(-)