2011-06-24 13:34:40

by David Wagner

[permalink] [raw]
Subject: [RFC] ubiblk: read-only block layer on top of UBI

From: David Wagner <[email protected]>

It creates one device for each UBI volume and dynamically add/remove one when a
UBI volume is created or deleted : it registers to UBI notifications. The
devices are names "ubiblkX_Y" where X is the UBI device number and Y is the
volume ID.

I'm submitting it for review, comments (on the concept as well as on the
implementation) and advice. It is my first kernel module - a lot of code was
taken from mtd_blkdevs and some from gluebi.

It is only known to work well with SquashFS. Tests reported that with other
filesystems (ext2/3 and vfat), some bytes in some files (I couldn't find any
pattern), when read through ubiblk, don't match the original content (the
content of a file becomes random on several "lines" and goes back to normal).
The reason for that is still unknown.

Some other known issues are listed in the commit message.

drivers/mtd/ubi/Kconfig | 9 +
drivers/mtd/ubi/Makefile | 1 +
drivers/mtd/ubi/ubiblk.c | 462 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 472 insertions(+), 0 deletions(-)
create mode 100644 drivers/mtd/ubi/ubiblk.c

--
David Wagner, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com


2011-06-24 13:34:51

by David Wagner

[permalink] [raw]
Subject: [PATCH] UBI: new module ubiblk: block layer on top of UBI

From: David Wagner <[email protected]>

ubiblk is a read-only block layer on top of UBI. It presents UBI volumes as
read-only block devices.

It is used by putting a block filesystem image on a UBI volume and then mounting
the corresponding device.

It uses the UBI API to register to UBI notifications (to dynamically create and
delete devices as volumes are added or removed) and to read from the volumes.

Known issues:
* UBI_VOLUME_RESIZED notification hook isn't implemented yet ;

* only squashfs is known to work. With ext2/3 and vfat, errors appear randomly
in the middle of some files. The reason for that is still unknown ;

* the modules keeps a table of the devices which length is the maximum number
of UBI volumes. It should make use of a linked list.

A lot of code is taken from mtd_blkdevs and gluebi

Signed-off-by: David Wagner <[email protected]>
---
drivers/mtd/ubi/Kconfig | 9 +
drivers/mtd/ubi/Makefile | 1 +
drivers/mtd/ubi/ubiblk.c | 462 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 472 insertions(+), 0 deletions(-)
create mode 100644 drivers/mtd/ubi/ubiblk.c

diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index 4dcc752..389a996 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -60,4 +60,13 @@ config MTD_UBI_DEBUG
help
This option enables UBI debugging.

+config MTD_UBI_UBIBLK
+ tristate "Read-only block transition layer on top of UBI"
+ help
+ Read-only block interface on top of UBI.
+
+ Creates block devices that can be used to mount read-only block
+ filesystems. Caches reads on a raw UBI volume. Filesystems images
+ can be flashed using ubiupdatevol or put in a UBI image with
+ ubinize
endif # MTD_UBI
diff --git a/drivers/mtd/ubi/Makefile b/drivers/mtd/ubi/Makefile
index c9302a5..354b2df 100644
--- a/drivers/mtd/ubi/Makefile
+++ b/drivers/mtd/ubi/Makefile
@@ -5,3 +5,4 @@ ubi-y += misc.o

