[PATCH 1/2] UBI: Add initial support for scatter gather

[PATCH 1/2] UBI: Add initial support for scatter gather

[Richard Weinberger]

Adds a new set of functions to deal with scatter gather.
ubi_eba_read_leb_sg() will read from a LEB into a scatter gather list.
The new data structure struct ubi_sgl will be used within UBI to
hold the scatter gather list itself and metadata to have a cursor
within the list.

Signed-off-by: Richard Weinberger <richard@nod.at>
Tested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/ubi/eba.c   | 56 +++++++++++++++++++++++++++++
 drivers/mtd/ubi/kapi.c  | 95 +++++++++++++++++++++++++++++++++++++++++--------
 drivers/mtd/ubi/ubi.h   |  3 ++
 include/linux/mtd/ubi.h | 33 +++++++++++++++++
 4 files changed, 172 insertions(+), 15 deletions(-)

diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
index b698534..0aaf707 100644
--- a/drivers/mtd/ubi/eba.c
+++ b/drivers/mtd/ubi/eba.c
@@ -481,6 +481,62 @@ out_unlock:
 }
 
 /**
+ * ubi_eba_read_leb_sg - read data into a scatter gather list.
+ * @ubi: UBI device description object
+ * @vol: volume description object
+ * @lnum: logical eraseblock number
+ * @sgl: UBI scatter gather list to store the read data
+ * @offset: offset from where to read
+ * @len: how many bytes to read
+ * @check: data CRC check flag
+ *
+ * This function works exactly like ubi_eba_read_leb(). But instead of
+ * storing the read data into a buffer it writes to an UBI scatter gather
+ * list.
+ */
+int ubi_eba_read_leb_sg(struct ubi_device *ubi, struct ubi_volume *vol,
+			struct ubi_sgl *sgl, int lnum, int offset, int len,
+			int check)
+{
+	int to_read;
+	int ret;
+	struct scatterlist *sg;
+
+	for (;;) {
+		ubi_assert(sgl->list_pos < UBI_MAX_SG_COUNT);
+		sg = &sgl->sg[sgl->list_pos];
+		if (len < sg->length - sgl->page_pos)
+			to_read = len;
+		else
+			to_read = sg->length - sgl->page_pos;
+
+		ret = ubi_eba_read_leb(ubi, vol, lnum,
+				       sg_virt(sg) + sgl->page_pos, offset,
+				       to_read, check);
+		if (ret < 0)
+			goto out;
+
+		offset += to_read;
+		len -= to_read;
+		if (!len) {
+			sgl->page_pos += to_read;
+			if (sgl->page_pos == sg->length) {
+				sgl->list_pos++;
+				sgl->page_pos = 0;
+			}
+
+			break;
+		}
+
+		sgl->list_pos++;
+		sgl->page_pos = 0;
+	}
+
+out:
+	return ret;
+}
+
+/**
  * recover_peb - recover from write failure.
  * @ubi: UBI device description object
  * @pnum: the physical eraseblock to recover
diff --git a/drivers/mtd/ubi/kapi.c b/drivers/mtd/ubi/kapi.c
index f3bab66..d0055a2 100644
--- a/drivers/mtd/ubi/kapi.c
+++ b/drivers/mtd/ubi/kapi.c
@@ -355,6 +355,43 @@ void ubi_close_volume(struct ubi_volume_desc *desc)
 EXPORT_SYMBOL_GPL(ubi_close_volume);
 
 /**
+ * leb_read_sanity_check - does sanity checks on read requests.
+ * @desc: volume descriptor
+ * @lnum: logical eraseblock number to read from
+ * @offset: offset within the logical eraseblock to read from
+ * @len: how many bytes to read
+ *
+ * This function is used by ubi_leb_read() and ubi_leb_read_sg()
+ * to perfrom sanity checks.
+ */
+static int leb_read_sanity_check(struct ubi_volume_desc *desc, int lnum,
+				 int offset, int len)
+{
+	struct ubi_volume *vol = desc->vol;
+	struct ubi_device *ubi = vol->ubi;
+	int vol_id = vol->vol_id;
+
+	if (vol_id < 0 || vol_id >= ubi->vtbl_slots || lnum < 0 ||
+	    lnum >= vol->used_ebs || offset < 0 || len < 0 ||
+	    offset + len > vol->usable_leb_size)
+		return -EINVAL;
+
+	if (vol->vol_type == UBI_STATIC_VOLUME) {
+		if (vol->used_ebs == 0)
+			/* Empty static UBI volume */
+			return 0;
+		if (lnum == vol->used_ebs - 1 &&
+		    offset + len > vol->last_eb_bytes)
+			return -EINVAL;
+	}
+
+	if (vol->upd_marker)
+		return -EBADF;
+
+	return 0;
+}
+
+/**
  * ubi_leb_read - read data.
  * @desc: volume descriptor
  * @lnum: logical eraseblock number to read from
@@ -390,22 +427,10 @@ int ubi_leb_read(struct ubi_volume_desc *desc, int lnum, char *buf, int offset,
 
 	dbg_gen("read %d bytes from LEB %d:%d:%d", len, vol_id, lnum, offset);
 
-	if (vol_id < 0 || vol_id >= ubi->vtbl_slots || lnum < 0 ||
-	    lnum >= vol->used_ebs || offset < 0 || len < 0 ||
-	    offset + len > vol->usable_leb_size)
-		return -EINVAL;
-
-	if (vol->vol_type == UBI_STATIC_VOLUME) {
-		if (vol->used_ebs == 0)
-			/* Empty static UBI volume */
-			return 0;
-		if (lnum == vol->used_ebs - 1 &&
-		    offset + len > vol->last_eb_bytes)
-			return -EINVAL;
-	}
+	err = leb_read_sanity_check(desc, lnum, offset, len);
+	if (err < 0)
+		return err;
 
