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

[Greg KH <gregkh@linuxfoundation.org>]

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@kernel.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