linux-kernel - Re: [PATCH v2] sched: Provide USF for the portable equipment.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 31 Jul 2020 15:22:07 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Dongdong Yang <contribute.kernel@...il.com>
Cc:     rjw@...ysocki.net, viresh.kumar@...aro.org, mingo@...hat.com,
        peterz@...radead.org, juri.lelli@...hat.com,
        vincent.guittot@...aro.org, dietmar.eggemann@....com,
        rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
        linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org,
        linux-pm@...r.kernel.org, yangdongdong@...omi.com,
        tanggeliang@...omi.com, taojun@...omi.com, huangqiwu@...omi.com,
        rocking@...ux.alibaba.com, fengwei@...omi.com,
        zhangguoquan@...omi.com, gulinghua@...omi.com, duhui@...omi.com
Subject: Re: [PATCH v2] sched: Provide USF for the portable equipment.

On Fri, Jul 31, 2020 at 08:46:30PM +0800, Dongdong Yang wrote:
> --- /dev/null
> +++ b/drivers/staging/fbsched/usf.c
> @@ -0,0 +1,346 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 XiaoMi Inc.
> + * Author: Yang Dongdong <yangdongdong@...omi.com>
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> + * See http://www.gnu.org/licenses/gpl-2.0.html for more details.

Please remove the license "boilerplate" text as you have the SPDX line
on top.

> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/kthread.h>
> +#include <linux/cpu.h>
> +#include <linux/sysfs.h>
> +#include <linux/kthread.h>
> +#include <linux/kobject.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/kallsyms.h>
> +#include <linux/debugfs.h>
> +#include <linux/fb.h>
> +#include <linux/notifier.h>
> +
> +#define BOOST_MIN_V -100
> +#define BOOST_MAX_V 100
> +#define LEVEL_TOP 3
> +
> +#define USF_TAG	"[usf_sched]"
> +
> +DEFINE_PER_CPU(unsigned long[PID_MAX_LIMIT], task_hist_nivcsw);
> +
> +static struct {
> +	bool is_sched_usf_enabled;
> +	int enable_debug;
> +	int is_screen_on;
> +	struct kobject *kobj;

A raw kobject?  For a driver?  are you _SURE_???

> +	struct dentry *debugfs_entry;

Why do you need this?

> +	usf_vdev.enable_debug = 0;
> +	usf_vdev.debugfs_entry = debugfs_create_file("usf_dbg",
> +						     0660, NULL, NULL,
> +						     &usf_dbg_fops);
> +	if (!usf_vdev.debugfs_entry)
> +		pr_err("Failed to create usf_dbg!\n");

How can that value be NULL?

There is no need to check debugfs functions for error values.

But in this case, why are you writing a single file to the root of
debugfs?  Why not put it in a directory instead?  That would be much
nicer, don't you think?

thanks,

greg k-h