-	if (vol->upd_marker)
-		return -EBADF;
 	if (len == 0)
 		return 0;
 
@@ -419,6 +444,46 @@ int ubi_leb_read(struct ubi_volume_desc *desc, int lnum, char *buf, int offset,
 }
 EXPORT_SYMBOL_GPL(ubi_leb_read);
 
+
+/**
+ * ubi_leb_read_sg - read data into a scatter gather list.
+ * @desc: volume descriptor
+ * @lnum: logical eraseblock number to read from
+ * @buf: buffer where to store the read data
+ * @offset: offset within the logical eraseblock to read from
+ * @len: how many bytes to read
+ * @check: whether UBI has to check the read data's CRC or not.
+ *
+ * This function works exactly like ubi_leb_read_sg(). But instead of
+ * storing the read data into a buffer it writes to an UBI scatter gather
+ * list.
+ */
+int ubi_leb_read_sg(struct ubi_volume_desc *desc, int lnum, struct ubi_sgl *sgl,
+		    int offset, int len, int check)
+{
+	struct ubi_volume *vol = desc->vol;
+	struct ubi_device *ubi = vol->ubi;
+	int err, vol_id = vol->vol_id;
+
+	dbg_gen("read %d bytes from LEB %d:%d:%d", len, vol_id, lnum, offset);
+
+	err = leb_read_sanity_check(desc, lnum, offset, len);
+	if (err < 0)
+		return err;
+
+	if (len == 0)
+		return 0;
+
+	err = ubi_eba_read_leb_sg(ubi, vol, sgl, lnum, offset, len, check);
+	if (err && mtd_is_eccerr(err) && vol->vol_type == UBI_STATIC_VOLUME) {
+		ubi_warn(ubi, "mark volume %d as corrupted", vol_id);
+		vol->corrupted = 1;
+	}
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(ubi_leb_read_sg);
+
 /**
  * ubi_leb_write - write data.
  * @desc: volume descriptor
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index ee7ac0b..a5283cf 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -791,6 +791,9 @@ int ubi_eba_unmap_leb(struct ubi_device *ubi, struct ubi_volume *vol,
 		      int lnum);
 int ubi_eba_read_leb(struct ubi_device *ubi, struct ubi_volume *vol, int lnum,
 		     void *buf, int offset, int len, int check);
+int ubi_eba_read_leb_sg(struct ubi_device *ubi, struct ubi_volume *vol,
+			struct ubi_sgl *sgl, int lnum, int offset, int len,
+			int check);
 int ubi_eba_write_leb(struct ubi_device *ubi, struct ubi_volume *vol, int lnum,
 		      const void *buf, int offset, int len);
 int ubi_eba_write_leb_st(struct ubi_device *ubi, struct ubi_volume *vol,
diff --git a/include/linux/mtd/ubi.h b/include/linux/mtd/ubi.h
index c3918a0..406c7cd 100644
--- a/include/linux/mtd/ubi.h
+++ b/include/linux/mtd/ubi.h
@@ -23,11 +23,16 @@
 
 #include <linux/ioctl.h>
 #include <linux/types.h>
+#include <linux/scatterlist.h>
 #include <mtd/ubi-user.h>
 
 /* All voumes/LEBs */
 #define UBI_ALL -1
 
+/* Maximum number of scatter gather list entries,
+ * we use only 64 to have a lower memory foot print. */
+#define UBI_MAX_SG_COUNT 64
+
 /*
  * enum ubi_open_mode - UBI volume open mode constants.
  *
@@ -116,6 +121,22 @@ struct ubi_volume_info {
 };
 
 /**
+ * struct ubi_sgl - UBI scatter gather list data structure.
+ * @list_pos: current position in @sg[]
+ * @page_pos: current position in @sg[@list_pos]
+ * @sg: the scatter gather list itself
+ *
+ * ubi_sgl is a wrapper around a scatter list which keeps track of the
+ * current position in the list and the current list item such that
+ * it can be used across multiple ubi_leb_read_sg() calls.
+ */
+struct ubi_sgl {
+	int list_pos;
+	int page_pos;
+	struct scatterlist sg[UBI_MAX_SG_COUNT];
+};
+
+/**
  * struct ubi_device_info - UBI device description data structure.
  * @ubi_num: ubi device number
  * @leb_size: logical eraseblock size on this UBI device
@@ -210,6 +231,8 @@ int ubi_unregister_volume_notifier(struct notifier_block *nb);
 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_read_sg(struct ubi_volume_desc *desc, int lnum, struct ubi_sgl *sgl,
+		   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 ubi_leb_change(struct ubi_volume_desc *desc, int lnum, const void *buf,
@@ -230,4 +253,14 @@ 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_read_sg()' function, but it does
+ * not provide the checking capability.
+ */
+static inline int ubi_read_sg(struct ubi_volume_desc *desc, int lnum,
+			      struct ubi_sgl *sgl, int offset, int len)
+{
+	return ubi_leb_read_sg(desc, lnum, sgl, offset, len, 0);
+}
 #endif /* !__LINUX_UBI_H__ */
-- 
1.8.4.5

[PATCH 2/2] UBI: Block: Add blk-mq support

[Richard Weinberger]

Convert the driver to blk-mq.
Beside of moving to the modern block interface this change boosts
also the performance of the driver.

nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xda
nand: Micron NAND 256MiB 3,3V 8-bit
nand: 256MiB, SLC, page size: 2048, OOB size: 64

root@debian-armhf:~# dd if=/dev/ubiblock0_0 of=/dev/zero bs=1M
243+1 records in
243+1 records out
255080448 bytes (255 MB) copied, 4.39295 s, 58.1 MB/s

vs.

root@debian-armhf:~# dd if=/dev/ubiblock0_0 of=/dev/zero bs=1M
243+1 records in
243+1 records out
255080448 bytes (255 MB) copied, 2.87676 s, 88.7 MB/s

Cc: hch@infradead.org
Cc: axboe@fb.com
Cc: tom.leiming@gmail.com
Signed-off-by: Richard Weinberger <richard@nod.at>
Tested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/ubi/block.c | 213 +++++++++++++++++++++++-------------------------
 1 file changed, 104 insertions(+), 109 deletions(-)

diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index 6b6bce2..bc088ab 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -42,11 +42,12 @@
 #include <linux/list.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
-#include <linux/vmalloc.h>
 #include <linux/mtd/ubi.h>
 #include <linux/workqueue.h>
 #include <linux/blkdev.h>
+#include <linux/blk-mq.h>
 #include <linux/hdreg.h>
+#include <linux/scatterlist.h>
 #include <asm/div64.h>
 
 #include "ubi-media.h"
@@ -67,6 +68,13 @@ struct ubiblock_param {
 	char name[UBIBLOCK_PARAM_LEN+1];
 };
 
+struct ubiblock_pdu {
+	struct request *req;
+	struct ubiblock *dev;
+	struct work_struct work;
+	struct ubi_sgl usgl;
+};
+
 /* Numbers of elements set in the @ubiblock_param array */
 static int ubiblock_devs __initdata;
 
@@ -84,11 +92,10 @@ struct ubiblock {
 	struct request_queue *rq;
 
 	struct workqueue_struct *wq;
-	struct work_struct work;
 
 	struct mutex dev_mutex;
-	spinlock_t queue_lock;
 	struct list_head list;
+	struct blk_mq_tag_set tag_set;
 };
 
 /* Linked list of all ubiblock instances */
@@ -181,31 +188,17 @@ static struct ubiblock *find_dev_nolock(int ubi_num, int vol_id)
 	return NULL;
 }
 
