On Fri, May 22, 2020 at 09:29:35AM +0300, Andra Paraschiv wrote:
> +static char *ne_cpus;
> +module_param(ne_cpus, charp, 0644);
> +MODULE_PARM_DESC(ne_cpus, "<cpu-list> - CPU pool used for Nitro Enclaves");
This is not the 1990's, don't use module parameters if you can help it.
Why is this needed, and where is it documented?
> +/* CPU pool used for Nitro Enclaves. */
> +struct ne_cpu_pool {
> + /* Available CPUs in the pool. */
> + cpumask_var_t avail;
> + struct mutex mutex;
> +};
> +
> +static struct ne_cpu_pool ne_cpu_pool;
> +
> +static int ne_open(struct inode *node, struct file *file)
> +{
> + return 0;
> +}
If open does nothing, just don't even provide it.
> +
> +static long ne_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + switch (cmd) {
> +
> + default:
> + return -ENOTTY;
> + }
> +
> + return 0;
> +}
Same for ioctl.
> +
> +static int ne_release(struct inode *inode, struct file *file)
> +{
> + return 0;
> +}
Same for release.
> +
> +static const struct file_operations ne_fops = {
> + .owner = THIS_MODULE,
> + .llseek = noop_llseek,
> + .unlocked_ioctl = ne_ioctl,
> + .open = ne_open,
> + .release = ne_release,
> +};
> +
> +struct miscdevice ne_miscdevice = {
> + .minor = MISC_DYNAMIC_MINOR,
> + .name = NE_DEV_NAME,
> + .fops = &ne_fops,
> + .mode = 0660,
> +};
> +
> +static int __init ne_init(void)
> +{
> + unsigned int cpu = 0;
> + unsigned int cpu_sibling = 0;
> + int rc = -EINVAL;
> +
> + memset(&ne_cpu_pool, 0, sizeof(ne_cpu_pool));
Why did you just set a structure to 0 that was already initialized by
the system to 0? Are you sure about this?
> +
> + if (!zalloc_cpumask_var(&ne_cpu_pool.avail, GFP_KERNEL))
> + return -ENOMEM;
> +
> + mutex_init(&ne_cpu_pool.mutex);
> +
> + rc = cpulist_parse(ne_cpus, ne_cpu_pool.avail);
> + if (rc < 0) {
> + pr_err_ratelimited(NE "Error in cpulist parse [rc=%d]\n", rc);
Again, drop all ratelimited stuff please.
thanks,
greg k-h
