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

Re: [Xen-devel] [PATCH 2/2] dom_cow is needed for mem-sharing only



Hi Jan,

On 5/31/19 11:03 AM, Jan Beulich wrote:
On 31.05.19 at 11:52, <julien.grall@xxxxxxx> wrote:
On 5/31/19 10:49 AM, Jan Beulich wrote:
On 31.05.19 at 11:42, <julien.grall@xxxxxxx> wrote:
On 5/31/19 10:35 AM, Jan Beulich wrote:
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -644,6 +644,9 @@ static inline void filtered_flush_tlb_ma
/* Private domain structs for DOMID_XEN, DOMID_IO, etc. */
    extern struct domain *dom_xen, *dom_io, *dom_cow;

Don't you want to protect dom_cow with "#ifdef CONFIG_HAS_MEM_SHARING"?

There's no need to with ...

+#ifndef CONFIG_HAS_MEM_SHARING
+# define dom_cow NULL
+#endif

... this, and this way there's less clutter overall.

I am all for avoiding cluttering but not at the expense of making the
code less intuitive. In this case, I would prefer the decleration
dom_cow to be guarded.

While it would be easy enough to do, I fail to see how the chosen
construct is not "intuitive".

Even if I know the pre-preprocessor will do the right thing here, you could spare the developper to trip over this code and wonder why it is first defined and then override with NULL.

In fact I don't think this would be the
first instance of a #define overriding a prior declaration. Doing so
utilizes one of the very basic C preprocessor principles.

You are the first one usually to say that some choices in Xen were not correct and therefore no more instance should be added.

This is one case where the resulting code is counter-intuitive and ugly.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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