diff mbox series

[v11,22/26] block/rnbd: server: functionality for IO submission to file or block dev

Message ID 20200320121657.1165-23-jinpu.wang@cloud.ionos.com (mailing list archive)
State Changes Requested
Headers show
Series RTRS (former IBTRS) RDMA Transport Library and RNBD (former IBNBD) RDMA Network Block Device | expand

Commit Message

Jinpu Wang March 20, 2020, 12:16 p.m. UTC 
This provides helper functions for IO submission to file or block dev.

Signed-off-by: Danil Kipnis <danil.kipnis@cloud.ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
 drivers/block/rnbd/rnbd-srv-dev.c | 94 +++++++++++++++++++++++++++++++
 drivers/block/rnbd/rnbd-srv-dev.h | 88 +++++++++++++++++++++++++++++
 2 files changed, 182 insertions(+)
 create mode 100644 drivers/block/rnbd/rnbd-srv-dev.c
 create mode 100644 drivers/block/rnbd/rnbd-srv-dev.h

Comments

Bart Van Assche March 28, 2020, 6:39 p.m. UTC | #1 
On 2020-03-20 05:16, Jack Wang wrote:
> This provides helper functions for IO submission to file or block dev.

Regarding the title of this patch: is file I/O still supported? Wasn't
that support removed some time ago?

> +struct rnbd_dev *rnbd_dev_open(const char *path, fmode_t flags,
> +			       void (*io_cb)(void *priv, int error))
> +{
> +	struct rnbd_dev *dev;
> +	int ret;
> +
> +	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> +	if (!dev)
> +		return ERR_PTR(-ENOMEM);
> +
> +	dev->blk_open_flags = flags;
> +	dev->bdev = blkdev_get_by_path(path, flags, THIS_MODULE);
> +	ret = PTR_ERR_OR_ZERO(dev->bdev);
> +	if (ret)
> +		goto err;
> +
> +	dev->blk_open_flags	= flags;
> +	dev->io_cb		= io_cb;
> +	bdevname(dev->bdev, dev->name);
> +
> +	return dev;
> +
> +err:
> +	kfree(dev);
> +	return ERR_PTR(ret);
> +}

This function only has one caller so io_cb is always equal to the
argument passed by that single caller, namely rnbd_endio. If that
argument and also dev->io_cb would be removed, would that make the hot
path faster?