ubi-$(CONFIG_MTD_UBI_DEBUG) += debug.o
obj-$(CONFIG_MTD_UBI_GLUEBI) += gluebi.o
+obj-$(CONFIG_MTD_UBI_UBIBLK) += ubiblk.o
diff --git a/drivers/mtd/ubi/ubiblk.c b/drivers/mtd/ubi/ubiblk.c
new file mode 100644
index 0000000..aab940a
--- /dev/null
+++ b/drivers/mtd/ubi/ubiblk.c
@@ -0,0 +1,462 @@
+/*
+ * Copyright (c) Free Electrons, 2011
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
+ * the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Author: David Wagner
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/vmalloc.h>
+#include <linux/mtd/ubi.h>
+#include <linux/blkdev.h>
+#include <linux/kthread.h>
+#include <linux/mutex.h>
+#include "ubi.h"
+
+#define BLK_SIZE 512
+
+#define UBIBLK_MAX_DEVS (UBI_MAX_DEVICES * UBI_MAX_VOLUMES)
+
+/*
+ * Structure representing a ubiblk device, proxying a UBI volume
+ */
+struct ubiblk_dev {
+ struct ubi_volume_desc *vol_desc;
+ struct ubi_volume_info *vol_info;
+ int ubi_num;
+ int vol_id;
+
+ /* Block stuff */
+ struct gendisk *gd;
+ struct request_queue *rq;
+ struct task_struct *thread;
+
+ /* Protects the access to the UBI volume */
+ struct mutex lock;
+
+ /* Avoids concurrent accesses to the request queue */
+ spinlock_t queue_lock;
+};
+
+/*
+ * Contains the pointers to all ubiblk_dev instances
+ * TODO: use a linked list
+ */
+static struct ubiblk_dev *ubiblk_devs[UBIBLK_MAX_DEVS];
+static struct mutex devtable_lock;
+
+int major;
+static const struct block_device_operations ubiblk_ops;
+
+/*
+ * Read a LEB and fill the request buffer with the requested sector
+ */
+static int do_ubiblk_request(struct request *req, struct ubiblk_dev *dev)
+{
+ unsigned long start, len;
+ int offset;
+ int leb;
+ int ret;
+
+ start = blk_rq_pos(req) << 9;
+ len = blk_rq_cur_bytes(req);
+
+ /* We are always reading. No need to handle writing for now */
+
+ leb = start / dev->vol_info->usable_leb_size;
+ offset = start % dev->vol_info->usable_leb_size;
+
+ if (offset + len > dev->vol_info->usable_leb_size)
+ len = dev->vol_info->usable_leb_size - offset;
+
+ pr_debug("%s(%s) of sector %llu (LEB %d). start=%lu, len=%lu\n",
+ __func__, rq_data_dir(req) ? "Write" : "Read",
+ blk_rq_pos(req), leb, start, len);
+
+ ret = ubi_read(dev->vol_desc, leb, req->buffer, offset, len);
+
+ if (ret) {
+ pr_err("ubi_read error\n");
+ return ret;
+ }
+
+ pr_debug("ubi_read done.\n");
+
+ return 0;
+}
+
+static void ubi_ubiblk_request(struct request_queue *rq)
+{
+ struct ubiblk_dev *dev;
+ struct request *req = NULL;
+
+ dev = rq->queuedata;
+
+ if (!dev)
+ while ((req = blk_fetch_request(rq)) != NULL)
+ __blk_end_request_all(req, -ENODEV);
+ else
+ wake_up_process(dev->thread);
+}
+
+/*
+ * Open a UBI volume (get the volume descriptor)
+ */
+static int ubiblk_open(struct block_device *bdev, fmode_t mode)
+{
+ struct ubiblk_dev *dev = bdev->bd_disk->private_data;
+ pr_debug("%s() disk_name=%s, mode=%d\n", __func__,
+ bdev->bd_disk->disk_name, mode);
+
+ dev->vol_desc = ubi_open_volume(dev->ubi_num, dev->vol_id,
+ UBI_READONLY);
+ if (!dev->vol_desc) {
+ pr_err("open_volume failed");
+ return -EINVAL;
+ }
+
+ dev->vol_info = kzalloc(sizeof(struct ubi_volume_info), GFP_KERNEL);
+ if (!dev->vol_info) {
+ ubi_close_volume(dev->vol_desc);
+ dev->vol_desc = NULL;
+ return -ENOMEM;
+ }
+ ubi_get_volume_info(dev->vol_desc, dev->vol_info);
+
+ return 0;
+}
+
+/*
+ * Close a UBI volume (close the volume descriptor)
+ */
+static int ubiblk_release(struct gendisk *gd, fmode_t mode)
+{
+ struct ubiblk_dev *dev = gd->private_data;
+ pr_debug("%s() disk_name=%s, mode=%d\n", __func__, gd->disk_name,
+ mode);
+
+ kfree(dev->vol_info);
+ dev->vol_info = NULL;
+ if (dev->vol_desc) {
+ ubi_close_volume(dev->vol_desc);
+ dev->vol_desc = NULL;
+ }
+
+ return 0;
+}
+
+/*
+ * Loop on the block request queue and wait for new requests ; run them with
+ * do_ubiblk_request()
+ *
+ * Mostly stolen from mtd_blkdevs.c
+ */
+static int ubi_ubiblk_thread(void *arg)
+{
+ struct ubiblk_dev *dev = arg;
+ struct request_queue *rq = dev->rq;
+ struct request *req = NULL;
+
+ spin_lock_irq(rq->queue_lock);
+
+ while (!kthread_should_stop()) {
+ int res;
+
+ if (!req && !(req = blk_fetch_request(rq))) {
+ set_current_state(TASK_INTERRUPTIBLE);
+
+ if (kthread_should_stop())
+ set_current_state(TASK_RUNNING);
+
+ spin_unlock_irq(rq->queue_lock);
+ schedule();
+ spin_lock_irq(rq->queue_lock);
+ continue;
+ }
+
+ spin_unlock_irq(rq->queue_lock);
+
+ mutex_lock(&dev->lock);
+ res = do_ubiblk_request(req, dev);
+ pr_debug("return from request: %d\n", res);
+ mutex_unlock(&dev->lock);
+
+ spin_lock_irq(rq->queue_lock);
+
+ if (!__blk_end_request_cur(req, res))
+ req = NULL;
+ }
+
+ if (req)
+ __blk_end_request_all(req, -EIO);
+
+ spin_unlock_irq(rq->queue_lock);
+
+ return 0;
+}
+
+static int ubiblk_create(struct ubi_device_info *dev_info,
+ struct ubi_volume_info *vol_info)
+{
+ struct ubiblk_dev *dev;
+ struct gendisk *gd;
+ int i;
+ int ret = 0;
+
+ mutex_lock(&devtable_lock);
+ for (i = 0; i < UBIBLK_MAX_DEVS; i++)
+ if (!ubiblk_devs[i])
+ break;
+
+ if (i == UBIBLK_MAX_DEVS) {
+ /* Shouldn't happen: UBI can't make more volumes than that */
+ pr_err("no slot left for a new ubiblk device.\n");
+ mutex_unlock(&devtable_lock);
+ return -ENOMEM;
+ }
+
+ dev = kzalloc(sizeof(struct ubiblk_dev), GFP_KERNEL);
+ if (!dev) {
+ pr_err("UBIBLK: ENOMEM when trying to create a new"
+ "ubiblk dev\n");
+ mutex_unlock(&devtable_lock);
+ return -ENOMEM;
+ }
+ ubiblk_devs[i] = dev;
+ mutex_unlock(&devtable_lock);
+
+ mutex_init(&dev->lock);
+ mutex_lock(&dev->lock);
+
+ dev->ubi_num = vol_info->ubi_num;
+ dev->vol_id = vol_info->vol_id;
+
+ dev->vol_desc = ubi_open_volume(dev->ubi_num, dev->vol_id,
+ UBI_READONLY);
+ if (IS_ERR(dev->vol_desc)) {
+ pr_err("open_volume failed\n");
+ ret = PTR_ERR(dev->vol_desc);
+ goto out_vol;
+ }
+
+ dev->vol_info = kzalloc(sizeof(struct ubi_volume_info), GFP_KERNEL);
+ if (!dev->vol_info) {
+ ret = -ENOMEM;
+ goto out_info;
+ }
+ ubi_get_volume_info(dev->vol_desc, dev->vol_info);
+
+ pr_info("Got volume %s: device %d/volume %d of size %d\n",
+ dev->vol_info->name, dev->ubi_num, dev->vol_id,
+ dev->vol_info->size);
+
+ /* Initialize the gendisk of this ubiblk device */
+ gd = alloc_disk(1);
+ if (!gd) {
+ pr_err("alloc_disk failed\n");
+ ret = -ENODEV;
+ goto out_disk;
+ }
+
+ gd->fops = &ubiblk_ops;
+ gd->major = major;
+ gd->first_minor = dev->ubi_num * UBI_MAX_VOLUMES + dev->vol_id;
+ gd->private_data = dev;
+ sprintf(gd->disk_name, "ubiblk%d_%d", dev->ubi_num, dev->vol_id);
+ pr_debug("creating a gd '%s'\n", gd->disk_name);
+ set_capacity(gd,
+ (dev->vol_info->size *
+ dev->vol_info->usable_leb_size) >> 9);
+ set_disk_ro(gd, 1);
+ dev->gd = gd;
+
+ spin_lock_init(&dev->queue_lock);
+ dev->rq = blk_init_queue(ubi_ubiblk_request, &dev->queue_lock);
+ if (!dev->rq) {
+ pr_err("init_queue failed\n");
+ ret = -ENODEV;
+ goto out_queue;
+ }
+ dev->rq->queuedata = dev;
+ blk_queue_logical_block_size(dev->rq, BLK_SIZE);
+ dev->gd->queue = dev->rq;
+
+ /* Stolen from mtd_blkdevs.c */
+ /* Create processing thread */
+ dev->thread = kthread_run(ubi_ubiblk_thread, dev, "%s%d_%d",
+ "kubiblkd", dev->ubi_num, dev->vol_id);
+ if (IS_ERR(dev->thread)) {
+ ret = PTR_ERR(dev->thread);
+ goto out_thread;
+ }
+
+ add_disk(dev->gd);
+ kfree(dev->vol_info);
+ dev->vol_info = NULL;
+ ubi_close_volume(dev->vol_desc);
+ dev->vol_desc = NULL;
+ mutex_unlock(&dev->lock);
+
+ return 0;
+
+out_thread:
+ blk_cleanup_queue(dev->rq);
+out_queue:
+ put_disk(dev->gd);
+out_disk:
+ kfree(dev->vol_info);
+ dev->vol_info = NULL;
+out_info:
+ ubi_close_volume(dev->vol_desc);
+ dev->vol_desc = NULL;
+out_vol:
+ mutex_unlock(&dev->lock);
+
+ return ret;
+}
+
+static int ubiblk_remove(struct ubi_volume_info *vol_info)
+{
+ int i;
+ struct ubiblk_dev *dev;
+
+ for (i = 0; i < UBIBLK_MAX_DEVS; i++) {
+ dev = ubiblk_devs[i];
+ if (dev && dev->ubi_num == vol_info->ubi_num &&
+ dev->vol_id == vol_info->vol_id)
+ break;
+ }
+ if (i == UBIBLK_MAX_DEVS) {
+ pr_warn("Trying to remove %s, which is unknown from ubiblk\n",
+ vol_info->name);
+ return -ENODEV;
+ }
+
+ pr_info("Removing %s\n", vol_info->name);
+
+ if (dev->vol_desc)
+ ubi_close_volume(dev->vol_desc);
+
+ del_gendisk(dev->gd);
+ blk_cleanup_queue(dev->rq);
+ kthread_stop(dev->thread);
+ put_disk(dev->gd);
+
+ kfree(dev->vol_info);
+
+ kfree(ubiblk_devs[i]);
+ ubiblk_devs[i] = NULL;
+
+ return 0;
+}
+
+static int ubiblk_notify(struct notifier_block *nb,
+ unsigned long notification_type, void *ns_ptr)
+{
+ struct ubi_notification *nt = ns_ptr;
+
+ switch (notification_type) {
+ case UBI_VOLUME_ADDED:
+ ubiblk_create(&nt->di, &nt->vi);
+ break;
+ case UBI_VOLUME_REMOVED:
+ /* TODO */
+ ubiblk_remove(&nt->vi);
+ break;
+ case UBI_VOLUME_RESIZED:
+ /* TODO. needed ? */
+ /* gluebi_resized(&nt->vi); */
+ break;
+ case UBI_VOLUME_UPDATED:
+ break;
+ case UBI_VOLUME_RENAMED:
+ break;
+ default:
+ break;
+ }
+ return NOTIFY_OK;
+}
+
+static const struct block_device_operations ubiblk_ops = {
+ .owner = THIS_MODULE,
+ .open = ubiblk_open,
+ .release = ubiblk_release,
+};
+
+static struct notifier_block ubiblk_notifier = {
+ .notifier_call = ubiblk_notify,
+};
+
+/*
+ * Initialize the module
+ */
+static int __init ubi_ubiblk_init(void)
+{
+ int ret;
+
+ pr_info("UBIBLK starting\n");
+
+ ret = register_blkdev(0, "ubiblk");
+ if (ret <= 0) {
+ pr_err("UBIBLK: could not register_blkdev\n");
+ return -ENODEV;
+ }
+ major = ret;
+ pr_info("UBIBLK: device's major: %d\n", major);
+
+ mutex_init(&devtable_lock);
+ ret = ubi_register_volume_notifier(&ubiblk_notifier, 0);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static void __exit ubi_ubiblk_exit(void)
+{
+ int i;
+
+ pr_info("UBIBLK: going to exit\n");
+
+ ubi_unregister_volume_notifier(&ubiblk_notifier);
+
+ for (i = 0; i < UBIBLK_MAX_DEVS; i++) {
+ struct ubiblk_dev *dev = ubiblk_devs[i];
+ if (!dev)
+ continue;
+
+ if (dev->vol_desc)
+ ubi_close_volume(dev->vol_desc);
+
+ del_gendisk(dev->gd);
+ blk_cleanup_queue(dev->rq);
+ kthread_stop(dev->thread);
+ put_disk(dev->gd);
+
+ kfree(dev->vol_info);
+ kfree(dev);
+ }
+
+ unregister_blkdev(major, "ubiblk");
+ pr_info("UBIBLK: The End\n");
+}
+
+module_init(ubi_ubiblk_init);
+module_exit(ubi_ubiblk_exit);
+MODULE_DESCRIPTION("Read-only block transition layer on top of UBI");
+MODULE_AUTHOR("David Wagner");
+MODULE_LICENSE("GPL");
--
1.7.0.4

2011-06-24 13:45:06

by David Wagner

[permalink] [raw]
Subject: [Addendum][RFC] ubiblk: read-only block layer on top of UBI

On 06/24/2011 03:34 PM, [email protected] wrote:

> drivers/mtd/ubi/Kconfig | 9 +
> drivers/mtd/ubi/Makefile | 1 +
> drivers/mtd/ubi/ubiblk.c | 462 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 472 insertions(+), 0 deletions(-)
> create mode 100644 drivers/mtd/ubi/ubiblk.c

The patch was made against tag v2.6.39

The Kconfig description states that the module caches the reads from UBI
volume ; it isn't true.
--
David Wagner, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

2011-06-27 19:14:40

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [RFC] ubiblk: read-only block layer on top of UBI

Hi,

thanks, for the driver.

On Fri, 2011-06-24 at 15:34 +0200, [email protected]
wrote:
> I'm submitting it for review, comments (on the concept as well as on the
> implementation) and advice. It is my first kernel module - a lot of code was
> taken from mtd_blkdevs and some from gluebi.

