[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201209143606.GA494@redhat.com>
Date: Wed, 9 Dec 2020 09:36:06 -0500
From: Mike Snitzer <snitzer@...hat.com>
To: Sergei Shtepa <sergei.shtepa@...am.com>
Cc: axboe@...nel.dk, johannes.thumshirn@....com, koct9i@...il.com,
ming.lei@...hat.com, hare@...e.de, josef@...icpanda.com,
steve@....org, linux-block@...r.kernel.org,
linux-kernel@...r.kernel.org, pavel.tide@...am.com
Subject: Re: [PATCH 2/3] block: blk_interposer - sample
On Wed, Dec 09 2020 at 8:01am -0500,
Sergei Shtepa <sergei.shtepa@...am.com> 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 <sergei.shtepa@...am.com>
> ---
> 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 <snitzer@...hat.com>
Powered by blists - more mailing lists