-static int ubiblock_read_to_buf(struct ubiblock *dev, char *buffer,
-				int leb, int offset, int len)
-{
-	int ret;
-
-	ret = ubi_read(dev->desc, leb, buffer, offset, len);
-	if (ret) {
-		dev_err(disk_to_dev(dev->gd), "%d while reading from LEB %d (offset %d, length %d)",
-			ret, leb, offset, len);
-		return ret;
-	}
-	return 0;
-}
-
-static int ubiblock_read(struct ubiblock *dev, char *buffer,
-			 sector_t sec, int len)
+static int ubiblock_read(struct ubiblock_pdu *pdu)
 {
-	int ret, leb, offset;
-	int bytes_left = len;
-	int to_read = len;
-	u64 pos = sec << 9;
+	int ret, leb, offset, bytes_left;
+	int to_read = blk_rq_bytes(pdu->req);
+	struct ubiblock *dev = pdu->dev;
+	u64 pos = blk_rq_pos(pdu->req) << 9;
 
 	/* Get LEB:offset address to read from */
 	offset = do_div(pos, dev->leb_size);
 	leb = pos;
+	bytes_left = to_read;
 
 	while (bytes_left) {
 		/*
@@ -215,11 +208,10 @@ static int ubiblock_read(struct ubiblock *dev, char *buffer,
 		if (offset + to_read > dev->leb_size)
 			to_read = dev->leb_size - offset;
 
-		ret = ubiblock_read_to_buf(dev, buffer, leb, offset, to_read);
-		if (ret)
+		ret = ubi_read_sg(dev->desc, leb, &pdu->usgl, offset, to_read);
+		if (ret < 0)
 			return ret;
 
-		buffer += to_read;
 		bytes_left -= to_read;
 		to_read = bytes_left;
 		leb += 1;
@@ -228,79 +220,6 @@ static int ubiblock_read(struct ubiblock *dev, char *buffer,
 	return 0;
 }
 
-static int do_ubiblock_request(struct ubiblock *dev, struct request *req)
-{
-	int len, ret;
-	sector_t sec;
-
-	if (req->cmd_type != REQ_TYPE_FS)
-		return -EIO;
-
-	if (blk_rq_pos(req) + blk_rq_cur_sectors(req) >
-	    get_capacity(req->rq_disk))
-		return -EIO;
-
-	if (rq_data_dir(req) != READ)
-		return -ENOSYS; /* Write not implemented */
-
-	sec = blk_rq_pos(req);
-	len = blk_rq_cur_bytes(req);
-
-	/*
-	 * Let's prevent the device from being removed while we're doing I/O
-	 * work. Notice that this means we serialize all the I/O operations,
-	 * but it's probably of no impact given the NAND core serializes
-	 * flash access anyway.
-	 */
-	mutex_lock(&dev->dev_mutex);
-	ret = ubiblock_read(dev, bio_data(req->bio), sec, len);
-	mutex_unlock(&dev->dev_mutex);
-
-	return ret;
-}
-
-static void ubiblock_do_work(struct work_struct *work)
-{
-	struct ubiblock *dev =
-		container_of(work, struct ubiblock, work);
-	struct request_queue *rq = dev->rq;
-	struct request *req;
-	int res;
-
-	spin_lock_irq(rq->queue_lock);
-
-	req = blk_fetch_request(rq);
-	while (req) {
-
-		spin_unlock_irq(rq->queue_lock);
-		res = do_ubiblock_request(dev, req);
-		spin_lock_irq(rq->queue_lock);
-
-		/*
-		 * If we're done with this request,
-		 * we need to fetch a new one
-		 */
-		if (!__blk_end_request_cur(req, res))
-			req = blk_fetch_request(rq);
-	}
-
-	spin_unlock_irq(rq->queue_lock);
-}
-
-static void ubiblock_request(struct request_queue *rq)
-{
-	struct ubiblock *dev;
-	struct request *req;
-
-	dev = rq->queuedata;
-
-	if (!dev)
-		while ((req = blk_fetch_request(rq)) != NULL)
-			__blk_end_request_all(req, -ENODEV);
-	else
-		queue_work(dev->wq, &dev->work);
-}
-
 static int ubiblock_open(struct block_device *bdev, fmode_t mode)
 {
 	struct ubiblock *dev = bdev->bd_disk->private_data;
@@ -374,6 +293,67 @@ static const struct block_device_operations ubiblock_ops = {
 	.getgeo	= ubiblock_getgeo,
 };
 
+static void ubiblock_do_work(struct work_struct *work)
+{
+	int ret;
+	struct ubiblock_pdu *pdu = container_of(work, struct ubiblock_pdu, work);
+
+	ret = ubiblock_read(pdu);
+	blk_mq_end_request(pdu->req, ret ?: 0);
+}
+
+static int ubiblock_queue_rq(struct blk_mq_hw_ctx *hctx,
+			     const struct blk_mq_queue_data *bd)
+{
+	int ret;
+	struct request *req = bd->rq;
+	struct ubiblock *dev = hctx->queue->queuedata;
+	struct ubiblock_pdu *pdu = blk_mq_rq_to_pdu(req);
+
+	if (req->cmd_type != REQ_TYPE_FS)
+		return BLK_MQ_RQ_QUEUE_ERROR;
+
+	if (blk_rq_pos(req) + blk_rq_cur_sectors(req) >
+	    get_capacity(req->rq_disk))
+		return BLK_MQ_RQ_QUEUE_ERROR;
+
+	if (rq_data_dir(req) != READ)
+		return BLK_MQ_RQ_QUEUE_ERROR; /* Write not implemented */
+
+	pdu->usgl.list_pos = 0;
+	pdu->usgl.page_pos = 0;
+
+	blk_mq_start_request(req);
+	ret = blk_rq_map_sg(hctx->queue, req, pdu->usgl.sg);
+
+	queue_work(dev->wq, &pdu->work);
+
+	return BLK_MQ_RQ_QUEUE_OK;
+}
+
+static int ubiblock_init_request(void *data, struct request *req,
+				 unsigned int hctx_idx,
+				 unsigned int request_idx,
+				 unsigned int numa_node)
+{
+	struct ubiblock *dev = data;
+	struct ubiblock_pdu *pdu = blk_mq_rq_to_pdu(req);
+
+	pdu->dev = dev;
+	pdu->req = req;
+
+	sg_init_table(pdu->usgl.sg, UBI_MAX_SG_COUNT);
+	INIT_WORK(&pdu->work, ubiblock_do_work);
+
+	return 0;
+}
+
+static struct blk_mq_ops ubiblock_mq_ops = {
+	.queue_rq       = ubiblock_queue_rq,
+	.init_request	= ubiblock_init_request,
+	.map_queue      = blk_mq_map_queue,
+};
+
 int ubiblock_create(struct ubi_volume_info *vi)
 {
 	struct ubiblock *dev;
@@ -417,13 +397,27 @@ int ubiblock_create(struct ubi_volume_info *vi)
 	set_capacity(gd, disk_capacity);
 	dev->gd = gd;
 
-	spin_lock_init(&dev->queue_lock);
-	dev->rq = blk_init_queue(ubiblock_request, &dev->queue_lock);
+	dev->tag_set.ops = &ubiblock_mq_ops;
+	dev->tag_set.queue_depth = 64;
+	dev->tag_set.numa_node = NUMA_NO_NODE;
+	dev->tag_set.flags = BLK_MQ_F_SHOULD_MERGE;
+	dev->tag_set.cmd_size = sizeof(struct ubiblock_pdu);
+	dev->tag_set.driver_data = dev;
+	dev->tag_set.nr_hw_queues = 1;
+
+	ret = blk_mq_alloc_tag_set(&dev->tag_set);
+	if (ret) {
+		dev_err(disk_to_dev(dev->gd), "blk_mq_alloc_tag_set failed");
+		goto out_put_disk;
+	}
+
+	dev->rq = blk_mq_init_queue(&dev->tag_set);
 	if (!dev->rq) {
-		dev_err(disk_to_dev(gd), "blk_init_queue failed");
+		dev_err(disk_to_dev(gd), "blk_mq_init_queue failed");
 		ret = -ENODEV;
-		goto out_put_disk;
+		goto out_free_tags;
 	}
+	blk_queue_max_segments(dev->rq, UBI_MAX_SG_COUNT);
 
 	dev->rq->queuedata = dev;
 	dev->gd->queue = dev->rq;
@@ -437,7 +431,6 @@ int ubiblock_create(struct ubi_volume_info *vi)
 		ret = -ENOMEM;
 		goto out_free_queue;
 	}
-	INIT_WORK(&dev->work, ubiblock_do_work);
 
 	mutex_lock(&devices_mutex);
 	list_add_tail(&dev->list, &ubiblock_devices);
@@ -451,6 +444,8 @@ int ubiblock_create(struct ubi_volume_info *vi)
 
 out_free_queue:
 	blk_cleanup_queue(dev->rq);
+out_free_tags:
+	blk_mq_free_tag_set(&dev->tag_set);
 out_put_disk:
 	put_disk(dev->gd);
 out_free_dev:
@@ -461,8 +456,13 @@ out_free_dev:
 
 static void ubiblock_cleanup(struct ubiblock *dev)
 {
+	/* Stop new requests to arrive */
 	del_gendisk(dev->gd);
+	/* Flush pending work */
+	destroy_workqueue(dev->wq);
+	/* Finally destroy the blk queue */
 	blk_cleanup_queue(dev->rq);
+	blk_mq_free_tag_set(&dev->tag_set);
 	dev_info(disk_to_dev(dev->gd), "released");
 	put_disk(dev->gd);
 }
@@ -490,9 +490,6 @@ int ubiblock_remove(struct ubi_volume_info *vi)
 	list_del(&dev->list);
 	mutex_unlock(&devices_mutex);
 
-	/* Flush pending work and stop this workqueue */
-	destroy_workqueue(dev->wq);
-
 	ubiblock_cleanup(dev);
 	mutex_unlock(&dev->dev_mutex);
 	kfree(dev);
@@ -620,8 +617,6 @@ static void ubiblock_remove_all(void)
 	struct ubiblock *dev;
 
 	list_for_each_entry_safe(dev, next, &ubiblock_devices, list) {
-		/* Flush pending work and stop workqueue */
-		destroy_workqueue(dev->wq);
 		/* The module is being forcefully removed */
 		WARN_ON(dev->desc);
 		/* Remove from device list */
-- 
1.8.4.5

Re: [PATCH 2/2] UBI: Block: Add blk-mq support

[Christoph Hellwig]

> +struct ubiblock_pdu {
> +	struct request *req;

No need to store the request, you can trivially get at it using
blk_mq_rq_from_pdu().

> +	struct ubiblock *dev;

Why do you need the dev pointer?  You can always trivially get
it using req->queuedata.

> +static void ubiblock_do_work(struct work_struct *work)
> +{
> +	int ret;
> +	struct ubiblock_pdu *pdu = container_of(work, struct ubiblock_pdu, work);
> +
> +	ret = ubiblock_read(pdu);
> +	blk_mq_end_request(pdu->req, ret ?: 0);

Why not just pass ret as-is?

> +	if (blk_rq_pos(req) + blk_rq_cur_sectors(req) >
> +	    get_capacity(req->rq_disk))
> +		return BLK_MQ_RQ_QUEUE_ERROR;

The upper layers take are of this check.

> +	pdu->usgl.list_pos = 0;
> +	pdu->usgl.page_pos = 0;

Having a helper to initialize a ubi_sgl would be nicer than having
to open code it ike here.

> +
> +	blk_mq_start_request(req);
> +	ret = blk_rq_map_sg(hctx->queue, req, pdu->usgl.sg);
> +
> +	queue_work(dev->wq, &pdu->work);

Why don't you move these calls into the work queue as well?  The
queue_rq call would literally just become a queue_work call.

And given that this is a fairly common patter this should allow
us to refactor / optimize this case a bit later on.

Re: [PATCH 2/2] UBI: Block: Add blk-mq support

[Richard Weinberger]

Am 10.01.2015 um 19:58 schrieb Christoph Hellwig:
>> +struct ubiblock_pdu {
>> +	struct request *req;
> 
> No need to store the request, you can trivially get at it using
> blk_mq_rq_from_pdu().

Very handy, I was not aware of blk_mq_rq_from_pdu().

>> +	struct ubiblock *dev;
> 
> Why do you need the dev pointer?  You can always trivially get
> it using req->queuedata.

Same here.

>> +static void ubiblock_do_work(struct work_struct *work)
>> +{
>> +	int ret;
>> +	struct ubiblock_pdu *pdu = container_of(work, struct ubiblock_pdu, work);
>> +
>> +	ret = ubiblock_read(pdu);
>> +	blk_mq_end_request(pdu->req, ret ?: 0);
> 
> Why not just pass ret as-is?

Obviously a brain fart. :-\

>> +	if (blk_rq_pos(req) + blk_rq_cur_sectors(req) >
>> +	    get_capacity(req->rq_disk))
>> +		return BLK_MQ_RQ_QUEUE_ERROR;
> 
> The upper layers take are of this check.

Ok.

>> +	pdu->usgl.list_pos = 0;
>> +	pdu->usgl.page_pos = 0;
> 
> Having a helper to initialize a ubi_sgl would be nicer than having
> to open code it ike here.

Ok.

>> +
>> +	blk_mq_start_request(req);
>> +	ret = blk_rq_map_sg(hctx->queue, req, pdu->usgl.sg);
>> +
>> +	queue_work(dev->wq, &pdu->work);
> 
> Why don't you move these calls into the work queue as well?  The
> queue_rq call would literally just become a queue_work call.

I did not know that I'm allowed to get hctx->queue also via req->q.

> And given that this is a fairly common patter this should allow
> us to refactor / optimize this case a bit later on.

Will send a v2 in a jiffy!

Thanks,
//richard

UBI: Add support for scatter gather and blk-mq

[Richard Weinberger]

This two patches implement blk-mq support for the UBI block driver.
As the scatter gather part is rather generic I've moved it directl
into UBI such that it can be reused later.
So far only reading data into a scatter gather list is possible.
After implementing  ubi_eba_read_leb_sg() we could slowly start
removing a bunch of vmalloc() from the UBI core code and use scatter gather.
This should lower the memory footprint of UBI.

[PATCH 1/2] UBI: Add initial support for scatter gather
[PATCH 2/2] UBI: Block: Add blk-mq support

Thanks,
//richard

[PATCH 2/2] UBI: Block: Add blk-mq support

[Richard Weinberger]

Convert the driver to blk-mq.
Beside of moving to the modern block interface this change boosts
also the performance of the driver.

nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xda
nand: Micron NAND 256MiB 3,3V 8-bit
nand: 256MiB, SLC, page size: 2048, OOB size: 64

root@debian-armhf:~# dd if=/dev/ubiblock0_0 of=/dev/zero bs=1M
243+1 records in
243+1 records out
255080448 bytes (255 MB) copied, 4.39295 s, 58.1 MB/s

vs.

root@debian-armhf:~# dd if=/dev/ubiblock0_0 of=/dev/zero bs=1M
243+1 records in
243+1 records out
255080448 bytes (255 MB) copied, 2.87676 s, 88.7 MB/s

Cc: hch@infradead.org
Cc: axboe@fb.com
Cc: tom.leiming@gmail.com
Signed-off-by: Richard Weinberger <richard@nod.at>
---
 drivers/mtd/ubi/block.c | 212 +++++++++++++++++++++++-------------------------
 1 file changed, 103 insertions(+), 109 deletions(-)

diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index 6b6bce2..a0587f9 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -42,11 +42,12 @@
 #include <linux/list.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
-#include <linux/vmalloc.h>
 #include <linux/mtd/ubi.h>
 #include <linux/workqueue.h>
 #include <linux/blkdev.h>
+#include <linux/blk-mq.h>
 #include <linux/hdreg.h>
+#include <linux/scatterlist.h>
 #include <asm/div64.h>
 
 #include "ubi-media.h"
@@ -67,6 +68,13 @@ struct ubiblock_param {
 	char name[UBIBLOCK_PARAM_LEN+1];
 };
 
+struct ubiblock_pdu {
+	struct request *req;
+	struct ubiblock *dev;
+	struct work_struct work;
+	struct ubi_sgl usgl;
+};
+
 /* Numbers of elements set in the @ubiblock_param array */
 static int ubiblock_devs __initdata;
 
@@ -84,11 +92,10 @@ struct ubiblock {
 	struct request_queue *rq;
 
 	struct workqueue_struct *wq;
-	struct work_struct work;
 
 	struct mutex dev_mutex;
-	spinlock_t queue_lock;
 	struct list_head list;
+	struct blk_mq_tag_set tag_set;
 };
 
 /* Linked list of all ubiblock instances */
@@ -181,31 +188,17 @@ static struct ubiblock *find_dev_nolock(int ubi_num, int vol_id)
 	return NULL;
 }
 
-static int ubiblock_read_to_buf(struct ubiblock *dev, char *buffer,
-				int leb, int offset, int len)
-{
-	int ret;
-
-	ret = ubi_read(dev->desc, leb, buffer, offset, len);
-	if (ret) {
-		dev_err(disk_to_dev(dev->gd), "%d while reading from LEB %d (offset %d, length %d)",
-			ret, leb, offset, len);
-		return ret;
-	}
-	return 0;
-}
-
-static int ubiblock_read(struct ubiblock *dev, char *buffer,
-			 sector_t sec, int len)
+static int ubiblock_read(struct ubiblock_pdu *pdu)
 {
-	int ret, leb, offset;
-	int bytes_left = len;
-	int to_read = len;
-	u64 pos = sec << 9;
+	int ret, leb, offset, bytes_left;
+	int to_read = blk_rq_bytes(pdu->req);
+	struct ubiblock *dev = pdu->dev;
+	u64 pos = blk_rq_pos(pdu->req) << 9;
 
 	/* Get LEB:offset address to read from */
 	offset = do_div(pos, dev->leb_size);
 	leb = pos;
+	bytes_left = to_read;
 
 	while (bytes_left) {
 		/*
@@ -215,11 +208,10 @@ static int ubiblock_read(struct ubiblock *dev, char *buffer,
 		if (offset + to_read > dev->leb_size)
 			to_read = dev->leb_size - offset;
 
-		ret = ubiblock_read_to_buf(dev, buffer, leb, offset, to_read);
-		if (ret)
+		ret = ubi_read_sg(dev->desc, leb, &pdu->usgl, offset, to_read);
+		if (ret < 0)
 			return ret;
 
-		buffer += to_read;
 		bytes_left -= to_read;
 		to_read = bytes_left;
 		leb += 1;
@@ -228,79 +220,6 @@ static int ubiblock_read(struct ubiblock *dev, char *buffer,
 	return 0;
 }
 
-static int do_ubiblock_request(struct ubiblock *dev, struct request *req)
-{
-	int len, ret;
-	sector_t sec;
-
-	if (req->cmd_type != REQ_TYPE_FS)
-		return -EIO;
-
-	if (blk_rq_pos(req) + blk_rq_cur_sectors(req) >
-	    get_capacity(req->rq_disk))
-		return -EIO;
-
-	if (rq_data_dir(req) != READ)
-		return -ENOSYS; /* Write not implemented */
-
-	sec = blk_rq_pos(req);
-	len = blk_rq_cur_bytes(req);
-
-	/*
-	 * Let's prevent the device from being removed while we're doing I/O
-	 * work. Notice that this means we serialize all the I/O operations,
-	 * but it's probably of no impact given the NAND core serializes
-	 * flash access anyway.
-	 */
-	mutex_lock(&dev->dev_mutex);
-	ret = ubiblock_read(dev, bio_data(req->bio), sec, len);
-	mutex_unlock(&dev->dev_mutex);
-
-	return ret;
-}
-
-static void ubiblock_do_work(struct work_struct *work)
-{
-	struct ubiblock *dev =
-		container_of(work, struct ubiblock, work);
-	struct request_queue *rq = dev->rq;
-	struct request *req;
-	int res;
-
-	spin_lock_irq(rq->queue_lock);
-
-	req = blk_fetch_request(rq);
-	while (req) {
-
-		spin_unlock_irq(rq->queue_lock);
-		res = do_ubiblock_request(dev, req);
-		spin_lock_irq(rq->queue_lock);
-
-		/*
-		 * If we're done with this request,
-		 * we need to fetch a new one
-		 */
-		if (!__blk_end_request_cur(req, res))
-			req = blk_fetch_request(rq);
-	}
-
-	spin_unlock_irq(rq->queue_lock);
-}
-
-static void ubiblock_request(struct request_queue *rq)
-{
-	struct ubiblock *dev;
-	struct request *req;
-
-	dev = rq->queuedata;
-
-	if (!dev)
-		while ((req = blk_fetch_request(rq)) != NULL)
-			__blk_end_request_all(req, -ENODEV);
-	else
-		queue_work(dev->wq, &dev->work);
-}
-
 static int ubiblock_open(struct block_device *bdev, fmode_t mode)
 {
 	struct ubiblock *dev = bdev->bd_disk->private_data;
@@ -374,6 +293,66 @@ static const struct block_device_operations ubiblock_ops = {
 	.getgeo	= ubiblock_getgeo,
 };
 
+static void ubiblock_do_work(struct work_struct *work)
+{
+	int ret;
+	struct ubiblock_pdu *pdu = container_of(work, struct ubiblock_pdu, work);
+
+	ret = ubiblock_read(pdu);
+	blk_mq_end_request(pdu->req, ret ?: 0);
+}
+
+static int ubiblock_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req,
+			     bool last)
+{
+	int ret;
+	struct ubiblock *dev = hctx->queue->queuedata;
+	struct ubiblock_pdu *pdu = blk_mq_rq_to_pdu(req);
+
+	if (req->cmd_type != REQ_TYPE_FS)
+		return BLK_MQ_RQ_QUEUE_ERROR;
+
+	if (blk_rq_pos(req) + blk_rq_cur_sectors(req) >
+	    get_capacity(req->rq_disk))
+		return BLK_MQ_RQ_QUEUE_ERROR;
+
+	if (rq_data_dir(req) != READ)
+		return BLK_MQ_RQ_QUEUE_ERROR; /* Write not implemented */
+
+	pdu->usgl.list_pos = 0;
+	pdu->usgl.page_pos = 0;
+
+	blk_mq_start_request(req);
+	ret = blk_rq_map_sg(hctx->queue, req, pdu->usgl.sg);
+
+	queue_work(dev->wq, &pdu->work);
+
+	return BLK_MQ_RQ_QUEUE_OK;
+}
+
+static int ubiblock_init_request(void *data, struct request *req,
+				 unsigned int hctx_idx,
+				 unsigned int request_idx,
+				 unsigned int numa_node)
+{
+	struct ubiblock *dev = data;
+	struct ubiblock_pdu *pdu = blk_mq_rq_to_pdu(req);
+
+	pdu->dev = dev;
+	pdu->req = req;
+
+	sg_init_table(pdu->usgl.sg, UBI_MAX_SG_COUNT);
+	INIT_WORK(&pdu->work, ubiblock_do_work);
+
+	return 0;
+}
+
+static struct blk_mq_ops ubiblock_mq_ops = {
+	.queue_rq       = ubiblock_queue_rq,
+	.init_request	= ubiblock_init_request,
+	.map_queue      = blk_mq_map_queue,
+};
+
 int ubiblock_create(struct ubi_volume_info *vi)
 {
 	struct ubiblock *dev;
@@ -417,13 +396,27 @@ int ubiblock_create(struct ubi_volume_info *vi)
 	set_capacity(gd, disk_capacity);
 	dev->gd = gd;
 
-	spin_lock_init(&dev->queue_lock);
-	dev->rq = blk_init_queue(ubiblock_request, &dev->queue_lock);
+	dev->tag_set.ops = &ubiblock_mq_ops;
+	dev->tag_set.queue_depth = 64;
+	dev->tag_set.numa_node = NUMA_NO_NODE;
+	dev->tag_set.flags = BLK_MQ_F_SHOULD_MERGE;
+	dev->tag_set.cmd_size = sizeof(struct ubiblock_pdu);
+	dev->tag_set.driver_data = dev;
+	dev->tag_set.nr_hw_queues = 1;
+
+	ret = blk_mq_alloc_tag_set(&dev->tag_set);
+	if (ret) {
+		dev_err(disk_to_dev(dev->gd), "blk_mq_alloc_tag_set failed");
+		goto out_put_disk;
+	}
+
+	dev->rq = blk_mq_init_queue(&dev->tag_set);
 	if (!dev->rq) {
-		dev_err(disk_to_dev(gd), "blk_init_queue failed");
+		dev_err(disk_to_dev(gd), "blk_mq_init_queue failed");
 		ret = -ENODEV;
-		goto out_put_disk;
+		goto out_free_tags;
 	}
+	blk_queue_max_segments(dev->rq, UBI_MAX_SG_COUNT);
 
 	dev->rq->queuedata = dev;
 	dev->gd->queue = dev->rq;
@@ -437,7 +430,6 @@ int ubiblock_create(struct ubi_volume_info *vi)
 		ret = -ENOMEM;
 		goto out_free_queue;
 	}
-	INIT_WORK(&dev->work, ubiblock_do_work);
 
 	mutex_lock(&devices_mutex);
 	list_add_tail(&dev->list, &ubiblock_devices);
@@ -451,6 +443,8 @@ int ubiblock_create(struct ubi_volume_info *vi)
 
 out_free_queue:
 	blk_cleanup_queue(dev->rq);
+out_free_tags:
+	blk_mq_free_tag_set(&dev->tag_set);
 out_put_disk:
 	put_disk(dev->gd);
 out_free_dev:
@@ -461,8 +455,13 @@ out_free_dev:
 
 static void ubiblock_cleanup(struct ubiblock *dev)
 {
+	/* Stop new requests to arrive */
 	del_gendisk(dev->gd);
+	/* Flush pending work */
+	destroy_workqueue(dev->wq);
+	/* Finally destroy the blk queue */
 	blk_cleanup_queue(dev->rq);
+	blk_mq_free_tag_set(&dev->tag_set);
 	dev_info(disk_to_dev(dev->gd), "released");
 	put_disk(dev->gd);
 }
@@ -490,9 +489,6 @@ int ubiblock_remove(struct ubi_volume_info *vi)
 	list_del(&dev->list);
 	mutex_unlock(&devices_mutex);
 
-	/* Flush pending work and stop this workqueue */
-	destroy_workqueue(dev->wq);
-
 	ubiblock_cleanup(dev);
 	mutex_unlock(&dev->dev_mutex);
 	kfree(dev);
@@ -620,8 +616,6 @@ static void ubiblock_remove_all(void)
 	struct ubiblock *dev;
 
 	list_for_each_entry_safe(dev, next, &ubiblock_devices, list) {
-		/* Flush pending work and stop workqueue */
-		destroy_workqueue(dev->wq);
 		/* The module is being forcefully removed */
 		WARN_ON(dev->desc);
 		/* Remove from device list */
-- 
1.8.4.5