Re: [Xen-devel] [PATCH v4 11/31] x86/mm: split out writable pagetable emulation code

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.08.17 at 16:44, <wei.liu2@xxxxxxxxxx> wrote:
> Move the code to pv/emul-ptwr-op.c. Fix coding style issues while
> moving the code.
> 
> Rename ptwr_emulated_read to pv_emul_ptwr_read and export it via
> pv/mm.h because it is needed by other emulation code.

If other emulated code uses it, renaming the function would better
imply dropping the ptwr infix from it. pv_emulated_read() perhaps?

> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  xen/arch/x86/mm.c              | 308 +-------------------------------------
>  xen/arch/x86/pv/Makefile       |   1 +
>  xen/arch/x86/pv/emul-ptwr-op.c | 327 
> +++++++++++++++++++++++++++++++++++++++++

Would you mind calling this just ptwr.c?

> +/*************************
> + * Writable Pagetables
> + */
> +
> +struct ptwr_emulate_ctxt {
> +    struct x86_emulate_ctxt ctxt;
> +    unsigned long cr2;
> +    l1_pgentry_t  pte;
> +};
>[...]
> +static int ptwr_emulated_update(unsigned long addr, paddr_t old, paddr_t val,
> +                                unsigned int bytes, unsigned int do_cmpxchg,
> +                                struct ptwr_emulate_ctxt *ptwr_ctxt)

I've meanwhile noticed that in prior patches of yours such movement
was needlessly retaining the component prefixes. With you splitting
things into separate files, these aren't really useful anymore - stack
traces will have them disambiguated by being prefixed with their
file names. They merely eat valuable serial line bandwidth / ring
buffer space and clutter the (serial) log. I could accept the structure
tags to stay the way they are, but please shorten the local function
names as much as possible without losing information. That'll likely
mean dropping more than just the ptwr_ prefix.

Jan


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