Re: [PATCH 2/2] ublk: Make ublks_max configurable

[Ming Lei <ming.lei@redhat.com>]

On Sun, Oct 01, 2023 at 01:54:48PM -0500, Mike Christie wrote:
> We are converting tcmu applications to ublk, but have systems with up
> to 1k devices. This patch allows us to configure the ublks_max from
> userspace with the ublks_max modparam.
> 
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> ---
>  drivers/block/ublk_drv.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 18e352f8cd6d..2833a81e05c0 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -2940,7 +2940,36 @@ static void __exit ublk_exit(void)
>  module_init(ublk_init);
>  module_exit(ublk_exit);
>  
> -module_param(ublks_max, int, 0444);
> +static int ublk_set_max_ublks(const char *buf, const struct kernel_param *kp)
> +{
> +	unsigned int max;
> +	int ret;
> +
> +	ret = kstrtouint(buf, 10, &max);
> +	if (ret)
> +		return ret;
> +
> +	if (max > UBLK_MAX_UBLKS) {
> +		pr_warn("Invalid ublks_max. Max supported is %d\n",
> +			UBLK_MAX_UBLKS);
> +		return -EINVAL;
> +	}
> +
> +	ublks_max = max;
> +	return ret;

It might be nice to reuse builtin helper:

	return param_set_uint_minmax(buf, kp, 0, UBLK_MAX_UBLKS);

> +}
> +
> +static int ublk_get_max_ublks(char *buf, const struct kernel_param *kp)
> +{
> +	return sysfs_emit(buf, "%d\n", ublks_max);
> +}
> +
> +static const struct kernel_param_ops ublk_max_ublks_ops = {
> +	.set = ublk_set_max_ublks,
> +	.get = ublk_get_max_ublks,

Same with above, '.get    = param_get_int,' could be better.

Otherwise, this patch looks fine.

Thanks,
Ming