If you borrow code from anywhere, you should preserve Copyrights.

--
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

2011-06-27 19:26:20

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [PATCH] UBI: new module ubiblk: block layer on top of UBI

On Fri, 2011-06-24 at 15:34 +0200, [email protected]
wrote:
> + /* Stolen from mtd_blkdevs.c */
> + /* Create processing thread */
> + dev->thread = kthread_run(ubi_ubiblk_thread, dev, "%s%d_%d",
> + "kubiblkd", dev->ubi_num, dev->vol_id);
> + if (IS_ERR(dev->thread)) {
> + ret = PTR_ERR(dev->thread);
> + goto out_thread;
> + }

Why we need a kernel thread? Could you please describe when exactly it
is needed and why we cannot avoid having it?

> + ret = register_blkdev(0, "ubiblk");
> + if (ret <= 0) {
> + pr_err("UBIBLK: could not register_blkdev\n");
> + return -ENODEV;
> + }
> + major = ret;
> + pr_info("UBIBLK: device's major: %d\n", major);
> +
> + mutex_init(&devtable_lock);
> + ret = ubi_register_volume_notifier(&ubiblk_notifier, 0);
> + if (ret < 0)
> + return ret;

You should probably de-register the blkdev when you fail here.

--
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

2011-06-28 11:35:37

