This sample demonstrates how to use blk_interposer.
It show how to properly connect the interposer module to kernel,
and perform the simplest monitoring of the number of bio.
Signed-off-by: Sergei Shtepa <[email protected]>
---
samples/blk_interposer/Makefile | 2 +
samples/blk_interposer/blk-interposer.c | 276 ++++++++++++++++++++++++
2 files changed, 278 insertions(+)
create mode 100644 samples/blk_interposer/Makefile
create mode 100644 samples/blk_interposer/blk-interposer.c
diff --git a/samples/blk_interposer/Makefile b/samples/blk_interposer/Makefile
new file mode 100644
index 000000000000..b11aefde2b1c
--- /dev/null
+++ b/samples/blk_interposer/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_SAMPLE_BLK_INTERPOSER) += blk-interposer.o
diff --git a/samples/blk_interposer/blk-interposer.c b/samples/blk_interposer/blk-interposer.c
new file mode 100644
index 000000000000..92b4c1fcf8f7
--- /dev/null
+++ b/samples/blk_interposer/blk-interposer.c
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Block layer interposer allow to interpose bio requests from kernel module.
+ * This allows you to monitor requests, modify requests, add new request,
+ * or even redirect requests to another devices.
+ *
+ * This sample demonstrates how to use blk_interposer.
+ * It show how to properly connect the interposer module to kernel,
+ * and perform the simplest monitoring of the number of bio.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/blkdev.h>
+#include <linux/genhd.h>
+#include <linux/blk-mq.h>
+
+int device_major = 8;
+int device_minor;
+int fmode = FMODE_READ | FMODE_WRITE;
+
+/*
+ * Each interposer must have a common part in the form of the blk_interposer structure,
+ * as well as its own unique data.
+ */
+struct my_interposer {
+ /*
+ * Common part of block device interposer.
+ */
+ struct blk_interposer interposer;
+ /*
+ * Specific part for our interposer data.
+ */
+ atomic_t counter;
+};
+
+struct my_interposer my_ip;
+
+/**
+ * blk_interposer_attach - Attach interposer to disk
+ * @disk: target disk
+ * @interposer: block device interposer
+ */
+static int blk_interposer_attach(struct gendisk *disk, struct blk_interposer *interposer)
+{
+ int ret = 0;
+
+ /*
+ * Stop disks queue processing.
+ */
+ blk_mq_freeze_queue(disk->queue);
+ blk_mq_quiesce_queue(disk->queue);
+
+ /*
+ * Check if the interposer is already busy.
+ * The interposer will only connect if it is not busy.
+ */
+ if (blk_has_interposer(disk)) {
+ pr_info("The interposer is already busy.\n");
+ ret = -EBUSY;
+ goto out;
+ }
+
+ /*
+ * Attach the interposer.
+ */
+ disk->interposer = interposer;
+ /*
+ * And while the queue is stopped, we can do something specific for our module.
+ */
+ pr_info("Block device interposer attached successfully.\n");
+
+out:
+ /*
+ * Resume disks queue processing
+ */
+ blk_mq_unquiesce_queue(disk->queue);
+ blk_mq_unfreeze_queue(disk->queue);
+
+ return ret;
+}
+
+/**
+ * blk_interposer_detach - Detach interposer from disk
+ * @disk: target disk
+ * @interposer: block device interposer
+ */
+static int blk_interposer_detach(struct gendisk *disk, struct blk_interposer *interposer)
+{
+ int ret = 0;
+
+ if (WARN_ON(!disk))
+ return -EINVAL;
+
+ /*
+ * Stop disks queue processing.
+ */
+ blk_mq_freeze_queue(disk->queue);
+ blk_mq_quiesce_queue(disk->queue);
+
+ /*
+ * Check if the interposer is still available.
+ */
+ if (!disk->interposer) {
+ pr_info("The interposer is not available.\n");
+ return -ENOENT;
+ goto out;
+ }
+ /*
+ * Check if it is really our interposer.
+ */
+ if (disk->interposer->ip_submit_bio != interposer->ip_submit_bio) {
+ pr_info("The interposer found is not ours.\n");
+ return -EPERM;
+ goto out;
+ }
+
+ /*
+ * Detach interposer.
+ */
+ disk->interposer = NULL;
+ /*
+ * And while the queue is stopped, we can do something specific for our module.
+ */
+ pr_info("Block device interposer detached successfully.\n");
+
+out:
+ /*
+ * Resume disks queue processing.
+ */
+ blk_mq_unquiesce_queue(disk->queue);
+ blk_mq_unfreeze_queue(disk->queue);
+
+ return ret;
+}
+
+/**
+ * blk_interposer_submit_bio - Interpose bio
+ * @interposer: block device interposer
+ * @bio: interposed bio
+ */
+static void blk_interposer_submit_bio(struct blk_interposer *interposer, struct bio *bio)
+{
+ struct my_interposer *ip = container_of(interposer, struct my_interposer, interposer);
+
+ atomic_inc(&ip->counter);
+ if (atomic_read(&ip->counter) < 10)
+ pr_info("%s - %d", __func__, atomic_read(&ip->counter));
+ else if (atomic_read(&ip->counter) == 10)
+ pr_info("%s the flooding of log has stopped", __func__);
+
+ /*
+ * Here we can submit a new child bio requests before the original.
+ * If the new bio should not be interposed, set BIO_INTERPOSED flag.
+ */
+
+ /*
+ * We can resubmit the original bio or just add it to the current->bio_list.
+ * In the second case, we do not call submit_bio_checks() twice.
+ */
+ bio_list_add(¤t->bio_list[0], bio);
+
+ /*
+ * Here we can submit a new bio requests after the original,
+ * although I don't know how it might be useful.
+ */
+
+ /*
+ * All submitted bio will be just placed in the current->bio_list.
+ * We can't wait for the bio request to complete.
+ */
+
+ /*
+ * The function doesn't return anything, because it can only return BLK_QC_T_NONE.
+ */
+}
+
+static int __init blk_interposer_init(void)
+{
+ int ret = 0;
+ dev_t dev_id;
+ struct block_device *bdev;
+
+ pr_info("Loading module\n");
+
+ pr_info("Tracking device: %d:%d\n", device_major, device_minor);
+ dev_id = MKDEV(device_major, device_minor);
+
+ /*
+ * Open block device
+ */
+ bdev = blkdev_get_by_dev(dev_id, fmode, &my_ip);
+ if (IS_ERR(bdev)) {
+ ret = PTR_ERR(bdev);
+ pr_err("Unable to open device [%d:%d]: blkdev_get_by_dev return %d\n",
+ MAJOR(dev_id), MINOR(dev_id), 0-ret);
+ return ret;
+ }
+
+ /*
+ * Initialize block device interposer fields.
+ */
+ my_ip.interposer.ip_submit_bio = blk_interposer_submit_bio;
+
+ /*
+ * Initialize our interposer specific fields
+ */
+ atomic_set(&my_ip.counter, 0);
+
+ /*
+ * Attach interposer to disk
+ */
+ ret = blk_interposer_attach(bdev->bd_disk, &my_ip.interposer);
+ if (ret)
+ pr_info("Failed to attach the block device interposer.\n");
+ /*
+ * Close device
+ */
+ blkdev_put(bdev, fmode);
+
+ return ret;
+}
+
+static void __exit blk_interposer_exit(void)
+{
+ int ret;
+ dev_t dev_id;
+ struct block_device *bdev;
+
+ pr_info("Unloading module\n");
+
+ dev_id = MKDEV(device_major, device_minor);
+
+ /*
+ * Open block device
+ */
+ bdev = blkdev_get_by_dev(dev_id, fmode, &my_ip);
+ if (IS_ERR(bdev)) {
+ ret = PTR_ERR(bdev);
+ pr_err("Unable to open device [%d:%d]: blkdev_get_by_dev return %d\n",
+ MAJOR(dev_id), MINOR(dev_id), 0-ret);
+ return;
+ }
+
+ /*
+ * Detach interposer from disk
+ */
+ ret = blk_interposer_detach(bdev->bd_disk, &my_ip.interposer);
+ if (ret)
+ pr_info("Failed to detach the block device interposer.\n");
+ /*
+ * Close device
+ */
+ blkdev_put(bdev, fmode);
+
+ /*
+ * The counter shows how much bio was interposed.
+ */
+ pr_info("%d bio was interposed.\n", atomic_read(&my_ip.counter));
+}
+
+module_init(blk_interposer_init);
+module_exit(blk_interposer_exit);
+
+MODULE_DESCRIPTION("Block Layer Interposer Sample kernel module");
+MODULE_AUTHOR("Veeam Software Group GmbH");
+MODULE_LICENSE("GPL");
+
+module_param_named(device_major, device_major, int, 0644);
+MODULE_PARM_DESC(device_major, "Tracking device major number");
+module_param_named(device_minor, device_minor, int, 0644);
+MODULE_PARM_DESC(device_minor, "Tracking device minor number");
--
2.20.1
On Wed, Dec 09 2020 at 8:01am -0500,
Sergei Shtepa <[email protected]> wrote:
> This sample demonstrates how to use blk_interposer.
> It show how to properly connect the interposer module to kernel,
> and perform the simplest monitoring of the number of bio.
>
> Signed-off-by: Sergei Shtepa <[email protected]>
> ---
> samples/blk_interposer/Makefile | 2 +
> samples/blk_interposer/blk-interposer.c | 276 ++++++++++++++++++++++++
> 2 files changed, 278 insertions(+)
> create mode 100644 samples/blk_interposer/Makefile
> create mode 100644 samples/blk_interposer/blk-interposer.c
>
> diff --git a/samples/blk_interposer/Makefile b/samples/blk_interposer/Makefile
> new file mode 100644
> index 000000000000..b11aefde2b1c
> --- /dev/null
> +++ b/samples/blk_interposer/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_SAMPLE_BLK_INTERPOSER) += blk-interposer.o
> diff --git a/samples/blk_interposer/blk-interposer.c b/samples/blk_interposer/blk-interposer.c
> new file mode 100644
> index 000000000000..92b4c1fcf8f7
> --- /dev/null
> +++ b/samples/blk_interposer/blk-interposer.c
> @@ -0,0 +1,276 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Block layer interposer allow to interpose bio requests from kernel module.
> + * This allows you to monitor requests, modify requests, add new request,
> + * or even redirect requests to another devices.
> + *
> + * This sample demonstrates how to use blk_interposer.
> + * It show how to properly connect the interposer module to kernel,
> + * and perform the simplest monitoring of the number of bio.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/errno.h>
> +#include <linux/blkdev.h>
> +#include <linux/genhd.h>
> +#include <linux/blk-mq.h>
> +
> +int device_major = 8;
> +int device_minor;
> +int fmode = FMODE_READ | FMODE_WRITE;
> +
> +/*
> + * Each interposer must have a common part in the form of the blk_interposer structure,
> + * as well as its own unique data.
> + */
> +struct my_interposer {
> + /*
> + * Common part of block device interposer.
> + */
> + struct blk_interposer interposer;
> + /*
> + * Specific part for our interposer data.
> + */
> + atomic_t counter;
> +};
> +
> +struct my_interposer my_ip;
> +
> +/**
> + * blk_interposer_attach - Attach interposer to disk
> + * @disk: target disk
> + * @interposer: block device interposer
> + */
> +static int blk_interposer_attach(struct gendisk *disk, struct blk_interposer *interposer)
> +{
> + int ret = 0;
> +
> + /*
> + * Stop disks queue processing.
> + */
> + blk_mq_freeze_queue(disk->queue);
> + blk_mq_quiesce_queue(disk->queue);
> +
> + /*
> + * Check if the interposer is already busy.
> + * The interposer will only connect if it is not busy.
> + */
> + if (blk_has_interposer(disk)) {
> + pr_info("The interposer is already busy.\n");
> + ret = -EBUSY;
> + goto out;
> + }
> +
> + /*
> + * Attach the interposer.
> + */
> + disk->interposer = interposer;
> + /*
> + * And while the queue is stopped, we can do something specific for our module.
> + */
> + pr_info("Block device interposer attached successfully.\n");
> +
> +out:
> + /*
> + * Resume disks queue processing
> + */
> + blk_mq_unquiesce_queue(disk->queue);
> + blk_mq_unfreeze_queue(disk->queue);
> +
> + return ret;
> +}
> +
> +/**
> + * blk_interposer_detach - Detach interposer from disk
> + * @disk: target disk
> + * @interposer: block device interposer
> + */
> +static int blk_interposer_detach(struct gendisk *disk, struct blk_interposer *interposer)
> +{
> + int ret = 0;
> +
> + if (WARN_ON(!disk))
> + return -EINVAL;
> +
> + /*
> + * Stop disks queue processing.
> + */
> + blk_mq_freeze_queue(disk->queue);
> + blk_mq_quiesce_queue(disk->queue);
> +
> + /*
> + * Check if the interposer is still available.
> + */
> + if (!disk->interposer) {
> + pr_info("The interposer is not available.\n");
> + return -ENOENT;
> + goto out;
> + }
> + /*
> + * Check if it is really our interposer.
> + */
> + if (disk->interposer->ip_submit_bio != interposer->ip_submit_bio) {
> + pr_info("The interposer found is not ours.\n");
> + return -EPERM;
> + goto out;
> + }
> +
> + /*
> + * Detach interposer.
> + */
> + disk->interposer = NULL;
> + /*
> + * And while the queue is stopped, we can do something specific for our module.
> + */
> + pr_info("Block device interposer detached successfully.\n");
> +
> +out:
> + /*
> + * Resume disks queue processing.
> + */
> + blk_mq_unquiesce_queue(disk->queue);
> + blk_mq_unfreeze_queue(disk->queue);
> +
> + return ret;
> +}
This attach and detach code needs to be elevated out of samples so that
any future consumer of blk_interposer doesn't reinvent it. It is far
too fundamental.
The way you've proposed this be merged is very much unacceptable.
Nacked-by: Mike Snitzer <[email protected]>
The 12/09/2020 17:36, Mike Snitzer wrote:
> On Wed, Dec 09 2020 at 8:01am -0500,
> Sergei Shtepa <[email protected]> wrote:
>
> > This sample demonstrates how to use blk_interposer.
> > It show how to properly connect the interposer module to kernel,
> > and perform the simplest monitoring of the number of bio.
> >
> > Signed-off-by: Sergei Shtepa <[email protected]>
> > ---
> > samples/blk_interposer/Makefile | 2 +
> > samples/blk_interposer/blk-interposer.c | 276 ++++++++++++++++++++++++
> > 2 files changed, 278 insertions(+)
> > create mode 100644 samples/blk_interposer/Makefile
> > create mode 100644 samples/blk_interposer/blk-interposer.c
> >
> > diff --git a/samples/blk_interposer/Makefile b/samples/blk_interposer/Makefile
> > new file mode 100644
> > index 000000000000..b11aefde2b1c
> > --- /dev/null
> > +++ b/samples/blk_interposer/Makefile
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +obj-$(CONFIG_SAMPLE_BLK_INTERPOSER) += blk-interposer.o
> > diff --git a/samples/blk_interposer/blk-interposer.c b/samples/blk_interposer/blk-interposer.c
> > new file mode 100644
> > index 000000000000..92b4c1fcf8f7
> > --- /dev/null
> > +++ b/samples/blk_interposer/blk-interposer.c
> > @@ -0,0 +1,276 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * Block layer interposer allow to interpose bio requests from kernel module.
> > + * This allows you to monitor requests, modify requests, add new request,
> > + * or even redirect requests to another devices.
> > + *
> > + * This sample demonstrates how to use blk_interposer.
> > + * It show how to properly connect the interposer module to kernel,
> > + * and perform the simplest monitoring of the number of bio.
> > + */
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/module.h>
> > +#include <linux/types.h>
> > +#include <linux/errno.h>
> > +#include <linux/blkdev.h>
> > +#include <linux/genhd.h>
> > +#include <linux/blk-mq.h>
> > +
> > +int device_major = 8;
> > +int device_minor;
> > +int fmode = FMODE_READ | FMODE_WRITE;
> > +
> > +/*
> > + * Each interposer must have a common part in the form of the blk_interposer structure,
> > + * as well as its own unique data.
> > + */
> > +struct my_interposer {
> > + /*
> > + * Common part of block device interposer.
> > + */
> > + struct blk_interposer interposer;
> > + /*
> > + * Specific part for our interposer data.
> > + */
> > + atomic_t counter;
> > +};
> > +
> > +struct my_interposer my_ip;
> > +
> > +/**
> > + * blk_interposer_attach - Attach interposer to disk
> > + * @disk: target disk
> > + * @interposer: block device interposer
> > + */
> > +static int blk_interposer_attach(struct gendisk *disk, struct blk_interposer *interposer)
> > +{
> > + int ret = 0;
> > +
> > + /*
> > + * Stop disks queue processing.
> > + */
> > + blk_mq_freeze_queue(disk->queue);
> > + blk_mq_quiesce_queue(disk->queue);
> > +
> > + /*
> > + * Check if the interposer is already busy.
> > + * The interposer will only connect if it is not busy.
> > + */
> > + if (blk_has_interposer(disk)) {
> > + pr_info("The interposer is already busy.\n");
> > + ret = -EBUSY;
> > + goto out;
> > + }
> > +
> > + /*
> > + * Attach the interposer.
> > + */
> > + disk->interposer = interposer;
> > + /*
> > + * And while the queue is stopped, we can do something specific for our module.
> > + */
> > + pr_info("Block device interposer attached successfully.\n");
> > +
> > +out:
> > + /*
> > + * Resume disks queue processing
> > + */
> > + blk_mq_unquiesce_queue(disk->queue);
> > + blk_mq_unfreeze_queue(disk->queue);
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * blk_interposer_detach - Detach interposer from disk
> > + * @disk: target disk
> > + * @interposer: block device interposer
> > + */
> > +static int blk_interposer_detach(struct gendisk *disk, struct blk_interposer *interposer)
> > +{
> > + int ret = 0;
> > +
> > + if (WARN_ON(!disk))
> > + return -EINVAL;
> > +
> > + /*
> > + * Stop disks queue processing.
> > + */
> > + blk_mq_freeze_queue(disk->queue);
> > + blk_mq_quiesce_queue(disk->queue);
> > +
> > + /*
> > + * Check if the interposer is still available.
> > + */
> > + if (!disk->interposer) {
> > + pr_info("The interposer is not available.\n");
> > + return -ENOENT;
> > + goto out;
> > + }
> > + /*
> > + * Check if it is really our interposer.
> > + */
> > + if (disk->interposer->ip_submit_bio != interposer->ip_submit_bio) {
> > + pr_info("The interposer found is not ours.\n");
> > + return -EPERM;
> > + goto out;
> > + }
> > +
> > + /*
> > + * Detach interposer.
> > + */
> > + disk->interposer = NULL;
> > + /*
> > + * And while the queue is stopped, we can do something specific for our module.
> > + */
> > + pr_info("Block device interposer detached successfully.\n");
> > +
> > +out:
> > + /*
> > + * Resume disks queue processing.
> > + */
> > + blk_mq_unquiesce_queue(disk->queue);
> > + blk_mq_unfreeze_queue(disk->queue);
> > +
> > + return ret;
> > +}
>
> This attach and detach code needs to be elevated out of samples so that
> any future consumer of blk_interposer doesn't reinvent it. It is far
> too fundamental.
>
> The way you've proposed this be merged is very much unacceptable.
>
> Nacked-by: Mike Snitzer <[email protected]>
>
Yes, but on the other hand, while the queue is suspended, the module can perform
some other actions specific to it.
And since the functions blk_mq_freeze_queue(), blk_mq_quiesce_queue(),
blk_mq_unquiesce_queue() and blk_mq_unfreeze_queue() are public,
the module creator can implement its module connection functionality regardless of
whether we make the functions blk_interposer_attach() and blk_interposer_detach()
in the kernel or not.
I'll think about it and try to come up with a better solution.
--
Sergei Shtepa
Veeam Software developer.
On Thu, Dec 10 2020 at 10:54am -0500,
Sergei Shtepa <[email protected]> wrote:
> The 12/09/2020 17:36, Mike Snitzer wrote:
> > On Wed, Dec 09 2020 at 8:01am -0500,
> > Sergei Shtepa <[email protected]> wrote:
> >
> > > This sample demonstrates how to use blk_interposer.
> > > It show how to properly connect the interposer module to kernel,
> > > and perform the simplest monitoring of the number of bio.
> > >
> > > Signed-off-by: Sergei Shtepa <[email protected]>
> > > ---
> > > samples/blk_interposer/Makefile | 2 +
> > > samples/blk_interposer/blk-interposer.c | 276 ++++++++++++++++++++++++
> > > 2 files changed, 278 insertions(+)
> > > create mode 100644 samples/blk_interposer/Makefile
> > > create mode 100644 samples/blk_interposer/blk-interposer.c
> > >
> > > diff --git a/samples/blk_interposer/Makefile b/samples/blk_interposer/Makefile
> > > new file mode 100644
> > > index 000000000000..b11aefde2b1c
> > > --- /dev/null
> > > +++ b/samples/blk_interposer/Makefile
> > > @@ -0,0 +1,2 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +obj-$(CONFIG_SAMPLE_BLK_INTERPOSER) += blk-interposer.o
> > > diff --git a/samples/blk_interposer/blk-interposer.c b/samples/blk_interposer/blk-interposer.c
> > > new file mode 100644
> > > index 000000000000..92b4c1fcf8f7
> > > --- /dev/null
> > > +++ b/samples/blk_interposer/blk-interposer.c
> > > @@ -0,0 +1,276 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +
> > > +/*
> > > + * Block layer interposer allow to interpose bio requests from kernel module.
> > > + * This allows you to monitor requests, modify requests, add new request,
> > > + * or even redirect requests to another devices.
> > > + *
> > > + * This sample demonstrates how to use blk_interposer.
> > > + * It show how to properly connect the interposer module to kernel,
> > > + * and perform the simplest monitoring of the number of bio.
> > > + */
> > > +
> > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > +
> > > +#include <linux/module.h>
> > > +#include <linux/types.h>
> > > +#include <linux/errno.h>
> > > +#include <linux/blkdev.h>
> > > +#include <linux/genhd.h>
> > > +#include <linux/blk-mq.h>
> > > +
> > > +int device_major = 8;
> > > +int device_minor;
> > > +int fmode = FMODE_READ | FMODE_WRITE;
> > > +
> > > +/*
> > > + * Each interposer must have a common part in the form of the blk_interposer structure,
> > > + * as well as its own unique data.
> > > + */
> > > +struct my_interposer {
> > > + /*
> > > + * Common part of block device interposer.
> > > + */
> > > + struct blk_interposer interposer;
> > > + /*
> > > + * Specific part for our interposer data.
> > > + */
> > > + atomic_t counter;
> > > +};
> > > +
> > > +struct my_interposer my_ip;
> > > +
> > > +/**
> > > + * blk_interposer_attach - Attach interposer to disk
> > > + * @disk: target disk
> > > + * @interposer: block device interposer
> > > + */
> > > +static int blk_interposer_attach(struct gendisk *disk, struct blk_interposer *interposer)
> > > +{
> > > + int ret = 0;
> > > +
> > > + /*
> > > + * Stop disks queue processing.
> > > + */
> > > + blk_mq_freeze_queue(disk->queue);
> > > + blk_mq_quiesce_queue(disk->queue);
> > > +
> > > + /*
> > > + * Check if the interposer is already busy.
> > > + * The interposer will only connect if it is not busy.
> > > + */
> > > + if (blk_has_interposer(disk)) {
> > > + pr_info("The interposer is already busy.\n");
> > > + ret = -EBUSY;
> > > + goto out;
> > > + }
> > > +
> > > + /*
> > > + * Attach the interposer.
> > > + */
> > > + disk->interposer = interposer;
> > > + /*
> > > + * And while the queue is stopped, we can do something specific for our module.
> > > + */
> > > + pr_info("Block device interposer attached successfully.\n");
> > > +
> > > +out:
> > > + /*
> > > + * Resume disks queue processing
> > > + */
> > > + blk_mq_unquiesce_queue(disk->queue);
> > > + blk_mq_unfreeze_queue(disk->queue);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +/**
> > > + * blk_interposer_detach - Detach interposer from disk
> > > + * @disk: target disk
> > > + * @interposer: block device interposer
> > > + */
> > > +static int blk_interposer_detach(struct gendisk *disk, struct blk_interposer *interposer)
> > > +{
> > > + int ret = 0;
> > > +
> > > + if (WARN_ON(!disk))
> > > + return -EINVAL;
> > > +
> > > + /*
> > > + * Stop disks queue processing.
> > > + */
> > > + blk_mq_freeze_queue(disk->queue);
> > > + blk_mq_quiesce_queue(disk->queue);
> > > +
> > > + /*
> > > + * Check if the interposer is still available.
> > > + */
> > > + if (!disk->interposer) {
> > > + pr_info("The interposer is not available.\n");
> > > + return -ENOENT;
> > > + goto out;
> > > + }
> > > + /*
> > > + * Check if it is really our interposer.
> > > + */
> > > + if (disk->interposer->ip_submit_bio != interposer->ip_submit_bio) {
> > > + pr_info("The interposer found is not ours.\n");
> > > + return -EPERM;
> > > + goto out;
> > > + }
> > > +
> > > + /*
> > > + * Detach interposer.
> > > + */
> > > + disk->interposer = NULL;
> > > + /*
> > > + * And while the queue is stopped, we can do something specific for our module.
> > > + */
> > > + pr_info("Block device interposer detached successfully.\n");
> > > +
> > > +out:
> > > + /*
> > > + * Resume disks queue processing.
> > > + */
> > > + blk_mq_unquiesce_queue(disk->queue);
> > > + blk_mq_unfreeze_queue(disk->queue);
> > > +
> > > + return ret;
> > > +}
> >
> > This attach and detach code needs to be elevated out of samples so that
> > any future consumer of blk_interposer doesn't reinvent it. It is far
> > too fundamental.
> >
> > The way you've proposed this be merged is very much unacceptable.
> >
> > Nacked-by: Mike Snitzer <[email protected]>
> >
> Yes, but on the other hand, while the queue is suspended, the module can perform
> some other actions specific to it.
>
> And since the functions blk_mq_freeze_queue(), blk_mq_quiesce_queue(),
> blk_mq_unquiesce_queue() and blk_mq_unfreeze_queue() are public,
> the module creator can implement its module connection functionality regardless of
> whether we make the functions blk_interposer_attach() and blk_interposer_detach()
> in the kernel or not.
>
> I'll think about it and try to come up with a better solution.
This practice of sprinkling code in samples/ subdir and getting a
minimalist hook into the kernel (with no actual in-kernel consumer) is
_not_ how we do things.
I'm not well-versed on the samples/ subdir nor when it came into
existence; but it should die in a fire!
Mike