2012-05-12 15:15:48

by Joel Reardon

[permalink] [raw]
Subject: [patch] UBI: add lnum to ubi_work data structure

This is part of a multipart patch to allow UBI to force the erasure of
particular logical eraseblock numbers. In this patch, the logical erase block
number is added to ubi_work data structure, and it is also passed as a
parameter to schedule erase to set it appropriately. Whenever ubi_wl_put_peb
is called, the lnum is also passed to be forwarded to schedule erase. Later,
a new ubi_sync_lnum will be added to execute immediately all work related to
that lnum.

This was tested by observing the recorded lnum value whenever an eraseblock
was erased (i.e., the work done). UBIFS was changed to repeat leb_change
calls 10 times, integck was executed, and the sequence of 10 equal lnums
with varying pnums was observed.

Signed-off-by: Joel Reardon <[email protected]>
---
drivers/mtd/ubi/eba.c | 16 ++++++++--------
drivers/mtd/ubi/ubi.h | 2 +-
drivers/mtd/ubi/wl.c | 23 ++++++++++++++---------
3 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
index 2455d62..4f3ffc1 100644
--- a/drivers/mtd/ubi/eba.c
+++ b/drivers/mtd/ubi/eba.c
@@ -341,7 +341,7 @@ int ubi_eba_unmap_leb(struct ubi_device *ubi, struct ubi_volume *vol,
dbg_eba("erase LEB %d:%d, PEB %d", vol_id, lnum, pnum);

vol->eba_tbl[lnum] = UBI_LEB_UNMAPPED;
- err = ubi_wl_put_peb(ubi, pnum, 0);
+ err = ubi_wl_put_peb(ubi, pnum, 0, lnum);

out_unlock:
leb_write_unlock(ubi, vol_id, lnum);
@@ -550,7 +550,7 @@ retry:
ubi_free_vid_hdr(ubi, vid_hdr);

vol->eba_tbl[lnum] = new_pnum;
- ubi_wl_put_peb(ubi, pnum, 1);
+ ubi_wl_put_peb(ubi, pnum, 1, lnum);

ubi_msg("data was successfully recovered");
return 0;
@@ -558,7 +558,7 @@ retry:
out_unlock:
mutex_unlock(&ubi->buf_mutex);
out_put:
- ubi_wl_put_peb(ubi, new_pnum, 1);
+ ubi_wl_put_peb(ubi, new_pnum, 1, lnum);
ubi_free_vid_hdr(ubi, vid_hdr);
return err;

@@ -568,7 +568,7 @@ write_error:
* get another one.
*/
ubi_warn("failed to write to PEB %d", new_pnum);
- ubi_wl_put_peb(ubi, new_pnum, 1);
+ ubi_wl_put_peb(ubi, new_pnum, 1, lnum);
if (++tries > UBI_IO_RETRIES) {
ubi_free_vid_hdr(ubi, vid_hdr);
return err;
@@ -687,7 +687,7 @@ write_error:
* eraseblock, so just put it and request a new one. We assume that if
* this physical eraseblock went bad, the erase code will handle that.
*/
- err = ubi_wl_put_peb(ubi, pnum, 1);
+ err = ubi_wl_put_peb(ubi, pnum, 1, lnum);
if (err || ++tries > UBI_IO_RETRIES) {
ubi_ro_mode(ubi);
leb_write_unlock(ubi, vol_id, lnum);
@@ -807,7 +807,7 @@ write_error:
return err;
}

- err = ubi_wl_put_peb(ubi, pnum, 1);
+ err = ubi_wl_put_peb(ubi, pnum, 1, lnum);
if (err || ++tries > UBI_IO_RETRIES) {
ubi_ro_mode(ubi);
leb_write_unlock(ubi, vol_id, lnum);
@@ -905,7 +905,7 @@ retry:
}

if (vol->eba_tbl[lnum] >= 0) {
- err = ubi_wl_put_peb(ubi, vol->eba_tbl[lnum], 0);
+ err = ubi_wl_put_peb(ubi, vol->eba_tbl[lnum], 0, lnum);
if (err)
goto out_leb_unlock;
}
@@ -930,7 +930,7 @@ write_error:
goto out_leb_unlock;
}

- err = ubi_wl_put_peb(ubi, pnum, 1);
+ err = ubi_wl_put_peb(ubi, pnum, 1, lnum);
if (err || ++tries > UBI_IO_RETRIES) {
ubi_ro_mode(ubi);
goto out_leb_unlock;
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index b162790..14443a0 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -537,7 +537,7 @@ int ubi_eba_init_scan(struct ubi_device *ubi, struct ubi_scan_info *si);

/* wl.c */
int ubi_wl_get_peb(struct ubi_device *ubi, int dtype);
-int ubi_wl_put_peb(struct ubi_device *ubi, int pnum, int torture);
+int ubi_wl_put_peb(struct ubi_device *ubi, int pnum, int torture, int lnum);
int ubi_wl_flush(struct ubi_device *ubi);
int ubi_wl_scrub_peb(struct ubi_device *ubi, int pnum);
int ubi_wl_init_scan(struct ubi_device *ubi, struct ubi_scan_info *si);
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index 7c1a9bf..7f6dd7e 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -148,6 +148,7 @@
* @func: worker function
* @e: physical eraseblock to erase
* @torture: if the physical eraseblock has to be tortured
+ * @lnum: the logical number of the erase block
*
* The @func pointer points to the worker function. If the @cancel argument is
* not zero, the worker has to free the resources and exit immediately. The
@@ -160,6 +161,7 @@ struct ubi_work {
/* The below fields are only relevant to erasure works */
struct ubi_wl_entry *e;
int torture;
+ int lnum;
};

#ifdef CONFIG_MTD_UBI_DEBUG
@@ -628,12 +630,13 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
* @ubi: UBI device description object
* @e: the WL entry of the physical eraseblock to erase
* @torture: if the physical eraseblock has to be tortured
+ * @lnum: the last used logical eraseblock number for the PEB
*
* This function returns zero in case of success and a %-ENOMEM in case of
* failure.
*/
static int schedule_erase(struct ubi_device *ubi, struct ubi_wl_entry *e,
- int torture)
+ int torture, int lnum)
{
struct ubi_work *wl_wrk;

@@ -647,6 +650,7 @@ static int schedule_erase(struct ubi_device *ubi, struct ubi_wl_entry *e,
wl_wrk->func = &erase_worker;
wl_wrk->e = e;
wl_wrk->torture = torture;
+ wl_wrk->lnum = lnum;

schedule_ubi_work(ubi, wl_wrk);
return 0;
@@ -846,7 +850,7 @@ static int wear_leveling_worker(struct ubi_device *ubi, struct ubi_work *wrk,
ubi->move_to_put = ubi->wl_scheduled = 0;
spin_unlock(&ubi->wl_lock);

- err = schedule_erase(ubi, e1, 0);
+ err = schedule_erase(ubi, e1, 0, lnum);
if (err) {
kmem_cache_free(ubi_wl_entry_slab, e1);
if (e2)
@@ -861,7 +865,7 @@ static int wear_leveling_worker(struct ubi_device *ubi, struct ubi_work *wrk,
*/
dbg_wl("PEB %d (LEB %d:%d) was put meanwhile, erase",
e2->pnum, vol_id, lnum);
- err = schedule_erase(ubi, e2, 0);
+ err = schedule_erase(ubi, e2, 0, lnum);
if (err) {
kmem_cache_free(ubi_wl_entry_slab, e2);
goto out_ro;
@@ -900,7 +904,7 @@ out_not_moved:
spin_unlock(&ubi->wl_lock);

ubi_free_vid_hdr(ubi, vid_hdr);
- err = schedule_erase(ubi, e2, torture);
+ err = schedule_erase(ubi, e2, torture, lnum);
if (err) {
kmem_cache_free(ubi_wl_entry_slab, e2);
goto out_ro;
@@ -1027,7 +1031,7 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
return 0;
}

- dbg_wl("erase PEB %d EC %d", pnum, e->ec);
+ dbg_wl("erase PEB %d EC %d LEB %d", pnum, e->ec, wl_wrk->lnum);

err = sync_erase(ubi, e, wl_wrk->torture);
if (!err) {
@@ -1057,7 +1061,7 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
int err1;

/* Re-schedule the LEB for erasure */
- err1 = schedule_erase(ubi, e, 0);
+ err1 = schedule_erase(ubi, e, 0, wl_wrk->lnum);
if (err1) {
err = err1;
goto out_ro;
@@ -1127,13 +1131,14 @@ out_ro:
* @ubi: UBI device description object
* @pnum: physical eraseblock to return
* @torture: if this physical eraseblock has to be tortured
+ * @lnum: the last used logical eraseblock number for the PEB
*
* This function is called to return physical eraseblock @pnum to the pool of
* free physical eraseblocks. The @torture flag has to be set if an I/O error
* occurred to this @pnum and it has to be tested. This function returns zero
* in case of success, and a negative error code in case of failure.
*/
-int ubi_wl_put_peb(struct ubi_device *ubi, int pnum, int torture)
+int ubi_wl_put_peb(struct ubi_device *ubi, int pnum, int torture, int lnum)
{
int err;
struct ubi_wl_entry *e;
@@ -1199,7 +1204,7 @@ retry:
}
spin_unlock(&ubi->wl_lock);

- err = schedule_erase(ubi, e, torture);
+ err = schedule_erase(ubi, e, torture, lnum);
if (err) {
spin_lock(&ubi->wl_lock);
wl_tree_add(e, &ubi->used);
@@ -1464,7 +1469,7 @@ int ubi_wl_init_scan(struct ubi_device *ubi, struct ubi_scan_info *si)
e->pnum = seb->pnum;
e->ec = seb->ec;
ubi->lookuptbl[e->pnum] = e;
- if (schedule_erase(ubi, e, 0)) {
+ if (schedule_erase(ubi, e, 0, seb->lnum)) {
kmem_cache_free(ubi_wl_entry_slab, e);
goto out_free;
}
--
1.7.5.4


2012-05-13 08:47:42

by Joel Reardon

[permalink] [raw]
Subject: [patch] UBI: add means to clear ubi work queue for particular lnums

This is the second part of a patch to allow UBI to force the erasure of
particular logical eraseblock numbers. In this patch, a new function,
ubi_lnum_purge, is added that allows the caller to synchronously erase all
unmapped erase blocks whose LEB number corresponds to the parameter. This
requires a previous patch that stores the LEB number in struct ubi_work.

This was tested by disabling the call to do_work in ubi thread, which results
in the work queue remaining unless explicitly called to remove. UBIFS was
changed to call ubifs_leb_change 50 times for three different LEBs. Then the
new function was called to clear the queue for the three differnt LEB numbers
one at a time. The work queue was dumped each time and the selective removal
of the particular LEB numbers was observed.

Calls to down_read(&ubi->work_sem) are not being done (unlike in
do_work).. I'm not sure
exactly where its needed / what it does, so perhaps someone can enlighten
me.


Signed-off-by: Joel Reardon <[email protected]>
---
drivers/mtd/ubi/kapi.c | 31 +++++++++++++++++++++++++++++++
drivers/mtd/ubi/ubi.h | 1 +
drivers/mtd/ubi/wl.c | 35 +++++++++++++++++++++++++++++++++++
include/linux/mtd/ubi.h | 1 +
4 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/ubi/kapi.c b/drivers/mtd/ubi/kapi.c
index 9fdb353..bfd4e22 100644
--- a/drivers/mtd/ubi/kapi.c
+++ b/drivers/mtd/ubi/kapi.c
@@ -720,6 +720,37 @@ int ubi_sync(int ubi_num)
}
EXPORT_SYMBOL_GPL(ubi_sync);

+/**
+ * ubi_lnum_purge - synchronously erase unmapped PEBs by LEB number.
+ * @ubi_num: UBI device to erase PEBs
+ * @lnum: the LEB number to erase old unmapped PEBs.
+ *
+ * This function is designed to offer a means to ensure that the contents of
+ * old, unmapped LEBs are securely erased without having to flush the entire
+ * work queue of all erase blocks that need erasure. Simply erasing the block
+ * at the time of unmapped is insufficient, as the wear-levelling subsystem
+ * may have already moved the contents. This function navigates the list of
+ * erase blocks that need erasures, and performs an immediate and synchronous
+ * erasure of any erase block that has held data for this particular @lnum.
+ * This may include eraseblocks that held older versions of the same @lnum.
+ * Returns zero in case of success and a negative error code in case of
+ * failure.
+ */
+int ubi_lnum_purge(int ubi_num, int lnum)
+{
+ int err;
+ struct ubi_device *ubi;
+
+ ubi = ubi_get_device(ubi_num);
+ if (!ubi)
+ return -ENODEV;
+
+ err = ubi_wl_flush_lnum(ubi, lnum);
+ ubi_put_device(ubi);
+ return err;
+}
+EXPORT_SYMBOL_GPL(ubi_lnum_purge);
+
BLOCKING_NOTIFIER_HEAD(ubi_notifiers);

/**
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 14443a0..e1a29ea 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -539,6 +539,7 @@ int ubi_eba_init_scan(struct ubi_device *ubi, struct ubi_scan_info *si);
int ubi_wl_get_peb(struct ubi_device *ubi, int dtype);
int ubi_wl_put_peb(struct ubi_device *ubi, int pnum, int torture, int lnum);
int ubi_wl_flush(struct ubi_device *ubi);
+int ubi_wl_flush_lnum(struct ubi_device *ubi, int lnum);
int ubi_wl_scrub_peb(struct ubi_device *ubi, int pnum);
int ubi_wl_init_scan(struct ubi_device *ubi, struct ubi_scan_info *si);
void ubi_wl_close(struct ubi_device *ubi);
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index 7f6dd7e..bc8111d 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -1278,6 +1278,41 @@ retry:
}

/**
+ * ubi_wl_flush_lnum - flush all pending works associated with a LEB number
+ * @ubi: UBI device description object
+ * @lnum: the LEB number only for which work should be synchronously executed
+ *
+ * This function executes all pending works for PEBs whose associated with a
+ * particular LEB number. This function returns zero in case of success and a
+ * negative error code in case of failure.
+ */
+int ubi_wl_flush_lnum(struct ubi_device *ubi, int lnum)
+{
+ int err = 0;
+ struct ubi_work *wrk, *tmp;
+
+ /* For each pending work, if it corresponds to the parameter @lnum,
+ * then execute the work.
+ */
+ dbg_wl("flush lnum %d", lnum);
+ list_for_each_entry_safe(wrk, tmp, &ubi->works, list) {
+ if (wrk->lnum == lnum) {
+ spin_lock(&ubi->wl_lock);
+ list_del(&wrk->list);
+ ubi->works_count -= 1;
+ ubi_assert(ubi->works_count >= 0);
+ spin_unlock(&ubi->wl_lock);
+
+ err = wrk->func(ubi, wrk, 0);
+ if (err)
+ ubi_err("work failed with error code %d", err);
+ }
+ }
+
+ return err;
+}
+
+/**
* ubi_wl_flush - flush all pending works.
* @ubi: UBI device description object
*
diff --git a/include/linux/mtd/ubi.h b/include/linux/mtd/ubi.h
index db4836b..1d1b6a1 100644
--- a/include/linux/mtd/ubi.h
+++ b/include/linux/mtd/ubi.h
@@ -216,6 +216,7 @@ int ubi_leb_unmap(struct ubi_volume_desc *desc, int lnum);
int ubi_leb_map(struct ubi_volume_desc *desc, int lnum, int dtype);
int ubi_is_mapped(struct ubi_volume_desc *desc, int lnum);
int ubi_sync(int ubi_num);
+int ubi_lnum_purge(int ubi_num, int lnum);

/*
* This function is the same as the 'ubi_leb_read()' function, but it does not
--
1.7.5.4

2012-05-13 09:52:07

by Richard Weinberger

[permalink] [raw]
Subject: Re: [patch] UBI: add means to clear ubi work queue for particular lnums

Am 13.05.2012 10:47, schrieb Joel Reardon:
> +int ubi_wl_flush_lnum(struct ubi_device *ubi, int lnum)
> +{
> + int err = 0;
> + struct ubi_work *wrk, *tmp;
> +
> + /* For each pending work, if it corresponds to the parameter @lnum,
> + * then execute the work.
> + */
> + dbg_wl("flush lnum %d", lnum);
> + list_for_each_entry_safe(wrk, tmp, &ubi->works, list) {
> + if (wrk->lnum == lnum) {
> + spin_lock(&ubi->wl_lock);
> + list_del(&wrk->list);
> + ubi->works_count -= 1;
> + ubi_assert(ubi->works_count >= 0);
> + spin_unlock(&ubi->wl_lock);
> +
> + err = wrk->func(ubi, wrk, 0);
> + if (err)
> + ubi_err("work failed with error code %d", err);
> + }
> + }
> +
> + return err;
> +}

I think you also have to grab ubi->work_sem in read mode here.

Thanks,
//richard


Attachments:
signature.asc (490.00 B)
OpenPGP digital signature

2012-05-14 17:11:06

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [patch] UBI: add means to clear ubi work queue for particular lnums

On Sun, 2012-05-13 at 10:47 +0200, Joel Reardon wrote:
> This is the second part of a patch to allow UBI to force the erasure of
> particular logical eraseblock numbers. In this patch, a new function,
> ubi_lnum_purge, is added that allows the caller to synchronously erase all
> unmapped erase blocks whose LEB number corresponds to the parameter. This
> requires a previous patch that stores the LEB number in struct ubi_work.
>
> This was tested by disabling the call to do_work in ubi thread, which results
> in the work queue remaining unless explicitly called to remove. UBIFS was
> changed to call ubifs_leb_change 50 times for three different LEBs. Then the
> new function was called to clear the queue for the three differnt LEB numbers
> one at a time. The work queue was dumped each time and the selective removal
> of the particular LEB numbers was observed.
>
> Calls to down_read(&ubi->work_sem) are not being done (unlike in
> do_work).. I'm not sure
> exactly where its needed / what it does, so perhaps someone can enlighten
> me.
>

Joel, I've just pushed patches from Richard which will conflict with
your patch-set. His patches are ready to go in right away so I took them
first. Please, re-base and re-send your changes.

I've re-based your "joel" branch against latest UBI/UBIFS stuff. Your
previous branch is now "joel_old".

Thanks!

--
Best Regards,
Artem Bityutskiy


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part