linux-kernel - Re: [PATCH 1/4] drivers: hv: dxgkrnl: core code

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]

Message-ID: <20200814125528.GA56456@kroah.com>
Date:   Fri, 14 Aug 2020 14:55:28 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Sasha Levin <sashal@...nel.org>
Cc:     kys@...rosoft.com, haiyangz@...rosoft.com, sthemmin@...rosoft.com,
        wei.liu@...nel.org, iourit@...rosoft.com,
        linux-kernel@...r.kernel.org, linux-hyperv@...r.kernel.org
Subject: Re: [PATCH 1/4] drivers: hv: dxgkrnl: core code

On Fri, Aug 14, 2020 at 08:38:53AM -0400, Sasha Levin wrote:
> Add support for a Hyper-V based vGPU implementation that exposes the
> DirectX API to Linux userspace.
> 
> Signed-off-by: Sasha Levin <sashal@...nel.org>

Better, but what is this mess:

> +#define ISERROR(ret)					(ret < 0)

?

> +#define EINTERNALERROR					EINVAL

And that?

> +
> +#define DXGKRNL_ASSERT(exp)
> +#define UNUSED(x) (void)(x)

Ick, no, please.

> +#undef pr_fmt

In a .h file?

> +#define pr_fmt(fmt)	"dxgk:err: " fmt
> +#define pr_fmt1(fmt)	"dxgk: " fmt
> +#define pr_fmt2(fmt)	"dxgk:    " fmt

Why?

> +
> +#define DXGKDEBUG 1
> +/* #define USEPRINTK 1 */
> +
> +#ifndef DXGKDEBUG
> +#define TRACE_DEBUG(...)
> +#define TRACE_DEFINE(...)
> +#define TRACE_FUNC_ENTER(...)
> +#define TRACE_FUNC_EXIT(...)

No, please do not to custom "trace" printk messages, that is what ftrace
is for, no individual driver should ever need to do that.

Just use the normal dev_*() calls for error messages and the like, do
not try to come up with a custom tracing framework for one tiny
individual driver.  If every driver in kernel did that, we would have a
nightmare...

thanks,

greg k-h