Re: [PATCH v2 4/5] zram: add per-backend caps and validate parameters early

On (26/07/28 17:29), Haoqin Huang wrote:
[..]
> +unsigned int zcomp_get_caps(const char *comp)
> +{
> +	const struct zcomp_ops *backend = lookup_backend_ops(comp);
> +
> +	return backend ? backend->caps : 0;
> +}
> +
> +int zcomp_validate_level(const char *comp, s32 level)
> +{
> +	const struct zcomp_ops *backend = lookup_backend_ops(comp);
> +
> +	if (!backend)
> +		return -EINVAL;
> +	if (!(backend->caps & ZCOMP_CAP_LEVEL))
> +		return -EOPNOTSUPP;
> +	if (level < backend->level_min || level > backend->level_max)
> +		return -EINVAL;
> +	return 0;
> +}

[..]

> +unsigned int zcomp_get_caps(const char *comp);
> +int zcomp_validate_level(const char *comp, s32 level);

[..]

> @@ -1796,6 +1796,25 @@ static ssize_t algorithm_params_store(struct device *dev,
>  			return -EINVAL;
>  	}
>  
> +	if (zram->comp_algs[prio]) {
> +		unsigned int caps = zcomp_get_caps(zram->comp_algs[prio]);
> +
> +		if (dict_path && !(caps & ZCOMP_CAP_DICT)) {
> +			pr_err("zram: %s does not support dictionary\n",
> +			       zram->comp_algs[prio]);
> +			return -EOPNOTSUPP;
> +		}
> +
> +		if (level != ZCOMP_PARAM_NOT_SET) {
> +			ret = zcomp_validate_level(zram->comp_algs[prio], level);
> +			if (ret) {
> +				pr_err("zram: invalid level for %s\n",
> +				       zram->comp_algs[prio]);
> +				return ret;
> +			}
> +		}
> +	}

So I wonder if instead of introducing 2 new zcomp functions (zcomp_get_caps()
and zcomp_validate_level()) and still basically open-coding params verification
in zram, maybe we we can just have one
	int zcomp_validate_params(comp, level, dict_path)
and handle all the validation in zcomp internally.  So that in
algorithm_params_store() it will be just

	ret = zcomp_validate_params(zram->comp_algs[prio], level, dict_path);
	if (ret)
		return ret;