From b36a261e8c0ab323d04db9cdd1f6bb4c273c4b32 Mon Sep 17 00:00:00 2001 From: Richard Weinberger Date: Mon, 14 May 2012 17:55:51 +0200 Subject: UBI: Kill data type hint We do not need this feature and to our shame it even was not working and there was a bug found very recently. -- Artem Bityutskiy Without the data type hint UBI2 (fastmap) will be easier to implement. Signed-off-by: Richard Weinberger Signed-off-by: Artem Bityutskiy --- include/linux/mtd/ubi.h | 27 +++------------------------ 1 file changed, 3 insertions(+), 24 deletions(-) (limited to 'include') diff --git a/include/linux/mtd/ubi.h b/include/linux/mtd/ubi.h index db4836bed514..9838dce7e235 100644 --- a/include/linux/mtd/ubi.h +++ b/include/linux/mtd/ubi.h @@ -208,12 +208,12 @@ void ubi_close_volume(struct ubi_volume_desc *desc); int ubi_leb_read(struct ubi_volume_desc *desc, int lnum, char *buf, int offset, int len, int check); int ubi_leb_write(struct ubi_volume_desc *desc, int lnum, const void *buf, - int offset, int len, int dtype); + int offset, int len); int ubi_leb_change(struct ubi_volume_desc *desc, int lnum, const void *buf, - int len, int dtype); + int len); int ubi_leb_erase(struct ubi_volume_desc *desc, int lnum); 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_leb_map(struct ubi_volume_desc *desc, int lnum); int ubi_is_mapped(struct ubi_volume_desc *desc, int lnum); int ubi_sync(int ubi_num); @@ -226,25 +226,4 @@ static inline int ubi_read(struct ubi_volume_desc *desc, int lnum, char *buf, { return ubi_leb_read(desc, lnum, buf, offset, len, 0); } - -/* - * This function is the same as the 'ubi_leb_write()' functions, but it does - * not have the data type argument. - */ -static inline int ubi_write(struct ubi_volume_desc *desc, int lnum, - const void *buf, int offset, int len) -{ - return ubi_leb_write(desc, lnum, buf, offset, len, UBI_UNKNOWN); -} - -/* - * This function is the same as the 'ubi_leb_change()' functions, but it does - * not have the data type argument. - */ -static inline int ubi_change(struct ubi_volume_desc *desc, int lnum, - const void *buf, int len) -{ - return ubi_leb_change(desc, lnum, buf, len, UBI_UNKNOWN); -} - #endif /* !__LINUX_UBI_H__ */ -- cgit v1.2.3 From a65a0eb6d198e058687a9214683bd1c418f20d39 Mon Sep 17 00:00:00 2001 From: Richard Weinberger Date: Mon, 14 May 2012 17:55:52 +0200 Subject: UBI: remove data type hint from ubi-user.h This finally removes the data type hint from the UBI ABI. >From now on the "dtype" field will be ignored and must not used anymore. Signed-off-by: Richard Weinberger Signed-off-by: Artem Bityutskiy --- include/mtd/ubi-user.h | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) (limited to 'include') diff --git a/include/mtd/ubi-user.h b/include/mtd/ubi-user.h index 3c4109777aff..370c750115ce 100644 --- a/include/mtd/ubi-user.h +++ b/include/mtd/ubi-user.h @@ -195,23 +195,6 @@ /* Maximum amount of UBI volumes that can be re-named at one go */ #define UBI_MAX_RNVOL 32 -/* - * UBI data type hint constants. - * - * UBI_LONGTERM: long-term data - * UBI_SHORTTERM: short-term data - * UBI_UNKNOWN: data persistence is unknown - * - * These constants are used when data is written to UBI volumes in order to - * help the UBI wear-leveling unit to find more appropriate physical - * eraseblocks. - */ -enum { - UBI_LONGTERM = 1, - UBI_SHORTTERM = 2, - UBI_UNKNOWN = 3, -}; - /* * UBI volume type constants. * @@ -375,25 +358,23 @@ struct ubi_rnvol_req { * requests. * @lnum: logical eraseblock number to change * @bytes: how many bytes will be written to the logical eraseblock - * @dtype: data type (%UBI_LONGTERM, %UBI_SHORTTERM, %UBI_UNKNOWN) * @padding: reserved for future, not used, has to be zeroed */ struct ubi_leb_change_req { __s32 lnum; __s32 bytes; - __s8 dtype; + __s8 dtype; /* obsolete, do not use! */ __s8 padding[7]; } __packed; /** * struct ubi_map_req - a data structure used in map LEB requests. * @lnum: logical eraseblock number to unmap - * @dtype: data type (%UBI_LONGTERM, %UBI_SHORTTERM, %UBI_UNKNOWN) * @padding: reserved for future, not used, has to be zeroed */ struct ubi_map_req { __s32 lnum; - __s8 dtype; + __s8 dtype; /* obsolete, do not use! */ __s8 padding[3]; } __packed; -- cgit v1.2.3 From 4415626732defb5a4567a0a757c7c5baae7ca846 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Mon, 14 May 2012 19:49:35 +0300 Subject: UBI: amend commentaries WRT dtype Richard removed the "dtype" hint, but few commentaries were left and this patch removes them. I've also added a better description about the "dtype" field in the ubi-user.h for people who may ever wonder what was that dtype thing about. This patch also adds an important note that it is better to use value "3" for the "dtype" field. Signed-off-by: Artem Bityutskiy --- drivers/mtd/ubi/wl.c | 9 +-------- fs/ubifs/super.c | 5 ++--- include/mtd/ubi-user.h | 11 +++++++++++ 3 files changed, 14 insertions(+), 11 deletions(-) (limited to 'include') diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c index f0bc10743bc0..64ce9930dacb 100644 --- a/drivers/mtd/ubi/wl.c +++ b/drivers/mtd/ubi/wl.c @@ -41,12 +41,6 @@ * physical eraseblocks with low erase counter to free physical eraseblocks * with high erase counter. * - * The 'ubi_wl_get_peb()' function accepts data type hints which help to pick - * an "optimal" physical eraseblock. For example, when it is known that the - * physical eraseblock will be "put" soon because it contains short-term data, - * the WL sub-system may pick a free physical eraseblock with low erase - * counter, and so forth. - * * If the WL sub-system fails to erase a physical eraseblock, it marks it as * bad. * @@ -70,8 +64,7 @@ * to the user; instead, we first want to let users fill them up with data; * * o there is a chance that the user will put the physical eraseblock very - * soon, so it makes sense not to move it for some time, but wait; this is - * especially important in case of "short term" physical eraseblocks. + * soon, so it makes sense not to move it for some time, but wait. * * Physical eraseblocks stay protected only for limited time. But the "time" is * measured in erase cycles in this case. This is implemented with help of the diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index d7cbf7aabdb7..001acccac0d6 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -814,9 +814,8 @@ static int alloc_wbufs(struct ubifs_info *c) } /* - * Garbage Collector head likely contains long-term data and - * does not need to be synchronized by timer. Also GC head nodes are - * not grouped. + * Garbage Collector head does not need to be synchronized by timer. + * Also GC head nodes are not grouped. */ c->jheads[GCHD].wbuf.no_timer = 1; c->jheads[GCHD].grouped = 0; diff --git a/include/mtd/ubi-user.h b/include/mtd/ubi-user.h index 370c750115ce..8787349fbafe 100644 --- a/include/mtd/ubi-user.h +++ b/include/mtd/ubi-user.h @@ -358,7 +358,17 @@ struct ubi_rnvol_req { * requests. * @lnum: logical eraseblock number to change * @bytes: how many bytes will be written to the logical eraseblock + * @dtype: pass "3" for better compatibility with old kernels * @padding: reserved for future, not used, has to be zeroed + * + * The @dtype field used to inform UBI about what kind of data will be written + * to the LEB: long term (value 1), short term (value 2), unknown (value 3). + * UBI tried to pick a PEB with lower erase counter for short term data and a + * PEB with higher erase counter for long term data. But this was not really + * used because users usually do not know this and could easily mislead UBI. We + * removed this feature in May 2012. UBI currently just ignores the @dtype + * field. But for better compatibility with older kernels it is recommended to + * set @dtype to 3 (unknown). */ struct ubi_leb_change_req { __s32 lnum; @@ -369,6 +379,7 @@ struct ubi_leb_change_req { /** * struct ubi_map_req - a data structure used in map LEB requests. + * @dtype: pass "3" for better compatibility with old kernels * @lnum: logical eraseblock number to unmap * @padding: reserved for future, not used, has to be zeroed */ -- cgit v1.2.3 From 05a3cb7dcec5a15ed9b18a5317ba2075355c7547 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sun, 20 May 2012 21:14:22 +0300 Subject: UBI: introduce UBI_ALL constant Joel will use it in his 'ubi_flush()' extention to specify all eraseblocks. Also amend the comment for UBI_UNKNOWN - it is used beyond attaching info structure now. Signed-off-by: Artem Bityutskiy --- drivers/mtd/ubi/ubi.h | 10 +++++----- include/linux/mtd/ubi.h | 3 +++ 2 files changed, 8 insertions(+), 5 deletions(-) (limited to 'include') diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h index 5e1182ca289b..38cfb0f2adf2 100644 --- a/drivers/mtd/ubi/ubi.h +++ b/drivers/mtd/ubi/ubi.h @@ -65,7 +65,10 @@ /* Background thread name pattern */ #define UBI_BGT_NAME_PATTERN "ubi_bgt%dd" -/* This marker in the EBA table means that the LEB is um-mapped */ +/* + * This marker in the EBA table means that the LEB is um-mapped. + * NOTE! It has to have the same value as %UBI_ALL. + */ #define UBI_LEB_UNMAPPED -1 /* @@ -81,10 +84,7 @@ */ #define UBI_PROT_QUEUE_LEN 10 -/* - * When a field of the attaching information has this value - its real value is - * unknown. - */ +/* The volume ID/LEB number/erase counter is unknown */ #define UBI_UNKNOWN -1 /* diff --git a/include/linux/mtd/ubi.h b/include/linux/mtd/ubi.h index 9838dce7e235..f636f5df883f 100644 --- a/include/linux/mtd/ubi.h +++ b/include/linux/mtd/ubi.h @@ -25,6 +25,9 @@ #include #include +/* All voumes/LEBs */ +#define UBI_ALL -1 + /* * enum ubi_open_mode - UBI volume open mode constants. * -- cgit v1.2.3 From 62f384552b6756cf1ea71f8762d1e97dc77dbd90 Mon Sep 17 00:00:00 2001 From: Joel Reardon Date: Sun, 20 May 2012 21:27:11 +0200 Subject: UBI: modify ubi_wl_flush function to clear work queue for a lnum This patch modifies ubi_wl_flush to force the erasure of particular volume id / logical eraseblock number pairs. Previous functionality is preserved when passing UBI_ALL for both values. The locations where ubi_wl_flush were called are appropriately changed: ubi_leb_erase only flushes for the erased LEB, and ubi_create_volume forces only flushing for its volume id. External code can call this new feature via the new function ubi_flush() added to kapi.c, which simply passes through to ubi_wl_flush(). 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 four different LEBs. Then the new function was called to clear the queue: passing wrong volume ids / lnum, correct ones, and finally UBI_ALL for both to ensure it was finally all cleard. The work queue was dumped each time and the selective removal of the particular LEB numbers was observed. Extra checks were enabled and ubifs's integck was also run. Finally, the drive was repeatedly filled and emptied to ensure that the queue was cleared normally. Artem: amended the patch. Signed-off-by: Joel Reardon Signed-off-by: Artem Bityutskiy --- drivers/mtd/ubi/cdev.c | 2 +- drivers/mtd/ubi/kapi.c | 29 ++++++++++++++++++++++- drivers/mtd/ubi/ubi.h | 2 +- drivers/mtd/ubi/upd.c | 4 ++-- drivers/mtd/ubi/vmt.c | 2 +- drivers/mtd/ubi/wl.c | 61 +++++++++++++++++++++++++++++-------------------- include/linux/mtd/ubi.h | 1 + 7 files changed, 70 insertions(+), 31 deletions(-) (limited to 'include') diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c index 2364c00f66d0..acec85deb6af 100644 --- a/drivers/mtd/ubi/cdev.c +++ b/drivers/mtd/ubi/cdev.c @@ -514,7 +514,7 @@ static long vol_cdev_ioctl(struct file *file, unsigned int cmd, if (err) break; - err = ubi_wl_flush(ubi); + err = ubi_wl_flush(ubi, UBI_ALL, UBI_ALL); break; } diff --git a/drivers/mtd/ubi/kapi.c b/drivers/mtd/ubi/kapi.c index d76fe47477e5..3aac1acceeb4 100644 --- a/drivers/mtd/ubi/kapi.c +++ b/drivers/mtd/ubi/kapi.c @@ -551,7 +551,7 @@ int ubi_leb_erase(struct ubi_volume_desc *desc, int lnum) if (err) return err; - return ubi_wl_flush(ubi); + return ubi_wl_flush(ubi, vol->vol_id, lnum); } EXPORT_SYMBOL_GPL(ubi_leb_erase); @@ -704,6 +704,33 @@ int ubi_sync(int ubi_num) } EXPORT_SYMBOL_GPL(ubi_sync); +/** + * ubi_flush - flush UBI work queue. + * @ubi_num: UBI device to flush work queue + * @vol_id: volume id to flush for + * @lnum: logical eraseblock number to flush for + * + * This function executes all pending works for a particular volume id / logical + * eraseblock number pair. If either value is set to %UBI_ALL, then it acts as + * a wildcard for all of the corresponding volume numbers or logical + * eraseblock numbers. It returns zero in case of success and a negative error + * code in case of failure. + */ +int ubi_flush(int ubi_num, int vol_id, int lnum) +{ + struct ubi_device *ubi; + int err = 0; + + ubi = ubi_get_device(ubi_num); + if (!ubi) + return -ENODEV; + + err = ubi_wl_flush(ubi, vol_id, lnum); + ubi_put_device(ubi); + return err; +} +EXPORT_SYMBOL_GPL(ubi_flush); + BLOCKING_NOTIFIER_HEAD(ubi_notifiers); /** diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h index 38cfb0f2adf2..a1a81c9ea8ce 100644 --- a/drivers/mtd/ubi/ubi.h +++ b/drivers/mtd/ubi/ubi.h @@ -669,7 +669,7 @@ int ubi_eba_init(struct ubi_device *ubi, struct ubi_attach_info *ai); int ubi_wl_get_peb(struct ubi_device *ubi); int ubi_wl_put_peb(struct ubi_device *ubi, int vol_id, int lnum, int pnum, int torture); -int ubi_wl_flush(struct ubi_device *ubi); +int ubi_wl_flush(struct ubi_device *ubi, int vol_id, int lnum); int ubi_wl_scrub_peb(struct ubi_device *ubi, int pnum); int ubi_wl_init(struct ubi_device *ubi, struct ubi_attach_info *ai); void ubi_wl_close(struct ubi_device *ubi); diff --git a/drivers/mtd/ubi/upd.c b/drivers/mtd/ubi/upd.c index 11a28f9ce0db..9f2ebd8750e7 100644 --- a/drivers/mtd/ubi/upd.c +++ b/drivers/mtd/ubi/upd.c @@ -147,7 +147,7 @@ int ubi_start_update(struct ubi_device *ubi, struct ubi_volume *vol, } if (bytes == 0) { - err = ubi_wl_flush(ubi); + err = ubi_wl_flush(ubi, UBI_ALL, UBI_ALL); if (err) return err; @@ -361,7 +361,7 @@ int ubi_more_update_data(struct ubi_device *ubi, struct ubi_volume *vol, ubi_assert(vol->upd_received <= vol->upd_bytes); if (vol->upd_received == vol->upd_bytes) { - err = ubi_wl_flush(ubi); + err = ubi_wl_flush(ubi, UBI_ALL, UBI_ALL); if (err) return err; /* The update is finished, clear the update marker */ diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c index e4b897ab4543..0669cff8ac3c 100644 --- a/drivers/mtd/ubi/vmt.c +++ b/drivers/mtd/ubi/vmt.c @@ -280,7 +280,7 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req) * Finish all pending erases because there may be some LEBs belonging * to the same volume ID. */ - err = ubi_wl_flush(ubi); + err = ubi_wl_flush(ubi, vol_id, UBI_ALL); if (err) goto out_acc; diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c index 70ebfa7bc384..9df100a4ec38 100644 --- a/drivers/mtd/ubi/wl.c +++ b/drivers/mtd/ubi/wl.c @@ -1241,44 +1241,55 @@ retry: /** * ubi_wl_flush - flush all pending works. * @ubi: UBI device description object + * @vol_id: the volume id to flush for + * @lnum: the logical eraseblock number to flush for * - * This function returns zero in case of success and a negative error code in - * case of failure. + * This function executes all pending works for a particular volume id / + * logical eraseblock number pair. If either value is set to %UBI_ALL, then it + * acts as a wildcard for all of the corresponding volume numbers or logical + * eraseblock numbers. It returns zero in case of success and a negative error + * code in case of failure. */ -int ubi_wl_flush(struct ubi_device *ubi) +int ubi_wl_flush(struct ubi_device *ubi, int vol_id, int lnum) { - int err; + int err = 0; + int found = 1; /* * Erase while the pending works queue is not empty, but not more than * the number of currently pending works. */ - dbg_wl("flush (%d pending works)", ubi->works_count); - while (ubi->works_count) { - err = do_work(ubi); - if (err) - return err; - } + dbg_wl("flush pending work for LEB %d:%d (%d pending works)", + vol_id, lnum, ubi->works_count); - /* - * Make sure all the works which have been done in parallel are - * finished. - */ down_write(&ubi->work_sem); - up_write(&ubi->work_sem); + while (found) { + struct ubi_work *wrk; + found = 0; - /* - * And in case last was the WL worker and it canceled the LEB - * movement, flush again. - */ - while (ubi->works_count) { - dbg_wl("flush more (%d pending works)", ubi->works_count); - err = do_work(ubi); - if (err) - return err; + spin_lock(&ubi->wl_lock); + list_for_each_entry(wrk, &ubi->works, list) { + if ((vol_id == UBI_ALL || wrk->vol_id == vol_id) && + (lnum == UBI_ALL || wrk->lnum == lnum)) { + 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) + goto out; + spin_lock(&ubi->wl_lock); + found = 1; + break; + } + } + spin_unlock(&ubi->wl_lock); } - return 0; +out: + up_write(&ubi->work_sem); + return err; } /** diff --git a/include/linux/mtd/ubi.h b/include/linux/mtd/ubi.h index f636f5df883f..c3918a0684fe 100644 --- a/include/linux/mtd/ubi.h +++ b/include/linux/mtd/ubi.h @@ -219,6 +219,7 @@ int ubi_leb_unmap(struct ubi_volume_desc *desc, int lnum); int ubi_leb_map(struct ubi_volume_desc *desc, int lnum); int ubi_is_mapped(struct ubi_volume_desc *desc, int lnum); int ubi_sync(int ubi_num); +int ubi_flush(int ubi_num, int vol_id, int lnum); /* * This function is the same as the 'ubi_leb_read()' function, but it does not -- cgit v1.2.3