> +int rnbd_dev_submit_io(struct rnbd_dev *dev, sector_t sector, void *data,
> +			size_t len, u32 bi_size, enum rnbd_io_flags flags,
> +			short prio, void *priv)
> +{
> +	struct request_queue *q = bdev_get_queue(dev->bdev);
> +	struct rnbd_dev_blk_io *io;
> +	struct bio *bio;
> +
> +	/* check if the buffer is suitable for bdev */
> +	if (WARN_ON(!blk_rq_aligned(q, (unsigned long)data, len)))
> +		return -EINVAL;

The blk_rq_aligned() check looks weird to me. bio_map_kern() can handle
data buffers that do not match the DMA alignment requirements, so why to
refuse data buffers that are not satisfy DMA alignment requirements?

> +	/* Generate bio with pages pointing to the rdma buffer */
> +	bio = bio_map_kern(q, data, len, GFP_KERNEL);
> +	if (IS_ERR(bio))
> +		return PTR_ERR(bio);
> +
> +	io = kmalloc(sizeof(*io), GFP_KERNEL);
> +	if (unlikely(!io)) {
> +		bio_put(bio);
> +		return -ENOMEM;
> +	}
> +
> +	io->dev		= dev;
> +	io->priv	= priv;
> +
> +	bio->bi_end_io		= rnbd_dev_bi_end_io;
> +	bio->bi_private		= io;
> +	bio->bi_opf		= rnbd_to_bio_flags(flags);
> +	bio->bi_iter.bi_sector	= sector;
> +	bio->bi_iter.bi_size	= bi_size;
> +	bio_set_prio(bio, prio);
> +	bio_set_dev(bio, dev->bdev);

I think Jason strongly prefers to have a single space at the left of the
assignment operator.

Thanks,

Bart.
Jinpu Wang March 31, 2020, 10:06 a.m. UTC | #2 
On Sat, Mar 28, 2020 at 7:39 PM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 2020-03-20 05:16, Jack Wang wrote:
> > This provides helper functions for IO submission to file or block dev.
>
> Regarding the title of this patch: is file I/O still supported? Wasn't
> that support removed some time ago?
Sorry, we forget to update it, will fix.
>
> > +struct rnbd_dev *rnbd_dev_open(const char *path, fmode_t flags,
> > +                            void (*io_cb)(void *priv, int error))
> > +{
> > +     struct rnbd_dev *dev;
> > +     int ret;
> > +
> > +     dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> > +     if (!dev)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     dev->blk_open_flags = flags;
> > +     dev->bdev = blkdev_get_by_path(path, flags, THIS_MODULE);
> > +     ret = PTR_ERR_OR_ZERO(dev->bdev);
> > +     if (ret)
> > +             goto err;
> > +
> > +     dev->blk_open_flags     = flags;
> > +     dev->io_cb              = io_cb;
> > +     bdevname(dev->bdev, dev->name);
> > +
> > +     return dev;
> > +
> > +err:
> > +     kfree(dev);
> > +     return ERR_PTR(ret);
> > +}
>
> This function only has one caller so io_cb is always equal to the
> argument passed by that single caller, namely rnbd_endio. If that
> argument and also dev->io_cb would be removed, would that make the hot
> path faster?
Sounds good, will do it.

>
> > +int rnbd_dev_submit_io(struct rnbd_dev *dev, sector_t sector, void *data,
> > +                     size_t len, u32 bi_size, enum rnbd_io_flags flags,
> > +                     short prio, void *priv)
> > +{
> > +     struct request_queue *q = bdev_get_queue(dev->bdev);
> > +     struct rnbd_dev_blk_io *io;
> > +     struct bio *bio;
> > +
> > +     /* check if the buffer is suitable for bdev */
> > +     if (WARN_ON(!blk_rq_aligned(q, (unsigned long)data, len)))
> > +             return -EINVAL;
>
> The blk_rq_aligned() check looks weird to me. bio_map_kern() can handle
> data buffers that do not match the DMA alignment requirements, so why to
> refuse data buffers that are not satisfy DMA alignment requirements?
We add the check since 2014 to make sure the data buffer is aligned.
AFAIR we nerver see the WARN triggered.
so will remove it.
>
> > +     /* Generate bio with pages pointing to the rdma buffer */
> > +     bio = bio_map_kern(q, data, len, GFP_KERNEL);
> > +     if (IS_ERR(bio))
> > +             return PTR_ERR(bio);
> > +
> > +     io = kmalloc(sizeof(*io), GFP_KERNEL);
> > +     if (unlikely(!io)) {
> > +             bio_put(bio);
> > +             return -ENOMEM;
> > +     }
> > +
> > +     io->dev         = dev;
> > +     io->priv        = priv;
> > +
> > +     bio->bi_end_io          = rnbd_dev_bi_end_io;
> > +     bio->bi_private         = io;
> > +     bio->bi_opf             = rnbd_to_bio_flags(flags);
> > +     bio->bi_iter.bi_sector  = sector;
> > +     bio->bi_iter.bi_size    = bi_size;
> > +     bio_set_prio(bio, prio);
> > +     bio_set_dev(bio, dev->bdev);
>
> I think Jason strongly prefers to have a single space at the left of the
> assignment operator.
ok.
>
> Thanks,
>
> Bart.
Thanks!
diff mbox series

Patch

diff --git a/drivers/block/rnbd/rnbd-srv-dev.c b/drivers/block/rnbd/rnbd-srv-dev.c
new file mode 100644
index 000000000000..370f4a43e10c
--- /dev/null
+++ b/drivers/block/rnbd/rnbd-srv-dev.c
@@ -0,0 +1,94 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * RDMA Network Block Driver
+ *
+ * Copyright (c) 2014 - 2018 ProfitBricks GmbH. All rights reserved.
+ * Copyright (c) 2018 - 2019 1&1 IONOS Cloud GmbH. All rights reserved.
+ * Copyright (c) 2019 - 2020 1&1 IONOS SE. All rights reserved.
+ */
+#undef pr_fmt
+#define pr_fmt(fmt) KBUILD_MODNAME " L" __stringify(__LINE__) ": " fmt
+
+#include "rnbd-srv-dev.h"
+#include "rnbd-log.h"
+
+struct rnbd_dev *rnbd_dev_open(const char *path, fmode_t flags,
+			       void (*io_cb)(void *priv, int error))
+{
+	struct rnbd_dev *dev;
+	int ret;
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return ERR_PTR(-ENOMEM);
+
+	dev->blk_open_flags = flags;
+	dev->bdev = blkdev_get_by_path(path, flags, THIS_MODULE);
+	ret = PTR_ERR_OR_ZERO(dev->bdev);
+	if (ret)
+		goto err;
+
+	dev->blk_open_flags	= flags;
+	dev->io_cb		= io_cb;
+	bdevname(dev->bdev, dev->name);
+
+	return dev;
+
+err:
+	kfree(dev);
+	return ERR_PTR(ret);
+}
+
+void rnbd_dev_close(struct rnbd_dev *dev)
+{
+	blkdev_put(dev->bdev, dev->blk_open_flags);
+	kfree(dev);
+}
+
+static void rnbd_dev_bi_end_io(struct bio *bio)
+{
+	struct rnbd_dev_blk_io *io = bio->bi_private;
+
+	io->dev->io_cb(io->priv, blk_status_to_errno(bio->bi_status));
+	bio_put(bio);
+	kfree(io);
+}
+
+int rnbd_dev_submit_io(struct rnbd_dev *dev, sector_t sector, void *data,
+			size_t len, u32 bi_size, enum rnbd_io_flags flags,
+			short prio, void *priv)
+{
+	struct request_queue *q = bdev_get_queue(dev->bdev);
+	struct rnbd_dev_blk_io *io;
+	struct bio *bio;
+
+	/* check if the buffer is suitable for bdev */
+	if (WARN_ON(!blk_rq_aligned(q, (unsigned long)data, len)))
+		return -EINVAL;
+
+	/* Generate bio with pages pointing to the rdma buffer */
+	bio = bio_map_kern(q, data, len, GFP_KERNEL);
+	if (IS_ERR(bio))
+		return PTR_ERR(bio);
+
+	io = kmalloc(sizeof(*io), GFP_KERNEL);
+	if (unlikely(!io)) {
+		bio_put(bio);
+		return -ENOMEM;
+	}
+
+	io->dev		= dev;
+	io->priv	= priv;
+
+	bio->bi_end_io		= rnbd_dev_bi_end_io;
+	bio->bi_private		= io;
+	bio->bi_opf		= rnbd_to_bio_flags(flags);
+	bio->bi_iter.bi_sector	= sector;
+	bio->bi_iter.bi_size	= bi_size;
+	bio_set_prio(bio, prio);
+	bio_set_dev(bio, dev->bdev);
+
+	submit_bio(bio);
+
+	return 0;
+}
diff --git a/drivers/block/rnbd/rnbd-srv-dev.h b/drivers/block/rnbd/rnbd-srv-dev.h
new file mode 100644
index 000000000000..6d07b9323f84
--- /dev/null
+++ b/drivers/block/rnbd/rnbd-srv-dev.h
@@ -0,0 +1,88 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * RDMA Network Block Driver
+ *
+ * Copyright (c) 2014 - 2018 ProfitBricks GmbH. All rights reserved.
+ * Copyright (c) 2018 - 2019 1&1 IONOS Cloud GmbH. All rights reserved.
+ * Copyright (c) 2019 - 2020 1&1 IONOS SE. All rights reserved.
+ */
+#ifndef RNBD_SRV_DEV_H
+#define RNBD_SRV_DEV_H
+
+#include <linux/fs.h>
+#include "rnbd-proto.h"
+
+struct rnbd_dev {
+	struct block_device	*bdev;
+	fmode_t			blk_open_flags;
+	char			name[BDEVNAME_SIZE];
+	void			(*io_cb)(void *priv, int error);
+};
+
+struct rnbd_dev_blk_io {
+	struct rnbd_dev *dev;
+	void		 *priv;
+};
+
+/**
+ * rnbd_dev_open() - Open a device
+ * @flags:	open flags
+ * @io_cb:	is called when I/O finished
+ */
+struct rnbd_dev *rnbd_dev_open(const char *path, fmode_t flags,
+			       void (*io_cb)(void *priv, int error));
+
+/**
+ * rnbd_dev_close() - Close a device
+ */
+void rnbd_dev_close(struct rnbd_dev *dev);
+
+static inline int rnbd_dev_get_max_segs(const struct rnbd_dev *dev)
+{
+	return queue_max_segments(bdev_get_queue(dev->bdev));
+}
+
+static inline int rnbd_dev_get_max_hw_sects(const struct rnbd_dev *dev)
+{
+	return queue_max_hw_sectors(bdev_get_queue(dev->bdev));
+}
+
+static inline int rnbd_dev_get_secure_discard(const struct rnbd_dev *dev)
+{
+	return blk_queue_secure_erase(bdev_get_queue(dev->bdev));
+}
+
+static inline int rnbd_dev_get_max_discard_sects(const struct rnbd_dev *dev)
+{
+	if (!blk_queue_discard(bdev_get_queue(dev->bdev)))
+		return 0;
+
+	return blk_queue_get_max_sectors(bdev_get_queue(dev->bdev),
+					 REQ_OP_DISCARD);
+}
+
+static inline int rnbd_dev_get_discard_granularity(const struct rnbd_dev *dev)
+{
+	return bdev_get_queue(dev->bdev)->limits.discard_granularity;
+}
+
+static inline int rnbd_dev_get_discard_alignment(const struct rnbd_dev *dev)
+{
+	return bdev_get_queue(dev->bdev)->limits.discard_alignment;
+}
+
+/**
+ * rnbd_dev_submit_io() - Submit an I/O to the disk
+ * @dev:	device to that the I/O is submitted
+ * @sector:	address to read/write data to
+ * @data:	I/O data to write or buffer to read I/O date into
+ * @len:	length of @data
+ * @bi_size:	Amount of data that will be read/written
+ * @prio:       IO priority
+ * @priv:	private data passed to @io_fn
+ */
+int rnbd_dev_submit_io(struct rnbd_dev *dev, sector_t sector, void *data,
+			size_t len, u32 bi_size, enum rnbd_io_flags flags,
+			short prio, void *priv);
+
+#endif /* RNBD_SRV_DEV_H */