by David Wagner

[permalink] [raw]
Subject: Re: [PATCH] UBI: new module ubiblk: block layer on top of UBI

Hi,

On 06/27/2011 09:26 PM, Artem Bityutskiy wrote:
> On Fri, 2011-06-24 at 15:34 +0200, [email protected]
> wrote:
>> + /* Stolen from mtd_blkdevs.c */
>> + /* Create processing thread */
>> + dev->thread = kthread_run(ubi_ubiblk_thread, dev, "%s%d_%d",
>> + "kubiblkd", dev->ubi_num, dev->vol_id);
>> + if (IS_ERR(dev->thread)) {
>> + ret = PTR_ERR(dev->thread);
>> + goto out_thread;
>> + }
>
> Why we need a kernel thread? Could you please describe when exactly it
> is needed and why we cannot avoid having it?

Do you mean that there could be another/better way ?
I read that workqueues could be used for that but since they seem to
internally use kthreads, I don't see the advantage yet. Simpler API ?

I also tried without a kthread altogether (and call do_ubiblk_request
directly within the callback registered with blk_init_queue) but got
lost in locks/context debugging ...

It seems that do_ubiblk_request needs to be in process context because
there are thousands causes for blocking (locking, page fault, for
instance, are the one I encountered). And on the other hand,
blk_run_queue must not block ; So we need to wake the thread up and
return (what ubi_ubiblk_request does).
So, would this be a sufficient justification ?


It's probably possible, however, to have only one thread for the whole
module instead of having one for each volume ; but that seemed good
enough on first approach.


I fixed the read errors issue with filesystems != SquashFS, so they
should all work, now.
I'll send the next iteration, probably later today.

--
David Wagner, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

2011-06-28 14:52:39

by Matthieu CASTET

[permalink] [raw]
Subject: Re: [PATCH] UBI: new module ubiblk: block layer on top of UBI

Hi,

Artem Bityutskiy a écrit :
> On Fri, 2011-06-24 at 15:34 +0200, [email protected]
> wrote:
>> + /* Stolen from mtd_blkdevs.c */
>> + /* Create processing thread */
>> + dev->thread = kthread_run(ubi_ubiblk_thread, dev, "%s%d_%d",
>> + "kubiblkd", dev->ubi_num, dev->vol_id);
>> + if (IS_ERR(dev->thread)) {
>> + ret = PTR_ERR(dev->thread);
>> + goto out_thread;
>> + }
>
> Why we need a kernel thread? Could you please describe when exactly it
> is needed and why we cannot avoid having it?
Also what are the advantage against gluebi + mtdblock_ro ?

2011-06-28 15:27:04

by David Wagner

[permalink] [raw]
Subject: [RFC PATCHv2] UBI: new module ubiblk: block layer on top of UBI

From: David Wagner <[email protected]>

Hi,

This is next iteration ; the read errors issue with FS that do large requests
has been fixed (it means that all block filesystems should now work) ; the
'resized' callback is implemented.

The reuse of code from mtd_blkdevs and gluebi is now mentionned is the file
header.
The use of a kernel thread is also justified (line 353).
The Kconfig has also been fixed.

