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

Re: [Xen-devel] [PATCH v2 9/9] xen: Convert __page_to_mfn and __mfn_to_page to use typesafe MFN



Hi Jan,

On 09/10/17 12:42, Jan Beulich wrote:
On 05.10.17 at 19:42, <julien.grall@xxxxxxxxxx> wrote:
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -50,8 +50,6 @@ struct map_range_data
  /* Override macros from asm/page.h to make them work with mfn_t */
  #undef virt_to_mfn
  #define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
-#undef page_to_mfn
-#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))

With the patch dropping (I assume) all overrides of this kind, what
is the difference between the double-underscore-prefixed versions
of the two constructs you convert here and the plain ones? If
there's none (which I think is what the result here is meant to be),
then ideally the patch would drop the former altogether. In case
this means touching a lot more code, then at least I'd expect you
to convert all instances you touch anyway, and that you in
particular don't introduce any new ones.

But wait - the patch even introduces new overrides (doing the
inverse). What's the deal here? If that's again to limit patch size,
then I'd still prefer the global aliases to go away, and local (per
file) aliases to be retained as needed.

It introduces new overrides because some of the code is not trivial to convert to use mfn_t. It needs more effort to see whether it is worth it to convert them and I think is out of scope of this series.

This series is meant to reduce the number of place we override page_to_mfn to an handful numbers whilst still enforcing the use of mfn_t by default.

But I am not entirely sure what you are suggesting here. Are you saying we could define page_to_mfn/mfn_to_page on every file?

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

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