[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CADyq12yugY0g2EMYvyrPVn98x3Tp4PR+eVddmmjZpKSYyrrzag@mail.gmail.com>
Date: Mon, 31 Jan 2022 09:17:17 -0500
From: Brian Geffon <bgeffon@...gle.com>
To: Alasdair Kergon <agk@...hat.com>, Mike Snitzer <snitzer@...hat.com>
Cc: dm-devel@...hat.com, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] dm: introduce a DM_ENFORCE_OPEN_COUNT flag.
On Wed, Jan 26, 2022 at 2:22 PM Brian Geffon <bgeffon@...gle.com> wrote:
>
> This change introduces a new flag which can be used with
> DM_DEV_CREATE to establish the maximum open count allowed
> for a device. When this flag is set on DM_DEV_CREATE the
> open_count on dm_ioctl will be intrpreted as an input
> parameter. This value must be >= 1 or DM_DEV_CREATE will
> return -ERANGE.
>
> When this flag is set when the open count is equal to
> the max open count any future opens will result in an
> -EBUSY.
>
Hi Alasdair,
I was curious if you had any thoughts on this particular alternative
approach to this problem, I'm open to any suggestions of alternative
implementations.
Thank you in advance,
Brian
>
> Signed-off-by: Brian Geffon <bgeffon@...gle.com>
> ---
> drivers/md/dm-core.h | 2 ++
> drivers/md/dm-ioctl.c | 13 ++++++++++++
> drivers/md/dm.c | 39 ++++++++++++++++++++++++++++++++---
> drivers/md/dm.h | 7 +++++++
> include/uapi/linux/dm-ioctl.h | 9 +++++++-
> 5 files changed, 66 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
> index 55dccdfbcb22..57922a80026e 100644
> --- a/drivers/md/dm-core.h
> +++ b/drivers/md/dm-core.h
> @@ -57,6 +57,7 @@ struct mapped_device {
>
> atomic_t holders;
> atomic_t open_count;
> + int max_open_count;
>
> struct dm_target *immutable_target;
> struct target_type *immutable_target_type;
> @@ -139,6 +140,7 @@ struct mapped_device {
> #define DMF_SUSPENDED_INTERNALLY 7
> #define DMF_POST_SUSPENDING 8
> #define DMF_EMULATE_ZONE_APPEND 9
> +#define DMF_ENFORCE_OPEN_COUNT 10
>
> void disable_discard(struct mapped_device *md);
> void disable_write_same(struct mapped_device *md);
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index 21fe8652b095..8ddf3ab99ef6 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -814,6 +814,9 @@ static void __dev_status(struct mapped_device *md, struct dm_ioctl *param)
> if (dm_test_deferred_remove_flag(md))
> param->flags |= DM_DEFERRED_REMOVE;
>
> + if (dm_test_enforce_open_count_flag(md))
> + param->flags |= DM_ENFORCE_OPEN_COUNT_FLAG;
> +
> param->dev = huge_encode_dev(disk_devt(disk));
>
> /*
> @@ -866,6 +869,16 @@ static int dev_create(struct file *filp, struct dm_ioctl *param, size_t param_si
> if (r)
> return r;
>
> + if (param->flags & DM_ENFORCE_OPEN_COUNT_FLAG) {
> + if (param->open_count < 1) {
> + dm_put(md);
> + dm_destroy(md);
> + return -ERANGE;
> + }
> +
> + dm_set_max_open_count(md, param->open_count);
> + }
> +
> r = dm_hash_insert(param->name, *param->uuid ? param->uuid : NULL, md);
> if (r) {
> dm_put(md);
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 76d9da49fda7..718bc9fce7c1 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -307,6 +307,7 @@ int dm_deleting_md(struct mapped_device *md)
> static int dm_blk_open(struct block_device *bdev, fmode_t mode)
> {
> struct mapped_device *md;
> + int ret = -ENXIO;
>
> spin_lock(&_minor_lock);
>
> @@ -316,16 +317,28 @@ static int dm_blk_open(struct block_device *bdev, fmode_t mode)
>
> if (test_bit(DMF_FREEING, &md->flags) ||
> dm_deleting_md(md)) {
> - md = NULL;
> goto out;
> }
>
> dm_get(md);
> +
> + if (test_bit(DMF_ENFORCE_OPEN_COUNT, &md->flags)) {
> + /*
> + * No opens or closes can happen in parallel as both
> + * paths hold the _minor_lock.
> + */
> + if (atomic_read(&md->open_count) + 1 > md->max_open_count) {
> + dm_put(md);
> + ret = -EBUSY;
> + goto out;
> + }
> + }
> +
> atomic_inc(&md->open_count);
> + ret = 0;
> out:
> spin_unlock(&_minor_lock);
> -
> - return md ? 0 : -ENXIO;
> + return ret;
> }
>
> static void dm_blk_close(struct gendisk *disk, fmode_t mode)
> @@ -2219,6 +2232,21 @@ void dm_put(struct mapped_device *md)
> }
> EXPORT_SYMBOL_GPL(dm_put);
>
> +/*
> + * dm_set_max_open count can only be called when the device is created,
> + * it cannot be changed once set.
> + */
> +void dm_set_max_open_count(struct mapped_device *md, int count)
> +{
> + /*
> + * The max open count cannot be changed
> + */
> + BUG_ON(test_bit(DMF_ENFORCE_OPEN_COUNT, &md->flags));
> +
> + set_bit(DMF_ENFORCE_OPEN_COUNT, &md->flags);
> + md->max_open_count = count;
> +}
> +
> static bool md_in_flight_bios(struct mapped_device *md)
> {
> int cpu;
> @@ -2795,6 +2823,11 @@ int dm_test_deferred_remove_flag(struct mapped_device *md)
> return test_bit(DMF_DEFERRED_REMOVE, &md->flags);
> }
>
> +int dm_test_enforce_open_count_flag(struct mapped_device *md)
> +{
> + return test_bit(DMF_ENFORCE_OPEN_COUNT, &md->flags);
> +}
> +
> int dm_suspended(struct dm_target *ti)
> {
> return dm_suspended_md(ti->table->md);
> diff --git a/drivers/md/dm.h b/drivers/md/dm.h
> index 742d9c80efe1..82f56a066b83 100644
> --- a/drivers/md/dm.h
> +++ b/drivers/md/dm.h
> @@ -84,6 +84,8 @@ void dm_set_md_type(struct mapped_device *md, enum dm_queue_mode type);
> enum dm_queue_mode dm_get_md_type(struct mapped_device *md);
> struct target_type *dm_get_immutable_target_type(struct mapped_device *md);
>
> +void dm_set_max_open_count(struct mapped_device *md, int count);
> +
> int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t);
>
> /*
> @@ -162,6 +164,11 @@ void dm_internal_resume(struct mapped_device *md);
> */
> int dm_test_deferred_remove_flag(struct mapped_device *md);
>
> +/*
> + * Test if the device is enforcing an open count.
> + */
> +int dm_test_enforce_open_count_flag(struct mapped_device *md);
> +
> /*
> * Try to remove devices marked for deferred removal.
> */
> diff --git a/include/uapi/linux/dm-ioctl.h b/include/uapi/linux/dm-ioctl.h
> index c12ce30b52df..9da3700c0442 100644
> --- a/include/uapi/linux/dm-ioctl.h
> +++ b/include/uapi/linux/dm-ioctl.h
> @@ -123,7 +123,7 @@ struct dm_ioctl {
> * relative to start of this struct */
>
> __u32 target_count; /* in/out */
> - __s32 open_count; /* out */
> + __s32 open_count; /* in/out, in on DM_DEV_CREATE only */
> __u32 flags; /* in/out */
>
> /*
> @@ -382,4 +382,11 @@ enum {
> */
> #define DM_IMA_MEASUREMENT_FLAG (1 << 19) /* In */
>
> +/*
> + * If set with DM_DEV_CREATE then the open_count on device creation
> + * will be set as the maximum concurrent opens allowed on the device.
> + * Once the open_count has been hit any new opens will result in
> + * -EBUSY until other users close the device.
> + */
> +#define DM_ENFORCE_OPEN_COUNT_FLAG (1 << 20) /* In/Out */
> #endif /* _LINUX_DM_IOCTL_H */
> --
> 2.35.0.rc0.227.g00780c9af4-goog
>
Powered by blists - more mailing lists