I also tested compiling ubiblk statically into the kernel ; it worked and I was
able to mount a squashfs ubiblk device as root with, for instance:
"ubi.mtd=1 root=/dev/ubiblk0_0"

Performances still haven't been tested, though.


Thanks to those for review it !
David.

PS: I included the commit message+diff in the same mail as the introduction for
convenience ; tell me if it annoys you - I'll split the next iterations as I did
for the first one.


ubiblk is a read-only block layer on top of UBI. It presents UBI volumes as
read-only block devices.

It is used by putting a block filesystem image on a UBI volume and then mounting
the corresponding device.

It uses the UBI API to register to UBI notifications (to dynamically create and
delete devices as volumes are added or removed) and to read from the volumes.

Known issues:
* the modules keeps a table of the devices which length is the maximum number
of UBI volumes. It should make use of a linked list.

Some code is taken from mtd_blkdevs and gluebi

Signed-off-by: David Wagner <[email protected]>
---
drivers/mtd/ubi/Kconfig | 12 +
drivers/mtd/ubi/Makefile | 1 +
drivers/mtd/ubi/ubiblk.c | 554 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 567 insertions(+), 0 deletions(-)
create mode 100644 drivers/mtd/ubi/ubiblk.c

diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index 4dcc752..d19508b 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -60,4 +60,16 @@ config MTD_UBI_DEBUG
help
This option enables UBI debugging.

+config MTD_UBI_UBIBLK
+ tristate "Read-only block transition layer on top of UBI"
+ help
+ Read-only block interface on top of UBI.
+
+ This option adds ubiblk, which creates a read-ony block device for
+ each UBI volume. It makes it possible to use block filesystems on
+ top of UBI (and thus, on top of MTDs while avoiding bad blocks).
+
+ The devices are named ubiblkX_Y where X is the UBI number and Y is
+ the Volume ID.
+
endif # MTD_UBI
diff --git a/drivers/mtd/ubi/Makefile b/drivers/mtd/ubi/Makefile
index c9302a5..354b2df 100644
--- a/drivers/mtd/ubi/Makefile
+++ b/drivers/mtd/ubi/Makefile
@@ -5,3 +5,4 @@ ubi-y += misc.o

