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