Patchwork [1/7,RFC] UBI: Export next_sqnum()

login
register
mail settings
Submitter Richard Weinberger
Date May 15, 2012, 5:11 p.m.
Message ID <1337101871-31181-2-git-send-email-richard@nod.at>
Download mbox | patch
Permalink /patch/159392/
State New
Headers show

Comments

Richard Weinberger - May 15, 2012, 5:11 p.m.
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(-)
Shmulik Ladkani - May 16, 2012, 2:03 p.m.
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
Artem Bityutskiy - May 16, 2012, 2:27 p.m.
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.
Shmulik Ladkani - May 17, 2012, 9:45 a.m.
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
Artem Bityutskiy - May 17, 2012, 11:44 a.m.
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 :-)
Richard Weinberger - May 17, 2012, 11:47 a.m.
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
Artem Bityutskiy - May 17, 2012, 12:34 p.m.
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?

Patch

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);