ubi-$(CONFIG_MTD_UBI_DEBUG) += debug.o
obj-$(CONFIG_MTD_UBI_GLUEBI) += gluebi.o
+obj-$(CONFIG_MTD_UBI_UBIBLK) += ubiblk.o
diff --git a/drivers/mtd/ubi/ubiblk.c b/drivers/mtd/ubi/ubiblk.c
new file mode 100644
index 0000000..53e6d4e
--- /dev/null
+++ b/drivers/mtd/ubi/ubiblk.c
@@ -0,0 +1,554 @@
+/*
+ * Copyright (c) Free Electrons, 2011
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
+ * the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Author: David Wagner
+ * Some code taken from gluebi.c (Artem Bityutskiy (Битюцкий Артём),
+ * Joern Engel)
+ * Some code taken from mtd_blkdevs.c (David Woodhouse)
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/vmalloc.h>
+#include <linux/mtd/ubi.h>
+#include <linux/blkdev.h>
+#include <linux/kthread.h>
+#include <linux/mutex.h>
+#include "ubi.h"
+
+#define BLK_SIZE 512
+
+#define UBIBLK_MAX_DEVS (UBI_MAX_DEVICES * UBI_MAX_VOLUMES)
+
+/*
+ * Structure representing a ubiblk device, proxying a UBI volume
+ */
+struct ubiblk_dev {
+ struct ubi_volume_desc *vol_desc;
+ struct ubi_volume_info *vol_info;
+ int ubi_num;
+ int vol_id;
+
+ /* Block stuff */
+ struct gendisk *gd;
+ struct request_queue *rq;
+ struct task_struct *thread;
+
+ /* Protects the access to the UBI volume */
+ struct mutex lock;
+
+ /* Avoids concurrent accesses to the request queue */
+ spinlock_t queue_lock;
+};
+
+/*
+ * Contains the pointers to all ubiblk_dev instances
+ * TODO: use a linked list
+ */
+static struct ubiblk_dev *ubiblk_devs[UBIBLK_MAX_DEVS];
+static struct mutex devtable_lock;
+
+int major;
+static const struct block_device_operations ubiblk_ops;
+
+static struct ubiblk_dev *ubiblk_find_dev(struct ubi_volume_info *vol_info)
+{
+ int i;
+ struct ubiblk_dev *dev;
+
+ mutex_lock(&devtable_lock);
+ for (i = 0; i < UBIBLK_MAX_DEVS; i++) {
+ dev = ubiblk_devs[i];
+ if (dev && dev->ubi_num == vol_info->ubi_num &&
+ dev->vol_id == vol_info->vol_id)
+ break;
+ }
+ mutex_unlock(&devtable_lock);
+ if (i == UBIBLK_MAX_DEVS)
+ return NULL;
+ return dev;
+}
+
+/*
+ * Read a LEB and fill the request buffer with the requested sector
+ */
+static int do_ubiblk_request(struct request *req, struct ubiblk_dev *dev)
+{
+ unsigned long start, len, read_bytes;
+ int offset;
+ int leb;
+ int ret;
+
+ start = blk_rq_pos(req) << 9;
+ len = blk_rq_cur_bytes(req);
+ read_bytes = 0;
+
+ /* We are always reading. No need to handle writing for now */
+
+ leb = start / dev->vol_info->usable_leb_size;
+ offset = start % dev->vol_info->usable_leb_size;
+
+ do {
+ int overlap = 0;
+
+ if (offset + len > dev->vol_info->usable_leb_size) {
+ len = dev->vol_info->usable_leb_size - offset;
+ overlap = 1;
+ }
+
+ if (blk_rq_pos(req) + blk_rq_cur_sectors(req) >
+ get_capacity(req->rq_disk)) {
+ pr_err("UBIBLK: attempting to read too far\n");
+ return -EIO;
+ }
+
+ pr_debug("%s(%s) of sector %llu (LEB %d). offset=%d, len=%lu\n",
+ __func__, rq_data_dir(req) ? "Write" : "Read",
+ blk_rq_pos(req), leb, offset, len);
+
+ /* Read (len) bytes of LEB (leb) from (offset) and put the
+ * result in the buffer given by the request ; if the request
+ * is overlapping on several lebs, (read) will be > 0 and the
+ * data will be put in the buffer at offset (read) */
+ ret = ubi_read(dev->vol_desc, leb, req->buffer + read_bytes,
+ offset, len);
+
+ if (ret) {
+ pr_err("ubi_read error\n");
+ return ret;
+ }
+
+ read_bytes += len;
+ len = blk_rq_cur_bytes(req) - read_bytes;
+
+ /* If we needed to cap the length of ubi_read, the next
+ * ubi_read will be done on the beginning of the next LEB */
+ if (overlap) {
+ leb++;
+ offset = 0;
+ overlap = 0;
+ }
+ } while (read_bytes < blk_rq_cur_bytes(req));
+
+ pr_debug("ubi_read done.\n");
+
+ return 0;
+}
+
+static void ubi_ubiblk_request(struct request_queue *rq)
+{
+ struct ubiblk_dev *dev;
+ struct request *req = NULL;
+
+ dev = rq->queuedata;
+
+ if (!dev)
+ while ((req = blk_fetch_request(rq)) != NULL)
+ __blk_end_request_all(req, -ENODEV);
+ else
+ wake_up_process(dev->thread);
+}
+
+/*
+ * Open a UBI volume (get the volume descriptor)
+ */
+static int ubiblk_open(struct block_device *bdev, fmode_t mode)
+{
+ struct ubiblk_dev *dev = bdev->bd_disk->private_data;
+ pr_debug("%s() disk_name=%s, mode=%d\n", __func__,
+ bdev->bd_disk->disk_name, mode);
+
+ dev->vol_desc = ubi_open_volume(dev->ubi_num, dev->vol_id,
+ UBI_READONLY);
+ if (!dev->vol_desc) {
+ pr_err("open_volume failed");
+ return -EINVAL;
+ }
+
+ dev->vol_info = kzalloc(sizeof(struct ubi_volume_info), GFP_KERNEL);
+ if (!dev->vol_info) {
+ ubi_close_volume(dev->vol_desc);
+ dev->vol_desc = NULL;
+ return -ENOMEM;
+ }
+ ubi_get_volume_info(dev->vol_desc, dev->vol_info);
+
+ return 0;
+}
+
+/*
+ * Close a UBI volume (close the volume descriptor)
+ */
+static int ubiblk_release(struct gendisk *gd, fmode_t mode)
+{
+ struct ubiblk_dev *dev = gd->private_data;
+ pr_debug("%s() disk_name=%s, mode=%d\n", __func__, gd->disk_name, mode);
+
+ kfree(dev->vol_info);
+ dev->vol_info = NULL;
+ if (dev->vol_desc) {
+ ubi_close_volume(dev->vol_desc);
+ dev->vol_desc = NULL;
+ }
+
+ return 0;
+}
+
+/*
+ * Loop on the block request queue and wait for new requests ; run them with
+ * do_ubiblk_request()
+ *
+ * Mostly stolen from mtd_blkdevs.c
+ */
+static int ubi_ubiblk_thread(void *arg)
+{
+ struct ubiblk_dev *dev = arg;
+ struct request_queue *rq = dev->rq;
+ struct request *req = NULL;
+
+ spin_lock_irq(rq->queue_lock);
+
+ while (!kthread_should_stop()) {
+ int res;
+
+ if (!req && !(req = blk_fetch_request(rq))) {
+ set_current_state(TASK_INTERRUPTIBLE);
+
+ if (kthread_should_stop())
+ set_current_state(TASK_RUNNING);
+
+ spin_unlock_irq(rq->queue_lock);
+ schedule();
+ spin_lock_irq(rq->queue_lock);
+ continue;
+ }
+
+ spin_unlock_irq(rq->queue_lock);
+
+ mutex_lock(&dev->lock);
+ res = do_ubiblk_request(req, dev);
+ pr_debug("return from request: %d\n", res);
+ mutex_unlock(&dev->lock);
+
+ spin_lock_irq(rq->queue_lock);
+
+ if (!__blk_end_request_cur(req, res))
+ req = NULL;
+ }
+
+ if (req)
+ __blk_end_request_all(req, -EIO);
+
+ spin_unlock_irq(rq->queue_lock);
+
+ return 0;
+}
+
+/*
+ * An UBI volume has been created ; create a corresponding ubiblk device:
+ * Initialize the locks, the structure, the block layer infos and start a
+ * thread.
+ */
+static int ubiblk_create(struct ubi_device_info *dev_info,
+ struct ubi_volume_info *vol_info)
+{
+ struct ubiblk_dev *dev;
+ struct gendisk *gd;
+ int i;
+ int ret = 0;
+
+ mutex_lock(&devtable_lock);
+ for (i = 0; i < UBIBLK_MAX_DEVS; i++)
+ if (!ubiblk_devs[i])
+ break;
+
+ if (i == UBIBLK_MAX_DEVS) {
+ /* Shouldn't happen: UBI can't make more volumes than that */
+ pr_err("no slot left for a new ubiblk device.\n");
+ mutex_unlock(&devtable_lock);
+ return -ENOMEM;
+ }
+
+ dev = kzalloc(sizeof(struct ubiblk_dev), GFP_KERNEL);
+ if (!dev) {
+ pr_err("UBIBLK: ENOMEM when trying to create a new"
+ "ubiblk dev\n");
+ mutex_unlock(&devtable_lock);
+ return -ENOMEM;
+ }
+ ubiblk_devs[i] = dev;
+ mutex_unlock(&devtable_lock);
+
+ mutex_init(&dev->lock);
+ mutex_lock(&dev->lock);
+
+ dev->ubi_num = vol_info->ubi_num;
+ dev->vol_id = vol_info->vol_id;
+
+ dev->vol_desc = ubi_open_volume(dev->ubi_num, dev->vol_id,
+ UBI_READONLY);
+ if (IS_ERR(dev->vol_desc)) {
+ pr_err("open_volume failed\n");
+ ret = PTR_ERR(dev->vol_desc);
+ goto out_vol;
+ }
+
+ dev->vol_info = kzalloc(sizeof(struct ubi_volume_info), GFP_KERNEL);
+ if (!dev->vol_info) {
+ ret = -ENOMEM;
+ goto out_info;
+ }
+ ubi_get_volume_info(dev->vol_desc, dev->vol_info);
+
+ pr_info("Got volume %s: device %d/volume %d of size %d\n",
+ dev->vol_info->name, dev->ubi_num, dev->vol_id,
+ dev->vol_info->size);
+
+ /* Initialize the gendisk of this ubiblk device */
+ gd = alloc_disk(1);
+ if (!gd) {
+ pr_err("alloc_disk failed\n");
+ ret = -ENODEV;
+ goto out_disk;
+ }
+
+ gd->fops = &ubiblk_ops;
+ gd->major = major;
+ gd->first_minor = dev->ubi_num * UBI_MAX_VOLUMES + dev->vol_id;
+ gd->private_data = dev;
+ sprintf(gd->disk_name, "ubiblk%d_%d", dev->ubi_num, dev->vol_id);
+ pr_debug("creating a gd '%s'\n", gd->disk_name);
+ set_capacity(gd,
+ (dev->vol_info->size *
+ dev->vol_info->usable_leb_size) >> 9);
+ set_disk_ro(gd, 1);
+ dev->gd = gd;
+
+ spin_lock_init(&dev->queue_lock);
+ dev->rq = blk_init_queue(ubi_ubiblk_request, &dev->queue_lock);
+ if (!dev->rq) {
+ pr_err("init_queue failed\n");
+ ret = -ENODEV;
+ goto out_queue;
+ }
+ dev->rq->queuedata = dev;
+ blk_queue_logical_block_size(dev->rq, BLK_SIZE);
+ dev->gd->queue = dev->rq;
+
+ /* Stolen from mtd_blkdevs.c */
+ /* Create processing thread
+ *
+ * The processing of the request has to be done in process context (it
+ * might sleep) but blk_run_queue can't block ; so we need to separate
+ * the event of a request being added to the queue (which triggers the
+ * callback ubi_ubiblk_request - that is set with blk_init_queue())
+ * and the processing of that request.
+ *
+ * Thus, the sole purpose of ubi_ubiblk_reuqest is to wake the kthread
+ * up so that it will process the request queue
+ */
+ dev->thread = kthread_run(ubi_ubiblk_thread, dev, "%s%d_%d",
+ "kubiblk", dev->ubi_num, dev->vol_id);
+ if (IS_ERR(dev->thread)) {
+ ret = PTR_ERR(dev->thread);
+ goto out_thread;
+ }
+
+ add_disk(dev->gd);
+ kfree(dev->vol_info);
+ dev->vol_info = NULL;
+ ubi_close_volume(dev->vol_desc);
+ dev->vol_desc = NULL;
+ mutex_unlock(&dev->lock);
+
+ return 0;
+
+out_thread:
+ blk_cleanup_queue(dev->rq);
+out_queue:
+ put_disk(dev->gd);
+out_disk:
+ kfree(dev->vol_info);
+ dev->vol_info = NULL;
+out_info:
+ ubi_close_volume(dev->vol_desc);
+ dev->vol_desc = NULL;
+out_vol:
+ mutex_unlock(&dev->lock);
+
+ return ret;
+}
+
+/*
+ * A UBI has been removed ; destroy the corresponding ubiblk device
+ */
+static int ubiblk_remove(struct ubi_volume_info *vol_info)
+{
+ struct ubiblk_dev *dev;
+
+ dev = ubiblk_find_dev(vol_info);
+
+ if (!dev) {
+ pr_warn("Trying to remove %s, which is unknown from ubiblk\n",
+ vol_info->name);
+ return -ENODEV;
+ }
+
+ pr_info("Removing %s\n", vol_info->name);
+
+ if (dev->vol_desc)
+ ubi_close_volume(dev->vol_desc);
+
+ del_gendisk(dev->gd);
+ blk_cleanup_queue(dev->rq);
+ kthread_stop(dev->thread);
+ put_disk(dev->gd);
+
+ kfree(dev->vol_info);
+
+ mutex_lock(&devtable_lock);
+ kfree(dev);
+ dev = NULL;
+ mutex_unlock(&devtable_lock);
+
+ return 0;
+}
+
+static int ubiblk_resized(struct ubi_volume_info *vol_info)
+{
+ struct ubiblk_dev *dev;
+
+ dev = ubiblk_find_dev(vol_info);
+ if (!dev) {
+ pr_warn("Trying to resize %s, which is unknown from ubiblk\n",
+ vol_info->name);
+ return -ENODEV;
+ }
+
+ mutex_lock(&dev->lock);
+ set_capacity(dev->gd,
+ (vol_info->size * vol_info->usable_leb_size) >> 9);
+ mutex_unlock(&dev->lock);
+ pr_debug("Resized ubiblk%d_%d to %d LEBs\n", vol_info->ubi_num,
+ vol_info->vol_id, vol_info->size);
+ return 0;
+}
+
+/*
+ * Dispatches the UBI notifications
+ * copied from gluebi.c
+ */
+static int ubiblk_notify(struct notifier_block *nb,
+ unsigned long notification_type, void *ns_ptr)
+{
+ struct ubi_notification *nt = ns_ptr;
+
+ switch (notification_type) {
+ case UBI_VOLUME_ADDED:
+ ubiblk_create(&nt->di, &nt->vi);
+ break;
+ case UBI_VOLUME_REMOVED:
+ ubiblk_remove(&nt->vi);
+ break;
+ case UBI_VOLUME_RESIZED:
+ ubiblk_resized(&nt->vi);
+ break;
+ case UBI_VOLUME_UPDATED:
+ break;
+ case UBI_VOLUME_RENAMED:
+ break;
+ default:
+ break;
+ }
+ return NOTIFY_OK;
+}
+
+static const struct block_device_operations ubiblk_ops = {
+ .owner = THIS_MODULE,
+ .open = ubiblk_open,
+ .release = ubiblk_release,
+};
+
+static struct notifier_block ubiblk_notifier = {
+ .notifier_call = ubiblk_notify,
+};
+
+/*
+ * Initialize the module
+ * (Get a major number and register to UBI notifications)
+ */
+static int __init ubi_ubiblk_init(void)
+{
+ int ret = 0;
+
+ pr_info("UBIBLK starting\n");
+
+ ret = register_blkdev(0, "ubiblk");
+ if (ret <= 0) {
+ pr_err("UBIBLK: could not register_blkdev\n");
+ return -ENODEV;
+ }
+ major = ret;
+ pr_info("UBIBLK: device's major: %d\n", major);
+
+ mutex_init(&devtable_lock);
+ ret = ubi_register_volume_notifier(&ubiblk_notifier, 0);
+ if (ret < 0)
+ unregister_blkdev(major, "ubiblk");
+
+ return ret;
+}
+
+/*
+ * End of life
+ * unregister the block device major, unregister from UBI notifications,
+ * stop the threads and free the memory.
+ */
+static void __exit ubi_ubiblk_exit(void)
+{
+ int i;
+
+ pr_info("UBIBLK: going to exit\n");
+
+ ubi_unregister_volume_notifier(&ubiblk_notifier);
+
+ for (i = 0; i < UBIBLK_MAX_DEVS; i++) {
+ struct ubiblk_dev *dev = ubiblk_devs[i];
+ if (!dev)
+ continue;
+
+ if (dev->vol_desc)
+ ubi_close_volume(dev->vol_desc);
+
+ del_gendisk(dev->gd);
+ blk_cleanup_queue(dev->rq);
+ kthread_stop(dev->thread);
+ put_disk(dev->gd);
+
+ kfree(dev->vol_info);
+ kfree(dev);
+ }
+
+ unregister_blkdev(major, "ubiblk");
+ pr_info("UBIBLK: The End\n");
+}
+
+module_init(ubi_ubiblk_init);
+module_exit(ubi_ubiblk_exit);
+MODULE_DESCRIPTION("Read-only block transition layer on top of UBI");
+MODULE_AUTHOR("David Wagner");
+MODULE_LICENSE("GPL");
--
1.7.0.4

