[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [for-4.17] xen/arm: domain_build: Do not use dprintk unconditionally



Hi,

On 16/09/2022 08:19, Michal Orzel wrote:
Using dprintk results in printing additionally file name and line
number. This is something we do not want when printing regular
information unconditionally as it looks like as if there was some issue.
I am OK if you want to switch to a printk() but I disagree with this argument. dprintk() is not about error, it is about anything that doesn't matter in release build.

I don't think we should just switch to printk() because dprintk() add the line/file. There are message we don't necessarily want to have in release build. So dprintk(XENLOG_INFO, ...) would be right for them.

Personally, I find them useful as there no grep required and/or confusion (but that's a matter of taste). If it were me, I would add the line/file everywhere. But I understand this takes space in the binary (hence why this is not present in release build).

A better argument to switch to printk() is this information is useful to the user even outside of the debug build.


Fix this by switching to printk because this information may also be
helpful on the release builds (it would still require setting loglvl to
"info" or lower level).

I think we should drop XENLOG_INFO to be consistent with the other printk() in domain_build.c (after all this is a domain information like the other) or use XENLOG_INFO everywhere.

My preference will be the former because otherwise most of the information will not printed in release build by default.


Fixes: 5597f32f409c ("xen/arm: assign static shared memory to the default owner 
dom_io")

Fixes should only be used for bugs. This is not one.

Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
---
Rationale for taking this patch for 4.17:
Current code results in an abnormal behavior [1] and was introduced by

It is not abnormal (see above). This is an expected behavior when you use dprintk().

the 4.17 feature (static shared memory). Even though it can only be seen
on a debug build, it should be fixed now so that we have a consistent
behavior across all the logs.

As I wrote above, I agree this should be printed in release build. But I disagree with your arguments.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.