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

Greg KH
2020-08-14T12:55:09Z

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