2011-06-28 15:35:50

by David Wagner

[permalink] [raw]
Subject: Re: [PATCH] UBI: new module ubiblk: block layer on top of UBI

On 06/28/2011 04:50 PM, Matthieu CASTET wrote:
> Hi,
>
[...]
> Also what are the advantage against gluebi + mtdblock_ro ?

The main advantage is a reduced number of layers ; I must say I cannot
see much more for now. I could add that the Kconfig help of gluebi
advises not to use it except when needed by legacy software.

--
David Wagner, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

2011-06-29 06:25:04

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [PATCH] UBI: new module ubiblk: block layer on top of UBI

On Tue, 2011-06-28 at 17:32 +0200, David Wagner wrote:
> On 06/28/2011 04:50 PM, Matthieu CASTET wrote:
> > Hi,
> >
> [...]
> > Also what are the advantage against gluebi + mtdblock_ro ?
>
> The main advantage is a reduced number of layers ; I must say I cannot
> see much more for now. I could add that the Kconfig help of gluebi
> advises not to use it except when needed by legacy software.

Well, I think Matthieu has a valid poit, you should try to come up with
a set of advantages, otherwise why would this drivers be needed? Why
would people spend time reviewing it? May be less memcpy's? Do we do an
extra memcpy in gluebi? If yes, can we avoid doing this. Anyway, please,
try to sell this driver a bit better.

