Re: [PATCH 01/20] block, blk_filter: enable block device filters

Randy Dunlap
2022-06-13T21:51:12Z

Hi--

On 6/13/22 08:52, Sergei Shtepa wrote:
> Allows to attach block device filters to the block devices.
> Kernel modules can use this functionality to extend the
> capabilities of the block layer.
>
> Signed-off-by: Sergei Shtepa <sergei.shtepa@veeam.com>
> ---
> block/Kconfig | 8 +++
> block/bdev.c | 129 ++++++++++++++++++++++++++++++++++++++
> block/blk-core.c | 88 ++++++++++++++++++++++++++
> include/linux/blk_types.h | 22 +++++++
> include/linux/blkdev.h | 81 ++++++++++++++++++++++++
> 5 files changed, 328 insertions(+)
>

> diff --git a/block/bdev.c b/block/bdev.c
> index 5fe06c1f2def..4bcd9f4378e3 100644
> --- a/block/bdev.c
> +++ b/block/bdev.c
> @@ -426,8 +426,15 @@ static void init_once(void *data)
> inode_init_once(&ei->vfs_inode);
> }
>
> +#ifdef CONFIG_BLK_FILTER
> +static void bdev_filter_cleanup(struct block_device *bdev);
> +#endif
> +
> static void bdev_evict_inode(struct inode *inode)
> {
> +#ifdef CONFIG_BLK_FILTER
> + bdev_filter_cleanup(I_BDEV(inode));
> +#endif
> truncate_inode_pages_final(&inode->i_data);
> invalidate_inode_buffers(inode); /* is it needed here? */
> clear_inode(inode);
> @@ -503,6 +510,11 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno)
> return NULL;
> }
> bdev->bd_disk = disk;
> +
> +#ifdef CONFIG_BLK_FILTER
> + memset(bdev->bd_filters, 0, sizeof(bdev->bd_filters));
> + spin_lock_init(&bdev->bd_filters_lock);
> +#endif
> return bdev;
> }
>
> @@ -1071,3 +1083,120 @@ void sync_bdevs(bool wait)
> spin_unlock(&blockdev_superblock->s_inode_list_lock);
> iput(old_inode);
> }
> +
> +#ifdef CONFIG_BLK_FILTER
> +static void bdev_filter_cleanup(struct block_device *bdev)
> +{
> + int altitude;
> + struct bdev_filter *flt;
> +
> + for (altitude = 0; altitude < bdev_filter_alt_end; altitude++) {
> + spin_lock(&bdev->bd_filters_lock);
> + flt = bdev->bd_filters[altitude];
> + bdev->bd_filters[altitude] = NULL;
> + spin_unlock(&bdev->bd_filters_lock);
> +
> + bdev_filter_put(flt);
> + }
> +}
> +
> +/**
> + * bdev_filter_attach - Attach a filter to the original block device.
> + * @bdev:
> + * Block device.
> + * @name:
> + * Name of the block device filter.
> + * @altitude:
> + * Altituda number of the block device filter.

What is "Altituda"? Just a typo?

> + * @flt:
> + * Pointer to the filter structure.
> + *
> + * Before adding a filter, it is necessary to initialize &struct bdev_filter.
> + *
> + * The bdev_filter_detach() function allows to detach the filter from the block
> + * device.
> + *
> + * Return:
> + * 0 - OK
> + * -EALREADY - a filter with this name already exists
> + */
> +int bdev_filter_attach(struct block_device *bdev, const char *name,
> + const enum bdev_filter_altitudes altitude,
> + struct bdev_filter *flt)
> +{
> + int ret = 0;
> +
> + spin_lock(&bdev->bd_filters_lock);
> + if (bdev->bd_filters[altitude])
> + ret = -EALREADY;
> + else
> + bdev->bd_filters[altitude] = flt;
> + spin_unlock(&bdev->bd_filters_lock);
> +
> + if (!ret)
> + pr_info("block device filter '%s' has been attached to %d:%d",
> + name, MAJOR(bdev->bd_dev), MINOR(bdev->bd_dev));
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(bdev_filter_attach);
> +
> +/**
> + * bdev_filter_detach - Detach a filter from the block device.
> + * @bdev:
> + * Block device.
> + * @name:
> + * Name of the block device filter.
> + * @altitude:
> + * Altituda number of the block device filter.

Ditto.

> + *
> + * The filter should be added using the bdev_filter_attach() function.
> + *
> + * Return:
> + * 0 - OK
> + * -ENOENT - the filter was not found in the linked list
> + */
> +int bdev_filter_detach(struct block_device *bdev, const char *name,
> + const enum bdev_filter_altitudes altitude)
> +{
> + struct bdev_filter *flt = NULL;
> +
> + spin_lock(&bdev->bd_filters_lock);
> + flt = bdev->bd_filters[altitude];
> + bdev->bd_filters[altitude] = NULL;
> + spin_unlock(&bdev->bd_filters_lock);
> +
> + if (!flt)
> + return -ENOENT;
> +
> + bdev_filter_put(flt);
> + pr_info("block device filter '%s' has been detached from %d:%d",
> + name, MAJOR(bdev->bd_dev), MINOR(bdev->bd_dev));
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(bdev_filter_detach);
> +
> +/**
> + * bdev_filter_get_by_altitude - Get filter by altitude.
> + * @bdev:
> + * Pointer to the block device structure.

kernel-doc says:
bdev.c:1190: warning: Function parameter or member 'altitude' not described in '
bdev_filter_get_by_altitude'

> + *
> + * Return:
> + * pointer - pointer to filters structure from &struct blk_filter
> + * NULL - no filter has been set
> + */
> +struct bdev_filter *bdev_filter_get_by_altitude(struct block_device *bdev,
> + const enum bdev_filter_altitudes altitude)
> +{
> + struct bdev_filter *flt;
> +
> + spin_lock(&bdev->bd_filters_lock);
> + flt = bdev->bd_filters[altitude];
> + if (flt)
> + bdev_filter_get(flt);
> + spin_unlock(&bdev->bd_filters_lock);
> +
> + return flt;
> +}
> +EXPORT_SYMBOL_GPL(bdev_filter_get_by_altitude);
> +#endif

> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 608d577734c2..24cb5293897f 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1573,4 +1573,85 @@ struct io_comp_batch {
>
> #define DEFINE_IO_COMP_BATCH(name) struct io_comp_batch name = { }
>
> +#ifdef CONFIG_BLK_FILTER
> +/**
> + * enum bdev_filter_result - The result of bio processing by
> + * the block device filter.
> + *
> + * @bdev_filter_skip:
> + * Original bio does not need to be submitted.
> + * @bdev_filter_pass:
> + * It is necessary to submit the original request.
> + * @bdev_filter_repeat:
> + * Bio processing has not been completed, a second call is required.
> + * @bdev_filter_redirect:
> + * Original bio was redirected to another block device. The set
> + * of filters on it is different, so processing must be repeated.
> + */
> +enum bdev_filter_result {
> + bdev_filter_skip = 0,
> + bdev_filter_pass,
> + bdev_filter_repeat,
> + bdev_filter_redirect
> +};
> +struct bdev_filter;
> +/**
> + * bdev_filter_operations - List of callback functions for the filter.

blkdev.h:1607: warning: cannot understand function prototype: 'struct bdev_filter_operations '

Missing "struct " before the struct name above.

> + *
> + * @submit_bio_cb:
> + * A callback function for bio processing.
> + * @detach_cb:
> + * A callback function to disable the filter when removing a block
> + * device from the system.
> + */
> +struct bdev_filter_operations {
> + enum bdev_filter_result (*submit_bio_cb)(struct bio *bio,
> + struct bdev_filter *flt);
> + void (*detach_cb)(struct kref *kref);
> +};
> +/**
> + * struct bdev_filter - Block device filter.
> + *
> + * @kref:
> + * Kernel reference counter.
> + * @fops:
> + * The pointer to &struct bdev_filter_operations with callback
> + * functions for the filter.
> + */
> +struct bdev_filter {
> + struct kref kref;
> + const struct bdev_filter_operations *fops;
> +};

thanks.
--
~Randy