--
Best Regards,
Artem Bityutskiy

2011-06-29 06:51:16

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [PATCH] UBI: new module ubiblk: block layer on top of UBI

On Tue, 2011-06-28 at 13:35 +0200, David Wagner wrote:
> Hi,
>
> On 06/27/2011 09:26 PM, Artem Bityutskiy wrote:
> > On Fri, 2011-06-24 at 15:34 +0200, [email protected]
> > wrote:
> >> + /* Stolen from mtd_blkdevs.c */
> >> + /* Create processing thread */
> >> + dev->thread = kthread_run(ubi_ubiblk_thread, dev, "%s%d_%d",
> >> + "kubiblkd", dev->ubi_num, dev->vol_id);
> >> + if (IS_ERR(dev->thread)) {
> >> + ret = PTR_ERR(dev->thread);
> >> + goto out_thread;
> >> + }
> >
> > Why we need a kernel thread? Could you please describe when exactly it
> > is needed and why we cannot avoid having it?
>
> Do you mean that there could be another/better way ?

No, I just do not understand why it is there. I think this is juts block
layer's design, but I wanted you to explain this - the design, how block
requests are handled, and where exactly the thread is needed. I expected
you just have the explanation.

--
Best Regards,
Artem Bityutskiy

2011-06-29 06:53:49

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [RFC PATCHv2] UBI: new module ubiblk: block layer on top of UBI

On Tue, 2011-06-28 at 17:24 +0200, [email protected]
wrote:
> + * Author: David Wagner
> + * Some code taken from gluebi.c (Artem Bityutskiy (Битюцкий Артём),
> + * Joern Engel)
> + * Some code taken from mtd_blkdevs.c (David Woodhouse)

It is important to preserve Copyrights, because it has to do with legal
stuff.

--
Best Regards,
Artem Bityutskiy